Skip to content

Commit

Permalink
[FIXED JENKINS-18407]
Browse files Browse the repository at this point in the history
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.)
  • Loading branch information
kohsuke committed Jun 25, 2013
1 parent 6ab4cd6 commit f719bd0
Show file tree
Hide file tree
Showing 6 changed files with 221 additions and 38 deletions.
7 changes: 3 additions & 4 deletions core/src/main/java/hudson/matrix/MatrixConfiguration.java
Expand Up @@ -393,17 +393,16 @@ public boolean scheduleBuild(ParametersAction parameters) {
* @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>();
Expand All @@ -414,7 +413,7 @@ public boolean scheduleBuild(List<? extends Action> actions, Cause c) {
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();
}

/**
Expand Down
Expand Up @@ -41,7 +41,7 @@
* <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>
Expand Down
14 changes: 7 additions & 7 deletions core/src/main/java/hudson/model/AbstractProject.java
Expand Up @@ -48,8 +48,8 @@
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;
Expand Down Expand Up @@ -918,9 +918,9 @@ public QueueTaskFuture<R> scheduleBuild2(int quietPeriod, Cause c, Collection<?
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;
}

Expand Down Expand Up @@ -1812,9 +1812,9 @@ public void doBuild( StaplerRequest req, StaplerResponse rsp, @QueryParameter Ti
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(".");
}
Expand Down
44 changes: 35 additions & 9 deletions core/src/main/java/hudson/model/Queue.java
Expand Up @@ -46,6 +46,8 @@
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;
Expand Down Expand Up @@ -473,6 +475,14 @@ public boolean add(AbstractProject p, int quietPeriod) {
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.
*
Expand All @@ -483,14 +493,18 @@ public boolean add(AbstractProject p, int quietPeriod) {
* 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();) {
Expand All @@ -500,7 +514,7 @@ public synchronized WaitingItem schedule(Task p, int quietPeriod, List<Action> a

for(QueueDecisionHandler h : QueueDecisionHandler.all())
if (!h.shouldSchedule(p, actions))
return null; // veto
return ScheduleResult.refused(); // veto

return scheduleInternal(p, quietPeriod, actions);
}
Expand All @@ -517,7 +531,7 @@ public synchronized WaitingItem schedule(Task p, int quietPeriod, List<Action> a
*
* 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);

Expand All @@ -542,7 +556,7 @@ private synchronized WaitingItem scheduleInternal(Task p, int quietPeriod, List<
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);
Expand Down Expand Up @@ -576,7 +590,12 @@ private synchronized WaitingItem scheduleInternal(Task p, int quietPeriod, List<
}

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));
}


Expand Down Expand Up @@ -604,7 +623,14 @@ public synchronized boolean add(Task p, int quietPeriod, Action... actions) {
* 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));
}

/**
Expand Down
111 changes: 111 additions & 0 deletions core/src/main/java/hudson/model/queue/ScheduleResult.java
@@ -0,0 +1,111 @@
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();
}
}

0 comments on commit f719bd0

Please sign in to comment.