Skip to content

Commit

Permalink
[JENKINS-27299] Define disabled in ParameterizedJob rather than Abstr…
Browse files Browse the repository at this point in the history
…actProject (#2866)

* Offering default methods on ParameterizedJob.

* Javadoc typo.

* Cleaner use of default methods in ParameterizedJob.

* Need to pick up jenkinsci/bridge-method-injector#15 to be able to build.

* Sketch of pulling disabled functionality into ParameterizedJob.

* EnableJobCommandTest.groovy → EnableJobCommandTest.java, and replacing deprecated Remoting-based CLI calls with CLICommandInvoker.

* All CLI commands could be broken by a missing CLI.*.shortDescription key on just one!

* Forgot to move CLI method short descriptions to new package.

* Needed a @CLIResolver for ParameterizedJob. Adding an OptionHandler while we are here.

* Trying to fix up access-modifier versions; started failing in CI today for unknown reasons.

* Introduced <p:makeDisabled/> by analogy with <p:config-disableBuild/>.

* Using new type bounds.

* access-modifier 1.11 released.

* MatrixProject and MavenModuleSet both expect to have access to makeDisabled.jelly.

* Trying to generalize some more.

* Minor simplification.

* [JENKINS-34716] Generalizing doPolling and schedulePolling.

* isBuildable

* Obsolete comment.

* Updated comments.

* bridge-method-injector 1.17

* Unfortunately AbstractProject.schedulePolling cannot delegate to SCMTriggerItem.

* bridge-method-injector 1.17
  • Loading branch information
jglick authored and oleg-nenashev committed May 14, 2017
1 parent 37dfa99 commit 3af0cc6
Show file tree
Hide file tree
Showing 72 changed files with 365 additions and 191 deletions.
1 change: 0 additions & 1 deletion cli/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
<dependency>
<groupId>org.kohsuke</groupId>
<artifactId>access-modifier-annotation</artifactId>
<version>1.7</version>
</dependency>
<dependency>
<groupId>commons-codec</groupId>
Expand Down
2 changes: 2 additions & 0 deletions core/move-l10n.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ for (p in new File(resDir, oldview).parentFile.listFiles()) {
def n = p.name;
if (n == "${basename}.properties" || n.startsWith("${basename}_") && n.endsWith(".properties")) {
def lines = p.readLines('ISO-8859-1');
// TODO does not handle multiline values correctly
def matches = lines.findAll({it.startsWith("${key}=")});
if (!matches.isEmpty()) {
lines.removeAll(matches);
Expand All @@ -24,6 +25,7 @@ for (p in new File(resDir, oldview).parentFile.listFiles()) {
} else {
def nue = new File(resDir, newview + n.substring(basename.length()));
println("moving ${matches.size()} matches from ${n} to ${nue.name}");
// TODO if the original lacked a trailing newline, this will corrupt previously final key
nue.withWriterAppend('ISO-8859-1') {out ->
matches.each {line -> out.writeLine(line)}
}
Expand Down
2 changes: 0 additions & 2 deletions core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,6 @@ THE SOFTWARE.
<dependency>
<groupId>org.kohsuke</groupId>
<artifactId>access-modifier-annotation</artifactId>
<version>1.4</version>
</dependency>

<dependency>
Expand Down Expand Up @@ -704,7 +703,6 @@ THE SOFTWARE.
<plugin>
<groupId>org.kohsuke</groupId>
<artifactId>access-modifier-checker</artifactId>
<!-- version specified in grandparent pom -->
<executions>
<execution>
<goals>
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/hudson/cli/BuildCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ protected int run() throws Exception {

if (!job.isBuildable()) {
String msg = Messages.BuildCommand_CLICause_CannotBuildUnknownReasons(job.getFullDisplayName());
if (job instanceof AbstractProject<?, ?> && ((AbstractProject<?, ?>)job).isDisabled()) {
if (job instanceof ParameterizedJobMixIn.ParameterizedJob && ((ParameterizedJobMixIn.ParameterizedJob) job).isDisabled()) {
msg = Messages.BuildCommand_CLICause_CannotBuildDisabled(job.getFullDisplayName());
} else if (job.isHoldOffBuildUntilSave()){
msg = Messages.BuildCommand_CLICause_CannotBuildConfigNotSaved(job.getFullDisplayName());
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/java/hudson/cli/declarative/CLIRegisterer.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.MissingResourceException;
import java.util.Stack;
import static java.util.logging.Level.SEVERE;

Expand Down Expand Up @@ -279,7 +280,7 @@ protected int run() throws Exception {
throw new UnsupportedOperationException();
}
}));
} catch (ClassNotFoundException e) {
} catch (ClassNotFoundException | MissingResourceException e) {
LOGGER.log(SEVERE,"Failed to process @CLIMethod: "+m,e);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* The MIT License
*
* Copyright 2017 CloudBees, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

package hudson.cli.handlers;

import jenkins.model.ParameterizedJobMixIn;
import org.kohsuke.MetaInfServices;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
import org.kohsuke.args4j.CmdLineParser;
import org.kohsuke.args4j.OptionDef;
import org.kohsuke.args4j.spi.OptionHandler;
import org.kohsuke.args4j.spi.Setter;

/**
* Refer to {@link jenkins.model.ParameterizedJobMixIn.ParameterizedJob} by its name.
*/
@Restricted(DoNotUse.class)
@MetaInfServices(OptionHandler.class)
@SuppressWarnings("rawtypes")
public class ParameterizedJobOptionHandler extends GenericItemOptionHandler<ParameterizedJobMixIn.ParameterizedJob> {

public ParameterizedJobOptionHandler(CmdLineParser parser, OptionDef option, Setter<ParameterizedJobMixIn.ParameterizedJob> setter) {
super(parser, option, setter);
}

@Override
protected Class<ParameterizedJobMixIn.ParameterizedJob> type() {
return ParameterizedJobMixIn.ParameterizedJob.class;
}

@Override
public String getDefaultMetaVariable() {
return "JOB";
}

}
2 changes: 1 addition & 1 deletion core/src/main/java/hudson/model/AbstractItem.java
Original file line number Diff line number Diff line change
Expand Up @@ -854,7 +854,7 @@ public static AbstractItem resolveForCLI(
// TODO can this (and its pseudo-override in AbstractProject) share code with GenericItemOptionHandler, used for explicit CLICommand’s rather than CLIMethod’s?
AbstractItem item = Jenkins.getInstance().getItemByFullName(name, AbstractItem.class);
if (item==null) {
AbstractProject project = AbstractProject.findNearest(name);
AbstractProject project = AbstractProject.findNearest(name); // TODO should be Items.findNearest
throw new CmdLineException(null, project == null ? Messages.AbstractItem_NoSuchJobExistsWithoutSuggestion(name)
: Messages.AbstractItem_NoSuchJobExists(name, project.getFullName()));
}
Expand Down
59 changes: 16 additions & 43 deletions core/src/main/java/hudson/model/AbstractProject.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import hudson.Functions;
import hudson.Launcher;
import hudson.Util;
import hudson.cli.declarative.CLIMethod;
import hudson.cli.declarative.CLIResolver;
import hudson.model.Cause.LegacyCodeCause;
import hudson.model.Descriptor.FormException;
Expand All @@ -48,7 +47,6 @@
import hudson.model.Queue.Task;
import hudson.model.labels.LabelAtom;
import hudson.model.labels.LabelExpression;
import hudson.model.listeners.ItemListener;
import hudson.model.listeners.SCMPollListener;
import hudson.model.queue.CauseOfBlockage;
import hudson.model.queue.QueueTaskFuture;
Expand Down Expand Up @@ -110,6 +108,7 @@
import jenkins.scm.SCMCheckoutStrategy;
import jenkins.scm.SCMCheckoutStrategyDescriptor;
import jenkins.scm.SCMDecisionHandler;
import jenkins.triggers.SCMTriggerItem;
import jenkins.util.TimeDuration;
import net.sf.json.JSONObject;
import org.acegisecurity.Authentication;
Expand Down Expand Up @@ -640,7 +639,7 @@ public boolean hasCustomScmCheckoutRetryCount(){

@Override
public boolean isBuildable() {
return !isDisabled() && !isHoldOffBuildUntilSave();
return ParameterizedJobMixIn.ParameterizedJob.super.isBuildable();
}

/**
Expand Down Expand Up @@ -669,10 +668,17 @@ public void setBlockBuildWhenUpstreamBuilding(boolean b) throws IOException {
save();
}

@Override
public boolean isDisabled() {
return disabled;
}

@Restricted(DoNotUse.class)
@Override
public void setDisabled(boolean disabled) {
this.disabled = disabled;
}

/**
* Validates the retry count Regex
*/
Expand All @@ -687,38 +693,22 @@ public FormValidation doCheckRetryCount(@QueryParameter String value)throws IOEx
}

/**
* Marks the build as disabled.
* The method will ignore the disable command if {@link #supportsMakeDisabled()}
* returns false. The enable command will be executed in any case.
* @param b true - disable, false - enable
* @since 1.585 Do not disable projects if {@link #supportsMakeDisabled()} returns false
*/
public void makeDisabled(boolean b) throws IOException {
if(disabled==b) return; // noop
if (b && !supportsMakeDisabled()) return; // do nothing if the disabling is unsupported
this.disabled = b;
if(b)
Jenkins.getInstance().getQueue().cancel(this);

save();
ItemListener.fireOnUpdated(this);
}

/**
* Specifies whether this project may be disabled by the user.
* {@inheritDoc}
* By default, it can be only if this is a {@link TopLevelItem};
* would be false for matrix configurations, etc.
* @return true if the GUI should allow {@link #doDisable} and the like
* @since 1.475
*/
@Override
public boolean supportsMakeDisabled() {
return this instanceof TopLevelItem;
}

// Seems to be used only by tests; do not bother pulling up.
public void disable() throws IOException {
makeDisabled(true);
}

// Ditto.
public void enable() throws IOException {
makeDisabled(false);
}
Expand Down Expand Up @@ -866,6 +856,7 @@ public QueueTaskFuture<R> scheduleBuild2(int quietPeriod, Action... actions) {

/**
* Schedules a polling of this project.
* @see SCMTriggerItem#schedulePolling
*/
public boolean schedulePolling() {
if(isDisabled()) return false;
Expand Down Expand Up @@ -1173,6 +1164,7 @@ public List<SubTask> getSubTasks() {
return r;
}

@Override // same as ParameterizedJob version except calls possibly overridden newBuild
public @CheckForNull R createExecutable() throws IOException {
if(isDisabled()) return null;
return newBuild();
Expand Down Expand Up @@ -1742,9 +1734,7 @@ public void doBuildWithParameters(StaplerRequest req, StaplerResponse rsp) throw
doBuildWithParameters(req, rsp, TimeDuration.fromString(req.getParameter("delay")));
}

/**
* Schedules a new SCM polling command.
*/
@Override // in case schedulePolling was overridden
public void doPolling( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException {
BuildAuthorizationToken.checkPermission((Job) this, authToken, req, rsp);
schedulePolling();
Expand Down Expand Up @@ -1888,23 +1878,6 @@ public HttpResponse doDoWipeOutWorkspace() throws IOException, ServletException,
}
}

@CLIMethod(name="disable-job")
@RequirePOST
public HttpResponse doDisable() throws IOException, ServletException {
checkPermission(CONFIGURE);
makeDisabled(true);
return new HttpRedirect(".");
}

@CLIMethod(name="enable-job")
@RequirePOST
public HttpResponse doEnable() throws IOException, ServletException {
checkPermission(CONFIGURE);
makeDisabled(false);
return new HttpRedirect(".");
}


/**
* {@link AbstractProject} subtypes should implement this base class as a descriptor.
*
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/java/hudson/model/ListView.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import javax.annotation.concurrent.GuardedBy;
import javax.servlet.ServletException;
import jenkins.model.Jenkins;
import jenkins.model.ParameterizedJobMixIn;

import net.sf.json.JSONObject;
import org.jenkinsci.Symbol;
Expand Down Expand Up @@ -197,8 +198,8 @@ public List<TopLevelItem> getItems() {
for (TopLevelItem item : candidates) {
if (!names.contains(item.getRelativeNameFrom(getOwnerItemGroup()))) continue;
// Add if no status filter or filter matches enabled/disabled status:
if(statusFilter == null || !(item instanceof AbstractProject)
|| ((AbstractProject)item).isDisabled() ^ statusFilter)
if(statusFilter == null || !(item instanceof ParameterizedJobMixIn.ParameterizedJob) // TODO or better to call the more generic Job.isBuildable?
|| ((ParameterizedJobMixIn.ParameterizedJob)item).isDisabled() ^ statusFilter)
items.add(item);
}

Expand Down
Loading

0 comments on commit 3af0cc6

Please sign in to comment.