diff --git a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java index 105252e84e..241c2c7e99 100644 --- a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java +++ b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java @@ -218,7 +218,8 @@ public class HoldServiceImpl extends ServiceBaseImpl for (ChildAssociationRef holdAssoc : holdsAssocs) { NodeRef hold = holdAssoc.getChildRef(); - if (isHold(hold)) + boolean hasPermissionOnHold = (permissionService.hasPermission(hold, RMPermissionModel.FILING) == AccessStatus.ALLOWED); + if (isHold(hold) && hasPermissionOnHold) { // add to list of holds holds.add(hold); diff --git a/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/service/HoldServiceImplTest.java b/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/service/HoldServiceImplTest.java index 20fb38c2a3..b3f602481e 100644 --- a/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/service/HoldServiceImplTest.java +++ b/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/service/HoldServiceImplTest.java @@ -18,10 +18,12 @@ */ package org.alfresco.module.org_alfresco_module_rm.test.service; +import java.util.ArrayList; import java.util.List; import org.alfresco.error.AlfrescoRuntimeException; import org.alfresco.module.org_alfresco_module_rm.capability.RMPermissionModel; +import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase; import org.alfresco.service.cmr.repository.NodeRef; @@ -33,6 +35,14 @@ import org.alfresco.service.cmr.repository.NodeRef; */ public class HoldServiceImplTest extends BaseRMTestCase { + /** Constants for the holds */ + private static final String HOLD1_NAME = "hold one"; + private static final String HOLD2_NAME = "hold two"; + private static final String HOLD1_REASON = "I have my reasons"; + private static final String HOLD2_REASON = "secrets are everything"; + private static final String HOLD1_DESC = "but I'll not describe them here!"; + private static final String HOLD2_DESC = "no then! that's just not on!"; + @Override protected boolean isRecordTest() { @@ -45,6 +55,34 @@ public class HoldServiceImplTest extends BaseRMTestCase return true; } + /** + * Creates a hold and checks if the hold is null or not + * + * @return {@link NodeRef} Node reference of the hold. + */ + private NodeRef createAndCheckHold() + { + NodeRef hold = holdService.createHold(filePlan, HOLD1_NAME, HOLD1_REASON, HOLD1_DESC); + assertNotNull(hold); + return hold; + } + + /** + * Creates two holds and checks them if they are null or not + * + * @return List of {@link NodeRef}s of the holds. + */ + private List createAndCheckHolds() + { + List holds = new ArrayList(2); + holds.add(createAndCheckHold()); + NodeRef hold2 = holdService.createHold(filePlan, HOLD2_NAME, HOLD2_REASON, HOLD2_DESC); + assertNotNull(hold2); + holds.add(hold2); + assertEquals(2, holds.size()); + return holds; + } + public void testDeleteHoldBehaviourForRecordFolder() { doTestInTransaction(new Test() @@ -53,8 +91,7 @@ public class HoldServiceImplTest extends BaseRMTestCase public Void run() throws Exception { // create test holds - NodeRef hold1 = holdService.createHold(filePlan, "hold one", "I have my reasons", "but I'll not describe them here!"); - assertNotNull(hold1); + NodeRef hold1 = createAndCheckHold(); // add the record folder to hold1 holdService.addToHold(hold1, rmFolder); @@ -94,10 +131,9 @@ public class HoldServiceImplTest extends BaseRMTestCase public Void run() throws Exception { // create test holds - NodeRef hold1 = holdService.createHold(filePlan, "hold one", "I have my reasons", "but I'll not describe them here!"); - assertNotNull(hold1); - NodeRef hold2 = holdService.createHold(filePlan, "hold two", "secrets are everything", "no then! that's just not on!"); - assertNotNull(hold2); + List holds = createAndCheckHolds(); + NodeRef hold1 = holds.get(0); + NodeRef hold2 = holds.get(1); // add the record folder to hold1 holdService.addToHold(hold1, rmFolder); @@ -150,8 +186,7 @@ public class HoldServiceImplTest extends BaseRMTestCase public void testAddRecordFolderToHoldWithoutFilingPermissionOnRecordFolder() { // Create hold - final NodeRef hold = holdService.createHold(filePlan, "hold one", "I have my reasons", "but I'll not describe them here!"); - assertNotNull(hold); + final NodeRef hold = createAndCheckHold(); doTestInTransaction(new Test() { @@ -169,7 +204,7 @@ public class HoldServiceImplTest extends BaseRMTestCase return null; } - }, "admin"); + }); doTestInTransaction(new FailureTest(AlfrescoRuntimeException.class) { @@ -184,8 +219,7 @@ public class HoldServiceImplTest extends BaseRMTestCase public void testAddRecordFolderToHoldWithoutFilingPermissionOnHold() { // Create hold - final NodeRef hold = holdService.createHold(filePlan, "hold one", "I have my reasons", "but I'll not describe them here!"); - assertNotNull(hold); + final NodeRef hold = createAndCheckHold(); doTestInTransaction(new Test() { @@ -203,7 +237,7 @@ public class HoldServiceImplTest extends BaseRMTestCase return null; } - }, "admin"); + }); doTestInTransaction(new FailureTest(AlfrescoRuntimeException.class) { @@ -215,4 +249,115 @@ public class HoldServiceImplTest extends BaseRMTestCase }, userName); } + public void testGettingHolds() + { + final List listWithTwoHolds = new ArrayList(2); + + doTestInTransaction(new Test() + { + @Override + public Void run() throws Exception + { + // No holds + List emptyHoldList = holdService.getHolds(filePlan); + assertNotNull(emptyHoldList); + assertTrue(emptyHoldList.isEmpty()); + + // Create 2 holds + createAndCheckHolds(); + + // Check the list of holds + listWithTwoHolds.addAll(holdService.getHolds(filePlan)); + assertNotNull(listWithTwoHolds); + assertEquals(2, listWithTwoHolds.size()); + + // Check the first hold + NodeRef hold1 = listWithTwoHolds.get(0); + assertEquals(RecordsManagementModel.TYPE_HOLD, nodeService.getType(hold1)); + assertEquals(HOLD1_NAME, (String) nodeService.getProperty(hold1, PROP_NAME)); + assertEquals(HOLD1_REASON, (String) nodeService.getProperty(hold1, PROP_HOLD_REASON)); + assertEquals(HOLD1_DESC, (String) nodeService.getProperty(hold1, PROP_DESCRIPTION)); + + // Add the user to the RM Manager role + filePlanRoleService.assignRoleToAuthority(filePlan, ROLE_NAME_RECORDS_MANAGER, userName); + + return null; + } + }); + + doTestInTransaction(new Test() + { + @Override + public Void run() throws Exception + { + // Get the holds the test user without having any permissions on the holds + List holds = holdService.getHolds(filePlan); + assertNotNull(holds); + assertEquals(0, holds.size()); + + return null; + } + }, userName); + + final NodeRef hold2 = listWithTwoHolds.get(1); + doTestInTransaction(new Test() + { + @Override + public Void run() throws Exception + { + // Give the user read permissions on the hold + permissionService.setPermission(hold2, userName, RMPermissionModel.FILING, true); + + return null; + } + }); + + doTestInTransaction(new Test() + { + @Override + public Void run() throws Exception + { + List holds = holdService.getHolds(filePlan); + assertNotNull(holds); + assertEquals(1, holds.size()); + assertEquals(RecordsManagementModel.TYPE_HOLD, nodeService.getType(hold2)); + assertEquals(HOLD2_NAME, (String) nodeService.getProperty(hold2, PROP_NAME)); + assertEquals(HOLD2_REASON, (String) nodeService.getProperty(hold2, PROP_HOLD_REASON)); + assertEquals(HOLD2_DESC, (String) nodeService.getProperty(hold2, PROP_DESCRIPTION)); + + return null; + } + }, userName); + } + + public void testHeldByNothing() + { + doTestInTransaction(new Test() + { + @Override + public Void run() throws Exception + { + // Create the test holds + createAndCheckHolds(); + + // Check that the record folder isn't held by anything + List holds = new ArrayList(); + holds.addAll(holdService.heldBy(rmFolder, true)); + assertTrue(holds.isEmpty()); + holds.clear(); + holds.addAll(holdService.heldBy(rmFolder, false)); + assertEquals(2, holds.size()); + + // Check that record isn't held by anything (recordOne is a child of the rmFolder) + holds.clear(); + holds.addAll(holdService.heldBy(recordOne, true)); + assertTrue(holds.isEmpty()); + holds.clear(); + holds.addAll(holdService.heldBy(recordOne, false)); + assertEquals(2, holds.size()); + + return null; + } + }); + } } diff --git a/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java b/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java index 3588c9b706..70345de440 100644 --- a/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java +++ b/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java @@ -71,7 +71,7 @@ public class HoldServiceImplUnitTest extends BaseUnitTest protected NodeRef holdContainer; protected NodeRef hold; - protected NodeRef hold2; + protected NodeRef hold2; @Spy @InjectMocks HoldServiceImpl holdService; @@ -88,8 +88,6 @@ public class HoldServiceImplUnitTest extends BaseUnitTest // setup interactions doReturn(holdContainer).when(mockedFilePlanService).getHoldContainer(filePlan); - - } @Test @@ -99,56 +97,6 @@ public class HoldServiceImplUnitTest extends BaseUnitTest assertFalse(holdService.isHold(recordFolder)); } - @Test - public void getHolds() - { - // with no holds - List emptyHoldList = holdService.getHolds(filePlan); - verify(mockedNodeService).getChildAssocs(holdContainer, ContentModel.ASSOC_CONTAINS, RegexQNamePattern.MATCH_ALL); - assertNotNull(emptyHoldList); - assertTrue(emptyHoldList.isEmpty()); - - // setup holds (hold1 and hold 2) - setupHolds(); - - // with 2 holds - List holdsList = holdService.getHolds(filePlan); - verify(mockedNodeService, times(2)).getChildAssocs(holdContainer, ContentModel.ASSOC_CONTAINS, RegexQNamePattern.MATCH_ALL); - assertNotNull(holdsList); - assertEquals(2, holdsList.size()); - - // 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 list = new ArrayList(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 heldByNothing() - { - // setup interactions - doReturn(new ArrayList()).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 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() { @@ -357,7 +305,6 @@ public class HoldServiceImplUnitTest extends BaseUnitTest 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")