Repository navigation
refactor(jdbc): use Locale.ROOT for every case-folding call site - #285
Conversation
11dc29f to
f569357
Compare
|
PASS+NOTES Independent verification of the exact head commit, not the PR prose. Identity
What I ran
I also mutation-tested the new pin in a throwaway copy outside the worktree (the reviewed worktree was never touched): removing the locale argument at all three production sites produces two failures — So the Notes1. The foreign-key pin is not a behavior pin. In the same mutation run the FK test still passed. 2. Two case-folding sites still use the default locale, so the PR body's "22 case-folding sites, all
Proven against the built classes with a Turkish default locale: This is pre-existing and outside the three-site scope of #283, so it does not block this PR, but it is the same class of bug the PR claims to have eliminated. Recommend a follow-up issue for both sites and a correction to the "one locale policy" claim in the description. 3. Test count in the PR body is stale. The body says 1610 tests; the head commit measures 1631, which is the 36 new tests plus the existing suite. Not a defect, just a description that no longer matches the head. 4. VerdictThe three-token change is correct, behavior-preserving, and green in every lane I could run, including the conformance suite and a Java 26 JVM. No public API, exception type, or message changed. The gaps are test-strength and description accuracy, not defects in the change. PASS+NOTES. No approval and no merge from me. |
) ## Description `TransactionMode.from` and `MetadataSql.tables` called `toUpperCase()` without a locale. Under a Turkish default locale, `"immediate"` folded to `"İMMEDİATE"`, so `TransactionMode.from("immediate")` threw `IllegalArgumentException`, and the `VIEW` table-type filter folded to `"VİEW"`, so `getTables(..., new String[] {"VIEW"})` matched nothing. Both sites now use `Locale.ROOT`, matching the policy the rest of the driver adopted in #283. ### Call sites | File | Line | Folding | |---|---:|---| | `libsql-jdbc/src/main/java/io/github/magnusp/libsql/jdbc/LibsqlConnection.java` | 99 | transaction mode in `TransactionMode.from` | | `libsql-jdbc/src/main/java/io/github/magnusp/libsql/jdbc/MetadataSql.java` | 36 | table type in `tables` | ### Tests - `transactionModeFoldsUnderATurkishDefaultLocale` sets `Locale.forLanguageTag("tr")` and asserts `TransactionMode.from("immediate")` is `IMMEDIATE` and `from("exclusive")` is `EXCLUSIVE`. - `tableTypeFilterFoldsUnderATurkishDefaultLocale` builds the predicate with `{"table", "view"}` under the same default and asserts `AND TABLE_TYPE IN ('TABLE','VIEW')`. - The existing foreign-key locale pin now uses `restrict` instead of `set null`, so it is mutation-sensitive too (`set null` and `cascade` contain no Turkish-sensitive letter). The `set null` mapping stays covered by `LibsqlDatabaseMetaDataTest.getImportedKeysMapsForeignKeyRowsAndNames`. Both new tests fail before the fix and pass after it. Before the fix, the transaction-mode test fails with `No enum constant ...İMMEDİATE`, and the table-type test fails on the `VİEW` literal. Reverting the `ForeignKeyMetadata` call site to the default locale makes the strengthened FK pin fail. No public API, exception type, or message changed. `Locale.ROOT` and `Locale.ENGLISH` fold identically for these inputs, as probed in #285. ## Type of Change - [x] Bug fix (non-breaking change fixing an issue) ## Checklist - [x] My code follows the code style of this project (`./mvnw spotless:check`). - [x] I have added tests that prove my fix is effective or that my feature works. - [x] All unit tests pass (`./mvnw -pl libsql-jdbc -am test`: 1622 tests, 0 failures). The container lane runs in CI. - [x] I have updated documentation where appropriate (none needed). Fixes #286
What changed
The three remaining
Locale.ENGLISHcase-folding call sites now useLocale.ROOT. Two pre-existing default-locale sites remain inLibsqlConnection.TransactionMode.fromandMetadataSql, and they are tracked in #286. The production diff is three tokens. Thejava.util.Localeimport stays at each site.Probe result
A scratch Java probe compared
String.toUpperCaseandString.toLowerCaseunderLocale.ENGLISHandLocale.ROOTfor every Unicode code point from U+0000 to U+10FFFF (surrogates excluded) and for the four call-site corpora: declared type names, foreign key rules, SQLite error codes and messages, and identifiers.Result: 0 upper-case differences and 0 lower-case differences. The two locales are interchangeable for every input the driver can see, so the change alters no result.
Call sites
libsql-jdbc/src/main/java/io/github/magnusp/libsql/jdbc/ColumnMetadata.javacolumnShapelibsql-jdbc/src/main/java/io/github/magnusp/libsql/jdbc/DeclaredType.javalibsql-jdbc/src/main/java/io/github/magnusp/libsql/jdbc/ForeignKeyMetadata.javakeyRuleTest added
libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/LocaleFoldingTest.java, 36 tests:toUpperCase(Locale.ENGLISH)equalstoUpperCase(Locale.ROOT)and the same for lower case. The corpus includesi,ı,İ, andß.Locale.forLanguageTag("tr")) that drives all three call sites through their package-private seams and asserts theLocale.ROOTresult:ColumnMetadata.columns(...)readsTYPE_NAMEfor anumericpragma row,DeclaredType.of(new Col("c", "numeric")).typeName()isNUMERIC, andForeignKeyMetadata.fromImported(...)mapscascadeandset nullrules. The default locale is restored in@AfterEach.No existing assertion was edited. No public API, exception type, or message changed.
Verification lanes
mise exec -- ./mvnw -pl libsql-jdbc -am testmise exec -- ./mvnw spotless:applythenmise exec -- ./mvnw spotless:checkmise exec -- ./mvnw -pl libsql-conformance -am verifyNotes
cascadeandset null, which contain no Turkish-sensitive letter. TheColumnMetadataandDeclaredTypecases are the mutation-sensitive pins. Arestrictcase would strengthen the FK pin and is recorded in fix(jdbc): fold transaction modes and table types with Locale.ROOT #286.PASS+NOTESat refactor(jdbc): use Locale.ROOT for every case-folding call site #285 (comment).Acceptance checklist
grep -rn "Locale.ENGLISH" libsql-jdbc/src/mainreturns nothing../mvnw -pl libsql-jdbc -am testis green with no existing assertion edited../mvnw spotless:checkis green../mvnw -pl libsql-conformance -am verifyis green.Closes #283
Refs #282