diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java index 657e8d8e7c..e1dd5c0e07 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java @@ -303,6 +303,8 @@ public class HoldServiceImpl extends ServiceBaseImpl if (!includedInHold) { // invert list to get list of holds that do not contain this node + //TODO Find a way to get rid of the isFilePlanComponent(nodeRef) check. Currently it is used because + // integration tests create multiple rm sites with generated ids NodeRef filePlan = isFilePlanComponent(nodeRef) ? filePlanService.getFilePlan(nodeRef) : filePlanService.getFilePlanBySiteId(FilePlanService.DEFAULT_RM_SITE_ID); List allHolds = getHolds(filePlan); result = ListUtils.subtract(allHolds, new ArrayList<>(holdsNotIncludingNodeRef)); diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddActiveContentToHoldTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddActiveContentToHoldTest.java index 77dd7322c3..8755328eea 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddActiveContentToHoldTest.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddActiveContentToHoldTest.java @@ -59,6 +59,14 @@ public class AddActiveContentToHoldTest extends BaseRMTestCase return true; } + /** + * Given active content + * And file permission on the hold + * And the appropriate capability to add to hold + * When I try to add the active content to the hold + * Then the active content is frozen + * And the active content is contained within the hold + */ public void testAddDocumentToHold() { doBehaviourDrivenTest(new BehaviourDrivenTest() @@ -111,6 +119,12 @@ public class AddActiveContentToHoldTest extends BaseRMTestCase }); } + /** + * Given active content + * And a non rm user with write permission on active content + * When user tries to add the active content to hold + * Then AccessDeniedException is thrown + */ public void testAddDocumentToHoldAsNonRMUser() { doBehaviourDrivenTest(new BehaviourDrivenTest(AccessDeniedException.class) @@ -141,6 +155,12 @@ public class AddActiveContentToHoldTest extends BaseRMTestCase }); } + /** + * Given active content + * And a rm user with Filing permission on hold and Add to Hold Capability, but only read permission on active content + * When user tries to add the active content to hold + * Then an exception is thrown + */ public void testAddDocumentToHoldNoWritePermissionOnDoc() { doBehaviourDrivenTest(new BehaviourDrivenTest() @@ -183,9 +203,15 @@ public class AddActiveContentToHoldTest extends BaseRMTestCase }); } + /** + * Given active content + * And a rm user with Add to Hold Capability, write permission on active content and only Read permission on hold + * When user tries to add the active content to hold + * Then AccessDeniedException is thrown + */ public void testAddDocumentToHoldNoFilingPermissionOnHold() { - doBehaviourDrivenTest(new BehaviourDrivenTest(recordsManagerName, false) + doBehaviourDrivenTest(new BehaviourDrivenTest(AccessDeniedException.class, recordsManagerName, false) { private NodeRef hold; @@ -213,28 +239,22 @@ public class AddActiveContentToHoldTest extends BaseRMTestCase public void when() { - AuthenticationUtil.runAs( - (RunAsWork) () -> { - // add the active content to hold as a RM manager who has Add to Hold Capability and write permission on - // doc, but no filing permission on hold - try - { - holdService.addToHold(hold, dmDocument); - fail("Expected AccessDeniedException to be thrown."); - } - catch (AccessDeniedException e) - { - //expected - } - return null; - }, recordsManagerName); + // add the active content to hold as a RM manager who has Add to Hold Capability and write permission on + // doc, but no filing permission on hold + holdService.addToHold(hold, dmDocument); } }); } + /** + * Given active content + * And a rm user with write permission on active content and Filing permission on hold, but no Add to Hold Capability + * When user tries to add the active content to hold + * Then AccessDeniedException is thrown + */ public void testAddDocumentToHoldNoCapability() { - doBehaviourDrivenTest(new BehaviourDrivenTest(powerUserName, false) + doBehaviourDrivenTest(new BehaviourDrivenTest(AccessDeniedException.class, powerUserName, false) { private NodeRef hold; @@ -264,19 +284,7 @@ public class AddActiveContentToHoldTest extends BaseRMTestCase { // add the active content to hold as a RM power user who has write permission on doc and filing // permission on hold, but no Add To Hold capability - AuthenticationUtil.runAs( - (RunAsWork) () -> { - try - { - holdService.addToHold(hold, dmDocument); - fail("Expected AccessDeniedException to be thrown."); - } - catch (AccessDeniedException e) - { - //expected - } - return null; - }, powerUserName); + holdService.addToHold(hold, dmDocument); } }); } diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/BaseRMTestCase.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/BaseRMTestCase.java index 204ca4c8e8..21cb4ce5d8 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/BaseRMTestCase.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/BaseRMTestCase.java @@ -922,6 +922,13 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase } } + public BehaviourDrivenTest(Class expectedException, String runAsUser, boolean runInTransactionTests) + { + this.expectedException = expectedException; + this.runAsUser = runAsUser; + this.runInTransactionTests = runInTransactionTests; + } + public void given() throws Exception { /** empty implementation */ } public void when() throws Exception { /** empty implementation */ } diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java index 19fa820027..8bcd8980fc 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java @@ -64,7 +64,9 @@ import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; import org.alfresco.service.namespace.RegexQNamePattern; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Spy; @@ -83,6 +85,7 @@ public class HoldServiceImplUnitTest extends BaseUnitTest private static final String HOLD_NAME = "holdname"; private static final String HOLD_REASON = "holdreason"; private static final String HOLD_DESCRIPTION = "holddescription"; + private static final String ACTIVE_CONTENT_NAME = "activeContentName"; protected NodeRef holdContainer; protected NodeRef hold; @@ -91,6 +94,9 @@ public class HoldServiceImplUnitTest extends BaseUnitTest @Spy @InjectMocks HoldServiceImpl holdService; + @Rule + public ExpectedException expectedEx = ExpectedException.none(); + @Before @Override public void before() throws Exception @@ -376,16 +382,25 @@ public class HoldServiceImplUnitTest extends BaseUnitTest verify(mockedRecordsManagementAuditService).auditEvent(eq(activeContent), anyString()); } - @Test (expected = AlfrescoRuntimeException.class) + @Test public void addActiveContentToHoldNoPermissionsOnHold() { + expectedEx.expect(AlfrescoRuntimeException.class); + expectedEx.expectMessage("'" + ACTIVE_CONTENT_NAME + "' can't be added to the hold container as filing permission for '" + HOLD_NAME + "' is needed."); + + when(mockedNodeService.getProperty(activeContent, ContentModel.PROP_NAME)).thenReturn(ACTIVE_CONTENT_NAME); + when(mockedNodeService.getProperty(hold, ContentModel.PROP_NAME)).thenReturn(HOLD_NAME); when(mockedPermissionService.hasPermission(hold, RMPermissionModel.FILING)).thenReturn(AccessStatus.DENIED); holdService.addToHold(hold, activeContent); } - @Test (expected = AlfrescoRuntimeException.class) + @Test public void addActiveContentToHoldNoPermissionsOnContent() { + expectedEx.expect(AlfrescoRuntimeException.class); + expectedEx.expectMessage("Write permission on '" + ACTIVE_CONTENT_NAME + "' is needed."); + + when(mockedNodeService.getProperty(activeContent, ContentModel.PROP_NAME)).thenReturn(ACTIVE_CONTENT_NAME); when(mockedPermissionService.hasPermission(activeContent, PermissionService.WRITE)).thenReturn(AccessStatus.DENIED); holdService.addToHold(hold, activeContent); }