RM-1326: Write tests for Hold Service

* unit test completed for hold service
  * some improvements to unit test base



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

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

@@ -136,29 +136,7 @@ public class HoldServiceImpl extends ServiceBaseImpl
                     List<NodeRef> frozenNodes = getHeld(hold);
                     for (NodeRef frozenNode : frozenNodes)
                     {
-                        List<NodeRef> otherHolds = heldBy(frozenNode, true);
+                        removeFreezeAspect(frozenNode, 1);
-                        if (otherHolds.size() == 1)
-                        {   
-                            // remove the freeze aspect from the node
-                            nodeService.removeAspect(frozenNode, ASPECT_FROZEN);
-                            
-                            if (isRecordFolder(frozenNode))
-                            {
-                                List<NodeRef> records = recordService.getRecords(frozenNode);
-                                for (NodeRef record : records)
-                                {
-                                    if (nodeService.hasAspect(record, ASPECT_FROZEN))
-                                    {
-                                        List<NodeRef> recordsOtherHolds = heldBy(record, true);
-                                        if (recordsOtherHolds.size() == 1)
-                                        {
-                                            // remove the freeze aspect from the node
-                                            nodeService.removeAspect(record, ASPECT_FROZEN);
-                                        }
-                                    }
-                                }
-                            }
-                        }
                     }
                     
                     return null;
@@ -170,6 +148,43 @@ public class HoldServiceImpl extends ServiceBaseImpl
         }
     }
     
+    /**
+     * Helper method removes the freeze aspect from the record and record folder if it is no longer
+     * in a hold.
+     * 
+     * @param nodeRef
+     */
+    private void removeFreezeAspect(NodeRef nodeRef, int index)
+    {
+        List<NodeRef> otherHolds = heldBy(nodeRef, true);
+        if (otherHolds.size() == index)
+        {   
+            if (nodeService.hasAspect(nodeRef, ASPECT_FROZEN))
+            {
+                // remove the freeze aspect from the node
+                nodeService.removeAspect(nodeRef, ASPECT_FROZEN);
+            }
+            
+            if (isRecordFolder(nodeRef))
+            {
+                List<NodeRef> records = recordService.getRecords(nodeRef);
+                for (NodeRef record : records)
+                {
+                    if (nodeService.hasAspect(record, ASPECT_FROZEN))
+                    {
+                        List<NodeRef> recordsOtherHolds = heldBy(record, true);
+                        if (recordsOtherHolds.size() == index)
+                        {
+                            // remove the freeze aspect from the node
+                            nodeService.removeAspect(record, ASPECT_FROZEN);
+                        }
+                    }
+                }
+            }
+        }
+        
+    }
+
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.hold.HoldService#getHolds(org.alfresco.service.cmr.repository.NodeRef)
      */
@@ -285,8 +300,11 @@ public class HoldServiceImpl extends ServiceBaseImpl
         ParameterCheck.mandatory("hold", hold);
         List<NodeRef> children = new ArrayList<NodeRef>();
         
-        if (nodeService.exists(hold) && isHold(hold))
+        if (!isHold(hold))
         {     
+            throw new AlfrescoRuntimeException("Can't get the node's held, because passed node reference isn't a hold. (hold=" + hold.toString() + ")");
+        }
+         
         List<ChildAssociationRef> childAssocs = nodeService.getChildAssocs(hold, ASSOC_FROZEN_RECORDS, RegexQNamePattern.MATCH_ALL);
         if (childAssocs != null && !childAssocs.isEmpty())
         {
@@ -295,7 +313,6 @@ public class HoldServiceImpl extends ServiceBaseImpl
                 children.add(childAssociationRef.getChildRef());
             }
         }
-        }
         
         return children;
     }
@@ -373,12 +390,14 @@ public class HoldServiceImpl extends ServiceBaseImpl
     {
         ParameterCheck.mandatory("hold", hold);
         
-        if (nodeService.exists(hold) && isHold(hold))
+        if (!isHold(hold))
         {
+            throw new AlfrescoRuntimeException("Can't delete hold, becuase passed node is not a hold. (hold=" + hold.toString() + ")");
+        }
+        
         // delete the hold node
         nodeService.deleteNode(hold);        
     }
-    }
 
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.hold.HoldService#addToHold(org.alfresco.service.cmr.repository.NodeRef, org.alfresco.service.cmr.repository.NodeRef)
@@ -418,7 +437,20 @@ public class HoldServiceImpl extends ServiceBaseImpl
         ParameterCheck.mandatoryCollection("holds", holds);
         ParameterCheck.mandatory("nodeRef", nodeRef);
         
+        if (!isRecord(nodeRef) && !isRecordFolder(nodeRef))
+        {
+            throw new AlfrescoRuntimeException("Can only add records or record folders to a hold.");
+        }
+
         for (NodeRef hold : holds)
+        {
+            if (!isHold(hold))
+            {
+                throw new AlfrescoRuntimeException("Can't add to hold, because it isn't a hold. (hold=" + hold.toString() + ")");
+            }
+            
+            // check that the node isn't already in the hold
+            if (getHeld(hold).contains(nodeRef) == false)
             {
                 // Link the record to the hold
                 nodeService.addChild(hold, nodeRef, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS);
@@ -463,6 +495,7 @@ public class HoldServiceImpl extends ServiceBaseImpl
                 }
             }
         }
+    }
 
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.hold.HoldService#removeFromHold(org.alfresco.service.cmr.repository.NodeRef, org.alfresco.service.cmr.repository.NodeRef)
@@ -502,23 +535,29 @@ public class HoldServiceImpl extends ServiceBaseImpl
         ParameterCheck.mandatory("holds", holds);
         ParameterCheck.mandatory("nodeRef", nodeRef);
                
+        if (holds != null && !holds.isEmpty())
+        {
             for (NodeRef hold : holds)
             {
-            nodeService.removeChild(hold, nodeRef);
+                if (!instanceOf(hold, TYPE_HOLD))
+                {
+                    throw new AlfrescoRuntimeException("Can't remove from hold, because it isn't a hold. (hold=" + hold + ")");
                 }
                 
-        // check to see if this node can be unfrozen
+                if (getHeld(hold).contains(nodeRef) == true)
-        List<NodeRef> holdList = heldBy(nodeRef, true);
-        if (holdList.size() == 0)
                 {
+                    // remove from hold
+                    nodeService.removeChild(hold, nodeRef);                
+                }
+            }
+    
             // run as system as we can't be sure if have remove aspect rights on node
             runAsSystem(new RunAsWork<Void>()
             {
                 @Override
                 public Void doWork()
                 {
-                    // remove frozen aspect
+                    removeFreezeAspect(nodeRef, 0);                   
-                    nodeService.removeAspect(nodeRef, ASPECT_FROZEN);
                     return null;
                 }
              });            

rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/BaseUnitTest.java

@@ -18,8 +18,10 @@
  */
 package org.alfresco.module.org_alfresco_module_rm;
 
+import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.when;
 
+import org.alfresco.model.ContentModel;
 import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService;
 import org.alfresco.module.org_alfresco_module_rm.identifier.IdentifierService;
 import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
@@ -51,19 +53,19 @@ public class BaseUnitTest implements RecordsManagementModel
     @Mock(name="namespaceService")      protected NamespaceService mockedNamespaceService; 
     @Mock(name="identifierService")     protected IdentifierService mockedIdentifierService;
     
-    /** rm service mocks */
     @Mock(name="filePlanService")       protected FilePlanService mockedFilePlanService;
     
+    /**
+     * Test method setup
+     */
     @Before
     public void before()
     {
         MockitoAnnotations.initMocks(this);
 
-        filePlanComponent = generateNodeRef();
         filePlan = generateNodeRef(TYPE_FILE_PLAN);
-        
+        filePlanComponent = generateNodeRef();
-        // set-up node service 
+        setupAsFilePlanComponent(filePlanComponent);
-        when(mockedNodeService.getProperty(filePlanComponent, PROP_ROOT_NODEREF)).thenReturn(filePlan);
         
         // set-up namespace service
         when(mockedNamespaceService.getNamespaceURI(RM_PREFIX)).thenReturn(RM_URI);
@@ -71,11 +73,39 @@ public class BaseUnitTest implements RecordsManagementModel
         
     }
     
+    /**
+     * Helper method to generate a qname.
+     * 
+     * @return  QName   qualified name
+     */
     protected static QName generateQName()
     {
         return QName.createQName(RM_URI, GUID.generate());
     }
     
+    protected NodeRef generateRecordFolder()
+    {
+        NodeRef recordFolder = generateNodeRef(TYPE_RECORD_FOLDER);
+        setupAsFilePlanComponent(recordFolder);       
+        return recordFolder;
+        
+    }
+    
+    protected NodeRef generateRecord()
+    {
+        NodeRef record = generateNodeRef(ContentModel.TYPE_CONTENT);
+        setupAsFilePlanComponent(record);
+        doReturn(true).when(mockedNodeService).hasAspect(record, ASPECT_RECORD);        
+        return record;
+    }
+    
+    protected void setupAsFilePlanComponent(NodeRef nodeRef)
+    {
+        doReturn(true).when(mockedNodeService).hasAspect(nodeRef, ASPECT_FILE_PLAN_COMPONENT);
+        doReturn(filePlan).when(mockedFilePlanService).getFilePlan(nodeRef);
+        doReturn(filePlan).when(mockedNodeService).getProperty(nodeRef, PROP_ROOT_NODEREF);
+    }
+        
     protected NodeRef generateNodeRef()
     {
         return generateNodeRef(null);

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

@@ -26,30 +26,36 @@ import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
-import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.doAnswer;
 
 import java.io.Serializable;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
 import org.alfresco.error.AlfrescoRuntimeException;
 import org.alfresco.model.ContentModel;
 import org.alfresco.module.org_alfresco_module_rm.BaseUnitTest;
-import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
+import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService;
+import org.alfresco.module.org_alfresco_module_rm.record.RecordService;
+import org.alfresco.module.org_alfresco_module_rm.recordfolder.RecordFolderService;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.namespace.NamespaceService;
 import org.alfresco.service.namespace.QName;
+import org.alfresco.service.namespace.QNamePattern;
 import org.alfresco.service.namespace.RegexQNamePattern;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
 import org.mockito.InjectMocks;
+import org.mockito.Mock;
 import org.mockito.Spy;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
@@ -70,7 +76,11 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
     protected NodeRef holdContainer;
     protected NodeRef hold;
     protected NodeRef hold2;
-    protected NodeRef notHold;
+    protected NodeRef recordFolder;
+    protected NodeRef record;
+    
+    @Mock(name="recordFolderService")   protected RecordFolderService mockedRecordFolderService;
+    @Mock(name="recordService")         protected RecordService mockedRecordService;
     
     @Spy @InjectMocks HoldServiceImpl holdService;
     
@@ -80,36 +90,30 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
     {
         super.before();
         
+        // setup objects used in mock interactions
         holdContainer = generateNodeRef(TYPE_HOLD_CONTAINER);
         hold = generateNodeRef(TYPE_HOLD);
         hold2 = generateNodeRef(TYPE_HOLD);
-        notHold = generateNodeRef(TYPE_RECORD_CATEGORY);
+        recordFolder = generateRecordFolder();
+        record = generateRecord();
         
-        when(mockedFilePlanService.getHoldContainer(filePlan)).thenReturn(holdContainer);   
+        // setup interactions
+        doReturn(holdContainer).when(mockedFilePlanService).getHoldContainer(filePlan);   
         
-    }
+        // set record as child of record folder
-
+        List<ChildAssociationRef> result = new ArrayList<ChildAssociationRef>(1);
-    @SuppressWarnings({ "unchecked", "rawtypes" })
+        result.add(new ChildAssociationRef(ContentModel.ASSOC_CONTAINS, recordFolder, generateQName(), record, true, 1));        
-    @Test
+        doReturn(result).when(mockedNodeService).getChildAssocs(eq(recordFolder), eq(ContentModel.ASSOC_CONTAINS), any(QNamePattern.class));
-    public void beforeDeleteNode()
+        doReturn(result).when(mockedNodeService).getParentAssocs(record);        
-    {
+        doReturn(Collections.singletonList(recordFolder)).when(mockedRecordFolderService).getRecordFolders(record);
-        doAnswer(new Answer<Void>()
+        doReturn(Collections.singletonList(record)).when(mockedRecordService).getRecords(recordFolder);
-        {
-            @Override
-            public Void answer(InvocationOnMock invocation) throws Exception
-            {
-                Object[] args = invocation.getArguments();
-                ((RunAsWork)args[0]).doWork();
-                return null;
-            }
-        }).when(holdService).runAsSystem(any(RunAsWork.class));
     }
     
     @Test
     public void isHold()
     {
         assertTrue(holdService.isHold(hold));
-        assertFalse(holdService.isHold(notHold));
+        assertFalse(holdService.isHold(recordFolder));
     }
     
     @Test
@@ -121,11 +125,8 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
         assertNotNull(emptyHoldList);
         assertTrue(emptyHoldList.isEmpty());
 
-        // set up list of two holds
+        // setup holds (hold1 and hold 2)
-        List<ChildAssociationRef> list = new ArrayList<ChildAssociationRef>(2);        
+        setupHolds();
-        list.add(new ChildAssociationRef(generateQName(), holdContainer, generateQName(), hold));
-        list.add(new ChildAssociationRef(generateQName(), holdContainer, generateQName(), hold2));        
-        when(mockedNodeService.getChildAssocs(holdContainer, ContentModel.ASSOC_CONTAINS, RegexQNamePattern.MATCH_ALL)).thenReturn(list);
         
         // with 2 holds
         List<NodeRef> holdsList = holdService.getHolds(filePlan);
@@ -136,13 +137,55 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
         // check one of the holds
         NodeRef holdFromList = holdsList.get(0);
         assertEquals(TYPE_HOLD, mockedNodeService.getType(holdFromList));    
+    }
     
+    private void setupHolds()
+    {
+        // set up list of two holds
+        List<ChildAssociationRef> list = new ArrayList<ChildAssociationRef>(2);        
+        list.add(new ChildAssociationRef(generateQName(), holdContainer, generateQName(), hold, true, 1));
+        list.add(new ChildAssociationRef(generateQName(), holdContainer, generateQName(), hold2, true, 2));        
+        when(mockedNodeService.getChildAssocs(holdContainer, ContentModel.ASSOC_CONTAINS, RegexQNamePattern.MATCH_ALL)).thenReturn(list);
     }
     
     @Test
-    public void heldBy()
+    public void heldByNothing()
     {
-        // TODO
+        // setup interactions
+        doReturn(new ArrayList<ChildAssociationRef>()).when(mockedNodeService).getParentAssocs(recordFolder, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS);
+        setupHolds();
+
+        // check that the record folder isn't held by anything
+        assertTrue(holdService.heldBy(recordFolder, true).isEmpty());
+        List<NodeRef> holds = holdService.heldBy(recordFolder, false);
+        assertEquals(2, holds.size());
+        
+        // check that the record isn't held by anything (record is a child of the record folder)
+        assertTrue(holdService.heldBy(record, true).isEmpty());
+        holds = holdService.heldBy(record, false);
+        assertEquals(2, holds.size());
+    }
+    
+    @Test
+    public void heldByMultipleResults()
+    {
+        // setup record folder in multiple holds
+        List<ChildAssociationRef> holds = new ArrayList<ChildAssociationRef>(2);
+        holds.add(new ChildAssociationRef(ASSOC_FROZEN_RECORDS, hold, ASSOC_FROZEN_RECORDS, recordFolder, true, 1));
+        holds.add(new ChildAssociationRef(ASSOC_FROZEN_RECORDS, hold2, ASSOC_FROZEN_RECORDS, recordFolder, true, 2));
+        doReturn(holds).when(mockedNodeService).getParentAssocs(recordFolder, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS);
+        
+        // check that both holds are found for record folder
+        List<NodeRef> heldByHolds = holdService.heldBy(recordFolder, true);
+        assertNotNull(heldByHolds);
+        assertEquals(2, heldByHolds.size());
+        assertTrue(holdService.heldBy(recordFolder, false).isEmpty());
+        
+        // check that both holds are found for record (even thou they are one level deep)
+        heldByHolds = holdService.heldBy(record, true);
+        assertNotNull(heldByHolds);
+        assertEquals(2, heldByHolds.size());
+        assertTrue(holdService.heldBy(record, false).isEmpty());
     }
     
     @Test (expected=AlfrescoRuntimeException.class)
@@ -151,7 +194,7 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
         // setup node service interactions
         when(mockedNodeService.getChildByName(eq(holdContainer), eq(ContentModel.ASSOC_CONTAINS), anyString())).thenReturn(null)
                                                                                                                .thenReturn(hold)
-                                                                                                               .thenReturn(notHold);
+                                                                                                               .thenReturn(recordFolder);
         
         // no hold
         NodeRef noHold = holdService.getHold(filePlan, "notAHold");
@@ -166,10 +209,29 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
         holdService.getHold(filePlan, "notHold");
     }
     
-    @Test
+    @Test (expected=RuntimeException.class)
-    public void getHeld()
+    public void getHeldNotAHold()
     {
+        holdService.getHeld(recordFolder);
+    }
     
+    @Test
+    public void getHeldNoResults()
+    {
+        assertTrue(holdService.getHeld(hold).isEmpty());
+    }
+    
+    @Test
+    public void getHeldWithResults()
+    {
+        // setup record folder in hold
+        List<ChildAssociationRef> holds = new ArrayList<ChildAssociationRef>(1);
+        holds.add(new ChildAssociationRef(ASSOC_FROZEN_RECORDS, hold, ASSOC_FROZEN_RECORDS, recordFolder, true, 1));
+        doReturn(holds).when(mockedNodeService).getChildAssocs(hold, ASSOC_FROZEN_RECORDS, RegexQNamePattern.MATCH_ALL);
+        
+        List<NodeRef> list = holdService.getHeld(hold);
+        assertEquals(1, list.size());
+        assertEquals(recordFolder, list.get(0));        
     }
     
     @SuppressWarnings({ "unchecked", "rawtypes" })
@@ -225,7 +287,7 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
         assertNull(holdService.getHoldReason(hold));
         
         // node isn't a hold
-        assertNull(holdService.getHoldReason(notHold));
+        assertNull(holdService.getHoldReason(recordFolder));
         
         // hold reason isn't set
         assertNull(holdService.getHoldReason(hold));
@@ -248,7 +310,7 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
         verify(mockedNodeService, never()).setProperty(hold, PROP_HOLD_REASON, HOLD_REASON);     
         
         // node isn't a hold
-        holdService.setHoldReason(notHold, HOLD_REASON);
+        holdService.setHoldReason(recordFolder, HOLD_REASON);
         verify(mockedNodeService, never()).setProperty(hold, PROP_HOLD_REASON, HOLD_REASON);  
         
         // set hold reason
@@ -256,23 +318,16 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
         verify(mockedNodeService).setProperty(hold, PROP_HOLD_REASON, HOLD_REASON);        
     }
     
+    @Test (expected=AlfrescoRuntimeException.class)
+    public void deleteHoldNotAHold()
+    {
+        holdService.deleteHold(recordFolder);
+        verify(mockedNodeService, never()).deleteNode(hold);        
+    }
+    
     @Test
     public void deleteHold()
     {
-        // setup node service interactions
-        when(mockedNodeService.exists(hold))
-            .thenReturn(false)
-            .thenReturn(true)
-            .thenReturn(true);
-        
-        // node does not exist
-        holdService.deleteHold(hold);
-        verify(mockedNodeService, never()).deleteNode(hold);     
-        
-        // node isn't a hold
-        holdService.deleteHold(notHold);
-        verify(mockedNodeService, never()).deleteNode(hold);  
-        
         // delete hold
         holdService.deleteHold(hold);
         verify(mockedNodeService).deleteNode(hold);   
@@ -280,33 +335,186 @@ public class HoldServiceImplUnitTest extends BaseUnitTest
         // TODO check interactions with policy component!!!
     }
     
-    @Test
+    @Test (expected=AlfrescoRuntimeException.class)
-    public void addToHold()
+    public void addToHoldNotAHold()
     {
-        // TODO
+        holdService.addToHold(recordFolder, recordFolder);
     }
     
+    @Test (expected=AlfrescoRuntimeException.class)
+    public void addToHoldNotARecordFolderOrRecord()
+    {
+        holdService.addToHold(hold, filePlanComponent);
+    }
+    
+    @SuppressWarnings("unchecked")
+    @Test
+    public void addToHoldNotInHold()
+    {
+        holdService.addToHold(hold, recordFolder);
+        
+        verify(mockedNodeService).addChild(hold, recordFolder, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS);
+        verify(mockedNodeService).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class));
+        verify(mockedNodeService).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class));
+        
+        holdService.addToHold(hold, record);
+        verify(mockedNodeService).addChild(hold, record, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS);
+        verify(mockedNodeService).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class));
+        verify(mockedNodeService, times(2)).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class));
+    }
+    
+    @SuppressWarnings("unchecked")
+    @Test
+    public void addToHoldAlreadyInHold()
+    {
+        doReturn(Collections.singletonList(recordFolder)).when(holdService).getHeld(hold);
+        
+        holdService.addToHold(hold, recordFolder);
+        
+        verify(mockedNodeService, never()).addChild(hold, recordFolder, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS);
+        verify(mockedNodeService, never()).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class));
+        verify(mockedNodeService, never()).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class));
+        
+    }
+    
+    @SuppressWarnings("unchecked")
+    @Test
+    public void addToHoldAldeadyFrozen()
+    {
+        doReturn(true).when(mockedNodeService).hasAspect(recordFolder, ASPECT_FROZEN);
+        doReturn(true).when(mockedNodeService).hasAspect(record, ASPECT_FROZEN);
+        
+        holdService.addToHold(hold, recordFolder);
+        
+        verify(mockedNodeService, times(1)).addChild(hold, recordFolder, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS);
+        verify(mockedNodeService, never()).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class));
+        verify(mockedNodeService, never()).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class));
+    }
+    
+    @SuppressWarnings("unchecked")
     @Test
     public void addToHolds()
     {
-        // TODO
+        // ensure the interaction indicates that a node has the frozen aspect applied if it has
+        doAnswer(new Answer<Void>()
+        {
+            public Void answer(InvocationOnMock invocation)
+            {
+                NodeRef nodeRef = (NodeRef)invocation.getArguments()[0];
+                doReturn(true).when(mockedNodeService).hasAspect(nodeRef, ASPECT_FROZEN);
+                return null;
+            }
+            
+        }).when(mockedNodeService).addAspect(any(NodeRef.class), eq(ASPECT_FROZEN), any(Map.class));
+        
+        // build a list of holds
+        List<NodeRef> holds = new ArrayList<NodeRef>(2);
+        holds.add(hold);
+        holds.add(hold2);
+        
+        // add the record folder to both holds
+        holdService.addToHolds(holds, recordFolder);
+        
+        // verify the interactions
+        verify(mockedNodeService, times(1)).addChild(hold, recordFolder, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS);
+        verify(mockedNodeService, times(1)).addChild(hold2, recordFolder, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS);
+        verify(mockedNodeService, times(1)).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class));
+        verify(mockedNodeService, times(1)).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class));                
+    }
+    
+    @Test (expected=AlfrescoRuntimeException.class)
+    public void removeFromHoldNotAHold()
+    {
+        holdService.removeFromHold(recordFolder, recordFolder);
+    }
+    
+    @Test
+    public void removeFromHoldNotInHold()
+    {
+        holdService.removeFromHold(hold, recordFolder);
+        
+        verify(mockedNodeService, never()).removeChild(hold, recordFolder);
+        verify(mockedNodeService, never()).removeAspect(recordFolder, ASPECT_FROZEN);
+        verify(mockedNodeService, never()).removeAspect(record, ASPECT_FROZEN);
     }
     
     @Test
     public void removeFromHold()
     {
-        // TODO
+        doReturn(Collections.singletonList(recordFolder)).when(holdService).getHeld(hold);
+        doReturn(true).when(mockedNodeService).hasAspect(recordFolder, ASPECT_FROZEN);
+        doReturn(true).when(mockedNodeService).hasAspect(record, ASPECT_FROZEN);
+        
+        holdService.removeFromHold(hold, recordFolder);
+        
+        verify(mockedNodeService, times(1)).removeChild(hold, recordFolder);
+        verify(mockedNodeService, times(1)).removeAspect(recordFolder, ASPECT_FROZEN);
+        verify(mockedNodeService, times(1)).removeAspect(record, ASPECT_FROZEN);
     }
     
     @Test
     public void removeFromHolds()
     {
-        // TODO
+        doReturn(Collections.singletonList(recordFolder)).when(holdService).getHeld(hold);
+        doReturn(Collections.singletonList(recordFolder)).when(holdService).getHeld(hold2);
+        doReturn(true).when(mockedNodeService).hasAspect(recordFolder, ASPECT_FROZEN);
+        doReturn(true).when(mockedNodeService).hasAspect(record, ASPECT_FROZEN);
+        
+        // build a list of holds
+        List<NodeRef> holds = new ArrayList<NodeRef>(2);
+        holds.add(hold);
+        holds.add(hold2);
+        
+        holdService.removeFromHolds(holds, recordFolder);
+        
+        verify(mockedNodeService, times(1)).removeChild(hold, recordFolder);
+        verify(mockedNodeService, times(1)).removeChild(hold2, recordFolder);
+        verify(mockedNodeService, times(1)).removeAspect(recordFolder, ASPECT_FROZEN);
+        verify(mockedNodeService, times(1)).removeAspect(record, ASPECT_FROZEN);
     }
     
     @Test
     public void removeFromAllHolds()
     {
+        // build a list of holds
+        List<NodeRef> holds = new ArrayList<NodeRef>(2);
+        holds.add(hold);
+        holds.add(hold2);
         
+        doAnswer(new Answer<Void>()
+        {
+            public Void answer(InvocationOnMock invocation)
+            {
+                doReturn(Collections.singletonList(hold2)).when(holdService).heldBy(recordFolder, true);
+                return null;
+            }
+            
+        }).when(mockedNodeService).removeChild(hold, recordFolder);
+        
+        doAnswer(new Answer<Void>()
+        {
+            public Void answer(InvocationOnMock invocation)
+            {
+                doReturn(new ArrayList<NodeRef>()).when(holdService).heldBy(recordFolder, true);
+                return null;
+            }
+            
+        }).when(mockedNodeService).removeChild(hold2, recordFolder);
+        
+        // define interactions
+        doReturn(holds).when(holdService).heldBy(recordFolder, true);
+        doReturn(Collections.singletonList(recordFolder)).when(holdService).getHeld(hold);
+        doReturn(Collections.singletonList(recordFolder)).when(holdService).getHeld(hold2);
+        doReturn(true).when(mockedNodeService).hasAspect(recordFolder, ASPECT_FROZEN);
+        doReturn(true).when(mockedNodeService).hasAspect(record, ASPECT_FROZEN);
+        
+        // remove record folder from all holds
+        holdService.removeFromAllHolds(recordFolder);
+        
+        // verify interactions
+        verify(mockedNodeService, times(1)).removeChild(hold, recordFolder);
+        verify(mockedNodeService, times(1)).removeChild(hold2, recordFolder);
+        verify(mockedNodeService, times(1)).removeAspect(recordFolder, ASPECT_FROZEN);
+        verify(mockedNodeService, times(1)).removeAspect(record, ASPECT_FROZEN);        
     }
 }

rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/record/RecordMetadataBootstrapUnitTest.java

@@ -36,7 +36,7 @@ import org.mockito.Mock;
  */
 public class RecordMetadataBootstrapUnitTest extends BaseUnitTest
 {
-    @Mock(name="recordService") private RecordService mockedRecordService;   
+    @Mock(name="recordService")         protected RecordService mockedRecordService;
     
     @InjectMocks private RecordMetadataBootstrap bootstrap;