From b2ae5242e9bed664963f16ce4f1e9b851cc5893c Mon Sep 17 00:00:00 2001 From: Sara Aspery Date: Mon, 18 Nov 2019 01:25:34 +0000 Subject: [PATCH] RM-6930 review updates --- .../hold/HoldServiceImpl.java | 4 +- .../hold/HoldServiceImplUnitTest.java | 78 ++++++++++++------- 2 files changed, 50 insertions(+), 32 deletions(-) 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 c5e0e9a151..01e56a7026 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 @@ -578,7 +578,7 @@ public class HoldServiceImpl extends ServiceBaseImpl } catch (AccessDeniedException ade) { - throw new AlfrescoRuntimeException(I18NUtil.getMessage(MSG_ERR_HOLD_PERMISSION_GENERIC_ERROR), ade); + throw new AccessDeniedException(I18NUtil.getMessage(MSG_ERR_HOLD_PERMISSION_GENERIC_ERROR), ade); } } @@ -592,7 +592,7 @@ public class HoldServiceImpl extends ServiceBaseImpl sb.append(name); sb.append("'"); } - throw new AlfrescoRuntimeException(I18NUtil.getMessage(MSG_ERR_HOLD_PERMISSION_DETAILED_ERROR) + sb.toString()); + throw new AccessDeniedException(I18NUtil.getMessage(MSG_ERR_HOLD_PERMISSION_DETAILED_ERROR) + sb.toString()); } invokeBeforeDeleteHold(hold); 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 c4a78ffc30..875066d10a 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 @@ -222,9 +222,7 @@ 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()); + mockPoliciesForCreateHold(); // create hold NodeRef newHold = holdService.createHold(filePlan, HOLD_NAME, HOLD_REASON, HOLD_DESCRIPTION); @@ -312,9 +310,7 @@ public class HoldServiceImplUnitTest extends BaseUnitTest @Test public void deleteHold() { - // mocks for policies - doNothing().when(holdService).invokeBeforeDeleteHold(any()); - doNothing().when(holdService).invokeOnDeleteHold(any(), any()); + mockPoliciesForDeleteHold(); // delete hold holdService.deleteHold(hold); @@ -597,17 +593,14 @@ public class HoldServiceImplUnitTest extends BaseUnitTest @Test (expected = AlfrescoRuntimeException.class) public void testDeleteHoldThrowsExceptionForActiveContentWithoutReadPermission() { - NodeRef holdNode = generateNodeRef(TYPE_HOLD); NodeRef heldContent = generateNodeRef(TYPE_CONTENT); - List holds = new ArrayList<>(2); - holds.add(new ChildAssociationRef(ASSOC_FROZEN_CONTENT, holdNode, ASSOC_FROZEN_CONTENT, heldContent, true, 1)); - when(mockedNodeService.getChildAssocs(holdNode, ASSOC_FROZEN_CONTENT, RegexQNamePattern.MATCH_ALL)).thenReturn(holds); - when(mockedRecordService.isRecord(heldContent)).thenReturn(false); - when(mockedRecordFolderService.isRecordFolder(heldContent)).thenReturn(false); + List holds = createListOfHoldAssociations(heldContent); + + when(mockedNodeService.getChildAssocs(hold, ASSOC_FROZEN_CONTENT, RegexQNamePattern.MATCH_ALL)).thenReturn(holds); when(mockedPermissionService.hasPermission(heldContent, PermissionService.READ)).thenReturn(AccessStatus.DENIED); when(mockedNodeService.getProperty(heldContent, ContentModel.PROP_NAME)).thenReturn("foo"); - holdService.deleteHold(holdNode); + holdService.deleteHold(hold); } /** @@ -616,15 +609,13 @@ public class HoldServiceImplUnitTest extends BaseUnitTest @Test (expected = AlfrescoRuntimeException.class) public void testDeleteHoldThrowsExceptionForARecordWithoutReadPermission() { - NodeRef holdNode = generateNodeRef(TYPE_HOLD); NodeRef heldContent = generateNodeRef(); - List holds = new ArrayList<>(2); - holds.add(new ChildAssociationRef(ASSOC_FROZEN_CONTENT, holdNode, ASSOC_FROZEN_CONTENT, heldContent, true, 1)); - when(mockedNodeService.getChildAssocs(holdNode, ASSOC_FROZEN_CONTENT, RegexQNamePattern.MATCH_ALL)).thenReturn(holds); + List holds = createListOfHoldAssociations(heldContent); + + when(mockedNodeService.getChildAssocs(hold, ASSOC_FROZEN_CONTENT, RegexQNamePattern.MATCH_ALL)).thenReturn(holds); when(mockedRecordService.isRecord(heldContent)).thenThrow(new AccessDeniedException("")); - holdService.deleteHold(holdNode); - + holdService.deleteHold(hold); } /** @@ -633,16 +624,15 @@ public class HoldServiceImplUnitTest extends BaseUnitTest @Test (expected = AlfrescoRuntimeException.class) public void testDeleteHoldThrowsExceptionForARecordWithoutFilePermission() { - NodeRef holdNode = generateNodeRef(TYPE_HOLD); NodeRef heldContent = generateNodeRef(); - List holds = new ArrayList<>(2); - holds.add(new ChildAssociationRef(ASSOC_FROZEN_CONTENT, holdNode, ASSOC_FROZEN_CONTENT, heldContent, true, 1)); - when(mockedNodeService.getChildAssocs(holdNode, ASSOC_FROZEN_CONTENT, RegexQNamePattern.MATCH_ALL)).thenReturn(holds); + List holds = createListOfHoldAssociations(heldContent); + + when(mockedNodeService.getChildAssocs(hold, ASSOC_FROZEN_CONTENT, RegexQNamePattern.MATCH_ALL)).thenReturn(holds); when(mockedRecordService.isRecord(heldContent)).thenReturn(true); when(mockedPermissionService.hasPermission(heldContent, RMPermissionModel.FILING)).thenReturn(AccessStatus.DENIED); when(mockedNodeService.getProperty(heldContent, ContentModel.PROP_NAME)).thenReturn("foo"); - holdService.deleteHold(holdNode); + holdService.deleteHold(hold); } /** @@ -651,19 +641,47 @@ public class HoldServiceImplUnitTest extends BaseUnitTest @Test public void testDeleteHoldChecksReadPermissionForActiveContent() { - NodeRef holdNode = generateNodeRef(TYPE_HOLD); NodeRef heldContent = generateNodeRef(TYPE_CONTENT); - List holds = new ArrayList<>(2); - holds.add(new ChildAssociationRef(ASSOC_FROZEN_CONTENT, holdNode, ASSOC_FROZEN_CONTENT, heldContent, true, 1)); - when(mockedNodeService.getChildAssocs(holdNode, ASSOC_FROZEN_CONTENT, RegexQNamePattern.MATCH_ALL)).thenReturn(holds); + List holds = createListOfHoldAssociations(heldContent); + + mockPoliciesForDeleteHold(); + when(mockedNodeService.getChildAssocs(hold, ASSOC_FROZEN_CONTENT, RegexQNamePattern.MATCH_ALL)).thenReturn(holds); when(mockedRecordService.isRecord(heldContent)).thenReturn(false); when(mockedRecordFolderService.isRecordFolder(heldContent)).thenReturn(false); when(mockedPermissionService.hasPermission(heldContent, PermissionService.READ)).thenReturn(AccessStatus.ALLOWED); when(mockedNodeService.getProperty(heldContent, ContentModel.PROP_NAME)).thenReturn("foo"); - holdService.deleteHold(holdNode); + holdService.deleteHold(hold); - verify(mockedNodeService, times(1)).deleteNode(holdNode); + verify(mockedNodeService, times(1)).deleteNode(hold); + } + + /** + * Helper method to create hold and associations with given content + */ + private List createListOfHoldAssociations(NodeRef heldContent) + { + List holds = new ArrayList<>(2); + holds.add(new ChildAssociationRef(ASSOC_FROZEN_CONTENT, hold, ASSOC_FROZEN_CONTENT, heldContent, true, 1)); + return holds; + } + + /** + * mocks policies for create hold + */ + private void mockPoliciesForCreateHold() + { + doNothing().when(holdService).invokeBeforeCreateHold(any(), anyString(), anyString()); + doNothing().when(holdService).invokeOnCreateHold(any()); + } + + /** + * mocks policies for delete hold + */ + private void mockPoliciesForDeleteHold() + { + doNothing().when(holdService).invokeBeforeDeleteHold(any()); + doNothing().when(holdService).invokeOnDeleteHold(any(), any()); } /**