From 210dc6183c36cf90685ea77e98d872909cbe4d0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20G=C3=B3mez-S=C3=A1nchez?= Date: Fri, 22 Sep 2017 15:26:41 +0200 Subject: [PATCH] Add CommandLineParserConfig --- .../argparser/CommandLineArgumentParser.java | 19 ++-- .../argparser/CommandLineParserConfig.java | 51 ++++++++++ .../argparser/CommandLineParserOptions.java | 21 ---- .../help/DefaultDocWorkUnitHandler.java | 2 +- .../CollectionArgumentUnitTests.java | 99 ++++++++++++++----- .../argparser/CommandLinePluginUnitTest.java | 16 +-- 6 files changed, 142 insertions(+), 66 deletions(-) create mode 100644 src/main/java/org/broadinstitute/barclay/argparser/CommandLineParserConfig.java delete mode 100644 src/main/java/org/broadinstitute/barclay/argparser/CommandLineParserOptions.java diff --git a/src/main/java/org/broadinstitute/barclay/argparser/CommandLineArgumentParser.java b/src/main/java/org/broadinstitute/barclay/argparser/CommandLineArgumentParser.java index 88d1e33d..a8e54a6d 100644 --- a/src/main/java/org/broadinstitute/barclay/argparser/CommandLineArgumentParser.java +++ b/src/main/java/org/broadinstitute/barclay/argparser/CommandLineArgumentParser.java @@ -57,9 +57,6 @@ public final class CommandLineArgumentParser implements CommandLineParser { public static final String COMMENT = "#"; public static final String POSITIONAL_ARGUMENTS_NAME = "Positional Argument"; - // Extension for collection argument list files - static final String COLLECTION_LIST_FILE_EXTENSION = ".args"; - private static final Logger logger = LogManager.getLogger(); // Map from (full class) name of each CommandLinePluginDescriptor requested and @@ -113,7 +110,7 @@ private boolean inArgumentMap(ArgumentDefinition arg){ // and into which the values will be assigned. private final Object callerArguments; - private final Set parserOptions; + private final CommandLineParserConfig config; // null if no @PositionalArguments annotation private Field positionalArguments; @@ -151,7 +148,7 @@ public CommandLineArgumentParser(final Object callerArguments) { this( callerArguments, Collections.emptyList(), - Collections.emptySet() + new CommandLineParserConfig() {} ); } @@ -162,17 +159,18 @@ public CommandLineArgumentParser(final Object callerArguments) { * should be used by this command line parser to extend the list of * command line arguments with dynamically discovered plugins. If * null, no descriptors are loaded. + * @param parserConfig configuration for the behaviour of the parser. */ public CommandLineArgumentParser( final Object callerArguments, final List> pluginDescriptors, - final Set parserOptions) { + final CommandLineParserConfig parserConfig) { Utils.nonNull(callerArguments, "The object with command line arguments cannot be null"); Utils.nonNull(pluginDescriptors, "The list of pluginDescriptors cannot be null"); - Utils.nonNull(parserOptions, "The set of parser options cannot be null"); + Utils.nonNull(parserConfig, "The parser configuration cannot be null"); this.callerArguments = callerArguments; - this.parserOptions = parserOptions; + this.config = parserConfig; createArgumentDefinitions(callerArguments, null); createCommandLinePluginArgumentDefinitions(pluginDescriptors); @@ -622,7 +620,7 @@ private void setArgument(ArgumentDefinition argumentDefinition, List val throw new CommandLineException("Argument '" + argumentDefinition.getNames() + "' cannot be specified more than once."); } if (argumentDefinition.isCollection) { - if (!parserOptions.contains(CommandLineParserOptions.APPEND_TO_COLLECTIONS)) { + if (!config.getAppendToCollections()) { // if this is a collection then we only want to clear it once at the beginning, before we process // any of the values, unless we're in APPEND_TO_COLLECTIONS mode, in which case we leave the initial // and append to it @@ -718,8 +716,9 @@ private void setArgument(ArgumentDefinition argumentDefinition, List val */ private List expandListFile(final List originalValues) { List expandedValues = new ArrayList<>(originalValues.size()); + for (String stringValue: originalValues) { - if (stringValue.endsWith(COLLECTION_LIST_FILE_EXTENSION)) { + if (config.getExpansionFileExtensions().stream().anyMatch(stringValue::endsWith)) { expandedValues.addAll(loadCollectionListFile(stringValue)); } else { diff --git a/src/main/java/org/broadinstitute/barclay/argparser/CommandLineParserConfig.java b/src/main/java/org/broadinstitute/barclay/argparser/CommandLineParserConfig.java new file mode 100644 index 00000000..1eb2ac99 --- /dev/null +++ b/src/main/java/org/broadinstitute/barclay/argparser/CommandLineParserConfig.java @@ -0,0 +1,51 @@ +package org.broadinstitute.barclay.argparser; + +import java.util.Collection; +import java.util.Collections; + +/** + * Interface for the command line parser configuration. + * + * @author Daniel Gomez-Sanchez (magicDGS) + */ +public interface CommandLineParserConfig { + + /** + * Default extension for collection argument list files. + */ + public static final String DEFAULT_COLLECTION_LIST_FILE_EXTENSION = ".args"; + + /** + * Configuration for append/replace values in a Collection argument. + * + *

The default behavior is returning {@code false}, which: + * + *

    + *
  • Replace the contents of a collection argument with any values from the command line.
    • + *
  • Optionally allow the special singleton value of "null" to clear the contents of the collection.
    • + *

+ * + *

Returning {@code true} changes the behavior so that any collection arguments are ADDED to the + * initial values of the collection, and allows the special value "null" to be used first to clear the initial + * values. + * + * @return {@code true} if the result should be append to the initial values of the collection; + * {@code false} if they should be substituted. + */ + default public boolean getAppendToCollections() { + return false; + } + + /** + * Configuration for loading Collection arguments from files ending with concrete extensions. + * + *

Default behaviour returns {@link #DEFAULT_COLLECTION_LIST_FILE_EXTENSION} as the file + * extension to expand. + * + * @return extension(s) for detect an argument as a file for loading a Collection argument. + */ + default Collection getExpansionFileExtensions() { + return Collections.singleton(DEFAULT_COLLECTION_LIST_FILE_EXTENSION); + } + +} diff --git a/src/main/java/org/broadinstitute/barclay/argparser/CommandLineParserOptions.java b/src/main/java/org/broadinstitute/barclay/argparser/CommandLineParserOptions.java deleted file mode 100644 index 57e6c3b2..00000000 --- a/src/main/java/org/broadinstitute/barclay/argparser/CommandLineParserOptions.java +++ /dev/null @@ -1,21 +0,0 @@ -package org.broadinstitute.barclay.argparser; - -/** - * Options used to control command line parser behavior. - */ -public enum CommandLineParserOptions { - - /** - * The default behavior for the parser is to: - * - *

    - *
  • Replace the contents of a collection argument with any values from the command line
    • - *
  • Optionally allow the special singleton value of "null" to clear the contents of the collection.
    • - *

- * - * Specifying "APPEND_TO_COLLECTIONS" changes the behavior so that any collection arguments are ADDED to the - * initial values of the collection, and allows the special value "null" to be used first to clear the initial - * values. - */ - APPEND_TO_COLLECTIONS // default behavior is "replace" -} diff --git a/src/main/java/org/broadinstitute/barclay/help/DefaultDocWorkUnitHandler.java b/src/main/java/org/broadinstitute/barclay/help/DefaultDocWorkUnitHandler.java index 3c560b00..41f04d9d 100644 --- a/src/main/java/org/broadinstitute/barclay/help/DefaultDocWorkUnitHandler.java +++ b/src/main/java/org/broadinstitute/barclay/help/DefaultDocWorkUnitHandler.java @@ -196,7 +196,7 @@ public void processWorkUnit( if (argumentContainer instanceof CommandLinePluginProvider) { pluginDescriptors = ((CommandLinePluginProvider) argumentContainer).getPluginDescriptors(); clp = new CommandLineArgumentParser( - argumentContainer, pluginDescriptors, Collections.emptySet() + argumentContainer, pluginDescriptors, new CommandLineParserConfig() {} ); } else { clp = new CommandLineArgumentParser(argumentContainer); diff --git a/src/test/java/org/broadinstitute/barclay/argparser/CollectionArgumentUnitTests.java b/src/test/java/org/broadinstitute/barclay/argparser/CollectionArgumentUnitTests.java index 9b21bae5..002a12d6 100644 --- a/src/test/java/org/broadinstitute/barclay/argparser/CollectionArgumentUnitTests.java +++ b/src/test/java/org/broadinstitute/barclay/argparser/CollectionArgumentUnitTests.java @@ -38,7 +38,7 @@ public Object[][] uninitializedCollections() { // the same behavior on collections with no initial values { inputArgs, - Collections.EMPTY_SET, // replace mode + new CommandLineParserConfig() {}, // replace mode makeList("L1", "L2"), makeList("S1"), makeList("HS1"), @@ -46,7 +46,13 @@ public Object[][] uninitializedCollections() { }, { inputArgs, - Collections.singleton(CommandLineParserOptions.APPEND_TO_COLLECTIONS), // append mode + // append mode + new CommandLineParserConfig() { + @Override + public boolean getAppendToCollections() { + return true; + } + }, makeList("L1", "L2"), makeList("S1"), makeList("HS1"), @@ -58,14 +64,14 @@ public Object[][] uninitializedCollections() { @Test(dataProvider="uninitializedCollections") public void testUninitializedCollections( final String[] args, - final Set parserOptions, + final CommandLineParserConfig parserConfig, final List expectedList, final List expectedArrayList, final List expectedHashSet, final List expectedCollection) { final UninitializedCollections o = new UninitializedCollections(); - final CommandLineArgumentParser clp = new CommandLineArgumentParser(o, Collections.emptyList(), parserOptions); + final CommandLineArgumentParser clp = new CommandLineArgumentParser(o, Collections.emptyList(), parserConfig); Assert.assertTrue(clp.parseArguments(System.err, args)); Assert.assertEquals(o.LIST, expectedList); Assert.assertEquals(o.ARRAY_LIST, expectedArrayList); @@ -84,45 +90,52 @@ public Object[][] initializedCollections() { final String[] inputArgsWithNullAtStart = new String[] {"--LIST", "null", "--LIST", "baz", "--LIST", "frob"}; final String[] inputArgsWithNullMidStream = new String[] {"--LIST", "baz", "--LIST", "null", "--LIST", "frob"}; + final CommandLineParserConfig appendConfig = new CommandLineParserConfig() { + @Override + public boolean getAppendToCollections() { + return true; + } + }; + return new Object[][]{ { inputArgs, - Collections.singleton(CommandLineParserOptions.APPEND_TO_COLLECTIONS), + appendConfig, makeList("foo", "bar", "baz", "frob") // original values retained }, { inputArgs, - Collections.emptySet(), + new CommandLineParserConfig() {}, makeList("baz", "frob") // original values replaced }, { inputArgsWithNullAtStart, - Collections.singleton(CommandLineParserOptions.APPEND_TO_COLLECTIONS), + appendConfig, makeList("baz", "frob") // original values replaced }, { inputArgsWithNullAtStart, - Collections.emptySet(), + new CommandLineParserConfig() {}, makeList("baz", "frob") // original values replaced }, { inputArgsWithNullMidStream, - Collections.singleton(CommandLineParserOptions.APPEND_TO_COLLECTIONS), + appendConfig, makeList("frob") // reset mid-stream via midstream null }, { inputArgsWithNullMidStream, - Collections.emptySet(), + new CommandLineParserConfig() {}, makeList("frob") // reset mid-stream via midstream null }, { new String[]{}, - Collections.singleton(CommandLineParserOptions.APPEND_TO_COLLECTIONS), + appendConfig, makeList("foo", "bar") }, { new String[]{}, - Collections.singleton(Collections.emptySet()), + new CommandLineParserConfig() {}, makeList("foo", "bar") } }; @@ -131,13 +144,13 @@ public Object[][] initializedCollections() { @Test(dataProvider="initializedCollections") public void testInitializedCollections( final String[] args, - final Set parserOptions, + final CommandLineParserConfig parserConfig, final List expectedResult) { final InitializedCollections o = new InitializedCollections(); final CommandLineParser clp = new CommandLineArgumentParser( o, Collections.emptyList(), - parserOptions + parserConfig ); Assert.assertTrue(clp.parseArguments(System.err, args)); Assert.assertEquals(o.LIST, expectedResult); @@ -157,16 +170,22 @@ class CollectionForListFileArguments { @DataProvider(name="listFileArguments") public Object[][] listFileArguments() { final String[] inputArgs = new String[] { "shmiggle0", "shmiggle1", "shmiggle2" }; + return new Object[][] { { - // replace mode - Collections.EMPTY_SET, // parser options - inputArgs, // args - new String[] {"shmiggle0", "shmiggle1", "shmiggle2"}, // expected result + // replace mode (default configuration) + new CommandLineParserConfig() {}, // parser config + inputArgs, // args + new String[] {"shmiggle0", "shmiggle1", "shmiggle2"}, // expected result }, { // append mode - Collections.singleton(CommandLineParserOptions.APPEND_TO_COLLECTIONS), // parser options + new CommandLineParserConfig() { + @Override + public boolean getAppendToCollections() { + return true; + } + }, // parser config inputArgs, // args new String[] {"foo", "bar", "shmiggle0", "shmiggle1", "shmiggle2"}, // expected result }, @@ -176,7 +195,7 @@ public Object[][] listFileArguments() { // Test that .list files populate collections with file contents, both mpdes @Test(dataProvider="listFileArguments") public void testCollectionFromListFile( - final Set parserOptions, + final CommandLineParserConfig parserConfig, final String [] argList, final String[] expectedList) throws IOException { @@ -187,7 +206,7 @@ public void testCollectionFromListFile( final CommandLineParser clp = new CommandLineArgumentParser( o, Collections.emptyList(), - parserOptions + parserConfig ); final String[] args = {"--LIST", argListFile.getAbsolutePath()}; @@ -196,14 +215,37 @@ public void testCollectionFromListFile( Assert.assertEquals(o.LIST, makeList(expectedList)); } + @Test + public void testDoNotExpandCollectionListFile() { + final CollectionForListFileArguments o = new CollectionForListFileArguments(); + final CommandLineParser clp = new CommandLineArgumentParser( + o, + Collections.emptyList(), + new CommandLineParserConfig() { + @Override + public Collection getExpansionFileExtensions() { + return Collections.emptySet(); + } + } + ); + + final String noListFile = "no_arg_list_file" + CommandLineParserConfig.DEFAULT_COLLECTION_LIST_FILE_EXTENSION; + + final String[] args = {"--LIST", noListFile}; + Assert.assertTrue(clp.parseArguments(System.err, args)); + + Assert.assertEquals(o.LIST, makeList(noListFile)); + } + @DataProvider(name="mixedListFileArguments") public Object[][] mixedListFileArguments() { final String[] inputArgList1 = { "shmiggle0", "shmiggle1", "shmiggle2" }; final String[] inputArgList2 = { "test2_shmiggle0", "test2_shmiggle1", "test2_shmiggle2" }; + return new Object[][] { { // replace mode - Collections.EMPTY_SET, // parser options + new CommandLineParserConfig() {}, // parser config inputArgList1, // args list 1 inputArgList2, // args list 2 new String[] {"shmiggle0", "shmiggle1", "shmiggle2"}, // expected result list 1 @@ -217,7 +259,12 @@ public Object[][] mixedListFileArguments() { }, { // append mode - Collections.singleton(CommandLineParserOptions.APPEND_TO_COLLECTIONS), // parser options + new CommandLineParserConfig() { + @Override + public boolean getAppendToCollections() { + return true; + } + }, // parser config inputArgList1, // args list 1 inputArgList2, // args list 2 new String[] {"foo", "bar", "shmiggle0", "shmiggle1", "shmiggle2"}, // expected result list 1 @@ -236,7 +283,7 @@ public Object[][] mixedListFileArguments() { // Test that .list files intermixed with explicit command line values populate collections correctly, both mpdes @Test(dataProvider="mixedListFileArguments") public void testCollectionFromListFileMixed( - final Set parserOptions, + final CommandLineParserConfig parserConfig, final String [] argList1, final String [] argList2, final String[] expectedList1, @@ -250,7 +297,7 @@ public void testCollectionFromListFileMixed( final CommandLineParser clp = new CommandLineArgumentParser( o, Collections.emptyList(), - parserOptions + parserConfig ); // mix command line values and file values @@ -281,7 +328,7 @@ public void testCollectionThatCannotBeAutoInitialized() { // Helper methods private File createListArgumentFile(final String fileName, final String[] argList) throws IOException { - final File listFile = File.createTempFile(fileName, CommandLineArgumentParser.COLLECTION_LIST_FILE_EXTENSION); + final File listFile = File.createTempFile(fileName, CommandLineParserConfig.DEFAULT_COLLECTION_LIST_FILE_EXTENSION); listFile.deleteOnExit(); try (final PrintWriter writer = new PrintWriter(listFile)) { Arrays.stream(argList).forEach(arg -> writer.println(arg)); diff --git a/src/test/java/org/broadinstitute/barclay/argparser/CommandLinePluginUnitTest.java b/src/test/java/org/broadinstitute/barclay/argparser/CommandLinePluginUnitTest.java index cba34875..f4a38a11 100644 --- a/src/test/java/org/broadinstitute/barclay/argparser/CommandLinePluginUnitTest.java +++ b/src/test/java/org/broadinstitute/barclay/argparser/CommandLinePluginUnitTest.java @@ -215,7 +215,7 @@ public void testBasicPlugin(final String[] args, final int expectedInstanceCount final CommandLineArgumentParser clp = new CommandLineArgumentParser( plugInTest, Collections.singletonList(new TestPluginDescriptor(Collections.singletonList(new TestDefaultPlugin()))), - Collections.emptySet()); + new CommandLineParserConfig() {}); Assert.assertTrue(clp.parseArguments(System.err, args)); @@ -233,7 +233,7 @@ public void testPluginUsage() { final CommandLineArgumentParser clp = new CommandLineArgumentParser( plugInTest, Collections.singletonList(new TestPluginDescriptor(Collections.singletonList(new TestDefaultPlugin()))), - Collections.emptySet()); + new CommandLineParserConfig() {}); final String out = clp.usage(true, false); // with common args, without hidden TestPluginDescriptor pid = clp.getPluginDescriptor(TestPluginDescriptor.class); @@ -263,7 +263,7 @@ public void testRequiredDependentArguments( { CommandLineParser clp = new CommandLineArgumentParser(new Object(), Collections.singletonList(new TestPluginDescriptor(Collections.singletonList(new TestDefaultPlugin()))), - Collections.emptySet()); + new CommandLineParserConfig() {}); String[] args = { "--" + TestPluginDescriptor.testPluginArgumentName, plugin // no args, just enable plugin }; @@ -288,7 +288,7 @@ public void testDanglingFilterArguments( { CommandLineParser clp = new CommandLineArgumentParser(new Object(), Collections.singletonList(new TestPluginDescriptor(Collections.singletonList(new TestDefaultPlugin()))), - Collections.emptySet()); + new CommandLineParserConfig() {}); String[] args = { argName, argValue }; // plugin args are specified but no plugin actually specified @@ -299,7 +299,7 @@ public void testDanglingFilterArguments( public void testNoPluginsSpecified() { CommandLineParser clp = new CommandLineArgumentParser(new Object(), Collections.singletonList(new TestPluginDescriptor(Collections.singletonList(new TestDefaultPlugin()))), - Collections.emptySet()); + new CommandLineParserConfig() {}); clp.parseArguments(System.out, new String[]{}); // get the command line read plugins @@ -312,7 +312,7 @@ public void testNoPluginsSpecified() { public void testEnableMultiplePlugins() { CommandLineParser clp = new CommandLineArgumentParser(new Object(), Collections.singletonList(new TestPluginDescriptor(Collections.singletonList(new TestDefaultPlugin()))), - Collections.emptySet()); + new CommandLineParserConfig() {}); String[] args = { "--" + TestPluginDescriptor.testPluginArgumentName, TestPluginWithRequiredArg.class.getSimpleName(), "--" + TestPluginWithRequiredArg.requiredArgName, "fake", @@ -333,7 +333,7 @@ public void testEnableMultiplePlugins() { public void testEnableNonExistentPlugin() { CommandLineParser clp = new CommandLineArgumentParser(new Object(), Collections.singletonList(new TestPluginDescriptor(Collections.singletonList(new TestDefaultPlugin()))), - Collections.emptySet()); + new CommandLineParserConfig() {}); clp.parseArguments(System.out, new String[] {"--" + TestPluginDescriptor.testPluginArgumentName, "nonExistentPlugin"}); } @@ -450,7 +450,7 @@ public void testPluginArgumentNameCollision(){ new CommandLineArgumentParser( PlugInTestObject, Collections.singletonList(new TestPluginArgCollisionDescriptor()), - Collections.emptySet()); + new CommandLineParserConfig() {}); } }