110 lines
5.3 KiB
Markdown
110 lines
5.3 KiB
Markdown
# 代码审查清单
|
||
|
||
## 概述
|
||
|
||
代码审查（Code Review）是保障代码质量的重要环节。本文提供标准化的审查清单，覆盖安全、性能、规范、边界等维度，帮助审查者系统性地检查代码质量。
|
||
|
||
:::tip 审查原则
|
||
- 每次提交必须经过至少一人审查
|
||
- 审查关注点：正确性 > 安全性 > 可维护性 > 性能
|
||
- 审查意见应具体、可操作，避免主观评价
|
||
:::
|
||
|
||
## 安全检查
|
||
|
||
- [ ] **DEBUG 关闭**：生产配置 `TORNADO_DEBUG=False`
|
||
- [ ] **JWT 安全**：`JWT_SECRET` 使用强密码，未硬编码在代码中
|
||
- [ ] **CORS 配置**：未使用 `allow_origins=["*"]`，仅列出信任域名
|
||
- [ ] **SQL 注入**：使用 ORM 参数化查询，禁止字符串拼接 SQL
|
||
- [ ] **权限校验**：写操作接口有 `@permission_required` 和 `@check_demo` 装饰器
|
||
- [ ] **敏感信息**：日志中不记录密码、token、密钥等敏感信息
|
||
- [ ] **文件上传**：有文件类型白名单验证，文件名已重命名
|
||
- [ ] **输入校验**：所有用户输入通过 Pydantic Schema 校验
|
||
- [ ] **.env 安全**：`.env` 文件未提交到 Git（在 `.gitignore` 中）
|
||
- [ ] **密码存储**：使用 bcrypt 哈希存储，禁止明文
|
||
|
||
## 性能检查
|
||
|
||
- [ ] **N+1 查询**：关联查询使用 joinedload/subqueryload，避免循环查询
|
||
- [ ] **索引覆盖**：WHERE/ORDER BY 字段已添加索引
|
||
- [ ] **分页查询**：大列表接口使用分页，禁止全表查询返回
|
||
- [ ] **SELECT ***：只查询需要的字段，避免 `SELECT *`
|
||
- [ ] **缓存使用**：热点数据使用 Redis 缓存
|
||
- [ ] **同步/异步**：`async def` 端点中未直接调用同步 DB 查询
|
||
- [ ] **批量操作**：批量插入/更新使用 `executemany`，非逐条执行
|
||
- [ ] **连接池**：数据库连接池大小合理，不超过数据库 max_connections
|
||
|
||
## 代码规范检查
|
||
|
||
- [ ] **装饰器顺序**：`@router` → `@permission_required` → `@check_demo`
|
||
- [ ] **命名规范**：文件名 snake_case，类名 PascalCase，函数名 snake_case
|
||
- [ ] **注释规范**：关键逻辑有注释说明，docstring 描述函数用途
|
||
- [ ] **导入规范**：导入分组（标准库 → 第三方 → 本地模块），无未使用的导入
|
||
- [ ] **R 封装**：所有接口通过 `R.ok()`/`R.failed()` 返回，禁止直接返回 dict
|
||
- [ ] **删除规范**：使用 `delete` 方法，禁止直接调用 `repo.batch_delete`
|
||
- [ ] **Repository 单例**：模块级单例模式（`xxx_repo = XxxRepository(Model)`）
|
||
- [ ] **Service 单例**：模块级单例模式（`xxx_service = XxxService()`）
|
||
|
||
## 边界条件检查
|
||
|
||
- [ ] **空值处理**：可选字段支持 None 值，不抛异常
|
||
- [ ] **空列表**：批量操作传入空列表时安全返回
|
||
- [ ] **超大输入**：字符串长度限制合理（`max_length`）
|
||
- [ ] **数值范围**：数值字段有 `ge`/`le`/`gt`/`lt` 约束
|
||
- [ ] **ID 有效性**：ID 参数校验 > 0，兼容字符串 ID
|
||
- [ ] **重复操作**：重复添加/删除同一数据的幂等性
|
||
- [ ] **并发安全**：并发修改同一记录的处理（乐观锁/唯一性校验）
|
||
- [ ] **软删除过滤**：查询方法自动过滤 `is_delete = 0`
|
||
|
||
## 数据库检查
|
||
|
||
- [ ] **模型定义**：继承 `base_model` 和 `BaseModel`
|
||
- [ ] **表名前缀**：`__tablename__ = DB_PREFIX + "表名"`
|
||
- [ ] **字段注释**：`comment` 参数描述字段用途
|
||
- [ ] **索引定义**：常用查询字段有索引
|
||
- [ ] **默认值**：`server_default` 与数据库默认值一致
|
||
- [ ] **软删除**：不使用物理删除，统一软删除
|
||
|
||
## API 接口检查
|
||
|
||
- [ ] **HTTP 方法**：查询用 GET，新增用 POST，编辑用 PUT，删除用 DELETE
|
||
- [ ] **权限标识**：格式 `sys:{module}:{action}`
|
||
- [ ] **请求参数**：Pydantic Schema 定义完整约束
|
||
- [ ] **响应格式**：统一使用 `R.ok()`/`R.failed()`/`R.page()`
|
||
- [ ] **错误提示**：错误消息清晰、用户友好
|
||
- [ ] **路由注册**：新模块已在 `api/v1/router.py` 中注册
|
||
|
||
## 前端检查
|
||
|
||
- [ ] **API 函数命名**：`getXxxPage`/`addXxx`/`updateXxx`/`deleteXxx`
|
||
- [ ] **错误处理**：响应拦截器统一处理错误
|
||
- [ ] **loading 状态**：数据加载时显示 loading
|
||
- [ ] **确认弹窗**：删除操作有二次确认
|
||
- [ ] **表单校验**：提交前前端校验必填项
|
||
|
||
## 提交检查
|
||
|
||
- [ ] **提交信息**：遵循 Conventional Commits 格式
|
||
- [ ] **变更范围**：每次提交只做一件事
|
||
- [ ] **测试覆盖**：新功能/修复有对应测试
|
||
- [ ] **文档更新**：接口变更同步更新文档
|
||
|
||
## 审查意见模板
|
||
|
||
```
|
||
[严重] 安全问题：SQL 拼接存在注入风险，应使用 ORM 参数化查询
|
||
[建议] 性能优化：此处可用 joinedload 避免 N+1 查询
|
||
[疑问] 这里的 try-except 吞掉了异常，是否需要记录日志？
|
||
[肯定] 使用 delete 统一封装删除响应，符合规范
|
||
```
|
||
|
||
:::tip 审查态度
|
||
- 区分「必须修改」和「建议优化」
|
||
- 给出具体的修改方案，而非只指出问题
|
||
- 对好的实践给予肯定
|
||
:::
|
||
|
||
## 总结
|
||
|
||
代码审查清单覆盖安全、性能、规范、边界、数据库、API、前端、提交等 8 个维度。审查时按清单逐项检查，区分严重问题和建议优化。核心关注点：权限校验是否完整、SQL 是否安全、同步/异步是否正确、响应格式是否统一。审查意见应具体可操作，帮助团队持续提升代码质量。
|