FasterXML / jackson-databind

General data-binding package for Jackson (2.x): works on streaming API (core) implementation(s)
Apache License 2.0
3.53k stars 1.39k forks source link

Serialization failure for fields after updating to Jackson 2.18.0 #4752

Closed arvgord closed 1 month ago

arvgord commented 1 month ago

Search before asking

Describe the bug

After upgrading Jackson from version 2.17.2 to 2.18.0, the serialization process for SecondObject and ThirdObject has completely broken. The fields are either missing or partially serialized, making the resulting JSON incomplete.

I expected that only the field order might change, but now fields that were present in the input JSON are missing from the serialized output entirely.

Version Information

2.18.0

Reproduction

Tested objects:

@JsonAutoDetect(
    fieldVisibility = JsonAutoDetect.Visibility.ANY,
    getterVisibility = JsonAutoDetect.Visibility.NONE
)
class FirstObject {
    @field:JsonAnySetter
    @field:JsonAnyGetter
    val data: Map<String, Any?> = LinkedHashMap()

    val transactionId: String
        get() = data["transactionId"] as String
}

@JsonAutoDetect(
    fieldVisibility = JsonAutoDetect.Visibility.ANY,
    getterVisibility = JsonAutoDetect.Visibility.NONE
)
class SecondObject(
    val transactionId: String
) {
    @field:JsonAnySetter
    @field:JsonAnyGetter
    val data: Map<String, Any?> = LinkedHashMap()
}

@JsonAutoDetect(
    fieldVisibility = JsonAutoDetect.Visibility.ANY,
    getterVisibility = JsonAutoDetect.Visibility.NONE
)
class ThirdObject(
    val transactionId: String,
    @field:JsonAnySetter
    @field:JsonAnyGetter
    val data: Map<String, Any?> = LinkedHashMap()
)

Input JSON for tests:

{
  "b": 2,
  "a": 1,
  "transactionId": "test",
  "c": [
    {
      "id": "3",
      "value": "c"
    },
    {
      "id": "1",
      "value": "a"
    },
    {
      "id": "2",
      "value": "b"
    }
  ]
}

Test Case:

Here’s a test case to reproduce the issue. The following Kotlin test serializes and deserializes three different classes (FirstObject, SecondObject, and ThirdObject), comparing the serialized JSON with the original input JSON. JacksonSortingTest.kt branch 2.18.0.

For FirstObject, however, the test passed successfully. All fields were present in the serialized JSON, and they appeared in the expected order, as in the original input JSON.

Failing Test Results:

Expected:

{
  "b": 2,
  "a": 1,
  "transactionId": "test",
  "c": [
    {
      "id": "3",
      "value": "c"
    },
    {
      "id": "1",
      "value": "a"
    },
    {
      "id": "2",
      "value": "b"
    }
  ]
}

For SecondObject was:

{
  "transactionId": "test",
  "c": [
    {
      "id": "3",
      "value": "c"
    },
    {
      "id": "1",
      "value": "a"
    },
    {
      "id": "2",
      "value": "b"
    }
  ]
}

For ThirdObject was:

{
  "transactionId": "test"
}

Repository for Reproduction: You can find a repository with the full reproduction of the issue at jackson-databind-sorting-issue - branch 2.18.0.

Expected behavior

After updating from Jackson 2.17.2 to 2.18.0, the serialization for SecondObject and ThirdObject should work as it did in the previous version (except for field ordering after deserialization and serialization). The expected JSON output after deserialization and serialization should contain all fields, with their values exactly as they are in the original input, regardless of field order.

Additional context

No response

JooHyukKim commented 1 month ago

I think there was similar issue, recently. #4639 I think.

Could u test out with latest 2.18 snapshot?

arvgord commented 1 month ago

Do you mean 2.18.1-SNAPSHOT?

pjfanning commented 1 month ago

Do you mean 2.18.1-SNAPSHOT?

yes - see https://oss.sonatype.org/content/repositories/snapshots/com/fasterxml/jackson/core/jackson-databind/

cowtowncoder commented 1 month ago

Quick note: I don't know what @field:JsonAnySetter is -- is that something for Kotlin?

If so, we need one of 2 things:

  1. Removal of annotation to make test Java-only -- can remain here
  2. Move to jackson-module-kotlin -- if Kotlin specific.
arvgord commented 1 month ago

Quick note: I don't know what @field:JsonAnySetter is -- is that something for Kotlin?

@field:JsonAnySetter is a Kotlin-specific syntax used to apply annotations to the field of a property. I'll try to reproduce it with java code. I'll also create issues in jackson-module-kotlin at the same time.

arvgord commented 1 month ago

I created tests for Java: jackson-databind-sorting-issue-java branch 2.18.0. For Jackson version 2.18.0, everything works fine for SecondObject and ThirdObject. The fields exist in the result, and the field order is preserved as I expected:

{
    "transactionId": "test",
    "b": 2,
    "a": 1,
    "c": [
        {
            "id": "3",
            "value": "c"
        },
        {
            "id": "1",
            "value": "a"
        },
        {
            "id": "2",
            "value": "b"
        }
    ]
}

It seems that the issue is related to jackson-module-kotlin #843. Therefore, this issue can be closed. Thank you for your help!

arvgord commented 1 month ago

Based on this comment, I was able to reproduce this issue with Java, so I reopened it. Test to reproduce.

pjfanning commented 1 month ago

Can you separate the tests? The tests should be split into the ones relating to this issue and the ones relating to #4751

arvgord commented 1 month ago

I’ve separated the tests into different branches. To reproduce this issue, I created another branch 2.18.0.

pjfanning commented 1 month ago

I cannot follow what you have done to that POC project. Can't you just paste the 4752 test code here - with nothing related to 4751?

pjfanning commented 1 month ago

I created #4765 but I have no idea which of the 3 tests relate to this issue.

testSerializationAndDeserializationForFirstObject passes for me - the other 2 fail

pjfanning commented 1 month ago

I debugged the ThirdObject test. The deserialization to ThirdObject captures the transactionId and puts the rest of the input JSON in the data map. This seems correct to me. I think this test case at least is invalid and that Jackson is actually behaving correctly. I don't agree with @arvgord's assessment.

arvgord commented 1 month ago

I cannot follow what you have done to that POC project. Can't you just paste the 4752 test code here - with nothing related to 4751?

I’ve copied the relevant code from my repository, focusing only on issue 4752. Hopefully, this makes it easier for you to reproduce.

@JsonAutoDetect(
        fieldVisibility = JsonAutoDetect.Visibility.ANY,
        getterVisibility = JsonAutoDetect.Visibility.NONE
)
public class FirstObject {

    @JsonAnySetter
    @JsonAnyGetter
    private Map<String, Object> data = new LinkedHashMap<>();

    public String getTransactionId() {
        return (String) data.get("transactionId");
    }
}
@JsonAutoDetect(
        fieldVisibility = JsonAutoDetect.Visibility.ANY,
        getterVisibility = JsonAutoDetect.Visibility.NONE
)
public class SecondObject {

    private final String transactionId;
    @JsonAnySetter
    @JsonAnyGetter
    private final Map<String, Object>  data;

    @JsonCreator
    public SecondObject(@JsonProperty("transactionId") String transactionId) {
        this.transactionId = transactionId;
        this.data = new LinkedHashMap<>();
    }

    public String getTransactionId() {
        return this.transactionId;
    }

    public Map<String, Object> getData() {
        return this.data;
    }
}
@JsonAutoDetect(
        fieldVisibility = JsonAutoDetect.Visibility.ANY,
        getterVisibility = JsonAutoDetect.Visibility.NONE
)
public class ThirdObject {

    private final String transactionId;
    @JsonAnySetter
    @JsonAnyGetter
    private final Map<String, Object> data;

    @JsonCreator
    public ThirdObject(
            @JsonProperty("transactionId") String transactionId,
            @JsonProperty("data") Map<String, Object> data
    ) {
        this.transactionId = transactionId;
        this.data = data;
    }

    public String getTransactionId() {
        return this.transactionId;
    }

    public Map<String, Object> getData() {
        return this.data;
    }
}
public class JacksonSerializationDeserializationTest {

    private final ObjectMapper objectMapper = new ObjectMapper();

    private final String INPUT_JSON = """
        {
            "b": 2,
            "a": 1,
            "transactionId": "test",
            "c": [
                {
                    "id": "3",
                    "value": "c"
                },
                {
                    "id": "1",
                    "value": "a"
                },
                {
                    "id": "2",
                    "value": "b"
                }
            ]
        }
    """;

    private <T> void testSerializationDeserialization(Class<T> clazz) throws Exception {
        T deserializedObject = objectMapper.readValue(INPUT_JSON, clazz);
        var serializedJson = objectMapper.writeValueAsString(deserializedObject);

        var expectedJson = objectMapper.readTree(INPUT_JSON);
        var actualJson = objectMapper.readTree(serializedJson);

        assertEquals(expectedJson, actualJson);
    }

    @Test
    public void testSerializationAndDeserializationForFirstObject() throws Exception {
        testSerializationDeserialization(FirstObject.class);
    }

    @Test
    public void testSerializationAndDeserializationForSecondObject() throws Exception {
        testSerializationDeserialization(SecondObject.class);
    }

    @Test
    public void testSerializationAndDeserializationForThirdObject() throws Exception {
        testSerializationDeserialization(ThirdObject.class);
    }
}
pjfanning commented 1 month ago

@arvgord please read https://github.com/FasterXML/jackson-databind/issues/4752#issuecomment-2439556956 - I don't see an issue here

arvgord commented 1 month ago

I debugged the ThirdObject test. The deserialization to ThirdObject captures the transactionId and puts the rest of the input JSON in the data map. This seems correct to me. I think this test case at least is invalid and that Jackson is actually behaving correctly. I don't agree with @arvgord's assessment.

What about SecondObject? I'm also can't understand why it worked in 2.17.2 but doesn't in 2.18.0, and why this is considered expected behavior. It seems something was clearly broken.

arvgord commented 1 month ago

I’ve rewritten the test to focus on issues when switching between versions 2.17.2 and 2.18.0. I compare JsonNodes to verify that serialization and deserialization work correctly in both versions (test without comparing field ordering). All tests pass for 2.17.2. With 2.18.0 for SecondObject and ThirdObject I can see the following errors:

org.opentest4j.AssertionFailedError:
Expected: {"b":2,"a":1,"transactionId":"test","c":[{"id":"3","value":"c"},{"id":"1","value":"a"},{"id":"2","value":"b"}]}
Actual: {"transactionId":"test","c":[{"id":"3","value":"c"},{"id":"1","value":"a"},{"id":"2","value":"b"}]}
org.opentest4j.AssertionFailedError:
Expected: {"b":2,"a":1,"transactionId":"test","c":[{"id":"3","value":"c"},{"id":"1","value":"a"},{"id":"2","value":"b"}]}
Actual: {"transactionId":"test"}
cowtowncoder commented 1 month ago

@arvgord Just to make absolutely sure: did you try with latest from 2.18 branch (2.18.0-SNAPSHOT) -- at this point there are enough highly relevant fixes in there that there is no point in testing with stale 2.18.0 version (just wastes our time).

If issue still exists, I think this should be split into 2 new issues minimal reproductions.

arvgord commented 1 month ago

Just to make absolutely sure: did you try with latest from 2.18 branch (2.18.0-SNAPSHOT) -- at this point there are enough highly relevant fixes in there that there is no point in testing with stale 2.18.0 version (just wastes our time).

I tested jackson-databind with version 2.18.1-20241024.043825-21 - all these tests passed successfully.

cowtowncoder commented 1 month ago

@arvgord Ok. So are there still tests that fail? (sorry, I just want to make sure)

arvgord commented 1 month ago

@arvgord Ok. So are there still tests that fail? (sorry, I just want to make sure)

Everything works with jackson-databind snapshot version 2.18.1-20241024.043825-21. Thanks for the help!

cowtowncoder commented 1 month ago

@arvgord So things can be closed? Excellent.

Note: 2.18.1 was released yesterday, so fixes should be in.