优化plugin/datasource模块下AbstractMapper类的insert、update方法。使得方法的逻辑更加清晰，提升程序性能 #13493
Conversation
…，使得代码简洁清晰，并添加相关测试用例
…eature/optimize-code-js # Conflicts: # plugin/datasource/src/main/java/com/alibaba/nacos/plugin/datasource/mapper/AbstractMapper.java # plugin/datasource/src/test/java/com/alibaba/nacos/plugin/datasource/mapper/AbstractMapperTest.java
…eature/optimize-code-js
…eature/optimize-code-js
…ate方法进行重构，使得逻辑更清晰，避免嵌套循环和冗余判断条件
|
Thanks for your this PR. 🙏 感谢您提交的PR。 🙏 |
优化AbstractMapper类的insert/update方法并新增测试用例变更文件
时序图sequenceDiagram
participant AM as AbstractMapper
participant SJ as StringJoiner
participant SB as StringBuilder
AM->>SJ: create StringJoiner for columns
AM->>SJ: add column parts[0]
AM->>SJ: create value Joiner with getFunction(parts[1])
SJ-->>AM: generate column/value strings
AM->>SB: build final SQL with table name
AM->>AM: appendWhereClause处理where条件
note right of AM: 新增@符号解析逻辑<br>动态处理函数表达式
💡 小贴士与 lingma-agents 交流的方式📜 直接回复评论
📜 在代码行处标记
📜 在讨论中提问
|
There was a problem hiding this comment.
🔍 代码评审报告
🎯 评审意见概览
| 严重度 | 数量 | 说明 |
|---|---|---|
| 🔴 Blocker | 0 | 阻断性问题，需立即修复。例如：系统崩溃、关键功能不可用或严重安全漏洞。 |
| 🟠 Critical | 0 | 严重问题，高优先级修复。例如：核心功能异常或性能瓶颈影响用户体验。 |
| 🟡 Major | 0 | 主要问题，建议修复。例如：非核心功能缺陷或代码维护性较差。 |
| 🟢 Minor | 1 | 次要问题，酌情优化。例如：代码格式不规范或注释缺失。 |
总计： 1 个问题
📋 评审意见详情
💡 单文件建议
以下是文件级别的代码建议，聚焦于代码的可读性、可维护性和潜在问题。
☕ plugin/datasource/src/main/java/com/alibaba/nacos/plugin/datasource/mapper/AbstractMapper.java (1 💬)
- 将硬编码的分隔符'@'替换为常量定义，提升代码可维护性 (L56-L68)
🚀 跨文件建议
以下是对代码架构和设计的综合分析，聚焦于跨文件交互、系统一致性和潜在优化空间。
🔍 1. 重复代码导致的可维护性风险
insert和update方法中对列名@符号的分割逻辑存在重复代码。两个方法均使用String[] parts = col.split("@", 2)处理列定义，但未提取公共方法。这种重复会导致未来维护时需要同步修改两处代码，增加出错风险。建议将该逻辑封装为protected方法，供两个方法调用。
📌 关键代码：
String[] parts = col.split("@", 2); // insert方法String[] parts = col.split("@", 2); // update方法🔍 2. 潜在SQL注入风险
getFunction(parts[1])的返回值直接拼接到SQL字符串中，若columns参数来自外部输入且未经过滤，可能存在SQL注入风险。需要确认columns参数来源是否为可信数据，或对parts[1]进行参数化处理/白名单校验。
📌 关键代码：
valueJoiner.add(parts.length > 1 ? getFunction(parts[1]) : "?");🔍 3. 测试覆盖不完整
新增测试用例未覆盖@符号缺失或多余分割的情况。例如：当列定义中没有@符号时，或包含多个@符号时（如"col@a@b"），需要验证是否符合预期行为。当前测试仅验证了标准用例。
📌 关键代码：
testInsertContainsAt 测试仅验证标准用例🔍 4. 架构一致性问题
新增的StringJoiner用法与项目其他SQL生成代码存在模式差异。需要确认项目中是否统一使用StringJoiner，或是否存在其他SQL构建工具（如MyBatis）未被考虑。
📌 关键代码：
StringJoiner columnJoiner = new StringJoiner(...);💡 小贴士
与 lingma-agents 交流的方式
📜 直接回复评论
直接回复本条评论，lingma-agents 将自动处理您的请求。例如：
-
在当前代码中添加详细的注释说明。
-
请详细介绍一下你说的 LRU 改造方案，并使用伪代码加以说明。
📜 在代码行处标记
在文件的特定位置创建评论并 @lingma-agents。例如：
-
@Lingma-Agent 分析这个方法的性能瓶颈并提供优化建议。
-
@Lingma-Agent 对这个方法生成优化代码。
📜 在讨论中提问
在任何讨论中 @lingma-agents 来获取帮助。例如：
-
@Lingma-Agent 请总结上述讨论并提出解决方案。
-
@Lingma-Agent 请根据讨论内容生成优化代码。
| String[] parts = col.split("@", 2); // 最多分割成两部分 | ||
| columnJoiner.add(parts[0]); | ||
| valueJoiner.add(parts.length > 1 ? getFunction(parts[1]) : "?"); | ||
| } | ||
| sql.append(")"); | ||
| return sql.toString(); | ||
|
|
||
| return "INSERT INTO " + getTableName() + columnJoiner + " VALUES" + valueJoiner; | ||
| } | ||
|
|
||
| @Override | ||
| public String update(List<String> columns, List<String> where) { | ||
| StringBuilder sql = new StringBuilder(); | ||
| String method = "UPDATE "; | ||
| sql.append(method); | ||
| sql.append(getTableName()).append(" ").append("SET "); | ||
|
|
||
| for (int i = 0; i < columns.size(); i++) { | ||
| String[] parts = columns.get(i).split("@"); | ||
| String column = parts[0]; | ||
| if (parts.length == 2) { | ||
| sql.append(column).append(" = ").append(getFunction(parts[1])); | ||
| } else { | ||
| sql.append(column).append(" = ").append("?"); | ||
| } | ||
| if (i != columns.size() - 1) { | ||
| sql.append(","); | ||
| } | ||
| StringJoiner setJoiner = new StringJoiner(","); | ||
| for (String col : columns) { | ||
| String[] parts = col.split("@", 2); |
There was a problem hiding this comment.
将硬编码的分隔符'@'替换为常量定义，提升代码可维护性
🟢 Minor | 🧹 Code Smells
📋 问题详情
代码中直接使用字符串'@'作为分隔符，未定义为常量。硬编码的魔法字符串会影响代码可维护性和可读性，当需要修改分隔符时需全局搜索替换，存在维护风险。建议将分隔符定义为类级别常量以提高规范性。
💡 解决方案
将分隔符定义为类常量：
+ private static final String COLUMN_SEPARATOR = "@";
- String[] parts = col.split("@", 2);
+ String[] parts = col.split(COLUMN_SEPARATOR, 2);您的反馈对我们很重要！(建议右键在新标签页中打开以下链接)
|
@20144101037 CI没有通过 |
重新做了提交，麻烦您再看下 |
|
|
@KomachiSion 看了CI报错原因，不是这一次提交代码导致的问题 |
同步一下develop分支的代码再试试 |
…eature/optimize-code-js
好的，已同步最新develop分支代码，并push到个人分支 |
|
|
Please do not create a Pull Request without creating an issue first.
What is the purpose of the change
XXXXX
Brief changelog
XX
Verifying this change
XXXX
Follow this checklist to help us incorporate your contribution quickly and easily:
[ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.mvn -B clean package apache-rat:check findbugs:findbugs -Dmaven.test.skip=trueto make sure basic checks pass. Runmvn clean install -DskipITsto make sure unit-test pass. Runmvn clean test-compile failsafe:integration-testto make sure integration-test pass.