Repository navigation
Update XML with recent changes for JSONObject optLong vs getLong #790
Description
Activity
I would like to work on this
- added a commit that references this issue
on Oct 14, 2023 @stleary - I have raised a PR for this. The number conversion and potential number check is moved to a separate utility and that has been used in both parsers.
@rafu01 - I already had the code pattern chalked out, and have implemented. Please go through the PR and let me know if you had thought it to be implemented other way.Comments have been added to the PR
@stleary - Adding these changes into XML is going to affect some of the existing unit test cases.
e.g.,
/**
* JSON string with lost leading zero and converted "True" to true. See test
* result in comment below.
*/
@test
public void testToJSONArray_jsonOutput() {
final String originalXml = "011000<item id="01"/><title>True</title>";
final String expectedJsonString = "["root",["id","01"],["id",1],["id","00"],["id",0],["item",{"id":"01"}],["title",true]]";
final JSONArray actualJsonOutput = JSONML.toJSONArray(originalXml, false);
assertEquals(expectedJsonString, actualJsonOutput.toString());
}In the comment of the test case, it is mentioned that leading zeros being removed is the expectation. But, in the test case expectation string, leading zeros are kept intact. This is caused by the present implementation, which throws NumberFormatException while parsing number with leading zeros and numbers are parsed as strings.
And, when this logic will be replaced by the new implementation, numbers with leading zeros will be parsed as numbers, and will reflect in the resultant JSON object. This probably is the intended behavior.
Question lies, shall we go ahead and break the existing test cases, or maintain status quo?
@rudrajyotib Thanks for bringing this up.
Not sure about the relevance of the comment, it looks like its just documenting the test behavior.
The general rule is don't break the unit tests. But sometimes it has to be done.
XML is a special case because of the imperfect transformation with JSON, and by extension so is JSONML, so sometimes we can allow for more flexibility in the behavior.
I will take a look and respond later today.Reacted by Rudrajyoti Biswas- added a commit that references this issue
on Oct 19, 2023
See #783 and apply the same updates to XML.java for consistent behavior.
Post here if you have questions about how to implement the changes.