Merge pull request #3462 from jtnord/JENKINS-51584

[JENKINS-51584] Actions from a TransientActionFactory should not be persisted (cherry picked from commit d4737b2)
jenkinsci · Jun 7, 2018 · 4f57641 · 4f57641
1 parent f031194
commit 4f57641
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 5 deletions.
diff --git a/core/src/main/java/hudson/model/Queue.java b/core/src/main/java/hudson/model/Queue.java
@@ -2196,8 +2196,10 @@ protected Item(Task task, List<Action> actions, long id, FutureImpl future, long
             for (Action action: actions) addAction(action);
         }
 
+        @SuppressWarnings("deprecation") // JENKINS-51584
         protected Item(Item item) {
-        	this(item.task, new ArrayList<Action>(item.getAllActions()), item.id, item.future, item.inQueueSince);
+            // do not use item.getAllActions() here as this will persist actions from a TransientActionFactory
+            this(item.task, new ArrayList<Action>(item.getActions()), item.id, item.future, item.inQueueSince);
         }
 
         /**

diff --git a/core/src/main/java/hudson/model/queue/WorkUnitContext.java b/core/src/main/java/hudson/model/queue/WorkUnitContext.java
@@ -67,8 +67,8 @@ public WorkUnitContext(BuildableItem item) {
         this.item = item;
         this.task = item.task;
         this.future = (FutureImpl)item.getFuture();
-        this.actions = new ArrayList<Action>(item.getAllActions());
-
+        // JENKINS-51584 do not use item.getAllActions() here.
+        this.actions = new ArrayList<Action>(item.getActions());
         // +1 for the main task
         int workUnitSize = task.getSubTasks().size();
         startLatch = new Latch(workUnitSize) {

diff --git a/test/src/test/java/jenkins/model/TransientActionFactoryTest.java b/test/src/test/java/jenkins/model/TransientActionFactoryTest.java
@@ -28,21 +28,34 @@
 import hudson.model.AbstractItem;
 import hudson.model.AbstractProject;
 import hudson.model.Action;
+import hudson.model.Actionable;
+import hudson.model.FreeStyleBuild;
 import hudson.model.FreeStyleProject;
 import hudson.model.InvisibleAction;
 import hudson.model.ProminentProjectAction;
 import hudson.model.queue.FoldableAction;
+
+import java.io.PrintWriter;
+import java.io.StringWriter;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import org.hamcrest.Matchers;
-import static org.junit.Assert.*;
+
 import org.junit.Rule;
 import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.MockFolder;
 import org.jvnet.hudson.test.TestExtension;
 
+import javax.annotation.Nonnull;
+
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.*;
+
 public class TransientActionFactoryTest {
 
     @Rule public JenkinsRule r = new JenkinsRule();
@@ -147,6 +160,45 @@ private static class MyAction implements Action {
         }
     }
 
-    private static class MyProminentProjectAction extends InvisibleAction implements ProminentProjectAction {}
+    @Issue("JENKINS-51584")
+    @Test
+    public void transientActionsAreNotPersistedOnQueueItems() throws Exception {
+        FreeStyleProject p = r.createFreeStyleProject();
+        FreeStyleBuild build = r.buildAndAssertSuccess(p);
+        // MyProminentProjectAction is only added via the TransientActionFactory and should never be persisted.
+        assertThat(Util.filter(build.getActions(), MyProminentProjectAction.class), is(empty()));
+        assertThat(Util.filter(build.getAllActions(), MyProminentProjectAction.class), hasSize(1));
+    }
+
+    @TestExtension("transientActionsAreNotPersistedOnQueueItems")
+    public static class AllFactory extends TransientActionFactory<Actionable> {
+
+        @Override
+        public Class<Actionable> type() {
+            return Actionable.class;
+        }
+
+        @Nonnull
+        @Override
+        public Collection<? extends Action> createFor(@Nonnull Actionable target) {
+            return Collections.singleton(new MyProminentProjectAction());
+        }
+    }
+
+    private static class MyProminentProjectAction extends InvisibleAction implements ProminentProjectAction {
+
+        private String allocation;
+
+        public MyProminentProjectAction() {
+            StringWriter sw = new StringWriter();
+            PrintWriter pw = new PrintWriter(sw);
+            new Exception("MyProminentProjectAction allocated at: ").printStackTrace(pw);
+            allocation = sw.toString();
+        }
+
+        public String toString() {
+            return allocation;
+        }
+    }
 
 }