MatrixProject modified to use Set instead of null value to identify whether property was overriden.

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

3 files changed, 183 insertions, 72 deletions

diff --git a/hudson-core/src/main/java/hudson/matrix/MatrixProject.java b/hudson-core/src/main/java/hudson/matrix/MatrixProject.java
index 57e4a62..b9af622 100644
--- a/hudson-core/src/main/java/hudson/matrix/MatrixProject.java
+++ b/hudson-core/src/main/java/hudson/matrix/MatrixProject.java
@@ -90,7 +90,11 @@ public class MatrixProject extends AbstractProject<MatrixProject, MatrixBuild> i
     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";
-
+    protected static final String RUN_SEQUENTIALLY_PROPERTY_NAME = "runSequentially";
+    protected static final String COMBINATION_FILTER_PROPERTY_NAME = "combinationFilter";
+    protected static final String TOUCH_STONE_COMBINATION_FILTER_PROPERTY_NAME = "touchStoneCombinationFilter";
+    protected static final String TOUCH_STONE_RESULT_CONDITION_PROPERTY_NAME = "touchStoneResultCondition";
+    protected static final String CUSTOM_WORKSPACE_PROPERTY_NAME = "customWorkspace";
     /**
      * Configuration axes.
      */
@@ -99,11 +103,10 @@ public class MatrixProject extends AbstractProject<MatrixProject, MatrixBuild> i
     /**
      * The filter that is applied to combinations. It is a Groovy if condition.
      * This can be null, which means "true".
-     * Package visible for the tests only.
      *
      * @see #getCombinationFilter()
      */
-    volatile String combinationFilter;
+    private volatile String combinationFilter;
 
     /**
      * List of active {@link Builder}s configured for this project.
@@ -134,14 +137,12 @@ public class MatrixProject extends AbstractProject<MatrixProject, MatrixBuild> i
     @CopyOnWrite
     private transient /*final*/ Set<MatrixConfiguration> activeConfigurations = new LinkedHashSet<MatrixConfiguration>();
 
-    //package visible for the tests only.
-    Boolean runSequentially;
+    private boolean runSequentially;
 
     /**
      * Filter to select a number of combinations to build first.
-     * Package visible for the tests only.
      */
-    String touchStoneCombinationFilter;
+    private String touchStoneCombinationFilter;
 
     /**
      * Required result on the touchstone combinations, in order to
@@ -188,37 +189,52 @@ public class MatrixProject extends AbstractProject<MatrixProject, MatrixBuild> i
      * @inheritDoc
      */
     public boolean isRunSequentially() {
-        return runSequentially!= null ? runSequentially : (hasCascadingProject() && getCascadingProject().isRunSequentially());
+        if (hasCascadingProject()) {
+            return isOverriddenProperty(RUN_SEQUENTIALLY_PROPERTY_NAME) ? runSequentially
+                : getCascadingProject().isRunSequentially();
+        } else {
+            return runSequentially;
+        }
     }
 
     /**
-     * @deprecated use #setRunSequentially(Boolean runSequentially) instead
+     * @inheritDoc
      */
-    @Deprecated
     public void setRunSequentially(boolean runSequentially) throws IOException {
-        setRunSequentially(Boolean.valueOf(runSequentially));
+        if (!hasCascadingProject()) {
+            this.runSequentially = runSequentially;
+        } else if (!ObjectUtils.equals(getCascadingProject().isRunSequentially(), runSequentially)) {
+            this.runSequentially = runSequentially;
+            registerOverriddenProperty(RUN_SEQUENTIALLY_PROPERTY_NAME);
+        } else {
+            unRegisterOverriddenProperty(RUN_SEQUENTIALLY_PROPERTY_NAME);
+        }
+        save();
     }
 
     /**
      * @inheritDoc
      */
-    public void setRunSequentially(Boolean runSequentially) throws IOException {
-        if (!(hasCascadingProject() && ObjectUtils.equals(getCascadingProject().isRunSequentially(), runSequentially))) {
-            this.runSequentially = runSequentially;
+    public String getCombinationFilter() {
+        if (hasCascadingProject()) {
+            return isOverriddenProperty(COMBINATION_FILTER_PROPERTY_NAME) ? combinationFilter
+                : getCascadingProject().getCombinationFilter();
         } else {
-            this.runSequentially = null;
+            return combinationFilter;
         }
-        // TODO verify me
-        save();
     }
 
     /**
      * @inheritDoc
      */
     public void setCombinationFilter(String combinationFilter) throws IOException {
-        if (!(hasCascadingProject() && ObjectUtils.equals(getCascadingProject().getCombinationFilter(), combinationFilter))) {
+        if (!hasCascadingProject()) {
+            this.combinationFilter = combinationFilter;
+        } else if (!ObjectUtils.equals(getCascadingProject().getCombinationFilter(), combinationFilter)) {
             this.combinationFilter = combinationFilter;
+            registerOverriddenProperty(COMBINATION_FILTER_PROPERTY_NAME);
         } else {
+            unRegisterOverriddenProperty(COMBINATION_FILTER_PROPERTY_NAME);
             this.combinationFilter = null;
         }
         rebuildConfigurations();
@@ -228,27 +244,27 @@ public class MatrixProject extends AbstractProject<MatrixProject, MatrixBuild> i
     /**
      * @inheritDoc
      */
-    public String getCombinationFilter() {
-        return combinationFilter != null ? combinationFilter
-            : (hasCascadingProject() ? getCascadingProject().getCombinationFilter() : null);
-    }
-
-    /**
-     * @inheritDoc
-     */
     public String getTouchStoneCombinationFilter() {
-        return touchStoneCombinationFilter != null ? touchStoneCombinationFilter
-            : (hasCascadingProject() ? getCascadingProject().getTouchStoneCombinationFilter() : null);
+        if (hasCascadingProject()) {
+            return isOverriddenProperty(TOUCH_STONE_COMBINATION_FILTER_PROPERTY_NAME) ? touchStoneCombinationFilter
+                : getCascadingProject().getTouchStoneCombinationFilter();
+        } else {
+            return touchStoneCombinationFilter;
+        }
     }
 
     /**
      * @inheritDoc
      */
     public void setTouchStoneCombinationFilter(String touchStoneCombinationFilter) {
-        if (!(hasCascadingProject() && ObjectUtils.equals(getCascadingProject().getTouchStoneCombinationFilter(),
-            touchStoneCombinationFilter))) {
+        if (!hasCascadingProject()) {
             this.touchStoneCombinationFilter = touchStoneCombinationFilter;
+        } else if (!ObjectUtils.equals(getCascadingProject().getTouchStoneCombinationFilter(),
+            touchStoneCombinationFilter)) {
+            this.touchStoneCombinationFilter = touchStoneCombinationFilter;
+            registerOverriddenProperty(TOUCH_STONE_COMBINATION_FILTER_PROPERTY_NAME);
         } else {
+            unRegisterOverriddenProperty(TOUCH_STONE_COMBINATION_FILTER_PROPERTY_NAME);
             this.touchStoneCombinationFilter = null;
         }
     }
@@ -257,18 +273,26 @@ public class MatrixProject extends AbstractProject<MatrixProject, MatrixBuild> i
      * @inheritDoc
      */
     public Result getTouchStoneResultCondition() {
-        return touchStoneResultCondition != null ? touchStoneResultCondition
-            : (hasCascadingProject() ? getCascadingProject().getTouchStoneResultCondition() : null);
+        if (hasCascadingProject()) {
+            return isOverriddenProperty(TOUCH_STONE_RESULT_CONDITION_PROPERTY_NAME) ? touchStoneResultCondition
+                : getCascadingProject().getTouchStoneResultCondition();
+        } else {
+            return touchStoneResultCondition;
+        }
     }
 
     /**
      * @inheritDoc
      */
     public void setTouchStoneResultCondition(Result touchStoneResultCondition) {
-        if (!(hasCascadingProject() && ObjectUtils.equals(getCascadingProject().getTouchStoneResultCondition(),
-            touchStoneResultCondition))) {
+        if (!hasCascadingProject()) {
+            this.touchStoneResultCondition = touchStoneResultCondition;
+        } else if (!ObjectUtils.equals(getCascadingProject().getTouchStoneResultCondition(),
+            touchStoneCombinationFilter)) {
             this.touchStoneResultCondition = touchStoneResultCondition;
+            registerOverriddenProperty(TOUCH_STONE_RESULT_CONDITION_PROPERTY_NAME);
         } else {
+            unRegisterOverriddenProperty(TOUCH_STONE_RESULT_CONDITION_PROPERTY_NAME);
             this.touchStoneResultCondition = null;
         }
     }
@@ -277,18 +301,25 @@ public class MatrixProject extends AbstractProject<MatrixProject, MatrixBuild> i
      * @inheritDoc
      */
     public String getCustomWorkspace() {
-        return customWorkspace != null ? customWorkspace
-            : (hasCascadingProject() ? getCascadingProject().getCustomWorkspace() : null);
+        if (hasCascadingProject()) {
+            return isOverriddenProperty(CUSTOM_WORKSPACE_PROPERTY_NAME) ? customWorkspace
+                : getCascadingProject().getCustomWorkspace();
+        } else {
+            return customWorkspace;
+        }
     }
 
     /**
      * @inheritDoc
      */
     public void setCustomWorkspace(String customWorkspace) throws IOException {
-        if (!(hasCascadingProject() && ObjectUtils.equals(getCascadingProject().getTouchStoneResultCondition(),
-            customWorkspace))) {
+        if (!hasCascadingProject()) {
+            this.customWorkspace = customWorkspace;
+        } else if (!ObjectUtils.equals(getCascadingProject().getCustomWorkspace(), customWorkspace)) {
             this.customWorkspace = customWorkspace;
+            registerOverriddenProperty(CUSTOM_WORKSPACE_PROPERTY_NAME);
         } else {
+            unRegisterOverriddenProperty(CUSTOM_WORKSPACE_PROPERTY_NAME);
             this.customWorkspace = null;
         }
     }
diff --git a/hudson-core/src/main/java/org/eclipse/hudson/api/matrix/IMatrixProject.java b/hudson-core/src/main/java/org/eclipse/hudson/api/matrix/IMatrixProject.java
index 48399a6..b614c17 100644
--- a/hudson-core/src/main/java/org/eclipse/hudson/api/matrix/IMatrixProject.java
+++ b/hudson-core/src/main/java/org/eclipse/hudson/api/matrix/IMatrixProject.java
@@ -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/test/java/hudson/matrix/MatrixProjectTest.java b/hudson-core/src/test/java/hudson/matrix/MatrixProjectTest.java
index cecfb70..bfc3295 100644
--- a/hudson-core/src/test/java/hudson/matrix/MatrixProjectTest.java
+++ b/hudson-core/src/test/java/hudson/matrix/MatrixProjectTest.java
@@ -29,7 +29,7 @@ public class MatrixProjectTest {
     public void testIsRunSequentiallyParentTrue() throws IOException {
         MatrixProject parentProject = new MatrixProjectMock("parent");
         parentProject.setAllowSave(false);
-        parentProject.setRunSequentially(Boolean.TRUE);
+        parentProject.setRunSequentially(true);
 
         MatrixProject childProject1 = new MatrixProject("child1");
         childProject1.setCascadingProject(parentProject);
@@ -41,7 +41,7 @@ public class MatrixProjectTest {
     public void testIsRunSequentiallyParentFalse() throws IOException {
         MatrixProject parentProject = new MatrixProjectMock("parent");
         parentProject.setAllowSave(false);
-        parentProject.setRunSequentially(Boolean.FALSE);
+        parentProject.setRunSequentially(false);
 
         MatrixProject childProject1 = new MatrixProject("child1");
         childProject1.setCascadingProject(parentProject);
@@ -60,12 +60,12 @@ public class MatrixProjectTest {
     public void testIsRunSequentiallyParentFalseChildTrue() throws IOException {
         MatrixProject parentProject = new MatrixProjectMock("parent");
         parentProject.setAllowSave(false);
-        parentProject.setRunSequentially(Boolean.FALSE);
+        parentProject.setRunSequentially(false);
 
         MatrixProject childProject1 = new MatrixProject("child1");
-        childProject1.setCascadingProject(parentProject);
-        childProject1.runSequentially = Boolean.TRUE;
         childProject1.setAllowSave(false);
+        childProject1.setCascadingProject(parentProject);
+        childProject1.setRunSequentially(Boolean.TRUE);
         assertTrue(childProject1.isRunSequentially());
     }
 
@@ -76,23 +76,10 @@ public class MatrixProjectTest {
         parentProject.setRunSequentially(Boolean.TRUE);
 
         MatrixProject childProject1 = new MatrixProject("child1");
-        childProject1.setCascadingProject(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.setCascadingProject(parentProject);
-        childProject1.runSequentially = Boolean.TRUE;
-        childProject1.setAllowSave(false);
-        assertTrue(childProject1.isRunSequentially());
+        childProject1.setRunSequentially(false);
+        assertFalse(childProject1.isRunSequentially());
     }
 
     @Test
@@ -117,9 +104,9 @@ public class MatrixProjectTest {
             }
         };
         childProject1.setAllowSave(false);
-        childProject1.setCombinationFilter(childCombinationFilter);
         childProject1.setCascadingProject(parentProject);
-        assertEquals(childProject1.getCombinationFilter(), childCombinationFilter);
+        childProject1.setCombinationFilter(childCombinationFilter);
+        assertEquals(childCombinationFilter, childProject1.getCombinationFilter());
     }
 
     @Test
@@ -155,7 +142,7 @@ public class MatrixProjectTest {
         childProject1.setAllowSave(false);
         childProject1.setCascadingProject(parentProject);
         childProject1.setCombinationFilter(childCombinationFilter);
-        assertEquals(childProject1.combinationFilter, childCombinationFilter);
+        assertEquals(childProject1.getCombinationFilter(), childCombinationFilter);
     }
 
     @Test
@@ -173,7 +160,6 @@ public class MatrixProjectTest {
         childProject1.setAllowSave(false);
         childProject1.setCascadingProject(parentProject);
         childProject1.setCombinationFilter(combinationFilter);
-        assertNull(childProject1.combinationFilter);
         assertEquals(childProject1.getCombinationFilter(), combinationFilter);
     }
 
@@ -188,7 +174,6 @@ public class MatrixProjectTest {
         };
         childProject1.setAllowSave(false);
         childProject1.setCombinationFilter(combinationFilter);
-        assertEquals(childProject1.combinationFilter, combinationFilter);
         assertEquals(childProject1.getCombinationFilter(), combinationFilter);
     }
 
@@ -203,6 +188,7 @@ public class MatrixProjectTest {
         assertNull(childProject1.getCombinationFilter());
     }
 
+
     @Test
     public void testGetTouchStoneCombinationFilterChildValue() throws IOException {
         String parentCombinationFilter = "parent_filter";
@@ -217,8 +203,8 @@ public class MatrixProjectTest {
             }
         };
         childProject1.setAllowSave(false);
-        childProject1.setTouchStoneCombinationFilter(childCombinationFilter);
         childProject1.setCascadingProject(parentProject);
+        childProject1.setTouchStoneCombinationFilter(childCombinationFilter);
         assertEquals(childProject1.getTouchStoneCombinationFilter(), childCombinationFilter);
     }
 
@@ -266,7 +252,7 @@ public class MatrixProjectTest {
         childProject1.setAllowSave(false);
         childProject1.setCascadingProject(parentProject);
         childProject1.setTouchStoneCombinationFilter(childCombinationFilter);
-        assertEquals(childProject1.touchStoneCombinationFilter, childCombinationFilter);
+        assertEquals(childProject1.getTouchStoneCombinationFilter(), childCombinationFilter);
     }
 
     @Test
@@ -284,7 +270,6 @@ public class MatrixProjectTest {
         childProject1.setAllowSave(false);
         childProject1.setCascadingProject(parentProject);
         childProject1.setTouchStoneCombinationFilter(combinationFilter);
-        assertNull(childProject1.touchStoneCombinationFilter);
         assertEquals(childProject1.getTouchStoneCombinationFilter(), combinationFilter);
     }
 
@@ -299,7 +284,6 @@ public class MatrixProjectTest {
         };
         childProject1.setAllowSave(false);
         childProject1.setTouchStoneCombinationFilter(combinationFilter);
-        assertEquals(childProject1.touchStoneCombinationFilter, combinationFilter);
         assertEquals(childProject1.getTouchStoneCombinationFilter(), combinationFilter);
     }
 
@@ -317,8 +301,8 @@ public class MatrixProjectTest {
             }
         };
         childProject1.setAllowSave(false);
-        childProject1.setTouchStoneResultCondition(childResultCondition);
         childProject1.setCascadingProject(parentProject);
+        childProject1.setTouchStoneResultCondition(childResultCondition);
         assertEquals(childProject1.getTouchStoneResultCondition(), childResultCondition);
     }
 
@@ -355,7 +339,7 @@ public class MatrixProjectTest {
         childProject1.setAllowSave(false);
         childProject1.setCascadingProject(parentProject);
         childProject1.setTouchStoneResultCondition(childResultCondition);
-        assertEquals(childProject1.touchStoneResultCondition, childResultCondition);
+        assertEquals(childProject1.getTouchStoneResultCondition(), childResultCondition);
     }
 
     @Test
@@ -373,7 +357,6 @@ public class MatrixProjectTest {
         childProject1.setAllowSave(false);
         childProject1.setCascadingProject(parentProject);
         childProject1.setTouchStoneResultCondition(parentResultCondition);
-        assertNull(childProject1.touchStoneResultCondition);
         assertEquals(childProject1.getTouchStoneResultCondition(), parentResultCondition);
     }
 
@@ -388,7 +371,6 @@ public class MatrixProjectTest {
         };
         childProject1.setAllowSave(false);
         childProject1.setTouchStoneResultCondition(childResultCondition);
-        assertEquals(childProject1.touchStoneResultCondition, childResultCondition);
         assertEquals(childProject1.getTouchStoneResultCondition(), childResultCondition);
     }
 
@@ -404,6 +386,104 @@ public class MatrixProjectTest {
         assertNull(childProject1.getTouchStoneResultCondition());
     }
 
+    @Test
+    public void testGetCustomWorkspaceChildValue() throws IOException {
+        String parentWorkspace = "/tmp";
+        String childWorkspace = "/tmp2";
+        MatrixProject parentProject = new MatrixProjectMock("parent");
+        parentProject.setAllowSave(false);
+        parentProject.setCustomWorkspace(parentWorkspace);
+
+        MatrixProject childProject1 = new MatrixProject("child1"){
+            @Override
+            void rebuildConfigurations() throws IOException {
+            }
+        };
+        childProject1.setAllowSave(false);
+        childProject1.setCascadingProject(parentProject);
+        childProject1.setCustomWorkspace(childWorkspace);
+        assertEquals(childProject1.getCustomWorkspace(), childWorkspace);
+    }
+
+    @Test
+    public void testGetCustomWorkspaceParentValue() throws IOException {
+        String parentWorkspace = "/tmp";
+        MatrixProject parentProject = new MatrixProjectMock("parent");
+        parentProject.setAllowSave(false);
+        parentProject.setCustomWorkspace(parentWorkspace);
+
+        MatrixProject childProject1 = new MatrixProject("child1"){
+            @Override
+            void rebuildConfigurations() throws IOException {
+            }
+        };
+        childProject1.setAllowSave(false);
+        childProject1.setCascadingProject(parentProject);
+        assertEquals(childProject1.getCustomWorkspace(), parentWorkspace);
+    }
+
+    @Test
+    public void testGetCustomWorkspaceNull() throws IOException {
+        MatrixProject childProject1 = new MatrixProject("child1"){
+            @Override
+            void rebuildConfigurations() throws IOException {
+            }
+        };
+        childProject1.setAllowSave(false);
+        assertNull(childProject1.getCustomWorkspace());
+    }
+
+    @Test
+    public void testSetCustomWorkspaceDifferentValues() throws IOException {
+        String parentWorkspace = "/tmp";
+        String childWorkspace = "/tmp2";
+        MatrixProject parentProject = new MatrixProjectMock("parent");
+        parentProject.setAllowSave(false);
+        parentProject.setTouchStoneCombinationFilter(parentWorkspace);
+
+        MatrixProject childProject1 = new MatrixProject("child1"){
+            @Override
+            void rebuildConfigurations() throws IOException {
+            }
+        };
+        childProject1.setAllowSave(false);
+        childProject1.setCascadingProject(parentProject);
+        childProject1.setCustomWorkspace(childWorkspace);
+        assertEquals(childProject1.getCustomWorkspace(), childWorkspace);
+    }
+
+    @Test
+    public void testSetCustomWorkspaceTheSameValues() throws IOException {
+        String parentWorkspace = "/tmp";
+        MatrixProject parentProject = new MatrixProjectMock("parent");
+        parentProject.setAllowSave(false);
+        parentProject.setCustomWorkspace(parentWorkspace);
+
+        MatrixProject childProject1 = new MatrixProject("child1"){
+            @Override
+            void rebuildConfigurations() throws IOException {
+            }
+        };
+        childProject1.setAllowSave(false);
+        childProject1.setCascadingProject(parentProject);
+        childProject1.setCustomWorkspace(parentWorkspace);
+        assertEquals(childProject1.getCustomWorkspace(), parentWorkspace);
+    }
+
+    @Test
+    public void testSetCustomWorkspaceParentNull() throws IOException {
+        String parentWorkspace = "/tmp";
+
+        MatrixProject childProject1 = new MatrixProject("child1"){
+            @Override
+            void rebuildConfigurations() throws IOException {
+            }
+        };
+        childProject1.setAllowSave(false);
+        childProject1.setCustomWorkspace(parentWorkspace);
+        assertEquals(childProject1.getCustomWorkspace(), parentWorkspace);
+    }
+
     private class MatrixProjectMock extends MatrixProject {
 
         private MatrixProjectMock(String name) {