From cea1c7e6b14fb4cc8966f8c8524fb1fa4d0bd101 Mon Sep 17 00:00:00 2001 From: Leon van Zantvoort Date: Mon, 10 Aug 2026 16:22:25 +0200 Subject: [PATCH 1/2] fix!: keep explicit dialects across withConfig and fail fast on ambiguous dialect resolution withConfig re-resolved the dialect from the classpath, silently discarding a dialect set via withDialect. The template now tracks the explicitly set dialect and retains it across withConfig; a classpath-resolved dialect is re-resolved under the new configuration, as dialects capture their configuration at construction. Providers.getSqlDialect(StormConfig) now selects its provider through the same unique-selection path as the connection and transaction providers, so an ambiguous classpath fails with an error naming the candidates instead of picking by classpath order. Ambient dialect resolution is deferred to first use: SqlTemplate.PS/JPA no longer resolve a dialect during class initialization, and database-bound templates never trigger classpath resolution at all. The JPA classpath fallback is deferred the same way so a provider filter can still be applied on an ambiguous classpath. --- .../main/java/st/orm/core/spi/Providers.java | 17 +- .../st/orm/core/template/SqlTemplate.java | 9 +- .../core/template/impl/JpaTemplateImpl.java | 40 +++-- .../core/template/impl/SqlTemplateImpl.java | 71 ++++++--- .../spi/SqlDialectProviderResolutionTest.java | 145 ++++++++++++++++++ .../template/impl/SqlTemplateImplTest.java | 42 +++++ 6 files changed, 283 insertions(+), 41 deletions(-) create mode 100644 storm-core/src/test/java/st/orm/core/spi/SqlDialectProviderResolutionTest.java diff --git a/storm-core/src/main/java/st/orm/core/spi/Providers.java b/storm-core/src/main/java/st/orm/core/spi/Providers.java index 578e6cb2c..7ef2e1b3e 100644 --- a/storm-core/src/main/java/st/orm/core/spi/Providers.java +++ b/storm-core/src/main/java/st/orm/core/spi/Providers.java @@ -235,11 +235,20 @@ public static SqlDialect getSqlDialect() { return getSqlDialect(StormConfig.defaults()); } + /** + * Resolves the SQL dialect from the classpath, without a database in view. + * + *

Enablement is re-evaluated on every resolution, and an ambiguous resolution fails fast.

+ * + * @param config the Storm configuration to apply. + * @return the SQL dialect. + * @throws PersistenceException if no dialect provider is found or the resolution is ambiguous. + */ public static SqlDialect getSqlDialect(@Nonnull StormConfig config) { - return enabled(SQL_DIALECT_PROVIDERS) - .map(p -> p.getSqlDialect(config)) - .findFirst() - .orElseThrow(); + return selectUnique(SQL_DIALECT_PROVIDERS, "SQL dialect provider", + "SqlTemplate.withDialect(...), or by binding the template to a DataSource or Connection so the " + + "dialect is derived from the database") + .getSqlDialect(config); } public static SqlDialect getSqlDialect(@Nonnull Predicate filter) { diff --git a/storm-core/src/main/java/st/orm/core/template/SqlTemplate.java b/storm-core/src/main/java/st/orm/core/template/SqlTemplate.java index 33956d94f..4c5055fb9 100644 --- a/storm-core/src/main/java/st/orm/core/template/SqlTemplate.java +++ b/storm-core/src/main/java/st/orm/core/template/SqlTemplate.java @@ -286,14 +286,19 @@ interface BatchListener { /** * Returns the SQL dialect used by this template. * + *

When no dialect has been set via {@link #withDialect(SqlDialect)}, the dialect is resolved from the + * classpath on first use. That resolution fails with a descriptive exception when multiple dialect providers + * are eligible without a defined order.

+ * * @return the SQL dialect used by this template. * @since 1.2 */ SqlDialect dialect(); /** - * Returns a new SQL template with the specified Storm configuration. The SQL dialect is re-resolved from the - * provided configuration. + * Returns a new SQL template with the specified Storm configuration. A dialect set via + * {@link #withDialect(SqlDialect)} is retained; a classpath-resolved dialect is re-resolved under the new + * configuration, as dialects capture their configuration at construction. * * @param config the Storm configuration to apply. * @return a new SQL template. diff --git a/storm-core/src/main/java/st/orm/core/template/impl/JpaTemplateImpl.java b/storm-core/src/main/java/st/orm/core/template/impl/JpaTemplateImpl.java index 59c27dff8..0ddbbca74 100644 --- a/storm-core/src/main/java/st/orm/core/template/impl/JpaTemplateImpl.java +++ b/storm-core/src/main/java/st/orm/core/template/impl/JpaTemplateImpl.java @@ -19,6 +19,7 @@ import static jakarta.persistence.TemporalType.TIME; import static jakarta.persistence.TemporalType.TIMESTAMP; import static st.orm.core.template.SqlTemplate.JPA; +import static st.orm.core.template.impl.LazySupplier.lazy; import static st.orm.core.template.impl.RecordValidation.validate; import jakarta.annotation.Nonnull; @@ -28,6 +29,7 @@ import java.util.List; import java.util.Map; import java.util.function.Predicate; +import java.util.function.Supplier; import java.util.stream.Stream; import java.util.stream.StreamSupport; import javax.sql.DataSource; @@ -83,9 +85,21 @@ private interface TemplateProcessor { private final TableAliasResolver tableAliasResolver; private final Predicate providerFilter; private final RefFactory refFactory; - private final SqlTemplate sqlTemplate; + + /** + * Built on first use: without a provider filter it consumes the resolved dialect, whose classpath fallback must + * stay untouched until the template is actually used. + */ + private final Supplier sqlTemplate; + private final StormConfig config; - private final SqlDialect dialect; + + /** + * The dialect of the persistence unit's database, or a classpath fallback resolved on first use when the database + * is unknown. Deferring the fallback keeps an ambiguous classpath from failing template construction, so a + * provider filter can still be applied via {@link #withProviderFilter(Predicate)}. + */ + private final Supplier dialect; public JpaTemplateImpl(@Nonnull EntityManager entityManager) { this(entityManager, StormConfig.defaults()); @@ -112,7 +126,7 @@ public JpaTemplateImpl(@Nonnull EntityManager entityManager, @Nonnull StormConfi this.config = config; this.dialect = resolveDialect(entityManager, config); this.refFactory = new RefFactoryImpl(this, modelBuilder, providerFilter); - this.sqlTemplate = createSqlTemplate(); + this.sqlTemplate = lazy(this::createSqlTemplate); } /** @@ -121,19 +135,19 @@ public JpaTemplateImpl(@Nonnull EntityManager entityManager, @Nonnull StormConfi *

Asking the persistence unit for its data source is the portable way to reach the database: unwrapping an * entity manager to a {@link java.sql.Connection} is not supported by every provider. A persistence unit * configured with a connection URL rather than a data source, or one whose database cannot be reached while the - * template is being built, leaves the database unknown, and the dialect then comes from the classpath as - * before.

+ * template is being built, leaves the database unknown, and the dialect then comes from the classpath, resolved + * on first use.

*/ - private static SqlDialect resolveDialect(@Nonnull EntityManager entityManager, @Nonnull StormConfig config) { + private static Supplier resolveDialect(@Nonnull EntityManager entityManager, @Nonnull StormConfig config) { DataSource dataSource = dataSourceOf(entityManager); if (dataSource == null) { - return Providers.getSqlDialect(config); + return lazy(() -> Providers.getSqlDialect(config)); } try { - return Providers.getSqlDialect(dataSource, config); + return new LazySupplier<>(Providers.getSqlDialect(dataSource, config)); } catch (RuntimeException e) { LOGGER.debug("Failed to determine the database of the persistence unit.", e); - return Providers.getSqlDialect(config); + return lazy(() -> Providers.getSqlDialect(config)); } } @@ -163,7 +177,7 @@ private JpaTemplateImpl(@Nonnull TemplateProcessor templateProcessor, @Nonnull TableAliasResolver tableAliasResolver, @Nullable Predicate providerFilter, @Nonnull StormConfig config, - @Nonnull SqlDialect dialect) { + @Nonnull Supplier dialect) { this.dialect = dialect; this.templateProcessor = templateProcessor; this.modelBuilder = modelBuilder; @@ -171,7 +185,7 @@ private JpaTemplateImpl(@Nonnull TemplateProcessor templateProcessor, this.providerFilter = providerFilter; this.config = config; this.refFactory = new RefFactoryImpl(this, modelBuilder, providerFilter); - this.sqlTemplate = createSqlTemplate(); + this.sqlTemplate = lazy(this::createSqlTemplate); } private SqlTemplate createSqlTemplate() { @@ -183,7 +197,7 @@ private SqlTemplate createSqlTemplate() { // The shared template resolves a dialect without a database in view, so the dialect resolved for this // persistence unit is applied on top. An explicit provider filter still wins. return template.withDialect( - providerFilter != null ? Providers.getSqlDialect(providerFilter, config) : dialect); + providerFilter != null ? Providers.getSqlDialect(providerFilter, config) : dialect.get()); } private void setParameters(@Nonnull jakarta.persistence.Query query, @Nonnull List parameters) { @@ -252,7 +266,7 @@ private jakarta.persistence.Query query(@Nonnull TemplateString template, @Nonnu */ @Override public SqlTemplate sqlTemplate() { - return SqlInterceptorManager.customize(sqlTemplate); + return SqlInterceptorManager.customize(sqlTemplate.get()); } /** diff --git a/storm-core/src/main/java/st/orm/core/template/impl/SqlTemplateImpl.java b/storm-core/src/main/java/st/orm/core/template/impl/SqlTemplateImpl.java index 1157f7653..fbd97bf73 100644 --- a/storm-core/src/main/java/st/orm/core/template/impl/SqlTemplateImpl.java +++ b/storm-core/src/main/java/st/orm/core/template/impl/SqlTemplateImpl.java @@ -24,12 +24,14 @@ import static st.orm.core.template.impl.SqlInterceptorManager.intercept; import jakarta.annotation.Nonnull; +import jakarta.annotation.Nullable; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.Function; +import java.util.function.Supplier; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import st.orm.BindVars; @@ -84,15 +86,33 @@ record Wrapped(@Nonnull List elements) implements Element { private final boolean inlineParameters; private final ModelBuilder modelBuilder; private final TableAliasResolver tableAliasResolver; - private final SqlDialect dialect; - private final TemplatePreparation templatePreparation; + + /** + * The dialect set via {@link #withDialect(SqlDialect)} or a dialect-taking constructor, or {@code null} when the + * dialect is resolved from the classpath. An explicit dialect is preserved across {@link #withConfig(StormConfig)}, + * whereas a classpath-resolved dialect is re-resolved under the new configuration. + */ + private final @Nullable SqlDialect explicitDialect; + + /** + * The dialect in use, resolved on first use. Classpath resolution fails fast when multiple dialect providers are + * eligible without a defined order, so templates that receive an explicit dialect before processing, such as the + * shared {@link SqlTemplate#PS} and {@link SqlTemplate#JPA} instances customized by database-bound templates, + * must never trigger it. All dialect-dependent state is therefore initialized lazily. + */ + private final LazySupplier dialect; + + private final Supplier templatePreparation; private final Function keyGenerator; private final StormConfig config; - private final SegmentedLruCache cache; + + /** The template cache, keyed by the resolved dialect and therefore lazy; {@code null} when caching is disabled. */ + private final @Nullable Supplier> cache; + private final TemplateMetrics templateMetrics; public SqlTemplateImpl(boolean positionalOnly, boolean expandCollection, boolean supportRecords) { - this(positionalOnly, expandCollection, supportRecords, false, ModelBuilder.newInstance(), TableAliasResolver.DEFAULT, getSqlDialect()); + this(positionalOnly, expandCollection, supportRecords, false, ModelBuilder.newInstance(), TableAliasResolver.DEFAULT, null, StormConfig.defaults()); } public SqlTemplateImpl(boolean positionalOnly, @@ -102,7 +122,7 @@ public SqlTemplateImpl(boolean positionalOnly, @Nonnull ModelBuilder modelBuilder, @Nonnull TableAliasResolver tableAliasResolver, @Nonnull SqlDialect dialect) { - this(positionalOnly, expandCollection, supportRecords, inlineParameters, modelBuilder, tableAliasResolver, dialect, StormConfig.defaults()); + this(positionalOnly, expandCollection, supportRecords, inlineParameters, modelBuilder, tableAliasResolver, requireNonNull(dialect), StormConfig.defaults()); } SqlTemplateImpl(boolean positionalOnly, @@ -111,7 +131,7 @@ public SqlTemplateImpl(boolean positionalOnly, boolean inlineParameters, @Nonnull ModelBuilder modelBuilder, @Nonnull TableAliasResolver tableAliasResolver, - @Nonnull SqlDialect dialect, + @Nullable SqlDialect dialect, @Nonnull StormConfig config) { this.positionalOnly = positionalOnly; this.expandCollection = expandCollection; @@ -119,17 +139,20 @@ public SqlTemplateImpl(boolean positionalOnly, this.inlineParameters = inlineParameters; this.modelBuilder = requireNonNull(modelBuilder); this.tableAliasResolver = requireNonNull(tableAliasResolver); - this.dialect = requireNonNull(dialect); + this.explicitDialect = dialect; this.config = requireNonNull(config); - this.templatePreparation = new TemplatePreparation(this, modelBuilder); + this.dialect = dialect != null ? new LazySupplier<>(dialect) : new LazySupplier<>(() -> getSqlDialect(config)); + this.templatePreparation = new LazySupplier<>(() -> new TemplatePreparation(this, modelBuilder)); this.keyGenerator = keyGenerator(); int templateCacheSize = Math.max(0, getInt(config, TEMPLATE_CACHE_SIZE, 2048)); if (templateCacheSize == 0 || inlineParameters) { // We don't want to cache templates with inline parameters. No caching takes place if inline parameters are enabled. this.cache = null; } else { - var key = List.of(positionalOnly, expandCollection, supportRecords, new IdentityKey(modelBuilder), new IdentityKey(tableAliasResolver), dialect.name(), configCacheKey(config)); - this.cache = CacheHolder.INSTANCE.getOrCompute(key, () -> new SegmentedLruCache<>(templateCacheSize)); + this.cache = new LazySupplier<>(() -> { + var key = List.of(positionalOnly, expandCollection, supportRecords, new IdentityKey(modelBuilder), new IdentityKey(tableAliasResolver), dialect().name(), configCacheKey(config)); + return CacheHolder.INSTANCE.getOrCompute(key, () -> new SegmentedLruCache<>(templateCacheSize)); + }); } this.templateMetrics = TemplateMetrics.getInstance(); this.templateMetrics.registerCacheSize(templateCacheSize); @@ -148,7 +171,7 @@ private static Map configCacheKey(@Nonnull StormConfig config) { private Function keyGenerator() { return template -> { try { - return getCompilationKey(templatePreparation.preprocess(template)); + return getCompilationKey(templatePreparation.get().preprocess(template)); } catch (SqlTemplateException e) { throw new UncheckedSqlTemplateException(e); } @@ -158,7 +181,7 @@ private Function keyGenerator() { private Function shapeGenerator() { return template -> { try { - return getShapeKey(templatePreparation.preprocess(template)); + return getShapeKey(templatePreparation.get().preprocess(template)); } catch (SqlTemplateException e) { throw new UncheckedSqlTemplateException(e); } @@ -197,7 +220,7 @@ public SqlTemplateImpl withTableNameResolver(@Nonnull TableNameResolver tableNam if (tableNameResolver == modelBuilder.tableNameResolver()) { return this; } - return new SqlTemplateImpl(positionalOnly, expandCollection, supportRecords, inlineParameters, modelBuilder.tableNameResolver(tableNameResolver), tableAliasResolver, dialect, config); + return new SqlTemplateImpl(positionalOnly, expandCollection, supportRecords, inlineParameters, modelBuilder.tableNameResolver(tableNameResolver), tableAliasResolver, explicitDialect, config); } /** @@ -221,7 +244,7 @@ public SqlTemplateImpl withTableAliasResolver(@Nonnull TableAliasResolver tableA if (tableAliasResolver == this.tableAliasResolver) { return this; } - return new SqlTemplateImpl(positionalOnly, expandCollection, supportRecords, inlineParameters, modelBuilder, tableAliasResolver, dialect, config); + return new SqlTemplateImpl(positionalOnly, expandCollection, supportRecords, inlineParameters, modelBuilder, tableAliasResolver, explicitDialect, config); } /** @@ -245,7 +268,7 @@ public SqlTemplateImpl withColumnNameResolver(@Nonnull ColumnNameResolver column if (columnNameResolver == modelBuilder.columnNameResolver()) { return this; } - return new SqlTemplateImpl(positionalOnly, expandCollection, supportRecords, inlineParameters, modelBuilder.columnNameResolver(columnNameResolver), tableAliasResolver, dialect, config); + return new SqlTemplateImpl(positionalOnly, expandCollection, supportRecords, inlineParameters, modelBuilder.columnNameResolver(columnNameResolver), tableAliasResolver, explicitDialect, config); } /** @@ -269,7 +292,7 @@ public SqlTemplateImpl withForeignKeyResolver(@Nonnull ForeignKeyResolver foreig if (foreignKeyResolver == modelBuilder.foreignKeyResolver()) { return this; } - return new SqlTemplateImpl(positionalOnly, expandCollection, supportRecords, inlineParameters, modelBuilder.foreignKeyResolver(foreignKeyResolver), tableAliasResolver, dialect, config); + return new SqlTemplateImpl(positionalOnly, expandCollection, supportRecords, inlineParameters, modelBuilder.foreignKeyResolver(foreignKeyResolver), tableAliasResolver, explicitDialect, config); } /** @@ -290,21 +313,23 @@ public ForeignKeyResolver foreignKeyResolver() { */ @Override public SqlTemplate withDialect(@Nonnull SqlDialect dialect) { - if (dialect == this.dialect) { + requireNonNull(dialect); + if (dialect == this.explicitDialect || this.dialect.value().orElse(null) == dialect) { return this; } return new SqlTemplateImpl(positionalOnly, expandCollection, supportRecords, inlineParameters, modelBuilder, tableAliasResolver, dialect, config); } /** - * Returns the SQL dialect used by this template. + * Returns the SQL dialect used by this template, resolving it from the classpath on first use when no dialect + * has been set explicitly. * * @return the SQL dialect used by this template. * @since 1.2 */ @Override public SqlDialect dialect() { - return dialect; + return dialect.get(); } @Override @@ -312,7 +337,7 @@ public SqlTemplate withConfig(@Nonnull StormConfig config) { if (config == this.config) { return this; } - return new SqlTemplateImpl(positionalOnly, expandCollection, supportRecords, inlineParameters, modelBuilder, tableAliasResolver, getSqlDialect(config), config); + return new SqlTemplateImpl(positionalOnly, expandCollection, supportRecords, inlineParameters, modelBuilder, tableAliasResolver, explicitDialect, config); } /** @@ -326,7 +351,7 @@ public SqlTemplateImpl withSupportRecords(boolean supportRecords) { if (supportRecords == this.supportRecords) { return this; } - return new SqlTemplateImpl(positionalOnly, expandCollection, supportRecords, inlineParameters, modelBuilder, tableAliasResolver, dialect, config); + return new SqlTemplateImpl(positionalOnly, expandCollection, supportRecords, inlineParameters, modelBuilder, tableAliasResolver, explicitDialect, config); } /** @@ -353,7 +378,7 @@ public SqlTemplate withInlineParameters(boolean inlineParameters) { if (inlineParameters == this.inlineParameters) { return this; } - return new SqlTemplateImpl(positionalOnly, expandCollection, supportRecords, inlineParameters, modelBuilder, tableAliasResolver, dialect, config); + return new SqlTemplateImpl(positionalOnly, expandCollection, supportRecords, inlineParameters, modelBuilder, tableAliasResolver, explicitDialect, config); } /** @@ -408,6 +433,8 @@ Sql process(@Nonnull TemplateString template, boolean applyInterceptors) throws TemplateProcessor processor; try { try (var request = templateMetrics.startRequest()) { + var templatePreparation = this.templatePreparation.get(); + var cache = this.cache == null ? null : this.cache.get(); bindingContext = templatePreparation.preprocess(template); compilationKey = cache == null ? null : getCompilationKey(bindingContext); processor = compilationKey == null ? null : cache.get(compilationKey); diff --git a/storm-core/src/test/java/st/orm/core/spi/SqlDialectProviderResolutionTest.java b/storm-core/src/test/java/st/orm/core/spi/SqlDialectProviderResolutionTest.java new file mode 100644 index 000000000..1c0493e2b --- /dev/null +++ b/storm-core/src/test/java/st/orm/core/spi/SqlDialectProviderResolutionTest.java @@ -0,0 +1,145 @@ +/* + * Copyright 2024 - 2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package st.orm.core.spi; + +import static java.lang.Thread.currentThread; +import static java.util.Collections.enumeration; +import static java.util.stream.Collectors.joining; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import jakarta.annotation.Nonnull; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.URL; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Enumeration; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import st.orm.PersistenceException; +import st.orm.StormConfig; +import st.orm.core.template.SqlDialect; +import st.orm.core.template.SqlTemplate; + +/** + * Tests for the fail-fast classpath resolution of the SQL dialect. + */ +public class SqlDialectProviderResolutionTest { + + @TempDir + Path tempDir; + + /** Dialect provider without ordering constraints; an unordered peer of {@link UnorderedProviderB}. */ + public static class UnorderedProviderA implements SqlDialectProvider { + @Override + public SqlDialect getSqlDialect(@Nonnull StormConfig config) { + return new DefaultSqlDialect(config); + } + } + + /** Dialect provider without ordering constraints; an unordered peer of {@link UnorderedProviderA}. */ + public static class UnorderedProviderB implements SqlDialectProvider { + @Override + public SqlDialect getSqlDialect(@Nonnull StormConfig config) { + return new DefaultSqlDialect(config); + } + } + + /** Class loader that substitutes the {@link SqlDialectProvider} service registrations. */ + private static final class ServiceSubstitutingClassLoader extends ClassLoader { + + private static final String SERVICE_RESOURCE = "META-INF/services/" + SqlDialectProvider.class.getName(); + + private final URL services; + + ServiceSubstitutingClassLoader(@Nonnull ClassLoader parent, @Nonnull URL services) { + super(parent); + this.services = services; + } + + @Override + public Enumeration getResources(String name) throws IOException { + if (SERVICE_RESOURCE.equals(name)) { + return enumeration(List.of(services)); + } + return super.getResources(name); + } + } + + private void withDialectProviders(@Nonnull List> providers, @Nonnull Runnable runnable) { + URL services; + try { + Path file = tempDir.resolve("dialect-providers"); + Files.writeString(file, providers.stream().map(Class::getName).collect(joining("\n"))); + services = file.toUri().toURL(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + Thread thread = currentThread(); + ClassLoader original = thread.getContextClassLoader(); + thread.setContextClassLoader(new ServiceSubstitutingClassLoader(getClass().getClassLoader(), services)); + try { + runnable.run(); + } finally { + thread.setContextClassLoader(original); + } + } + + @Test + public void testUnorderedPeersFailFast() { + withDialectProviders(List.of(UnorderedProviderA.class, UnorderedProviderB.class), () -> { + var exception = assertThrows(PersistenceException.class, + () -> Providers.getSqlDialect(StormConfig.defaults())); + assertTrue(exception.getMessage().contains(UnorderedProviderA.class.getName()), + "Error must name the candidate providers"); + assertTrue(exception.getMessage().contains(UnorderedProviderB.class.getName()), + "Error must name the candidate providers"); + }); + } + + @Test + public void testSingleProviderResolves() { + withDialectProviders(List.of(DefaultSqlDialectProviderImpl.class), () -> + assertEquals(new DefaultSqlDialect(StormConfig.defaults()).name(), + Providers.getSqlDialect(StormConfig.defaults()).name())); + } + + @Test + public void testOrderedProvidersResolveUnique() { + withDialectProviders(List.of(FetchSizeSqlDialectProviderImpl.class, DefaultSqlDialectProviderImpl.class), () -> + assertEquals("FetchSizeTest", Providers.getSqlDialect(StormConfig.defaults()).name(), + "The provider ordered before any other must win without an ambiguity error")); + } + + @Test + public void testAmbientDialectResolutionIsLazy() { + withDialectProviders(List.of(UnorderedProviderA.class, UnorderedProviderB.class), () -> { + // Deriving a template from the shared ambient instance must not resolve the dialect: database-bound + // templates derive and then set the dialect resolved for their database. + SqlTemplate template = SqlTemplate.PS.withConfig(StormConfig.of(Map.of())); + SqlDialect dialect = new DefaultSqlDialect(); + assertSame(dialect, template.withDialect(dialect).dialect(), + "An explicitly set dialect must not trigger classpath resolution"); + assertThrows(PersistenceException.class, template::dialect, + "Using the ambient dialect on an ambiguous classpath must fail fast"); + }); + } +} diff --git a/storm-core/src/test/java/st/orm/core/template/impl/SqlTemplateImplTest.java b/storm-core/src/test/java/st/orm/core/template/impl/SqlTemplateImplTest.java index 315231e8b..efc48bc1d 100644 --- a/storm-core/src/test/java/st/orm/core/template/impl/SqlTemplateImplTest.java +++ b/storm-core/src/test/java/st/orm/core/template/impl/SqlTemplateImplTest.java @@ -6,11 +6,15 @@ import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; +import static st.orm.StormConfig.ANSI_ESCAPING; import static st.orm.core.template.TemplateString.raw; +import java.util.Map; import org.junit.jupiter.api.Test; import st.orm.StormConfig; +import st.orm.core.spi.DefaultSqlDialect; import st.orm.core.template.Sql; +import st.orm.core.template.SqlDialect; import st.orm.core.template.SqlTemplate; import st.orm.core.template.SqlTemplateException; import st.orm.core.template.TableAliasResolver; @@ -121,6 +125,44 @@ public void testWithConfigReturnsSameWhenIdentical() { assertNotNull(result); } + @Test + public void testWithConfigRetainsExplicitDialect() { + SqlDialect dialect = new DefaultSqlDialect(); + SqlTemplate template = SqlTemplate.PS.withDialect(dialect) + .withConfig(StormConfig.of(Map.of(ANSI_ESCAPING, "true"))); + assertSame(dialect, template.dialect(), "Explicitly set dialect must survive withConfig"); + } + + @Test + public void testWithConfigAppliesConfigToClasspathResolvedDialect() { + assertEquals("\"name\"", + SqlTemplate.PS.withConfig(StormConfig.of(Map.of(ANSI_ESCAPING, "true"))).dialect().escape("name"), + "Classpath-resolved dialect must be re-resolved under the new configuration"); + assertEquals("name", + SqlTemplate.PS.withConfig(StormConfig.of(Map.of(ANSI_ESCAPING, "false"))).dialect().escape("name"), + "Classpath-resolved dialect must be re-resolved under the new configuration"); + } + + @Test + public void testWithResolversRetainExplicitDialect() { + SqlDialect dialect = new DefaultSqlDialect(); + SqlTemplate template = SqlTemplate.PS.withDialect(dialect) + .withTableNameResolver(type -> "prefix_" + type.type().getSimpleName()) + .withTableAliasResolver((type, counter) -> "alias"); + assertSame(dialect, template.dialect(), "Explicitly set dialect must survive resolver customization"); + } + + @Test + public void testWithResolversRetainClasspathResolution() { + SqlTemplate template = SqlTemplate.PS.withTableNameResolver(type -> "prefix_" + type.type().getSimpleName()); + assertEquals("\"name\"", + template.withConfig(StormConfig.of(Map.of(ANSI_ESCAPING, "true"))).dialect().escape("name"), + "Resolver customization must not pin the classpath-resolved dialect"); + assertEquals("name", + template.withConfig(StormConfig.of(Map.of(ANSI_ESCAPING, "false"))).dialect().escape("name"), + "Resolver customization must not pin the classpath-resolved dialect"); + } + // positionalOnly and expandCollection @Test From fb64108287652cb44d77a4466ce3b99c728c788e Mon Sep 17 00:00:00 2001 From: Leon van Zantvoort Date: Mon, 10 Aug 2026 16:27:58 +0200 Subject: [PATCH 2/2] perf: return resolved LazySupplier values with a single volatile read LazySupplier.get() performed a compare-and-set and a volatile store on every call, including after the value was resolved. Suppliers shared across threads on per-query paths, such as the lazily resolved dialect and template preparation, turned that into contended cache-line writes. A resolved value is now returned after a single volatile read. --- .../java/st/orm/core/template/impl/LazySupplier.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/storm-core/src/main/java/st/orm/core/template/impl/LazySupplier.java b/storm-core/src/main/java/st/orm/core/template/impl/LazySupplier.java index ceae6d47a..ff9013fb7 100644 --- a/storm-core/src/main/java/st/orm/core/template/impl/LazySupplier.java +++ b/storm-core/src/main/java/st/orm/core/template/impl/LazySupplier.java @@ -68,13 +68,18 @@ public LazySupplier(@Nonnull T initialValue) { /** * Gets a result. On the first invocation, the supplier is called to produce the value. The supplier is then - * released for garbage collection. + * released for garbage collection. A resolved value is returned with a single volatile read, so shared + * suppliers on hot paths stay free of write contention. * * @return a result. */ @Override public T get() { - T result = reference.updateAndGet(value -> requireNonNullElseGet(value, supplier)); + T result = reference.get(); + if (result != null) { + return result; + } + result = reference.updateAndGet(value -> requireNonNullElseGet(value, supplier)); supplier = null; // Release the supplier (and its captured context) for GC. return result; }