Merge branch 'master' of https://git.alfresco.com/records-management/records-management into feature/RM-7020_AllowUpdateOfPropertiesForSystemNamespaceURIRefactor

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

@@ -47,10 +47,14 @@ import org.alfresco.module.org_alfresco_module_rm.audit.event.AuditEvent;
 import org.alfresco.module.org_alfresco_module_rm.capability.CapabilityService;
 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.hold.HoldServicePolicies.BeforeAddToHoldPolicy;
 import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.BeforeCreateHoldPolicy;
 import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.BeforeDeleteHoldPolicy;
+import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.BeforeRemoveFromHoldPolicy;
+import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.OnAddToHoldPolicy;
 import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.OnCreateHoldPolicy;
 import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.OnDeleteHoldPolicy;
+import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.OnRemoveFromHoldPolicy;
 import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
 import org.alfresco.module.org_alfresco_module_rm.record.RecordService;
 import org.alfresco.module.org_alfresco_module_rm.recordfolder.RecordFolderService;
@@ -206,6 +210,10 @@ public class HoldServiceImpl extends ServiceBaseImpl
     private ClassPolicyDelegate<OnCreateHoldPolicy> onCreateHoldPolicyDelegate;
     private ClassPolicyDelegate<BeforeDeleteHoldPolicy> beforeDeleteHoldPolicyDelegate;
     private ClassPolicyDelegate<OnDeleteHoldPolicy> onDeleteHoldPolicyDelegate;
+    private ClassPolicyDelegate<BeforeAddToHoldPolicy> beforeAddToHoldPolicyDelegate;
+    private ClassPolicyDelegate<OnAddToHoldPolicy> onAddToHoldPolicyDelegate;
+    private ClassPolicyDelegate<BeforeRemoveFromHoldPolicy> beforeRemoveFromHoldPolicyDelegate;
+    private ClassPolicyDelegate<OnRemoveFromHoldPolicy> onRemoveFromHoldPolicyDelegate;
 
     /**
      * Initialise hold service
@@ -228,6 +236,11 @@ public class HoldServiceImpl extends ServiceBaseImpl
         onCreateHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(OnCreateHoldPolicy.class);
         beforeDeleteHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(BeforeDeleteHoldPolicy.class);
         onDeleteHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(OnDeleteHoldPolicy.class);
+        beforeAddToHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(BeforeAddToHoldPolicy.class);
+        onAddToHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(OnAddToHoldPolicy.class);
+        beforeRemoveFromHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(BeforeRemoveFromHoldPolicy.class);
+        onRemoveFromHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(OnRemoveFromHoldPolicy.class);
+
     }
 
     /**
@@ -637,6 +650,8 @@ public class HoldServiceImpl extends ServiceBaseImpl
             // check that the node isn't already in the hold
             if (!getHeld(hold).contains(nodeRef))
             {
+                // fire before add to hold policy
+                invokeBeforeAddToHold(hold, nodeRef);
                 // run as system to ensure we have all the appropriate permissions to perform the manipulations we require
                 authenticationUtil.runAsSystem((RunAsWork<Void>) () -> {
                     // gather freeze properties
@@ -661,6 +676,9 @@ public class HoldServiceImpl extends ServiceBaseImpl
                         records.forEach(record -> addFrozenAspect(record, props));
                     }
 
+                    // fire on add to hold policy
+                    invokeOnAddToHold(hold, nodeRef);
+
                     return null;
                 });
             }
@@ -797,6 +815,8 @@ 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((RunAsWork<Void>) () -> {
+                        // fire before remove from hold policy
+                        invokeBeforeRemoveFromHold(hold, nodeRef);
                         // remove from hold
                         //set in transaction cache in order not to trigger update policy when removing the child association
                         transactionalResourceHelper.getSet("frozen").add(nodeRef);
@@ -806,6 +826,9 @@ public class HoldServiceImpl extends ServiceBaseImpl
                         // TODO add details of the hold that the node was removed from
                         recordsManagementAuditService.auditEvent(nodeRef, AUDIT_REMOVE_FROM_HOLD);
 
+                        // fire on remove from hold policy
+                        invokeOnRemoveFromHold(hold, nodeRef);
+
                         return null;
 
                     });
@@ -916,4 +939,52 @@ public class HoldServiceImpl extends ServiceBaseImpl
 
     }
 
+    /**
+     * Invoke beforeAddToHold policy
+     *
+     * @param hold           hold node reference
+     * @param contentNodeRef content node reference
+     */
+    protected void invokeBeforeAddToHold(NodeRef hold, NodeRef contentNodeRef)
+    {
+        BeforeAddToHoldPolicy policy = beforeAddToHoldPolicyDelegate.get(getTypeAndApsects(hold));
+        policy.beforeAddToHold(hold, contentNodeRef);
+    }
+
+    /**
+     * Invoke onAddToHold policy
+     *
+     * @param hold           hold node reference
+     * @param contentNodeRef content node reference
+     */
+    protected void invokeOnAddToHold(NodeRef hold, NodeRef contentNodeRef)
+    {
+        OnAddToHoldPolicy policy = onAddToHoldPolicyDelegate.get(getTypeAndApsects(hold));
+        policy.onAddToHold(hold, contentNodeRef);
+    }
+
+    /**
+     * Invoke beforeRemoveFromHold policy
+     *
+     * @param hold           hold node reference
+     * @param contentNodeRef content node reference
+     */
+    protected void invokeBeforeRemoveFromHold(NodeRef hold, NodeRef contentNodeRef)
+    {
+        BeforeRemoveFromHoldPolicy policy = beforeRemoveFromHoldPolicyDelegate.get(getTypeAndApsects(hold));
+        policy.beforeRemoveFromHold(hold, contentNodeRef);
+    }
+
+    /**
+     * Invoke onRemoveFromHold policy
+     *
+     * @param hold           hold node reference
+     * @param contentNodeRef content node reference
+     */
+    protected void invokeOnRemoveFromHold(NodeRef hold, NodeRef contentNodeRef)
+    {
+        OnRemoveFromHoldPolicy policy = onRemoveFromHoldPolicyDelegate.get(getTypeAndApsects(hold));
+        policy.onRemoveFromHold(hold, contentNodeRef);
+
+    }
 }

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

@@ -36,6 +36,7 @@ import org.alfresco.service.namespace.QName;
  * Hold Service Policies
  *
  * @author Ramona Popa
+ * @author Roxana Lucanu
  * @since 3.3
  */
 
@@ -86,4 +87,57 @@ public interface HoldServicePolicies
          */
         void onDeleteHold(String holdname);
     }
+
+    interface BeforeAddToHoldPolicy extends ClassPolicy
+    {
+        QName QNAME = QName.createQName(NamespaceService.ALFRESCO_URI, "beforeAddToHold");
+
+        /**
+         * Called before adding content to hold.
+         *
+         * @param hold           the hold to be added into
+         * @param contentNodeRef the item to be added to hold
+         */
+        void beforeAddToHold(NodeRef hold, NodeRef contentNodeRef);
+    }
+
+    interface OnAddToHoldPolicy extends ClassPolicy
+    {
+        QName QNAME = QName.createQName(NamespaceService.ALFRESCO_URI, "onAddToHold");
+
+        /**
+         * Called when content is added to hold.
+         *
+         * @param hold           the hold to be added into
+         * @param contentNodeRef the item to be added to hold
+         */
+        void onAddToHold(NodeRef hold, NodeRef contentNodeRef);
+    }
+
+    interface BeforeRemoveFromHoldPolicy extends ClassPolicy
+    {
+        QName QNAME = QName.createQName(NamespaceService.ALFRESCO_URI, "beforeRemoveFromHold");
+
+        /**
+         * Called before removing content from hold.
+         *
+         * @param hold           the hold to be removed from
+         * @param contentNodeRef the item to be removed from hold
+         */
+        void beforeRemoveFromHold(NodeRef hold, NodeRef contentNodeRef);
+    }
+
+    interface OnRemoveFromHoldPolicy extends ClassPolicy
+    {
+        QName QNAME = QName.createQName(NamespaceService.ALFRESCO_URI, "onRemoveFromHold");
+
+        /**
+         * Called when removing content from hold.
+         *
+         * @param hold           the hold to be removed from
+         * @param contentNodeRef the item to be removed from hold
+         */
+        void onRemoveFromHold(NodeRef hold, NodeRef contentNodeRef);
+    }
+
 }

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

@@ -24,109 +24,119 @@
  * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
  * #L%
  */
-
 package org.alfresco.module.org_alfresco_module_rm.test.integration.hold;
 
 import java.util.ArrayList;
 import java.util.List;
 
 import org.alfresco.model.ContentModel;
+import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.BeforeAddToHoldPolicy;
+import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.OnAddToHoldPolicy;
+import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
 import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase;
+import org.alfresco.repo.policy.Behaviour.NotificationFrequency;
+import org.alfresco.repo.policy.BehaviourDefinition;
+import org.alfresco.repo.policy.ClassBehaviourBinding;
+import org.alfresco.repo.policy.JavaBehaviour;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.springframework.extensions.webscripts.GUID;
 
 /**
- * Hold service integration test.
+ * Add To Hold Integration Tests
  *
  * @author Roy Wetherall
  * @since 2.2
  */
-public class AddRemoveFromHoldTest extends BaseRMTestCase
+
+public class AddToHoldTest extends BaseRMTestCase implements BeforeAddToHoldPolicy, OnAddToHoldPolicy
 {
     private static final int RECORD_COUNT = 10;
 
+    private boolean beforeAddToHoldFlag = false;
+    private boolean onAddToHoldFlag = false;
+
     public void testAddRecordToHold()
-    { 
+    {
         doBehaviourDrivenTest(new BehaviourDrivenTest()
         {
             private NodeRef hold;
             private NodeRef recordCategory;
             private NodeRef recordFolder;
             private NodeRef record;
-            
+
             public void given()
-            {   
+            {
                 // create a hold
                 hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate());
-                
-                // create a record folder that contains records  
+
+                // create a record folder that contains records
                 recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate());
                 recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate());
                 record = recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null);
-                
+
                 // assert current states
                 assertFalse(freezeService.isFrozen(recordFolder));
                 assertFalse(freezeService.isFrozen(record));
                 assertFalse(freezeService.hasFrozenChildren(recordFolder));
-                
+
                 // additional check for child held caching
                 assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN));
                 assertEquals(0, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT));
             }
-            
+
             public void when() throws Exception
-            {      
+            {
                 // add the record to hold
                 holdService.addToHold(hold, record);
             }
-            
+
             public void then()
             {
-                // record is held    
+                // record is held
                 assertTrue(freezeService.isFrozen(record));
-                
+
                 // record folder has frozen children
                 assertFalse(freezeService.isFrozen(recordFolder));
                 assertTrue(freezeService.hasFrozenChildren(recordFolder));
-                
+
                 // record folder is not held
                 assertFalse(holdService.getHeld(hold).contains(recordFolder));
                 assertFalse(holdService.heldBy(recordFolder, true).contains(hold));
-                
+
                 // hold contains record
                 assertTrue(holdService.getHeld(hold).contains(record));
                 assertTrue(holdService.heldBy(record, true).contains(hold));
-                
+
                 // additional check for child held caching
                 assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN));
                 assertEquals(1, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT));
-            }            
+            }
         });
-        
+
     }
-    
+
     public void testAddRecordsToHold()
-    { 
+    {
         doBehaviourDrivenTest(new BehaviourDrivenTest()
-        {   
+        {
             private NodeRef hold;
             private NodeRef recordCategory;
             private NodeRef recordFolder;
             private List<NodeRef> records = new ArrayList<>(RECORD_COUNT);
-            
+
             public void given()
-            {   
+            {
                 // create a hold
                 hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate());
-                
-                // create a record folder that contains records  
+
+                // create a record folder that contains records
                 recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate());
                 recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate());
                 for (int i = 0; i < RECORD_COUNT; i++)
                 {
                     records.add(recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null));
                 }
-                
+
                 // assert current states
                 assertFalse(freezeService.isFrozen(recordFolder));
                 assertFalse(freezeService.hasFrozenChildren(recordFolder));
@@ -134,70 +144,70 @@ public class AddRemoveFromHoldTest extends BaseRMTestCase
                 {
                     assertFalse(freezeService.isFrozen(record));
                 }
-                
+
                 // additional check for child held caching
                 assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN));
                 assertEquals(0, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT));
             }
-            
+
             public void when() throws Exception
-            {      
+            {
                 // add the record to hold
                 holdService.addToHold(hold, records);
             }
-            
+
             public void then()
             {
-                // record is held    
+                // record is held
                 for (NodeRef record : records)
-                {                
+                {
                     assertTrue(freezeService.isFrozen(record));
                 }
-                
+
                 // record folder has frozen children
                 assertFalse(freezeService.isFrozen(recordFolder));
                 assertTrue(freezeService.hasFrozenChildren(recordFolder));
-                
+
                 // record folder is not held
                 assertFalse(holdService.getHeld(hold).contains(recordFolder));
                 assertFalse(holdService.heldBy(recordFolder, true).contains(hold));
-                
+
                 for (NodeRef record : records)
-                {                                    
+                {
                     // hold contains record
                     assertTrue(holdService.getHeld(hold).contains(record));
                     assertTrue(holdService.heldBy(record, true).contains(hold));
                 }
-                
+
                 // additional check for child held caching
                 assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN));
                 assertEquals(RECORD_COUNT, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT));
-            }            
-        });        
+            }
+        });
     }
-    
+
     public void testAddRecordFolderToHold()
     {
         doBehaviourDrivenTest(new BehaviourDrivenTest()
-        {   
+        {
             private NodeRef hold;
             private NodeRef recordCategory;
             private NodeRef recordFolder;
             private List<NodeRef> records = new ArrayList<>(RECORD_COUNT);
-            
+
             public void given()
-            {   
+            {
                 // create a hold
                 hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate());
-                
-                // create a record folder that contains records  
+
+                // create a record folder that contains records
                 recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate());
                 recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate());
                 for (int i = 0; i < RECORD_COUNT; i++)
                 {
                     records.add(recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null));
                 }
-                
+
                 // assert current states
                 assertFalse(freezeService.isFrozen(recordFolder));
                 assertFalse(freezeService.hasFrozenChildren(recordFolder));
@@ -205,220 +215,101 @@ public class AddRemoveFromHoldTest extends BaseRMTestCase
                 {
                     assertFalse(freezeService.isFrozen(record));
                 }
-                
+
                 // additional check for child held caching
                 assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN));
                 assertEquals(0, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT));
             }
-            
+
             public void when() throws Exception
-            {                      
+            {
                 // add the record to hold
                 holdService.addToHold(hold, recordFolder);
             }
-            
+
             public void then()
             {
                 for (NodeRef record : records)
                 {
-                    // record is held    
+                    // record is held
                     assertTrue(freezeService.isFrozen(record));
                     assertFalse(holdService.getHeld(hold).contains(record));
                     assertTrue(holdService.heldBy(record, true).contains(hold));
                 }
-                
+
                 // record folder has frozen children
                 assertTrue(freezeService.isFrozen(recordFolder));
                 assertTrue(freezeService.hasFrozenChildren(recordFolder));
-                
+
                 // hold contains record folder
                 assertTrue(holdService.getHeld(hold).contains(recordFolder));
                 assertTrue(holdService.heldBy(recordFolder, true).contains(hold));
-                
+
                 // additional check for child held caching
                 assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN));
                 assertEquals(RECORD_COUNT, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT));
-            }            
+            }
         });
-        
+
     }
-    
-    public void testRemoveRecordsFromHold()
+
+    public void testPolicyNotificationForAddToHold()
     {
         doBehaviourDrivenTest(new BehaviourDrivenTest()
-        {   
+        {
             private NodeRef hold;
             private NodeRef recordCategory;
             private NodeRef recordFolder;
-            private List<NodeRef> records = new ArrayList<>(RECORD_COUNT);
-            
+            BehaviourDefinition<ClassBehaviourBinding> beforeAddToHoldBehaviour;
+            BehaviourDefinition<ClassBehaviourBinding> onAddToHoldBehaviour;
+
             public void given()
-            {   
-                // create a hold
-                hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate());
-                
-                // create a record folder that contains records  
-                recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate());
-                recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate());
-                for (int i = 0; i < RECORD_COUNT; i++)
-                {
-                    records.add(recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null));
-                }
-                
-                // add records to hold
-                holdService.addToHold(hold, records);
-            }
-            
-            public void when() throws Exception
-            {      
-                // remove *some* of the records
-                holdService.removeFromHold(hold, records.subList(0, 5));
-            }
-            
-            public void then()
             {
-                // check record state (no longer held)   
-                for (NodeRef record : records.subList(0, 5))
-                {                
-                    assertFalse(freezeService.isFrozen(record));
-                    assertFalse(holdService.getHeld(hold).contains(record));
-                    assertFalse(holdService.heldBy(record, true).contains(hold));
-                }
-                
-                // check record state (still held)   
-                for (NodeRef record : records.subList(5, 10))
-                {                
-                    assertTrue(freezeService.isFrozen(record));
-                    assertTrue(holdService.getHeld(hold).contains(record));
-                    assertTrue(holdService.heldBy(record, true).contains(hold));
-                }
-                
-                // record folder has frozen children
-                assertFalse(freezeService.isFrozen(recordFolder));
-                assertTrue(freezeService.hasFrozenChildren(recordFolder));
-                
-                // record folder is not held
-                assertFalse(holdService.getHeld(hold).contains(recordFolder));
-                assertFalse(holdService.heldBy(recordFolder, true).contains(hold));
-                
-                // additional check for child held caching
-                assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN));
-                assertEquals(5, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT));
-            }            
-        });         
-    }
-    
-    public void testRemoveAllRecordsFromHold()
-    {
-        doBehaviourDrivenTest(new BehaviourDrivenTest()
-        {   
-            private NodeRef hold;
-            private NodeRef recordCategory;
-            private NodeRef recordFolder;
-            private List<NodeRef> records = new ArrayList<>(RECORD_COUNT);
-            
-            public void given()
-            {   
                 // create a hold
                 hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate());
-                
-                // create a record folder that contains records  
+                // create a record category -> record folder
                 recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate());
                 recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate());
-                for (int i = 0; i < RECORD_COUNT; i++)
-                {
-                    records.add(recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null));
-                }
-                
-                // add records to hold
-                holdService.addToHold(hold, records);
+
+                beforeAddToHoldBehaviour = policyComponent.bindClassBehaviour(BeforeAddToHoldPolicy.QNAME,
+                        RecordsManagementModel.TYPE_HOLD, new JavaBehaviour(AddToHoldTest.this, "beforeAddToHold", NotificationFrequency.EVERY_EVENT));
+
+                onAddToHoldBehaviour = policyComponent.bindClassBehaviour(OnAddToHoldPolicy.QNAME,
+                        RecordsManagementModel.TYPE_HOLD, new JavaBehaviour(AddToHoldTest.this, "onAddToHold", NotificationFrequency.EVERY_EVENT));
+
+                assertFalse(beforeAddToHoldFlag);
+                assertFalse(onAddToHoldFlag);
             }
-            
+
             public void when() throws Exception
-            {      
-                // remove all of the records
-                holdService.removeFromHold(hold, records);
-            }
-            
-            public void then()
             {
-                // check record state (no longer held)   
-                for (NodeRef record : records)
-                {                
-                    assertFalse(freezeService.isFrozen(record));
-                    assertFalse(holdService.getHeld(hold).contains(record));
-                    assertFalse(holdService.heldBy(record, true).contains(hold));
-                }
-                
-                // record folder has frozen children
-                assertFalse(freezeService.isFrozen(recordFolder));
-                assertFalse(freezeService.hasFrozenChildren(recordFolder));
-                
-                // record folder is not held
-                assertFalse(holdService.getHeld(hold).contains(recordFolder));
-                assertFalse(holdService.heldBy(recordFolder, true).contains(hold));
-                
-                // additional check for child held caching
-                assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN));
-                assertEquals(0, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT));
-            }            
-        });         
-    }
-    
-    public void testRemoveRecordFolderFromHold()
-    {
-        doBehaviourDrivenTest(new BehaviourDrivenTest()
-        {   
-            private NodeRef hold;
-            private NodeRef recordCategory;
-            private NodeRef recordFolder;
-            private List<NodeRef> records = new ArrayList<>(RECORD_COUNT);
-            
-            public void given()
-            {   
-                // create a hold
-                hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate());
-                
-                // create a record folder that contains records  
-                recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate());
-                recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate());
-                for (int i = 0; i < RECORD_COUNT; i++)
-                {
-                    records.add(recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null));
-                }
-                
-                // add record folder to hold
+                // add the record folder to hold
                 holdService.addToHold(hold, recordFolder);
             }
-            
-            public void when() throws Exception
-            {      
-                // remove record folder from hold
-                holdService.removeFromHold(hold, recordFolder);
-            }
-            
+
             public void then()
             {
-                // check record states   
-                for (NodeRef record : records)
-                {                
-                    assertFalse(freezeService.isFrozen(record));
-                    assertFalse(holdService.getHeld(hold).contains(record));
-                    assertFalse(holdService.heldBy(record, true).contains(hold));
-                }
-                
-                // record folder has frozen children
-                assertFalse(freezeService.isFrozen(recordFolder));
-                assertFalse(freezeService.hasFrozenChildren(recordFolder));
-                
-                // record folder is not held
-                assertFalse(holdService.getHeld(hold).contains(recordFolder));
-                assertFalse(holdService.heldBy(recordFolder, true).contains(hold));
-                
-                // additional check for child held caching
-                assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN));
-                assertEquals(0, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT));
-            }            
-        });         
+                assertTrue(beforeAddToHoldFlag);
+                assertTrue(onAddToHoldFlag);
+            }
+
+            public void after()
+            {
+                policyComponent.removeClassDefinition(beforeAddToHoldBehaviour);
+                policyComponent.removeClassDefinition(onAddToHoldBehaviour);
+            }
+        });
+    }
+
+    @Override
+    public void beforeAddToHold(NodeRef hold, NodeRef contentNodeRef)
+    {
+        beforeAddToHoldFlag = true;
+    }
+
+    @Override
+    public void onAddToHold(NodeRef hold, NodeRef contentNodeRef)
+    {
+        onAddToHoldFlag = true;
     }
 }

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

@@ -0,0 +1,298 @@
+/*
+ * #%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 java.util.ArrayList;
+import java.util.List;
+
+import org.alfresco.model.ContentModel;
+import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.BeforeRemoveFromHoldPolicy;
+import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.OnRemoveFromHoldPolicy;
+import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
+import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase;
+import org.alfresco.repo.policy.Behaviour.NotificationFrequency;
+import org.alfresco.repo.policy.BehaviourDefinition;
+import org.alfresco.repo.policy.ClassBehaviourBinding;
+import org.alfresco.repo.policy.JavaBehaviour;
+import org.alfresco.service.cmr.repository.NodeRef;
+import org.springframework.extensions.webscripts.GUID;
+
+/**
+ * Remove From Hold integration tests.
+ *
+ * @author Roy Wetherall
+ * @since 2.2
+ */
+public class RemoveFromHoldTest extends BaseRMTestCase implements BeforeRemoveFromHoldPolicy, OnRemoveFromHoldPolicy
+{
+    private static final int RECORD_COUNT = 10;
+
+    private boolean beforeRemoveFromHoldFlag = false;
+    private boolean onRemoveFromHoldFlag = false;
+
+    public void testRemoveRecordsFromHold()
+    {
+        doBehaviourDrivenTest(new BehaviourDrivenTest()
+        {
+            private NodeRef hold;
+            private NodeRef recordCategory;
+            private NodeRef recordFolder;
+            private List<NodeRef> records = new ArrayList<>(RECORD_COUNT);
+
+            public void given()
+            {
+                // create a hold
+                hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate());
+
+                // create a record folder that contains records  
+                recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate());
+                recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate());
+                for (int i = 0; i < RECORD_COUNT; i++)
+                {
+                    records.add(recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null));
+                }
+
+                // add records to hold
+                holdService.addToHold(hold, records);
+            }
+
+            public void when() throws Exception
+            {
+                // remove *some* of the records
+                holdService.removeFromHold(hold, records.subList(0, 5));
+            }
+
+            public void then()
+            {
+                // check record state (no longer held)   
+                for (NodeRef record : records.subList(0, 5))
+                {
+                    assertFalse(freezeService.isFrozen(record));
+                    assertFalse(holdService.getHeld(hold).contains(record));
+                    assertFalse(holdService.heldBy(record, true).contains(hold));
+                }
+
+                // check record state (still held)   
+                for (NodeRef record : records.subList(5, 10))
+                {
+                    assertTrue(freezeService.isFrozen(record));
+                    assertTrue(holdService.getHeld(hold).contains(record));
+                    assertTrue(holdService.heldBy(record, true).contains(hold));
+                }
+
+                // record folder has frozen children
+                assertFalse(freezeService.isFrozen(recordFolder));
+                assertTrue(freezeService.hasFrozenChildren(recordFolder));
+
+                // record folder is not held
+                assertFalse(holdService.getHeld(hold).contains(recordFolder));
+                assertFalse(holdService.heldBy(recordFolder, true).contains(hold));
+
+                // additional check for child held caching
+                assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN));
+                assertEquals(5, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT));
+            }
+        });
+    }
+
+    public void testRemoveAllRecordsFromHold()
+    {
+        doBehaviourDrivenTest(new BehaviourDrivenTest()
+        {
+            private NodeRef hold;
+            private NodeRef recordCategory;
+            private NodeRef recordFolder;
+            private List<NodeRef> records = new ArrayList<>(RECORD_COUNT);
+
+            public void given()
+            {
+                // create a hold
+                hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate());
+
+                // create a record folder that contains records  
+                recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate());
+                recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate());
+                for (int i = 0; i < RECORD_COUNT; i++)
+                {
+                    records.add(recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null));
+                }
+
+                // add records to hold
+                holdService.addToHold(hold, records);
+            }
+
+            public void when() throws Exception
+            {
+                // remove all of the records
+                holdService.removeFromHold(hold, records);
+            }
+
+            public void then()
+            {
+                // check record state (no longer held)   
+                for (NodeRef record : records)
+                {
+                    assertFalse(freezeService.isFrozen(record));
+                    assertFalse(holdService.getHeld(hold).contains(record));
+                    assertFalse(holdService.heldBy(record, true).contains(hold));
+                }
+
+                // record folder has frozen children
+                assertFalse(freezeService.isFrozen(recordFolder));
+                assertFalse(freezeService.hasFrozenChildren(recordFolder));
+
+                // record folder is not held
+                assertFalse(holdService.getHeld(hold).contains(recordFolder));
+                assertFalse(holdService.heldBy(recordFolder, true).contains(hold));
+
+                // additional check for child held caching
+                assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN));
+                assertEquals(0, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT));
+            }
+        });
+    }
+
+    public void testRemoveRecordFolderFromHold()
+    {
+        doBehaviourDrivenTest(new BehaviourDrivenTest()
+        {
+            private NodeRef hold;
+            private NodeRef recordCategory;
+            private NodeRef recordFolder;
+            private List<NodeRef> records = new ArrayList<>(RECORD_COUNT);
+
+            public void given()
+            {
+                // create a hold
+                hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate());
+
+                // create a record folder that contains records  
+                recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate());
+                recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate());
+                for (int i = 0; i < RECORD_COUNT; i++)
+                {
+                    records.add(recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null));
+                }
+
+                // add record folder to hold
+                holdService.addToHold(hold, recordFolder);
+            }
+
+            public void when() throws Exception
+            {
+                // remove record folder from hold
+                holdService.removeFromHold(hold, recordFolder);
+            }
+
+            public void then()
+            {
+                // check record states   
+                for (NodeRef record : records)
+                {
+                    assertFalse(freezeService.isFrozen(record));
+                    assertFalse(holdService.getHeld(hold).contains(record));
+                    assertFalse(holdService.heldBy(record, true).contains(hold));
+                }
+
+                // record folder has frozen children
+                assertFalse(freezeService.isFrozen(recordFolder));
+                assertFalse(freezeService.hasFrozenChildren(recordFolder));
+
+                // record folder is not held
+                assertFalse(holdService.getHeld(hold).contains(recordFolder));
+                assertFalse(holdService.heldBy(recordFolder, true).contains(hold));
+
+                // additional check for child held caching
+                assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN));
+                assertEquals(0, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT));
+            }
+        });
+    }
+
+    public void testPolicyNotificationForRemoveFromHold()
+    {
+        doBehaviourDrivenTest(new BehaviourDrivenTest()
+        {
+            private NodeRef hold;
+            private NodeRef recordCategory;
+            private NodeRef recordFolder;
+            BehaviourDefinition<ClassBehaviourBinding> beforeRemoveFromHoldBehaviour;
+            BehaviourDefinition<ClassBehaviourBinding> onRemoveFromHoldBehaviour;
+
+            public void given()
+            {
+                // create a hold
+                hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate());
+                // create a record category -> record folder
+                recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate());
+                recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate());
+                // add the record folder to hold
+                holdService.addToHold(hold, recordFolder);
+
+                beforeRemoveFromHoldBehaviour = policyComponent.bindClassBehaviour(BeforeRemoveFromHoldPolicy.QNAME,
+                        RecordsManagementModel.TYPE_HOLD, new JavaBehaviour(RemoveFromHoldTest.this, "beforeRemoveFromHold", NotificationFrequency.EVERY_EVENT));
+
+                onRemoveFromHoldBehaviour = policyComponent.bindClassBehaviour(OnRemoveFromHoldPolicy.QNAME,
+                        RecordsManagementModel.TYPE_HOLD, new JavaBehaviour(RemoveFromHoldTest.this, "onRemoveFromHold", NotificationFrequency.EVERY_EVENT));
+
+                assertFalse(beforeRemoveFromHoldFlag);
+                assertFalse(onRemoveFromHoldFlag);
+            }
+
+            public void when() throws Exception
+            {
+                // remove the record folder from the hold
+                holdService.removeFromHold(hold, recordFolder);
+            }
+
+            public void then()
+            {
+                assertTrue(beforeRemoveFromHoldFlag);
+                assertTrue(onRemoveFromHoldFlag);
+            }
+
+            public void after()
+            {
+                policyComponent.removeClassDefinition(beforeRemoveFromHoldBehaviour);
+                policyComponent.removeClassDefinition(onRemoveFromHoldBehaviour);
+            }
+        });
+    }
+
+    @Override
+    public void beforeRemoveFromHold(NodeRef hold, NodeRef contentNodeRef)
+    {
+        beforeRemoveFromHoldFlag = true;
+    }
+
+    @Override
+    public void onRemoveFromHold(NodeRef hold, NodeRef contentNodeRef)
+    {
+        onRemoveFromHoldFlag = true;
+    }
+}

rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java

@@ -71,7 +71,6 @@ import org.junit.Before;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
 import org.mockito.InjectMocks;
-import org.mockito.Matchers;
 import org.mockito.Mock;
 import org.mockito.Spy;
 import org.mockito.invocation.InvocationOnMock;
@@ -340,6 +339,8 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
     @Test
     public void addToHoldNotInHold()
     {
+        mockPoliciesForAddToHold();
+
         holdService.addToHold(hold, recordFolder);
 
         verify(mockedNodeService).addChild(hold, recordFolder, ASSOC_FROZEN_CONTENT, ASSOC_FROZEN_CONTENT);
@@ -387,6 +388,8 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
         doReturn(true).when(mockedNodeService).hasAspect(record, ASPECT_FROZEN);
         doReturn(true).when(mockedNodeService).hasAspect(activeContent, ASPECT_FROZEN);
 
+        mockPoliciesForAddToHold();
+
         holdService.addToHold(hold, recordFolder);
 
         verify(mockedNodeService).addChild(hold, recordFolder, ASSOC_FROZEN_CONTENT, ASSOC_FROZEN_CONTENT);
@@ -448,6 +451,8 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
 
         }).when(mockedNodeService).addAspect(any(NodeRef.class), eq(ASPECT_FROZEN), any(Map.class));
 
+        mockPoliciesForAddToHold();
+
         // build a list of holds
         List<NodeRef> holds = new ArrayList<>(2);
         holds.add(hold);
@@ -473,6 +478,8 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
     @Test
     public void removeFromHoldNotInHold()
     {
+        mockPoliciesForRemoveFromHold();
+
         holdService.removeFromHold(hold, recordFolder);
 
         verify(mockedNodeService, never()).removeChild(hold, recordFolder);
@@ -488,6 +495,8 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
         doReturn(true).when(mockedNodeService).hasAspect(recordFolder, ASPECT_FROZEN);
         doReturn(true).when(mockedNodeService).hasAspect(record, ASPECT_FROZEN);
 
+        mockPoliciesForRemoveFromHold();
+
         holdService.removeFromHold(hold, recordFolder);
 
         verify(mockedNodeService, times(1)).removeChild(hold, recordFolder);
@@ -504,6 +513,8 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
         doReturn(true).when(mockedNodeService).hasAspect(recordFolder, ASPECT_FROZEN);
         doReturn(true).when(mockedNodeService).hasAspect(record, ASPECT_FROZEN);
 
+        mockPoliciesForRemoveFromHold();
+
         // build a list of holds
         List<NodeRef> holds = new ArrayList<>(2);
         holds.add(hold);
@@ -536,6 +547,8 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
 
         }).when(mockedNodeService).removeChild(hold, recordFolder);
 
+        mockPoliciesForRemoveFromHold();
+
         doAnswer(new Answer<Void>()
         {
             public Void answer(InvocationOnMock invocation)
@@ -576,4 +589,22 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
         holds.add(hold2);
         holdService.removeFromHolds(holds, activeContent);
     }
+
+    /**
+     * mocks policies for add to hold
+     */
+    private void mockPoliciesForAddToHold()
+    {
+        doNothing().when(holdService).invokeBeforeAddToHold(any(), any());
+        doNothing().when(holdService).invokeOnAddToHold(any(), any());
+    }
+
+    /**
+     * mocks policies for remove from hold
+     */
+    private void mockPoliciesForRemoveFromHold()
+    {
+        doNothing().when(holdService).invokeBeforeRemoveFromHold(any(), any());
+        doNothing().when(holdService).invokeOnRemoveFromHold(any(), any());
+    }
 }