Deserializes "null" to "0.0" for `java.lang.Double` (wrapper)

[bill-phast]

When a bean has a big-d Double, and the value is null, it gets decoded as 0.0 instead. The two are not equivalent.

This code shows the problem:

package test;

import com.fasterxml.jackson.jr.ob.JSON;

import java.util.List;

public class DoubleTest {

    private Double value;

    public static void main(String[] args) throws Exception {
        for (var testInput: List.of(
            "{\"value\": 1.0}", "{\"value\": 0.0}", "{}", "{\"value\": null}")) {
            System.out.println("Input JSON: " + testInput + "  -  Output bean: " + JSON.std.beanFrom(DoubleTest.class, testInput));
        }
    }

    public Double getValue() {
        return value;
    }

    public void setValue(Double value) {
        this.value = value;
    }

    public @Override String toString() {
        return "DoubleTest[" + value + "]";
    }
}

Output:

Input JSON: {"value": 1.0}  -  Output bean: DoubleTest[1.0]
Input JSON: {"value": 0.0}  -  Output bean: DoubleTest[0.0]
Input JSON: {}  -  Output bean: DoubleTest[null]
Input JSON: {"value": null}  -  Output bean: DoubleTest[0.0]

The fourth JSON object should be deserialized the same as the third.

[bill-phast]

Found out that big-B Booleans have the same problem. Most likely Integer, etc. as well.

[cowtowncoder]

Thank you for reporting this, it does sound like a bug.
I assume this is with 2.12.2?

[bill-phast]

I'm actually on 2.12.1, sorry I didn't know 2.12.2 was out. I found a workaround; add a reader/writer provider that provides this object when a reader for Double.class is needed:

    /**
     * Jackson Jr. by default converts null to 0.0. We need to keep it as null.
     */
    private static final ValueReader DOUBLE_READER = new ValueReader(Double.class) {
        @Override
        public Object read(JSONReader reader, JsonParser jsonParser) throws IOException {
            return jsonParser.currentToken() == JsonToken.VALUE_NULL ? null : jsonParser.getValueAsDouble();
        }
    };

But it seems it would be best to make this workaround unnecessary, to just preserve nulls. (I have verified that yes, Integer, Boolean, Float, and Character, all have this problem, all can be fixed by the same workaround technique. I guess I have some of complex beans.)

[cowtowncoder]

Yes, I agree that nulls should be preserved for wrapper types by default (and if there was a coercion, that'd be separate setting).

But I now realize that there's definitely the backwards-compatibility angle to consider... meaning that the change in behavior may need to wait until 2.13.0 (patch releases should minimize regression likelihood for users, and this is one of those possibly "bug or feature?" cases).

[bill-phast]

Since I have my workaround in place I'm OK with waiting until 2.13. Thanks for listening.

I'm always in favor of strictness in serialization/deserialization; if I have a little-d double, and get a null, I'd rather see an exception thrown (since this means my schema doesn't match the data) than see it quietly turned into 0.0. But I know other people prefer the "do whatever it takes to shove the data into the bean" approach.

[cowtowncoder]

@bill-phast exactly. There are two quite different philosophies... and some people find it difficult to believe there is the "other side", too. :)

In this case behavior is definitely not intentional but oversight. But over time even unintended behavior becomes sort of de fact specification.

BTW, if you would like to get exception for "null for primitives" case, I'd be happy to add that as a JSON.Feature -- not sure if I have time to implement it immediately, but if you or anyone else wants to contribute a PR I do my best to help and get it merged. And at very least if you'd like to see it, please create a separate issue? We can mark it as "good first issue", too, as there are some devs who are looking for easier things to work on and this would be such, I think.

[bill-phast]

Thanks! Yes I can see the other side of it. I may write that pull request, it doesn't look like it would be too hard.

[cowtowncoder]

Hmmh. On this particular problem, I realized that this is bit trickier because deserializer side does not currently track primitive/tracker difference: internally there are numeric constants like ValueLocatorBase.SER_NUMBER_INTEGER which cover both primitive and wrapper variants (mostly since Reflection only deals with wrapped version), and thereby SimpleValueReader is passed the same int constant value for both. So that needs to be changed to separate handling. Not a big deal, just more work.

[bill-phast]

I looked at stricter handling of values, and ran into the same problem - want to fail on null with an "int" and pass the null back on an "Integer" but that doesn't work since at the deserializer level we can't tell them apart. It looks like you've started work on 2.13, I'll wait until that appears then return to it.

[cowtowncoder]

2.13 branch is open, so PRs are possible against.

Unfortunately I don't think I will have time to work on this problem in near future. I think that to make this change, new type constants would be needed for standard deserializers (one for each primitive/wrapper case), and then matching changes to ValueReaders. Not necessarily very difficult, just quite a bit of work.