Improve upstream/downstream triggers handling for cascading functionality. Add additional validation for triggers recursion

Signed-off-by: Winston Prakash <winston.prakash@gmail.com>

5 files changed, 85 insertions, 14 deletions

diff --git a/hudson-core/src/main/java/hudson/model/AbstractProject.java b/hudson-core/src/main/java/hudson/model/AbstractProject.java
index d936f4d..5d132cf 100644
--- a/hudson-core/src/main/java/hudson/model/AbstractProject.java
+++ b/hudson-core/src/main/java/hudson/model/AbstractProject.java
@@ -137,6 +137,7 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
     public static final String SCM_PROPERTY_NAME = "scm";
     public static final String HAS_QUIET_PERIOD_PROPERTY_NAME = "hasQuietPeriod";
     public static final String HAS_SCM_CHECKOUT_RETRY_COUNT_PROPERTY_NAME = "hasScmCheckoutRetryCount";
+    public static final String BUILD_TRIGGER_PROPERTY_NAME = "hudson-tasks-BuildTrigger";
 
     /**
      * {@link SCM} associated with the project.
@@ -912,7 +913,8 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
                     pl.replace(new BuildTrigger(newChildProjects,
                         existing==null?Result.SUCCESS:existing.getThreshold()));
                 }
-                p.putAllProjectProperties(DescribableListUtil.convertToProjectProperties(pl, p), false);
+                BuildTrigger buildTrigger = pl.get(BuildTrigger.class);
+                CascadingUtil.getExternalProjectProperty(p, BUILD_TRIGGER_PROPERTY_NAME).setValue(buildTrigger);
             }
         }
 
diff --git a/hudson-core/src/main/java/hudson/tasks/BuildTrigger.java b/hudson-core/src/main/java/hudson/tasks/BuildTrigger.java
index 50f07f0..5ea82da 100644
--- a/hudson-core/src/main/java/hudson/tasks/BuildTrigger.java
+++ b/hudson-core/src/main/java/hudson/tasks/BuildTrigger.java
@@ -40,6 +40,7 @@ import hudson.model.listeners.ItemListener;
 import hudson.util.AutoCompleteSeeder;
 import hudson.util.FormValidation;
 import net.sf.json.JSONObject;
+import org.apache.commons.lang3.StringUtils;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.AncestorInPath;
@@ -192,14 +193,17 @@ public class BuildTrigger extends Recorder implements DependecyDeclarer {
     }
 
     public void buildDependencyGraph(AbstractProject owner, DependencyGraph graph) {
-        for (AbstractProject p : getChildProjects())
-            graph.addDependency(new Dependency(owner, p) {
-                @Override
-                public boolean shouldTriggerBuild(AbstractBuild build, TaskListener listener,
-                                                  List<Action> actions) {
-                    return build.getResult().isBetterOrEqualTo(threshold);
-                }
-            });
+        for (AbstractProject p : getChildProjects()) {
+            if (!StringUtils.equals(p.getName(), owner.getName())) {
+                graph.addDependency(new Dependency(owner, p) {
+                    @Override
+                    public boolean shouldTriggerBuild(AbstractBuild build, TaskListener listener,
+                                                      List<Action> actions) {
+                        return build.getResult().isBetterOrEqualTo(threshold);
+                    }
+                });
+            }
+        }
     }
 
     @Override
@@ -280,7 +284,8 @@ public class BuildTrigger extends Recorder implements DependecyDeclarer {
         /**
          * Form validation method.
          */
-        public FormValidation doCheck(@AncestorInPath AccessControlled subject, @QueryParameter String value ) {
+        public FormValidation doCheck(@AncestorInPath AccessControlled subject, @AncestorInPath AbstractProject current,
+                                      @QueryParameter String value ) {
             // Require CONFIGURE permission on this project
             if(!subject.hasPermission(Item.CONFIGURE))      return FormValidation.ok();
 
@@ -292,6 +297,9 @@ public class BuildTrigger extends Recorder implements DependecyDeclarer {
                     return FormValidation.error(Messages.BuildTrigger_NoSuchProject(projectName,AbstractProject.findNearest(projectName).getName()));
                 if(!(item instanceof AbstractProject))
                     return FormValidation.error(Messages.BuildTrigger_NotBuildable(projectName));
+                if (StringUtils.equals(projectName, current.getName())) {
+                    return FormValidation.error(Messages.BuildTrigger_FailedUsingCurrentProject());
+                }
             }
 
             return FormValidation.ok();
diff --git a/hudson-core/src/main/java/hudson/util/CascadingUtil.java b/hudson-core/src/main/java/hudson/util/CascadingUtil.java
index ad3734c..f0c2947 100644
--- a/hudson-core/src/main/java/hudson/util/CascadingUtil.java
+++ b/hudson-core/src/main/java/hudson/util/CascadingUtil.java
@@ -24,6 +24,7 @@ import hudson.triggers.Trigger;
 import hudson.triggers.TriggerDescriptor;
 import java.lang.reflect.InvocationTargetException;
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import net.sf.json.JSONObject;
@@ -201,7 +202,7 @@ public class CascadingUtil {
      *
      * @param currentJob job that should be analyzed.
      * @param key key.
-     * @return {@link org.eclipse.hudson.model.project.property.TriggerProjectProperty} instance.
+     * @return {@link org.eclipse.hudson.api.model.project.property.TriggerProjectProperty} instance.
      * @throws NullPointerException if currentJob is null.
      */
     public static TriggerProjectProperty getTriggerProjectProperty(Job currentJob, String key) {
@@ -271,9 +272,28 @@ public class CascadingUtil {
      */
     public static boolean unlinkProjectFromCascadingParents(Job cascadingProject, String projectToUnlink) {
         if (null != cascadingProject && null != projectToUnlink) {
-            cascadingProject.removeCascadingChild(projectToUnlink);
+            Job job = Functions.getItemByName(Hudson.getInstance().getAllItems(Job.class), projectToUnlink);
+            Set<String> set = new HashSet<String>(job.getCascadingChildrenNames());
+            set.add(projectToUnlink);
+            return unlinkProjectFromCascadingParents(cascadingProject, set);
+        }
+        return false;
+    }
+
+    /**
+     * Recursively unlink set of projects from cascading hierarchy.
+     *
+     * @param cascadingProject cascading project to start from.
+     * @param projectsToUnlink projects that should be unlinked.
+     * @return if project was unlinked
+     */
+    private static boolean unlinkProjectFromCascadingParents(Job cascadingProject, Set<String> projectsToUnlink) {
+        if (null != cascadingProject && null != projectsToUnlink) {
+            for (String toUnlink : projectsToUnlink) {
+                cascadingProject.removeCascadingChild(toUnlink);
+            }
             if (cascadingProject.hasCascadingProject()) {
-                unlinkProjectFromCascadingParents(cascadingProject.getCascadingProject(), projectToUnlink);
+                unlinkProjectFromCascadingParents(cascadingProject.getCascadingProject(), projectsToUnlink);
             }
             return true;
         }
diff --git a/hudson-core/src/main/resources/hudson/tasks/Messages.properties b/hudson-core/src/main/resources/hudson/tasks/Messages.properties
index d5321cd..74029b4 100644
--- a/hudson-core/src/main/resources/hudson/tasks/Messages.properties
+++ b/hudson-core/src/main/resources/hudson/tasks/Messages.properties
@@ -40,6 +40,7 @@ BuildTrigger.InQueue={0} is already in the queue
 BuildTrigger.NoSuchProject=No such project ''{0}''. Did you mean ''{1}''?
 BuildTrigger.NotBuildable={0} is not buildable
 BuildTrigger.Triggering=Triggering a new build of {0}
+BuildTrigger.FailedUsingCurrentProject=Current project is not allowed to trigger.
 
 CommandInterpreter.CommandFailed=command execution failed
 CommandInterpreter.UnableToDelete=Unable to delete script file {0}
diff --git a/hudson-core/src/test/java/hudson/util/CascadingUtilTest.java b/hudson-core/src/test/java/hudson/util/CascadingUtilTest.java
index 7f0138c..b881595 100644
--- a/hudson-core/src/test/java/hudson/util/CascadingUtilTest.java
+++ b/hudson-core/src/test/java/hudson/util/CascadingUtilTest.java
@@ -45,14 +45,27 @@ import static org.powermock.api.easymock.PowerMock.verify;
 public class CascadingUtilTest {
 
     @Test
+    @PrepareForTest(Hudson.class)
     public void testUnlinkProjectFromCascadingParents() {
         //Prepare data
         FreeStyleProject project1 = new FreeStyleProjectMock("project1");
         FreeStyleProjectMock child1 = new FreeStyleProjectMock("child1");
-        child1.setCascadingProject(project1);
         String cascadingName = "newCascadingProject";
+        FreeStyleProjectMock child = new FreeStyleProjectMock(cascadingName);
+        child1.setCascadingProject(project1);
         CascadingUtil.linkCascadingProjectsToChild(child1, cascadingName);
 
+        List<Job> jobs = new ArrayList<Job>();
+        jobs.add(project1);
+        jobs.add(child1);
+        jobs.add(child);
+
+        Hudson hudson = createMock(Hudson.class);
+        mockStatic(Hudson.class);
+        expect(hudson.getAllItems(Job.class)).andReturn(jobs).anyTimes();
+        expect(Hudson.getInstance()).andReturn(hudson).anyTimes();
+        replay(Hudson.class, hudson);
+
         //Can't unlink from null project
         assertFalse(CascadingUtil.unlinkProjectFromCascadingParents(null, cascadingName));
         //Can't unlink null cascading name
@@ -76,6 +89,33 @@ public class CascadingUtilTest {
     }
 
     @Test
+    @PrepareForTest(Hudson.class)
+    public void testUnlinkProjectFromCascadingParents2() {
+        FreeStyleProject project1 = new FreeStyleProjectMock("p1");
+        FreeStyleProjectMock project2 = new FreeStyleProjectMock("p2");
+        FreeStyleProjectMock project3 = new FreeStyleProjectMock("p3");
+        project2.setCascadingProject(project1);
+        CascadingUtil.linkCascadingProjectsToChild(project1, "p2");
+        project3.setCascadingProject(project2);
+        CascadingUtil.linkCascadingProjectsToChild(project2, "p3");
+
+        List<Job> jobs = new ArrayList<Job>();
+        jobs.add(project1);
+        jobs.add(project2);
+        jobs.add(project3);
+
+        Hudson hudson = createMock(Hudson.class);
+        mockStatic(Hudson.class);
+        expect(hudson.getAllItems(Job.class)).andReturn(jobs);
+        expect(Hudson.getInstance()).andReturn(hudson);
+        replay(Hudson.class, hudson);
+
+        CascadingUtil.unlinkProjectFromCascadingParents(project1, "p2");
+        //Project3 should disappear from project1's children.
+        assertTrue(project1.getCascadingChildrenNames().isEmpty());
+    }
+
+    @Test
     public void testLinkCascadingProjectsToChild() {
         FreeStyleProject project1 = new FreeStyleProjectMock("project1");
         FreeStyleProjectMock child1 = new FreeStyleProjectMock("child1");