Extended MatrixProject to process parent template, added tests.

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

6 files changed, 288 insertions, 60 deletions

diff --git a/hudson-core/src/main/java/hudson/matrix/Axis.java b/hudson-core/src/main/java/hudson/matrix/Axis.java
index 0a4c4d9..52d15af 100644
--- a/hudson-core/src/main/java/hudson/matrix/Axis.java
+++ b/hudson-core/src/main/java/hudson/matrix/Axis.java
@@ -23,6 +23,7 @@ import hudson.model.AbstractDescribableImpl;
 import hudson.model.Descriptor;
 import hudson.model.Hudson;
 import hudson.util.QuotedStringTokenizer;
+import org.apache.commons.collections.CollectionUtils;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.DataBoundConstructor;
 
@@ -228,7 +229,8 @@ public class Axis extends AbstractDescribableImpl<Axis> implements Comparable<Ax
         if (name != null ? !name.equals(axis.name) : axis.name != null) {
             return false;
         }
-        if (values != null ? !values.equals(axis.values) : axis.values != null) {
+
+        if (values != null ? !CollectionUtils.isEqualCollection(values, axis.values) : axis.values != null) {
             return false;
         }
 
diff --git a/hudson-core/src/main/java/hudson/matrix/IMatrixProject.java b/hudson-core/src/main/java/hudson/matrix/IMatrixProject.java
index 623bf66..8367382 100644
--- a/hudson-core/src/main/java/hudson/matrix/IMatrixProject.java
+++ b/hudson-core/src/main/java/hudson/matrix/IMatrixProject.java
@@ -9,7 +9,7 @@
  *
  * Contributors:
  *
- *    Фтещт Лщяфл
+ *    Anton Kozak
  *
  *******************************************************************************/
 package hudson.matrix;
@@ -60,7 +60,7 @@ public interface IMatrixProject extends IAbstractProject {
      * @param runSequentially If true, {@link hudson.matrix.MatrixRun}s are run sequentially, instead of running in parallel.
      * @throws IOException exception.
      */
-    void setRunSequentially(boolean runSequentially) throws IOException;
+    void setRunSequentially(Boolean runSequentially) throws IOException;
 
     /**
      * Sets the combination filter.
diff --git a/hudson-core/src/main/java/hudson/matrix/MatrixProject.java b/hudson-core/src/main/java/hudson/matrix/MatrixProject.java
index 1da46f9..5540d45 100644
--- a/hudson-core/src/main/java/hudson/matrix/MatrixProject.java
+++ b/hudson-core/src/main/java/hudson/matrix/MatrixProject.java
@@ -52,6 +52,7 @@ import hudson.util.DescribableList;
 import hudson.util.FormValidation;
 import hudson.util.FormValidation.Kind;
 import net.sf.json.JSONObject;
+import org.apache.commons.lang3.ObjectUtils;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
@@ -82,13 +83,22 @@ import static hudson.Util.*;
  *
  * @author Kohsuke Kawaguchi
  */
-public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> implements IMatrixProject, TopLevelItem,
+public class MatrixProject extends AbstractProject<MatrixProject, MatrixBuild> implements IMatrixProject, TopLevelItem,
     SCMedItem, ItemGroup<MatrixConfiguration>, Saveable, FlyweightTask, BuildableItemWithBuildWrappers {
+
+    protected static final String HAS_COMBINATION_FILTER_PARAM = "hasCombinationFilter";
+    protected static final String COMBINATION_FILTER_PARAM = "combinationFilter";
+    protected static final String HAS_TOUCH_STONE_COMBINATION_FILTER_PARAM = "hasTouchStoneCombinationFilter";
+    protected static final String TOUCH_STONE_COMBINATION_FILTER_PARAM = "touchStoneCombinationFilter";
+    protected static final String TOUCH_STONE_RESULT_CONDITION_PARAM = "touchStoneResultCondition";
+    protected static final String CUSTOM_WORKSPACE_PARAM = "customWorkspace";
+    protected static final String CUSTOM_WORKSPACE_DIRECTORY_PARAM = "customWorkspace.directory";
+
     /**
      * Configuration axes.
      */
-    private volatile AxisList axes = new AxisList();
-    
+    private volatile AxisList axes; //TODO verify it = new AxisList();
+
     /**
      * The filter that is applied to combinations. It is a Groovy if condition.
      * This can be null, which means "true".
@@ -126,13 +136,14 @@ public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> im
     @CopyOnWrite
     private transient /*final*/ Set<MatrixConfiguration> activeConfigurations = new LinkedHashSet<MatrixConfiguration>();
 
-    private boolean runSequentially;
-    
+    //package visible for the tests only.
+    Boolean runSequentially;
+
     /**
      * Filter to select a number of combinations to build first
      */
     private String touchStoneCombinationFilter;
-    
+
     /**
      * Required result on the touchstone combinations, in order to
      * continue with the rest
@@ -143,7 +154,7 @@ public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> im
      * See {@link #setCustomWorkspace(String)}.
      */
     private String customWorkspace;
-    
+
     public MatrixProject(String name) {
         this(Hudson.getInstance(), name);
     }
@@ -156,14 +167,19 @@ public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> im
      * @inheritDoc
      */
     public AxisList getAxes() {
-        return axes;
+        return axes!= null ? axes : (hasParentTemplate()? getTemplate().getAxes() : null);
     }
 
     /**
      * @inheritDoc
      */
     public void setAxes(AxisList axes) throws IOException {
-        this.axes = new AxisList(axes);
+        if (!(hasParentTemplate() && ObjectUtils.equals(getTemplate().getAxes(), axes))) {
+            this.axes = new AxisList(axes);
+        } else {
+            this.axes = null;
+        }
+        // TODO verify me
         rebuildConfigurations();
         save();
     }
@@ -172,14 +188,27 @@ public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> im
      * @inheritDoc
      */
     public boolean isRunSequentially() {
-        return runSequentially;
+        return runSequentially!= null ? runSequentially : (hasParentTemplate() && getTemplate().isRunSequentially());
     }
 
     /**
-     * @inheritDoc
+     * @deprecated use #setRunSequentially(Boolean runSequentially) instead
      */
+    @Deprecated
     public void setRunSequentially(boolean runSequentially) throws IOException {
-        this.runSequentially = runSequentially;
+        setRunSequentially(Boolean.valueOf(runSequentially));
+    }
+
+    /**
+     * @inheritDoc
+     */
+    public void setRunSequentially(Boolean runSequentially) throws IOException {
+        if (!(hasParentTemplate() && ObjectUtils.equals(getTemplate().isRunSequentially(), runSequentially))) {
+            this.runSequentially = runSequentially;
+        } else {
+            this.runSequentially = null;
+        }
+        // TODO verify me
         save();
     }
 
@@ -187,7 +216,12 @@ public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> im
      * @inheritDoc
      */
     public void setCombinationFilter(String combinationFilter) throws IOException {
-        this.combinationFilter = combinationFilter;
+        if (!(hasParentTemplate() && ObjectUtils.equals(getTemplate().getCombinationFilter(), combinationFilter))) {
+            this.combinationFilter = combinationFilter;
+        } else {
+            this.combinationFilter = null;
+        }
+        // TODO verify me
         rebuildConfigurations();
         save();
     }
@@ -196,83 +230,116 @@ public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> im
      * @inheritDoc
      */
     public String getCombinationFilter() {
-        return combinationFilter;
+        return combinationFilter != null ? combinationFilter
+            : (hasParentTemplate() ? getTemplate().getCombinationFilter() : null);
     }
 
     /**
      * @inheritDoc
      */
     public String getTouchStoneCombinationFilter() {
-        return touchStoneCombinationFilter;
+        return touchStoneCombinationFilter != null ? touchStoneCombinationFilter
+            : (hasParentTemplate() ? getTemplate().getTouchStoneCombinationFilter() : null);
     }
 
     /**
      * @inheritDoc
      */
-    public void setTouchStoneCombinationFilter(
-            String touchStoneCombinationFilter) {
-        this.touchStoneCombinationFilter = touchStoneCombinationFilter;
+    public void setTouchStoneCombinationFilter(String touchStoneCombinationFilter) {
+        if (!(hasParentTemplate() && ObjectUtils.equals(getTemplate().getTouchStoneCombinationFilter(),
+            touchStoneCombinationFilter))) {
+            this.touchStoneCombinationFilter = touchStoneCombinationFilter;
+        } else {
+            this.touchStoneCombinationFilter = null;
+        }
     }
 
     /**
      * @inheritDoc
      */
     public Result getTouchStoneResultCondition() {
-        return touchStoneResultCondition;
+        return touchStoneResultCondition != null ? touchStoneResultCondition
+            : (hasParentTemplate() ? getTemplate().getTouchStoneResultCondition() : null);
     }
 
     /**
      * @inheritDoc
      */
     public void setTouchStoneResultCondition(Result touchStoneResultCondition) {
-        this.touchStoneResultCondition = touchStoneResultCondition;
+        if (!(hasParentTemplate() && ObjectUtils.equals(getTemplate().getTouchStoneResultCondition(),
+            touchStoneResultCondition))) {
+            this.touchStoneResultCondition = touchStoneResultCondition;
+        } else {
+            this.touchStoneResultCondition = null;
+        }
     }
 
     /**
      * @inheritDoc
      */
     public String getCustomWorkspace() {
-      return customWorkspace;
+        return customWorkspace != null ? customWorkspace
+            : (hasParentTemplate() ? getTemplate().getCustomWorkspace() : null);
     }
 
     /**
      * @inheritDoc
      */
     public void setCustomWorkspace(String customWorkspace) throws IOException {
-        this.customWorkspace= customWorkspace;
+        if (!(hasParentTemplate() && ObjectUtils.equals(getTemplate().getTouchStoneResultCondition(),
+            customWorkspace))) {
+            this.customWorkspace = customWorkspace;
+        } else {
+            this.customWorkspace = null;
+        }
     }
 
     /**
      * @inheritDoc
      */
     public List<Builder> getBuilders() {
-        return builders.toList();
+        DescribableList<Builder,Descriptor<Builder>> buildersList = getBuildersList();
+        return (buildersList != null ? buildersList.toList() : null);
     }
 
+    //TODO improve it
     public DescribableList<Builder,Descriptor<Builder>> getBuildersList() {
-        return builders;
+        return !(builders == null || builders.isEmpty()) ? builders
+            : (hasParentTemplate() ? getTemplate().getBuildersList() : null);
     }
 
     /**
      * @inheritDoc
      */
     public Map<Descriptor<Publisher>,Publisher> getPublishers() {
-        return publishers.toMap();
+        DescribableList<Publisher,Descriptor<Publisher>> publishersList = getPublishersList();
+        return (publishersList != null ? publishersList.toMap() : null);
     }
 
+    /**
+     * @inheritDoc
+     */
+    //TODO improve it
     public DescribableList<Publisher,Descriptor<Publisher>> getPublishersList() {
-        return publishers;
+        return !(publishers == null || publishers.isEmpty()) ? publishers
+            : (hasParentTemplate() ? getTemplate().getPublishersList() : null);
     }
 
+    /**
+     * @inheritDoc
+     */
+    //TODO improve it
     public DescribableList<BuildWrapper, Descriptor<BuildWrapper>> getBuildWrappersList() {
-        return buildWrappers;
+        return !(buildWrappers == null || buildWrappers.isEmpty()) ? buildWrappers
+            : (hasParentTemplate() ? getTemplate().getBuildWrappersList() : null);
     }
 
     /**
      * @inheritDoc
      */
     public Map<Descriptor<BuildWrapper>,BuildWrapper> getBuildWrappers() {
-        return buildWrappers.toMap();
+        DescribableList<BuildWrapper, Descriptor<BuildWrapper>> buildWrappersList = getBuildWrappersList();
+        return (buildWrappersList != null ? buildWrappersList.toMap() : null);
     }
 
     @Override
@@ -315,11 +382,11 @@ public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> im
 
     @Override
     public void onLoad(ItemGroup<? extends Item> parent, String name) throws IOException {
-        super.onLoad(parent,name);
+        super.onLoad(parent, name);
         Collections.sort(axes); // perhaps the file was edited on disk and the sort order might have been broken
-        builders.setOwner(this);
-        publishers.setOwner(this);
-        buildWrappers.setOwner(this);
+        getBuildersList().setOwner(this);
+        getPublishersList().setOwner(this);
+        getBuildWrappersList().setOwner(this);
         rebuildConfigurations();
     }
 
@@ -327,7 +394,7 @@ public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> im
     public void logRotate() throws IOException, InterruptedException {
         super.logRotate();
         // perform the log rotation of inactive configurations to make sure
-        // their logs get eventually discarded 
+        // their logs get eventually discarded
         for (MatrixConfiguration config : configurations.values()) {
             if(!config.isActiveConfiguration())
                 config.logRotate();
@@ -553,9 +620,9 @@ public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> im
     }
 
     protected void buildDependencyGraph(DependencyGraph graph) {
-        publishers.buildDependencyGraph(this,graph);
-        builders.buildDependencyGraph(this,graph);
-        buildWrappers.buildDependencyGraph(this,graph);
+        getPublishersList().buildDependencyGraph(this,graph);
+        getBuildersList().buildDependencyGraph(this,graph);
+        getBuildWrappersList().buildDependencyGraph(this,graph);
     }
 
     public MatrixProject asProject() {
@@ -580,26 +647,20 @@ public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> im
 
         JSONObject json = req.getSubmittedForm();
 
-        if(req.getParameter("hasCombinationFilter")!=null) {
-            this.combinationFilter = Util.nullify(req.getParameter("combinationFilter"));
-        } else {
-            this.combinationFilter = null;
-        }
-        
-        if (req.getParameter("hasTouchStoneCombinationFilter")!=null) {
-            this.touchStoneCombinationFilter = Util.nullify(req.getParameter("touchStoneCombinationFilter"));
-            String touchStoneResultCondition = req.getParameter("touchStoneResultCondition");
-            this.touchStoneResultCondition = Result.fromString(touchStoneResultCondition);
-        } else {
-            this.touchStoneCombinationFilter = null;
-        }
+        setCombinationFilter(
+            req.getParameter(HAS_COMBINATION_FILTER_PARAM) != null ? Util.nullify(req.getParameter(
+                COMBINATION_FILTER_PARAM)) : null);
 
-        if(req.hasParameter("customWorkspace")) {
-            customWorkspace = req.getParameter("customWorkspace.directory");
+        if (req.getParameter(HAS_TOUCH_STONE_COMBINATION_FILTER_PARAM)!=null) {
+            setTouchStoneCombinationFilter(Util.nullify(req.getParameter(TOUCH_STONE_COMBINATION_FILTER_PARAM)));
+            setTouchStoneResultCondition(Result.fromString(req.getParameter(TOUCH_STONE_RESULT_CONDITION_PARAM)));
         } else {
-            customWorkspace = null;        
+            setTouchStoneCombinationFilter(null);
         }
-        
+
+        setCustomWorkspace(
+            req.hasParameter(CUSTOM_WORKSPACE_PARAM) ? req.getParameter(CUSTOM_WORKSPACE_DIRECTORY_PARAM) : null);
+
         // parse system axes
         DescribableList<Axis,AxisDescriptor> newAxes = new DescribableList<Axis,AxisDescriptor>(this);
         newAxes.rebuildHetero(req, json, Axis.all(),"axis");
@@ -608,9 +669,9 @@ public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> im
 
         runSequentially = json.has("runSequentially");
 
-        buildWrappers.rebuild(req, json, BuildWrappers.getFor(this));
-        builders.rebuildHetero(req, json, Builder.all(), "builder");
-        publishers.rebuild(req, json, BuildStepDescriptor.filter(Publisher.all(),this.getClass()));
+        getBuildWrappersList().rebuild(req, json, BuildWrappers.getFor(this));
+        getBuildersList().rebuildHetero(req, json, Builder.all(), "builder");
+        getPublishersList().rebuild(req, json, BuildStepDescriptor.filter(Publisher.all(),this.getClass()));
 
         rebuildConfigurations();
     }
@@ -672,5 +733,23 @@ public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> im
         }
     }
 
+    /**
+     * For the unit tests only. Sets template for the job.
+     *
+     * @param template parent job
+     */
+    void setTemplate(MatrixProject template) {
+        this.template = template;
+    }
+
+    /**
+     * For the unit tests only.
+     *
+     * @param allowSave allow set.
+     */
+    void setAllowSave(Boolean allowSave) {
+        this.allowSave.set(allowSave);
+    }
+
     private static final Logger LOGGER = Logger.getLogger(MatrixProject.class.getName());
 }
diff --git a/hudson-core/src/main/java/hudson/model/Job.java b/hudson-core/src/main/java/hudson/model/Job.java
index 76d982b..445634e 100644
--- a/hudson-core/src/main/java/hudson/model/Job.java
+++ b/hudson-core/src/main/java/hudson/model/Job.java
@@ -152,7 +152,7 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
     /**
      * Selected template for this job.
      */
-    private transient JobT template;
+    protected transient JobT template;
 
     protected transient volatile ThreadLocal<Boolean> allowSave = new ThreadLocal<Boolean>() {
         @Override
diff --git a/hudson-core/src/main/java/hudson/model/Result.java b/hudson-core/src/main/java/hudson/model/Result.java
index d5531e7..5687d42 100644
--- a/hudson-core/src/main/java/hudson/model/Result.java
+++ b/hudson-core/src/main/java/hudson/model/Result.java
@@ -187,4 +187,43 @@ public final class Result implements Serializable, CustomExportedBean {
             return "STATUS";
         }
     }
+
+
+    /**
+     * @inheritDoc
+     */
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+
+        Result result = (Result) o;
+
+        if (ordinal != result.ordinal) {
+            return false;
+        }
+        if (color != result.color) {
+            return false;
+        }
+        if (name != null ? !name.equals(result.name) : result.name != null) {
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * @inheritDoc
+     */
+    @Override
+    public int hashCode() {
+        int result = name != null ? name.hashCode() : 0;
+        result = 31 * result + ordinal;
+        result = 31 * result + (color != null ? color.hashCode() : 0);
+        return result;
+    }
 }
diff --git a/hudson-core/src/test/java/hudson/matrix/MatrixProjectTest.java b/hudson-core/src/test/java/hudson/matrix/MatrixProjectTest.java
new file mode 100644
index 0000000..3b312db
--- a/dev/null
+++ b/hudson-core/src/test/java/hudson/matrix/MatrixProjectTest.java
@@ -0,0 +1,108 @@
+/*******************************************************************************
+ *
+ * Copyright (c) 2011 Oracle Corporation.
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *
+ *    Anton Kozak
+ *
+ *******************************************************************************/
+package hudson.matrix;
+
+import java.io.IOException;
+import org.junit.Test;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Test for {@link hudson.matrix.MatrixProject}
+ */
+public class MatrixProjectTest {
+
+    @Test
+    public void testIsRunSequentiallyParentTrue() throws IOException {
+        MatrixProject parentProject = new MatrixProjectMock("parent");
+        parentProject.setAllowSave(false);
+        parentProject.setRunSequentially(Boolean.TRUE);
+
+        MatrixProject childProject1 = new MatrixProject("child1");
+        childProject1.setTemplate(parentProject);
+        childProject1.setAllowSave(false);
+        assertTrue(childProject1.isRunSequentially());
+    }
+
+    @Test
+    public void testIsRunSequentiallyParentFalse() throws IOException {
+        MatrixProject parentProject = new MatrixProjectMock("parent");
+        parentProject.setAllowSave(false);
+        parentProject.setRunSequentially(Boolean.FALSE);
+
+        MatrixProject childProject1 = new MatrixProject("child1");
+        childProject1.setTemplate(parentProject);
+        childProject1.setAllowSave(false);
+        assertFalse(childProject1.isRunSequentially());
+    }
+
+    @Test
+    public void testIsRunSequentiallyDefaultValue() throws IOException {
+        MatrixProject childProject1 = new MatrixProject("child1");
+        childProject1.setAllowSave(false);
+        assertFalse(childProject1.isRunSequentially());
+    }
+
+    @Test
+    public void testIsRunSequentiallyParentFalseChildTrue() throws IOException {
+        MatrixProject parentProject = new MatrixProjectMock("parent");
+        parentProject.setAllowSave(false);
+        parentProject.setRunSequentially(Boolean.FALSE);
+
+        MatrixProject childProject1 = new MatrixProject("child1");
+        childProject1.setTemplate(parentProject);
+        childProject1.runSequentially = Boolean.TRUE;
+        childProject1.setAllowSave(false);
+        assertTrue(childProject1.isRunSequentially());
+    }
+
+    @Test
+    public void testIsRunSequentiallyParentTrueChildFalse() throws IOException {
+        MatrixProject parentProject = new MatrixProjectMock("parent");
+        parentProject.setAllowSave(false);
+        parentProject.setRunSequentially(Boolean.TRUE);
+
+        MatrixProject childProject1 = new MatrixProject("child1");
+        childProject1.setTemplate(parentProject);
+        childProject1.runSequentially = Boolean.FALSE;
+        childProject1.setAllowSave(false);
+        assertFalse(childProject1.isRunSequentially());
+    }
+
+    @Test
+    public void testIsRunSequentiallyParentNullChildTrue() throws IOException {
+        MatrixProject parentProject = new MatrixProjectMock("parent");
+        parentProject.setAllowSave(false);
+        parentProject.setRunSequentially(null);
+
+        MatrixProject childProject1 = new MatrixProject("child1");
+        childProject1.setTemplate(parentProject);
+        childProject1.runSequentially = Boolean.TRUE;
+        childProject1.setAllowSave(false);
+        assertTrue(childProject1.isRunSequentially());
+    }
+
+    private class MatrixProjectMock extends MatrixProject {
+
+        private MatrixProjectMock(String name) {
+            super(null, name);
+        }
+
+        @Override
+        protected void updateTransientActions() {
+        }
+    }
+}