Mercurial > hg > thermostat-ng > agent
changeset 2478:097cf8492c2f
Ensure selected subcommand options are not overridden by other subcommands
Reviewed-by: jerboaa
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2016-October/021300.html
author | Andrew Azores <aazores@redhat.com> |
---|---|
date | Tue, 18 Oct 2016 09:52:10 -0400 |
parents | 0a1bb03d407c |
children | 81cdf24da562 |
files | launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java launcher/src/test/java/com/redhat/thermostat/launcher/internal/LauncherImplTest.java |
diffstat | 2 files changed, 39 insertions(+), 4 deletions(-) [+] |
line wrap: on
line diff
--- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java Thu Oct 13 15:09:54 2016 -0400 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java Tue Oct 18 09:52:10 2016 -0400 @@ -391,16 +391,23 @@ // command or any "sibling" subcommand and it is left up to the command implementation to reject or ignore the errant // option. // See http://icedtea.classpath.org/pipermail/thermostat/2016-October/021198.html - private Options mergeSubcommandOptionsWithParent(CommandInfo cmdInfo, String[] cmdArgs) { + // package-private for testing only + Options mergeSubcommandOptionsWithParent(CommandInfo cmdInfo, String[] cmdArgs) { Options options = cmdInfo.getOptions(); PluginConfiguration.Subcommand selectedSubcommand = getSelectedSubcommand(cmdInfo, cmdArgs); - String selectedName = selectedSubcommand == null ? "" : selectedSubcommand.getName(); for (PluginConfiguration.Subcommand subcommand : cmdInfo.getSubcommands()) { for (Option option : (Collection<Option>) subcommand.getOptions().getOptions()) { Option copy = new Option(option.getOpt(), option.getLongOpt(), option.hasArg(), option.getDescription()); - boolean required = selectedName.equals(subcommand.getName()) && option.isRequired(); - copy.setRequired(required); + copy.setRequired(false); + options.addOption(copy); + } + } + + if (selectedSubcommand != null) { + for (Option option : (Collection<Option>) selectedSubcommand.getOptions().getOptions()) { + Option copy = new Option(option.getOpt(), option.getLongOpt(), option.hasArg(), option.getDescription()); + copy.setRequired(option.isRequired()); options.addOption(copy); } }
--- a/launcher/src/test/java/com/redhat/thermostat/launcher/internal/LauncherImplTest.java Thu Oct 13 15:09:54 2016 -0400 +++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/LauncherImplTest.java Tue Oct 18 09:52:10 2016 -0400 @@ -55,6 +55,7 @@ import java.nio.file.Path; import java.nio.file.attribute.BasicFileAttributes; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.EnumSet; @@ -548,6 +549,33 @@ } @Test + public void testSelectedSubcommandOptionsNotOverriddenByOtherSubcommands() { + PluginConfiguration.Subcommand subInfo1 = mock(PluginConfiguration.Subcommand.class); + when(subInfo1.getName()).thenReturn("sub1"); + when(subInfo1.getDescription()).thenReturn("subcommand description"); + + PluginConfiguration.Subcommand subInfo2 = mock(PluginConfiguration.Subcommand.class); + when(subInfo2.getName()).thenReturn("sub2"); + when(subInfo2.getDescription()).thenReturn("subcommand description"); + + Options subOptions1 = mock(Options.class); + Options subOptions2 = mock(Options.class); + Option requiredOption = new Option(null, "option", false, null); + requiredOption.setRequired(true); + Option nonRequiredOption = new Option(null, "option", false, null); + nonRequiredOption.setRequired(false); + when(subOptions1.getOptions()).thenReturn(Collections.singletonList(requiredOption)); + when(subOptions2.getOptions()).thenReturn(Collections.singletonList(nonRequiredOption)); + when(subInfo1.getOptions()).thenReturn(subOptions1); + when(subInfo2.getOptions()).thenReturn(subOptions2); + + when(info1.getSubcommands()).thenReturn(Arrays.asList(subInfo1, subInfo2)); + + Options options = launcher.mergeSubcommandOptionsWithParent(info1, new String[] { "sub1" }); + assertTrue(options.getOption("option").isRequired()); + } + + @Test public void testBadOption() { String expected = "Could not parse options: Unrecognized option: --argNotAccepted\n" + "usage: thermostat test1 <--arg1 <arg>> [--arg2 <arg>]\n"