-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
add cli commons to factor out common parsing code #7301
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
plugins { | ||
id "java-library" | ||
} | ||
|
||
dependencies { | ||
implementation 'commons-cli:commons-cli:1.4' | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
This module houses utility functions for the `commons-cli` library. It is separate from `commons`, because it depends on external library `commons-cli` which we do not want to introduce as a dependency to every module. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
/* | ||
* Copyright (c) 2021 Airbyte, Inc., all rights reserved. | ||
*/ | ||
|
||
package io.airbyte.commons.cli; | ||
|
||
import java.util.ArrayList; | ||
import java.util.List; | ||
import org.apache.commons.cli.CommandLine; | ||
import org.apache.commons.cli.CommandLineParser; | ||
import org.apache.commons.cli.DefaultParser; | ||
import org.apache.commons.cli.HelpFormatter; | ||
import org.apache.commons.cli.Option; | ||
import org.apache.commons.cli.OptionGroup; | ||
import org.apache.commons.cli.Options; | ||
import org.apache.commons.cli.ParseException; | ||
|
||
public class Clis { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should probably have at least one test for each public method in this class |
||
|
||
/** | ||
* Parse an options object | ||
* | ||
* @param args - command line args | ||
* @param options - expected options | ||
* @return object with parsed values. | ||
*/ | ||
public static CommandLine parse(final String[] args, final Options options, final CommandLineParser parser, final String commandLineSyntax) { | ||
final HelpFormatter helpFormatter = new HelpFormatter(); | ||
|
||
try { | ||
return parser.parse(options, args); | ||
} catch (final ParseException e) { | ||
if (commandLineSyntax != null && !commandLineSyntax.isEmpty()) { | ||
helpFormatter.printHelp(commandLineSyntax, options); | ||
} | ||
throw new IllegalArgumentException(e); | ||
} | ||
} | ||
|
||
public static CommandLine parse(final String[] args, final Options options, final String commandLineSyntax) { | ||
return parse(args, options, new DefaultParser(), commandLineSyntax); | ||
} | ||
|
||
public static CommandLine parse(final String[] args, final Options options, final CommandLineParser parser) { | ||
return parse(args, options, parser, null); | ||
} | ||
|
||
public static CommandLine parse(final String[] args, final Options options) { | ||
return parse(args, options, new DefaultParser()); | ||
} | ||
|
||
/** | ||
* Provide a fluent interface for building an OptionsGroup. | ||
* | ||
* @param isRequired - is the option group required | ||
* @param options - options in the option group | ||
* @return the created option group. | ||
*/ | ||
public static OptionGroup createOptionGroup(final boolean isRequired, final Option... options) { | ||
final OptionGroup optionGroup = new OptionGroup(); | ||
optionGroup.setRequired(isRequired); | ||
for (final Option option : options) { | ||
optionGroup.addOption(option); | ||
} | ||
return optionGroup; | ||
} | ||
|
||
public static CommandLineParser getRelaxedParser() { | ||
return new RelaxedParser(); | ||
} | ||
|
||
// https://stackoverflow.com/questions/33874902/apache-commons-cli-1-3-1-how-to-ignore-unknown-arguments | ||
private static class RelaxedParser extends DefaultParser { | ||
|
||
@Override | ||
public CommandLine parse(final Options options, final String[] arguments) throws ParseException { | ||
final List<String> knownArgs = new ArrayList<>(); | ||
for (int i = 0; i < arguments.length; i++) { | ||
if (options.hasOption(arguments[i])) { | ||
knownArgs.add(arguments[i]); | ||
if (i + 1 < arguments.length && options.getOption(arguments[i]).hasArg()) { | ||
knownArgs.add(arguments[i + 1]); | ||
} | ||
} | ||
} | ||
return super.parse(options, knownArgs.toArray(new String[0])); | ||
} | ||
|
||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
/* | ||
* Copyright (c) 2021 Airbyte, Inc., all rights reserved. | ||
*/ | ||
|
||
package io.airbyte.commons.cli; | ||
|
||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
import static org.junit.jupiter.api.Assertions.assertThrows; | ||
|
||
import org.apache.commons.cli.CommandLine; | ||
import org.apache.commons.cli.DefaultParser; | ||
import org.apache.commons.cli.Option; | ||
import org.apache.commons.cli.OptionGroup; | ||
import org.apache.commons.cli.Options; | ||
import org.junit.jupiter.api.Test; | ||
|
||
class ClisTest { | ||
|
||
@Test | ||
void testCreateOptionGroup() { | ||
final Option optionA = new Option("a", "alpha"); | ||
final Option optionB = new Option("b", "beta"); | ||
final OptionGroup optionGroupExpected = new OptionGroup(); | ||
optionGroupExpected.addOption(optionA); | ||
optionGroupExpected.addOption(optionB); | ||
|
||
final OptionGroup optionGroupActual = Clis.createOptionGroup( | ||
false, | ||
optionA, | ||
optionB); | ||
|
||
// hack: OptionGroup does not define hashcode, so compare its string instead of the object itself. | ||
assertEquals(optionGroupExpected.toString(), optionGroupActual.toString()); | ||
} | ||
|
||
@Test | ||
void testParse() { | ||
final Option optionA = Option.builder("a").required(true).hasArg(true).build(); | ||
final Option optionB = Option.builder("b").required(true).hasArg(true).build(); | ||
final Options options = new Options().addOption(optionA).addOption(optionB); | ||
final String[] args = {"-a", "alpha", "-b", "beta"}; | ||
final CommandLine parsed = Clis.parse(args, options, new DefaultParser()); | ||
assertEquals("alpha", parsed.getOptions()[0].getValue()); | ||
assertEquals("beta", parsed.getOptions()[1].getValue()); | ||
} | ||
|
||
@Test | ||
void testParseNonConforming() { | ||
final Option optionA = Option.builder("a").required(true).hasArg(true).build(); | ||
final Option optionB = Option.builder("b").required(true).hasArg(true).build(); | ||
final Options options = new Options().addOption(optionA).addOption(optionB); | ||
final String[] args = {"-a", "alpha", "-b", "beta", "-c", "charlie"}; | ||
assertThrows(IllegalArgumentException.class, () -> Clis.parse(args, options, new DefaultParser())); | ||
} | ||
|
||
@Test | ||
void testParseNonConformingWithSyntax() { | ||
final Option optionA = Option.builder("a").required(true).hasArg(true).build(); | ||
final Option optionB = Option.builder("b").required(true).hasArg(true).build(); | ||
final Options options = new Options().addOption(optionA).addOption(optionB); | ||
final String[] args = {"-a", "alpha", "-b", "beta", "-c", "charlie"}; | ||
assertThrows(IllegalArgumentException.class, () -> Clis.parse(args, options, new DefaultParser(), "search")); | ||
} | ||
|
||
@Test | ||
void testRelaxedParser() { | ||
final Option optionA = Option.builder("a").required(true).hasArg(true).build(); | ||
final Option optionB = Option.builder("b").required(true).hasArg(true).build(); | ||
final Options options = new Options().addOption(optionA).addOption(optionB); | ||
final String[] args = {"-a", "alpha", "-b", "beta", "-c", "charlie"}; | ||
final CommandLine parsed = Clis.parse(args, options, Clis.getRelaxedParser()); | ||
assertEquals("alpha", parsed.getOptions()[0].getValue()); | ||
assertEquals("beta", parsed.getOptions()[1].getValue()); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,19 +5,14 @@ | |
package io.airbyte.integrations.base; | ||
|
||
import com.google.common.base.Preconditions; | ||
import io.airbyte.commons.cli.Clis; | ||
import java.nio.file.Path; | ||
import java.util.ArrayList; | ||
import java.util.HashMap; | ||
import java.util.List; | ||
import java.util.Map; | ||
import org.apache.commons.cli.CommandLine; | ||
import org.apache.commons.cli.CommandLineParser; | ||
import org.apache.commons.cli.DefaultParser; | ||
import org.apache.commons.cli.HelpFormatter; | ||
import org.apache.commons.cli.Option; | ||
import org.apache.commons.cli.OptionGroup; | ||
import org.apache.commons.cli.Options; | ||
import org.apache.commons.cli.ParseException; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
|
@@ -30,52 +25,40 @@ public class IntegrationCliParser { | |
|
||
private static final Logger LOGGER = LoggerFactory.getLogger(IntegrationCliParser.class); | ||
|
||
private static final OptionGroup COMMAND_GROUP = new OptionGroup(); | ||
|
||
static { | ||
COMMAND_GROUP.setRequired(true); | ||
COMMAND_GROUP.addOption(Option.builder() | ||
.longOpt(Command.SPEC.toString().toLowerCase()) | ||
.desc("outputs the json configuration specification") | ||
.build()); | ||
COMMAND_GROUP.addOption(Option.builder() | ||
.longOpt(Command.CHECK.toString().toLowerCase()) | ||
.desc("checks the config can be used to connect") | ||
.build()); | ||
COMMAND_GROUP.addOption(Option.builder() | ||
.longOpt(Command.DISCOVER.toString().toLowerCase()) | ||
.desc("outputs a catalog describing the source's catalog") | ||
.build()); | ||
COMMAND_GROUP.addOption(Option.builder() | ||
.longOpt(Command.READ.toString().toLowerCase()) | ||
.desc("reads the source and outputs messages to STDOUT") | ||
.build()); | ||
COMMAND_GROUP.addOption(Option.builder() | ||
.longOpt(Command.WRITE.toString().toLowerCase()) | ||
.desc("writes messages from STDIN to the integration") | ||
.build()); | ||
} | ||
private static final OptionGroup COMMAND_GROUP = Clis.createOptionGroup( | ||
true, | ||
Option.builder() | ||
.longOpt(Command.SPEC.toString().toLowerCase()) | ||
.desc("outputs the json configuration specification") | ||
.build(), | ||
Option.builder() | ||
.longOpt(Command.CHECK.toString().toLowerCase()) | ||
.desc("checks the config can be used to connect") | ||
.build(), | ||
Option.builder() | ||
.longOpt(Command.DISCOVER.toString().toLowerCase()) | ||
.desc("outputs a catalog describing the source's catalog") | ||
.build(), | ||
Option.builder() | ||
.longOpt(Command.READ.toString().toLowerCase()) | ||
.desc("reads the source and outputs messages to STDOUT") | ||
.build(), | ||
Option.builder() | ||
.longOpt(Command.WRITE.toString().toLowerCase()) | ||
.desc("writes messages from STDIN to the integration") | ||
.build()); | ||
|
||
public IntegrationConfig parse(final String[] args) { | ||
final Command command = parseCommand(args); | ||
return parseOptions(args, command); | ||
} | ||
|
||
private static Command parseCommand(final String[] args) { | ||
final CommandLineParser parser = new RelaxedParser(); | ||
final HelpFormatter helpFormatter = new HelpFormatter(); | ||
|
||
final Options options = new Options(); | ||
options.addOptionGroup(COMMAND_GROUP); | ||
|
||
try { | ||
final CommandLine parsed = parser.parse(options, args); | ||
return Command.valueOf(parsed.getOptions()[0].getLongOpt().toUpperCase()); | ||
// if discover, then validate, etc... | ||
} catch (final ParseException e) { | ||
helpFormatter.printHelp("java-base", options); | ||
throw new IllegalArgumentException(e); | ||
} | ||
final CommandLine parsed = Clis.parse(args, options, Clis.getRelaxedParser()); | ||
return Command.valueOf(parsed.getOptions()[0].getLongOpt().toUpperCase()); | ||
} | ||
|
||
private static IntegrationConfig parseOptions(final String[] args, final Command command) { | ||
|
@@ -87,26 +70,46 @@ private static IntegrationConfig parseOptions(final String[] args, final Command | |
case SPEC -> { | ||
// no args. | ||
} | ||
case CHECK, DISCOVER -> options.addOption(Option | ||
.builder().longOpt(JavaBaseConstants.ARGS_CONFIG_KEY).desc(JavaBaseConstants.ARGS_CONFIG_DESC).hasArg(true).required(true).build()); | ||
case CHECK, DISCOVER -> options.addOption(Option.builder() | ||
.longOpt(JavaBaseConstants.ARGS_CONFIG_KEY) | ||
.desc(JavaBaseConstants.ARGS_CONFIG_DESC) | ||
.hasArg(true) | ||
.required(true) | ||
.build()); | ||
case READ -> { | ||
options.addOption(Option | ||
.builder().longOpt(JavaBaseConstants.ARGS_CONFIG_KEY).desc(JavaBaseConstants.ARGS_CONFIG_DESC).hasArg(true).required(true).build()); | ||
options.addOption(Option | ||
.builder().longOpt(JavaBaseConstants.ARGS_CATALOG_KEY).desc(JavaBaseConstants.ARGS_CATALOG_DESC).hasArg(true).build()); | ||
options.addOption(Option | ||
.builder().longOpt(JavaBaseConstants.ARGS_STATE_KEY).desc(JavaBaseConstants.ARGS_PATH_DESC).hasArg(true).build()); | ||
options.addOption(Option.builder() | ||
.longOpt(JavaBaseConstants.ARGS_CONFIG_KEY) | ||
.desc(JavaBaseConstants.ARGS_CONFIG_DESC) | ||
.hasArg(true) | ||
.required(true) | ||
.build()); | ||
options.addOption(Option.builder() | ||
.longOpt(JavaBaseConstants.ARGS_CATALOG_KEY) | ||
.desc(JavaBaseConstants.ARGS_CATALOG_DESC) | ||
.hasArg(true) | ||
.build()); | ||
options.addOption(Option.builder() | ||
.longOpt(JavaBaseConstants.ARGS_STATE_KEY) | ||
.desc(JavaBaseConstants.ARGS_PATH_DESC) | ||
.hasArg(true) | ||
.build()); | ||
} | ||
case WRITE -> { | ||
options.addOption(Option | ||
.builder().longOpt(JavaBaseConstants.ARGS_CONFIG_KEY).desc(JavaBaseConstants.ARGS_CONFIG_DESC).hasArg(true).required(true).build()); | ||
options.addOption(Option | ||
.builder().longOpt(JavaBaseConstants.ARGS_CATALOG_KEY).desc(JavaBaseConstants.ARGS_CATALOG_DESC).hasArg(true).build()); | ||
options.addOption(Option.builder() | ||
.longOpt(JavaBaseConstants.ARGS_CONFIG_KEY) | ||
.desc(JavaBaseConstants.ARGS_CONFIG_DESC) | ||
.hasArg(true) | ||
.required(true).build()); | ||
options.addOption(Option.builder() | ||
.longOpt(JavaBaseConstants.ARGS_CATALOG_KEY) | ||
.desc(JavaBaseConstants.ARGS_CATALOG_DESC) | ||
.hasArg(true) | ||
.build()); | ||
} | ||
default -> throw new IllegalStateException("Unexpected value: " + command); | ||
} | ||
|
||
final CommandLine parsed = runParse(options, args, command); | ||
final CommandLine parsed = Clis.parse(args, options, command.toString().toLowerCase()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why the command enum is not used here and we use string instead? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's the trade off of DRYing this is that the common |
||
Preconditions.checkNotNull(parsed); | ||
final Map<String, String> argsMap = new HashMap<>(); | ||
for (final Option option : parsed.getOptions()) { | ||
|
@@ -139,35 +142,4 @@ private static IntegrationConfig parseOptions(final String[] args, final Command | |
} | ||
} | ||
|
||
private static CommandLine runParse(final Options options, final String[] args, final Command command) { | ||
final CommandLineParser parser = new DefaultParser(); | ||
final HelpFormatter helpFormatter = new HelpFormatter(); | ||
|
||
try { | ||
return parser.parse(options, args); | ||
} catch (final ParseException e) { | ||
helpFormatter.printHelp(command.toString().toLowerCase(), options); | ||
throw new IllegalArgumentException(e); | ||
} | ||
} | ||
|
||
// https://stackoverflow.com/questions/33874902/apache-commons-cli-1-3-1-how-to-ignore-unknown-arguments | ||
private static class RelaxedParser extends DefaultParser { | ||
|
||
@Override | ||
public CommandLine parse(final Options options, final String[] arguments) throws ParseException { | ||
final List<String> knownArgs = new ArrayList<>(); | ||
for (int i = 0; i < arguments.length; i++) { | ||
if (options.hasOption(arguments[i])) { | ||
knownArgs.add(arguments[i]); | ||
if (i + 1 < arguments.length && options.getOption(arguments[i]).hasArg()) { | ||
knownArgs.add(arguments[i + 1]); | ||
} | ||
} | ||
} | ||
return super.parse(options, knownArgs.toArray(new String[0])); | ||
} | ||
|
||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it overkill to add a README for this module describing it's purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol. i guess if you have to ask it's not overkill :D I'll add it.