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

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

Open
cowtowncoder opened this issue Feb 16, 2024 · 6 comments
Labels
2.17 Issues planned at earliest for 2.17

Comments

@cowtowncoder
Copy link
Member

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 cowtowncoder added to-evaluate Issue that has been received but not yet evaluated 2.17 Issues planned at earliest for 2.17 and removed to-evaluate Issue that has been received but not yet evaluated labels Feb 16, 2024
@JooHyukKim
Copy link
Member

JooHyukKim commented Feb 17, 2024

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

@cowtowncoder
Copy link
Member Author

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

@JooHyukKim
Copy link
Member

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

@cowtowncoder
Copy link
Member Author

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

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

@JooHyukKim
Copy link
Member

JooHyukKim commented Feb 19, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.17 Issues planned at earliest for 2.17
Projects
None yet
Development

No branches or pull requests

2 participants