Merged DEV to HEAD

31651: Fixed up concurrency tests: target concurrent aspect adds in addition to numeric property increments
   31652: Ensure DB-based concurrency problems are propagated when updating alf_node (not just optimistic lock detections)
   31823: TransactionalCache provides REPEATABLE READ
          - Values found in shared cache are placed into transactional cache
          - Previously, values could keep changing until first write (READ COMMITTED)
            but now the first read sets the value until it is changed by the current
            transaction
   31825: Minor comment about node version rollover after version=32767
   31826: Immutable node caches: properties, aspects and parent assocs are immutable
          - cache entries are only put but never updated
          - zero cluster overhead for these 3 caches
          - Stale nodeCache detection when reading properties, aspects or parent assocs
          - Added tests to introspect on the caches directly to validate behaviour
          - Ensure that each node gets a single version increment per transaction
   31854: Cater for cm:auditable changes during touchNode()


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@31912 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

config/alfresco/ibatis/org.hibernate.dialect.Dialect/node-common-SqlMap.xml

@@ -403,7 +403,7 @@
             </if>
         where
             id = #{id}
-        <if test="version gt 0">
+        <if test="version gt 0"><!-- Switches back to 0 after 32767 -->
             and version = (#{version} - 1)
         </if>
     </update>

source/java/org/alfresco/repo/cache/CacheTest.java

@@ -204,6 +204,13 @@ public class CacheTest extends TestCase
             assertNull("Get didn't return null", transactionalCache.get(NEW_GLOBAL_ONE));
             assertTrue("Item was removed from backing cache", backingCache.contains(NEW_GLOBAL_ONE));
             
+            // read 2 from the cache
+            assertEquals("Item not read from backing cache", NEW_GLOBAL_TWO, transactionalCache.get(NEW_GLOBAL_TWO));
+            // Change the backing cache
+            backingCache.put(NEW_GLOBAL_TWO, NEW_GLOBAL_TWO + "-updated");
+            // Ensure read-committed
+            assertEquals("Read-committed not preserved", NEW_GLOBAL_TWO, transactionalCache.get(NEW_GLOBAL_TWO));
+            
             // update 3 in the cache
             transactionalCache.put(UPDATE_TXN_THREE, "XXX");
             assertEquals("Item not updated in txn cache", "XXX", transactionalCache.get(UPDATE_TXN_THREE));

source/java/org/alfresco/repo/cache/TransactionalCache.java

@@ -310,8 +310,6 @@ public class TransactionalCache<K extends Serializable, V extends Object>
                 // txn resources.
             }
             else            // The txn is still active
-            {
-                try
             {
                 if (!txnData.isClearOn)   // deletions cache only useful before a clear
                 {
@@ -344,10 +342,19 @@ public class TransactionalCache<K extends Serializable, V extends Object>
                     }
                     return value;
                 }
-                }
+                else if (txnData.isClearOn)
-                catch (CacheException e)
                 {
-                    throw new AlfrescoRuntimeException("Cache failure", e);
+                    // Can't store values in the current txn any more
+                    ignoreSharedCache = true;
+                }
+                else
+                {
+                    // There is no in-txn entry for the key
+                    // Use the value direct from the shared cache
+                    V value = getSharedCacheValue(key);
+                    bucket = new ReadCacheBucket<V>(value);
+                    txnData.updatedItemsCache.put(key, bucket);
+                    return value;
                 }
                 // check if the cleared flag has been set - cleared flag means ignore shared as unreliable
                 ignoreSharedCache = txnData.isClearOn;
@@ -929,6 +936,37 @@ public class TransactionalCache<K extends Serializable, V extends Object>
         }
     }
     
+    /**
+     * Data holder to represent data read from the shared cache.  It will not attempt to
+     * update the shared cache.
+     */
+    private static class ReadCacheBucket<BV> implements CacheBucket<BV>
+    {
+        private static final long serialVersionUID = 7885689778259779578L;
+        
+        private final BV value;
+        public ReadCacheBucket(BV value)
+        {
+            this.value = value;
+        }
+        public BV getValue()
+        {
+            return value;
+        }
+        public void doPreCommit(
+                SimpleCache<Serializable, Object> sharedCache,
+                Serializable key,
+                boolean mutable, boolean readOnly)
+        {
+        }
+        public void doPostCommit(
+                SimpleCache<Serializable, Object> sharedCache,
+                Serializable key,
+                boolean mutable, boolean readOnly)
+        {
+        }
+    }
+    
     /** Data holder to bind data to the transaction */
     private class TransactionData
     {

source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java

@@ -1292,7 +1292,6 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
                     childNode.getId(),
                     newChildNodeId);
             // The new node will have new data not present in the cache, yet
-            // TODO: Look to move the data in a cache-efficient way
             invalidateNodeCaches(newChildNodeId);
             invalidateNodeChildrenCaches(newChildNodeId, true, true);
             invalidateNodeChildrenCaches(newChildNodeId, false, true);
@@ -1309,13 +1308,13 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
             childNodeUpdate.setUpdateDeleted(true);
             // Update the entity.
             // Note: We don't use delete here because that will attempt to clean everything up again.
-            updateNodeImpl(childNode, childNodeUpdate);
+            updateNodeImpl(childNode, childNodeUpdate, null);
             // There is no need to invalidate the caches as the touched node's version will have progressed
         }
         else
         {
             // Touch the node; make sure parent assocs are invalidated
-            touchNode(childNodeId, null, false, false, true);
+            touchNode(childNodeId, null, null, false, false, true);
         }
         
         final Long newChildNodeId = newChildNode.getId();
@@ -1417,7 +1416,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
             nodeUpdate.setUpdateLocaleId(true);
         }
 
-        return updateNodeImpl(oldNode, nodeUpdate);
+        return updateNodeImpl(oldNode, nodeUpdate, null);
     }
     
     /**
@@ -1432,9 +1431,13 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
      * node is modified but in the same transaction, then the cache entries are considered good and
      * pull forward against the current version of the node ... <b>unless</b> the cache was specicially
      * tagged for invalidation.
+     * <p/>
+     * It is sometime necessary to provide the node's current aspects, particularly during
+     * changes to the aspect list.  If not provided, they will be looked up.
      * 
      * @param nodeId                        the ID of the node (must refer to a live node)
      * @param auditableProps                optionally override the <b>cm:auditable</b> values
+     * @param nodeAspects                   the node's aspects or <tt>null</tt> to look them up
      * @param invalidateNodeAspectsCache    <tt>true</tt> if the node's cached aspects are unreliable
      * @param invalidateNodePropertiesCache <tt>true</tt> if the node's cached properties are unreliable
      * @param invalidateParentAssocsCache   <tt>true</tt> if the node's cached parent assocs are unreliable
@@ -1442,7 +1445,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
      * @see #updateNodeImpl(NodeEntity, NodeUpdateEntity)
      */
     private boolean touchNode(
-            Long nodeId, AuditablePropertiesEntity auditableProps,
+            Long nodeId, AuditablePropertiesEntity auditableProps, Set<QName> nodeAspects,
             boolean invalidateNodeAspectsCache,
             boolean invalidateNodePropertiesCache,
             boolean invalidateParentAssocsCache)
@@ -1463,12 +1466,12 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
         nodeUpdate.setId(nodeId);
         nodeUpdate.setAuditableProperties(auditableProps);
         // Update it
-        boolean updatedNode = updateNodeImpl(node, nodeUpdate);
+        boolean updatedNode = updateNodeImpl(node, nodeUpdate, nodeAspects);
         // Handle the cache invalidation requests
-        Node newNode = getNodeNotNull(nodeId);
         NodeVersionKey nodeVersionKey = node.getNodeVersionKey();
         if (updatedNode)
         {
+            Node newNode = getNodeNotNull(nodeId);
             NodeVersionKey newNodeVersionKey = newNode.getNodeVersionKey();
             // The version will have moved on, effectively rendering our caches invalid.
             // Copy over caches that DON'T need invalidating
@@ -1508,9 +1511,10 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
      * 
      * @param oldNode               the existing node, fully populated
      * @param nodeUpdate            the node update with all update elements populated
+     * @param nodeAspects           the node's aspects or <tt>null</tt> to look them up
      * @return                      <tt>true</tt> if any updates were made
      */
-    private boolean updateNodeImpl(Node oldNode, NodeUpdateEntity nodeUpdate)
+    private boolean updateNodeImpl(Node oldNode, NodeUpdateEntity nodeUpdate, Set<QName> nodeAspects)
     {
         Long nodeId = oldNode.getId();
         
@@ -1552,7 +1556,10 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
             nodeUpdate.setUpdateTransaction(true);
         }
         // Update auditable
-        Set<QName> nodeAspects = getNodeAspects(nodeId);
+        if (nodeAspects == null)
+        {
+            nodeAspects = getNodeAspects(nodeId);
+        }
         if (nodeAspects.contains(ContentModel.ASPECT_AUDITABLE))
         {
             NodeRef oldNodeRef = oldNode.getNodeRef();
@@ -1607,7 +1614,16 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
         }
         
         // The node is remaining in the current store
-        int count = updateNode(nodeUpdate);
+        int count = 0;
+        Throwable concurrencyException = null;
+        try
+        {
+            count = updateNode(nodeUpdate);
+        }
+        catch (Throwable e)
+        {
+            concurrencyException = e;
+        }
         // Do concurrency check
         if (count != 1)
         {
@@ -1615,15 +1631,23 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
             nodesCache.removeByKey(nodeId);
             nodesCache.removeByValue(nodeUpdate);
             
-            throw new ConcurrencyFailureException("Failed to update node " + nodeId);
+            throw new ConcurrencyFailureException("Failed to update node " + nodeId, concurrencyException);
         }
         else
         {
+            // Check for wrap-around in the version number
+            if (nodeUpdate.getVersion().equals(LONG_ZERO))
+            {
+                // The version was wrapped back to zero
+                // The caches that are keyed by version are now unreliable
+                propertiesCache.clear();
+                aspectsCache.clear();
+                parentAssocsCache.clear();
+            }
             // Update the caches
             nodeUpdate.lock();
             nodesCache.setValue(nodeId, nodeUpdate);
             // The node's version has moved on so no need to invalidate caches
-            // TODO: Should we copy values between the cache keys?
         }
 
         // Done
@@ -1644,7 +1668,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
         nodeUpdateEntity.setId(nodeId);
         nodeUpdateEntity.setAclId(aclId);
         nodeUpdateEntity.setUpdateAclId(true);
-        updateNodeImpl(oldNode, nodeUpdateEntity);
+        updateNodeImpl(oldNode, nodeUpdateEntity, null);
     }
     
     public void setPrimaryChildrenSharedAclId(
@@ -1682,7 +1706,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
         nodeUpdate.setTypeQNameId(deletedQNameId);
         nodeUpdate.setUpdateTypeQNameId(true);
         
-        boolean updated = updateNodeImpl(node, nodeUpdate);
+        boolean updated = updateNodeImpl(node, nodeUpdate, nodeAspects);
         if (!updated)
         {
             invalidateNodeCaches(nodeId);
@@ -1701,9 +1725,11 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
         // Delete content usage deltas
         usageDAO.deleteDeltas(nodeId);
 
+        // Handle sys:aspect_root
         if (nodeAspects.contains(ContentModel.ASPECT_ROOT))
         {
-            allRootNodesCache.remove(node.getNodePair().getSecond().getStoreRef());
+            StoreRef storeRef = node.getStore().getStoreRef();
+            allRootNodesCache.remove(storeRef);
         }
         
         // Remove peer associations (no associated cache)
@@ -1970,6 +1996,26 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
         boolean modifyProps = propsToDelete.size() > 0 || propsToAdd.size() > 0;
         boolean updated = modifyProps || nodeUpdate.isUpdateAnything();
         
+        // Bring the node into the current transaction
+        if (nodeUpdate.isUpdateAnything())
+        {
+            // We have to explicitly update the node (sys:locale or cm:auditable)
+            if (updateNodeImpl(node, nodeUpdate, null))
+            {
+                // Copy the caches across
+                NodeVersionKey nodeVersionKey = node.getNodeVersionKey();
+                NodeVersionKey newNodeVersionKey = getNodeNotNull(nodeId).getNodeVersionKey();
+                copyNodeAspectsCached(nodeVersionKey, newNodeVersionKey);
+                copyNodePropertiesCached(nodeVersionKey, newNodeVersionKey);
+                copyParentAssocsCached(nodeVersionKey, newNodeVersionKey);
+            }
+        }
+        else if (modifyProps)
+        {
+            // Touch the node; all caches are fine
+            touchNode(nodeId, null, null, false, false, false);
+        }
+        
         // Touch to bring into current txn
         if (modifyProps)
         {
@@ -2033,12 +2079,6 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
             // Update cache
             setNodePropertiesCached(nodeId, propsToCache);
         }
-        // Touch to bring into current transaction
-        if (updated)
-        {
-            // We have to explicitly update the node (sys:locale or cm:auditable)
-            updateNodeImpl(node, nodeUpdate);
-        }
         
         // Done
         if (isDebugEnabled && updated)
@@ -2100,12 +2140,12 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
 
         if (deleteCount > 0)
         {
+            // Touch the node; all caches are fine
+            touchNode(nodeId, null, null, false, false, false);
             // Update cache
             Map<QName, Serializable> cachedProps = getNodePropertiesCached(nodeId);
             cachedProps.keySet().removeAll(propertyQNames);
             setNodePropertiesCached(nodeId, cachedProps);
-            // Touch the node; all caches are fine
-            touchNode(nodeId, null, false, false, false);
         }
         // Done
         return deleteCount > 0;
@@ -2143,7 +2183,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
             {
                 policyBehaviourFilter.disableBehaviour(nodeRef, ContentModel.ASPECT_AUDITABLE);
                 // Touch the node; all caches are fine
-                return touchNode(nodeId, auditableProps, false, false, false);
+                return touchNode(nodeId, auditableProps, null, false, false, false);
             }
             finally
             {
@@ -2238,7 +2278,6 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
         {
             Long nodeId = nodeVersionKey.getNodeId();
             Map<NodeVersionKey, Map<NodePropertyKey, NodePropertyValue>> propsRawByNodeVersionKey = selectNodeProperties(nodeId);
-            // Check the node Txn ID for mismatch
             Map<NodePropertyKey, NodePropertyValue> propsRaw = propsRawByNodeVersionKey.get(nodeVersionKey);
             if (propsRaw == null)
             {
@@ -2250,7 +2289,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
                 }
                 else
                 {
-                    // We found properties associated with a different node ID and txn
+                    // We found properties associated with a different node ID and version
                     invalidateNodeCaches(nodeId);
                     throw new DataIntegrityViolationException(
                             "Detected stale node entry: " + nodeVersionKey +
@@ -2334,42 +2373,44 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
             executeBatch();
         }
         
-        // Manually update the cache
+        // Collate the new aspect set, so that touch recognizes the addtion of cm:auditable
         Set<QName> newAspectQNames = new HashSet<QName>(existingAspectQNames);
         newAspectQNames.addAll(aspectQNamesToAdd);
-        setNodeAspectsCached(nodeId, newAspectQNames);
 
-        if (aspectQNamesToAdd.contains(ContentModel.ASPECT_ROOT))
+        // Handle sys:aspect_root
+        if (aspectQNames.contains(ContentModel.ASPECT_ROOT))
         {
-            // This is a special case.  The presence of the aspect affects the path
-            // calculations, which are stored in the parent assocs cache
-            touchNode(nodeId, null, false, false, true);
-
             // invalidate root nodes cache for the store
-            Pair<Long, NodeRef> nodePair = getNodePair(nodeId);
+            StoreRef storeRef = getNodeNotNull(nodeId).getStore().getStoreRef();
-            StoreRef storeRef = nodePair.getSecond().getStoreRef();
             allRootNodesCache.remove(storeRef);
+            // Touch the node; parent assocs need invalidation
+            touchNode(nodeId, null, newAspectQNames, false, false, true);
         }
         else
         {
             // Touch the node; all caches are fine
-            touchNode(nodeId, null, false, false, false);
+            touchNode(nodeId, null, newAspectQNames, false, false, false);
         }
 
+        // Manually update the cache
+        setNodeAspectsCached(nodeId, newAspectQNames);
+
         // Done
         return true;
     }
 
     public boolean removeNodeAspects(Long nodeId)
     {
+        Set<QName> newAspectQNames = Collections.<QName>emptySet();
+        
+        // Touch the node; all caches are fine
+        touchNode(nodeId, null, newAspectQNames, false, false, false);
+
         // Just delete all the node's aspects
         int deleteCount = deleteNodeAspects(nodeId, null);
         
         // Manually update the cache
-        setNodeAspectsCached(nodeId, Collections.<QName>emptySet());
+        setNodeAspectsCached(nodeId, newAspectQNames);
-
-        // Touch the node; all caches are fine
-        touchNode(nodeId, null, false, false, false);
 
         // Done
         return deleteCount > 0;
@@ -2383,6 +2424,14 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
         }
         // Get the current aspects
         Set<QName> existingAspectQNames = getNodeAspects(nodeId);
+
+        // Collate the new set of aspects so that touch works correctly against cm:auditable
+        Set<QName> newAspectQNames = new HashSet<QName>(existingAspectQNames);
+        newAspectQNames.removeAll(aspectQNames);
+
+        // Touch the node; all caches are fine
+        touchNode(nodeId, null, newAspectQNames, false, false, false);
+
         // Now remove each aspect
         Set<Long> aspectQNameIdsToRemove = qnameDAO.convertQNamesToIds(aspectQNames, false);
         int deleteCount = deleteNodeAspects(nodeId, aspectQNameIdsToRemove);
@@ -2391,29 +2440,24 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
             return false;
         }
 
-        // Manually update the cache
+        // Handle sys:aspect_root
-        Set<QName> newAspectQNames = new HashSet<QName>(existingAspectQNames);
-        newAspectQNames.removeAll(aspectQNames);
-        setNodeAspectsCached(nodeId, newAspectQNames);
-
-        // If we are removing the sys:aspect_root, then the parent assocs cache is unreliable
         if (aspectQNames.contains(ContentModel.ASPECT_ROOT))
         {
-            // This is a special case.  The presence of the aspect affects the path
-            // calculations, which are stored in the parent assocs cache
-            touchNode(nodeId, null, false, false, true);
-            
             // invalidate root nodes cache for the store
-            Pair<Long, NodeRef> nodePair = getNodePair(nodeId);
+            StoreRef storeRef = getNodeNotNull(nodeId).getStore().getStoreRef();
-            StoreRef storeRef = nodePair.getSecond().getStoreRef();
             allRootNodesCache.remove(storeRef);
+            // Touch the node; parent assocs need invalidation
+            touchNode(nodeId, null, newAspectQNames, false, false, true);
         }
         else
         {
             // Touch the node; all caches are fine
-            touchNode(nodeId, null, false, false, false);
+            touchNode(nodeId, null, newAspectQNames, false, false, false);
         }
 
+        // Manually update the cache
+        setNodeAspectsCached(nodeId, newAspectQNames);
+
         // Done
         return deleteCount > 0;
     }
@@ -2498,7 +2542,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
                 }
                 else
                 {
-                    // We found properties associated with a different node ID and txn
+                    // We found properties associated with a different node ID and version
                     invalidateNodeCaches(nodeId);
                     throw new DataIntegrityViolationException(
                             "Detected stale node entry: " + nodeVersionKey +
@@ -2523,7 +2567,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
         }
         
         // Touch the node; all caches are fine
-        touchNode(sourceNodeId, null, false, false, false);
+        touchNode(sourceNodeId, null, null, false, false, false);
 
         // Resolve type QName
         Long assocTypeQNameId = qnameDAO.getOrCreateQName(assocTypeQName).getFirst();
@@ -2574,7 +2618,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
         if (deleted > 0)
         {
             // Touch the node; all caches are fine
-            touchNode(sourceNodeId, null, false, false, false);
+            touchNode(sourceNodeId, null, null, false, false, false);
         }
         return deleted;
     }
@@ -2585,7 +2629,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
         if (deleted > 0)
         {
             // Touch the node; all caches are fine
-            touchNode(nodeId, null, false, false, false);
+            touchNode(nodeId, null, null, false, false, false);
         }
         return deleted;
     }
@@ -2603,7 +2647,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
         if (deleted > 0)
         {
             // Touch the node; all caches are fine
-            touchNode(nodeId, null, false, false, false);
+            touchNode(nodeId, null, null, false, false, false);
         }
         return deleted;
     }
@@ -2795,7 +2839,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
                 parentNodeId, childNodeId, false, assocTypeQName, assocQName, childNodeName);
         Long assocId = assoc.getId();
         // Touch the node; all caches are fine
-        touchNode(childNodeId, null, false, false, false);
+        touchNode(childNodeId, null, null, false, false, false);
         // update cache
         parentAssocInfo = parentAssocInfo.addAssoc(assocId, assoc);
         setParentAssocsCached(childNodeId, parentAssocInfo);
@@ -2820,7 +2864,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
             throw new ConcurrencyFailureException("Child association not deleted: " + assocId);
         }
         // Touch the node; all caches are fine
-        touchNode(childNodeId, null, false, false, false);
+        touchNode(childNodeId, null, null, false, false, false);
         // Update cache
         parentAssocInfo = parentAssocInfo.removeAssoc(assocId);
         setParentAssocsCached(childNodeId, parentAssocInfo);
@@ -2832,7 +2876,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
         if (count > 0)
         {
             // Touch the node; parent assocs are out of sync
-            touchNode(childNodeId, null, false, false, true);
+            touchNode(childNodeId, null, null, false, false, true);
         }
         return count;
     }
@@ -2865,7 +2909,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
         if (count > 0)
         {
             // Touch the node; parent assocs are out of sync
-            touchNode(childNodeId, null, false, false, true);
+            touchNode(childNodeId, null, null, false, false, true);
         }
         
         if (isDebugEnabled)
@@ -3618,7 +3662,6 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
      * Bulk caching
      */
 
-    // TODO there must be a way to limit the repeated code here and in cacheNodes(List<NodeRef>)
     public void cacheNodesById(List<Long> nodeIds)
     {
         /*

source/java/org/alfresco/repo/domain/node/ParentAssocsInfo.java

@@ -36,7 +36,7 @@ import org.apache.commons.logging.LogFactory;
  * @author Derek Hulley
  * @since 3.4
  */
-/* package */ class ParentAssocsInfo implements Serializable
+public class ParentAssocsInfo implements Serializable
 {
     private static final long serialVersionUID = -2167221525380802365L;
     

source/java/org/alfresco/repo/node/BaseNodeServiceTest_model.xml

@@ -468,6 +468,27 @@
          </properties>
       </aspect>
             
+      <aspect name="test:Thread-0-0" />
+      <aspect name="test:Thread-0-1" />
+      <aspect name="test:Thread-0-2" />
+      <aspect name="test:Thread-0-3" />
+      <aspect name="test:Thread-0-4" />
+      <aspect name="test:Thread-0-5" />
+      <aspect name="test:Thread-0-6" />
+      <aspect name="test:Thread-0-7" />
+      <aspect name="test:Thread-0-8" />
+      <aspect name="test:Thread-0-9" />
+      <aspect name="test:Thread-1-0" />
+      <aspect name="test:Thread-1-1" />
+      <aspect name="test:Thread-1-2" />
+      <aspect name="test:Thread-1-3" />
+      <aspect name="test:Thread-1-4" />
+      <aspect name="test:Thread-1-5" />
+      <aspect name="test:Thread-1-6" />
+      <aspect name="test:Thread-1-7" />
+      <aspect name="test:Thread-1-8" />
+      <aspect name="test:Thread-1-9" />
+            
    </aspects>
 
 </model>

source/java/org/alfresco/repo/node/ConcurrentNodeServiceTest.java

@@ -19,35 +19,24 @@
 package org.alfresco.repo.node;
 
 import java.io.InputStream;
+import java.io.Serializable;
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-
+import java.util.Set;
-import javax.transaction.UserTransaction;
 
 import junit.framework.TestCase;
 
-import org.alfresco.model.ContentModel;
 import org.alfresco.repo.dictionary.DictionaryDAO;
 import org.alfresco.repo.dictionary.M2Model;
-import org.alfresco.repo.search.impl.lucene.fts.FullTextSearchIndexer;
 import org.alfresco.repo.security.authentication.AuthenticationComponent;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
-import org.alfresco.repo.tagging.TaggingServiceImpl;
 import org.alfresco.repo.transaction.RetryingTransactionHelper;
 import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback;
 import org.alfresco.service.ServiceRegistry;
-import org.alfresco.service.cmr.repository.ChildAssociationRef;
-import org.alfresco.service.cmr.repository.MLText;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
 import org.alfresco.service.cmr.repository.StoreRef;
-import org.alfresco.service.cmr.search.ResultSet;
-import org.alfresco.service.cmr.search.SearchService;
-import org.alfresco.service.namespace.DynamicNamespacePrefixResolver;
-import org.alfresco.service.namespace.NamespacePrefixResolver;
-import org.alfresco.service.namespace.NamespaceService;
 import org.alfresco.service.namespace.QName;
 import org.alfresco.service.transaction.TransactionService;
 import org.alfresco.util.ApplicationContextHelper;
@@ -56,11 +45,9 @@ import org.apache.commons.logging.LogFactory;
 import org.springframework.context.ApplicationContext;
 
 /**
- * @author Andy Hind
  * @author Nick Burch
  * @author Derek Hulley
  */
-@SuppressWarnings("unused")
 public class ConcurrentNodeServiceTest extends TestCase
 {
     public static final String NAMESPACE = "http://www.alfresco.org/test/BaseNodeServiceTest";
@@ -79,7 +66,6 @@ public class ConcurrentNodeServiceTest extends TestCase
 
     private NodeService nodeService;
     private TransactionService transactionService;
-    private RetryingTransactionHelper retryingTransactionHelper;
     private NodeRef rootNodeRef;
     private AuthenticationComponent authenticationComponent;
 
@@ -103,10 +89,10 @@ public class ConcurrentNodeServiceTest extends TestCase
         model = M2Model.createModel(modelStream);
         dictionaryDao.putModel(model);
 
-        nodeService = (NodeService) ctx.getBean("dbNodeService");
+        ServiceRegistry serviceRegistry = (ServiceRegistry) ctx.getBean(ServiceRegistry.SERVICE_REGISTRY);
-        transactionService = (TransactionService) ctx.getBean("transactionComponent");
+        nodeService = serviceRegistry.getNodeService();
-        retryingTransactionHelper = (RetryingTransactionHelper) ctx.getBean("retryingTransactionHelper");
+        transactionService = serviceRegistry.getTransactionService();
-        this.authenticationComponent = (AuthenticationComponent) ctx.getBean("authenticationComponent");
+        authenticationComponent = (AuthenticationComponent) ctx.getBean("authenticationComponent");
 
         this.authenticationComponent.setSystemUserAsCurrentUser();
 
@@ -121,7 +107,7 @@ public class ConcurrentNodeServiceTest extends TestCase
                 return null;
             }
         };
-        retryingTransactionHelper.doInTransaction(createRootNodeCallback);
+        transactionService.getRetryingTransactionHelper().doInTransaction(createRootNodeCallback);
     }
 
     @Override
@@ -131,184 +117,6 @@ public class ConcurrentNodeServiceTest extends TestCase
         super.tearDown();
     }
     
-    protected Map<QName, ChildAssociationRef> buildNodeGraph() throws Exception
-    {
-        return BaseNodeServiceTest.buildNodeGraph(nodeService, rootNodeRef);
-    }
-
-    protected Map<QName, ChildAssociationRef> commitNodeGraph() throws Exception
-    {
-        RetryingTransactionCallback<Map<QName, ChildAssociationRef>> buildGraphCallback =
-            new RetryingTransactionCallback<Map<QName, ChildAssociationRef>>()
-        {
-            public Map<QName, ChildAssociationRef> execute() throws Exception
-            {
-
-                Map<QName, ChildAssociationRef> answer = buildNodeGraph();
-                return answer;
-            }
-        };
-        return retryingTransactionHelper.doInTransaction(buildGraphCallback);
-    }
-
-    public void xtest1() throws Exception
-    {
-        testConcurrent();
-    }
-
-    public void xtest2() throws Exception
-    {
-        testConcurrent();
-    }
-
-    public void xtest3() throws Exception
-    {
-        testConcurrent();
-    }
-
-    public void xtest4() throws Exception
-    {
-        testConcurrent();
-    }
-
-    public void xtest5() throws Exception
-    {
-        testConcurrent();
-    }
-
-    public void xtest6() throws Exception
-    {
-        testConcurrent();
-    }
-
-    public void xtest7() throws Exception
-    {
-        testConcurrent();
-    }
-
-    public void xtest8() throws Exception
-    {
-        testConcurrent();
-    }
-
-    public void xtest9() throws Exception
-    {
-        testConcurrent();
-    }
-
-    public void xtest10() throws Exception
-    {
-        testConcurrent();
-    }
-
-    public void testConcurrent() throws Exception
-    {
-        Map<QName, ChildAssociationRef> assocRefs = commitNodeGraph();
-        Thread runner = null;
-
-        for (int i = 0; i < COUNT; i++)
-        {
-            runner = new Nester("Concurrent-" + i, runner, REPEATS);
-        }
-        if (runner != null)
-        {
-            runner.start();
-
-            try
-            {
-                runner.join();
-                System.out.println("Query thread has waited for " + runner.getName());
-            }
-            catch (InterruptedException e)
-            {
-                e.printStackTrace();
-            }
-        }
-
-        /*
-         * Builds a graph of child associations as follows:
-         * <pre>
-         * Level 0:     root
-         * Level 1:     root_p_n1   root_p_n2
-         * Level 2:     n1_p_n3     n2_p_n4     n1_n4       n2_p_n5     n1_n8
-         * Level 3:     n3_p_n6     n4_n6       n5_p_n7
-         * Level 4:     n6_p_n8     n7_n8
-         * </pre>
-         */
-        RetryingTransactionCallback<Object> testCallback = new RetryingTransactionCallback<Object>()
-        {
-            public Object execute() throws Exception
-            {
-                // There are two nodes at the base level in each test
-                assertEquals(2 * ((COUNT * REPEATS) + 1), nodeService.getChildAssocs(rootNodeRef).size());
-
-                SearchService searcher = (SearchService) ctx.getBean(ServiceRegistry.SEARCH_SERVICE.getLocalName());
-                assertEquals(
-                        2 * ((COUNT * REPEATS) + 1),
-                        searcher.selectNodes(rootNodeRef, "/*", null, getNamespacePrefixResolver(""), false).size());
-
-                return null;
-            }
-        };
-        retryingTransactionHelper.doInTransaction(testCallback);
-    }
-
-    /**
-     * Daemon thread
-     */
-    private class Nester extends Thread
-    {
-        Thread waiter;
-
-        int repeats;
-
-        Nester(String name, Thread waiter, int repeats)
-        {
-            super(name);
-            this.setDaemon(true);
-            this.waiter = waiter;
-            this.repeats = repeats;
-        }
-
-        public void run()
-        {
-            authenticationComponent.setSystemUserAsCurrentUser();
-
-            if (waiter != null)
-            {
-                System.out.println("Starting " + waiter.getName());
-                waiter.start();
-            }
-            try
-            {
-                System.out.println("Start " + this.getName());
-                for (int i = 0; i < repeats; i++)
-                {
-                    Map<QName, ChildAssociationRef> assocRefs = commitNodeGraph();
-                    System.out.println(" " + this.getName() + " " + i);
-                }
-                System.out.println("End " + this.getName());
-            }
-            catch (Exception e)
-            {
-                e.printStackTrace();
-            }
-            if (waiter != null)
-            {
-                try
-                {
-                    waiter.join();
-                    System.out.println("Thread "
-                            + this.getName() + " has waited for " + (waiter == null ? "null" : waiter.getName()));
-                }
-                catch (InterruptedException e)
-                {
-                    System.err.println(e);
-                }
-            }
-        }
-    }
-    
     /**
      * Tests that when multiple threads try to edit different
      *  properties on a node, that transactions + retries always
@@ -316,10 +124,10 @@ public class ConcurrentNodeServiceTest extends TestCase
      *  
      * @since 3.4 
      */
-    public void testMultiThreadedNodePropertiesWrites() throws Exception
+    public void testMultiThreaded_PropertyWrites() throws Exception
     {
         final List<Thread> threads = new ArrayList<Thread>();
-        final int loops = 200;
+        final int loops = 1000;
 
         // Have 5 threads, each trying to edit their own properties on the same node
         // Loop repeatedly
@@ -328,11 +136,13 @@ public class ConcurrentNodeServiceTest extends TestCase
                 QName.createQName("test2", "MadeUp2"),
                 QName.createQName("test3", "MadeUp3"),
                 QName.createQName("test4", "MadeUp4"),
-                QName.createQName("test5", "MadeUp5")
+                QName.createQName("test5", "MadeUp5"),
         };
-        for(QName prop : properties)
+        final int[] propCounts = new int[properties.length];
+        for (int propNum = 0; propNum < properties.length; propNum++)
         {
-            final QName property = prop;
+            final QName property = properties[propNum];
+            final int propNumFinal = propNum;
 
             // Zap the property if it is there
             transactionService.getRetryingTransactionHelper().doInTransaction(
@@ -364,7 +174,7 @@ public class ConcurrentNodeServiceTest extends TestCase
 
                     // Loop, incrementing each time
                     // If we miss an update, then at the end it'll be obvious
-                    AuthenticationUtil.setFullyAuthenticatedUser(AuthenticationUtil.getSystemUserName());
+                    AuthenticationUtil.setRunAsUserSystem();
                     for (int i = 0; i < loops; i++)
                     {
                         RetryingTransactionCallback<Integer> callback = new RetryingTransactionCallback<Integer>()
@@ -381,14 +191,31 @@ public class ConcurrentNodeServiceTest extends TestCase
                                 }
                                 // Increment by one. Really should be this!
                                 current++;
-                                // Save the new value
                                 nodeService.setProperty(rootNodeRef, property, Integer.valueOf(current));
+                                // Check that the value is what we expected it to be
+                                // We do this after the update so that we don't fall on retries
+                                int expectedCurrent = propCounts[propNumFinal];
+                                if (expectedCurrent != (current - 1))
+                                {
+                                    // We have a difference here already
+                                    // It will never catch up, but we'll detect that later
+                                    System.out.println("Found difference: " + Thread.currentThread().getName() + " " + current);
+                                }
                                 return current;
                             }
                         };
+                        try
+                        {
                             RetryingTransactionHelper txnHelper = transactionService.getRetryingTransactionHelper();
                             txnHelper.setMaxRetries(loops);
-                        txnHelper.doInTransaction(callback, false, true);
+                            Integer newCount = txnHelper.doInTransaction(callback, false, true);
+//                            System.out.println("Set value: " + Thread.currentThread().getName() + " " + newCount);
+                            propCounts[propNumFinal] = newCount;
+                        }
+                        catch (Throwable e)
+                        {
+                            logger.error("Failed to set value: ", e);
+                        }
                     }
 
                     // Report us as finished
@@ -412,36 +239,19 @@ public class ConcurrentNodeServiceTest extends TestCase
             t.join();
         }
         
-        // Check each property in turn
+        Map<QName, Serializable> nodeProperties = nodeService.getProperties(rootNodeRef);
-        RetryingTransactionCallback<Void> checkCallback = new RetryingTransactionCallback<Void>()
-        {
-            @Override
-            public Void execute() throws Throwable
-            {
-                HashMap<QName, Integer> values = new HashMap<QName, Integer>();
-                for(QName prop : properties)
-                {
-                    Object val = nodeService.getProperty(rootNodeRef, prop);
-                    Integer value = -1;
-                    if(val instanceof MLText)
-                    {
-                        value = Integer.valueOf( ((MLText)val).getValues().iterator().next() );
-                    }
-                    else
-                    {
-                        value = (Integer)val;
-                    }
-                    
-                    values.put(prop,value);
-                }
-                
         List<String> errors = new ArrayList<String>();
-                for(QName prop : properties)
+        for (int i =0; i < properties.length; i++)
         {
-                    Integer value = values.get(prop);
+            Integer value = (Integer) nodeProperties.get(properties[i]);
-                    if (value == null || !value.equals(new Integer(loops)))
+            if (value == null)
             {
-                        errors.add("\n   Prop " + prop + " : " + value);
+                errors.add("\n   Prop " + properties[i] + " : " + value);
+            }
+            if (!value.equals(new Integer(loops)))
+            {
+                errors.add("\n   Prop " + properties[i] + " : " + value);
+            }
         }
         if (errors.size() > 0)
         {
@@ -454,20 +264,74 @@ public class ConcurrentNodeServiceTest extends TestCase
             fail(sb.toString());
         }
     }
+    
+    /**
+     * Adds 'residual' aspects that are named according to the thread.  Multiple threads should all
+     * get their changes in. 
+     */
+    public void testMultithreaded_AspectWrites() throws Exception
+    {
+        final Thread[] threads = new Thread[2];
+        final int loops = 10;
+        
+        for (int i = 0; i < threads.length; i++)
+        {
+            final String name = "Thread-" + i + "-";
+            Runnable runnable = new Runnable()
+            {
+                @Override
+                public void run()
+                {
+                    AuthenticationUtil.setRunAsUserSystem();
+                    for (int loop = 0; loop < loops; loop++)
+                    {
+                        final String nameWithLoop = name + loop;
+                        RetryingTransactionCallback<Void> runCallback = new RetryingTransactionCallback<Void>()
+                        {
+                            @Override
+                            public Void execute() throws Throwable
+                            {
+                                // Add another aspect to the node
+                                QName qname = QName.createQName(NAMESPACE, nameWithLoop);
+                                nodeService.addAspect(rootNodeRef, qname, null);
                                 return null;
                             }
                         };
-        transactionService.getRetryingTransactionHelper().doInTransaction(checkCallback, true);
+                        RetryingTransactionHelper txnHelper = transactionService.getRetryingTransactionHelper();
-    }
+                        txnHelper.setMaxRetries(40);
-
+                        try
-    private NamespacePrefixResolver getNamespacePrefixResolver(String defaultURI)
                         {
-        DynamicNamespacePrefixResolver nspr = new DynamicNamespacePrefixResolver(null);
+                            txnHelper.doInTransaction(runCallback);
-        nspr.registerNamespace(NamespaceService.SYSTEM_MODEL_PREFIX, NamespaceService.SYSTEM_MODEL_1_0_URI);
+                        }
-        nspr.registerNamespace(NamespaceService.CONTENT_MODEL_PREFIX, NamespaceService.CONTENT_MODEL_1_0_URI);
+                        catch (Throwable e)
-        nspr.registerNamespace(NamespaceService.APP_MODEL_PREFIX, NamespaceService.APP_MODEL_1_0_URI);
+                        {
-        nspr.registerNamespace("namespace", "namespace");
+                            logger.error(e);
-        nspr.registerNamespace(NamespaceService.DEFAULT_PREFIX, defaultURI);
+                        }
-        return nspr;
+                    }
+                }
+            };
+            threads[i] = new Thread(runnable, name);
+        }
+        // Start all the threads
+        for (int i = 0; i < threads.length; i++)
+        {
+            threads[i].start();
+        }
+        // Wait for them all to finish
+        for (int i = 0; i < threads.length; i++)
+        {
+            threads[i].join();
+        }
+        // Check the aspects
+        Set<QName> aspects = nodeService.getAspects(rootNodeRef);
+        for (int i = 0; i < threads.length; i++)
+        {
+            for (int j = 0; j < loops; j++)
+            {
+                String nameWithLoop = "Thread-" + i + "-" + j;
+                QName qname = QName.createQName(NAMESPACE, nameWithLoop);
+                assertTrue("Missing aspect: "+ nameWithLoop, aspects.contains(qname));
+            }
+        }
     }
 }

source/java/org/alfresco/repo/node/NodeServiceTest.java

@@ -19,6 +19,7 @@
 package org.alfresco.repo.node;
 
 import java.io.Serializable;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -29,6 +30,11 @@ import java.util.Set;
 import junit.framework.TestCase;
 
 import org.alfresco.model.ContentModel;
+import org.alfresco.repo.cache.SimpleCache;
+import org.alfresco.repo.domain.node.Node;
+import org.alfresco.repo.domain.node.NodeVersionKey;
+import org.alfresco.repo.domain.node.ParentAssocsInfo;
+import org.alfresco.repo.policy.BehaviourFilter;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback;
 import org.alfresco.service.ServiceRegistry;
@@ -45,6 +51,7 @@ import org.alfresco.service.namespace.NamespaceService;
 import org.alfresco.service.namespace.QName;
 import org.alfresco.service.transaction.TransactionService;
 import org.alfresco.util.ApplicationContextHelper;
+import org.alfresco.util.GUID;
 import org.springframework.context.ApplicationContext;
 import org.springframework.extensions.surf.util.I18NUtil;
 
@@ -67,10 +74,15 @@ public class NodeServiceTest extends TestCase
     protected ServiceRegistry serviceRegistry;
     protected NodeService nodeService;
     private TransactionService txnService;
+    private SimpleCache<Serializable, Serializable> nodesCache;
+    private SimpleCache<Serializable, Serializable> propsCache;
+    private SimpleCache<Serializable, Serializable> aspectsCache;
+    private SimpleCache<Serializable, Serializable> parentAssocsCache;
     
     /** populated during setup */
     protected NodeRef rootNodeRef;
 
+    @SuppressWarnings("unchecked")
     @Override
     protected void setUp() throws Exception
     {
@@ -80,6 +92,18 @@ public class NodeServiceTest extends TestCase
         nodeService = serviceRegistry.getNodeService();
         txnService = serviceRegistry.getTransactionService();
         
+        // Get the caches for later testing
+        nodesCache = (SimpleCache<Serializable, Serializable>) ctx.getBean("node.nodesSharedCache");
+        propsCache = (SimpleCache<Serializable, Serializable>) ctx.getBean("node.propertiesSharedCache");
+        aspectsCache = (SimpleCache<Serializable, Serializable>) ctx.getBean("node.aspectsSharedCache");
+        parentAssocsCache = (SimpleCache<Serializable, Serializable>) ctx.getBean("node.parentAssocsSharedCache");
+        
+        // Clear the caches to remove fluff
+        nodesCache.clear();
+        propsCache.clear();
+        aspectsCache.clear();
+        parentAssocsCache.clear();
+        
         AuthenticationUtil.setRunAsUserSystem();
         
         // create a first store directly
@@ -210,14 +234,16 @@ public class NodeServiceTest extends TestCase
             @Override
             public Void execute() throws Throwable
             {
+                Map<QName, Serializable> props = new HashMap<QName, Serializable>(3);
+                props.put(ContentModel.PROP_NAME, "depth-" + 0 + "-" + GUID.generate());
                 liveNodeRefs[0] = nodeService.createNode(
                         workspaceRootNodeRef,
                         ContentModel.ASSOC_CHILDREN,
                         QName.createQName(NAMESPACE, "depth-" + 0),
-                        ContentModel.TYPE_FOLDER).getChildRef();
+                        ContentModel.TYPE_FOLDER,
+                        props).getChildRef();
                 for (int i = 1; i < liveNodeRefs.length; i++)
                 {
-                    Map<QName, Serializable> props = new HashMap<QName, Serializable>(3);
                     props.put(ContentModel.PROP_NAME, "depth-" + i);
                     liveNodeRefs[i] = nodeService.createNode(
                             liveNodeRefs[i-1],
@@ -566,4 +592,327 @@ public class NodeServiceTest extends TestCase
         assertNotNull("Did not find node by name", nodeRefCheck);
         assertEquals("Node found was not correct", newChildNodeRef, nodeRefCheck);
     }
+    
+    /**
+     * Looks for a key that contains the toString() of the value
+     */
+    private Object findCacheValue(SimpleCache<Serializable, Serializable> cache, Serializable key)
+    {
+        Collection<Serializable> keys = cache.getKeys();
+        for (Serializable keyInCache : keys)
+        {
+            String keyInCacheStr = keyInCache.toString();
+            String keyStr = key.toString();
+            if (keyInCacheStr.endsWith(keyStr))
+            {
+                Object value = cache.get(keyInCache);
+                return value;
+            }
+        }
+        return null;
+    }
+    
+    private static final QName PROP_RESIDUAL = QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, GUID.generate());
+    /**
+     * Check that simple node property modifications advance the node caches correctly
+     */
+    @SuppressWarnings("unchecked")
+    public void testCaches_ImmutableNodeCaches() throws Exception
+    {
+        final NodeRef[] nodeRefs = new NodeRef[2];
+        final NodeRef workspaceRootNodeRef = nodeService.getRootNode(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE);
+        buildNodeHierarchy(workspaceRootNodeRef, nodeRefs);
+        final NodeRef nodeRef = nodeRefs[1];
+
+        // Get the current node cache key
+        Long nodeId = (Long) findCacheValue(nodesCache, nodeRef);
+        assertNotNull("Node not found in cache", nodeId);
+        Node nodeOne = (Node) findCacheValue(nodesCache, nodeId);
+        assertNotNull("Node not found in cache", nodeOne);
+        NodeVersionKey nodeKeyOne = nodeOne.getNodeVersionKey();
+        
+        // Get the node cached values
+        Map<QName, Serializable> nodePropsOne = (Map<QName, Serializable>) findCacheValue(propsCache, nodeKeyOne);
+        Set<QName> nodeAspectsOne = (Set<QName>) findCacheValue(aspectsCache, nodeKeyOne);
+        ParentAssocsInfo nodeParentAssocsOne = (ParentAssocsInfo) findCacheValue(parentAssocsCache, nodeKeyOne);
+        
+        // Check the values
+        assertEquals("The node version is incorrect", Long.valueOf(1L), nodeKeyOne.getVersion());
+        assertNotNull("No cache entry for properties", nodePropsOne);
+        assertNotNull("No cache entry for aspects", nodeAspectsOne);
+        assertNotNull("No cache entry for parent assocs", nodeParentAssocsOne);
+        assertEquals("Property count incorrect", 1, nodePropsOne.size());
+        assertNotNull("Expected a cm:name property", nodePropsOne.get(ContentModel.PROP_NAME));
+        assertEquals("Aspect count incorrect", 1, nodeAspectsOne.size());
+        assertTrue("Expected a cm:auditable aspect", nodeAspectsOne.contains(ContentModel.ASPECT_AUDITABLE));
+        assertEquals("Parent assoc count incorrect", 1, nodeParentAssocsOne.getParentAssocs().size());
+        
+        // Add a property
+        nodeService.setProperty(nodeRef, PROP_RESIDUAL, GUID.generate());
+        
+        // Get the values for the previous version
+        Map<QName, Serializable> nodePropsOneCheck = (Map<QName, Serializable>) findCacheValue(propsCache, nodeKeyOne);
+        Set<QName> nodeAspectsOneCheck = (Set<QName>) findCacheValue(aspectsCache, nodeKeyOne);
+        ParentAssocsInfo nodeParentAssocsOneCheck = (ParentAssocsInfo) findCacheValue(parentAssocsCache, nodeKeyOne);
+        assertTrue("Previous cache entries must be left alone", nodePropsOneCheck == nodePropsOne);
+        assertTrue("Previous cache entries must be left alone", nodeAspectsOneCheck == nodeAspectsOne);
+        assertTrue("Previous cache entries must be left alone", nodeParentAssocsOneCheck == nodeParentAssocsOne);
+
+        // Get the current node cache key
+        Node nodeTwo = (Node) findCacheValue(nodesCache, nodeId);
+        assertNotNull("Node not found in cache", nodeTwo);
+        NodeVersionKey nodeKeyTwo = nodeTwo.getNodeVersionKey();
+        
+        // Get the node cached values
+        Map<QName, Serializable> nodePropsTwo = (Map<QName, Serializable>) findCacheValue(propsCache, nodeKeyTwo);
+        Set<QName> nodeAspectsTwo = (Set<QName>) findCacheValue(aspectsCache, nodeKeyTwo);
+        ParentAssocsInfo nodeParentAssocsTwo = (ParentAssocsInfo) findCacheValue(parentAssocsCache, nodeKeyTwo);
+
+        // Check the values
+        assertEquals("The node version is incorrect", Long.valueOf(2L), nodeKeyTwo.getVersion());
+        assertNotNull("No cache entry for properties", nodePropsTwo);
+        assertNotNull("No cache entry for aspects", nodeAspectsTwo);
+        assertNotNull("No cache entry for parent assocs", nodeParentAssocsTwo);
+        assertTrue("Properties must have moved on", nodePropsTwo != nodePropsOne);
+        assertEquals("Property count incorrect", 2, nodePropsTwo.size());
+        assertNotNull("Expected a cm:name property", nodePropsTwo.get(ContentModel.PROP_NAME));
+        assertNotNull("Expected a residual property", nodePropsTwo.get(PROP_RESIDUAL));
+        assertTrue("Aspects must be carried", nodeAspectsTwo == nodeAspectsOne);
+        assertTrue("Parent assocs must be carried", nodeParentAssocsTwo == nodeParentAssocsOne);
+        
+        // Remove a property
+        nodeService.removeProperty(nodeRef, PROP_RESIDUAL);
+        
+        // Get the values for the previous version
+        Map<QName, Serializable> nodePropsTwoCheck = (Map<QName, Serializable>) findCacheValue(propsCache, nodeKeyTwo);
+        Set<QName> nodeAspectsTwoCheck = (Set<QName>) findCacheValue(aspectsCache, nodeKeyTwo);
+        ParentAssocsInfo nodeParentAssocsTwoCheck = (ParentAssocsInfo) findCacheValue(parentAssocsCache, nodeKeyTwo);
+        assertTrue("Previous cache entries must be left alone", nodePropsTwoCheck == nodePropsTwo);
+        assertTrue("Previous cache entries must be left alone", nodeAspectsTwoCheck == nodeAspectsTwo);
+        assertTrue("Previous cache entries must be left alone", nodeParentAssocsTwoCheck == nodeParentAssocsTwo);
+
+        // Get the current node cache key
+        Node nodeThree = (Node) findCacheValue(nodesCache, nodeId);
+        assertNotNull("Node not found in cache", nodeThree);
+        NodeVersionKey nodeKeyThree = nodeThree.getNodeVersionKey();
+        
+        // Get the node cached values
+        Map<QName, Serializable> nodePropsThree = (Map<QName, Serializable>) findCacheValue(propsCache, nodeKeyThree);
+        Set<QName> nodeAspectsThree = (Set<QName>) findCacheValue(aspectsCache, nodeKeyThree);
+        ParentAssocsInfo nodeParentAssocsThree = (ParentAssocsInfo) findCacheValue(parentAssocsCache, nodeKeyThree);
+
+        // Check the values
+        assertEquals("The node version is incorrect", Long.valueOf(3L), nodeKeyThree.getVersion());
+        assertNotNull("No cache entry for properties", nodePropsThree);
+        assertNotNull("No cache entry for aspects", nodeAspectsThree);
+        assertNotNull("No cache entry for parent assocs", nodeParentAssocsThree);
+        assertTrue("Properties must have moved on", nodePropsThree != nodePropsTwo);
+        assertEquals("Property count incorrect", 1, nodePropsThree.size());
+        assertNotNull("Expected a cm:name property", nodePropsThree.get(ContentModel.PROP_NAME));
+        assertNull("Expected no residual property", nodePropsThree.get(PROP_RESIDUAL));
+        assertTrue("Aspects must be carried", nodeAspectsThree == nodeAspectsTwo);
+        assertTrue("Parent assocs must be carried", nodeParentAssocsThree == nodeParentAssocsTwo);
+        
+        // Add an aspect
+        nodeService.addAspect(nodeRef, ContentModel.ASPECT_TITLED, null);
+        
+        // Get the values for the previous version
+        Map<QName, Serializable> nodePropsThreeCheck = (Map<QName, Serializable>) findCacheValue(propsCache, nodeKeyThree);
+        Set<QName> nodeAspectsThreeCheck = (Set<QName>) findCacheValue(aspectsCache, nodeKeyThree);
+        ParentAssocsInfo nodeParentAssocsThreeCheck = (ParentAssocsInfo) findCacheValue(parentAssocsCache, nodeKeyThree);
+        assertTrue("Previous cache entries must be left alone", nodePropsThreeCheck == nodePropsThree);
+        assertTrue("Previous cache entries must be left alone", nodeAspectsThreeCheck == nodeAspectsThree);
+        assertTrue("Previous cache entries must be left alone", nodeParentAssocsThreeCheck == nodeParentAssocsThree);
+
+        // Get the current node cache key
+        Node nodeFour = (Node) findCacheValue(nodesCache, nodeId);
+        assertNotNull("Node not found in cache", nodeFour);
+        NodeVersionKey nodeKeyFour = nodeFour.getNodeVersionKey();
+        
+        // Get the node cached values
+        Map<QName, Serializable> nodePropsFour = (Map<QName, Serializable>) findCacheValue(propsCache, nodeKeyFour);
+        Set<QName> nodeAspectsFour = (Set<QName>) findCacheValue(aspectsCache, nodeKeyFour);
+        ParentAssocsInfo nodeParentAssocsFour = (ParentAssocsInfo) findCacheValue(parentAssocsCache, nodeKeyFour);
+
+        // Check the values
+        assertEquals("The node version is incorrect", Long.valueOf(4L), nodeKeyFour.getVersion());
+        assertNotNull("No cache entry for properties", nodePropsFour);
+        assertNotNull("No cache entry for aspects", nodeAspectsFour);
+        assertNotNull("No cache entry for parent assocs", nodeParentAssocsFour);
+        assertTrue("Properties must be carried", nodePropsFour == nodePropsThree);
+        assertTrue("Aspects must have moved on", nodeAspectsFour != nodeAspectsThree);
+        assertTrue("Expected cm:titled aspect", nodeAspectsFour.contains(ContentModel.ASPECT_TITLED));
+        assertTrue("Parent assocs must be carried", nodeParentAssocsFour == nodeParentAssocsThree);
+        
+        // Remove an aspect
+        nodeService.removeAspect(nodeRef, ContentModel.ASPECT_TITLED);
+        
+        // Get the values for the previous version
+        Map<QName, Serializable> nodePropsFourCheck = (Map<QName, Serializable>) findCacheValue(propsCache, nodeKeyFour);
+        Set<QName> nodeAspectsFourCheck = (Set<QName>) findCacheValue(aspectsCache, nodeKeyFour);
+        ParentAssocsInfo nodeParentAssocsFourCheck = (ParentAssocsInfo) findCacheValue(parentAssocsCache, nodeKeyFour);
+        assertTrue("Previous cache entries must be left alone", nodePropsFourCheck == nodePropsFour);
+        assertTrue("Previous cache entries must be left alone", nodeAspectsFourCheck == nodeAspectsFour);
+        assertTrue("Previous cache entries must be left alone", nodeParentAssocsFourCheck == nodeParentAssocsFour);
+
+        // Get the current node cache key
+        Node nodeFive = (Node) findCacheValue(nodesCache, nodeId);
+        assertNotNull("Node not found in cache", nodeFive);
+        NodeVersionKey nodeKeyFive = nodeFive.getNodeVersionKey();
+        
+        // Get the node cached values
+        Map<QName, Serializable> nodePropsFive = (Map<QName, Serializable>) findCacheValue(propsCache, nodeKeyFive);
+        Set<QName> nodeAspectsFive = (Set<QName>) findCacheValue(aspectsCache, nodeKeyFive);
+        ParentAssocsInfo nodeParentAssocsFive = (ParentAssocsInfo) findCacheValue(parentAssocsCache, nodeKeyFive);
+
+        // Check the values
+        assertEquals("The node version is incorrect", Long.valueOf(5L), nodeKeyFive.getVersion());
+        assertNotNull("No cache entry for properties", nodePropsFive);
+        assertNotNull("No cache entry for aspects", nodeAspectsFive);
+        assertNotNull("No cache entry for parent assocs", nodeParentAssocsFive);
+        assertTrue("Properties must be carried", nodePropsFive == nodePropsFour);
+        assertTrue("Aspects must have moved on", nodeAspectsFive != nodeAspectsFour);
+        assertFalse("Expected no cm:titled aspect ", nodeAspectsFive.contains(ContentModel.ASPECT_TITLED));
+        assertTrue("Parent assocs must be carried", nodeParentAssocsFive == nodeParentAssocsFour);
+        
+        // Add an aspect, some properties and secondary association
+        RetryingTransactionCallback<Void> nodeSixWork = new RetryingTransactionCallback<Void>()
+        {
+            @Override
+            public Void execute() throws Throwable
+            {
+                Map<QName, Serializable> props = new HashMap<QName, Serializable>();
+                props.put(ContentModel.PROP_TITLE, "some title");
+                nodeService.addAspect(nodeRef, ContentModel.ASPECT_TITLED, props);
+                nodeService.setProperty(nodeRef, ContentModel.PROP_DESCRIPTION, "Some description");
+                nodeService.addChild(
+                        Collections.singletonList(workspaceRootNodeRef),
+                        nodeRef,
+                        ContentModel.ASSOC_CHILDREN,
+                        QName.createQName(TEST_PREFIX, "secondary"));
+                return null;
+            }
+        };
+        txnService.getRetryingTransactionHelper().doInTransaction(nodeSixWork);
+        
+        // Get the values for the previous version
+        Map<QName, Serializable> nodePropsFiveCheck = (Map<QName, Serializable>) findCacheValue(propsCache, nodeKeyFive);
+        Set<QName> nodeAspectsFiveCheck = (Set<QName>) findCacheValue(aspectsCache, nodeKeyFive);
+        ParentAssocsInfo nodeParentAssocsFiveCheck = (ParentAssocsInfo) findCacheValue(parentAssocsCache, nodeKeyFive);
+        assertTrue("Previous cache entries must be left alone", nodePropsFiveCheck == nodePropsFive);
+        assertTrue("Previous cache entries must be left alone", nodeAspectsFiveCheck == nodeAspectsFive);
+        assertTrue("Previous cache entries must be left alone", nodeParentAssocsFiveCheck == nodeParentAssocsFive);
+
+        // Get the current node cache key
+        Node nodeSix = (Node) findCacheValue(nodesCache, nodeId);
+        assertNotNull("Node not found in cache", nodeSix);
+        NodeVersionKey nodeKeySix = nodeSix.getNodeVersionKey();
+        
+        // Get the node cached values
+        Map<QName, Serializable> nodePropsSix = (Map<QName, Serializable>) findCacheValue(propsCache, nodeKeySix);
+        Set<QName> nodeAspectsSix = (Set<QName>) findCacheValue(aspectsCache, nodeKeySix);
+        ParentAssocsInfo nodeParentAssocsSix = (ParentAssocsInfo) findCacheValue(parentAssocsCache, nodeKeySix);
+
+        // Check the values
+        assertEquals("The node version is incorrect", Long.valueOf(6L), nodeKeySix.getVersion());
+        assertNotNull("No cache entry for properties", nodePropsSix);
+        assertNotNull("No cache entry for aspects", nodeAspectsSix);
+        assertNotNull("No cache entry for parent assocs", nodeParentAssocsSix);
+        assertTrue("Properties must have moved on", nodePropsSix != nodePropsFive);
+        assertEquals("Property count incorrect", 3, nodePropsSix.size());
+        assertNotNull("Expected a cm:name property", nodePropsSix.get(ContentModel.PROP_NAME));
+        assertNotNull("Expected a cm:title property", nodePropsSix.get(ContentModel.PROP_TITLE));
+        assertNotNull("Expected a cm:description property", nodePropsSix.get(ContentModel.PROP_DESCRIPTION));
+        assertTrue("Aspects must have moved on", nodeAspectsSix != nodeAspectsFive);
+        assertTrue("Expected cm:titled aspect ", nodeAspectsSix.contains(ContentModel.ASPECT_TITLED));
+        assertTrue("Parent assocs must have moved on", nodeParentAssocsSix != nodeParentAssocsFive);
+        assertEquals("Incorrect number of parent assocs", 2, nodeParentAssocsSix.getParentAssocs().size());
+        
+        // Remove an aspect, some properties and a secondary association
+        RetryingTransactionCallback<Void> nodeSevenWork = new RetryingTransactionCallback<Void>()
+        {
+            @Override
+            public Void execute() throws Throwable
+            {
+                nodeService.removeAspect(nodeRef, ContentModel.ASPECT_TITLED);
+                nodeService.removeChild(workspaceRootNodeRef, nodeRef);
+                return null;
+            }
+        };
+        txnService.getRetryingTransactionHelper().doInTransaction(nodeSevenWork);
+        
+        // Get the values for the previous version
+        Map<QName, Serializable> nodePropsSixCheck = (Map<QName, Serializable>) findCacheValue(propsCache, nodeKeySix);
+        Set<QName> nodeAspectsSixCheck = (Set<QName>) findCacheValue(aspectsCache, nodeKeySix);
+        ParentAssocsInfo nodeParentAssocsSixCheck = (ParentAssocsInfo) findCacheValue(parentAssocsCache, nodeKeySix);
+        assertTrue("Previous cache entries must be left alone", nodePropsSixCheck == nodePropsSix);
+        assertTrue("Previous cache entries must be left alone", nodeAspectsSixCheck == nodeAspectsSix);
+        assertTrue("Previous cache entries must be left alone", nodeParentAssocsSixCheck == nodeParentAssocsSix);
+
+        // Get the current node cache key
+        Node nodeSeven = (Node) findCacheValue(nodesCache, nodeId);
+        assertNotNull("Node not found in cache", nodeSeven);
+        NodeVersionKey nodeKeySeven = nodeSeven.getNodeVersionKey();
+        
+        // Get the node cached values
+        Map<QName, Serializable> nodePropsSeven = (Map<QName, Serializable>) findCacheValue(propsCache, nodeKeySeven);
+        Set<QName> nodeAspectsSeven = (Set<QName>) findCacheValue(aspectsCache, nodeKeySeven);
+        ParentAssocsInfo nodeParentAssocsSeven = (ParentAssocsInfo) findCacheValue(parentAssocsCache, nodeKeySeven);
+
+        // Check the values
+        assertEquals("The node version is incorrect", Long.valueOf(7L), nodeKeySeven.getVersion());
+        assertNotNull("No cache entry for properties", nodePropsSeven);
+        assertNotNull("No cache entry for aspects", nodeAspectsSeven);
+        assertNotNull("No cache entry for parent assocs", nodeParentAssocsSeven);
+        assertTrue("Properties must have moved on", nodePropsSeven != nodePropsSix);
+        assertEquals("Property count incorrect", 1, nodePropsSeven.size());
+        assertNotNull("Expected a cm:name property", nodePropsSeven.get(ContentModel.PROP_NAME));
+        assertTrue("Aspects must have moved on", nodeAspectsSeven != nodeAspectsSix);
+        assertFalse("Expected no cm:titled aspect ", nodeAspectsSeven.contains(ContentModel.ASPECT_TITLED));
+        assertTrue("Parent assocs must have moved on", nodeParentAssocsSeven != nodeParentAssocsSix);
+        assertEquals("Incorrect number of parent assocs", 1, nodeParentAssocsSeven.getParentAssocs().size());
+        
+        // Modify cm:auditable
+        RetryingTransactionCallback<Void> nodeEightWork = new RetryingTransactionCallback<Void>()
+        {
+            @Override
+            public Void execute() throws Throwable
+            {
+                BehaviourFilter behaviourFilter = (BehaviourFilter) ctx.getBean("policyBehaviourFilter");
+                // Disable behaviour for txn
+                behaviourFilter.disableBehaviour(nodeRef, ContentModel.ASPECT_AUDITABLE);
+                nodeService.setProperty(nodeRef, ContentModel.PROP_MODIFIER, "Fred");
+                return null;
+            }
+        };
+        txnService.getRetryingTransactionHelper().doInTransaction(nodeEightWork);
+        
+        // Get the values for the previous version
+        Map<QName, Serializable> nodePropsSevenCheck = (Map<QName, Serializable>) findCacheValue(propsCache, nodeKeySeven);
+        Set<QName> nodeAspectsSevenCheck = (Set<QName>) findCacheValue(aspectsCache, nodeKeySeven);
+        ParentAssocsInfo nodeParentAssocsSevenCheck = (ParentAssocsInfo) findCacheValue(parentAssocsCache, nodeKeySeven);
+        assertTrue("Previous cache entries must be left alone", nodePropsSevenCheck == nodePropsSeven);
+        assertTrue("Previous cache entries must be left alone", nodeAspectsSevenCheck == nodeAspectsSeven);
+        assertTrue("Previous cache entries must be left alone", nodeParentAssocsSevenCheck == nodeParentAssocsSeven);
+
+        // Get the current node cache key
+        Node nodeEight = (Node) findCacheValue(nodesCache, nodeId);
+        assertNotNull("Node not found in cache", nodeEight);
+        NodeVersionKey nodeKeyEight = nodeEight.getNodeVersionKey();
+        
+        // Get the node cached values
+        Map<QName, Serializable> nodePropsEight = (Map<QName, Serializable>) findCacheValue(propsCache, nodeKeyEight);
+        Set<QName> nodeAspectsEight = (Set<QName>) findCacheValue(aspectsCache, nodeKeyEight);
+        ParentAssocsInfo nodeParentAssocsEight = (ParentAssocsInfo) findCacheValue(parentAssocsCache, nodeKeyEight);
+
+        // Check the values
+        assertEquals("The node version is incorrect", Long.valueOf(8L), nodeKeyEight.getVersion());
+        assertNotNull("No cache entry for properties", nodePropsEight);
+        assertNotNull("No cache entry for aspects", nodeAspectsEight);
+        assertNotNull("No cache entry for parent assocs", nodeParentAssocsEight);
+        assertEquals("Expected change to cm:modifier", "Fred", nodeEight.getAuditableProperties().getAuditModifier());
+        assertTrue("Properties must be carried", nodePropsEight == nodePropsSeven);
+        assertTrue("Aspects be carried", nodeAspectsEight == nodeAspectsSeven);
+        assertTrue("Parent assocs must be carried", nodeParentAssocsEight == nodeParentAssocsSeven);
+    }
 }