[grid] Fixing hub and node configuration loading from json file

To be able to read hub, hubHost and hubPort from json file we need to know what of there properties were read, and what are not. It is impossible if InstanceCoercer keeps default values in non-read fields. So I've changed it to fill non-read fields with null values. No test were broken, but it can break Simon's intention. In this case we have to parametrize InstanceCoercer to either keep default values or write nulls to non-read fields.

Author: barancev

java/client/src/org/openqa/selenium/json/InstanceCoercer.java

@@ -23,6 +23,7 @@
 import java.lang.reflect.Modifier;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.List;
@@ -71,6 +72,8 @@ public BiFunction<JsonInput, PropertySetting, Object> apply(Type type) {
 
         jsonInput.beginObject();
 
+        List<TypeAndWriter> usedWriters = new ArrayList<>();
+
         while (jsonInput.hasNext()) {
           String key = jsonInput.nextName();
 
@@ -79,13 +82,24 @@ public BiFunction<JsonInput, PropertySetting, Object> apply(Type type) {
             jsonInput.skipValue();
             continue;
           }
+          usedWriters.add(writer);
 
           Object value = coercer.coerce(jsonInput, writer.type, setter);
           writer.writer.accept(instance, value);
         }
 
         jsonInput.endObject();
 
+        // all unset fields should be set to null
+        allWriters.values().stream()
+            .filter(w -> ! usedWriters.contains(w))
+            .forEach(w -> {
+              try {
+                w.writer.accept(instance, null);
+              } catch (IllegalArgumentException ignore) {
+              }
+            });
+
         return instance;
       } catch (ReflectiveOperationException e) {
         throw new JsonException(e);
@@ -145,6 +159,18 @@ private Map<String, TypeAndWriter> getBeanWriters(Constructor<?> constructor) {
   }
 
   private Constructor<?> getConstructor(Type type) {
+    Class target = getClss(type);
+
+    try {
+      @SuppressWarnings("unchecked") Constructor<?> constructor = target.getConstructor();
+      constructor.setAccessible(true);
+      return constructor;
+    } catch (ReflectiveOperationException e) {
+      throw new JsonException(e);
+    }
+  }
+
+  private Class getClss(Type type) {
     Class target = null;
 
     if (type instanceof Class) {
@@ -159,14 +185,7 @@ private Constructor<?> getConstructor(Type type) {
     if (target == null) {
       throw new JsonException("Cannot determine base class");
     }
-
-    try {
-      @SuppressWarnings("unchecked") Constructor<?> constructor = target.getConstructor();
-      constructor.setAccessible(true);
-      return constructor;
-    } catch (ReflectiveOperationException e) {
-      throw new JsonException(e);
-    }
+    return target;
   }
 
   private class TypeAndWriter {

java/server/src/org/openqa/grid/internal/utils/configuration/GridHubConfiguration.java

@@ -17,9 +17,11 @@
 
 package org.openqa.grid.internal.utils.configuration;
 
+import org.openqa.grid.common.exception.GridConfigurationException;
 import org.openqa.grid.internal.listeners.Prioritizer;
 import org.openqa.grid.internal.utils.CapabilityMatcher;
 import org.openqa.grid.internal.utils.DefaultCapabilityMatcher;
+import org.openqa.selenium.json.JsonInput;
 
 import java.util.Map;
 
@@ -110,7 +112,28 @@ public GridHubConfiguration() {
    * @param filePath hub config json file to load configuration from
    */
   public static GridHubConfiguration loadFromJSON(String filePath) {
-    return StandaloneConfiguration.loadFromJson(filePath, GridHubConfiguration.class);
+    return loadFromJSON(StandaloneConfiguration.loadJsonFromResourceOrFile(filePath));
+  }
+
+  public static GridHubConfiguration loadFromJSON(JsonInput jsonInput) {
+    try {
+      GridHubConfiguration config = StandaloneConfiguration.loadFromJson(
+          jsonInput,
+          GridHubConfiguration.class);
+
+      GridHubConfiguration result = new GridHubConfiguration();
+      result.merge(config);
+      // copy non-mergeable fields
+      if (config.host != null) {
+        result.host = config.host;
+      }
+      if (config.port != null) {
+        result.port = config.port;
+      }
+      return result;
+    } catch (Throwable e) {
+      throw new GridConfigurationException("Error with the JSON of the config : " + e.getMessage(), e);
+    }
   }
 
   /**

java/server/src/org/openqa/grid/internal/utils/configuration/GridNodeConfiguration.java

@@ -403,9 +403,21 @@ public static GridNodeConfiguration loadFromJSON(JsonInput jsonInput) {
                 " the file to work with Selenium 3.See https://github.com" +
                 "/SeleniumHQ/selenium/wiki/Grid2#configuring-the-nodes-by-json" +
                 " for more details.");
-        }
+      }
 
-      return config;
+      GridNodeConfiguration result = new GridNodeConfiguration();
+      result.merge(config);
+      if (config.hub == null && (config.hubHost != null || config.hubPort != null)) {
+        result.hub = String.format("http://%s:%s", config.getHubHost(), config.getHubPort());
+      }
+      // copy non-mergeable fields
+      if (config.host != null) {
+        result.host = config.host;
+      }
+      if (config.port != null) {
+        result.port = config.port;
+      }
+      return result;
     } catch (Throwable e) {
       throw new GridConfigurationException("Error with the JSON of the config : " + e.getMessage(),
                                            e);

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

@@ -102,7 +102,7 @@ public void testLoadFromJson() throws IOException {
 
     try (Reader reader = new StringReader("{ \"host\": \"dummyhost\", \"port\": 1234 }");
         JsonInput jsonInput = new Json().newInput(reader)) {
-          ghc = GridHubConfiguration.loadFromJson(jsonInput, GridHubConfiguration.class);
+          ghc = GridHubConfiguration.loadFromJSON(jsonInput);
     }
 
     assertEquals("hub", ghc.role);