From 92289eae77d7767bc8c12186d9155eec10b55172 Mon Sep 17 00:00:00 2001 From: Adam Pocock Date: Sun, 26 Jan 2025 16:12:01 -0500 Subject: [PATCH 1/2] Adding better error messages for JSON parsing. --- .../mlrg/olcut/config/json/JsonLoader.java | 44 ++++++---- .../config/json/test/TypeCheckingTest.java | 80 +++++++++++++++++++ .../json/test/typeCheckingListConfig.json | 62 ++++++++++++++ .../json/test/typeCheckingMapConfig.json | 16 ++++ .../json/test/typeCheckingPropertyConfig.json | 18 +++++ 5 files changed, 204 insertions(+), 16 deletions(-) create mode 100644 olcut-config-json/src/test/java/com/oracle/labs/mlrg/olcut/config/json/test/TypeCheckingTest.java create mode 100644 olcut-config-json/src/test/resources/com/oracle/labs/mlrg/olcut/config/json/test/typeCheckingListConfig.json create mode 100644 olcut-config-json/src/test/resources/com/oracle/labs/mlrg/olcut/config/json/test/typeCheckingMapConfig.json create mode 100644 olcut-config-json/src/test/resources/com/oracle/labs/mlrg/olcut/config/json/test/typeCheckingPropertyConfig.json 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 index 513fbcad..04d3488d 100644 --- 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 @@ -291,20 +291,26 @@ protected void parseComponent(ObjectNode node) { while (listElementItr.hasNext()) { Entry elementEntry = listElementItr.next(); String elementName = elementEntry.getKey(); - switch (elementName) { - case ConfigLoader.ITEM: - listOutput.add(new SimpleProperty(elementEntry.getValue().textValue())); - break; - case ConfigLoader.TYPE: - try { - classListOutput.add(Class.forName(elementEntry.getValue().textValue())); - } catch (ClassNotFoundException cnfe) { - throw new ConfigLoaderException("Unable to find class " - + elementEntry.getValue().textValue() + " in component " + curComponent + ", propertylist " + propName); - } - break; - default: - throw new ConfigLoaderException("Unknown node in component " + curComponent + ", propertylist " + propName + ", node = " + e.getValue().toString()); + if (elementEntry.getValue().isTextual()) { + String value = elementEntry.getValue().textValue(); + switch (elementName) { + case ConfigLoader.ITEM: + listOutput.add(new SimpleProperty(value)); + break; + case ConfigLoader.TYPE: + try { + classListOutput.add(Class.forName(value)); + } catch (ClassNotFoundException cnfe) { + throw new ConfigLoaderException("Unable to find class " + + value + " in component " + curComponent + ", propertylist " + propName); + } + break; + default: + throw new ConfigLoaderException("Unknown node in component " + curComponent + ", propertylist " + propName + ", node = " + e.getValue().toString()); + } + } else { + throw new ConfigLoaderException("Invalid value in component " + curComponent + ", propertylist " + propName + ", node = " + e.getValue().toString() + "" + + ", all OLCUT values must be strings, numerical types are not parsed."); } } } @@ -324,13 +330,19 @@ protected void parseComponent(ObjectNode node) { if (mapEntry.getValue().isTextual()) { mapOutput.put(mapEntry.getKey(), new SimpleProperty(mapEntry.getValue().textValue())); } else { - throw new ConfigLoaderException("Unknown node in component " + curComponent + ", propertymap " + propName + ", node = " + e.getValue().toString()); + throw new ConfigLoaderException("Invalid value in component " + curComponent + ", propertymap " + propName + ", node = " + e.getValue().toString() + + ", all OLCUT values must be strings, numerical types are not parsed."); } } rpd.add(propName, new MapProperty(mapOutput)); } else { // Generic property. - rpd.add(propName, new SimpleProperty(e.getValue().textValue())); + if (e.getValue().isTextual()) { + rpd.add(propName, new SimpleProperty(e.getValue().textValue())); + } else { + throw new ConfigLoaderException("Invalid value in component " + curComponent + ", property " + propName + ", node = " + e.getValue().toString() + + ", all OLCUT values must be strings, numerical types are not parsed."); + } } } } diff --git a/olcut-config-json/src/test/java/com/oracle/labs/mlrg/olcut/config/json/test/TypeCheckingTest.java b/olcut-config-json/src/test/java/com/oracle/labs/mlrg/olcut/config/json/test/TypeCheckingTest.java new file mode 100644 index 00000000..da5ce2a1 --- /dev/null +++ b/olcut-config-json/src/test/java/com/oracle/labs/mlrg/olcut/config/json/test/TypeCheckingTest.java @@ -0,0 +1,80 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. + * + * Licensed under the 2-clause BSD license. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ + +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 org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import java.io.IOException; + +import static com.oracle.labs.mlrg.olcut.config.ConfigurationManager.createModuleResourceString; +import static org.junit.jupiter.api.Assertions.fail; + +public class TypeCheckingTest { + + @BeforeAll + public static void setUp() { + ConfigurationManager.addFileFormatFactory(new JsonConfigFactory()); + } + + @Test + public void arrayTest() throws IOException { + try { + ConfigurationManager cm = new ConfigurationManager(createModuleResourceString(this.getClass(), "typeCheckingListConfig.json")); + fail("Should have thrown ConfigLoaderException"); + } catch (ConfigLoaderException e) { + Assertions.assertTrue(e.getMessage().startsWith("Invalid value")); + } + } + + @Test + public void propertyTest() throws IOException { + try { + ConfigurationManager cm = new ConfigurationManager(createModuleResourceString(this.getClass(), "typeCheckingPropertyConfig.json")); + fail("Should have thrown ConfigLoaderException"); + } catch (ConfigLoaderException e) { + Assertions.assertTrue(e.getMessage().startsWith("Invalid value")); + } + } + + @Test + public void mapTest() throws IOException { + try { + ConfigurationManager cm = new ConfigurationManager(createModuleResourceString(this.getClass(), "typeCheckingMapConfig.json")); + fail("Should have thrown ConfigLoaderException"); + } catch (ConfigLoaderException e) { + Assertions.assertTrue(e.getMessage().startsWith("Invalid value")); + } + } + +} diff --git a/olcut-config-json/src/test/resources/com/oracle/labs/mlrg/olcut/config/json/test/typeCheckingListConfig.json b/olcut-config-json/src/test/resources/com/oracle/labs/mlrg/olcut/config/json/test/typeCheckingListConfig.json new file mode 100644 index 00000000..5314745b --- /dev/null +++ b/olcut-config-json/src/test/resources/com/oracle/labs/mlrg/olcut/config/json/test/typeCheckingListConfig.json @@ -0,0 +1,62 @@ +{ + "config" : { + "global-properties" : { }, + "components" : [ { + "name" : "a", + "type" : "com.oracle.labs.mlrg.olcut.test.config.ArrayConfigurable", + "export" : "false", + "import" : "false", + "properties" : { + "floatArray" : [ { + "item" : "1.1f" + }, { + "item" : "2.3" + }, { + "item" : "3.5" + } ], + "doubleArray" : [ { + "item" : 1e-16 + }, { + "item" : 2e-16 + }, { + "item" : "3.16" + } ], + "longArray" : [ { + "item" : 9223372036854775807 + }, { + "item" : "9223372036854775806" + }, { + "item" : "5" + } ], + "byteArray" : [ { + "item" : "1" + }, { + "item" : "2" + }, { + "item" : "3" + } ], + "shortArray" : [ { + "item" : "1" + }, { + "item" : "2" + }, { + "item" : "3" + } ], + "intArray" : [ { + "item" : 1 + }, { + "item" : 2 + }, { + "item": "3" + } ], + "charArray" : [ { + "item" : "a" + }, { + "item" : "b" + }, { + "item" : "c" + } ] + } + } ] + } +} \ No newline at end of file diff --git a/olcut-config-json/src/test/resources/com/oracle/labs/mlrg/olcut/config/json/test/typeCheckingMapConfig.json b/olcut-config-json/src/test/resources/com/oracle/labs/mlrg/olcut/config/json/test/typeCheckingMapConfig.json new file mode 100644 index 00000000..281c77e1 --- /dev/null +++ b/olcut-config-json/src/test/resources/com/oracle/labs/mlrg/olcut/config/json/test/typeCheckingMapConfig.json @@ -0,0 +1,16 @@ +{ + "config" : { + "components" : [ { + "name" : "mapTest", + "type" : "com.oracle.labs.mlrg.olcut.test.config.MapConfigurable", + "export" : "false", + "import" : "false", + "properties" : { + "map" : { + "foo" : "quux", + "things" : 5 + } + } + } ] + } +} \ No newline at end of file diff --git a/olcut-config-json/src/test/resources/com/oracle/labs/mlrg/olcut/config/json/test/typeCheckingPropertyConfig.json b/olcut-config-json/src/test/resources/com/oracle/labs/mlrg/olcut/config/json/test/typeCheckingPropertyConfig.json new file mode 100644 index 00000000..106600f9 --- /dev/null +++ b/olcut-config-json/src/test/resources/com/oracle/labs/mlrg/olcut/config/json/test/typeCheckingPropertyConfig.json @@ -0,0 +1,18 @@ +{ + "config": { + "global-properties": {}, + "components": [ + { + "name": "a", + "type": "com.oracle.labs.mlrg.olcut.test.config.BasicConfigurable", + "export": "false", + "import": "false", + "properties": { + "s": "one", + "d": 3.0, + "i": 2 + } + } + ] + } +} From cd568e81bd60e00d6657fc34a127e617f0064759 Mon Sep 17 00:00:00 2001 From: Adam Pocock Date: Tue, 28 Jan 2025 16:20:04 -0500 Subject: [PATCH 2/2] Updating exception message. --- .../com/oracle/labs/mlrg/olcut/config/json/JsonLoader.java | 6 +++--- 1 file changed, 3 insertions(+), 3 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 index 04d3488d..0ecc3da1 100644 --- 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 @@ -310,7 +310,7 @@ protected void parseComponent(ObjectNode node) { } } else { throw new ConfigLoaderException("Invalid value in component " + curComponent + ", propertylist " + propName + ", node = " + e.getValue().toString() + "" + - ", all OLCUT values must be strings, numerical types are not parsed."); + ", all OLCUT property list values must be strings, other types are not parsed."); } } } @@ -331,7 +331,7 @@ protected void parseComponent(ObjectNode node) { mapOutput.put(mapEntry.getKey(), new SimpleProperty(mapEntry.getValue().textValue())); } else { throw new ConfigLoaderException("Invalid value in component " + curComponent + ", propertymap " + propName + ", node = " + e.getValue().toString() + - ", all OLCUT values must be strings, numerical types are not parsed."); + ", all OLCUT property map values must be strings, other types are not parsed."); } } rpd.add(propName, new MapProperty(mapOutput)); @@ -341,7 +341,7 @@ protected void parseComponent(ObjectNode node) { rpd.add(propName, new SimpleProperty(e.getValue().textValue())); } else { throw new ConfigLoaderException("Invalid value in component " + curComponent + ", property " + propName + ", node = " + e.getValue().toString() + - ", all OLCUT values must be strings, numerical types are not parsed."); + ", all OLCUT property values must be strings, other types are not parsed."); } } }