Changes NULL handling in XML output #430
Conversation
fixes stleary#429. Updates JSON-> XML conversion to treat all NULL types the same. Previously a JSONObject having a key with a JSONObject.NULL value would output `<key>null</key>`. However, a JSONObject having a key with a value of JAVA `null` would output as `<key />`. This change unifies the output so both now output as `<key />`.
|
We have leeway for XML changes because of the imperfect nature of the XML / JSON transform. Does this change also fix a bug in the JSONML implementation? If so, is the bug so serious that it justifies changing existing behavior? Is there a way to implement the change such that it does not impact JSONML? |
|
I'll take a look. At the moment work is pretty busy, but I'll try to get to this in the next week or so. |
|
Thanks! |
|
I would like to have a look at it, too. But I don't understand this comment:
What is the bug in the JSONML? Why should it not be fixed? |
|
I'm not sure I'd label it a bug per-say... before this PR, the 2 different tag types would generate empty strings: <root>
<Tag1 />
<Tag2></Tag2>
<root>The JSONML would generate JSON that indicated Tag1 and Tag2 are empty strings. After the change, JSONML would indicate that Tag1 is null, and Tag2 is an empty string. I still need to check the JSONML spec to verify if that is a proper interpretation. |
|
JSONML behavior should not be changed, or a case made as to why it is currently incorrect. |
|
What happened to these changes? I don't see any of them in the latest releases. |
|
they were abandoned as I did not have time to validate the JSONML portions |
What problem does this code solve?
Adjusts null handling in XML output in reference to #429.
Risks
Due to the change in
nullhandling, release notes should be updated to reflect the change in behavior. Previously,{ "key": null }would output to<key>null</key>. It will now output to<key/>. Also, empty string values""would output to<key/>, but will now output to<key></key>.Changes to the API?
None.
Will this require a new release?
Yes
Should the documentation be updated?
Yes
Does it break the unit tests?
Yes. New unit test are provided in a separate PR (stleary/JSON-Java-unit-test#87)
Was any code refactored in this commit?
no
Review status
NOT REVIEWED