From 53f75085a55cc1d33d0b6e0fddeb63deb5548a59 Mon Sep 17 00:00:00 2001 From: cagache Date: Mon, 5 Aug 2019 16:40:26 +0300 Subject: [PATCH] Added check for archived nodes --- .../hold/HoldServiceImpl.java | 73 ++++++++++++------- .../hold/HoldServiceImplUnitTest.java | 21 +++++- 2 files changed, 63 insertions(+), 31 deletions(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java index e1dd5c0e07..8a5990ea76 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java @@ -285,33 +285,34 @@ public class HoldServiceImpl extends ServiceBaseImpl { ParameterCheck.mandatory("nodeRef", nodeRef); - List result = null; + List result = new ArrayList<>(); // get all the immediate parent holds - Set holdsNotIncludingNodeRef = getParentHolds(nodeRef); + Set holdsIncludingNodeRef = getParentHolds(nodeRef); - // check whether the record is held by vitue of it's record folder + // check whether the record is held by virtue of it's record folder if (isRecord(nodeRef)) { List recordFolders = recordFolderService.getRecordFolders(nodeRef); for (NodeRef recordFolder : recordFolders) { - holdsNotIncludingNodeRef.addAll(getParentHolds(recordFolder)); + holdsIncludingNodeRef.addAll(getParentHolds(recordFolder)); } } if (!includedInHold) { - // invert list to get list of holds that do not contain this node - //TODO Find a way to get rid of the isFilePlanComponent(nodeRef) check. Currently it is used because - // integration tests create multiple rm sites with generated ids - NodeRef filePlan = isFilePlanComponent(nodeRef) ? filePlanService.getFilePlan(nodeRef) : filePlanService.getFilePlanBySiteId(FilePlanService.DEFAULT_RM_SITE_ID); - List allHolds = getHolds(filePlan); - result = ListUtils.subtract(allHolds, new ArrayList<>(holdsNotIncludingNodeRef)); + if (!holdsIncludingNodeRef.isEmpty()) + { + // invert list to get list of holds that do not contain this node + NodeRef filePlan = filePlanService.getFilePlan(holdsIncludingNodeRef.iterator().next()); + List allHolds = getHolds(filePlan); + result = ListUtils.subtract(allHolds, new ArrayList<>(holdsIncludingNodeRef)); + } } else { - result = new ArrayList<>(holdsNotIncludingNodeRef); + result = new ArrayList<>(holdsIncludingNodeRef); } return result; @@ -541,23 +542,7 @@ public class HoldServiceImpl extends ServiceBaseImpl ParameterCheck.mandatoryCollection("holds", holds); ParameterCheck.mandatory("nodeRef", nodeRef); - String nodeName = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_NAME); - if (!isRecord(nodeRef) && !isRecordFolder(nodeRef) && !instanceOf(nodeRef, ContentModel.TYPE_CONTENT)) - { - throw new AlfrescoRuntimeException("'" + nodeName + "' is neither a record nor a record folder nor a document. " + - "Only records, record folders or active content can be added to a hold."); - } - - if ((isRecord(nodeRef) || isRecordFolder(nodeRef)) && - permissionService.hasPermission(nodeRef, RMPermissionModel.FILING) == AccessStatus.DENIED) - { - throw new AlfrescoRuntimeException("Filing permission on '" + nodeName + "' is needed."); - } - else if (instanceOf(nodeRef, ContentModel.TYPE_CONTENT) && - permissionService.hasPermission(nodeRef, PermissionService.WRITE) == AccessStatus.DENIED) - { - throw new AlfrescoRuntimeException("Write permission on '" + nodeName + "' is needed."); - } + checkNodeCanBeAddedToHold(nodeRef); for (final NodeRef hold : holds) { @@ -569,6 +554,7 @@ public class HoldServiceImpl extends ServiceBaseImpl if (permissionService.hasPermission(hold, RMPermissionModel.FILING) == AccessStatus.DENIED) { + String nodeName = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_NAME); String holdName = (String) nodeService.getProperty(hold, ContentModel.PROP_NAME); throw new AlfrescoRuntimeException("'" + nodeName + "' can't be added to the hold container as filing permission for '" + holdName + "' is needed."); } @@ -604,6 +590,37 @@ public class HoldServiceImpl extends ServiceBaseImpl } } + /** + * Check if the given node is eligible to be added into a hold + * + * @param nodeRef the node to be checked + */ + private void checkNodeCanBeAddedToHold(NodeRef nodeRef) + { + String nodeName = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_NAME); + if (!isRecord(nodeRef) && !isRecordFolder(nodeRef) && !instanceOf(nodeRef, ContentModel.TYPE_CONTENT)) + { + throw new AlfrescoRuntimeException("'" + nodeName + "' is neither a record nor a record folder nor a document. " + + "Only records, record folders or active content can be added to a hold."); + } + + if ((isRecord(nodeRef) || isRecordFolder(nodeRef)) && + permissionService.hasPermission(nodeRef, RMPermissionModel.FILING) == AccessStatus.DENIED) + { + throw new AlfrescoRuntimeException("Filing permission on '" + nodeName + "' is needed."); + } + else if (instanceOf(nodeRef, ContentModel.TYPE_CONTENT) && + permissionService.hasPermission(nodeRef, PermissionService.WRITE) == AccessStatus.DENIED) + { + throw new AlfrescoRuntimeException("Write permission on '" + nodeName + "' is needed."); + } + + if (nodeService.hasAspect(nodeRef, ASPECT_ARCHIVED)) + { + throw new AlfrescoRuntimeException("Archived nodes can't be added to hold."); + } + } + /** * Add Frozen aspect only if node isn't already frozen * diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java index 8bcd8980fc..f5ba65377f 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java @@ -54,7 +54,7 @@ import java.util.Map; import org.alfresco.error.AlfrescoRuntimeException; import org.alfresco.model.ContentModel; import org.alfresco.module.org_alfresco_module_rm.capability.RMPermissionModel; -import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService; +import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; import org.alfresco.module.org_alfresco_module_rm.test.util.BaseUnitTest; import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.NodeRef; @@ -115,7 +115,6 @@ public class HoldServiceImplUnitTest extends BaseUnitTest // setup interactions doReturn(holdContainer).when(mockedFilePlanService).getHoldContainer(filePlan); - doReturn(filePlan).when(mockedFilePlanService).getFilePlanBySiteId(FilePlanService.DEFAULT_RM_SITE_ID); } @Test @@ -129,14 +128,19 @@ public class HoldServiceImplUnitTest extends BaseUnitTest public void heldByMultipleResults() { // setup record folder in multiple holds - List holds = new ArrayList<>(2); + List holds = new ArrayList<>(4); holds.add(new ChildAssociationRef(ASSOC_FROZEN_RECORDS, hold, ASSOC_FROZEN_RECORDS, recordFolder, true, 1)); holds.add(new ChildAssociationRef(ASSOC_FROZEN_RECORDS, hold2, ASSOC_FROZEN_RECORDS, recordFolder, true, 2)); + holds.add(new ChildAssociationRef(ASSOC_FROZEN_RECORDS, hold, ASSOC_FROZEN_RECORDS, activeContent, true, 1)); + holds.add(new ChildAssociationRef(ASSOC_FROZEN_RECORDS, hold2, ASSOC_FROZEN_RECORDS, activeContent, true, 2)); doReturn(holds).when(mockedNodeService).getParentAssocs(recordFolder, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS); //setup active content in multiple holds doReturn(holds).when(mockedNodeService).getParentAssocs(activeContent, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS); + doReturn(filePlan).when(mockedFilePlanService).getFilePlan(hold); + doReturn(filePlan).when(mockedFilePlanService).getFilePlan(hold2); + // check that both holds are found for record folder List heldByHolds = holdService.heldBy(recordFolder, true); assertNotNull(heldByHolds); @@ -405,6 +409,17 @@ public class HoldServiceImplUnitTest extends BaseUnitTest holdService.addToHold(hold, activeContent); } + @Test + public void addArchivedContentToHold() + { + expectedEx.expect(AlfrescoRuntimeException.class); + expectedEx.expectMessage("Archived nodes can't be added to hold."); + + when(mockedNodeService.getProperty(activeContent, ContentModel.PROP_NAME)).thenReturn(ACTIVE_CONTENT_NAME); + when(mockedNodeService.hasAspect(activeContent, RecordsManagementModel.ASPECT_ARCHIVED)).thenReturn(true); + holdService.addToHold(hold, activeContent); + } + @SuppressWarnings("unchecked") @Test public void addToHolds()