From 22a1c93c8315d666b8f0b83852f196459ce5c903 Mon Sep 17 00:00:00 2001 From: Chris Shields Date: Tue, 19 Nov 2019 12:05:06 +0000 Subject: [PATCH 1/3] RM-7029 Audit remove from hold --- .../messages/audit-service.properties | 1 + .../rm-audit-context.xml | 6 + .../audit/event/RemoveFromHoldAuditEvent.java | 82 ++++++++ .../hold/HoldServiceImpl.java | 17 -- ...RecordsManagementAuditServiceImplTest.java | 193 ++++++++++++++++++ .../test/util/CommonRMTestUtils.java | 23 +++ .../RemoveFromHoldAuditEventUnitTest.java | 95 +++++++++ .../hold/HoldServiceImplUnitTest.java | 2 - 8 files changed, 400 insertions(+), 19 deletions(-) create mode 100644 rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/RemoveFromHoldAuditEvent.java create mode 100644 rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/audit/event/RemoveFromHoldAuditEventUnitTest.java diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/audit-service.properties b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/audit-service.properties index da0adf2b30..b5e7c4079c 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/audit-service.properties +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/audit-service.properties @@ -16,6 +16,7 @@ rm.audit.fileTo=File to rm.audit.createHold=Create Hold rm.audit.deleteHold=Delete Hold rm.audit.addToHold=Add To Hold +rm.audit.removeFromHold=Remove From Hold rm.audit.audit-start=Audit Start rm.audit.audit-stop=Audit Stop rm.audit.audit-clear=Audit Clear diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-audit-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-audit-context.xml index be43164702..ac1382d699 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-audit-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-audit-context.xml @@ -147,4 +147,10 @@ + + + + + + diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/RemoveFromHoldAuditEvent.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/RemoveFromHoldAuditEvent.java new file mode 100644 index 0000000000..48f419f701 --- /dev/null +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/RemoveFromHoldAuditEvent.java @@ -0,0 +1,82 @@ +/* + * #%L + * Alfresco Records Management Module + * %% + * Copyright (C) 2005 - 2019 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * - + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * - + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * - + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * - + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ + +package org.alfresco.module.org_alfresco_module_rm.audit.event; + +import java.io.Serializable; +import java.util.Map; + +import org.alfresco.model.ContentModel; +import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies; +import org.alfresco.repo.policy.Behaviour.NotificationFrequency; +import org.alfresco.repo.policy.annotation.Behaviour; +import org.alfresco.repo.policy.annotation.BehaviourBean; +import org.alfresco.repo.policy.annotation.BehaviourKind; +import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.cmr.repository.NodeService; +import org.alfresco.service.namespace.QName; + +/** + * Delete from hold audit event. + * + * @author Chris Shields + * @since 3.3 + */ +@BehaviourBean +public class RemoveFromHoldAuditEvent extends AuditEvent implements HoldServicePolicies.OnRemoveFromHoldPolicy +{ + /** Node Service */ + private NodeService nodeService; + + /** + * Sets the node service + * + * @param nodeService nodeService to set + */ + public void setNodeService(NodeService nodeService) + { + this.nodeService = nodeService; + } + + /** + * @see @see org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.OnRemoveFromHoldPolicy#onRemoveFromHold(org.alfresco.service.cmr.repository.NodeRef, org.alfresco.service.cmr.repository.NodeRef) + */ + @Override + @Behaviour + ( + kind = BehaviourKind.CLASS, + type = "rma:hold", + notificationFrequency = NotificationFrequency.EVERY_EVENT + ) + public void onRemoveFromHold(NodeRef holdNodeRef, NodeRef contentNodeRef) + { + Map auditProperties = HoldUtils.makePropertiesMap(holdNodeRef, nodeService); + auditProperties.put(ContentModel.PROP_NAME, nodeService.getProperty(contentNodeRef, ContentModel.PROP_NAME)); + + recordsManagementAuditService.auditEvent(contentNodeRef, getName(), auditProperties, null, true); + } +} 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 32400091e0..61f2e8b389 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 @@ -99,9 +99,6 @@ public class HoldServiceImpl extends ServiceBaseImpl /** Logger */ private static Log logger = LogFactory.getLog(HoldServiceImpl.class); - /** Audit event keys */ - private static final String AUDIT_REMOVE_FROM_HOLD = "removeFromHold"; - /** I18N */ private static final String MSG_ERR_ACCESS_DENIED = "permissions.err_access_denied"; @@ -219,16 +216,6 @@ public class HoldServiceImpl extends ServiceBaseImpl */ public void init() { - AuthenticationUtil.runAsSystem(new RunAsWork() - { - @Override - public Void doWork() throws Exception - { - recordsManagementAuditService.registerAuditEvent(new AuditEvent(AUDIT_REMOVE_FROM_HOLD, "capability.RemoveFromHold.title")); - return null; - } - }); - // Register the policies beforeCreateHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(BeforeCreateHoldPolicy.class); onCreateHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(OnCreateHoldPolicy.class); @@ -820,10 +807,6 @@ public class HoldServiceImpl extends ServiceBaseImpl transactionalResourceHelper.getSet("frozen").add(nodeRef); nodeService.removeChild(hold, nodeRef); - // audit that the node has been removed from the hold - // TODO add details of the hold that the node was removed from - recordsManagementAuditService.auditEvent(nodeRef, AUDIT_REMOVE_FROM_HOLD); - return null; }); removedHolds.add(hold); diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/RecordsManagementAuditServiceImplTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/RecordsManagementAuditServiceImplTest.java index 532d3f6d98..3d246fea3c 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/RecordsManagementAuditServiceImplTest.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/RecordsManagementAuditServiceImplTest.java @@ -28,11 +28,13 @@ package org.alfresco.module.org_alfresco_module_rm.test.legacy.service; import java.io.Serializable; +import java.util.Arrays; import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.stream.Collectors; import org.alfresco.model.ContentModel; import org.alfresco.module.org_alfresco_module_rm.audit.RecordsManagementAuditEntry; @@ -735,6 +737,183 @@ public class RecordsManagementAuditServiceImplTest extends BaseRMTestCase }); } + + /** + * Given I have an item in a hold + * When I remove the item from the hold + * Then there will be an audit entry for the item removed from the hold, including both the item name and hold name + */ + @org.junit.Test + public void testAuditForRemoveContentFromHold() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + final static String REMOVE_FROM_HOLD_AUDIT_EVENT = "Remove From Hold"; + + String holdName = "Hold " + GUID.generate(); + NodeRef hold; + + Map auditEventProperties; + + @Override + public void given() + { + rmAuditService.clearAuditLog(filePlan); + hold = utils.createHold(filePlan, holdName, "Reason " + GUID.generate()); + utils.addItemToHold(hold, dmDocument); + } + + @Override + public void when() + { + utils.removeItemFromHold(hold, dmDocument); + } + + @Override + public void then() + { + auditEventProperties = getAuditEntry(REMOVE_FROM_HOLD_AUDIT_EVENT).getBeforeProperties(); + + // check remove from hold audit event includes the hold name + assertEquals("Remove From Hold event does not include hold name.", holdName, + auditEventProperties.get(HOLD_NAME)); + + // check remove from hold audit event includes the content name + String contentName = (String) nodeService.getProperty(dmDocument, PROP_NAME); + assertEquals("Remove From Hold event does not include content name.", contentName, + auditEventProperties.get(PROP_NAME)); + } + + @Override + public void after() + { + // Stop and delete all entries + rmAuditService.stopAuditLog(filePlan); + rmAuditService.clearAuditLog(filePlan); + } + }); + + } + + + /** + * Given I have removed an item from multiple holds + * When I will get the RM audit filter by remove from hold events + * Then there will be entries for the item removed from each hold, including both the item name and hold name + */ + @org.junit.Test + public void testAuditForRemoveContentFromMultipleHolds() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + final static String REMOVE_FROM_HOLD_AUDIT_EVENT = "Remove From Hold"; + + String holdName1 = "Hold " + GUID.generate(); + String holdName2 = "Hold " + GUID.generate(); + NodeRef hold1, hold2; + + List> auditEventPropertiesList; + + @Override + public void given() + { + rmAuditService.clearAuditLog(filePlan); + + hold1 = utils.createHold(filePlan, holdName1, "Reason " + GUID.generate()); + hold2 = utils.createHold(filePlan, holdName2, "Reason " + GUID.generate()); + utils.addItemToHold(hold1, dmDocument); + utils.addItemToHold(hold2, dmDocument); + + } + + @Override + public void when() + { + utils.removeItemFromHold(hold1, dmDocument); + utils.removeItemFromHold(hold2, dmDocument); + } + + @Override + public void then() + { + auditEventPropertiesList = getAuditEntries(REMOVE_FROM_HOLD_AUDIT_EVENT) + .stream() + .map(RecordsManagementAuditEntry::getBeforeProperties) + .collect(Collectors.toList()); + + // check remove from hold audit event exists for both records + assertEquals(2, auditEventPropertiesList.size()); + } + + @Override + public void after() + { + // Stop and delete all entries + rmAuditService.stopAuditLog(filePlan); + rmAuditService.clearAuditLog(filePlan); + } + }); + + } + + + /** + * Given I have removed items from a hold + * When I will get the RM audit filter by remove from hold events + * Then there will be entries for the items removed from the hold, including both the item name and hold name + */ + @org.junit.Test + public void testAuditForRemoveMultipleContentFromHold() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + final static String REMOVE_FROM_HOLD_AUDIT_EVENT = "Remove From Hold"; + + String holdName = "Hold " + GUID.generate(); + NodeRef hold; + + List> auditEventPropertiesList; + + @Override + public void given() + { + rmAuditService.clearAuditLog(filePlan); + + hold = utils.createHold(filePlan, holdName, "Reason " + GUID.generate()); + utils.addItemToHold(hold, dmDocument); + utils.addItemToHold(hold, dmDocument1); + } + + @Override + public void when() + { + utils.removeItemsFromHolds(Arrays.asList(hold), Arrays.asList(dmDocument, dmDocument1)); + } + + @Override + public void then() + { + auditEventPropertiesList = getAuditEntries(REMOVE_FROM_HOLD_AUDIT_EVENT) + .stream() + .map(RecordsManagementAuditEntry::getBeforeProperties) + .collect(Collectors.toList()); + + // check remove from hold audit event exists for both records + assertEquals(2, auditEventPropertiesList.size()); + } + + @Override + public void after() + { + // Stop and delete all entries + rmAuditService.stopAuditLog(filePlan); + rmAuditService.clearAuditLog(filePlan); + } + }); + + } + + /** === Helper methods === */ private List getAuditTrail(String asUser) @@ -800,4 +979,18 @@ public class RecordsManagementAuditServiceImplTest extends BaseRMTestCase // return the properties of the audit event return auditEntry; } + + private List getAuditEntries(String auditEvent) + { + // set the audit query param for the given event + RecordsManagementAuditQueryParameters params = new RecordsManagementAuditQueryParameters(); + params.setEvent(auditEvent); + + // get the audit entries for the given event + List auditEntries; + auditEntries = getAuditTrail(params, -1, ADMIN_USER); + + return auditEntries; + } + } diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/CommonRMTestUtils.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/CommonRMTestUtils.java index cfccf57693..52cfd08dd1 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/CommonRMTestUtils.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/CommonRMTestUtils.java @@ -441,4 +441,27 @@ public class CommonRMTestUtils implements RecordsManagementModel { holdService.addToHold(holdNodeRef, contentNodeRef); } + + /** + * Util method to remove content from a hold. + * + * @param holdNodeRef hold node reference + * @param contentNodeRef content node reference + */ + public void removeItemFromHold(NodeRef holdNodeRef, NodeRef contentNodeRef) + { + holdService.removeFromHold(holdNodeRef, contentNodeRef); + } + + /** + * Util method to remove items from a holds. + * + * @param holdNodeRefs the list {@link NodeRef}s of the holds + * @param contentNodeRef the list of items which will be removed from the given holds + */ + public void removeItemsFromHolds(List holdNodeRefs, List contentNodeRef) + { + holdService.removeFromHolds(holdNodeRefs, contentNodeRef); + } + } diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/audit/event/RemoveFromHoldAuditEventUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/audit/event/RemoveFromHoldAuditEventUnitTest.java new file mode 100644 index 0000000000..8c84469428 --- /dev/null +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/audit/event/RemoveFromHoldAuditEventUnitTest.java @@ -0,0 +1,95 @@ +/* + * #%L + * Alfresco Records Management Module + * %% + * Copyright (C) 2005 - 2019 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * - + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * - + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * - + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * - + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ + +package org.alfresco.module.org_alfresco_module_rm.audit.event; + +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; +import static org.mockito.Matchers.isNull; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.mockito.MockitoAnnotations.initMocks; + +import java.util.Map; + +import org.alfresco.module.org_alfresco_module_rm.test.util.BaseUnitTest; +import org.alfresco.service.cmr.repository.ChildAssociationRef; +import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.cmr.repository.NodeService; +import org.alfresco.util.GUID; +import org.junit.Before; +import org.junit.Test; +import org.mockito.InjectMocks; +import org.mockito.Mock; + +/** + * Unit tests for {@link CreateHoldAuditEvent}. + * + * @author Chris Shields + * @since 3.3 + */ +public class RemoveFromHoldAuditEventUnitTest extends BaseUnitTest +{ + @InjectMocks + private RemoveFromHoldAuditEvent removeFromHoldAuditEvent; + + @Mock + private NodeService mockedNodeService; + + private NodeRef holdNodeRef; + private NodeRef contentNodeRef; + + /** + * Set up the mocks. + */ + @Before + public void setUp() + { + initMocks(this); + + holdNodeRef = generateNodeRef(); + String holdName = "Hold " + GUID.generate(); + + contentNodeRef = generateNodeRef(); + String contentName = "Content " + GUID.generate(); + + when(mockedNodeService.getProperty(holdNodeRef, PROP_NAME)).thenReturn(holdName); + when(mockedNodeService.getProperty(contentNodeRef, PROP_NAME)).thenReturn(contentName); + } + + /** + * Check that the remove from hold event calls an audit event. + */ + @Test + public void testAddToHoldCausesAuditEvent() + { + removeFromHoldAuditEvent.onRemoveFromHold(holdNodeRef, contentNodeRef); + verify(mockedRecordsManagementAuditService, times(1)).auditEvent(eq(contentNodeRef), any(String.class), any(Map.class), isNull(Map.class), eq(true)); + } + +} 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 fdcede7805..f86b3efb90 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 @@ -494,7 +494,6 @@ public class HoldServiceImplUnitTest extends BaseUnitTest verify(mockedNodeService, times(1)).removeChild(hold, recordFolder); verify(mockedNodeService, times(1)).removeAspect(recordFolder, ASPECT_FROZEN); verify(mockedNodeService, times(1)).removeAspect(record, ASPECT_FROZEN); - verify(mockedRecordsManagementAuditService, times(1)).auditEvent(eq(recordFolder), anyString()); } @Test @@ -518,7 +517,6 @@ public class HoldServiceImplUnitTest extends BaseUnitTest verify(mockedNodeService, times(1)).removeChild(hold2, recordFolder); verify(mockedNodeService, times(1)).removeAspect(recordFolder, ASPECT_FROZEN); verify(mockedNodeService, times(1)).removeAspect(record, ASPECT_FROZEN); - verify(mockedRecordsManagementAuditService, times(2)).auditEvent(any(NodeRef.class), anyString()); } @Test From a8223fbae830b510c2620ac95edfaea8fa0761d5 Mon Sep 17 00:00:00 2001 From: Chris Shields Date: Tue, 19 Nov 2019 21:11:50 +0000 Subject: [PATCH 2/3] Code review changes --- .../audit/event/RemoveFromHoldAuditEvent.java | 6 ++- ...RecordsManagementAuditServiceImplTest.java | 42 +++++++------------ .../test/util/CommonRMTestUtils.java | 2 +- .../RemoveFromHoldAuditEventUnitTest.java | 4 +- 4 files changed, 21 insertions(+), 33 deletions(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/RemoveFromHoldAuditEvent.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/RemoveFromHoldAuditEvent.java index 48f419f701..2141f524c6 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/RemoveFromHoldAuditEvent.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/RemoveFromHoldAuditEvent.java @@ -49,7 +49,9 @@ import org.alfresco.service.namespace.QName; @BehaviourBean public class RemoveFromHoldAuditEvent extends AuditEvent implements HoldServicePolicies.OnRemoveFromHoldPolicy { - /** Node Service */ + /** + * Node Service + */ private NodeService nodeService; /** @@ -63,7 +65,7 @@ public class RemoveFromHoldAuditEvent extends AuditEvent implements HoldServiceP } /** - * @see @see org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.OnRemoveFromHoldPolicy#onRemoveFromHold(org.alfresco.service.cmr.repository.NodeRef, org.alfresco.service.cmr.repository.NodeRef) + * @see org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.OnRemoveFromHoldPolicy#onRemoveFromHold(org.alfresco.service.cmr.repository.NodeRef, org.alfresco.service.cmr.repository.NodeRef) */ @Override @Behaviour diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/RecordsManagementAuditServiceImplTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/RecordsManagementAuditServiceImplTest.java index 3d246fea3c..6aee148602 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/RecordsManagementAuditServiceImplTest.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/RecordsManagementAuditServiceImplTest.java @@ -72,6 +72,12 @@ public class RecordsManagementAuditServiceImplTest extends BaseRMTestCase /** Test start time */ private Date testStartTime; + /** + * Remove from hold audit event name. + */ + private static final String REMOVE_FROM_HOLD_AUDIT_EVENT = "Remove From Hold"; + + /** * @see org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase#setUp() */ @@ -748,13 +754,9 @@ public class RecordsManagementAuditServiceImplTest extends BaseRMTestCase { doBehaviourDrivenTest(new BehaviourDrivenTest() { - final static String REMOVE_FROM_HOLD_AUDIT_EVENT = "Remove From Hold"; - String holdName = "Hold " + GUID.generate(); NodeRef hold; - Map auditEventProperties; - @Override public void given() { @@ -772,7 +774,7 @@ public class RecordsManagementAuditServiceImplTest extends BaseRMTestCase @Override public void then() { - auditEventProperties = getAuditEntry(REMOVE_FROM_HOLD_AUDIT_EVENT).getBeforeProperties(); + Map auditEventProperties = getAuditEntry(REMOVE_FROM_HOLD_AUDIT_EVENT).getBeforeProperties(); // check remove from hold audit event includes the hold name assertEquals("Remove From Hold event does not include hold name.", holdName, @@ -806,14 +808,10 @@ public class RecordsManagementAuditServiceImplTest extends BaseRMTestCase { doBehaviourDrivenTest(new BehaviourDrivenTest() { - final static String REMOVE_FROM_HOLD_AUDIT_EVENT = "Remove From Hold"; - String holdName1 = "Hold " + GUID.generate(); String holdName2 = "Hold " + GUID.generate(); NodeRef hold1, hold2; - List> auditEventPropertiesList; - @Override public void given() { @@ -823,26 +821,21 @@ public class RecordsManagementAuditServiceImplTest extends BaseRMTestCase hold2 = utils.createHold(filePlan, holdName2, "Reason " + GUID.generate()); utils.addItemToHold(hold1, dmDocument); utils.addItemToHold(hold2, dmDocument); - } @Override public void when() { - utils.removeItemFromHold(hold1, dmDocument); - utils.removeItemFromHold(hold2, dmDocument); + utils.removeItemsFromHolds(Arrays.asList(hold1, hold2), Arrays.asList(dmDocument)); } @Override public void then() { - auditEventPropertiesList = getAuditEntries(REMOVE_FROM_HOLD_AUDIT_EVENT) - .stream() - .map(RecordsManagementAuditEntry::getBeforeProperties) - .collect(Collectors.toList()); + List auditEntries = getAuditEntries(REMOVE_FROM_HOLD_AUDIT_EVENT); - // check remove from hold audit event exists for both records - assertEquals(2, auditEventPropertiesList.size()); + // check remove from hold audit event exists for both holds + assertEquals(2, auditEntries.size()); } @Override @@ -867,13 +860,9 @@ public class RecordsManagementAuditServiceImplTest extends BaseRMTestCase { doBehaviourDrivenTest(new BehaviourDrivenTest() { - final static String REMOVE_FROM_HOLD_AUDIT_EVENT = "Remove From Hold"; - String holdName = "Hold " + GUID.generate(); NodeRef hold; - List> auditEventPropertiesList; - @Override public void given() { @@ -893,13 +882,10 @@ public class RecordsManagementAuditServiceImplTest extends BaseRMTestCase @Override public void then() { - auditEventPropertiesList = getAuditEntries(REMOVE_FROM_HOLD_AUDIT_EVENT) - .stream() - .map(RecordsManagementAuditEntry::getBeforeProperties) - .collect(Collectors.toList()); + List auditEntries = getAuditEntries(REMOVE_FROM_HOLD_AUDIT_EVENT); - // check remove from hold audit event exists for both records - assertEquals(2, auditEventPropertiesList.size()); + // check remove from hold audit event exists for both documents + assertEquals(2, auditEntries.size()); } @Override diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/CommonRMTestUtils.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/CommonRMTestUtils.java index 63721187e4..692ac87edc 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/CommonRMTestUtils.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/CommonRMTestUtils.java @@ -456,7 +456,7 @@ public class CommonRMTestUtils implements RecordsManagementModel } /** - * Util method to remove items from a holds. + * Util method to remove items from holds. * * @param holdNodeRefs the list {@link NodeRef}s of the holds * @param contentNodeRef the list of items which will be removed from the given holds diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/audit/event/RemoveFromHoldAuditEventUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/audit/event/RemoveFromHoldAuditEventUnitTest.java index 8c84469428..57fcaad37f 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/audit/event/RemoveFromHoldAuditEventUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/audit/event/RemoveFromHoldAuditEventUnitTest.java @@ -48,7 +48,7 @@ import org.mockito.InjectMocks; import org.mockito.Mock; /** - * Unit tests for {@link CreateHoldAuditEvent}. + * Unit tests for {@link RemoveFromHoldAuditEvent}. * * @author Chris Shields * @since 3.3 @@ -86,7 +86,7 @@ public class RemoveFromHoldAuditEventUnitTest extends BaseUnitTest * Check that the remove from hold event calls an audit event. */ @Test - public void testAddToHoldCausesAuditEvent() + public void testRemoveFromHoldCausesAuditEvent() { removeFromHoldAuditEvent.onRemoveFromHold(holdNodeRef, contentNodeRef); verify(mockedRecordsManagementAuditService, times(1)).auditEvent(eq(contentNodeRef), any(String.class), any(Map.class), isNull(Map.class), eq(true)); From d8862b1ee37be401f3f8d2fc010aa152f7c58730 Mon Sep 17 00:00:00 2001 From: Chris Shields Date: Wed, 20 Nov 2019 11:16:04 +0000 Subject: [PATCH 3/3] Code review changes --- ...RecordsManagementAuditServiceImplTest.java | 44 ++++++++++++------- .../hold/HoldServiceImplUnitTest.java | 1 - 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/RecordsManagementAuditServiceImplTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/RecordsManagementAuditServiceImplTest.java index 6aee148602..6212c45b3f 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/RecordsManagementAuditServiceImplTest.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/RecordsManagementAuditServiceImplTest.java @@ -34,7 +34,6 @@ import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.stream.Collectors; import org.alfresco.model.ContentModel; import org.alfresco.module.org_alfresco_module_rm.audit.RecordsManagementAuditEntry; @@ -510,8 +509,7 @@ public class RecordsManagementAuditServiceImplTest extends BaseRMTestCase public void when() throws Exception { // set the audit wuery param - RecordsManagementAuditQueryParameters params = new RecordsManagementAuditQueryParameters(); - params.setEvent(DELETE_USER_AUDIT_EVENT); + RecordsManagementAuditQueryParameters params = createAuditQueryParameters(DELETE_USER_AUDIT_EVENT); // get the audit events for "Delete Person" entry = getAuditTrail(params, 1, ADMIN_USER); @@ -566,8 +564,7 @@ public class RecordsManagementAuditServiceImplTest extends BaseRMTestCase public void when() throws Exception { // set the audit query param - RecordsManagementAuditQueryParameters params = new RecordsManagementAuditQueryParameters(); - params.setEvent(CREATE_USER_AUDIT_EVENT); + RecordsManagementAuditQueryParameters params = createAuditQueryParameters(CREATE_USER_AUDIT_EVENT); // get the audit events for "Create Person" entry = getAuditTrail(params, 1, ADMIN_USER); @@ -950,13 +947,11 @@ public class RecordsManagementAuditServiceImplTest extends BaseRMTestCase private RecordsManagementAuditEntry getAuditEntry(String auditEvent) { - // set the audit query param for the given event - RecordsManagementAuditQueryParameters params = new RecordsManagementAuditQueryParameters(); - params.setEvent(auditEvent); + // create the audit query parameters for the given event + RecordsManagementAuditQueryParameters params = createAuditQueryParameters(auditEvent); // get the audit entries for the given event - List auditEntries; - auditEntries = getAuditTrail(params, 1, ADMIN_USER); + List auditEntries = getAuditEntryAssertOnlyOne(params); // verify we have the expected audit event RecordsManagementAuditEntry auditEntry = auditEntries.get(0); @@ -966,17 +961,36 @@ public class RecordsManagementAuditServiceImplTest extends BaseRMTestCase return auditEntry; } + private List getAuditEntryAssertOnlyOne(RecordsManagementAuditQueryParameters params) + { + List auditEntries; + auditEntries = getAuditTrail(params, 1, ADMIN_USER); + return auditEntries; + } + private List getAuditEntries(String auditEvent) { - // set the audit query param for the given event - RecordsManagementAuditQueryParameters params = new RecordsManagementAuditQueryParameters(); - params.setEvent(auditEvent); + // create the audit query parameters for the given event + RecordsManagementAuditQueryParameters params = createAuditQueryParameters(auditEvent); // get the audit entries for the given event - List auditEntries; - auditEntries = getAuditTrail(params, -1, ADMIN_USER); + List auditEntries = getAllAuditEntries(params); return auditEntries; } + private List getAllAuditEntries(RecordsManagementAuditQueryParameters params) + { + List auditEntries; + auditEntries = getAuditTrail(params, -1, ADMIN_USER); + return auditEntries; + } + + private RecordsManagementAuditQueryParameters createAuditQueryParameters(String auditEvent) + { + RecordsManagementAuditQueryParameters params = new RecordsManagementAuditQueryParameters(); + params.setEvent(auditEvent); + return params; + } + } 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 f86b3efb90..9543a3a3c5 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 @@ -477,7 +477,6 @@ public class HoldServiceImplUnitTest extends BaseUnitTest verify(mockedNodeService, never()).removeChild(hold, recordFolder); verify(mockedNodeService, never()).removeAspect(recordFolder, ASPECT_FROZEN); verify(mockedNodeService, never()).removeAspect(record, ASPECT_FROZEN); - verify(mockedRecordsManagementAuditService, never()).auditEvent(eq(recordFolder), anyString()); } @Test