From e282c0584e64d59ba56fd3b5598eefbd114bd9bb Mon Sep 17 00:00:00 2001 From: Roy Wetherall Date: Thu, 29 Aug 2019 12:28:04 +1000 Subject: [PATCH] Fix issue with inaccurate results returning from content URL query * added integration test for MNT scenarios * checked for nodeId's that don't exist in query results * changed DAO method to return set of node references rather tha nodeId strings * turned off logging by default --- .../org_alfresco_module_rm/log4j.properties | 3 +- .../query/RecordsManagementQueryDAO.java | 4 +- .../query/RecordsManagementQueryDAOImpl.java | 78 ++++++++++--------- .../util/ContentBinDuplicationUtility.java | 2 +- .../test/util/BaseRMTestCase.java | 1 - .../ContentBinDuplicationUtilityUnitTest.java | 8 +- 6 files changed, 51 insertions(+), 45 deletions(-) diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/log4j.properties b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/log4j.properties index 2f6cafb272..f5ecef752f 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/log4j.properties +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/log4j.properties @@ -57,5 +57,4 @@ log4j.logger.org.alfresco.module.org_alfresco_module_rm.patch=info # #log4j.logger.org.alfresco.module.org_alfresco_module_rm.job=debug log4j.logger.org.alfresco.repo.web.scripts.roles.DynamicAuthoritiesGet=info - -log4j.logger.org.alfresco.module.org_alfresco_module_rm.query.RecordsManagementQueryDAOImpl=debug \ No newline at end of file +log4j.logger.org.alfresco.module.org_alfresco_module_rm.query.RecordsManagementQueryDAOImpl=info \ No newline at end of file diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/query/RecordsManagementQueryDAO.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/query/RecordsManagementQueryDAO.java index 1a5dcf9916..a3c041fc9e 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/query/RecordsManagementQueryDAO.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/query/RecordsManagementQueryDAO.java @@ -64,7 +64,7 @@ public interface RecordsManagementQueryDAO /** * @param contentUrl the URL of the content url entity - * @return Return a set of UUIDs which reference the given node + * @return Set a set of nodes that reference the given content url */ - Set getNodeRefsWhichReferenceContentUrl(String contentUrl); + Set getNodeRefsWhichReferenceContentUrl(String contentUrl); } diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/query/RecordsManagementQueryDAOImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/query/RecordsManagementQueryDAOImpl.java index 69d9f47b29..5503f5b86c 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/query/RecordsManagementQueryDAOImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/query/RecordsManagementQueryDAOImpl.java @@ -34,7 +34,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import org.alfresco.model.ContentModel; import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; import org.alfresco.repo.domain.contentdata.ContentUrlEntity; import org.alfresco.repo.domain.node.NodeDAO; @@ -159,11 +158,11 @@ public class RecordsManagementQueryDAOImpl implements RecordsManagementQueryDAO, /** * Get a set of node reference which reference the provided content URL * - * @param String contentUrl content URL - * @return Set set of node references as strings + * @param String contentUrl content URL + * @return Set set of nodes that reference the provided content URL */ @Override - public Set getNodeRefsWhichReferenceContentUrl(String contentUrl) + public Set getNodeRefsWhichReferenceContentUrl(String contentUrl) { if (logger.isDebugEnabled()) { @@ -192,45 +191,54 @@ public class RecordsManagementQueryDAOImpl implements RecordsManagementQueryDAO, } // create a set of uuids which reference the content url - Set nodesReferencingContentUrl = new HashSet<>(); + Set nodesReferencingContentUrl = new HashSet(nodeIds.size()); for(Long nodeId : nodeIds) { StringBuilder logMessage = null; - String uuidToAdd; + NodeRef nodeRefToAdd; - if (logger.isDebugEnabled()) + if (nodeDAO.exists(nodeId)) { - logMessage = new StringBuilder("Adding uuid "); + if (logger.isDebugEnabled()) + { + logMessage = new StringBuilder("Adding uuid "); + } + + // if the referencing node is a version2Store reference to the content url, add the uuid for the version 2 frozen node ref + NodeRef version2FrozenNodeRef = (NodeRef) nodeDAO.getNodeProperty(nodeId, Version2Model.PROP_QNAME_FROZEN_NODE_REF); + if (version2FrozenNodeRef != null) + { + nodeRefToAdd = version2FrozenNodeRef; + + if (logger.isDebugEnabled()) + { + logMessage.append(nodeRefToAdd).append(" (from version)"); + } + } + + // add the uuid for the node ref of the referencing node + else + { + nodeRefToAdd = nodeDAO.getNodeIdStatus(nodeId).getNodeRef(); + if (logger.isDebugEnabled()) + { + logMessage.append(nodeRefToAdd); + } + } + + nodesReferencingContentUrl.add(nodeRefToAdd); + + if (logger.isDebugEnabled()) + { + logger.debug(logMessage.toString()); + } } - - // if the referencing node is a version2Store reference to the content url, add the uuid for the version 2 frozen node ref - NodeRef version2FrozenNodeRef = (NodeRef) nodeDAO.getNodeProperty(nodeId, Version2Model.PROP_QNAME_FROZEN_NODE_REF); - if (version2FrozenNodeRef != null) - { - uuidToAdd = version2FrozenNodeRef.getId(); - - if (logger.isDebugEnabled()) - { - logMessage.append(uuidToAdd).append(" (from version)"); - } - } - - // add the uuid for the node ref of the referencing node else { - uuidToAdd = (String) nodeDAO.getNodeProperty(nodeId, ContentModel.PROP_NODE_UUID); - - if (logger.isDebugEnabled()) - { - logMessage.append(uuidToAdd); - } - } - - nodesReferencingContentUrl.add(uuidToAdd); - - if (logger.isDebugEnabled()) - { - logger.debug(logMessage.toString()); + if (logger.isDebugEnabled()) + { + logger.debug("Not adding " + nodeId + " (exist==false)"); + } } } diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ContentBinDuplicationUtility.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ContentBinDuplicationUtility.java index c1453c906a..68a0bab6c3 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ContentBinDuplicationUtility.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ContentBinDuplicationUtility.java @@ -96,7 +96,7 @@ public class ContentBinDuplicationUtility extends ServiceBaseImpl boolean hasAtLeastOneOtherReference = false; String contentUrl = contentService.getReader(nodeRef, ContentModel.PROP_CONTENT).getContentUrl(); - Set referencesToContentNode = recordsManagementQueryDAO.getNodeRefsWhichReferenceContentUrl(contentUrl); + Set referencesToContentNode = recordsManagementQueryDAO.getNodeRefsWhichReferenceContentUrl(contentUrl); if (referencesToContentNode.size() > 1) { hasAtLeastOneOtherReference = true; diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/BaseRMTestCase.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/BaseRMTestCase.java index 4eeb564b49..06dc185a44 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/BaseRMTestCase.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/BaseRMTestCase.java @@ -54,7 +54,6 @@ import org.alfresco.module.org_alfresco_module_rm.recordfolder.RecordFolderServi import org.alfresco.module.org_alfresco_module_rm.relationship.RelationshipService; import org.alfresco.module.org_alfresco_module_rm.report.ReportService; import org.alfresco.module.org_alfresco_module_rm.role.FilePlanRoleService; -import org.alfresco.module.org_alfresco_module_rm.role.Role; import org.alfresco.module.org_alfresco_module_rm.search.RecordsManagementSearchService; import org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService; import org.alfresco.module.org_alfresco_module_rm.security.FilePlanPermissionService; diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/ContentBinDuplicationUtilityUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/ContentBinDuplicationUtilityUnitTest.java index d2c12a1005..b8f2727d4e 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/ContentBinDuplicationUtilityUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/ContentBinDuplicationUtilityUnitTest.java @@ -116,8 +116,8 @@ public class ContentBinDuplicationUtilityUnitTest @Test public void testHasAtLeastOneOtherReference() { - Set multipleReferences = new HashSet<>(); - Collections.addAll(multipleReferences, NODE_REF.getId(), NODE_REF2.getId()); + Set multipleReferences = new HashSet<>(); + Collections.addAll(multipleReferences, NODE_REF, NODE_REF2); when(contentService.getReader(NODE_REF, ContentModel.PROP_CONTENT)).thenReturn(contentReader); when(contentService.getReader(NODE_REF, ContentModel.PROP_CONTENT).getContentUrl()).thenReturn(CONTENT_URL); @@ -132,7 +132,7 @@ public class ContentBinDuplicationUtilityUnitTest @Test public void testHasNoOtherReference() { - Set singleReference = Collections.singleton(NODE_REF.getId()); + Set singleReference = Collections.singleton(NODE_REF); when(contentService.getReader(NODE_REF, ContentModel.PROP_CONTENT)).thenReturn(contentReader); when(contentService.getReader(NODE_REF, ContentModel.PROP_CONTENT).getContentUrl()).thenReturn(CONTENT_URL); @@ -147,7 +147,7 @@ public class ContentBinDuplicationUtilityUnitTest @Test public void testHasNoReferences() { - Set noReferences = Collections. emptySet(); + Set noReferences = Collections. emptySet(); when(contentService.getReader(NODE_REF, ContentModel.PROP_CONTENT)).thenReturn(contentReader); when(contentService.getReader(NODE_REF, ContentModel.PROP_CONTENT).getContentUrl()).thenReturn(CONTENT_URL);