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 https://github.com/FasterXML/jackson-databind/issues/4751#issuecomment-2439507091 - 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]

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