Refactored FreezeService after adding the HoldService

git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/modules/recordsmanagement/HEAD@63769 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

rm-server/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml

@@ -953,9 +953,8 @@
         <property name="dictionaryService" ref="DictionaryService" />
         <property name="recordService" ref="RecordService" />
         <property name="filePlanService" ref="FilePlanService" />
-        <property name="permissionService" ref="PermissionService" />
-        <property name="filePlanRoleService" ref="FilePlanRoleService" />
         <property name="recordFolderService" ref="RecordFolderService" />
+        <property name="holdService" ref="HoldService" />
     </bean>
 
     <bean id="FreezeService" class="org.springframework.aop.framework.ProxyFactoryBean">
@@ -1643,6 +1642,8 @@
          class="org.alfresco.module.org_alfresco_module_rm.fileplan.hold.HoldServiceImpl">
       <property name="filePlanService" ref="FilePlanService" />
       <property name="nodeService" ref="NodeService" />
+      <property name="recordService" ref="RecordService" />
+      <property name="recordFolderService" ref="RecordFolderService" />
    </bean>
 
    <bean id="HoldService"

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/fileplan/hold/HoldService.java

@@ -42,31 +42,31 @@ public interface HoldService
      * Adds the record to the given hold
      *
      * @param hold The {@link NodeRef} of the hold
-     * @param record The {@link NodeRef} of the record which will be added to the given hold
+     * @param nodeRef The {@link NodeRef} of the record / record folder which will be added to the given hold
      */
-    void addToHoldContainer(NodeRef hold, NodeRef record);
+    void addToHoldContainer(NodeRef hold, NodeRef nodeRef);
 
     /**
      * Adds the record to the given list of holds
      *
      * @param holds The list of {@link NodeRef}s of the holds
-     * @param record The {@link NodeRef} of the record which will be added to the given holds
+     * @param nodeRef The {@link NodeRef} of the record / record folder which will be added to the given holds
      */
-    void addToHoldContainers(List<NodeRef> holds, NodeRef record);
+    void addToHoldContainers(List<NodeRef> holds, NodeRef nodeRef);
 
     /**
      * Removes the record from the given hold
      *
      * @param hold The {@link NodeRef} of the hold
-     * @param record The {@link NodeRef} of the record which will be removed from the given hold
+     * @param nodeRef The {@link NodeRef} of the record / record folder which will be removed from the given hold
      */
-    void removeFromHoldContainer(NodeRef hold, NodeRef record);
+    void removeFromHoldContainer(NodeRef hold, NodeRef nodeRef);
 
     /**
      * Removes the record from the given list of hold
      *
      * @param holds The list {@link NodeRef}s of the holds
-     * @param record The {@link NodeRef} of the record which will be removed from the given holds
+     * @param nodeRef The {@link NodeRef} of the record / record folder which will be removed from the given holds
      */
-    void removeFromHoldContainers(List<NodeRef> holds, NodeRef record);
+    void removeFromHoldContainers(List<NodeRef> holds, NodeRef nodeRef);
 }

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/fileplan/hold/HoldServiceImpl.java

@@ -18,18 +18,28 @@
  */
 package org.alfresco.module.org_alfresco_module_rm.fileplan.hold;
 
+import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import org.alfresco.model.ContentModel;
 import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService;
+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.repo.security.authentication.AuthenticationUtil;
 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.NamespaceService;
 import org.alfresco.service.namespace.QName;
+import org.alfresco.service.namespace.RegexQNamePattern;
 import org.alfresco.util.ParameterCheck;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 
 /**
  * Hold service implementation
@@ -37,14 +47,23 @@ import org.alfresco.util.ParameterCheck;
  * @author Tuna Aksoy
  * @since 2.2
  */
-public class HoldServiceImpl implements HoldService
+public class HoldServiceImpl implements HoldService, RecordsManagementModel
 {
+    /** Logger */
+    private static Log logger = LogFactory.getLog(HoldServiceImpl.class);
+
     /** File Plan Service */
     private FilePlanService filePlanService;
 
     /** Node Service */
     private NodeService nodeService;
 
+    /** Record Service */
+    private RecordService recordService;
+
+    /** Record Folder Service */
+    private RecordFolderService recordFolderService;
+
     /**
      * Set the file plan service
      *
@@ -65,6 +84,26 @@ public class HoldServiceImpl implements HoldService
         this.nodeService = nodeService;
     }
 
+    /**
+     * Set the record service
+     *
+     * @param recordService the record service
+     */
+    public void setRecordService(RecordService recordService)
+    {
+        this.recordService = recordService;
+    }
+
+    /**
+     * Set the record folder service
+     *
+     * @param recordFolderService the record folder service
+     */
+    public void setRecordFolderService(RecordFolderService recordFolderService)
+    {
+        this.recordFolderService = recordFolderService;
+    }
+
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.fileplan.hold.HoldService#getHolds(org.alfresco.service.cmr.repository.NodeRef)
      */
@@ -74,11 +113,14 @@ public class HoldServiceImpl implements HoldService
         ParameterCheck.mandatory("filePlan", filePlan);
 
         NodeRef holdContainer = filePlanService.getHoldContainer(filePlan);
-        List<ChildAssociationRef> holdsAssocs = nodeService.getChildAssocs(holdContainer);
+        List<ChildAssociationRef> holdsAssocs = nodeService.getChildAssocs(holdContainer, ContentModel.ASSOC_CONTAINS, RegexQNamePattern.MATCH_ALL);
         List<NodeRef> holds = new ArrayList<NodeRef>(holdsAssocs.size());
-        for (ChildAssociationRef holdAssoc : holdsAssocs)
+        if (holdsAssocs != null && !holdsAssocs.isEmpty())
         {
-            holds.add(holdAssoc.getChildRef());
+            for (ChildAssociationRef holdAssoc : holdsAssocs)
+            {
+                holds.add(holdAssoc.getChildRef());
+            }
         }
 
         return holds;
@@ -88,32 +130,64 @@ public class HoldServiceImpl implements HoldService
      * @see org.alfresco.module.org_alfresco_module_rm.fileplan.hold.HoldService#addToHoldContainer(org.alfresco.service.cmr.repository.NodeRef, org.alfresco.service.cmr.repository.NodeRef)
      */
     @Override
-    public void addToHoldContainer(NodeRef hold, NodeRef record)
+    public void addToHoldContainer(NodeRef hold, NodeRef nodeRef)
     {
         ParameterCheck.mandatory("hold", hold);
-        ParameterCheck.mandatory("record", record);
+        ParameterCheck.mandatory("nodeRef", nodeRef);
 
         List<NodeRef> holds = new ArrayList<NodeRef>(1);
         holds.add(hold);
-        addToHoldContainers(Collections.unmodifiableList(holds), record);
+        addToHoldContainers(Collections.unmodifiableList(holds), nodeRef);
     }
 
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.fileplan.hold.HoldService#addToHoldContainers(java.util.List, org.alfresco.service.cmr.repository.NodeRef)
      */
     @Override
-    public void addToHoldContainers(List<NodeRef> holds, NodeRef record)
+    public void addToHoldContainers(List<NodeRef> holds, NodeRef nodeRef)
     {
         ParameterCheck.mandatoryCollection("holds", holds);
-        ParameterCheck.mandatory("record", record);
+        ParameterCheck.mandatory("nodeRef", nodeRef);
-
-        String recordName = (String) nodeService.getProperty(record, ContentModel.PROP_NAME);
-        String validLocalName = QName.createValidLocalName(recordName);
-        QName name = QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, validLocalName);
 
         for (NodeRef hold : holds)
         {
-            nodeService.addChild(hold, record, ContentModel.ASSOC_CONTAINS, name);
+            // Link the record to the hold
+            nodeService.addChild(hold, nodeRef, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS);
+
+            // Apply the freeze aspect
+            Map<QName, Serializable> props = new HashMap<QName, Serializable>(2);
+            props.put(PROP_FROZEN_AT, new Date());
+            props.put(PROP_FROZEN_BY, AuthenticationUtil.getFullyAuthenticatedUser());
+            nodeService.addAspect(nodeRef, ASPECT_FROZEN, props);
+
+            // Log a message about applying the the frozen aspect
+            if (logger.isDebugEnabled())
+            {
+                StringBuilder msg = new StringBuilder();
+                msg.append("Frozen aspect applied to '").append(nodeRef).append("'.");
+                logger.debug(msg.toString());
+            }
+
+            // Mark all the folders contents as frozen
+            if (recordFolderService.isRecordFolder(nodeRef))
+            {
+                List<NodeRef> records = recordService.getRecords(nodeRef);
+                for (NodeRef record : records)
+                {
+                    // no need to freeze if already frozen!
+                    if (nodeService.hasAspect(record, ASPECT_FROZEN) == false)
+                    {
+                        nodeService.addAspect(record, ASPECT_FROZEN, props);
+
+                        if (logger.isDebugEnabled())
+                        {
+                            StringBuilder msg = new StringBuilder();
+                            msg.append("Frozen aspect applied to '").append(record).append("'.");
+                            logger.debug(msg.toString());
+                        }
+                    }
+                }
+            }
         }
     }
 
@@ -121,28 +195,28 @@ public class HoldServiceImpl implements HoldService
      * @see org.alfresco.module.org_alfresco_module_rm.fileplan.hold.HoldService#removeFromHoldContainer(org.alfresco.service.cmr.repository.NodeRef, org.alfresco.service.cmr.repository.NodeRef)
      */
     @Override
-    public void removeFromHoldContainer(NodeRef hold, NodeRef record)
+    public void removeFromHoldContainer(NodeRef hold, NodeRef nodeRef)
     {
         ParameterCheck.mandatory("hold", hold);
-        ParameterCheck.mandatory("record", record);
+        ParameterCheck.mandatory("nodeRef", nodeRef);
 
         List<NodeRef> holds = new ArrayList<NodeRef>(1);
         holds.add(hold);
-        removeFromHoldContainers(Collections.unmodifiableList(holds), record);
+        removeFromHoldContainers(Collections.unmodifiableList(holds), nodeRef);
     }
 
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.fileplan.hold.HoldService#removeFromHoldContainers(java.util.List, org.alfresco.service.cmr.repository.NodeRef)
      */
     @Override
-    public void removeFromHoldContainers(List<NodeRef> holds, NodeRef record)
+    public void removeFromHoldContainers(List<NodeRef> holds, NodeRef nodeRef)
     {
         ParameterCheck.mandatory("holds", holds);
-        ParameterCheck.mandatory("record", record);
+        ParameterCheck.mandatory("nodeRef", nodeRef);
 
         for (NodeRef hold : holds)
         {
-            nodeService.removeChild(hold, record);
+            nodeService.removeChild(hold, nodeRef);
         }
     }
 }

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/freeze/FreezeService.java

@@ -21,25 +21,26 @@ package org.alfresco.module.org_alfresco_module_rm.freeze;
 import java.util.Date;
 import java.util.Set;
 
+import org.alfresco.module.org_alfresco_module_rm.fileplan.hold.HoldService;
 import org.alfresco.service.cmr.repository.NodeRef;
 
 /**
  * Freeze Service Interface
- * 
+ *
  * TODO
- * Implementation used to consolidate freeze behaviours in 2.0. 
+ * Implementation used to consolidate freeze behaviours in 2.0.
  * When implementing consider application of freeze to 'any' node references, not just records and record folders.
  * (Consider implications for security and capabilities)
- * 
+ *
  * @author Roy Wetherall
  * @since 2.0
  */
-public interface FreezeService 
+public interface FreezeService
 {
    /**
     * Indicates whether the passed node reference is a hold.  A hold is a container for a group of frozen object and contains the freeze
     * reason.
-    * 
+    *
     * @param nodeRef   hold node reference
     * @return boolean  true if hold, false otherwise
     */
@@ -47,7 +48,7 @@ public interface FreezeService
 
    /**
     * Indicates whether the passed node reference is frozen.
-    * 
+    *
     * @param nodeRef   node reference
     * @return boolean  true if frozen, false otherwise
     */
@@ -55,7 +56,7 @@ public interface FreezeService
 
    /**
     * Get the 'root' frozen node references in a hold.
-    * 
+    *
     * @param hold          hold node reference
     * @return Set<NodeRef> frozen node references
     */
@@ -63,7 +64,7 @@ public interface FreezeService
 
    /**
     * Freezes a node with the provided reason, creating a hold node reference.
-    * 
+    *
     * @param reason    freeze reason
     * @param nodeRef   node reference
     * @return NodeRef  hold node reference
@@ -72,35 +73,38 @@ public interface FreezeService
 
    /**
     * Freezes a node, adding it an existing hold.
-    * 
+    *
     * @param hold      hold node reference
     * @param nodeRef   node reference
+    *
+    * @deprecated as of 2.2, use {@link HoldService#addToHoldContainer(NodeRef, NodeRef)} instead
     */
+   @Deprecated
    void freeze(NodeRef hold, NodeRef nodeRef);
 
    /**
     * Freezes a collection of nodes with the given reason, creating a hold.
-    * 
+    *
     * @param reason        freeze reason
-    * @param nodeRefs  set of nodes to freeze 
+    * @param nodeRefs  set of nodes to freeze
     * @return NodeRef      hold node reference
     */
    NodeRef freeze(String reason, Set<NodeRef> nodeRefs);
 
    /**
     * Freeze a collection of nodes, adding them to an existing hold.
-    * 
+    *
     * @param hold      hold node reference
     * @param nodeRefs   set of nodes to freeze
     */
    void freeze(NodeRef hold, Set<NodeRef> nodeRefs);
 
    /**
-    * Unfreeze a frozen node.  
+    * Unfreeze a frozen node.
     * <p>
     * The unfrozen node is automatically removed from the hold(s) it is in.  If the hold is
     * subsequently empty, the hold is automatically deleted.
-    * 
+    *
     * @param nodeRef   node reference
     */
    void unFreeze(NodeRef nodeRef);
@@ -110,21 +114,21 @@ public interface FreezeService
     * <p>
     * The unfrozen nodes are automatically removed from the hold(s) the are in.  If the hold(s) is
     * subsequently empty, the hold is automatically deleted.
-    * 
+    *
     * @param nodeRefs  set of nodes to unfreeze
     */
    void unFreeze(Set<NodeRef> nodeRefs);
 
    /**
     * Unfreezes all nodes within a hold and deletes the hold.
-    * 
+    *
     * @param hold  hold node reference
     */
    void relinquish(NodeRef hold);
 
    /**
     * Gets the freeze reason for a hold.
-    * 
+    *
     * @param hold      hold node reference
     * @return String   freeze reason
     */
@@ -132,7 +136,7 @@ public interface FreezeService
 
    /**
     * Updates the freeze reason for a given hold.
-    * 
+    *
     * @param hold      hold node reference
     * @param reason    updated reason
     */
@@ -141,15 +145,18 @@ public interface FreezeService
    /**
     * Gets the hold node references for a given file plan
     * or an empty set if there is not any hold node available
-    * 
+    *
     * @param filePlan      file plan for which the hold nodes will be retrieved
     * @return Set<NodeRef> hold node references
+    *
+    * @deprecated as of 2.2, use {@link HoldService#getHolds(NodeRef)} instead
     */
+   @Deprecated
    Set<NodeRef> getHolds(NodeRef filePlan);
 
    /**
     * Checks whether or not the given node has frozen children
-    * 
+    *
     * @param nodeRef The nodeRef for which will be checked if it has frozen children
     * @return true   if the given nodeRef has frozen children, false otherwise
     */
@@ -157,15 +164,15 @@ public interface FreezeService
 
    /**
     * Gets the date of the freeze for the given node, null if the node is not frozen
-    * 
+    *
     * @param nodeRef The nodeRef for which the date check will be performed
-    * @return Date   The of the freeze or null 
+    * @return Date   The of the freeze or null
     */
    Date getFreezeDate(NodeRef nodeRef);
 
    /**
     * Gets the initiator of the freeze for the given node, null if the node is not frozen
-    * 
+    *
     * @param nodeRef The nodeRef for which the initiator check will be performed
     * @return String The initiator of the freeze or null
     */

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/freeze/FreezeServiceImpl.java

@@ -29,16 +29,14 @@ import java.util.Set;
 import org.alfresco.error.AlfrescoRuntimeException;
 import org.alfresco.model.ContentModel;
 import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService;
+import org.alfresco.module.org_alfresco_module_rm.fileplan.hold.HoldService;
 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.role.FilePlanRoleService;
 import org.alfresco.module.org_alfresco_module_rm.util.ServiceBaseImpl;
-import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.repo.transaction.AlfrescoTransactionSupport;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
 import org.alfresco.service.cmr.repository.NodeRef;
-import org.alfresco.service.cmr.security.PermissionService;
 import org.alfresco.service.namespace.QName;
 import org.alfresco.service.namespace.RegexQNamePattern;
 import org.alfresco.util.ParameterCheck;
@@ -74,15 +72,12 @@ public class FreezeServiceImpl extends    ServiceBaseImpl
     /** File Plan Service */
     protected FilePlanService filePlanService;
 
-    /** Permission service */
-    protected PermissionService permissionService;
-
-    /** File plan role service */
-    protected FilePlanRoleService filePlanRoleService;
-
     /** Record folder service */
     protected RecordFolderService recordFolderService;
-    
+
+    /** Hold service */
+    protected HoldService holdService;
+
     /**
      * @param recordService record service
      */
@@ -99,22 +94,6 @@ public class FreezeServiceImpl extends    ServiceBaseImpl
         this.filePlanService = filePlanService;
     }
 
-    /**
-     * @param permissionService  permission service
-     */
-    public void setPermissionService(PermissionService permissionService)
-    {
-		this.permissionService = permissionService;
-	}
-
-    /**
-     * @param filePlanRoleService	file plan role service
-     */
-    public void setFilePlanRoleService(FilePlanRoleService filePlanRoleService)
-    {
-		this.filePlanRoleService = filePlanRoleService;
-	}
-
     /**
      * @param recordFolderService record folder service
      */
@@ -123,6 +102,14 @@ public class FreezeServiceImpl extends    ServiceBaseImpl
         this.recordFolderService = recordFolderService;
     }
 
+    /**
+     * @param holdService hold service
+     */
+    public void setHoldService(HoldService holdService)
+    {
+        this.holdService = holdService;
+    }
+
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.freeze.FreezeService#isHold(org.alfresco.service.cmr.repository.NodeRef)
      */
@@ -227,43 +214,7 @@ public class FreezeServiceImpl extends    ServiceBaseImpl
         ParameterCheck.mandatory("hold", hold);
         ParameterCheck.mandatory("nodeRef", nodeRef);
 
-        // Link the record to the hold
+        holdService.addToHoldContainer(hold, nodeRef);
-        nodeService.addChild(hold, nodeRef, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS);
-
-        // Apply the freeze aspect
-        Map<QName, Serializable> props = new HashMap<QName, Serializable>(2);
-        props.put(PROP_FROZEN_AT, new Date());
-        props.put(PROP_FROZEN_BY, AuthenticationUtil.getFullyAuthenticatedUser());
-        nodeService.addAspect(nodeRef, ASPECT_FROZEN, props);
-
-        // Log a message about applying the the frozen aspect
-        if (logger.isDebugEnabled())
-        {
-            StringBuilder msg = new StringBuilder();
-            msg.append("Frozen aspect applied to '").append(nodeRef).append("'.");
-            logger.debug(msg.toString());
-        }
-
-        // Mark all the folders contents as frozen
-        if (recordFolderService.isRecordFolder(nodeRef))
-        {
-            List<NodeRef> records = recordService.getRecords(nodeRef);
-            for (NodeRef record : records)
-            {
-                // no need to freeze if already frozen!
-                if (nodeService.hasAspect(record, ASPECT_FROZEN) == false)
-                {
-                    nodeService.addAspect(record, ASPECT_FROZEN, props);
-
-                    if (logger.isDebugEnabled())
-                    {
-                        StringBuilder msg = new StringBuilder();
-                        msg.append("Frozen aspect applied to '").append(record).append("'.");
-                        logger.debug(msg.toString());
-                    }
-                }
-            }
-        }
     }
 
     /**
@@ -430,18 +381,7 @@ public class FreezeServiceImpl extends    ServiceBaseImpl
     {
         ParameterCheck.mandatory("filePlan", filePlan);
 
-        Set<NodeRef> holds = new HashSet<NodeRef>();
+        return new HashSet<NodeRef>(holdService.getHolds(filePlan));
-        NodeRef holdContainer = filePlanService.getHoldContainer(filePlan);
-        List<ChildAssociationRef> childAssocs = nodeService.getChildAssocs(holdContainer, ContentModel.ASSOC_CONTAINS, RegexQNamePattern.MATCH_ALL);
-        if (childAssocs != null && !childAssocs.isEmpty())
-        {
-            for (ChildAssociationRef childAssoc : childAssocs)
-            {
-                holds.add(childAssoc.getChildRef());
-            }
-        }
-
-        return holds;
     }
 
     /**

rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/issue/RM1008Test.java

@@ -22,7 +22,6 @@ import java.io.Serializable;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 import org.alfresco.model.ContentModel;
 import org.alfresco.module.org_alfresco_module_rm.capability.Capability;
@@ -128,7 +127,7 @@ public class RM1008Test extends BaseRMTestCase
             {
                 // create hold object
                 freezeService.freeze("test", rmFolder);
-                Set<NodeRef> holds = freezeService.getHolds(filePlan);
+                List<NodeRef> holds = holdService.getHolds(filePlan);
                 return holds.iterator().next();
             }
         }, rmAdminName);

rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/issue/RM1030Test.java

@@ -18,7 +18,7 @@
  */
 package org.alfresco.module.org_alfresco_module_rm.test.issue;
 
-import java.util.Set;
+import java.util.List;
 
 import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase;
 import org.alfresco.service.cmr.repository.NodeRef;
@@ -26,18 +26,18 @@ import org.alfresco.service.cmr.repository.NodeRef;
 
 /**
  * Unit test for RM-1030 .. can't freeze a record folder that already has a frozen record contained within
- * 
+ *
  * @author Roy Wetherall
  * @since 2.1
  */
-public class RM1030Test extends BaseRMTestCase 
+public class RM1030Test extends BaseRMTestCase
-{     
+{
     @Override
     protected boolean isRecordTest()
     {
         return true;
     }
-   
+
     public void testRM1030() throws Exception
     {
         final NodeRef recordHold = doTestInTransaction(new Test<NodeRef>()
@@ -46,100 +46,100 @@ public class RM1030Test extends BaseRMTestCase
             public NodeRef run()
             {
                 // show there are no holds when we start
-                Set<NodeRef> holds = freezeService.getHolds(filePlan);
+                List<NodeRef> holds = holdService.getHolds(filePlan);
                 assertNotNull(holds);
                 assertEquals(0, holds.size());
-                
+
                 // freeze record contained within the record folder
                 NodeRef hold = freezeService.freeze("in true life for serious", recordOne);
                 assertNotNull(hold);
-                
+
                 return hold;
             }
-            
+
             @Override
             public void test(NodeRef hold) throws Exception
             {
                 // show the record is frozen
                 assertTrue(freezeService.isFrozen(recordOne));
-                
+
                 // count the number of holds
-                Set<NodeRef> holds = freezeService.getHolds(filePlan);
+                List<NodeRef> holds = holdService.getHolds(filePlan);
                 assertNotNull(holds);
                 assertEquals(1, holds.size());
             }
-            
+
         });
-        
+
         final NodeRef recordFolderHold = doTestInTransaction(new Test<NodeRef>()
         {
             @Override
             public NodeRef run()
-            {        
+            {
                 // freeze the record folder that contains the frozen record
                 NodeRef folderHold = freezeService.freeze("innit but", rmFolder);
                 assertNotNull(folderHold);
-                
+
                 return folderHold;
             }
-            
+
             @Override
             public void test(NodeRef hold) throws Exception
             {
              // show that the record and the record folder are frozen
                 assertTrue(freezeService.isFrozen(recordOne));
                 assertTrue(freezeService.isFrozen(rmFolder));
-                
+
                 // count the number of holds
-                Set<NodeRef> holds = freezeService.getHolds(filePlan);
+                List<NodeRef> holds = holdService.getHolds(filePlan);
                 assertNotNull(holds);
                 assertEquals(2, holds.size());
             }
         });
-        
+
         doTestInTransaction(new Test<Void>()
         {
             @Override
             public Void run()
-            {        
+            {
                 // relinquish the record folder hold
-                freezeService.relinquish(recordFolderHold);                
+                freezeService.relinquish(recordFolderHold);
                 return null;
             }
-            
+
             @Override
             public void test(Void result) throws Exception
             {
                 assertTrue(freezeService.isFrozen(recordOne));
                 assertFalse(freezeService.isFrozen(rmFolder));
-                
+
-                Set<NodeRef> holds = freezeService.getHolds(filePlan);
+                List<NodeRef> holds = holdService.getHolds(filePlan);
                 assertNotNull(holds);
                 assertEquals(1, holds.size());
             }
         });
-        
+
         doTestInTransaction(new Test<Void>()
         {
             @Override
             public Void run()
-            {        
+            {
                 // relinquish the record hold
-                freezeService.relinquish(recordHold);                
+                freezeService.relinquish(recordHold);
                 return null;
             }
-            
+
             @Override
             public void test(Void result) throws Exception
             {
                 assertFalse(freezeService.isFrozen(recordOne));
                 assertFalse(freezeService.isFrozen(rmFolder));
-                
+
-                Set<NodeRef> holds = freezeService.getHolds(filePlan);
+                List<NodeRef> holds = holdService.getHolds(filePlan);
                 assertNotNull(holds);
                 assertEquals(0, holds.size());
             }
         });
-        
+
     }
 }

rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/service/FreezeServiceImplTest.java

@@ -29,7 +29,7 @@ import org.alfresco.service.cmr.repository.NodeRef;
 
 /**
  * Freeze service implementation test.
- * 
+ *
  * @author Tuna Aksoy
  * @since 2.1
  */
@@ -65,7 +65,7 @@ public class FreezeServiceImplTest extends BaseRMTestCase
             assertTrue(freezeService.hasFrozenChildren(rmFolder));
 
             // Check the hold exists
-            Set<NodeRef> holdAssocs = freezeService.getHolds(filePlan);
+            List<NodeRef> holdAssocs = holdService.getHolds(filePlan);
             assertNotNull(holdAssocs);
             assertEquals(1, holdAssocs.size());
             NodeRef holdNodeRef = holdAssocs.iterator().next();
@@ -98,7 +98,7 @@ public class FreezeServiceImplTest extends BaseRMTestCase
             assertTrue(freezeService.isHold(newHold));
 
             // Check the holds exist
-            holdAssocs = freezeService.getHolds(filePlan);
+            holdAssocs = holdService.getHolds(filePlan);
             assertNotNull(holdAssocs);
             assertEquals(2, holdAssocs.size());
             for (NodeRef hold : holdAssocs)
@@ -136,7 +136,7 @@ public class FreezeServiceImplTest extends BaseRMTestCase
             freezeService.unFreeze(recordThree);
 
             // Check the holds
-            holdAssocs = freezeService.getHolds(filePlan);
+            holdAssocs = holdService.getHolds(filePlan);
             assertNotNull(holdAssocs);
             assertEquals(2, holdAssocs.size());
             for (NodeRef hold : holdAssocs)
@@ -175,7 +175,7 @@ public class FreezeServiceImplTest extends BaseRMTestCase
             freezeService.relinquish(holdNodeRef);
 
             // Check the existing hold
-            holdAssocs = freezeService.getHolds(filePlan);
+            holdAssocs = holdService.getHolds(filePlan);
             assertNotNull(holdAssocs);
             assertEquals(1, holdAssocs.size());
 
@@ -184,7 +184,7 @@ public class FreezeServiceImplTest extends BaseRMTestCase
             freezeService.unFreeze(freezeService.getFrozen(holdNodeRef));
 
             // All holds should be deleted
-            holdAssocs = freezeService.getHolds(filePlan);
+            holdAssocs = holdService.getHolds(filePlan);
             assertEquals(0, holdAssocs.size());
 
             // Check the nodes are unfrozen
@@ -196,7 +196,7 @@ public class FreezeServiceImplTest extends BaseRMTestCase
 
             // Test freezing nodes, adding them to an existing hold
             NodeRef hold = freezeService.freeze("AnotherFreezeReason", recordFour);
-            freezeService.freeze(hold, recordOne);
+            holdService.addToHoldContainer(hold, recordOne);
             Set<NodeRef> nodes = new HashSet<NodeRef>();
             nodes.add(recordTwo);
             nodes.add(recordThree);
@@ -204,7 +204,7 @@ public class FreezeServiceImplTest extends BaseRMTestCase
             assertTrue(freezeService.hasFrozenChildren(rmFolder));
 
             // Check the hold
-            holdAssocs = freezeService.getHolds(filePlan);
+            holdAssocs = holdService.getHolds(filePlan);
             assertNotNull(holdAssocs);
             assertEquals(1, holdAssocs.size());
 

rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/BaseRMTestCase.java

@@ -20,8 +20,8 @@ package org.alfresco.module.org_alfresco_module_rm.test.util;
 
 import java.io.Serializable;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 import org.alfresco.model.ContentModel;
 import org.alfresco.module.org_alfresco_module_rm.action.RecordsManagementActionService;
@@ -34,6 +34,7 @@ import org.alfresco.module.org_alfresco_module_rm.disposition.DispositionSchedul
 import org.alfresco.module.org_alfresco_module_rm.disposition.DispositionService;
 import org.alfresco.module.org_alfresco_module_rm.event.RecordsManagementEventService;
 import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService;
+import org.alfresco.module.org_alfresco_module_rm.fileplan.hold.HoldService;
 import org.alfresco.module.org_alfresco_module_rm.freeze.FreezeService;
 import org.alfresco.module.org_alfresco_module_rm.identifier.IdentifierService;
 import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
@@ -146,6 +147,7 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase
     protected ReportService reportService;
     protected RecordsManagementAuditService rmAuditService;
     protected IdentifierService identifierService;
+    protected HoldService holdService;
 
     /** test data */
     protected String siteId;
@@ -251,15 +253,15 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase
     protected String dmCollaborator;
     protected NodeRef dmCollaboratorNodeRef;
 
-    /** 
+    /**
-     * Indicates whether this is a RM site test or not.  If true then the test RM site is created along with a basic 
+     * Indicates whether this is a RM site test or not.  If true then the test RM site is created along with a basic
      * file plan structure, otherwise not.
      */
     protected boolean isRMSiteTest()
     {
     	return true;
     }
-    
+
     /**
      * Indicates whether this is a multi-hierarchy test or not.  If it is then the multi-hierarchy record
      * taxonomy test data is loaded.
@@ -395,6 +397,7 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase
         reportService = (ReportService) applicationContext.getBean("ReportService");
         rmAuditService = (RecordsManagementAuditService) applicationContext.getBean("RecordsManagementAuditService");
         identifierService = (IdentifierService) applicationContext.getBean("recordsManagementIdentifierService");
+        holdService = (HoldService) applicationContext.getBean("HoldService");
     }
 
     /**
@@ -430,7 +433,7 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase
         {
             if (filePlan != null && nodeService.exists(filePlan) == true)
             {
-                Set<NodeRef> holds = freezeService.getHolds(filePlan);
+                List<NodeRef> holds = holdService.getHolds(filePlan);
                 for (NodeRef hold : holds)
                 {
                     freezeService.relinquish(hold);
@@ -500,7 +503,7 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase
 	                    utils.declareRecord(recordDeclaredOne);
 	                    utils.declareRecord(recordDeclaredTwo);
 	                }
-	
+
 	                // unfiled container
 	                unfiledContainer = filePlanService.getUnfiledContainer(filePlan);
 	                assertNotNull(unfiledContainer);
@@ -540,7 +543,7 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase
         permissionService.setPermission(folder, "rmadmin", PermissionService.ADD_CHILDREN, true);
 
         if (isRMSiteTest() == true)
-        {        
+        {
 	        siteId = GUID.generate();
 	        siteInfo = siteService.createSite(
 	                        "rm-site-dashboard",
@@ -549,17 +552,17 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase
 	                        "descrition",
 	                        SiteVisibility.PUBLIC,
 	                        RecordsManagementModel.TYPE_RM_SITE);
-	
+
 	        filePlan = siteService.getContainer(siteId, RmSiteType.COMPONENT_DOCUMENT_LIBRARY);
 	        assertNotNull("Site document library container was not created successfully.", filePlan);
-	
+
 	        // Create RM container
 	        rmContainer = filePlanService.createRecordCategory(filePlan, "rmContainer");
 	        assertNotNull("Could not create rm container", rmContainer);
-	
+
 	        // Create disposition schedule
 	        dispositionSchedule = utils.createBasicDispositionSchedule(rmContainer);
-	
+
 	        // Create RM folder
 	        rmFolder = recordFolderService.createRecordFolder(rmContainer, "rmFolder");
 	        assertNotNull("Could not create rm folder", rmFolder);