diff --git a/doc/changes/changelog.md b/doc/changes/changelog.md index 02180d1..01479fa 100644 --- a/doc/changes/changelog.md +++ b/doc/changes/changelog.md @@ -1,5 +1,6 @@ # Changes +* [18.0.3](changes_18.0.3.md) * [18.0.2](changes_18.0.2.md) * [18.0.1](changes_18.0.1.md) * [18.0.0](changes_18.0.0.md) diff --git a/doc/changes/changes_18.0.3.md b/doc/changes/changes_18.0.3.md new file mode 100644 index 0000000..eccea7c --- /dev/null +++ b/doc/changes/changes_18.0.3.md @@ -0,0 +1,11 @@ +# Common Module of Exasol Virtual Schemas Adapters 18.0.3, released 2026-06-08 + +Code name: Fix locale-dependent request serialization + +## Summary + +This release consistently uses `Locale.ROOT` for converting strings to upper/lower case. This avoids locale dependent bugs. + +## Bugfixes + +* #335: Fixed incomplete `Locale.ROOT` hardening in request parsing and rendering. Locale-sensitive casing is now avoided when parsing table metadata and `IS JSON` constraints, and when rendering pushdown JSON for node types, data types, character sets, and join types. diff --git a/pk_generated_parent.pom b/pk_generated_parent.pom index 4137a05..072229a 100644 --- a/pk_generated_parent.pom +++ b/pk_generated_parent.pom @@ -3,7 +3,7 @@ 4.0.0 com.exasol virtual-schema-common-java-generated-parent - 18.0.2 + 18.0.3 pom UTF-8 diff --git a/pom.xml b/pom.xml index 96a23a4..974d50c 100644 --- a/pom.xml +++ b/pom.xml @@ -2,7 +2,7 @@ 4.0.0 virtual-schema-common-java - 18.0.2 + 18.0.3 Common module of Exasol Virtual Schemas Adapters This is one of the modules of Virtual Schemas Adapters. The libraries provided by this project are the foundation of the adapter development, i.e. adapters must be implemented on top of them. @@ -93,7 +93,7 @@ virtual-schema-common-java-generated-parent com.exasol - 18.0.2 + 18.0.3 pk_generated_parent.pom diff --git a/src/main/java/com/exasol/adapter/request/parser/TablesMetadataParser.java b/src/main/java/com/exasol/adapter/request/parser/TablesMetadataParser.java index cbdee98..b9ca6f0 100644 --- a/src/main/java/com/exasol/adapter/request/parser/TablesMetadataParser.java +++ b/src/main/java/com/exasol/adapter/request/parser/TablesMetadataParser.java @@ -2,8 +2,7 @@ import static com.exasol.adapter.request.parser.RequestParserConstants.*; -import java.util.ArrayList; -import java.util.List; +import java.util.*; import com.exasol.adapter.metadata.*; import com.exasol.adapter.metadata.converter.SchemaMetadataJsonConverter; @@ -222,7 +221,7 @@ private static DataType.IntervalType intervalTypeFromString(final String interva } private DataType getDataType(final JsonObject dataType, final String columnDescription) { - final String typeName = requireString(dataType, "type", columnDescription + " data type").toUpperCase(); + final String typeName = requireString(dataType, "type", columnDescription + " data type").toUpperCase(Locale.ROOT); switch (typeName) { case "DECIMAL": return getDecimalDataType(dataType); diff --git a/src/main/java/com/exasol/adapter/request/renderer/PushdownSqlRenderer.java b/src/main/java/com/exasol/adapter/request/renderer/PushdownSqlRenderer.java index ea44c6f..9e2ee6a 100644 --- a/src/main/java/com/exasol/adapter/request/renderer/PushdownSqlRenderer.java +++ b/src/main/java/com/exasol/adapter/request/renderer/PushdownSqlRenderer.java @@ -3,6 +3,7 @@ import static com.exasol.adapter.request.RequestJsonKeys.*; import java.util.List; +import java.util.Locale; import java.util.Optional; import com.exasol.adapter.AdapterException; @@ -58,7 +59,7 @@ public JsonObject visit(final SqlStatementSelect select) throws AdapterException } private JsonObjectBuilder createObjectBuilderFor(final SqlNode type) { - return JSON.createObjectBuilder().add(TYPE, type.getType().name().toLowerCase()); + return JSON.createObjectBuilder().add(TYPE, type.getType().name().toLowerCase(Locale.ROOT)); } private void addIfPresent(final SqlNode node, final String key, final JsonObjectBuilder builder) @@ -159,7 +160,7 @@ public JsonObject visit(final SqlFunctionScalarCast function) throws AdapterExce private JsonObject render(final DataType dataType) { final JsonObjectBuilder builder = JSON.createObjectBuilder(); final DataType.ExaDataType type = dataType.getExaDataType(); - builder.add(TYPE, type.name().toUpperCase()); + builder.add(TYPE, type.name().toUpperCase(Locale.ROOT)); switch (type) { case DECIMAL: builder.add(PRECISION, dataType.getPrecision()); @@ -167,7 +168,7 @@ private JsonObject render(final DataType dataType) { break; case VARCHAR: case CHAR: - builder.add(CHARACTER_SET, dataType.getCharset().name().toLowerCase()); + builder.add(CHARACTER_SET, dataType.getCharset().name().toLowerCase(Locale.ROOT)); builder.add(SIZE, dataType.getSize()); break; case TIMESTAMP: @@ -462,7 +463,7 @@ public JsonObject visit(final SqlJoin sqlJoin) throws AdapterException { builder.add(LEFT, sqlJoin.getLeft().accept(this)); builder.add(RIGHT, sqlJoin.getRight().accept(this)); builder.add(CONDITION, sqlJoin.getCondition().accept(this)); - builder.add(JOIN_TYPE, sqlJoin.getJoinType().name().toLowerCase()); + builder.add(JOIN_TYPE, sqlJoin.getJoinType().name().toLowerCase(Locale.ROOT)); return builder.build(); } diff --git a/src/main/java/com/exasol/adapter/sql/AbstractSqlPredicateJson.java b/src/main/java/com/exasol/adapter/sql/AbstractSqlPredicateJson.java index ec4fc85..64d70dc 100644 --- a/src/main/java/com/exasol/adapter/sql/AbstractSqlPredicateJson.java +++ b/src/main/java/com/exasol/adapter/sql/AbstractSqlPredicateJson.java @@ -1,7 +1,6 @@ package com.exasol.adapter.sql; -import java.util.Arrays; -import java.util.List; +import java.util.*; /** * This class contains a common behavior for the {@link SqlNodeType#PREDICATE_IS_JSON} and @@ -15,11 +14,11 @@ public abstract class AbstractSqlPredicateJson extends SqlPredicate { /** * The Type constraint. */ - protected final SqlPredicateIsJson.TypeConstraints typeConstraint; + protected final TypeConstraints typeConstraint; /** * The Key uniqueness constraint. */ - protected final SqlPredicateIsJson.KeyUniquenessConstraint keyUniquenessConstraint; + protected final KeyUniquenessConstraint keyUniquenessConstraint; /** * Instantiates a new Abstract sql predicate json. @@ -29,9 +28,8 @@ public abstract class AbstractSqlPredicateJson extends SqlPredicate { * @param typeConstraint the type constraint * @param keyUniquenessConstraint the key uniqueness constraint */ - public AbstractSqlPredicateJson(final Predicate predicate, final SqlNode expression, - final SqlPredicateIsJson.TypeConstraints typeConstraint, - final SqlPredicateIsJson.KeyUniquenessConstraint keyUniquenessConstraint) { + protected AbstractSqlPredicateJson(final Predicate predicate, final SqlNode expression, final TypeConstraints typeConstraint, + final KeyUniquenessConstraint keyUniquenessConstraint) { super(predicate); this.expression = expression; this.typeConstraint = typeConstraint; @@ -117,12 +115,14 @@ public String toString() { * @param value the value * @return the sql predicate is json . key uniqueness constraint */ - public static SqlPredicateIsJson.KeyUniquenessConstraint of(final String value) { - final String formattedValue = String.join("_", value.split(" ")).toUpperCase(); - return SqlPredicateIsJson.KeyUniquenessConstraint.valueOf(formattedValue); + public static KeyUniquenessConstraint of(final String value) { + final String formattedValue = String.join("_", value.split(" ")).toUpperCase(Locale.ROOT); + return KeyUniquenessConstraint.valueOf(formattedValue); } } @Override - public List getChildren() { return Arrays.asList(this.expression); } + public List getChildren() { + return Arrays.asList(this.expression); + } } diff --git a/src/test/java/com/exasol/adapter/request/parser/PushDownSqlParserTest.java b/src/test/java/com/exasol/adapter/request/parser/PushDownSqlParserTest.java index 8f7e9de..c5da13a 100644 --- a/src/test/java/com/exasol/adapter/request/parser/PushDownSqlParserTest.java +++ b/src/test/java/com/exasol/adapter/request/parser/PushDownSqlParserTest.java @@ -25,6 +25,7 @@ import com.exasol.adapter.metadata.*; import com.exasol.adapter.sql.*; +import com.exasol.test.locale.WithLocale; import jakarta.json.*; @@ -537,24 +538,19 @@ void testParsePredicateIsNull() { } @Test + @WithLocale("tr") void testParsePredicateIsNullUsesLocaleIndependentUppercaseConversion() { - final Locale defaultLocale = Locale.getDefault(); - try { - Locale.setDefault(Locale.forLanguageTag("tr")); - final String sqlAsJson = "{" // - + " \"type\" : \"predicate_is_null\", " // - + " \"expression\" : { " // - + " \"type\" : \"literal_double\", " // - + " \"value\" : \"0.0\" " // - + " } " // - + "}"; - final JsonObject jsonObject = createJsonObjectFromString(sqlAsJson); - final SqlPredicateIsNull sqlPredicateIsNull = (SqlPredicateIsNull) this.defaultParser - .parseExpression(jsonObject); - assertThat(sqlPredicateIsNull.getType(), equalTo(PREDICATE_IS_NULL)); - } finally { - Locale.setDefault(defaultLocale); - } + final String sqlAsJson = "{" // + + " \"type\" : \"predicate_is_null\", " // + + " \"expression\" : { " // + + " \"type\" : \"literal_double\", " // + + " \"value\" : \"0.0\" " // + + " } " // + + "}"; + final JsonObject jsonObject = createJsonObjectFromString(sqlAsJson); + final SqlPredicateIsNull sqlPredicateIsNull = (SqlPredicateIsNull) this.defaultParser + .parseExpression(jsonObject); + assertThat(sqlPredicateIsNull.getType(), equalTo(PREDICATE_IS_NULL)); } @Test @@ -665,47 +661,47 @@ void testParseFunctionScalarCastToTimestamp() { @Test void testGetDataTypeDecimal() { - assertThat(getDataType("{" - + " \"type\" : \"DECIMAL\", " - + " \"precision\" : 31, " - + " \"scale\" : 41 " + assertThat(getDataType("{" + + " \"type\" : \"DECIMAL\", " + + " \"precision\" : 31, " + + " \"scale\" : 41 " + "}"), equalTo(createDecimal(31, 41))); } @Test void testGetDataTypeDouble() { - assertThat(getDataType("{" - + " \"type\" : \"DOUBLE\" " + assertThat(getDataType("{" + + " \"type\" : \"DOUBLE\" " + "}"), equalTo(DataType.createDouble())); } @Test void testGetDataTypeChar() { - assertThat(getDataType("{" - + " \"type\" : \"CHAR\", " - + " \"size\" : 22 " + assertThat(getDataType("{" + + " \"type\" : \"CHAR\", " + + " \"size\" : 22 " + "}"), equalTo(DataType.createChar(22, UTF8))); } @Test void testGetDataTypeBoolean() { - assertThat(getDataType("{" - + " \"type\" : \"BOOLEAN\" " + assertThat(getDataType("{" + + " \"type\" : \"BOOLEAN\" " + "}"), equalTo(DataType.createBool())); } @Test void testGetDataTypeDate() { - assertThat(getDataType("{" - + " \"type\" : \"DATE\" " + assertThat(getDataType("{" + + " \"type\" : \"DATE\" " + "}"), equalTo(DataType.createDate())); } @Test void testGetDataTypeGeometry() { - assertThat(getDataType("{" - + " \"type\" : \"GEOMETRY\", " - + " \"srid\" : 42 " + assertThat(getDataType("{" + + " \"type\" : \"GEOMETRY\", " + + " \"srid\" : 42 " + "}"), equalTo(DataType.createGeometry(42))); } diff --git a/src/test/java/com/exasol/adapter/request/parser/TablesMetadataParserTest.java b/src/test/java/com/exasol/adapter/request/parser/TablesMetadataParserTest.java index a62469e..2501dca 100644 --- a/src/test/java/com/exasol/adapter/request/parser/TablesMetadataParserTest.java +++ b/src/test/java/com/exasol/adapter/request/parser/TablesMetadataParserTest.java @@ -19,6 +19,7 @@ import org.junit.jupiter.params.provider.MethodSource; import com.exasol.adapter.metadata.*; +import com.exasol.test.locale.WithLocale; import jakarta.json.*; @@ -191,6 +192,26 @@ void testParseMetadataReadsAdapterNotes() { .identity(false).defaultValue("").comment("").build())); } + @Test + @WithLocale("tr") + void testParseMetadataUsesLocaleIndependentUppercaseConversion() { + final JsonArray tablesAsJson = arrayBuilder().add(objectBuilder() + .add("name", "T1") + .add("columns", arrayBuilder() + .add(objectBuilder().add("name", "INTERVAL_COL") + .add("dataType", objectBuilder().add("type", "interval").add("fromTo", + "YEAR TO MONTH"))))) + .build(); + + final List tables = TablesMetadataParser.create().parse(tablesAsJson); + + final List expectedColumns = List.of(ColumnMetadata.builder().name("INTERVAL_COL") + .adapterNotes("").type(DataType.createIntervalYearMonth(2)).nullable(true).identity(false) + .defaultValue("").comment("").build()); + + assertThat(tables, contains(new TableMetadata("T1", "", expectedColumns, ""))); + } + @ParameterizedTest @MethodSource("invalidTableMetadata") void testParseMetadataThrowsException(final JsonArray tablesAsJson, diff --git a/src/test/java/com/exasol/adapter/request/renderer/PushdownSqlRendererTest.java b/src/test/java/com/exasol/adapter/request/renderer/PushdownSqlRendererTest.java index b64d0f9..e4154b6 100644 --- a/src/test/java/com/exasol/adapter/request/renderer/PushdownSqlRendererTest.java +++ b/src/test/java/com/exasol/adapter/request/renderer/PushdownSqlRendererTest.java @@ -1,6 +1,7 @@ package com.exasol.adapter.request.renderer; import static com.exasol.adapter.metadata.DataType.*; +import static com.exasol.adapter.metadata.DataType.ExaCharset.ASCII; import static com.exasol.adapter.metadata.DataType.ExaCharset.UTF8; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; @@ -29,6 +30,7 @@ import com.exasol.adapter.metadata.TableMetadata; import com.exasol.adapter.request.parser.PushdownSqlParser; import com.exasol.adapter.sql.*; +import com.exasol.test.locale.WithLocale; import jakarta.json.*; @@ -987,6 +989,51 @@ void testRenderDataTypeIntervalYearToMonth() { () -> assertThat(rendered.getJsonObject("dataType").getString("fromTo"), equalTo("YEAR TO MONTH"))); } + @Test + @WithLocale("tr") + void testRenderUsesLocaleIndependentLowercaseForJoinTypes() { + final JsonObject rendered = renderToJsonObject(new SqlJoin(new SqlTable("A", null), new SqlTable("B", null), + new SqlLiteralBool(true), JoinType.INNER)); + + assertAll(() -> assertThat(rendered.getString("type"), equalTo("join")), + () -> assertThat(rendered.getString("join_type"), equalTo("inner"))); + } + + @Test + @WithLocale("tr") + void testRenderUsesLocaleIndependentCharacterSetAndDataTypeNames() { + final JsonObject rendered = renderToJsonObject( + new SqlFunctionScalarCast(createVarChar(10, ASCII), new SqlLiteralString("dummy"))); + final JsonObject dataType = rendered.getJsonObject("dataType"); + + assertAll(() -> assertThat(dataType.getString("type"), equalTo("VARCHAR")), + () -> assertThat(dataType.getString("characterSet"), equalTo("ascii"))); + } + + @Test + @WithLocale("tr") + void testRenderUsesLocaleIndependentUppercaseForDataTypeNames() { + final JsonObject rendered = renderToJsonObject( + new SqlFunctionScalarCast(createIntervalYearMonth(7), new SqlLiteralString("dummy"))); + final JsonObject dataType = rendered.getJsonObject("dataType"); + + assertAll(() -> assertThat(dataType.getString("type"), equalTo("INTERVAL")), + () -> assertThat(dataType.getJsonNumber("precision").intValue(), equalTo(7)), + () -> assertThat(dataType.getString("fromTo"), equalTo("YEAR TO MONTH"))); + } + + @Test + @WithLocale("tr") + void testRenderUsesLocaleIndependentLowercaseForPredicateTypes() { + final JsonObject rendered = renderToJsonObject(new SqlPredicateIsJson(new SqlLiteralString("x"), + AbstractSqlPredicateJson.TypeConstraints.OBJECT, + AbstractSqlPredicateJson.KeyUniquenessConstraint.WITH_UNIQUE_KEYS)); + + assertAll(() -> assertThat(rendered.getString("type"), equalTo("predicate_is_json")), + () -> assertThat(rendered.getString("typeConstraint"), equalTo("OBJECT")), + () -> assertThat(rendered.getString("keyUniquenessConstraint"), equalTo("WITH UNIQUE KEYS"))); + } + private JsonObject renderToJsonObject(final SqlNode node) { return (JsonObject) new PushdownSqlRenderer().render(node); } diff --git a/src/test/java/com/exasol/adapter/sql/SqlPredicateIsJsonTest.java b/src/test/java/com/exasol/adapter/sql/SqlPredicateIsJsonTest.java index e5fc5dd..4a7d764 100644 --- a/src/test/java/com/exasol/adapter/sql/SqlPredicateIsJsonTest.java +++ b/src/test/java/com/exasol/adapter/sql/SqlPredicateIsJsonTest.java @@ -12,8 +12,9 @@ import com.exasol.adapter.AdapterException; import com.exasol.mocking.MockUtils; +import com.exasol.test.locale.WithLocale; -@ExtendWith(MockitoExtension.class) +@ExtendWith({ MockitoExtension.class }) class SqlPredicateIsJsonTest { private SqlPredicateIsJson sqlPredicateIsJson; @Mock @@ -52,4 +53,13 @@ void testGetTypeConstraint() { void testGetKeyUniquenessConstraint() { assertThat(this.sqlPredicateIsJson.getKeyUniquenessConstraint(), equalTo("WITH UNIQUE KEYS")); } -} \ No newline at end of file + + @Test + @WithLocale("tr") + void testKeyUniquenessConstraintOfUsesLocaleIndependentUppercaseConversion() { + final AbstractSqlPredicateJson.KeyUniquenessConstraint result = AbstractSqlPredicateJson.KeyUniquenessConstraint + .of("without unique keys"); + + assertThat(result, equalTo(AbstractSqlPredicateJson.KeyUniquenessConstraint.WITHOUT_UNIQUE_KEYS)); + } +} diff --git a/src/test/java/com/exasol/test/locale/LocaleExtension.java b/src/test/java/com/exasol/test/locale/LocaleExtension.java new file mode 100644 index 0000000..fcb55b2 --- /dev/null +++ b/src/test/java/com/exasol/test/locale/LocaleExtension.java @@ -0,0 +1,40 @@ +package com.exasol.test.locale; + +import java.util.*; + +import org.junit.jupiter.api.extension.*; + +public class LocaleExtension implements BeforeEachCallback, AfterEachCallback { + private static final ExtensionContext.Namespace NAMESPACE = ExtensionContext.Namespace.create(LocaleExtension.class); + private static final String DEFAULT_LOCALE_KEY = "defaultLocale"; + + @Override + public void beforeEach(final ExtensionContext context) { + findLocaleTag(context).ifPresent(localeTag -> { + context.getStore(NAMESPACE).put(DEFAULT_LOCALE_KEY, Locale.getDefault()); + Locale.setDefault(Locale.forLanguageTag(localeTag)); + }); + } + + @Override + public void afterEach(final ExtensionContext context) { + final Locale defaultLocale = context.getStore(NAMESPACE).remove(DEFAULT_LOCALE_KEY, Locale.class); + if (defaultLocale != null) { + Locale.setDefault(defaultLocale); + } + } + + private Optional findLocaleTag(final ExtensionContext context) { + final Optional methodLocaleTag = context.getTestMethod() + .map(method -> method.getAnnotation(WithLocale.class)) + .filter(Objects::nonNull) + .map(WithLocale::value); + if (methodLocaleTag.isPresent()) { + return methodLocaleTag; + } + return context.getTestClass() + .map(testClass -> testClass.getAnnotation(WithLocale.class)) + .filter(Objects::nonNull) + .map(WithLocale::value); + } +} diff --git a/src/test/java/com/exasol/test/locale/LocaleExtensionTest.java b/src/test/java/com/exasol/test/locale/LocaleExtensionTest.java new file mode 100644 index 0000000..d12bf1c --- /dev/null +++ b/src/test/java/com/exasol/test/locale/LocaleExtensionTest.java @@ -0,0 +1,30 @@ +package com.exasol.test.locale; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.not; +import static org.junit.jupiter.api.Assertions.assertAll; + +import java.util.Locale; + +import org.junit.jupiter.api.*; +import org.junit.jupiter.api.MethodOrderer.OrderAnnotation; + +@TestMethodOrder(OrderAnnotation.class) +class LocaleExtensionTest { + private static final Locale ORIGINAL_LOCALE = Locale.getDefault(); + + @Test + @Order(1) + @WithLocale("tr") + void testWithLocaleChangesDefaultLocaleDuringTest() { + assertAll(() -> assertThat(Locale.getDefault(), not(equalTo(ORIGINAL_LOCALE))), + () -> assertThat(Locale.getDefault(), equalTo(Locale.forLanguageTag("tr")))); + } + + @Test + @Order(2) + void testLocaleIsRestoredAfterAnnotatedTest() { + assertThat(Locale.getDefault(), equalTo(ORIGINAL_LOCALE)); + } +} diff --git a/src/test/java/com/exasol/test/locale/WithLocale.java b/src/test/java/com/exasol/test/locale/WithLocale.java new file mode 100644 index 0000000..28d0b01 --- /dev/null +++ b/src/test/java/com/exasol/test/locale/WithLocale.java @@ -0,0 +1,23 @@ +package com.exasol.test.locale; + +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +import org.junit.jupiter.api.extension.ExtendWith; + +/** + * This annotation can be used to set the default locale for a test method or an entire test class. + */ +@Target({ TYPE, METHOD }) +@Retention(RUNTIME) +@ExtendWith(LocaleExtension.class) +public @interface WithLocale { + /** + * The locale to set for the annotated test method or all test methods in the annotated class. + */ + String value(); +}