Added type conversion support

[kumar529]

The lib doesn't support type conversion for a specific value.
I have added the feature to support type conversion for a specific tag.
Example:

//XML String
<root><id1 xsi:type="java.lang.String">1234</id1><id2 xsi:type="java.lang.Integer">1234</id2></root>

//Corresponding JSON String
{"root":{"id2":1234,"id1":"1234"}}

[stleary]

Sounds like you are trying to make the case that Java-specific XML xsi:type strings should be supported. If so, is this commonly used by projects other than yours? Otherwise, no real objection since we allow a bit more latitude for supporting XML to JSON transformations, which will always be imperfect.

We already have too many fixed parameters for XMLParserConfiguration. This needs to be fixed first. The existing constructors need to be deprecated, the properties made non-final, and setters added to set up the config manually. I will add an issue to address this.

[johnjaylward]

@kumar529 would it make sense to have mappings for xsi:type as a HashMap instead of a boolean?

i.e.

/**
Interface to support conversions of XML xsl:type values to Java objects
*/
public interface XMLXsiTypeConverter<T> {
    public T convert(String value);
}

public void someFunc() {
Map<string, XsiTypeConverter<?>> XsiTypeMap = new HashMap<>();
XsiTypeMap.put("string", new XMLXsiTypeConverter<String>(){ public String convert(String value){return value;}});
XsiTypeMap.put("int", new XMLXsiTypeConverter<Long>(){ public Long convert(String value){return Long.valueOf(value);}});
XsiTypeMap.put("bool", new XMLXsiTypeConverter<Long>(){ public Boolean convert(String value){return Boolean.FALSE;}});

XMLParserConfiguration config = new XMLParserConfiguration(false, "content", true, XsiTypeMap);

JSONObject jo = XML.toJSONObject(myInput, config);

}

[stleary]

Let's try the simplest solution first, and see if it works to everyone's satisfaction.

Sorry, ignore, thought that question was addressed to me.

[kumar529]

@johnjaylward I think HashMap will not serve the purpose.
Because I want to add data type support on an element.

@stleary I am facing the problem of data conversion. Currently, the lib supports two types of config for data conversion:

1. Convert all values to the respective data types: If the value is an integer in XML, it will convert it to an integer in JSON. So sometimes the conversion changes the values. It will convert the value to scientific notation.

2. Convert all values to a string: In this case, if want some integer as an actual integer in JSON. Then using this config, we will not get it.

So I want to add type support on specific elements.

For example:
<root><id1 xsi:type="java.lang.String">1234</id1><id2 xsi:type="java.lang.Integer">1234</id2><id3>1234</id3></root>

Here the value for id1 will be converted to a string, id2 will be converted to an integer, id3 has no type information so default behavior will be applicable.

We can do without adding any configuration change. If xsi:type is present, it will try to convert only. @stleary Please suggest what is your opinion.

We can also use "string", "number", "boolean" for type representation and we can handle these cases from code.

[johnjaylward]

@kumar529, that was why I suggested the hash map. The hashmap is to map the xsi:type to the java type.

The xsi:type would then be configurable per application. You don't need to pick how the type is represented in the xml now. Instead configure it at runtime with converters. I don't think we should presume that all providers of XML will have the same xsi:type schema, nor that all consumers will want conversions done the same way.

[stleary]

Hi @johnjaylward

>>>If the value is an integer in XML, it will convert it to an integer in JSON. So sometimes the conversion changes the values.
>>>It will convert the value to scientific notation.

Can it be confirmed that this is not a regression introduced in #453 or one of the subsequent BigNumber/Decimal/Integer commits?

[kumar529]

@johnjaylward Yes, HashMap can work. The client will provide the HashMap at the time of initialization.

[stleary]

@kumar529 This PR has several issues -

• XMLTest has changed due to a recent commit, merge is needed.
• Additional XMLParserConfiguration, XML, and XMLTest changes are pending in Refactor XMLConfiguration to use Builder Pattern #543, merge will probably be needed in a few days.
  - We still don't know if this observation is due to an underlying bug: sometimes the conversion changes the values.

[johnjaylward]

@stleary I think the "changing values" is due to not using the "keep strings" option (default parsing). The XSI processing as proposed here would basically be another flavor of "keep strings" but lets generators of XML specify the actual types instead of the tokener always using a string value.

[stleary]

See conversation comments.

[kumar529]

@stleary Sure, I will make the change.
I think the idea of passing HashMap as configurations suggested by @johnjaylward looks good.
So I will try to do this.

[johnjaylward]

Please add a test case in that tests an XML structure that looks something like this:

<root>
<asString xsi:type="string">12345</asString>
<asInt>54321</asInt>
</root>

I would expect a resulting JSON that looks similar to this:

{
"asString":"12345",
"asInt":54321
}

[stleary]

Please do not modify deprecated functions, or use them in your tests.

[johnjaylward]

Sorry, I finally had time for a closer look at the configuration changes. With the new changes in #543, this should be private with a Getter and no Setter. Preferably the getter should return an "Unmodifiable Map". To prevent wrapping the collection every time the getter is called, storing the unmodifiable map in this variable would be acceptable.

Note in the Getter javadoc that the map is "unmodifiable"

[johnjaylward]

This can be marked private instead of public.
Also, please change:

this.xsiTypeMap = xsiTypeMap

to:

this.xsiTypeMap = Collections.unmodifiableMap(new HashMap<String, XMLXsiTypeConverter<?>>(xsiTypeMap));

Our expected configuration should look something like this:

XMLParserConfiguration xmlConfig = new XMLParserConfiguration().withKeepStrings(false).WithXsiTypeMap(myXsiMap);

// use the config

After marking this private, also update the "clone" method (currently line 165). Be sure to follow the comment that is included in the "clone" method. If changing the constructor above to do the shallow clone/unmodifiable map wrapping, you can probably just update the clone to call this private constructor.

Lastly, be sure to create the new "with" method that will take the XSI:Type conversion map.

[stleary]

Manual gradlew build is failing due to javadoc problems, please fix.

@johnjaylward Do you have any more un-addressed issues?
Otherwise, after this PR is merged, I would like to follow up with a PR that removes clone() and the new XMLParserConfiguration ctor that @kumar529 just added. Any objections?

[johnjaylward]

@stleary
The test case already there.

    @Test
    public void testToJsonWithPartialXSITypeWhenTypeConversionEnabled() {
        String originalXml = "<root><asString xsi:type=\"string\">12345</asString><asInt>54321</asInt></root>";
        String expectedJsonString = "{\"root\":{\"asString\":\"12345\",\"asInt\":54321}}";
        JSONObject expectedJson = new JSONObject(expectedJsonString);
        Map<String, XMLXsiTypeConverter<?>> xsiTypeMap = new HashMap<String, XMLXsiTypeConverter<?>>();
        xsiTypeMap.put("string", new XMLXsiTypeConverter<String>() {
            @Override public String convert(final String value) {
                return value;
            }
        });
        JSONObject actualJson = XML.toJSONObject(originalXml, new XMLParserConfiguration().withXsiTypeMap(xsiTypeMap));
        Util.compareActualVsExpectedJsonObjects(actualJson,expectedJson);
    }

I do not see this test case in the change set. The XML I see tested is:

<root><asString xsi:type="string">12345</asString><asInt xsi:type="integer">54321</asInt></root>

NOT

<root><asString xsi:type="string">12345</asString><asInt>54321</asInt></root>

[kumar529]

@stleary @johnjaylward Please let me know if there is anything pending in this PR.

[johnjaylward]

@kumar529, I believe that my concerns have been addressed. Sorry I did not notice your most recent commit until you commented.

[stleary]

What problem does this code solve?
Provides additional support for XML / JSON transformation via xsitype, by adding an xsiTypeMap to XMLParserConfiguration. This is an enhancement, not a bug fix. Allowed because additional flexibility is allowed for XML / JSON code.

Risks
Low

Changes to the API?
No

Will this require a new release?
No

Should the documentation be updated?
No

Does it break the unit tests?
No

Was any code refactored in this commit?
No

Review status
APPROVED
Starting 3 day comment window.