5.3 KiB
5.3 KiB
代码审查清单
概述
代码审查(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 是否安全、同步/异步是否正确、响应格式是否统一。审查意见应具体可操作,帮助团队持续提升代码质量。