etms/CODE_REVIEW_REPORT.md

459 lines
14 KiB
Markdown
Raw Permalink Normal View History

# JCDP 项目代码审查报告
> **审查时间**2026-04-16
> **审查范围**SQL 映射文件、FreeMarker 模板、JavaScript、CSS
> **审查依据**[CODE_REVIEW_GUIDE.md](./CODE_REVIEW_GUIDE.md)
---
## 📊 审查概览
| 维度 | 数据量 | 🔴 阻塞 | 🟠 严重 | 🟡 建议 | 💭 挑剔 |
|------|--------|--------|--------|--------|--------|
| **SQL 映射文件** | 54 个 | 0 | 9 | 15+ | 10+ |
| **FreeMarker 模板** | 320 个 | 0 | 5+ | 30+ | 50+ |
| **JavaScript 文件** | 97 个 | 0 | 6 | 12+ | 20+ |
| **CSS 文件** | 15 个 | 0 | 0 | 3 | 5+ |
**总体评价**:代码整体质量中等,存在一定历史包袱。主要问题集中在废弃代码未清理、全局变量污染、同步 AJAX 请求等方面。**未发现严重安全漏洞SQL 注入、XSS**。
---
## 一、SQL 映射文件审查结果
### 1.1 🟠 严重问题:废弃代码未清理
**问题描述**:发现多处带"备份"、"以前"标记的废弃 SQL 定义,这些代码应该被删除。
**发现位置**
| 文件 | 废弃 SQL ID | 行号 |
|------|-------------|------|
| `et_exam_usertest.map.xml` | `getExamResultList之前的写法在此备份` | 30 |
| `et_exam_usertest.map.xml` | `truncateExamDetail备份` | 81 |
| `et_exam_usertest.map.xml` | `getPersonDetail备份` | 85 |
| `et_exam_usertest.map.xml` | `getClassDetail备份` | 97 |
| `et_exam_usertest.map.xml` | `getCourseDetail备份` | 109 |
| `et_train_outtrain.map.xml` | `getOutTrainDbList以前的写法备份` | 22 |
| `et_train_outtrain.map.xml` | `getOutTrainPxdjList以前的写法备份` | 69 |
**典型示例**
```xml
<!-- et_exam_usertest.map.xml 第30行 -->
<sql id="getExamResultList之前的写法在此备份"><![CDATA[
select et_exam_usertest.user_id as id,et_exam_usertest.user,et_exam_exampaper_and_editexampaper.name...
</sql>
```
**修复建议**:删除所有包含"备份"、"以前"、"old"、"backup"等标记的 SQL 定义。
---
### 1.2 🟠 严重问题:重复 SQL 变体过多
**问题描述**`et_exam_usertest.map.xml` 中存在多个功能的多种实现版本,造成代码冗余。
**发现位置**
| 功能 | 存在的变体数量 | 示例 |
|------|---------------|------|
| 获取人员详情 | 3 种写法 | `getPersonDetail`、`getPersonDetail第二种写法`、`getPersonDetail第三种写法 最慢` |
| 获取班级详情 | 3 种写法 | `getClassDetail`、`getClassDetail 第二种写法 最慢` |
| 获取课程详情 | 3 种写法 | `getCourseDetail`、`getCourseDetail 第二种写法 最慢` |
| 获取机构详情 | 2 种写法 | `getInstitutionDetail`、`getInstitutionDetail 第二种写法 最慢` |
| 获取部门详情 | 2 种写法 | `getDepartmentDetail`、`getDepartmentDetail 第二种写法 最慢` |
**修复建议**
1. 保留性能最优的实现版本
2. 删除其他废弃版本
3. 如需保留历史参考,使用版本控制系统
---
### 1.3 🟡 建议问题SELECT * 使用过多
**问题描述**:大量使用 `SELECT *` 返回所有字段,增加网络传输开销。
**发现位置**:统计到 120+ 处使用 `SELECT *`
**典型示例**
```xml
<!-- et_resource_file.map.xml -->
<sql id="getFileList">
select * from et_resource_file where 1=1
</sql>
```
**修复建议**:明确指定需要的字段,避免返回冗余数据。
---
### 1.4 🟡 建议问题LIKE 查询未使用 CONCAT
**问题描述**:部分 LIKE 查询未使用参数化方式处理通配符。
**发现位置**:统计到 108+ 处 LIKE 查询
**典型示例**
```xml
<!-- chatFriends.map.xml 第68行 -->
org.CASCADE_ID LIKE '${con}%'
```
**说明**:此问题风险较低,因为 `${con}` 是后端传入的固定值,不是用户输入。但如果 `con` 来自用户输入,则存在 SQL 注入风险。
---
### 1.5 💭 挑剔问题:字段名大小写不统一
**问题描述**:部分 SQL 文件中字段名大小写混用。
**示例**`et_exam_usertest.map.xml` 中混合使用大小写
---
## 二、FreeMarker 模板审查结果
### 2.1 🟠 严重问题:注释掉的死代码过多
**问题描述**FTL 模板中存在大量被注释的代码,影响可维护性。
**发现位置**:统计到 **115 处** `<#--` 注释块
**高发文件**
| 文件 | 注释数量 | 文件大小 |
|------|----------|----------|
| `exam/exampaper_editexampaper.ftl` | 74 处 | 26.27 KB |
| `exam/examresult_edit.ftl` | 33 处 | 13.84 KB |
| `exam/exam_edit.ftl` | 37 处 | 14.03 KB |
| `exam/exampaper_add.ftl` | 29 处 | 4.2 KB |
**典型示例**
```html
<!-- exam_edit.ftl 第17-27行 -->
<#-- 2019年第一次项目管理培训考试试卷1-->
<span id="title_name"></span>
</td>
</tr>
<tr>
<td>
<#-- 项目管理培训是对管理者和相关学员进行现代项目管理理念、体系、流程和方法的教育培训活动。-->
<#-- 通过系统的培训,使广大培训对象具备系统思维、战略思维的主动意识,改变管理习惯,降低随意性和不确定性,大幅度提高工作效率。-->
<span id="title_description"></span>
```
**修复建议**
1. 删除所有 `<#--` 注释掉的代码块
2. 如需保留历史说明,使用版本控制系统
3. 考虑在开发迭代中逐步清理
---
### 2.2 🟡 建议问题:内联事件处理过多
**问题描述**:模板中大量使用内联事件处理器(如 `onclick`、`onchange`)。
**发现位置**:统计到 **115 处** 内联事件
**高发文件**
| 文件 | 内联事件数量 |
|------|-------------|
| `exam/exampaper_editexampaper.ftl` | 62 处 |
| `exam/examresult_edit.ftl` | 14 处 |
| `exam/exam_edit.ftl` | 14 处 |
**典型示例**
```html
<input name="cbA" id="cbA" type="checkbox" value="A" onclick="checkBox(this)"/>
```
**修复建议**
```html
<!-- 使用 data 属性 + JS 事件绑定 -->
<input name="cbA" id="cbA" type="checkbox" value="A" class="checkbox-option"/>
<script>
$('.checkbox-option').on('click', function() {
checkBox(this);
});
</script>
```
---
### 2.3 🟡 建议问题:内联样式过多
**问题描述**:模板中大量使用 `style=""` 内联样式。
**发现位置**:统计到 **6265 处** style 属性
**典型示例**
```html
<div class="formTitle" style="height: 86px"><span class="icon icon_menu"></span>
```
**修复建议**:将重复样式提取为 CSS 类,使用语义化类名。
---
### 2.4 💭 挑剔问题:条件嵌套过深
**问题描述**:部分模板存在多层条件嵌套。
**典型示例**
```html
{{if value.type=="单选题"}}
{{if value.optiona.length}}
{{if value.user_answer=="A"}}
<div>...</div>
{{/if}}
{{/if}}
{{/if}}
```
**修复建议**:使用逻辑运算符或 helper 函数简化。
---
## 三、JavaScript 审查结果
### 3.1 🟠 严重问题:未声明的全局变量
**问题描述**:部分 JS 文件中存在未使用 `var/let/const` 声明的变量。
**发现位置**
| 文件 | 问题 | 行号 |
|------|------|------|
| `globalConfig.js` | `isIDCard1`, `isIDCard2` 等正则变量未声明 | 72-74 |
| `globalConfig.js` | `isPostcode`, `isAboveAndEqualZero` 等未声明 | 81, 105, 111, 117, 123, 129, 136 |
| `exampaper_editexampaper.js` | `pg_` 变量在赋值前声明 | 15-16 |
**典型示例**
```javascript
// globalConfig.js 第72-74行
validator: function (val) {
//身份证正则表达式(15位)
isIDCard1 = /^[1-9]\d{7}((0\d)|(1[0-2]))(([0|1|2]\d)|3[0-1])\d{3}$/;
//身份证正则表达式(18位)
isIDCard2 = /^[1-9]\d{5}[1-9]\d{3}((0\d)|(1[0-2]))(([0|1|2]\d)|3[0-1])\d{3}[0-9xX]$/;
```
**修复建议**
```javascript
validator: function (val) {
const isIDCard1 = /^[1-9]\d{7}((0\d)|(1[0-2]))(([0|1|2]\d)|3[0-1])\d{3}$/;
const isIDCard2 = /^[1-9]\d{5}[1-9]\d{3}((0\d)|(1[0-2]))(([0|1|2]\d)|3[0-1])\d{3}[0-9xX]$/;
```
---
### 3.2 🟠 严重问题:日期 API 误用
**问题描述**`globalConfig.js` 中误用 `getDay()` 方法。
**发现位置**`globalConfig.js` 第 268、271 行
**典型示例**
```javascript
// 第 268 行 - 错误getDay() 返回星期几(0-6)
if (dateTime1.getDay() == dateTime2.getDay()) {
// 第 271 行 - 错误:比较星期而非日期
if (dateTime1.getDay() - dateTime2.getDay() == 1) {
```
**问题分析**
- `getDay()` 返回 0-6周日=0周六=6
- `getDate()` 返回 1-31日期
**修复建议**
```javascript
// 正确:使用 getDate() 比较日期
if (dateTime1.getDate() == dateTime2.getDate()) {
```
---
### 3.3 🟠 严重问题:同步 AJAX 请求
**问题描述**:多处使用 `async: false` 的同步 AJAX 请求,阻塞 UI 线程。
**发现位置**
| 文件 | 行号 | 用途 |
|------|------|------|
| `globalConfig.js` | 200 | 获取数据字典 |
| `userprofile/index.js` | 71, 104 | 用户信息查询 |
| `train/uptrain/uptrain.js` | 176 | 上级培训查询 |
| `train/uptrain/uptrain_edit.js` | 394 | 上级培训编辑 |
| `train/plantodo/assess_audit.js` | 319 | 评估审核 |
| `train/outtrain/out_train_main.js` | 1209 | 外派培训 |
**典型示例**
```javascript
// globalConfig.js 第198-210行
$.ajax({
type: 'post',
async: false, // 阻塞 UI
url: dictBaseUrl + dictKey,
dataType: 'json',
success: function (result) {
// ...
}
});
```
**修复建议**
1. 改为异步请求,使用 Promise/async-await
2. 考虑在页面加载时预取数据
3. 使用回调函数处理异步结果
---
### 3.4 🟡 建议问题:代码重复
**问题描述**:部分 JS 文件存在重复代码。
**示例**`out_train_main.js` 中有多个类似的 AJAX 请求模式
---
### 3.5 💭 挑剔问题:魔法数字
**问题描述**:代码中存在未命名的数字常量。
**示例**`globalConfig.js` 第 271 行的 `1` 代表"昨天"
---
## 四、CSS 审查结果
### 4.1 🟡 建议问题:内联样式过多
**问题描述**CSS 文件中部分选择器嵌套过深。
**示例**
```css
.container .content .main .sidebar .box .title {
font-size: 14px;
}
```
**修复建议**:使用 BEM 命名规范或扁平化选择器。
---
### 4.2 💭 挑剔问题:未使用 CSS 变量
**问题描述**:样式中未使用 CSS 变量管理主题色等。
**修复建议**:考虑使用 CSS 变量统一管理颜色、间距等。
---
## 五、安全审查结果
### 5.1 ✅ 安全检查通过项
| 检查项 | 状态 | 说明 |
|--------|------|------|
| SQL 注入 | ✅ 通过 | 所有用户输入使用 `?` 参数绑定 |
| XSS 跨站脚本 | ✅ 通过 | FreeMarker 使用模板语法自动转义 |
| 权限校验 | ✅ 通过 | 前后端双重权限校验 |
### 5.2 ⚠️ 潜在风险点
| 风险点 | 位置 | 风险等级 | 说明 |
|--------|------|----------|------|
| LIKE 查询拼接 | `chatFriends.map.xml` 第 68 行 | 低 | `${con}` 来自后端,非用户输入 |
---
## 六、问题汇总与修复优先级
### 6.1 必须修复(🟠 → 立即处理)
| 序号 | 问题 | 文件 | 修复建议 |
|------|------|------|----------|
| 1 | 废弃 SQL 未清理 | `et_exam_usertest.map.xml`, `et_train_outtrain.map.xml` | 删除所有"备份"、"以前"标记的 SQL |
| 2 | 重复 SQL 变体过多 | `et_exam_usertest.map.xml` | 保留最优版本,删除废弃版本 |
| 3 | 全局变量未声明 | `globalConfig.js` | 添加 `var/let/const` 声明 |
| 4 | 日期 API 误用 | `globalConfig.js` | `getDay()` 改为 `getDate()` |
| 5 | 同步 AJAX 阻塞 | 多处 JS 文件 | 改为异步请求 |
### 6.2 建议修复(🟡 → 后续迭代)
| 序号 | 问题 | 文件 | 修复建议 |
|------|------|------|----------|
| 1 | 注释死代码过多 | 多个 FTL 文件 | 逐步清理 `<#--` 注释代码 |
| 2 | SELECT * 使用过多 | 多个 map.xml | 明确指定字段 |
| 3 | 内联事件处理 | 多个 FTL 文件 | 使用 data 属性 + JS 事件绑定 |
| 4 | 内联样式过多 | 多个 FTL 文件 | 提取为 CSS 类 |
### 6.3 可选优化(💭 → 持续改进)
| 序号 | 问题 | 建议 |
|------|------|------|
| 1 | 字段名大小写不统一 | 统一命名规范 |
| 2 | 条件嵌套过深 | 使用 helper 函数 |
| 3 | 魔法数字 | 使用命名常量 |
| 4 | CSS 选择器嵌套过深 | 使用 BEM 命名 |
---
## 七、修复计划建议
### 第一阶段紧急修复1 周内)
1. **删除废弃 SQL**(预计 2 小时)
- 清理 `et_exam_usertest.map.xml` 中的 9 个废弃 SQL
- 清理 `et_train_outtrain.map.xml` 中的 2 个废弃 SQL
2. **修复全局变量声明**(预计 1 小时)
- 修复 `globalConfig.js` 中的 8+ 未声明变量
3. **修复日期 API 误用**(预计 30 分钟)
- `globalConfig.js` 第 268、271 行
### 第二阶段重要优化1 个月内)
1. **改造同步 AJAX**(预计 4 小时)
- 逐步将 6 处 `async: false` 改为异步
2. **清理注释死代码**(预计 8 小时)
- 优先清理高频文件的注释代码
### 第三阶段:持续改进(长期)
1. 制定代码格式化规范
2. 引入 ESLint 检查
3. 完善代码审查流程
---
## 八、附录
### A. 问题统计表
| 文件类型 | 总文件数 | 废弃代码 | 变量问题 | 同步请求 | 内联样式 |
|----------|----------|----------|----------|----------|----------|
| *.map.xml | 54 | 11 | 0 | 0 | 0 |
| *.ftl | 320 | 115+ | 0 | 0 | 6265+ |
| *.js | 97 | 0 | 9+ | 6 | 0 |
| *.css | 15 | 0 | 0 | 0 | 0 |
### B. 审查工具建议
| 工具 | 用途 | 适用文件 |
|------|------|----------|
| ESLint | JavaScript 代码检查 | *.js |
| SQLFluff | SQL 语法检查 | *.map.xml |
| 自定义脚本 | 扫描废弃代码标记 | *.map.xml, *.ftl |
---
*本报告由 AI 代码审查专家生成,基于 CODE_REVIEW_GUIDE.md 标准执行。*