Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Offering default methods on ParameterizedJob #2864

Merged
merged 7 commits into from
May 13, 2017
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 8 additions & 72 deletions core/src/main/java/hudson/model/AbstractProject.java
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@
* @see AbstractBuild
*/
@SuppressWarnings("rawtypes")
public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends AbstractBuild<P,R>> extends Job<P,R> implements BuildableItem, LazyBuildMixIn.LazyLoadingJob<P,R>, ParameterizedJobMixIn.ParameterizedJob {
public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends AbstractBuild<P,R>> extends Job<P,R> implements BuildableItem, LazyBuildMixIn.LazyLoadingJob<P,R>, ParameterizedJobMixIn.ParameterizedJob<P, R> {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

implements BuildableItem is now redundant, could be removed I guess.

I have checked all usages of "instanceof BuildableItem".

The only implementations of ParameterizedJob were already BuildableItems so retrofitting it to implement that interface should not cause any behavioral change even if someone were doing that instanceof check.


/**
* {@link SCM} associated with the project.
Expand Down Expand Up @@ -292,15 +292,6 @@ private LazyBuildMixIn<P,R> createBuildMixIn() {
return buildMixIn;
}

private ParameterizedJobMixIn<P,R> getParameterizedJobMixIn() {
return new ParameterizedJobMixIn<P,R>() {
@SuppressWarnings("unchecked") // untypable
@Override protected P asJob() {
return (P) AbstractProject.this;
}
};
}

@Override
public synchronized void save() throws IOException {
super.save();
Expand Down Expand Up @@ -473,7 +464,7 @@ public String getPronoun() {
*/
public String getBuildNowText() {
// For compatibility, still use the deprecated replacer if specified.
return AlternativeUiTextProvider.get(BUILD_NOW_TEXT, this, getParameterizedJobMixIn().getBuildNowText());
return AlternativeUiTextProvider.get(BUILD_NOW_TEXT, this, ParameterizedJobMixIn.ParameterizedJob.super.getBuildNowText());
}

/**
Expand Down Expand Up @@ -805,39 +796,6 @@ public void doConfigSubmit( StaplerRequest req, StaplerResponse rsp ) throws IOE
Jenkins.getInstance().rebuildDependencyGraphAsync();
}

/**
* @deprecated
* Use {@link #scheduleBuild(Cause)}. Since 1.283
*/
@Deprecated
public boolean scheduleBuild() {
return getParameterizedJobMixIn().scheduleBuild();
}

/**
* @deprecated
* Use {@link #scheduleBuild(int, Cause)}. Since 1.283
*/
@Deprecated
public boolean scheduleBuild(int quietPeriod) {
return getParameterizedJobMixIn().scheduleBuild(quietPeriod);
}

/**
* Schedules a build of this project.
*
* @return
* true if the project is added to the queue.
* false if the task was rejected from the queue (such as when the system is being shut down.)
*/
public boolean scheduleBuild(Cause c) {
return getParameterizedJobMixIn().scheduleBuild(c);
}

public boolean scheduleBuild(int quietPeriod, Cause c) {
return getParameterizedJobMixIn().scheduleBuild(quietPeriod, c);
}

/**
* Schedules a build.
*
Expand Down Expand Up @@ -874,14 +832,13 @@ public QueueTaskFuture<R> scheduleBuild2(int quietPeriod, Cause c, Action... act
* For the convenience of the caller, this collection can contain null, and those will be silently ignored.
* @since 1.383
*/
@SuppressWarnings("unchecked")
@WithBridgeMethods(Future.class)
public QueueTaskFuture<R> scheduleBuild2(int quietPeriod, Cause c, Collection<? extends Action> actions) {
List<Action> queueActions = new ArrayList<Action>(actions);
if (c != null) {
queueActions.add(new CauseAction(c));
}
return getParameterizedJobMixIn().scheduleBuild2(quietPeriod, queueActions.toArray(new Action[queueActions.size()]));
return scheduleBuild2(quietPeriod, queueActions.toArray(new Action[queueActions.size()]));
}

/**
Expand All @@ -907,6 +864,11 @@ public QueueTaskFuture<R> scheduleBuild2(int quietPeriod, Cause c) {
return scheduleBuild2(quietPeriod, c, new Action[0]);
}

@Override
public QueueTaskFuture<R> scheduleBuild2(int quietPeriod, Action... actions) {
return ParameterizedJobMixIn.ParameterizedJob.super.scheduleBuild2(quietPeriod, actions);
}

/**
* Schedules a polling of this project.
*/
Expand Down Expand Up @@ -1746,21 +1708,11 @@ protected HistoryWidget createHistoryWidget() {
return buildMixIn.createHistoryWidget();
}

public boolean isParameterized() {
return getParameterizedJobMixIn().isParameterized();
}

//
//
// actions
//
//
/**
* Schedules a new build command.
*/
public void doBuild( StaplerRequest req, StaplerResponse rsp, @QueryParameter TimeDuration delay ) throws IOException, ServletException {
getParameterizedJobMixIn().doBuild(req, rsp, delay);
}

/** @deprecated use {@link #doBuild(StaplerRequest, StaplerResponse, TimeDuration)} */
@Deprecated
Expand Down Expand Up @@ -1789,14 +1741,6 @@ public int getDelay(StaplerRequest req) throws ServletException {
}
}

/**
* Supports build trigger with parameters via an HTTP GET or POST.
* Currently only String parameters are supported.
*/
public void doBuildWithParameters(StaplerRequest req, StaplerResponse rsp, @QueryParameter TimeDuration delay) throws IOException, ServletException {
getParameterizedJobMixIn().doBuildWithParameters(req, rsp, delay);
}

/** @deprecated use {@link #doBuildWithParameters(StaplerRequest, StaplerResponse, TimeDuration)} */
@Deprecated
public void doBuildWithParameters(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
Expand All @@ -1812,14 +1756,6 @@ public void doPolling( StaplerRequest req, StaplerResponse rsp ) throws IOExcept
rsp.sendRedirect(".");
}

/**
* Cancels a scheduled build.
*/
@RequirePOST
public void doCancelQueue( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException {
getParameterizedJobMixIn().doCancelQueue(req, rsp);
}

@Override
protected void submit(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException, FormException {
super.submit(req,rsp);
Expand Down
5 changes: 0 additions & 5 deletions core/src/main/java/hudson/model/Project.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@

import hudson.Util;
import hudson.model.Descriptor.FormException;
import hudson.model.queue.QueueTaskFuture;
import hudson.scm.SCM;
import hudson.tasks.BuildStep;
import hudson.tasks.BuildWrapper;
Expand Down Expand Up @@ -109,10 +108,6 @@ public void onLoad(ItemGroup<? extends Item> parent, String name) throws IOExcep
return this;
}

@Override public QueueTaskFuture<?> scheduleBuild2(int quietPeriod, Action... actions) {
return scheduleBuild2(quietPeriod, null, actions);
}

@Override public SCMTrigger getSCMTrigger() {
return getTrigger(SCMTrigger.class);
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/hudson/triggers/Trigger.java
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ public void run(AbstractProject p) {
}

// Process all triggers, except SCMTriggers when synchronousPolling is set
for (ParameterizedJobMixIn.ParameterizedJob p : inst.allItems(ParameterizedJobMixIn.ParameterizedJob.class)) {
for (ParameterizedJobMixIn.ParameterizedJob<?, ?> p : inst.allItems(ParameterizedJobMixIn.ParameterizedJob.class)) {
for (Trigger t : p.getTriggers().values()) {
if (!(t instanceof SCMTrigger && scmd.synchronousPolling)) {
if (t !=null && t.spec != null && t.tabs != null) {
Expand Down
133 changes: 113 additions & 20 deletions core/src/main/java/jenkins/model/ParameterizedJobMixIn.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import javax.servlet.ServletException;
import static javax.servlet.http.HttpServletResponse.SC_CREATED;
import static javax.servlet.http.HttpServletResponse.SC_CONFLICT;
import jenkins.triggers.SCMTriggerItem;
import jenkins.util.TimeDuration;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
Expand All @@ -65,10 +66,11 @@
/**
* Allows a {@link Job} to make use of {@link ParametersDefinitionProperty} and be scheduled in various ways.
* Stateless so there is no need to keep an instance of it in a field.
* Besides implementing {@link ParameterizedJob}, you should override {@link Job#makeSearchIndex} to call {@link #extendSearchIndex}.
* @since 1.556
*/
@SuppressWarnings("unchecked") // AbstractItem.getParent does not correctly override; scheduleBuild2 inherently untypable
public abstract class ParameterizedJobMixIn<JobT extends Job<JobT, RunT> & ParameterizedJobMixIn.ParameterizedJob & Queue.Task, RunT extends Run<JobT, RunT> & Queue.Executable> {
public abstract class ParameterizedJobMixIn<JobT extends Job<JobT, RunT> & ParameterizedJobMixIn.ParameterizedJob<JobT, RunT> & Queue.Task, RunT extends Run<JobT, RunT> & Queue.Executable> {

protected abstract JobT asJob();

Expand All @@ -95,11 +97,7 @@ public final boolean scheduleBuild(int quietPeriod, Cause c) {
}

/**
* Provides a standard implementation of an optional method of the same name in a {@link Job} type to schedule a build with the ability to wait for its result.
* That job method is often used during functional tests ({@code JenkinsRule.assertBuildStatusSuccess}).
* @param quietPeriod seconds to wait before starting (normally 0)
* @param actions various actions to associate with the scheduling, such as {@link ParametersAction} or {@link CauseAction}
* @return a handle by which you may wait for the build to complete (or just start); or null if the build was not actually scheduled for some reason
* Standard implementation of {@link ParameterizedJob#scheduleBuild2}.
*/
public final @CheckForNull QueueTaskFuture<RunT> scheduleBuild2(int quietPeriod, Action... actions) {
Queue.Item i = scheduleBuild2(quietPeriod, Arrays.asList(actions));
Expand Down Expand Up @@ -161,15 +159,14 @@ private List<ParameterValue> getDefaultParametersValues() {
}

/**
* A job should define a method of the same signature for use from {@link BuildButtonColumn}.
* Standard implementation of {@link ParameterizedJob#isParameterized}.
*/
public final boolean isParameterized() {
return asJob().getProperty(ParametersDefinitionProperty.class) != null;
}

/**
* Schedules a new build command.
* Create a method on your job with the same signature and delegate to this.
* Standard implementation of {@link ParameterizedJob#doBuild}.
*/
@SuppressWarnings("deprecation")
public final void doBuild(StaplerRequest req, StaplerResponse rsp, @QueryParameter TimeDuration delay) throws IOException, ServletException {
Expand Down Expand Up @@ -206,9 +203,7 @@ public final void doBuild(StaplerRequest req, StaplerResponse rsp, @QueryParamet
}

/**
* Supports build trigger with parameters via an HTTP GET or POST.
* Currently only String parameters are supported.
* Create a method on your job with the same signature and delegate to this.
* Standard implementation of {@link ParameterizedJob#doBuildWithParameters}.
*/
@SuppressWarnings("deprecation")
public final void doBuildWithParameters(StaplerRequest req, StaplerResponse rsp, @QueryParameter TimeDuration delay) throws IOException, ServletException {
Expand All @@ -226,8 +221,7 @@ public final void doBuildWithParameters(StaplerRequest req, StaplerResponse rsp,
}

/**
* Cancels a scheduled build.
* Create a method on your job marked {@link RequirePOST} but with the same signature and delegate to this.
* Standard implementation of {@link ParameterizedJob#doCancelQueue}.
*/
@RequirePOST
public final void doCancelQueue( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException {
Expand Down Expand Up @@ -275,7 +269,6 @@ public static final CauseAction getBuildCause(ParameterizedJob job, StaplerReque

/**
* Suggested implementation of {@link ParameterizedJob#getBuildNowText}.
* Uses {@link #BUILD_NOW_TEXT}.
*/
public final String getBuildNowText() {
return isParameterized() ? AlternativeUiTextProvider.get(BUILD_NOW_TEXT, asJob(), Messages.ParameterizedJobMixIn_build_with_parameters())
Expand All @@ -294,7 +287,7 @@ public final String getBuildNowText() {
if (!(job instanceof ParameterizedJob)) {
return null;
}
for (Trigger<?> t : ((ParameterizedJob) job).getTriggers().values()) {
for (Trigger<?> t : ((ParameterizedJob<?, ?>) job).getTriggers().values()) {
if (clazz.isInstance(t)) {
return clazz.cast(t);
}
Expand All @@ -303,16 +296,42 @@ public final String getBuildNowText() {
}

/**
* Marker for job using this mixin.
* Marker for job using this mixin, and default implementations of many methods.
*/
public interface ParameterizedJob extends hudson.model.Queue.Task, hudson.model.Item {
public interface ParameterizedJob<JobT extends Job<JobT, RunT> & ParameterizedJobMixIn.ParameterizedJob<JobT, RunT> & Queue.Task, RunT extends Run<JobT, RunT> & Queue.Executable> extends BuildableItem {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To follow-up on my bug in #2874 (comment) . Effectively it makes any any Parameterized job explicitly buildable. After some consideration, it seems to be a valid change according to the ParameterizedJobMixIn documentation.

🐜 since I expect ParameterizedJob Javadoc to explicitly say it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well the mixin Javadoc does say

be scheduled in various ways

which I guess is enough. Anyway BuildableItem is an interface which is not used all that much and not especially interesting to people grokking this code.


/**
* Creates a helper object.
* (Would have been done entirely as an interface with default methods had this been designed for Java 8.)
*/
default ParameterizedJobMixIn<JobT, RunT> getParameterizedJobMixIn() {
return new ParameterizedJobMixIn<JobT, RunT>() {
@SuppressWarnings("unchecked") // untypable
@Override protected JobT asJob() {
return (JobT) ParameterizedJob.this;
}
};
}

@SuppressWarnings("deprecation")
@CheckForNull hudson.model.BuildAuthorizationToken getAuthToken();

int getQuietPeriod();
/**
* Quiet period for the job.
* @return by default, {@link Jenkins#getQuietPeriod}
*/
default int getQuietPeriod() {
return Jenkins.getInstance().getQuietPeriod();
}

String getBuildNowText();
/**
* Text to display for a build button.
* Uses {@link #BUILD_NOW_TEXT}.
* @see ParameterizedJobMixIn#getBuildNowText
*/
default String getBuildNowText() {
return getParameterizedJobMixIn().getBuildNowText();
}

/**
* Gets currently configured triggers.
Expand All @@ -322,6 +341,80 @@ public interface ParameterizedJob extends hudson.model.Queue.Task, hudson.model.
*/
Map<TriggerDescriptor,Trigger<?>> getTriggers();

/**
* @deprecated use {@link #scheduleBuild(Cause)}
*/
@Deprecated
@Override
default boolean scheduleBuild() {
return getParameterizedJobMixIn().scheduleBuild();
}

@Override
default boolean scheduleBuild(Cause c) {
return getParameterizedJobMixIn().scheduleBuild(c);
}

/**
* @deprecated use {@link #scheduleBuild(int, Cause)}
*/
@Deprecated
@Override
default boolean scheduleBuild(int quietPeriod) {
return getParameterizedJobMixIn().scheduleBuild(quietPeriod);
}

@Override
default boolean scheduleBuild(int quietPeriod, Cause c) {
return getParameterizedJobMixIn().scheduleBuild(quietPeriod, c);
}

/**
* Provides a standard implementation of {@link SCMTriggerItem#scheduleBuild2} to schedule a build with the ability to wait for its result.
* That job method is often used during functional tests ({@code JenkinsRule.assertBuildStatusSuccess}).
* @param quietPeriod seconds to wait before starting (normally 0)
* @param actions various actions to associate with the scheduling, such as {@link ParametersAction} or {@link CauseAction}
* @return a handle by which you may wait for the build to complete (or just start); or null if the build was not actually scheduled for some reason
*/
@CheckForNull
default QueueTaskFuture<RunT> scheduleBuild2(int quietPeriod, Action... actions) {
return getParameterizedJobMixIn().scheduleBuild2(quietPeriod, actions);
}

/**
* Schedules a new build command.
* @see ParameterizedJobMixIn#doBuild
*/
default void doBuild(StaplerRequest req, StaplerResponse rsp, @QueryParameter TimeDuration delay) throws IOException, ServletException {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, no Restricted/RequirePOST annotations here? I am not sure it is safe to offer such default impl. 🐛 , but probably I miss something.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original had neither annotation. @Restricted cannot be added compatibly. @RequirePOST is unwanted since the implementation checks the request method explicitly.

getParameterizedJobMixIn().doBuild(req, rsp, delay);
}

/**
* Supports build trigger with parameters via an HTTP GET or POST.
* Currently only String parameters are supported.
* @see ParameterizedJobMixIn#doBuildWithParameters
*/
default void doBuildWithParameters(StaplerRequest req, StaplerResponse rsp, @QueryParameter TimeDuration delay) throws IOException, ServletException {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

getParameterizedJobMixIn().doBuildWithParameters(req, rsp, delay);
}

/**
* Cancels a scheduled build.
* @see ParameterizedJobMixIn#doCancelQueue
*/
@RequirePOST
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default void doCancelQueue(StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException {
getParameterizedJobMixIn().doCancelQueue(req, rsp);
}

/**
* For use from {@link BuildButtonColumn}.
* @see ParameterizedJobMixIn#isParameterized
*/
default boolean isParameterized() {
return getParameterizedJobMixIn().isParameterized();
}

}

}
Loading