diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/hold-service.properties b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/hold-service.properties new file mode 100644 index 0000000000..ae21a7ddb8 --- /dev/null +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/hold-service.properties @@ -0,0 +1,4 @@ +rm.hold.not-hold=The node {0} is not a hold. +rm.hold.add-to-hold-invalid-type={0} is neither a record nor a record folder nor active content. Only records, record \ + folders or active content can be added to a hold. +rm.hold.add-to-hold-archived-node=Archived nodes can't be added to hold. \ No newline at end of file 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 5618e90a80..04b7a32d05 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 @@ -70,6 +70,7 @@ alfresco.module.org_alfresco_module_rm.messages.dataset-service alfresco.module.org_alfresco_module_rm.messages.rm-system alfresco.module.org_alfresco_module_rm.messages.template + alfresco.module.org_alfresco_module_rm.messages.hold-service diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml index 7a903fc7b8..b463de52b8 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml @@ -1563,7 +1563,7 @@ org.alfresco.module.org_alfresco_module_rm.hold.HoldService.isHold=RM_ALLOW org.alfresco.module.org_alfresco_module_rm.hold.HoldService.getHolds=RM.Read.0,AFTER_RM.FilterNode org.alfresco.module.org_alfresco_module_rm.hold.HoldService.getHold=RM.Read.0,AFTER_RM.FilterNode - org.alfresco.module.org_alfresco_module_rm.hold.HoldService.heldBy=RM.Read.0,AFTER_RM.FilterNode + org.alfresco.module.org_alfresco_module_rm.hold.HoldService.heldBy=ACL_NODE.0.sys:base.Read,RM.Read.0,AFTER_RM.FilterNode org.alfresco.module.org_alfresco_module_rm.hold.HoldService.getHeld=RM.Read.0,AFTER_RM.FilterNode org.alfresco.module.org_alfresco_module_rm.hold.HoldService.createHold=RM_CAP.0.rma:filePlanComponent.CreateHold org.alfresco.module.org_alfresco_module_rm.hold.HoldService.getHoldReason=RM.Read.0 diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/freeze/FreezeServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/freeze/FreezeServiceImpl.java index 809094fc34..762e3188fb 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/freeze/FreezeServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/freeze/FreezeServiceImpl.java @@ -28,6 +28,9 @@ package org.alfresco.module.org_alfresco_module_rm.freeze; +import static org.alfresco.model.ContentModel.TYPE_FOLDER; +import static org.alfresco.repo.site.SiteModel.ASPECT_SITE_CONTAINER; + import java.io.Serializable; import java.util.ArrayList; import java.util.Date; @@ -168,7 +171,7 @@ public class FreezeServiceImpl extends ServiceBaseImpl NodeRef hold = null; if (!nodeRefs.isEmpty()) { - List list = new ArrayList<>(nodeRefs); + final List list = new ArrayList<>(nodeRefs); hold = createHold(list.get(0), reason); getHoldService().addToHold(hold, list); } @@ -273,8 +276,9 @@ public class FreezeServiceImpl extends ServiceBaseImpl boolean result = false; - // check that we are dealing with a record folder - if (isRecordFolder(nodeRef)) + // check that we are dealing with a record folder or a collaboration folder + if (isRecordFolder(nodeRef) || + (instanceOf(nodeRef, TYPE_FOLDER) && !nodeService.hasAspect(nodeRef, ASPECT_SITE_CONTAINER))) { int heldCount = 0; @@ -303,8 +307,8 @@ public class FreezeServiceImpl extends ServiceBaseImpl { for (ChildAssociationRef childAssociationRef : childAssocs) { - NodeRef record = childAssociationRef.getChildRef(); - if (childAssociationRef.isPrimary() && isRecord(record) && isFrozen(record)) + final NodeRef childRef = childAssociationRef.getChildRef(); + if (childAssociationRef.isPrimary() && isFrozen(childRef)) { heldCount ++; } diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldService.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldService.java index db3550a13a..9940e17007 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldService.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldService.java @@ -69,7 +69,7 @@ public interface HoldService /** * Gets the list of all the holds within the holds container for the given node reference * - * @param nodeRef The {@link NodeRef} of the record / record folder + * @param nodeRef The {@link NodeRef} of the record / record folder /active content * @param includedInHold true to retrieve the list of hold node references which will include the node reference * false to get a list of node references which will not have the given node reference * @return List of hold node references @@ -119,10 +119,10 @@ public interface HoldService void deleteHold(NodeRef hold); /** - * Adds the record to the given hold + * Adds the item to the given hold * - * @param hold The {@link NodeRef} of the hold - * @param nodeRef The {@link NodeRef} of the record / record folder which will be added to the given hold + * @param hold The {@link NodeRef} of the hold + * @param nodeRef The {@link NodeRef} of the record / record folder / active content which will be added to the given hold */ void addToHold(NodeRef hold, NodeRef nodeRef); @@ -135,10 +135,10 @@ public interface HoldService void addToHold(NodeRef hold, List nodeRefs); /** - * Adds the record to the given list of holds + * Adds the item to the given list of holds * * @param holds The list of {@link NodeRef}s of the holds - * @param nodeRef The {@link NodeRef} of the record / record folder which will be added to the given holds + * @param nodeRef The {@link NodeRef} of the record / record folder / active content which will be added to the given holds */ void addToHolds(List holds, NodeRef nodeRef); 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 a44f3c996b..93ca463d99 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 @@ -48,6 +48,7 @@ import org.alfresco.module.org_alfresco_module_rm.record.RecordService; import org.alfresco.module.org_alfresco_module_rm.recordfolder.RecordFolderService; import org.alfresco.module.org_alfresco_module_rm.util.ServiceBaseImpl; import org.alfresco.repo.node.NodeServicePolicies; +import org.alfresco.repo.node.integrity.IntegrityException; import org.alfresco.repo.policy.Behaviour.NotificationFrequency; import org.alfresco.repo.policy.annotation.Behaviour; import org.alfresco.repo.policy.annotation.BehaviourBean; @@ -64,9 +65,11 @@ import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; import org.alfresco.service.namespace.RegexQNamePattern; import org.alfresco.util.ParameterCheck; +import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.ListUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.extensions.surf.util.I18NUtil; /** * Hold service implementation @@ -87,6 +90,9 @@ public class HoldServiceImpl extends ServiceBaseImpl private static final String AUDIT_ADD_TO_HOLD = "addToHold"; private static final String AUDIT_REMOVE_FROM_HOLD = "removeFromHold"; + /** I18N */ + private static final String MSG_ERR_ACCESS_DENIED = "permissions.err_access_denied"; + /** File Plan Service */ private FilePlanService filePlanService; @@ -285,31 +291,39 @@ 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); + final 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) + final List recordFolders = recordFolderService.getRecordFolders(nodeRef); + for (final 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 - NodeRef filePlan = filePlanService.getFilePlan(nodeRef); - List allHolds = getHolds(filePlan); - result = ListUtils.subtract(allHolds, new ArrayList<>(holdsNotIncludingNodeRef)); + final Set filePlans = filePlanService.getFilePlans(); + if (!CollectionUtils.isEmpty(filePlans)) + { + final List holdsNotIncludingNodeRef = new ArrayList<>(); + filePlans.forEach(filePlan -> + { + // invert list to get list of holds that do not contain this node + final List allHolds = getHolds(filePlan); + holdsNotIncludingNodeRef.addAll(ListUtils.subtract(allHolds, new ArrayList<>(holdsIncludingNodeRef))); + }); + result = holdsNotIncludingNodeRef; + } } else { - result = new ArrayList<>(holdsNotIncludingNodeRef); + result = new ArrayList<>(holdsIncludingNodeRef); } return result; @@ -539,94 +553,101 @@ public class HoldServiceImpl extends ServiceBaseImpl ParameterCheck.mandatoryCollection("holds", holds); ParameterCheck.mandatory("nodeRef", nodeRef); - if (!isRecord(nodeRef) && !isRecordFolder(nodeRef)) - { - String nodeName = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_NAME); - throw new AlfrescoRuntimeException("'" + nodeName + "' is neither a record nor a record folder. Only records or record folders can be added to a hold."); - } - - if (permissionService.hasPermission(nodeRef, RMPermissionModel.FILING) == AccessStatus.DENIED) - { - String nodeName = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_NAME); - throw new AlfrescoRuntimeException("Filing permission on '" + nodeName + "' is needed."); - } + checkNodeCanBeAddedToHold(nodeRef); for (final NodeRef hold : holds) { if (!isHold(hold)) { - String holdName = (String) nodeService.getProperty(hold, ContentModel.PROP_NAME); - throw new AlfrescoRuntimeException("'" + holdName + "' is not a hold so record folders/records cannot be added."); + final String holdName = (String) nodeService.getProperty(hold, ContentModel.PROP_NAME); + throw new IntegrityException(I18NUtil.getMessage("rm.hold.not-hold", holdName), null); } 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."); + throw new AccessDeniedException(I18NUtil.getMessage(MSG_ERR_ACCESS_DENIED)); } // check that the node isn't already in the hold if (!getHeld(hold).contains(nodeRef)) { // run as system to ensure we have all the appropriate permissions to perform the manipulations we require - authenticationUtil.runAsSystem(new RunAsWork() - { - @Override - public Void doWork() + authenticationUtil.runAsSystem((RunAsWork) () -> { + // gather freeze properties + final Map props = new HashMap<>(2); + props.put(PROP_FROZEN_AT, new Date()); + props.put(PROP_FROZEN_BY, AuthenticationUtil.getFullyAuthenticatedUser()); + + addFrozenAspect(nodeRef, props); + + // Link the record to the hold + nodeService.addChild(hold, nodeRef, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS); + + // audit item being added to the hold + recordsManagementAuditService.auditEvent(nodeRef, AUDIT_ADD_TO_HOLD); + + // Mark all the folders contents as frozen + if (isRecordFolder(nodeRef)) { - // gather freeze properties - Map props = new HashMap<>(2); - props.put(PROP_FROZEN_AT, new Date()); - props.put(PROP_FROZEN_BY, AuthenticationUtil.getFullyAuthenticatedUser()); - - if (!nodeService.hasAspect(nodeRef, ASPECT_FROZEN)) - { - // add freeze aspect - nodeService.addAspect(nodeRef, ASPECT_FROZEN, props); - - if (logger.isDebugEnabled()) - { - StringBuilder msg = new StringBuilder(); - msg.append("Frozen aspect applied to '").append(nodeRef).append("'."); - logger.debug(msg.toString()); - } - } - - // Link the record to the hold - nodeService.addChild(hold, nodeRef, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS); - - // audit item being added to the hold - recordsManagementAuditService.auditEvent(nodeRef, AUDIT_ADD_TO_HOLD); - - // Mark all the folders contents as frozen - if (isRecordFolder(nodeRef)) - { - List records = recordService.getRecords(nodeRef); - for (NodeRef record : records) - { - // no need to freeze if already frozen! - if (!nodeService.hasAspect(record, ASPECT_FROZEN)) - { - nodeService.addAspect(record, ASPECT_FROZEN, props); - - if (logger.isDebugEnabled()) - { - StringBuilder msg = new StringBuilder(); - msg.append("Frozen aspect applied to '").append(record).append("'."); - logger.debug(msg.toString()); - } - } - } - } - - return null; + final List records = recordService.getRecords(nodeRef); + records.forEach(record -> addFrozenAspect(record, props)); } + + return null; }); } } } + /** + * 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) + { + if (!isRecord(nodeRef) && !isRecordFolder(nodeRef) && !instanceOf(nodeRef, ContentModel.TYPE_CONTENT)) + { + final String nodeName = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_NAME); + throw new IntegrityException(I18NUtil.getMessage("rm.hold.add-to-hold-invalid-type", nodeName), null); + } + + if (((isRecord(nodeRef) || isRecordFolder(nodeRef)) && + permissionService.hasPermission(nodeRef, RMPermissionModel.FILING) == AccessStatus.DENIED) || + (instanceOf(nodeRef, ContentModel.TYPE_CONTENT) && + permissionService.hasPermission(nodeRef, PermissionService.WRITE) == AccessStatus.DENIED)) + { + throw new AccessDeniedException(I18NUtil.getMessage(MSG_ERR_ACCESS_DENIED)); + } + + if (nodeService.hasAspect(nodeRef, ASPECT_ARCHIVED)) + { + throw new IntegrityException(I18NUtil.getMessage("rm.hold.add-to-hold-archived-node"), null); + } + } + + /** + * Add Frozen aspect only if node isn't already frozen + * + * @param nodeRef node on which aspect will be added + * @param props aspect properties map + */ + private void addFrozenAspect(NodeRef nodeRef, Map props) + { + if (!nodeService.hasAspect(nodeRef, ASPECT_FROZEN)) + { + // add freeze aspect + nodeService.addAspect(nodeRef, ASPECT_FROZEN, props); + + if (logger.isDebugEnabled()) + { + StringBuilder msg = new StringBuilder(); + msg.append("Frozen aspect applied to '").append(nodeRef).append("'."); + logger.debug(msg.toString()); + } + } + } + /** * @see org.alfresco.module.org_alfresco_module_rm.hold.HoldService#addToHolds(java.util.List, java.util.List) */ diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspect.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspect.java index ea933e5185..c3dcc1f4fb 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspect.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspect.java @@ -27,7 +27,14 @@ package org.alfresco.module.org_alfresco_module_rm.model.rma.aspect; +import static org.alfresco.model.ContentModel.TYPE_CONTENT; +import static org.alfresco.model.ContentModel.TYPE_FOLDER; +import static org.alfresco.repo.site.SiteModel.ASPECT_SITE_CONTAINER; + +import java.io.Serializable; +import java.util.HashMap; import java.util.List; +import java.util.Map; import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService; import org.alfresco.module.org_alfresco_module_rm.freeze.FreezeService; @@ -148,30 +155,33 @@ public class FrozenAspect extends BaseBehaviourBean kind = BehaviourKind.CLASS, notificationFrequency = NotificationFrequency.TRANSACTION_COMMIT ) - public void onAddAspect(final NodeRef record, final QName aspectTypeQName) + public void onAddAspect(final NodeRef nodeRef, final QName aspectTypeQName) { - AuthenticationUtil.runAsSystem(new RunAsWork() - { - @Override - public Void doWork() + AuthenticationUtil.runAsSystem((RunAsWork) () -> { + if (nodeService.exists(nodeRef) && (isRecord(nodeRef) || instanceOf(nodeRef, TYPE_CONTENT))) { - if (nodeService.exists(record) && - isRecord(record)) + // get the owning folder + final NodeRef parentRef = nodeService.getPrimaryParent(nodeRef).getParentRef(); + // check that the aspect has been added + if (nodeService.hasAspect(parentRef, ASPECT_HELD_CHILDREN)) { - // get the owning record folder - NodeRef recordFolder = nodeService.getPrimaryParent(record).getParentRef(); - // check that the aspect has been added - if (nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)) + // increment current count + int currentCount = (Integer) nodeService.getProperty(parentRef, PROP_HELD_CHILDREN_COUNT); + currentCount = currentCount + 1; + nodeService.setProperty(parentRef, PROP_HELD_CHILDREN_COUNT, currentCount); + } else + { + if (instanceOf(parentRef, TYPE_FOLDER) && !nodeService.hasAspect(parentRef, ASPECT_SITE_CONTAINER)) { - // increment current count - int currentCount = (Integer)nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT); - currentCount = currentCount + 1; - nodeService.setProperty(recordFolder, PROP_HELD_CHILDREN_COUNT, currentCount); + // add aspect and set count to 1 + final Map props = new HashMap<>(1); + props.put(PROP_HELD_CHILDREN_COUNT, 1); + getInternalNodeService().addAspect(parentRef, ASPECT_HELD_CHILDREN, props); } } - return null; } - }); + return null; + }); } @Override diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddActiveContentToHoldTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddActiveContentToHoldTest.java new file mode 100644 index 0000000000..191b170d59 --- /dev/null +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddActiveContentToHoldTest.java @@ -0,0 +1,248 @@ +/* + * #%L + * Alfresco Records Management Module + * %% + * Copyright (C) 2005 - 2019 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * - + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * - + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * - + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * - + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ +package org.alfresco.module.org_alfresco_module_rm.test.integration.hold; + +import static org.alfresco.repo.security.authentication.AuthenticationUtil.getAdminUserName; +import static org.alfresco.repo.site.SiteModel.SITE_MANAGER; + +import org.alfresco.module.org_alfresco_module_rm.capability.RMPermissionModel; +import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase; +import org.alfresco.repo.security.authentication.AuthenticationUtil; +import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; +import org.alfresco.repo.security.permissions.AccessDeniedException; +import org.alfresco.repo.site.SiteModel; +import org.alfresco.service.cmr.repository.NodeRef; +import org.springframework.extensions.webscripts.GUID; + +/** + * Add Active Content To Hold Integration Tests + * + * @author Claudia Agache + * @since 3.2 + */ +public class AddActiveContentToHoldTest extends BaseRMTestCase +{ + @Override + protected boolean isCollaborationSiteTest() + { + return true; + } + + @Override + protected boolean isUserTest() + { + return true; + } + + /** + * Given active content + * And file permission on the hold + * And the appropriate capability to add to hold + * When I try to add the active content to the hold + * Then the active content is frozen + * And the active content is contained within the hold + */ + public void testAddDocumentToHold() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + private NodeRef hold; + + public void given() + { + // create a hold + hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + } + + public void when() + { + // add the active content to hold + holdService.addToHold(hold, dmDocument); + } + + public void then() + { + // active content is held + assertTrue(freezeService.isFrozen(dmDocument)); + + // collaboration folder has frozen children + assertFalse(freezeService.isFrozen(dmFolder)); + assertTrue(freezeService.hasFrozenChildren(dmFolder)); + + // collaboration folder is not held + assertFalse(holdService.getHeld(hold).contains(dmFolder)); + assertFalse(holdService.heldBy(dmFolder, true).contains(hold)); + + // hold contains active content + assertTrue(holdService.getHeld(hold).contains(dmDocument)); + assertTrue(holdService.heldBy(dmDocument, true).contains(hold)); + + // additional check for child held caching + assertTrue(nodeService.hasAspect(dmFolder, ASPECT_HELD_CHILDREN)); + assertEquals(1, nodeService.getProperty(dmFolder, PROP_HELD_CHILDREN_COUNT)); + } + }); + } + + /** + * Given active content + * And a non rm user with write permission on active content + * When user tries to add the active content to hold + * Then AccessDeniedException is thrown + */ + public void testAddDocumentToHoldAsNonRMUser() + { + doBehaviourDrivenTest(new BehaviourDrivenTest(AccessDeniedException.class) + { + private NodeRef hold; + + public void given() + { + // create a hold + hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + } + + public void when() + { + // add the active content to hold as a non RM user + AuthenticationUtil.runAs( + (RunAsWork) () -> { + holdService.addToHold(hold, dmDocument); + return null; + }, dmCollaborator); + } + }); + } + + /** + * Given active content + * And a rm user with Filing permission on hold and Add to Hold Capability, but only read permission on active content + * When user tries to add the active content to hold + * Then an exception is thrown + */ + public void testAddDocumentToHoldNoWritePermissionOnDoc() + { + doBehaviourDrivenTest(new BehaviourDrivenTest(AccessDeniedException.class) + { + private NodeRef hold; + + public void given() + { + // create a hold + hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + + //add rm Admin as consumer in collaboration site to have read permissions on dmDocument + siteService.setMembership(collabSiteId, rmAdminName, SiteModel.SITE_CONSUMER); + } + + public void when() + { + // add the active content to hold as a RM admin who has Add to Hold Capability and filing permission on + // hold, but no Write permissions on doc + AuthenticationUtil.runAs( + (RunAsWork) () -> { + holdService.addToHold(hold, dmDocument); + return null; + }, rmAdminName); + } + }); + } + + /** + * Given active content + * And a rm user with Add to Hold Capability, write permission on active content and only Read permission on hold + * When user tries to add the active content to hold + * Then AccessDeniedException is thrown + */ + public void testAddDocumentToHoldNoFilingPermissionOnHold() + { + doBehaviourDrivenTest(new BehaviourDrivenTest(AccessDeniedException.class, recordsManagerName, false) + { + private NodeRef hold; + + public void given() + { + AuthenticationUtil.runAs( + (RunAsWork) () -> { + // create a hold + hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + + //add Read permission on hold + filePlanPermissionService.setPermission(hold, recordsManagerName, RMPermissionModel.READ_RECORDS); + + //add recordsManagerPerson as manager in collaboration site to have write permissions on dmDocument + siteService.setMembership(collabSiteId, recordsManagerName, SITE_MANAGER); + return null; + }, getAdminUserName()); + } + + public void when() + { + // add the active content to hold as a RM manager who has Add to Hold Capability and write permission on + // doc, but no filing permission on hold + holdService.addToHold(hold, dmDocument); + } + }); + } + + /** + * Given active content + * And a rm user with write permission on active content and Filing permission on hold, but no Add to Hold Capability + * When user tries to add the active content to hold + * Then AccessDeniedException is thrown + */ + public void testAddDocumentToHoldNoCapability() + { + doBehaviourDrivenTest(new BehaviourDrivenTest(AccessDeniedException.class, powerUserName, false) + { + private NodeRef hold; + + public void given() + { + AuthenticationUtil.runAs( + (RunAsWork) () -> { + // create a hold + hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + + //add powerUserPerson as manager in collaboration site to have write permissions on dmDocument + siteService.setMembership(collabSiteId, powerUserName, SiteModel.SITE_MANAGER); + + //assign powerUserPerson filing permission on hold + filePlanPermissionService.setPermission(hold, powerUserName, FILING); + return null; + }, getAdminUserName()); + } + + public void when() + { + // add the active content to hold as a RM power user who has write permission on doc and filing + // permission on hold, but no Add To Hold capability + holdService.addToHold(hold, dmDocument); + } + }); + } +} diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddRemoveFromHoldTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddRemoveFromHoldTest.java index 9d1d6d685b..0a31e20fa5 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddRemoveFromHoldTest.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddRemoveFromHoldTest.java @@ -44,7 +44,7 @@ import org.springframework.extensions.webscripts.GUID; public class AddRemoveFromHoldTest extends BaseRMTestCase { private static final int RECORD_COUNT = 10; - + public void testAddRecordToHold() { doBehaviourDrivenTest(new BehaviourDrivenTest() 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 57f2151c23..21cb4ce5d8 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 @@ -253,6 +253,7 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase protected String powerUserName; protected String securityOfficerName; protected String recordsManagerName; + protected String rmAdminName; /** test people */ protected NodeRef userPerson; @@ -260,6 +261,7 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase protected NodeRef powerUserPerson; protected NodeRef securityOfficerPerson; protected NodeRef recordsManagerPerson; + protected NodeRef rmAdminPerson; /** test records */ protected NodeRef recordOne; @@ -656,13 +658,18 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase recordsManagerPerson = createPerson(recordsManagerName); filePlanRoleService.assignRoleToAuthority(filePlan, FilePlanRoleService.ROLE_RECORDS_MANAGER, recordsManagerName); + rmAdminName = GUID.generate(); + rmAdminPerson = createPerson(rmAdminName); + filePlanRoleService.assignRoleToAuthority(filePlan, FilePlanRoleService.ROLE_ADMIN, rmAdminName); + testUsers = new String[] { userName, rmUserName, powerUserName, securityOfficerName, - recordsManagerName + recordsManagerName, + rmAdminName }; if (isFillingForAllUsers()) @@ -915,6 +922,13 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase } } + public BehaviourDrivenTest(Class expectedException, String runAsUser, boolean runInTransactionTests) + { + this.expectedException = expectedException; + this.runAsUser = runAsUser; + this.runInTransactionTests = runInTransactionTests; + } + public void given() throws Exception { /** empty implementation */ } public void when() throws Exception { /** empty implementation */ } 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 78c34fde09..1bdbe63d9e 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 @@ -27,6 +27,8 @@ package org.alfresco.module.org_alfresco_module_rm.hold; +import static java.util.Arrays.asList; + import static org.alfresco.module.org_alfresco_module_rm.test.util.AlfMock.generateQName; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -51,9 +53,15 @@ 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.model.RecordsManagementModel; import org.alfresco.module.org_alfresco_module_rm.test.util.BaseUnitTest; +import org.alfresco.repo.node.integrity.IntegrityException; +import org.alfresco.repo.security.permissions.AccessDeniedException; import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.cmr.security.AccessStatus; +import org.alfresco.service.cmr.security.PermissionService; import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; import org.alfresco.service.namespace.RegexQNamePattern; @@ -81,6 +89,7 @@ public class HoldServiceImplUnitTest extends BaseUnitTest protected NodeRef holdContainer; protected NodeRef hold; protected NodeRef hold2; + protected NodeRef activeContent; @Spy @InjectMocks HoldServiceImpl holdService; @@ -95,6 +104,11 @@ public class HoldServiceImplUnitTest extends BaseUnitTest hold = generateNodeRef(TYPE_HOLD); hold2 = generateNodeRef(TYPE_HOLD); + activeContent = generateNodeRef(); + QName contentSubtype = QName.createQName("contentSubtype", "contentSubtype"); + when(mockedNodeService.getType(activeContent)).thenReturn(contentSubtype); + when(mockedDictionaryService.isSubClass(contentSubtype, ContentModel.TYPE_CONTENT)).thenReturn(true); + // setup interactions doReturn(holdContainer).when(mockedFilePlanService).getHoldContainer(filePlan); } @@ -110,11 +124,18 @@ 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(Collections.singleton(filePlan)).when(mockedFilePlanService).getFilePlans(); + // check that both holds are found for record folder List heldByHolds = holdService.heldBy(recordFolder, true); assertNotNull(heldByHolds); @@ -126,6 +147,12 @@ public class HoldServiceImplUnitTest extends BaseUnitTest assertNotNull(heldByHolds); assertEquals(2, heldByHolds.size()); assertTrue(holdService.heldBy(record, false).isEmpty()); + + // check that both holds are found for active content + heldByHolds = holdService.heldBy(activeContent, true); + assertNotNull(heldByHolds); + assertEquals(2, heldByHolds.size()); + assertTrue(holdService.heldBy(activeContent, false).isEmpty()); } @Test (expected=AlfrescoRuntimeException.class) @@ -165,13 +192,15 @@ public class HoldServiceImplUnitTest extends BaseUnitTest public void getHeldWithResults() { // setup record folder in hold - List holds = new ArrayList<>(1); + List holds = new ArrayList<>(2); holds.add(new ChildAssociationRef(ASSOC_FROZEN_RECORDS, hold, ASSOC_FROZEN_RECORDS, recordFolder, true, 1)); + holds.add(new ChildAssociationRef(ASSOC_FROZEN_RECORDS, hold, ASSOC_FROZEN_RECORDS, activeContent, true, 1)); doReturn(holds).when(mockedNodeService).getChildAssocs(hold, ASSOC_FROZEN_RECORDS, RegexQNamePattern.MATCH_ALL); List list = holdService.getHeld(hold); - assertEquals(1, list.size()); + assertEquals(2, list.size()); assertEquals(recordFolder, list.get(0)); + assertEquals(activeContent, list.get(1)); } @SuppressWarnings({ "unchecked", "rawtypes" }) @@ -275,14 +304,14 @@ public class HoldServiceImplUnitTest extends BaseUnitTest // TODO check interactions with policy component!!! } - @Test (expected=AlfrescoRuntimeException.class) + @Test (expected = IntegrityException.class) public void addToHoldNotAHold() { holdService.addToHold(recordFolder, recordFolder); } - @Test (expected=AlfrescoRuntimeException.class) - public void addToHoldNotARecordFolderOrRecord() + @Test (expected = IntegrityException.class) + public void addToHoldNotARecordFolderOrRecordOrActiveContent() { NodeRef anotherThing = generateNodeRef(TYPE_RECORD_CATEGORY); holdService.addToHold(hold, anotherThing); @@ -297,20 +326,25 @@ public class HoldServiceImplUnitTest extends BaseUnitTest verify(mockedNodeService).addChild(hold, recordFolder, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS); verify(mockedNodeService).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class)); verify(mockedNodeService).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class)); - verify(mockedRecordsManagementAuditService, times(1)).auditEvent(eq(recordFolder), anyString()); + verify(mockedRecordsManagementAuditService).auditEvent(eq(recordFolder), anyString()); holdService.addToHold(hold, record); verify(mockedNodeService).addChild(hold, record, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS); verify(mockedNodeService).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class)); verify(mockedNodeService, times(2)).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class)); - verify(mockedRecordsManagementAuditService, times(1)).auditEvent(eq(record), anyString()); + verify(mockedRecordsManagementAuditService).auditEvent(eq(record), anyString()); + + holdService.addToHold(hold, activeContent); + verify(mockedNodeService).addChild(hold, activeContent, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS); + verify(mockedNodeService).addAspect(eq(activeContent), eq(ASPECT_FROZEN), any(Map.class)); + verify(mockedRecordsManagementAuditService).auditEvent(eq(activeContent), anyString()); } @SuppressWarnings("unchecked") @Test public void addToHoldAlreadyInHold() { - doReturn(Collections.singletonList(recordFolder)).when(holdService).getHeld(hold); + doReturn(asList(recordFolder, activeContent)).when(holdService).getHeld(hold); holdService.addToHold(hold, recordFolder); @@ -318,21 +352,54 @@ public class HoldServiceImplUnitTest extends BaseUnitTest verify(mockedNodeService, never()).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class)); verify(mockedNodeService, never()).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class)); verify(mockedRecordsManagementAuditService, never()).auditEvent(eq(recordFolder), anyString()); + + holdService.addToHold(hold, activeContent); + + verify(mockedNodeService, never()).addChild(hold, activeContent, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS); + verify(mockedNodeService, never()).addAspect(eq(activeContent), eq(ASPECT_FROZEN), any(Map.class)); + verify(mockedRecordsManagementAuditService, never()).auditEvent(eq(activeContent), anyString()); } @SuppressWarnings("unchecked") @Test - public void addToHoldAldeadyFrozen() + public void addToHoldAlreadyFrozen() { doReturn(true).when(mockedNodeService).hasAspect(recordFolder, ASPECT_FROZEN); doReturn(true).when(mockedNodeService).hasAspect(record, ASPECT_FROZEN); + doReturn(true).when(mockedNodeService).hasAspect(activeContent, ASPECT_FROZEN); holdService.addToHold(hold, recordFolder); - verify(mockedNodeService, times(1)).addChild(hold, recordFolder, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS); + verify(mockedNodeService).addChild(hold, recordFolder, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS); verify(mockedNodeService, never()).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class)); verify(mockedNodeService, never()).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class)); - verify(mockedRecordsManagementAuditService, times(1)).auditEvent(eq(recordFolder), anyString()); + verify(mockedRecordsManagementAuditService).auditEvent(eq(recordFolder), anyString()); + + holdService.addToHold(hold, activeContent); + verify(mockedNodeService).addChild(hold, activeContent, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS); + verify(mockedNodeService, never()).addAspect(eq(activeContent), eq(ASPECT_FROZEN), any(Map.class)); + verify(mockedRecordsManagementAuditService).auditEvent(eq(activeContent), anyString()); + } + + @Test (expected = AccessDeniedException.class) + public void addActiveContentToHoldNoPermissionsOnHold() + { + when(mockedPermissionService.hasPermission(hold, RMPermissionModel.FILING)).thenReturn(AccessStatus.DENIED); + holdService.addToHold(hold, activeContent); + } + + @Test (expected = AccessDeniedException.class) + public void addActiveContentToHoldNoPermissionsOnContent() + { + when(mockedPermissionService.hasPermission(activeContent, PermissionService.WRITE)).thenReturn(AccessStatus.DENIED); + holdService.addToHold(hold, activeContent); + } + + @Test (expected = IntegrityException.class) + public void addArchivedContentToHold() + { + when(mockedNodeService.hasAspect(activeContent, RecordsManagementModel.ASPECT_ARCHIVED)).thenReturn(true); + holdService.addToHold(hold, activeContent); } @SuppressWarnings("unchecked")