Fixing an NPE when the component which is being inherited from doesn'…

…t exist. (#69)
oracle · Oct 5, 2023 · dcde0af · dcde0af
1 parent ad4e8a0
commit dcde0af
Show file tree

Hide file tree

Showing 9 changed files with 99 additions and 16 deletions.
diff --git a/olcut-config-json/src/main/java/com/oracle/labs/mlrg/olcut/config/json/JsonLoader.java b/olcut-config-json/src/main/java/com/oracle/labs/mlrg/olcut/config/json/JsonLoader.java
@@ -248,10 +248,10 @@ protected void parseComponent(ObjectNode node) {
             // just in case.
             ConfigurationData spd = rpdMap.get(override);
             if (spd == null) {
-                spd = existingRPD.get(override);
-                if (spd == null) {
-                    throw new ConfigLoaderException("Override for undefined component: "
-                            + override + ", with name " + curComponent);
+                if (existingRPD == null || !existingRPD.containsKey(override)) {
+                    throw new ConfigLoaderException("Failed to find base component '"+override+"' inherited from '"+curComponent+"'.");
+                } else {
+                    spd = existingRPD.get(override);
                 }
             }
             if (curType != null && !curType.equals(spd.getClassName())) {

diff --git a/...t-config-json/src/test/java/com/oracle/labs/mlrg/olcut/config/json/test/OverrideTest.java b/...t-config-json/src/test/java/com/oracle/labs/mlrg/olcut/config/json/test/OverrideTest.java
@@ -29,11 +29,14 @@
 package com.oracle.labs.mlrg.olcut.config.json.test;
 
 import com.oracle.labs.mlrg.olcut.config.ConfigurationManager;
+import com.oracle.labs.mlrg.olcut.config.io.ConfigLoaderException;
 import com.oracle.labs.mlrg.olcut.config.json.JsonConfigFactory;
 import com.oracle.labs.mlrg.olcut.test.config.StringConfigurable;
 import com.oracle.labs.mlrg.olcut.test.config.StringleConfigurable;
 
 import java.io.IOException;
+
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
@@ -99,4 +102,13 @@ public void overrideWithSubType() throws IOException {
         assertEquals("d", sc2.three);
         assertEquals("e", sc2.four);
     }
+
+    @Test
+    public void overrideIncorrectName() {
+        Assertions.assertThrows(ConfigLoaderException.class,
+                () -> {
+                    ConfigurationManager cm = new ConfigurationManager(createModuleResourceString(this.getClass(),
+                            "overrideIncorrect.json"));
+                });
+    }
 }
diff --git a/...son/src/test/resources/com/oracle/labs/mlrg/olcut/config/json/test/overrideIncorrect.json b/...son/src/test/resources/com/oracle/labs/mlrg/olcut/config/json/test/overrideIncorrect.json
@@ -0,0 +1,21 @@
+{
+  "config": {
+    "global-properties": {},
+    "components": [
+      {
+        "name": "a",
+        "type": "com.oracle.labs.mlrg.olcut.test.config.StringConfigurable",
+        "properties": {
+          "one": "a",
+          "two": "b",
+          "three": "c"
+        }
+      },
+      {
+        "name": "bErr",
+        "inherit": "aErr",
+        "properties": {}
+      }
+    ]
+  }
+}
diff --git a/...config-protobuf/src/main/java/com/oracle/labs/mlrg/olcut/config/protobuf/ProtoLoader.java b/...config-protobuf/src/main/java/com/oracle/labs/mlrg/olcut/config/protobuf/ProtoLoader.java
@@ -248,10 +248,10 @@ private void parseComponentProto(ComponentProto component) {
             // just in case.
             ConfigurationData spd = rpdMap.get(override);
             if (spd == null) {
-                spd = existingRPD.get(override);
-                if (spd == null) {
-                    throw new ConfigLoaderException("Override for undefined component: "
-                            + override + ", with name " + name);
+                if (existingRPD == null || !existingRPD.containsKey(override)) {
+                    throw new ConfigLoaderException("Failed to find base component '"+override+"' inherited from '"+name+"'.");
+                } else {
+                    spd = existingRPD.get(override);
                 }
             }
             if (!type.isEmpty() && !type.equals(spd.getClassName())) {

diff --git a/...-protobuf/src/test/java/com/oracle/labs/mlrg/olcut/config/protobuf/test/OverrideTest.java b/...-protobuf/src/test/java/com/oracle/labs/mlrg/olcut/config/protobuf/test/OverrideTest.java
@@ -29,9 +29,12 @@
 package com.oracle.labs.mlrg.olcut.config.protobuf.test;
 
 import com.oracle.labs.mlrg.olcut.config.ConfigurationManager;
+import com.oracle.labs.mlrg.olcut.config.io.ConfigLoaderException;
 import com.oracle.labs.mlrg.olcut.config.protobuf.ProtoConfigFactory;
+import com.oracle.labs.mlrg.olcut.config.protobuf.ProtoTxtConfigFactory;
 import com.oracle.labs.mlrg.olcut.test.config.StringConfigurable;
 import com.oracle.labs.mlrg.olcut.test.config.StringleConfigurable;
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
@@ -49,6 +52,7 @@ public class OverrideTest {
     @BeforeAll
     public static void setUpClass() throws Exception {
         ConfigurationManager.addFileFormatFactory(new ProtoConfigFactory());
+        ConfigurationManager.addFileFormatFactory(new ProtoTxtConfigFactory());
     }
 
     @Test
@@ -99,4 +103,13 @@ public void overrideWithSubType() throws IOException {
         assertEquals("d", sc2.three);
         assertEquals("e", sc2.four);
     }
+
+    @Test
+    public void overrideIncorrectName() {
+        Assertions.assertThrows(ConfigLoaderException.class,
+                () -> {
+                    ConfigurationManager cm = new ConfigurationManager(createModuleResourceString(this.getClass(),
+                            "overrideIncorrect.pbtxt"));
+                });
+    }
 }
diff --git a/...rc/test/resources/com/oracle/labs/mlrg/olcut/config/protobuf/test/overrideIncorrect.pbtxt b/...rc/test/resources/com/oracle/labs/mlrg/olcut/config/protobuf/test/overrideIncorrect.pbtxt
@@ -0,0 +1,20 @@
+components {
+  name: "a"
+  type: "com.oracle.labs.mlrg.olcut.test.config.StringConfigurable"
+  properties {
+    key: "one"
+    value: "a"
+  }
+  properties {
+    key: "second"
+    value: "b"
+  }
+  properties {
+    key: "three"
+    value: "c"
+  }
+}
+components {
+  name: "bErr",
+  override: "aErr"
+}
diff --git a/olcut-core-test/src/test/java/com/oracle/labs/mlrg/olcut/test/config_tests/OverrideTest.java b/olcut-core-test/src/test/java/com/oracle/labs/mlrg/olcut/test/config_tests/OverrideTest.java
@@ -31,6 +31,7 @@
 import java.io.IOException;
 
 import com.oracle.labs.mlrg.olcut.config.ConfigurationManager;
+import com.oracle.labs.mlrg.olcut.config.io.ConfigLoaderException;
 import com.oracle.labs.mlrg.olcut.test.config.StringConfigurable;
 import com.oracle.labs.mlrg.olcut.test.config.StringleConfigurable;
 import org.junit.jupiter.api.Assertions;
@@ -44,10 +45,6 @@
  */
 public class OverrideTest {
 
-    public OverrideTest() {
-    }
-
-
     @Test
     public void overrideWithSameType() throws IOException {
         ConfigurationManager cm = new ConfigurationManager(createModuleResourceString(this.getClass(), "overrideConfig.xml"));
@@ -96,4 +93,13 @@ public void overrideWithSubType() throws IOException {
         Assertions.assertEquals("d", sc2.three);
         Assertions.assertEquals("e", sc2.four);
     }
+
+    @Test
+    public void overrideIncorrectName() {
+        Assertions.assertThrows(ConfigLoaderException.class,
+                () -> {
+                    ConfigurationManager cm = new ConfigurationManager(createModuleResourceString(this.getClass(),
+                            "overrideIncorrect.xml"));
+                });
+    }
 }
diff --git a/...est/src/test/resources/com/oracle/labs/mlrg/olcut/test/config_tests/overrideIncorrect.xml b/...est/src/test/resources/com/oracle/labs/mlrg/olcut/test/config_tests/overrideIncorrect.xml
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<config>
+
+    <component name="a" type="com.oracle.labs.mlrg.olcut.test.config.StringConfigurable">
+        <property name="one" value="a"/>
+        <property name="two" value="b"/>
+        <property name="three" value="c"/>
+    </component>
+
+    <component name="bErr" inherit="aErr"/>
+</config>
diff --git a/olcut-core/src/main/java/com/oracle/labs/mlrg/olcut/config/xml/SAXLoader.java b/olcut-core/src/main/java/com/oracle/labs/mlrg/olcut/config/xml/SAXLoader.java
@@ -264,10 +264,10 @@ public void startElement(String uri, String localName, String qName,
                         // just in case.
                         ConfigurationData spd = rpdMap.get(override);
                         if (spd == null) {
-                            spd = existingRPD.get(override);
-                            if (spd == null) {
-                                throw new SAXParseException("Override for undefined component: "
-                                        + override, locator);
+                            if (existingRPD == null || !existingRPD.containsKey(override)) {
+                                throw new SAXParseException("Failed to find base component '"+override+"' inherited from '"+curComponent+"'.", locator);
+                            } else {
+                                spd = existingRPD.get(override);
                             }
                         }
                         if (curType != null && !curType.equals(spd.getClassName())) {