[FIXED JENKINS-18407]

Added Queue.schedule2 to allow the caller to retrieve the existing item in the queue. AbstractProject.doBuild() changed the behavior a bit to reply 201 if the item was already found in the queue (instead of a new one created.)

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

@@ -393,17 +393,16 @@ public class MatrixConfiguration extends Project<MatrixConfiguration,MatrixRun>
      * @param parameters
      *      Can be null.
      * @deprecated
-	 *    Use {@link #scheduleBuild(List<? extends Action>, Cause)}.  Since 1.480
+	 *    Use {@link #scheduleBuild(List, Cause)}.  Since 1.480
      */
     public boolean scheduleBuild(ParametersAction parameters, Cause c) {
-
         return scheduleBuild(Collections.singletonList(parameters), c);
     }
     /**
      * Starts the build with the actions that are passed in.
      *
      * @param actions   Can be null.
-     * @param cause     Reason for starting the build
+     * @param c     Reason for starting the build
      */
     public boolean scheduleBuild(List<? extends Action> actions, Cause c) {
         List<Action> allActions = new ArrayList<Action>();
@@ -414,7 +413,7 @@ public class MatrixConfiguration extends Project<MatrixConfiguration,MatrixRun>
         allActions.add(new ParentBuildAction());
         allActions.add(new CauseAction(c));
 
-        return Jenkins.getInstance().getQueue().schedule(this, getQuietPeriod(), allActions )!=null;
+        return Jenkins.getInstance().getQueue().schedule2(this, getQuietPeriod(), allActions ).isAccepted();
     }
 
     /**

core/src/main/java/hudson/matrix/listeners/MatrixBuildListener.java

@@ -41,7 +41,7 @@ import java.util.List;
  * <p>
  * Plugins can implement this extension point to filter out the subset of matrix project to build.
  * Most typically, such a plugin would add a custom {@link Action} to a build when it goes to the queue
- * ({@link Queue#schedule(Task, int, List)}, then access this from {@link MatrixBuild} to drive
+ * ({@link Queue#schedule2(Task, int, List)}, then access this from {@link MatrixBuild} to drive
  * the filtering logic.
  *
  * <p>

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

@@ -48,8 +48,8 @@ import hudson.model.PermalinkProjectAction.Permalink;
 import hudson.model.Queue.Executable;
 import hudson.model.Queue.Task;
 import hudson.model.queue.QueueTaskFuture;
+import hudson.model.queue.ScheduleResult;
 import hudson.model.queue.SubTask;
-import hudson.model.Queue.WaitingItem;
 import hudson.model.RunMap.Constructor;
 import hudson.model.labels.LabelAtom;
 import hudson.model.labels.LabelExpression;
@@ -918,9 +918,9 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
             queueActions.add(new CauseAction(c));
         }
 
-        WaitingItem i = Jenkins.getInstance().getQueue().schedule(this, quietPeriod, queueActions);
-        if(i!=null)
-            return (QueueTaskFuture)i.getFuture();
+        ScheduleResult i = Jenkins.getInstance().getQueue().schedule2(this, quietPeriod, queueActions);
+        if(i.isAccepted())
+            return (QueueTaskFuture)i.getItem().getFuture();
         return null;
     }
 
@@ -1812,9 +1812,9 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
         if (!isBuildable())
             throw HttpResponses.error(SC_INTERNAL_SERVER_ERROR,new IOException(getFullName()+" is not buildable"));
 
-        WaitingItem item = Jenkins.getInstance().getQueue().schedule(this, (int) delay.getTime(), getBuildCause(req));
-        if (item!=null) {
-            rsp.sendRedirect(SC_CREATED,req.getContextPath()+'/'+item.getUrl());
+        ScheduleResult r = Jenkins.getInstance().getQueue().schedule2(this, delay.getTime(), getBuildCause(req));
+        if (r.isAccepted()) {
+            rsp.sendRedirect(SC_CREATED,req.getContextPath()+'/'+r.getItem().getUrl());
         } else
             rsp.sendRedirect(".");
     }

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

@@ -46,6 +46,8 @@ import hudson.model.queue.AbstractQueueTask;
 import hudson.model.queue.Executables;
 import hudson.model.queue.QueueListener;
 import hudson.model.queue.QueueTaskFuture;
+import hudson.model.queue.ScheduleResult;
+import hudson.model.queue.ScheduleResult.Created;
 import hudson.model.queue.SubTask;
 import hudson.model.queue.FutureImpl;
 import hudson.model.queue.MappingWorksheet;
@@ -473,6 +475,14 @@ public class Queue extends ResourceController implements Saveable {
         return schedule(p, quietPeriod)!=null;
     }
 
+    /**
+     * @deprecated as of 1.521
+     *  Use {@link #schedule2(Task, int, List)}
+     */
+    public synchronized WaitingItem schedule(Task p, int quietPeriod, List<Action> actions) {
+        return schedule2(p, quietPeriod, actions).getCreateItem();
+    }
+
     /**
      * Schedules an execution of a task.
      *
@@ -483,14 +493,18 @@ public class Queue extends ResourceController implements Saveable {
      *      For the convenience of the caller, this list can contain null, and those will be silently ignored.
      * @since 1.311
      * @return
-     *      null if this task is already in the queue and therefore the add operation was no-op.
-     *      Otherwise indicates the {@link WaitingItem} object added, although the nature of the queue
+     *      {@link ScheduleResult.Refused} if Jenkins refused to add this task into the queue (for example because the system
+     *      is about to shutdown.) Otherwise the task is either merged into existing items in the queue
+     *      (in which case you get {@link ScheduleResult.Existing} instance back), or a new item
+     *      gets created in the queue (in which case you get {@link Created}.
+     *
+     *      Note the nature of the queue
      *      is that such {@link Item} only captures the state of the item at a particular moment,
      *      and by the time you inspect the object, some of its information can be already stale.
      *
-     *      That said, one can still look at {@link WaitingItem#future}, {@link WaitingItem#id}, etc.
+     *      That said, one can still look at {@link Item#future}, {@link Item#id}, etc.
      */
-    public synchronized WaitingItem schedule(Task p, int quietPeriod, List<Action> actions) {
+    public synchronized @Nonnull ScheduleResult schedule2(Task p, int quietPeriod, List<Action> actions) {
         // remove nulls
         actions = new ArrayList<Action>(actions);
         for (Iterator<Action> itr = actions.iterator(); itr.hasNext();) {
@@ -500,7 +514,7 @@ public class Queue extends ResourceController implements Saveable {
 
     	for(QueueDecisionHandler h : QueueDecisionHandler.all())
     		if (!h.shouldSchedule(p, actions))
-                return null;    // veto
+                return ScheduleResult.refused();    // veto
 
         return scheduleInternal(p, quietPeriod, actions);
     }
@@ -517,7 +531,7 @@ public class Queue extends ResourceController implements Saveable {
      *
      *      That said, one can still look at {@link WaitingItem#future}, {@link WaitingItem#id}, etc.
      */
-    private synchronized WaitingItem scheduleInternal(Task p, int quietPeriod, List<Action> actions) {
+    private synchronized @Nonnull ScheduleResult scheduleInternal(Task p, int quietPeriod, List<Action> actions) {
         Calendar due = new GregorianCalendar();
     	due.add(Calendar.SECOND, quietPeriod);
 
@@ -542,7 +556,7 @@ public class Queue extends ResourceController implements Saveable {
             WaitingItem added = new WaitingItem(due,p,actions);
             added.enter(this);
             scheduleMaintenance();   // let an executor know that a new item is in the queue.
-            return added;
+            return ScheduleResult.created(added);
     	}
 
         LOGGER.log(Level.FINE, "{0} is already in the queue", p);
@@ -576,7 +590,12 @@ public class Queue extends ResourceController implements Saveable {
         }
 
         if (queueUpdated)   scheduleMaintenance();
-        return null;
+
+        // REVISIT: when there are multiple existing items in the queue that matches the incoming one,
+        // whether the new one should affect all existing ones or not is debateable. I for myself
+        // thought this would only affect one, so the code was bit of surprise, but I'm keeping the current
+        // behaviour.
+        return ScheduleResult.existing(duplicatesInQueue.get(0));
     }
 
 
@@ -604,7 +623,14 @@ public class Queue extends ResourceController implements Saveable {
      * Convenience wrapper method around {@link #schedule(Task, int, List)}
      */
     public synchronized WaitingItem schedule(Task p, int quietPeriod, Action... actions) {
-    	return schedule(p, quietPeriod, Arrays.asList(actions));
+    	return schedule2(p, quietPeriod, actions).getCreateItem();
+    }
+
+    /**
+     * Convenience wrapper method around {@link #schedule2(Task, int, List)}
+     */
+    public synchronized @Nonnull ScheduleResult schedule2(Task p, int quietPeriod, Action... actions) {
+    	return schedule2(p, quietPeriod, Arrays.asList(actions));
     }
 
     /**

core/src/main/java/hudson/model/queue/ScheduleResult.java

+package hudson.model.queue;
+
+import hudson.model.Action;
+import hudson.model.Queue;
+import hudson.model.Queue.Item;
+import hudson.model.Queue.Task;
+import hudson.model.Queue.WaitingItem;
+
+/**
+ * Result of {@link Queue#schedule2}
+ *
+ * @author Kohsuke Kawaguchi
+ * @since 1.521
+ * @see Queue#schedule(Task, int, Action...)
+ */
+public abstract class ScheduleResult {
+
+    /**
+     * If true, the {@link #getItem()} is newly created
+     * as a result of {@link Queue#schedule2}.
+     */
+    public boolean isCreated() {
+        return false;
+    }
+
+    /**
+     * The scheduling of the task was refused and the queue didn't change.
+     * If this method returns true, {@link #getItem()} will return null.
+     */
+    public boolean isRefused() {
+        return false;
+    }
+
+    /**
+     * Unless {@link #isRefused()} is true, this method either returns
+     * the newly created item in the queue or the existing item that's already
+     * in the queue that matched the submitted task.
+     */
+    public Item getItem() {
+        return null;
+    }
+
+    /**
+     * If {@link #isCreated()} returns true, then this method returns
+     * the newly created item, which is always of the type {@link WaitingItem}.
+     */
+    public WaitingItem getCreateItem() {
+        return null;
+    }
+
+    /**
+     * Opposite of {@link #isRefused()}
+     */
+    public final boolean isAccepted() {
+        return !isRefused();
+    }
+
+    public static final class Created extends ScheduleResult {
+        private final WaitingItem item;
+        private Created(WaitingItem item) {
+            this.item = item;
+        }
+
+        @Override
+        public boolean isCreated() {
+            return true;
+        }
+
+        @Override
+        public WaitingItem getCreateItem() {
+            return item;
+        }
+
+        @Override
+        public Item getItem() {
+            return item;
+        }
+    }
+
+    public static final class Existing extends ScheduleResult {
+        private final Item item;
+
+        private Existing(Item item) {
+            this.item = item;
+        }
+
+        @Override
+        public Item getItem() {
+            return item;
+        }
+    }
+
+    public static final class Refused extends ScheduleResult {
+        @Override
+        public boolean isRefused() {
+            return true;
+        }
+    }
+
+    public static Created created(WaitingItem i) {
+        return new Created(i);
+    }
+
+    public static Existing existing(Item i) {
+        return new Existing(i);
+    }
+
+    public static Refused refused() {
+        return new Refused();
+    }
+}

test/src/test/groovy/hudson/model/AbstractProjectTest.groovy

@@ -23,7 +23,8 @@
  */
 package hudson.model;
 
-import com.gargoylesoftware.htmlunit.ElementNotFoundException;
+import com.gargoylesoftware.htmlunit.ElementNotFoundException
+import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
 import com.gargoylesoftware.htmlunit.HttpMethod;
 import com.gargoylesoftware.htmlunit.WebRequestSettings;
 import com.gargoylesoftware.htmlunit.html.HtmlForm;
@@ -69,7 +70,7 @@ import org.jvnet.hudson.test.MockFolder;
  * @author Kohsuke Kawaguchi
  */
 public class AbstractProjectTest extends HudsonTestCase {
-    public void testConfigRoundtrip() throws Exception {
+    public void testConfigRoundtrip() {
         def project = createFreeStyleProject();
         def l = jenkins.getLabel("foo && bar");
         project.assignedLabel = l;
@@ -81,7 +82,7 @@ public class AbstractProjectTest extends HudsonTestCase {
     /**
      * Tests the workspace deletion.
      */
-    public void testWipeWorkspace() throws Exception {
+    public void testWipeWorkspace() {
         def project = createFreeStyleProject();
         project.buildersList.add(new Shell("echo hello"));
 
@@ -98,7 +99,7 @@ public class AbstractProjectTest extends HudsonTestCase {
      * Makes sure that the workspace deletion is protected.
      */
     @PresetData(DataSet.NO_ANONYMOUS_READACCESS)
-    public void testWipeWorkspaceProtected() throws Exception {
+    public void testWipeWorkspaceProtected() {
         def project = createFreeStyleProject();
         project.getBuildersList().add(new Shell("echo hello"));
 
@@ -115,7 +116,7 @@ public class AbstractProjectTest extends HudsonTestCase {
      * when the user doesn't have an access.
      */
     @PresetData(DataSet.ANONYMOUS_READONLY)
-    public void testWipeWorkspaceProtected2() throws Exception {
+    public void testWipeWorkspaceProtected2() {
         ((GlobalMatrixAuthorizationStrategy) jenkins.getAuthorizationStrategy()).add(AbstractProject.WORKSPACE,"anonymous");
 
         // make sure that the deletion is protected in the same way
@@ -138,7 +139,7 @@ public class AbstractProjectTest extends HudsonTestCase {
     /**
      * Tests the <optionalBlock @field> round trip behavior by using {@link AbstractProject#concurrentBuild}
      */
-    public void testOptionalBlockDataBindingRoundtrip() throws Exception {
+    public void testOptionalBlockDataBindingRoundtrip() {
         def p = createFreeStyleProject();
         [true,false].each { b ->
             p.concurrentBuild = b;
@@ -151,7 +152,7 @@ public class AbstractProjectTest extends HudsonTestCase {
      * Tests round trip configuration of the blockBuildWhenUpstreamBuilding field
      */
     @Bug(4423)
-    public void testConfiguringBlockBuildWhenUpstreamBuildingRoundtrip() throws Exception {
+    public void testConfiguringBlockBuildWhenUpstreamBuildingRoundtrip() {
         def p = createFreeStyleProject();
         p.blockBuildWhenUpstreamBuilding = false;
 
@@ -173,7 +174,7 @@ public class AbstractProjectTest extends HudsonTestCase {
      * to avoid allocating unnecessary workspaces.
      */
     @Bug(4202)
-    public void testPollingAndBuildExclusion() throws Exception {
+    public void testPollingAndBuildExclusion() {
         final OneShotEvent sync = new OneShotEvent();
 
         final FreeStyleProject p = createFreeStyleProject();
@@ -224,7 +225,7 @@ public class AbstractProjectTest extends HudsonTestCase {
     }
 
     @Bug(1986)
-    public void testBuildSymlinks() throws Exception {
+    public void testBuildSymlinks() {
         // If we're on Windows, don't bother doing this.
         if (Functions.isWindows())
             return;
@@ -261,7 +262,7 @@ public class AbstractProjectTest extends HudsonTestCase {
     }
 
     @Bug(2543)
-    public void testSymlinkForPostBuildFailure() throws Exception {
+    public void testSymlinkForPostBuildFailure() {
         // If we're on Windows, don't bother doing this.
         if (Functions.isWindows())
             return;
@@ -286,7 +287,7 @@ public class AbstractProjectTest extends HudsonTestCase {
     }
 
     @Bug(15156)
-    public void testGetBuildAfterGC() throws Exception {
+    public void testGetBuildAfterGC() {
         FreeStyleProject job = createFreeStyleProject();
         job.scheduleBuild2(0, new Cause.UserIdCause()).get();
         jenkins.queue.clearLeftItems();
@@ -295,7 +296,7 @@ public class AbstractProjectTest extends HudsonTestCase {
     }
 
     @Bug(13502)
-    public void testHandleBuildTrigger() throws Exception {
+    public void testHandleBuildTrigger() {
         Project u = createFreeStyleProject("u"),
                 d = createFreeStyleProject("d"),
                 e = createFreeStyleProject("e");
@@ -330,7 +331,7 @@ public class AbstractProjectTest extends HudsonTestCase {
     }
 
     @Bug(17137)
-    public void testExternalBuildDirectorySymlinks() throws Exception {
+    public void testExternalBuildDirectorySymlinks() {
         // TODO when using JUnit 4 add: Assume.assumeFalse(Functions.isWindows()); // symlinks may not be available
         def form = createWebClient().goTo("configure").getFormByName("config");
         def builds = createTmpDir();
@@ -361,7 +362,7 @@ public class AbstractProjectTest extends HudsonTestCase {
     }
 
     @Bug(17138)
-    public void testExternalBuildDirectoryRenameDelete() throws Exception {
+    public void testExternalBuildDirectoryRenameDelete() {
         def form = createWebClient().goTo("configure").getFormByName("config");
         def builds = createTmpDir();
         form.getInputByName("_.rawBuildsDir").setValueAttribute(builds.toString() + "/\${ITEM_FULL_NAME}");
@@ -381,7 +382,7 @@ public class AbstractProjectTest extends HudsonTestCase {
     }
 
     @Bug(17575)
-    public void testDeleteRedirect() throws Exception {
+    public void testDeleteRedirect() {
         createFreeStyleProject("j1");
         assert "" == deleteRedirectTarget("job/j1");
         createFreeStyleProject("j2");
@@ -394,12 +395,58 @@ public class AbstractProjectTest extends HudsonTestCase {
         assert "job/d/view/v2/" == deleteRedirectTarget("job/d/view/v2/job/j4");
         assert "view/v1/job/d/" == deleteRedirectTarget("view/v1/job/d/job/j5");
     }
-    private String deleteRedirectTarget(String job) throws Exception {
-        WebClient wc = createWebClient();
+
+    private String deleteRedirectTarget(String job) {
+        def wc = createWebClient();
         String base = wc.getContextPath();
         String loc = wc.getPage(wc.addCrumb(new WebRequestSettings(new URL(base + job + "/doDelete"), HttpMethod.POST))).getWebResponse().getUrl().toString();
         assert loc.startsWith(base): loc;
         return loc.substring(base.length());
     }
 
+    @Bug(18407)
+    public void testQueueSuccessBehavior() {
+        // prevent any builds to test the behaviour
+        jenkins.numExecutors = 0;
+        jenkins.updateComputerList(false);
+
+        def p = createFreeStyleProject()
+        def f = p.scheduleBuild2(0)
+        assert f!=null;
+        def g = p.scheduleBuild2(0)
+        assert f==g;
+
+        p.makeDisabled(true)
+        assert p.scheduleBuild2(0)==null
+    }
+
+    /**
+     * Do the same as {@link #testQueueSuccessBehavior()} but over HTTP
+     */
+    @Bug(18407)
+    public void testQueueSuccessBehaviorOverHTTP() {
+        // prevent any builds to test the behaviour
+        jenkins.numExecutors = 0;
+        jenkins.updateComputerList(false);
+
+        def p = createFreeStyleProject()
+        def wc = createWebClient();
+
+        def rsp = wc.getPage("${getURL()}${p.url}build").webResponse
+        assert rsp.statusCode==201;
+        assert rsp.getResponseHeaderValue("Location")!=null;
+
+        def rsp2 = wc.getPage("${getURL()}${p.url}build").webResponse
+        assert rsp2.statusCode==201;
+        assert rsp.getResponseHeaderValue("Location")==rsp2.getResponseHeaderValue("Location")
+
+        p.makeDisabled(true)
+
+        try {
+            wc.getPage("${getURL()}${p.url}build")
+            fail();
+        } catch (FailingHttpStatusCodeException e) {
+            // request should fail
+        }
+    }
 }