From 5a31f9ef5f571a77292a8dfadaa3bbea9d4cd6a6 Mon Sep 17 00:00:00 2001 From: "John J. Aylward" Date: Tue, 21 Jul 2020 11:08:40 -0400 Subject: [PATCH 1/2] Refs #541 Updates XML configuration to use a builder pattern instead of constructors with many parameters --- src/main/java/org/json/XML.java | 16 +- .../java/org/json/XMLParserConfiguration.java | 143 ++++++++++++++++-- .../java/org/json/junit/JSONArrayTest.java | 2 +- .../org/json/junit/XMLConfigurationTest.java | 50 +++--- src/test/java/org/json/junit/XMLTest.java | 5 +- 5 files changed, 178 insertions(+), 38 deletions(-) diff --git a/src/main/java/org/json/XML.java b/src/main/java/org/json/XML.java index c09fb035c..c81f15ff5 100644 --- a/src/main/java/org/json/XML.java +++ b/src/main/java/org/json/XML.java @@ -286,7 +286,7 @@ private static boolean parse(XMLTokener x, JSONObject context, String name, XMLP if (x.next() == '[') { string = x.nextCDATA(); if (string.length() > 0) { - context.accumulate(config.cDataTagName, string); + context.accumulate(config.getcDataTagName(), string); } return false; } @@ -350,13 +350,13 @@ private static boolean parse(XMLTokener x, JSONObject context, String name, XMLP throw x.syntaxError("Missing value"); } - if (config.convertNilAttributeToNull + if (config.isConvertNilAttributeToNull() && NULL_ATTR.equals(string) && Boolean.parseBoolean((String) token)) { nilAttributeFound = true; } else if (!nilAttributeFound) { jsonObject.accumulate(string, - config.keepStrings + config.isKeepStrings() ? ((String) token) : stringToValue((String) token)); } @@ -392,8 +392,8 @@ private static boolean parse(XMLTokener x, JSONObject context, String name, XMLP } else if (token instanceof String) { string = (String) token; if (string.length() > 0) { - jsonObject.accumulate(config.cDataTagName, - config.keepStrings ? string : stringToValue(string)); + jsonObject.accumulate(config.getcDataTagName(), + config.isKeepStrings() ? string : stringToValue(string)); } } else if (token == LT) { @@ -402,8 +402,8 @@ private static boolean parse(XMLTokener x, JSONObject context, String name, XMLP if (jsonObject.length() == 0) { context.accumulate(tagName, ""); } else if (jsonObject.length() == 1 - && jsonObject.opt(config.cDataTagName) != null) { - context.accumulate(tagName, jsonObject.opt(config.cDataTagName)); + && jsonObject.opt(config.getcDataTagName()) != null) { + context.accumulate(tagName, jsonObject.opt(config.getcDataTagName())); } else { context.accumulate(tagName, jsonObject); } @@ -748,7 +748,7 @@ public static String toString(final Object object, final String tagName, final X } // Emit content in body - if (key.equals(config.cDataTagName)) { + if (key.equals(config.getcDataTagName())) { if (value instanceof JSONArray) { ja = (JSONArray) value; int jaLength = ja.length(); diff --git a/src/main/java/org/json/XMLParserConfiguration.java b/src/main/java/org/json/XMLParserConfiguration.java index c0186f72d..cf5e10caa 100644 --- a/src/main/java/org/json/XMLParserConfiguration.java +++ b/src/main/java/org/json/XMLParserConfiguration.java @@ -24,44 +24,57 @@ of this software and associated documentation files (the "Software"), to deal */ /** - * Configuration object for the XML parser. + * Configuration object for the XML parser. The configuration is immutable. * @author AylwardJ - * */ +@SuppressWarnings({""}) public class XMLParserConfiguration { /** Original Configuration of the XML Parser. */ - public static final XMLParserConfiguration ORIGINAL = new XMLParserConfiguration(); + public static final XMLParserConfiguration ORIGINAL + = new XMLParserConfiguration(); /** Original configuration of the XML Parser except that values are kept as strings. */ - public static final XMLParserConfiguration KEEP_STRINGS = new XMLParserConfiguration(true); + public static final XMLParserConfiguration KEEP_STRINGS + = new XMLParserConfiguration().withKeepStrings(true); + /** - * When parsing the XML into JSON, specifies if values should be kept as strings (true), or if + * When parsing the XML into JSON, specifies if values should be kept as strings (true), or if * they should try to be guessed into JSON values (numeric, boolean, string) */ - public final boolean keepStrings; + private boolean keepStrings; + /** * The name of the key in a JSON Object that indicates a CDATA section. Historically this has * been the value "content" but can be changed. Use null to indicate no CDATA * processing. */ - public final String cDataTagName; + private String cDataTagName; + /** * When parsing the XML into JSON, specifies if values with attribute xsi:nil="true" - * should be kept as attribute(false), or they should be converted to null(true) + * should be kept as attribute(false), or they should be converted to + * null(true) */ - public final boolean convertNilAttributeToNull; + private boolean convertNilAttributeToNull; /** - * Default parser configuration. Does not keep strings, and the CDATA Tag Name is "content". + * Default parser configuration. Does not keep strings (tries to implicitly convert + * values), and the CDATA Tag Name is "content". */ public XMLParserConfiguration () { - this(false, "content", false); + this.keepStrings = false; + this.cDataTagName = "content"; + this.convertNilAttributeToNull = false; } /** * Configure the parser string processing and use the default CDATA Tag Name as "content". * @param keepStrings true to parse all values as string. * false to try and convert XML string values into a JSON value. + * @deprecated This constructor has been deprecated in favor of using the new builder + * pattern for the configuration. + * This constructor may be removed in a future release. */ + @Deprecated public XMLParserConfiguration (final boolean keepStrings) { this(keepStrings, "content", false); } @@ -72,7 +85,11 @@ public XMLParserConfiguration (final boolean keepStrings) { * disable CDATA processing * @param cDataTagNamenull to disable CDATA processing. Any other value * to use that value as the JSONObject key name to process as CDATA. + * @deprecated This constructor has been deprecated in favor of using the new builder + * pattern for the configuration. + * This constructor may be removed in a future release. */ + @Deprecated public XMLParserConfiguration (final String cDataTagName) { this(false, cDataTagName, false); } @@ -83,7 +100,11 @@ public XMLParserConfiguration (final String cDataTagName) { * false to try and convert XML string values into a JSON value. * @param cDataTagNamenull to disable CDATA processing. Any other value * to use that value as the JSONObject key name to process as CDATA. + * @deprecated This constructor has been deprecated in favor of using the new builder + * pattern for the configuration. + * This constructor may be removed in a future release. */ + @Deprecated public XMLParserConfiguration (final boolean keepStrings, final String cDataTagName) { this.keepStrings = keepStrings; this.cDataTagName = cDataTagName; @@ -98,10 +119,110 @@ public XMLParserConfiguration (final boolean keepStrings, final String cDataTagN * to use that value as the JSONObject key name to process as CDATA. * @param convertNilAttributeToNull true to parse values with attribute xsi:nil="true" as null. * false to parse values with attribute xsi:nil="true" as {"xsi:nil":true}. + * @deprecated This constructor has been deprecated in favor of using the new builder + * pattern for the configuration. + * This constructor may be removed or marked private in a future release. */ + @Deprecated public XMLParserConfiguration (final boolean keepStrings, final String cDataTagName, final boolean convertNilAttributeToNull) { this.keepStrings = keepStrings; this.cDataTagName = cDataTagName; this.convertNilAttributeToNull = convertNilAttributeToNull; } + + /** + * Provides a new instance of the same configuration. + */ + @Override + protected XMLParserConfiguration clone() { + // future modifications to this method should always ensure a "deep" + // clone in the case of collections. i.e. if a Map is added as a configuration + // item, a new map instance should be created and if possible each value in the + // map should be cloned as well. If the values of the map are known to also + // be immutable, then a shallow clone of the map is acceptable. + return new XMLParserConfiguration( + this.keepStrings, + this.cDataTagName, + this.convertNilAttributeToNull + ); + } + + /** + * When parsing the XML into JSON, specifies if values should be kept as strings (true), or if + * they should try to be guessed into JSON values (numeric, boolean, string) + * + * @return The {@link #keepStrings} configuration value. + */ + public boolean isKeepStrings() { + return this.keepStrings; + } + + /** + * When parsing the XML into JSON, specifies if values should be kept as strings (true), or if + * they should try to be guessed into JSON values (numeric, boolean, string) + * + * @param newVal + * new value to use for the {@link #keepStrings} configuration option. + * + * @return The existing configuration will not be modified. A new configuration is returned. + */ + public XMLParserConfiguration withKeepStrings(final boolean newVal) { + XMLParserConfiguration newConfig = this.clone(); + newConfig.keepStrings = newVal; + return newConfig; + } + + /** + * The name of the key in a JSON Object that indicates a CDATA section. Historically this has + * been the value "content" but can be changed. Use null to indicate no CDATA + * processing. + * + * @return The {@link #cDataTagName} configuration value. + */ + public String getcDataTagName() { + return this.cDataTagName; + } + + /** + * The name of the key in a JSON Object that indicates a CDATA section. Historically this has + * been the value "content" but can be changed. Use null to indicate no CDATA + * processing. + * + * @param newVal + * new value to use for the {@link #cDataTagName} configuration option. + * + * @return The existing configuration will not be modified. A new configuration is returned. + */ + public XMLParserConfiguration withcDataTagName(final String newVal) { + XMLParserConfiguration newConfig = this.clone(); + newConfig.cDataTagName = newVal; + return newConfig; + } + + /** + * When parsing the XML into JSON, specifies if values with attribute xsi:nil="true" + * should be kept as attribute(false), or they should be converted to + * null(true) + * + * @return The {@link #convertNilAttributeToNull} configuration value. + */ + public boolean isConvertNilAttributeToNull() { + return this.convertNilAttributeToNull; + } + + /** + * When parsing the XML into JSON, specifies if values with attribute xsi:nil="true" + * should be kept as attribute(false), or they should be converted to + * null(true) + * + * @param newVal + * new value to use for the {@link #convertNilAttributeToNull} configuration option. + * + * @return The existing configuration will not be modified. A new configuration is returned. + */ + public XMLParserConfiguration withConvertNilAttributeToNull(final boolean newVal) { + XMLParserConfiguration newConfig = this.clone(); + newConfig.convertNilAttributeToNull = newVal; + return newConfig; + } } diff --git a/src/test/java/org/json/junit/JSONArrayTest.java b/src/test/java/org/json/junit/JSONArrayTest.java index eda3c06bd..536b32d12 100644 --- a/src/test/java/org/json/junit/JSONArrayTest.java +++ b/src/test/java/org/json/junit/JSONArrayTest.java @@ -1131,7 +1131,7 @@ public void testJSONArrayInt() { assertNotNull(new JSONArray(5)); // Check Size -> Even though the capacity of the JSONArray can be specified using a positive // integer but the length of JSONArray always reflects upon the items added into it. - assertEquals(0l, (long)new JSONArray(10).length()); + assertEquals(0l, new JSONArray(10).length()); try { assertNotNull("Should throw an exception", new JSONArray(-1)); } catch (JSONException e) { diff --git a/src/test/java/org/json/junit/XMLConfigurationTest.java b/src/test/java/org/json/junit/XMLConfigurationTest.java index 14c4ba048..35365ea35 100755 --- a/src/test/java/org/json/junit/XMLConfigurationTest.java +++ b/src/test/java/org/json/junit/XMLConfigurationTest.java @@ -209,7 +209,7 @@ public void shouldHandleInvalidCDATABangInTag() { ""; try { XMLParserConfiguration config = - new XMLParserConfiguration("altContent"); + new XMLParserConfiguration().withcDataTagName("altContent"); XML.toJSONObject(xmlStr, config); fail("Expecting a JSONException"); } catch (JSONException e) { @@ -300,7 +300,7 @@ public void shouldHandleSimpleXML() { "XMLSchema-instance\"}}"; XMLParserConfiguration config = - new XMLParserConfiguration("altContent"); + new XMLParserConfiguration().withcDataTagName("altContent"); compareStringToJSONObject(xmlStr, expectedStr, config); compareReaderToJSONObject(xmlStr, expectedStr, config); compareFileToJSONObject(xmlStr, expectedStr); @@ -325,7 +325,7 @@ public void shouldHandleCommentsInXML() { " \n"+ ""; XMLParserConfiguration config = - new XMLParserConfiguration("altContent"); + new XMLParserConfiguration().withcDataTagName("altContent"); JSONObject jsonObject = XML.toJSONObject(xmlStr, config); String expectedStr = "{\"addresses\":{\"address\":{\"street\":\"Baker "+ "street 5\",\"name\":\"Joe Tester\",\"altContent\":\" this is -- "+ @@ -378,7 +378,7 @@ public void shouldHandleContentNoArraytoString() { String expectedStr = "{\"addresses\":{\"altContent\":\">\"}}"; JSONObject expectedJsonObject = new JSONObject(expectedStr); - XMLParserConfiguration config = new XMLParserConfiguration("altContent"); + XMLParserConfiguration config = new XMLParserConfiguration().withcDataTagName("altContent"); String finalStr = XML.toString(expectedJsonObject, null, config); String expectedFinalStr = ">"; assertTrue("Should handle expectedFinal: ["+expectedStr+"] final: ["+ @@ -395,7 +395,7 @@ public void shouldHandleContentArraytoString() { String expectedStr = "{\"addresses\":{\"altContent\":[1, 2, 3]}}"; JSONObject expectedJsonObject = new JSONObject(expectedStr); - XMLParserConfiguration config = new XMLParserConfiguration("altContent"); + XMLParserConfiguration config = new XMLParserConfiguration().withcDataTagName("altContent"); String finalStr = XML.toString(expectedJsonObject, null, config); String expectedFinalStr = ""+ "1\n2\n3"+ @@ -595,7 +595,9 @@ public void contentOperations() { // multiple consecutive standalone cdatas are accumulated into an array xmlStr = " 0) then return]]>"; jsonObject = XML.toJSONObject(xmlStr, - new XMLParserConfiguration(true, "altContent")); + new XMLParserConfiguration() + .withKeepStrings(true) + .withcDataTagName("altContent")); assertTrue("2. 3 items", 3 == jsonObject.length()); assertTrue("2. empty tag1", "".equals(jsonObject.get("tag1"))); assertTrue("2. empty tag2", "".equals(jsonObject.get("tag2"))); @@ -612,7 +614,9 @@ public void contentOperations() { */ xmlStr = "value 1"; jsonObject = XML.toJSONObject(xmlStr, - new XMLParserConfiguration(true, "altContent")); + new XMLParserConfiguration() + .withKeepStrings(true) + .withcDataTagName("altContent")); assertTrue("3. 2 items", 1 == jsonObject.length()); assertTrue("3. value tag1", "value 1".equals(jsonObject.get("tag1"))); @@ -623,7 +627,9 @@ public void contentOperations() { */ xmlStr = "value 12true"; jsonObject = XML.toJSONObject(xmlStr, - new XMLParserConfiguration(true, "altContent")); + new XMLParserConfiguration() + .withKeepStrings(true) + .withcDataTagName("altContent")); assertTrue("4. 1 item", 1 == jsonObject.length()); assertTrue("4. content array found", jsonObject.get("tag1") instanceof JSONArray); jsonArray = jsonObject.getJSONArray("tag1"); @@ -639,7 +645,9 @@ public void contentOperations() { */ xmlStr = "val1val2"; jsonObject = XML.toJSONObject(xmlStr, - new XMLParserConfiguration(true, "altContent")); + new XMLParserConfiguration() + .withKeepStrings(true) + .withcDataTagName("altContent")); assertTrue("5. 1 item", 1 == jsonObject.length()); assertTrue("5. jsonObject found", jsonObject.get("tag1") instanceof JSONObject); @@ -659,7 +667,9 @@ public void contentOperations() { */ xmlStr = "val1"; jsonObject = XML.toJSONObject(xmlStr, - new XMLParserConfiguration(true, "altContent")); + new XMLParserConfiguration() + .withKeepStrings(true) + .withcDataTagName("altContent")); assertTrue("6. 1 item", 1 == jsonObject.length()); assertTrue("6. jsonObject found", jsonObject.get("tag1") instanceof JSONObject); jsonObject = jsonObject.getJSONObject("tag1"); @@ -674,7 +684,9 @@ public void contentOperations() { */ xmlStr = "val1"; jsonObject = XML.toJSONObject(xmlStr, - new XMLParserConfiguration(true, "altContent")); + new XMLParserConfiguration() + .withKeepStrings(true) + .withcDataTagName("altContent")); assertTrue("7. 1 item", 1 == jsonObject.length()); assertTrue("7. jsonArray found", jsonObject.get("tag1") instanceof JSONArray); @@ -713,7 +725,9 @@ public void contentOperations() { "}"; jsonObject = new JSONObject(jsonStr); xmlStr = XML.toString(jsonObject, null, - new XMLParserConfiguration(true, "altContent")); + new XMLParserConfiguration() + .withKeepStrings(true) + .withcDataTagName("altContent")); /* * This is the created XML. Looks like content was mistaken for * complex (child node + text) XML. @@ -739,7 +753,7 @@ public void testToJSONArray_jsonOutput() { final String originalXml = "011000True"; final JSONObject expected = new JSONObject("{\"root\":{\"item\":{\"id\":\"01\"},\"id\":[\"01\",1,\"00\",0],\"title\":true}}"); final JSONObject actualJsonOutput = XML.toJSONObject(originalXml, - new XMLParserConfiguration(false)); + new XMLParserConfiguration().withKeepStrings(false)); Util.compareActualVsExpectedJsonObjects(actualJsonOutput,expected); } @@ -749,7 +763,7 @@ public void testToJSONArray_jsonOutput() { @Test public void testToJSONArray_reversibility() { final String originalXml = "011000True"; - XMLParserConfiguration config = new XMLParserConfiguration(false); + XMLParserConfiguration config = new XMLParserConfiguration().withKeepStrings(false); final String revertedXml = XML.toString(XML.toJSONObject(originalXml, config), null, config); @@ -765,7 +779,7 @@ public void testToJsonXML() { final JSONObject expected = new JSONObject("{\"root\":{\"item\":{\"id\":\"01\"},\"id\":[\"01\",\"1\",\"00\",\"0\"],\"title\":\"True\"}}"); final JSONObject json = XML.toJSONObject(originalXml, - new XMLParserConfiguration(true)); + new XMLParserConfiguration().withKeepStrings(true)); Util.compareActualVsExpectedJsonObjects(json, expected); final String reverseXml = XML.toString(json); @@ -850,7 +864,9 @@ public void testConfig() { // keep strings, use the altContent tag XMLParserConfiguration config = - new XMLParserConfiguration(true, "altContent"); + new XMLParserConfiguration() + .withKeepStrings(true) + .withcDataTagName("altContent"); JSONObject jsonObject = XML.toJSONObject(xmlStr, config); // num is parsed as a string assertEquals(jsonObject.getJSONObject("addresses"). @@ -875,7 +891,7 @@ public void testConfig() { Util.compareActualVsExpectedJsonArrays(jsonArray, expectedJsonArray); // use alternate content name - config = new XMLParserConfiguration("altContent"); + config = new XMLParserConfiguration().withcDataTagName("altContent"); jsonObject = XML.toJSONObject(xmlStr, config); // num is parsed as a number assertEquals(jsonObject.getJSONObject("addresses"). diff --git a/src/test/java/org/json/junit/XMLTest.java b/src/test/java/org/json/junit/XMLTest.java index d59496133..b43aadf8b 100644 --- a/src/test/java/org/json/junit/XMLTest.java +++ b/src/test/java/org/json/junit/XMLTest.java @@ -883,7 +883,10 @@ public void testToJsonWithNullWhenNilConversionEnabled() { final String originalXml = ""; final String expectedJsonString = "{\"root\":{\"id\":null}}"; - final JSONObject json = XML.toJSONObject(originalXml, new XMLParserConfiguration(false, "content", true)); + final JSONObject json = XML.toJSONObject(originalXml, + new XMLParserConfiguration() + .withKeepStrings(false).withcDataTagName("content") + .withConvertNilAttributeToNull(true)); assertEquals(expectedJsonString, json.toString()); } From 4f1c8b2d6fcdfe1a86784c2f0efb17f02156e7b0 Mon Sep 17 00:00:00 2001 From: "John J. Aylward" Date: Tue, 21 Jul 2020 11:30:35 -0400 Subject: [PATCH 2/2] formatting --- src/test/java/org/json/junit/XMLTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/json/junit/XMLTest.java b/src/test/java/org/json/junit/XMLTest.java index b43aadf8b..882a69062 100644 --- a/src/test/java/org/json/junit/XMLTest.java +++ b/src/test/java/org/json/junit/XMLTest.java @@ -885,7 +885,8 @@ public void testToJsonWithNullWhenNilConversionEnabled() { final JSONObject json = XML.toJSONObject(originalXml, new XMLParserConfiguration() - .withKeepStrings(false).withcDataTagName("content") + .withKeepStrings(false) + .withcDataTagName("content") .withConvertNilAttributeToNull(true)); assertEquals(expectedJsonString, json.toString()); }