Merge remote-tracking branch 'remotes/origin/feature/RM-6904_PreventUpdateHeldActiveContent' into feature/RM-6914_AddRemoveToHold_Tests

rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java

@@ -231,6 +231,8 @@ public class HoldServiceImpl extends ServiceBaseImpl
             if (nodeService.hasAspect(nodeRef, ASPECT_FROZEN))
             {
                 // remove the freeze aspect from the node
+                //set in transaction cache in order not to trigger update policy when removing the aspect
+                transactionalResourceHelper.getSet(nodeRef).add("frozen");
                 nodeService.removeAspect(nodeRef, ASPECT_FROZEN);
             }
 
@@ -245,6 +247,8 @@ public class HoldServiceImpl extends ServiceBaseImpl
                         if (recordsOtherHolds.size() == index)
                         {
                             // remove the freeze aspect from the node
+                            //set in transaction cache in order not to trigger update policy when removing the aspect
+                            transactionalResourceHelper.getSet(record).add("frozen");
                             nodeService.removeAspect(record, ASPECT_FROZEN);
                         }
                     }
@@ -646,7 +650,7 @@ 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");
+            transactionalResourceHelper.getSet("frozen").add(nodeRef);
             // add freeze aspect
             nodeService.addAspect(nodeRef, ASPECT_FROZEN, props);
 
@@ -725,12 +729,12 @@ public class HoldServiceImpl extends ServiceBaseImpl
                 {
                     // run as system so we don't run into further permission issues
                     // we already know we have to have the correct capability to get here
-                    authenticationUtil.runAsSystem(new RunAsWork<Void>()
+                    authenticationUtil.runAsSystem((RunAsWork<Void>) () -> {
-                    {
-                        @Override
-                        public Void doWork()
                         {
+
                             // remove from hold
+                            //set in transaction cache in order not to trigger update policy when removing the child association
+                            transactionalResourceHelper.getSet(nodeRef).add("frozen");
                             nodeService.removeChild(hold, nodeRef);
 
                             // audit that the node has been remove from the hold
@@ -744,14 +748,9 @@ public class HoldServiceImpl extends ServiceBaseImpl
             }
 
             // run as system as we can't be sure if have remove aspect rights on node
-            authenticationUtil.runAsSystem(new RunAsWork<Void>()
+            authenticationUtil.runAsSystem((RunAsWork<Void>) () -> {
-            {
-                @Override
-                public Void doWork()
-                {
                 removeFreezeAspect(nodeRef, 0);
                 return null;
-                }
             });
         }
     }

rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/FrozenAspect.java

@@ -38,8 +38,6 @@ import java.util.Map;
 
 import org.alfresco.module.org_alfresco_module_rm.freeze.FreezeService;
 import org.alfresco.module.org_alfresco_module_rm.model.BaseBehaviourBean;
-import org.alfresco.repo.content.ContentServicePolicies;
-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;
@@ -67,9 +65,7 @@ public class FrozenAspect extends    BaseBehaviourBean
                                      NodeServicePolicies.OnAddAspectPolicy,
                                      NodeServicePolicies.OnRemoveAspectPolicy,
                                      NodeServicePolicies.OnUpdatePropertiesPolicy,
-                                     NodeServicePolicies.OnMoveNodePolicy,
+                                     NodeServicePolicies.BeforeMoveNodePolicy
-                                     ContentServicePolicies.OnContentUpdatePolicy,
-                                     CopyServicePolicies.BeforeCopyPolicy
 {
     /** freeze service */
     protected FreezeService freezeService;
@@ -209,14 +205,14 @@ public class FrozenAspect extends    BaseBehaviourBean
     @Override
     @Behaviour
             (
-                    kind = BehaviourKind.ASSOCIATION,
+                    kind = BehaviourKind.CLASS,
                     notificationFrequency = NotificationFrequency.FIRST_EVENT
             )
-    public void onMoveNode(final ChildAssociationRef oldChildAssocRef, final ChildAssociationRef newChildAssocRef)
+    public void beforeMoveNode(ChildAssociationRef oldChildAssocRef, NodeRef newParentRef)
     {
         AuthenticationUtil.runAsSystem((RunAsWork<Void>) () -> {
-            if (nodeService.exists(newChildAssocRef.getParentRef()) &&
+            if (nodeService.exists(oldChildAssocRef.getChildRef()) &&
-                    nodeService.exists(newChildAssocRef.getChildRef()))
+                    freezeService.isFrozen(oldChildAssocRef.getChildRef()))
             {
                 throw new AccessDeniedException("Frozen nodes can not be moved.");
             }
@@ -233,7 +229,7 @@ public class FrozenAspect extends    BaseBehaviourBean
     @Behaviour
             (
                     kind = BehaviourKind.CLASS,
-                    notificationFrequency = NotificationFrequency.TRANSACTION_COMMIT
+                    notificationFrequency = NotificationFrequency.FIRST_EVENT
             )
     public void onUpdateProperties(NodeRef nodeRef, Map<QName, Serializable> before, Map<QName, Serializable> after)
 
@@ -241,51 +237,11 @@ public class FrozenAspect extends    BaseBehaviourBean
         AuthenticationUtil.runAsSystem((RunAsWork<Void>) () -> {
             // check to not throw exception when the aspect is being added
             if (nodeService.exists(nodeRef) && freezeService.isFrozen(nodeRef) &&
-                    !transactionalResourceHelper.getSet(nodeRef).contains("frozen") )
+                    !transactionalResourceHelper.getSet("frozen").contains(nodeRef) )
                 {
                     throw new AccessDeniedException("Frozen nodes can not be updated.");
                 }
             return null;
         });
     }
-
-    /**
-     * Behaviour associated with updating the content
-     * <p>
-     * 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((RunAsWork<Void>) () -> {
-            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.runAsSystem((RunAsWork<Void>) () -> {
-            if (nodeService.exists(sourceNodeRef) && freezeService.isFrozen(sourceNodeRef))
-            {
-                throw new AccessDeniedException("Frozen nodes can not be copied.");
-            }
-
-            return null;
-        });
-    }
 }

rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/UpdateHeldActiveContentTest.java

@@ -0,0 +1,211 @@
+/*
+ * #%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 <http://www.gnu.org/licenses/>.
+ * #L%
+ */
+package org.alfresco.module.org_alfresco_module_rm.test.integration.hold;
+
+import org.alfresco.model.ContentModel;
+import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase;
+import org.alfresco.repo.content.MimetypeMap;
+import org.alfresco.repo.security.permissions.AccessDeniedException;
+import org.alfresco.service.cmr.model.FileNotFoundException;
+import org.alfresco.service.cmr.repository.ContentData;
+import org.alfresco.service.cmr.repository.NodeRef;
+import org.springframework.extensions.webscripts.GUID;
+
+/**
+ * Prevent Updating Held Active Content Integration Tests
+ *
+ * @author Claudia Agache
+ * @since 3.2
+ */
+public class UpdateHeldActiveContentTest extends BaseRMTestCase
+{
+    @Override
+    protected boolean isCollaborationSiteTest()
+    {
+        return true;
+    }
+
+    /**
+     * Given active content on hold
+     * When I try to delete the content
+     * Then I am not successful
+     */
+    public void testDeleteHeldDocument()
+    {
+        doBehaviourDrivenTest(new BehaviourDrivenTest()
+        {
+            public void given()
+            {
+                // create a hold
+                NodeRef hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate());
+
+                // add the active content to hold
+                holdService.addToHold(hold, dmDocument);
+            }
+
+            public void when()
+            {
+                try
+                {
+                    fileFolderService.delete(dmDocument);
+                    fail("Expected AccessDeniedException to be thrown");
+                }
+                catch (AccessDeniedException ade)
+                {
+                    assertTrue(ade.getMessage().contains("Frozen nodes can not be deleted."));
+                }
+            }
+        });
+    }
+
+    /**
+     * Given active content on hold
+     * When I try to copy the content
+     * Then I am not successful
+     */
+    public void testCopyHeldDocument()
+    {
+        doBehaviourDrivenTest(new BehaviourDrivenTest(AccessDeniedException.class)
+        {
+            public void given()
+            {
+                // create a hold
+                NodeRef hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate());
+
+                // add the active content to hold
+                holdService.addToHold(hold, dmDocument);
+            }
+
+            public void when() throws FileNotFoundException
+            {
+                fileFolderService.copy(dmDocument, dmFolder1, null);
+            }
+        });
+    }
+
+    /**
+     * Given active content on hold
+     * When I try to move the content
+     * Then I am not successful
+     */
+    public void testMoveHeldDocument()
+    {
+        doBehaviourDrivenTest(new BehaviourDrivenTest()
+        {
+            public void given()
+            {
+                // create a hold
+                NodeRef hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate());
+
+                // add the active content to hold
+                holdService.addToHold(hold, dmDocument);
+            }
+
+            public void when() throws FileNotFoundException
+            {
+                try
+                {
+                    fileFolderService.move(dmDocument, dmFolder1, null);
+                    fail("Expected AccessDeniedException to be thrown");
+                }
+                catch (AccessDeniedException ade)
+                {
+                    assertTrue(ade.getMessage().contains("Frozen nodes can not be moved."));
+                }
+            }
+        });
+    }
+
+    /**
+     * Given active content on hold
+     * When I try to edit the properties
+     * Or perform an action that edits the properties
+     * Then I am not successful
+     */
+    public void testUpdateHeldDocumentProperties()
+    {
+        doBehaviourDrivenTest(new BehaviourDrivenTest()
+        {
+            public void given()
+            {
+                // create a hold
+                NodeRef hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate());
+
+                // add the active content to hold
+                holdService.addToHold(hold, dmDocument);
+            }
+
+            public void when()
+            {
+                try
+                {
+                    nodeService.setProperty(dmDocument, ContentModel.PROP_DESCRIPTION, "description");
+                    fail("Expected AccessDeniedException to be thrown");
+                }
+                catch (AccessDeniedException ade)
+                {
+                    assertTrue(ade.getMessage().contains("Frozen nodes can not be updated."));
+                }
+            }
+        });
+    }
+
+    /**
+     * Given active content on hold
+     * When I try to update the content
+     * Then I am not successful
+     */
+    public void testUpdateHeldDocumentContent()
+    {
+        doBehaviourDrivenTest(new BehaviourDrivenTest()
+        {
+            public void given()
+            {
+                // create a hold
+                NodeRef hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate());
+
+                // add the active content to hold
+                holdService.addToHold(hold, dmDocument);
+            }
+
+            public void when()
+            {
+                try
+                {
+                    ContentData content = (ContentData) nodeService.getProperty(dmDocument, PROP_CONTENT);
+                    nodeService.setProperty(dmDocument, PROP_CONTENT, ContentData.setMimetype(content,
+                            MimetypeMap.MIMETYPE_TEXT_PLAIN));
+                    fail("Expected AccessDeniedException to be thrown");
+                }
+                catch (AccessDeniedException ade)
+                {
+                    assertTrue(ade.getMessage().contains("Frozen nodes can not be updated."));
+                }
+            }
+        });
+    }
+}