Merge pull request #263 from Cognifide/bugfix/system-prop-overrides-e…

…verything Fixed config override using system properties
Knotx · Feb 16, 2017 · f3b849a · f3b849a
2 parents bdbf440 + cb30ede
commit f3b849a
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 14 deletions.
diff --git a/knotx-core/src/main/java/io/knotx/launcher/KnotxModuleVerticleFactory.java b/knotx-core/src/main/java/io/knotx/launcher/KnotxModuleVerticleFactory.java
@@ -57,37 +57,34 @@ public void resolve(String id, DeploymentOptions deploymentOptions, ClassLoader
       JsonObject knotOptions = descriptor.getJsonObject(OPTIONS_KEY, new JsonObject());
       JsonObject knotConfig = knotOptions.getJsonObject(CONFIG_KEY, new JsonObject());
       depOptions.mergeIn(knotOptions);
-      knotConfig = JsonObjectUtil.deepMerge(knotConfig, depConfig);
+      depOptions.put(CONFIG_KEY, JsonObjectUtil.deepMerge(knotConfig, depConfig));
+
+      JsonObject serviceDescriptor = new JsonObject().put(OPTIONS_KEY, depOptions);
 
       // Any options or config provided by system properties will override anything specified
       // at deployment time and on starter Json config
-      overrideConfigWithSystemProperties(identifier, descriptor, depOptions, knotConfig);
+      serviceDescriptor = overrideConfigWithSystemProperties(identifier, serviceDescriptor);
 
-      depOptions.put(CONFIG_KEY, knotConfig);
-      deploymentOptions.fromJson(depOptions);
+      deploymentOptions.fromJson(serviceDescriptor.getJsonObject(OPTIONS_KEY));
       resolution.complete(main);
     } catch (Exception e) {
       resolution.fail(e);
     }
   }
 
-  private void overrideConfigWithSystemProperties(String identifier, JsonObject descriptor,
-      JsonObject depOptions, JsonObject knotConfig) {
+  private JsonObject overrideConfigWithSystemProperties(String identifier, JsonObject descriptor) {
+    JsonObject result = descriptor;
     try {
       SystemPropsConfiguration systemPropsConfiguration = new SystemPropsConfiguration(
           identifier);
       if (!systemPropsConfiguration.envConfig().isEmpty()) {
-        JsonObject updatedDescriptor = systemPropsConfiguration.updateJsonObject(descriptor);
-        JsonObject updatedKnotOptions = updatedDescriptor
-            .getJsonObject(OPTIONS_KEY, new JsonObject());
-        JsonObject updatedKnotConfig = updatedKnotOptions
-            .getJsonObject(CONFIG_KEY, new JsonObject());
-        depOptions.mergeIn(updatedKnotOptions);
-        knotConfig.mergeIn(updatedKnotConfig);
+        result = systemPropsConfiguration.updateJsonObject(descriptor);
       }
     } catch (IllegalArgumentException ex) {
       LOGGER.warn("Unable to parse given system properties due to exception", ex);
     }
+
+    return result;
   }
 
   @Override

diff --git a/knotx-core/src/main/java/io/knotx/launcher/SystemPropsConfiguration.java b/knotx-core/src/main/java/io/knotx/launcher/SystemPropsConfiguration.java
@@ -16,6 +16,7 @@
 package io.knotx.launcher;
 
 import com.google.common.collect.Maps;
+import io.knotx.exceptions.ConfigurationException;
 import io.vertx.core.json.JsonObject;
 import io.vertx.core.logging.Logger;
 import io.vertx.core.logging.LoggerFactory;
@@ -69,6 +70,7 @@ private boolean onlyPropertyForIdentifier(Entry<Object, Object> entry, String id
    * <li>-Dio.knotx.KnotxServer.splitter=file:/aaa/bb/cc.json - this will merge the given cc.json
    * file from the field specified</li>
    * </ul>
+   *
    * @param descriptor - JsonObject with module descriptor
    * @return JsonObject - updated descriptor
    */
@@ -80,7 +82,13 @@ public JsonObject updateJsonObject(JsonObject descriptor) {
           JsonObject element = object;
           for (int idx = 0; idx < path.length; idx++) {
             if (idx < path.length - 1) {
-              element = element.getJsonObject(path[idx]);
+              if (element.containsKey(path[idx])) {
+                element = element.getJsonObject(path[idx]);
+              } else {
+                throw new IllegalArgumentException (
+                    "Wrong config override. There is no matching element " + entry.getKey()
+                        + " in the configuration");
+              }
             } else { //last
               if (entry.getValue().getObject() instanceof JsonObject) {
                 element.getJsonObject(path[idx]).mergeIn((JsonObject) entry.getValue().getObject());