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

Serialization failure for fields after updating to Jackson 2.18.0 #4752

Closed
1 task done
arvgord opened this issue Oct 15, 2024 · 21 comments
Closed
1 task done

Serialization failure for fields after updating to Jackson 2.18.0 #4752

arvgord opened this issue Oct 15, 2024 · 21 comments
Labels
need-test-case To work on issue, a reproduction (ideally unit test) needed to-evaluate Issue that has been received but not yet evaluated
Milestone

Comments

@arvgord
Copy link

arvgord commented Oct 15, 2024

Search before asking

  • I searched in the issues and found nothing similar.

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

@arvgord arvgord added the to-evaluate Issue that has been received but not yet evaluated label Oct 15, 2024
@JooHyukKim
Copy link
Member

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

Could u test out with latest 2.18 snapshot?

@arvgord
Copy link
Author

arvgord commented Oct 15, 2024

Do you mean 2.18.1-SNAPSHOT?

@pjfanning
Copy link
Member

@cowtowncoder cowtowncoder added the need-test-case To work on issue, a reproduction (ideally unit test) needed label Oct 15, 2024
@cowtowncoder
Copy link
Member

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

arvgord commented Oct 15, 2024

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

arvgord commented Oct 15, 2024

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 arvgord closed this as completed Oct 15, 2024
lberrymage added a commit to accrescent/parcelo that referenced this issue Oct 24, 2024
Running Parcelo in a container via our current Dockerfile revealed that
"uses-sdk" fields were not being parsed from applications' Android
manifests, effectively preventing app uploads since the targetSdk
property of uses-sdk is required by Parcelo. This bug wasn't caught
until now because it only seems to manifest itself when running via the
Dockerfile; running locally as in our recommended development
environment does not have the issue. The Jackson 2.18.0 upgrade has not
yet been included in a production release, so it's uncertain whether the
issue would have surfaced in our production environment.

We tracked the issue down to a regression in Jackson 2.18.0. The exact
cause is unknown. However, a number of seemingly related issues were
reported for Jackson 2.18.0 [1], so we plan to closely monitor those
issues and test upcoming Jackson releases carefully to prevent breakage.

[1]: See below:

- FasterXML/jackson-module-kotlin#841
- FasterXML/jackson-module-kotlin#842
- FasterXML/jackson-module-kotlin#843
- FasterXML/jackson-module-kotlin#832
- FasterXML/jackson-databind#4508
- FasterXML/jackson-databind#4752
@arvgord arvgord reopened this Oct 26, 2024
@arvgord
Copy link
Author

arvgord commented Oct 26, 2024

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

@pjfanning
Copy link
Member

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

@arvgord
Copy link
Author

arvgord commented Oct 26, 2024

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

@pjfanning
Copy link
Member

pjfanning commented Oct 26, 2024

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

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

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

arvgord commented Oct 26, 2024

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

@arvgord please read #4752 (comment) - I don't see an issue here

@arvgord
Copy link
Author

arvgord commented Oct 26, 2024

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

arvgord commented Oct 27, 2024

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

@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.

lberrymage added a commit to accrescent/parcelo that referenced this issue Oct 27, 2024
Running Parcelo in a container via our current Dockerfile revealed that
"uses-sdk" fields were not being parsed from applications' Android
manifests, effectively preventing app uploads since the targetSdk
property of uses-sdk is required by Parcelo. This bug wasn't caught
until now because it only seems to manifest itself when running via the
Dockerfile; running locally as in our recommended development
environment does not have the issue. The Jackson 2.18.0 upgrade has not
yet been included in a production release, so it's uncertain whether the
issue would have surfaced in our production environment.

We tracked the issue down to a regression in Jackson 2.18.0. The exact
cause is unknown. However, a number of seemingly related issues were
reported for Jackson 2.18.0 [1], so we plan to closely monitor those
issues and test upcoming Jackson releases carefully to prevent breakage.

[1]: See below:

- FasterXML/jackson-module-kotlin#841
- FasterXML/jackson-module-kotlin#842
- FasterXML/jackson-module-kotlin#843
- FasterXML/jackson-module-kotlin#832
- FasterXML/jackson-databind#4508
- FasterXML/jackson-databind#4752
@arvgord
Copy link
Author

arvgord commented Oct 28, 2024

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

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

@arvgord
Copy link
Author

arvgord commented Oct 29, 2024

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

@arvgord So things can be closed? Excellent.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need-test-case To work on issue, a reproduction (ideally unit test) needed to-evaluate Issue that has been received but not yet evaluated
Projects
None yet
Development

No branches or pull requests

4 participants