Fix NPE on job load. Add null checks for cascading children

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

4 files changed, 83 insertions, 22 deletions

diff --git a/hudson-core/src/main/java/hudson/model/Job.java b/hudson-core/src/main/java/hudson/model/Job.java
index 17baabb..2dcb2fb 100644
--- a/hudson-core/src/main/java/hudson/model/Job.java
+++ b/hudson-core/src/main/java/hudson/model/Job.java
@@ -428,7 +428,6 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
     void convertJobProperty() {
         if (null != properties && null == getProperty(PARAMETERS_DEFINITION_JOB_PROPERTY_PROPERTY_NAME)) {
             setParameterDefinitionProperties(properties);
-            properties = null;
         }
     }
     /**
diff --git a/hudson-core/src/main/java/hudson/util/CascadingUtil.java b/hudson-core/src/main/java/hudson/util/CascadingUtil.java
index c2df54d..85772e1 100644
--- a/hudson-core/src/main/java/hudson/util/CascadingUtil.java
+++ b/hudson-core/src/main/java/hudson/util/CascadingUtil.java
@@ -430,7 +430,7 @@ public class CascadingUtil {
      */
     @SuppressWarnings("unchecked")
     public static void setChildrenTrigger(Job job, TriggerDescriptor descriptor, String key, StaplerRequest req,
-                                      JSONObject json) throws Descriptor.FormException {
+                                          JSONObject json) throws Descriptor.FormException {
         TriggerProjectProperty property = CascadingUtil.getTriggerProjectProperty(job, key);
         if (property.getValue() != null) {
             property.getValue().stop();
@@ -442,19 +442,20 @@ public class CascadingUtil {
         }
         property.setValue(trigger);
         Set<String> cascadingChildrenNames = job.getCascadingChildrenNames();
-        for (String childName : cascadingChildrenNames) {
-            Job childJob = (Job) Hudson.getInstance().getItem(childName);
-            if (StringUtils.equals(job.getName(), childJob.getCascadingProjectName())) {
-                TriggerProjectProperty childProperty = CascadingUtil.getTriggerProjectProperty(childJob, key);
-                if (!childProperty.isOverridden()) {
-                    setChildrenTrigger(childJob, descriptor, key, req, json);
-                } else if (!childProperty.allowOverrideValue(trigger, childProperty.getValue())) {
-                    childProperty.setOverridden(false);
+        if (null != cascadingChildrenNames) {
+            for (String childName : cascadingChildrenNames) {
+                Job childJob = (Job) Hudson.getInstance().getItem(childName);
+                if (StringUtils.equals(job.getName(), childJob.getCascadingProjectName())) {
+                    TriggerProjectProperty childProperty = CascadingUtil.getTriggerProjectProperty(childJob, key);
+                    if (!childProperty.isOverridden()) {
+                        setChildrenTrigger(childJob, descriptor, key, req, json);
+                    } else if (!childProperty.allowOverrideValue(trigger, childProperty.getValue())) {
+                        childProperty.setOverridden(false);
+                    }
                 }
             }
         }
     }
-
     /**
      * Sets parameterDefinitionProperties for current job. This method is recursively executed for cascading children
      * for setting valid {@link ParametersDefinitionProperty#owner} value.
@@ -480,17 +481,19 @@ public class CascadingUtil {
         projectProperty.setValue(pdProperties);
         Set<String> cascadingChildrenNames = job.getCascadingChildrenNames();
         //Iterate through cascading children and recursively update property for each child.
-        for (String childName : cascadingChildrenNames) {
-            AbstractProject childJob = (AbstractProject) Hudson.getInstance().getItem(childName);
-            //Check only direct children in order to avoid deep checking for properties overridden properties.
-            if (StringUtils.equals(job.getName(), childJob.getCascadingProjectName())) {
-                CopyOnWriteListProjectProperty childProperty = getCopyOnWriteListProjectProperty(childJob, key);
-                //If child value is equal to parent - mark this value as unmodified.
-                if (!projectProperty.allowOverrideValue(childProperty.getValue(), pdProperties)) {
-                    projectProperty.setOverridden(false);
-                } else if (!childProperty.isOverridden()) {
-                    //If child property was not overridden, update this property and cascading children if any.
-                    setParameterDefinitionProperties(childJob, key, parameterDefinitionProperties);
+        if (null != cascadingChildrenNames) {
+            for (String childName : cascadingChildrenNames) {
+                AbstractProject childJob = (AbstractProject) Hudson.getInstance().getItem(childName);
+                //Check only direct children in order to avoid deep checking for properties overridden properties.
+                if (StringUtils.equals(job.getName(), childJob.getCascadingProjectName())) {
+                    CopyOnWriteListProjectProperty childProperty = getCopyOnWriteListProjectProperty(childJob, key);
+                    //If child value is equal to parent - mark this value as unmodified.
+                    if (!projectProperty.allowOverrideValue(childProperty.getValue(), pdProperties)) {
+                        projectProperty.setOverridden(false);
+                    } else if (!childProperty.isOverridden()) {
+                        //If child property was not overridden, update this property and cascading children if any.
+                        setParameterDefinitionProperties(childJob, key, parameterDefinitionProperties);
+                    }
                 }
             }
         }
diff --git a/hudson-core/src/test/java/hudson/model/LegacyProjectTest.java b/hudson-core/src/test/java/hudson/model/LegacyProjectTest.java
index d13392a..e53aace 100644
--- a/hudson-core/src/test/java/hudson/model/LegacyProjectTest.java
+++ b/hudson-core/src/test/java/hudson/model/LegacyProjectTest.java
@@ -22,11 +22,14 @@ import hudson.triggers.SCMTrigger;
 import hudson.triggers.TimerTrigger;
 import hudson.triggers.Trigger;
 import hudson.triggers.TriggerDescriptor;
+import hudson.util.CopyOnWriteList;
 import java.io.File;
 import java.io.IOException;
 import java.net.URISyntaxException;
+import java.util.List;
 import org.eclipse.hudson.api.model.IProjectProperty;
 import org.eclipse.hudson.api.model.project.property.BooleanProjectProperty;
+import org.eclipse.hudson.api.model.project.property.CopyOnWriteListProjectProperty;
 import org.junit.Before;
 import org.junit.Ignore;
 import org.junit.Test;
@@ -339,4 +342,43 @@ public class LegacyProjectTest {
         assertNotNull(project.getProperty(AbstractProject.APPOINTED_NODE_PROPERTY_NAME));
         assertTrue(project.isAdvancedAffinityChooser());
     }
+
+    @Test
+    @SuppressWarnings("unchecked")
+    public void testConvertLegacyParameterDefinitionProperties() throws IOException {
+        Job project = (Job) Items.getConfigFile(config).read();
+        project.setAllowSave(false);
+        project.initProjectProperties();
+        assertNull(project.getProperty(Job.PARAMETERS_DEFINITION_JOB_PROPERTY_PROPERTY_NAME));
+        project.convertJobProperty();
+        //Properties should be initialized
+        assertNotNull(project.properties);
+        CopyOnWriteListProjectProperty property = (CopyOnWriteListProjectProperty) project.getProperty(
+            Job.PARAMETERS_DEFINITION_JOB_PROPERTY_PROPERTY_NAME);
+        assertNotNull(property);
+        CopyOnWriteList<ParametersDefinitionProperty> propertyValue = property.getValue();
+        assertFalse(propertyValue.isEmpty());
+        assertEquals(1, propertyValue.size());
+        ParametersDefinitionProperty pdp = propertyValue.get(0);
+        List<ParameterDefinition> parameterDefinitions = pdp.getParameterDefinitions();
+        assertNotNull(parameterDefinitions);
+        assertFalse(parameterDefinitions.isEmpty());
+        String parameterName1 = "test1";
+        String parameterName2 = "test2";
+        assertTrue(pdp.getParameterDefinitionNames().contains(parameterName1));
+        assertTrue(pdp.getParameterDefinitionNames().contains(parameterName2));
+        ParameterDefinition parameterDefinition = pdp.getParameterDefinition(parameterName1);
+        assertNotNull(parameterDefinition);
+        assertTrue(parameterDefinition instanceof StringParameterDefinition);
+        assertEquals(parameterName1, parameterDefinition.getName());
+        assertEquals("df1", ((StringParameterDefinition) parameterDefinition).getDefaultValue());
+        assertEquals("string value description", parameterDefinition.getDescription());
+
+        parameterDefinition = pdp.getParameterDefinition(parameterName2);
+        assertNotNull(parameterDefinition);
+        assertTrue(parameterDefinition instanceof BooleanParameterDefinition);
+        assertEquals(parameterName2, parameterDefinition.getName());
+        assertEquals(true, ((BooleanParameterDefinition) parameterDefinition).isDefaultValue());
+        assertEquals("boolean value description", parameterDefinition.getDescription());
+    }
 }
diff --git a/hudson-core/src/test/resources/hudson/model/freestyle/config.xml b/hudson-core/src/test/resources/hudson/model/freestyle/config.xml
index 8fcdc60..21a0070 100644
--- a/hudson-core/src/test/resources/hudson/model/freestyle/config.xml
+++ b/hudson-core/src/test/resources/hudson/model/freestyle/config.xml
@@ -50,4 +50,21 @@
     </hudson.tasks.Maven>
   </builders>
   <customWorkspace>/tmp</customWorkspace>
+  <properties>
+    <hudson.model.ParametersDefinitionProperty>
+      <parameterDefinitions>
+        <hudson.model.StringParameterDefinition>
+          <name>test1</name>
+          <description>string value description</description>
+          <defaultValue>df1</defaultValue>
+        </hudson.model.StringParameterDefinition>
+        <hudson.model.BooleanParameterDefinition>
+          <name>test2</name>
+          <description>boolean value description</description>
+          <defaultValue>true</defaultValue>
+        </hudson.model.BooleanParameterDefinition>
+      </parameterDefinitions>
+    </hudson.model.ParametersDefinitionProperty>
+  </properties>
+
 </project>