From 689b6d232cd0f6cdadbc90b8ac5e7b2984e37fc8 Mon Sep 17 00:00:00 2001 From: Alexandru-Eusebiu Epure Date: Fri, 9 Apr 2021 16:50:12 +0300 Subject: [PATCH] [skip tests]Merge pull request #1407 from Alfresco/merge-3.2/MNT-22310 Merge 3.2/mnt 22310 (cherry picked from commit aff9f5e7daf354aae226c8d769a31e04e29ed05b) --- .../alfresco-global.properties | 5 + .../org_alfresco_module_rm/rm-job-context.xml | 1 + .../job/DispositionLifecycleJobExecuter.java | 81 +++++++------- ...spositionLifecycleJobExecuterUnitTest.java | 104 ++++++++++-------- 4 files changed, 109 insertions(+), 82 deletions(-) diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/alfresco-global.properties b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/alfresco-global.properties index 0df8fa38c8..5c3ba95bde 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/alfresco-global.properties +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/alfresco-global.properties @@ -52,6 +52,11 @@ rm.autocompletesuggestion.nodeParameterSuggester.aspectsAndTypes=rma:record,cm:c # rm.dispositionlifecycletrigger.cronexpression=0 0/5 * * * ? +# +# Global RM retention lifecycle cron job execution batch size +# +rm.dispositionlifecycletrigger.batchsize=500 + # # Global RM notify of records due for review cron job expression # diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-job-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-job-context.xml index 047aaa4a70..139ffbd577 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-job-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-job-context.xml @@ -80,6 +80,7 @@ + diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/job/DispositionLifecycleJobExecuter.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/job/DispositionLifecycleJobExecuter.java index a19913db4e..b44cf229d3 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/job/DispositionLifecycleJobExecuter.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/job/DispositionLifecycleJobExecuter.java @@ -60,6 +60,9 @@ public class DispositionLifecycleJobExecuter extends RecordsManagementJobExecute /** logger */ private static Log logger = LogFactory.getLog(DispositionLifecycleJobExecuter.class); + /** batching properties */ + private int batchSize; + /** list of disposition actions to automatically execute */ private List dispositionActions; @@ -88,6 +91,11 @@ public class DispositionLifecycleJobExecuter extends RecordsManagementJobExecute this.dispositionActions = dispositionActions; } + public void setBatchSize(int batchSize) + { + this.batchSize = batchSize; + } + /** * @param recordsManagementActionService records management action service */ @@ -167,13 +175,14 @@ public class DispositionLifecycleJobExecuter extends RecordsManagementJobExecute { boolean hasMore = true; int skipCount = 0; - while(hasMore) + while (hasMore) { SearchParameters params = new SearchParameters(); params.addStore(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE); params.setLanguage(SearchService.LANGUAGE_FTS_ALFRESCO); params.setQuery(getQuery()); params.setSkipCount(skipCount); + params.setMaxItems(batchSize); // execute search ResultSet results = searchService.query(params); @@ -188,13 +197,12 @@ public class DispositionLifecycleJobExecuter extends RecordsManagementJobExecute } // process search results - for (NodeRef node : resultNodes) + if (!resultNodes.isEmpty()) { - executeAction(node); + executeAction(resultNodes); } } } - logger.debug("Job Finished"); } catch (AlfrescoRuntimeException exception) @@ -209,57 +217,52 @@ public class DispositionLifecycleJobExecuter extends RecordsManagementJobExecute /** * Helper method that executes a disposition action * - * @param actionNode - the disposition action to execute + * @param actionNodes - the disposition actions to execute */ - private void executeAction(final NodeRef actionNode) + private void executeAction(final List actionNodes) { - RetryingTransactionCallback processTranCB = new RetryingTransactionCallback() - { - public Boolean execute() + RetryingTransactionCallback processTranCB = () -> { + for (NodeRef actionNode : actionNodes) { - final String dispAction = (String) nodeService.getProperty(actionNode, - RecordsManagementModel.PROP_DISPOSITION_ACTION); - - // Run disposition action - if (dispAction != null && dispositionActions.contains(dispAction)) + if (nodeService.exists(actionNode)) { - ChildAssociationRef parent = nodeService.getPrimaryParent(actionNode); - if (parent.getTypeQName().equals(RecordsManagementModel.ASSOC_NEXT_DISPOSITION_ACTION)) + final String dispAction = (String) nodeService + .getProperty(actionNode, RecordsManagementModel.PROP_DISPOSITION_ACTION); + + // Run disposition action + if (dispAction != null && dispositionActions.contains(dispAction)) { - Map props = new HashMap<>(1); - props.put(RMDispositionActionExecuterAbstractBase.PARAM_NO_ERROR_CHECK, - Boolean.FALSE); - - try + ChildAssociationRef parent = nodeService.getPrimaryParent(actionNode); + if (parent.getTypeQName().equals(RecordsManagementModel.ASSOC_NEXT_DISPOSITION_ACTION)) { - // execute disposition action - recordsManagementActionService.executeRecordsManagementAction( - parent.getParentRef(), dispAction, props); + Map props = new HashMap<>(1); + props.put(RMDispositionActionExecuterAbstractBase.PARAM_NO_ERROR_CHECK, Boolean.FALSE); - if (logger.isDebugEnabled()) + try { - logger.debug("Processed action: " + dispAction + "on" + parent); + // execute disposition action + recordsManagementActionService + .executeRecordsManagementAction(parent.getParentRef(), dispAction, props); + + if (logger.isDebugEnabled()) + { + logger.debug("Processed action: " + dispAction + "on" + parent); + } } - } - catch (AlfrescoRuntimeException exception) - { - if (logger.isDebugEnabled()) + catch (AlfrescoRuntimeException exception) { - logger.debug(exception); + if (logger.isDebugEnabled()) + { + logger.debug(exception); + } } } } } - - return Boolean.TRUE; } + return Boolean.TRUE; }; - - // if exists - if (nodeService.exists(actionNode)) - { - retryingTransactionHelper.doInTransaction(processTranCB); - } + retryingTransactionHelper.doInTransaction(processTranCB, false, true); } public PersonService getPersonService() diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/job/DispositionLifecycleJobExecuterUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/job/DispositionLifecycleJobExecuterUnitTest.java index 08c40e50b4..d1cb4d4e77 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/job/DispositionLifecycleJobExecuterUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/job/DispositionLifecycleJobExecuterUnitTest.java @@ -33,6 +33,7 @@ import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyMap; import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -56,8 +57,8 @@ import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; +import org.mockito.Matchers; import org.mockito.Mock; -import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; /** @@ -73,7 +74,7 @@ public class DispositionLifecycleJobExecuterUnitTest extends BaseUnitTest private static final String RETAIN = "retain"; private static final String DESTROY = "destroy"; - /** test query snipit */ + /** test query snippet */ private static final String QUERY = "\"" + CUTOFF + "\" OR \"" + RETAIN + "\""; /** mocked result set */ @@ -91,9 +92,18 @@ public class DispositionLifecycleJobExecuterUnitTest extends BaseUnitTest { super.before(); + // Because of the fix implemented in MNT-22310, a new setup for retrying transaction helper is required. + Answer doInTransactionAnswer = invocation -> { + RetryingTransactionCallback callback = (RetryingTransactionCallback)invocation.getArguments()[0]; + return callback.execute(); + }; + doAnswer(doInTransactionAnswer).when(mockedRetryingTransactionHelper).doInTransaction(any(RetryingTransactionCallback.class), + Matchers.anyBoolean(), Matchers.anyBoolean()); + // setup data List dispositionActions = buildList(CUTOFF, RETAIN); executer.setDispositionActions(dispositionActions); + executer.setBatchSize(1); // setup interactions doReturn(mockedResultSet).when(mockedSearchService).query(any(SearchParameters.class)); @@ -102,14 +112,15 @@ public class DispositionLifecycleJobExecuterUnitTest extends BaseUnitTest /** * Helper method to verify that the query has been executed and closed + * @param numberOfInvocation number of times the query has been executed and closed */ - private void verifyQuery() + private void verifyQueryTimes(int numberOfInvocation) { ArgumentCaptor paramsCaptor = ArgumentCaptor.forClass(SearchParameters.class); - verify(mockedSearchService, times(1)).query(paramsCaptor.capture()); + verify(mockedSearchService, times(numberOfInvocation)).query(paramsCaptor.capture()); assertTrue(paramsCaptor.getValue().getQuery().contains(QUERY)); - verify(mockedResultSet, times(1)).getNodeRefs(); - verify(mockedResultSet, times(1)).close(); + verify(mockedResultSet, times(numberOfInvocation)).getNodeRefs(); + verify(mockedResultSet, times(numberOfInvocation)).close(); } /** @@ -127,7 +138,7 @@ public class DispositionLifecycleJobExecuterUnitTest extends BaseUnitTest // then // ensure the query is executed and closed - verifyQuery(); + verifyQueryTimes(1); // ensure nothing else happens becuase we have no results verifyZeroInteractions(mockedNodeService, mockedRecordFolderService, mockedRetryingTransactionHelper); @@ -143,24 +154,31 @@ public class DispositionLifecycleJobExecuterUnitTest extends BaseUnitTest // test data NodeRef node1 = generateNodeRef(); NodeRef node2 = generateNodeRef(); - List nodeRefs = buildList(node1, node2); // given - doReturn(nodeRefs).when(mockedResultSet).getNodeRefs(); doReturn(DESTROY).when(mockedNodeService).getProperty(node1, RecordsManagementModel.PROP_DISPOSITION_ACTION); doReturn(DESTROY).when(mockedNodeService).getProperty(node2, RecordsManagementModel.PROP_DISPOSITION_ACTION); + when(mockedResultSet.getNodeRefs()) + .thenReturn(buildList(node1)) + .thenReturn(buildList(node2)); + + when(mockedResultSet.hasMore()) + .thenReturn(true) + .thenReturn(false); + // when executer.executeImpl(); // then // ensure the query is executed and closed - verifyQuery(); + verifyQueryTimes(2); // ensure work is executed in transaction for each node processed verify(mockedNodeService, times(2)).exists(any(NodeRef.class)); - verify(mockedRetryingTransactionHelper, times(2)).doInTransaction(any(RetryingTransactionCallback.class)); + verify(mockedRetryingTransactionHelper, times(2)).doInTransaction(any(RetryingTransactionCallback.class), + Matchers.anyBoolean(), Matchers.anyBoolean()); // ensure each node is process correctly verify(mockedNodeService, times(1)).getProperty(node1, RecordsManagementModel.PROP_DISPOSITION_ACTION); @@ -191,14 +209,14 @@ public class DispositionLifecycleJobExecuterUnitTest extends BaseUnitTest // then // ensure the query is executed and closed - verifyQuery(); + verifyQueryTimes(1); // ensure the node exist check is made for the node verify(mockedNodeService, times(1)).exists(any(NodeRef.class)); // ensure no more interactions verifyNoMoreInteractions(mockedNodeService); - verifyZeroInteractions(mockedRecordsManagementActionService, mockedRetryingTransactionHelper); + verifyZeroInteractions(mockedRecordsManagementActionService); } /** @@ -211,27 +229,33 @@ public class DispositionLifecycleJobExecuterUnitTest extends BaseUnitTest // test data NodeRef node1 = generateNodeRef(); NodeRef node2 = generateNodeRef(); - List nodeRefs = buildList(node1, node2); NodeRef parent = generateNodeRef(); ChildAssociationRef parentAssoc = new ChildAssociationRef(ASSOC_NEXT_DISPOSITION_ACTION, parent, generateQName(), generateNodeRef()); - // given - doReturn(nodeRefs).when(mockedResultSet).getNodeRefs(); doReturn(CUTOFF).when(mockedNodeService).getProperty(node1, RecordsManagementModel.PROP_DISPOSITION_ACTION); doReturn(RETAIN).when(mockedNodeService).getProperty(node2, RecordsManagementModel.PROP_DISPOSITION_ACTION); doReturn(parentAssoc).when(mockedNodeService).getPrimaryParent(any(NodeRef.class)); + when(mockedResultSet.getNodeRefs()) + .thenReturn(buildList(node1)) + .thenReturn(buildList(node2)); + + when(mockedResultSet.hasMore()) + .thenReturn(true) + .thenReturn(false); + // when executer.executeImpl(); // then // ensure the query is executed and closed - verifyQuery(); + verifyQueryTimes(2); // ensure work is executed in transaction for each node processed verify(mockedNodeService, times(2)).exists(any(NodeRef.class)); - verify(mockedRetryingTransactionHelper, times(2)).doInTransaction(any(RetryingTransactionCallback.class)); + verify(mockedRetryingTransactionHelper, times(2)).doInTransaction(any(RetryingTransactionCallback.class), + Matchers.anyBoolean(), Matchers.anyBoolean()); // ensure each node is process correctly // node1 @@ -279,32 +303,26 @@ public class DispositionLifecycleJobExecuterUnitTest extends BaseUnitTest final NodeRef node4 = generateNodeRef(); // mock the search service to return the right page - when(mockedSearchService.query(any(SearchParameters.class))).thenAnswer( - new Answer() + when(mockedSearchService.query(any(SearchParameters.class))).thenAnswer((Answer) invocation -> { + SearchParameters params = invocation.getArgumentAt(0, SearchParameters.class); + if (params.getSkipCount() == 0) { - @Override - public ResultSet answer(InvocationOnMock invocation) - { - SearchParameters params = invocation.getArgumentAt(0, SearchParameters.class); - if (params.getSkipCount() == 0) - { - // mock first page - ResultSet result1 = mock(ResultSet.class); - when(result1.getNodeRefs()).thenReturn(Arrays.asList(node1, node2)); - when(result1.hasMore()).thenReturn(true); - return result1; - } - else if (params.getSkipCount() == 2) - { - // mock second page - ResultSet result2 = mock(ResultSet.class); - when(result2.getNodeRefs()).thenReturn(Arrays.asList(node3, node4)); - when(result2.hasMore()).thenReturn(false); - return result2; - } - throw new IndexOutOfBoundsException("Pagination did not stop after the second page!"); - } - }); + // mock first page + ResultSet result1 = mock(ResultSet.class); + when(result1.getNodeRefs()).thenReturn(Arrays.asList(node1, node2)); + when(result1.hasMore()).thenReturn(true); + return result1; + } + else if (params.getSkipCount() == 2) + { + // mock second page + ResultSet result2 = mock(ResultSet.class); + when(result2.getNodeRefs()).thenReturn(Arrays.asList(node3, node4)); + when(result2.hasMore()).thenReturn(false); + return result2; + } + throw new IndexOutOfBoundsException("Pagination did not stop after the second page!"); + }); // call the service executer.executeImpl();