diff --git a/config/alfresco/cache-context.xml b/config/alfresco/cache-context.xml index c1bcea9a32..41c9341ae1 100644 --- a/config/alfresco/cache-context.xml +++ b/config/alfresco/cache-context.xml @@ -309,6 +309,39 @@ + + + + + + + + + + + + + + org.alfresco.cache.node.childByNameCache + + + + + + + + + + + + + org.alfresco.cache.node.childByNameTransactionalCache + + + + + + diff --git a/config/alfresco/dao/dao-context.xml b/config/alfresco/dao/dao-context.xml index 1c0edc83f9..87c53182ca 100644 --- a/config/alfresco/dao/dao-context.xml +++ b/config/alfresco/dao/dao-context.xml @@ -120,6 +120,7 @@ + diff --git a/config/alfresco/ehcache-default.xml b/config/alfresco/ehcache-default.xml index 9890c62d23..183444854a 100644 --- a/config/alfresco/ehcache-default.xml +++ b/config/alfresco/ehcache-default.xml @@ -35,6 +35,9 @@ overflowToDisk="false" statistics="false" /> + + + + diff --git a/config/alfresco/extension/ehcache-custom.xml.sample.cluster b/config/alfresco/extension/ehcache-custom.xml.sample.cluster index d2cde19763..3964c019eb 100644 --- a/config/alfresco/extension/ehcache-custom.xml.sample.cluster +++ b/config/alfresco/extension/ehcache-custom.xml.sample.cluster @@ -197,6 +197,16 @@ replicateAsynchronously = false"/> + + + + parentAssocsCache; + /** + * Cache for fast lookups of child nodes by cm:name. + */ + private SimpleCache childByNameCache; + /** * Constructor. Set up various instance-specific members such as caches and locks. */ @@ -187,6 +193,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO aspectsCache = new EntityLookupCache, Serializable>(new AspectsCallbackDAO()); propertiesCache = new EntityLookupCache, Serializable>(new PropertiesCallbackDAO()); parentAssocsCache = new EntityLookupCache(new ParentAssocsCallbackDAO()); + childByNameCache = new NullCache(); } /** @@ -345,7 +352,17 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO CACHE_REGION_PARENT_ASSOCS, new ParentAssocsCallbackDAO()); } - + + /** + * Set the cache that maintains lookups by child cm:name + * + * @param childByNameCache the cache + */ + public void setChildByNameCache(SimpleCache childByNameCache) + { + this.childByNameCache = childByNameCache; + } + /* * Initialize */ @@ -2018,6 +2035,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO // Touch to bring into current transaction if (updated) { + // We have to explicitly update the node (sys:locale or cm:auditable) updateNodeImpl(node, nodeUpdate); } @@ -3023,9 +3041,59 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO } } + /** + * Checks a cache and then queries. + *

+ * Note: If we were to cach misses, then we would have to ensure that the cache is + * kept up to date whenever any affection association is changed. This is actually + * not possible without forcing the cache to be fully clustered. So to + * avoid clustering the cache, we instead watch the node child version, + * which relies on a cache that is already clustered. + */ public Pair getChildAssoc(Long parentNodeId, QName assocTypeQName, String childName) { - ChildAssocEntity assoc = selectChildAssoc(parentNodeId, assocTypeQName, childName); + ChildByNameKey key = new ChildByNameKey(parentNodeId, assocTypeQName, childName); + ChildAssocEntity assoc = childByNameCache.get(key); + boolean query = false; + if (assoc == null) + { + query = true; + } + else + { + // Check that the resultant child node has not moved on + Node childNode = assoc.getChildNode(); + Long childNodeId = childNode.getId(); + NodeVersionKey childNodeVersionKey = childNode.getNodeVersionKey(); + Pair childNodeFromCache = nodesCache.getByKey(childNodeId); + if (childNodeFromCache == null) + { + // Child node no longer exists (or never did) + query = true; + } + else + { + NodeVersionKey childNodeFromCacheVersionKey = childNodeFromCache.getSecond().getNodeVersionKey(); + if (!childNodeFromCacheVersionKey.equals(childNodeVersionKey)) + { + // The child node has moved on. We don't know why, but must query again. + query = true; + } + } + } + if (query) + { + assoc = selectChildAssoc(parentNodeId, assocTypeQName, childName); + if (assoc != null) + { + childByNameCache.put(key, assoc); + } + else + { + // We do not cache misses. See javadoc. + } + } + // Now return, checking the assoc's ID for null return assoc == null ? null : assoc.getPair(qnameDAO); } diff --git a/source/java/org/alfresco/repo/node/NodeServiceTest.java b/source/java/org/alfresco/repo/node/NodeServiceTest.java index bc2039112b..75aa615128 100644 --- a/source/java/org/alfresco/repo/node/NodeServiceTest.java +++ b/source/java/org/alfresco/repo/node/NodeServiceTest.java @@ -500,4 +500,39 @@ public class NodeServiceTest extends TestCase List parentAssocsPost = nodeService.getParentAssocs(secondaryNodeRef); assertEquals("Incorrect number of parent assocs", 1, parentAssocsPost.size()); } + + /** + * Checks that file renames are handled when getting children + */ + public void testCaches_RenameNode() + { + final NodeRef[] nodeRefs = new NodeRef[2]; + final NodeRef workspaceRootNodeRef = nodeService.getRootNode(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE); + buildNodeHierarchy(workspaceRootNodeRef, nodeRefs); + + // What is the name of the first child? + String name = (String) nodeService.getProperty(nodeRefs[1], ContentModel.PROP_NAME); + // Now query for it + NodeRef nodeRefCheck = nodeService.getChildByName(nodeRefs[0], ContentModel.ASSOC_CONTAINS, name); + assertNotNull("Did not find node by name", nodeRefCheck); + assertEquals("Node found was not correct", nodeRefs[1], nodeRefCheck); + + // Rename the node + nodeService.setProperty(nodeRefs[1], ContentModel.PROP_NAME, "New Name"); + // Should find nothing + nodeRefCheck = nodeService.getChildByName(nodeRefs[0], ContentModel.ASSOC_CONTAINS, name); + assertNull("Should not have found anything", nodeRefCheck); + + // Add another child with the same original name + NodeRef newChildNodeRef = nodeService.createNode( + nodeRefs[0], + ContentModel.ASSOC_CONTAINS, + QName.createQName(NAMESPACE, name), + ContentModel.TYPE_FOLDER, + Collections.singletonMap(ContentModel.PROP_NAME, (Serializable)name)).getChildRef(); + // We should find this new node when looking for the name + nodeRefCheck = nodeService.getChildByName(nodeRefs[0], ContentModel.ASSOC_CONTAINS, name); + assertNotNull("Did not find node by name", nodeRefCheck); + assertEquals("Node found was not correct", newChildNodeRef, nodeRefCheck); + } }