From e0cef2c6e1b9cae1fd6e7195c1aae6390d0a8cf6 Mon Sep 17 00:00:00 2001 From: Roy Wetherall Date: Tue, 15 Mar 2016 10:49:19 +1100 Subject: [PATCH 1/3] Ensure that rules are not inherited onto the file plan, unfiled records, holds and transfers root containers (RM-3148) --- .../action/impl/DeclareRecordAction.java | 3 + .../fileplan/FilePlanServiceImpl.java | 26 +- .../model/rma/type/FilePlanType.java | 8 +- .../repo/rule/ExtendedRuleServiceImpl.java | 2 +- .../rule/FilePlanRuleInheritanceTest.java | 239 ++++++++++++++++++ 5 files changed, 258 insertions(+), 20 deletions(-) create mode 100644 rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/rule/FilePlanRuleInheritanceTest.java diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/action/impl/DeclareRecordAction.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/action/impl/DeclareRecordAction.java index f872c6c63a..a6454c1f31 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/action/impl/DeclareRecordAction.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/action/impl/DeclareRecordAction.java @@ -59,6 +59,9 @@ import org.springframework.extensions.surf.util.I18NUtil; */ public class DeclareRecordAction extends RMActionExecuterAbstractBase { + /** action name */ + public static final String NAME = "declareRecord"; + /** I18N */ private static final String MSG_UNDECLARED_ONLY_RECORDS = "rm.action.undeclared-only-records"; private static final String MSG_NO_DECLARE_MAND_PROP = "rm.action.no-declare-mand-prop"; diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/fileplan/FilePlanServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/fileplan/FilePlanServiceImpl.java index 4a26bb62ce..df8389cd3b 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/fileplan/FilePlanServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/fileplan/FilePlanServiceImpl.java @@ -49,6 +49,7 @@ import org.alfresco.module.org_alfresco_module_rm.security.ExtendedWriterDynamic import org.alfresco.module.org_alfresco_module_rm.util.ServiceBaseImpl; import org.alfresco.repo.cache.SimpleCache; import org.alfresco.repo.domain.node.NodeDAO; +import org.alfresco.repo.rule.RuleModel; import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.StoreRef; @@ -358,23 +359,14 @@ public class FilePlanServiceImpl extends ServiceBaseImpl containerType, properties).getChildRef(); - // if (!inheritPermissions) - // { - // set inheritance to false - getPermissionService().setInheritParentPermissions(container, false); - getPermissionService().setPermission(container, allRoles, RMPermissionModel.READ_RECORDS, true); - getPermissionService().setPermission(container, ExtendedReaderDynamicAuthority.EXTENDED_READER, RMPermissionModel.READ_RECORDS, true); - getPermissionService().setPermission(container, ExtendedWriterDynamicAuthority.EXTENDED_WRITER, RMPermissionModel.FILING, true); - - // TODO set the admin users to have filing permissions on the unfiled container!!! - // TODO we will need to be able to get a list of the admin roles from the service - // } - // else - // { - // just inherit eveything - // TODO will change this when we are able to set permissions on holds and transfers! - // getPermissionService().setInheritParentPermissions(container, true); - // } + // set inheritance to false + getPermissionService().setInheritParentPermissions(container, false); + getPermissionService().setPermission(container, allRoles, RMPermissionModel.READ_RECORDS, true); + getPermissionService().setPermission(container, ExtendedReaderDynamicAuthority.EXTENDED_READER, RMPermissionModel.READ_RECORDS, true); + getPermissionService().setPermission(container, ExtendedWriterDynamicAuthority.EXTENDED_WRITER, RMPermissionModel.FILING, true); + + // prevent inheritance of rules + nodeService.addAspect(container, RuleModel.ASPECT_IGNORE_INHERITED_RULES, null); return container; } diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/type/FilePlanType.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/type/FilePlanType.java index 248614f7cd..ef930ddc9e 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/type/FilePlanType.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/type/FilePlanType.java @@ -42,6 +42,7 @@ import org.alfresco.repo.policy.Behaviour.NotificationFrequency; import org.alfresco.repo.policy.annotation.Behaviour; import org.alfresco.repo.policy.annotation.BehaviourBean; import org.alfresco.repo.policy.annotation.BehaviourKind; +import org.alfresco.repo.rule.RuleModel; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; import org.alfresco.service.cmr.repository.ChildAssociationRef; @@ -181,8 +182,11 @@ public class FilePlanType extends BaseBehaviourBean { public Object doWork() { - if (nodeService.hasAspect(filePlan, ASPECT_FILE_PLAN_COMPONENT) && - nodeService.getProperty(filePlan, PROP_IDENTIFIER) == null) + // ensure rules are not inherited + nodeService.addAspect(filePlan, RuleModel.ASPECT_IGNORE_INHERITED_RULES, null); + + // set the identifier + if (nodeService.getProperty(filePlan, PROP_IDENTIFIER) == null) { String id = getIdentifierService().generateIdentifier(filePlan); nodeService.setProperty(filePlan, RecordsManagementModel.PROP_IDENTIFIER, id); diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/repo/rule/ExtendedRuleServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/repo/rule/ExtendedRuleServiceImpl.java index 05f49dd424..e6c0ed2f8e 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/repo/rule/ExtendedRuleServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/repo/rule/ExtendedRuleServiceImpl.java @@ -195,7 +195,7 @@ public class ExtendedRuleServiceImpl extends RuleServiceImpl else { // run as current user - ExtendedRuleServiceImpl.super.executeRule(rule, nodeRef, executedRules); + super.executeRule(rule, nodeRef, executedRules); } } } diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/rule/FilePlanRuleInheritanceTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/rule/FilePlanRuleInheritanceTest.java new file mode 100644 index 0000000000..55caa28ecf --- /dev/null +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/rule/FilePlanRuleInheritanceTest.java @@ -0,0 +1,239 @@ +/* + * #%L + * Alfresco Records Management Module + * %% + * Copyright (C) 2005 - 2016 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.rule; + +import java.util.List; + +import org.alfresco.module.org_alfresco_module_rm.action.impl.DeclareRecordAction; +import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase; +import org.alfresco.service.cmr.action.Action; +import org.alfresco.service.cmr.repository.NodeRef; +import org.alfresco.service.cmr.rule.Rule; +import org.alfresco.service.cmr.rule.RuleService; + +/** + * File plan rule inheritance test + * + * @author Roy Wetherall + * @since 2.4 + */ +public class FilePlanRuleInheritanceTest extends BaseRMTestCase +{ + private RuleService ruleService; + + @Override + protected void initServices() + { + super.initServices(); + ruleService = (RuleService)applicationContext.getBean("RuleService"); + } + + @Override + protected boolean isRMSiteTest() + { + return false; + } + + private NodeRef createFilePlan() + { + return filePlanService.createFilePlan(folder, "My File Plan"); + } + + /** + * Given that a singel rule is set on the parent folder of the file plan root + * And that it is configured to apply to children + * When we ask for the rules on the file plan, inclusing those inherited + * Then it will not include thos defined on the parent folder + */ + public void testFilePlanDoesNotInheritRulesFromParentFolder() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + private NodeRef filePlan = null; + private List rules = null; + + public void given() + { + filePlan = createFilePlan(); + + // create a rule that applies to childre + Action completeRecordAction = actionService.createAction(DeclareRecordAction.NAME); + Rule rule = new Rule(); + rule.setRuleType("inbound"); + rule.setAction(completeRecordAction); + rule.applyToChildren(true); + + // save rule on file plan root parent folder + ruleService.saveRule(folder, rule); + } + + public void when() + { + // get rules, including those inherited + rules = ruleService.getRules(filePlan, true); + } + + public void then() + { + // rules aren't inhreited from file plan root parent folder + assertEquals(0, rules.size()); + } + }); + } + + /** + * Given that a single rule is set on the file plan root + * And that it is configured to apply to children + * When we ask for the rules on the unfiled record container including those inherited + * Then it will not include those defined on the file plan root + * + * See https://issues.alfresco.com/jira/browse/RM-3148 + */ + public void testFilePlanRulesInheritedInUnfiledContainer() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + private NodeRef filePlan = null; + private List rules = null; + + public void given() + { + filePlan = createFilePlan(); + + // create a rule that applies to childre + Action completeRecordAction = actionService.createAction(DeclareRecordAction.NAME); + Rule rule = new Rule(); + rule.setRuleType("inbound"); + rule.setAction(completeRecordAction); + rule.applyToChildren(true); + + // save rule on file plan root + ruleService.saveRule(filePlan, rule); + } + + public void when() + { + // get rules, including those inherited + NodeRef unfiledRecordContainer = filePlanService.getUnfiledContainer(filePlan); + rules = ruleService.getRules(unfiledRecordContainer, true); + } + + public void then() + { + // rules aren't inhreited from file plan root + assertEquals(0, rules.size()); + } + }); + } + + /** + * Given that a single rule is set on the file plan root + * And that it is configured to apply to children + * When we ask for the rules on the hold container including those inherited + * Then it will not include those defined on the file plan root + */ + public void testFilePlanRulesInheritedInHoldContainer() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + private NodeRef filePlan = null; + private List rules = null; + + public void given() + { + filePlan = createFilePlan(); + + // create a rule that applies to childre + Action completeRecordAction = actionService.createAction(DeclareRecordAction.NAME); + Rule rule = new Rule(); + rule.setRuleType("inbound"); + rule.setAction(completeRecordAction); + rule.applyToChildren(true); + + // save rule on file plan root + ruleService.saveRule(filePlan, rule); + } + + public void when() + { + // get rules, including those inherited + NodeRef container = filePlanService.getHoldContainer(filePlan); + rules = ruleService.getRules(container, true); + } + + public void then() + { + // rules aren't inhreited from file plan root + assertEquals(0, rules.size()); + } + }); + } + + /** + * Given that a single rule is set on the file plan root + * And that it is configured to apply to children + * When we ask for the rules on the transfer container including those inherited + * Then it will not include those defined on the file plan root + */ + public void testFilePlanRulesInheritedInTransferContainer() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + private NodeRef filePlan = null; + private List rules = null; + + public void given() + { + filePlan = createFilePlan(); + + // create a rule that applies to childre + Action completeRecordAction = actionService.createAction(DeclareRecordAction.NAME); + Rule rule = new Rule(); + rule.setRuleType("inbound"); + rule.setAction(completeRecordAction); + rule.applyToChildren(true); + + // save rule on file plan root + ruleService.saveRule(filePlan, rule); + } + + public void when() + { + // get rules, including those inherited + NodeRef container = filePlanService.getTransferContainer(filePlan); + rules = ruleService.getRules(container, true); + } + + public void then() + { + // rules aren't inhreited from file plan root + assertEquals(0, rules.size()); + } + }); + } +} From 1c6e95efe88cdb09d84080475a0d9845f7e1e98b Mon Sep 17 00:00:00 2001 From: Roy Wetherall Date: Tue, 15 Mar 2016 11:20:53 +1100 Subject: [PATCH 2/3] Extend integration test to show rule inheritance on record categories working as expected (RM-3148) --- .../rule/FilePlanRuleInheritanceTest.java | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/rule/FilePlanRuleInheritanceTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/rule/FilePlanRuleInheritanceTest.java index 55caa28ecf..ab97dc4ecd 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/rule/FilePlanRuleInheritanceTest.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/rule/FilePlanRuleInheritanceTest.java @@ -35,6 +35,7 @@ import org.alfresco.service.cmr.action.Action; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.rule.Rule; import org.alfresco.service.cmr.rule.RuleService; +import org.springframework.extensions.webscripts.GUID; /** * File plan rule inheritance test @@ -236,4 +237,48 @@ public class FilePlanRuleInheritanceTest extends BaseRMTestCase } }); } + + /** + * Given that a single rule is set on the file plan root + * And that it is configured to apply to children + * When we ask for the rules on a record category including those inherited + * Then it will include those defined on the file plan root + */ + public void testFilePlanRulesInheritedOnRecordCategory() + { + doBehaviourDrivenTest(new BehaviourDrivenTest() + { + private NodeRef filePlan = null; + private NodeRef recordCategory = null; + private List rules = null; + + public void given() + { + filePlan = createFilePlan(); + recordCategory = filePlanService.createRecordCategory(filePlan, GUID.generate()); + + // create a rule that applies to childre + Action completeRecordAction = actionService.createAction(DeclareRecordAction.NAME); + Rule rule = new Rule(); + rule.setRuleType("inbound"); + rule.setAction(completeRecordAction); + rule.applyToChildren(true); + + // save rule on file plan root + ruleService.saveRule(filePlan, rule); + } + + public void when() + { + // get rules, including those inherited + rules = ruleService.getRules(recordCategory, true); + } + + public void then() + { + // rules aren't inhreited from file plan root + assertEquals(1, rules.size()); + } + }); + } } From 44a188dabf45d091292ef1f0bf23f2c19f9e34cc Mon Sep 17 00:00:00 2001 From: Roy Wetherall Date: Tue, 15 Mar 2016 11:59:13 +1100 Subject: [PATCH 3/3] Remove now unessesary checks from rule execution (RM-3148) --- .../repo/rule/ExtendedRuleServiceImpl.java | 100 +++++------------- .../rule/FilePlanRuleInheritanceTest.java | 6 +- 2 files changed, 32 insertions(+), 74 deletions(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/repo/rule/ExtendedRuleServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/repo/rule/ExtendedRuleServiceImpl.java index e6c0ed2f8e..7965967d8d 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/repo/rule/ExtendedRuleServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/repo/rule/ExtendedRuleServiceImpl.java @@ -171,39 +171,37 @@ public class ExtendedRuleServiceImpl extends RuleServiceImpl if (nodeService.exists(nodeRef)) { QName typeQName = nodeService.getType(nodeRef); - if (shouldRuleBeAppliedToNode(rule, nodeRef, typeQName)) + + // check if this is a rm rule on a rm artifact + if (filePlanService.isFilePlanComponent(nodeRef) && + isFilePlanComponentRule(rule)) { - // check if this is a rm rule on a rm artifact - if (filePlanService.isFilePlanComponent(nodeRef) && - isFilePlanComponentRule(rule)) - { - // ignore and - if (!isIgnoredType(typeQName)) - { - if (runAsAdmin) - { - AuthenticationUtil.runAs(new RunAsWork() + // ignore and + if (!isIgnoredType(typeQName)) + { + if (runAsAdmin) + { + AuthenticationUtil.runAs(new RunAsWork() + { + @Override + public Void doWork() { - @Override - public Void doWork() - { - ExtendedRuleServiceImpl.super.executeRule(rule, nodeRef, executedRules); - return null; - } - }, AuthenticationUtil.getAdminUserName()); - } - else - { - // run as current user - super.executeRule(rule, nodeRef, executedRules); - } - } - } - else - { - // just execute the rule as the current user - super.executeRule(rule, nodeRef, executedRules); - } + ExtendedRuleServiceImpl.super.executeRule(rule, nodeRef, executedRules); + return null; + } + }, AuthenticationUtil.getAdminUserName()); + } + else + { + // run as current user + super.executeRule(rule, nodeRef, executedRules); + } + } + } + else + { + // just execute the rule as the current user + super.executeRule(rule, nodeRef, executedRules); } } } @@ -228,44 +226,4 @@ public class ExtendedRuleServiceImpl extends RuleServiceImpl { return ignoredTypes.contains(typeQName); } - - /** - * Check if the rule is associated with the file plan component that the node it is being - * applied to isn't a hold container, a hold, a transfer container, a transfer, an unfiled - * record container, an unfiled record folder or unfiled content - * - * @param rule - * @param nodeRef - * @param typeQName - * @return - */ - private boolean shouldRuleBeAppliedToNode(final Rule rule, final NodeRef nodeRef, final QName typeQName) - { - return AuthenticationUtil.runAsSystem(new RunAsWork() - { - public Boolean doWork() throws Exception - { - boolean result = true; - NodeRef ruleNodeRef = getOwningNodeRef(rule); - if (filePlanService.isFilePlan(ruleNodeRef)) - { - // if this rule is defined at the root of the file plan then - // we do not want to apply - // it to holds/transfers/unfiled content... - result = !(RecordsManagementModel.TYPE_HOLD.equals(typeQName) - || RecordsManagementModel.TYPE_HOLD_CONTAINER.equals(typeQName) - || RecordsManagementModel.TYPE_TRANSFER.equals(typeQName) - || RecordsManagementModel.TYPE_TRANSFER_CONTAINER.equals(typeQName) - || RecordsManagementModel.TYPE_UNFILED_RECORD_CONTAINER - .equals(typeQName) - || RecordsManagementModel.TYPE_UNFILED_RECORD_FOLDER.equals(typeQName) - || nodeService.hasAspect(nodeRef, - RecordsManagementModel.ASPECT_TRANSFERRING) - || nodeService.hasAspect(nodeRef, RecordsManagementModel.ASPECT_FROZEN) - || !recordService.isFiled(nodeRef)); - } - return result; - } - }); - } } diff --git a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/rule/FilePlanRuleInheritanceTest.java b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/rule/FilePlanRuleInheritanceTest.java index ab97dc4ecd..e4cecd7033 100644 --- a/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/rule/FilePlanRuleInheritanceTest.java +++ b/rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/rule/FilePlanRuleInheritanceTest.java @@ -66,10 +66,10 @@ public class FilePlanRuleInheritanceTest extends BaseRMTestCase } /** - * Given that a singel rule is set on the parent folder of the file plan root + * Given that a single rule is set on the parent folder of the file plan root * And that it is configured to apply to children - * When we ask for the rules on the file plan, inclusing those inherited - * Then it will not include thos defined on the parent folder + * When we ask for the rules on the file plan, including those inherited + * Then it will not include those defined on the parent folder */ public void testFilePlanDoesNotInheritRulesFromParentFolder() {