[FIXED JENKINS-15817] XStream form of projects excessively strict about null fields.

changelog.html

@@ -55,6 +55,9 @@ Upcoming changes</a>
 <!-- Record your changes in the trunk here. -->
 <div id="trunk" style="display:none"><!--=TRUNK-BEGIN=-->
 <ul class=image>
+  <li class=rfe>
+    XStream form of projects excessively strict about null fields.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-15817">issue 15817</a>)
   <li class=bug>
     Build records were broken if timezone was changed while running.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-15816">issue 15816</a>)

core/src/main/java/hudson/model/AbstractProject.java

@@ -39,7 +39,6 @@ import hudson.Launcher;
 import hudson.Util;
 import hudson.cli.declarative.CLIMethod;
 import hudson.cli.declarative.CLIResolver;
-import hudson.diagnosis.OldDataMonitor;
 import hudson.model.Cause.LegacyCodeCause;
 import hudson.model.Cause.RemoteCause;
 import hudson.model.Cause.UserIdCause;
@@ -80,7 +79,6 @@ import hudson.util.AlternativeUiTextProvider.Message;
 import hudson.util.DescribableList;
 import hudson.util.EditDistance;
 import hudson.util.FormValidation;
-import hudson.util.RunList;
 import hudson.widgets.BuildHistoryWidget;
 import hudson.widgets.HistoryWidget;
 import jenkins.model.Jenkins;
@@ -285,13 +283,7 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
             }
         }
         this.builds = builds;
-
-        if(triggers==null) {
-            // it didn't exist in < 1.28
-            triggers = new Vector<Trigger<?>>();
-            OldDataMonitor.report(this, "1.28");
-        }
-        for (Trigger t : triggers)
+        for (Trigger t : triggers())
             t.start(this, Items.updatingByXml.get());
         if(scm==null)
             scm = new NullSCM(); // perhaps it was pointing to a plugin that no longer exists.
@@ -301,6 +293,13 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
         updateTransientActions();
     }
 
+    private synchronized List<Trigger<?>> triggers() {
+        if (triggers == null) {
+            triggers = new Vector<Trigger<?>>();
+        }
+        return triggers;
+    }
+
     @Override
     public EnvVars getEnvironment(Node node, TaskListener listener) throws IOException, InterruptedException {
         EnvVars env =  super.getEnvironment(node, listener);
@@ -1531,11 +1530,11 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
      * Adds a new {@link Trigger} to this {@link Project} if not active yet.
      */
     public void addTrigger(Trigger<?> trigger) throws IOException {
-        addToList(trigger,triggers);
+        addToList(trigger,triggers());
     }
 
     public void removeTrigger(TriggerDescriptor trigger) throws IOException {
-        removeFromList(trigger,triggers);
+        removeFromList(trigger,triggers());
     }
 
     protected final synchronized <T extends Describable<T>>
@@ -1569,14 +1568,14 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
 
     @SuppressWarnings("unchecked")
     public synchronized Map<TriggerDescriptor,Trigger> getTriggers() {
-        return (Map)Descriptor.toMap(triggers);
+        return (Map)Descriptor.toMap(triggers());
     }
 
     /**
      * Gets the specific trigger, or null if the propert is not configured for this job.
      */
     public <T extends Trigger> T getTrigger(Class<T> clazz) {
-        for (Trigger p : triggers) {
+        for (Trigger p : triggers()) {
             if(clazz.isInstance(p))
                 return clazz.cast(p);
         }
@@ -1862,7 +1861,7 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
 
         setScm(SCMS.parseSCM(req,this));
 
-        for (Trigger t : triggers)
+        for (Trigger t : triggers())
             t.stop();
         triggers = buildDescribable(req, Trigger.for_(this));
         for (Trigger t : triggers)

core/src/main/java/hudson/model/Project.java

@@ -25,10 +25,8 @@
 package hudson.model;
 
 import hudson.Util;
-import hudson.diagnosis.OldDataMonitor;
 import hudson.model.Descriptor.FormException;
 import hudson.tasks.BuildStep;
-import hudson.tasks.BuildStepDescriptor;
 import hudson.tasks.BuildWrapper;
 import hudson.tasks.BuildWrappers;
 import hudson.tasks.Builder;
@@ -61,20 +59,17 @@ public abstract class Project<P extends Project<P,B>,B extends Build<P,B>>
     /**
      * List of active {@link Builder}s configured for this project.
      */
-    private DescribableList<Builder,Descriptor<Builder>> builders =
-            new DescribableList<Builder,Descriptor<Builder>>(this);
+    private DescribableList<Builder,Descriptor<Builder>> builders;
 
     /**
      * List of active {@link Publisher}s configured for this project.
      */
-    private DescribableList<Publisher,Descriptor<Publisher>> publishers =
-            new DescribableList<Publisher,Descriptor<Publisher>>(this);
+    private DescribableList<Publisher,Descriptor<Publisher>> publishers;
 
     /**
      * List of active {@link BuildWrapper}s configured for this project.
      */
-    private DescribableList<BuildWrapper,Descriptor<BuildWrapper>> buildWrappers =
-            new DescribableList<BuildWrapper,Descriptor<BuildWrapper>>(this);
+    private DescribableList<BuildWrapper,Descriptor<BuildWrapper>> buildWrappers;
 
     /**
      * Creates a new project.
@@ -86,15 +81,9 @@ public abstract class Project<P extends Project<P,B>,B extends Build<P,B>>
     @Override
     public void onLoad(ItemGroup<? extends Item> parent, String name) throws IOException {
         super.onLoad(parent, name);
-
-        if (buildWrappers==null) {
-            // it didn't exist in < 1.64
-            buildWrappers = new DescribableList<BuildWrapper, Descriptor<BuildWrapper>>(this);
-            OldDataMonitor.report(this, "1.64");
-        }
-        builders.setOwner(this);
-        publishers.setOwner(this);
-        buildWrappers.setOwner(this);
+        getBuildersList().setOwner(this);
+        getPublishersList().setOwner(this);
+        getBuildWrappersList().setOwner(this);
     }
 
     public AbstractProject<?, ?> asProject() {
@@ -102,7 +91,7 @@ public abstract class Project<P extends Project<P,B>,B extends Build<P,B>>
     }
 
     public List<Builder> getBuilders() {
-        return builders.toList();
+        return getBuildersList().toList();
     }
 
     /**
@@ -111,22 +100,31 @@ public abstract class Project<P extends Project<P,B>,B extends Build<P,B>>
      *      Use {@link #getPublishersList()} instead.
      */
     public Map<Descriptor<Publisher>,Publisher> getPublishers() {
-        return publishers.toMap();
+        return getPublishersList().toMap();
     }
 
-    public DescribableList<Builder,Descriptor<Builder>> getBuildersList() {
+    public synchronized DescribableList<Builder,Descriptor<Builder>> getBuildersList() {
+        if (builders == null) {
+            builders = new DescribableList<Builder,Descriptor<Builder>>(this);
+        }
         return builders;
     }
     
-    public DescribableList<Publisher,Descriptor<Publisher>> getPublishersList() {
+    public synchronized DescribableList<Publisher,Descriptor<Publisher>> getPublishersList() {
+        if (publishers == null) {
+            publishers = new DescribableList<Publisher,Descriptor<Publisher>>(this);
+        }
         return publishers;
     }
 
     public Map<Descriptor<BuildWrapper>,BuildWrapper> getBuildWrappers() {
-        return buildWrappers.toMap();
+        return getBuildWrappersList().toMap();
     }
 
-    public DescribableList<BuildWrapper, Descriptor<BuildWrapper>> getBuildWrappersList() {
+    public synchronized DescribableList<BuildWrapper, Descriptor<BuildWrapper>> getBuildWrappersList() {
+        if (buildWrappers == null) {
+            buildWrappers = new DescribableList<BuildWrapper,Descriptor<BuildWrapper>>(this);
+        }
         return buildWrappers;
     }
 
@@ -135,9 +133,9 @@ public abstract class Project<P extends Project<P,B>,B extends Build<P,B>>
         final Set<ResourceActivity> activities = new HashSet<ResourceActivity>();
 
         activities.addAll(super.getResourceActivities());
-        activities.addAll(Util.filter(builders,ResourceActivity.class));
-        activities.addAll(Util.filter(publishers,ResourceActivity.class));
-        activities.addAll(Util.filter(buildWrappers,ResourceActivity.class));
+        activities.addAll(Util.filter(getBuildersList(),ResourceActivity.class));
+        activities.addAll(Util.filter(getPublishersList(),ResourceActivity.class));
+        activities.addAll(Util.filter(getBuildWrappersList(),ResourceActivity.class));
 
         return activities;
     }
@@ -149,7 +147,7 @@ public abstract class Project<P extends Project<P,B>,B extends Build<P,B>>
      *      Use {@code getPublishersList().add(x)}
      */
     public void addPublisher(Publisher buildStep) throws IOException {
-        publishers.add(buildStep);
+        getPublishersList().add(buildStep);
     }
 
     /**
@@ -159,11 +157,11 @@ public abstract class Project<P extends Project<P,B>,B extends Build<P,B>>
      *      Use {@code getPublishersList().remove(x)}
      */
     public void removePublisher(Descriptor<Publisher> descriptor) throws IOException {
-        publishers.remove(descriptor);
+        getPublishersList().remove(descriptor);
     }
 
     public Publisher getPublisher(Descriptor<Publisher> descriptor) {
-        for (Publisher p : publishers) {
+        for (Publisher p : getPublishersList()) {
             if(p.getDescriptor()==descriptor)
                 return p;
         }
@@ -171,9 +169,9 @@ public abstract class Project<P extends Project<P,B>,B extends Build<P,B>>
     }
 
     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);
     }
 
     @Override
@@ -198,9 +196,9 @@ public abstract class Project<P extends Project<P,B>,B extends Build<P,B>>
 
         JSONObject json = req.getSubmittedForm();
 
-        buildWrappers.rebuild(req,json, BuildWrappers.getFor(this));
-        builders.rebuildHetero(req,json, Builder.all(), "builder");
-        publishers.rebuildHetero(req, json, Publisher.all(), "publisher");
+        getBuildWrappersList().rebuild(req,json, BuildWrappers.getFor(this));
+        getBuildersList().rebuildHetero(req,json, Builder.all(), "builder");
+        getPublishersList().rebuildHetero(req, json, Publisher.all(), "publisher");
     }
 
     @Override

test/src/test/java/hudson/model/FreeStyleProjectTest.java

@@ -25,8 +25,10 @@ package hudson.model;
 
 import com.gargoylesoftware.htmlunit.html.HtmlForm;
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
+import edu.umd.cs.findbugs.annotations.SuppressWarnings;
 import hudson.tasks.Builder;
 import hudson.tasks.Shell;
+import java.io.ByteArrayInputStream;
 import org.jvnet.hudson.test.Bug;
 import org.jvnet.hudson.test.HudsonTestCase;
 
@@ -121,4 +123,26 @@ public class FreeStyleProjectTest extends HudsonTestCase {
         assertFalse(path.contains("${JOB_NAME}"));
         assertEquals(b.getWorkspace().getName(),f.getName());
     }
+
+    @Bug(15817)
+    @SuppressWarnings("DM_DEFAULT_ENCODING")
+    public void testMinimalConfigXml() throws Exception {
+        // Make sure it can be created without exceptions:
+        FreeStyleProject project = (FreeStyleProject) jenkins.createProjectFromXML("stuff", new ByteArrayInputStream("<project/>".getBytes()));
+        System.out.println(project.getConfigFile().asString());
+        // and round-tripped:
+        Shell shell = new Shell("echo hello");
+        project.getBuildersList().add(shell);
+        WebClient webClient = new WebClient();
+        HtmlPage page = webClient.getPage(project,"configure");
+        HtmlForm form = page.getFormByName("config");
+        submit(form);
+        List<Builder> builders = project.getBuilders();
+        assertEquals(1,builders.size());
+        assertEquals(Shell.class,builders.get(0).getClass());
+        assertEquals("echo hello",((Shell)builders.get(0)).getCommand().trim());
+        assertTrue(builders.get(0)!=shell);
+        System.out.println(project.getConfigFile().asString());
+    }
+
 }