MODE-1822 Improvements to previous commit as recommended by code review

ModeShape · Mar 7, 2013 · 2e7e439 · 2e7e439
1 parent 82e5ef7
commit 2e7e439
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 74 deletions.
diff --git a/modeshape-jcr/src/main/java/org/modeshape/jcr/cache/document/AbstractSessionCache.java b/modeshape-jcr/src/main/java/org/modeshape/jcr/cache/document/AbstractSessionCache.java
@@ -28,12 +28,12 @@
 import java.util.Iterator;
 import java.util.Set;
 import java.util.UUID;
+import java.util.concurrent.atomic.AtomicReference;
 import javax.transaction.RollbackException;
 import javax.transaction.Status;
 import javax.transaction.Synchronization;
 import javax.transaction.SystemException;
 import javax.transaction.Transaction;
-import javax.transaction.TransactionManager;
 import org.modeshape.common.annotation.Immutable;
 import org.modeshape.common.logging.Logger;
 import org.modeshape.jcr.ExecutionContext;
@@ -45,7 +45,6 @@
 import org.modeshape.jcr.cache.NodeKey;
 import org.modeshape.jcr.cache.SessionCache;
 import org.modeshape.jcr.cache.SessionEnvironment;
-import org.modeshape.jcr.txn.Transactions;
 import org.modeshape.jcr.value.NameFactory;
 import org.modeshape.jcr.value.Path;
 import org.modeshape.jcr.value.PathFactory;
@@ -78,7 +77,7 @@ public String getUserId() {
     }
 
     private final WorkspaceCache sharedWorkspaceCache;
-    private WorkspaceCache workspaceCache;
+    private final AtomicReference<WorkspaceCache> workspaceCache = new AtomicReference<WorkspaceCache>();
     private final NameFactory nameFactory;
     private final PathFactory pathFactory;
     private final Path rootPath;
@@ -91,7 +90,7 @@ protected AbstractSessionCache( ExecutionContext context,
                                     SessionEnvironment sessionContext ) {
         this.context = context;
         this.sharedWorkspaceCache = sharedWorkspaceCache;
-        this.workspaceCache = sharedWorkspaceCache;
+        this.workspaceCache.set(sharedWorkspaceCache);
         ValueFactories factories = this.context.getValueFactories();
         this.nameFactory = factories.getNameFactory();
         this.pathFactory = factories.getPathFactory();
@@ -112,46 +111,33 @@ protected AbstractSessionCache( ExecutionContext context,
     @Override
     public void checkForTransaction() {
         try {
-            Transactions transactions = sessionContext.getTransactions();
+            Transaction txn = sessionContext.getTransactions().getTransactionManager().getTransaction();
-            if (transactions == null) return;
+            if (txn != null && txn.getStatus() == Status.STATUS_ACTIVE) {
-            TransactionManager txnMgr = transactions.getTransactionManager();
+                // There is an active transaction, so we need a transaction-specific workspace cache ...
-            if (txnMgr == null) return;
+                workspaceCache.set(sessionContext.getTransactionalWorkspaceCacheFactory()
-            Transaction txn = txnMgr.getTransaction();
+                                                 .getTransactionalWorkspaceCache(sharedWorkspaceCache));
-            if (txn != null) {
-                workspaceCache = sessionContext.getTransactionalWorkspaceCacheFactory()
-                                               .getTransactionalWorkspaceCache(sharedWorkspaceCache);
                 // Register a synchronization to reset this workspace cache when the transaction completes ...
-                if (txn.getStatus() == Status.STATUS_ACTIVE) {
+                txn.registerSynchronization(new Synchronization() {
-                    // This is an active transaction, so upon completion of the transaction this session needs
+
-                    // to know to no longer use the transaction-specific workspace cache and to instead start
+                    @Override
-                    // using the shared workspace cache again.
+                    public void beforeCompletion() {
-                    txn.registerSynchronization(new Synchronization() {
+                        // do nothing ...
-
+                    }
-                        @Override
+
-                        public void beforeCompletion() {
+                    @Override
-                            // do nothing ...
+                    public void afterCompletion( int status ) {
-                        }
+                        // Tell the session that the transaction has completed ...
-
+                        completeTransaction();
-                        @Override
+                    }
-                        public void afterCompletion( int status ) {
+                });
-                            // Tell the session that the transaction has completed ...
+            } else {
-                            completeTransaction();
+                // There is no active transaction, so just use the shared workspace cache ...
-                        }
+                workspaceCache.set(sharedWorkspaceCache);
-                    });
-                }
-            } else if (workspaceCache == null || workspaceCache != sharedWorkspaceCache) {
-                workspaceCache = sharedWorkspaceCache;
             }
         } catch (SystemException e) {
-            logger().error(e,
+            logger().error(e, JcrI18n.errorDeterminingCurrentTransactionAssumingNone, workspaceName(), e.getMessage());
-                           JcrI18n.errorDeterminingCurrentTransactionAssumingNone,
-                           workspaceCache.getWorkspaceName(),
-                           e.getMessage());
         } catch (RollbackException e) {
-            logger().error(e,
+            logger().error(e, JcrI18n.errorDeterminingCurrentTransactionAssumingNone, workspaceName(), e.getMessage());
-                           JcrI18n.errorDeterminingCurrentTransactionAssumingNone,
-                           workspaceCache.getWorkspaceName(),
-                           e.getMessage());
         }
     }
 
@@ -160,7 +146,7 @@ public void afterCompletion( int status ) {
      * should no longer use a transaction-specific workspace cache.
      */
     protected void completeTransaction() {
-        workspaceCache = sharedWorkspaceCache;
+        workspaceCache.set(sharedWorkspaceCache);
     }
 
     @Override
@@ -169,7 +155,7 @@ public final SessionCache unwrap() {
     }
 
     protected final String workspaceName() {
-        return workspaceCache.getWorkspaceName();
+        return workspaceCache().getWorkspaceName();
     }
 
     @Override
@@ -179,11 +165,11 @@ public final ExecutionContext getContext() {
 
     @Override
     public final WorkspaceCache workspaceCache() {
-        return workspaceCache;
+        return workspaceCache.get();
     }
 
     final DocumentTranslator translator() {
-        return workspaceCache.translator();
+        return workspaceCache().translator();
     }
 
     final ExecutionContext context() {
@@ -238,17 +224,17 @@ protected String generateIdentifier() {
 
     @Override
     public NodeKey getRootKey() {
-        return workspaceCache.getRootKey();
+        return workspaceCache().getRootKey();
     }
 
     @Override
     public NodeCache getWorkspace() {
-        return workspaceCache;
+        return workspaceCache();
     }
 
     @Override
     public CachedNode getNode( NodeKey key ) {
-        return workspaceCache.getNode(key);
+        return workspaceCache().getNode(key);
     }
 
     @Override
@@ -288,16 +274,20 @@ public Iterator<NodeKey> getAllNodeKeysAtAndBelow( NodeKey startingKey ) {
     @Override
     public final void clear( CachedNode node ) {
         doClear(node);
-        if (workspaceCache != sharedWorkspaceCache && workspaceCache instanceof TransactionalWorkspaceCache) {
+        WorkspaceCache wscache = workspaceCache.get();
-            ((TransactionalWorkspaceCache)workspaceCache).clear();
+        if (wscache != sharedWorkspaceCache) {
+            assert wscache instanceof TransactionalWorkspaceCache;
+            wscache.clear();
         }
     }
 
     @Override
     public final void clear() {
         doClear();
-        if (workspaceCache != sharedWorkspaceCache && workspaceCache instanceof TransactionalWorkspaceCache) {
+        WorkspaceCache wscache = workspaceCache.get();
-            ((TransactionalWorkspaceCache)workspaceCache).clear();
+        if (wscache != sharedWorkspaceCache) {
+            assert wscache instanceof TransactionalWorkspaceCache;
+            wscache.clear();
         }
     }
 

diff --git a/modeshape-jcr/src/test/java/org/modeshape/jcr/cache/document/AbstractSessionCacheTest.java b/modeshape-jcr/src/test/java/org/modeshape/jcr/cache/document/AbstractSessionCacheTest.java
@@ -57,13 +57,15 @@ protected NodeCache createCache() {
         DocumentTranslator translator = new DocumentTranslator(context, documentStore, 100L);
         workspaceCache = new WorkspaceCache(context, "repo", "ws", documentStore, translator, ROOT_KEY_WS1, nodeCache, listener);
         loadJsonDocuments(resource(resourceNameForWorkspaceContentDocument()));
-        session1 = createSessionCache(context, workspaceCache);
+        SessionEnvironment sessionEnv = createSessionContext();
-        session2 = createSessionCache(context, workspaceCache);
+        session1 = createSessionCache(context, workspaceCache, sessionEnv);
+        session2 = createSessionCache(context, workspaceCache, sessionEnv);
         return session1;
     }
 
     protected abstract SessionCache createSessionCache( ExecutionContext context,
-                                                        WorkspaceCache cache );
+                                                        WorkspaceCache cache,
+                                                        SessionEnvironment sessionEnv );
 
     protected SessionEnvironment createSessionContext() {
         final TransactionManager txnMgr = txnManager();

diff --git a/modeshape-jcr/src/test/java/org/modeshape/jcr/cache/document/DocumentTranslatorTest.java b/modeshape-jcr/src/test/java/org/modeshape/jcr/cache/document/DocumentTranslatorTest.java
@@ -39,15 +39,17 @@
 import org.modeshape.jcr.cache.MutableCachedNode;
 import org.modeshape.jcr.cache.NodeKey;
 import org.modeshape.jcr.cache.SessionCache;
+import org.modeshape.jcr.cache.SessionEnvironment;
 import org.modeshape.jcr.value.Name;
 import org.modeshape.jcr.value.Path.Segment;
 
 public class DocumentTranslatorTest extends AbstractSessionCacheTest {
 
     @Override
     protected SessionCache createSessionCache( ExecutionContext context,
-                                               WorkspaceCache cache ) {
+                                               WorkspaceCache cache,
-        return new WritableSessionCache(context, workspaceCache, createSessionContext());
+                                               SessionEnvironment sessionEnv ) {
+        return new WritableSessionCache(context, workspaceCache, sessionEnv);
     }
 
     @Test
@@ -130,7 +132,7 @@ public void shouldSplitDocumentThatRepeatedlyContainsTooManyChildReferencesIntoM
         // Make it optimum to start out ...
         NodeKey key = nodeB.getKey();
         workspaceCache.translator().optimizeChildrenBlocks(key, null, 5, 2); // will merge into a single block ...
-        //Save the session, otherwise the database is inconsistent after the optimize operation
+        // Save the session, otherwise the database is inconsistent after the optimize operation
         session1.save();
         nodeB = check(session1).mutableNode("/childB");
         print(false);

diff --git a/modeshape-jcr/src/test/java/org/modeshape/jcr/cache/document/ReadOnlySessionCacheTest.java b/modeshape-jcr/src/test/java/org/modeshape/jcr/cache/document/ReadOnlySessionCacheTest.java
@@ -26,7 +26,6 @@
 import org.modeshape.jcr.ExecutionContext;
 import org.modeshape.jcr.cache.SessionCache;
 import org.modeshape.jcr.cache.SessionEnvironment;
-import org.modeshape.jcr.txn.Transactions;
 
 /**
  * Tests that operate against a {@link ReadOnlySessionCache}.
@@ -35,19 +34,8 @@ public class ReadOnlySessionCacheTest extends AbstractSessionCacheTest {
 
     @Override
     protected SessionCache createSessionCache( ExecutionContext context,
-                                               WorkspaceCache cache ) {
+                                               WorkspaceCache cache,
-        return new ReadOnlySessionCache(context, workspaceCache, new SessionEnvironment() {
+                                               SessionEnvironment sessionEnv ) {
-            private final TransactionalWorkspaceCaches transactionalWorkspaceCacheFactory = new TransactionalWorkspaceCaches(null);
+        return new ReadOnlySessionCache(context, workspaceCache, sessionEnv);
-
-            @Override
-            public TransactionalWorkspaceCaches getTransactionalWorkspaceCacheFactory() {
-                return transactionalWorkspaceCacheFactory;
-            }
-
-            @Override
-            public Transactions getTransactions() {
-                return null;
-            }
-        });
     }
 }
diff --git a/modeshape-jcr/src/test/java/org/modeshape/jcr/cache/document/WritableSessionCacheTest.java b/modeshape-jcr/src/test/java/org/modeshape/jcr/cache/document/WritableSessionCacheTest.java
@@ -37,6 +37,7 @@
 import org.modeshape.jcr.cache.MutableCachedNode;
 import org.modeshape.jcr.cache.NodeKey;
 import org.modeshape.jcr.cache.SessionCache;
+import org.modeshape.jcr.cache.SessionEnvironment;
 
 /**
  * Tests that operate against a {@link WritableSessionCache}. Each test method starts with a clean slate of content, which is
@@ -45,8 +46,9 @@ public class WritableSessionCacheTest extends AbstractSessionCacheTest {
 
     @Override
     protected SessionCache createSessionCache( ExecutionContext context,
-                                               WorkspaceCache cache ) {
+                                               WorkspaceCache cache,
-        return new WritableSessionCache(context, workspaceCache, createSessionContext());
+                                               SessionEnvironment sessionEnv ) {
+        return new WritableSessionCache(context, workspaceCache, sessionEnv);
     }
 
     @Test