FasterXML / jackson-datatypes-misc

Collection of common Jackson datatype modules not part of other multi-project repos
Apache License 2.0
22 stars 19 forks source link

Regression in 2.15: parsing double value returns BigDecimal instead of Double #39

Closed dlipin closed 1 year ago

dlipin commented 1 year ago

I'm filing this issue as the result of conversation in pull request: https://github.com/FasterXML/jackson-datatypes-misc/pull/32

Here is the test:


import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.datatype.jsonorg.JsonOrgModule;
import org.json.JSONObject;
import org.junit.Assert;
import org.junit.Test;

public class JsonDoubleTest {

    @Test
    public void testDoubleParsing() throws Exception {
        ObjectMapper objectMapper = new ObjectMapper().registerModule(new JsonOrgModule());
        JSONObject jsonObject = objectMapper.readValue("{\"value\": 0.5 }", JSONObject.class);
        Assert.assertEquals("java.lang.Double", jsonObject.get("value").getClass().getCanonicalName());
    }
}

When I run it with Jackson 2.13.4 then it passes. When I run it with Jackson 2.15.0, it fails with error:

org.junit.ComparisonFailure: 
Expected :java.lang.Double
Actual   :java.math.BigDecimal
cowtowncoder commented 1 year ago

Ok so the question is: is the new behavior erroneous? Yes, it is a change and in hindsight unfortunate. But given there is new behavior for 2.15 I am not sure reverting change is the right thing to do.

Also see #38 for more discussion.

One possible way forward would be to support DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS, but problem there is that it is disabled by default so using it to determine behavior would mean reverting behavior. Yet another possibility would be module-specific Feature.

@pjfanning WDYT?

pjfanning commented 1 year ago

This is very easily worked around - see https://github.com/FasterXML/jackson-datatypes-misc/pull/32#issuecomment-1665267589

The old behaviour had a bug. It did not handle big numbers properly. Users who want doubles can still get doubles by using the API to ask for a Double. Examples in https://github.com/FasterXML/jackson-datatypes-misc/pull/38/files

The code in https://github.com/FasterXML/jackson-datatypes-misc/pull/38 should work in older versions of Jackson too (see https://github.com/FasterXML/jackson-datatypes-misc/pull/40)

cowtowncoder commented 1 year ago

Ok I think I'll close this then, after tests were added to document the new behavior.