Improve DescribableListProjectProperty handling. Override allowOverride method for ignoring List Owner

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

3 files changed, 205 insertions, 42 deletions

diff --git a/hudson-core/src/main/java/hudson/matrix/AxisList.java b/hudson-core/src/main/java/hudson/matrix/AxisList.java
index 483cfe9..f0f4305 100644
--- a/hudson-core/src/main/java/hudson/matrix/AxisList.java
+++ b/hudson-core/src/main/java/hudson/matrix/AxisList.java
@@ -18,16 +18,13 @@ package hudson.matrix;
 
 import com.thoughtworks.xstream.XStream;
 import hudson.Util;
-import hudson.model.Label;
 import hudson.util.RobustCollectionConverter;
 
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Arrays;
-import java.util.Set;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.collections.ListUtils;
 
diff --git a/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/DescribableListProjectProperty.java b/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/DescribableListProjectProperty.java
index f6c7688..34f012d 100644
--- a/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/DescribableListProjectProperty.java
+++ b/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/DescribableListProjectProperty.java
@@ -15,6 +15,7 @@
 package org.eclipse.hudson.api.model.project.property;
 
 import hudson.util.DescribableList;
+import org.apache.commons.collections.CollectionUtils;
 import org.eclipse.hudson.api.model.IJob;
 
 /**
@@ -33,4 +34,12 @@ public class DescribableListProjectProperty extends BaseProjectProperty<Describa
     public DescribableList getDefaultValue() {
         return new DescribableList(getJob());
     }
+
+    @Override
+    protected boolean allowOverrideValue(DescribableList cascadingValue, DescribableList candidateValue) {
+        return (null != candidateValue || null != cascadingValue)
+            && ((null == cascadingValue || null == candidateValue)
+            || !CollectionUtils.isEqualCollection(cascadingValue.toList(), candidateValue.toList()));
+    }
+
 }
diff --git a/hudson-core/src/test/java/org/eclipse/hudson/api/model/project/property/ProjectPropertyTest.java b/hudson-core/src/test/java/org/eclipse/hudson/api/model/project/property/ProjectPropertyTest.java
index 992ef96..672c088 100644
--- a/hudson-core/src/test/java/org/eclipse/hudson/api/model/project/property/ProjectPropertyTest.java
+++ b/hudson-core/src/test/java/org/eclipse/hudson/api/model/project/property/ProjectPropertyTest.java
@@ -15,8 +15,13 @@
 
 package org.eclipse.hudson.api.model.project.property;
 
+import hudson.matrix.Axis;
+import hudson.matrix.AxisList;
 import hudson.model.FreeStyleProjectMock;
 import hudson.tasks.LogRotator;
+import hudson.tasks.Shell;
+import hudson.util.DescribableList;
+import java.io.IOException;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -50,79 +55,111 @@ public class ProjectPropertyTest {
      * Verify all constructors for ProjectProperties hierarchy.
      */
     @Test
-    public void testConstructor() {
+    public void testBaseProjectConstructor() {
         try {
             new BaseProjectProperty(null);
             fail("Null should be handled by ProjectProperty constructor.");
         } catch (Exception e) {
             assertEquals(BaseProjectProperty.INVALID_JOB_EXCEPTION, e.getMessage());
         }
+        BaseProjectProperty property = new BaseProjectProperty(project);
+        assertNotNull(property.getJob());
+        assertEquals(project, property.getJob());
+    }
+
+    @Test
+    public void testStringProjectConstructor() {
         try {
             new StringProjectProperty(null);
             fail("Null should be handled by ProjectProperty constructor.");
         } catch (Exception e) {
             assertEquals(BaseProjectProperty.INVALID_JOB_EXCEPTION, e.getMessage());
         }
+    }
+
+    @Test
+    public void testIntegerProjectConstructor() {
         try {
             new IntegerProjectProperty(null);
             fail("Null should be handled by ProjectProperty constructor.");
         } catch (Exception e) {
             assertEquals(BaseProjectProperty.INVALID_JOB_EXCEPTION, e.getMessage());
         }
+    }
+
+    @Test
+    public void testBooleanProjectPropertyConstructor() {
         try {
             new BooleanProjectProperty(null);
             fail("Null should be handled by ProjectProperty constructor.");
         } catch (Exception e) {
             assertEquals(BaseProjectProperty.INVALID_JOB_EXCEPTION, e.getMessage());
         }
+    }
+
+    @Test
+    public void testLogRotatorProjectPropertyConstructor() {
         try {
             new LogRotatorProjectProperty(null);
             fail("Null should be handled by ProjectProperty constructor.");
         } catch (Exception e) {
             assertEquals(BaseProjectProperty.INVALID_JOB_EXCEPTION, e.getMessage());
         }
+    }
+
+    @Test
+    public void testAxisListProjectPropertyConstructor() {
         try {
-            new DescribableListProjectProperty(null);
+            new AxisListProjectProperty(null);
             fail("Null should be handled by ProjectProperty constructor.");
         } catch (Exception e) {
             assertEquals(BaseProjectProperty.INVALID_JOB_EXCEPTION, e.getMessage());
         }
-        BaseProjectProperty property = new BaseProjectProperty(project);
-        assertNotNull(property.getJob());
-        assertEquals(project, property.getJob());
     }
 
     /**
      * Checks prepareValue method.
      */
     @Test
-    public void testPrepareValue() {
+    public void testBaseProjectPropertyPrepareValue() {
         //BaseProject property doesn't perform any changes with value.
         BaseProjectProperty property = new BaseProjectProperty(project);
         assertNull(property.prepareValue(null));
         Object value = new Object();
         assertEquals(value, property.prepareValue(value));
+    }
 
+    @Test
+    public void testBooleanProjectPropertyPrepareValue() {
         //Boolean property acts as BaseProperty
-        property = new BooleanProjectProperty(project);
+        BaseProjectProperty property = new BooleanProjectProperty(project);
         assertNull(property.prepareValue(null));
         assertFalse((Boolean) property.prepareValue(false));
         assertTrue((Boolean) property.prepareValue(true));
+    }
 
+    @Test
+    public void testLogRotatorProjectPropertyPrepareValue() {
         //Boolean property acts as BaseProperty
-        property = new LogRotatorProjectProperty(project);
+        BaseProjectProperty property = new LogRotatorProjectProperty(project);
         assertNull(property.prepareValue(null));
-        value = new LogRotator(1, 1, 1, 1);
+        LogRotator value = new LogRotator(1, 1, 1, 1);
         assertEquals(value, property.prepareValue(value));
+    }
 
+    @Test
+    public void testIntegerProjectPropertyPrepareValue() {
         //Integer property acts as BaseProperty
-        property = new IntegerProjectProperty(project);
+        BaseProjectProperty property = new IntegerProjectProperty(project);
         assertNull(property.prepareValue(null));
         int intValue = 10;
         assertEquals(intValue, property.prepareValue(intValue));
+    }
 
+    @Test
+    public void testStringProjectPropertyPrepareValue() {
         //String project property trims string values to null and uses StringUtils.trimToNull logic.
-        property = new StringProjectProperty(project);
+        BaseProjectProperty property = new StringProjectProperty(project);
         assertNull(property.prepareValue(null));
         assertNull(property.prepareValue(""));
         assertNull(property.prepareValue("     "));
@@ -130,48 +167,93 @@ public class ProjectPropertyTest {
         assertEquals("abc", property.prepareValue("    abc    "));
     }
 
+    @Test
+    public void testAxisListProjectPropertyPrepareValue() {
+        //Boolean property acts as BaseProperty
+        BaseProjectProperty property = new AxisListProjectProperty(project);
+        assertNull(property.prepareValue(null));
+        AxisList value = new AxisList();
+        assertEquals(value, property.prepareValue(value));
+    }
+
     /**
      * Verify getDefaultValue method.
      */
     @Test
-    public void testGetDefaultValue() {
-        assertNull(new BaseProjectProperty(project).getDefaultValue());
-        assertNull(new StringProjectProperty(project).getDefaultValue());
-        assertEquals(new Integer(0), new IntegerProjectProperty(project).getDefaultValue());
-        assertFalse(new BooleanProjectProperty(project).getDefaultValue());
-        assertNull(new LogRotatorProjectProperty(project).getDefaultValue());
-        assertNotNull(new DescribableListProjectProperty(project).getDefaultValue());
+    public void testBaseProjectPropertyGetDefaultValue() {
+        BaseProjectProperty property = new BaseProjectProperty(project);
+        assertNull(property.getDefaultValue());
+    }
+
+    @Test
+    public void testStringProjectPropertyGetDefaultValue() {
+        BaseProjectProperty property = new StringProjectProperty(project);
+        assertNull(property.getDefaultValue());
+    }
+
+    @Test
+    public void testIntegerProjectPropertyGetDefaultValue() {
+        BaseProjectProperty property = new IntegerProjectProperty(project);
+        assertEquals(0, property.getDefaultValue());
+    }
+
+    @Test
+    public void testBooleanProjectPropertyGetDefaultValue() {
+        BaseProjectProperty property = new BooleanProjectProperty(project);
+        assertFalse((Boolean) property.getDefaultValue());
+    }
+
+    @Test
+    public void testLogRotatorProjectPropertyGetDefaultValue() {
+        BaseProjectProperty property = new LogRotatorProjectProperty(project);
+        assertNull(property.getDefaultValue());
+    }
+
+    @Test
+    public void testAxisListProjectPropertyGetDefaultValue() {
+        BaseProjectProperty property = new AxisListProjectProperty(project);
+        assertNull(property.getDefaultValue());
     }
 
+    @Test
+    public void testDescribableListProjectPropertyGetDefaultValue() {
+        BaseProjectProperty property = new DescribableListProjectProperty(project);
+        assertNotNull(property.getDefaultValue());
+    }
 
     /**
      * Verify allowOverrideValue method.
      */
     @Test
-    public void testAllowOverrideValue() {
-        //Test BaseProjectProperty.
+    public void testBaseProjectPropertyAllowOverrideValue() {
         BaseProjectProperty property = new BaseProjectProperty(project);
         assertFalse(property.allowOverrideValue(null, null));
         assertTrue(property.allowOverrideValue(new Object(), null));
         assertTrue(property.allowOverrideValue(null, new Object()));
+    }
 
-        //Test BooleanProjectProperty.
-        property = new BooleanProjectProperty(project);
+    @Test
+    public void testBooleanProjectPropertyAllowOverrideValue() {
+        BaseProjectProperty property = new BooleanProjectProperty(project);
         assertFalse(property.allowOverrideValue(null, null));
         assertFalse(property.allowOverrideValue(false, false));
         assertFalse(property.allowOverrideValue(true, true));
         assertTrue(property.allowOverrideValue(true, false));
         assertTrue(property.allowOverrideValue(false, true));
+    }
 
-        //Test IntegerProjectProperty.
-        property = new IntegerProjectProperty(project);
+    @Test
+    public void testIntegerProjectPropertyAllowOverrideValue() {
+        BaseProjectProperty property = new IntegerProjectProperty(project);
         assertFalse(property.allowOverrideValue(null, null));
         assertFalse(property.allowOverrideValue(1, 1));
         assertTrue(property.allowOverrideValue(1, 0));
         assertTrue(property.allowOverrideValue(0, 1));
+    }
 
-        //Test StringProjectProperty.
-        property = new StringProjectProperty(project);
+    @Test
+    public void testStringProjectPropertyAllowOverrideValue() {
+        BaseProjectProperty property = new StringProjectProperty(project);
         assertFalse(property.allowOverrideValue(null, null));
         assertFalse(property.allowOverrideValue("", ""));
         assertFalse(property.allowOverrideValue("abc", "abc"));
@@ -179,19 +261,58 @@ public class ProjectPropertyTest {
         assertTrue(property.allowOverrideValue(null, "abc"));
         assertTrue(property.allowOverrideValue("abc", null));
         assertTrue(property.allowOverrideValue("abc", "abcd"));
+    }
 
-        property = new LogRotatorProjectProperty(project);
+    @Test
+    public void testLogRotatorProjectPropertyAllowOverrideValue() {
+        BaseProjectProperty property = new LogRotatorProjectProperty(project);
         assertFalse(property.allowOverrideValue(null, null));
         assertTrue(property.allowOverrideValue(new LogRotator(1, 1, 1, 1), null));
         assertTrue(property.allowOverrideValue(null, new LogRotator(1, 1, 1, 1)));
         assertTrue(property.allowOverrideValue(new LogRotator(1, 1, 1, 2), new LogRotator(1, 1, 1, 1)));
     }
 
+    @Test
+    public void testDescribableListProjectPropertyAllowOverrideValue() throws IOException {
+        BaseProjectProperty property = new DescribableListProjectProperty(project);
+        //Don't need to override null values
+        assertFalse(property.allowOverrideValue(null, null));
+        //Allow override if cascading or candidate are null
+        assertTrue(property.allowOverrideValue(new DescribableList(project), null));
+        assertTrue(property.allowOverrideValue(null, new DescribableList(project)));
+        //Don't need to override Describable lists which has same Describable#data values, even if owners are not equal.
+        assertFalse(property.allowOverrideValue(new DescribableList(project), new DescribableList(project)));
+        assertFalse(property.allowOverrideValue(new DescribableList(project), new DescribableList(parent)));
+        DescribableList describableList1 = new DescribableList(project);
+        DescribableList describableList2 = new DescribableList(project);
+        describableList1.add(new Shell("echo 'test3'"));
+        describableList1.add(new Shell("echo 'test2'"));
+        describableList2.add(new Shell("echo 'test2'"));
+        describableList2.add(new Shell("echo 'test3'"));
+        assertFalse(property.allowOverrideValue(describableList1, describableList2));
+
+        DescribableList describableList3 = new DescribableList(parent);
+        describableList3.replaceBy(describableList2.toList());
+        assertFalse(property.allowOverrideValue(describableList1, describableList3));
+
+        describableList2.add(new Shell("echo 'test1'"));
+        assertTrue(property.allowOverrideValue(describableList1, describableList2));
+    }
+
+    @Test
+    public void testAxisListProjectPropertyAllowOverrideValue() {
+        BaseProjectProperty property = new AxisListProjectProperty(project);
+        assertFalse(property.allowOverrideValue(null, null));
+        assertTrue(property.allowOverrideValue(new AxisList(), null));
+        assertTrue(property.allowOverrideValue(null, new AxisList()));
+        assertTrue(property.allowOverrideValue(new AxisList().add(new Axis("DB", "mysql")), new AxisList()));
+    }
+
     /**
      * Verify getCascadingValue method.
      */
     @Test
-    public void testGetCascadingValue() {
+    public void testBaseProjectPropertyGetCascadingValue() {
         String parentValue = "parentValue";
 
         BaseProjectProperty property = new BaseProjectProperty(project);
@@ -221,39 +342,60 @@ public class ProjectPropertyTest {
      * Verify getOriginalValue method.
      */
     @Test
-    public void testGetOriginalValue() {
+    public void testBaseProjectPropertyGetOriginalValue() {
         BaseProjectProperty property = new BaseProjectProperty(project);
         assertNull(property.getOriginalValue());
         Object value = new Object();
-
         property.setKey(propertyKey);
         property.setValue(value);
         assertEquals(value, property.getOriginalValue());
         property.setValue(null);
         assertNull(property.getOriginalValue());
+    }
 
-        value = 10;
-        property = new IntegerProjectProperty(project);
+    @Test
+    public void testIntegerProjectPropertyGetOriginalValue() {
+        int value = 10;
+        BaseProjectProperty property = new IntegerProjectProperty(project);
         property.setKey(propertyKey);
         property.setValue(value);
         assertEquals(value, property.getOriginalValue());
+    }
 
-        value = "abs";
-        property = new StringProjectProperty(project);
+    @Test
+    public void testStringProjectPropertyGetOriginalValue() {
+        String value = "abs";
+        BaseProjectProperty property = new StringProjectProperty(project);
         property.setKey(propertyKey);
         property.setValue(value);
         assertEquals(value, property.getOriginalValue());
+    }
 
-        value = Boolean.TRUE;
-        property = new BooleanProjectProperty(project);
+    @Test
+    public void testBooleanProjectPropertyGetOriginalValue() {
+        boolean value = Boolean.TRUE;
+        BaseProjectProperty property = new BooleanProjectProperty(project);
         property.setKey(propertyKey);
         property.setValue(value);
         assertEquals(value, property.getOriginalValue());
         property.setValue(null);
         assertFalse((Boolean) property.getOriginalValue());
+    }
 
-        value = new LogRotator(1, 1, 1, 1);
-        property = new LogRotatorProjectProperty(project);
+    @Test
+    public void testLogRotatorProjectPropertyGetOriginalValue() {
+        LogRotator value = new LogRotator(1, 1, 1, 1);
+        BaseProjectProperty property = new LogRotatorProjectProperty(project);
+        property.setKey(propertyKey);
+        property.setValue(value);
+        assertEquals(value, property.getOriginalValue());
+    }
+
+    @Test
+    public void testAxisListProjectPropertyGetOriginalValue() {
+        AxisList value = new AxisList();
+        value.add(new Axis("DB", "mysql"));
+        BaseProjectProperty property = new AxisListProjectProperty(project);
         property.setKey(propertyKey);
         property.setValue(value);
         assertEquals(value, property.getOriginalValue());
@@ -340,7 +482,6 @@ public class ProjectPropertyTest {
         assertNull(property.getValue());
     }
 
-
     /**
      * Property should have not null property key.
      */
@@ -377,6 +518,22 @@ public class ProjectPropertyTest {
     }
 
     /**
+     * Resets project property value,
+     */
+    @Test
+    public void testResetValue() {
+        BaseProjectProperty property = new BaseProjectProperty(project);
+        property.setKey(propertyKey);
+        property.setValue(new Object());
+        property.setOverridden(true);
+        assertNotNull(property.getOriginalValue());
+        assertTrue(property.isOverridden());
+        property.resetValue();
+        assertNull(property.getOriginalValue());
+        assertFalse(property.isOverridden());
+    }
+
+    /**
      * Test setOverridden method.
      */
     @Test