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

…t exist. (#69)
oracle · Nov 1, 2024 · 4486cea · 4486cea
1 parent 4fc0f7c
commit 4486cea
Show file tree

Hide file tree

Showing 9 changed files with 96 additions and 12 deletions.
diff --git a/olcut-config-edn/src/test/java/com/oracle/labs/mlrg/olcut/config/edn/OverrideTest.java b/olcut-config-edn/src/test/java/com/oracle/labs/mlrg/olcut/config/edn/OverrideTest.java
@@ -28,11 +28,13 @@
 
 package com.oracle.labs.mlrg.olcut.config.edn;
 
+import com.oracle.labs.mlrg.olcut.config.io.ConfigLoaderException;
 import com.oracle.labs.mlrg.olcut.config.ConfigurationManager;
 import com.oracle.labs.mlrg.olcut.config.StringConfigurable;
 import com.oracle.labs.mlrg.olcut.config.StringleConfigurable;
 
 import java.io.IOException;
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
@@ -96,4 +98,12 @@ 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("overrideIncorrect.xml");
+                });
+    }
 }
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/olcut-config-json/src/test/java/com/oracle/labs/mlrg/olcut/config/json/OverrideTest.java b/olcut-config-json/src/test/java/com/oracle/labs/mlrg/olcut/config/json/OverrideTest.java
@@ -31,8 +31,11 @@
 import com.oracle.labs.mlrg.olcut.config.ConfigurationManager;
 import com.oracle.labs.mlrg.olcut.config.StringConfigurable;
 import com.oracle.labs.mlrg.olcut.config.StringleConfigurable;
+import com.oracle.labs.mlrg.olcut.config.io.ConfigLoaderException;
 
 import java.io.IOException;
+
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
@@ -97,4 +100,12 @@ 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("overrideIncorrect.json");
+                });
+    }
 }
diff --git a/...t-config-json/src/test/resources/com/oracle/labs/mlrg/olcut/config/overrideIncorrect.json b/...t-config-json/src/test/resources/com/oracle/labs/mlrg/olcut/config/overrideIncorrect.json
@@ -0,0 +1,21 @@
+{
+  "config": {
+    "global-properties": {},
+    "components": [
+      {
+        "name": "a",
+        "type": "com.oracle.labs.mlrg.olcut.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/...onfig-protobuf/src/test/java/com/oracle/labs/mlrg/olcut/config/protobuf/OverrideTest.java b/...onfig-protobuf/src/test/java/com/oracle/labs/mlrg/olcut/config/protobuf/OverrideTest.java
@@ -31,6 +31,8 @@
 import com.oracle.labs.mlrg.olcut.config.ConfigurationManager;
 import com.oracle.labs.mlrg.olcut.config.StringConfigurable;
 import com.oracle.labs.mlrg.olcut.config.StringleConfigurable;
+import com.oracle.labs.mlrg.olcut.config.io.ConfigLoaderException;
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
@@ -47,6 +49,7 @@ public class OverrideTest {
     @BeforeAll
     public static void setUpClass() throws Exception {
         ConfigurationManager.addFileFormatFactory(new ProtoConfigFactory());
+        ConfigurationManager.addFileFormatFactory(new ProtoTxtConfigFactory());
     }
 
     @Test
@@ -97,4 +100,12 @@ 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("overrideIncorrect.pbtxt");
+                });
+    }
 }
diff --git a/...fig-protobuf/src/test/resources/com/oracle/labs/mlrg/olcut/config/overrideIncorrect.pbtxt b/...fig-protobuf/src/test/resources/com/oracle/labs/mlrg/olcut/config/overrideIncorrect.pbtxt
@@ -0,0 +1,20 @@
+components {
+  name: "a"
+  type: "com.oracle.labs.mlrg.olcut.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/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())) {

diff --git a/olcut-core/src/test/resources/com/oracle/labs/mlrg/olcut/config/overrideIncorrect.xml b/olcut-core/src/test/resources/com/oracle/labs/mlrg/olcut/config/overrideIncorrect.xml
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<config>
+
+    <component name="a" type="com.oracle.labs.mlrg.olcut.config.StringConfigurable">
+        <property name="one" value="a"/>
+        <property name="two" value="b"/>
+        <property name="three" value="c"/>
+    </component>
+
+    <component name="bErr" inherit="aErr"/>
+</config>