Fix for rev 31223: Missing child node cache invalidation call (ALF-10699)

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

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

@@ -85,6 +85,7 @@ import org.alfresco.util.PropertyCheck;
 import org.alfresco.util.ReadWriteLockExecuter;
 import org.alfresco.util.SerializationUtils;
 import org.alfresco.util.EqualsHelper.MapValueComparison;
+import org.apache.commons.lang.mutable.MutableInt;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.dao.ConcurrencyFailureException;
@@ -447,13 +448,14 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
      * where the child associations or nodes are modified en-masse.
      * 
      * @param parentNodeId          the parent node of all child nodes to be invalidated (may be <tt>null</tt>)
+     * @return                      the number of child associations found (might be capped)
      */
-    private void invalidateNodeChildrenCaches(Long parentNodeId)
+    private int invalidateNodeChildrenCaches(Long parentNodeId)
     {
         // Select all children
+        final MutableInt count = new MutableInt(0);
         ChildAssocRefQueryCallback callback = new ChildAssocRefQueryCallback()
         {
-            private int count = 0;
             private boolean isClearOn = false;
             
             public boolean preLoadNodes()
@@ -471,7 +473,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
                     // We have already decided to drop ALL cache entries
                     return false;
                 }
-                else if (count >= 1000)
+                else if (count.intValue() >= 1000)
                 {
                     // That's enough.  Instead of walking thousands of entries
                     // we just drop the cache at this stage
@@ -479,7 +481,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
                     isClearOn = true;
                     return false;               // No more, please
                 }
-                count++;
+                count.increment();
                 invalidateNodeCaches(childNodePair.getFirst());
                 return true;
             }
@@ -489,6 +491,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
             }                               
         };
         selectChildAssocs(parentNodeId, null, null, null, null, null, callback);
+        return count.intValue();
     }
 
     /**
@@ -1676,16 +1679,19 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
             allRootNodesCache.remove(node.getNodePair().getSecond().getStoreRef());
         }
         
+        // Remove peer associations (no associated cache)
+        deleteNodeAssocsToAndFrom(nodeId);
+        
+        // Remove child associations (invalidate children)
+        invalidateNodeChildrenCaches(nodeId);
+        deleteChildAssocsToAndFrom(nodeId);
+        
         // Remove aspects
         deleteNodeAspects(nodeId, null);
         
         // Remove properties
         deleteNodeProperties(nodeId, (Set<Long>) null);
         
-        // Remove associations
-        deleteNodeAssocsToAndFrom(nodeId);
-        deleteChildAssocsToAndFrom(nodeId);
-        
         // Remove subscriptions
         deleteSubscriptions(nodeId);
 
@@ -2742,7 +2748,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
         // Touch the node; all caches are fine
         touchNode(childNodeId, null, false, false, false);
         // update cache
-        parentAssocInfo = parentAssocInfo.addAssoc(assocId, assoc, getCurrentTransactionId());
+        parentAssocInfo = parentAssocInfo.addAssoc(assocId, assoc);
         setParentAssocsCached(childNodeId, parentAssocInfo);
         // Done
         return assoc.getPair(qnameDAO);
@@ -2767,7 +2773,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO
         // Touch the node; all caches are fine
         touchNode(childNodeId, null, false, false, false);
         // Update cache
-        parentAssocInfo = parentAssocInfo.removeAssoc(assocId, getCurrentTransactionId());
+        parentAssocInfo = parentAssocInfo.removeAssoc(assocId);
         setParentAssocsCached(childNodeId, parentAssocInfo);
     }
 
@@ -2810,7 +2816,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, false);
+            touchNode(childNodeId, null, false, false, true);
         }
         
         if (isDebugEnabled)

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

@@ -21,6 +21,7 @@ package org.alfresco.repo.node;
 import java.io.Serializable;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 
@@ -31,6 +32,7 @@ import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback;
 import org.alfresco.service.ServiceRegistry;
 import org.alfresco.service.cmr.repository.AssociationRef;
+import org.alfresco.service.cmr.repository.ChildAssociationRef;
 import org.alfresco.service.cmr.repository.DuplicateChildNodeNameException;
 import org.alfresco.service.cmr.repository.MLText;
 import org.alfresco.service.cmr.repository.NodeRef;
@@ -423,4 +425,41 @@ public class NodeServiceTest extends TestCase
             // Expected
         }
     }
+    
+    /**
+     * Checks that the node caches react correct when a node is deleted
+     */
+    public void testCaches_DeleteNode()
+    {
+        final NodeRef[] liveNodeRefs = new NodeRef[10];
+        final NodeRef workspaceRootNodeRef = nodeService.getRootNode(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE);
+
+        buildNodeHierarchy(workspaceRootNodeRef, liveNodeRefs);
+        nodeService.addAspect(liveNodeRefs[3], ContentModel.ASPECT_TEMPORARY, null);
+        
+        // Create a child under node 2
+        Map<QName, Serializable> props = new HashMap<QName, Serializable>(3);
+        props.put(ContentModel.PROP_NAME, "Secondary");
+        NodeRef secondaryNodeRef = nodeService.createNode(
+                liveNodeRefs[2],
+                ContentModel.ASSOC_CONTAINS,
+                QName.createQName(NAMESPACE, "secondary"),
+                ContentModel.TYPE_FOLDER,
+                props).getChildRef();
+        // Make it a child of node 3
+        nodeService.addChild(liveNodeRefs[3], secondaryNodeRef, ContentModel.ASSOC_CONTAINS, QName.createQName(NAMESPACE, "secondary"));
+        // Make it a child of node 4
+        nodeService.addChild(liveNodeRefs[4], secondaryNodeRef, ContentModel.ASSOC_CONTAINS, QName.createQName(NAMESPACE, "secondary"));
+        
+        // Check
+        List<ChildAssociationRef> parentAssocsPre = nodeService.getParentAssocs(secondaryNodeRef);
+        assertEquals("Incorrect number of parent assocs", 3, parentAssocsPre.size());
+        
+        // Delete node 3 (should affect 2 of the parent associations);
+        nodeService.deleteNode(liveNodeRefs[3]);
+        
+        // Check
+        List<ChildAssociationRef> parentAssocsPost = nodeService.getParentAssocs(secondaryNodeRef);
+        assertEquals("Incorrect number of parent assocs", 1, parentAssocsPost.size());
+    }
 }

source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java

@@ -172,7 +172,8 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl
         Pair<Long, NodeRef> unchecked = nodeDAO.getNodePair(nodeRef);
         if (unchecked == null)
         {
-            throw new InvalidNodeRefException("Node does not exist: " + nodeRef, nodeRef);
+            Status nodeStatus = nodeDAO.getNodeRefStatus(nodeRef);
+            throw new InvalidNodeRefException("Node does not exist: " + nodeRef + "(" + nodeStatus + ")", nodeRef);
         }
         return unchecked;
     }