REPO-3029/MNT-13055: fix recursion for "manual" running of rules.

When a person explicitly runs a set of rules and requests that they run
recursively, the recursion was failing.

The original tests even proved that.

src/main/java/org/alfresco/repo/action/executer/ExecuteAllRulesActionExecuter.java

@@ -1,32 +1,30 @@
-/*
+/*
- * #%L
+ * #%L
- * Alfresco Repository
+ * Alfresco Repository
- * %%
+ * %%
- * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * Copyright (C) 2005 - 2016 Alfresco Software Limited
- * %%
+ * %%
- * This file is part of the Alfresco software. 
+ * This file is part of the Alfresco software. 
- * If the software was purchased under a paid Alfresco license, the terms of 
+ * If the software was purchased under a paid Alfresco license, the terms of 
- * the paid license agreement will prevail.  Otherwise, the software is 
+ * the paid license agreement will prevail.  Otherwise, the software is 
- * provided under the following open source license terms:
+ * provided under the following open source license terms:
- * 
+ * 
- * Alfresco is free software: you can redistribute it and/or modify
+ * 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
+ * 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
+ * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
+ * (at your option) any later version.
- * 
+ * 
- * Alfresco is distributed in the hope that it will be useful,
+ * Alfresco is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
+ * GNU Lesser General Public License for more details.
- * 
+ * 
- * You should have received a copy of the GNU Lesser General Public License
+ * You should have received a copy of the GNU Lesser General Public License
- * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
- * #L%
+ * #L%
- */
+ */
 package org.alfresco.repo.action.executer;
 
-import java.util.List;
-
 import org.alfresco.model.ContentModel;
 import org.alfresco.repo.action.ParameterDefinitionImpl;
 import org.alfresco.repo.rule.RuntimeRuleService;
@@ -41,6 +39,9 @@ import org.alfresco.service.cmr.rule.Rule;
 import org.alfresco.service.cmr.rule.RuleService;
 import org.alfresco.service.namespace.QName;
 
+import java.util.ArrayList;
+import java.util.List;
+
 /**
  * This action executes all rules present on the actioned upon node 
  * 
@@ -116,7 +117,12 @@ public class ExecuteAllRulesActionExecuter extends ActionExecuterAbstractBase
     /**
      * @see org.alfresco.repo.action.executer.ActionExecuter#execute(Action, NodeRef)
      */
-    public void executeImpl(Action ruleAction, NodeRef actionedUponNodeRef)
+    public void executeImpl(final Action ruleAction, NodeRef actionedUponNodeRef)
+    {
+        executeImpl(ruleAction, actionedUponNodeRef, null);
+    }
+    
+    private void executeImpl(final Action ruleAction, NodeRef actionedUponNodeRef, List<Rule> parentRules)
     {
         if (this.nodeService.exists(actionedUponNodeRef) == true)
         {
@@ -134,10 +140,34 @@ public class ExecuteAllRulesActionExecuter extends ActionExecuterAbstractBase
             {
                 runAllChildren = runAllChildrenValue.booleanValue();
             }
+
+            // Collect all the rules to execute on the current node.
+            List<Rule> rules = new ArrayList<>();
+            // This is a recursive method, collect the rules specified for this particular invocation's node.
+            List<Rule> currentNodeRules = ruleService.getRules(actionedUponNodeRef, includeInherited);
+            if (currentNodeRules != null)
+            {
+                rules.addAll(currentNodeRules);
+            }
+
+            if (runAllChildren)
+            {
+                if (parentRules != null)
+                {
+                    // Currently recursing into a child folder, add the rules from the recursive set
+                    // to any other rules we've collected for this node.
+                    rules.addAll(parentRules);
+                }
+                else
+                {
+                    // Currently executing on the "top-level" folder.
+                    // We'll propagate the rules of the top-level node during recursive calls,
+                    // so save the rules (parentRules) to pass down the line.
+                    parentRules = currentNodeRules;
+                }
+            }
             
-            // Get the rules
+            if (!rules.isEmpty())
-            List<Rule> rules = ruleService.getRules(actionedUponNodeRef, includeInherited);
-            if (rules != null && rules.isEmpty() == false)
             {
                 // Get the child nodes for the actioned upon node
                 List<ChildAssociationRef> children = nodeService.getChildAssocs(actionedUponNodeRef);
@@ -167,8 +197,8 @@ public class ExecuteAllRulesActionExecuter extends ActionExecuterAbstractBase
                         if (runAllChildren == true &&
                             dictionaryService.isSubClass(childType, ContentModel.TYPE_FOLDER) == true)
                         {
-                            // Recurse with the child folder
+                            // Recurse with the child folder, passing down the top-level folder rules.
-                            executeImpl(ruleAction, child);
+                            executeImpl(ruleAction, child, parentRules);
                         }
                     }                    
                 }

src/test/java/org/alfresco/repo/action/executer/ExecuteAllRulesActionExecuterTest.java

@@ -1,36 +1,33 @@
-/*
+/*
- * #%L
+ * #%L
- * Alfresco Repository
+ * Alfresco Repository
- * %%
+ * %%
- * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * Copyright (C) 2005 - 2016 Alfresco Software Limited
- * %%
+ * %%
- * This file is part of the Alfresco software. 
+ * This file is part of the Alfresco software. 
- * If the software was purchased under a paid Alfresco license, the terms of 
+ * If the software was purchased under a paid Alfresco license, the terms of 
- * the paid license agreement will prevail.  Otherwise, the software is 
+ * the paid license agreement will prevail.  Otherwise, the software is 
- * provided under the following open source license terms:
+ * provided under the following open source license terms:
- * 
+ * 
- * Alfresco is free software: you can redistribute it and/or modify
+ * 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
+ * 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
+ * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
+ * (at your option) any later version.
- * 
+ * 
- * Alfresco is distributed in the hope that it will be useful,
+ * Alfresco is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
+ * GNU Lesser General Public License for more details.
- * 
+ * 
- * You should have received a copy of the GNU Lesser General Public License
+ * You should have received a copy of the GNU Lesser General Public License
- * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
- * #L%
+ * #L%
- */
+ */
 package org.alfresco.repo.action.executer;
 
-import java.io.Serializable;
-import java.util.HashMap;
-import java.util.Map;
-
 import org.alfresco.model.ContentModel;
 import org.alfresco.repo.action.ActionImpl;
+import org.alfresco.repo.rule.LinkRules;
 import org.alfresco.repo.security.authentication.AuthenticationComponent;
 import org.alfresco.repo.transaction.RetryingTransactionHelper;
 import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback;
@@ -38,6 +35,10 @@ import org.alfresco.repo.version.VersionModel;
 import org.alfresco.service.cmr.action.Action;
 import org.alfresco.service.cmr.action.ActionService;
 import org.alfresco.service.cmr.coci.CheckOutCheckInService;
+import org.alfresco.service.cmr.model.FileFolderService;
+import org.alfresco.service.cmr.model.FileInfo;
+import org.alfresco.service.cmr.repository.ContentService;
+import org.alfresco.service.cmr.repository.ContentWriter;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
 import org.alfresco.service.cmr.repository.StoreRef;
@@ -48,11 +49,17 @@ import org.alfresco.service.cmr.version.Version;
 import org.alfresco.service.cmr.version.VersionType;
 import org.alfresco.service.namespace.QName;
 import org.alfresco.test_category.BaseSpringTestsCategory;
-import org.alfresco.test_category.OwnJVMTestsCategory;
 import org.alfresco.util.BaseSpringTest;
 import org.alfresco.util.GUID;
 import org.junit.experimental.categories.Category;
 
+import java.io.Serializable;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.alfresco.repo.rule.RuleModel.ASPECT_IGNORE_INHERITED_RULES;
+
 /**
  * Execute all rules action execution test
  * 
@@ -75,6 +82,10 @@ public class ExecuteAllRulesActionExecuterTest extends BaseSpringTest
     
     private RetryingTransactionHelper transactionHelper;
     
+    private FileFolderService fileFolderService;
+    
+    private ContentService contentService;
+    
     /** The store reference */
     private StoreRef testStoreRef;
     
@@ -98,6 +109,8 @@ public class ExecuteAllRulesActionExecuterTest extends BaseSpringTest
         this.ruleService = (RuleService)this.applicationContext.getBean("ruleService");
         this.actionService = (ActionService)this.applicationContext.getBean("actionService");
         transactionHelper = (RetryingTransactionHelper)applicationContext.getBean("retryingTransactionHelper");
+        fileFolderService = applicationContext.getBean("fileFolderService", FileFolderService.class);
+        contentService = applicationContext.getBean("contentService", ContentService.class);
         
         AuthenticationComponent authenticationComponent = (AuthenticationComponent)applicationContext.getBean("authenticationComponent");
         authenticationComponent.setCurrentUser(authenticationComponent.getSystemUserName());
@@ -301,8 +314,10 @@ public class ExecuteAllRulesActionExecuterTest extends BaseSpringTest
                 assertTrue(nodeService.hasAspect(doc2, ContentModel.ASPECT_VERSIONABLE));
                 assertTrue(nodeService.hasAspect(doc2, ContentModel.ASPECT_CLASSIFIABLE));
                 assertFalse(nodeService.hasAspect(doc2, ContentModel.ASPECT_TITLED));
-                assertFalse(nodeService.hasAspect(doc3, ContentModel.ASPECT_VERSIONABLE));
+                // Running recursively (PARAM_RUN_ALL_RULES_ON_CHILDREN), so folder2 has folder1's rules applied
-                assertFalse(nodeService.hasAspect(doc3, ContentModel.ASPECT_CLASSIFIABLE));
+                // in addition to its own rule.
+                assertTrue(nodeService.hasAspect(doc3, ContentModel.ASPECT_VERSIONABLE));
+                assertTrue(nodeService.hasAspect(doc3, ContentModel.ASPECT_CLASSIFIABLE));
                 assertTrue(nodeService.hasAspect(doc3, ContentModel.ASPECT_TITLED));
                 
                 clearAspects(doc1);
@@ -314,6 +329,184 @@ public class ExecuteAllRulesActionExecuterTest extends BaseSpringTest
         }); 
     }
     
+    /**
+     * Test for MNT-13055:
+     * 
+     * We create a folder structure thus:
+     * <pre>
+     * rootFolder (rule: add an aspect)
+     * |
+     * +- folderA ("do not inherit rules" + link to rootFolder's rule set) 
+     *    |
+     *    +- fileA
+     *    |
+     *    +- folderB
+     *       |
+     *       +- fileB 
+     * </pre>
+     * This test case specifies a rule on rootFolder that adds an aspect to all files within.
+     * folderA has the ignoreInheritedRules aspect applied, but also links to rootFolder's rule set.
+     * <p>
+     * We then explicitly execute folderA's rules, including for subfolders. Ultimately, we want to
+     * know that fileB did indeed have the ruleset executed on it, and therefore gained the classifiable aspect.
+     */
+    public void testRulesFireOnNonInheritedFolderUsingLinkedRuleSetInstead()
+    {
+        final NodeRef rootFolder = this.nodeService.createNode(
+                this.rootNodeRef,
+                ContentModel.ASSOC_CHILDREN,
+                QName.createQName("{test}rootFolderForTest"),
+                ContentModel.TYPE_FOLDER).getChildRef();
+        
+        final NodeRef folderA = fileFolderService.
+                create(rootFolder, "folderA", ContentModel.TYPE_FOLDER).getNodeRef();
+        
+        // Set folderA to "Don't Inherit Rules".
+        nodeService.addAspect(folderA, ASPECT_IGNORE_INHERITED_RULES, Collections.emptyMap());
+        
+        NodeRef fileA = createFile(folderA, "fileA.txt").getNodeRef();
+        
+        final NodeRef folderB = fileFolderService.
+                create(folderA, "folderB", ContentModel.TYPE_FOLDER).getNodeRef();
+        
+        NodeRef fileB = createFile(folderB, "fileB.txt").getNodeRef();
+        
+        // Create a rule on rootFolder
+        Rule rule = new Rule();
+        rule.setRuleType(RuleType.INBOUND);
+        Action action = actionService.createAction(AddFeaturesActionExecuter.NAME);
+        action.setParameterValue(AddFeaturesActionExecuter.PARAM_ASPECT_NAME, ContentModel.ASPECT_CLASSIFIABLE);
+        rule.setAction(action);
+        // It mustn't matter whether this rule "applies to children", since we'll explicitly
+        // run it on "the folder and its subfolders".
+        rule.applyToChildren(false);
+        rule.setExecuteAsynchronously(false);
+        ruleService.enableRule(rule);
+        ruleService.saveRule(rootFolder, rule);
+
+        // Link to Rule Set of rootFolder to folderA
+        Action linkAction = actionService.createAction(LinkRules.NAME);
+        linkAction.setParameterValue(LinkRules.PARAM_LINK_FROM_NODE, rootFolder);
+        linkAction.setExecuteAsynchronously(false);
+        actionService.executeAction(linkAction, folderA);
+        
+        setComplete();
+        endTransaction();
+
+        transactionHelper.doInTransaction(() -> {
+            assertFalse(nodeService.hasAspect(folderA, ContentModel.ASPECT_CLASSIFIABLE));
+            assertFalse(nodeService.hasAspect(fileA, ContentModel.ASPECT_CLASSIFIABLE));
+            assertFalse(nodeService.hasAspect(folderB, ContentModel.ASPECT_CLASSIFIABLE));
+            assertFalse(nodeService.hasAspect(fileB, ContentModel.ASPECT_CLASSIFIABLE));
+            return null;
+        });
+        
+        
+        // rootFolder: "Run rule for this folder and its subfolders".
+        transactionHelper.doInTransaction(() -> {
+            Map<String, Serializable> actionParams = new HashMap<>();
+            actionParams.put(ExecuteAllRulesActionExecuter.PARAM_RUN_ALL_RULES_ON_CHILDREN, true);
+            // In Share, when you manually trigger rules on a folder, this parameter (execute-inherited-rules)
+            // will be false if the aspect rule:ignoreInheritedRules is present, and true otherwise.
+            actionParams.put(ExecuteAllRulesActionExecuter.PARAM_EXECUTE_INHERITED_RULES, false);
+            Action rulesAction = actionService.createAction(ExecuteAllRulesActionExecuter.NAME, actionParams);
+            executer.execute(rulesAction, folderA);
+            return null;
+        });
+
+        transactionHelper.doInTransaction(() -> {
+            // We're running the rule on folderA, so the folder itself won't be processed.
+            assertFalse(nodeService.hasAspect(folderA, ContentModel.ASPECT_CLASSIFIABLE));
+            
+            // The contents of folderA will all have the aspect added.
+            assertTrue(nodeService.hasAspect(fileA, ContentModel.ASPECT_CLASSIFIABLE));
+            assertTrue(nodeService.hasAspect(folderB, ContentModel.ASPECT_CLASSIFIABLE));
+            // The contents of folderB will all have the aspect added.
+            assertTrue(nodeService.hasAspect(fileB, ContentModel.ASPECT_CLASSIFIABLE));
+            return null;
+        });
+    }
+    
+    private FileInfo createFile(NodeRef parent, String name)
+    {
+        FileInfo file = fileFolderService.create(parent, name, ContentModel.TYPE_CONTENT);
+        ContentWriter writer = contentService.getWriter(file.getNodeRef(), ContentModel.PROP_CONTENT, true);
+        writer.putContent(file.getName()+" contents");
+        return file;
+    }
+
+    /**
+     * Test execution as per MNT-13055.
+     * <p>
+     * When we use the ExecuteAllRulesActionExecuter to explicitly (e.g. "manually")
+     * execute all the rules on a folder and its subfolders, the rule must correctly
+     * recurse into those subfolders. This is a more simple test case than the steps
+     * described in the above issue, but is crucial to the more complicated case
+     * succeeding.
+     */
+    public void testRuleAppliesToSubfoldersSuccessfully()
+    {
+        final NodeRef rootFolder = this.nodeService.createNode(
+                this.rootNodeRef,
+                ContentModel.ASSOC_CHILDREN,
+                QName.createQName("{test}rootFolderForTest"),
+                ContentModel.TYPE_FOLDER).getChildRef();
+
+        final NodeRef folderA = fileFolderService.
+                create(rootFolder, "folderA", ContentModel.TYPE_FOLDER).getNodeRef();
+
+        NodeRef fileA = createFile(folderA, "fileA.txt").getNodeRef();
+
+        final NodeRef folderB = fileFolderService.
+                create(folderA, "folderB", ContentModel.TYPE_FOLDER).getNodeRef();
+
+        NodeRef fileB = createFile(folderB, "fileB.txt").getNodeRef();
+
+        setComplete();
+        endTransaction();
+
+        // On RootFolder create a rule
+        transactionHelper.doInTransaction(() -> {
+            Rule rule1 = new Rule();
+            rule1.setRuleType(RuleType.INBOUND);
+            Action action1 = actionService.createAction(AddFeaturesActionExecuter.NAME);
+            action1.setParameterValue(AddFeaturesActionExecuter.PARAM_ASPECT_NAME, ContentModel.ASPECT_CLASSIFIABLE);
+            rule1.setAction(action1);
+            rule1.applyToChildren(true);
+            rule1.setExecuteAsynchronously(false);
+            ruleService.enableRule(rule1);
+            ruleService.saveRule(rootFolder, rule1);
+            return null;
+        });
+        
+        // Check the the docs don't have the aspects yet
+        transactionHelper.doInTransaction(() -> {
+            assertFalse(nodeService.hasAspect(folderA, ContentModel.ASPECT_CLASSIFIABLE));
+            assertFalse(nodeService.hasAspect(folderB, ContentModel.ASPECT_CLASSIFIABLE));
+            assertFalse(nodeService.hasAspect(fileA, ContentModel.ASPECT_CLASSIFIABLE));
+            assertFalse(nodeService.hasAspect(fileB, ContentModel.ASPECT_CLASSIFIABLE));
+            return null;
+        });
+        
+        // Execute the action
+        transactionHelper.doInTransaction(() -> {
+            Map<String, Serializable> actionParams = new HashMap<>();
+            actionParams.put(ExecuteAllRulesActionExecuter.PARAM_RUN_ALL_RULES_ON_CHILDREN, true);
+            actionParams.put(ExecuteAllRulesActionExecuter.PARAM_EXECUTE_INHERITED_RULES, true);
+            Action action = actionService.createAction(ExecuteAllRulesActionExecuter.NAME, actionParams);
+            executer.execute(action, rootFolder);
+            return null;
+        });
+        
+        transactionHelper.doInTransaction(() -> {
+            assertTrue(nodeService.hasAspect(folderA, ContentModel.ASPECT_CLASSIFIABLE));
+            assertTrue(nodeService.hasAspect(folderB, ContentModel.ASPECT_CLASSIFIABLE));
+            assertTrue(nodeService.hasAspect(fileA, ContentModel.ASPECT_CLASSIFIABLE));
+            assertTrue(nodeService.hasAspect(fileB, ContentModel.ASPECT_CLASSIFIABLE));
+            return null;
+        });
+    }
+
     private void clearAspects(NodeRef nodeRef)
     {
         nodeService.removeAspect(nodeRef, ContentModel.ASPECT_VERSIONABLE);