Merge pull request #1059 from jglick/TransientActionFactory-JENKINS-1…

…8224 [JENKINS-18224] TransientActionFactory
jenkinsci · Jan 9, 2014 · 0f7b144 · 0f7b144
2 parents e998c2a + 540a529
commit 0f7b144
Show file tree

Hide file tree

Showing 28 changed files with 279 additions and 94 deletions.
diff --git a/changelog.html b/changelog.html
@@ -55,6 +55,9 @@
 <!-- Record your changes in the trunk here. -->
 <div id="trunk" style="display:none"><!--=TRUNK-BEGIN=-->
 <ul class=image>
+  <li class=rfe>
+    API for adding actions to a wide class of model objects at once.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-18224">issue 18224</a>)
   <li class='major rfe'>
     Added infrastructure for moving items into or out of folders.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-20008">issue 20008</a>)

diff --git a/core/src/main/java/hudson/cli/SetBuildParameterCommand.java b/core/src/main/java/hudson/cli/SetBuildParameterCommand.java
@@ -37,9 +37,7 @@ protected int run() throws Exception {
 
         ParametersAction a = r.getAction(ParametersAction.class);
         if (a!=null) {
-            ParametersAction b = a.createUpdated(Collections.singleton(p));
-            r.addAction(b);
-            r.getActions().remove(a);
+            r.replaceAction(a.createUpdated(Collections.singleton(p)));
         } else {
             r.addAction(new ParametersAction(p));
         }

diff --git a/core/src/main/java/hudson/matrix/DefaultMatrixExecutionStrategyImpl.java b/core/src/main/java/hudson/matrix/DefaultMatrixExecutionStrategyImpl.java
@@ -3,7 +3,6 @@
 import groovy.lang.GroovyRuntimeException;
 import hudson.AbortException;
 import hudson.Extension;
-import hudson.Util;
 import hudson.console.ModelHyperlinkNote;
 import hudson.matrix.MatrixBuild.MatrixBuildExecution;
 import hudson.matrix.listeners.MatrixBuildListener;
@@ -238,7 +237,7 @@ private void scheduleConfigurationBuild(MatrixBuildExecution exec, MatrixConfigu
         exec.getListener().getLogger().println(Messages.MatrixBuild_Triggering(ModelHyperlinkNote.encodeTo(c)));
 
         // filter the parent actions for those that can be passed to the individual jobs.
-        List<MatrixChildAction> childActions = Util.filter(build.getActions(), MatrixChildAction.class);
+        List<MatrixChildAction> childActions = build.getActions(MatrixChildAction.class);
         c.scheduleBuild(childActions, new UpstreamCause((Run)build));
     }
 

diff --git a/core/src/main/java/hudson/model/AbstractBuild.java b/core/src/main/java/hudson/model/AbstractBuild.java
@@ -30,7 +30,6 @@
 import hudson.FilePath;
 import hudson.Functions;
 import hudson.Launcher;
-import hudson.Util;
 import hudson.console.AnnotatedLargeText;
 import hudson.console.ExpandableDetailsNote;
 import hudson.console.ModelHyperlinkNote;
@@ -922,7 +921,7 @@ public EnvVars getEnvironment(TaskListener log) throws IOException, InterruptedE
             for (Environment e : buildEnvironments)
                 e.buildEnvVars(env);
 
-        for (EnvironmentContributingAction a : Util.filter(getActions(),EnvironmentContributingAction.class))
+        for (EnvironmentContributingAction a : getActions(EnvironmentContributingAction.class))
             a.buildEnvVars(this,env);
 
         EnvVars.resolve(env);
@@ -956,6 +955,7 @@ public Calendar due() {
         return getTimestamp();
     }
 
+    @SuppressWarnings("deprecation")
     public List<Action> getPersistentActions(){
         return super.getActions();
     }

diff --git a/core/src/main/java/hudson/model/AbstractProject.java b/core/src/main/java/hudson/model/AbstractProject.java
@@ -777,13 +777,7 @@ public void addProperty(JobProperty<? super P> jobProp) throws IOException {
     }
 
     public List<ProminentProjectAction> getProminentActions() {
-        List<Action> a = getActions();
-        List<ProminentProjectAction> pa = new Vector<ProminentProjectAction>();
-        for (Action action : a) {
-            if(action instanceof ProminentProjectAction)
-                pa.add((ProminentProjectAction) action);
-        }
-        return pa;
+        return getActions(ProminentProjectAction.class);
     }
 
     @Override
@@ -1175,6 +1169,7 @@ protected R loadBuild(File dir) throws IOException {
      *
      * @see TransientProjectActionFactory
      */
+    @SuppressWarnings("deprecation")
     @Override
     public List<Action> getActions() {
         // add all the transient actions, too

diff --git a/core/src/main/java/hudson/model/Actionable.java b/core/src/main/java/hudson/model/Actionable.java
@@ -23,27 +23,20 @@
  */
 package hudson.model;
 
-import hudson.Functions;
-import hudson.model.queue.Tasks;
+import hudson.Util;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
+import jenkins.model.Jenkins;
 import jenkins.model.ModelObjectWithContextMenu;
-import org.apache.commons.jelly.JellyContext;
-import org.apache.commons.jelly.JellyException;
-import org.apache.commons.jelly.JellyTagException;
-import org.apache.commons.jelly.Script;
-import org.apache.commons.jelly.XMLOutput;
+import jenkins.model.TransientActionFactory;
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
-import org.kohsuke.stapler.WebApp;
 import org.kohsuke.stapler.export.Exported;
 import org.kohsuke.stapler.export.ExportedBean;
-import org.kohsuke.stapler.jelly.JellyClassTearOff;
-import org.kohsuke.stapler.jelly.JellyFacet;
-import org.xml.sax.helpers.DefaultHandler;
-
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Vector;
-import java.util.concurrent.CopyOnWriteArrayList;
 
 /**
  * {@link ModelObject} that can have additional {@link Action}s.
@@ -60,15 +53,23 @@ public abstract class Actionable extends AbstractModelObject implements ModelObj
     private volatile CopyOnWriteArrayList<Action> actions;
 
     /**
-     * Gets actions contributed to this build.
+     * Gets actions contributed to this object.
      *
      * <p>
-     * A new {@link Action} can be added by {@code getActions().add(...)}.
+     * A new {@link Action} can be added by {@link #addAction}.
+     *
+     * <p>If you are <em>reading</em> the list, rather than <em>modifying</em> it,
+     * use {@link #getAllActions} instead.
+     * This method by default returns only <em>persistent</em> actions
+     * (though some subclasses override it to return an extended unmodifiable list).
      *
      * @return
      *      may be empty but never null.
+     * @deprecated Normally outside code should not call this method any more.
+     *             Use {@link #getAllActions}, or {@link #addAction}, or {@link #replaceAction}.
+     *             May still be called for compatibility reasons from subclasses predating {@link TransientActionFactory}.
      */
-	@Exported
+    @Deprecated
 	public List<Action> getActions() {
 		if(actions == null) {
 			synchronized (this) {
@@ -81,31 +82,67 @@ public List<Action> getActions() {
 	}
 
     /**
-     * Gets all actions of a specified type that contributed to this build.
+     * Gets all actions, transient or persistent.
+     * {@link #getActions} is supplemented with anything contributed by {@link TransientActionFactory}.
+     * @return an unmodifiable, possible empty list
+     * @since 1.548
+     */
+    @Exported(name="actions")
+    public final List<? extends Action> getAllActions() {
+        List<Action> _actions = new ArrayList<Action>(getActions());
+        for (TransientActionFactory<?> taf : Jenkins.getInstance().getExtensionList(TransientActionFactory.class)) {
+            if (taf.type().isInstance(this)) {
+                _actions.addAll(createFor(taf));
+            }
+        }
+        return Collections.unmodifiableList(_actions);
+    }
+    private <T extends Actionable> Collection<? extends Action> createFor(TransientActionFactory<T> taf) {
+        return taf.createFor(taf.type().cast(this));
+    }
+
+    /**
+     * Gets all actions of a specified type that contributed to this object.
      *
      * @param type The type of action to return.
      * @return
      *      may be empty but never null.
      * @see #getAction(Class)
      */
     public <T extends Action> List<T> getActions(Class<T> type) {
-        List<T> result = new Vector<T>();
-        for (Action a : getActions())
-            if (type.isInstance(a))
-                result.add(type.cast(a));
-        return result;
+        return Util.filter(getAllActions(), type);
     }
 
     /**
      * Adds a new action.
      *
-     * Short for <tt>getActions().add(a)</tt>
+     * The default implementation calls {@code getActions().add(a)}.
      */
     public void addAction(Action a) {
         if(a==null) throw new IllegalArgumentException();
         getActions().add(a);
     }
 
+    /**
+     * Add an action, replacing any existing action of the (exact) same class.
+     * @param a an action to add/replace
+     * @since 1.548
+     */
+    public void replaceAction(Action a) {
+        // CopyOnWriteArrayList does not support Iterator.remove, so need to do it this way:
+        List<Action> old = new ArrayList<Action>(1);
+        List<Action> current = getActions();
+        for (Action a2 : current) {
+            if (a2.getClass() == a.getClass()) {
+                old.add(a2);
+            }
+        }
+        current.removeAll(old);
+        addAction(a);
+    }
+
+    /** @deprecated No clear purpose, since subclasses may have overridden {@link #getActions}, and does not consider {@link TransientActionFactory}. */
+    @Deprecated
     public Action getAction(int index) {
         if(actions==null)   return null;
         return actions.get(index);
@@ -119,14 +156,14 @@ public Action getAction(int index) {
      * @see #getActions(Class)
      */
     public <T extends Action> T getAction(Class<T> type) {
-        for (Action a : getActions())
+        for (Action a : getAllActions())
             if (type.isInstance(a))
                 return type.cast(a);
         return null;
     }
 
     public Object getDynamic(String token, StaplerRequest req, StaplerResponse rsp) {
-        for (Action a : getActions()) {
+        for (Action a : getAllActions()) {
             if(a==null)
                 continue;   // be defensive
             String urlName = a.getUrlName();

diff --git a/core/src/main/java/hudson/model/CauseAction.java b/core/src/main/java/hudson/model/CauseAction.java
@@ -144,7 +144,7 @@ public void foldIntoExisting(hudson.model.Queue.Item item, Task owner, List<Acti
             return;
         }
         // no CauseAction found, so add a copy of this one
-        item.getActions().add(new CauseAction(this));
+        item.addAction(new CauseAction(this));
     }
 
     public static class ConverterImpl extends XStream2.PassthruConverter<CauseAction> {

diff --git a/core/src/main/java/hudson/model/Computer.java b/core/src/main/java/hudson/model/Computer.java
@@ -180,6 +180,7 @@ public List<ComputerPanelBox> getComputerPanelBoxs(){
     /**
      * Returns the transient {@link Action}s associated with the computer.
      */
+    @SuppressWarnings("deprecation")
     public List<Action> getActions() {
     	List<Action> result = new ArrayList<Action>();
     	result.addAll(super.getActions());
@@ -189,9 +190,10 @@ public List<Action> getActions() {
     		}
     		result.addAll(transientActions);
     	}
-    	return result;
+    	return Collections.unmodifiableList(result);
     }
 
+    @SuppressWarnings("deprecation")
     @Override
     public void addAction(Action a) {
         if(a==null) throw new IllegalArgumentException();

diff --git a/core/src/main/java/hudson/model/Job.java b/core/src/main/java/hudson/model/Job.java
@@ -975,11 +975,8 @@ public long getEstimatedDuration() {
     public PermalinkList getPermalinks() {
         // TODO: shall we cache this?
         PermalinkList permalinks = new PermalinkList(Permalink.BUILTIN);
-        for (Action a : getActions()) {
-            if (a instanceof PermalinkProjectAction) {
-                PermalinkProjectAction ppa = (PermalinkProjectAction) a;
-                permalinks.addAll(ppa.getPermalinks());
-            }
+        for (PermalinkProjectAction ppa : getActions(PermalinkProjectAction.class)) {
+            permalinks.addAll(ppa.getPermalinks());
         }
         return permalinks;
     }

diff --git a/core/src/main/java/hudson/model/Queue.java b/core/src/main/java/hudson/model/Queue.java
@@ -539,7 +539,7 @@ public synchronized WaitingItem schedule(Task p, int quietPeriod, List<Action> a
                 shouldScheduleItem |= action.shouldSchedule(actions);
     		}
     		for (QueueAction action: Util.filter(actions,QueueAction.class)) {
-                shouldScheduleItem |= action.shouldSchedule(item.getActions());
+                shouldScheduleItem |= action.shouldSchedule((new ArrayList<Action>(item.getAllActions())));
     		}
     		if(!shouldScheduleItem) {
     			duplicatesInQueue.add(item);
@@ -1417,7 +1417,7 @@ protected Item(Task task, List<Action> actions, int id, FutureImpl future, long
         }
 
         protected Item(Item item) {
-        	this(item.task, item.getActions(), item.id, item.future, item.inQueueSince);
+        	this(item.task, new ArrayList<Action>(item.getAllActions()), item.id, item.future, item.inQueueSince);
         }
 
         /**
@@ -1453,13 +1453,10 @@ public final String getWhy() {
         @Exported
         public String getParams() {
         	StringBuilder s = new StringBuilder();
-        	for(Action action : getActions()) {
-        		if(action instanceof ParametersAction) {
-        			ParametersAction pa = (ParametersAction)action;
-        			for (ParameterValue p : pa.getParameters()) {
-        				s.append('\n').append(p.getShortDescription());
-        			}
-        		}
+        	for (ParametersAction pa : getActions(ParametersAction.class)) {
+                for (ParameterValue p : pa.getParameters()) {
+                    s.append('\n').append(p.getShortDescription());
+                }
         	}
         	return s.toString();
         }

diff --git a/core/src/main/java/hudson/model/Run.java b/core/src/main/java/hudson/model/Run.java
@@ -332,7 +332,7 @@ public void reload() throws IOException {
      */
     @SuppressWarnings("deprecation")
     protected void onLoad() {
-        for (Action a : getActions()) {
+        for (Action a : getAllActions()) {
             if (a instanceof RunAction2) {
                 ((RunAction2) a).onLoad(this);
             } else if (a instanceof RunAction) {
@@ -348,7 +348,9 @@ protected void onLoad() {
      * Return all transient actions associated with this build.
      * 
      * @return the list can be empty but never null. read only.
+     * @deprecated Use {@link #getAllActions} instead.
      */
+    @Deprecated
     public List<Action> getTransientActions() {
         List<Action> actions = new ArrayList<Action>();
         for (TransientBuildActionFactory factory: TransientBuildActionFactory.all()) {
@@ -443,21 +445,11 @@ public void setResult(Result r) {
      * Gets the subset of {@link #getActions()} that consists of {@link BuildBadgeAction}s.
      */
     public List<BuildBadgeAction> getBadgeActions() {
-        List<BuildBadgeAction> r = null;
-        for (Action a : getActions()) {
-            if(a instanceof BuildBadgeAction) {
-                if(r==null)
-                    r = new ArrayList<BuildBadgeAction>();
-                r.add((BuildBadgeAction)a);
-            }
-        }
+        List<BuildBadgeAction> r = getActions(BuildBadgeAction.class);
         if(isKeepLog()) {
-            if(r==null)
-                r = new ArrayList<BuildBadgeAction>();
             r.add(new KeepLogBuildBadge());
         }
-        if(r==null)     return Collections.emptyList();
-        else            return r;
+        return r;
     }
 
     /**
@@ -1389,7 +1381,7 @@ protected SearchIndexBuilder makeSearchIndex() {
         SearchIndexBuilder builder = super.makeSearchIndex()
                 .add("console")
                 .add("changes");
-        for (Action a : getActions()) {
+        for (Action a : getAllActions()) {
             if(a.getIconFileName()!=null)
                 builder.add(a.getUrlName());
         }

diff --git a/core/src/main/java/hudson/model/TransientBuildActionFactory.java b/core/src/main/java/hudson/model/TransientBuildActionFactory.java
@@ -6,6 +6,7 @@
 import jenkins.model.Jenkins;
 import java.util.Collection;
 import java.util.Collections;
+import jenkins.model.TransientActionFactory;
 
 /**
  * Extension point for inserting transient {@link Action}s into {@link Run}s.
@@ -15,8 +16,9 @@
  * @author Lucie Votypkova
  * @since 1.458
  * @see Action
+ * @deprecated Does not contribute to {@link Run#getActions}. Use {@link TransientActionFactory} instead.
  */
-
+@Deprecated
 public abstract class TransientBuildActionFactory implements ExtensionPoint {
     /**
      * Creates actions for the given build.