You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aries.apache.org by ti...@apache.org on 2010/02/03 12:20:48 UTC

svn commit: r906003 - in /incubator/aries/trunk/jpa/jpa-container-context/src: main/java/org/apache/aries/jpa/container/context/transaction/impl/ test/java/org/apache/aries/jpa/container/context/transaction/impl/

Author: timothyjward
Date: Wed Feb  3 11:20:40 2010
New Revision: 906003

URL: http://svn.apache.org/viewvc?rev=906003&view=rev
Log:
ARIES-140 : Ensure EntityManagers are properly cleaned up

Modified:
    incubator/aries/trunk/jpa/jpa-container-context/src/main/java/org/apache/aries/jpa/container/context/transaction/impl/JTAPersistenceContextRegistry.java
    incubator/aries/trunk/jpa/jpa-container-context/src/test/java/org/apache/aries/jpa/container/context/transaction/impl/JTAPersistenceContextRegistryTest.java

Modified: incubator/aries/trunk/jpa/jpa-container-context/src/main/java/org/apache/aries/jpa/container/context/transaction/impl/JTAPersistenceContextRegistry.java
URL: http://svn.apache.org/viewvc/incubator/aries/trunk/jpa/jpa-container-context/src/main/java/org/apache/aries/jpa/container/context/transaction/impl/JTAPersistenceContextRegistry.java?rev=906003&r1=906002&r2=906003&view=diff
==============================================================================
--- incubator/aries/trunk/jpa/jpa-container-context/src/main/java/org/apache/aries/jpa/container/context/transaction/impl/JTAPersistenceContextRegistry.java (original)
+++ incubator/aries/trunk/jpa/jpa-container-context/src/main/java/org/apache/aries/jpa/container/context/transaction/impl/JTAPersistenceContextRegistry.java Wed Feb  3 11:20:40 2010
@@ -64,13 +64,14 @@
   {
     //There will only ever be one thread associated with a transaction at a given time
     //As a result, it is only the outer map that needs to be thread safe.
+    
     Object transactionKey = tranRegistry.getTransactionKey();
     
     //TODO Globalize and log this problem
     //Throw the error on to the client
-    if(transactionKey == null)
+    if(transactionKey == null) {
       throw new TransactionRequiredException();
-    
+    }
     //Get hold of the Map. If there is no Map already registered then add one.
     //We don't need to worry about a race condition, as no other thread will
     //share our transaction
@@ -80,7 +81,13 @@
     if(contextsForTransaction == null) {
       contextsForTransaction = new IdentityHashMap<EntityManagerFactory, EntityManager>();
       persistenceContextRegistry.put(transactionKey, contextsForTransaction);
-      tranRegistry.registerInterposedSynchronization(new EntityManagerClearUp(transactionKey));
+      try {
+        tranRegistry.registerInterposedSynchronization(new EntityManagerClearUp(transactionKey));
+      } catch (IllegalStateException e) {
+        persistenceContextRegistry.remove(transactionKey);
+        //TODO add a message
+        throw new TransactionRequiredException();
+      }
     }
     
     //Still only one thread for this transaction, so don't worry about any race conditions
@@ -89,6 +96,8 @@
     if(toReturn == null) {
       toReturn = (properties == null) ? persistenceUnit.createEntityManager() : persistenceUnit.createEntityManager(properties);
       contextsForTransaction.put(persistenceUnit, toReturn);
+    } else {
+      //TODO maybe add debug
     }
     
     return toReturn;
@@ -113,6 +122,7 @@
       return persistenceUnit.createEntityManager(properties);
   }
 
+  
   /**
    * Get the persistence context for the current transaction if a transaction is active. 
    * {@link getCurrentPersistenceContext}
@@ -153,15 +163,20 @@
     }
     
     public void afterCompletion(int arg0) {
-      Map<EntityManagerFactory, EntityManager> tidyUp = persistenceContextRegistry.remove(key);
-      if(tidyUp != null) {
-        for(EntityManager em : tidyUp.values())
-          em.close();
-      }
+      //This is a no-op;
     }
 
     public void beforeCompletion() {
-      //This is a no-op;
+      Map<EntityManagerFactory, EntityManager> tidyUp = persistenceContextRegistry.remove(key);
+      if(tidyUp != null) {
+        for(EntityManager em : tidyUp.values()) {
+          try {
+            em.close();
+          } catch (Exception e) {
+            //TODO Log this, but continue
+          }
+        }
+      }
     }
   }
 }

Modified: incubator/aries/trunk/jpa/jpa-container-context/src/test/java/org/apache/aries/jpa/container/context/transaction/impl/JTAPersistenceContextRegistryTest.java
URL: http://svn.apache.org/viewvc/incubator/aries/trunk/jpa/jpa-container-context/src/test/java/org/apache/aries/jpa/container/context/transaction/impl/JTAPersistenceContextRegistryTest.java?rev=906003&r1=906002&r2=906003&view=diff
==============================================================================
--- incubator/aries/trunk/jpa/jpa-container-context/src/test/java/org/apache/aries/jpa/container/context/transaction/impl/JTAPersistenceContextRegistryTest.java (original)
+++ incubator/aries/trunk/jpa/jpa-container-context/src/test/java/org/apache/aries/jpa/container/context/transaction/impl/JTAPersistenceContextRegistryTest.java Wed Feb  3 11:20:40 2010
@@ -18,15 +18,14 @@
  */
 package org.apache.aries.jpa.container.context.transaction.impl;
 
-import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertSame;
 
 import java.util.HashMap;
 import java.util.Map;
 
 import javax.persistence.EntityManager;
 import javax.persistence.EntityManagerFactory;
-import javax.transaction.Status;
 import javax.transaction.Synchronization;
 import javax.transaction.TransactionSynchronizationRegistry;
 
@@ -55,9 +54,9 @@
       syncs.put(key, arg0);
     }
     
-    public void afterCompletion(String s)
+    public void beforeCompletion(String s)
     {
-      syncs.get(s).afterCompletion(Status.STATUS_COMMITTED);
+      syncs.get(s).beforeCompletion();
     }
   }
   
@@ -74,11 +73,27 @@
   public void setup() 
   {
     reg = new TranSyncRegistryMock();
+
+    props1 = new HashMap<Object, Object>();
+    props1.put("prop1", "value1");
+    
+    props2 = new HashMap<Object, Object>();
+    props2.put("prop2", "value2");
     
     emf1 = Skeleton.newMock(EntityManagerFactory.class);
-    props1 = new HashMap<Object, Object>();
+    
+    Skeleton.getSkeleton(emf1).setReturnValue(new MethodCall(EntityManagerFactory.class, 
+        "createEntityManager", props1), Skeleton.newMock(EntityManager.class));
+    Skeleton.getSkeleton(emf1).setReturnValue(new MethodCall(EntityManagerFactory.class, 
+        "createEntityManager", props2), Skeleton.newMock(EntityManager.class));
+    
     emf2 = Skeleton.newMock(EntityManagerFactory.class);
-    props2 = new HashMap<Object, Object>();
+
+    Skeleton.getSkeleton(emf2).setReturnValue(new MethodCall(EntityManagerFactory.class, 
+        "createEntityManager", props1), Skeleton.newMock(EntityManager.class));
+    Skeleton.getSkeleton(emf2).setReturnValue(new MethodCall(EntityManagerFactory.class, 
+        "createEntityManager", props2), Skeleton.newMock(EntityManager.class));
+
     
     contexts = new JTAPersistenceContextRegistry();
     contexts.setTranRegistry(Skeleton.newMock(reg, TransactionSynchronizationRegistry.class));
@@ -105,12 +120,13 @@
     Skeleton.getSkeleton(em2a).assertNotCalled(new MethodCall(EntityManager.class, "close"));
     assertSame("We should get the same delegate!", em2a, em2b);
     
-    reg.afterCompletion("");
+    reg.beforeCompletion("");
     
     Skeleton.getSkeleton(em1a).assertCalledExactNumberOfTimes(new MethodCall(EntityManager.class, "close"),1);
     Skeleton.getSkeleton(em2a).assertCalledExactNumberOfTimes(new MethodCall(EntityManager.class, "close"),1);
   }
   
+  @Test
   public void testMultiGetsMultiTrans()
   {
     reg.setTransactionKey("a");
@@ -121,7 +137,7 @@
     Skeleton.getSkeleton(emf1).assertCalledExactNumberOfTimes(new MethodCall(EntityManagerFactory.class, "createEntityManager", props1), 1);
     Skeleton.getSkeleton(emf1).assertCalledExactNumberOfTimes(new MethodCall(EntityManagerFactory.class, "createEntityManager", props2), 1);
    
-    assertNotSame("We should get the same delegate!", em1a, em1b);
+    assertNotSame("We should not get the same delegate!", em1a, em1b);
     
     Skeleton.getSkeleton(em1a).assertNotCalled(new MethodCall(EntityManager.class, "close"));
     Skeleton.getSkeleton(em1b).assertNotCalled(new MethodCall(EntityManager.class, "close"));
@@ -140,14 +156,14 @@
     Skeleton.getSkeleton(em2b).assertNotCalled(new MethodCall(EntityManager.class, "close"));
     
     
-    reg.afterCompletion("b");
+    reg.beforeCompletion("b");
     
     Skeleton.getSkeleton(em1a).assertNotCalled(new MethodCall(EntityManager.class, "close"));
     Skeleton.getSkeleton(em1b).assertCalledExactNumberOfTimes(new MethodCall(EntityManager.class, "close"), 1);
     Skeleton.getSkeleton(em2a).assertNotCalled(new MethodCall(EntityManager.class, "close"));
     Skeleton.getSkeleton(em2b).assertCalledExactNumberOfTimes(new MethodCall(EntityManager.class, "close"), 1);
 
-    reg.afterCompletion("a");
+    reg.beforeCompletion("a");
     
     Skeleton.getSkeleton(em1a).assertCalledExactNumberOfTimes(new MethodCall(EntityManager.class, "close"), 1);
     Skeleton.getSkeleton(em1b).assertCalledExactNumberOfTimes(new MethodCall(EntityManager.class, "close"), 1);