Update JavaDocs for PP. Implement unit-tests for ExternalProjectProperty

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

3 files changed, 66 insertions, 25 deletions

diff --git a/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/BaseProjectProperty.java b/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/BaseProjectProperty.java
index e5de78d..43e5fca 100644
--- a/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/BaseProjectProperty.java
+++ b/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/BaseProjectProperty.java
@@ -53,13 +53,6 @@ public class BaseProjectProperty<T> implements IProjectProperty<T> {
     }
 
     /**
-     * @return key for current property
-     */
-    final String getPropertyKey() {
-        return propertyKey;
-    }
-
-    /**
      * {@inheritDoc}
      */
     public void setJob(IJob job) {
@@ -151,14 +144,16 @@ public class BaseProjectProperty<T> implements IProjectProperty<T> {
      *
      * @param value new value to be set.
      * @param cascadingValue current cascading value.
+     * @return true - if property was updated, false - otherwise.
      */
-    protected void updateOriginalValue(T value, T cascadingValue) {
+    protected boolean updateOriginalValue(T value, T cascadingValue) {
         T candidateValue = null == value ? getDefaultValue() : value;
         if (allowOverrideValue(cascadingValue, candidateValue)) {
             setOriginalValue(value, true);
         } else {
             resetValue();
         }
+        return true;
     }
 
     /**
diff --git a/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/ExternalProjectProperty.java b/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/ExternalProjectProperty.java
index 45c19e3..b171edd 100644
--- a/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/ExternalProjectProperty.java
+++ b/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/ExternalProjectProperty.java
@@ -17,7 +17,12 @@ package org.eclipse.hudson.api.model.project.property;
 import org.eclipse.hudson.api.model.IJob;
 
 /**
- * //TODO class description
+ * Class property is intended to be used for ProjectProperties without correct equals and hashCode methods, such as
+ * Builders, Publishers, etc.
+ * <p/>
+ * This property has additional {@link #modified} flag, that is used to define, whether current property was changed
+ * from UI. If yes, cascading value will be updated. {@link #updateOriginalValue(Object, Object)} method for details.
+ * <p/>
  * <p/>
  * Date: 10/20/11
  *
@@ -56,11 +61,10 @@ public class ExternalProjectProperty<T> extends BaseProjectProperty<T> {
      *
      * @param value new value to be set.
      * @param cascadingValue current cascading value.
+     * @return true if value was updated, false - otherwise.
      */
     @Override
-    protected void updateOriginalValue(T value, T cascadingValue) {
-        if (isModified()) {
-            super.updateOriginalValue(value, cascadingValue);
-        }
+    protected boolean updateOriginalValue(T value, T cascadingValue) {
+        return isModified() && super.updateOriginalValue(value, cascadingValue);
     }
 }
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 1d77dee..3307c2d 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
@@ -72,7 +72,7 @@ public class ProjectPropertyTest {
     public void testBaseProjectConstructor() {
         try {
             new BaseProjectProperty(null);
-            fail("Null should be handled by ProjectProperty constructor.");
+            fail("Null should be handled by BaseProjectProperty constructor.");
         } catch (Exception e) {
             assertEquals(BaseProjectProperty.INVALID_JOB_EXCEPTION, e.getMessage());
         }
@@ -85,7 +85,7 @@ public class ProjectPropertyTest {
     public void testStringProjectConstructor() {
         try {
             new StringProjectProperty(null);
-            fail("Null should be handled by ProjectProperty constructor.");
+            fail("Null should be handled by StringProjectProperty constructor.");
         } catch (Exception e) {
             assertEquals(BaseProjectProperty.INVALID_JOB_EXCEPTION, e.getMessage());
         }
@@ -95,7 +95,7 @@ public class ProjectPropertyTest {
     public void testIntegerProjectConstructor() {
         try {
             new IntegerProjectProperty(null);
-            fail("Null should be handled by ProjectProperty constructor.");
+            fail("Null should be handled by IntegerProjectProperty constructor.");
         } catch (Exception e) {
             assertEquals(BaseProjectProperty.INVALID_JOB_EXCEPTION, e.getMessage());
         }
@@ -105,7 +105,7 @@ public class ProjectPropertyTest {
     public void testBooleanProjectPropertyConstructor() {
         try {
             new BooleanProjectProperty(null);
-            fail("Null should be handled by ProjectProperty constructor.");
+            fail("Null should be handled by BooleanProjectProperty constructor.");
         } catch (Exception e) {
             assertEquals(BaseProjectProperty.INVALID_JOB_EXCEPTION, e.getMessage());
         }
@@ -115,7 +115,17 @@ public class ProjectPropertyTest {
     public void testLogRotatorProjectPropertyConstructor() {
         try {
             new LogRotatorProjectProperty(null);
-            fail("Null should be handled by ProjectProperty constructor.");
+            fail("Null should be handled by LogRotatorProjectProperty constructor.");
+        } catch (Exception e) {
+            assertEquals(BaseProjectProperty.INVALID_JOB_EXCEPTION, e.getMessage());
+        }
+    }
+
+    @Test
+    public void testDescribableListProjectPropertyConstructor() {
+        try {
+            new DescribableListProjectProperty(null);
+            fail("Null should be handled by DescribableListProjectProperty constructor.");
         } catch (Exception e) {
             assertEquals(BaseProjectProperty.INVALID_JOB_EXCEPTION, e.getMessage());
         }
@@ -125,7 +135,7 @@ public class ProjectPropertyTest {
     public void testAxisListProjectPropertyConstructor() {
         try {
             new AxisListProjectProperty(null);
-            fail("Null should be handled by ProjectProperty constructor.");
+            fail("Null should be handled by AxisListProjectProperty constructor.");
         } catch (Exception e) {
             assertEquals(BaseProjectProperty.INVALID_JOB_EXCEPTION, e.getMessage());
         }
@@ -135,7 +145,17 @@ public class ProjectPropertyTest {
     public void testSCMProjectPropertyConstructor() {
         try {
             new SCMProjectProperty(null);
-            fail("Null should be handled by ProjectProperty constructor.");
+            fail("Null should be handled by SCMProjectProperty constructor.");
+        } catch (Exception e) {
+            assertEquals(BaseProjectProperty.INVALID_JOB_EXCEPTION, e.getMessage());
+        }
+    }
+
+    @Test
+    public void testExternalProjectPropertyConstructor() {
+        try {
+            new ExternalProjectProperty(null);
+            fail("Null should be handled by ExternalProjectProperty constructor.");
         } catch (Exception e) {
             assertEquals(BaseProjectProperty.INVALID_JOB_EXCEPTION, e.getMessage());
         }
@@ -254,6 +274,7 @@ public class ProjectPropertyTest {
         assertNotNull(property.getDefaultValue());
     }
 
+
     @Test
     public void testScmtProjectPropertyGetDefaultValue() {
         BaseProjectProperty property = new SCMProjectProperty(project);
@@ -359,6 +380,7 @@ public class ProjectPropertyTest {
         assertTrue(property.allowOverrideValue(new NullSCM(), new FakeSCM()));
     }
 
+
     /**
      * Verify getCascadingValue method.
      */
@@ -550,19 +572,19 @@ public class ProjectPropertyTest {
         BaseProjectProperty property = new BaseProjectProperty(project);
         try {
             property.setValue("value");
-            fail("PropertyKey shouldn't be null");
+            fail("PropertyKey shouldn't be null while calling setValue");
         } catch (Exception e) {
             assertEquals(BaseProjectProperty.INVALID_PROPERTY_KEY_EXCEPTION, e.getMessage());
         }
         try {
             property.getCascadingValue();
-            fail("PropertyKey shouldn't be null");
+            fail("PropertyKey shouldn't be null while calling getCascadingValue()");
         } catch (Exception e) {
             assertEquals(BaseProjectProperty.INVALID_PROPERTY_KEY_EXCEPTION, e.getMessage());
         }
         try {
             property.getValue();
-            fail("PropertyKey shouldn't be null");
+            fail("PropertyKey shouldn't be null while calling getValue()");
         } catch (Exception e) {
             assertEquals(BaseProjectProperty.INVALID_PROPERTY_KEY_EXCEPTION, e.getMessage());
         }
@@ -625,9 +647,11 @@ public class ProjectPropertyTest {
 
     /**
      * Test returnOriginalValue method for DescribableListProjectProperty.
+     *
+     * @throws IOException if any.
      */
     @Test
-    public void testDescribableListProjectPropertyReturnOriginalValue() throws IOException{
+    public void testDescribableListProjectPropertyReturnOriginalValue() throws IOException {
         DescribableListProjectProperty property = new DescribableListProjectProperty(project);
         //False - If isOverridden flag is false and original value is null
         assertFalse(property.returnOriginalValue());
@@ -647,6 +671,24 @@ public class ProjectPropertyTest {
         assertTrue(property.returnOriginalValue());
     }
 
+    /**
+     * Test updateOriginalValue method for ExternalProjectProperty.
+     */
+    @Test
+    public void testExternalProjectPropertyUpdateOriginalValue() {
+        ExternalProjectProperty property = new ExternalProjectProperty(project);
+        //If property is not modified, than it shouldn't be updated.
+        assertFalse(property.isModified());
+        assertFalse(property.updateOriginalValue(new Object(), new Object()));
+
+        //If property is modified, than BaseProjectProperty#updateOriginalValue method will be called.
+        //Result will be true, because in this case property value will be updated.
+        property.setModified(true);
+        assertTrue(property.isModified());
+        assertTrue(property.updateOriginalValue(new Object(), new Object()));
+
+    }
+
     private class FakeSCM extends SCM {
         @Override
         public SCMRevisionState calcRevisionsFromBuild(AbstractBuild<?, ?> build, Launcher launcher,
@@ -676,4 +718,4 @@ public class ProjectPropertyTest {
         }
     }
 
-}
\ No newline at end of file
+}