jackson-databind icon indicating copy to clipboard operation
jackson-databind copied to clipboard

Allow using `@JsonPropertyOrder` with "any" (`@JsonAnyGetter`) and "unwrapped" (`@JsonUnwrapped`) properties

Open cowtowncoder opened this issue 4 months ago • 6 comments

Describe your Issue

Currently @JsonPropertyOrder can only be used with regular (or "true") properties, and it will have no effect on:

  1. "any properties" specified with @JsonAnyGetter
  2. "unwrapped properties" that result from @JsonUnwrapped

It would be good to improve so that at least groups could be ordered, so that f.ex following:

@JsonPropertyOrder({ "id", "unwrapped", "name", "any" })
public class POJO {
   public int id;
   public String name;

   @JsonAnyGetter
   protected Map<String, Object> any;

   @JsonUnwrapped
   protected Values unwrapped;
}

would sort output properties as expected, so unwrapped properties of Values unwrapped would be after id but before name, and "any properties" from Map<> any would be serialized after everything else. No ordering would be imposed based within "any" or "unwrapped" properties (although @JsonPropertyOrder(alphabetic = true) could be taken into account as well; this would be #518.

cowtowncoder avatar Feb 16 '24 04:02 cowtowncoder

I tried writing test for this one, but seems to already work? 🤔 Ran below test against 2.16 and above, all passes. Could it be #4387 test is done against way earlier version or something?

public class JsonPropertyOrder4388Test
{

    // "entityId", "entityName", "totalTests", "products", "childEntities"
    @JsonPropertyOrder({"childEntities", "entityId", "totalTests", "entityName", "products"})
    static class POJO {
        public String entityName;
        public int entityId;
        public Integer totalTests;
        @JsonAnyGetter
        public Map<String, Object> products;
        @JsonUnwrapped
        public Location childEntities;
    }

    static class Location {
        public int child1;
        public int child2;
    }

    private final ObjectMapper MAPPER = newJsonMapper();

    /**
     * Test to verify that the order of properties is preserved when using @JsonPropertyOrder
     * with @JsonUnwrapped and @JsonAnyGetter
     * @throws Exception
     */
    @Test
    public void testSerializationOrder() throws Exception {
        POJO input = new POJO();
        input.entityId = 1;
        input.entityName = "Bob";
        input.totalTests = 2;
        input.childEntities = new Location();
        input.childEntities.child1 = 3;
        input.childEntities.child2 = 3;
        input.products = new HashMap<>();
        input.products.put("product1", 4);

        String json = MAPPER.writeValueAsString(input);

        assertEquals(a2q("{" +
                "'child1':3," +
                "'child2':3," +
                "'entityId':1," +
                "'totalTests':2," +
                "'entityName':'Bob'," +
                "'product1':4}"),
            json);
    }
}

JooHyukKim avatar Feb 17 '24 07:02 JooHyukKim

@JooHyukKim I think it might happen to work because the default ordering puts "any" and "unwrapped" properties after regualr ones?

cowtowncoder avatar Feb 19 '24 02:02 cowtowncoder

@cowtowncoder Thought the same at first, so tried shuffling order. But still working.. 🤔 Modified above test

JooHyukKim avatar Feb 19 '24 03:02 JooHyukKim

Oh ok. Well that's a nice finding if true :)

Since containers for "any" and "unwrapped" properties do have logical name, I can see how that might actually work. And if so, having unit test to confirm is pretty good.

I wonder if we should then mark this issue as Fixed for 2.17, even tho technically it has worked since whatever version(s).

cowtowncoder avatar Feb 19 '24 03:02 cowtowncoder

@JooHyukKim I think adding test(s) via PR would make sense. As long as any- and unwrapped-properties are not the first or last entries grouped, that'd be sufficient validation to me.

cowtowncoder avatar Feb 19 '24 03:02 cowtowncoder

@JooHyukKim I think adding test(s) via PR would make sense. As long as any- and unwrapped-properties are not the first or last entries grouped, that'd be sufficient validation to me.

Will do now 👍🏼 @cowtowncoder And probably add more tests with different order to verify.

JooHyukKim avatar Feb 19 '24 03:02 JooHyukKim