diff --git a/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java b/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java index d73f9f7353..a7416490da 100644 --- a/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java +++ b/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 null) + * @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) 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) diff --git a/source/java/org/alfresco/repo/node/NodeServiceTest.java b/source/java/org/alfresco/repo/node/NodeServiceTest.java index da67c07a04..441373a55b 100644 --- a/source/java/org/alfresco/repo/node/NodeServiceTest.java +++ b/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 props = new HashMap(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 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 parentAssocsPost = nodeService.getParentAssocs(secondaryNodeRef); + assertEquals("Incorrect number of parent assocs", 1, parentAssocsPost.size()); + } } diff --git a/source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java b/source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java index aa16a5c3c3..3b36c93427 100644 --- a/source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java +++ b/source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java @@ -172,7 +172,8 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl Pair 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; }