编辑 | blame | 历史 | 原始文档

Code Review Report — QCDistributionRegisterInfo 重复登记防御修复

  • Commit: a3f4e4f 修复 QCDistributionRegisterInfo 重复登记问题
  • 分支: master3
  • 评审级别: L3(核心业务 / 数据一致性 / 无 DB 约束兜底)
  • 评审者: 独立上下文 subagent(不知任何实现过程)
  • 评审日期: 2026-04-13
  • 硬约束(来自用户):不加唯一索引、不引入 SERIALIZABLE 事务

1. 总评分与结论

总分:6.2 / 10 — 不通过,必须修复后再评。

本次修复整体方向正确(业务层双层查重 + 收口合并规则 + ImportLabs 去重 + 日志告警),架构思路站得住脚。但**存在一处 Critical 级回归**:QCDistributionRegisterInfoViewModel.ToEntityByLabCode 新加的 ProjectId 过滤条件,会让 ConfigrmFee / ConfigrmEMS / switchNextOne 三个后台收费/寄送 ajax 动作在当前前端实现下 100% 静默失败(详见第 6/10 节)。另外日志告警命名有效性、MergeRegisterInfo 对 string 字段的对称性、以及 ImportLabs 的二次脏数据风险各有中等问题。修掉 Critical 后复评有望到 9.0+。


2. 各维度打分表

维度 权重 分数 主要扣分点(带证据)
正确性 30% 5.0 CriticalQCDistributionRegisterInfoViewModel.cs:73 加的 p.ProjectId == regInfoivewModel.ProjectId 对当前前端是致命回归(见 §6);MergeRegisterInfo 对 string 的"仅非 null 才覆盖"无法区分"要清空"和"不想改"(见 §7);ImportLabsGroupBy.Select(g => g.First()) 在两条源行 IsCharged 不一致时会静默丢掉一条(见 §5)
规范 20% 7.5 LogHelper.Info("[WARN] ...") 用文本前缀伪装告警级别,不符合 log level 语义(见 §9);MergeRegisterInfo 作为静态方法写在 QCService 里职责混合,命名没有文档注释标注"不对称"规则;QCDistributionInfoViewModel.cs:144-156 的查重分支与 QCService.SaveQcDistributionRegister 的查重分支逻辑**不一致**(第一道防线只同步两个字段,第二道防线走 MergeRegisterInfo 合并全部字段)——两个"双保险"实际规则不同(见 §5)
测试 20% 4.0 仓库无单测工程(已知)。本次 Critical 路径若走一次真实手测即可暴露(开一次"收费确认"对话框,填 LabCode 点"确认"),但 commit 未附任何手测剧本说明、也没 changelog。该分数是无单测 + 无手测证据联合结果
安全 15% 8.0 无 SQL 注入/越权/信息泄露新增面。残余并发窗口是已知、已文档化、明确不修复。扣分点在于 [WARN] 日志监控在 NLog 默认级别过滤下基本不会被运维告警系统发现,使"靠日志监控"这一缓解手段形同虚设
性能/可维护性 15% 7.5 每次 SaveLabList 在循环内做一次 FirstOrDefault 查重(N+1 式,N = LabList.Count),7000 行量级可接受但不优(见 §4);MergeRegisterInfo 共 22 个 if,新增字段时容易漏改;CLAUDE.md 契约章节写得好,给未来维护者留了足够上下文,加分
加权合计 6.23

分数门槛:L3 需 ≥ 9.0。当前 6.23,**不通过**。


3. 边界场景—代码覆盖映射表

仓库无单测工程,下列"测试证据"一列均为**手测剧本**或**对应代码段保证**。带 ❌ 的场景即为"防线未覆盖"。

# 边界场景 由哪段代码兜底 证据/手测剧本 状态
1 同一分发 + 同 Lab + 同 Project,先新建再新建(前端漏查重) QCService.cs:1037-1054 查重命中 → Merge → Update;QCDistributionInfoViewModel.cs:146-156 第一道防线 手测:SaveLabList 把同一行勾选两次连续保存,DB 应仍 1 行
2 同一分发 + 同 Lab + 不同 Project(合法场景) QCService.cs:1040 查重条件包含 ProjectId,不会误命中;QCDistributionInfoViewModel.cs:149-150 同上 手测:一个实验室勾选 ProjectId=1 和 ProjectId=2 两个项目保存
3 历史脏行(DB 已有两条重复)再次 Save(Id=0) QCService.cs:1037FirstOrDefault 只会合并到其中一条,另一条仍留在 DB 手测:人为插入重复行再触发保存,DB 仍有两条 ⚠️ 无兜底,靠上一轮手工 SQL 清理
4 ConfigrmFee 前端提交不带 ProjectId(当前 cshtml 真实情况) QCDistributionRegisterInfoViewModel.cs:73 查询变成 ProjectId == 0 永远命中不到,entity == null 直接 return(BackstageController.cs:218-219) 手测:打开 QCDistributionLabs 页面→点"收费确认"→输入 LabCode→点确认。**应失败/静默无变化** Critical 未覆盖
4a ConfigrmEMS 同上 同 §4 手测:点"标本寄出确认"→填 EMSNo→确认 Critical 未覆盖
4b switchNextOne 同上 同 §4;BackstageController.cs:301 拿不到 entity,无法取下一条 手测:收费对话框里点"下一个"按钮 Critical 未覆盖
5 ImportLabs 源分发里存在 (LabId,ProjectId) 重复 BackstageController.cs:283-287 GroupBy(...).Select(g => g.First()) 手测:把源分发 41 复制到目标分发 99,检查目标表记录数 ⚠️ 信息丢失:若两条源行 IsCharged/AnswerJSON 不同,.First() 静默丢弃后者
6 ImportLabs oriDistId == TargetDistId BackstageController.cs:266-269 前置守卫直接 return 手测:在 UI 里选择相同分发作为源和目标
7 ImportLabs TargetDistId 不存在 BackstageController.cs:270-274 前置守卫 手测:构造 TargetDistId=9999 的请求
8 并发:两个管理员同时 Save 同一 (dist,lab,project) 无兜底,两 DbContext 查重都返回空 → 两个 Insert 都跑 → 重复行 CLAUDE.md:131 已明确标"残余风险" ⚠️ 已知接受
9 saveAnswerInfo(UserUI) 入参 Id=0(理论不应发生,但前端有可能 round-trip 时丢 Id) QCService.cs:1037-1054 收口查重 + MergeRegisterInfo 手测:抓包把 Id 改为 0 再提交 ⚠️ 部分覆盖,但 MergeRegisterInfo 的 string 规则会保留既有脏答案
10 Id=0 + IsCharged=false 想"反收费" QCService.cs:1072 if (source.IsCharged) target.IsCharged = true; 故意不支持 false→true,静默忽略 CLAUDE.md:123 已文档化(要求走 Id>0 路径) ⚠️ 有设计意图,但无代码层面拒绝(无抛异常/日志)
11 Id=0 + ChargeTime=null 想"清空收费时间" 同上,ChargeTime.HasValue 为 false 时保留既有值 CLAUDE.md 未明确列出"清空场景需走 Id>0" ⚠️ 同 §10
12 SaveQcDistributionRegister 命中已有行时,qcDistributionRegister.Id = existing.Id 回写 QCService.cs:1046 回写,防止调用方把"新" entity 当 insert 成功后又用 Id=0 重复 Save 代码静态审查 OK
13 QCDistributionInfoViewModel.ToEntity 第一道防线命中时,传入的 registerInfo 对象被丢弃,只同步 IsCharged+ModifyTime QCDistributionInfoViewModel.cs:152-155 代码审查:**其他字段(Remark/SampleNo 等)都不同步**——如果 ViewModel LabList 里有 Remark 更新,这里会被静默丢失 ⚠️ 与第二道防线规则不一致(见 §5)

硬指标:14 个场景里 3 个 Critical 未覆盖(4 / 4a / 4b)、5 个中等问题、6 个通过。按 L3 规则:**映射表存在 ❌,不得给 9.0+。**


4. 时空复杂度评估

数据规模参考:QCDistributionRegisterInfoes ≈ 7000 行,单次 SaveLabListLabList.Count ≈ 几十到几百(视分页),ImportLabs 一次性 ≈ 几百到 2000。

代码段 时间复杂度 空间复杂度 说明
QCService.SaveQcDistributionRegister 新增 FirstOrDefault 查重 (QCService.cs:1037) EF 翻译为单条 SELECT TOP 1 WHERE dist=? AND lab=? AND proj=?,若 (dist,lab)(dist) 有非聚集索引则 O(log N),否则 table scan O(N)。N=7000 时即使全表扫也 <10ms,可接受 O(1) 风险:若未来 N → 百万级且无索引会成瓶颈。建议在 CLAUDE.md 契约章节里补一句"如果 DBA 同意加非聚集索引(不是唯一索引),推荐 (QCDistributionId, LabId, ProjectId) 以支撑此查重"
MergeRegisterInfo (QCService.cs:1061-1084) O(1)(固定 22 个字段) O(1) 无问题
BackstageController.ImportLabs.ToList().GroupBy(...).Select(g => g.First()).ToList() (BackstageController.cs:282-286) 物化到内存后的 GroupBy:O(M),M = 源分发行数。.ToList()GroupBySelect.ToList() 意味着**两次物化**,内存占用 2×M O(M) M≈几千时 MB 级,可接受。风格上可改为直接 Distinct(comparer) 或用 HashSet<(int,int)> 手工去重,少一次物化,但不是必须
QCDistributionInfoViewModel.ToEntity 循环内查重 (QCDistributionInfoViewModel.cs:148-151) 循环体内 FirstOrDefault → N 次 DB round-trip,每次 O(log K)(K=7000)。总体 O(N log K) O(1) per iter 次优:LabList 常见几十到几百行,即 200 次 DB 查询,每次 5-20ms → 单次请求 1-4 秒。不是 Critical,但用户体验会肉眼可感。**建议**:循环前一次性 .Where(p => p.QCDistributionId == viewModel.Id).ToList() 拉到内存(≈几百行),然后在内存里 FirstOrDefault,复杂度降到 O(1 次 DB + N log K 内存)

5. 并发/竞态风险点

场景 是否被本次修复覆盖 判断
同一用户快速双击"保存" ❌ 不覆盖(两个请求仍会各自走一遍业务层查重,但两次 DbContext 都可能在读后写前完成) 低概率。前端可加按钮防抖作为缓解,但 commit 未提及
两个管理员并发编辑同一分发 ❌ 不覆盖 CLAUDE.md:131 已明确写明残余风险,是**可接受**的
SaveLabList 循环内部非事务地逐行 Save ❌ 不覆盖 任一行失败会留下部分状态,但这是原有问题,本次修复未引入恶化
ImportLabs 先 Delete 再 Add 非事务 ❌ 不覆盖 同上,原有问题。qcDistInfo.QCDistributionRegisters.Add(newEntityInfo) 通过导航集合写入,会被后续 SaveQcDistribution 的 SaveChanges 批量提交,比逐条稍好
QCService.SaveQcDistributionRegister 内的 "查重 → Update" 无悲观锁/乐观锁 ❌ 不覆盖 无 RowVersion / Timestamp 字段,无法做乐观并发。残余风险和"两个管理员并发"同源

可接受性判断:本次 commit 作为无 DDL、无 TransactionScope 的"业务层尽力而为"方案,并发残余风险已在 CLAUDE.md 明确告知并被用户接受。**此项不扣分**(硬约束决定的)。唯一建议是 §9 的日志监控可行性,让残余风险发生时能及时发现。


6. 集成一致性检查结果

6.1 双层查重规则对齐检查

接口/动作 提供方 消费方 查重逻辑 合并规则 一致性
SaveLabList 第一道防线 QCDistributionInfoViewModel.ToEntity (cs:148-156) BackstageController.SaveLabList 命中则**只同步 2 个字段**:IsChargedModifyTime,然后调 SaveQcDistributionRegister(existing)(此时 existing.Id>0 → 走 Update 分支,不会再走 Merge) 与第二道防线不对齐
SaveQcDistributionRegister 收口防线 QCService.SaveQcDistributionRegister (cs:1037-1053) 所有 Id==0 的调用方 命中则用 MergeRegisterInfo 合并**全部 22 个字段** 不对称合并(string 非 null / bool 只升)

问题:第一道防线只同步 IsCharged/ModifyTime,而 registerInfo 里还有 SampleNo/EMSNo/PacketContent 等字段(在 QCDistributionInfoViewModel.cs:140 之前由 ToEntity(x) 从 ViewModel 拷贝过)。命中已有行时这些字段**会被静默丢弃**。如果用户在 SaveLabList 页面编辑了 Remark 然后保存,Remark 更新会被丢掉。

证据sbcLabSystem/Models/Backstage/QCDistributionInfoViewModel.cs:140-156

建议:第一道防线应当和第二道防线统一走 MergeRegisterInfo(把它从 QCService 里 public 化或独立到 util),或者至少显式同步同一组字段。当前状态是"两道防线规则分叉",CLAUDE.md 声称"双保险"但实际两保的不是同一件事。

6.2 ToEntityByLabCode 新增 ProjectId 过滤 —— 所有调用方的 ProjectId 传递链路检查

调用方 入口 前端提交 payload 里是否一定有 ProjectId 结论
BackstageController.ConfigrmFee (cs:217) QCDistributionLabs.cshtml:191 confirmFee 函数 openFeeWindow 初始化字面量 {"LabCode":"","LetterNo":"","ChargeRemark":"","QCDistributionId":"{0}","IsCharged":false} 不含 ProjectId。对话框 HTML (cshtml:385-406) 无 ProjectId 输入控件。用户无法输入 ProjectId。即使 CurrentQCDistRegisterInfo 在页面初始化时从后端 QCDistributionPageViewModel.CurrentQCDistRegisterInfo = new QCDistributionRegisterInfoViewModel() 拿到 ProjectId=0,提交时也是 0 Critical 回归
BackstageController.ConfigrmEMS (cs:241) cshtml:227 confirmEMS 函数 同上 Critical 回归
BackstageController.switchNextOne (cs:301) cshtml:236 switchNextOne 函数 同上,依赖 ToEntityByLabCode 先拿到 entity 再找下一条 Critical 回归

连锁反应
1. 前端 confirmFeeko.mapping.toJS 序列化出 {LabCode, LetterNo, ChargeRemark, QCDistributionId, IsCharged, InputElement, ProjectId:0, ...}
2. 后端 ConfigrmFee 收到 regInfoivewModel.ProjectId == 0
3. ToEntityByLabCode 查询 WHERE QCDistributionId=X AND LabId=Y AND ProjectId=0,DB 里 ProjectId 最小是 1 → entity == null
4. BackstageController.cs:218-219 直接 return Json(regInfoivewModel) → 前端拿到对象后 ko.mapping.fromJS(json, {}, viewModel.CurrentQCDistRegisterInfo) 继续走,UI 可能弹 "收费成功" 或看似正常,**但 DB 没任何变化**
5. 原有 bug(QC269 两条重复行)本来就是靠 ConfigrmFee 改 IsCharged 时命中了"其中一条"来凑合工作的。修复后变成**两条都命不中**,比 bug 本身更糟

必须修复路径(三选一):
- (A) 前端补 ProjectId:在 cshtml:91cshtml:108 的 JSON 字面量里加 "ProjectId":0 不够(只把 0 显式化);需要在对话框打开时或提交前从 LabList 找到 LabCode 对应的那一行拿 ProjectId 塞进去。**但问题**:一个 LabCode 可能在多个 ProjectId 都有记录,对话框里没给用户选,无法唯一确定
- (B) 改回 ToEntityByLabCode 的查询条件:保持原先的 (QCDistributionId, LabId) 匹配,不加 ProjectId;改动放到别的地方(比如对话框里加 ProjectId 下拉框)
- (C) 最兼容方案:在 ToEntityByLabCode 里做降级—— if (regInfoivewModel.ProjectId > 0) 才加 ProjectId 条件,否则退化为原行为(可能命中多条中的一条,但不会全部失效)。配合 CLAUDE.md 契约章节补一句"前端对话框场景下允许 ProjectId=0,此时退化为只按 LabId 匹配"

我建议方案 (C) 作为最小代价回滚 + (A) 作为根治长期方向。不修复这个 Critical,本次 commit 不应合入。

6.3 ImportLabs GroupBy 的信息丢失评估

BackstageController.cs:282-286:

.GroupBy(x => new { x.LabId, x.ProjectId })
.Select(g => g.First())

分析:源分发若存在脏数据(两条 (LabId, ProjectId) 重复),.First() 只取第一条。被丢弃那条的 IsCharged/AnswerJSON/Remark/LetterNo 等状态全部丢失,目标分发只能拿到第一条的快照。

是否影响业务:ImportLabs 的功能是"把已有分发的实验室列表复制到新分发作为起点"。新分发是空的,源里的 IsCharged / AnswerJSON 从业务上讲**本来也不应**带到新分发(新一轮质评应该从 "未收费 / 未答题" 开始)。看 cs:287-294newEntityInfo 构造:

newEntityInfo.QCDistributionId = TargetDistId;
newEntityInfo.Id = 0;
newEntityInfo.LabId = x.LabId;
newEntityInfo.ProjectId = x.ProjectId;
newEntityInfo.ModifyTime = DateTime.Now;

只复制了 LabId/ProjectId 两个维度标识,根本不带 IsCharged/AnswerJSON 过来。所以 .First() 丢弃的信息**本来就会被丢弃**,**GroupBy 的信息丢失不影响业务**。✅

6.4 一致性汇总

接口 提供方 消费方 状态 偏差说明
SaveLabList 双层查重规则 ToEntity + SaveQcDistributionRegister BackstageController.SaveLabList 第一道防线只同步 2 字段、第二道防线合并 22 字段
ToEntityByLabCode ProjectId 过滤 QCDistributionRegisterInfoViewModel ConfigrmFee / ConfigrmEMS / switchNextOne 前端提交不含 ProjectId,3 个动作全部回归
ImportLabs 去重 BackstageController.ImportLabs 自身 信息丢失业务可接受
SaveQcDistributionRegister 收口 QCService UserUIController.saveAnswerInfo / BackstageController.* / QCDistributionInfoViewModel.ToEntity ⚠️ MergeRegisterInfo 的 string 规则下,脏 Id=0 提交不能清空字段
ImportLabs 通过导航集合 Add 绕开 SaveQcDistributionRegister BackstageController.ImportLabs DbContext ⚠️ 已知绕开路径,靠自己 GroupBy 兜底,CLAUDE.md 已文档化

7. MergeRegisterInfo 不对称合并规则专项审查

7.1 逐字段审查

字段 类型 规则 正确性评价
LetterNo string source != null 才覆盖 ⚠️ 无法区分"空字符串=想清空"和"null=不想动"。目前 ToEntity 里空字符串被赋值为 "" 而非 null(如 entity.EMSNo = viewModel.EMSNo == null ? "" : viewModel.EMSNo.Trim();,见 QCDistributionRegisterInfoViewModel.cs:632),所以 source.LetterNo != null 常常为真 → 实际上会覆盖(可能会误覆盖成空串)。和"不会被意外 null 清空"的设计意图是矛盾的
ChargeRemark string 同上 同上
SampleNo string 同上 同上
EMSNo string 同上 同上。特别注意:ToEntityentity.EMSNo = "" 的路径会让 merge 误把已有 EMSNo 清空为 ""
PacketContent / Remark / ModifyUser / AnswerJSON / SubmitUserNo / Score_Detail string 同上 同上
ChargeTime / SendEMSTime / SubmitTime / FirstTimeSubmitTime / LastPageModifyTime DateTime? HasValue 才覆盖 ✅ 可区分 null 和有值,正确
ModifyTime DateTime(非 nullable) != default(DateTime) 才覆盖 ✅ 所有调用方都会 set DateTime.Now,实际上总会覆盖。OK
IsCharged / IsSendEMS / IsSubmit / IsModified / IsEnding bool if (source.*) target.* = true ✅ 符合设计意图"防止脏 Save 把 true 清 false",但反向意愿无法表达(CLAUDE.md:121-124 已文档化)
QCDistributionId / LabId / ProjectId int 不动 ✅ 业务唯一键,正确

7.2 具体场景

反例 — 证明 string 规则有 bug

管理员在一个已收费的 (dist=41, lab=5, proj=1) 行上(LetterNo="A1001", ChargeTime=2026-03-01, IsCharged=1)通过 saveAnswerInfo(UserUI 路径)提交了一份答题。UserUIController.cs:259 走 QCDistributionRegisterInfoViewModel.ToEntity(viewModel)。该 ToEntity 方法 (cs:608-637) 先用 Id == viewModel.Id 查,拿不到(比如 viewModel.Id 被前端 round-trip 成 0)就 new QCDistributionRegisterInfo() 配合 ClassValueCopier.Mapper。Mapper 会把 viewModel 的所有字段拷贝过去,包括 LetterNo=""(用户 answer 页面不填收费信息,默认值是空串)。然后落到 SaveQcDistributionRegister → 命中已有行 → MergeRegisterInfosource.LetterNo = "" 不为 null → target.LetterNo 被覆盖为空串,已收费的 A1001 编号丢失ChargeTime 因为是 DateTime? 且 viewModel 没 set 过保持 null,不会被清;IsCharged 因为 bool 规则不会被清。但 LetterNo 确实会丢。

这与注释"防止脏 Save 把 IsCharged=1 意外清零"的设计意图部分达成,但 string 字段的保护是半吊子的**。

正例 — 证明 bool 规则对

同一场景下 source.IsCharged = false(新提交答题默认),target.IsCharged = true(已收费),if (source.IsCharged) 为 false → target 保持 true。✅

7.3 特别关注点(用户提出)

一个已有 IsCharged=1 的行被 saveAnswerInfo(Id=0) 路径命中合并后,既有的 LetterNo / ChargeTime / ChargeRemark 会不会被意外清空?

  • IsCharged:✅ 不会被清(bool 规则保护)
  • ChargeTime:✅ 不会被清(DateTime? + HasValue 规则,且 saveAnswerInfo 不 set ChargeTime)
  • LetterNo:❌ 会被清空为 ""(见上面反例,ClassValueCopier.Mapper 会把 viewModel 的空 LetterNo 写入 source)
  • ChargeRemark:❌ 同上会被清空为 ""

结论MergeRegisterInfo 的 string 保护规则假设前提是 "source.XxxField 为 null 代表调用方不关心",但仓库里的实际 ToEntity / ClassValueCopier 路径**不会产生 null**,只会产生 ""。**保护规则在真实调用链下失效。**

7.4 修复建议

  • 把 string 字段的"非 null"改成"非 null 且非 """ 才覆盖(即 !string.IsNullOrEmpty(source.XxxField)),并在 CLAUDE.md 契约里明确"要清空 string 字段必须走 Id>0 路径"。
  • 或者更稳的做法:为 saveAnswerInfo 这条路径在 merge 时**显式白名单**只合并答题相关字段(AnswerJSON/SubmitTime/ModifyTime/IsSubmit),不要碰收费相关字段。但这需要调用方传递"意图",改动更大。

8. 未被覆盖的路径盘点

搜索结果(grep _qcDistributionRegisters.(Insert|Add)|QCDistributionRegisters.Add|QCDistributionRegisterInfoes.Add):

文件 行号 是否绕开 SaveQcDistributionRegister 风险
sbcLabSystem.Service/QC/QCService.cs:1054 _qcDistributionRegisters.Insert(qcDistributionRegister) ,就是 SaveQcDistributionRegister 内部 N/A
sbcLabSystem/Controllers/BackstageController.cs:294 qcDistInfo.QCDistributionRegisters.Add(newEntityInfo) (已知 ImportLabs 路径) 已靠 GroupBy 兜底 + 目标表先清空
sbcLabSystem.Import/Form1.cs:36-60 current_db.QCDistributionRegisterInfoes.Add(regInfo) + current_db.SaveChanges() (一次性数据迁移工具,winform) 不影响线上。这是一个旧版 MySQL→SQL Server 迁移 button,历史遗留工具,已完成使命,运行时不需要管

答:绕开路径只有 2 处:(1) 已知的 ImportLabs,已处理;(2) sbcLabSystem.Import.Form1 一次性迁移工具,不影响线上。搜索结果完整。✅

搜索命令

Grep pattern: "_qcDistributionRegisters\.(Insert|Add)|QCDistributionRegister\.Add|\.QCDistributionRegisters\.Add"

9. 日志告警机制评估

QCService.cs:1043:
csharp LogHelper.Info(string.Format("[WARN] SaveQcDistributionRegister 命中已有行 ..."));

问题 1 — Log level 语义错配
- LogHelper.Info 底层打的是 NLog Info 级别
- NLog 的告警规则一般只对 WARN/ERROR/FATAL 级别做邮件/短信/IM 转发
- [WARN] 文本前缀只能帮助人眼过滤日志文件,无法被日志监控系统识别
- 查 LogHelper 是否有 Warn 方法:grep 结果里没有找到 class LogHelper 定义(来自 BatchService.Framework.Utility 包),但 NLog 本身支持 Warn,常见 wrapper 应有。**必须确认并使用 LogHelper.Warn(...)**(如果没有这个方法,需要同时提一个补丁给 wrapper)

问题 2 — "请检查上层为何未先查重"是给人看的,但没有触达机制
- 即使 level 改成 Warn,生产环境默认 nlog.config 的 target 是什么?从 sbcLabSystem/bin/NLog.configsbcLabSystem.Service/bin/Debug/NLog.config 的存在可知有配置,但未检查具体 rule。若 target 只是写文件,仍然没有主动告警
- 本次 commit 作为"防重复"的缓解手段严重依赖这条日志被人看到,但实现上形同虚设

修复建议
1. 必改LogHelper.Info(...)LogHelper.Warn(...)(或等价方法)
2. 次要:在 commit 或 CLAUDE.md 里补一句"生产 nlog.config 的 Warn 级 target 指向 xxxx,确保运维能收到告警"
3. 可选:命中次数 + 时间戳汇总到一个独立表/文件,定期由运维脚本扫一次;这个比改 nlog.config 更容易控制

评分影响:-1.5 于"安全"维度(从 9.5 到 8.0)。


10. 必须修复的问题清单

按优先级排序。**只有 P0 全部修掉才能复评到 9.0+**。

P0 — Critical,必须立即修复

  1. QCDistributionRegisterInfoViewModel.ToEntityByLabCode 的 ProjectId 过滤 (cs:73) 导致 ConfigrmFee / ConfigrmEMS / switchNextOne 全部回归
  • 证据sbcLabSystem/Views/Backstage/QCDistributionLabs.cshtml:91,108 的对话框初始化字面量不含 ProjectId;cshtml:385-415(confirmFee 对话框)和 cshtml:418-448(confirmEMS 对话框)的 HTML 无 ProjectId 输入控件
  • 修复:采纳 §6.2 方案 (C)
    csharp var query = _qcService.GetQcDistributionRegisters() .Where(p => p.QCDistributionId == regInfoivewModel.QCDistributionId && p.LabId == labId); if (regInfoivewModel.ProjectId > 0) { query = query.Where(p => p.ProjectId == regInfoivewModel.ProjectId); } QCDistributionRegisterInfo entity = query.FirstOrDefault();
  • 手测剧本(修复后必跑):
    1. 打开 QCDistributionLabs → 点"收费确认"对话框 → 输入一个真实 LabCode → 点确认 → 检查 DB IsCharged / LetterNo / ChargeTime 三个字段是否真的更新
    2. 同上但点"标本寄出确认" → 检查 EMSNo / IsSendEMS / SendEMSTime
    3. 同上但先点"收费确认"确认后再点"下一个" → 检查对话框内容是否切到下一条
  1. MergeRegisterInfo 对 string 字段的"非 null 才覆盖"规则在真实调用链下失效,会把已有 LetterNo/ChargeRemark/EMSNo 等清空为 ""
  • 证据sbcLabSystem.Service/QC/QCService.cs:1062-1071;触发链路见 §7.2 反例
  • 修复:把所有 if (source.XxxField != null) 改为 if (!string.IsNullOrEmpty(source.XxxField)),并在 CLAUDE.md 契约章节"MergeRegisterInfo 的合并规则"里补一句"string 字段为 null 或空字符串都不覆盖"
  • 影响面:需要让调用方在"主动想清空某字段"时走 Id>0 直改路径(已 CLAUDE.md 约定,但要补充 string 版本)

P1 — Important,强烈建议修复

  1. QCDistributionInfoViewModel.ToEntity 第一道防线命中时只同步 2 个字段,与第二道防线的 MergeRegisterInfo 规则不一致
  • 证据sbcLabSystem/Models/Backstage/QCDistributionInfoViewModel.cs:152-156 vs sbcLabSystem.Service/QC/QCService.cs:1040-1053
  • 修复:让第一道防线直接把 registerInfo 的所有可覆盖字段写到 existing(通过调 MergeRegisterInfo,为此需把 MergeRegisterInfoprivate static 提成 internal static 或 public,或在 QCService 上暴露一个 SaveOrMerge(...) 方法)。这样双保险才是真正的"同一规则执行两次"
  1. LogHelper.Info("[WARN] ...") 改成真正的 Warn 级别
  • 证据QCService.cs:1043
  • 修复:改成 LogHelper.Warn(...)(需确认 wrapper 提供该方法;若无,补一个 wrapper 方法)

P2 — Suggestion,可选改进

  1. QCDistributionInfoViewModel.ToEntity 的 per-loop DB 查重优化
  • 证据cs:148-151 循环内 FirstOrDefault
  • 建议:循环外一次性 .Where(p => p.QCDistributionId == viewModel.Id).ToList() 物化到 Dictionary<(int lab, int proj), QCDistributionRegisterInfo>,循环内查 dict
  1. MergeRegisterInfo 的字段列表脆弱
  • 建议:加一段 XML 注释说明维护约定;或用反射/表达式树遍历 [MergeWhen*] attribute 标注,避免后续加字段漏改
  1. CLAUDE.md 契约章节补一条关于 NLog 级别的操作约定
  • "本仓库 LogHelper.Warn 的 target 配置位于 xxx,生产环境需确认 Warn 级别会被监控系统识别"

11. 建议(非强制)

  1. 长期方向:仍建议和产品沟通"加非聚集索引 (QCDistributionId, LabId, ProjectId)"(**不是唯一索引**,用户硬约束只禁了唯一索引)——对查重性能和未来数据量增长都有好处
  2. 手测剧本文档化:L3 任务 commit 应附带手测 checklist,本次未见。建议以后在 docs/impl/ 下补一份
  3. 考虑把 CurrentQCDistRegisterInfoopenFeeWindow / openEMSWindow 初始化时直接从 LabList.find(l => l.LabCode() === 输入的code) 取整行(LabCode 输入后 blur 触发),这样对话框里天然有 ProjectId 了,配合 §10.P0.1 方案 (C) 可以演进到 (A)

附:关键文件行号索引

  • sbcLabSystem.Service/QC/QCService.cs:1032-1085SaveQcDistributionRegister + MergeRegisterInfo
  • sbcLabSystem/Models/Backstage/QCDistributionInfoViewModel.cs:135-170ToEntity 第一道防线
  • sbcLabSystem/Models/Backstage/QCDistributionRegisterInfoViewModel.cs:59-84ToEntityByLabCode ProjectId 过滤(Critical 回归源头)
  • sbcLabSystem/Controllers/BackstageController.cs:215-230ConfigrmFee(受影响)
  • sbcLabSystem/Controllers/BackstageController.cs:239-256ConfigrmEMS(受影响)
  • sbcLabSystem/Controllers/BackstageController.cs:263-298ImportLabs(已修复)
  • sbcLabSystem/Controllers/BackstageController.cs:299-311switchNextOne(受影响)
  • sbcLabSystem/Controllers/BackstageController.cs:1225-1235OutputUnderTakenLabs 列清理
  • sbcLabSystem/Views/Backstage/QCDistributionLabs.cshtml:88-111openFeeWindow / openEMSWindow 字面量(Critical 回归的前端源头)
  • sbcLabSystem/Views/Backstage/QCDistributionLabs.cshtml:181-245QCDistRegisterInfo ko 构造器
  • sbcLabSystem/Views/Backstage/QCDistributionLabs.cshtml:383-450 — 收费/EMS 对话框 HTML
  • sbcLabSystem/Controllers/UserUIController.cs:259-274saveAnswerInfoMergeRegisterInfo 失效路径)
  • CLAUDE.md:113-132QCDistributionRegisterInfo 保存契约
  • sbcLabSystem.Import/Form1.cs:28-65 — 一次性迁移工具(绕开路径,不影响线上)