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"