From 083154bba57ca69f373eb71b4a35139e1976ded5 Mon Sep 17 00:00:00 2001 From: Leon van Zantvoort Date: Mon, 10 Aug 2026 22:38:53 +0200 Subject: [PATCH] fix: resolve JPA transaction managers for Storm-initiated transactions The transaction bridge resolved the manager for a DataSource by filtering on DataSourceTransactionManager. An application with spring-boot-starter-data-jpa gets a JpaTransactionManager instead, so Storm-initiated transaction blocks failed with "No TransactionManager found for DataSource" while @Transactional kept working through DataSourceUtils. Resolution now matches any ResourceTransactionManager working directly on the DataSource, plus a JpaTransactionManager whose entity manager factory is backed by it. The JPA branch sits behind a class-presence check with spring-orm as an optional dependency, so JDBC-only applications are unaffected. When several managers own the same DataSource, resolution fails fast naming the candidates: the choice decides which manager completes the transaction, so it must be made by configuration rather than list order. The transaction auto-configuration also gains ordering hints for the Hibernate JPA auto-configuration (Spring Boot 3 and 4 locations), so @ConditionalOnBean(PlatformTransactionManager) sees the JPA-registered manager. Fixes #383 --- storm-spring/pom.xml | 11 + storm-spring/src/main/java/module-info.java | 2 + .../SpringTransactionTemplateProvider.java | 5 +- .../StormTransactionAutoConfiguration.java | 15 +- .../spring/impl/SpringTransactionContext.java | 73 ++++++- .../SpringJpaTransactionBridgeTest.java | 195 ++++++++++++++++++ ...StormTransactionAutoConfigurationTest.java | 84 ++++++++ 7 files changed, 371 insertions(+), 14 deletions(-) create mode 100644 storm-spring/src/test/java/st/orm/spring/SpringJpaTransactionBridgeTest.java create mode 100644 storm-spring/src/test/java/st/orm/spring/boot/StormTransactionAutoConfigurationTest.java diff --git a/storm-spring/pom.xml b/storm-spring/pom.xml index 7e1683239..442893d09 100644 --- a/storm-spring/pom.xml +++ b/storm-spring/pom.xml @@ -141,6 +141,17 @@ spring-tx provided + + + org.springframework + spring-orm + provided + + + jakarta.persistence + jakarta.persistence-api + provided + org.springframework diff --git a/storm-spring/src/main/java/module-info.java b/storm-spring/src/main/java/module-info.java index 14ce0232b..bbb71c6ea 100644 --- a/storm-spring/src/main/java/module-info.java +++ b/storm-spring/src/main/java/module-info.java @@ -5,6 +5,8 @@ requires static micrometer.tracing; requires static micrometer.commons; requires spring.jdbc; + requires static spring.orm; + requires static jakarta.persistence; requires spring.tx; requires spring.context; requires spring.beans; diff --git a/storm-spring/src/main/java/st/orm/spring/SpringTransactionTemplateProvider.java b/storm-spring/src/main/java/st/orm/spring/SpringTransactionTemplateProvider.java index c9688ed8c..ba37ef435 100644 --- a/storm-spring/src/main/java/st/orm/spring/SpringTransactionTemplateProvider.java +++ b/storm-spring/src/main/java/st/orm/spring/SpringTransactionTemplateProvider.java @@ -83,8 +83,9 @@ public SpringTransactionTemplateProvider() { /** * Creates a provider that bridges Storm-initiated transactions through the given transaction managers; the - * matching {@code DataSourceTransactionManager} is resolved lazily, when the first data source touches the - * transaction. + * manager owning the touched {@code DataSource} is resolved lazily, when the first data source touches the + * transaction. Both JDBC transaction managers and JPA transaction managers backed by the data source + * qualify. * * @param transactionManagers supplies the transaction managers of the owning application context. */ diff --git a/storm-spring/src/main/java/st/orm/spring/boot/StormTransactionAutoConfiguration.java b/storm-spring/src/main/java/st/orm/spring/boot/StormTransactionAutoConfiguration.java index f82dbf0a0..7c8841863 100644 --- a/storm-spring/src/main/java/st/orm/spring/boot/StormTransactionAutoConfiguration.java +++ b/storm-spring/src/main/java/st/orm/spring/boot/StormTransactionAutoConfiguration.java @@ -36,19 +36,22 @@ * {@code ORMTemplate} created by the starter's auto-configuration; nothing is registered globally. Define your * own {@link ConnectionProvider} or {@link TransactionTemplateProvider} bean to override.

* - *

The ordering hints reference DataSourceTransactionManagerAutoConfiguration by name rather than by class - * literal: the class moved to the modular spring-boot-jdbc jar in Spring Boot 4, and a class literal to - * whichever location is absent would fail annotation introspection. Name-based hints are ignored when the - * class is not on the classpath, so both locations can be listed safely.

+ *

The ordering hints cover both auto-configurations that register a transaction manager: the JDBC one and, + * for applications with JPA on the class path where the JDBC one backs off, the Hibernate JPA one. They + * reference the classes by name rather than by class literal: the classes moved to modular jars in Spring + * Boot 4, and a class literal to whichever location is absent would fail annotation introspection. Name-based + * hints are ignored when the class is not on the classpath, so both locations can be listed safely.

* * @since 1.13 */ @AutoConfiguration( afterName = { - // Spring Boot 3.x location. + // Spring Boot 3.x locations. "org.springframework.boot.autoconfigure.jdbc.DataSourceTransactionManagerAutoConfiguration", - // Spring Boot 4.x location. + "org.springframework.boot.autoconfigure.orm.jpa.HibernateJpaAutoConfiguration", + // Spring Boot 4.x locations. "org.springframework.boot.jdbc.autoconfigure.DataSourceTransactionManagerAutoConfiguration", + "org.springframework.boot.hibernate.autoconfigure.HibernateJpaAutoConfiguration", }) @ConditionalOnBean(PlatformTransactionManager.class) public class StormTransactionAutoConfiguration { diff --git a/storm-spring/src/main/java/st/orm/spring/impl/SpringTransactionContext.java b/storm-spring/src/main/java/st/orm/spring/impl/SpringTransactionContext.java index 6ccd5f43e..b87ee5bb9 100644 --- a/storm-spring/src/main/java/st/orm/spring/impl/SpringTransactionContext.java +++ b/storm-spring/src/main/java/st/orm/spring/impl/SpringTransactionContext.java @@ -38,13 +38,16 @@ import java.util.Map; import java.util.Optional; import java.util.function.Supplier; +import java.util.stream.Collectors; import javax.sql.DataSource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.jdbc.datasource.DataSourceTransactionManager; +import org.springframework.orm.jpa.JpaTransactionManager; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.TransactionDefinition; import org.springframework.transaction.TransactionStatus; +import org.springframework.transaction.support.ResourceTransactionManager; +import org.springframework.util.ClassUtils; import st.orm.Entity; import st.orm.PersistenceException; import st.orm.TransactionTimedOutException; @@ -409,17 +412,75 @@ public void setRollbackOnly() { } } + /** + * Resolves the transaction manager that owns the given data source. + * + *

A manager owns the data source when it is a {@link ResourceTransactionManager} working directly on it, + * which covers {@code DataSourceTransactionManager} and {@code JdbcTransactionManager}, or a JPA transaction + * manager whose entity manager factory is backed by it, which is what Spring Boot registers when JPA is on + * the class path. Resolution fails fast when several managers own the same data source: the choice decides + * which manager completes Storm-initiated transactions, so it must be made by configuration rather than by + * list order.

+ */ private PlatformTransactionManager resolveTransactionManager(@Nonnull DataSource dataSource) { - return transactionManagers.get().stream() - .filter(DataSourceTransactionManager.class::isInstance) - .map(DataSourceTransactionManager.class::cast) - .filter(manager -> manager.getDataSource() == dataSource) - .map(PlatformTransactionManager.class::cast) + var candidates = transactionManagers.get().stream() + .filter(manager -> managesDataSource(manager, dataSource)) + .toList(); + if (candidates.size() > 1) { + throw new IllegalStateException( + "Multiple TransactionManagers found for DataSource " + dataSource + ": " + + candidates.stream() + .map(manager -> manager.getClass().getName()) + .collect(Collectors.joining(", ")) + + ". Keep a single transaction manager per DataSource, or define a " + + "TransactionTemplateProvider bean constructed with the manager that must own " + + "Storm-initiated transactions."); + } + return candidates.stream() .findFirst() .orElseThrow(() -> new IllegalStateException( "No TransactionManager found for DataSource " + dataSource + ".")); } + private static final boolean JPA_PRESENT = ClassUtils.isPresent( + "org.springframework.orm.jpa.JpaTransactionManager", + SpringTransactionContext.class.getClassLoader()); + + private static boolean managesDataSource(@Nonnull PlatformTransactionManager manager, + @Nonnull DataSource dataSource) { + if (JPA_PRESENT && JpaSupport.isJpaTransactionManager(manager)) { + return JpaSupport.managesDataSource(manager, dataSource); + } + if (manager instanceof ResourceTransactionManager resourceManager) { + return resourceFactoryOrNull(resourceManager) == dataSource; + } + return false; + } + + @Nullable + private static Object resourceFactoryOrNull(@Nonnull ResourceTransactionManager manager) { + try { + return manager.getResourceFactory(); + } catch (IllegalStateException e) { + // The manager has no resource factory configured, so it owns no data source. + return null; + } + } + + /** + * Touches spring-orm types; only loaded when spring-orm is on the class path. + */ + private static final class JpaSupport { + static boolean isJpaTransactionManager(@Nonnull PlatformTransactionManager manager) { + return manager instanceof JpaTransactionManager; + } + + static boolean managesDataSource(@Nonnull PlatformTransactionManager manager, + @Nonnull DataSource dataSource) { + return manager instanceof JpaTransactionManager jpaManager && jpaManager.getDataSource() == dataSource; + } + } + /** * Starts a Spring TransactionStatus for the given state if not already started. * diff --git a/storm-spring/src/test/java/st/orm/spring/SpringJpaTransactionBridgeTest.java b/storm-spring/src/test/java/st/orm/spring/SpringJpaTransactionBridgeTest.java new file mode 100644 index 000000000..c586f779f --- /dev/null +++ b/storm-spring/src/test/java/st/orm/spring/SpringJpaTransactionBridgeTest.java @@ -0,0 +1,195 @@ +/* + * 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.spring; + +import static java.util.Objects.requireNonNull; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static st.orm.TransactionPropagation.REQUIRES_NEW; +import static st.orm.template.Transactions.transaction; + +import jakarta.persistence.EntityManagerFactory; +import java.time.Instant; +import java.time.LocalDate; +import java.util.List; +import javax.sql.DataSource; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.boot.jdbc.DataSourceBuilder; +import org.springframework.core.io.ClassPathResource; +import org.springframework.jdbc.datasource.DataSourceTransactionManager; +import org.springframework.jdbc.datasource.init.ResourceDatabasePopulator; +import org.springframework.orm.jpa.JpaTransactionManager; +import org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean; +import org.springframework.orm.jpa.vendor.HibernateJpaVendorAdapter; +import st.orm.repository.EntityRepository; +import st.orm.spring.model.Pet; +import st.orm.spring.model.Visit; +import st.orm.template.ORMTemplate; +import st.orm.template.Transactions; + +/** + * Verifies that Storm's programmatic transaction API ({@link Transactions}) is bridged into a + * {@link JpaTransactionManager}, which is the transaction manager a Spring Boot application gets when JPA is + * on the class path. The manager is matched by the {@code DataSource} backing its entity manager factory, so + * Storm-initiated transactions and JPA share one transaction system without a + * {@code DataSourceTransactionManager} being present. + */ +class SpringJpaTransactionBridgeTest { + + private DataSource dataSource; + private LocalContainerEntityManagerFactoryBean entityManagerFactoryBean; + private JpaTransactionManager transactionManager; + private ORMTemplate orm; + private EntityRepository visits; + private Pet pet; + + @BeforeEach + void setUp() { + dataSource = DataSourceBuilder.create() + .url("jdbc:h2:mem:jpabridgetest;DB_CLOSE_DELAY=-1;DB_CLOSE_ON_EXIT=false") + .username("sa") + .password("") + .driverClassName("org.h2.Driver") + .build(); + new ResourceDatabasePopulator(new ClassPathResource("data.sql")).execute(dataSource); + entityManagerFactoryBean = new LocalContainerEntityManagerFactoryBean(); + entityManagerFactoryBean.setDataSource(dataSource); + entityManagerFactoryBean.setJpaVendorAdapter(new HibernateJpaVendorAdapter()); + entityManagerFactoryBean.setPersistenceUnitName("jpa-bridge-test"); + // The persistence unit carries no JPA entities; it exists to back the transaction manager. + entityManagerFactoryBean.setPackagesToScan(getClass().getPackageName()); + entityManagerFactoryBean.afterPropertiesSet(); + EntityManagerFactory entityManagerFactory = requireNonNull(entityManagerFactoryBean.getObject()); + transactionManager = new JpaTransactionManager(entityManagerFactory); + orm = SpringOrmTemplate.of(dataSource, () -> List.of(transactionManager)); + visits = orm.entity(Visit.class); + pet = orm.entity(Pet.class).getById(1); + } + + @AfterEach + void tearDown() { + entityManagerFactoryBean.destroy(); + } + + private void insertVisit(String description) { + visits.insert(new Visit(null, LocalDate.now(), description, pet, Instant.now())); + } + + @Test + void programmaticTransactionCommitsThroughJpaManager() { + long before = visits.count(); + transaction(tx -> { + insertVisit("committed"); + return null; + }); + assertEquals(before + 1, visits.count()); + } + + @Test + void programmaticRollbackDiscardsWrites() { + long before = visits.count(); + transaction(tx -> { + insertVisit("discarded"); + assertEquals(before + 1, visits.count()); + tx.setRollbackOnly(); + return null; + }); + assertEquals(before, visits.count()); + } + + @Test + void requiresNewCommitsIndependentlyOfOuterRollback() { + long before = visits.count(); + transaction(outer -> { + insertVisit("outer, discarded"); + transaction(REQUIRES_NEW, inner -> { + insertVisit("inner, committed"); + return null; + }); + outer.setRollbackOnly(); + return null; + }); + assertEquals(before + 1, visits.count()); + } + + @Test + void stormBlockJoinsJpaManagedTransaction() { + long before = visits.count(); + var springTransaction = new org.springframework.transaction.support.TransactionTemplate(transactionManager); + springTransaction.executeWithoutResult(status -> { + // A Storm programmatic block inside a JPA-managed transaction joins it (REQUIRED). + transaction(tx -> { + insertVisit("joined, discarded"); + return null; + }); + assertEquals(before + 1, visits.count()); + status.setRollbackOnly(); + }); + // The Spring rollback discarded the write made by the joined Storm block. + assertEquals(before, visits.count()); + } + + @Test + void jpaManagerForAnotherDataSourceIsNotMatched() { + DataSource otherDataSource = DataSourceBuilder.create() + .url("jdbc:h2:mem:jpabridgeother;DB_CLOSE_DELAY=-1;DB_CLOSE_ON_EXIT=false") + .username("sa") + .password("") + .driverClassName("org.h2.Driver") + .build(); + // Resolution inspects the manager's DataSource only, so no entity manager factory is needed. + var otherManager = new JpaTransactionManager(); + otherManager.setDataSource(otherDataSource); + ORMTemplate bridged = SpringOrmTemplate.of(dataSource, + () -> List.of(otherManager, new DataSourceTransactionManager(dataSource))); + EntityRepository bridgedVisits = bridged.entity(Visit.class); + long before = bridgedVisits.count(); + transaction(tx -> { + bridgedVisits.insert(new Visit(null, LocalDate.now(), "committed", pet, Instant.now())); + return null; + }); + assertEquals(before + 1, bridgedVisits.count()); + } + + @Test + void multipleManagersForTheSameDataSourceFailFast() { + ORMTemplate ambiguous = SpringOrmTemplate.of(dataSource, + () -> List.of(new DataSourceTransactionManager(dataSource), transactionManager)); + EntityRepository ambiguousVisits = ambiguous.entity(Visit.class); + long before = ambiguousVisits.count(); + Exception exception = assertThrows(Exception.class, () -> + transaction(tx -> { + ambiguousVisits.insert(new Visit(null, LocalDate.now(), "never", pet, Instant.now())); + return null; + })); + String message = messageChain(exception); + assertTrue(message.contains("Multiple TransactionManagers found"), message); + assertTrue(message.contains(DataSourceTransactionManager.class.getName()), message); + assertTrue(message.contains(JpaTransactionManager.class.getName()), message); + assertEquals(before, ambiguousVisits.count()); + } + + private static String messageChain(Throwable throwable) { + var builder = new StringBuilder(); + for (Throwable current = throwable; current != null; current = current.getCause()) { + builder.append(current.getMessage()).append('\n'); + } + return builder.toString(); + } +} diff --git a/storm-spring/src/test/java/st/orm/spring/boot/StormTransactionAutoConfigurationTest.java b/storm-spring/src/test/java/st/orm/spring/boot/StormTransactionAutoConfigurationTest.java new file mode 100644 index 000000000..e948c3e32 --- /dev/null +++ b/storm-spring/src/test/java/st/orm/spring/boot/StormTransactionAutoConfigurationTest.java @@ -0,0 +1,84 @@ +/* + * 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.spring.boot; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + +import org.junit.jupiter.api.Test; +import org.springframework.boot.autoconfigure.AutoConfigurations; +import org.springframework.boot.autoconfigure.jdbc.DataSourceAutoConfiguration; +import org.springframework.boot.autoconfigure.jdbc.DataSourceTransactionManagerAutoConfiguration; +import org.springframework.boot.autoconfigure.orm.jpa.HibernateJpaAutoConfiguration; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.jdbc.support.JdbcTransactionManager; +import org.springframework.orm.jpa.JpaTransactionManager; +import org.springframework.transaction.PlatformTransactionManager; +import st.orm.core.spi.ConnectionProvider; +import st.orm.core.spi.TransactionTemplateProvider; + +/** + * Verifies that the transaction integration activates for every transaction manager a Spring Boot application + * can end up with: the JDBC manager, and the JPA manager that replaces it when JPA is on the class path. The + * Storm auto-configuration is listed first so the test relies on the declared ordering hints, not on the + * listing order, to see the manager bean. + */ +public class StormTransactionAutoConfigurationTest { + + @Test + void activatesWithJdbcTransactionManager() { + new ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of( + StormTransactionAutoConfiguration.class, + DataSourceAutoConfiguration.class, + DataSourceTransactionManagerAutoConfiguration.class)) + .withPropertyValues("spring.datasource.url=jdbc:h2:mem:tx-autoconfig-jdbc;DB_CLOSE_DELAY=-1") + .run(context -> { + assertInstanceOf(JdbcTransactionManager.class, context.getBean(PlatformTransactionManager.class)); + assertEquals(1, context.getBeansOfType(ConnectionProvider.class).size()); + assertEquals(1, context.getBeansOfType(TransactionTemplateProvider.class).size()); + }); + } + + @Test + void activatesWithJpaTransactionManager() { + new ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of( + StormTransactionAutoConfiguration.class, + DataSourceAutoConfiguration.class, + HibernateJpaAutoConfiguration.class)) + .withPropertyValues("spring.datasource.url=jdbc:h2:mem:tx-autoconfig-jpa;DB_CLOSE_DELAY=-1") + .run(context -> { + assertInstanceOf(JpaTransactionManager.class, context.getBean(PlatformTransactionManager.class)); + assertEquals(1, context.getBeansOfType(ConnectionProvider.class).size()); + assertEquals(1, context.getBeansOfType(TransactionTemplateProvider.class).size()); + }); + } + + @Test + void backsOffWithoutTransactionManager() { + new ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of( + StormTransactionAutoConfiguration.class, + DataSourceAutoConfiguration.class)) + .withPropertyValues("spring.datasource.url=jdbc:h2:mem:tx-autoconfig-none;DB_CLOSE_DELAY=-1") + .run(context -> { + assertFalse(context.containsBean("stormConnectionProvider")); + assertFalse(context.containsBean("stormTransactionTemplateProvider")); + }); + } +}