Allow JSON Integer to deserialize into a single-arg constructor of parameter type double

[davidmoten]

Is your feature request related to a problem? Please describe.

This test fails with a MismatchedInputException:

    @Test
    public void testJsonValueIntToDouble() throws JsonMappingException, JsonProcessingException {
        ObjectMapper mapper = new ObjectMapper();
        Stuff a = mapper.readValue("5", Stuff.class);
        assertEquals(5, a.value);
    }
    
    public static final class Stuff {
        public final double value;

        public Stuff(double value) {
            this.value = value;
        }
    }

If the value being deserialized was 5.0 then it works. 5 does not work. I'm not interested in workarounds like adding extra constructors of type int or long, this should just work.

Normally there would be a @JsonValue annotation on the field for serialization but that's not relevant for this issue so I've left it out.

I'm finding this hideous to solve in terms of overriding Jackson's default behaviour. Ideally Jackson would do it for me as default behaviour (or configured).

I'd like to override the default ValueInstantiator used by an ObjectMapper but that does not seem possible. Is it?

Can we change Jackson's default behaviour to support this use case? Or support a new DeserializationFeature for this?

Describe the solution you'd like

see above

Usage example

see above

Additional context

No response

[JooHyukKim]

Hello, @davidmoten, would this solution help your case? Note that I modified assertEquals because JUnit assertEquals does not work for comparing int against double(Stuff.value).

    @Test
    public void testJsonValueIntToDouble() throws JsonMappingException, JsonProcessingException {
        ObjectMapper mapper = new ObjectMapper();
        mapper.registerModule(cusstomModule());
        Stuff a = mapper.readValue("5", Stuff.class);
        assertEquals(5.0, a.value, 0.0);
    }

    private Module cusstomModule() {
        return new SimpleModule()
                .addValueInstantiator(Stuff.class, new ValueInstantiator() {
                    @Override
                    public Object createFromInt(DeserializationContext ctxt, int value) throws IOException {
                        return new Stuff(Double.valueOf(value));
                    }
                });
    }

    public static final class Stuff {
        public final double value;
        public Stuff(double value) {
            this.value = value;
        }
    }

[davidmoten]

Thanks @JooHyukKim, I'm aware of that sort of fix but I'm after something more general. In effect I'd like to add use of _fromDoubleCreator and even _fromFloatCreator to this method.

jackson-databind/src/main/java/com/fasterxml/jackson/databind/deser/std/StdValueInstantiator.java

Lines 353 to 388 in 199d3ac

	public Object createFromInt(DeserializationContext ctxt, int value) throws IOException
	{
	// First: "native" int methods work best:
	if (_fromIntCreator != null) {
	Object arg = Integer.valueOf(value);
	try {
	return _fromIntCreator.call1(arg);
	} catch (Exception t0) {
	return ctxt.handleInstantiationProblem(_fromIntCreator.getDeclaringClass(),
	arg, rewrapCtorProblem(ctxt, t0));
	}
	}
	// but if not, can do widening conversion
	if (_fromLongCreator != null) {
	Object arg = Long.valueOf(value);
	try {
	return _fromLongCreator.call1(arg);
	} catch (Exception t0) {
	return ctxt.handleInstantiationProblem(_fromLongCreator.getDeclaringClass(),
	arg, rewrapCtorProblem(ctxt, t0));
	}
	}
	if (_fromBigIntegerCreator != null) {
	Object arg = BigInteger.valueOf(value);
	try {
	return _fromBigIntegerCreator.call1(arg);
	} catch (Exception t0) {
	return ctxt.handleInstantiationProblem(_fromBigIntegerCreator.getDeclaringClass(),
	arg, rewrapCtorProblem(ctxt, t0)
	);
	}
	}
	return super.createFromInt(ctxt, value);
	}

[cowtowncoder]

Sounds reasonable... although in case of long its value range exceeds both float and double.

So possibly would need a new DeserializationFeature to enable. And/or checking CoercionConfig.

PR welcome!

[davidmoten]

Sounds reasonable... although in case of long its value range exceeds both float and double.

Not loss of range but can lose precision, yep. If a user types something as double or especially float they should take responsibility for the precision limitations anyway.

So possibly would need a new DeserializationFeature to enable. And/or checking CoercionConfig.

PR welcome!

Thanks, yep I'll sort out a PR.

[davidmoten]

@cowtowncoder, when I use a JsonProperty there is no deserialization problem. On a consistency basis shouldn't we just fix StdValueInstantiator?

For example, this test passes:

    @Test
    public void testJsonValueIntToDoubleMulti() throws JsonMappingException, JsonProcessingException {
        ObjectMapper mapper = new ObjectMapper();
        Stuff2 a = mapper.readValue("{\"value\":5}", Stuff2.class);
        assertEquals(5, a.value);
    }
    
    public static final class Stuff2 {
        public final double value;

        public Stuff2(@JsonProperty("value") double value) {
            this.value = value;
        }
    }

[JooHyukKim]

For example, this test passes:

Strange, wonder if it is intended behavior 🤔

[cowtowncoder]

@davidmoten good point, was thinking about that after writing what I did.
You are right, it would still be consistent with that behavior... so no need to add configurability.
PR should be against 2.18.

[davidmoten]

I've had a brief look. I'd modify StdValueInstantiator.createFromInt and StdValueInstantiator.createFromLong to deserialize to a double value (and add tests of course).

I've noticed that JsonProperty annotated values will deserialize to a float but floats aren't mentioned in StdValueInstantiator at all. I'm happy to include float support in the same PR. Would you prefer that in a different PR?

[cowtowncoder]

Ideally separate one I think, if we are talking about functionality changes? (Javadoc/comment changes are fine to be tagged on into one PR)

[JooHyukKim]

It seems like we almost had it done via davidmoten@23a635d. LMK if you need any help with anything, @davidmoten!😆

[davidmoten]

Hi, yep I'll get on to that this week. Extended Easter hols have prevented me.

[davidmoten]

PR up for review #4474

[davidmoten]

Thanks for review and merge. I did some tests and noticed that @JsonProperty will happily deserialize integer JSON values to float, short and byte. For consistency we should do the same with single-arg constructors too. Would you like me to prepare a PR?

[cowtowncoder]

Let's add ones to float first... I have to think about short and byte (if they are to be added, can go together).

[davidmoten]

Let's add ones to float first... I have to think about short and byte (if they are to be added, can go together).

Cool, makes sense. When I get the float change approved right then can apply the same pattern to other types. I'll do some more testing on existing JsonProperty deser behaviour too, see what happens when range exceeded (throws or not).

[cowtowncoder]

Sounds good! I think some accessors from JsonParser do actually very against over-/underflow, but mostly for getIntValue() etc where it is straight-forward to do.
But not (yet?) for getDoubleValue(); bounds-checks would add some overhead (f.ex casting from long to double to back to verify no loss).

[davidmoten]

Re these two issues:

• loss of precision
• overflow/underflow

I think the first issue is something that a user should wear if they choose a float for a high precision JSON decimal value for instance. So IMO this is acceptable loss. The second case varies a bit in effect across byte/short/int/long and float/double but is where throwing might be reasonable with the exception where underflow (to 0) is just like a loss of precision. Anyway, I'll first document the JsonProperty deser behaviour for these, see where we end up and we can discuss.

[cowtowncoder]

@davidmoten Agreed.