[grid] Fixing CLI processing if -nodeConfig or -hubConfig present

JCommander toggles boolean options instead of setting them.

Author: barancev

java/server/src/org/openqa/grid/internal/cli/CommonGridCliOptions.java

@@ -119,7 +119,7 @@ String builtIn(String resource) {
     }
   }
 
-  String fromConfigFile(String file) {
+  static String fromConfigFile(String file) {
     try(BufferedReader br = new BufferedReader(new InputStreamReader(new FileInputStream(file)))) {
       return br.lines().collect(Collectors.joining("\n"));
     } catch (IOException e) {

java/server/src/org/openqa/grid/internal/cli/GridHubCliOptions.java

@@ -33,6 +33,28 @@
 
 public class GridHubCliOptions extends CommonGridCliOptions {
 
+  public static class Parser {
+
+    public GridHubCliOptions parse(String[] args) {
+      GridHubCliOptions result = new GridHubCliOptions();
+      JCommander.newBuilder().addObject(result).build().parse(args);
+
+      if (result.configFile != null) {
+        // Second round
+        String configFile = result.configFile;
+        result = new GridHubCliOptions();
+        JCommander.newBuilder().addObject(result)
+            .defaultProvider(defaults(fromConfigFile(configFile))).build().parse(args);
+      }
+
+      return result;
+    }
+  }
+
+  /**
+   * @deprecated Use GridHubCliOptions.Parser instead
+   */
+  @Deprecated
   public GridHubCliOptions parse(String[] args) {
     JCommander.newBuilder().addObject(this).build().parse(args);
 
@@ -44,7 +66,7 @@ public GridHubCliOptions parse(String[] args) {
     return this;
   }
 
-  private IDefaultProvider defaults(String json) {
+  private static IDefaultProvider defaults(String json) {
     Map<String, Object> map = (Map<String, Object>) new Json().toType(json, Map.class);
     map.remove("custom");
     return optionName -> {

java/server/src/org/openqa/grid/internal/cli/GridNodeCliOptions.java

@@ -28,11 +28,34 @@
 import org.openqa.selenium.MutableCapabilities;
 import org.openqa.selenium.json.Json;
 
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 
 public class GridNodeCliOptions extends CommonGridCliOptions {
 
+  public static class Parser {
+
+    public GridNodeCliOptions parse(String[] args) {
+      GridNodeCliOptions result = new GridNodeCliOptions();
+      JCommander.newBuilder().addObject(result).build().parse(args);
+
+      if (result.configFile != null) {
+        // Second round
+        String configFile = result.configFile;
+        result = new GridNodeCliOptions();
+        JCommander.newBuilder().addObject(result)
+            .defaultProvider(defaults(fromConfigFile(configFile))).build().parse(args);
+      }
+
+      return result;
+    }
+  }
+
+  /**
+   * @deprecated Use GridNodeCliOptions.Parser instead
+   */
+  @Deprecated
   public GridNodeCliOptions parse(String[] args) {
     JCommander.newBuilder().addObject(this).build().parse(args);
 
@@ -44,7 +67,7 @@ public GridNodeCliOptions parse(String[] args) {
     return this;
   }
 
-  private IDefaultProvider defaults(String json) {
+  private static IDefaultProvider defaults(String json) {
     Map<String, Object> map = new Json().toType(json, Map.class);
     map.remove("custom");
     map.remove("capabilities");

java/server/src/org/openqa/grid/selenium/GridLauncherV3.java

@@ -72,7 +72,6 @@ default void printUsage(PrintStream out) {
       StringBuilder sb = new StringBuilder();
       new JCommander(getOptions()).usage(sb);
       out.print(sb);
-
     }
   }
 
@@ -276,7 +275,7 @@ public Stoppable launch() {
 
         })
         .put(GridRole.HUB.toString(), (args) -> new GridItemLauncher() {
-          GridHubCliOptions options = new GridHubCliOptions().parse(args);
+          GridHubCliOptions options = new GridHubCliOptions.Parser().parse(args);
 
           @Override
           public CommonCliOptions getOptions() {
@@ -294,7 +293,7 @@ public Stoppable launch() throws Exception {
           }
         })
         .put(GridRole.NODE.toString(), (args) -> new GridItemLauncher() {
-          GridNodeCliOptions options = new GridNodeCliOptions().parse(args);
+          GridNodeCliOptions options = new GridNodeCliOptions.Parser().parse(args);
 
           @Override
           public CommonCliOptions getOptions() {

java/server/test/org/openqa/grid/common/RegistrationRequestTest.java

@@ -376,6 +376,6 @@ private void assertConstruction(RegistrationRequest req) {
   }
 
   private GridNodeConfiguration parseCliOptions(String... args) {
-    return new GridNodeCliOptions().parse(args).toConfiguration();
+    return new GridNodeCliOptions.Parser().parse(args).toConfiguration();
   }
 }

java/server/test/org/openqa/grid/e2e/utils/GridTestHelper.java

@@ -42,7 +42,7 @@ public static SelfRegisteringRemote getRemoteWithoutCapabilities(URL hub, GridRo
                      "-host","localhost",
                      "-hub",hub.toString(),
                      "-port",String.valueOf(PortProber.findFreePort())};
-    GridNodeConfiguration config = new GridNodeCliOptions().parse(args).toConfiguration();
+    GridNodeConfiguration config = new GridNodeCliOptions.Parser().parse(args).toConfiguration();
     RegistrationRequest req = RegistrationRequest.build(config);
     SelfRegisteringRemote remote = new SelfRegisteringRemote(req);
     remote.deleteAllBrowsers();

java/server/test/org/openqa/grid/internal/BaseRemoteProxyTest.java

@@ -189,6 +189,6 @@ public void teardown() {
   }
 
   private GridNodeConfiguration parseCliOptions(String... args) {
-    return new GridNodeCliOptions().parse(args).toConfiguration();
+    return new GridNodeCliOptions.Parser().parse(args).toConfiguration();
   }
 }

java/server/test/org/openqa/grid/internal/UserDefinedCapabilityMatcherTests.java

@@ -33,7 +33,7 @@ public class UserDefinedCapabilityMatcherTests {
   public void defaultsToDefaultMatcher() {
     GridRegistry registry = DefaultGridRegistry.newInstance(new Hub(new GridHubConfiguration()));
     String[] args = new String[]{"-role", "webdriver","-id", "abc","-host","localhost"};
-    GridNodeConfiguration nodeConfiguration = new GridNodeCliOptions().parse(args).toConfiguration();
+    GridNodeConfiguration nodeConfiguration = new GridNodeCliOptions.Parser().parse(args).toConfiguration();
     RegistrationRequest req = RegistrationRequest.build(nodeConfiguration);
     req.getConfiguration().proxy = null;
     RemoteProxy p = BaseRemoteProxy.getNewInstance(req, registry);
@@ -49,7 +49,7 @@ public void capabilityMatcherCanBeSpecified() {
     hubConfig.capabilityMatcher = new MyCapabilityMatcher();
     Hub hub = new Hub(hubConfig);
     String[] args = new String[]{"-role", "webdriver","-id", "abc","-host","localhost"};
-    GridNodeConfiguration nodeConfiguration = new GridNodeCliOptions().parse(args).toConfiguration();
+    GridNodeConfiguration nodeConfiguration = new GridNodeCliOptions.Parser().parse(args).toConfiguration();
     RegistrationRequest req = RegistrationRequest.build(nodeConfiguration);
     req.getConfiguration().proxy = null;
     RemoteProxy p = BaseRemoteProxy.getNewInstance(req, hub.getRegistry());

java/server/test/org/openqa/grid/internal/utils/configuration/GridHubConfigurationTest.java

@@ -173,7 +173,7 @@ public void testToString() {
     ghc = new GridHubConfiguration();
     String[] args = ("-servlet com.foo.bar.ServletA -servlet com.foo.bar.ServletB"
                      + " -custom foo=bar,bar=baz").split(" ");
-    ghc = new GridHubCliOptions().parse(args).toConfiguration();
+    ghc = new GridHubCliOptions.Parser().parse(args).toConfiguration();
 
     assertTrue(ghc.toString().contains("-servlets com.foo.bar.ServletA"
                                        + " -servlets com.foo.bar.ServletB"));
@@ -186,7 +186,7 @@ public void testToString() {
   public void testJcommanderConverterCapabilityMatcher() {
     String[] hubArgs = {"-capabilityMatcher", "org.openqa.grid.internal.utils.DefaultCapabilityMatcher",
                         "-prioritizer", "org.openqa.grid.internal.utils.configuration.PlaceHolderTestingPrioritizer"};
-    GridHubConfiguration ghc = new GridHubCliOptions().parse(hubArgs).toConfiguration();
+    GridHubConfiguration ghc = new GridHubCliOptions.Parser().parse(hubArgs).toConfiguration();
     assertEquals("org.openqa.grid.internal.utils.DefaultCapabilityMatcher",
                  ghc.capabilityMatcher.getClass().getCanonicalName());
     assertEquals("org.openqa.grid.internal.utils.configuration.PlaceHolderTestingPrioritizer",

java/server/test/org/openqa/grid/internal/utils/configuration/GridNodeConfigurationTest.java

@@ -207,7 +207,7 @@ public void testAsJson() {
   public void testWithCapabilitiesArgs() {
     final String[] args = new String[] { "-capabilities",
                                        "browserName=chrome,platform=linux,maxInstances=10,boolean=false" };
-    GridNodeConfiguration gnc = new GridNodeCliOptions().parse(args).toConfiguration();
+    GridNodeConfiguration gnc = new GridNodeCliOptions.Parser().parse(args).toConfiguration();
     assertTrue(gnc.capabilities.size() == 1);
     assertEquals("chrome", gnc.capabilities.get(0).getBrowserName());
     assertEquals(10L, gnc.capabilities.get(0).getCapability("maxInstances"));
@@ -372,6 +372,6 @@ public void hubOptionHasPrecedenceOverNodeConfig() throws IOException {
   }
 
   private GridNodeConfiguration parseCliOptions(String... args) {
-    return new GridNodeCliOptions().parse(args).toConfiguration();
+    return new GridNodeCliOptions.Parser().parse(args).toConfiguration();
   }
 }