Jackson handling of numbers is wrong?

[tlefevre]

Groovy script to demonstrate the problem:

@Grab(group='com.fasterxml.jackson.core', module='jackson-databind', version='2.9.10.3')
// @Grab(group='com.google.code.gson', module='gson', version='2.8.6')
@Grab(group='net.javacrumbs.json-unit', module='json-unit', version='2.17.0')

import static net.javacrumbs.jsonunit.JsonAssert.assertJsonEquals

class Test {

    BigDecimal demo;

    Test(BigDecimal demo) {
        this.demo = demo
    }
}

Test expected = new Test(new BigDecimal("2.00"))
String actual = "{ \"demo\": 2.00 }"

assertJsonEquals(expected, actual)

org.opentest4j.AssertionFailedError: JSON documents are different:
Different value found in node "demo", expected: <2> but was: <2.0>.

But why??

Using gson, it works just fine.

[lukas-krecan]

https://github.com/lukas-krecan/JsonUnit#numbers

[tlefevre]

All well and fine, but this still doesn't make sense to me.

Could you explain to me what part of that links is relevant?

The numbers are exactly the same, so there's no reason to add tolerance.

[lukas-krecan]

Sorry, I mis-read the example, will take a look at it.

[tlefevre]

For the string value the Jackson mapper produces a Double node with a value of 2.0
For the object value the Jackson mapper produces a Decimal node with a value of 2, scale 0.

That comparison then fails. I can't debug to the place where it happens. You would probably know.

[tlefevre]

For context - I ran into this problem when upgrading from Grails 3.3 to 4.0.

3.3 uses Jackson databind in version 2.8.11.3 and Grails 4.0 uses databind in version 2.9.10.3.
For some reason, my test the script is based on works in Grails 3.3 but fails in 4.0.

[lukas-krecan]

You are right, Jackson indeed converts 2.0 to Decimal node with a value of 2, scale 0. There is not much I can do about it. You can either

• set tolerance to 0, which would fix it
• serialize the object to String before comparison
• configure Jackson (I do not know if it's possible, JsonUnit provides support for custom Jackson configuration)

[tlefevre]

Digging into it, it seems that both nodes are counted as "object nodes" and compared as such.

Why aren't they caught as Number nodes? It seems there's something wrong with the way jackson nodes are handled.

I'll attach a screenshot.

[tlefevre]

[lukas-krecan]

Actually, if I remember it correctly the one parsed from String is DoubleNode (on the screenshot) an the one converted from object is DecimalNode. if you are debugging it, it's this line in JsonUnit https://github.com/lukas-krecan/JsonUnit/blob/master/json-unit-core/src/main/java/net/javacrumbs/jsonunit/core/internal/Diff.java#L329

[tlefevre]

I tracked it down all the way.

It ends with 2 BigDecimal nodes. One is 2, scale 0, precision 1 and the other is 2.0, scale 1, precision 2.

Isn't there a more generic way to compare numbers? Perhaps check if both implement compareTo and are of the same type? Then use compareTo instead of equals?

[lukas-krecan]

Yes, that's exactly what happens if you set tolerance to 0

[tlefevre]

And I guess you don't think that's a better way to do it in general?
Ie. if tolerance is not set, treat is at default 0?

Edit:
https://json-schema.org/understanding-json-schema/reference/numeric.html#integer
https://tools.ietf.org/html/rfc7159#page-6

It doesn't seem like JSON really has a distinction between integers and decimals. Do you not end up drawing more conclusions than reasonable from libraries such as jackson in this case? The most precise way to compare numbers from a json document would be specifically as it is written, ie not interpret it as a number at all or do a mathematical comparison (force to same type, ie bigdecimal and then compare). To me that seems like the most reasonable way to do it.

As an option, users could always just enable something else than jackson in the test compile scope and for that matter I'd guess it's good that Jackson is the last least preferred node factory.

[lukas-krecan]

JsonUnit is first and foremost unit testing library. So when I started the library it seemed reasonable that the default behavior should be exact match (equals in Java). There are actually several reasonable use-cases

1. Compare for Java equality (equals)
2. Compare for mathematical equality (compareTo())
3. Just check that it's a number
   I have no idea which of the use-cases is more common, I only hear from people like you, which would prefer different default, I do not hear from the people who are happy with option 1. So unless I have data to show me that option 1. is clearly wrong, I will stick to it to maintain backward compatibility.

Luckily we are programmers so you can always write a one-line wrapper method that changes the default.

[tlefevre]

And one can't even use JsonNodeFactory.withExactBigDecimals(true) to set the ObjectMappers nodeFactory with.
From object -> JsonNode it works fine, it'll not normalize 0s.
From string -> JsonNode it doesn't work, it'll normalize 0s.

I can't figure out how to make that part stop, otherwise it might be possible to simply configure the jackson objectmapper :(

[tlefevre]

I see your point, it's always a tricky thing to change and risk breaking changes and how that affects people.

How would you go about writing a method like that? I'm not sure where and how.

[lukas-krecan]

I will not help you with this one, try to ask on StackOverflow or create issue in Jackson project.

[tlefevre]

Alright, that's enough of a pointer I guess - I was wondering if it was a json unit specific thing you were referring to.

[lukas-krecan]

About the method, something like this should do:

protected ConfigurableJsonAssert myAssertThatJson(Object actual) {
      return assertThatJson(actual).withTolerance(0);
}

[tlefevre]

Ah yeah - ok, thanks. My head was in a totally different place ;D