From 075794981b499985f2a1d6ab0a28258bed2734e0 Mon Sep 17 00:00:00 2001 From: Ross Gale Date: Wed, 10 Jul 2019 10:53:27 +0100 Subject: [PATCH 1/9] MNT-20740 initial commit for delete protection for copied records --- .../model/rma/aspect/RecordAspect.java | 22 +++++++++++++++- .../util/ContentBinDuplicationUtility.java | 25 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspect.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspect.java index cf5e19cf26..03d81ec314 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspect.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspect.java @@ -77,7 +77,8 @@ public class RecordAspect extends AbstractDisposableItem RecordsManagementPolicies.OnRemoveReference, NodeServicePolicies.OnMoveNodePolicy, CopyServicePolicies.OnCopyCompletePolicy, - ContentServicePolicies.OnContentPropertyUpdatePolicy + ContentServicePolicies.OnContentPropertyUpdatePolicy, + NodeServicePolicies.BeforeDeleteNodePolicy { /** Well-known location of the scripts folder. */ // TODO make configurable @@ -421,4 +422,23 @@ public class RecordAspect extends AbstractDisposableItem } }, AuthenticationUtil.getSystemUserName()); } + + /** + * Behaviour to protect access to copied records binary files created prior to 2.7.2 + * @param nodeRef Node with the binary in need of protection + */ + @Override + @Behaviour + ( + kind = BehaviourKind.CLASS, + notificationFrequency = NotificationFrequency.FIRST_EVENT + ) + public void beforeDeleteNode(NodeRef nodeRef) + { + if (!nodeService.getTargetAssocs(nodeRef, ContentModel.ASSOC_ORIGINAL).isEmpty() || + !nodeService.getSourceAssocs(nodeRef, ContentModel.ASSOC_ORIGINAL).isEmpty()) + { + contentBinDuplicationUtility.orphanContent(nodeRef); + } + } } 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 c1d7e879f0..1e06b2c017 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 @@ -26,12 +26,16 @@ */ package org.alfresco.module.org_alfresco_module_rm.util; +import java.util.Set; + import org.alfresco.model.ContentModel; import org.alfresco.repo.policy.BehaviourFilter; +import org.alfresco.repo.transaction.TransactionalResourceHelper; import org.alfresco.service.cmr.repository.ContentReader; import org.alfresco.service.cmr.repository.ContentService; import org.alfresco.service.cmr.repository.ContentWriter; import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.cmr.repository.NodeService; /** * Utility class to duplicate the content of a node without triggering the audit or versioning behaviours @@ -51,6 +55,8 @@ public class ContentBinDuplicationUtility */ private ContentService contentService; + private NodeService nodeService; + /** * Setter for behaviour filter * @param behaviourFilter BehaviourFilter @@ -104,4 +110,23 @@ public class ContentBinDuplicationUtility writer.putContent(reader); } } + + /** + * Purposely orphans a binary file. + * + * This will prevent the removal of a binary file to address MNT-20740 where a binary + * file may be shared across nodes that try to delete the binary on deletion. + * + * Nodes created before 2.7.2 won't have been picked up by the binary duplication code + * and will cause problems if the file is removed. Orphaning the binary will mean if + * there is a node sharing this it will still be available and if there isn't the orphaned + * file will be picked up by the content cleaner on it's next pass. + * + * @param nodeRef The node who's binary file we want to orphan + */ + public void orphanContent(NodeRef nodeRef) + { + Set urlsToDelete = TransactionalResourceHelper.getSet("ContentStoreCleaner.PostCommitDeletionUrls"); + urlsToDelete.remove(contentService.getReader(nodeRef, ContentModel.PROP_CONTENT).getContentUrl()); + } } From 7c4d8e91abdacc65755d0e3a73dfa15ceaacf62c Mon Sep 17 00:00:00 2001 From: Sara Aspery Date: Tue, 30 Jul 2019 16:35:48 +0100 Subject: [PATCH 2/9] MNT-20740 BinDuplicationHotFixSqlRefactor --- .../content-context.xml | 1 + .../org_alfresco_module_rm/module-context.xml | 1 + .../query/rm-common-SqlMapConfig.xml | 16 ++++++ .../content/ContentDestructionComponent.java | 34 ++++++++--- .../model/rma/aspect/RecordAspect.java | 22 +------- .../query/RecordsManagementQueryDAO.java | 8 +++ .../query/RecordsManagementQueryDAOImpl.java | 16 ++++++ .../util/ContentBinDuplicationUtility.java | 56 +++++++++++-------- .../ContentBinDuplicationUtilityUnitTest.java | 42 ++++++++++++++ 9 files changed, 142 insertions(+), 54 deletions(-) diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/content-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/content-context.xml index d7d9999f20..1dd621974b 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/content-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/content-context.xml @@ -18,6 +18,7 @@ + diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/module-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/module-context.xml index 84b6bb8ba6..4c64e5021a 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/module-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/module-context.xml @@ -251,6 +251,7 @@ + diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/query/rm-common-SqlMapConfig.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/query/rm-common-SqlMapConfig.xml index 8144e44bf6..e1dbaf1bea 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/query/rm-common-SqlMapConfig.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/query/rm-common-SqlMapConfig.xml @@ -2,12 +2,28 @@ + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/content/ContentDestructionComponent.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/content/ContentDestructionComponent.java index fb77f3efb6..b86b1b0c2a 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/content/ContentDestructionComponent.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/content/ContentDestructionComponent.java @@ -33,6 +33,7 @@ import java.util.Set; import org.alfresco.model.ContentModel; import org.alfresco.model.RenditionModel; +import org.alfresco.module.org_alfresco_module_rm.util.ContentBinDuplicationUtility; import org.alfresco.repo.policy.BehaviourFilter; import org.alfresco.repo.policy.annotation.BehaviourBean; import org.alfresco.service.cmr.dictionary.DictionaryService; @@ -63,6 +64,9 @@ public class ContentDestructionComponent /** behaviour filter */ private BehaviourFilter behaviourFilter; + /** Utility class for duplicating content */ + private ContentBinDuplicationUtility contentBinDuplicationUtility; + /** indicates whether cleansing is enabled or not */ private boolean cleansingEnabled = false; @@ -138,6 +142,15 @@ public class ContentDestructionComponent this.behaviourFilter = behaviourFilter; } + /** + * Setter for content duplication utility class + * @param contentBinDuplicationUtility ContentBinDuplicationUtility + */ + public void setContentBinDuplicationUtility(ContentBinDuplicationUtility contentBinDuplicationUtility) + { + this.contentBinDuplicationUtility = contentBinDuplicationUtility; + } + /** * @param cleansingEnabled true if cleansing enabled, false otherwise */ @@ -214,16 +227,19 @@ public class ContentDestructionComponent // get content data ContentData dataContent = (ContentData)entry.getValue(); - // if enabled cleanse content - if (isCleansingEnabled()) + if (!contentBinDuplicationUtility.hasAtLeastOneOtherReference(nodeRef)) { - // register for cleanse then immediate destruction - getEagerContentStoreCleaner().registerOrphanedContentUrlForCleansing(dataContent.getContentUrl()); - } - else - { - // register for immediate destruction - getEagerContentStoreCleaner().registerOrphanedContentUrl(dataContent.getContentUrl(), true); + // if enabled cleanse content + if (isCleansingEnabled()) + { + // register for cleanse then immediate destruction + getEagerContentStoreCleaner().registerOrphanedContentUrlForCleansing(dataContent.getContentUrl()); + } + else + { + // register for immediate destruction + getEagerContentStoreCleaner().registerOrphanedContentUrl(dataContent.getContentUrl(), true); + } } // clear the property diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspect.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspect.java index 03d81ec314..cf5e19cf26 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspect.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspect.java @@ -77,8 +77,7 @@ public class RecordAspect extends AbstractDisposableItem RecordsManagementPolicies.OnRemoveReference, NodeServicePolicies.OnMoveNodePolicy, CopyServicePolicies.OnCopyCompletePolicy, - ContentServicePolicies.OnContentPropertyUpdatePolicy, - NodeServicePolicies.BeforeDeleteNodePolicy + ContentServicePolicies.OnContentPropertyUpdatePolicy { /** Well-known location of the scripts folder. */ // TODO make configurable @@ -422,23 +421,4 @@ public class RecordAspect extends AbstractDisposableItem } }, AuthenticationUtil.getSystemUserName()); } - - /** - * Behaviour to protect access to copied records binary files created prior to 2.7.2 - * @param nodeRef Node with the binary in need of protection - */ - @Override - @Behaviour - ( - kind = BehaviourKind.CLASS, - notificationFrequency = NotificationFrequency.FIRST_EVENT - ) - public void beforeDeleteNode(NodeRef nodeRef) - { - if (!nodeService.getTargetAssocs(nodeRef, ContentModel.ASSOC_ORIGINAL).isEmpty() || - !nodeService.getSourceAssocs(nodeRef, ContentModel.ASSOC_ORIGINAL).isEmpty()) - { - contentBinDuplicationUtility.orphanContent(nodeRef); - } - } } 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 88c29d63cd..43c91c32e0 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 @@ -29,6 +29,7 @@ package org.alfresco.module.org_alfresco_module_rm.query; import java.util.Set; +import org.alfresco.repo.domain.contentdata.ContentUrlEntity; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.namespace.QName; @@ -61,4 +62,11 @@ public interface RecordsManagementQueryDAO * @return list of distinct property values */ public Set getChildrenStringPropertyValues(NodeRef parent, QName property); + + /** + * @param contentUrl the URL of the content url entity + * @return Return the entity or null if it doesn't exist or is still + * referenced by a content_data entity + */ + public ContentUrlEntity getContentUrlEntityUnreferenced(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 207f3f69d5..5f8ed0d999 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,6 +34,7 @@ import java.util.Map; import java.util.Set; import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; +import org.alfresco.repo.domain.contentdata.ContentUrlEntity; import org.alfresco.repo.domain.node.NodeDAO; import org.alfresco.repo.domain.qname.QNameDAO; import org.alfresco.repo.tenant.TenantService; @@ -53,6 +54,7 @@ public class RecordsManagementQueryDAOImpl implements RecordsManagementQueryDAO, { private static final String COUNT_IDENTIFIER = "alfresco.query.rm.select_CountRMIndentifier"; private static final String GET_CHILDREN_PROPERTY_VALUES = "select_GetStringPropertyValuesOfChildren"; + private static final String SELECT_CONTENT_URL_BY_KEY_UNREFERENCED = "alfresco.content.select_ContentUrlByKeyUnreferenced"; /** SQL session template */ protected SqlSessionTemplate template; @@ -143,4 +145,18 @@ public class RecordsManagementQueryDAOImpl implements RecordsManagementQueryDAO, return new HashSet<>(template.selectList(GET_CHILDREN_PROPERTY_VALUES, queryParams)); } + + @Override + public ContentUrlEntity getContentUrlEntityUnreferenced(String contentUrl) + { + ContentUrlEntity contentUrlEntity = new ContentUrlEntity(); + contentUrlEntity.setContentUrl(contentUrl); + if (contentUrlEntity.getContentUrlShort() != null) + { + contentUrlEntity.setContentUrlShort(contentUrlEntity.getContentUrlShort().toLowerCase()); + } + contentUrlEntity = (ContentUrlEntity) template.selectOne(SELECT_CONTENT_URL_BY_KEY_UNREFERENCED, contentUrlEntity); + + return contentUrlEntity; + } } 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 1e06b2c017..da5992a0c0 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 @@ -26,16 +26,14 @@ */ package org.alfresco.module.org_alfresco_module_rm.util; -import java.util.Set; - import org.alfresco.model.ContentModel; +import org.alfresco.module.org_alfresco_module_rm.query.RecordsManagementQueryDAO; +import org.alfresco.repo.domain.contentdata.ContentUrlEntity; import org.alfresco.repo.policy.BehaviourFilter; -import org.alfresco.repo.transaction.TransactionalResourceHelper; import org.alfresco.service.cmr.repository.ContentReader; import org.alfresco.service.cmr.repository.ContentService; import org.alfresco.service.cmr.repository.ContentWriter; import org.alfresco.service.cmr.repository.NodeRef; -import org.alfresco.service.cmr.repository.NodeService; /** * Utility class to duplicate the content of a node without triggering the audit or versioning behaviours @@ -55,7 +53,8 @@ public class ContentBinDuplicationUtility */ private ContentService contentService; - private NodeService nodeService; + /** Records Management Query DAO */ + private RecordsManagementQueryDAO recordsManagementQueryDAO; /** * Setter for behaviour filter @@ -75,6 +74,34 @@ public class ContentBinDuplicationUtility this.contentService = contentService; } + /** + * Setter for the Records Management QueryDAO + * + * @param recordsManagementQueryDAO The RM query DAO to set + */ + public void setRecordsManagementQueryDAO(RecordsManagementQueryDAO recordsManagementQueryDAO) + { + this.recordsManagementQueryDAO = recordsManagementQueryDAO; + } + + /** + * Determines whether the bin file for a given node has at least one other reference to it + * Will return true if the binary exists and is referenced by at least one other node + * @param nodeRef Node with the binary in question + * @return boolean for if the bin has at least one other reference to it + */ + public boolean hasAtLeastOneOtherReference(NodeRef nodeRef) + { + boolean hasAtLeastOneOtherReference = false; + String contentUrl = contentService.getReader(nodeRef, ContentModel.PROP_CONTENT).getContentUrl(); + ContentUrlEntity contentUrlEntity = recordsManagementQueryDAO.getContentUrlEntityUnreferenced(contentUrl); + if (contentUrlEntity == null) + { + hasAtLeastOneOtherReference = true; + } + return hasAtLeastOneOtherReference; + } + /** * Duplicate the content of a node without triggering the audit or versioning behaviours * @@ -110,23 +137,4 @@ public class ContentBinDuplicationUtility writer.putContent(reader); } } - - /** - * Purposely orphans a binary file. - * - * This will prevent the removal of a binary file to address MNT-20740 where a binary - * file may be shared across nodes that try to delete the binary on deletion. - * - * Nodes created before 2.7.2 won't have been picked up by the binary duplication code - * and will cause problems if the file is removed. Orphaning the binary will mean if - * there is a node sharing this it will still be available and if there isn't the orphaned - * file will be picked up by the content cleaner on it's next pass. - * - * @param nodeRef The node who's binary file we want to orphan - */ - public void orphanContent(NodeRef nodeRef) - { - Set urlsToDelete = TransactionalResourceHelper.getSet("ContentStoreCleaner.PostCommitDeletionUrls"); - urlsToDelete.remove(contentService.getReader(nodeRef, ContentModel.PROP_CONTENT).getContentUrl()); - } } 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 c37437433a..b8267013fb 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 @@ -28,6 +28,8 @@ package org.alfresco.module.org_alfresco_module_rm.util; import org.alfresco.model.ContentModel; +import org.alfresco.module.org_alfresco_module_rm.query.RecordsManagementQueryDAO; +import org.alfresco.repo.domain.contentdata.ContentUrlEntity; import org.alfresco.repo.policy.BehaviourFilter; import org.alfresco.service.cmr.repository.ContentReader; import org.alfresco.service.cmr.repository.ContentService; @@ -39,6 +41,8 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -63,6 +67,9 @@ public class ContentBinDuplicationUtilityUnitTest @Mock private ContentWriter contentWriter; + @Mock + private RecordsManagementQueryDAO recordsManagementQueryDAO; + @InjectMocks private ContentBinDuplicationUtility contentBinDuplicationUtility; @@ -99,6 +106,41 @@ public class ContentBinDuplicationUtilityUnitTest checkBehaviours(1); } + /** + * Test hasAtLeastOneOtherReference returns true when node has another reference to it + */ + @Test + public void testHasAtLeastOneOtherReference() + { + NodeRef nodeRef = new NodeRef("some://test/noderef"); + String contentUrl = "someContentUrl"; + + when(contentService.getReader(nodeRef, ContentModel.PROP_CONTENT)).thenReturn(contentReader); + when(contentService.getReader(nodeRef, ContentModel.PROP_CONTENT).getContentUrl()).thenReturn(contentUrl); + when(recordsManagementQueryDAO.getContentUrlEntityUnreferenced(contentUrl)).thenReturn(null); + + boolean hasReference = contentBinDuplicationUtility.hasAtLeastOneOtherReference(nodeRef); + assertTrue(hasReference); + } + + /** + * Test hasAtLeastOneOtherReference returns false when node has no other reference to it + */ + @Test + public void testHasNoOtherReference() + { + NodeRef nodeRef = new NodeRef("some://test/noderef"); + String contentUrl = "someContentUrl"; + ContentUrlEntity contentUrlEntity = new ContentUrlEntity(); + + when(contentService.getReader(nodeRef, ContentModel.PROP_CONTENT)).thenReturn(contentReader); + when(contentService.getReader(nodeRef, ContentModel.PROP_CONTENT).getContentUrl()).thenReturn(contentUrl); + when(recordsManagementQueryDAO.getContentUrlEntityUnreferenced(contentUrl)).thenReturn(contentUrlEntity); + + boolean hasReference = contentBinDuplicationUtility.hasAtLeastOneOtherReference(nodeRef); + assertFalse(hasReference); + } + /** * Check that the behaviours are disabled and re-enabled the correct number of times * @param times the times the behaviours should be called From 0f0b0962b828a6a8f3bdc3d1246a883a9e6ebf40 Mon Sep 17 00:00:00 2001 From: Sara Aspery Date: Mon, 5 Aug 2019 10:20:38 +0100 Subject: [PATCH 3/9] MNT-20740 Updates from review --- .../util/ContentBinDuplicationUtilityUnitTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 b8267013fb..2c5299ff4b 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 @@ -137,8 +137,7 @@ public class ContentBinDuplicationUtilityUnitTest when(contentService.getReader(nodeRef, ContentModel.PROP_CONTENT).getContentUrl()).thenReturn(contentUrl); when(recordsManagementQueryDAO.getContentUrlEntityUnreferenced(contentUrl)).thenReturn(contentUrlEntity); - boolean hasReference = contentBinDuplicationUtility.hasAtLeastOneOtherReference(nodeRef); - assertFalse(hasReference); + assertFalse(contentBinDuplicationUtility.hasAtLeastOneOtherReference(nodeRef)); } /** From 18bd7d7fac845fdeaf1bd8e36404d1ac6a17d90e Mon Sep 17 00:00:00 2001 From: Sara Aspery Date: Thu, 22 Aug 2019 11:19:34 +0100 Subject: [PATCH 4/9] MNT-20740 New db query --- .../query/rm-common-SqlMap.xml | 19 ++++++ .../query/rm-common-SqlMapConfig.xml | 11 ---- .../query/RecordsManagementQueryDAO.java | 8 +-- .../query/RecordsManagementQueryDAOImpl.java | 39 ++++++++++-- .../util/ContentBinDuplicationUtility.java | 8 ++- .../ContentBinDuplicationUtilityUnitTest.java | 62 ++++++++++++------- 6 files changed, 100 insertions(+), 47 deletions(-) diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/query/rm-common-SqlMap.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/query/rm-common-SqlMap.xml index 766c80c519..457b70432e 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/query/rm-common-SqlMap.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/query/rm-common-SqlMap.xml @@ -7,6 +7,10 @@ + + + + + + \ No newline at end of file diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/query/rm-common-SqlMapConfig.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/query/rm-common-SqlMapConfig.xml index e1dbaf1bea..fd97f207cb 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/query/rm-common-SqlMapConfig.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/query/rm-common-SqlMapConfig.xml @@ -3,18 +3,8 @@ - - - - - - - - - - @@ -23,7 +13,6 @@ - \ 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 43c91c32e0..1a5dcf9916 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 @@ -29,7 +29,6 @@ package org.alfresco.module.org_alfresco_module_rm.query; import java.util.Set; -import org.alfresco.repo.domain.contentdata.ContentUrlEntity; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.namespace.QName; @@ -64,9 +63,8 @@ public interface RecordsManagementQueryDAO public Set getChildrenStringPropertyValues(NodeRef parent, QName property); /** - * @param contentUrl the URL of the content url entity - * @return Return the entity or null if it doesn't exist or is still - * referenced by a content_data entity + * @param contentUrl the URL of the content url entity + * @return Return a set of UUIDs which reference the given node */ - public ContentUrlEntity getContentUrlEntityUnreferenced(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 5f8ed0d999..235fc891cf 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 @@ -30,18 +30,24 @@ package org.alfresco.module.org_alfresco_module_rm.query; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +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; import org.alfresco.repo.domain.qname.QNameDAO; import org.alfresco.repo.tenant.TenantService; +import org.alfresco.repo.version.Version2Model; import org.alfresco.service.cmr.repository.InvalidNodeRefException; import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.namespace.QName; import org.alfresco.util.Pair; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.mybatis.spring.SqlSessionTemplate; /** @@ -54,7 +60,7 @@ public class RecordsManagementQueryDAOImpl implements RecordsManagementQueryDAO, { private static final String COUNT_IDENTIFIER = "alfresco.query.rm.select_CountRMIndentifier"; private static final String GET_CHILDREN_PROPERTY_VALUES = "select_GetStringPropertyValuesOfChildren"; - private static final String SELECT_CONTENT_URL_BY_KEY_UNREFERENCED = "alfresco.content.select_ContentUrlByKeyUnreferenced"; + private static final String SELECT_NODE_IDS_WHICH_REFERENCE_CONTENT_URL = "select_NodeIdsWhichReferenceContentUrl"; /** SQL session template */ protected SqlSessionTemplate template; @@ -103,7 +109,7 @@ public class RecordsManagementQueryDAOImpl implements RecordsManagementQueryDAO, if (pair != null) { // create query params - Map params = new HashMap(2); + Map params = new HashMap<>(2); params.put("qnameId", pair.getFirst()); params.put("idValue", identifierValue); @@ -147,7 +153,7 @@ public class RecordsManagementQueryDAOImpl implements RecordsManagementQueryDAO, } @Override - public ContentUrlEntity getContentUrlEntityUnreferenced(String contentUrl) + public Set getNodeRefsWhichReferenceContentUrl(String contentUrl) { ContentUrlEntity contentUrlEntity = new ContentUrlEntity(); contentUrlEntity.setContentUrl(contentUrl); @@ -155,8 +161,31 @@ public class RecordsManagementQueryDAOImpl implements RecordsManagementQueryDAO, { contentUrlEntity.setContentUrlShort(contentUrlEntity.getContentUrlShort().toLowerCase()); } - contentUrlEntity = (ContentUrlEntity) template.selectOne(SELECT_CONTENT_URL_BY_KEY_UNREFERENCED, contentUrlEntity); - return contentUrlEntity; + // Get all the node ids which reference the given content url + List nodeIds = template.selectList(SELECT_NODE_IDS_WHICH_REFERENCE_CONTENT_URL, contentUrlEntity); + + // create a set of uuids which reference the content url + Set nodesReferencingContentUrl = new HashSet<>(); + for(Long nodeId : nodeIds) + { + String uuidToAdd; + + // 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(); + } + + // add the uuid for the node ref of the referencing node + else + { + uuidToAdd = (String) nodeDAO.getNodeProperty(nodeId, ContentModel.PROP_NODE_UUID); + } + nodesReferencingContentUrl.add(uuidToAdd); + } + + return nodesReferencingContentUrl; } } 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 da5992a0c0..83b3f9ba9f 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 @@ -28,13 +28,14 @@ package org.alfresco.module.org_alfresco_module_rm.util; import org.alfresco.model.ContentModel; import org.alfresco.module.org_alfresco_module_rm.query.RecordsManagementQueryDAO; -import org.alfresco.repo.domain.contentdata.ContentUrlEntity; import org.alfresco.repo.policy.BehaviourFilter; import org.alfresco.service.cmr.repository.ContentReader; import org.alfresco.service.cmr.repository.ContentService; import org.alfresco.service.cmr.repository.ContentWriter; import org.alfresco.service.cmr.repository.NodeRef; +import java.util.Set; + /** * Utility class to duplicate the content of a node without triggering the audit or versioning behaviours * @author Ross Gale @@ -94,8 +95,9 @@ public class ContentBinDuplicationUtility { boolean hasAtLeastOneOtherReference = false; String contentUrl = contentService.getReader(nodeRef, ContentModel.PROP_CONTENT).getContentUrl(); - ContentUrlEntity contentUrlEntity = recordsManagementQueryDAO.getContentUrlEntityUnreferenced(contentUrl); - if (contentUrlEntity == null) + + Set referencesToContentNode = recordsManagementQueryDAO.getNodeRefsWhichReferenceContentUrl(contentUrl); + if (referencesToContentNode.size() > 1) { hasAtLeastOneOtherReference = true; } 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 2c5299ff4b..d2c12a1005 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 @@ -29,7 +29,6 @@ package org.alfresco.module.org_alfresco_module_rm.util; import org.alfresco.model.ContentModel; import org.alfresco.module.org_alfresco_module_rm.query.RecordsManagementQueryDAO; -import org.alfresco.repo.domain.contentdata.ContentUrlEntity; import org.alfresco.repo.policy.BehaviourFilter; import org.alfresco.service.cmr.repository.ContentReader; import org.alfresco.service.cmr.repository.ContentService; @@ -41,6 +40,10 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.times; @@ -54,6 +57,9 @@ import static org.mockito.Mockito.when; */ public class ContentBinDuplicationUtilityUnitTest { + private final static NodeRef NODE_REF = new NodeRef("some://test/noderef"); + private final static NodeRef NODE_REF2 = new NodeRef("some://test/anothernoderef"); + private final static String CONTENT_URL = "someContentUrl"; @Mock private ContentService contentService; @@ -85,10 +91,9 @@ public class ContentBinDuplicationUtilityUnitTest @Test public void testContentUrlIsUpdated() { - NodeRef nodeRef = new NodeRef("some://test/noderef"); - when(contentService.getReader(nodeRef, ContentModel.PROP_CONTENT)).thenReturn(contentReader); - when(contentService.getWriter(nodeRef, ContentModel.PROP_CONTENT, true)).thenReturn(contentWriter); - contentBinDuplicationUtility.duplicate(nodeRef); + when(contentService.getReader(NODE_REF, ContentModel.PROP_CONTENT)).thenReturn(contentReader); + when(contentService.getWriter(NODE_REF, ContentModel.PROP_CONTENT, true)).thenReturn(contentWriter); + contentBinDuplicationUtility.duplicate(NODE_REF); verify(contentWriter, times(1)).putContent(contentReader); checkBehaviours(1); } @@ -99,9 +104,8 @@ public class ContentBinDuplicationUtilityUnitTest @Test public void testDuplicationDoesntHappenWithNoContent() { - NodeRef nodeRef = new NodeRef("some://test/noderef"); - when(contentService.getReader(nodeRef, ContentModel.PROP_CONTENT)).thenReturn(null); - contentBinDuplicationUtility.duplicate(nodeRef); + when(contentService.getReader(NODE_REF, ContentModel.PROP_CONTENT)).thenReturn(null); + contentBinDuplicationUtility.duplicate(NODE_REF); verify(contentWriter, times(0)).putContent(contentReader); checkBehaviours(1); } @@ -112,32 +116,44 @@ public class ContentBinDuplicationUtilityUnitTest @Test public void testHasAtLeastOneOtherReference() { - NodeRef nodeRef = new NodeRef("some://test/noderef"); - String contentUrl = "someContentUrl"; + Set multipleReferences = new HashSet<>(); + Collections.addAll(multipleReferences, NODE_REF.getId(), NODE_REF2.getId()); - when(contentService.getReader(nodeRef, ContentModel.PROP_CONTENT)).thenReturn(contentReader); - when(contentService.getReader(nodeRef, ContentModel.PROP_CONTENT).getContentUrl()).thenReturn(contentUrl); - when(recordsManagementQueryDAO.getContentUrlEntityUnreferenced(contentUrl)).thenReturn(null); + when(contentService.getReader(NODE_REF, ContentModel.PROP_CONTENT)).thenReturn(contentReader); + when(contentService.getReader(NODE_REF, ContentModel.PROP_CONTENT).getContentUrl()).thenReturn(CONTENT_URL); + when(recordsManagementQueryDAO.getNodeRefsWhichReferenceContentUrl(CONTENT_URL)).thenReturn(multipleReferences); - boolean hasReference = contentBinDuplicationUtility.hasAtLeastOneOtherReference(nodeRef); - assertTrue(hasReference); + assertTrue(contentBinDuplicationUtility.hasAtLeastOneOtherReference(NODE_REF)); } /** - * Test hasAtLeastOneOtherReference returns false when node has no other reference to it + * Test hasAtLeastOneOtherReference returns false when node has no other reference to it other than its own content ref */ @Test public void testHasNoOtherReference() { - NodeRef nodeRef = new NodeRef("some://test/noderef"); - String contentUrl = "someContentUrl"; - ContentUrlEntity contentUrlEntity = new ContentUrlEntity(); + Set singleReference = Collections.singleton(NODE_REF.getId()); - when(contentService.getReader(nodeRef, ContentModel.PROP_CONTENT)).thenReturn(contentReader); - when(contentService.getReader(nodeRef, ContentModel.PROP_CONTENT).getContentUrl()).thenReturn(contentUrl); - when(recordsManagementQueryDAO.getContentUrlEntityUnreferenced(contentUrl)).thenReturn(contentUrlEntity); + when(contentService.getReader(NODE_REF, ContentModel.PROP_CONTENT)).thenReturn(contentReader); + when(contentService.getReader(NODE_REF, ContentModel.PROP_CONTENT).getContentUrl()).thenReturn(CONTENT_URL); + when(recordsManagementQueryDAO.getNodeRefsWhichReferenceContentUrl(CONTENT_URL)).thenReturn(singleReference); - assertFalse(contentBinDuplicationUtility.hasAtLeastOneOtherReference(nodeRef)); + assertFalse(contentBinDuplicationUtility.hasAtLeastOneOtherReference(NODE_REF)); + } + + /** + * Test hasAtLeastOneOtherReference returns false when node has no references to it at all + */ + @Test + public void testHasNoReferences() + { + 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); + when(recordsManagementQueryDAO.getNodeRefsWhichReferenceContentUrl(CONTENT_URL)).thenReturn(noReferences); + + assertFalse(contentBinDuplicationUtility.hasAtLeastOneOtherReference(NODE_REF)); } /** From 211ebba2f232d2e8cf86325bdce4941b8bac9284 Mon Sep 17 00:00:00 2001 From: Roy Wetherall Date: Thu, 29 Aug 2019 09:50:57 +1000 Subject: [PATCH 5/9] Add logging to rm query DAO implementation --- .../org_alfresco_module_rm/log4j.properties | 4 +- .../query/RecordsManagementQueryDAOImpl.java | 52 ++++++++++++++++++- .../test/integration/issue/RM4804Test.java | 2 - 3 files changed, 53 insertions(+), 5 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 76742abc10..2f6cafb272 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 @@ -56,4 +56,6 @@ log4j.logger.org.alfresco.module.org_alfresco_module_rm.patch=info # Job debug # #log4j.logger.org.alfresco.module.org_alfresco_module_rm.job=debug -log4j.logger.org.alfresco.repo.web.scripts.roles.DynamicAuthoritiesGet=info \ No newline at end of file +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 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 235fc891cf..69d9f47b29 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 @@ -43,7 +43,6 @@ import org.alfresco.repo.tenant.TenantService; import org.alfresco.repo.version.Version2Model; import org.alfresco.service.cmr.repository.InvalidNodeRefException; import org.alfresco.service.cmr.repository.NodeRef; -import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.namespace.QName; import org.alfresco.util.Pair; import org.apache.commons.logging.Log; @@ -58,6 +57,11 @@ import org.mybatis.spring.SqlSessionTemplate; */ public class RecordsManagementQueryDAOImpl implements RecordsManagementQueryDAO, RecordsManagementModel { + /** logger */ + @SuppressWarnings("unused") + private static final Log logger = LogFactory.getLog(RecordsManagementQueryDAOImpl.class); + + /** query names */ private static final String COUNT_IDENTIFIER = "alfresco.query.rm.select_CountRMIndentifier"; private static final String GET_CHILDREN_PROPERTY_VALUES = "select_GetStringPropertyValuesOfChildren"; private static final String SELECT_NODE_IDS_WHICH_REFERENCE_CONTENT_URL = "select_NodeIdsWhichReferenceContentUrl"; @@ -152,9 +156,21 @@ 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 + */ @Override public Set getNodeRefsWhichReferenceContentUrl(String contentUrl) { + if (logger.isDebugEnabled()) + { + logger.debug("Getting nodes that reference content URL = " + contentUrl); + } + + // create the content URL entity used to query for nodes ContentUrlEntity contentUrlEntity = new ContentUrlEntity(); contentUrlEntity.setContentUrl(contentUrl); if (contentUrlEntity.getContentUrlShort() != null) @@ -162,28 +178,60 @@ public class RecordsManagementQueryDAOImpl implements RecordsManagementQueryDAO, contentUrlEntity.setContentUrlShort(contentUrlEntity.getContentUrlShort().toLowerCase()); } + if (logger.isDebugEnabled()) + { + logger.debug("Executing query " + SELECT_NODE_IDS_WHICH_REFERENCE_CONTENT_URL); + } + // Get all the node ids which reference the given content url List nodeIds = template.selectList(SELECT_NODE_IDS_WHICH_REFERENCE_CONTENT_URL, contentUrlEntity); + + if (logger.isDebugEnabled()) + { + logger.debug("Query " + SELECT_NODE_IDS_WHICH_REFERENCE_CONTENT_URL + " returned " + nodeIds.size() + " results"); + } // create a set of uuids which reference the content url Set nodesReferencingContentUrl = new HashSet<>(); for(Long nodeId : nodeIds) { + StringBuilder logMessage = null; String uuidToAdd; + + 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) - { + { 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()); + } } return nodesReferencingContentUrl; diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/issue/RM4804Test.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/issue/RM4804Test.java index e6ed70983b..5de7ee766c 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/issue/RM4804Test.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/issue/RM4804Test.java @@ -46,8 +46,6 @@ import org.apache.commons.io.IOUtils; import org.apache.commons.lang.StringUtils; import org.springframework.extensions.webscripts.GUID; -import javax.xml.soap.Node; - /** * Integration test for RM-4804 * From 1c539f9f05835b06ccdbecd00fdfae7d7ff467c3 Mon Sep 17 00:00:00 2001 From: Roy Wetherall Date: Thu, 29 Aug 2019 12:28:04 +1000 Subject: [PATCH 6/9] 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); From 8e26f5aad54db4037e541eb90cd2c3a55538fc93 Mon Sep 17 00:00:00 2001 From: Roy Wetherall Date: Thu, 29 Aug 2019 21:35:00 +1000 Subject: [PATCH 7/9] Addition tests for records, fixed up a couple of issues in the query method as a result. --- .../query/RecordsManagementQueryDAOImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 5503f5b86c..50a6675ccd 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 @@ -197,7 +197,7 @@ public class RecordsManagementQueryDAOImpl implements RecordsManagementQueryDAO, StringBuilder logMessage = null; NodeRef nodeRefToAdd; - if (nodeDAO.exists(nodeId)) + if (nodeId != null && nodeDAO.exists(nodeId)) { if (logger.isDebugEnabled()) { @@ -206,7 +206,7 @@ public class RecordsManagementQueryDAOImpl implements RecordsManagementQueryDAO, // 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) + if (version2FrozenNodeRef != null && nodeDAO.exists(version2FrozenNodeRef)) { nodeRefToAdd = version2FrozenNodeRef; From a48cb2755ccf7c7340b107b9a4e91eef0e8d70ad Mon Sep 17 00:00:00 2001 From: Roy Wetherall Date: Fri, 30 Aug 2019 11:28:44 +1000 Subject: [PATCH 8/9] Deal with review comments --- .../query/RecordsManagementQueryDAOImpl.java | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) 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 50a6675ccd..be3700a031 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 @@ -171,11 +171,7 @@ public class RecordsManagementQueryDAOImpl implements RecordsManagementQueryDAO, // create the content URL entity used to query for nodes ContentUrlEntity contentUrlEntity = new ContentUrlEntity(); - contentUrlEntity.setContentUrl(contentUrl); - if (contentUrlEntity.getContentUrlShort() != null) - { - contentUrlEntity.setContentUrlShort(contentUrlEntity.getContentUrlShort().toLowerCase()); - } + contentUrlEntity.setContentUrl(contentUrl.toLowerCase()); if (logger.isDebugEnabled()) { @@ -201,10 +197,10 @@ public class RecordsManagementQueryDAOImpl implements RecordsManagementQueryDAO, { if (logger.isDebugEnabled()) { - logMessage = new StringBuilder("Adding uuid "); + logMessage = new StringBuilder("Adding noderef "); } - // if the referencing node is a version2Store reference to the content url, add the uuid for the version 2 frozen node ref + // if the referencing node is a version2Store reference to the content url, add the version 2 frozen node ref NodeRef version2FrozenNodeRef = (NodeRef) nodeDAO.getNodeProperty(nodeId, Version2Model.PROP_QNAME_FROZEN_NODE_REF); if (version2FrozenNodeRef != null && nodeDAO.exists(version2FrozenNodeRef)) { @@ -216,7 +212,7 @@ public class RecordsManagementQueryDAOImpl implements RecordsManagementQueryDAO, } } - // add the uuid for the node ref of the referencing node + // add the node ref of the referencing node else { nodeRefToAdd = nodeDAO.getNodeIdStatus(nodeId).getNodeRef(); From fa48983bb6704df0e5df7287944355278f929786 Mon Sep 17 00:00:00 2001 From: cagache Date: Mon, 2 Sep 2019 13:33:37 +0300 Subject: [PATCH 9/9] apply code style --- .../query/RecordsManagementQueryDAOImpl.java | 118 +++++++++--------- 1 file changed, 59 insertions(+), 59 deletions(-) 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 be3700a031..a13a6fcd44 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 @@ -56,11 +56,11 @@ import org.mybatis.spring.SqlSessionTemplate; */ public class RecordsManagementQueryDAOImpl implements RecordsManagementQueryDAO, RecordsManagementModel { - /** logger */ - @SuppressWarnings("unused") - private static final Log logger = LogFactory.getLog(RecordsManagementQueryDAOImpl.class); - - /** query names */ + /** logger */ + @SuppressWarnings ("unused") + private static final Log logger = LogFactory.getLog(RecordsManagementQueryDAOImpl.class); + + /** query names */ private static final String COUNT_IDENTIFIER = "alfresco.query.rm.select_CountRMIndentifier"; private static final String GET_CHILDREN_PROPERTY_VALUES = "select_GetStringPropertyValuesOfChildren"; private static final String SELECT_NODE_IDS_WHICH_REFERENCE_CONTENT_URL = "select_NodeIdsWhichReferenceContentUrl"; @@ -157,84 +157,84 @@ 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 nodes that reference the provided content URL + * @param String contentUrl content URL */ @Override public Set getNodeRefsWhichReferenceContentUrl(String contentUrl) { - if (logger.isDebugEnabled()) - { - logger.debug("Getting nodes that reference content URL = " + contentUrl); - } - - // create the content URL entity used to query for nodes + if (logger.isDebugEnabled()) + { + logger.debug("Getting nodes that reference content URL = " + contentUrl); + } + + // create the content URL entity used to query for nodes ContentUrlEntity contentUrlEntity = new ContentUrlEntity(); contentUrlEntity.setContentUrl(contentUrl.toLowerCase()); if (logger.isDebugEnabled()) - { - logger.debug("Executing query " + SELECT_NODE_IDS_WHICH_REFERENCE_CONTENT_URL); - } - + { + logger.debug("Executing query " + SELECT_NODE_IDS_WHICH_REFERENCE_CONTENT_URL); + } + // Get all the node ids which reference the given content url List nodeIds = template.selectList(SELECT_NODE_IDS_WHICH_REFERENCE_CONTENT_URL, contentUrlEntity); - + if (logger.isDebugEnabled()) { - logger.debug("Query " + SELECT_NODE_IDS_WHICH_REFERENCE_CONTENT_URL + " returned " + nodeIds.size() + " results"); + logger.debug("Query " + SELECT_NODE_IDS_WHICH_REFERENCE_CONTENT_URL + " returned " + nodeIds.size() + " results"); } // create a set of uuids which reference the content url Set nodesReferencingContentUrl = new HashSet(nodeIds.size()); - for(Long nodeId : nodeIds) + for (Long nodeId : nodeIds) { - StringBuilder logMessage = null; + StringBuilder logMessage = null; NodeRef nodeRefToAdd; - + if (nodeId != null && nodeDAO.exists(nodeId)) { - if (logger.isDebugEnabled()) - { - logMessage = new StringBuilder("Adding noderef "); - } - - // if the referencing node is a version2Store reference to the content url, add the version 2 frozen node ref - NodeRef version2FrozenNodeRef = (NodeRef) nodeDAO.getNodeProperty(nodeId, Version2Model.PROP_QNAME_FROZEN_NODE_REF); - if (version2FrozenNodeRef != null && nodeDAO.exists(version2FrozenNodeRef)) - { - nodeRefToAdd = version2FrozenNodeRef; - - if (logger.isDebugEnabled()) - { - logMessage.append(nodeRefToAdd).append(" (from version)"); - } - } - - // add 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 (logger.isDebugEnabled()) + { + logMessage = new StringBuilder("Adding noderef "); + } + + // if the referencing node is a version2Store reference to the content url, add the version 2 frozen node ref + NodeRef version2FrozenNodeRef = (NodeRef) nodeDAO.getNodeProperty(nodeId, Version2Model.PROP_QNAME_FROZEN_NODE_REF); + if (version2FrozenNodeRef != null && nodeDAO.exists(version2FrozenNodeRef)) + { + nodeRefToAdd = version2FrozenNodeRef; + + if (logger.isDebugEnabled()) + { + logMessage.append(nodeRefToAdd).append(" (from version)"); + } + } + + // add 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()); + } } else { - if (logger.isDebugEnabled()) - { - logger.debug("Not adding " + nodeId + " (exist==false)"); - } + if (logger.isDebugEnabled()) + { + logger.debug("Not adding " + nodeId + " (exist==false)"); + } } }