Commit 1559e913051be148186b01353834a2da56fd093a
1 parent
c82fd010
chore(usr): REQ-USR-001 review approve + 勾选 docs/08 REQ-USR-001
Showing
2 changed files
with
34 additions
and
1 deletions
docs/08-模块任务管理.md
docs/superpowers/reviews/2026-05-13-REQ-USR-001.md
0 → 100644
| 1 | +--- | |
| 2 | +req_id: REQ-USR-001 | |
| 3 | +date: 2026-05-13 | |
| 4 | +round: 1 | |
| 5 | +reviewer: superpower-code-reviewer | |
| 6 | +verdict: approve | |
| 7 | +--- | |
| 8 | + | |
| 9 | +# Review: REQ-USR-001 — round 1 | |
| 10 | + | |
| 11 | +## 结论 | |
| 12 | +approve | |
| 13 | + | |
| 14 | +## Must-fix | |
| 15 | +(无) | |
| 16 | + | |
| 17 | +## Nice-to-have | |
| 18 | +- `backend/src/main/java/com/xly/test4/common/exception/GlobalExceptionHandler.java:19-56` — 为 `BusinessException` / `DuplicateKeyException` 显式声明 `@ResponseStatus(HttpStatus.OK)`,避免依赖默认 200 行为 | |
| 19 | +- `backend/src/main/java/com/xly/test4/common/response/Result.java:1-27` — 后续可补 `traceId` 字段以对齐 docs/04 § 1.3 | |
| 20 | +- `backend/src/main/java/com/xly/test4/module/usr/service/impl/UserServiceImpl.java:98-109` — `tUserPermission` 循环 insert,后续若 permissionIds 体量增长可改 `saveBatch` | |
| 21 | +- `backend/src/main/java/com/xly/test4/module/usr/service/impl/UserServiceImpl.java:59-66` — 两次 `selectCount` 可合并为单查询(本 REQ 不必,spec § B-001 明确要求 Service 层预检) | |
| 22 | +- `backend/src/main/java/com/xly/test4/common/security/JwtTokenProvider.java:45-65` — `claims.get("auth", List.class)` 强转 `List<String>` 缺类型守护,可加 `instanceof` 检查 | |
| 23 | +- `backend/src/main/java/com/xly/test4/module/usr/entity/User.java:33,35,37` — 匈牙利前缀字段(i/s/t) 语义建议在 entity 顶部加注释 | |
| 24 | +- `backend/src/main/java/com/xly/test4/module/usr/converter/UserConverter.java:17-35` — JavaBeans 大写化 property 名(`IIncrement` / `SUserCode` 等)文件头注释已说明,可再强化"禁止改成小写"提示 | |
| 25 | +- `sql/migrations/V2__seed_admin_and_permissions.sql:23` — 种子 admin 密码哈希文件头补一行"生产部署后必须立刻改密"提示 | |
| 26 | +- `backend/src/test/java/com/xly/test4/module/usr/controller/UserControllerIT.java:29-32` — `@TestMethodOrder` 让 8 个测试方法共享 DB 状态,链式依赖较脆弱;下个 REQ 加更多 IT 时建议改为每测试独立 setup/teardown | |
| 27 | + | |
| 28 | +## 反例 / 测试覆盖缺口 | |
| 29 | +- **事务回滚端到端未真测**:`spec § B-005` + `§ 验收 6` 要求"permissionIds 含非法项 → tUser 也不写入(整体回滚)"。当前实现是"预检在前,实际无回滚机会",形式正确但未对 `@Transactional` 真正的回滚语义做端到端证明。建议后续补一个集成测试:让 tUser.insert 成功 → tUserPermission.insert 抛 DataIntegrityViolation → 断言 tUser 行最终不存在 | |
| 30 | +- **日志泄露断言未补**:`spec § 验收 15` "应用日志中不出现明文 password 字段值" 当前无测试覆盖。可用 LogCaptor 或 Spring TestLogger 监听 `com.xly.test4` 包做断言 | |
| 31 | +- **错误响应裸度断言不完整**:`spec § 验收 19` "错误响应不含 stacktrace/class name/SQL 文本",`GlobalExceptionHandlerTest#handleGenericException_returns50000_noStackInResponse` 只覆盖通用兜底分支;`DuplicateKeyException` 分支虽硬编码 message 安全,但可补一个 cause message 含 'SELECT * FROM tUser ...' 的样本断言 `Result.message` 不泄露 SQL | |
| 32 | +- **sId 全局 ID 策略**:spec § B-004 写"暂置 NULL",实现正确;建议 docs/03 § 种子数据 节后补一条说明,等全局 ID 生成器落地后回填 | |
| 33 | +- **多租户隔离一致性**:spec 未要求 service 校验 employeeId/permissionIds 是否同租户;当前实现允许跨租户关联。建议在 REQ-USR-002/003 spec 中显式声明多租户 filter 策略 | ... | ... |