fix: 修复配置转换、SQL 方言与 JEXL 配置失效#711
Conversation
保留声明集合语义并按数据库方言生成合法 SQL,同时确保 JEXL 配置变更后重建引擎。
FxRayHughes
left a comment
There was a problem hiding this comment.
Code Review — #711 fix: 修复配置转换、SQL 方言与 JEXL 配置失效
结论先说:三块改动的方向都对,而且各自修的都是真 bug——ObjectConverter 的 Map 字段确实拿不到转换后的嵌套对象,ActionInsert 在 SQLite/PostgreSQL 上确实一直生成非法 SQL,JexlCompiler 的配置确实在首次编译后就永久失效。
没有发现阻塞级问题。但有一处修得不彻底(SQLite 版本门槛),一处行为变更未进兼容性说明(列表元素现在会走 unwrap),以及与 #710 的合并顺序需要确认。
审阅方式:读 patch + 对照仓库源码验证 + 用本机 sqlite3 实测语法。未实跑 gradle 测试,其余结论均基于源码推导,已注明依据位置。
🟡 问题 1 — SQLite 省略冲突目标的写法要求 SQLite ≥ 3.35.0,而 TabooLib 不控制这个版本
事实基础
addDuplicateUpdate 的 SQLITE 分支在 conflictKeys 为空时生成:
INSERT INTO `order` (`key`, `value`) VALUES (?, ?) ON CONFLICT DO UPDATE SET `value` = ?测试 uses sqlite conflict syntax without guessing a conflict target 把这个形态固化了下来,注释也写明「without guessing a conflict target」——意图是好的,不猜就不会猜错。
我用本机 sqlite3 3.51.0 实测,这条语句确实可用:
$ sqlite3 t.db "CREATE TABLE entries (`key` TEXT PRIMARY KEY, `value` INTEGER);
INSERT INTO `entries` (`key`,`value`) VALUES ('entry',1) ON CONFLICT DO UPDATE SET `value`=2;
INSERT INTO `entries` (`key`,`value`) VALUES ('entry',1) ON CONFLICT DO UPDATE SET `value`=2;
SELECT * FROM entries;"
entry|2
但**「DO UPDATE 可以省略冲突目标」是 SQLite 3.35.0(2021-03)才放宽的**。官方 UPSERT 文档的原话是「The conflict target is required for DO UPDATE upserts, but is optional for DO NOTHING」,3.35.5 release log 里则记着「The final ON CONFLICT clause may omit the conflict target and yet still use DO UPDATE」。3.35.0 之前这条语句是语法错误。
为什么这在 TabooLib 里是个实际风险
module/database/src/main/kotlin/taboolib/module/database/Database.kt:13-30 的 @RuntimeDependencies 只声明了 slf4j、HikariCP、mysql-connector-j,没有 sqlite-jdbc。HostSQLite.driverClass 是 "org.sqlite.JDBC",也就是说 SQLite 驱动完全依赖服务端/其他插件提供,版本不受 TabooLib 控制。测试用的是 testImplementation("org.xerial:sqlite-jdbc:3.42.0.0")(module/database/build.gradle.kts:11),这个版本自带的 SQLite 远高于 3.35,所以测试必过——但测出来的不是生产环境的版本。
需要说明的是,这不是回退:改之前 SQLite 走 ON DUPLICATE KEY UPDATE,在任何版本上都是语法错误。改之后至少在 ≥3.35 上能用。问题只是修得不够彻底。
建议
conflictKeys 已经传进来时,SQLite 分支就把它写出来(现在的代码已经这么做了,targetKeys.isNotEmpty() 时会 addKeys)——带冲突目标的 UPSERT 只需要 SQLite ≥ 3.24.0(2018-06),兼容面宽得多。所以真正的问题只在「调用方没传 conflictKeys」这一条路径上。两个选择:
- 像 PostgreSQL 一样要求 SQLite 也显式传
conflictKeys,把版本门槛从 3.35 降到 3.24。代价是 API 上多一个必填参数,但错误会在开发期暴露而不是在用户服务器上。 - 保留现状,但在
onDuplicateKeyUpdate的 KDoc 里写明「SQLite 省略冲突字段时要求 SQLite ≥ 3.35.0」,并在兼容性说明里提一句。
我倾向 1,理由是这个失败发生在用户的服务器上而不是开发机上,而且报错是裸的 SQLSyntaxError,很难让人联想到 SQLite 版本。
参考:SQLite UPSERT 文档、3.35.5 release log
🟡 问题 2 — 集合元素现在会走 ConfigSection.unwrap,"null" / "~" 会变成 null
事实基础
旧代码在「元素类型已兼容」时走的是直接赋值,元素原样进字段:
if (srcBottomType == null || dstBottomType == null || dstBottomType.isAssignableFrom(srcBottomType)) {
AnnotationUtils.checkField(field, value);
field.set(object, value); // ← 整个集合原样塞进去,元素不做任何处理
}新代码所有元素都过 convertValue,其中标量分支是:
Object unwrapped = ConfigSection.Companion.unwrap(value);而 ConfigSection.kt:323-338 的 unwrap 做了这些事:
fun unwrap(v: Any?): Any? {
return when (v) {
"~", "null" -> null // ← 字符串 "null" 变成真 null
"''", "\"\"" -> "" // ← 字符串 "''" 变成空串
else -> when (v) {
is String -> v.decodeUnicode() // ← 字符串会做 unicode 解码
...
}
}
}后果
配置里 List<String> 含字面量 "null"、"~"、"''" 的,升级后元素值会变。更值得注意的是 List<String>(Kotlin 非空泛型)现在可以装进 null——convertValue 开头就是 if (value == null) return null,Kotlin 的非空泛型参数在运行时不做元素校验,null 会一路进到集合里,直到某处解引用才炸,且栈顶和配置无关。
decodeUnicode() 同理:List<String> 里的 你 现在会被解码。
这个改动是否合理,我倾向认为「合理但需要写出来」
ConfigSection.kt:174 的 getStringList 本来就是 (get(path) as? List<*>)?.map { unwrap(it) }——也就是说走 getStringList 的路径一直在 unwrap,只有 ObjectConverter 这条路径没有。本 PR 让两者一致了,方向是对的。
但 PR 的兼容性说明只写了「反序列化结果会更准确地遵循字段泛型声明」,读起来像是纯类型层面的改进,看不出标量值本身会变。建议在兼容性说明里明确列出:集合/映射元素现在与 getStringList 行为一致,会做 ~/null/'' 归一化与 unicode 解码。
🟡 问题 3 — 集合/映射字段不再与配置共享实例,且不再复用字段已有值
两处语义变化,都没在说明里体现:
a. 实例不再共享。 旧代码「简单列表」走 field.set(object, value),字段拿到的就是配置内部那个集合对象;新代码 convertCollection 永远 createCollection 出新实例再逐元素填。依赖「改字段等于改配置」的代码会静默失效。这类写法本来就脆弱,但确实是行为变更。
b. 不再复用字段已有值。 旧代码在需要转换时会先取 field.get(object),非 null 就往里 add:
Collection<Object> dst = (Collection<Object>) field.get(object);
if (dst == null) { ... field.set(object, dst); }
convertConfigsToObject(src, dst, dstTypes, 0); // ← 往已有集合里追加字段有默认值时(Kotlin var list: List<X> = listOf(a, b) 这种)旧代码是追加,新代码是整体替换。替换才是符合直觉的语义,我认为新行为更对——旧行为在同一对象被 toObject 两次时还会翻倍——但同样属于需要写明的变更。
🟡 问题 4 — convertValue 不查 ConverterRegistry,List<UUID> 仍然失败
ConverterRegistry.kt:16-20 预注册了两个全局转换器:
register(Map::class.java, MapConverter())
register(UUID::class.java, UUIDConverter())字段级转换在 ObjectConverter.java:300-309 会查这个注册表,所以 var id: UUID 能从字符串正确恢复。但新的 convertValue 处理元素时完全没有查注册表,于是 List<UUID> / Map<String, UUID> 走到最后一步:
throw new InvalidValueException("Unexpected element of type " + unwrapped.getClass() + " for " + declaredType);因为 UUID 通过 isStructuredObjectType 检查(不是 String/Boolean/Character/Number/Enum/Collection/Map),但值是 String 而非 UnmodifiableConfig/Map,进不去结构化对象分支;再往下 UUID.isAssignableFrom(String) 为 false、不是 enum、不是 Number、declaredClass != String.class,直接抛异常。
这不是回退——旧代码同样会抛(convertConfigsToObject 对非 Collection、非 UnmodifiableConfig 的元素也是 throw new InvalidValueException)。但本 PR 的目标正是「按声明泛型递归恢复」,而 convertValue 是全新的、唯一的元素转换入口,在这里补一行注册表查询是很自然的收尾:
Converter<Object, Object> registryConverter = ConverterRegistry.INSTANCE.getConverter(declaredClass);
if (registryConverter != null) {
return registryConverter.convertToField(unwrapped);
}考虑到 UUIDConverter 是模块自带的默认转换器,用户合理预期 List<UUID> 能用。建议一并处理,否则这个不一致会长期存在。
🔵 次要
a. 元素级枚举转换忽略 @SpecEnum。 字段级在 ObjectConverter.java:369 会读 @SpecEnum 决定 EnumGetMethod,而 convertValue 里硬编码 EnumGetMethod.NAME_IGNORECASE。@SpecEnum(ORDINAL) var modes: EnumSet<Mode> 的注解对元素不生效。旧代码根本不支持枚举元素,所以不是回退,只是新功能没做全。
b. ActionInsert.query 的异常类型变了。 新增的四条 require 把「空 values / 空白列名 / 值数与列数不匹配」从执行期 SQLException 提前成构造期 IllegalArgumentException。提前失败是对的,但 catch (SQLException) 的调用方会漏掉。elements getter 没加同样的校验,所以只有先读 query 才会触发——executeUpdate(action.query, action) 是先算 query,顺序上没问题。建议在兼容性说明里提一句异常类型变化。
c. 索引名/表名加引号可能拆分含点的索引名。 ExecutableSource.kt:295-299 现在对 index.name 和 table.name 都调 asFormattedColumnName()。该函数(Util.kt:88-90)会按 . 分段分别加引号——表名要的就是这个效果(public.order → "public"."order",测试已覆盖),但索引名如果含点会被拆成两段。索引名含点极不常见,提一下备查。
d. configFromMap 用 config.set(String.valueOf(key), value)。 nightconfig 的 set 按点号拆路径,map 的 key 含点会被解释成嵌套路径而非字面 key。该方法只用于「map 当结构化对象读」的场景,key 是字段名,实际不会含点,风险很低。
e. 一个 PR 装了三块无关改动。 配置转换、SQL 方言、JEXL 引擎缓存分属三个模块、三种失效机制,回滚粒度上耦合在一起了。#704-#719 这批整体是按主题切分的,这个 PR 是少数把不相关项目合并的。不影响正确性,只是评审与回滚成本更高。
🔗 与 #710 的合并顺序
onDuplicateKeyUpdate 在当前主干里没有任何调用方——我 grep 过整个仓库(排除 ActionInsert.kt 自身与 build 目录),零命中。也就是说本 PR 的方言修复目前只服务于用户插件。
而 #710 会引入第一个 in-tree 调用方:
// #710 database-player/Database.kt
is TypeSQL -> table.insert(dataSource, "user", "key", "value") {
value(user, key, data)
onDuplicateKeyUpdate { update("value", data) }
}
is TypeSQLite -> upsertSQLite(user, key, data) // ← 手写 INSERT OR REPLACE#710 的 SQLite 路径没有走 onDuplicateKeyUpdate,而是手写了 INSERT OR REPLACE INTO ...。两者语义并不等价:INSERT OR REPLACE 是删旧行再插新行,会重置未列出的列、并推进 autoincrement 计数器;ON CONFLICT DO UPDATE 是真正的原地更新。
如果本 PR 先合,#710 的 SQLite 分支就可以直接复用 onDuplicateKeyUpdate(listOf("user", "key")) { ... }——既拿到原地更新语义,又因为显式传了冲突字段而只需要 SQLite ≥ 3.24。建议两个 PR 的作者对齐一下,或在 #720 整合时统一收口。
顺带:本 PR 与 #710 都没改同一个文件,git 层面不冲突,只是语义上值得协调。
🟢 已核对无误
| 项 | 结论 |
|---|---|
ObjectConverter 新增类型的 import |
文件头已是 java.lang.reflect.* + java.util.* + taboolib.module.configuration.*,EnumSet/NavigableSet/Deque/TreeMap/WildcardType/TypeVariable 全部覆盖,无需新增 import |
ConfigSection.Companion.unwrap 可从 Java 访问 |
ConfigSection.kt:323 是 companion 内 public 函数,且同文件其他 Java 调用点(:302、:308)已在用同样写法 |
Map 字段修复是真 bug |
MapConverter(Converters.kt:9-18)是恒等转换,旧代码把 map 原样塞进字段,嵌套值仍是裸 Map,取用时 ClassCastException。新 convertMap 逐值递归,PR 描述对得上 |
List<Int> 装 Long 的修复 |
旧代码 Integer.isAssignableFrom(Long) 为 false → 走转换分支 → convertConfigsToObject 对非 Config 元素 throw。新代码走 convertNumber 正确收窄。真修复 |
| Kotlin 通配符签名兼容 | boundedType 对 WildcardType 取上界、对 TypeVariable 取 bounds,rawClass 同样递归;Kotlin List<Any> 擦除成 ? extends Object 时被 declaredClass == Object.class 提前返回,原值保留 |
createCollection 分支顺序 |
EnumSet → SortedSet/NavigableSet → Set → Deque/Queue → Collection 由窄到宽,且每步都加了 declaredClass.isAssignableFrom(具体类) 守卫,不会把 Set 字段填成 ArrayList |
createMap 分支顺序 |
NavigableMap/SortedMap → Map 同上,SortedMap 字段得到 TreeMap,测试断言到位 |
| 无法构造时的行为 | 抽象/接口且无匹配实现时抛 ReflectionException 并带上类名,比旧代码静默塞错类型再 ClassCastException 好 |
删除的 bottomElementType(ParameterizedType) / elementTypes / detectElementTypes / convertConfigsToObject |
均无其他调用点,bottomElementType(Collection) 重载保留(反向转换仍在用),删除安全 |
Host 三个子类的 when 分派 |
HostSQL / HostSQLite / HostPostgreSQL 都直接继承 Host<T>,互无继承关系,is HostPostgreSQL / is HostSQLite / else 覆盖完整;自定义 Host 落到 MYSQL,与改动前一致 |
| PostgreSQL 参数顺序 | elements getter 是 values 展平后接 duplicateUpdate,与 INSERT ... VALUES (?) ON CONFLICT ... DO UPDATE SET x = ? 的占位符顺序一致,测试 assertEquals(listOf("entry", 1, 2), action.elements) 覆盖了 MySQL 侧 |
| PostgreSQL 双引号 | setupQuoterForHost(Util.kt:27-32)对 HostPostgreSQL 设 DOUBLE_QUOTE,测试断言 "public"."order" 形态正确 |
Statement 链式调用 |
addSegment / addKeys / addValues / addSegmentIfTrue 均返回 Statement(Statement.kt:21-86),把 addValues 移出 addSegmentIfTrue 后链条仍成立 |
setupDialect 调用时机 |
ExecutableSource.insert 两个重载都在 func(it) 之前调 setupDialect,所以 onDuplicateKeyUpdate 里读到的方言已就绪,顺序正确 |
require 跳过空 keys 的位置校验 |
keys.isNotEmpty() 才校验值数匹配,位置插入(INSERT INTO t VALUES (?, ?))仍可用,测试 keeps positional insert compatibility when keys are omitted 覆盖 |
| JEXL 引擎失效机制 | configure 在 synchronized(engineLock) 内改 builder 并置 currentEngine = null;getter 双检加锁 + @Volatile,DCL 正确。替换 unsafeLazy 是必要的——lazy 一旦初始化就永不重建 |
jexlBuilder 外部误改风险 |
声明为 internal,插件侧拿不到,只能走 configure 包装的那些方法,不存在绕过失效的公开路径 |
JexlHelper 仍需 unsafeLazy import |
JexlHelper.kt:15 的 defaultJexlCompiler by unsafeLazy { ... } 还在用,只有 JexlCompiler.kt 里的 import 被删,正确 |
| commons-logging 是真缺失 | jexl3 3.2.1 内部用 commons-logging 打日志,而原 RuntimeDependency 是 transitive = false,所以它从来没被下载过 → 特定环境 NoClassDefFoundError。新增独立 RuntimeDependency + 两侧都加 org.apache.commons.logging 重定位对,写法与 Database.kt:20-24 里 HikariCP 带 slf4j 重定位的既有模式一致 |
RuntimeDependencies 注解存在且 relocate 支持多对 |
common-env/src/main/java/taboolib/common/env/RuntimeDependency.java 的 relocate 是 String[],Database.kt:23 已有四元素先例 |
| 测试 ThreadLocal 清理 | ActionInsertDialectTest.resetIdentifierQuoter 用 @AfterEach 调 currentQuoter.remove(),避免方言污染同线程后续测试,细节到位 |
| SQLite 实跑 upsert 测试 | executes generated sqlite upsert 真连了 jdbc:sqlite::memory: 执行两遍并断言最终值为 2,不只是断字符串 |
总结
三块改动都是真 bug 修复,JexlCompiler 的 DCL + @Volatile 失效重建写得很干净,boundedType / rawClass 对 wildcard 和 type variable 的递归处理也考虑到了 Kotlin 擦除后的签名形态。
建议处理的顺序:
- 问题 1(SQLite 版本门槛)——要么强制 SQLite 也传
conflictKeys把门槛降到 3.24,要么在 KDoc 和兼容性说明里写明 ≥3.35 的要求。当前状态下失败会以裸SQLSyntaxError出现在用户服务器上。 - 问题 2、3(
unwrap生效范围、集合实例不再共享/不再追加)——代码行为我认为是对的,但兼容性说明需要补。尤其是List<String>现在可能含 null 这一点。 - 问题 4(
convertValue不查ConverterRegistry)——建议顺手补上,List<UUID>的预期落差比较明显。 - 与 #710 的方言复用问题,建议两位作者或 #720 整合时对齐。
说明:本次审阅未实跑 gradle 测试(含 PR 描述列出的三条构建命令),SQL 语法结论来自本机 sqlite3 3.51.0 实测 + SQLite 官方文档,其余均为 patch 与仓库源码推导,已逐条注明依据位置。测试用例本身逐个读过,覆盖面与 PR 描述相符。
原有问题
配置转换、SQL 构造和 JEXL 引擎缓存都存在“表面类型或配置已变化,内部仍按旧信息处理”的问题:
ObjectConverter在递归转换集合和映射时丢失声明泛型,嵌套对象可能按原始Object、错误集合类型或错误元素类型恢复。ActionInsert在 MySQL、PostgreSQL、SQLite 间复用不兼容的 upsert 语法,空字段、占位符和参数顺序也可能不一致。典型触发场景与后果
List<Map<String, Bean>>、Set、Queue、SortedSet 或 EnumSet:读取后出现ClassCastException、元素类型错误或集合语义丢失。本 PR 修改
修改目的
让配置声明、数据库方言和运行时脚本配置真正决定最终行为,避免类型漂移、SQL 在不同数据库上失效以及配置修改不生效。
兼容性与行为变化
验证
./gradlew :module:basic:basic-configuration:test :module:database:test :module:script:script-jexl:test --rerun-tasks --no-parallel --stacktrace./gradlew :module:script:script-jexl:test :module:script:script-jexl:shadowJar --rerun-tasks --no-parallel --stacktracegit diff --checkRefs #703