Create Issue4752Test.java

[pjfanning]

relates to #4752

refactored to support Java 8 compilation

I have no idea what @arvgord is expecting - the tests make little or no sense to me.

[arvgord]

This issue didn't exist in version 2.17 and appeared in 2.18.0, so something clearly broke between these versions.

[arvgord]

I created issue #4752, highlighting SECOND_UNEXPECTED_JSON_OUTPUT and THIRD_UNEXPECTED_JSON_OUTPUT to show unexpected results for SecondObject and ThirdObject. My expectation is to receive INPUT_JSON as the result for these classes, similar to FirstObject. For your case you shuld change tests:

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

  @Test
  public void testSerializationAndDeserializationForThirdObject() throws Exception {
    testSerializationDeserialization(INPUT_JSON, ThirdObject.class);
  }

[pjfanning]

This issue didn't exist in version 2.17 and appeared in 2.18.0, so something clearly broke between these versions.

@arvgord please read #4751 (comment) - the reason for releases is to change behaviour - changing behaviour is not a bug. If we document something and say that is how things work, then changing that behaviour is a bug - but find me the docs that say Jackson used to work the way you appear to want to.

[arvgord]

At least for me, the following result could also be acceptable for SecondObject and ThirdObject:

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

This makes sense because transactionId is declared in the constructor, so having it appear first feels logical and consistent with expected behavior.

[pjfanning]

You are using a plain Map in ThirdObject - and maps are not ordered. If the code ever worked the way you wanted, it was pure fluke.

Write your own custom serializer if you don't like the default Jackson behaviour. It is widely documented.

I haven't debugged SecondObject case yet.

[arvgord]

Can you try implementing the following test? It should ensure that deserialization and serialization work correctly when switching between Jackson versions (without comparing field order).

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);
    }
}

[cowtowncoder]

Would adding @JsonAnySetter in here make any difference?

In fact, @JsonAnySetter SHOULD NOT be added to field in this case as the intent is to propagate any properties via constructor arguments.

And @JsonAnyGetter should be added on getData() similarly, not in field.

[arvgord]

Would adding @JsonAnySetter in here make any difference?

Do you mean put @JsonAnySetter near to @JsonProperty("data")?

In fact, @JsonAnySetter SHOULD NOT be added to field in this case as the intent is to propagate any properties via constructor arguments.
And @JsonAnyGetter should be added on getData() similarly, not in field.

Sounds reasonable.

[cowtowncoder]

Yes, exactly. FWTW ideally usage as shown should also work, but trying to think of possible work arounds.

[arvgord]

I created class:

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

    private final String transactionId;
    private final Map<String, Object> data;

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

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

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

When running the test for jackson-databind 2.18.1-20241024.043825-21 I got:

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",
  "data" : {
    "a" : 1,
    "b" : 2,
    "c" : [ {
      "id" : "3",
      "value" : "c"
    }, {
      "id" : "1",
      "value" : "a"
    }, {
      "id" : "2",
      "value" : "b"
    } ]
  },
  "a" : 1,
  "b" : 2,
  "c" : [ {
    "id" : "3",
    "value" : "c"
  }, {
    "id" : "1",
    "value" : "a"
  }, {
    "id" : "2",
    "value" : "b"
  } ]
}

For 2.17.2 I got:

com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException: Unrecognized field "b" (class com.arvgord.FourthObject), not marked as ignorable (2 known properties: "transactionId", "data"])
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 19, column: 6] (through reference chain: com.arvgord.FourthObject["b"])

[cowtowncoder]

Since #4752 is closed (as things working), I think this should be closed as well?