Merge pull request #17357 from marcelhanser/configuration-transactions

Configure transaction timeouts via properties

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

@@ -31,4 +31,19 @@
      * @return The transaction timeout in seconds.
      */
     int timeout() default UNSET_TIMEOUT;
+
+    String UNSET_TIMEOUT_CONFIG_PROPERTY = "<<unset>>";
+
+    /**
+     * The configuration property to use in order to determine the value of the timeout in seconds.
+     * If the property exists, it must be an integer value representing the transaction timeout in seconds.
+     *
+     * An example configuration in {@code application.properties} could be: {@code my-transaction.timeout=5}.
+     *
+     * If both {@code timeoutFromConfigProperty} and {@code timeout} are set, then Quarkus will attempt to resolve
+     * {@code timeoutFromConfigProperty} and if a value for it has been provided, the timeout is set to that value.
+     * If no value has been provided at runtime for the property, then the value of {@code timeout} will be used
+     * as the fallback.
+     */
+    String timeoutFromConfigProperty() default UNSET_TIMEOUT_CONFIG_PROPERTY;
 }

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

@@ -2,10 +2,14 @@
 
 import java.io.Serializable;
 import java.lang.annotation.Annotation;
+import java.lang.reflect.Method;
+import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.CompletionException;
 import java.util.concurrent.CompletionStage;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Function;
 
 import javax.inject.Inject;
 import javax.interceptor.InvocationContext;
@@ -15,6 +19,8 @@
 import javax.transaction.TransactionManager;
 import javax.transaction.Transactional;
 
+import org.eclipse.microprofile.config.ConfigProvider;
+import org.jboss.logging.Logger;
 import org.jboss.tm.usertx.client.ServerVMClientUserTransaction;
 import org.reactivestreams.Publisher;
 
@@ -27,13 +33,11 @@
 import io.smallrye.reactive.converters.ReactiveTypeConverter;
 import io.smallrye.reactive.converters.Registry;
 
-/**
- * @author [email protected] 02/05/2013
- */
-
 public abstract class TransactionalInterceptorBase implements Serializable {
 
     private static final long serialVersionUID = 1L;
+    private static final Logger log = Logger.getLogger(TransactionalInterceptorBase.class);
+    private final Map<Method, Integer> methodTransactionTimeoutDefinedByPropertyCache = new ConcurrentHashMap<>();
 
     @Inject
     TransactionManager transactionManager;
@@ -66,8 +70,7 @@ public Object intercept(InvocationContext ic) throws Exception {
      * Method handles CDI types to cover cases where extensions are used. In
      * case of EE container uses reflection.
      *
-     * @param ic
-     *        invocation context of the interceptor
+     * @param ic invocation context of the interceptor
      * @return instance of {@link Transactional} annotation or null
      */
     private Transactional getTransactional(InvocationContext ic) {
@@ -104,18 +107,20 @@ protected Object invokeInOurTx(InvocationContext ic, TransactionManager tm) thro
     protected Object invokeInOurTx(InvocationContext ic, TransactionManager tm, RunnableWithException afterEndTransaction)
             throws Exception {
 
-        TransactionConfiguration configAnnotation = getTransactionConfiguration(ic);
+        int timeoutConfiguredForMethod = getTransactionTimeoutFromAnnotation(ic);
+
         int currentTmTimeout = ((CDIDelegatingTransactionManager) transactionManager).getTransactionTimeout();
-        if (configAnnotation != null && configAnnotation.timeout() != TransactionConfiguration.UNSET_TIMEOUT) {
-            tm.setTransactionTimeout(configAnnotation.timeout());
+
+        if (timeoutConfiguredForMethod > 0) {
+            tm.setTransactionTimeout(timeoutConfiguredForMethod);
         }
+
         Transaction tx;
         try {
             tm.begin();
             tx = tm.getTransaction();
         } finally {
-            if (configAnnotation != null && configAnnotation.timeout() != TransactionConfiguration.UNSET_TIMEOUT) {
-                //restore the default behaviour
+            if (timeoutConfiguredForMethod > 0) {
                 tm.setTransactionTimeout(currentTmTimeout);
             }
         }
@@ -167,6 +172,51 @@ protected Object invokeInOurTx(InvocationContext ic, TransactionManager tm, Runn
         return ret;
     }
 
+    private int getTransactionTimeoutFromAnnotation(InvocationContext ic) {
+        TransactionConfiguration configAnnotation = getTransactionConfiguration(ic);
+
+        if (configAnnotation == null) {
+            return -1;
+        }
+
+        int transactionTimeout = -1;
+
+        if (!configAnnotation.timeoutFromConfigProperty().equals(TransactionConfiguration.UNSET_TIMEOUT_CONFIG_PROPERTY)) {
+            Integer timeoutForMethod = methodTransactionTimeoutDefinedByPropertyCache.get(ic.getMethod());
+            if (timeoutForMethod != null) {
+                transactionTimeout = timeoutForMethod;
+            } else {
+                transactionTimeout = methodTransactionTimeoutDefinedByPropertyCache.computeIfAbsent(ic.getMethod(),
+                        new Function<Method, Integer>() {
+                            @Override
+                            public Integer apply(Method m) {
+                                return TransactionalInterceptorBase.this.getTransactionTimeoutPropertyValue(configAnnotation);
+                            }
+                        });
+            }
+        }
+
+        if (transactionTimeout == -1 && (configAnnotation.timeout() != TransactionConfiguration.UNSET_TIMEOUT)) {
+            transactionTimeout = configAnnotation.timeout();
+        }
+
+        return transactionTimeout;
+    }
+
+    private Integer getTransactionTimeoutPropertyValue(TransactionConfiguration configAnnotation) {
+        Optional<Integer> configTimeout = ConfigProvider.getConfig()
+                .getOptionalValue(configAnnotation.timeoutFromConfigProperty(), Integer.class);
+        if (configTimeout.isEmpty()) {
+            if (log.isDebugEnabled()) {
+                log.debugf("Configuration property '%s' was not provided, so it will not affect the transaction's timeout.",
+                        configAnnotation.timeoutFromConfigProperty());
+            }
+            return -1;
+        }
+
+        return configTimeout.get();
+    }
+
     protected Object handleAsync(TransactionManager tm, Transaction tx, InvocationContext ic, Object ret,
             RunnableWithException afterEndTransaction) throws Exception {
         // Suspend the transaction to remove it from the main request thread
@@ -263,7 +313,9 @@ protected Object invokeInNoTx(InvocationContext ic) throws Exception {
 
     private void checkConfiguration(InvocationContext ic) {
         TransactionConfiguration configAnnotation = getTransactionConfiguration(ic);
-        if (configAnnotation != null && configAnnotation.timeout() != TransactionConfiguration.UNSET_TIMEOUT) {
+        if (configAnnotation != null && ((configAnnotation.timeout() != TransactionConfiguration.UNSET_TIMEOUT)
+                || !TransactionConfiguration.UNSET_TIMEOUT_CONFIG_PROPERTY
+                        .equals(configAnnotation.timeoutFromConfigProperty()))) {
             throw new RuntimeException("Changing timeout via @TransactionConfiguration can only be done " +
                     "at the entry level of a transaction");
         }
@@ -333,7 +385,7 @@ protected void resetUserTransactionAvailability(boolean previousUserTransactionA
      * An utility method to throw any exception as a {@link RuntimeException}.
      * We may throw a checked exception (subtype of {@code Throwable} or {@code Exception}) as un-checked exception.
      * This considers the Java 8 inference rule that states that a {@code throws E} is inferred as {@code RuntimeException}.
-     *
+     * <p>
      * This method can be used in {@code throw} statement such as: {@code throw sneakyThrow(exception);}.
      */
     @SuppressWarnings("unchecked")

integration-tests/jpa-h2/src/main/java/io/quarkus/it/jpa/h2/proxy/ProxyTestEndpoint.java

@@ -13,6 +13,7 @@
 import javax.servlet.http.HttpServletResponse;
 import javax.transaction.Transactional;
 
+import io.quarkus.narayana.jta.runtime.TransactionConfiguration;
 import io.quarkus.runtime.StartupEvent;
 
 @WebServlet(urlPatterns = "/jpa-h2/testproxy")
@@ -23,6 +24,7 @@ public class ProxyTestEndpoint extends HttpServlet {
     EntityManager entityManager;
 
     @Transactional
+    @TransactionConfiguration(timeoutFromConfigProperty = "dummy.transaction.timeout")
     public void setup(@Observes StartupEvent startupEvent) {
         Pet pet = new Pet();
         pet.setId(1);

integration-tests/jpa-h2/src/main/resources/application.properties

@@ -1 +1,3 @@
 quarkus.datasource.jdbc.max-size=8
+dummy.transaction.timeout=30
+transaction.timeout.1s=1

integration-tests/jpa-h2/src/test/java/io/quarkus/it/jpa/h2/DummyTransactionalService.java

@@ -0,0 +1,51 @@
+package io.quarkus.it.jpa.h2;
+
+import javax.enterprise.context.ApplicationScoped;
+import javax.transaction.Transactional;
+
+import io.quarkus.narayana.jta.runtime.TransactionConfiguration;
+import io.quarkus.narayana.jta.runtime.interceptor.RunnableWithException;
+import io.quarkus.test.junit.QuarkusTest;
+
+@ApplicationScoped
+@QuarkusTest
+public class DummyTransactionalService {
+    @Transactional
+    public void doWithinPlainTransactional(RunnableWithException todo) throws Exception {
+        todo.run();
+    }
+
+    @Transactional
+    @TransactionConfiguration(timeoutFromConfigProperty = "transaction.timeout.unknown")
+    public void doWithinTransactionWithMissingProperty(RunnableWithException todo) throws Exception {
+        todo.run();
+    }
+
+    @Transactional
+    @TransactionConfiguration(timeoutFromConfigProperty = "transaction.timeout.unknown", timeout = 2)
+    public void doWithinTransactionWithMissingPropertyAndSetTimeoutInAnnotation(RunnableWithException todo)
+            throws Exception {
+        todo.run();
+    }
+
+    @Transactional
+    @TransactionConfiguration(timeoutFromConfigProperty = "transaction.timeout.1s", timeout = 2)
+    public void doWithinTransactionWithPropertyAndSetTimeoutInAnnotation(RunnableWithException todo)
+            throws Exception {
+        todo.run();
+    }
+
+    @Transactional
+    @TransactionConfiguration(timeoutFromConfigProperty = "transaction.timeout.1s")
+    public void doWithinTransactionWithProperty(RunnableWithException todo)
+            throws Exception {
+        todo.run();
+    }
+
+    @Transactional
+    @TransactionConfiguration()
+    public void doWithinTransactionWithNothing(RunnableWithException todo)
+            throws Exception {
+        todo.run();
+    }
+}

integration-tests/jpa-h2/src/test/java/io/quarkus/it/jpa/h2/TransactionConfigurationTest.java

@@ -0,0 +1,69 @@
+package io.quarkus.it.jpa.h2;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import javax.inject.Inject;
+import javax.transaction.RollbackException;
+
+import org.junit.jupiter.api.Test;
+
+import io.quarkus.narayana.jta.runtime.interceptor.RunnableWithException;
+import io.quarkus.test.junit.QuarkusTest;
+
+@QuarkusTest
+/**
+ * Timeouts:
+ *
+ */
+public class TransactionConfigurationTest {
+    @Inject
+    DummyTransactionalService dummyService;
+
+    @Test
+    public void testMissingTransactionConfigurationPropertyIsIgnoredAndDefaultTimeoutIsUsed() throws Exception {
+        dummyService.doWithinTransactionWithMissingProperty(someTaskTakingABitLongerThan1Second());
+        dummyService.doWithinTransactionWithMissingProperty(someTaskTakingABitLongerThan2Second());
+    }
+
+    @Test
+    public void testMissingTransactionConfigurationPropertyIsIgnoredAndAnnotationsTimeoutIsUsed() throws Exception {
+        dummyService.doWithinTransactionWithMissingPropertyAndSetTimeoutInAnnotation(someTaskTakingABitLongerThan1Second());
+        assertThrows(RollbackException.class, () -> dummyService
+                .doWithinTransactionWithMissingPropertyAndSetTimeoutInAnnotation(someTaskTakingABitLongerThan2Second()));
+    }
+
+    @Test
+    public void testTransactionConfigurationPropertyOverridesAnnotationsTimeout() {
+        assertThrows(RollbackException.class, () -> dummyService
+                .doWithinTransactionWithPropertyAndSetTimeoutInAnnotation(someTaskTakingABitLongerThan1Second()));
+    }
+
+    @Test
+    public void testTransactionConfigurationPropertyOverridesDefaultTimeout() {
+        assertThrows(RollbackException.class, () -> dummyService
+                .doWithinTransactionWithProperty(someTaskTakingABitLongerThan1Second()));
+    }
+
+    @Test
+    public void testTransactionConfigurationCheckFailsIfTransactionConfigurationIsNotUsedAtTransactionEntryLevel() {
+        assertThrows(RuntimeException.class, () -> dummyService
+                .doWithinPlainTransactional(() -> dummyService.doWithinTransactionWithProperty(() -> {
+                })));
+    }
+
+    @Test
+    public void testTransactionConfigurationCheckSucceedsIfTransactionConfigurationIsNotUsedAtTransactionEntryLevelButHasNoValuesSet()
+            throws Exception {
+        dummyService
+                .doWithinPlainTransactional(() -> dummyService.doWithinTransactionWithNothing(() -> {
+                }));
+    }
+
+    private RunnableWithException someTaskTakingABitLongerThan1Second() {
+        return () -> Thread.sleep(1100);
+    }
+
+    private RunnableWithException someTaskTakingABitLongerThan2Second() {
+        return () -> Thread.sleep(2100);
+    }
+}