RM-7035: Add policies for holds

- some minor formating
   - fix unit test

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

@@ -41,16 +41,19 @@ import java.util.Set;
 
 import org.alfresco.error.AlfrescoRuntimeException;
 import org.alfresco.model.ContentModel;
-import org.alfresco.module.org_alfresco_module_rm.RecordsManagementPolicies;
+import org.alfresco.module.org_alfresco_module_rm.RecordsManagementPolicies.BeforeFileRecord;
 import org.alfresco.module.org_alfresco_module_rm.audit.RecordsManagementAuditService;
 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.BeforeCreateHoldPolicy;
+import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.BeforeDeleteHoldPolicy;
+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.model.RecordsManagementModel;
 import org.alfresco.module.org_alfresco_module_rm.record.RecordService;
 import org.alfresco.module.org_alfresco_module_rm.recordfolder.RecordFolderService;
-import org.alfresco.module.org_alfresco_module_rm.util.PoliciesUtil;
 import org.alfresco.module.org_alfresco_module_rm.util.ServiceBaseImpl;
 import org.alfresco.repo.node.NodeServicePolicies;
 import org.alfresco.repo.node.integrity.IntegrityException;
@@ -181,7 +184,7 @@ public class HoldServiceImpl extends ServiceBaseImpl
      *
      * @return The policy component instance
      */
-    private PolicyComponent getPolicyComponent()
+    protected PolicyComponent getPolicyComponent()
     {
         return this.policyComponent;
     }
@@ -199,10 +202,10 @@ public class HoldServiceImpl extends ServiceBaseImpl
     /**
      * Policy delegates
      */
-    private ClassPolicyDelegate<HoldServicePolicies.BeforeCreateHoldPolicy> beforeCreateHoldPolicyDelegate;
+    private ClassPolicyDelegate<BeforeCreateHoldPolicy> beforeCreateHoldPolicyDelegate;
-    private ClassPolicyDelegate<HoldServicePolicies.OnCreateHoldPolicy> onCreateHoldPolicyDelegate;
+    private ClassPolicyDelegate<OnCreateHoldPolicy> onCreateHoldPolicyDelegate;
-    private ClassPolicyDelegate<HoldServicePolicies.BeforeDeleteHoldPolicy> beforeDeleteHoldPolicyDelegate;
+    private ClassPolicyDelegate<BeforeDeleteHoldPolicy> beforeDeleteHoldPolicyDelegate;
-    private ClassPolicyDelegate<HoldServicePolicies.OnDeleteHoldPolicy> onDeleteHoldPolicyDelegate;
+    private ClassPolicyDelegate<OnDeleteHoldPolicy> onDeleteHoldPolicyDelegate;
 
     /**
      * Initialise hold service
@@ -221,10 +224,10 @@ public class HoldServiceImpl extends ServiceBaseImpl
         });
 
         // Register the policies
-        beforeCreateHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(HoldServicePolicies.BeforeCreateHoldPolicy.class);
+        beforeCreateHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(BeforeCreateHoldPolicy.class);
-        onCreateHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(HoldServicePolicies.OnCreateHoldPolicy.class);
+        onCreateHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(OnCreateHoldPolicy.class);
-        beforeDeleteHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(HoldServicePolicies.BeforeDeleteHoldPolicy.class);
+        beforeDeleteHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(BeforeDeleteHoldPolicy.class);
-        onDeleteHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(HoldServicePolicies.OnDeleteHoldPolicy.class);
+        onDeleteHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(OnDeleteHoldPolicy.class);
     }
 
     /**
@@ -458,8 +461,7 @@ public class HoldServiceImpl extends ServiceBaseImpl
         // get the root hold container
         NodeRef holdContainer = filePlanService.getHoldContainer(filePlan);
 
-        //invoke before create hold
+        invokeBeforeCreateHold(holdContainer, name, reason);
-        beforeCreateHoldPolicyDelegate.get(getTypeAndApsects(holdContainer)).beforeCreateHold(name, reason);
 
         // create map of properties
         Map<QName, Serializable> properties = new HashMap<>(3);
@@ -478,8 +480,7 @@ public class HoldServiceImpl extends ServiceBaseImpl
 
         NodeRef holdNodeRef = childAssocRef.getChildRef();
 
-        //invoke after hold is created
+        invokeOnCreateHold(holdNodeRef);
-        onCreateHoldPolicyDelegate.get(getTypeAndApsects(holdNodeRef)).onCreateHold(holdNodeRef);
 
         return holdNodeRef;
     }
@@ -569,12 +570,11 @@ public class HoldServiceImpl extends ServiceBaseImpl
             throw new AlfrescoRuntimeException("Can't delete hold, because filing permissions for the following items are needed: " + sb.toString());
         }
 
-        beforeDeleteHoldPolicyDelegate.get(getTypeAndApsects(hold)).beforeDeleteHold(hold);
+        invokeBeforeDeleteHold(hold);
-
         // delete the hold node
         nodeService.deleteNode(hold);
 
-        onDeleteHoldPolicyDelegate.get(getTypeAndApsects(hold)).onDeleteHold(hold);
+        invokeOnDeleteHold(hold);
     }
 
     /**
@@ -862,4 +862,52 @@ public class HoldServiceImpl extends ServiceBaseImpl
             removeFromAllHolds(nodeRef);
         }
     }
+
+    /**
+     * Invoke beforeCreateHold policy
+     *
+     * @param nodeRef node reference
+     * @param name    hold name
+     * @param reason  hold reason
+     */
+    protected void invokeBeforeCreateHold(NodeRef nodeRef, String name, String reason)
+    {
+        // execute policy for node type and aspects
+        BeforeCreateHoldPolicy policy = beforeCreateHoldPolicyDelegate.get(getTypeAndApsects(nodeRef));
+        policy.beforeCreateHold(name, reason);
+    }
+
+    /**
+     * Invoke onCreateHold policy
+     *
+     * @param nodeRef node reference
+     */
+    protected void invokeOnCreateHold(NodeRef nodeRef)
+    {
+        OnCreateHoldPolicy policy = onCreateHoldPolicyDelegate.get(getTypeAndApsects(nodeRef));
+        policy.onCreateHold(nodeRef);
+    }
+
+    /**
+     * Invoke beforeDeleteHold policy
+     *
+     * @param nodeRef node reference
+     */
+    protected void invokeBeforeDeleteHold(NodeRef nodeRef)
+    {
+        BeforeDeleteHoldPolicy policy = beforeDeleteHoldPolicyDelegate.get(getTypeAndApsects(nodeRef));
+        policy.beforeDeleteHold(nodeRef);
+    }
+
+    /**
+     * Invoke onDeleteHold policy
+     *
+     * @param nodeRef node reference
+     */
+    protected void invokeOnDeleteHold(NodeRef nodeRef)
+    {
+        // execute policy for node type and aspects
+        OnDeleteHoldPolicy policy = onDeleteHoldPolicyDelegate.get(getTypeAndApsects(nodeRef));
+        policy.onDeleteHold(nodeRef);
+    }
 }

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

@@ -41,17 +41,9 @@ import org.alfresco.service.namespace.QName;
 
 public interface HoldServicePolicies
 {
-
-    /**
-     * Policy names
-     */
-    public static final QName BEFORE_CREATE_HOLD = QName.createQName(NamespaceService.ALFRESCO_URI, "beforeCreateHold");
-    public static final QName ON_CREATE_HOLD = QName.createQName(NamespaceService.ALFRESCO_URI, "onCreateHold");
-    public static final QName BEFORE_DELETE_HOLD = QName.createQName(NamespaceService.ALFRESCO_URI, "beforeDeleteHold");
-    public static final QName ON_DELETE_HOLD = QName.createQName(NamespaceService.ALFRESCO_URI, "onDeleteHold");
-
     interface BeforeCreateHoldPolicy extends ClassPolicy
     {
+        QName BEFORE_CREATE_HOLD = QName.createQName(NamespaceService.ALFRESCO_URI, "beforeCreateHold");
         /**
          * Called before a hold is created.
          *
@@ -63,6 +55,7 @@ public interface HoldServicePolicies
 
     interface OnCreateHoldPolicy extends ClassPolicy
     {
+        QName ON_CREATE_HOLD = QName.createQName(NamespaceService.ALFRESCO_URI, "onCreateHold");
         /**
          * Called when a hold is created.
          *
@@ -73,6 +66,7 @@ public interface HoldServicePolicies
 
     interface BeforeDeleteHoldPolicy extends ClassPolicy
     {
+        QName BEFORE_DELETE_HOLD = QName.createQName(NamespaceService.ALFRESCO_URI, "beforeDeleteHold");
         /**
          * Called before a hold is created.
          *
@@ -83,6 +77,8 @@ public interface HoldServicePolicies
 
     interface OnDeleteHoldPolicy extends ClassPolicy
     {
+        QName ON_DELETE_HOLD = QName.createQName(NamespaceService.ALFRESCO_URI, "onDeleteHold");
+
         /**
          * Called when a hold is deleted.
          *

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

@@ -158,11 +158,11 @@ public class CreateHoldTest extends BaseRMTestCase implements BeforeCreateHoldPo
             public Void run()
             {
                 BehaviourDefinition<ClassBehaviourBinding> beforeCreateHoldBehaviour = policyComponent.bindClassBehaviour(
-                            HoldServicePolicies.BEFORE_CREATE_HOLD, RecordsManagementModel.TYPE_HOLD_CONTAINER,
+                            HoldServicePolicies.BeforeCreateHoldPolicy.BEFORE_CREATE_HOLD, RecordsManagementModel.TYPE_HOLD_CONTAINER,
                             new JavaBehaviour(CreateHoldTest.this, "beforeCreateHold", NotificationFrequency.EVERY_EVENT));
 
                 BehaviourDefinition<ClassBehaviourBinding> onCreateHoldBehaviour = policyComponent.bindClassBehaviour(
-                            HoldServicePolicies.ON_CREATE_HOLD, RecordsManagementModel.TYPE_HOLD,
+                            HoldServicePolicies.OnCreateHoldPolicy.ON_CREATE_HOLD, RecordsManagementModel.TYPE_HOLD,
                             new JavaBehaviour(CreateHoldTest.this, "onCreateHold", NotificationFrequency.EVERY_EVENT));
 
                 assertFalse(beforeCreateHoldFlag);

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

@@ -258,11 +258,11 @@ public class DeleteHoldTest extends BaseRMTestCase implements BeforeDeleteHoldPo
             public Void run()
             {
                 BehaviourDefinition<ClassBehaviourBinding> beforeDeleteHoldBehaviour = policyComponent.bindClassBehaviour(
-                            HoldServicePolicies.BEFORE_DELETE_HOLD, RecordsManagementModel.TYPE_HOLD,
+                            HoldServicePolicies.BeforeDeleteHoldPolicy.BEFORE_DELETE_HOLD, RecordsManagementModel.TYPE_HOLD,
                             new JavaBehaviour(DeleteHoldTest.this, "beforeDeleteHold", NotificationFrequency.EVERY_EVENT));
 
                 BehaviourDefinition<ClassBehaviourBinding> onDeleteHoldBehaviour = policyComponent.bindClassBehaviour(
-                            HoldServicePolicies.ON_DELETE_HOLD, RecordsManagementModel.TYPE_HOLD,
+                            HoldServicePolicies.OnDeleteHoldPolicy.ON_DELETE_HOLD, RecordsManagementModel.TYPE_HOLD,
                             new JavaBehaviour(DeleteHoldTest.this, "onDeleteHold", NotificationFrequency.EVERY_EVENT));
 
                 NodeRef hold = holdService.createHold(filePlan, generate(), generate(), generate());

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

@@ -39,6 +39,7 @@ import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
@@ -70,6 +71,7 @@ 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;
@@ -220,6 +222,10 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
         when(mockedNodeService.createNode(eq(holdContainer), eq(ContentModel.ASSOC_CONTAINS), any(QName.class) , eq(TYPE_HOLD), any(Map.class)))
             .thenReturn(new ChildAssociationRef(ContentModel.ASSOC_CONTAINS, holdContainer, generateQName(), hold));
 
+        // mocks for policies
+        doNothing().when(holdService).invokeBeforeCreateHold(any(), anyString(), anyString());
+        doNothing().when(holdService).invokeOnCreateHold(any());
+
         // create hold
         NodeRef newHold = holdService.createHold(filePlan, HOLD_NAME, HOLD_REASON, HOLD_DESCRIPTION);
         assertNotNull(newHold);
@@ -306,6 +312,10 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
     @Test
     public void deleteHold()
     {
+        // mocks for policies
+        doNothing().when(holdService).invokeBeforeDeleteHold(any());
+        doNothing().when(holdService).invokeOnDeleteHold(any());
+
         // delete hold
         holdService.deleteHold(hold);
         verify(mockedNodeService).deleteNode(hold);