Merge pull request #35242 from gsmet/transaction-manager-fixes

Transaction manager-related backports and fixes

extensions/agroal/deployment/src/main/java/io/quarkus/agroal/deployment/AgroalProcessor.java

@@ -266,11 +266,9 @@ void generateDataSourceBeans(AgroalRecorder recorder,
             return;
         }
 
-        for (Map.Entry<String, DataSourceSupport.Entry> entry : getDataSourceSupport(aggregatedBuildTimeConfigBuildItems,
-                sslNativeConfig,
-                capabilities).entries.entrySet()) {
+        for (AggregatedDataSourceBuildTimeConfigBuildItem aggregatedBuildTimeConfigBuildItem : aggregatedBuildTimeConfigBuildItems) {
 
-            String dataSourceName = entry.getKey();
+            String dataSourceName = aggregatedBuildTimeConfigBuildItem.getName();
 
             SyntheticBeanBuildItem.ExtendedBeanConfigurator configurator = SyntheticBeanBuildItem
                     .configure(AgroalDataSource.class)
@@ -282,7 +280,7 @@ void generateDataSourceBeans(AgroalRecorder recorder,
                     // are created after runtime configuration has been set up
                     .supplier(recorder.agroalDataSourceSupplier(dataSourceName, dataSourcesRuntimeConfig));
 
-            if (entry.getValue().isDefault) {
+            if (aggregatedBuildTimeConfigBuildItem.isDefault()) {
                 configurator.addQualifier(Default.class);
             } else {
                 // this definitely not ideal, but 'elytron-jdbc-security' uses it (although it could be easily changed)
@@ -296,9 +294,10 @@ void generateDataSourceBeans(AgroalRecorder recorder,
             syntheticBeanBuildItemBuildProducer.produce(configurator.done());
 
             jdbcDataSource.produce(new JdbcDataSourceBuildItem(dataSourceName,
-                    entry.getValue().resolvedDbKind,
-                    entry.getValue().dbVersion,
-                    entry.getValue().isDefault));
+                    aggregatedBuildTimeConfigBuildItem.getDbKind(),
+                    aggregatedBuildTimeConfigBuildItem.getDataSourceConfig().dbVersion,
+                    aggregatedBuildTimeConfigBuildItem.getJdbcConfig().transactions != TransactionIntegration.DISABLED,
+                    aggregatedBuildTimeConfigBuildItem.isDefault()));
         }
     }
 

extensions/agroal/spi/src/main/java/io/quarkus/agroal/spi/JdbcDataSourceBuildItem.java

@@ -17,12 +17,17 @@ public final class JdbcDataSourceBuildItem extends MultiBuildItem {
     private final String dbKind;
 
     private final Optional<String> dbVersion;
+
+    private final boolean transactionIntegrationEnabled;
+
     private final boolean isDefault;
 
-    public JdbcDataSourceBuildItem(String name, String kind, Optional<String> dbVersion, boolean isDefault) {
+    public JdbcDataSourceBuildItem(String name, String kind, Optional<String> dbVersion,
+            boolean transactionIntegrationEnabled, boolean isDefault) {
         this.name = name;
         this.dbKind = kind;
         this.dbVersion = dbVersion;
+        this.transactionIntegrationEnabled = transactionIntegrationEnabled;
         this.isDefault = isDefault;
     }
 
@@ -38,6 +43,10 @@ public Optional<String> getDbVersion() {
         return dbVersion;
     }
 
+    public boolean isTransactionIntegrationEnabled() {
+        return transactionIntegrationEnabled;
+    }
+
     public boolean isDefault() {
         return isDefault;
     }

extensions/narayana-jta/deployment/src/main/java/io/quarkus/narayana/jta/deployment/NarayanaJtaProcessor.java

@@ -2,10 +2,12 @@
 
 import static io.quarkus.deployment.annotations.ExecutionTime.RUNTIME_INIT;
 
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
 
 import jakarta.annotation.Priority;
 import jakarta.interceptor.Interceptor;
@@ -44,6 +46,7 @@
 import io.quarkus.arc.deployment.GeneratedBeanGizmoAdaptor;
 import io.quarkus.arc.deployment.SyntheticBeansRuntimeInitBuildItem;
 import io.quarkus.arc.deployment.UnremovableBeanBuildItem;
+import io.quarkus.datasource.common.runtime.DataSourceUtil;
 import io.quarkus.deployment.Feature;
 import io.quarkus.deployment.IsTest;
 import io.quarkus.deployment.annotations.BuildProducer;
@@ -155,10 +158,16 @@ public void build(NarayanaJtaRecorder recorder,
     @Consume(SyntheticBeansRuntimeInitBuildItem.class)
     public void startRecoveryService(NarayanaJtaRecorder recorder,
             List<JdbcDataSourceBuildItem> jdbcDataSourceBuildItems, TransactionManagerConfiguration transactions) {
-        Map<Boolean, String> namedDataSources = new HashMap<>();
-
-        jdbcDataSourceBuildItems.forEach(i -> namedDataSources.put(i.isDefault(), i.getName()));
-        recorder.startRecoveryService(transactions, namedDataSources);
+        Map<String, String> configuredDataSourcesConfigKeys = jdbcDataSourceBuildItems.stream()
+                .map(j -> j.getName())
+                .collect(Collectors.toMap(Function.identity(),
+                        n -> DataSourceUtil.dataSourcePropertyKey(n, "jdbc.transactions")));
+        Set<String> dataSourcesWithTransactionIntegration = jdbcDataSourceBuildItems.stream()
+                .filter(j -> j.isTransactionIntegrationEnabled())
+                .map(j -> j.getName())
+                .collect(Collectors.toSet());
+
+        recorder.startRecoveryService(transactions, configuredDataSourcesConfigKeys, dataSourcesWithTransactionIntegration);
     }
 
     @BuildStep(onlyIf = IsTest.class)

extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/quarkus/TransactionJdbcObjectStoreValidationFailureTest.java

@@ -0,0 +1,40 @@
+package io.quarkus.narayana.quarkus;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import java.util.List;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.builder.Version;
+import io.quarkus.maven.dependency.Dependency;
+import io.quarkus.runtime.configuration.ConfigurationException;
+import io.quarkus.runtime.util.ExceptionUtil;
+import io.quarkus.test.QuarkusUnitTest;
+
+public class TransactionJdbcObjectStoreValidationFailureTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest config = new QuarkusUnitTest()
+            .withApplicationRoot((jar) -> jar
+                    .addAsResource("jdbc-object-store-validation.properties", "application.properties"))
+            .setForcedDependencies(List.of(Dependency.of("io.quarkus", "quarkus-jdbc-h2", Version.getVersion())))
+            .assertException(t -> {
+                Throwable rootCause = ExceptionUtil.getRootCause(t);
+                if (rootCause instanceof ConfigurationException) {
+                    assertTrue(rootCause.getMessage().contains(
+                            "The Narayana JTA extension is configured to use the datasource 'test' but that datasource is not configured."));
+                } else {
+                    fail(t);
+                }
+            });
+
+    @Test
+    public void test() {
+        // needs to be there in order to run test
+        Assertions.fail("Application was supposed to fail.");
+    }
+}

extensions/narayana-jta/deployment/src/test/resources/jdbc-object-store-validation.properties

@@ -0,0 +1,9 @@
+quarkus.transaction-manager.object-store.type=jdbc
+quarkus.transaction-manager.object-store.datasource=test
+quarkus.datasource.test.db-kind=h2
+quarkus.datasource.test.jdbc.url=jdbc:h2:mem:default
+quarkus.datasource.test.jdbc.transactions=xa
+
+# we must not start database as CI is also executed on Windows without (Docker) Linux containers
+quarkus.datasource.devservices.enabled=false
+quarkus.devservices.enabled=false

extensions/narayana-jta/runtime/pom.xml

@@ -29,6 +29,10 @@
             <groupId>io.quarkus</groupId>
             <artifactId>quarkus-mutiny</artifactId>
         </dependency>
+        <dependency>
+            <groupId>io.quarkus</groupId>
+            <artifactId>quarkus-datasource-common</artifactId>
+        </dependency>
         <dependency>
             <groupId>io.smallrye</groupId>
             <artifactId>smallrye-context-propagation-jta</artifactId>

extensions/narayana-jta/runtime/src/main/java/io/quarkus/narayana/jta/runtime/NarayanaJtaRecorder.java

@@ -6,6 +6,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
+import java.util.Set;
 
 import org.jboss.logging.Logger;
 
@@ -15,13 +16,13 @@
 import com.arjuna.ats.arjuna.common.arjPropertyManager;
 import com.arjuna.ats.arjuna.coordinator.TransactionReaper;
 import com.arjuna.ats.arjuna.coordinator.TxControl;
-import com.arjuna.ats.arjuna.recovery.RecoveryManager;
 import com.arjuna.ats.internal.arjuna.objectstore.jdbc.JDBCStore;
 import com.arjuna.ats.jta.common.JTAEnvironmentBean;
 import com.arjuna.ats.jta.common.jtaPropertyManager;
 import com.arjuna.common.internal.util.propertyservice.BeanPopulator;
 import com.arjuna.common.util.propertyservice.PropertiesFactory;
 
+import io.quarkus.datasource.common.runtime.DataSourceUtil;
 import io.quarkus.runtime.ShutdownContext;
 import io.quarkus.runtime.annotations.Recorder;
 import io.quarkus.runtime.configuration.ConfigurationException;
@@ -104,29 +105,47 @@ private void setJDBCObjectStore(String name, TransactionManagerConfiguration con
         instance.setTablePrefix(config.objectStore.tablePrefix);
     }
 
-    public void startRecoveryService(final TransactionManagerConfiguration transactions, Map<Boolean, String> dataSources) {
+    public void startRecoveryService(final TransactionManagerConfiguration transactions,
+            Map<String, String> configuredDataSourcesConfigKeys,
+            Set<String> dataSourcesWithTransactionIntegration) {
+
         if (transactions.objectStore.type.equals(ObjectStoreType.JDBC)) {
+            final String objectStoreDataSourceName;
             if (transactions.objectStore.datasource.isEmpty()) {
-                dataSources.keySet().stream().filter(i -> i).findFirst().orElseThrow(
-                        () -> new ConfigurationException(
-                                "The Narayana JTA extension does not have a datasource configured,"
-                                        + " so it defaults to the default datasource,"
-                                        + " but that datasource is not configured."
-                                        + " To solve this, either configure the default datasource,"
-                                        + " referring to https://quarkus.io/guides/datasource for guidance,"
-                                        + " or configure the datasource to use in the Narayana JTA extension "
-                                        + " by setting property 'quarkus.transaction-manager.object-store.datasource' to the name of a configured datasource."));
+                if (!DataSourceUtil.hasDefault(configuredDataSourcesConfigKeys.keySet())) {
+                    throw new ConfigurationException(
+                            "The Narayana JTA extension does not have a datasource configured as the JDBC object store,"
+                                    + " so it defaults to the default datasource,"
+                                    + " but that datasource is not configured."
+                                    + " To solve this, either configure the default datasource,"
+                                    + " referring to https://quarkus.io/guides/datasource for guidance,"
+                                    + " or configure the datasource to use in the Narayana JTA extension "
+                                    + " by setting property 'quarkus.transaction-manager.object-store.datasource' to the name of a configured datasource.");
+                }
+                objectStoreDataSourceName = DataSourceUtil.DEFAULT_DATASOURCE_NAME;
             } else {
-                String dsName = transactions.objectStore.datasource.get();
-                dataSources.values().stream().filter(i -> i.equals(dsName)).findFirst()
-                        .orElseThrow(() -> new ConfigurationException(
-                                "The Narayana JTA extension is configured to use the datasource '"
-                                        + dsName
-                                        + "' but that datasource is not configured."
-                                        + " To solve this, either configure datasource " + dsName
-                                        + " referring to https://quarkus.io/guides/datasource for guidance,"
-                                        + " or configure another datasource to use in the Narayana JTA extension "
-                                        + " by setting property 'quarkus.transaction-manager.object-store.datasource' to the name of a configured datasource."));
+                objectStoreDataSourceName = transactions.objectStore.datasource.get();
+
+                if (!configuredDataSourcesConfigKeys.keySet().contains(objectStoreDataSourceName)) {
+                    throw new ConfigurationException(
+                            "The Narayana JTA extension is configured to use the datasource '"
+                                    + objectStoreDataSourceName
+                                    + "' but that datasource is not configured."
+                                    + " To solve this, either configure datasource " + objectStoreDataSourceName
+                                    + " referring to https://quarkus.io/guides/datasource for guidance,"
+                                    + " or configure another datasource to use in the Narayana JTA extension "
+                                    + " by setting property 'quarkus.transaction-manager.object-store.datasource' to the name of a configured datasource.");
+                }
+            }
+            if (dataSourcesWithTransactionIntegration.contains(objectStoreDataSourceName)) {
+                throw new ConfigurationException(String.format(
+                        "The Narayana JTA extension is configured to use the '%s' JDBC "
+                                + "datasource as the transaction log storage, "
+                                + "but that datasource does not have transaction capabilities disabled. "
+                                + "To solve this, please set '%s=disabled', or configure another datasource "
+                                + "with disabled transaction capabilities as the JDBC object store. "
+                                + "Please refer to the https://quarkus.io/guides/transaction#jdbcstore for more information.",
+                        objectStoreDataSourceName, configuredDataSourcesConfigKeys.get(objectStoreDataSourceName)));
             }
         }
         if (transactions.enableRecovery) {
@@ -136,10 +155,19 @@ public void startRecoveryService(final TransactionManagerConfiguration transacti
     }
 
     public void handleShutdown(ShutdownContext context, TransactionManagerConfiguration transactions) {
-        context.addLastShutdownTask(() -> {
+        context.addShutdownTask(() -> {
             if (transactions.enableRecovery) {
-                RecoveryManager.manager().terminate(true);
+                try {
+                    QuarkusRecoveryService.getInstance().stop();
+                } catch (Exception e) {
+                    // the recovery manager throws IllegalStateException if it has already been shutdown
+                    log.warn("The recovery manager has already been shutdown", e);
+                } finally {
+                    QuarkusRecoveryService.getInstance().destroy();
+                }
             }
+        });
+        context.addLastShutdownTask(() -> {
             TransactionReaper.terminate(false);
         });
     }

extensions/narayana-jta/runtime/src/main/java/io/quarkus/narayana/jta/runtime/QuarkusRecoveryService.java

@@ -51,4 +51,11 @@ public void create() {
         }
         xaResources.clear();
     }
+
+    @Override
+    public void destroy() {
+        super.destroy();
+        isCreated = false;
+        recoveryManagerService = null;
+    }
 }

integration-tests/narayana-jta/src/main/resources/application.properties

@@ -11,3 +11,6 @@ quarkus.log.category."io.quarkus".level=INFO
 quarkus.datasource.db-kind=h2
 
 quarkus.transaction-manager.object-store.directory=target/tx-object-store
+
+quarkus.datasource.test.jdbc.transactions=disabled
+quarkus.datasource.test.db-kind=h2

integration-tests/narayana-jta/src/test/java/io/quarkus/narayana/jta/JdbcObjectStoreTestProfile.java

@@ -11,11 +11,10 @@ public Map<String, String> getConfigOverrides() {
         HashMap<String, String> props = new HashMap<>();
         props.put("quarkus.transaction-manager.object-store.type", "jdbc");
         props.put("quarkus.transaction-manager.object-store.create-table", "true");
+        props.put("quarkus.transaction-manager.object-store.datasource", "test");
         props.put("quarkus.transaction-manager.enable-recovery", "true");
 
-        props.put("quarkus.datasource.test.db-kind", "h2");
         props.put("quarkus.datasource.test.jdbc.url", "jdbc:h2:mem:default");
-        props.put("quarkus.datasource.test.jdbc.transactions", "xa");
 
         return props;
     }