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

Register OptionHandlers through META-INF/services/annotations and Annotation Indexer rather than META-INF/services and Commons Discovery #9980

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
29 changes: 12 additions & 17 deletions core/src/main/java/hudson/cli/CLICommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,6 @@
this.stdout = stdout;
this.stderr = stderr;
this.locale = locale;
registerOptionHandlers();
CmdLineParser p = getCmdLineParser();

// add options from the authenticator
Expand Down Expand Up @@ -527,20 +526,6 @@
}
}

/**
* Auto-discovers {@link OptionHandler}s and add them to the given command line parser.
*/
protected void registerOptionHandlers() {
try {
for (Class c : Index.list(OptionHandlerExtension.class, Jenkins.get().pluginManager.uberClassLoader, Class.class)) {
Type t = Types.getBaseClass(c, OptionHandler.class);
CmdLineParser.registerHandler(Types.erasure(Types.getTypeArgument(t, 0)), c);
}
} catch (IOException e) {
throw new Error(e);
}
}

/**
* Returns all the registered {@link CLICommand}s.
*/
Expand Down Expand Up @@ -577,11 +562,21 @@

static {
// register option handlers that are defined
ClassLoaders cls = new ClassLoaders();
Jenkins j = Jenkins.getInstanceOrNull();
if (j != null) { // only when running on the controller
cls.put(j.getPluginManager().uberClassLoader);
// Register OptionHandlers through META-INF/services/annotations and Annotation Indexer
try {
for (Class c : Index.list(OptionHandlerExtension.class, Jenkins.get().pluginManager.uberClassLoader, Class.class)) {
Type t = Types.getBaseClass(c, OptionHandler.class);
CmdLineParser.registerHandler(Types.erasure(Types.getTypeArgument(t, 0)), c);
}
Comment on lines +569 to +572
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved from lines 535-537, so please do not review this as original code authored by me.

} catch (IOException e) {
throw new UncheckedIOException(e);

Check warning on line 574 in core/src/main/java/hudson/cli/CLICommand.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 573-574 are not covered by tests
}

// Register OptionHandlers through META-INF/services and Commons Discovery
ClassLoaders cls = new ClassLoaders();
cls.put(j.getPluginManager().uberClassLoader);
ResourceNameIterator servicesIter =
new DiscoverServiceNames(cls).findResourceNames(OptionHandler.class.getName());
final ResourceClassIterator itr =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ protected CmdLineParser getCmdLineParser() {

private CmdLineParser bindMethod(List<MethodBinder> binders) {

registerOptionHandlers();
ParserProperties properties = ParserProperties.defaults().withAtSyntax(ALLOW_AT_SYNTAX);
CmdLineParser parser = new CmdLineParser(null, properties);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@

/**
* {@link OptionHandler}s that should be auto-discovered.
* TODO is this actually necessary? {@code @MetaInfServices(OptionHandler.class)} seems to work as well.
Copy link
Member Author

Choose a reason for hiding this comment

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

Answer: it is actually necessary if we want to remove Commons Discovery someday, as ServiceLoader cannot work with the OptionHandler classes that have no public zero-argument constructor.

* @author Kohsuke Kawaguchi
*/
@Indexed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,18 @@

package hudson.cli.handlers;

import hudson.cli.declarative.OptionHandlerExtension;
import hudson.model.AbstractItem;
import org.kohsuke.MetaInfServices;
import org.kohsuke.args4j.CmdLineParser;
import org.kohsuke.args4j.OptionDef;
import org.kohsuke.args4j.spi.OptionHandler;
import org.kohsuke.args4j.spi.Setter;

/**
* Refers to an {@link AbstractItem} by name.
* @since 1.538
*/
@MetaInfServices(OptionHandler.class) public class AbstractItemOptionHandler extends GenericItemOptionHandler<AbstractItem> {
@OptionHandlerExtension
public class AbstractItemOptionHandler extends GenericItemOptionHandler<AbstractItem> {

public AbstractItemOptionHandler(CmdLineParser parser, OptionDef option, Setter<AbstractItem> setter) {
super(parser, option, setter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,18 @@

package hudson.cli.handlers;

import hudson.cli.declarative.OptionHandlerExtension;
import hudson.model.AbstractProject;
import org.kohsuke.MetaInfServices;
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 AbstractProject} by its name.
*
* @author Kohsuke Kawaguchi
*/
@MetaInfServices(OptionHandler.class)
@OptionHandlerExtension
@SuppressWarnings("rawtypes")
public class AbstractProjectOptionHandler extends GenericItemOptionHandler<AbstractProject> {
public AbstractProjectOptionHandler(CmdLineParser parser, OptionDef option, Setter<AbstractProject> setter) {
Expand Down
5 changes: 2 additions & 3 deletions core/src/main/java/hudson/cli/handlers/JobOptionHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,18 @@

package hudson.cli.handlers;

import hudson.cli.declarative.OptionHandlerExtension;
import hudson.model.Job;
import org.kohsuke.MetaInfServices;
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 Job} by its name.
*
* @author Kohsuke Kawaguchi
*/
@MetaInfServices(OptionHandler.class)
@OptionHandlerExtension
@SuppressWarnings("rawtypes")
public class JobOptionHandler extends GenericItemOptionHandler<Job> {
public JobOptionHandler(CmdLineParser parser, OptionDef option, Setter<Job> setter) {
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/hudson/cli/handlers/NodeOptionHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@

package hudson.cli.handlers;

import hudson.cli.declarative.OptionHandlerExtension;
import hudson.model.Node;
import jenkins.model.Jenkins;
import org.kohsuke.MetaInfServices;
import org.kohsuke.args4j.CmdLineException;
import org.kohsuke.args4j.CmdLineParser;
import org.kohsuke.args4j.OptionDef;
Expand All @@ -40,7 +40,7 @@
* @author ogondza
* @since 1.526
*/
@MetaInfServices
@OptionHandlerExtension
public class NodeOptionHandler extends OptionHandler<Node> {

public NodeOptionHandler(CmdLineParser parser, OptionDef option, Setter<Node> setter) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,19 @@

package hudson.cli.handlers;

import hudson.cli.declarative.OptionHandlerExtension;
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)
@OptionHandlerExtension
@SuppressWarnings("rawtypes")
public class ParameterizedJobOptionHandler extends GenericItemOptionHandler<ParameterizedJobMixIn.ParameterizedJob> {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
package hudson.cli.handlers;

import hudson.cli.declarative.OptionHandlerExtension;
import hudson.model.TopLevelItem;
import org.kohsuke.MetaInfServices;
import org.kohsuke.args4j.CmdLineParser;
import org.kohsuke.args4j.OptionDef;
import org.kohsuke.args4j.spi.OptionHandler;
import org.kohsuke.args4j.spi.Setter;

/**
* Refers to {@link TopLevelItem} by its name.
*
* @author Kohsuke Kawaguchi
*/
@MetaInfServices(OptionHandler.class)
@OptionHandlerExtension
public class TopLevelItemOptionHandler extends GenericItemOptionHandler<TopLevelItem> {
public TopLevelItemOptionHandler(CmdLineParser parser, OptionDef option, Setter<TopLevelItem> setter) {
super(parser, option, setter);
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/hudson/cli/handlers/ViewOptionHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@
package hudson.cli.handlers;

import edu.umd.cs.findbugs.annotations.CheckForNull;
import hudson.cli.declarative.OptionHandlerExtension;
import hudson.model.View;
import hudson.model.ViewGroup;
import java.util.StringTokenizer;
import jenkins.model.Jenkins;
import org.kohsuke.MetaInfServices;
import org.kohsuke.args4j.CmdLineException;
import org.kohsuke.args4j.CmdLineParser;
import org.kohsuke.args4j.OptionDef;
Expand Down Expand Up @@ -58,7 +58,7 @@
* @author ogondza
* @since 1.538
*/
@MetaInfServices
@OptionHandlerExtension
public class ViewOptionHandler extends OptionHandler<View> {

public ViewOptionHandler(CmdLineParser parser, OptionDef option, Setter<View> setter) {
Expand Down
Loading