From a512c3443c63c778f0c5bb5b112dea3c3058a04d Mon Sep 17 00:00:00 2001 From: evasques Date: Mon, 26 Sep 2022 19:05:03 +0100 Subject: [PATCH] MNT-23174 - RM upgrade from 3.4.1.1 to 11.153 fails (#1434) * MNT-23174 - RM upgrade from 3.4.1.1 to 11.153 fails * Added batching capability to process records in hold * Changed the retrieval of child assocs witout preload to not fill up the caches when we have very big holds * Added property rm.patch.v35.holdNewChildAssocPatch.batchSize to be able to configure the batchSize * Adapt mocks on unit test to the new calls --- .../alfresco-global.properties | 4 + .../patch/rm-patch-v35-context.xml | 1 + .../v35/RMv35HoldNewChildAssocPatch.java | 152 ++++++++++++++++-- .../RMv35HoldNewChildAssocPatchUnitTest.java | 55 ++++++- 4 files changed, 194 insertions(+), 18 deletions(-) diff --git a/amps/ags/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/alfresco-global.properties b/amps/ags/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/alfresco-global.properties index a669cf8351..e6f4de0337 100644 --- a/amps/ags/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/alfresco-global.properties +++ b/amps/ags/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/alfresco-global.properties @@ -109,6 +109,10 @@ rm.completerecord.mandatorypropertiescheck.enabled=true # rm.patch.v22.convertToStandardFilePlan=false +# +# Max Batch size for adding the associations between the frozen nodes and the hold +rm.patch.v35.holdNewChildAssocPatch.batchSize=1000 + # Permission mapping # these take a comma separated string of permissions from org.alfresco.service.cmr.security.PermissionService # read maps to ReadRecords and write to FileRecords diff --git a/amps/ags/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/patch/rm-patch-v35-context.xml b/amps/ags/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/patch/rm-patch-v35-context.xml index 6159adab5f..c29851a583 100644 --- a/amps/ags/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/patch/rm-patch-v35-context.xml +++ b/amps/ags/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/patch/rm-patch-v35-context.xml @@ -17,5 +17,6 @@ + diff --git a/amps/ags/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/patch/v35/RMv35HoldNewChildAssocPatch.java b/amps/ags/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/patch/v35/RMv35HoldNewChildAssocPatch.java index 12f2fc30a0..507f61cdc1 100644 --- a/amps/ags/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/patch/v35/RMv35HoldNewChildAssocPatch.java +++ b/amps/ags/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/patch/v35/RMv35HoldNewChildAssocPatch.java @@ -30,6 +30,9 @@ import static org.alfresco.model.ContentModel.ASSOC_CONTAINS; import static org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementCustomModel.RM_CUSTOM_URI; import static org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel.ASSOC_FROZEN_CONTENT; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Iterator; import java.util.List; import org.alfresco.model.ContentModel; @@ -37,11 +40,14 @@ import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService; import org.alfresco.module.org_alfresco_module_rm.hold.HoldService; import org.alfresco.module.org_alfresco_module_rm.patch.AbstractModulePatch; import org.alfresco.repo.policy.BehaviourFilter; +import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.namespace.QName; import org.alfresco.service.namespace.RegexQNamePattern; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Patch to create new hold child association to link the record to the hold @@ -52,8 +58,15 @@ import org.alfresco.service.namespace.RegexQNamePattern; */ public class RMv35HoldNewChildAssocPatch extends AbstractModulePatch { + /** logger */ + protected static final Logger LOGGER = LoggerFactory.getLogger(RMv35HoldNewChildAssocPatch.class); + /** A name for the associations created by this patch. */ - protected static final QName PATCH_ASSOC_NAME = QName.createQName(RM_CUSTOM_URI, RMv35HoldNewChildAssocPatch.class.getSimpleName()); + protected static final QName PATCH_ASSOC_NAME = QName.createQName(RM_CUSTOM_URI, + RMv35HoldNewChildAssocPatch.class.getSimpleName()); + + /** The batch size for processing frozen nodes. */ + private int batchSize = 1000; /** * File plan service interface @@ -75,7 +88,8 @@ public class RMv35HoldNewChildAssocPatch extends AbstractModulePatch /** * Setter for fileplanservice * - * @param filePlanService File plan service interface + * @param filePlanService + * File plan service interface */ public void setFilePlanService(FilePlanService filePlanService) { @@ -85,7 +99,8 @@ public class RMv35HoldNewChildAssocPatch extends AbstractModulePatch /** * Setter for hold service * - * @param holdService Hold service interface. + * @param holdService + * Hold service interface. */ public void setHoldService(HoldService holdService) { @@ -95,7 +110,8 @@ public class RMv35HoldNewChildAssocPatch extends AbstractModulePatch /** * Setter for node service * - * @param nodeService Interface for public and internal node and store operations. + * @param nodeService + * Interface for public and internal node and store operations. */ public void setNodeService(NodeService nodeService) { @@ -112,33 +128,49 @@ public class RMv35HoldNewChildAssocPatch extends AbstractModulePatch this.behaviourFilter = behaviourFilter; } + /** + * Setter for maximum batch size + * + * @param maxBatchSize + * The max amount of associations to be created between the frozen nodes and the hold in a transaction + */ + public void setBatchSize(int batchSize) + { + this.batchSize = batchSize; + } + @Override public void applyInternal() { behaviourFilter.disableBehaviour(ContentModel.ASPECT_AUDITABLE); behaviourFilter.disableBehaviour(ContentModel.ASPECT_VERSIONABLE); + try { + int patchedNodesCounter = 0; + for (NodeRef filePlan : filePlanService.getFilePlans()) { for (NodeRef hold : holdService.getHolds(filePlan)) { - List frozenAssoc = nodeService.getChildAssocs(hold, ASSOC_FROZEN_CONTENT, RegexQNamePattern.MATCH_ALL); - for (ChildAssociationRef ref : frozenAssoc) + LOGGER.debug("Analyzing hold {}", hold.getId()); + + BatchWorker batchWorker = new BatchWorker(hold); + + LOGGER.debug("Hold has {} items to be analyzed", batchWorker.getWorkSize()); + + while (batchWorker.hasMoreResults()) { - NodeRef childNodeRef = ref.getChildRef(); - // In testing we found that this was returning more than just "contains" associations. - // Possibly this is due to the code in Node2ServiceImpl.getParentAssocs not using the second parameter. - List parentAssocs = nodeService.getParentAssocs(childNodeRef, ASSOC_CONTAINS, RegexQNamePattern.MATCH_ALL); - boolean childContainedByHold = - parentAssocs.stream().anyMatch(entry -> entry.getParentRef().equals(hold) && entry.getTypeQName().equals(ASSOC_CONTAINS)); - if (!childContainedByHold) - { - nodeService.addChild(hold, childNodeRef, ASSOC_CONTAINS, PATCH_ASSOC_NAME); - } + processBatch(hold, batchWorker); } + + LOGGER.debug("Patched {} items in hold", batchWorker.getTotalPatchedNodes()); + + patchedNodesCounter += batchWorker.getTotalPatchedNodes(); } } + + LOGGER.debug("Patch applied to {} children across all holds", patchedNodesCounter); } finally { @@ -146,4 +178,92 @@ public class RMv35HoldNewChildAssocPatch extends AbstractModulePatch behaviourFilter.enableBehaviour(ContentModel.ASPECT_VERSIONABLE); } } + + private void processBatch(NodeRef hold, BatchWorker batch) + { + transactionService.getRetryingTransactionHelper().doInTransaction(() -> { + + Collection childRefs = batch.getNextWork(); + + LOGGER.debug("Processing batch of {} children in hold", childRefs.size()); + + for (ChildAssociationRef child : childRefs) + { + NodeRef childNodeRef = child.getChildRef(); + + if (!isChildContainedByHold(hold, childNodeRef)) + { + nodeService.addChild(hold, childNodeRef, ASSOC_CONTAINS, PATCH_ASSOC_NAME); + batch.countPatchedNode(); + } + } + + return null; + }, false, true); + } + + private boolean isChildContainedByHold(NodeRef hold, NodeRef child) + { + // In testing we found that this was returning more than just "contains" associations. + // Possibly this is due to the code in Node2ServiceImpl.getParentAssocs not using the second + // parameter. + List parentAssocs = nodeService.getParentAssocs(child, ASSOC_CONTAINS, RegexQNamePattern.MATCH_ALL); + return parentAssocs.stream() + .anyMatch(entry -> entry.getParentRef().equals(hold) && entry.getTypeQName().equals(ASSOC_CONTAINS)); + } + + private class BatchWorker + { + NodeRef hold; + int totalPatchedNodes = 0; + int workSize; + Iterator iterator; + + public BatchWorker(NodeRef hold) + { + this.hold = hold; + setupHold(); + } + + public boolean hasMoreResults() + { + return iterator == null ? true : iterator.hasNext(); + } + + public void countPatchedNode() + { + this.totalPatchedNodes += 1; + } + + public int getTotalPatchedNodes() + { + return totalPatchedNodes; + } + + public int getWorkSize() + { + return workSize; + } + + public void setupHold() + { + // Get child assocs without preloading + List holdChildren = nodeService.getChildAssocs(hold, ASSOC_FROZEN_CONTENT, + RegexQNamePattern.MATCH_ALL, Integer.MAX_VALUE, false); + this.iterator = holdChildren.listIterator(); + this.workSize = holdChildren.size(); + } + + public Collection getNextWork() + { + List frozenNodes = new ArrayList(batchSize); + while (iterator.hasNext() && frozenNodes.size() < batchSize) + { + frozenNodes.add(iterator.next()); + } + return frozenNodes; + } + + } + } diff --git a/amps/ags/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/patch/v35/RMv35HoldNewChildAssocPatchUnitTest.java b/amps/ags/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/patch/v35/RMv35HoldNewChildAssocPatchUnitTest.java index e799923b1c..4fabc47300 100644 --- a/amps/ags/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/patch/v35/RMv35HoldNewChildAssocPatchUnitTest.java +++ b/amps/ags/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/patch/v35/RMv35HoldNewChildAssocPatchUnitTest.java @@ -34,7 +34,9 @@ import static org.alfresco.model.ContentModel.ASSOC_CONTAINS; import static org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel.ASSOC_FROZEN_CONTENT; import static org.alfresco.module.org_alfresco_module_rm.patch.v35.RMv35HoldNewChildAssocPatch.PATCH_ASSOC_NAME; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -51,16 +53,21 @@ import java.util.Set; import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService; import org.alfresco.module.org_alfresco_module_rm.hold.HoldService; import org.alfresco.repo.policy.BehaviourFilter; +import org.alfresco.repo.transaction.RetryingTransactionHelper; +import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.namespace.QName; import org.alfresco.service.namespace.RegexQNamePattern; +import org.alfresco.service.transaction.TransactionService; import org.junit.Before; import org.junit.Test; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; /** * RM V3.5 Create new hold child association to link the record to the hold @@ -81,6 +88,12 @@ public class RMv35HoldNewChildAssocPatchUnitTest @Mock private BehaviourFilter mockBehaviourFilter; + @Mock + private TransactionService mockTransactionService; + + @Mock + private RetryingTransactionHelper mockRetryingTransactionHelper; + @InjectMocks private RMv35HoldNewChildAssocPatch patch; @@ -112,25 +125,63 @@ public class RMv35HoldNewChildAssocPatchUnitTest /** * Test secondary associations are created for held items so that they are "contained" in the hold. */ + @SuppressWarnings("unchecked") @Test public void testAddChildDuringUpgrade() { when(mockFilePlanService.getFilePlans()).thenReturn(fileplans); when(mockHoldService.getHolds(filePlanRef)).thenReturn(holds); - when(mockNodeService.getChildAssocs(holdRef, ASSOC_FROZEN_CONTENT, RegexQNamePattern.MATCH_ALL)).thenReturn(childAssocs); + when(mockNodeService.getChildAssocs(holdRef, ASSOC_FROZEN_CONTENT, RegexQNamePattern.MATCH_ALL, Integer.MAX_VALUE, false)) + .thenReturn(childAssocs); when(childAssociationRef.getChildRef()).thenReturn(heldItemRef); + // setup retrying transaction helper + Answer doInTransactionAnswer = new Answer() + { + @SuppressWarnings("rawtypes") + @Override + public Object answer(InvocationOnMock invocation) throws Throwable + { + RetryingTransactionCallback callback = (RetryingTransactionCallback) invocation.getArguments()[0]; + // when(childAssociationRef.getChildRef()).thenReturn(heldItemRef); + return callback.execute(); + } + }; + doAnswer(doInTransactionAnswer).when(mockRetryingTransactionHelper) + . doInTransaction(any(RetryingTransactionCallback.class), anyBoolean(), anyBoolean()); + + when(mockTransactionService.getRetryingTransactionHelper()).thenReturn(mockRetryingTransactionHelper); + patch.applyInternal(); verify(mockNodeService, times(1)).addChild(holdRef, heldItemRef, ASSOC_CONTAINS, PATCH_ASSOC_NAME); } + @SuppressWarnings("unchecked") @Test public void patchRunWithSuccessWhenNoHeldChildren() { when(mockFilePlanService.getFilePlans()).thenReturn(fileplans); when(mockHoldService.getHolds(filePlanRef)).thenReturn(holds); - when(mockNodeService.getChildAssocs(holdRef, ASSOC_FROZEN_CONTENT, RegexQNamePattern.MATCH_ALL)).thenReturn(emptyList()); + when(mockNodeService.getChildAssocs(holdRef, ASSOC_FROZEN_CONTENT, RegexQNamePattern.MATCH_ALL, Integer.MAX_VALUE, false)) + .thenReturn(emptyList()); + + // setup retrying transaction helper + Answer doInTransactionAnswer = new Answer() + { + @SuppressWarnings("rawtypes") + @Override + public Object answer(InvocationOnMock invocation) throws Throwable + { + RetryingTransactionCallback callback = (RetryingTransactionCallback) invocation.getArguments()[0]; + when(childAssociationRef.getChildRef()).thenReturn(heldItemRef); + return callback.execute(); + } + }; + doAnswer(doInTransactionAnswer).when(mockRetryingTransactionHelper) + . doInTransaction(any(RetryingTransactionCallback.class), anyBoolean(), anyBoolean()); + + when(mockTransactionService.getRetryingTransactionHelper()).thenReturn(mockRetryingTransactionHelper); patch.applyInternal();