kra-new/docs/code-review-issues.md

187 lines
18 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# 代码审查问题清单internal + pkg
- 审查日期2026-08-27 ~ 2026-08-28共四轮全量审查。第四轮2026-08-28核查第三至六轮处置声称是否真实落地 + 全量回归审查(重点:重构引入的新问题),`go build ./...` 编译验证通过
- 文档结构:只保留待修复问题,按**类型**归类(不按轮次);每条标注发现轮次;已修复并经复查确认的直接删除
- gva/ 目录是遗留参考库(独立 module 不参与 kra 编译),不在审查范围
- 依赖方向合规确认pkg 无 import internalintegration 不 import data/servicedata 不再 import integration/payment第四轮重构后复验单向无循环依赖
---
## 一、正确性缺陷(第四轮回归审查新发现,最高优先级)
| # | 问题 | 位置 | 说明 |
|---|------|------|------|
| ~~W-1~~ | ~~payment 空注册表导致应用启动失败。~~ **已修复:模板允许无业务模块启动,调用未配置能力时返回明确错误。** | ~~原位置~~ | ~~已完成~~ |
| ~~W-2~~ | ~~CompleteUpload 吞掉 ListChunks 底层错误。~~ **已修复:底层错误与分片数量不足已拆分处理。** | ~~internal/biz/system/media_upload.go~~ | ~~已完成~~ |
| ~~W-3~~ | ~~MergeRuntimeConfig 嵌套子节合并缺口。~~ **已修复并增加回归测试。** | ~~internal/config/clone.goruntime_test.go~~ | ~~已完成~~ |
| ~~W-4~~ | ~~退款状态词表语义边角。~~ **已修复并补充测试。** | ~~internal/paymentkit/status.go~~ | ~~已完成~~ |
| ~~W-5~~ | ~~公告/参数/版本列表无 ORDER BY。~~ **已修复:统一按 `id desc` 排序。** | ~~data/system/*.go~~ | ~~已完成~~ |
| ~~W-6~~ | ~~外部退款分支存在不可达死分支。~~ **已修复。** | ~~internal/biz/payment/payment.go~~ | ~~已完成~~ |
## 二、死代码与零消费者机制
### 2.1 三层死方法残留(第三至五轮处置后仍未删,均经全仓 Grep 反查确认零调用)【二轮发现,四轮复核仍在】
| 层 | 死代码 | 位置 |
|----|--------|------|
| ~~biz 接口+data 实现~~ | ~~`RecordDataAccess` 写入链已删除;查询侧仍保留。~~ | ~~相关文件~~ |
| ~~service 包装~~ | ~~security_session.go 中 8 个无消费者透传方法已删除。~~ | ~~相关文件~~ |
| biz 注入面 | `RegisterBusinessModule`W-1 的成因之一,两阶段注册+回滚补偿零调用);`PaymentBusinessModule`/`PayInternal`/`RefundInternal`/`AuthorizeRefund` 接口面仍无生产实现biz 调用链真实存在,仅实现者缺——与 W-1 一并处理) | biz/payment/payment.go:400-412payment_order.go:139-152 |
| ~~dto 死字段(已确认项)~~ | ~~GetAuthorityButtonsRequest.Selected 已删除;其余字段因仍参与响应或兼容契约暂保留。~~ | ~~dto/permission.go~~ |
| 死分支 | export_excel.go 的 `case []byte` 在 data 层按列类型转换R-2 修复)后成为死分支 | service/system/export_excel.go:85-86 |
### 2.2 mq / websocket 零消费者基础设施【既定排除范围,历轮明确不处理,现状保持】
- mq 全链(声明式订阅+legacy API+簿记/dispatcher/reconcile业务消费者为零`_ mq.Client` 幻影参数cmd/main.go:65integration/provider.go:33-38 三重死绑定 + Hub 死绑定
- websocketHub 接口零消费者integration 层 On* 四注册方法零调用,双层 handler 登记机制两层都为空
- namedClient/Client() 整型死代码integration/mq/emqx.go:632-650
### 2.3 零散死代码(第四轮新扫描)【四轮】
| 死代码 | 位置 | 证据 |
|--------|------|------|
| `config.CloneData` | internal/config/clone.go:8 | 全仓零调用(其余 Clone* 均有生产调用) |
| `paymentkit.XMLValues`/`XMLEncode` | internal/paymentkit/xml.go:16,41 | 仅测试调用,生产 XML 走 gopay 库 |
| `paymentkit.NestedString` | internal/paymentkit/json.go:18 | 生产+测试均零调用(旧 shim 删除后的孤儿) |
| `logging.NewZapLogger` | internal/logging/zap.go:542 | 仅测试调用,生产用 NewReloadableZapLogger |
| ~~`data/payment.contains`~~ | ~~已改用 `paymentkit.ContainsFold`,本地实现已删除。~~ | ~~data/payment/payment.go~~ |
| `paymentkit status.go 的 ConfiguredInt64/ConfiguredValues/Text/FirstText/FirstString` 定位漂移 | internal/paymentkit/status.go:36-90 | 属"供应商配置解析"超出 README 声称范围(文档漂移,非死代码) |
## 三、重复实现 / 双份维护
### 3.1 大块可消除
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| ~~D-2部分完成~~ | ~~支付方式归一化骨架已统一到 `paymentkit.NormalizePaymentMethod`;各渠道状态词表与退款身份校验因语义不同保留。~~ | ~~integration/paymentinternal/paymentkit~~ | ~~部分完成~~ |
| D-4 | handler 四段式样板约 70 处ShouldBindJSON→Fail→service→Write | server/handler/* | 二轮 |
| D-6 | mq/websocket 两包各写一套 map 解码 helper 且逐字符相同TestConfig 探测骨架三处同构【属 2.2 排除范围交叉项】 | emqx.go:226-260 vs websocket/server.go:189-240 | 二轮 |
### 3.2 配置/数据不变式双份维护
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| D-9 | config.Store 与 runtimeconfig.Store 各写一套同构 listener/通知/克隆机制【既定不采用原建议,保持分离——待写决策注释固化】 | config/runtime.go:29,150-196runtimeconfig/store.go:62-167 | 一轮 |
| D-10 | "storage/email 不落盘"不变式三重执行persistConfigValues 置 nil+Delete、persistDatabaseConfig 再 Delete、removeIntegrationConfigFromFile 启动时又删) | config_store.go:45-47,102,106-122data.go:279-283 | 一轮 |
### 3.3 中小重复
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| D-11 | authority 树构建算法两份【既定不采用,保留】 | biz authority.go:48-78 vs menu.go:80-99 | 一轮 |
| ~~D-15~~ | ~~defaults 合并逻辑三层三份。~~ **已修复:统一使用 `integrationbiz.MergeIntegrationDefaults`。** | ~~相关文件~~ | ~~已完成~~ |
| ~~D-17 剩余~~ | ~~`values()` 与 `testRow()` 已收敛为共享读取逻辑,并保留启用状态差异。~~ | ~~data/payment/payment.go~~ | ~~已完成~~ |
| ~~D-19评估后保留~~ | ~~payment 金额守恒校验四处重复。~~ **经复核保留四处输入字段与供应商容错语义不同强行合并会破坏分层biz 最终守恒校验作为跨边界不变式。** | ~~biz/payment 与各供应商适配器~~ | ~~不改动~~ |
| ~~D-21~~ | ~~支付订单响应映射已统一复用 `paymentOrderResponse`。~~ | ~~service/payment/payment.go~~ | ~~已完成~~ |
| ~~D-22~~ | ~~同 2.3,已改用 `paymentkit.ContainsFold`。~~ | ~~data/payment/payment.go~~ | ~~已完成~~ |
## 四、过度分层:转发门面 / 透传壳 / 回调穿透
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| F-2 | integration/payment/result.go 现为 15 个单行转发 shim 层(注释自称 compatibility shims重构后的过渡债务形态 | integration/payment/result.go:12-84 | 一轮(四轮形态更新) |
| F-4 | handler/http.go 便捷门面【既定暂留决策】 | server/handler/http.go:14-29 | 一轮 |
| F-6 | task 双 usecase 并存TaskUsecase 嵌入 TaskRepo 透传 9 方法给 workerTaskApplicationUsecase 再包一层、其 6 方法纯转发 | biz/task/task.go:76-79,159-237 | 一/三轮 |
| F-7 | Backend 三层缝合biz InitializationRepo → initialize.Repo → data.Data【既定不采用保留编排】 | initialize/initialize.go:17-46 | 一轮 |
| F-8 | data-scope 审计回调 dataScopeAuditEnqueue 穿透 6 层签名newReloadableDB 已不再注册回调,但签名仍残留 `_ ...dataScopeAuditEnqueue` 匿名变参垫片——清理垫片即闭环) | data/runtime_clients.go:168data_scope.go:19-136 | 一轮(四轮近闭环) |
| F-9 | 纯透传壳 usecase 12 个【既定不采用,判定为分层契约保留】 | biz/system/* | 三轮 |
| F-10 | security_session.go 14 方法全透传8 个已确认死,见 2.1——按死代码处理而非合并) | service/system/security_session.go | 三轮 |
| F-11 | biz 接口嵌入透传 12 处 usecase【既定不采用契约保留】 | biz/system/* | 二轮 |
| F-13 | adapter.go 双入口与别名残留:`type Adapter = bizpayment.PaymentAdapter` 别名;包级 `New()``Factory.New` 并存Factory 仅是为满足 wire 的壳) | integration/payment/adapter.go:11,44,53-55 | 四轮 |
## 五、过分拆分 / 文件组织
**根因模式三条**【三轮】:①零逻辑 usecase 壳wire 强制每域一个构造器放大);②"每资源 N 文件"机械切分;③为 import 美观引入的中间缝合包/门面。
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| ~~S-1第一批~~ | ~~已合并 actor/data-scope 上下文载荷文件;其余微文件仍待按职责归并。~~ | ~~internal/biz/system/context.go~~ | ~~部分完成~~ |
| ~~S-2~~ | ~~SystemConfigService 已合并为单文件。~~ | ~~internal/service/system/system.go~~ | ~~已完成~~ |
| ~~S-3第一批~~ | ~~routes.go 已改为有序注册表;各领域路由文件仍保留。~~ | ~~internal/server/router/routes.go~~ | ~~部分完成~~ |
| ~~S-4~~ | ~~四个 data 子包的 provider 文件已合并。~~ | ~~internal/data/*/provider.go~~ | ~~已完成~~ |
| S-5 | 巨微两极biz/payment/payment.go 1150 行 vs 同域微文件dto 超小文件 vs settings.go 170 行跨四域 | biz/payment、service/dto | 三轮 |
| S-6 | dto 包组织混乱system.go 混装三域settings.go 横跨四域 | service/dto/system.go、settings.go | 一轮 |
| S-7 | data 层组织纪律转换函数命名四种风格PO 分布无规则audit.go 名不副实runtime.go 拼盘 | data/system/* | 二轮 |
| S-8 | media 域同域四文件 | biz/system/media* | 三轮 |
| S-9 | 单方法 handler 各占结构体+Set 23 字段 | server/handler/session.go、navigation.go、set.go | 一轮 |
## 六、包归属问题
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| P-3 | pkg/module 内移建议【既定不采用,维持现状】 | pkg/module | 三轮 |
| P-5 | pkg/mq+websocket(Hub) 收缩【属 2.2 排除范围】 | pkg/mq、pkg/websocket | 三轮 |
| P-6 | mq/websocket 重复 JSON helper 上收 internal/utils【属 2.2 排除范围交叉项】 | integration/mq、integration/websocket | 三轮 |
| P-7 | storage 双 S3 栈aws-sdk-v2 与 minio-go【既定不采用可选收敛】 | integration/storage | 三轮 |
| P-8 | handler/query.go、middleware/request.go 纯函数、parseTime 上收 pkg/utils 候选 | server/handler/query.go 等 | 三轮 |
| P-9 | initialize/configuration.go 三种职责混杂management* DTO 塑形/JSON 规范化/掩码) | initialize/configuration.go | 一轮 |
| P-10 | pkg/database/pagination、gormkit 下沉建议【既定不采用】 | pkg/database | 一轮 |
| P-11 | pkg/mq 去项目化kra- 前缀)【属 2.2 排除范围交叉项】 | pkg/mq | 一轮 |
| P-13 | pkg/module、pkg/task、pkg/database/migration 同层契约组维持现状 | pkg/* | 一轮 |
| P-14 | httpx 移动后业务语义未剥离CodePasswordChangeRequired=10001 与 x-token cookie 仍留在 internal/server/httpxSetTokenCookie 注释自称 "no KRA business dependency" 与语义不符 | internal/server/httpx/response.go:17,58-64 | 四轮P-1 移动残留) |
| ~~P-15~~ | ~~包移动注释与 logging 栈跳过标记已修正。~~ | ~~相关文件~~ | ~~已完成~~ |
| P-16 | CLAUDE.md 结构描述整体过时(描述 api/、internal/global/ 等不存在目录),与 AGENTS.md 不同步 | CLAUDE.md:9-17 | 四轮 |
## 七、分层 / 职责违规
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| ~~L-3部分完成~~ | ~~PaymentResult/PaymentTestResult 的死 JSON 标签已移除PaymentRequest 字段与指纹语义保留Definition 家族因仍被 service 消费暂不迁移。~~ | ~~biz/payment/payment.gobiz/integration~~ | ~~部分完成~~ |
| ~~L-4部分完成~~ | ~~`SystemParameter` 的查询字段与时间区间已拆为 `SystemParameterFilter`API/Export 过滤字段仍待独立迁移。~~ | ~~biz/service/data system parameter~~ | ~~部分完成~~ |
| L-5 | middleware 硬编码业务语义:中文消息黑名单判断审计(改文案即改审计行为);业务路径硬编码;支付回调专用逻辑内嵌通用中间件;限流策略内联 | server/middleware/* | 二轮 |
| L-6 | biz 契约泄漏存储/表现原语QueryExport 返回 []map[string]anyexport DO 携带 SQL 片段UserOptions UI 形状AuthenticationResult 携带密码哈希 | biz/system/* | 二轮 |
| L-7 | data 层纪律Table("字符串") 绕过 POsaveRelations 回写入参 DOOriginSetting 裸转换 | data/system/* | 二轮 |
| L-8 | 编排类文件过重seedSystem 126 行 10 类职责authority.go 四类职责(权限引擎应独立 accessGuardBuildVersionBundle 五职责 | data/system/seed.go、authority.go、version.go | 二轮 |
| L-9 | dto 契约问题ID 类型三处分叉AuthorityResponse.DeletedAt 泄漏ErrorRecordMutationRequest 指针/值不自洽DTO 反向依赖 biz 类型 | service/dto/* | 二轮 |
| L-10 | 错误体系双轨errors.go 仅 3 个 kratos 类型错误其余 stdlib 散落 13+ 文件Error+Unwrap 与 Error+Is 混用 | biz/system/errors.go 等 | 二轮 |
| L-12 | 校验双轨制handler 手工 if 与 dto binding 标签混用 | server/handler/* + service/dto/* | 一/三轮 |
## 八、简单实现复杂化
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| C-1 | payment Create 过度防御9+1 字段回比+指纹层两道已做) | biz/payment/payment.go:385-402 | 二轮 |
| C-2 | loadUser/loadUsers 双实现 | data/system/user.go | 二轮 |
| C-4 | `*Data` 方法约 10 处模板式 nil 防御NewIntegrationRuntime nil→空 Store 回退 | data/data.go | 一/二轮 |
| C-6 | 媒体上传三重大小防御 | server/handler/media.go | 二轮 |
| C-7 | websocket 双层 handler 登记【属 2.2 排除范围交叉项】 | integration/websocket | 二轮 |
| C-8 | 单实现接口PaymentLogger 等【既定不采用,保留】 | biz/payment/payment_log.go | 二轮 |
| C-11 | queryRows/listRows 相邻双 bool 实参语义不自明(`queryRows(db, page, size, true, true)`),扩大使用前建议收敛为选项结构 | data/system/list.go:13 | 四轮 |
| C-12 | DailyWriter.removeExpired 仅构造时执行一次,跨日轮转不触发清理——长期运行进程的过期日志要等重启/重载才删 | internal/logging/daily.go:24,52-66 | 四轮 |
## 九、结构性设计(大动作需决策)
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| X-1 | router 与 routecatalog 双声明(表驱动合并为单一声明源) | server/router/*routecatalog/catalog.go | 一轮 |
| X-2 | 新增资源触碰 7 处 | — | 一轮 |
| X-3 | 集成配置三形状两通道storage/email 走 config.Storemq/websocket 走 runtimeconfig | data/integration_config.go 等 | 一轮 |
| X-4 | swagger 运行时文档手拼仍在 server 根包 | server/swagger.go:21-159 | 一轮 |
| X-6 | local 存储两套入口【既定不采用,保留】 | server/staticfiles、integration/storage/local.go | 一轮 |
| X-7 | gopay_helpers.go 杂物间(渠道专属谓词/状态机应下沉各渠道文件) | integration/payment/gopay_helpers.go | 二轮 |
| X-8 | vendor.go 14 键金额 DSL 投机通用性18 字段全 Required【既定不采用保留】 | integration/payment/vendor.go | 二轮 |
| X-9 | config 热重载双通道watchLoop 只换快照不重建客户端,全量重载需手动触发 | config/runtime.go:263-287 | 三轮 |
| X-10 | payment 新增渠道需改 4 处散弹式修改 | biz/payment 常量+adapter 工厂+配置定义 | 三轮 |
| X-11 | TaskScheduler 多锁【真实并发需求,仅记录不改】 | worker/task_scheduler.go | 二轮 |
## 审查后认为合理、不建议改动的部分
- **modules 与 routecatalog 分离**:启动期装配 vs 请求期热路径,消费方零重叠
- **Provider 接口缝模式** + provider.Database 中性接口正当system 包内 Provider 与 DatabaseProvider 两个近义缝命名易混淆,建议注释互指)
- **config.Store 与 runtimeconfig.Store 分离**正确D-9 词表同构为已知保留项)
- **utils/routepath、uploadpolicy**:纪律良好
- **worker→biz 正向+接口倒置**:任务链路最规范的一段
- **data 根多文件同包**:符合 data/README 约定
- **apple_jws.go 证书链校验**:必要安全设计(经 gopay 源码核对补真实漏洞);单一根指纹需运维轮换预案注释
- **capture/auth 中间件质量**:有据可依
- **新增缝质量(第四轮验证)**PaymentAdapterFactorybiz 接口+integration 实现+wire 绑定单向无环、MergeRuntimeConfig单点三调用、deletePrefixViaList函数式注入失败关闭正确、AST 缓存size+mtime 失效、systeminfointegration 定位正确)——均内聚、依赖最小、无越界 import
## 处置建议(按优先级)
1. **先修 W-1 payment 启动阻塞**(应用当前无法启动,最高优先级)+ W-2/W-5用户可感知的正确性缺陷
2. **清 2.1 死代码残留 + 2.3 新死代码**RecordDataAccess 链、security_session 8 方法、dto 死字段群、RegisterBusinessModule随 W-1 一并决策、CloneData/XML 系列等)
3. **补 W-3 嵌套合并回归 + W-4 词表修正 + P-15 移动残留清理**(小而具体)
4. **继续 S 系列文件级减法**S-1/S-3 仍有剩余微文件与路由文件可按职责整理)
5. **D-2/D-19 payment 剩余重复 + L 系列归位**(独立批次)
6. **X-1 路由单源化等大动作**(最后)