Files
kra-oa/docs/code-review-issues.md
T
2026-08-28 09:19:54 +08:00

188 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 internal;integration 不 import data/service;data 不再 import integration/payment(第四轮重构后复验单向);无循环依赖
---
## 一、正确性缺陷(第四轮回归审查新发现,最高优先级)
| # | 问题 | 位置 | 说明 |
|---|------|------|------|
| W-1 | **payment 空注册表导致应用必然启动失败(高危,阻塞)**:wire 装配链创建空注册表(`NewPaymentOrderSourceRegistry()`/`NewPaymentFulfillmentRegistry()` 返回空 map)后仅流向 `NewConfiguredPaymentUsecase` 的 fail-fast 检查;`RegisterBusinessModule` 全库零调用、`PaymentBusinessModule` 无任何生产实现 → sources/fulfillments 恒为空 → `NewConfiguredPaymentUsecase` 必报"支付业务订单来源未注册"→ wireApp 失败,应用无法启动。biz.go 无条件包含 payment.ProviderSet,无跳过路径。task 域有 `taskRegistry()` 自定义 provider 做装配前注册,payment 域缺等价机制 | cmd/wire_gen.go:114-120;internal/biz/payment/payment.go:390-398,400-412;biz/payment/payment_order.go:144-152(接口无实现) | R-9 的 fail-fast 机制本身正确落地,但与"仓库内零业务模块"叠加产生阻塞。修法:装配前注册机制(仿 taskRegistry)或默认业务模块或允许显式禁用 |
| W-2 | **CompleteUpload 吞掉 ListChunks 底层错误并误置会话失败**:`err != nil || len(chunks) != ChunkTotal` 合并判定,DB 瞬断被误报为"分片不全",且 fail() 将会话置 failed → 用户必须重建会话全量重传。直接违背 D-16 修复意图(仓储层已正确透传,仅此处用例层吞掉) | internal/biz/system/media_upload.go:208-211 | 修法:拆开 err 与数量不足两个分支分别返回 |
| W-3 | MergeRuntimeConfig 嵌套子节合并缺口:仅当 `next.Data == nil` 才补 current.Data;next.Data 非 nil 但子节(Database 等)为 nil 时静默丢失,watchLoop 无 reloadConfig 那样的后置校验;Admin 子节同理 | internal/config/clone.go:29-40;config/runtime.go:271 | 建议补"next.Data 非 nil 但子节 nil"回归测试并做嵌套合并 |
| W-4 | 退款状态词表两处语义边角:① `NormalizeStatus` 将裸词 "REFUND" 归 failed——微信 v2 已退款订单在 vendor 查询回退路径会被归为失败,与 `PaymentStatusRefunded` 语义冲突;② `NormalizeRefundStatus` 成功词表缺 "TRADE_SUCCESS"(支付宝风格),会触发保守失败关闭 | internal/paymentkit/status.go:12,16,25 | 低危但需知晓/修正词表 |
| W-5 | 公告/参数/版本列表无 ORDER BY:统一接入 listRows 后这三个列表无排序,LIMIT/OFFSET 翻页顺序不稳定(MySQL/PG 均不保证) | data/system/announcement.go:74、parameter.go:73、version.go:67 | 收敛时遗留;补 `id desc` 或 `created_at desc` |
| W-6 | 外部退款分支存在不可达死分支(复制未裁剪):`accepted := providerErr == nil` 后 `else if !accepted` 永不触发 | internal/biz/payment/payment.go:923-928 | 删除死分支 |
## 二、死代码与零消费者机制
### 2.1 三层死方法残留(第三至五轮处置后仍未删,均经全仓 Grep 反查确认零调用)【二轮发现,四轮复核仍在】
| 层 | 死代码 | 位置 |
|----|--------|------|
| biz 接口+data 实现 | `RecordDataAccess` 整链(接口+实现,查询侧 ListDataAccess/DeleteDataAccess 是活的,仅写入侧死) | biz/system/audit.go:103;data/system/data_access_log.go:24 |
| service 包装 | security_session.go 14 个方法中 8 个死:ActiveToken/LoginLocked/IncrementLoginFailure/LockLogin/ClearLoginState/IncrementLoginIP/UseMultipoint/RotateActiveToken(biz 内部直调 usecase,这些包装无人调用;存活 6 个被 public/rate_limit 消费) | service/system/security_session.go:17-67 |
| biz 注入面 | `RegisterBusinessModule`(W-1 的成因之一,两阶段注册+回滚补偿零调用);`PaymentBusinessModule`/`PayInternal`/`RefundInternal`/`AuthorizeRefund` 接口面仍无生产实现(biz 调用链真实存在,仅实现者缺——与 W-1 一并处理) | biz/payment/payment.go:400-412;payment_order.go:139-152 |
| dto 死字段 | GetAuthorityButtonsRequest.Selected(输入被丢弃);MenuResponse.Authorities 恒 null;DynamicMenuResponse.MenuButtons/Authorities 恒 nil;SysBaseMenuID 输入侧两处被丢弃;version 导出结构体零值噪声字段群(ID:0/CreatedAt 零时间/authoritys:null) | dto/permission.go:6、menu.go:19,24,109,126,128;service/system/version.go:22-96 |
| 死分支 | 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:65);integration/provider.go:33-38 三重死绑定 + Hub 死绑定
- websocket:Hub 接口零消费者;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` | data/payment/payment.go:520-527 | 与 paymentkit.ContainsFold 功能重复 |
| `paymentkit status.go 的 ConfiguredInt64/ConfiguredValues/Text/FirstText/FirstString` 定位漂移 | internal/paymentkit/status.go:36-90 | 属"供应商配置解析"超出 README 声称范围(文档漂移,非死代码) |
## 三、重复实现 / 双份维护
### 3.1 大块可消除
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| D-2 | payment 渠道适配器剩余重复(第一非空字符串/JSON 编码/双状态归一化已收敛):下单方式归一化骨架仍 9 份(Replacer 归一化行逐字出现 9 次);退款身份校验 4 份同构;状态归一化 SDK 专属词表 7 个 normalize*State 与 paymentkit 通用归一化双轨维护(同一状态词需两族词表同步) | integration/payment/alipay.go:434-456、douyin.go:108-126、qq.go:124-144 等;gopay_helpers.go:145-225 | 二轮(四轮部分收敛) |
| 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-196;runtimeconfig/store.go:62-167 | 一轮 |
| D-10 | "storage/email 不落盘"不变式三重执行(persistConfigValues 置 nil+Delete、persistDatabaseConfig 再 Delete、removeIntegrationConfigFromFile 启动时又删) | config_store.go:45-47,102,106-122;data.go:279-283 | 一轮 |
### 3.3 中小重复
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| D-11 | authority 树构建算法两份【既定不采用,保留】 | biz authority.go:48-78 vs menu.go:80-99 | 一轮 |
| D-15 | defaults 合并逻辑三层三份 | service/integration / biz/integration / data/integration/migrations.go:24-29 | 二轮 |
| D-17 剩余 | CallbackFields 已复用 paymentkit(见文末);剩余 values() 与 testRow() 近重复 | data/payment/payment.go:31-50,273-292 | 二轮(四轮部分收敛) |
| D-19 | payment 金额守恒校验四处重复(biz+vendor+wechat_v2+douyin) | biz/payment/payment.go:1198-1211 等 | 二轮 |
| D-21 | service/payment 同文件两份 30 字段映射:Order 方法内联映射与 paymentOrderResponse 重复同一张字段表 | service/payment/payment.go:14-34,65-82 | 四轮 |
| D-22 | data/payment 本地 contains 与 paymentkit.ContainsFold 重复(同 2.3) | data/payment/payment.go:520-527 | 四轮 |
## 四、过度分层:转发门面 / 透传壳 / 回调穿透
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| 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 方法给 worker;TaskApplicationUsecase 再包一层、其 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:168;data_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 | biz/system 33 个非测试文件,17 个 <60 行微文件群仍在(errors 14/cache 15/maintenance 19/actor 19/access_control 20/storage 26/data_scope 27 等,合计约 575 行,可归并 8-10 个文件) | biz/system/* | 三轮(四轮复核未动) |
| S-2 | SystemConfigService 一型仍四文件:system.go(19)+system_config.go(26)+system_init.go(38)+system_info.go(23) | service/system/* | 三轮(四轮复核未动) |
| S-3 | router 22 文件 464 行平均 21 行/文件;routes.go 手工 21 连调(与 X-1 一并解决) | server/router/* | 三轮 |
| S-4 | provider.go+providers.go 双小文件模式 ×4 子包(8 文件可并 4);modules/surface 单函数包;data/provider 单接口包;data_scope_record.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/httpx;SetTokenCookie 注释自称 "no KRA business dependency" 与语义不符 | internal/server/httpx/response.go:17,58-64 | 四轮(P-1 移动残留) |
| P-15 | 包移动注释漂移:http.go:2、response.go:59 仍写 "pkg/httpx";zap.go:29 写 "pkg/logging";source.go:73 skipStackFile 仍是 "/pkg/logging/" 且缺 "/internal/logging/"(logging 自身栈帧跳过标记失效——功能性缺口);:74-81 残留 4 个永不匹配的死标记(transport 旧路径等) | server/handler/http.go:2、server/httpx/response.go:59、internal/logging/zap.go:29、source.go:73-81 | 四轮(P-2 移动残留) |
| P-16 | CLAUDE.md 结构描述整体过时(描述 api/、internal/global/ 等不存在目录),与 AGENTS.md 不同步 | CLAUDE.md:9-17 | 四轮 |
## 七、分层 / 职责违规
| # | 问题 | 位置 | 轮次 |
|---|------|------|------|
| L-3 | biz DO 带 json 标签:PaymentResult/PaymentTestResult(死标签);PaymentRequest(指纹编码格式锚死 DO);biz/integration Definition 家族充当前端契约 | biz/payment/payment.go:59-204;biz/integration | 二轮 |
| L-4 | DO 兼过滤器:API.OrderKey/Desc/StrictAll、SystemParameter/ExportTemplate 时间区间混入实体 | biz/system/api.go、parameter.go、export.go | 二轮 |
| L-5 | middleware 硬编码业务语义:中文消息黑名单判断审计(改文案即改审计行为);业务路径硬编码;支付回调专用逻辑内嵌通用中间件;限流策略内联 | server/middleware/* | 二轮 |
| L-6 | biz 契约泄漏存储/表现原语:QueryExport 返回 []map[string]any;export DO 携带 SQL 片段;UserOptions UI 形状;AuthenticationResult 携带密码哈希 | biz/system/* | 二轮 |
| L-7 | data 层纪律:Table("字符串") 绕过 PO;saveRelations 回写入参 DO;OriginSetting 裸转换 | data/system/* | 二轮 |
| L-8 | 编排类文件过重:seedSystem 126 行 10 类职责;authority.go 四类职责(权限引擎应独立 accessGuard);BuildVersionBundle 五职责 | 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.Store,mq/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 中间件质量**:有据可依
- **新增缝质量(第四轮验证)**:PaymentAdapterFactory(biz 接口+integration 实现+wire 绑定,单向无环)、MergeRuntimeConfig(单点三调用)、deletePrefixViaList(函数式注入,失败关闭正确)、AST 缓存(size+mtime 失效)、systeminfo(integration 定位正确)——均内聚、依赖最小、无越界 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-2/S-4 未动,零行为变更)
5. **D-2/D-19 payment 剩余重复 + L 系列归位**(独立批次)
6. **X-1 路由单源化等大动作**(最后)