Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
bill-phast opened this issue Feb 26, 2021 · 11 comments
Closed

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

bill-phast opened this issue Feb 26, 2021 · 11 comments
Labels
Milestone

Comments

@bill-phast
Copy link

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
Copy link
Author

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

@cowtowncoder
Copy link
Member

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

@bill-phast
Copy link
Author

bill-phast commented Feb 26, 2021

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
Copy link
Member

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
Copy link
Author

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 added a commit that referenced this issue Feb 26, 2021
@cowtowncoder
Copy link
Member

@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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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.

@cowtowncoder cowtowncoder added 2.14 and removed 2.13 labels Jan 10, 2022
cowtowncoder added a commit that referenced this issue Feb 16, 2024
…ulls

Fix part of #78: handle `java.lang.Boolean` separate from primitive `boolean`
@cowtowncoder cowtowncoder changed the title Deserializes "null" to "0.0" in big-D Double Deserializes "null" to "0.0" for java.lang.Double (wrapper) Feb 16, 2024
cowtowncoder added a commit that referenced this issue Feb 16, 2024
…ulls

Fixes #78: add wrapper types for most numbers
@cowtowncoder cowtowncoder added 2.17 and removed 2.14 labels Feb 16, 2024
@cowtowncoder cowtowncoder added this to the 2.17.0 milestone Feb 16, 2024
@cowtowncoder
Copy link
Member

Finally got back and implemented for int/long/float/double/boolean.
To be included in 2.17.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants