From 2f1022ded61f980fe7e66341420970dcd7fc5ea5 Mon Sep 17 00:00:00 2001 From: Roxana Lucanu Date: Tue, 29 Oct 2019 11:37:18 +0200 Subject: [PATCH 01/11] RM-7034 add policies for held content --- .../hold/HoldServiceImpl.java | 24 ++++++++- .../hold/HoldServicePolicies.java | 53 +++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) 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 403d9a1c26..80dfb1c1c2 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 @@ -41,16 +41,19 @@ import java.util.Set; import org.alfresco.error.AlfrescoRuntimeException; import org.alfresco.model.ContentModel; -import org.alfresco.module.org_alfresco_module_rm.RecordsManagementPolicies.BeforeFileRecord; import org.alfresco.module.org_alfresco_module_rm.audit.RecordsManagementAuditService; import org.alfresco.module.org_alfresco_module_rm.audit.event.AuditEvent; import org.alfresco.module.org_alfresco_module_rm.capability.CapabilityService; import org.alfresco.module.org_alfresco_module_rm.capability.RMPermissionModel; import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService; +import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.BeforeAddToHoldPolicy; import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.BeforeCreateHoldPolicy; import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.BeforeDeleteHoldPolicy; +import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.BeforeRemoveFromHoldPolicy; +import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.OnAddToHoldPolicy; import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.OnCreateHoldPolicy; import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.OnDeleteHoldPolicy; +import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.OnRemoveFromHoldPolicy; import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; import org.alfresco.module.org_alfresco_module_rm.record.RecordService; import org.alfresco.module.org_alfresco_module_rm.recordfolder.RecordFolderService; @@ -206,6 +209,10 @@ public class HoldServiceImpl extends ServiceBaseImpl private ClassPolicyDelegate onCreateHoldPolicyDelegate; private ClassPolicyDelegate beforeDeleteHoldPolicyDelegate; private ClassPolicyDelegate onDeleteHoldPolicyDelegate; + private ClassPolicyDelegate beforeAddToHoldPolicyDelegate; + private ClassPolicyDelegate onAddToHoldPolicyDelegate; + private ClassPolicyDelegate beforeRemoveFromHoldPolicyDelegate; + private ClassPolicyDelegate onRemoveFromHoldPolicyDelegate; /** * Initialise hold service @@ -228,6 +235,11 @@ public class HoldServiceImpl extends ServiceBaseImpl onCreateHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(OnCreateHoldPolicy.class); beforeDeleteHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(BeforeDeleteHoldPolicy.class); onDeleteHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(OnDeleteHoldPolicy.class); + beforeAddToHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(BeforeAddToHoldPolicy.class); + onAddToHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(OnAddToHoldPolicy.class); + beforeRemoveFromHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(BeforeRemoveFromHoldPolicy.class); + onRemoveFromHoldPolicyDelegate = getPolicyComponent().registerClassPolicy(OnRemoveFromHoldPolicy.class); + } /** @@ -634,6 +646,8 @@ public class HoldServiceImpl extends ServiceBaseImpl // check that the node isn't already in the hold if (!getHeld(hold).contains(nodeRef)) { + // fire before add to hold policy + beforeAddToHoldPolicyDelegate.get(getTypeAndApsects(hold)).beforeAddToHold(hold, nodeRef); // run as system to ensure we have all the appropriate permissions to perform the manipulations we require authenticationUtil.runAsSystem((RunAsWork) () -> { // gather freeze properties @@ -658,6 +672,9 @@ public class HoldServiceImpl extends ServiceBaseImpl records.forEach(record -> addFrozenAspect(record, props)); } + // fire on add to hold policy + onAddToHoldPolicyDelegate.get(getTypeAndApsects(hold)).onAddToHold(hold, nodeRef); + return null; }); } @@ -794,6 +811,8 @@ public class HoldServiceImpl extends ServiceBaseImpl // run as system so we don't run into further permission issues // we already know we have to have the correct capability to get here authenticationUtil.runAsSystem((RunAsWork) () -> { + // fire before remove from hold policy + beforeRemoveFromHoldPolicyDelegate.get(getTypeAndApsects(hold)).beforeRemoveFromHold(hold, nodeRef); // remove from hold //set in transaction cache in order not to trigger update policy when removing the child association transactionalResourceHelper.getSet("frozen").add(nodeRef); @@ -803,6 +822,9 @@ public class HoldServiceImpl extends ServiceBaseImpl // TODO add details of the hold that the node was removed from recordsManagementAuditService.auditEvent(nodeRef, AUDIT_REMOVE_FROM_HOLD); + // fire on remove from hold policy + onRemoveFromHoldPolicyDelegate.get(getTypeAndApsects(hold)).onRemoveFromHold(hold, nodeRef); + return null; }); diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServicePolicies.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServicePolicies.java index 062af9fcc4..2114297859 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServicePolicies.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServicePolicies.java @@ -36,6 +36,7 @@ import org.alfresco.service.namespace.QName; * Hold Service Policies * * @author Ramona Popa + * @author Roxana Lucanu * @since 3.3 */ @@ -86,4 +87,56 @@ public interface HoldServicePolicies */ void onDeleteHold(NodeRef hold); } + + interface BeforeAddToHoldPolicy extends ClassPolicy + { + QName QNAME = QName.createQName(NamespaceService.ALFRESCO_URI, "beforeAddToHold"); + + /** + * Called before adding content to hold. + * + * @param hold the hold to be added into + * @param contentNodeRef the item to be added to hold + */ + void beforeAddToHold(NodeRef hold, NodeRef contentNodeRef); + } + + interface OnAddToHoldPolicy extends ClassPolicy + { + QName QNAME = QName.createQName(NamespaceService.ALFRESCO_URI, "onAddToHold"); + + /** + * Called when content is added to hold. + * + * @param hold the hold to be added into + * @param contentNodeRef the item to be added to hold + */ + void onAddToHold(NodeRef hold, NodeRef contentNodeRef); + } + + interface BeforeRemoveFromHoldPolicy extends ClassPolicy + { + QName QNAME = QName.createQName(NamespaceService.ALFRESCO_URI, "beforeRemoveFromHold"); + + /** + * Called before removing content from hold. + * + * @param hold the hold to be removed from + * @param contentNodeRef the item to be removed from hold + */ + void beforeRemoveFromHold(NodeRef hold, NodeRef contentNodeRef); + } + + interface OnRemoveFromHoldPolicy extends ClassPolicy + { + QName QNAME = QName.createQName(NamespaceService.ALFRESCO_URI, "onRemoveFromHold"); + + /** + * Called when removing content from hold. + * + * @param hold the hold to be removed from + * @param contentNodeRef the item to be removed from hold + */ + void onRemoveFromHold(NodeRef hold, NodeRef contentNodeRef); + } } From 657f03b85dfc1eb1ca6847d384b0494fc26c7e52 Mon Sep 17 00:00:00 2001 From: Rodica Sutu Date: Fri, 1 Nov 2019 09:51:48 +0200 Subject: [PATCH 02/11] add a list of namespace URIs for properties, which should be always editable for a frozen node --- .../rm-model-context.xml | 6 +++++ .../PropertyModificationAllowedCheck.java | 26 ++++++++++++++++++- ...pertyModificationAllowedCheckUnitTest.java | 4 +++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml index 932d24c525..d849ae16b1 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml @@ -177,6 +177,11 @@ + + + + http://www.alfresco.org/model/system/1.0 + @@ -185,6 +190,7 @@ + diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java index 8e98601b61..52c7f956dd 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java @@ -45,6 +45,29 @@ public class PropertyModificationAllowedCheck * List of qnames that can be modified */ private List whiteList; + + /** + * List of model URI's for which the properties can be updated + */ + private List editableURIs; + + /** + * Setter for list of model URI's + * @param editableURIs List + */ + public void setEditableURIs(List editableURIs) + { + this.editableURIs = editableURIs; + } + + /** + * Getter for list of model URI's + * @return return the list of model URI's + */ + public List getEditableURIs() + { + return editableURIs; + } /** * Setter for list of qnames @@ -93,7 +116,7 @@ public class PropertyModificationAllowedCheck if (before.get(key) != null && after.get(key) != null && !(after.get(key).equals(before.get(key)))) { //Property modified check to see if allowed - proceed = whiteList.contains(key); + proceed = whiteList.contains(key) || getEditableURIs().contains(key.getNamespaceURI()); if (!proceed) { break; @@ -103,4 +126,5 @@ public class PropertyModificationAllowedCheck return proceed; } + } diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheckUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheckUnitTest.java index 1b192a62c0..3cb5e89c0d 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheckUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheckUnitTest.java @@ -58,6 +58,7 @@ public class PropertyModificationAllowedCheckUnitTest private QName qName, qName2; private List list; + private List editableURIs; @Mock private Serializable serializable, serializable2; @@ -74,6 +75,9 @@ public class PropertyModificationAllowedCheckUnitTest before.put(qName, serializable); after.put(qName, serializable2); list = new ArrayList(); + editableURIs =new ArrayList<>(); + propertyModificationAllowedCheck.setWhiteList(list); + propertyModificationAllowedCheck.setEditableURIs(editableURIs); } /** From 3f4e35ba90f1bd7284cdca54a9757be7c48dc317 Mon Sep 17 00:00:00 2001 From: Rodica Sutu Date: Mon, 4 Nov 2019 11:39:35 +0200 Subject: [PATCH 03/11] add unit tests --- .../PropertyModificationAllowedCheck.java | 15 ++++++++++--- ...pertyModificationAllowedCheckUnitTest.java | 22 +++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java index 52c7f956dd..b1d90afa65 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java @@ -95,7 +95,7 @@ public class PropertyModificationAllowedCheck if (!before.containsKey(key) || !after.containsKey(key)) { //Property modified check to see if allowed - proceed = whiteList.contains(key); + proceed = allowPropertyUpdate(key); if (!proceed) { break; @@ -106,7 +106,7 @@ public class PropertyModificationAllowedCheck (after.get(key) == null && before.get(key) != null)) { //Property modified check to see if allowed - proceed = whiteList.contains(key); + proceed = allowPropertyUpdate(key); if (!proceed) { break; @@ -116,7 +116,7 @@ public class PropertyModificationAllowedCheck if (before.get(key) != null && after.get(key) != null && !(after.get(key).equals(before.get(key)))) { //Property modified check to see if allowed - proceed = whiteList.contains(key) || getEditableURIs().contains(key.getNamespaceURI()); + proceed = allowPropertyUpdate(key); if (!proceed) { break; @@ -126,5 +126,14 @@ public class PropertyModificationAllowedCheck return proceed; } + /** + * Determines whether the property should be allowed to be updated or not. + * @param key property + * @return true if property update us allowed + */ + private boolean allowPropertyUpdate(QName key) + { + return whiteList.contains(key) || getEditableURIs().contains(key.getNamespaceURI()); + } } diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheckUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheckUnitTest.java index 3cb5e89c0d..fca0a8b9cb 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheckUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheckUnitTest.java @@ -236,4 +236,26 @@ public class PropertyModificationAllowedCheckUnitTest after.put(qName, null); assertTrue(propertyModificationAllowedCheck.check(before, after)); } + + /** + * Test update of a property from the model URI for which properties can be updated + */ + @Test + public void testUpdatePropertyFromAllowedModelURI() + { + editableURIs.add("foo"); + propertyModificationAllowedCheck.setEditableURIs(editableURIs); + assertTrue(propertyModificationAllowedCheck.check(before, after)); + } + + /** + * Test update of a property that is not in the model URI for which properties can be updated + */ + @Test + public void testUpdatePropertyFromNotAllowedModelURI() + { + editableURIs.add("bar"); + propertyModificationAllowedCheck.setEditableURIs(editableURIs); + assertFalse(propertyModificationAllowedCheck.check(before, after)); + } } From e079b2f50b3e2f173e7cbf3707080366cad1f0e1 Mon Sep 17 00:00:00 2001 From: Roxana Lucanu Date: Mon, 4 Nov 2019 12:23:22 +0200 Subject: [PATCH 04/11] RM-7034 Added tests --- .../hold/AddRemoveFromHoldTest.java | 138 +++++++++++++++++- 1 file changed, 137 insertions(+), 1 deletion(-) diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddRemoveFromHoldTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddRemoveFromHoldTest.java index 0a31e20fa5..5ca7c3bd94 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddRemoveFromHoldTest.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddRemoveFromHoldTest.java @@ -31,7 +31,16 @@ import java.util.ArrayList; import java.util.List; import org.alfresco.model.ContentModel; +import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.BeforeAddToHoldPolicy; +import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.BeforeRemoveFromHoldPolicy; +import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.OnAddToHoldPolicy; +import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.OnRemoveFromHoldPolicy; +import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase; +import org.alfresco.repo.policy.Behaviour.NotificationFrequency; +import org.alfresco.repo.policy.BehaviourDefinition; +import org.alfresco.repo.policy.ClassBehaviourBinding; +import org.alfresco.repo.policy.JavaBehaviour; import org.alfresco.service.cmr.repository.NodeRef; import org.springframework.extensions.webscripts.GUID; @@ -41,10 +50,15 @@ import org.springframework.extensions.webscripts.GUID; * @author Roy Wetherall * @since 2.2 */ -public class AddRemoveFromHoldTest extends BaseRMTestCase +public class AddRemoveFromHoldTest extends BaseRMTestCase implements BeforeAddToHoldPolicy, OnAddToHoldPolicy, BeforeRemoveFromHoldPolicy, OnRemoveFromHoldPolicy { private static final int RECORD_COUNT = 10; + private boolean beforeAddToHoldFlag = false; + private boolean onAddToHoldFlag = false; + private boolean beforeRemoveFromHoldFlag = false; + private boolean onRemoveFromHoldFlag = false; + public void testAddRecordToHold() { doBehaviourDrivenTest(new BehaviourDrivenTest() @@ -421,4 +435,126 @@ public class AddRemoveFromHoldTest extends BaseRMTestCase } }); } + + public void testPolicyNotificationForAddToHold() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + private NodeRef hold; + private NodeRef recordCategory; + private NodeRef recordFolder; + BehaviourDefinition beforeAddToHoldBehaviour; + BehaviourDefinition onAddToHoldBehaviour; + + public void given() + { + // create a hold + hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + // create a record category -> record folder + recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate()); + recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate()); + + beforeAddToHoldBehaviour = policyComponent.bindClassBehaviour(BeforeAddToHoldPolicy.QNAME, + RecordsManagementModel.TYPE_HOLD, new JavaBehaviour(AddRemoveFromHoldTest.this, "beforeAddToHold", NotificationFrequency.EVERY_EVENT)); + + onAddToHoldBehaviour = policyComponent.bindClassBehaviour(OnAddToHoldPolicy.QNAME, + RecordsManagementModel.TYPE_HOLD, new JavaBehaviour(AddRemoveFromHoldTest.this, "onAddToHold", NotificationFrequency.EVERY_EVENT)); + + assertFalse(beforeAddToHoldFlag); + assertFalse(onAddToHoldFlag); + } + + public void when() throws Exception + { + // add the record folder to hold + holdService.addToHold(hold, recordFolder); + } + + public void then() + { + assertTrue(beforeAddToHoldFlag); + assertTrue(onAddToHoldFlag); + } + + public void after() + { + policyComponent.removeClassDefinition(beforeAddToHoldBehaviour); + policyComponent.removeClassDefinition(onAddToHoldBehaviour); + } + }); + } + + public void testPolicyNotificationForRemoveFromHold() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + private NodeRef hold; + private NodeRef recordCategory; + private NodeRef recordFolder; + BehaviourDefinition beforeRemoveFromHoldBehaviour; + BehaviourDefinition onRemoveFromHoldBehaviour; + + public void given() + { + // create a hold + hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + // create a record category -> record folder + recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate()); + recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate()); + // add the record folder to hold + holdService.addToHold(hold, recordFolder); + + beforeRemoveFromHoldBehaviour = policyComponent.bindClassBehaviour(BeforeRemoveFromHoldPolicy.QNAME, + RecordsManagementModel.TYPE_HOLD, new JavaBehaviour(AddRemoveFromHoldTest.this, "beforeRemoveFromHold", NotificationFrequency.EVERY_EVENT)); + + onRemoveFromHoldBehaviour = policyComponent.bindClassBehaviour(OnRemoveFromHoldPolicy.QNAME, + RecordsManagementModel.TYPE_HOLD, new JavaBehaviour(AddRemoveFromHoldTest.this, "onRemoveFromHold", NotificationFrequency.EVERY_EVENT)); + + assertFalse(beforeRemoveFromHoldFlag); + assertFalse(onRemoveFromHoldFlag); + } + + public void when() throws Exception + { + // remove the record folder from the hold + holdService.removeFromHold(hold, recordFolder); + } + + public void then() + { + assertTrue(beforeRemoveFromHoldFlag); + assertTrue(onRemoveFromHoldFlag); + } + + public void after() + { + policyComponent.removeClassDefinition(beforeRemoveFromHoldBehaviour); + policyComponent.removeClassDefinition(onRemoveFromHoldBehaviour); + } + }); + } + + @Override + public void beforeAddToHold(NodeRef hold, NodeRef contentNodeRef) + { + beforeAddToHoldFlag = true; + } + + @Override + public void onAddToHold(NodeRef hold, NodeRef contentNodeRef) + { + onAddToHoldFlag = true; + } + + @Override + public void beforeRemoveFromHold(NodeRef hold, NodeRef contentNodeRef) + { + beforeRemoveFromHoldFlag = true; + } + + @Override + public void onRemoveFromHold(NodeRef hold, NodeRef contentNodeRef) + { + onRemoveFromHoldFlag = true; + } } From 970bee3228bb0e20b9d34025df17f2020587cf52 Mon Sep 17 00:00:00 2001 From: Roxana Lucanu Date: Mon, 4 Nov 2019 16:40:18 +0200 Subject: [PATCH 05/11] RM-7034 Fixed unit tests and split class for add/remove from hold tests. --- .../hold/HoldServiceImpl.java | 56 +- .../hold/AddRemoveFromHoldTest.java | 560 ------------------ .../test/integration/hold/AddToHoldTest.java | 315 ++++++++++ .../integration/hold/RemoveFromHoldTest.java | 298 ++++++++++ .../hold/HoldServiceImplUnitTest.java | 32 + 5 files changed, 697 insertions(+), 564 deletions(-) delete mode 100644 rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddRemoveFromHoldTest.java create mode 100644 rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddToHoldTest.java create mode 100644 rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveFromHoldTest.java 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 feacd1bef7..0a57d8b3b4 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 @@ -651,7 +651,7 @@ public class HoldServiceImpl extends ServiceBaseImpl if (!getHeld(hold).contains(nodeRef)) { // fire before add to hold policy - beforeAddToHoldPolicyDelegate.get(getTypeAndApsects(hold)).beforeAddToHold(hold, nodeRef); + invokeBeforeAddToHold(hold, nodeRef); // run as system to ensure we have all the appropriate permissions to perform the manipulations we require authenticationUtil.runAsSystem((RunAsWork) () -> { // gather freeze properties @@ -677,7 +677,7 @@ public class HoldServiceImpl extends ServiceBaseImpl } // fire on add to hold policy - onAddToHoldPolicyDelegate.get(getTypeAndApsects(hold)).onAddToHold(hold, nodeRef); + invokeOnAddToHold(hold, nodeRef); return null; }); @@ -816,7 +816,7 @@ public class HoldServiceImpl extends ServiceBaseImpl // we already know we have to have the correct capability to get here authenticationUtil.runAsSystem((RunAsWork) () -> { // fire before remove from hold policy - beforeRemoveFromHoldPolicyDelegate.get(getTypeAndApsects(hold)).beforeRemoveFromHold(hold, nodeRef); + invokeBeforeRemoveFromHold(hold, nodeRef); // remove from hold //set in transaction cache in order not to trigger update policy when removing the child association transactionalResourceHelper.getSet("frozen").add(nodeRef); @@ -827,7 +827,7 @@ public class HoldServiceImpl extends ServiceBaseImpl recordsManagementAuditService.auditEvent(nodeRef, AUDIT_REMOVE_FROM_HOLD); // fire on remove from hold policy - onRemoveFromHoldPolicyDelegate.get(getTypeAndApsects(hold)).onRemoveFromHold(hold, nodeRef); + invokeOnRemoveFromHold(hold, nodeRef); return null; @@ -939,4 +939,52 @@ public class HoldServiceImpl extends ServiceBaseImpl } + /** + * Invoke beforeAddToHold policy + * + * @param hold hold node reference + * @param contentNodeRef content node reference + */ + protected void invokeBeforeAddToHold(NodeRef hold, NodeRef contentNodeRef) + { + BeforeAddToHoldPolicy policy = beforeAddToHoldPolicyDelegate.get(getTypeAndApsects(hold)); + policy.beforeAddToHold(hold, contentNodeRef); + } + + /** + * Invoke onAddToHold policy + * + *@param hold hold node reference + *@param contentNodeRef content node reference + */ + protected void invokeOnAddToHold(NodeRef hold, NodeRef contentNodeRef) + { + OnAddToHoldPolicy policy = onAddToHoldPolicyDelegate.get(getTypeAndApsects(hold)); + policy.onAddToHold(hold, contentNodeRef); + } + + /** + * Invoke beforeRemoveFromHold policy + * + *@param hold hold node reference + *@param contentNodeRef content node reference + */ + protected void invokeBeforeRemoveFromHold(NodeRef hold, NodeRef contentNodeRef) + { + BeforeRemoveFromHoldPolicy policy = beforeRemoveFromHoldPolicyDelegate.get(getTypeAndApsects(hold)); + policy.beforeRemoveFromHold(hold, contentNodeRef); + } + + /** + * Invoke onRemoveFromHold policy + * + * @param hold hold node reference + * @param contentNodeRef content node reference + */ + protected void invokeOnRemoveFromHold(NodeRef hold, NodeRef contentNodeRef) + { + OnRemoveFromHoldPolicy policy = onRemoveFromHoldPolicyDelegate.get(getTypeAndApsects(hold)); + policy.onRemoveFromHold(hold, contentNodeRef); + + } } diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddRemoveFromHoldTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddRemoveFromHoldTest.java deleted file mode 100644 index 5ca7c3bd94..0000000000 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddRemoveFromHoldTest.java +++ /dev/null @@ -1,560 +0,0 @@ -/* - * #%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.test.integration.hold; - -import java.util.ArrayList; -import java.util.List; - -import org.alfresco.model.ContentModel; -import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.BeforeAddToHoldPolicy; -import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.BeforeRemoveFromHoldPolicy; -import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.OnAddToHoldPolicy; -import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.OnRemoveFromHoldPolicy; -import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; -import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase; -import org.alfresco.repo.policy.Behaviour.NotificationFrequency; -import org.alfresco.repo.policy.BehaviourDefinition; -import org.alfresco.repo.policy.ClassBehaviourBinding; -import org.alfresco.repo.policy.JavaBehaviour; -import org.alfresco.service.cmr.repository.NodeRef; -import org.springframework.extensions.webscripts.GUID; - -/** - * Hold service integration test. - * - * @author Roy Wetherall - * @since 2.2 - */ -public class AddRemoveFromHoldTest extends BaseRMTestCase implements BeforeAddToHoldPolicy, OnAddToHoldPolicy, BeforeRemoveFromHoldPolicy, OnRemoveFromHoldPolicy -{ - private static final int RECORD_COUNT = 10; - - private boolean beforeAddToHoldFlag = false; - private boolean onAddToHoldFlag = false; - private boolean beforeRemoveFromHoldFlag = false; - private boolean onRemoveFromHoldFlag = false; - - public void testAddRecordToHold() - { - doBehaviourDrivenTest(new BehaviourDrivenTest() - { - private NodeRef hold; - private NodeRef recordCategory; - private NodeRef recordFolder; - private NodeRef record; - - public void given() - { - // create a hold - hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); - - // create a record folder that contains records - recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate()); - recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate()); - record = recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null); - - // assert current states - assertFalse(freezeService.isFrozen(recordFolder)); - assertFalse(freezeService.isFrozen(record)); - assertFalse(freezeService.hasFrozenChildren(recordFolder)); - - // additional check for child held caching - assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)); - assertEquals(0, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT)); - } - - public void when() throws Exception - { - // add the record to hold - holdService.addToHold(hold, record); - } - - public void then() - { - // record is held - assertTrue(freezeService.isFrozen(record)); - - // record folder has frozen children - assertFalse(freezeService.isFrozen(recordFolder)); - assertTrue(freezeService.hasFrozenChildren(recordFolder)); - - // record folder is not held - assertFalse(holdService.getHeld(hold).contains(recordFolder)); - assertFalse(holdService.heldBy(recordFolder, true).contains(hold)); - - // hold contains record - assertTrue(holdService.getHeld(hold).contains(record)); - assertTrue(holdService.heldBy(record, true).contains(hold)); - - // additional check for child held caching - assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)); - assertEquals(1, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT)); - } - }); - - } - - public void testAddRecordsToHold() - { - doBehaviourDrivenTest(new BehaviourDrivenTest() - { - private NodeRef hold; - private NodeRef recordCategory; - private NodeRef recordFolder; - private List records = new ArrayList<>(RECORD_COUNT); - - public void given() - { - // create a hold - hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); - - // create a record folder that contains records - recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate()); - recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate()); - for (int i = 0; i < RECORD_COUNT; i++) - { - records.add(recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null)); - } - - // assert current states - assertFalse(freezeService.isFrozen(recordFolder)); - assertFalse(freezeService.hasFrozenChildren(recordFolder)); - for (NodeRef record : records) - { - assertFalse(freezeService.isFrozen(record)); - } - - // additional check for child held caching - assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)); - assertEquals(0, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT)); - } - - public void when() throws Exception - { - // add the record to hold - holdService.addToHold(hold, records); - } - - public void then() - { - // record is held - for (NodeRef record : records) - { - assertTrue(freezeService.isFrozen(record)); - } - - // record folder has frozen children - assertFalse(freezeService.isFrozen(recordFolder)); - assertTrue(freezeService.hasFrozenChildren(recordFolder)); - - // record folder is not held - assertFalse(holdService.getHeld(hold).contains(recordFolder)); - assertFalse(holdService.heldBy(recordFolder, true).contains(hold)); - - for (NodeRef record : records) - { - // hold contains record - assertTrue(holdService.getHeld(hold).contains(record)); - assertTrue(holdService.heldBy(record, true).contains(hold)); - } - - // additional check for child held caching - assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)); - assertEquals(RECORD_COUNT, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT)); - } - }); - } - - public void testAddRecordFolderToHold() - { - doBehaviourDrivenTest(new BehaviourDrivenTest() - { - private NodeRef hold; - private NodeRef recordCategory; - private NodeRef recordFolder; - private List records = new ArrayList<>(RECORD_COUNT); - - public void given() - { - // create a hold - hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); - - // create a record folder that contains records - recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate()); - recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate()); - for (int i = 0; i < RECORD_COUNT; i++) - { - records.add(recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null)); - } - - // assert current states - assertFalse(freezeService.isFrozen(recordFolder)); - assertFalse(freezeService.hasFrozenChildren(recordFolder)); - for (NodeRef record : records) - { - assertFalse(freezeService.isFrozen(record)); - } - - // additional check for child held caching - assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)); - assertEquals(0, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT)); - } - - public void when() throws Exception - { - // add the record to hold - holdService.addToHold(hold, recordFolder); - } - - public void then() - { - for (NodeRef record : records) - { - // record is held - assertTrue(freezeService.isFrozen(record)); - assertFalse(holdService.getHeld(hold).contains(record)); - assertTrue(holdService.heldBy(record, true).contains(hold)); - } - - // record folder has frozen children - assertTrue(freezeService.isFrozen(recordFolder)); - assertTrue(freezeService.hasFrozenChildren(recordFolder)); - - // hold contains record folder - assertTrue(holdService.getHeld(hold).contains(recordFolder)); - assertTrue(holdService.heldBy(recordFolder, true).contains(hold)); - - // additional check for child held caching - assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)); - assertEquals(RECORD_COUNT, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT)); - } - }); - - } - - public void testRemoveRecordsFromHold() - { - doBehaviourDrivenTest(new BehaviourDrivenTest() - { - private NodeRef hold; - private NodeRef recordCategory; - private NodeRef recordFolder; - private List records = new ArrayList<>(RECORD_COUNT); - - public void given() - { - // create a hold - hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); - - // create a record folder that contains records - recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate()); - recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate()); - for (int i = 0; i < RECORD_COUNT; i++) - { - records.add(recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null)); - } - - // add records to hold - holdService.addToHold(hold, records); - } - - public void when() throws Exception - { - // remove *some* of the records - holdService.removeFromHold(hold, records.subList(0, 5)); - } - - public void then() - { - // check record state (no longer held) - for (NodeRef record : records.subList(0, 5)) - { - assertFalse(freezeService.isFrozen(record)); - assertFalse(holdService.getHeld(hold).contains(record)); - assertFalse(holdService.heldBy(record, true).contains(hold)); - } - - // check record state (still held) - for (NodeRef record : records.subList(5, 10)) - { - assertTrue(freezeService.isFrozen(record)); - assertTrue(holdService.getHeld(hold).contains(record)); - assertTrue(holdService.heldBy(record, true).contains(hold)); - } - - // record folder has frozen children - assertFalse(freezeService.isFrozen(recordFolder)); - assertTrue(freezeService.hasFrozenChildren(recordFolder)); - - // record folder is not held - assertFalse(holdService.getHeld(hold).contains(recordFolder)); - assertFalse(holdService.heldBy(recordFolder, true).contains(hold)); - - // additional check for child held caching - assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)); - assertEquals(5, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT)); - } - }); - } - - public void testRemoveAllRecordsFromHold() - { - doBehaviourDrivenTest(new BehaviourDrivenTest() - { - private NodeRef hold; - private NodeRef recordCategory; - private NodeRef recordFolder; - private List records = new ArrayList<>(RECORD_COUNT); - - public void given() - { - // create a hold - hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); - - // create a record folder that contains records - recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate()); - recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate()); - for (int i = 0; i < RECORD_COUNT; i++) - { - records.add(recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null)); - } - - // add records to hold - holdService.addToHold(hold, records); - } - - public void when() throws Exception - { - // remove all of the records - holdService.removeFromHold(hold, records); - } - - public void then() - { - // check record state (no longer held) - for (NodeRef record : records) - { - assertFalse(freezeService.isFrozen(record)); - assertFalse(holdService.getHeld(hold).contains(record)); - assertFalse(holdService.heldBy(record, true).contains(hold)); - } - - // record folder has frozen children - assertFalse(freezeService.isFrozen(recordFolder)); - assertFalse(freezeService.hasFrozenChildren(recordFolder)); - - // record folder is not held - assertFalse(holdService.getHeld(hold).contains(recordFolder)); - assertFalse(holdService.heldBy(recordFolder, true).contains(hold)); - - // additional check for child held caching - assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)); - assertEquals(0, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT)); - } - }); - } - - public void testRemoveRecordFolderFromHold() - { - doBehaviourDrivenTest(new BehaviourDrivenTest() - { - private NodeRef hold; - private NodeRef recordCategory; - private NodeRef recordFolder; - private List records = new ArrayList<>(RECORD_COUNT); - - public void given() - { - // create a hold - hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); - - // create a record folder that contains records - recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate()); - recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate()); - for (int i = 0; i < RECORD_COUNT; i++) - { - records.add(recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null)); - } - - // add record folder to hold - holdService.addToHold(hold, recordFolder); - } - - public void when() throws Exception - { - // remove record folder from hold - holdService.removeFromHold(hold, recordFolder); - } - - public void then() - { - // check record states - for (NodeRef record : records) - { - assertFalse(freezeService.isFrozen(record)); - assertFalse(holdService.getHeld(hold).contains(record)); - assertFalse(holdService.heldBy(record, true).contains(hold)); - } - - // record folder has frozen children - assertFalse(freezeService.isFrozen(recordFolder)); - assertFalse(freezeService.hasFrozenChildren(recordFolder)); - - // record folder is not held - assertFalse(holdService.getHeld(hold).contains(recordFolder)); - assertFalse(holdService.heldBy(recordFolder, true).contains(hold)); - - // additional check for child held caching - assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)); - assertEquals(0, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT)); - } - }); - } - - public void testPolicyNotificationForAddToHold() - { - doBehaviourDrivenTest(new BehaviourDrivenTest() - { - private NodeRef hold; - private NodeRef recordCategory; - private NodeRef recordFolder; - BehaviourDefinition beforeAddToHoldBehaviour; - BehaviourDefinition onAddToHoldBehaviour; - - public void given() - { - // create a hold - hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); - // create a record category -> record folder - recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate()); - recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate()); - - beforeAddToHoldBehaviour = policyComponent.bindClassBehaviour(BeforeAddToHoldPolicy.QNAME, - RecordsManagementModel.TYPE_HOLD, new JavaBehaviour(AddRemoveFromHoldTest.this, "beforeAddToHold", NotificationFrequency.EVERY_EVENT)); - - onAddToHoldBehaviour = policyComponent.bindClassBehaviour(OnAddToHoldPolicy.QNAME, - RecordsManagementModel.TYPE_HOLD, new JavaBehaviour(AddRemoveFromHoldTest.this, "onAddToHold", NotificationFrequency.EVERY_EVENT)); - - assertFalse(beforeAddToHoldFlag); - assertFalse(onAddToHoldFlag); - } - - public void when() throws Exception - { - // add the record folder to hold - holdService.addToHold(hold, recordFolder); - } - - public void then() - { - assertTrue(beforeAddToHoldFlag); - assertTrue(onAddToHoldFlag); - } - - public void after() - { - policyComponent.removeClassDefinition(beforeAddToHoldBehaviour); - policyComponent.removeClassDefinition(onAddToHoldBehaviour); - } - }); - } - - public void testPolicyNotificationForRemoveFromHold() - { - doBehaviourDrivenTest(new BehaviourDrivenTest() - { - private NodeRef hold; - private NodeRef recordCategory; - private NodeRef recordFolder; - BehaviourDefinition beforeRemoveFromHoldBehaviour; - BehaviourDefinition onRemoveFromHoldBehaviour; - - public void given() - { - // create a hold - hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); - // create a record category -> record folder - recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate()); - recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate()); - // add the record folder to hold - holdService.addToHold(hold, recordFolder); - - beforeRemoveFromHoldBehaviour = policyComponent.bindClassBehaviour(BeforeRemoveFromHoldPolicy.QNAME, - RecordsManagementModel.TYPE_HOLD, new JavaBehaviour(AddRemoveFromHoldTest.this, "beforeRemoveFromHold", NotificationFrequency.EVERY_EVENT)); - - onRemoveFromHoldBehaviour = policyComponent.bindClassBehaviour(OnRemoveFromHoldPolicy.QNAME, - RecordsManagementModel.TYPE_HOLD, new JavaBehaviour(AddRemoveFromHoldTest.this, "onRemoveFromHold", NotificationFrequency.EVERY_EVENT)); - - assertFalse(beforeRemoveFromHoldFlag); - assertFalse(onRemoveFromHoldFlag); - } - - public void when() throws Exception - { - // remove the record folder from the hold - holdService.removeFromHold(hold, recordFolder); - } - - public void then() - { - assertTrue(beforeRemoveFromHoldFlag); - assertTrue(onRemoveFromHoldFlag); - } - - public void after() - { - policyComponent.removeClassDefinition(beforeRemoveFromHoldBehaviour); - policyComponent.removeClassDefinition(onRemoveFromHoldBehaviour); - } - }); - } - - @Override - public void beforeAddToHold(NodeRef hold, NodeRef contentNodeRef) - { - beforeAddToHoldFlag = true; - } - - @Override - public void onAddToHold(NodeRef hold, NodeRef contentNodeRef) - { - onAddToHoldFlag = true; - } - - @Override - public void beforeRemoveFromHold(NodeRef hold, NodeRef contentNodeRef) - { - beforeRemoveFromHoldFlag = true; - } - - @Override - public void onRemoveFromHold(NodeRef hold, NodeRef contentNodeRef) - { - onRemoveFromHoldFlag = true; - } -} diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddToHoldTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddToHoldTest.java new file mode 100644 index 0000000000..0f44d5ef0e --- /dev/null +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/AddToHoldTest.java @@ -0,0 +1,315 @@ +/* + * #%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.test.integration.hold; + +import java.util.ArrayList; +import java.util.List; + +import org.alfresco.model.ContentModel; +import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.BeforeAddToHoldPolicy; +import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.OnAddToHoldPolicy; +import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; +import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase; +import org.alfresco.repo.policy.Behaviour.NotificationFrequency; +import org.alfresco.repo.policy.BehaviourDefinition; +import org.alfresco.repo.policy.ClassBehaviourBinding; +import org.alfresco.repo.policy.JavaBehaviour; +import org.alfresco.service.cmr.repository.NodeRef; +import org.springframework.extensions.webscripts.GUID; + +/** + * Add To Hold Integration Tests + * + * @author Roy Wetherall + * @since 2.2 + */ + +public class AddToHoldTest extends BaseRMTestCase implements BeforeAddToHoldPolicy, OnAddToHoldPolicy +{ + private static final int RECORD_COUNT = 10; + + private boolean beforeAddToHoldFlag = false; + private boolean onAddToHoldFlag = false; + + public void testAddRecordToHold() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + private NodeRef hold; + private NodeRef recordCategory; + private NodeRef recordFolder; + private NodeRef record; + + public void given() + { + // create a hold + hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + + // create a record folder that contains records + recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate()); + recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate()); + record = recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null); + + // assert current states + assertFalse(freezeService.isFrozen(recordFolder)); + assertFalse(freezeService.isFrozen(record)); + assertFalse(freezeService.hasFrozenChildren(recordFolder)); + + // additional check for child held caching + assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)); + assertEquals(0, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT)); + } + + public void when() throws Exception + { + // add the record to hold + holdService.addToHold(hold, record); + } + + public void then() + { + // record is held + assertTrue(freezeService.isFrozen(record)); + + // record folder has frozen children + assertFalse(freezeService.isFrozen(recordFolder)); + assertTrue(freezeService.hasFrozenChildren(recordFolder)); + + // record folder is not held + assertFalse(holdService.getHeld(hold).contains(recordFolder)); + assertFalse(holdService.heldBy(recordFolder, true).contains(hold)); + + // hold contains record + assertTrue(holdService.getHeld(hold).contains(record)); + assertTrue(holdService.heldBy(record, true).contains(hold)); + + // additional check for child held caching + assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)); + assertEquals(1, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT)); + } + }); + + } + + public void testAddRecordsToHold() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + private NodeRef hold; + private NodeRef recordCategory; + private NodeRef recordFolder; + private List records = new ArrayList<>(RECORD_COUNT); + + public void given() + { + // create a hold + hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + + // create a record folder that contains records + recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate()); + recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate()); + for (int i = 0; i < RECORD_COUNT; i++) + { + records.add(recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null)); + } + + // assert current states + assertFalse(freezeService.isFrozen(recordFolder)); + assertFalse(freezeService.hasFrozenChildren(recordFolder)); + for (NodeRef record : records) + { + assertFalse(freezeService.isFrozen(record)); + } + + // additional check for child held caching + assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)); + assertEquals(0, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT)); + } + + public void when() throws Exception + { + // add the record to hold + holdService.addToHold(hold, records); + } + + public void then() + { + // record is held + for (NodeRef record : records) + { + assertTrue(freezeService.isFrozen(record)); + } + + // record folder has frozen children + assertFalse(freezeService.isFrozen(recordFolder)); + assertTrue(freezeService.hasFrozenChildren(recordFolder)); + + // record folder is not held + assertFalse(holdService.getHeld(hold).contains(recordFolder)); + assertFalse(holdService.heldBy(recordFolder, true).contains(hold)); + + for (NodeRef record : records) + { + // hold contains record + assertTrue(holdService.getHeld(hold).contains(record)); + assertTrue(holdService.heldBy(record, true).contains(hold)); + } + + // additional check for child held caching + assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)); + assertEquals(RECORD_COUNT, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT)); + } + }); + } + + public void testAddRecordFolderToHold() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + private NodeRef hold; + private NodeRef recordCategory; + private NodeRef recordFolder; + private List records = new ArrayList<>(RECORD_COUNT); + + public void given() + { + // create a hold + hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + + // create a record folder that contains records + recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate()); + recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate()); + for (int i = 0; i < RECORD_COUNT; i++) + { + records.add(recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null)); + } + + // assert current states + assertFalse(freezeService.isFrozen(recordFolder)); + assertFalse(freezeService.hasFrozenChildren(recordFolder)); + for (NodeRef record : records) + { + assertFalse(freezeService.isFrozen(record)); + } + + // additional check for child held caching + assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)); + assertEquals(0, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT)); + } + + public void when() throws Exception + { + // add the record to hold + holdService.addToHold(hold, recordFolder); + } + + public void then() + { + for (NodeRef record : records) + { + // record is held + assertTrue(freezeService.isFrozen(record)); + assertFalse(holdService.getHeld(hold).contains(record)); + assertTrue(holdService.heldBy(record, true).contains(hold)); + } + + // record folder has frozen children + assertTrue(freezeService.isFrozen(recordFolder)); + assertTrue(freezeService.hasFrozenChildren(recordFolder)); + + // hold contains record folder + assertTrue(holdService.getHeld(hold).contains(recordFolder)); + assertTrue(holdService.heldBy(recordFolder, true).contains(hold)); + + // additional check for child held caching + assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)); + assertEquals(RECORD_COUNT, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT)); + } + }); + + } + + public void testPolicyNotificationForAddToHold() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + private NodeRef hold; + private NodeRef recordCategory; + private NodeRef recordFolder; + BehaviourDefinition beforeAddToHoldBehaviour; + BehaviourDefinition onAddToHoldBehaviour; + + public void given() + { + // create a hold + hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + // create a record category -> record folder + recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate()); + recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate()); + + beforeAddToHoldBehaviour = policyComponent.bindClassBehaviour(BeforeAddToHoldPolicy.QNAME, + RecordsManagementModel.TYPE_HOLD, new JavaBehaviour(AddToHoldTest.this, "beforeAddToHold", NotificationFrequency.EVERY_EVENT)); + + onAddToHoldBehaviour = policyComponent.bindClassBehaviour(OnAddToHoldPolicy.QNAME, + RecordsManagementModel.TYPE_HOLD, new JavaBehaviour(AddToHoldTest.this, "onAddToHold", NotificationFrequency.EVERY_EVENT)); + + assertFalse(beforeAddToHoldFlag); + assertFalse(onAddToHoldFlag); + } + + public void when() throws Exception + { + // add the record folder to hold + holdService.addToHold(hold, recordFolder); + } + + public void then() + { + assertTrue(beforeAddToHoldFlag); + assertTrue(onAddToHoldFlag); + } + + public void after() + { + policyComponent.removeClassDefinition(beforeAddToHoldBehaviour); + policyComponent.removeClassDefinition(onAddToHoldBehaviour); + } + }); + } + + @Override + public void beforeAddToHold(NodeRef hold, NodeRef contentNodeRef) + { + beforeAddToHoldFlag = true; + } + + @Override + public void onAddToHold(NodeRef hold, NodeRef contentNodeRef) + { + onAddToHoldFlag = true; + } +} diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveFromHoldTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveFromHoldTest.java new file mode 100644 index 0000000000..ab3106d91c --- /dev/null +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/hold/RemoveFromHoldTest.java @@ -0,0 +1,298 @@ +/* + * #%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.test.integration.hold; + +import java.util.ArrayList; +import java.util.List; + +import org.alfresco.model.ContentModel; +import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.BeforeRemoveFromHoldPolicy; +import org.alfresco.module.org_alfresco_module_rm.hold.HoldServicePolicies.OnRemoveFromHoldPolicy; +import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; +import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase; +import org.alfresco.repo.policy.Behaviour.NotificationFrequency; +import org.alfresco.repo.policy.BehaviourDefinition; +import org.alfresco.repo.policy.ClassBehaviourBinding; +import org.alfresco.repo.policy.JavaBehaviour; +import org.alfresco.service.cmr.repository.NodeRef; +import org.springframework.extensions.webscripts.GUID; + +/** + * Remove From Hold integration tests. + * + * @author Roy Wetherall + * @since 2.2 + */ +public class RemoveFromHoldTest extends BaseRMTestCase implements BeforeRemoveFromHoldPolicy, OnRemoveFromHoldPolicy +{ + private static final int RECORD_COUNT = 10; + + private boolean beforeRemoveFromHoldFlag = false; + private boolean onRemoveFromHoldFlag = false; + + public void testRemoveRecordsFromHold() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + private NodeRef hold; + private NodeRef recordCategory; + private NodeRef recordFolder; + private List records = new ArrayList<>(RECORD_COUNT); + + public void given() + { + // create a hold + hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + + // create a record folder that contains records + recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate()); + recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate()); + for (int i = 0; i < RECORD_COUNT; i++) + { + records.add(recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null)); + } + + // add records to hold + holdService.addToHold(hold, records); + } + + public void when() throws Exception + { + // remove *some* of the records + holdService.removeFromHold(hold, records.subList(0, 5)); + } + + public void then() + { + // check record state (no longer held) + for (NodeRef record : records.subList(0, 5)) + { + assertFalse(freezeService.isFrozen(record)); + assertFalse(holdService.getHeld(hold).contains(record)); + assertFalse(holdService.heldBy(record, true).contains(hold)); + } + + // check record state (still held) + for (NodeRef record : records.subList(5, 10)) + { + assertTrue(freezeService.isFrozen(record)); + assertTrue(holdService.getHeld(hold).contains(record)); + assertTrue(holdService.heldBy(record, true).contains(hold)); + } + + // record folder has frozen children + assertFalse(freezeService.isFrozen(recordFolder)); + assertTrue(freezeService.hasFrozenChildren(recordFolder)); + + // record folder is not held + assertFalse(holdService.getHeld(hold).contains(recordFolder)); + assertFalse(holdService.heldBy(recordFolder, true).contains(hold)); + + // additional check for child held caching + assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)); + assertEquals(5, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT)); + } + }); + } + + public void testRemoveAllRecordsFromHold() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + private NodeRef hold; + private NodeRef recordCategory; + private NodeRef recordFolder; + private List records = new ArrayList<>(RECORD_COUNT); + + public void given() + { + // create a hold + hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + + // create a record folder that contains records + recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate()); + recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate()); + for (int i = 0; i < RECORD_COUNT; i++) + { + records.add(recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null)); + } + + // add records to hold + holdService.addToHold(hold, records); + } + + public void when() throws Exception + { + // remove all of the records + holdService.removeFromHold(hold, records); + } + + public void then() + { + // check record state (no longer held) + for (NodeRef record : records) + { + assertFalse(freezeService.isFrozen(record)); + assertFalse(holdService.getHeld(hold).contains(record)); + assertFalse(holdService.heldBy(record, true).contains(hold)); + } + + // record folder has frozen children + assertFalse(freezeService.isFrozen(recordFolder)); + assertFalse(freezeService.hasFrozenChildren(recordFolder)); + + // record folder is not held + assertFalse(holdService.getHeld(hold).contains(recordFolder)); + assertFalse(holdService.heldBy(recordFolder, true).contains(hold)); + + // additional check for child held caching + assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)); + assertEquals(0, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT)); + } + }); + } + + public void testRemoveRecordFolderFromHold() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + private NodeRef hold; + private NodeRef recordCategory; + private NodeRef recordFolder; + private List records = new ArrayList<>(RECORD_COUNT); + + public void given() + { + // create a hold + hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + + // create a record folder that contains records + recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate()); + recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate()); + for (int i = 0; i < RECORD_COUNT; i++) + { + records.add(recordService.createRecordFromContent(recordFolder, GUID.generate(), ContentModel.TYPE_CONTENT, null, null)); + } + + // add record folder to hold + holdService.addToHold(hold, recordFolder); + } + + public void when() throws Exception + { + // remove record folder from hold + holdService.removeFromHold(hold, recordFolder); + } + + public void then() + { + // check record states + for (NodeRef record : records) + { + assertFalse(freezeService.isFrozen(record)); + assertFalse(holdService.getHeld(hold).contains(record)); + assertFalse(holdService.heldBy(record, true).contains(hold)); + } + + // record folder has frozen children + assertFalse(freezeService.isFrozen(recordFolder)); + assertFalse(freezeService.hasFrozenChildren(recordFolder)); + + // record folder is not held + assertFalse(holdService.getHeld(hold).contains(recordFolder)); + assertFalse(holdService.heldBy(recordFolder, true).contains(hold)); + + // additional check for child held caching + assertTrue(nodeService.hasAspect(recordFolder, ASPECT_HELD_CHILDREN)); + assertEquals(0, nodeService.getProperty(recordFolder, PROP_HELD_CHILDREN_COUNT)); + } + }); + } + + public void testPolicyNotificationForRemoveFromHold() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + private NodeRef hold; + private NodeRef recordCategory; + private NodeRef recordFolder; + BehaviourDefinition beforeRemoveFromHoldBehaviour; + BehaviourDefinition onRemoveFromHoldBehaviour; + + public void given() + { + // create a hold + hold = holdService.createHold(filePlan, GUID.generate(), GUID.generate(), GUID.generate()); + // create a record category -> record folder + recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate()); + recordFolder = recordFolderService.createRecordFolder(recordCategory, GUID.generate()); + // add the record folder to hold + holdService.addToHold(hold, recordFolder); + + beforeRemoveFromHoldBehaviour = policyComponent.bindClassBehaviour(BeforeRemoveFromHoldPolicy.QNAME, + RecordsManagementModel.TYPE_HOLD, new JavaBehaviour(RemoveFromHoldTest.this, "beforeRemoveFromHold", NotificationFrequency.EVERY_EVENT)); + + onRemoveFromHoldBehaviour = policyComponent.bindClassBehaviour(OnRemoveFromHoldPolicy.QNAME, + RecordsManagementModel.TYPE_HOLD, new JavaBehaviour(RemoveFromHoldTest.this, "onRemoveFromHold", NotificationFrequency.EVERY_EVENT)); + + assertFalse(beforeRemoveFromHoldFlag); + assertFalse(onRemoveFromHoldFlag); + } + + public void when() throws Exception + { + // remove the record folder from the hold + holdService.removeFromHold(hold, recordFolder); + } + + public void then() + { + assertTrue(beforeRemoveFromHoldFlag); + assertTrue(onRemoveFromHoldFlag); + } + + public void after() + { + policyComponent.removeClassDefinition(beforeRemoveFromHoldBehaviour); + policyComponent.removeClassDefinition(onRemoveFromHoldBehaviour); + } + }); + } + + @Override + public void beforeRemoveFromHold(NodeRef hold, NodeRef contentNodeRef) + { + beforeRemoveFromHoldFlag = true; + } + + @Override + public void onRemoveFromHold(NodeRef hold, NodeRef contentNodeRef) + { + onRemoveFromHoldFlag = 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 15cf5f8bd1..bc9ae7ca41 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 @@ -340,6 +340,8 @@ public class HoldServiceImplUnitTest extends BaseUnitTest @Test public void addToHoldNotInHold() { + mockPoliciesForAddToHold(); + holdService.addToHold(hold, recordFolder); verify(mockedNodeService).addChild(hold, recordFolder, ASSOC_FROZEN_CONTENT, ASSOC_FROZEN_CONTENT); @@ -387,6 +389,8 @@ public class HoldServiceImplUnitTest extends BaseUnitTest doReturn(true).when(mockedNodeService).hasAspect(record, ASPECT_FROZEN); doReturn(true).when(mockedNodeService).hasAspect(activeContent, ASPECT_FROZEN); + mockPoliciesForAddToHold(); + holdService.addToHold(hold, recordFolder); verify(mockedNodeService).addChild(hold, recordFolder, ASSOC_FROZEN_CONTENT, ASSOC_FROZEN_CONTENT); @@ -448,6 +452,8 @@ public class HoldServiceImplUnitTest extends BaseUnitTest }).when(mockedNodeService).addAspect(any(NodeRef.class), eq(ASPECT_FROZEN), any(Map.class)); + mockPoliciesForAddToHold(); + // build a list of holds List holds = new ArrayList<>(2); holds.add(hold); @@ -473,6 +479,8 @@ public class HoldServiceImplUnitTest extends BaseUnitTest @Test public void removeFromHoldNotInHold() { + mockPoliciesForRemoveFromHold(); + holdService.removeFromHold(hold, recordFolder); verify(mockedNodeService, never()).removeChild(hold, recordFolder); @@ -488,6 +496,8 @@ public class HoldServiceImplUnitTest extends BaseUnitTest doReturn(true).when(mockedNodeService).hasAspect(recordFolder, ASPECT_FROZEN); doReturn(true).when(mockedNodeService).hasAspect(record, ASPECT_FROZEN); + mockPoliciesForRemoveFromHold(); + holdService.removeFromHold(hold, recordFolder); verify(mockedNodeService, times(1)).removeChild(hold, recordFolder); @@ -504,6 +514,8 @@ public class HoldServiceImplUnitTest extends BaseUnitTest doReturn(true).when(mockedNodeService).hasAspect(recordFolder, ASPECT_FROZEN); doReturn(true).when(mockedNodeService).hasAspect(record, ASPECT_FROZEN); + mockPoliciesForRemoveFromHold(); + // build a list of holds List holds = new ArrayList<>(2); holds.add(hold); @@ -536,6 +548,8 @@ public class HoldServiceImplUnitTest extends BaseUnitTest }).when(mockedNodeService).removeChild(hold, recordFolder); + mockPoliciesForRemoveFromHold(); + doAnswer(new Answer() { public Void answer(InvocationOnMock invocation) @@ -576,4 +590,22 @@ public class HoldServiceImplUnitTest extends BaseUnitTest holds.add(hold2); holdService.removeFromHolds(holds, activeContent); } + + /** + * mocks policies for add to hold + */ + private void mockPoliciesForAddToHold() + { + doNothing().when(holdService).invokeBeforeAddToHold(any(), any()); + doNothing().when(holdService).invokeOnAddToHold(any(), any()); + } + + /** + * mocks policies for remove from hold + */ + private void mockPoliciesForRemoveFromHold() + { + doNothing().when(holdService).invokeBeforeRemoveFromHold(any(), any()); + doNothing().when(holdService).invokeOnRemoveFromHold(any(), any()); + } } From 3919622169285ea20d41c8ffc597b90b830e274a Mon Sep 17 00:00:00 2001 From: Rodica Sutu Date: Mon, 4 Nov 2019 17:24:09 +0200 Subject: [PATCH 06/11] refactor the code to fix the sonar issue "Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed." --- .../PropertyModificationAllowedCheck.java | 56 +++++++++---------- 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java index b1d90afa65..49dc26b716 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java @@ -30,6 +30,7 @@ import java.io.Serializable; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import org.alfresco.service.namespace.QName; @@ -80,49 +81,44 @@ public class PropertyModificationAllowedCheck /** * Compares the node properties with the requested update to make sure all potential updates are permitted + * * @param before current node properties - * @param after updated properties for the node + * @param after updated properties for the node * @return true - if all modified property keys are in the whitelist */ public boolean check(Map before, Map after) { boolean proceed = true; - HashSet unionKeys = new HashSet<>(before.keySet()); - unionKeys.addAll(after.keySet()); - for (QName key : unionKeys) + // Initially check for changes to existing keys and values. + for (Map.Entry entry : before.entrySet()) { - //Check if property has been added or removed - if (!before.containsKey(key) || !after.containsKey(key)) + QName key = entry.getKey(); + Serializable beforeValue = entry.getValue(); + //check if property has been updated + boolean modified = after.containsKey(key) && after.get(key) != null + && !after.get(key).equals(beforeValue); + + //check if the property has been emptied or removed + boolean propertyRemovedEmptied = after.get(key) == null || !after.containsKey(key); + if (modified || propertyRemovedEmptied) { - //Property modified check to see if allowed proceed = allowPropertyUpdate(key); - if (!proceed) - { - break; - } } - //Check if property emptied or empty property filled - if ((before.get(key) == null && after.get(key) != null) || - (after.get(key) == null && before.get(key) != null)) + if (!proceed) { - //Property modified check to see if allowed - proceed = allowPropertyUpdate(key); - if (!proceed) - { - break; - } - } - //If properties aren't missing or empty check equality - if (before.get(key) != null && after.get(key) != null && !(after.get(key).equals(before.get(key)))) - { - //Property modified check to see if allowed - proceed = allowPropertyUpdate(key); - if (!proceed) - { - break; - } + return proceed; } } + + // Check for new values. Record individual values and group as a single map. + Set newKeys = new HashSet<>(after.keySet()); + newKeys.removeAll(before.keySet()); + for (QName key : newKeys) + { + proceed = allowPropertyUpdate(key); + if (!proceed) + break; + } return proceed; } From 7805e2116e907a11d8c15da7883492bdf0228f84 Mon Sep 17 00:00:00 2001 From: Rodica Sutu Date: Mon, 4 Nov 2019 17:42:25 +0200 Subject: [PATCH 07/11] fix the case when before and after values are null --- .../util/PropertyModificationAllowedCheck.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java index 49dc26b716..592f704971 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java @@ -99,7 +99,8 @@ public class PropertyModificationAllowedCheck && !after.get(key).equals(beforeValue); //check if the property has been emptied or removed - boolean propertyRemovedEmptied = after.get(key) == null || !after.containsKey(key); + boolean propertyRemovedEmptied = (after.get(key) == null && beforeValue !=null) + || !after.containsKey(key); if (modified || propertyRemovedEmptied) { proceed = allowPropertyUpdate(key); From da4e9986f7afd6999a1713593f8d61788c956140 Mon Sep 17 00:00:00 2001 From: Roxana Lucanu Date: Tue, 5 Nov 2019 10:32:16 +0200 Subject: [PATCH 08/11] RM-7034 style changes --- .../org_alfresco_module_rm/hold/HoldServiceImpl.java | 12 ++++++------ .../hold/HoldServiceImplUnitTest.java | 1 - 2 files changed, 6 insertions(+), 7 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 0a57d8b3b4..4b817cabc7 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 @@ -942,7 +942,7 @@ public class HoldServiceImpl extends ServiceBaseImpl /** * Invoke beforeAddToHold policy * - * @param hold hold node reference + * @param hold hold node reference * @param contentNodeRef content node reference */ protected void invokeBeforeAddToHold(NodeRef hold, NodeRef contentNodeRef) @@ -954,8 +954,8 @@ public class HoldServiceImpl extends ServiceBaseImpl /** * Invoke onAddToHold policy * - *@param hold hold node reference - *@param contentNodeRef content node reference + * @param hold hold node reference + * @param contentNodeRef content node reference */ protected void invokeOnAddToHold(NodeRef hold, NodeRef contentNodeRef) { @@ -966,8 +966,8 @@ public class HoldServiceImpl extends ServiceBaseImpl /** * Invoke beforeRemoveFromHold policy * - *@param hold hold node reference - *@param contentNodeRef content node reference + * @param hold hold node reference + * @param contentNodeRef content node reference */ protected void invokeBeforeRemoveFromHold(NodeRef hold, NodeRef contentNodeRef) { @@ -978,7 +978,7 @@ public class HoldServiceImpl extends ServiceBaseImpl /** * Invoke onRemoveFromHold policy * - * @param hold hold node reference + * @param hold hold node reference * @param contentNodeRef content node reference */ protected void invokeOnRemoveFromHold(NodeRef hold, NodeRef contentNodeRef) 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 bc9ae7ca41..55de14c20d 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 @@ -71,7 +71,6 @@ import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; -import org.mockito.Matchers; import org.mockito.Mock; import org.mockito.Spy; import org.mockito.invocation.InvocationOnMock; From be4b035b4d33dd697a457235654bda44233359c6 Mon Sep 17 00:00:00 2001 From: Rodica Sutu Date: Tue, 5 Nov 2019 11:43:32 +0200 Subject: [PATCH 09/11] code reviews changes and couple sonar fixes --- .../rm-model-context.xml | 2 +- .../PropertyModificationAllowedCheck.java | 46 +++++++++++-------- ...pertyModificationAllowedCheckUnitTest.java | 6 +-- 3 files changed, 30 insertions(+), 24 deletions(-) diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml index d849ae16b1..9119c2f01c 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-model-context.xml @@ -178,7 +178,7 @@ - + http://www.alfresco.org/model/system/1.0 diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java index 592f704971..68de8e85d8 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java @@ -46,14 +46,25 @@ public class PropertyModificationAllowedCheck * List of qnames that can be modified */ private List whiteList; - - /** + + /** * List of model URI's for which the properties can be updated */ private List editableURIs; + /** + * Getter for list of model URI's + * + * @return return the list of model URI's + */ + private List getEditableURIs() + { + return editableURIs; + } + /** * Setter for list of model URI's + * * @param editableURIs List */ public void setEditableURIs(List editableURIs) @@ -61,17 +72,9 @@ public class PropertyModificationAllowedCheck this.editableURIs = editableURIs; } - /** - * Getter for list of model URI's - * @return return the list of model URI's - */ - public List getEditableURIs() - { - return editableURIs; - } - /** * Setter for list of qnames + * * @param whiteList List */ public void setWhiteList(List whiteList) @@ -90,17 +93,17 @@ public class PropertyModificationAllowedCheck { boolean proceed = true; // Initially check for changes to existing keys and values. - for (Map.Entry entry : before.entrySet()) + for (final Map.Entry entry : before.entrySet()) { - QName key = entry.getKey(); - Serializable beforeValue = entry.getValue(); + final QName key = entry.getKey(); + final Serializable beforeValue = entry.getValue(); //check if property has been updated - boolean modified = after.containsKey(key) && after.get(key) != null + final boolean modified = after.containsKey(key) && after.get(key) != null && !after.get(key).equals(beforeValue); //check if the property has been emptied or removed - boolean propertyRemovedEmptied = (after.get(key) == null && beforeValue !=null) - || !after.containsKey(key); + final boolean propertyRemovedEmptied = (after.get(key) == null && beforeValue != null) + || !after.containsKey(key); if (modified || propertyRemovedEmptied) { proceed = allowPropertyUpdate(key); @@ -112,21 +115,24 @@ public class PropertyModificationAllowedCheck } // Check for new values. Record individual values and group as a single map. - Set newKeys = new HashSet<>(after.keySet()); + final Set newKeys = new HashSet<>(after.keySet()); newKeys.removeAll(before.keySet()); - for (QName key : newKeys) + for (final QName key : newKeys) { proceed = allowPropertyUpdate(key); if (!proceed) + { break; + } } return proceed; } /** * Determines whether the property should be allowed to be updated or not. + * * @param key property - * @return true if property update us allowed + * @return true if property update is allowed */ private boolean allowPropertyUpdate(QName key) { diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheckUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheckUnitTest.java index fca0a8b9cb..a152a7c533 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheckUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheckUnitTest.java @@ -68,14 +68,14 @@ public class PropertyModificationAllowedCheckUnitTest { MockitoAnnotations.initMocks(this); propertyModificationAllowedCheck = new PropertyModificationAllowedCheck(); - before = new HashMap(); + before = new HashMap(); after = new HashMap(); - qName = QName.createQName("foo","bar"); + qName = QName.createQName("foo", "bar"); qName2 = QName.createQName("bar", "foo"); before.put(qName, serializable); after.put(qName, serializable2); list = new ArrayList(); - editableURIs =new ArrayList<>(); + editableURIs = new ArrayList<>(); propertyModificationAllowedCheck.setWhiteList(list); propertyModificationAllowedCheck.setEditableURIs(editableURIs); } From af2b45f9ce205e7fe96c575c2cbdd31b5670c5df Mon Sep 17 00:00:00 2001 From: Rodica Sutu Date: Tue, 5 Nov 2019 16:43:57 +0200 Subject: [PATCH 10/11] update java docs & fix sonar "Mutable members should not be stored or returned directly" from PropertyModificationAllowedCheck class --- .../util/PropertyModificationAllowedCheck.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java index 68de8e85d8..88544fbbd5 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/PropertyModificationAllowedCheck.java @@ -26,6 +26,8 @@ */ package org.alfresco.module.org_alfresco_module_rm.util; +import static java.util.Collections.unmodifiableList; + import java.io.Serializable; import java.util.HashSet; import java.util.List; @@ -59,7 +61,7 @@ public class PropertyModificationAllowedCheck */ private List getEditableURIs() { - return editableURIs; + return unmodifiableList(editableURIs); } /** @@ -69,7 +71,7 @@ public class PropertyModificationAllowedCheck */ public void setEditableURIs(List editableURIs) { - this.editableURIs = editableURIs; + this.editableURIs = unmodifiableList(editableURIs); } /** @@ -79,7 +81,8 @@ public class PropertyModificationAllowedCheck */ public void setWhiteList(List whiteList) { - this.whiteList = whiteList; + this.whiteList = unmodifiableList(whiteList); + } /** @@ -87,7 +90,8 @@ public class PropertyModificationAllowedCheck * * @param before current node properties * @param after updated properties for the node - * @return true - if all modified property keys are in the whitelist + * @return true - if all modified property keys are in the whitelist or + * in the list of model URI's for which the properties can be modified */ public boolean check(Map before, Map after) { From caac83aa68c38653768cdcef30e072f0ad3287c7 Mon Sep 17 00:00:00 2001 From: Sara Aspery Date: Wed, 6 Nov 2019 16:00:53 +0000 Subject: [PATCH 11/11] RM-7026 Updated hold name display property --- .../org_alfresco_module_rm/audit/event/HoldUtils.java | 5 ++++- .../service/RecordsManagementAuditServiceImplTest.java | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/HoldUtils.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/HoldUtils.java index c87aa61309..641b7f016d 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/HoldUtils.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/HoldUtils.java @@ -28,6 +28,7 @@ package org.alfresco.module.org_alfresco_module_rm.audit.event; import org.alfresco.model.ContentModel; +import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.namespace.QName; @@ -44,6 +45,8 @@ import java.util.Map; */ class HoldUtils { + /** A QName to display for the hold name. */ + public static final QName HOLD_NAME = QName.createQName(RecordsManagementModel.RM_URI, "Hold Name"); /** * Create a properties map containing the hold name for the given hold. * @@ -55,7 +58,7 @@ class HoldUtils { Map auditProperties = new HashMap<>(); - auditProperties.put(ContentModel.PROP_NAME, nodeService.getProperty(nodeRef, ContentModel.PROP_NAME)); + auditProperties.put(HOLD_NAME, nodeService.getProperty(nodeRef, ContentModel.PROP_NAME)); return auditProperties; } 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 5633fbcf62..ee1528263a 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 @@ -40,6 +40,7 @@ import org.alfresco.module.org_alfresco_module_rm.audit.RecordsManagementAuditQu import org.alfresco.module.org_alfresco_module_rm.audit.RecordsManagementAuditService; import org.alfresco.module.org_alfresco_module_rm.audit.event.AuditEvent; 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.repo.security.authentication.AuthenticationException; import org.alfresco.repo.security.authentication.AuthenticationUtil; @@ -60,6 +61,9 @@ import org.alfresco.util.Pair; public class RecordsManagementAuditServiceImplTest extends BaseRMTestCase implements RMPermissionModel { + /** A QName to display for the hold name. */ + public static final QName HOLD_NAME = QName.createQName(RecordsManagementModel.RM_URI, "Hold Name"); + /** Test record */ private NodeRef record; @@ -606,7 +610,7 @@ public class RecordsManagementAuditServiceImplTest extends BaseRMTestCase { // check create hold audit event includes the hold name assertEquals("Create Hold event does not include hold name.", holdName, - auditEventProperties.get(PROP_NAME)); + auditEventProperties.get(HOLD_NAME)); // check create hold audit event includes the hold reason assertEquals("Create Hold event does not include hold reason.", holdReason,