From 7c4d8e91abdacc65755d0e3a73dfa15ceaacf62c Mon Sep 17 00:00:00 2001 From: Sara Aspery Date: Tue, 30 Jul 2019 16:35:48 +0100 Subject: [PATCH] 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