From 712de30133f921b5ac1f1ae2a1b77fff8b709c20 Mon Sep 17 00:00:00 2001 From: song.jun <lion0756@qq.com> Date: 星期一, 13 四月 2026 12:49:16 +0800 Subject: [PATCH] 修正代码评审发现的 Critical 问题（a3f4e4f 后续修复） --- CLAUDE.md | 11 ++++++----- 1 files changed, 6 insertions(+), 5 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 58b8751..66a797c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -118,18 +118,19 @@ - **唯一的保存入口**：所有保存 `QCDistributionRegisterInfo` 的路径都必须经过 `QCService.SaveQcDistributionRegister`。新写任何涉及登记表写入的代码，**不要**直接 `_qcDistributionRegisters.Insert`、不要绕开此方法。 - **Id==0 的语义**：该方法把 `Id==0` 视为"新增"请求，进入前会按 `(QCDistributionId, LabId, ProjectId)` 做查重： - 查不到 → 正常 Insert。 - - 查到 → 打一条 `[WARN]` 前缀的 Info 日志，按 `MergeRegisterInfo` 规则**合并**到既有行后 UPDATE；同时把入参 `qcDistributionRegister.Id` 回写为 `existing.Id`，便于调用方后续引用。 + - 查到 → 打一条 `LogHelper.Error` 日志（强制告警），按 `MergeRegisterInfo` 规则**合并**到既有行后 UPDATE；同时把入参 `qcDistributionRegister.Id` 回写为 `existing.Id`，便于调用方后续引用。 - **MergeRegisterInfo 的合并规则（刻意不对称）**： - - `string` 字段：`source != null` 才覆盖（保留既有内容，不会被意外 null 清空）。 + - `string` 字段：`!string.IsNullOrEmpty(source)` 才覆盖（保留既有内容；空串也不覆盖，因为 `QCDistributionRegisterInfoViewModel.ToEntity` 里的 `entity.EMSNo = "" ?? ...` 这类 force-set 代码会把 null 变成 "", 若按 `!= null` 判断就会清空既有 `EMSNo`）。 - `DateTime?` 字段：`HasValue` 才覆盖。 - `DateTime ModifyTime`：非默认值才覆盖（调用方都用 `DateTime.Now`，天然非默认）。 - `bool` 字段：**只能 `false → true`，不能 `true → false`**。这是为了防止一次 Id==0 的脏 Save 把既有的 `IsCharged=1` 意外清零。 - `QCDistributionId / LabId / ProjectId`：业务唯一键，不动。 - **要把 bool 清回 false、或清空 string/Date 字段，必须走 Id>0 路径**：先用 `GetQcDistributionRegister(id)` 查出 tracked entity，直接改字段再 `SaveQcDistributionRegister`。用 `Id=0 + IsCharged=false` 这种方式期望"反收费"会被静默忽略。 -- **生产日志里出现 `[WARN] SaveQcDistributionRegister 命中已有行`** = 某个上层调用方漏做了查重或前端提交了脏 Id，**必须排查根因**，不要忽略。 -- **`SaveLabList` 路径（`QCDistributionInfoViewModel.ToEntity`）**：作为第一道防线，保留了自己的 `(QCDist, LabId, ProjectId)` 查重分支 —— 与 `SaveQcDistributionRegister` 里的收口形成双保险。修改这两处任一处时都要同步检查另一处。 +- **生产日志里出现 `SaveQcDistributionRegister 命中已有行`（Error 级别）** = 某个上层调用方漏做了查重或前端提交了脏 Id，**必须排查根因**。这条日志**刻意用 `LogHelper.Error`** 而不是 Info —— 因为本仓库的 `LogHelper` 没有 Warn 方法，而 Info 级别不会触发告警系统；用 Error 保证运维能第一时间收到通知。 +- **`SaveLabList` 路径（`QCDistributionInfoViewModel.ToEntity`）**：作为第一道防线，保留了自己的 `(QCDist, LabId, ProjectId)` 查重分支 —— 与 `SaveQcDistributionRegister` 里的收口形成双保险。修改这两处任一处时都要同步检查另一处。注意第一道防线命中后**只同步 `IsCharged` 和 `ModifyTime`**（基于 SaveLabList 的业务语义），第二道防线命中后走 `MergeRegisterInfo` 全字段合并（通用兜底）；两者行为刻意不对称。 +- **`QCDistributionRegisterInfoViewModel.ToEntityByLabCode`** 的 ProjectId 过滤是**条件的**：只有 `regInfoivewModel.ProjectId > 0` 才加入 WHERE 子句，否则退化为只按 `(QCDistributionId, LabId)` 匹配。原因：前端 `Views/Backstage/QCDistributionLabs.cshtml` 的 `openFeeWindow` / `openEMSWindow` 把 `CurrentQCDistRegisterInfo` 重置为不含 ProjectId 字段的 JSON 字面量，序列化后服务端 `regInfoivewModel.ProjectId == 0`，无条件加过滤会让 `ConfigrmFee` / `ConfigrmEMS` / `switchNextOne` 全部静默失效。**未来若前端补上 ProjectId 选择控件**，服务端会自动切换到精准匹配，不需要改服务端代码。多项目共存时当前会匹配第一条（与修复前行为一致）。 - **`BackstageController.ImportLabs`**：是唯一绕过 `SaveQcDistributionRegister` 的写入路径（直接操作 `QCDistribution.QCDistributionRegisters` 导航集合）。它自己在源读取后用 `GroupBy(LabId, ProjectId).Select(g => g.First())` 去重；**修改 ImportLabs 时必须保留这段 GroupBy**，否则脏数据会跨分发传染。 -- **残余风险**（已知，不修复）：两个管理员并发编辑同一分发时，两个独立 DbContext 的查重都可能返回空，仍有产生重复的理论窗口。本仓库不通过 DB 约束兜底，缓解办法是前端按钮防抖 + 日志监控 `[WARN]` 条目。如果日后发现并发窗口被真实触发，加唯一索引是根治方案（见"一致性"章节末尾）。 +- **残余风险**（已知，不修复）：两个管理员并发编辑同一分发时，两个独立 DbContext 的查重都可能返回空，仍有产生重复的理论窗口。本仓库不通过 DB 约束兜底，缓解办法是前端按钮防抖 + 监控 `SaveQcDistributionRegister 命中已有行` 的 Error 日志。如果日后发现并发窗口被真实触发，加唯一索引是根治方案（见"一致性"章节末尾）。 ## 修改时的红线（针对本仓库的具体风险） -- Gitblit v1.8.0