diff --git a/core/src/main/java/hudson/model/Actionable.java b/core/src/main/java/hudson/model/Actionable.java index 68c31067b3aa2e1f53d888454ff343f57e119575..15fa61f38c8c71da5437c4d523705dd2e676c1c6 100644 --- a/core/src/main/java/hudson/model/Actionable.java +++ b/core/src/main/java/hudson/model/Actionable.java @@ -75,16 +75,26 @@ public abstract class Actionable extends AbstractModelObject implements ModelObj * May still be called for compatibility reasons from subclasses predating {@link TransientActionFactory}. */ @Deprecated - public List getActions() { - if(actions == null) { - synchronized (this) { - if(actions == null) { - actions = new CopyOnWriteArrayList(); - } - } - } - return actions; - } + public List getActions() { + return getOrCreateActions(); + } + + /** + * We need to handle the initialization of the actions list in Actionable so that child classes that override + * getActions() for historical reasons do not have to override the manipulation methods: {@link #addAction(Action)}, + * {@link #replaceAction(Action)}, {@link #removeAction(Action)}, etc. + * @return the CopyOnWriteArrayList of persisted actions. + */ + private CopyOnWriteArrayList getOrCreateActions() { + if(actions == null) { + synchronized (this) { + if(actions == null) { + actions = new CopyOnWriteArrayList(); + } + } + } + return actions; + } /** * Gets all actions, transient or persistent. @@ -123,9 +133,10 @@ public abstract class Actionable extends AbstractModelObject implements ModelObj } /** - * Adds a new action. + * Adds a new action. Note calls to {@link #getAllActions()} that happen before calls to this method may not see the + * update. * - * The default implementation calls {@code getActions().add(a)}. + * The default implementation is mostly equivalent to the call chain {@code getActions().add(a)}. */ @SuppressWarnings({"ConstantConditions","deprecation"}) @SuppressFBWarnings("RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE") @@ -133,11 +144,12 @@ public abstract class Actionable extends AbstractModelObject implements ModelObj if(a==null) { throw new IllegalArgumentException("Action must be non-null"); } - getActions().add(a); + getOrCreateActions().add(a); } /** - * Add an action, replacing any existing action of the (exact) same class. + * Add an action, replacing any existing action of the (exact) same class. Note calls to {@link #getAllActions()} + * that happen before calls to this method may not see the update. * @param a an action to add/replace * @since 1.548 */ @@ -149,7 +161,7 @@ public abstract class Actionable extends AbstractModelObject implements ModelObj } // CopyOnWriteArrayList does not support Iterator.remove, so need to do it this way: List old = new ArrayList(1); - List current = getActions(); + List current = getOrCreateActions(); boolean found = false; for (Action a2 : current) { if (!found && a.equals(a2)) { @@ -165,7 +177,8 @@ public abstract class Actionable extends AbstractModelObject implements ModelObj } /** - * Remove an action. + * Remove an action. Note calls to {@link #getAllActions()} that happen before calls to this method may not see the + * update. * * @param a an action to remove (if {@code null} then this will be a no-op) * @return {@code true} if this actions changed as a result of the call @@ -177,11 +190,12 @@ public abstract class Actionable extends AbstractModelObject implements ModelObj return false; } // CopyOnWriteArrayList does not support Iterator.remove, so need to do it this way: - return getActions().removeAll(Collections.singleton(a)); + return getOrCreateActions().removeAll(Collections.singleton(a)); } /** - * Removes any actions of the specified type. + * Removes any actions of the specified type. Note calls to {@link #getAllActions()} that happen before calls to + * this method may not see the update. * * @param clazz the type of actions to remove * @return {@code true} if this actions changed as a result of the call @@ -195,7 +209,7 @@ public abstract class Actionable extends AbstractModelObject implements ModelObj } // CopyOnWriteArrayList does not support Iterator.remove, so need to do it this way: List old = new ArrayList(); - List current = getActions(); + List current = getOrCreateActions(); for (Action a : current) { if (clazz.isInstance(a)) { old.add(a); @@ -205,7 +219,8 @@ public abstract class Actionable extends AbstractModelObject implements ModelObj } /** - * Replaces any actions of the specified type by the supplied action. + * Replaces any actions of the specified type by the supplied action. Note calls to {@link #getAllActions()} that + * happen before calls to this method may not see the update. * * @param clazz the type of actions to replace * @param a the action to replace with @@ -220,7 +235,7 @@ public abstract class Actionable extends AbstractModelObject implements ModelObj } // CopyOnWriteArrayList does not support Iterator.remove, so need to do it this way: List old = new ArrayList(); - List current = getActions(); + List current = getOrCreateActions(); boolean found = false; for (Action a1 : current) { if (!found && a.equals(a1)) { diff --git a/core/src/main/java/hudson/model/Computer.java b/core/src/main/java/hudson/model/Computer.java index 05d569c00555af76e89a284a2cd8b3d3c3426511..f372a5413c066749e02be0274394386708498a19 100644 --- a/core/src/main/java/hudson/model/Computer.java +++ b/core/src/main/java/hudson/model/Computer.java @@ -268,13 +268,6 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces return Collections.unmodifiableList(result); } - @SuppressWarnings("deprecation") - @Override - public void addAction(Action a) { - if(a==null) throw new IllegalArgumentException(); - super.getActions().add(a); - } - /** * This is where the log from the remote agent goes. * The method also creates a log directory if required. diff --git a/core/src/main/java/hudson/model/Run.java b/core/src/main/java/hudson/model/Run.java index 74aa283656549aed2f4dc8aa4937c4fbfbfa19c8..43c5e414c8c00e12f98188603f05aad5343d956c 100644 --- a/core/src/main/java/hudson/model/Run.java +++ b/core/src/main/java/hudson/model/Run.java @@ -349,7 +349,7 @@ public abstract class Run ,RunT extends Run