Refactors JtaConnection to allow status enforcement by JTA implementation

integrations/cdi/jpa-cdi/pom.xml

@@ -30,10 +30,13 @@
     <name>Helidon CDI Integrations JPA</name>
 
     <properties>
-      <doclint>-syntax</doclint>
-      <eclipselink.skip>false</eclipselink.skip>
-      <hibernate.skip>false</hibernate.skip>
-      <spotbugs.exclude>etc/spotbugs/exclude.xml</spotbugs.exclude>
+        <doclint>-syntax</doclint>
+        <eclipselink.skip>false</eclipselink.skip>
+        <helidon.jta.immediateEnlistment>false</helidon.jta.immediateEnlistment>
+        <helidon.jta.interposedSynchronizations>true</helidon.jta.interposedSynchronizations>
+        <helidon.jta.preemptiveEnlistmentChecks>true</helidon.jta.preemptiveEnlistmentChecks>
+        <hibernate.skip>false</hibernate.skip>
+        <spotbugs.exclude>etc/spotbugs/exclude.xml</spotbugs.exclude>
     </properties>
 
     <dependencies>
@@ -291,6 +294,9 @@
                     <reportNameSuffix>eclipselink</reportNameSuffix>
                     <skip>${eclipselink.skip}</skip>
                     <systemPropertyVariables>
+                        <helidon.jta.immediateEnlistment>${helidon.jta.immediateEnlistment}</helidon.jta.immediateEnlistment>
+                        <helidon.jta.interposedSynchronizations>${helidon.jta.interposedSynchronizations}</helidon.jta.interposedSynchronizations>
+                        <helidon.jta.preemptiveEnlistmentChecks>${helidon.jta.preemptiveEnlistmentChecks}</helidon.jta.preemptiveEnlistmentChecks>
                         <java.util.logging.config.file>${project.basedir}/src/test/logging.properties</java.util.logging.config.file>
                     </systemPropertyVariables>
                     <testClassesDirectory>${project.build.directory}/eclipselink/test-classes</testClassesDirectory>

integrations/cdi/jpa-cdi/src/main/java/io/helidon/integrations/cdi/jpa/JtaAdaptingDataSourceProvider.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2023, 2024 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -82,6 +82,8 @@ final class JtaAdaptingDataSourceProvider implements PersistenceUnitInfoBean.Dat
 
     private final boolean immediateEnlistment;
 
+    private final boolean preemptiveEnlistmentChecks;
+
     private final ExceptionConverter exceptionConverter;
 
 
@@ -118,6 +120,8 @@ final class JtaAdaptingDataSourceProvider implements PersistenceUnitInfoBean.Dat
             Boolean.parseBoolean(System.getProperty("helidon.jta.interposedSynchronizations", "true"));
         this.immediateEnlistment =
             Boolean.parseBoolean(System.getProperty("helidon.jta.immediateEnlistment", "false"));
+        this.preemptiveEnlistmentChecks =
+            Boolean.parseBoolean(System.getProperty("helidon.jta.preemptiveEnlistmentChecks", "true"));
         Instance<ExceptionConverter> i = objects.select(ExceptionConverter.class);
         this.exceptionConverter = i.isUnsatisfied() ? null : i.get();
     }
@@ -167,7 +171,8 @@ private JtaAdaptingDataSource getDefaultJtaDataSource() {
                                                                                 this.interposedSynchronizations,
                                                                                 this.exceptionConverter,
                                                                                 this.objects.select(DataSource.class).get(),
-                                                                                this.immediateEnlistment));
+                                                                                this.immediateEnlistment,
+                                                                                this.preemptiveEnlistmentChecks));
     }
 
     private JtaAdaptingDataSource getNamedJtaDataSource(String name) {
@@ -179,7 +184,8 @@ private JtaAdaptingDataSource getNamedJtaDataSource(String name) {
                                                                                 this.exceptionConverter,
                                                                                 this.objects.select(DataSource.class,
                                                                                                     NamedLiteral.of(n)).get(),
-                                                                                this.immediateEnlistment));
+                                                                                this.immediateEnlistment,
+                                                                                this.preemptiveEnlistmentChecks));
     }
 
     private DataSource getNamedNonJtaDataSource(String name) {

integrations/jta/jdbc/src/main/java/io/helidon/integrations/jta/jdbc/JtaAdaptingDataSource.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2023, 2024 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -80,13 +80,59 @@ public final class JtaAdaptingDataSource extends AbstractDataSource {
      * made immediately upon {@link Connection} allocation
      *
      * @exception NullPointerException if {@code ts}, {@code tsr} or {@code ds} is {@code null}
+     *
+     * @see #JtaAdaptingDataSource(TransactionSupplier, TransactionSynchronizationRegistry, boolean, ExceptionConverter,
+     * DataSource, boolean, boolean)
      */
     public JtaAdaptingDataSource(TransactionSupplier ts,
                                  TransactionSynchronizationRegistry tsr,
                                  boolean interposedSynchronizations,
                                  ExceptionConverter ec,
                                  DataSource ds,
                                  boolean immediateEnlistment) {
+        this(ts, tsr, interposedSynchronizations, ec, ds, immediateEnlistment, true);
+    }
+
+    /**
+     * Creates a new {@link JtaAdaptingDataSource} that wraps the supplied {@link DataSource} and helps its {@linkplain
+     * DataSource#getConnection() connections} participate in XA transactions.
+     *
+     * <p>Behavior is left deliberately undefined if the supplied {@link DataSource}'s {@link
+     * DataSource#getConnection()} or {@link DataSource#getConnection(String, String)} methods are implemented to return
+     * or augment the return value of an invocation of the {@link javax.sql.PooledConnection#getConnection()
+     * XAConnection#getConnection()} method.  Less formally, and in general, this class is deliberately not designed to
+     * work with JDBC constructs that are already XA-aware.</p>
+     *
+     * @param ts a {@link TransactionSupplier}; must not be {@code null}
+     *
+     * @param tsr a {@link TransactionSynchronizationRegistry}; must not be {@code null}
+     *
+     * @param interposedSynchronizations whether any {@link jakarta.transaction.Synchronization Synchronization}s
+     * registered should be registered as interposed synchronizations; see {@link
+     * TransactionSynchronizationRegistry#registerInterposedSynchronization(jakarta.transaction.Synchronization)} and
+     * {@link jakarta.transaction.Transaction#registerSynchronization(jakarta.transaction.Synchronization)}
+     *
+     * @param ec an {@link ExceptionConverter}; may be {@code null} in which case a default implementation will be used
+     * instead
+     *
+     * @param ds a {@link DataSource} that may not be XA-compliant; must not be {@code null}; normally supplied by a
+     * connection pool implementation
+     *
+     * @param immediateEnlistment whether attempts to enlist new {@link Connection}s in a global transaction should be
+     * made immediately upon {@link Connection} allocation
+     *
+     * @param preemptiveEnlistmentChecks whether early checks will be made to see if an enlistment attempt will succeed,
+     * or whether enlistment validation will be performed by the JTA implementation
+     *
+     * @exception NullPointerException if {@code ts}, {@code tsr} or {@code ds} is {@code null}
+     */
+    public JtaAdaptingDataSource(TransactionSupplier ts,
+                                 TransactionSynchronizationRegistry tsr,
+                                 boolean interposedSynchronizations,
+                                 ExceptionConverter ec,
+                                 DataSource ds,
+                                 boolean immediateEnlistment,
+                                 boolean preemptiveEnlistmentChecks) {
         super();
         Objects.requireNonNull(ts, "ts");
         Objects.requireNonNull(tsr, "tsr");
@@ -99,9 +145,24 @@ public JtaAdaptingDataSource(TransactionSupplier ts,
             // the (inherited) PooledConnection#close() method, which reads, in part: "An application never calls this
             // method directly; it is called by the connection pool module, or manager."  As of this writing this branch
             // of this constructor implements this non-standard behavior.
-            this.acs =
-                (u, p) -> xa(ts, tsr, interposedSynchronizations, ec, xads.getXAConnection(u, p), immediateEnlistment, true);
-            this.uacs = () -> xa(ts, tsr, interposedSynchronizations, ec, xads.getXAConnection(), immediateEnlistment, true);
+            this.acs = (u, p) ->
+                xa(ts,
+                   tsr,
+                   interposedSynchronizations,
+                   ec,
+                   xads.getXAConnection(u, p),
+                   immediateEnlistment,
+                   preemptiveEnlistmentChecks,
+                   true);
+            this.uacs = () ->
+                xa(ts,
+                   tsr,
+                   interposedSynchronizations,
+                   ec,
+                   xads.getXAConnection(),
+                   immediateEnlistment,
+                   preemptiveEnlistmentChecks,
+                   true);
         } else {
             Objects.requireNonNull(ds, "ds");
             this.acs =
@@ -156,6 +217,59 @@ public JtaAdaptingDataSource(TransactionSupplier ts,
                                  XADataSource xads,
                                  boolean immediateEnlistment,
                                  boolean closeXac) {
+        this(ts, tsr, interposedSynchronizations, ec, xads, immediateEnlistment, true, closeXac);
+    }
+
+    /**
+     * Creates a new {@link JtaAdaptingDataSource} that adapts the supplied {@link XADataSource} and helps {@link
+     * Connection}s it indirectly supplies (by way of its {@linkplain XADataSource#getXAConnection() associated
+     * <code>XAConnection</code>}) participate in XA transactions.
+     *
+     * @param ts a {@link TransactionSupplier}; must not be {@code null}
+     *
+     * @param tsr a {@link TransactionSynchronizationRegistry}; must not be {@code null}
+     *
+     * @param interposedSynchronizations whether any {@link jakarta.transaction.Synchronization Synchronization}s
+     * registered should be registered as interposed synchronizations; see {@link
+     * TransactionSynchronizationRegistry#registerInterposedSynchronization(jakarta.transaction.Synchronization)} and
+     * {@link jakarta.transaction.Transaction#registerSynchronization(jakarta.transaction.Synchronization)}
+     *
+     * @param ec an {@link ExceptionConverter}; may be {@code null} in which case a default implementation will be used
+     * instead
+     *
+     * @param xads an {@link XADataSource} supplied by a connection pool implementation; must not be {@code null}
+     *
+     * @param immediateEnlistment whether attempts to enlist new {@link Connection}s in a global transaction should be
+     * made immediately upon {@link Connection} allocation
+     *
+     * @param preemptiveEnlistmentChecks whether early checks will be made to see if an enlistment attempt will succeed,
+     * or whether enlistment validation will be performed by the JTA implementation
+     *
+     * @param closeXac whether or not {@link XAConnection}s {@linkplain XADataSource#getXAConnection() supplied} by the
+     * supplied {@link XADataSource} should be {@linkplain javax.sql.PooledConnection#close() closed} when {@linkplain
+     * XAConnection#getConnection() their <code>Connection</code>}s are {@linkplain Connection#close() closed} (in a
+     * non-standard fashion)
+     *
+     * @exception NullPointerException if {@code ts}, {@code tsr} or {@code xads} is {@code null}
+     *
+     * @deprecated This constructor exists only to handle certain XA-aware connection pools that allow an end-user
+     * caller to "borrow" {@link XAConnection}s and to "return" them using their {@link
+     * javax.sql.PooledConnection#close() close()} methods, a non-standard practice which is discouraged by the
+     * documentation of {@link javax.sql.PooledConnection} (from which {@link XAConnection} inherits).  For
+     * such connection pools, {@link XAConnection}s that are "borrowed" must be returned in this manner to avoid leaks.
+     * This constructor implements this behavior.  Before using it, you should make sure that the connection pool in
+     * question implementing or supplying the {@link XADataSource} has the behavior described above; normally an {@link
+     * XAConnection} should not be used directly or closed by end-user code.
+     */
+    @Deprecated(since = "3.1.0")
+    public JtaAdaptingDataSource(TransactionSupplier ts,
+                                 TransactionSynchronizationRegistry tsr,
+                                 boolean interposedSynchronizations,
+                                 ExceptionConverter ec,
+                                 XADataSource xads,
+                                 boolean immediateEnlistment,
+                                 boolean preemptiveEnlistmentChecks,
+                                 boolean closeXac) {
         super();
         Objects.requireNonNull(xads, "xads");
         Objects.requireNonNull(ts, "ts");
@@ -168,8 +282,24 @@ public JtaAdaptingDataSource(TransactionSupplier ts,
         // is called by the connection pool module, or manager."  As of this writing this constructor permits this
         // non-standard behavior when closeXac is true.
         this.acs =
-            (u, p) -> xa(ts, tsr, interposedSynchronizations, ec, xads.getXAConnection(u, p), immediateEnlistment, closeXac);
-        this.uacs = () -> xa(ts, tsr, interposedSynchronizations, ec, xads.getXAConnection(), immediateEnlistment, closeXac);
+            (u, p) ->
+            xa(ts,
+               tsr,
+               interposedSynchronizations,
+               ec,
+               xads.getXAConnection(u, p),
+               immediateEnlistment,
+               preemptiveEnlistmentChecks,
+               closeXac);
+        this.uacs = () ->
+            xa(ts,
+               tsr,
+               interposedSynchronizations,
+               ec,
+               xads.getXAConnection(),
+               immediateEnlistment,
+               preemptiveEnlistmentChecks,
+               closeXac);
     }
 
     @Override // DataSource
@@ -189,12 +319,14 @@ public Connection getConnection() throws SQLException {
 
 
     @Deprecated
+    @SuppressWarnings("ParameterNumber")
     private static JtaConnection xa(TransactionSupplier ts,
                                     TransactionSynchronizationRegistry tsr,
                                     boolean interposedSynchronizations,
                                     ExceptionConverter ec,
                                     XAConnection xac,
                                     boolean immediateEnlistment,
+                                    boolean preemptiveEnlistmentChecks,
                                     boolean closeXac)
         throws SQLException {
         if (closeXac) {
@@ -204,13 +336,16 @@ private static JtaConnection xa(TransactionSupplier ts,
             // reads: "An application never calls this method directly; it is called by the connection pool module, or
             // manager." This branch of this method implements this non-standard behavior, ensuring that both the
             // Connection and its sourcing XAConnection are closed appropriately.
-            return new JtaConnection(ts,
-                                     tsr,
-                                     interposedSynchronizations,
-                                     ec,
-                                     xac.getConnection(),
-                                     xac::getXAResource,
-                                     immediateEnlistment) {
+            return
+                new JtaConnection(ts,
+                                  tsr,
+                                  interposedSynchronizations,
+                                  ec,
+                                  xac.getConnection(),
+                                  xac::getXAResource,
+                                  null, // no Xid consumer
+                                  immediateEnlistment,
+                                  preemptiveEnlistmentChecks) {
                 @Override
                 protected void onClose() throws SQLException {
                     xac.close();
@@ -224,7 +359,9 @@ protected void onClose() throws SQLException {
                               ec,
                               xac.getConnection(),
                               xac::getXAResource,
-                              immediateEnlistment);
+                              null, // no Xid consumer
+                              immediateEnlistment,
+                              preemptiveEnlistmentChecks);
     }