From 73fc3ef789490f835558012e8d592c27076c8e57 Mon Sep 17 00:00:00 2001 From: Sara Aspery Date: Wed, 6 Nov 2019 23:20:28 +0000 Subject: [PATCH 1/2] RM-7028 Audit add to hold --- .../messages/audit-service.properties | 1 + .../rm-audit-context.xml | 7 ++ .../audit/event/AddToHoldAuditEvent.java | 83 +++++++++++++++++ .../hold/HoldServiceImpl.java | 5 - ...RecordsManagementAuditServiceImplTest.java | 59 ++++++++++++ .../test/util/BaseRMTestCase.java | 10 ++ .../event/AddToHoldAuditEventUnitTest.java | 92 +++++++++++++++++++ .../hold/HoldServiceImplUnitTest.java | 8 -- 8 files changed, 252 insertions(+), 13 deletions(-) create mode 100644 rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/AddToHoldAuditEvent.java create mode 100644 rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/audit/event/AddToHoldAuditEventUnitTest.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 e2ce5dc332..da0adf2b30 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 @@ -15,6 +15,7 @@ rm.audit.copyTo=Copy to rm.audit.fileTo=File to rm.audit.createHold=Create Hold rm.audit.deleteHold=Delete Hold +rm.audit.addToHold=Add To 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 364feeb76e..aec074cf27 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 @@ -138,4 +138,11 @@ + + + + + + + diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/AddToHoldAuditEvent.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/AddToHoldAuditEvent.java new file mode 100644 index 0000000000..a0c073bda8 --- /dev/null +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/AddToHoldAuditEvent.java @@ -0,0 +1,83 @@ +/* + * #%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 org.alfresco.model.ContentModel; +import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies; +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; + +import java.io.Serializable; +import java.util.Map; + +import static org.alfresco.repo.policy.Behaviour.NotificationFrequency.TRANSACTION_COMMIT; + +/** + * Add to hold audit event. + * + * @author Sara Aspery + * @since 3.3 + */ +@BehaviourBean +public class AddToHoldAuditEvent extends AuditEvent implements HoldServicePolicies.OnAddToHoldPolicy +{ + /** Node Service */ + private NodeService nodeService; + + /** + * Sets the node service + * + * @param nodeService nodeService to set + */ + public void setNodeService(NodeService nodeService) + { + this.nodeService = nodeService; + } + + /** + * @see org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.OnAddToHoldPolicy#onAddToHold(org.alfresco.service.cmr.repository.NodeRef, org.alfresco.service.cmr.repository.NodeRef) + */ + @Override + @Behaviour + ( + kind = BehaviourKind.CLASS, + type = "rma:hold", + notificationFrequency = TRANSACTION_COMMIT + ) + public void onAddToHold(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(), null, auditProperties); + } +} 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 4b817cabc7..0b89575c22 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 @@ -100,7 +100,6 @@ public class HoldServiceImpl extends ServiceBaseImpl private static Log logger = LogFactory.getLog(HoldServiceImpl.class); /** Audit event keys */ - private static final String AUDIT_ADD_TO_HOLD = "addToHold"; private static final String AUDIT_REMOVE_FROM_HOLD = "removeFromHold"; /** I18N */ @@ -225,7 +224,6 @@ public class HoldServiceImpl extends ServiceBaseImpl @Override public Void doWork() throws Exception { - recordsManagementAuditService.registerAuditEvent(new AuditEvent(AUDIT_ADD_TO_HOLD, "capability.AddToHold.title")); recordsManagementAuditService.registerAuditEvent(new AuditEvent(AUDIT_REMOVE_FROM_HOLD, "capability.RemoveFromHold.title")); return null; } @@ -666,9 +664,6 @@ public class HoldServiceImpl extends ServiceBaseImpl transactionalResourceHelper.getSet("frozen").add(nodeRef); nodeService.addChild(hold, nodeRef, ASSOC_FROZEN_CONTENT, ASSOC_FROZEN_CONTENT); - // audit item being added to the hold - recordsManagementAuditService.auditEvent(nodeRef, AUDIT_ADD_TO_HOLD); - // Mark all the folders contents as frozen if (isRecordFolder(nodeRef)) { 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 95e03ca779..5dc39d8af1 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 @@ -676,6 +676,65 @@ public class RecordsManagementAuditServiceImplTest extends BaseRMTestCase }); } + /** + * Given I have added an item a hold + * When I will get the RM audit filter by add to hold event + * Then there will be an entry for the item added to the hold, including both the item name and hold name + */ + @org.junit.Test + public void testAuditForAddContentToHold() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + final static String ADD_TO_HOLD_AUDIT_EVENT = "Add To Hold"; + + String holdName = "Hold " + GUID.generate(); + NodeRef hold; + + NodeRef content; + + Map auditEventProperties; + + @Override + public void given() + { + rmAuditService.clearAuditLog(filePlan); + + hold = createHold(holdName, "Reason " + GUID.generate()); + content = utils.createRecord(rmFolder, "Record " + GUID.generate() + ".txt"); + + addContentToHold(hold, content); + } + + @Override + public void when() + { + auditEventProperties = getAuditEntry(ADD_TO_HOLD_AUDIT_EVENT).getAfterProperties(); + } + + @Override + public void then() + { + // check add to hold audit event includes the hold name + assertEquals("Add To Hold event does not include hold name.", holdName, + auditEventProperties.get(HOLD_NAME)); + + // check add to hold audit event includes the content name + String contentName = (String) nodeService.getProperty(content, PROP_NAME); + assertEquals("Add To 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); + } + }); + } + /** === Helper methods === */ private List getAuditTrail(String asUser) 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 b6a1eb03f3..a8b931f1a3 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 @@ -729,6 +729,16 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase holdService.deleteHold(nodeRef); } + /** + * Util method to add content to a hold. + * @param holdNodeRef hold node reference + * @param contentNodeRef content node reference + */ + protected void addContentToHold(NodeRef holdNodeRef, NodeRef contentNodeRef) + { + holdService.addToHold(holdNodeRef, contentNodeRef); + } + /** * Setup multi hierarchy test data */ diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/audit/event/AddToHoldAuditEventUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/audit/event/AddToHoldAuditEventUnitTest.java new file mode 100644 index 0000000000..ea9c1f92cd --- /dev/null +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/audit/event/AddToHoldAuditEventUnitTest.java @@ -0,0 +1,92 @@ +/* + * #%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 org.alfresco.module.org_alfresco_module_rm.test.util.BaseUnitTest; +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; + +import java.util.Map; + +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; + +/** + * Unit tests for {@link AddToHoldAuditEvent}. + * + * @author Sara Aspery + * @since 3.3 + */ +public class AddToHoldAuditEventUnitTest extends BaseUnitTest +{ + @InjectMocks + private AddToHoldAuditEvent addToHoldAuditEvent; + + @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 add to hold event calls an audit event. + * + */ + @Test + public void testAddToHoldCausesAuditEvent() + { + addToHoldAuditEvent.onAddToHold(holdNodeRef, contentNodeRef); + verify(mockedRecordsManagementAuditService, times(1)).auditEvent(eq(contentNodeRef), any(String.class), isNull(Map.class), any(Map.class)); + } +} 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 55de14c20d..fdcede7805 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 @@ -346,18 +346,15 @@ public class HoldServiceImplUnitTest extends BaseUnitTest verify(mockedNodeService).addChild(hold, recordFolder, ASSOC_FROZEN_CONTENT, ASSOC_FROZEN_CONTENT); verify(mockedNodeService).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class)); verify(mockedNodeService).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class)); - verify(mockedRecordsManagementAuditService).auditEvent(eq(recordFolder), anyString()); holdService.addToHold(hold, record); verify(mockedNodeService).addChild(hold, record, ASSOC_FROZEN_CONTENT, ASSOC_FROZEN_CONTENT); verify(mockedNodeService).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class)); verify(mockedNodeService, times(2)).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class)); - verify(mockedRecordsManagementAuditService).auditEvent(eq(record), anyString()); holdService.addToHold(hold, activeContent); verify(mockedNodeService).addChild(hold, activeContent, ASSOC_FROZEN_CONTENT, ASSOC_FROZEN_CONTENT); verify(mockedNodeService).addAspect(eq(activeContent), eq(ASPECT_FROZEN), any(Map.class)); - verify(mockedRecordsManagementAuditService).auditEvent(eq(activeContent), anyString()); } @SuppressWarnings("unchecked") @@ -371,13 +368,11 @@ public class HoldServiceImplUnitTest extends BaseUnitTest verify(mockedNodeService, never()).addChild(hold, recordFolder, ASSOC_FROZEN_CONTENT, ASSOC_FROZEN_CONTENT); verify(mockedNodeService, never()).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class)); verify(mockedNodeService, never()).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class)); - verify(mockedRecordsManagementAuditService, never()).auditEvent(eq(recordFolder), anyString()); holdService.addToHold(hold, activeContent); verify(mockedNodeService, never()).addChild(hold, activeContent, ASSOC_FROZEN_CONTENT, ASSOC_FROZEN_CONTENT); verify(mockedNodeService, never()).addAspect(eq(activeContent), eq(ASPECT_FROZEN), any(Map.class)); - verify(mockedRecordsManagementAuditService, never()).auditEvent(eq(activeContent), anyString()); } @SuppressWarnings("unchecked") @@ -395,12 +390,10 @@ public class HoldServiceImplUnitTest extends BaseUnitTest verify(mockedNodeService).addChild(hold, recordFolder, ASSOC_FROZEN_CONTENT, ASSOC_FROZEN_CONTENT); verify(mockedNodeService, never()).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class)); verify(mockedNodeService, never()).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class)); - verify(mockedRecordsManagementAuditService).auditEvent(eq(recordFolder), anyString()); holdService.addToHold(hold, activeContent); verify(mockedNodeService).addChild(hold, activeContent, ASSOC_FROZEN_CONTENT, ASSOC_FROZEN_CONTENT); verify(mockedNodeService, never()).addAspect(eq(activeContent), eq(ASPECT_FROZEN), any(Map.class)); - verify(mockedRecordsManagementAuditService).auditEvent(eq(activeContent), anyString()); } @Test (expected = AccessDeniedException.class) @@ -466,7 +459,6 @@ public class HoldServiceImplUnitTest extends BaseUnitTest verify(mockedNodeService, times(1)).addChild(hold2, recordFolder, ASSOC_FROZEN_CONTENT, ASSOC_FROZEN_CONTENT); 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)); - verify(mockedRecordsManagementAuditService, times(2)).auditEvent(eq(recordFolder), anyString()); } @Test (expected = IntegrityException.class) From 2888c6dd10f6c980c6abf1749366a9ba2265bf77 Mon Sep 17 00:00:00 2001 From: Sara Aspery Date: Fri, 8 Nov 2019 08:35:50 +0000 Subject: [PATCH 2/2] RM-7028 fix integration tests --- .../test/integration/hold/DeleteHoldTest.java | 62 +++++++++++-------- 1 file changed, 37 insertions(+), 25 deletions(-) diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/DeleteHoldTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/DeleteHoldTest.java index fdf3c0367e..671aa3d239 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/DeleteHoldTest.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/DeleteHoldTest.java @@ -109,7 +109,11 @@ public class DeleteHoldTest extends BaseRMTestCase implements BeforeDeleteHoldPo public NodeRef run() throws Exception { // create test holds - return createAndCheckHold(); + NodeRef newHold = createAndCheckHold(); + // add the record folder to hold1 + holdService.addToHold(newHold, rmFolder); + + return newHold; } }); //Splitting transaction to fix onCreateNodePolicy issue where there was a node not found exception @@ -118,9 +122,6 @@ public class DeleteHoldTest extends BaseRMTestCase implements BeforeDeleteHoldPo @Override public Void run() throws Exception { - // add the record folder to hold1 - holdService.addToHold(hold1, rmFolder); - // assert that the folder and records are frozen assertTrue(freezeService.isFrozen(rmFolder)); assertTrue(freezeService.isFrozen(recordOne)); @@ -162,33 +163,44 @@ public class DeleteHoldTest extends BaseRMTestCase implements BeforeDeleteHoldPo //Splitting transaction to fix onCreateNodePolicy issue where there was a node not found exception doTestInTransaction(new Test() { - @Override - public Void run() throws Exception - { - NodeRef hold1 = holds.get(0); - NodeRef hold2 = holds.get(1); + @Override + public Void run() throws Exception + { + NodeRef hold1 = holds.get(0); + NodeRef hold2 = holds.get(1); - // add the record folder to hold1 - holdService.addToHold(hold1, rmFolder); + // add the record folder to hold1 + holdService.addToHold(hold1, rmFolder); - // assert that the folder and records are frozen - assertTrue(freezeService.isFrozen(rmFolder)); - assertTrue(freezeService.isFrozen(recordOne)); - assertTrue(freezeService.isFrozen(recordDeclaredOne)); + // assert that the folder and records are frozen + assertTrue(freezeService.isFrozen(rmFolder)); + assertTrue(freezeService.isFrozen(recordOne)); + assertTrue(freezeService.isFrozen(recordDeclaredOne)); - // check the contents of the hold - List frozenNodes = holdService.getHeld(hold1); - assertNotNull(frozenNodes); - assertEquals(1, frozenNodes.size()); - assertEquals(rmFolder, frozenNodes.get(0)); + // check the contents of the hold + List frozenNodes = holdService.getHeld(hold1); + assertNotNull(frozenNodes); + assertEquals(1, frozenNodes.size()); + assertEquals(rmFolder, frozenNodes.get(0)); - holdService.addToHold(hold2, recordOne); + holdService.addToHold(hold2, recordOne); - // assert that the folder and records are frozen - assertTrue(freezeService.isFrozen(rmFolder)); - assertTrue(freezeService.isFrozen(recordOne)); - assertTrue(freezeService.isFrozen(recordDeclaredOne)); + // assert that the folder and records are frozen + assertTrue(freezeService.isFrozen(rmFolder)); + assertTrue(freezeService.isFrozen(recordOne)); + assertTrue(freezeService.isFrozen(recordDeclaredOne)); + return null; + } + }); + + doTestInTransaction(new Test() + { + @Override + public Void run() throws Exception + { + NodeRef hold1 = holds.get(0); + NodeRef hold2 = holds.get(1); // delete the hold holdService.deleteHold(hold1);