Strange NumberFormatException when having maps with Long keys

[MrTetrixx]

Hello, I am using zjsonpatch in a project and I really like it.

I ran some of our data through it and came across a strange error. I have not looked at the zjsonpatch code yet but from the stacktrace it looks like it is doing Integer.parseInt() on that Long value used as a key in the map.

The stacktrace:

Exception in thread "main" java.lang.NumberFormatException: For input string: "2407321151"
	at java.base/java.lang.NumberFormatException.forInputString(NumberFormatException.java:65)
	at java.base/java.lang.Integer.parseInt(Integer.java:652)
	at java.base/java.lang.Integer.parseInt(Integer.java:770)
	at com.flipkart.zjsonpatch.JsonDiff.isAllowed(JsonDiff.java:104)
	at com.flipkart.zjsonpatch.JsonDiff.introduceCopyOperation(JsonDiff.java:76)
	at com.flipkart.zjsonpatch.JsonDiff.asJson(JsonDiff.java:60)
	at com.flipkart.zjsonpatch.JsonDiff.asJson(JsonDiff.java:38)
	at JsonDiffTest.main(JsonDiffTest.java:33)

Code to replicate the issue:

public class JsonDiffTest {
    public static void main(String[] args) throws IOException {
        String a = "{\n" +
                "    \"map\": {\n" +
                "      \"3000000000\": {\n" +
                "        \"field\": 3100000000,\n" +
                "        \"otherField\": 0\n" +
                "      }\n" +
                "    }\n" +
                "  }";

        String b = "{\n" +
                "    \"map\": {\n" +
                "      \"3000000000\": {\n" +
                "        \"field\": 3100000000,\n" +
                "        \"otherField\": 0,\n" +
                "        \"extraField\": 0\n" +
                "      }\n" +
                "    }\n" +
                "  }";

        ObjectMapper mapper = new ObjectMapper();
        JsonNode jsonA = mapper.readTree(a);
        JsonNode jsonB = mapper.readTree(b);

        JsonDiff.asJson(jsonA, jsonB); // causes above stacktrace
    }
}

What makes it strange is that

• if you remove either of otherField or extraField it works.
• if you change the value of either of the fields with value 0 to anything else it works too.

[vishwakarma]

Thanks for raising the issue, please allow me a day to look into the issue, I will get back on it very very soon.

[vishwakarma]

@MrTetrixx This clearly a bug, line #104 & # 105, should be replaced by Long parsing ( this way it wont lose the necessary precision ).
Would you like to raise a PR for same or shall I take over on it?

[MrTetrixx]

You can take over, no problem.

Please check if there are any other issues of using unusual keys in a map, like BigDecimal or something.

I extended the test with a real assertion:

        String a = "{" +
                "    \"map\": {" +
                "      \"3000000000\": {" +
                "        \"field\": 3100000000," +
                "        \"otherField\": 0" +
                "      }" +
                "    }" +
                "  }";

        String b = "{" +
                "    \"map\": {" +
                "      \"3000000000\": {" +
                "        \"field\": 3100000000," +
                "        \"otherField\": 0," +
                "        \"extraField\": 0" +
                "      }" +
                "    }" +
                "  }";

        JsonNode jsonA = objectMapper.readTree(a);
        JsonNode jsonB = objectMapper.readTree(b);

        JsonNode diff = JsonDiff.asJson(jsonA, jsonB);

        assertEquals(Operation.ADD.rfcName(), diff.get(0).get("op").textValue());
        assertEquals("/map/3000000000/extraField", diff.get(0).get("path").textValue());
        assertEquals("value", diff.get(0).get("value").textValue());

Should this test return a copy or an add operation? How does it decide between them?

[LouizFC]

@MrTetrixx copy comes over add when you create a field that has a value that is already present on your json.

In your case, you are creating a field called "extraField" with the same value as "otherField", so the copy takes "priority".

The library basically only generates Add and Remove operations if I am not wrong, then these operations are compacted into replace, move and copy operations. You can control this behavior using Flags

[MrTetrixx]

Yes, I guess I can set the OMIT_COPY_OPERATION and OMIT_MOVE_OPERATION flags to only get add, remove and replace.

[vishwakarma]

@MrTetrixx , i have put the fix in #70 , would you like to test it?

[MrTetrixx]

I have run our data through the diff again and it looks much better.
When will the fix be released?

[vishwakarma]

I am currently on vacation, I will release it on coming Thursday.

…

On Tue, Apr 10, 2018, 1:37 PM MrTetrixx ***@***.***> wrote: I have run our data through the diff again and it looks much better. When will the fix be released? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#69 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB-YqQ8j8G5H31ezFSps5YfZhipXYmJTks5tnGgvgaJpZM4TFQVA> .

[MrTetrixx]

Great, looking forward to it!
(also stop checking github and enjoy your vacation 🍹)

[vishwakarma]

released 0.4.4, please use it.
Thank you for raising the bug