review(onmyoji_gacha): fix 2x timeout + Pydantic v2 model_validator + review report
This commit is contained in:
47
danding_bot/plugins/onmyoji_gacha/REVIEW_REPORT.md
Normal file
47
danding_bot/plugins/onmyoji_gacha/REVIEW_REPORT.md
Normal file
@@ -0,0 +1,47 @@
|
||||
# onmyoji_gacha 插件代码审查报告
|
||||
|
||||
**审查日期**: 2026-05-10
|
||||
**审查范围**: 7个文件,约2318行代码
|
||||
**评级**: B+ (良好,有2处已修复的高风险问题)
|
||||
|
||||
## 文件清单
|
||||
|
||||
| 文件 | 行数 | 评级 | 说明 |
|
||||
|------|------|------|------|
|
||||
| config.py | 127 | B+ | 配置类,已修复Pydantic v2兼容 |
|
||||
| utils.py | 52 | A | 简单工具函数,无问题 |
|
||||
| gacha.py | 307 | A- | 概率计算逻辑正确 |
|
||||
| data_manager.py | 594 | A | SQLite资源管理规范 |
|
||||
| api_utils.py | 228 | B | 已修复2处timeout缺失 |
|
||||
| web_api.py | 210 | B+ | Web管理API,需注意生产环境token |
|
||||
| __init__.py | 802 | B+ | 核心handler,异常保护完善 |
|
||||
|
||||
## 已修复问题
|
||||
|
||||
### 🔴 HIGH - API调用缺少timeout (api_utils.py)
|
||||
- **L54**: `requests.get(url)` → `requests.get(url, timeout=10)`
|
||||
- **L109**: `requests.post(url=url, json=data)` → `requests.post(url=url, json=data, timeout=10)`
|
||||
- **影响**: 无timeout会导致线程永久阻塞,最终耗尽线程池
|
||||
|
||||
### 🟡 MEDIUM - Pydantic v2 BaseSettings兼容 (config.py)
|
||||
- **修复**: `__init__` override → `@model_validator(mode="after")`
|
||||
- **添加**: `import logging` + 运行时默认token警告
|
||||
- **影响**: Pydantic v2下override `__init__` 可能破坏环境变量注入
|
||||
|
||||
## 审查通过项
|
||||
|
||||
- ✅ **data_manager.py**: 所有DB操作使用`with sqlite3.connect()`上下文管理器,资源管理规范
|
||||
- ✅ **gacha.py**: 概率计算逻辑正确,加权随机实现无偏差
|
||||
- ✅ **__init__.py**: 所有handler有`except Exception`保护,外部API调用失败不会导致bot无响应
|
||||
- ✅ **web_api.py**: admin auth检查存在,session管理合理
|
||||
- ✅ **utils.py**: 简单工具函数,无风险
|
||||
|
||||
## 建议改进(非阻塞)
|
||||
|
||||
1. **config.py L111**: `WEB_ADMIN_TOKEN` 默认值应在生产环境覆盖
|
||||
2. **data_manager.py**: 可考虑添加连接池(高并发场景)
|
||||
3. **gacha.py**: 概率配置可通过config.py外部化
|
||||
|
||||
## 结论
|
||||
|
||||
onmyoji_gacha插件整体代码质量良好,2处高风险timeout缺失和1处Pydantic兼容问题已修复。SQLite资源管理规范,异常保护完善。建议在生产部署前确认WEB_ADMIN_TOKEN已通过环境变量设置。
|
||||
Reference in New Issue
Block a user