[grid] Deleting unnecessary checks, -hub just has precedence as state…

…d in the docs
SeleniumHQ · Aug 8, 2018 · ba7ad22 · ba7ad22
1 parent 20bdf47
commit ba7ad22
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 55 deletions.
diff --git a/java/server/src/org/openqa/grid/internal/utils/configuration/GridNodeConfiguration.java b/java/server/src/org/openqa/grid/internal/utils/configuration/GridNodeConfiguration.java
@@ -177,16 +177,12 @@ public GridNodeConfiguration(NodeJsonConfiguration jsonConfig) {
     proxy = jsonConfig.getProxy();
     enablePlatformVerification = jsonConfig.isEnablePlatformVerification();
     if (jsonConfig.getHub() != null) {
+      // -hub has precedence
       hub = jsonConfig.getHub();
 
     } else {
-      if (jsonConfig.getHubHost() == null) {
-        throw new RuntimeException("You must specify either a hubHost or hub parameter in a node JSON config.");
-      }
-      if (jsonConfig.getHubPort() == null) {
-        throw new RuntimeException("You must specify either a hubPort or hub parameter in a node JSON config.");
-      }
-      hub = hubHostPort.toString();
+      hubHost = jsonConfig.getHubHost();
+      hubPort = jsonConfig.getHubPort();
     }
   }
 
@@ -206,19 +202,10 @@ public GridNodeConfiguration(GridNodeCliOptions cliConfig) {
     ofNullable(cliConfig.getEnablePlatformVerification()).ifPresent(v -> enablePlatformVerification = v);
     if (cliConfig.getHub() != null) {
       hub = cliConfig.getHub();
-      // -hub has precedence
-      if (cliConfig.getHubHost() != null) {
-        throw new GridConfigurationException("You can't specify both -hubHost and -hub options at the same time");
-      }
-      if (cliConfig.getHubPort() != null) {
-        throw new GridConfigurationException("You can't specify both -hubPort and -hub options at the same time");
-      }
-      hubHost = null;
-      hubPort = null;
-    } else if (cliConfig.getHubHost() != null && cliConfig.getHubPort() != null) {
+    } else if (cliConfig.getHubHost() != null || cliConfig.getHubPort() != null) {
       hub = null;
-      hubHost = cliConfig.getHubHost();
-      hubPort = cliConfig.getHubPort();
+      ofNullable(cliConfig.getHubHost()).ifPresent(v -> hubHost = v);
+      ofNullable(cliConfig.getHubPort()).ifPresent(v -> hubPort = v);
     }
   }
 
@@ -241,20 +228,8 @@ private HostPort getHubHostPort() {
           throw new RuntimeException("-hub must be a valid url: " + hub, mURLe);
         }
       } else if (hubHost != null || hubPort != null) {
-        if (hubHost == null) {
-          throw new RuntimeException("You must specify either a -hubHost or -hub parameter.");
-        }
-        if (hubPort == null) {
-          throw new RuntimeException("You must specify either a -hubPort or -hub parameter.");
-        }
-        hubHostPort = new HostPort(hubHost, hubPort);
-      } else {
-        try {
-          URL u = new URL(hub);
-          hubHostPort = new HostPort(u.getHost(), u.getPort());
-        } catch (MalformedURLException mURLe) {
-          throw new RuntimeException("-hub must be a valid url: " + hub, mURLe);
-        }
+        hubHostPort = new HostPort(ofNullable(hubHost).orElse(DEFAULT_CONFIG_FROM_JSON.getHubHost()),
+                                   ofNullable(hubPort).orElse(DEFAULT_CONFIG_FROM_JSON.getHubPort()));
       }
     }
     return hubHostPort;

diff --git a/java/server/test/org/openqa/grid/common/RegistrationRequestTest.java b/java/server/test/org/openqa/grid/common/RegistrationRequestTest.java
@@ -123,7 +123,7 @@ public void registerParam() {
     assertEquals(true, req.getConfiguration().register);
 
     config = parseCliOptions(
-        "-role", "wd", "-hubHost", "ABC", "-hubPort", "1234","-host","localhost", "-register","false");
+        "-role", "wd", "-hubHost", "ABC", "-hubPort", "1234", "-host","localhost", "-register","false");
     RegistrationRequest req2 = RegistrationRequest.build(config);
     assertEquals(false, req2.getConfiguration().register);
   }

diff --git a/java/server/test/org/openqa/grid/internal/utils/configuration/GridNodeConfigurationTest.java b/java/server/test/org/openqa/grid/internal/utils/configuration/GridNodeConfigurationTest.java
@@ -235,27 +235,6 @@ public void testGetHubHostFromHubOption() {
     assertEquals("dummyhost", gnc.getHubHost());
   }
 
-  @Test
-  public void testHubHostAndHubCannotBeUsedAtTheSameTime() {
-    Throwable t = catchThrowable(() -> parseCliOptions(
-        "-hub", "http://smarthost:4321/wd/hub", "-hubHost", "dummyhost"));
-    assertTrue(t instanceof GridConfigurationException);
-  }
-
-  @Test
-  public void testHubPortAndHubCannotBeUsedAtTheSameTime() {
-    Throwable t = catchThrowable(() -> parseCliOptions(
-        "-hub", "http://smarthost:4321/wd/hub", "-hubPort", "1234"));
-    assertTrue(t instanceof GridConfigurationException);
-  }
-
-  @Test
-  public void testHubHostAndPortAndHubCannotBeUsedAtTheSameTime() {
-    Throwable t = catchThrowable(() -> parseCliOptions(
-        "-hub", "http://smarthost:4321/wd/hub", "-hubHost", "dummyhost", "-hubPort", "1234"));
-    assertTrue(t instanceof GridConfigurationException);
-  }
-
   @Test
   public void testGetHubPort() {
     GridNodeConfiguration gnc = parseCliOptions("-hubHost", "dummyhost", "-hubPort", "1234");