From 93084fac63c3377871da385535050780a7078a01 Mon Sep 17 00:00:00 2001 From: rlucanu Date: Thu, 8 Aug 2019 13:15:22 +0300 Subject: [PATCH 1/6] RM-6904 Prevent updating held active content --- .../model/rma/aspect/FrozenAspect.java | 111 +++++++++++++++++- 1 file changed, 108 insertions(+), 3 deletions(-) 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 c3dcc1f4fb..d44187f9a9 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 @@ -33,12 +33,17 @@ import static org.alfresco.repo.site.SiteModel.ASPECT_SITE_CONTAINER; import java.io.Serializable; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; +import org.alfresco.model.ApplicationModel; import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService; import org.alfresco.module.org_alfresco_module_rm.freeze.FreezeService; import org.alfresco.module.org_alfresco_module_rm.model.BaseBehaviourBean; +import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; +import org.alfresco.repo.content.ContentServicePolicies; import org.alfresco.repo.node.NodeServicePolicies; import org.alfresco.repo.policy.Behaviour.NotificationFrequency; import org.alfresco.repo.policy.annotation.Behaviour; @@ -47,9 +52,12 @@ import org.alfresco.repo.policy.annotation.BehaviourKind; 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.rest.framework.core.exceptions.PermissionDeniedException; import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.namespace.QName; +import org.alfresco.util.PropertyMap; +import org.springframework.dao.PermissionDeniedDataAccessException; /** * rma:frozen behaviour bean @@ -64,7 +72,10 @@ import org.alfresco.service.namespace.QName; public class FrozenAspect extends BaseBehaviourBean implements NodeServicePolicies.BeforeDeleteNodePolicy, NodeServicePolicies.OnAddAspectPolicy, - NodeServicePolicies.OnRemoveAspectPolicy + NodeServicePolicies.OnRemoveAspectPolicy, + NodeServicePolicies.OnCreateChildAssociationPolicy, + NodeServicePolicies.OnUpdatePropertiesPolicy, + ContentServicePolicies.OnContentUpdatePolicy { /** file plan service */ protected FilePlanService filePlanService; @@ -106,8 +117,7 @@ public class FrozenAspect extends BaseBehaviourBean @Override public Void doWork() { - if (nodeService.exists(nodeRef) && - filePlanService.isFilePlanComponent(nodeRef)) + if (nodeService.exists(nodeRef)) { if (freezeService.isFrozen(nodeRef)) { @@ -221,4 +231,99 @@ public class FrozenAspect extends BaseBehaviourBean } + /** + * Behaviour associated with moving/copying a frozen node + *

+ * Prevent frozen items being moved or copied + */ + @Override + @Behaviour + ( + kind = BehaviourKind.ASSOCIATION, + notificationFrequency = NotificationFrequency.FIRST_EVENT + ) + public void onCreateChildAssociation(ChildAssociationRef childAssocRef, boolean isNewNode) + { + AuthenticationUtil.runAsSystem(new RunAsWork() + { + @Override + public Void doWork() + { + NodeRef childNodeRef = childAssocRef.getChildRef(); + if (freezeService.isFrozen(childNodeRef)) + { + // never allow to move or copy a frozen node + throw new AccessDeniedException("Frozen nodes can not be moved or copied."); + } + return null; + } + }); + } + + /** + * Behaviour associated with updating properties + *

+ * Prevents frozen items being updated + */ + @Override + @Behaviour + ( + kind = BehaviourKind.CLASS, + notificationFrequency = NotificationFrequency.TRANSACTION_COMMIT + ) + public void onUpdateProperties(NodeRef nodeRef, Map before, Map after) + { + AuthenticationUtil.runAsSystem(new RunAsWork() + { + @Override + public Void doWork() + { + if (nodeService.exists(nodeRef)) + { + if (freezeService.isFrozen(nodeRef)) + { + // Determine the properties that have changed + Map changedProperties = PropertyMap.getChangedProperties(before, after); + // never allow to update a frozen node + if (changedProperties != null && !changedProperties.isEmpty()) + { + throw new AccessDeniedException("Frozen nodes can not be updated."); + } + } + } + return null; + } + }); + } + + /** + * Behaviour associated with updating the content + *

+ * Ensures that the content of a frozen node can not be updated + */ + @Override + @Behaviour + ( + kind = BehaviourKind.CLASS, + notificationFrequency = NotificationFrequency.TRANSACTION_COMMIT + ) + public void onContentUpdate(NodeRef nodeRef, boolean newContent) + { + AuthenticationUtil.runAsSystem(new RunAsWork() + { + @Override + public Void doWork() + { + if (nodeService.exists(nodeRef)) + { + if (freezeService.isFrozen(nodeRef)) + { + // never allow to update the content of a frozen node + throw new AccessDeniedException("Frozen nodes content can not be updated."); + } + } + return null; + } + }); + } } From ae64374c613284360ec66cec69aec07a3cc081a6 Mon Sep 17 00:00:00 2001 From: rlucanu Date: Tue, 13 Aug 2019 01:24:22 +0300 Subject: [PATCH 2/6] RM-6904 Tiding up --- .../model/rma/aspect/FrozenAspect.java | 77 ++++++++++++------- 1 file changed, 49 insertions(+), 28 deletions(-) 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 d44187f9a9..d9622c20af 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 @@ -44,6 +44,9 @@ import org.alfresco.module.org_alfresco_module_rm.freeze.FreezeService; import org.alfresco.module.org_alfresco_module_rm.model.BaseBehaviourBean; import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; import org.alfresco.repo.content.ContentServicePolicies; +import org.alfresco.repo.copy.CopyBehaviourCallback; +import org.alfresco.repo.copy.CopyDetails; +import org.alfresco.repo.copy.CopyServicePolicies; import org.alfresco.repo.node.NodeServicePolicies; import org.alfresco.repo.policy.Behaviour.NotificationFrequency; import org.alfresco.repo.policy.annotation.Behaviour; @@ -73,9 +76,10 @@ public class FrozenAspect extends BaseBehaviourBean implements NodeServicePolicies.BeforeDeleteNodePolicy, NodeServicePolicies.OnAddAspectPolicy, NodeServicePolicies.OnRemoveAspectPolicy, - NodeServicePolicies.OnCreateChildAssociationPolicy, NodeServicePolicies.OnUpdatePropertiesPolicy, - ContentServicePolicies.OnContentUpdatePolicy + NodeServicePolicies.OnMoveNodePolicy, + ContentServicePolicies.OnContentUpdatePolicy, + CopyServicePolicies.BeforeCopyPolicy { /** file plan service */ protected FilePlanService filePlanService; @@ -117,17 +121,14 @@ public class FrozenAspect extends BaseBehaviourBean @Override public Void doWork() { - if (nodeService.exists(nodeRef)) + if (nodeService.exists(nodeRef) && freezeService.isFrozen(nodeRef)) { - if (freezeService.isFrozen(nodeRef)) - { - // never allowed to delete a frozen node - throw new AccessDeniedException("Frozen nodes can not be deleted."); - } - - // check children - checkChildren(nodeService.getChildAssocs(nodeRef)); + // never allow to delete a frozen node + throw new AccessDeniedException("Frozen nodes can not be deleted."); } + + // check children + checkChildren(nodeService.getChildAssocs(nodeRef)); return null; } }); @@ -208,22 +209,22 @@ public class FrozenAspect extends BaseBehaviourBean public Void doWork() { if (nodeService.exists(record) && - isRecord(record)) + isRecord(record)) { // 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)) { // decrement current count - int currentCount = (Integer)nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT); + int currentCount = (Integer) nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT); if (currentCount > 0) { currentCount = currentCount - 1; nodeService.setProperty(recordFolder, PROP_HELD_CHILDREN_COUNT, currentCount); } - } + } } return null; } @@ -232,9 +233,9 @@ public class FrozenAspect extends BaseBehaviourBean } /** - * Behaviour associated with moving/copying a frozen node + * Behaviour associated with moving a frozen node *

- * Prevent frozen items being moved or copied + * Prevent frozen items being moved */ @Override @Behaviour @@ -242,22 +243,21 @@ public class FrozenAspect extends BaseBehaviourBean kind = BehaviourKind.ASSOCIATION, notificationFrequency = NotificationFrequency.FIRST_EVENT ) - public void onCreateChildAssociation(ChildAssociationRef childAssocRef, boolean isNewNode) + public void onMoveNode(final ChildAssociationRef oldChildAssocRef, final ChildAssociationRef newChildAssocRef) { - AuthenticationUtil.runAsSystem(new RunAsWork() + AuthenticationUtil.runAs(new RunAsWork() { @Override public Void doWork() { - NodeRef childNodeRef = childAssocRef.getChildRef(); - if (freezeService.isFrozen(childNodeRef)) + if (nodeService.exists(newChildAssocRef.getParentRef()) && + nodeService.exists(newChildAssocRef.getChildRef())) { - // never allow to move or copy a frozen node - throw new AccessDeniedException("Frozen nodes can not be moved or copied."); + throw new AccessDeniedException("Frozen nodes can not be moved."); } return null; } - }); + }, AuthenticationUtil.getSystemUserName()); } /** @@ -272,6 +272,7 @@ public class FrozenAspect extends BaseBehaviourBean notificationFrequency = NotificationFrequency.TRANSACTION_COMMIT ) public void onUpdateProperties(NodeRef nodeRef, Map before, Map after) + { AuthenticationUtil.runAsSystem(new RunAsWork() { @@ -314,16 +315,36 @@ public class FrozenAspect extends BaseBehaviourBean @Override public Void doWork() { - if (nodeService.exists(nodeRef)) - { - if (freezeService.isFrozen(nodeRef)) + if (nodeService.exists(nodeRef) && freezeService.isFrozen(nodeRef)) { // never allow to update the content of a frozen node throw new AccessDeniedException("Frozen nodes content can not be updated."); } - } return null; } }); } + + @Override + @Behaviour + ( + kind = BehaviourKind.CLASS + ) + public void beforeCopy(QName classRef, NodeRef sourceNodeRef, NodeRef targetNodeRef) + { + AuthenticationUtil.runAs(new RunAsWork() + { + @Override + public Void doWork() + { + //nodeService.getPrimaryParent(sourceNodeRef).getParentRef() + if (nodeService.exists(sourceNodeRef) && freezeService.isFrozen(sourceNodeRef)) + { + throw new AccessDeniedException("Frozen nodes can not be copied."); + } + + return null; + } + }, AuthenticationUtil.getSystemUserName()); + } } From 94936bc8a7928a7f2fd22694bf2240a135237738 Mon Sep 17 00:00:00 2001 From: rlucanu Date: Wed, 14 Aug 2019 12:12:54 +0300 Subject: [PATCH 3/6] RM-6904 Fixing tests --- .../rm-model-context.xml | 1 - .../hold/HoldServiceImpl.java | 2 + .../model/rma/aspect/FrozenAspect.java | 153 ++++++------------ 3 files changed, 49 insertions(+), 107 deletions(-) diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml index a52089bd08..07adb5d9bf 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml @@ -168,7 +168,6 @@ - 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 93ca463d99..0fdb4fb3c9 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 @@ -636,6 +636,8 @@ public class HoldServiceImpl extends ServiceBaseImpl { if (!nodeService.hasAspect(nodeRef, ASPECT_FROZEN)) { + //set in transaction cache in order not to trigger update policy when adding the aspect + transactionalResourceHelper.getSet(nodeRef).add("frozen"); // add freeze aspect nodeService.addAspect(nodeRef, ASPECT_FROZEN, props); 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 d9622c20af..00cdb290d2 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 @@ -33,19 +33,12 @@ import static org.alfresco.repo.site.SiteModel.ASPECT_SITE_CONTAINER; import java.io.Serializable; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; -import org.alfresco.model.ApplicationModel; -import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService; import org.alfresco.module.org_alfresco_module_rm.freeze.FreezeService; import org.alfresco.module.org_alfresco_module_rm.model.BaseBehaviourBean; -import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; import org.alfresco.repo.content.ContentServicePolicies; -import org.alfresco.repo.copy.CopyBehaviourCallback; -import org.alfresco.repo.copy.CopyDetails; import org.alfresco.repo.copy.CopyServicePolicies; import org.alfresco.repo.node.NodeServicePolicies; import org.alfresco.repo.policy.Behaviour.NotificationFrequency; @@ -55,12 +48,9 @@ import org.alfresco.repo.policy.annotation.BehaviourKind; 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.rest.framework.core.exceptions.PermissionDeniedException; import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.namespace.QName; -import org.alfresco.util.PropertyMap; -import org.springframework.dao.PermissionDeniedDataAccessException; /** * rma:frozen behaviour bean @@ -81,20 +71,9 @@ public class FrozenAspect extends BaseBehaviourBean ContentServicePolicies.OnContentUpdatePolicy, CopyServicePolicies.BeforeCopyPolicy { - /** file plan service */ - protected FilePlanService filePlanService; - /** freeze service */ protected FreezeService freezeService; - /** - * @param filePlanService file plan service - */ - public void setFilePlanService(FilePlanService filePlanService) - { - this.filePlanService = filePlanService; - } - /** * @param freezeService freeze service */ @@ -116,21 +95,16 @@ public class FrozenAspect extends BaseBehaviourBean ) public void beforeDeleteNode(final NodeRef nodeRef) { - AuthenticationUtil.runAsSystem(new RunAsWork() - { - @Override - public Void doWork() + AuthenticationUtil.runAsSystem((RunAsWork) () -> { + if (nodeService.exists(nodeRef) && freezeService.isFrozen(nodeRef)) { - if (nodeService.exists(nodeRef) && freezeService.isFrozen(nodeRef)) - { - // never allow to delete a frozen node - throw new AccessDeniedException("Frozen nodes can not be deleted."); - } - - // check children - checkChildren(nodeService.getChildAssocs(nodeRef)); - return null; + // never allow to delete a frozen node + throw new AccessDeniedException("Frozen nodes can not be deleted."); } + + // check children + checkChildren(nodeService.getChildAssocs(nodeRef)); + return null; }); } @@ -150,7 +124,7 @@ public class FrozenAspect extends BaseBehaviourBean NodeRef nodeRef = assoc.getChildRef(); if (freezeService.isFrozen(nodeRef)) { - // never allowed to delete a node with a frozen child + // never allow to delete a node with a frozen child throw new AccessDeniedException("Can not delete node, because it contains a frozen child node."); } @@ -203,32 +177,27 @@ public class FrozenAspect extends BaseBehaviourBean ) public void onRemoveAspect(final NodeRef record, QName aspectTypeQName) { - AuthenticationUtil.runAsSystem(new RunAsWork() - { - @Override - public Void doWork() + AuthenticationUtil.runAsSystem((RunAsWork) () -> { + if (nodeService.exists(record) && + isRecord(record)) { - if (nodeService.exists(record) && - isRecord(record)) - { - // get the owning record folder - NodeRef recordFolder = nodeService.getPrimaryParent(record).getParentRef(); + // 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)) + // check that the aspect has been added + if (nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)) + { + // decrement current count + int currentCount = (Integer) nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT); + if (currentCount > 0) { - // decrement current count - int currentCount = (Integer) nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT); - if (currentCount > 0) - { - currentCount = currentCount - 1; - nodeService.setProperty(recordFolder, PROP_HELD_CHILDREN_COUNT, currentCount); - } + currentCount = currentCount - 1; + nodeService.setProperty(recordFolder, PROP_HELD_CHILDREN_COUNT, currentCount); } } - return null; } - }); + return null; + }); } @@ -245,18 +214,13 @@ public class FrozenAspect extends BaseBehaviourBean ) public void onMoveNode(final ChildAssociationRef oldChildAssocRef, final ChildAssociationRef newChildAssocRef) { - AuthenticationUtil.runAs(new RunAsWork() - { - @Override - public Void doWork() + AuthenticationUtil.runAs((RunAsWork) () -> { + if (nodeService.exists(newChildAssocRef.getParentRef()) && + nodeService.exists(newChildAssocRef.getChildRef())) { - if (nodeService.exists(newChildAssocRef.getParentRef()) && - nodeService.exists(newChildAssocRef.getChildRef())) - { - throw new AccessDeniedException("Frozen nodes can not be moved."); - } - return null; + throw new AccessDeniedException("Frozen nodes can not be moved."); } + return null; }, AuthenticationUtil.getSystemUserName()); } @@ -274,26 +238,14 @@ public class FrozenAspect extends BaseBehaviourBean public void onUpdateProperties(NodeRef nodeRef, Map before, Map after) { - AuthenticationUtil.runAsSystem(new RunAsWork() - { - @Override - public Void doWork() - { - if (nodeService.exists(nodeRef)) + AuthenticationUtil.runAsSystem((RunAsWork) () -> { + // check to not throw exception when the aspect is being added + if (nodeService.exists(nodeRef) && freezeService.isFrozen(nodeRef) && + !transactionalResourceHelper.getSet(nodeRef).contains("frozen") ) { - if (freezeService.isFrozen(nodeRef)) - { - // Determine the properties that have changed - Map changedProperties = PropertyMap.getChangedProperties(before, after); - // never allow to update a frozen node - if (changedProperties != null && !changedProperties.isEmpty()) - { - throw new AccessDeniedException("Frozen nodes can not be updated."); - } - } + throw new AccessDeniedException("Frozen nodes can not be updated."); } - return null; - } + return null; }); } @@ -310,18 +262,13 @@ public class FrozenAspect extends BaseBehaviourBean ) public void onContentUpdate(NodeRef nodeRef, boolean newContent) { - AuthenticationUtil.runAsSystem(new RunAsWork() - { - @Override - public Void doWork() - { - if (nodeService.exists(nodeRef) && freezeService.isFrozen(nodeRef)) - { - // never allow to update the content of a frozen node - throw new AccessDeniedException("Frozen nodes content can not be updated."); - } - return null; - } + AuthenticationUtil.runAsSystem((RunAsWork) () -> { + if (nodeService.exists(nodeRef) && freezeService.isFrozen(nodeRef)) + { + // never allow to update the content of a frozen node + throw new AccessDeniedException("Frozen nodes content can not be updated."); + } + return null; }); } @@ -332,19 +279,13 @@ public class FrozenAspect extends BaseBehaviourBean ) public void beforeCopy(QName classRef, NodeRef sourceNodeRef, NodeRef targetNodeRef) { - AuthenticationUtil.runAs(new RunAsWork() - { - @Override - public Void doWork() + AuthenticationUtil.runAs((RunAsWork) () -> { + if (nodeService.exists(sourceNodeRef) && freezeService.isFrozen(sourceNodeRef)) { - //nodeService.getPrimaryParent(sourceNodeRef).getParentRef() - if (nodeService.exists(sourceNodeRef) && freezeService.isFrozen(sourceNodeRef)) - { - throw new AccessDeniedException("Frozen nodes can not be copied."); - } - - return null; + throw new AccessDeniedException("Frozen nodes can not be copied."); } + + return null; }, AuthenticationUtil.getSystemUserName()); } } From 2af340c5d62b079b4228620cc8d7f68cb9c63829 Mon Sep 17 00:00:00 2001 From: Ross Gale Date: Wed, 14 Aug 2019 13:40:39 +0100 Subject: [PATCH 4/6] RM-6873 sonar update --- .../org_alfresco_module_rm/model/rma/aspect/FrozenAspect.java | 4 ++-- ...ntToHoldTest.java => RemoveActiveContentFromHoldTest.java} | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) rename rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/{RemoveActiveContentToHoldTest.java => RemoveActiveContentFromHoldTest.java} (99%) 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 f5f174c73a..6864207560 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 @@ -197,13 +197,13 @@ public class FrozenAspect extends BaseBehaviourBean (isRecord(nodeRef) || instanceOf(nodeRef, TYPE_CONTENT))) { // get the owning folder - NodeRef owningFolder = nodeService.getPrimaryParent(nodeRef).getParentRef(); + final NodeRef owningFolder = nodeService.getPrimaryParent(nodeRef).getParentRef(); // check that the aspect has been added if (nodeService.hasAspect(owningFolder, ASPECT_HELD_CHILDREN)) { // decrement current count - int currentCount = (Integer) nodeService.getProperty(owningFolder, PROP_HELD_CHILDREN_COUNT); + final int currentCount = (Integer) nodeService.getProperty(owningFolder, PROP_HELD_CHILDREN_COUNT); if (currentCount > 0) { nodeService.setProperty(owningFolder, PROP_HELD_CHILDREN_COUNT, currentCount - 1); diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveActiveContentToHoldTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveActiveContentFromHoldTest.java similarity index 99% rename from rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveActiveContentToHoldTest.java rename to rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveActiveContentFromHoldTest.java index 6c597f15d4..8a4181116e 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveActiveContentToHoldTest.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveActiveContentFromHoldTest.java @@ -42,7 +42,7 @@ import org.springframework.extensions.webscripts.GUID; * @author Ross Gale * @since 3.2 */ -public class RemoveActiveContentToHoldTest extends BaseRMTestCase +public class RemoveActiveContentFromHoldTest extends BaseRMTestCase { @Override protected boolean isCollaborationSiteTest() From 4f03687e8317c7b3a776ad030cad770834fcd562 Mon Sep 17 00:00:00 2001 From: rlucanu Date: Wed, 14 Aug 2019 17:54:37 +0300 Subject: [PATCH 5/6] RM-6904 code review changes --- .../model/rma/aspect/FrozenAspect.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 00cdb290d2..7ddceb5808 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 @@ -214,14 +214,14 @@ public class FrozenAspect extends BaseBehaviourBean ) public void onMoveNode(final ChildAssociationRef oldChildAssocRef, final ChildAssociationRef newChildAssocRef) { - AuthenticationUtil.runAs((RunAsWork) () -> { + AuthenticationUtil.runAsSystem((RunAsWork) () -> { if (nodeService.exists(newChildAssocRef.getParentRef()) && nodeService.exists(newChildAssocRef.getChildRef())) { throw new AccessDeniedException("Frozen nodes can not be moved."); } return null; - }, AuthenticationUtil.getSystemUserName()); + }); } /** @@ -279,13 +279,13 @@ public class FrozenAspect extends BaseBehaviourBean ) public void beforeCopy(QName classRef, NodeRef sourceNodeRef, NodeRef targetNodeRef) { - AuthenticationUtil.runAs((RunAsWork) () -> { + AuthenticationUtil.runAsSystem((RunAsWork) () -> { if (nodeService.exists(sourceNodeRef) && freezeService.isFrozen(sourceNodeRef)) { throw new AccessDeniedException("Frozen nodes can not be copied."); } return null; - }, AuthenticationUtil.getSystemUserName()); + }); } } From 1d739008c2f17a1344c2a7ae7c36ff3547788775 Mon Sep 17 00:00:00 2001 From: Ross Gale Date: Fri, 16 Aug 2019 07:25:19 +0100 Subject: [PATCH 6/6] RM-6873 adding unit tests for edit --- .../rma/aspect/FrozenAspectUnitTest.java | 147 ++++++++++++++++++ 1 file changed, 147 insertions(+) diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspectUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspectUnitTest.java index 0820c896a9..e94f2ad103 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspectUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspectUnitTest.java @@ -29,15 +29,25 @@ package org.alfresco.module.org_alfresco_module_rm.model.rma.aspect; import static org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel.ASPECT_HELD_CHILDREN; import static org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel.ASPECT_RECORD; import static org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel.PROP_HELD_CHILDREN_COUNT; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyMap; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; + import org.alfresco.model.ContentModel; +import org.alfresco.module.org_alfresco_module_rm.freeze.FreezeService; +import org.alfresco.module.org_alfresco_module_rm.util.TransactionalResourceHelper; +import org.alfresco.repo.security.permissions.AccessDeniedException; import org.alfresco.service.cmr.dictionary.DictionaryService; 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.junit.Before; import org.junit.Test; import org.mockito.InjectMocks; @@ -64,12 +74,37 @@ public class FrozenAspectUnitTest @Mock private DictionaryService mockDictionaryService; + @Mock + private FreezeService mockFreezeService; + + @Mock + private TransactionalResourceHelper mockResourceHelper; + + @Mock + private ChildAssociationRef mockChildRef; + + @Mock + private ChildAssociationRef mockParentRef; + + @Mock + private ChildAssociationRef mockOldRef; + + @Mock + private ChildAssociationRef mockNewRef; + + @Mock + private Set mockSet; + @InjectMocks private FrozenAspect frozenAspect; + private final List children = new ArrayList<>(); + private NodeRef record = new NodeRef("workspace://record/node"); private NodeRef folder = new NodeRef("workspace://folder/node"); private NodeRef content = new NodeRef("workspace://content/node"); + private NodeRef child = new NodeRef("workspace://content/child"); + private NodeRef parent = new NodeRef("workspace://content/parent"); @Before public void setUp() @@ -80,6 +115,11 @@ public class FrozenAspectUnitTest when(mockNodeService.hasAspect(folder, ASPECT_HELD_CHILDREN)).thenReturn(true); when(mockNodeService.getProperty(folder, PROP_HELD_CHILDREN_COUNT)).thenReturn(1); when(mockApplicationContext.getBean("dbNodeService")).thenReturn(mockNodeService); + when(mockNodeService.exists(content)).thenReturn(true); + when(mockFreezeService.isFrozen(content)).thenReturn(false); + children.add(mockChildRef); + when(mockNodeService.getChildAssocs(content)).thenReturn(children); + when(mockChildRef.isPrimary()).thenReturn(true); } /** @@ -121,4 +161,111 @@ public class FrozenAspectUnitTest frozenAspect.onRemoveAspect(content, null); verify(mockNodeService, times(0)).setProperty(folder, PROP_HELD_CHILDREN_COUNT, 0); } + + /** + * Test before delete throws an error if a node is frozen + */ + @Test(expected = AccessDeniedException.class) + public void testBeforeDeleteNodeThrowsExceptionIfNodeFrozen() + { + when(mockFreezeService.isFrozen(content)).thenReturn(true); + frozenAspect.beforeDeleteNode(content); + } + + /** + * Test before delete is fine for non-frozen nodes + */ + @Test + public void testBeforeDeleteForNonFrozenNodes() + { + frozenAspect.beforeDeleteNode(content); + verify(mockNodeService, times(1)).getChildAssocs(content); + verify(mockChildRef, times(1)).getChildRef(); + } + + /** + * Test before delete throws an error for a node with frozen children + */ + @Test (expected = AccessDeniedException.class) + public void testBeforeDeleteThrowsExceptionForFrozenChild() + { + when(mockChildRef.getChildRef()).thenReturn(child); + when(mockFreezeService.isFrozen(child)).thenReturn(true); + frozenAspect.beforeDeleteNode(content); + } + + /** + * Test on add aspect for a record node + */ + @Test + public void testOnAddAspectForRecord() + { + when(mockNodeService.getType(record)).thenReturn(ContentModel.TYPE_CONTENT); + when(mockNodeService.getPrimaryParent(record)).thenReturn(mockParentRef); + when(mockParentRef.getParentRef()).thenReturn(parent); + when(mockNodeService.hasAspect(parent, ASPECT_HELD_CHILDREN)).thenReturn(true); + when(mockNodeService.getProperty(parent, PROP_HELD_CHILDREN_COUNT)).thenReturn(0); + frozenAspect.onAddAspect(record,null); + verify(mockNodeService, times(1)).setProperty(parent, PROP_HELD_CHILDREN_COUNT,1); + } + + /** + * Test on add for a content node + */ + @Test + public void testOnAddAspectForContent() + { + when(mockNodeService.getType(record)).thenReturn(ContentModel.TYPE_CONTENT); + when(mockNodeService.getPrimaryParent(record)).thenReturn(mockParentRef); + when(mockParentRef.getParentRef()).thenReturn(parent); + when(mockNodeService.hasAspect(parent, ASPECT_HELD_CHILDREN)).thenReturn(false); + when(mockNodeService.getType(parent)).thenReturn(ContentModel.TYPE_FOLDER); + frozenAspect.onAddAspect(record, null); + verify(mockNodeService, times(1)).addAspect(any(NodeRef.class), any(QName.class), anyMap()); + } + + /** + * Test on move throws an error for a frozen node + */ + @Test(expected = AccessDeniedException.class) + public void testOnMoveThrowsExceptionForFrozenNode() + { + when(mockNewRef.getParentRef()).thenReturn(parent); + when(mockNewRef.getChildRef()).thenReturn(child); + when(mockNodeService.exists(parent)).thenReturn(true); + when(mockNodeService.exists(child)).thenReturn(true); + frozenAspect.onMoveNode(mockOldRef, mockNewRef); + } + + /** + * Test update properties throws an error for frozen nodes + */ + @Test(expected = AccessDeniedException.class) + public void testUpdatePropertiesThrowsExceptionForFrozenNode() + { + when(mockFreezeService.isFrozen(content)).thenReturn(true); + when(mockResourceHelper.getSet(content)).thenReturn(mockSet); + when(mockSet.contains("frozen")).thenReturn(false); + frozenAspect.onUpdateProperties(content, null, null); + } + + /** + * Test on content update throws an error for frozen nodes + */ + @Test(expected = AccessDeniedException.class) + public void testOnContentUpdateThrowsExceptionForFrozenNode() + { + when(mockFreezeService.isFrozen(content)).thenReturn(true); + frozenAspect.onContentUpdate(content, false); + } + + /** + * Test before copy throws an error for frozen node + */ + @Test(expected = AccessDeniedException.class) + public void testBeforeCopyThrowsExceptionForFrozenNode() + { + when(mockFreezeService.isFrozen(content)).thenReturn(true); + frozenAspect.beforeCopy(null, content, null); + } }