From 3cd009116272ec6a8f5efa973868eecc91cde341 Mon Sep 17 00:00:00 2001 From: Derek Hulley Date: Sun, 16 Oct 2011 20:16:52 +0000 Subject: [PATCH] Fixed RuleService concurrency around enable/disable at NodeRef level - Done while rolling in ALF-10839: Eliminate rule discovery overhead on property update when rules have been disabled - Some checking of rule state done BEFORE walking up the node hierarchy - Also fixes ALF-4216: disabledRules List is not thread safe git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@31255 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- config/alfresco/rule-services-context.xml | 25 ++--- .../repo/rule/RuleServiceCoverageTest.java | 65 ++++++++----- .../alfresco/repo/rule/RuleServiceImpl.java | 43 ++++----- .../org/alfresco/repo/rule/RuleTypeImpl.java | 72 +++++++------- ...foreDeleteChildAssociationRuleTrigger.java | 6 ++ .../ruletrigger/CreateNodeRuleTrigger.java | 13 ++- .../OnContentUpdateRuleTrigger.java | 5 + .../OnCreateChildAssociationRuleTrigger.java | 6 ++ .../ruletrigger/OnMoveNodeRuleTrigger.java | 10 ++ .../OnPropertyUpdateRuleTrigger.java | 5 + .../ruletrigger/RuleTriggerAbstractBase.java | 96 +++++++++++-------- .../SingleNodeRefPolicyRuleTrigger.java | 5 + .../service/cmr/rule/RuleService.java | 1 - 13 files changed, 203 insertions(+), 149 deletions(-) diff --git a/config/alfresco/rule-services-context.xml b/config/alfresco/rule-services-context.xml index 5e90a60d92..8563cd291e 100644 --- a/config/alfresco/rule-services-context.xml +++ b/config/alfresco/rule-services-context.xml @@ -91,27 +91,16 @@ - - - - - - - - - - - - - - - + + + + + + - - - + true diff --git a/source/java/org/alfresco/repo/rule/RuleServiceCoverageTest.java b/source/java/org/alfresco/repo/rule/RuleServiceCoverageTest.java index e340e8ef71..22a7832bb9 100644 --- a/source/java/org/alfresco/repo/rule/RuleServiceCoverageTest.java +++ b/source/java/org/alfresco/repo/rule/RuleServiceCoverageTest.java @@ -925,7 +925,7 @@ public class RuleServiceCoverageTest extends TestCase this.ruleService.saveRule(this.nodeRef, rule); - MailActionExecuter mailService = (MailActionExecuter) ((ApplicationContextFactory) this.applicationContext + MailActionExecuter mailService = (MailActionExecuter) ((ApplicationContextFactory) applicationContext .getBean("OutboundSMTP")).getApplicationContext().getBean("mail"); mailService.setTestMode(true); mailService.clearLastTestMessage(); @@ -965,7 +965,7 @@ public class RuleServiceCoverageTest extends TestCase String illegalName = "MyName.txt "; // space at end - MailActionExecuter mailService = (MailActionExecuter) ((ApplicationContextFactory) this.applicationContext + MailActionExecuter mailService = (MailActionExecuter) ((ApplicationContextFactory) applicationContext .getBean("OutboundSMTP")).getApplicationContext().getBean("mail"); mailService.setTestMode(true); mailService.clearLastTestMessage(); @@ -1399,28 +1399,45 @@ public class RuleServiceCoverageTest extends TestCase NoConditionEvaluator.NAME, null); - this.ruleService.saveRule(this.nodeRef, rule); - this.ruleService.disableRules(this.nodeRef); - - NodeRef newNodeRef = this.nodeService.createNode( - this.nodeRef, - ContentModel.ASSOC_CHILDREN, - QName.createQName(TEST_NAMESPACE, "children"), - ContentModel.TYPE_CONTENT, - getContentProperties()).getChildRef(); - addContentToNode(newNodeRef); - assertFalse(this.nodeService.hasAspect(newNodeRef, ContentModel.ASPECT_VERSIONABLE)); - - this.ruleService.enableRules(this.nodeRef); - - NodeRef newNodeRef2 = this.nodeService.createNode( - this.nodeRef, - ContentModel.ASSOC_CHILDREN, - QName.createQName(TEST_NAMESPACE, "children"), - ContentModel.TYPE_CONTENT, - getContentProperties()).getChildRef(); - addContentToNode(newNodeRef2); - assertTrue(this.nodeService.hasAspect(newNodeRef2, ContentModel.ASPECT_VERSIONABLE)); + this.ruleService.saveRule(this.nodeRef, rule); + + RetryingTransactionCallback noRulesWork = new RetryingTransactionCallback() + { + @Override + public NodeRef execute() throws Throwable + { + ruleService.disableRules(nodeRef); + + NodeRef newNodeRef = nodeService.createNode( + nodeRef, + ContentModel.ASSOC_CHILDREN, + QName.createQName(TEST_NAMESPACE, "children"), + ContentModel.TYPE_CONTENT, + getContentProperties()).getChildRef(); + addContentToNode(newNodeRef); + return newNodeRef; + } + }; + NodeRef newNodeRef = transactionService.getRetryingTransactionHelper().doInTransaction(noRulesWork); + assertFalse(nodeService.hasAspect(newNodeRef, ContentModel.ASPECT_VERSIONABLE)); + + RetryingTransactionCallback withRulesWork = new RetryingTransactionCallback() + { + @Override + public NodeRef execute() throws Throwable + { + NodeRef newNodeRef2 = nodeService.createNode( + nodeRef, + ContentModel.ASSOC_CHILDREN, + QName.createQName(TEST_NAMESPACE, "children"), + ContentModel.TYPE_CONTENT, + getContentProperties()).getChildRef(); + addContentToNode(newNodeRef2); + return newNodeRef2; + } + }; + NodeRef newNodeRef2 = transactionService.getRetryingTransactionHelper().doInTransaction(withRulesWork); + assertTrue(nodeService.hasAspect(newNodeRef2, ContentModel.ASPECT_VERSIONABLE)); } /** diff --git a/source/java/org/alfresco/repo/rule/RuleServiceImpl.java b/source/java/org/alfresco/repo/rule/RuleServiceImpl.java index 18b1c14c79..44b136af1d 100644 --- a/source/java/org/alfresco/repo/rule/RuleServiceImpl.java +++ b/source/java/org/alfresco/repo/rule/RuleServiceImpl.java @@ -40,6 +40,7 @@ import org.alfresco.repo.policy.PolicyComponent; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.transaction.AlfrescoTransactionSupport; import org.alfresco.repo.transaction.TransactionListener; +import org.alfresco.repo.transaction.TransactionalResourceHelper; import org.alfresco.service.cmr.action.Action; import org.alfresco.service.cmr.action.ActionService; import org.alfresco.service.cmr.action.ActionServiceException; @@ -79,6 +80,12 @@ public class RuleServiceImpl NodeServicePolicies.OnUpdateNodePolicy, NodeServicePolicies.OnAddAspectPolicy { + /** key against which to store disabled rule types in the current txn */ + private static final String KEY_DISABLED_RULE_TYPES = "RuleServiceImpl.disabledRuleTypes"; + + /** key against which to store disabled rule nodes in the current txn */ + private static final String KEY_DISABLED_RULE_NODES = "RuleServiceImpl.disabledRuleNodes"; + /** key against which to store rules pending on the current transaction */ private static final String KEY_RULES_PENDING = "RuleServiceImpl.PendingRules"; @@ -116,23 +123,12 @@ public class RuleServiceImpl */ private SimpleCache> nodeRulesCache; - /** - * List of disabled node refs. The rules associated with these nodes will node be added to the pending list, and - * therefore not fired. This list is transient. - * - * TODO: (DH) Make this txn-local - */ - private Set disabledNodeRefs = new HashSet(5); - /** * List of disabled rules. Any rules that appear in this list will not be added to the pending list and therefore * not fired. */ private Set disabledRules = new HashSet(5); - /** List of disables rule types */ - private Set disabledRuleTypes = new HashSet(3); - /** * All the rule type currently registered */ @@ -388,21 +384,22 @@ public class RuleServiceImpl @Override public boolean rulesEnabled(NodeRef nodeRef) { - return (this.disabledNodeRefs.contains(nodeRef) == false); + Set disabledRuleNodes = TransactionalResourceHelper.getSet(KEY_DISABLED_RULE_NODES); + return !disabledRuleNodes.contains(nodeRef); } @Override public void disableRules(NodeRef nodeRef) { - // Add the node to the set of disabled nodes - this.disabledNodeRefs.add(nodeRef); + Set disabledRuleNodes = TransactionalResourceHelper.getSet(KEY_DISABLED_RULE_NODES); + disabledRuleNodes.add(nodeRef); } @Override public void enableRules(NodeRef nodeRef) { - // Remove the node from the set of disabled nodes - this.disabledNodeRefs.remove(nodeRef); + Set disabledRuleNodes = TransactionalResourceHelper.getSet(KEY_DISABLED_RULE_NODES); + disabledRuleNodes.remove(nodeRef); } @Override @@ -420,24 +417,22 @@ public class RuleServiceImpl @Override public void disableRuleType(String ruleType) { + Set disabledRuleTypes = TransactionalResourceHelper.getSet(KEY_DISABLED_RULE_TYPES); disabledRuleTypes.add(ruleType); } @Override public void enableRuleType(String ruleType) { - disabledRuleTypes.remove(ruleType); + Set disabledRuleTypes = TransactionalResourceHelper.getSet(KEY_DISABLED_RULE_TYPES); + disabledRuleTypes.remove(ruleType); } @Override public boolean isRuleTypeEnabled(String ruleType) { - boolean result = true; - if (disabledRuleTypes.contains(ruleType) == true) - { - result = false; - } - return result; + Set disabledRuleTypes = TransactionalResourceHelper.getSet(KEY_DISABLED_RULE_TYPES); + return !disabledRuleTypes.contains(ruleType); } @Override @@ -1021,7 +1016,7 @@ public class RuleServiceImpl // First check to see if the node has been disabled if (this.isEnabled() == true && - this.disabledNodeRefs.contains(this.getOwningNodeRef(rule)) == false && + this.rulesEnabled(this.getOwningNodeRef(rule)) && this.disabledRules.contains(rule) == false) { PendingRuleData pendingRuleData = new PendingRuleData(actionableNodeRef, actionedUponNodeRef, rule, executeAtEnd); diff --git a/source/java/org/alfresco/repo/rule/RuleTypeImpl.java b/source/java/org/alfresco/repo/rule/RuleTypeImpl.java index d7a1ce0bbe..b8703aae04 100644 --- a/source/java/org/alfresco/repo/rule/RuleTypeImpl.java +++ b/source/java/org/alfresco/repo/rule/RuleTypeImpl.java @@ -20,8 +20,6 @@ package org.alfresco.repo.rule; import java.util.List; -import org.springframework.extensions.surf.util.I18NUtil; -import org.alfresco.model.ContentModel; import org.alfresco.repo.action.CommonResourceAbstractBase; import org.alfresco.repo.rule.ruletrigger.RuleTrigger; import org.alfresco.service.cmr.repository.NodeRef; @@ -31,6 +29,7 @@ import org.alfresco.service.cmr.rule.RuleService; import org.alfresco.service.cmr.rule.RuleType; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.extensions.surf.util.I18NUtil; /** * Rule type implementation class. @@ -44,30 +43,30 @@ public class RuleTypeImpl extends CommonResourceAbstractBase implements RuleType */ private static Log logger = LogFactory.getLog(RuleTypeImpl.class); - /** - * The rule service - */ - private RuleService ruleService; - - /** - * The node service - */ - private NodeService nodeService; + /** + * The rule service + */ + private RuleService ruleService; + + /** + * The node service + */ + private NodeService nodeService; /** * Constructor * - * @param ruleTriggers the rule triggers + * @param ruleTriggers the rule triggers */ public RuleTypeImpl(List ruleTriggers) { - if (ruleTriggers != null) - { - for (RuleTrigger trigger : ruleTriggers) - { - trigger.registerRuleType(this); - } - } + if (ruleTriggers != null) + { + for (RuleTrigger trigger : ruleTriggers) + { + trigger.registerRuleType(this); + } + } } /** @@ -76,18 +75,18 @@ public class RuleTypeImpl extends CommonResourceAbstractBase implements RuleType * @param ruleService the rule service */ public void setRuleService(RuleService ruleService) - { - this.ruleService = ruleService; - } + { + this.ruleService = ruleService; + } /** * Set the node service * - * @param nodeService the node service + * @param nodeService the node service */ public void setNodeService(NodeService nodeService) { - this.nodeService = nodeService; + this.nodeService = nodeService; } /** @@ -95,7 +94,7 @@ public class RuleTypeImpl extends CommonResourceAbstractBase implements RuleType */ public void init() { - ((RuntimeRuleService)this.ruleService).registerRuleType(this); + ((RuntimeRuleService)this.ruleService).registerRuleType(this); } /** @@ -117,21 +116,18 @@ public class RuleTypeImpl extends CommonResourceAbstractBase implements RuleType /** * @see org.alfresco.service.cmr.rule.RuleType#triggerRuleType(org.alfresco.service.cmr.repository.NodeRef, org.alfresco.service.cmr.repository.NodeRef) */ - public void triggerRuleType(NodeRef nodeRef, NodeRef actionedUponNodeRef, boolean executeRuleImmediately) - { - if (ruleService.isEnabled() == true && - nodeService.exists(actionedUponNodeRef) == true && - nodeService.hasAspect(actionedUponNodeRef, ContentModel.ASPECT_TEMPORARY) == false && - // Temporary workaround to prevent rules running on cm:rating nodes (which happened for 'liked' folders ALF-8308 & ALF-8382) - ContentModel.TYPE_RATING.equals(nodeService.getType(actionedUponNodeRef)) == false && - ruleService.isRuleTypeEnabled(this.getName()) == true) + public void triggerRuleType(NodeRef nodeRef, NodeRef actionedUponNodeRef, boolean executeRuleImmediately) + { + if (ruleService.isEnabled() == true && + nodeService.exists(actionedUponNodeRef) == true && + ruleService.isRuleTypeEnabled(this.getName()) == true) { - List rules = ruleService.getRules( - nodeRef, + List rules = ruleService.getRules( + nodeRef, true, this.name); - - if (rules.size() != 0) + + if (rules.size() != 0) { for (Rule rule : rules) { @@ -168,5 +164,5 @@ public class RuleTypeImpl extends CommonResourceAbstractBase implements RuleType } } } - } + } } diff --git a/source/java/org/alfresco/repo/rule/ruletrigger/BeforeDeleteChildAssociationRuleTrigger.java b/source/java/org/alfresco/repo/rule/ruletrigger/BeforeDeleteChildAssociationRuleTrigger.java index 43ea840b44..6c2f23b3f4 100644 --- a/source/java/org/alfresco/repo/rule/ruletrigger/BeforeDeleteChildAssociationRuleTrigger.java +++ b/source/java/org/alfresco/repo/rule/ruletrigger/BeforeDeleteChildAssociationRuleTrigger.java @@ -82,6 +82,12 @@ public class BeforeDeleteChildAssociationRuleTrigger public void beforeDeleteChildAssociation(ChildAssociationRef childAssocRef) { + // Break out early if rules are not enabled + if (!areRulesEnabled()) + { + return; + } + NodeRef childNodeRef = childAssocRef.getChildRef(); // Avoid renamed nodes diff --git a/source/java/org/alfresco/repo/rule/ruletrigger/CreateNodeRuleTrigger.java b/source/java/org/alfresco/repo/rule/ruletrigger/CreateNodeRuleTrigger.java index 23de4a3c76..11ac2cd048 100644 --- a/source/java/org/alfresco/repo/rule/ruletrigger/CreateNodeRuleTrigger.java +++ b/source/java/org/alfresco/repo/rule/ruletrigger/CreateNodeRuleTrigger.java @@ -59,7 +59,7 @@ public class CreateNodeRuleTrigger extends RuleTriggerAbstractBase private boolean isClassBehaviour = false; /** Runtime rule service */ - RuntimeRuleService ruleService; + RuntimeRuleService runtimeRuleService; /** * Set whether this is a class behaviour or not @@ -72,9 +72,9 @@ public class CreateNodeRuleTrigger extends RuleTriggerAbstractBase /** * Set the rule service */ - public void setRuleService(RuntimeRuleService ruleService) + public void setRuntimeRuleService(RuntimeRuleService runtimeRuleService) { - this.ruleService = ruleService; + this.runtimeRuleService = runtimeRuleService; } /** @@ -113,6 +113,11 @@ public class CreateNodeRuleTrigger extends RuleTriggerAbstractBase */ public void onCreateNode(ChildAssociationRef childAssocRef) { + // Break out early if rules are not enabled + if (!areRulesEnabled()) + { + return; + } NodeRef nodeRef = childAssocRef.getChildRef(); // Keep track of new nodes to prevent firing of updates in the same transaction @@ -156,7 +161,7 @@ public class CreateNodeRuleTrigger extends RuleTriggerAbstractBase } // Removes any rules that have already been triggered for that node - ruleService.removeRulePendingExecution(nodeRef); + runtimeRuleService.removeRulePendingExecution(nodeRef); } /** diff --git a/source/java/org/alfresco/repo/rule/ruletrigger/OnContentUpdateRuleTrigger.java b/source/java/org/alfresco/repo/rule/ruletrigger/OnContentUpdateRuleTrigger.java index 9f5e576df3..57cb7042fd 100644 --- a/source/java/org/alfresco/repo/rule/ruletrigger/OnContentUpdateRuleTrigger.java +++ b/source/java/org/alfresco/repo/rule/ruletrigger/OnContentUpdateRuleTrigger.java @@ -88,6 +88,11 @@ public class OnContentUpdateRuleTrigger extends RuleTriggerAbstractBase */ public void onContentUpdate(NodeRef nodeRef, boolean newContent) { + // Break out early if rules are not enabled + if (!areRulesEnabled()) + { + return; + } // Check the new content and make sure that we do indeed want to trigger the rule boolean fail = false; diff --git a/source/java/org/alfresco/repo/rule/ruletrigger/OnCreateChildAssociationRuleTrigger.java b/source/java/org/alfresco/repo/rule/ruletrigger/OnCreateChildAssociationRuleTrigger.java index 96596e760b..9e8ca9d365 100644 --- a/source/java/org/alfresco/repo/rule/ruletrigger/OnCreateChildAssociationRuleTrigger.java +++ b/source/java/org/alfresco/repo/rule/ruletrigger/OnCreateChildAssociationRuleTrigger.java @@ -81,6 +81,12 @@ public class OnCreateChildAssociationRuleTrigger public void onCreateChildAssociation(ChildAssociationRef childAssocRef, boolean isNewNode) { + // Break out early if rules are not enabled + if (!areRulesEnabled()) + { + return; + } + // Avoid new nodes if (isNewNode) { diff --git a/source/java/org/alfresco/repo/rule/ruletrigger/OnMoveNodeRuleTrigger.java b/source/java/org/alfresco/repo/rule/ruletrigger/OnMoveNodeRuleTrigger.java index 9cf958e203..3d0db954de 100644 --- a/source/java/org/alfresco/repo/rule/ruletrigger/OnMoveNodeRuleTrigger.java +++ b/source/java/org/alfresco/repo/rule/ruletrigger/OnMoveNodeRuleTrigger.java @@ -39,6 +39,11 @@ public class OnMoveNodeRuleTrigger extends RuleTriggerAbstractBase implements No public void onMoveNode(ChildAssociationRef oldChildAssocRef, ChildAssociationRef newChildAssocRef) { + // Break out early if rules are not enabled + if (!areRulesEnabled()) + { + return; + } // Check that it is not rename operation. if (!oldChildAssocRef.getParentRef().equals(newChildAssocRef.getParentRef())) { @@ -48,6 +53,11 @@ public class OnMoveNodeRuleTrigger extends RuleTriggerAbstractBase implements No private void triggerChildrenRules(ChildAssociationRef oldChildAssocRef, ChildAssociationRef newChildAssocRef) { + // Break out early if rules are not enabled + if (!areRulesEnabled()) + { + return; + } triggerRules(newChildAssocRef.getParentRef(), newChildAssocRef.getChildRef()); for (ChildAssociationRef ref : nodeService.getChildAssocs(newChildAssocRef.getChildRef())) { diff --git a/source/java/org/alfresco/repo/rule/ruletrigger/OnPropertyUpdateRuleTrigger.java b/source/java/org/alfresco/repo/rule/ruletrigger/OnPropertyUpdateRuleTrigger.java index a1f6084da9..4ca79f884e 100644 --- a/source/java/org/alfresco/repo/rule/ruletrigger/OnPropertyUpdateRuleTrigger.java +++ b/source/java/org/alfresco/repo/rule/ruletrigger/OnPropertyUpdateRuleTrigger.java @@ -125,6 +125,11 @@ public class OnPropertyUpdateRuleTrigger extends RuleTriggerAbstractBase */ public void onUpdateProperties(NodeRef nodeRef, Map before, Map after) { + // Break out early if rules are not enabled + if (!areRulesEnabled()) + { + return; + } // Do not fire if the node has been created in this transaction Set newNodeRefSet = TransactionalResourceHelper.getSet(RULE_TRIGGER_NEW_NODES); boolean wasCreatedInTxn = newNodeRefSet.contains(nodeRef); diff --git a/source/java/org/alfresco/repo/rule/ruletrigger/RuleTriggerAbstractBase.java b/source/java/org/alfresco/repo/rule/ruletrigger/RuleTriggerAbstractBase.java index 8c7ab46840..45c5427d20 100644 --- a/source/java/org/alfresco/repo/rule/ruletrigger/RuleTriggerAbstractBase.java +++ b/source/java/org/alfresco/repo/rule/ruletrigger/RuleTriggerAbstractBase.java @@ -30,6 +30,7 @@ import org.alfresco.service.cmr.dictionary.DictionaryService; import org.alfresco.service.cmr.repository.ContentService; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; +import org.alfresco.service.cmr.rule.RuleService; import org.alfresco.service.cmr.rule.RuleType; import org.alfresco.service.namespace.QName; @@ -40,33 +41,34 @@ import org.alfresco.service.namespace.QName; */ public abstract class RuleTriggerAbstractBase implements RuleTrigger { + /** the types (hardcoded) to ignore generally */ + private static final Set IGNORE_TYPES; + /** the aspects (hardcoded) to ignore generally */ + private static final Set IGNORE_ASPECTS; + static + { + IGNORE_TYPES = new HashSet(13); + IGNORE_TYPES.add(RuleModel.TYPE_RULE); + IGNORE_TYPES.add(ActionModel.TYPE_ACTION); + IGNORE_TYPES.add(ContentModel.TYPE_THUMBNAIL); + // Workaround to prevent rules running on cm:rating nodes (which happened for 'liked' folders ALF-8308 & ALF-8382) + IGNORE_TYPES.add(ContentModel.TYPE_RATING); + + IGNORE_ASPECTS = new HashSet(13); + IGNORE_ASPECTS.add(ContentModel.ASPECT_TEMPORARY); + } + /** * A list of the rule types that are interested in this trigger */ private Set ruleTypes = new HashSet(); - /** - * The policy component - */ protected PolicyComponent policyComponent; - - /** - * The node service - */ protected NodeService nodeService; - - /** - * The content service - */ protected ContentService contentService; - - /** - * The authentication Component - */ protected AuthenticationComponent authenticationComponent; - - /** The dictionary service */ protected DictionaryService dictionaryService; + protected RuleService ruleService; /** * Indicates whether the rule should be executed immediately or at the end of the transaction. @@ -76,9 +78,6 @@ public abstract class RuleTriggerAbstractBase implements RuleTrigger /** * Set the policy component - * - * @param policyComponent - * the policy component */ public void setPolicyComponent(PolicyComponent policyComponent) { @@ -87,9 +86,6 @@ public abstract class RuleTriggerAbstractBase implements RuleTrigger /** * Set the node service - * - * @param nodeService - * the node service */ public void setNodeService(NodeService nodeService) { @@ -98,8 +94,6 @@ public abstract class RuleTriggerAbstractBase implements RuleTrigger /** * Set the content service - * - * @param contentService the content service */ public void setContentService(ContentService contentService) { @@ -116,17 +110,22 @@ public abstract class RuleTriggerAbstractBase implements RuleTrigger /** * Set the dictionary service - * - * @param dictionaryService the dictionary service */ public void setDictionaryService(DictionaryService dictionaryService) { this.dictionaryService = dictionaryService; } - + /** - * Sets the values that indicates whether the rule should be executed immediately - * or not. + * Set the RuleService to assist with enabled/disabled check + */ + public void setRuleService(RuleService ruleService) + { + this.ruleService = ruleService; + } + + /** + * Sets the values that indicates whether the rule should be executed immediately or not. * * @param executeRuleImmediately true execute the rule immediaely, false otherwise */ @@ -148,13 +147,16 @@ public abstract class RuleTriggerAbstractBase implements RuleTrigger * Trigger the rules that relate to any interested rule types for the node * references passed. * - * @param nodeRef - * the node reference who rules are to be triggered - * @param actionedUponNodeRef - * the node reference that will be actioned upon by the rules + * @param nodeRef the node reference who rules are to be triggered + * @param actionedUponNodeRef the node reference that will be actioned upon by the rules */ protected void triggerRules(NodeRef nodeRef, NodeRef actionedUponNodeRef) { + // Break out early if rules are off + if (!areRulesEnabled()) + { + return; + } // Do not trigger rules for rule and action type nodes if (ignoreTrigger(actionedUponNodeRef) == false) { @@ -165,22 +167,36 @@ public abstract class RuleTriggerAbstractBase implements RuleTrigger } } + /** + * Helper method to allow triggers to check if rules are enabled or disabled + * (ALF-10839: Eliminate rule discovery overhead on property update when rules have been disabled) + * @return true if rules are enabled + */ + protected boolean areRulesEnabled() + { + return ruleService.isEnabled(); + } + /** * Indicate whether the trigger should be ignored or not - * @param actionedUponNodeRef actioned upon node reference - * @return boolean true if the trigger should be ignored, false otherwise + * @param actionedUponNodeRef actioned upon node reference + * @return boolean true if the trigger should be ignored, false otherwise */ private boolean ignoreTrigger(NodeRef actionedUponNodeRef) { boolean result = false; QName typeQName = nodeService.getType(actionedUponNodeRef); - if (typeQName.equals(RuleModel.TYPE_RULE) == true || - typeQName.equals(ActionModel.TYPE_ACTION) == true || - typeQName.equals(ActionModel.TYPE_COMPOSITE_ACTION) == true || - typeQName.equals(ContentModel.TYPE_THUMBNAIL) == true) + if (IGNORE_TYPES.contains(typeQName)) { result = true; } + for (QName aspectToIgnore : IGNORE_ASPECTS) + { + if (nodeService.hasAspect(actionedUponNodeRef, aspectToIgnore)) + { + return true; + } + } return result; } } diff --git a/source/java/org/alfresco/repo/rule/ruletrigger/SingleNodeRefPolicyRuleTrigger.java b/source/java/org/alfresco/repo/rule/ruletrigger/SingleNodeRefPolicyRuleTrigger.java index 36b60f8aa0..a23f595b3c 100644 --- a/source/java/org/alfresco/repo/rule/ruletrigger/SingleNodeRefPolicyRuleTrigger.java +++ b/source/java/org/alfresco/repo/rule/ruletrigger/SingleNodeRefPolicyRuleTrigger.java @@ -67,6 +67,11 @@ public class SingleNodeRefPolicyRuleTrigger extends RuleTriggerAbstractBase public void policyBehaviour(NodeRef nodeRef) { + // Break out early if rules are not enabled + if (!areRulesEnabled()) + { + return; + } if (triggerParentRules == true) { List parentsAssocRefs = this.nodeService.getParentAssocs(nodeRef); diff --git a/source/java/org/alfresco/service/cmr/rule/RuleService.java b/source/java/org/alfresco/service/cmr/rule/RuleService.java index 96624d0c5e..9fc799112b 100644 --- a/source/java/org/alfresco/service/cmr/rule/RuleService.java +++ b/source/java/org/alfresco/service/cmr/rule/RuleService.java @@ -21,7 +21,6 @@ package org.alfresco.service.cmr.rule; import java.util.List; import org.alfresco.service.Auditable; -import org.alfresco.service.PublicService; import org.alfresco.service.cmr.action.Action; import org.alfresco.service.cmr.repository.NodeRef;