[FIXED HUDSON-1300] Nominating m2 jobs and matrix jobs as upstream wasn't working.

I also made the changes to update relevant data structures to use DescribableList (but in a way that doesn't break the signature --- this made it somewhat ugly.) I also had to update ReflectionConverted to deal with existing data that has class="vector".

git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@10423 71c3de6d-444a-0410-be80-ed276b4c234a

core/src/main/java/hudson/matrix/MatrixProject.java

@@ -2,10 +2,10 @@ package hudson.matrix;
 
 import hudson.CopyOnWrite;
 import hudson.FilePath;
-import hudson.XmlFile;
 import hudson.StructuredForm;
-import hudson.triggers.Trigger;
+import hudson.XmlFile;
 import hudson.model.AbstractProject;
+import hudson.model.Action;
 import hudson.model.DependencyGraph;
 import hudson.model.Descriptor;
 import hudson.model.Descriptor.FormException;
@@ -20,14 +20,16 @@ import hudson.model.Node;
 import hudson.model.SCMedItem;
 import hudson.model.TopLevelItem;
 import hudson.model.TopLevelItemDescriptor;
-import hudson.model.Action;
 import hudson.tasks.BuildStep;
+import hudson.tasks.BuildStepDescriptor;
 import hudson.tasks.BuildWrapper;
 import hudson.tasks.BuildWrappers;
 import hudson.tasks.Builder;
 import hudson.tasks.Publisher;
-import hudson.tasks.BuildStepDescriptor;
+import hudson.triggers.Trigger;
 import hudson.util.CopyOnWriteMap;
+import hudson.util.DescribableList;
+import net.sf.json.JSONObject;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
 
@@ -38,15 +40,14 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
-import java.util.Vector;
-import java.util.HashMap;
 import java.util.Map.Entry;
+import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -56,7 +57,7 @@ import java.util.logging.Logger;
  *
  * @author Kohsuke Kawaguchi
  */
-public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> implements TopLevelItem, SCMedItem, ItemGroup<MatrixConfiguration> {
+public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> implements TopLevelItem, SCMedItem, ItemGroup<MatrixConfiguration>, DescribableList.Owner {
     /**
      * Other configuration axes.
      *
@@ -67,17 +68,20 @@ public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> im
     /**
      * List of active {@link Builder}s configured for this project.
      */
-    private volatile List<Builder> builders = new Vector<Builder>();
+    private DescribableList<Builder,Descriptor<Builder>> builders =
+            new DescribableList<Builder,Descriptor<Builder>>(this);
 
     /**
      * List of active {@link Publisher}s configured for this project.
      */
-    private volatile List<Publisher> publishers = new Vector<Publisher>();
+    private DescribableList<Publisher,Descriptor<Publisher>> publishers =
+            new DescribableList<Publisher,Descriptor<Publisher>>(this);
 
     /**
      * List of active {@link BuildWrapper}s configured for this project.
      */
-    private volatile List<BuildWrapper> buildWrappers = new Vector<BuildWrapper>();
+    private DescribableList<BuildWrapper,Descriptor<BuildWrapper>> buildWrappers =
+            new DescribableList<BuildWrapper,Descriptor<BuildWrapper>>(this);
 
     /**
      * All {@link MatrixConfiguration}s, keyed by their {@link MatrixConfiguration#getName() names}.
@@ -147,7 +151,9 @@ public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> im
     public void onLoad(ItemGroup<? extends Item> parent, String name) throws IOException {
         super.onLoad(parent,name);
         Collections.sort(axes); // perhaps the file was edited on disk and the sort order might have been broken
-
+        builders.setOwner(this);
+        publishers.setOwner(this);
+        buildWrappers.setOwner(this);
         rebuildConfigurations();
     }
 
@@ -353,15 +359,19 @@ public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> im
     }
 
     public List<Builder> getBuilders() {
-        return Collections.unmodifiableList(builders);
+        return builders.toList();
     }
 
     public Map<Descriptor<Publisher>,Publisher> getPublishers() {
-        return Descriptor.toMap(publishers);
+        return publishers.toMap();
+    }
+
+    public DescribableList<Publisher,Descriptor<Publisher>> getPublishersList() {
+        return publishers;
     }
 
     public Map<Descriptor<BuildWrapper>,BuildWrapper> getBuildWrappers() {
-        return Descriptor.toMap(buildWrappers);
+        return buildWrappers.toMap();
     }
 
     public Publisher getPublisher(Descriptor<Publisher> descriptor) {
@@ -388,7 +398,9 @@ public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> im
     }
 
     protected void buildDependencyGraph(DependencyGraph graph) {
-        graph.addDependencyDeclarers(this,publishers);
+        publishers.buildDependencyGraph(this,graph);
+        builders.buildDependencyGraph(this,graph);
+        buildWrappers.buildDependencyGraph(this,graph);
     }
 
     public MatrixProject asProject() {
@@ -427,11 +439,11 @@ public class MatrixProject extends AbstractProject<MatrixProject,MatrixBuild> im
             newAxes.add(Axis.parsePrefixed(req,"label"));
         this.axes = newAxes;
 
-        buildWrappers = buildDescribable(req, BuildWrappers.getFor(this), "wrapper");
-        builders = Descriptor.newInstancesFromHeteroList(req,
-                StructuredForm.get(req), "builder", BuildStep.BUILDERS);
+        JSONObject json = StructuredForm.get(req);
 
-        publishers = buildDescribable(req, BuildStepDescriptor.filter(BuildStep.PUBLISHERS,this.getClass()), "publisher");
+        buildWrappers.rebuild(req, json, BuildWrappers.getFor(this), "wrapper");
+        builders.rebuildHetero(req, json, BuildStep.BUILDERS, "builder");
+        publishers.rebuild(req, json, BuildStepDescriptor.filter(BuildStep.PUBLISHERS,this.getClass()), "publisher");
         updateTransientActions(); // to pick up transient actions from builder, publisher, etc.
 
         rebuildConfigurations();

core/src/main/java/hudson/maven/MavenModule.java

@@ -20,6 +20,7 @@ import hudson.model.Label;
 import hudson.model.Node;
 import hudson.model.Resource;
 import hudson.tasks.LogRotator;
+import hudson.tasks.Publisher;
 import hudson.util.DescribableList;
 import org.apache.maven.project.MavenProject;
 import org.kohsuke.stapler.StaplerRequest;
@@ -220,6 +221,11 @@ public final class MavenModule extends AbstractMavenProject<MavenModule,MavenBui
         return getParent().getModuleRoot().child(relativePath);
     }
 
+    public DescribableList<Publisher,Descriptor<Publisher>> getPublishersList() {
+        // TODO
+        return new DescribableList<Publisher,Descriptor<Publisher>>(this);
+    }
+
     @Override
     public JDK getJDK() {
         // share one setting for the whole module set.

core/src/main/java/hudson/maven/MavenModuleSet.java

@@ -241,6 +241,11 @@ public final class MavenModuleSet extends AbstractMavenProject<MavenModuleSet,Ma
         return publishers;
     }
 
+    @Override
+    public DescribableList<Publisher, Descriptor<Publisher>> getPublishersList() {
+        return publishers;
+    }
+
     /**
      * List of active {@link BuildWrapper}s. Can be empty but never null.
      */
@@ -337,10 +342,13 @@ public final class MavenModuleSet extends AbstractMavenProject<MavenModuleSet,Ma
 
         if(reporters==null)
             reporters = new DescribableList<MavenReporter, Descriptor<MavenReporter>>(this);
+        reporters.setOwner(this);
         if(publishers==null)
             publishers = new DescribableList<Publisher,Descriptor<Publisher>>(this);
+        publishers.setOwner(this);
         if(buildWrappers==null)
             buildWrappers = new DescribableList<BuildWrapper, Descriptor<BuildWrapper>>(this);
+        buildWrappers.setOwner(this);
 
         updateTransientActions();
     }

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

@@ -18,11 +18,13 @@ import hudson.search.SearchIndexBuilder;
 import hudson.security.ACL;
 import hudson.security.Permission;
 import hudson.tasks.BuildTrigger;
+import hudson.tasks.Publisher;
 import hudson.triggers.SCMTrigger;
 import hudson.triggers.Trigger;
 import hudson.triggers.TriggerDescriptor;
 import hudson.triggers.Triggers;
 import hudson.util.EditDistance;
+import hudson.util.DescribableList;
 import hudson.widgets.BuildHistoryWidget;
 import hudson.widgets.HistoryWidget;
 import net.sf.json.JSONObject;
@@ -280,6 +282,15 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
         }
     }
 
+    /**
+     * Returns the live list of all {@link Publisher}s configured for this project.
+     *
+     * <p>
+     * This method couldn't be called <tt>getPublishers()</tt> because existing methods
+     * in sub-classes return different inconsistent types.
+     */
+    public abstract DescribableList<Publisher,Descriptor<Publisher>> getPublishersList();
+
     @Override
     public void addProperty(JobProperty<? super P> jobProp) throws IOException {
         super.addProperty(jobProp);
@@ -312,11 +323,12 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
         // this needs to be done after we release the lock on 'this',
         // or otherwise we could dead-lock
 
-        for (Project p : Hudson.getInstance().getProjects()) {
+        for (AbstractProject<?,?> p : Hudson.getInstance().getAllItems(AbstractProject.class)) {
             boolean isUpstream = upstream.contains(p);
             synchronized(p) {
                 // does 'p' include us in its BuildTrigger? 
-                BuildTrigger trigger = (BuildTrigger) p.getPublisher(BuildTrigger.DESCRIPTOR);
+                DescribableList<Publisher,Descriptor<Publisher>> pl = p.getPublishersList();
+                BuildTrigger trigger = (BuildTrigger) pl.get(BuildTrigger.DESCRIPTOR);
                 List<AbstractProject> newChildProjects = trigger == null ? new ArrayList<AbstractProject>():trigger.getChildProjects();
                 if(isUpstream) {
                     if(!newChildProjects.contains(this))
@@ -326,12 +338,12 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
                 }
 
                 if(newChildProjects.isEmpty()) {
-                    p.removePublisher(BuildTrigger.DESCRIPTOR);
+                    pl.remove(BuildTrigger.DESCRIPTOR);
                 } else {
-                    BuildTrigger existing = (BuildTrigger)p.getPublisher(BuildTrigger.DESCRIPTOR);
+                    BuildTrigger existing = (BuildTrigger)pl.get(BuildTrigger.DESCRIPTOR);
                     if(existing!=null && existing.hasSame(newChildProjects))
                         continue;   // no need to touch
-                    p.addPublisher(new BuildTrigger(newChildProjects,
+                    pl.add(new BuildTrigger(newChildProjects,
                         existing==null?Result.SUCCESS:existing.getThreshold()));
                 }
             }
@@ -999,7 +1011,7 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
      * Finds a {@link AbstractProject} that has the name closest to the given name.
      */
     public static AbstractProject findNearest(String name) {
-        List<AbstractProject> projects = Hudson.getInstance().getAllItems(AbstractProject.class);
+        List<AbstractProject> projects = Hudson.getInstance().getItems(AbstractProject.class);
         String[] names = new String[projects.size()];
         for( int i=0; i<projects.size(); i++ )
             names[i] = projects.get(i).getName();

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

 package hudson.model;
 
-import hudson.CopyOnWrite;
-import hudson.Util;
 import hudson.StructuredForm;
+import hudson.Util;
 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;
 import hudson.tasks.Fingerprinter;
 import hudson.tasks.Publisher;
-import hudson.tasks.BuildStepDescriptor;
 import hudson.triggers.Trigger;
+import hudson.util.DescribableList;
+import net.sf.json.JSONObject;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
 
@@ -21,8 +22,6 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.Vector;
-import java.util.Collections;
 
 /**
  * Buildable software project.
@@ -30,25 +29,25 @@ import java.util.Collections;
  * @author Kohsuke Kawaguchi
  */
 public abstract class Project<P extends Project<P,B>,B extends Build<P,B>>
-    extends AbstractProject<P,B> implements SCMedItem {
+    extends AbstractProject<P,B> implements SCMedItem, DescribableList.Owner {
 
     /**
      * List of active {@link Builder}s configured for this project.
      */
-    @CopyOnWrite
-    private volatile List<Builder> builders = new Vector<Builder>();
+    private DescribableList<Builder,Descriptor<Builder>> builders =
+            new DescribableList<Builder,Descriptor<Builder>>(this);
 
     /**
      * List of active {@link Publisher}s configured for this project.
      */
-    @CopyOnWrite
-    private volatile List<Publisher> publishers = new Vector<Publisher>();
+    private DescribableList<Publisher,Descriptor<Publisher>> publishers =
+            new DescribableList<Publisher,Descriptor<Publisher>>(this);
 
     /**
      * List of active {@link BuildWrapper}s configured for this project.
      */
-    @CopyOnWrite
-    private volatile List<BuildWrapper> buildWrappers = new Vector<BuildWrapper>();
+    private DescribableList<BuildWrapper,Descriptor<BuildWrapper>> buildWrappers =
+            new DescribableList<BuildWrapper,Descriptor<BuildWrapper>>(this);
 
     /**
      * Creates a new project.
@@ -62,7 +61,10 @@ public abstract class Project<P extends Project<P,B>,B extends Build<P,B>>
 
         if(buildWrappers==null)
             // it didn't exist in < 1.64
-            buildWrappers = new Vector<BuildWrapper>();
+            buildWrappers = new DescribableList<BuildWrapper, Descriptor<BuildWrapper>>(this);
+        builders.setOwner(this);
+        publishers.setOwner(this);
+        buildWrappers.setOwner(this);
     }
 
     public AbstractProject<?, ?> asProject() {
@@ -70,15 +72,19 @@ public abstract class Project<P extends Project<P,B>,B extends Build<P,B>>
     }
 
     public List<Builder> getBuilders() {
-        return Collections.unmodifiableList(builders);
+        return builders.toList();
     }
 
     public Map<Descriptor<Publisher>,Publisher> getPublishers() {
-        return Descriptor.toMap(publishers);
+        return publishers.toMap();
+    }
+
+    public DescribableList<Publisher,Descriptor<Publisher>> getPublishersList() {
+        return publishers;
     }
 
     public Map<Descriptor<BuildWrapper>,BuildWrapper> getBuildWrappers() {
-        return Descriptor.toMap(buildWrappers);
+        return buildWrappers.toMap();
     }
 
     @Override
@@ -97,14 +103,14 @@ public abstract class Project<P extends Project<P,B>,B extends Build<P,B>>
      * Adds a new {@link BuildStep} to this {@link Project} and saves the configuration.
      */
     public void addPublisher(Publisher buildStep) throws IOException {
-        addToList(buildStep,publishers);
+        publishers.add(buildStep);
     }
 
     /**
      * Removes a publisher from this project, if it's active.
      */
     public void removePublisher(Descriptor<Publisher> descriptor) throws IOException {
-        removeFromList(descriptor, publishers);
+        publishers.remove(descriptor);
     }
 
     public Publisher getPublisher(Descriptor<Publisher> descriptor) {
@@ -116,9 +122,9 @@ public abstract class Project<P extends Project<P,B>,B extends Build<P,B>>
     }
 
     protected void buildDependencyGraph(DependencyGraph graph) {
-        graph.addDependencyDeclarers(this,publishers);
-        graph.addDependencyDeclarers(this,builders);
-        graph.addDependencyDeclarers(this,buildWrappers);
+        publishers.buildDependencyGraph(this,graph);
+        builders.buildDependencyGraph(this,graph);
+        buildWrappers.buildDependencyGraph(this,graph);
     }
 
     @Override
@@ -144,11 +150,11 @@ public abstract class Project<P extends Project<P,B>,B extends Build<P,B>>
         super.submit(req,rsp);
 
         req.setCharacterEncoding("UTF-8");
+        JSONObject json = StructuredForm.get(req);
 
-        buildWrappers = buildDescribable(req, BuildWrappers.getFor(this), "wrapper");
-        builders = Descriptor.newInstancesFromHeteroList(req,
-                StructuredForm.get(req), "builder", BuildStep.BUILDERS);
-        publishers = buildDescribable(req, BuildStepDescriptor.filter(BuildStep.PUBLISHERS, this.getClass()), "publisher");
+        buildWrappers.rebuild(req,json, BuildWrappers.getFor(this), "wrapper");
+        builders.rebuildHetero(req,json, BuildStep.BUILDERS, "builder");
+        publishers.rebuild(req, json, BuildStepDescriptor.filter(BuildStep.PUBLISHERS, this.getClass()), "publisher");
         updateTransientActions(); // to pick up transient actions from builder, publisher, etc.
     }
 

core/src/main/java/hudson/scm/SCM.java

@@ -31,6 +31,11 @@ import java.util.Map;
  * The view gets the "builds" variable which is a list of builds that are
  * selected for the display.
  *
+ * <p>
+ * If you are interested in writing a subclass in a plugin,
+ * also take a look at <a href="http://hudson.gotdns.com/wiki/display/HUDSON/Writing+an+SCM+plugin">
+ * "Writing an SCM plugin"</a> wiki article.
+ *
  * @author Kohsuke Kawaguchi
  */
 public abstract class SCM implements Describable<SCM>, ExtensionPoint {

core/src/main/java/hudson/util/DescribableList.java

@@ -9,6 +9,7 @@ import com.thoughtworks.xstream.io.HierarchicalStreamWriter;
 import com.thoughtworks.xstream.mapper.Mapper;
 import hudson.model.*;
 import hudson.model.Descriptor.FormException;
+import hudson.tasks.Builder;
 import net.sf.json.JSONObject;
 import org.kohsuke.stapler.StaplerRequest;
 import org.apache.maven.model.Dependency;
@@ -19,11 +20,20 @@ import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Collections;
 
 /**
  * Persisted list of {@link Describable}s with some operations specific
  * to {@link Descriptor}s.
  *
+ * <p>
+ * This class allows multiple instances of the same descriptor. Some clients
+ * use this semantics, while other clients use it as "up to one instance per
+ * one descriptor" model.
+ *
+ * Some of the methods defined in this class only makes sense in the latter model,
+ * such as {@link #remove(Descriptor)}.
+ *
  * @author Kohsuke Kawaguchi
  */
 public class DescribableList<T extends Describable<T>, D extends Descriptor<T>> implements Iterable<T> {
@@ -76,6 +86,13 @@ public class DescribableList<T extends Describable<T>, D extends Descriptor<T>>
         return (Map)Descriptor.toMap(data);
     }
 
+    /**
+     * Returns the snapshot view of instances as list.
+     */
+    public List<T> toList() {
+        return data.getView();
+    }
+
     /**
      * Gets all the {@link Describable}s in an array.
      */
@@ -111,6 +128,18 @@ public class DescribableList<T extends Describable<T>, D extends Descriptor<T>>
         data.replaceBy(newList);
     }
 
+    /**
+     * Rebuilds the list by creating a fresh instances from the submitted form.
+     *
+     * <p>
+     * This version works with the the &lt;f:hetero-list> UI tag, where the user
+     * is allowed to create multiple instances of the same descriptor. Order is also
+     * significant.
+     */
+    public void rebuildHetero(StaplerRequest req, JSONObject formData, Collection<Descriptor<T>> descriptors, String key) throws FormException {
+        data.replaceBy(Descriptor.newInstancesFromHeteroList(req,formData,key,descriptors));
+    }
+
     /**
      * Picks up {@link DependecyDeclarer}s and allow it to build dependencies.
      */

core/src/main/java/hudson/util/RobustReflectionConverter.java

@@ -297,9 +297,15 @@ public class RobustReflectionConverter implements Converter {
 
     private Class determineType(HierarchicalStreamReader reader, boolean validField, Object result, String fieldName, Class definedInCls) {
         String classAttribute = reader.getAttribute(mapper.aliasForAttribute("class"));
+        Class fieldType = reflectionProvider.getFieldType(result, fieldName, definedInCls);
         if (classAttribute != null) {
-            return mapper.realClass(classAttribute);
-        } else if (!validField) {
+            Class specifiedType = mapper.realClass(classAttribute);
+            if(fieldType.isAssignableFrom(specifiedType))
+                // make sure that the specified type in XML is compatible with the field type.
+                // this allows the code to evolve in more flexible way.
+                return specifiedType;
+        }
+        if (!validField) {
             Class itemType = mapper.getItemTypeForItemFieldName(result.getClass(), fieldName);
             if (itemType != null) {
                 return itemType;
@@ -307,7 +313,7 @@ public class RobustReflectionConverter implements Converter {
                 return mapper.realClass(reader.getNodeName());
             }
         } else {
-            return mapper.defaultImplementationOf(reflectionProvider.getFieldType(result, fieldName, definedInCls));
+            return mapper.defaultImplementationOf(fieldType);
         }
     }