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

Buffering needed for @JsonTypeInfo property breaks parsing of Floats (depending on property order) (dup of #3133) #3202

Closed
lost-carrier opened this issue Jul 8, 2021 · 5 comments
Labels
duplicate Duplicate of an existing (usually earlier) issue has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue

Comments

@lost-carrier
Copy link

We're using a class structure similar to the following one using the @JsonTypeInfo to determine various SubTypes possible for that JSON:

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.EXISTING_PROPERTY,
        property = "classType", visible = true)
@JsonSubTypes({
        @JsonSubTypes.Type(value = ClassA.class, name = "a"),
        @JsonSubTypes.Type(value = ClassB.class, name = "b")
})
interface ClassInterface {
    String getClassType();
}


abstract class AbstractClass implements ClassInterface {
    private String classType;

    @Override
    public String getClassType() {
        return classType;
    }

    public void setClassType(String classType) {
        this.classType = classType;
    }
}


class ClassA extends AbstractClass {

    private Object testA;

    public Object getTestA() {
        return testA;
    }

    public void setTestA(Object testA) {
        this.testA = testA;
    }
}


class ClassB extends AbstractClass {
    private Object testB;

    public Object getTestB() {
        return testB;
    }

    public void setTestB(Object testB) {
        this.testB = testB;
    }
}

Now we were facing problems because we cannot ensure the order of the properties of the incoming JSONs. Unfortunately it turned out that Jackson would parse Doubles as BigDecimals depending of the order of the type info. E.g.:

    @Test
    public void testJsonSubtypes() throws JsonProcessingException {

        ClassA classA = new ObjectMapper()
                .configure(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS, false)
                .readerFor(ClassInterface.class)
                .readValue("""
                        {
                            "classType": "a",
                            "testA": 47.11
                        }
                        """); // IMPORTANT: classType goes first
        assertFalse(classA.getTestA() instanceof BigDecimal); // ...works!
        assertTrue(classA.getTestA() instanceof Double);

        ClassB classB = new ObjectMapper()
                .configure(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS, false)
                .readerFor(ClassInterface.class)
                .readValue("""
                        {
                            "testB": 47.11,
                            "classType": "b"
                        }
                        """); // IMPORTANT: classType goes second
        assertFalse(classB.getTestB() instanceof BigDecimal); // ...doesen't!
        assertTrue(classB.getTestB() instanceof Double);
    }

During my diggings I found that

... the _deserializeTypedForId(p, ctxt, tb, p.getText()); which then somehow triggers a ParserBase.resetFloats(), which sets this._numTypesValid = 0; where I'd say this then causes _parseSlowFloat(16) then to parse everything as BigDecimal. (...but I'm certainly no expert there...)

About the context: we're using Java 15 with the current Spring Boot V 2.5.2 which contains Jackson V 2.12.3. (...and I consciously used new ObjectMapper() to bypass the Spring wirings...)

@cowtowncoder
Copy link
Member

cowtowncoder commented Jul 8, 2021

Yes, there are challenges in buffering and double vs BigDecimal handling. Since Object and Type Id handling (as well as use of @JsonCreator sometimes) typically result in need to buffer content, these are cases usually affected.
The problem stems from the fact that to buffer a value in TokenBuffer (since underlying parser's state is changing), a valud needs to be accessed, and decision on target type is needed. Usually decision is based on accessor used for JsonParser, but in this case we do not really know the ultimate value.

There is at least one other issue for the general problem and there is no immediate solution. FasterXML/jackson-modules-java8#307 is related for example.

Ultimately this could mean one of:

  1. Always decode as BigDecimal for buffering purposes regardless of settings
  2. For textual formats, buffer String value and defer decoding; for binary formats (or in general formats that have physical floating-point numbers as opposed to string representations) retain exact value. Issue Allocate TokenBuffer instance via context objects (to allow format-specific buffer types) #2989 laid some groundwork for this, allowing alternate TokenBuffer implementations

@cowtowncoder cowtowncoder changed the title Order of JsonTypeInfo property breaks parsing of Floats Buffering needed for @JsonTypeInfo property breaks parsing of Floats (depending on property order) Jul 9, 2021
@cowtowncoder
Copy link
Member

I think #3730 that was just added should help here.

@cowtowncoder cowtowncoder added need-test-case To work on issue, a reproduction (ideally unit test) needed has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue and removed need-test-case To work on issue, a reproduction (ideally unit test) needed labels Sep 24, 2024
@cowtowncoder
Copy link
Member

I think this may have been fixed for 2.17 (2.17.2) or 2.18 (2.18.0-rc1).

Would need a verification: if I have time, I'll try that with given test case.

@cowtowncoder
Copy link
Member

Ok looking at existing tests, #3133 seems to be the same issue, fixed for 2.15.
Will double-check with test but expecting to close this.

@cowtowncoder
Copy link
Member

Test passes, closing as dup.

@cowtowncoder cowtowncoder added the duplicate Duplicate of an existing (usually earlier) issue label Sep 25, 2024
@cowtowncoder cowtowncoder changed the title Buffering needed for @JsonTypeInfo property breaks parsing of Floats (depending on property order) Buffering needed for @JsonTypeInfo property breaks parsing of Floats (depending on property order) (dup of #3133) Sep 25, 2024
@cowtowncoder cowtowncoder closed this as not planned Won't fix, can't repro, duplicate, stale Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate Duplicate of an existing (usually earlier) issue has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue
Projects
None yet
Development

No branches or pull requests

2 participants