docs(usr): review approved + check off REQ-USR-001

zichun
1 parent 33adcfb8
Showing 2 changed files with 40 additions and 1 deletions
docs/08-模块任务管理.md
docs/superpowers/reviews/2026-04-30-REQ-USR-001.md
@@ -70,7 +70,7 @@
   - 路径: backend/module/usr/, frontend/pages/usr/
   - MR: —
   - 功能:
-    - [ ] REQ-USR-001 用户新增
+    - [x] REQ-USR-001 用户新增
     - [ ] REQ-USR-002 用户修改
     - [ ] REQ-USR-003 用户查询
     - [ ] REQ-USR-004 用户登录
+---
+req_id: REQ-USR-001
+date: 2026-04-30
+round: 1
+reviewer: superpower-code-reviewer
+---
+
+# Review: REQ-USR-001 — round 1
+
+## 结论
+approve
+
+## Must-fix
+（无）
+
+## Nice-to-have
+
+- docs/superpowers/specs/2026-04-30-REQ-USR-001.md § 实现范围与边界抉择 #3 + § 验收标准 工程验收 #2 写「BCryptPasswordEncoder bean 在 SecurityConfig 注册」，但实现已移到 `common/config/PasswordEncoderConfig.java`（理由：SecurityConfig 上 `@ConditionalOnWebApplication(SERVLET)` 在 `webEnvironment=NONE` 的 mapperIT/serviceTest 上下文不加载会让 encoder 缺失）。**正向设计修正**——密码编码器是领域基础设施而非 Web 安全基础设施。建议把 spec 这两处改为「BCryptPasswordEncoder 在 common/config/PasswordEncoderConfig 注册」，并在 PasswordEncoderConfig.java 类注释里写一行可追溯说明。
+- backend/src/main/java/com/xly/erp/module/usr/service/impl/UserServiceImpl.java:104 — 循环里每次 `LocalDateTime.now()` 让 N 行 tUserPermission 的 `tCreateDate` 出现微秒级偏差。建议在 create 入口取一次 `LocalDateTime now`，user + 全部 permission 共享，更贴合「同一事务一次创建」的语义。
+- backend/src/main/java/com/xly/erp/module/usr/service/impl/UserServiceImpl.java:69-75 — `permissionCategoryIds` 含重复 id（如 `[1,1,2]` 全部有效）时 SQL IN 隐式去重 → `countActiveByIds=2 ≠ ids.size=3` → 误抛 40023 假阴性。spec 没要求去重；建议 service 层先 `ids = ids.stream().distinct().toList()`，4 行修复。
+- backend/src/test/java/com/xly/erp/module/usr/controller/UserControllerIT.java:73-75 — `postValidBody_with_jwt_returns200_andPersists` 没断言响应 `data` 中不含 `sPasswordHash`。spec § 验收 工程 #5 明言「不返回 sPasswordHash」。建议加 `assertThat(jb.get("data").has("sPasswordHash")).isFalse()` 锁住该不变量，未来若有人改 Map.of 加字段不会无声泄漏。
+- backend/src/test/java/com/xly/erp/module/usr/controller/UserControllerIT.java:170-198 — `postWithoutJwt_permitAllStub_returns200_andCreatedBySTUBADMIN` / `postTamperedJwt_returns20001` 沿袭 MOD 模块 stub 路径仍缺 `// REQ-MOD-001 stub: see USR-004 follow-up` 锚点。建议在 module-report 阶段统一一次性补齐（与 MOD-004 review 同处理）。
+- backend/src/main/java/com/xly/erp/common/security/SecurityConfig.java:26 — stub 锚点放在 `/api/mod/**` 与 `/api/usr/**` 共同上方。USR-004 实际收紧时需要分两条 requestMatchers 各自指向 hasAuthority/anyRequest，建议在 USR-004 plan 阶段显式记一笔回填动作。
+- backend/src/main/java/com/xly/erp/module/usr/service/impl/UserServiceImpl.java:59 — `create` 方法体缺 `// REQ-USR-001: 用户新增` 行内锚点（与 MOD 模块同源遗漏）。建议在 module-report 阶段一次性把 USR / MOD 两个模块的 REQ 行内锚点统一补齐。
+
+## 反例 / 测试覆盖缺口
+
+Spec 验收清单（service 8 + mapperIT 4 + controllerIT 9 + 工程 5）100% 落地，sourcing `.env.local` 后 `mvn -B test` 全量 89/89 全绿。
+
+- BCryptPasswordEncoder 重定位是必要正向修正，被 mapperIT × 5（NONE 环境）+ serviceTest × 8 + controllerIT × 9（RANDOM_PORT 环境）双向覆盖。
+- 校验顺序（type/lang 枚举 → staff 存在 → permission 存在 → INSERT user catch DuplicateKey → INSERT permission × N）严格匹配 spec § 业务规则 1–6，`@Transactional(rollbackFor=Exception.class)` 包整个流程。
+- `permissionCategoryMapper.countActiveByIds` 由 service 短路空 list 保护，避免 SQL `IN ()` 错误。
+- N+1 INSERT tUserPermission 是 spec § 业务规则 #7 显式 YAGNI 取舍；docs/04 § 3.4「循环中禁止执行 DB 查询」语义针对 SELECT N+1 不是 INSERT。
+- `sPasswordHash` 由 service 返回 Map 仅 put 两个 key 不会泄漏（构造保证），但缺直接断言锁不变量。
+- BCrypt 每次 salt 不同——单测用真实 `BCryptPasswordEncoder` + `startsWith("$2a$")` 断言（非 mock），正确。
+- IT cleanup 顺序 tUserPermission → tUser → tStaff → tPermissionCategory 满足 FK（iUserId CASCADE / iStaffId SET NULL / iCategoryId RESTRICT），无外键孤儿。
+- SecurityContextHelper 匿名处理与 MOD 同款，stub 锚点缺失沿袭 MOD-004 已点出的范畴。
+
+非阻塞缺口：spec 文字与实现 BCryptPasswordEncoder 位置不一致（见 nice-to-have #1）；`sPasswordHash` regression 锁断言（#4）；permissionCategoryIds 重复值假阴性（#3）；行内 REQ 锚点 + stub 锚点统一补齐（留 module-report）。