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

Generic class with generic field of runtime type Double is deserialized as BigDecimal when used with @JsonTypeInfo and JsonTypeInfo.As.EXISTING_PROPERTY #3251

Closed
BaesKevin opened this issue Aug 19, 2021 · 6 comments
Milestone

Comments

@BaesKevin
Copy link

BaesKevin commented Aug 19, 2021

Describe the bug

Bug appears when deserializing a generic class with a generic field with runtime type double, and the generic class
is annotated with @JsonTypeInfo with @JsonTypeInfo with JsonTypeInfo.As.EXISTING_PROPERTY.
The bug does not occur when using @JsonTypeInfo with JsonTypeInfo.As.PROPERTY.

Version information

  • bug exists in 2.12.4
  • bug does not exist in 2.11.3
  • Java version: 16

To Reproduce

Test with 1 succeeding and 1 failing case, expecting both to succeed.

package com.test;

import static org.assertj.core.api.Assertions.assertThat;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.datatype.jdk8.Jdk8Module;
import org.junit.Test;

public class JacksonMapperDeserializesDoubleAsDoubleTest {

    private final ObjectMapper objectMapper = new MyObjectMapper();

    static class MyObjectMapper extends ObjectMapper {
        public MyObjectMapper() {
            registerModule(new Jdk8Module());
            // this is default, set it to be sure
            configure(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS, false);
        }
    }

    @Test
    public void deserializesDoubleToDouble_givenJsonTypeInfoUsingNewProperty() throws JsonProcessingException {
        GenericWrapperWithNewProperty<?> wrapper = new GenericWrapperWithNewProperty<>(123.45);

        String json = objectMapper.writeValueAsString(wrapper);
        GenericWrapperWithNewProperty<?> actualWrapper = objectMapper.readValue(json, GenericWrapperWithNewProperty.class);

        assertThat(actualWrapper).satisfies(it -> assertThat(it.getValue()).isEqualTo(123.45));
        assertThat(json).contains("\"value\":123.45");
    }

    @Test
    public void deserializesDoubleToDouble_givenJsonTypeInfoUsingExistingProperty() throws JsonProcessingException {
        GenericWrapperWithExistingProperty<?> wrapper = new GenericWrapperWithExistingProperty<>(123.45);

        String json = objectMapper.writeValueAsString(wrapper);
        GenericWrapperWithExistingProperty<?> actualWrapper = objectMapper.readValue(json, GenericWrapperWithExistingProperty.class);

        assertThat(actualWrapper).satisfies(it -> assertThat(it.getValue()).isEqualTo(123.45));
        assertThat(json).contains("\"value\":123.45");
    }

    @JsonTypeInfo(
        use = JsonTypeInfo.Id.NAME,
        property = "type_alias"
    )
    static class GenericWrapperWithNewProperty<T> {
        private final T value;

        @JsonCreator
        public GenericWrapperWithNewProperty(@JsonProperty("value") T value) {
            this.value = value;
        }

        public T getValue() {
            return value;
        }
    }

// @JsonSubtypes or using abstract classes and such is not required to reproduce the issue
    @JsonTypeInfo(
        use = JsonTypeInfo.Id.NAME,
        include = JsonTypeInfo.As.EXISTING_PROPERTY,
        property = "fieldType",
        visible = true,
        defaultImpl = GenericWrapperWithExistingProperty.class
    )
    static class GenericWrapperWithExistingProperty<T> {
        private String fieldType;
        private final T value;

        @JsonCreator
        public GenericWrapperWithExistingProperty(@JsonProperty("value") T value) {
            this.value = value;
        }

        public T getValue() {
            return value;
        }

        public String getFieldType() {
            return fieldType;
        }

        public void setFieldType(String fieldType) {
            this.fieldType = fieldType;
        }
    }
}

Additional context

This bug started happening when upgrading Spring Boot from 2.4.1 to 2.5.3.
As we don't manage Jackson version ourselves, Jackson was upgraded to 2.12.4,
I have not tried to reproduce this issue with a project that has only Jackson dependencies, managed
by jackson BOM 2.12.4.

@BaesKevin BaesKevin added the to-evaluate Issue that has been received but not yet evaluated label Aug 19, 2021
@cowtowncoder
Copy link
Member

I wonder if this might be related to:

#3220

which will be fixed for 2.12.5, to be released later today.

@richturner
Copy link

richturner commented Oct 2, 2023

@cowtowncoder I just stumbled across what seems to be a connected issue with version 2.14.3 but is fixed in 2.15.2; wanted to provide some info to ensure the fix wasn't accidental and to avoid future regression; I can also confirm the value of @JsonTypeInfo#include does not affect this behaviour. Here's some info:

Problem

Deserializing JSON with floating point numbers will incorrectly use BigDecimal instead of Double when using sub types (even though DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS is false).

With sub type

Class definitions

@JsonTypeInfo(property = "type", use = JsonTypeInfo.Id.NAME)
@JsonSubTypes({
    @JsonSubTypes.Type(ValueTest.ValueTest1.class)
})
public class ValueTest {

    @JsonTypeName("valueTest1")
    public static class ValueTest1 extends ValueTest {
        Object[] allowedValues;

        @JsonCreator
        protected ValueTest1() {
        }

        public ValueTest1(Object... allowedValues) {
            this.allowedValues = allowedValues;
        }

        public Object[] getAllowedValues() {
            return allowedValues;
        }
    }
}

JSON string to deserialize

JSON.readValue("{\"allowedValues\": [ 1.5, 2.5 ], \"type\": \"valueTest1\"}", ValueTest.class)

Result

Hydrated ValueTest instance will have BigDecimal objects in the allowedValues property.

Without sub type

Class definition

public class ValueTest {
    Object[] allowedValues;

    @JsonCreator
    protected ValueTest() {
    }

    public ValueTest(Object... allowedValues) {
        this.allowedValues = allowedValues;
    }

    public Object[] getAllowedValues() {
        return allowedValues;
    }
}

JSON string to deserialize

JSON.readValue("{ \"allowedValues\": [ 1.5, 2.5 ] }", ValueTest.class)

Result

Hydrated ValueTest instance will have Double objects in the allowedValues property.

@cowtowncoder cowtowncoder removed the to-evaluate Issue that has been received but not yet evaluated label Oct 2, 2023
@cowtowncoder
Copy link
Member

@richturner Could you please file a new with reproduction and information? I cannot say with 100% certainty this is directly intentional but I do think fix "should stick". Adding a test against regression makes sense, so issue can say that ("Add test to verify that ...").

In fact if you had time and interest, providing a unit test as PR would be most appreciated. But even just issue including what you said here would be useful.

@cowtowncoder
Copy link
Member

@pjfanning I suspect fixes to lazy parsing of floating-point values might be the thing for issue reported by @richturner.

I wonder if this issue might be resolved as well -- I hope to find time to create a unit test for this (unless @JooHyukKim beats me to it :) ).

@richturner
Copy link

I've created #4138 not sure I have much time to do a PR for this but hopefully this will help and may allow you to close a couple of issues in the process.

@cowtowncoder cowtowncoder added this to the 2.16.0 milestone Oct 3, 2023
@cowtowncoder cowtowncoder changed the title Generic class with generic field of runtime type Double is deserialized as BigDecimal when used with @JsonTypeInfo and JsonTypeInfo.As.EXISTING_PROPERTY Generic class with generic field of runtime type Double is deserialized as BigDecimal when used with @JsonTypeInfo and JsonTypeInfo.As.EXISTING_PROPERTY Oct 3, 2023
@cowtowncoder
Copy link
Member

Ok I added test via #4139 and this issue has been fixed. I suspect it would have been fixed in 2.15.0 already, but will mark as 2.16.0 as that's where test is added.

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

No branches or pull requests

3 participants