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

187 lines
20 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 的绑定方式、错误文案、响应 envelope 和鉴权上下文差异明显;抽统一门面会隐藏 transport 语义并扩大回归面,暂不改动。~~ | ~~server/handler/*~~ | ~~评估完成保留2026-08-28~~ |
| 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/通知/克隆机制经复核不合并:前者负责文件快照与 fsnotify 全量替换,后者负责数据库集成配置按 provider key 通知;已补决策注释固化边界。~~ | ~~config/runtime.gointegration/runtimeconfig/store.go~~ | ~~评估完成保留分离2026-08-28~~ |
| ~~D-10~~ | ~~经复核保留三处清理:`persistConfigValues` 覆盖完整运行时保存,`persistDatabaseConfig` 覆盖初始化页的局部写入,`removeIntegrationConfigFromFile` 覆盖启动时对旧模板的兼容清理;入口不同且各自可独立触发,合并会削弱不落盘不变式。~~ | ~~internal/data/config_store.godata.go~~ | ~~评估完成保留2026-08-28~~ |
### 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~~ | ~~经复核保留payment 大文件承载跨供应商编排与契约微文件分别对应独立边界dto 合并会重新混装领域,收益不足。~~ | ~~biz/payment、service/dto~~ | ~~评估完成保留2026-08-28~~ |
| ~~S-6~~ | ~~经复核保留:`system.go`/`settings.go` 虽跨域,但移动类型会放大 service DTO 导入与生成契约变化;本批不做机械拆分。~~ | ~~service/dto/system.go、settings.go~~ | ~~评估完成保留2026-08-28~~ |
| ~~S-7~~ | ~~经复核保留data 层转换与 PO 命名差异来自不同存储关系和历史兼容,统一命名需全域迁移,当前无安全局部收益。~~ | ~~data/system/*~~ | ~~评估完成保留2026-08-28~~ |
| ~~S-8~~ | ~~经复核保留media 四文件分别覆盖资源、元数据、上传会话与上传流程,职责边界清晰,不为减少文件合并。~~ | ~~biz/system/media*~~ | ~~评估完成保留2026-08-28~~ |
| ~~S-9~~ | ~~经复核保留:单方法 handler 结构体由 Wire/路由注入约束形成,合并会改变构造与注册契约,暂不改动。~~ | ~~server/handler/session.go、navigation.go、set.go~~ | ~~评估完成保留2026-08-28~~ |
## 六、包归属问题
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| 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 过滤字段已分别迁移至独立 `APIFilter`、`ExportTemplateFilter`,实体 DO 不再承载列表过滤/排序字段。~~ | ~~biz/service/data system parameter、api、export~~ | ~~已完成2026-08-28针对性测试通过~~ |
| L-5 | middleware 硬编码业务语义:中文消息黑名单判断审计(改文案即改审计行为);业务路径硬编码;支付回调专用逻辑内嵌通用中间件;限流策略内联 | server/middleware/* | 二轮 |
| ~~L-6部分完成~~ | ~~`AuthenticationResult` 已在返回 service 前清空密码哈希并补回归测试。~~ `QueryExport` 动态行、export DO 的兼容 SQL 字段、`UserOptions` 选项形状仍保留:前两项涉及公开导入导出契约与存量数据兼容,后者虽命名偏 UI但实际是稳定的 label/value 投影;当前直接迁移收益不足以覆盖契约风险。 | biz/system/* | ~~部分完成2026-08-28针对性测试通过~~ |
| 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部分完成~~ | ~~`AuthorityResponse.DeletedAt` 已改为 `json:"-"`,不再泄漏软删字段。~~ ID 类型分叉涉及现有 handler/usecase/数据库键类型的兼容迁移;`ErrorRecordMutationRequest` 的指针字段用于区分省略与显式空值且已有测试原建议不成立保留DTO 对 integration biz 字段定义的反向依赖真实存在,但迁移会改变配置元数据契约,留待独立处理。 | service/dto/* | ~~部分完成2026-08-28针对性测试通过~~ |
| 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~~ | ~~已完成影响分析,暂不实施单一声明源迁移:`routecatalog` 承载审计/Swagger/模块同步元数据,`router` 负责 Gin handler 绑定;当前 catalog 还无法表达 handler 注入与注册顺序,强行合并会扩大启动与路由回归面。~~ | ~~server/router/*routecatalog/catalog.go~~ | ~~评估完成保留分离2026-08-28~~ |
| X-2 | 新增资源触碰 7 处 | — | 一轮 |
| ~~X-3~~ | ~~已完成影响分析暂不合并三种配置形状storage/email 需要强类型 `config.Store` 快照与文件兼容mq/websocket 需要按 provider 的 `runtimeconfig.Store` 热通知;统一形状会牺牲强类型校验或通知粒度。~~ | ~~data/integration_config.go 等~~ | ~~评估完成保留分离2026-08-28~~ |
| 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~~ | ~~已完成影响分析,现状并非“只换快照”:`watchLoop` 负责发布合并快照,`Data.reloadConfig` 显式重建数据库、Redis、Mongo、storage 与 integration runtime两者分工避免文件 watcher 直接持有基础设施生命周期。合并为单通道需重做锁、回滚与连接退休策略,暂不改动。~~ | ~~config/runtime.go:263-287data/config_store.go:124-252~~ | ~~评估完成保留分离2026-08-28~~ |
| 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 路由单源化等大动作**(最后)