You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tomee.apache.org by jg...@apache.org on 2019/07/22 13:14:06 UTC

[tomee] branch tomee-7.1.x updated (099bac0 -> a9a9cfb)

This is an automated email from the ASF dual-hosted git repository.

jgallimore pushed a change to branch tomee-7.1.x
in repository https://gitbox.apache.org/repos/asf/tomee.git.


    from 099bac0  Merge branch 'tomee-7.1.x' of github.com:jgallimore/tomee into tomee-7.1.x
     new 2c789fc  Attempting to avoid edge case with using multiple connections from the same datasource in the same transaction
     new e5019c9  Unit test for ManagedConnection behaviour
     new a039449  Checkstyle
     new a9a9cfb  Unused code

The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../openejb/resource/jdbc/managed/local/Key.java   | 81 ++++++++++++++++++++++
 .../jdbc/managed/local/ManagedConnection.java      | 62 +++++------------
 .../jdbc/managed/local/ManagedDataSource.java      | 34 +++++++--
 .../jdbc/managed/xa/ManagedXADataSource.java       |  5 ++
 .../jdbc/ManagedConnectionBehaviorTest.java        | 72 ++++++++++++++++++-
 5 files changed, 204 insertions(+), 50 deletions(-)
 create mode 100644 container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/local/Key.java


[tomee] 04/04: Unused code

Posted by jg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jgallimore pushed a commit to branch tomee-7.1.x
in repository https://gitbox.apache.org/repos/asf/tomee.git

commit a9a9cfb04d10584450028e914fd0d8239cd22fa1
Author: Jonathan Gallimore <jo...@jrg.me.uk>
AuthorDate: Thu Jul 4 21:38:09 2019 +0100

    Unused code
---
 .../apache/openejb/resource/jdbc/managed/xa/ManagedXADataSource.java   | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/xa/ManagedXADataSource.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/xa/ManagedXADataSource.java
index 42013c7..c111874 100644
--- a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/xa/ManagedXADataSource.java
+++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/xa/ManagedXADataSource.java
@@ -18,8 +18,6 @@
 package org.apache.openejb.resource.jdbc.managed.xa;
 
 import org.apache.openejb.resource.jdbc.managed.local.ManagedDataSource;
-import org.apache.openejb.util.LogCategory;
-import org.apache.openejb.util.Logger;
 
 import java.lang.reflect.Proxy;
 import java.sql.Connection;
@@ -30,7 +28,6 @@ import javax.transaction.TransactionSynchronizationRegistry;
 
 public class ManagedXADataSource extends ManagedDataSource {
 
-    private static final Logger LOGGER = Logger.getInstance(LogCategory.OPENEJB_RESOURCE_JDBC, ManagedXADataSource.class);
     private static final Class<?>[] CONNECTION_CLASS = new Class<?>[]{Connection.class};
 
     private final TransactionManager txMgr;


[tomee] 03/04: Checkstyle

Posted by jg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jgallimore pushed a commit to branch tomee-7.1.x
in repository https://gitbox.apache.org/repos/asf/tomee.git

commit a039449d96c6f0c13fb2b60b53f320ca6911ee2b
Author: Jonathan Gallimore <jo...@jrg.me.uk>
AuthorDate: Thu Jul 4 21:06:55 2019 +0100

    Checkstyle
---
 .../apache/openejb/resource/jdbc/managed/xa/ManagedXADataSource.java  | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/xa/ManagedXADataSource.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/xa/ManagedXADataSource.java
index 341cf23..42013c7 100644
--- a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/xa/ManagedXADataSource.java
+++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/xa/ManagedXADataSource.java
@@ -17,8 +17,6 @@
 
 package org.apache.openejb.resource.jdbc.managed.xa;
 
-import org.apache.openejb.resource.jdbc.managed.local.Key;
-import org.apache.openejb.resource.jdbc.managed.local.ManagedConnection;
 import org.apache.openejb.resource.jdbc.managed.local.ManagedDataSource;
 import org.apache.openejb.util.LogCategory;
 import org.apache.openejb.util.Logger;
@@ -27,8 +25,6 @@ import java.lang.reflect.Proxy;
 import java.sql.Connection;
 import java.sql.SQLException;
 import javax.sql.CommonDataSource;
-import javax.transaction.SystemException;
-import javax.transaction.Transaction;
 import javax.transaction.TransactionManager;
 import javax.transaction.TransactionSynchronizationRegistry;
 


[tomee] 02/04: Unit test for ManagedConnection behaviour

Posted by jg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jgallimore pushed a commit to branch tomee-7.1.x
in repository https://gitbox.apache.org/repos/asf/tomee.git

commit e5019c9e9fc7bd3b9249eb77d13e5361373bbf63
Author: Jonathan Gallimore <jo...@jrg.me.uk>
AuthorDate: Thu Jul 4 21:58:06 2019 +0100

    Unit test for ManagedConnection behaviour
---
 .../jdbc/ManagedConnectionBehaviorTest.java        | 72 +++++++++++++++++++++-
 1 file changed, 69 insertions(+), 3 deletions(-)

diff --git a/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/ManagedConnectionBehaviorTest.java b/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/ManagedConnectionBehaviorTest.java
index a5e65cd..c17bdfe 100644
--- a/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/ManagedConnectionBehaviorTest.java
+++ b/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/ManagedConnectionBehaviorTest.java
@@ -19,10 +19,13 @@ package org.apache.openejb.resource.jdbc;
 import org.apache.geronimo.transaction.manager.GeronimoTransactionManager;
 import org.apache.openejb.resource.GeronimoTransactionManagerFactory;
 import org.apache.openejb.resource.TransactionManagerWrapper;
+import org.apache.openejb.resource.jdbc.managed.local.ManagedConnection;
 import org.apache.openejb.resource.jdbc.managed.local.ManagedDataSource;
 import org.junit.Test;
 
 import java.io.PrintWriter;
+import java.lang.reflect.Field;
+import java.lang.reflect.Proxy;
 import java.sql.Array;
 import java.sql.Blob;
 import java.sql.CallableStatement;
@@ -51,9 +54,7 @@ import javax.sql.DataSource;
 import javax.transaction.Transaction;
 import javax.transaction.TransactionManager;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
 
 public class ManagedConnectionBehaviorTest {
     @Test
@@ -123,7 +124,72 @@ public class ManagedConnectionBehaviorTest {
             assertTrue(myDs.connections.iterator().next().commit);
             myDs.connections.clear();
         }
+        { // 2 connections, same TX
+            mgr.begin();
+            final Connection connection1 = ds.getConnection();
+
+            assertTrue(myDs.connections.isEmpty()); // not yet needed
+            connection1.createBlob(); // just to call something
+
+            // second connection should be the same as it comes from the tx registry
+            final Connection connection2 = ds.getConnection();
+            connection2.createBlob(); // just to call something
+
+            assertEquals(connection1, connection2);
+
+            for (final MyConn conn : myDs.connections) {
+                assertFalse(conn.closed);
+            }
+
+            mgr.commit();
+
+            for (final MyConn conn : myDs.connections) {
+                assertTrue(conn.closed);
+                assertTrue(conn.commit);
+                assertFalse(conn.rollback);
+            }
+
+            myDs.connections.clear();
+        }
+        { // 2 connections, same TX
+            final Connection connection1 = ds.getConnection();
+            final Connection connection2 = ds.getConnection();
+
+            assertNotEquals(connection1, connection2);
+
+            mgr.begin();
+            assertTrue(myDs.connections.isEmpty()); // not yet needed
+            connection1.createBlob(); // just to call something
+            connection2.createBlob(); // just to call something
+
+            for (final MyConn conn : myDs.connections) {
+                assertFalse(conn.closed);
+            }
+
+            final ManagedConnection mc1 = (ManagedConnection) Proxy.getInvocationHandler(connection1);
+            final ManagedConnection mc2 = (ManagedConnection) Proxy.getInvocationHandler(connection2);
+
+            assertEquals(getFieldValue(mc1, "xaConnection"), getFieldValue(mc2, "xaConnection"));
+            assertEquals(getFieldValue(mc1, "xaResource"), getFieldValue(mc2, "xaResource"));
+            assertEquals(getFieldValue(mc1, "delegate"), getFieldValue(mc2, "delegate"));
+
+            mgr.commit();
+
+            for (final MyConn conn : myDs.connections) {
+                assertTrue(conn.closed);
+                assertTrue(conn.commit);
+                assertFalse(conn.rollback);
+            }
+
+            myDs.connections.clear();
+        }
+
+    }
 
+    private Object getFieldValue(final Object object, final String fieldName) throws NoSuchFieldException, IllegalAccessException {
+        final Field xaConnectionField = object.getClass().getDeclaredField(fieldName);
+        xaConnectionField.setAccessible(true);
+        return xaConnectionField.get(object);
     }
 
     public static class MyDs implements DataSource {


[tomee] 01/04: Attempting to avoid edge case with using multiple connections from the same datasource in the same transaction

Posted by jg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jgallimore pushed a commit to branch tomee-7.1.x
in repository https://gitbox.apache.org/repos/asf/tomee.git

commit 2c789fc27123041332095d33ee57ab6d8135fc81
Author: Jonathan Gallimore <jo...@jrg.me.uk>
AuthorDate: Thu Jul 4 15:21:17 2019 +0100

    Attempting to avoid edge case with using multiple connections from the
    same datasource in the same transaction
---
 .../openejb/resource/jdbc/managed/local/Key.java   | 81 ++++++++++++++++++++++
 .../jdbc/managed/local/ManagedConnection.java      | 62 +++++------------
 .../jdbc/managed/local/ManagedDataSource.java      | 34 +++++++--
 .../jdbc/managed/xa/ManagedXADataSource.java       | 12 ++++
 4 files changed, 142 insertions(+), 47 deletions(-)

diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/local/Key.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/local/Key.java
new file mode 100644
index 0000000..87351c7
--- /dev/null
+++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/local/Key.java
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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
+ *
+ *     http://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 org.apache.openejb.resource.jdbc.managed.local;
+
+import javax.sql.CommonDataSource;
+import java.util.Objects;
+
+public class Key {
+    private final CommonDataSource ds;
+    private final String user;
+    private final String pwd;
+    private final int hash;
+
+    public Key(final CommonDataSource ds, final String user, final String pwd) {
+        this.ds = ds;
+        this.user = user;
+        this.pwd = pwd;
+
+        int result = ds.hashCode();
+        result = 31 * result + (user != null ? user.hashCode() : 0);
+        result = 31 * result + (pwd != null ? pwd.hashCode() : 0);
+        hash = result;
+    }
+
+    public CommonDataSource getDs() {
+        return ds;
+    }
+
+    public String getUser() {
+        return user;
+    }
+
+    public String getPwd() {
+        return pwd;
+    }
+
+    @Override
+    public boolean equals(final Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+
+        Key key = Key.class.cast(o);
+        return (ds == key.ds || ds.equals(key.ds)) &&
+                Objects.equals(user, key.user) &&
+                Objects.equals(pwd, key.pwd);
+    }
+
+    @Override
+    public String toString() {
+        return "Key{" +
+                "ds=" + ds +
+                ", user='" + user + '\'' +
+                ", pwd='*****'" +
+                ", hash=" + hash +
+                '}';
+    }
+
+    @Override
+    public int hashCode() {
+        return hash;
+    }
+}
diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/local/ManagedConnection.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/local/ManagedConnection.java
index 3d35b7d..ae0c9a5 100644
--- a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/local/ManagedConnection.java
+++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/local/ManagedConnection.java
@@ -142,7 +142,7 @@ public class ManagedConnection implements InvocationHandler {
                         throw new SQLException("Unable to enlist connection the transaction", e);
                     }
 
-                    registry.putResource(key, delegate);
+                    registry.putResource(key, proxy);
                     transaction.registerSynchronization(new ClosingSynchronization());
 
                     if (xaConnection == null) {
@@ -158,8 +158,19 @@ public class ManagedConnection implements InvocationHandler {
                             }
                         }
                     }
-                } else if (delegate == null) { // shouldn't happen
-                    delegate = connection;
+                } else if (delegate == null) {
+                    // this happens if the caller obtains subsequent connections from the *same* datasource
+                    // are enlisted in the *same* transaction:
+                    //   connection != null (because it comes from the tx registry)
+                    //   delegate == null (because its a new ManagedConnection instance)
+                    // we attempt to work-around this by looking up the connection in the tx registry in ManaagedDataSource
+                    // and ManagedXADataSource, but there is an edge case where the connection is fetch from the datasource
+                    // first, and a BMT tx is started by the user.
+
+                    final ManagedConnection managedConnection = ManagedConnection.class.cast(Proxy.getInvocationHandler(connection));
+                    this.delegate = managedConnection.delegate;
+                    this.xaConnection = managedConnection.xaConnection;
+                    this.xaResource = managedConnection.xaResource;
                 }
 
                 return invokeUnderTransaction(method, args);
@@ -194,9 +205,10 @@ public class ManagedConnection implements InvocationHandler {
     }
 
     protected Object newConnection() throws SQLException {
-        final Object connection = DataSource.class.isInstance(key.ds) ?
-                (key.user == null ? DataSource.class.cast(key.ds).getConnection() : DataSource.class.cast(key.ds).getConnection(key.user, key.pwd)) :
-                (key.user == null ? XADataSource.class.cast(key.ds).getXAConnection() : XADataSource.class.cast(key.ds).getXAConnection(key.user, key.pwd));
+        final Object connection = DataSource.class.isInstance(key.getDs()) ?
+                (key.getUser() == null ? DataSource.class.cast(key.getDs()).getConnection() : DataSource.class.cast(key.getDs()).getConnection(key.getUser(), key.getPwd())) :
+                (key.getUser() == null ? XADataSource.class.cast(key.getDs()).getXAConnection() : XADataSource.class.cast(key.getDs()).getXAConnection(key.getUser(), key.getPwd()));
+
         if (XAConnection.class.isInstance(connection)) {
             xaConnection = XAConnection.class.cast(connection);
             xaResource = xaConnection.getXAResource();
@@ -248,7 +260,7 @@ public class ManagedConnection implements InvocationHandler {
         return null;
     }
 
-    private static boolean isUnderTransaction(final int status) {
+    public static boolean isUnderTransaction(final int status) {
         return status == Status.STATUS_ACTIVE || status == Status.STATUS_MARKED_ROLLBACK;
     }
 
@@ -285,41 +297,5 @@ public class ManagedConnection implements InvocationHandler {
         }
     }
 
-    private static final class Key {
-        private final CommonDataSource ds;
-        private final String user;
-        private final String pwd;
-        private final int hash;
-
-        private Key(final CommonDataSource ds, final String user, final String pwd) {
-            this.ds = ds;
-            this.user = user;
-            this.pwd = pwd;
-
-            int result = ds.hashCode();
-            result = 31 * result + (user != null ? user.hashCode() : 0);
-            result = 31 * result + (pwd != null ? pwd.hashCode() : 0);
-            hash = result;
-        }
-
-        @Override
-        public boolean equals(final Object o) {
-            if (this == o) {
-                return true;
-            }
-            if (o == null || getClass() != o.getClass()) {
-                return false;
-            }
-
-            Key key = Key.class.cast(o);
-            return (ds == key.ds || ds.equals(key.ds)) &&
-                    !(user != null ? !user.equals(key.user) : key.user != null) &&
-                    !(pwd != null ? !pwd.equals(key.pwd) : key.pwd != null);
-        }
 
-        @Override
-        public int hashCode() {
-            return hash;
-        }
-    }
 }
diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/local/ManagedDataSource.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/local/ManagedDataSource.java
index 5335fef..508aae8 100644
--- a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/local/ManagedDataSource.java
+++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/local/ManagedDataSource.java
@@ -17,6 +17,14 @@
 
 package org.apache.openejb.resource.jdbc.managed.local;
 
+import org.apache.openejb.util.LogCategory;
+
+import javax.sql.CommonDataSource;
+import javax.sql.DataSource;
+import javax.transaction.SystemException;
+import javax.transaction.Transaction;
+import javax.transaction.TransactionManager;
+import javax.transaction.TransactionSynchronizationRegistry;
 import java.io.ObjectStreamException;
 import java.io.PrintWriter;
 import java.io.Serializable;
@@ -25,12 +33,9 @@ import java.sql.Connection;
 import java.sql.SQLException;
 import java.sql.SQLFeatureNotSupportedException;
 import java.util.logging.Logger;
-import javax.sql.CommonDataSource;
-import javax.sql.DataSource;
-import javax.transaction.TransactionManager;
-import javax.transaction.TransactionSynchronizationRegistry;
 
 public class ManagedDataSource implements DataSource, Serializable {
+    private static final org.apache.openejb.util.Logger LOGGER = org.apache.openejb.util.Logger.getInstance(LogCategory.OPENEJB_RESOURCE_JDBC, ManagedDataSource.class);
     private static final Class<?>[] CONNECTION_CLASS = new Class<?>[]{Connection.class};
 
     protected final CommonDataSource delegate;
@@ -95,10 +100,31 @@ public class ManagedDataSource implements DataSource, Serializable {
     }
 
     private Connection managed(final String u, final String p) {
+        final Connection resource = getTxConnection(delegate, u, p, transactionManager, registry);
+        if (resource != null) {
+            return resource;
+        }
+
         return (Connection) Proxy.newProxyInstance(Thread.currentThread().getContextClassLoader(), CONNECTION_CLASS,
                 new ManagedConnection(delegate, transactionManager, registry, u, p));
     }
 
+    protected static Connection getTxConnection(final CommonDataSource delegate, final String u, final String p, final TransactionManager transactionManager, final TransactionSynchronizationRegistry registry) {
+        try {
+            final Transaction transaction = transactionManager.getTransaction();
+            if (transaction != null && ManagedConnection.isUnderTransaction(transaction.getStatus())) {
+                final Object resource = registry.getResource(new Key(delegate, u, p));
+                if (Connection.class.isInstance(resource)) {
+                    return Connection.class.cast(resource);
+                }
+            }
+        } catch (SystemException e) {
+            // we wouldn't expect this to happen, but lets log it and fall through to the previous behaviour
+            LOGGER.warning("Attempting to get the current transaction failed with an error: " + e.getMessage(), e);
+        }
+        return null;
+    }
+
     public CommonDataSource getDelegate() {
         return delegate;
     }
diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/xa/ManagedXADataSource.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/xa/ManagedXADataSource.java
index c2ccced..341cf23 100644
--- a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/xa/ManagedXADataSource.java
+++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/managed/xa/ManagedXADataSource.java
@@ -17,16 +17,24 @@
 
 package org.apache.openejb.resource.jdbc.managed.xa;
 
+import org.apache.openejb.resource.jdbc.managed.local.Key;
+import org.apache.openejb.resource.jdbc.managed.local.ManagedConnection;
 import org.apache.openejb.resource.jdbc.managed.local.ManagedDataSource;
+import org.apache.openejb.util.LogCategory;
+import org.apache.openejb.util.Logger;
 
 import java.lang.reflect.Proxy;
 import java.sql.Connection;
 import java.sql.SQLException;
 import javax.sql.CommonDataSource;
+import javax.transaction.SystemException;
+import javax.transaction.Transaction;
 import javax.transaction.TransactionManager;
 import javax.transaction.TransactionSynchronizationRegistry;
 
 public class ManagedXADataSource extends ManagedDataSource {
+
+    private static final Logger LOGGER = Logger.getInstance(LogCategory.OPENEJB_RESOURCE_JDBC, ManagedXADataSource.class);
     private static final Class<?>[] CONNECTION_CLASS = new Class<?>[]{Connection.class};
 
     private final TransactionManager txMgr;
@@ -47,6 +55,10 @@ public class ManagedXADataSource extends ManagedDataSource {
     }
 
     private Connection managedXA(final String u, final String p) throws SQLException {
+        final Connection resource = getTxConnection(delegate, u, p, transactionManager, registry);
+        if (resource != null) {
+            return resource;
+        }
         return Connection.class.cast(Proxy.newProxyInstance(Thread.currentThread().getContextClassLoader(), CONNECTION_CLASS,
                 new ManagedXAConnection(delegate, txMgr, registry, u, p)));
     }