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

@JsonAnySetter on field ignoring unrecognized properties if they are declared before the last recognized properties in JSON #4639

Closed
1 task done
yihtserns opened this issue Jul 23, 2024 · 15 comments · Fixed by #4738
Labels
Milestone

Comments

@yihtserns
Copy link
Contributor

yihtserns commented Jul 23, 2024

Search before asking

  • I searched in the issues and found nothing similar.

Describe the bug

Say a bean class only recognize property/field p1, and we have the following JSONs:

{
    "p1": ..., // goes to p1 field
    "p2": ..., // goes to @JsonAnySetter field
    "p3": ... // goes to @JsonAnySetter field
}

{
    "p2": ..., // goes missing ⚠
    "p1": ..., // goes to p1 field
    "p3": ... // goes to @JsonAnySetter field
}


{
    "p2": ..., // goes missing ⚠
    "p3": ..., // goes missing ⚠
    "p1": ... // goes to p1 field
}

Version Information

2.18

Reproduction

public static class Bean {

    private int b;
    private int d;
    @JsonAnySetter
    private Map<String, ?> any;

    @JsonCreator
    public Bean(@JsonProperty("b") int b, @JsonProperty("d") int d) {
        this.b = b;
        this.d = d;
    }
}
...

String json = "{\"a\":1,\"b\":2,\"c\":3,\"d\":4,\"e\":5,\"f\":6}";

Bean bean = new ObjectMapper().readValue(json, Bean.class);
assertEquals(2, bean.b);
assertEquals(4, bean.d);
// failed with:
// org.opentest4j.AssertionFailedError: 
// Expected :{b=2, c=3, e=5, f=6}
// Actual   :{e=5, f=6}
assertEquals(Map.of("a", 1, "c", 3, "e", 5, "f", 6), bean.any);

Expected behavior

No response

Additional context

@yihtserns yihtserns added the to-evaluate Issue that has been received but not yet evaluated label Jul 23, 2024
@cowtowncoder cowtowncoder added 2.18 and removed to-evaluate Issue that has been received but not yet evaluated labels Jul 24, 2024
@erdi
Copy link

erdi commented Oct 7, 2024

This is clearly a regression and I'm affected by it. Is there a plan to tackle it any time soon, please?

@JooHyukKim
Copy link
Member

JooHyukKim commented Oct 7, 2024

Not for a while I think, because only recently we released 2.18.0.

I wrote #4738 hopefully enough to fix the issue. but even if it gets merged, timing tho.

@erdi
Copy link

erdi commented Oct 7, 2024

The important bit is that there is a fix ready to be merged so that the next release, whenever it happens, will no longer contain the regression. Thanks.

@davsclaus
Copy link

Thanks we noticed this too at Apache Camel in the camel-salesforce component
https://issues.apache.org/jira/browse/CAMEL-21346

@cowtowncoder cowtowncoder modified the milestones: 2.18.0, 2.18.1 Oct 15, 2024
@cowtowncoder
Copy link
Member

Merged for inclusion in 2.18.1

@erdi
Copy link

erdi commented Nov 4, 2024

I've just tried 2.18.1 out and sadly it looks like #4738 did not fix this issue for me, @yihtserns.

@cowtowncoder
Copy link
Member

@erdi You may need to file a new issue outlining specific remaining problem, with reproduction (sounds like there are multiple problems within same feature).
And as usual, if reproducible with Java-only, issue for jackson-databind; if just for Kotlin, on jackson-module-kotlin.

@erdi
Copy link

erdi commented Nov 6, 2024

That's a fair point, @cowtowncoder. I will need to provide a reproducer as the issue is most likely in the same are but the case is not as simple as the test case added with the fix which is clearly passing.

@bsa01
Copy link

bsa01 commented Nov 8, 2024

Hi @cowtowncoder. I got the same problem with 2.18.1 but instead when using @JsonAnySetter at setter level instead of field level.
Do you want me to open another issue, or track it here?

public class Bean {
    final int                b;
    final int                d;
    final Map<String,Object> any = new HashMap<>();

    @JsonCreator
    public Bean(@JsonProperty("b") int b, @JsonProperty("d") int d) {
        this.b = b;
        this.d = d;
    }

    @JsonAnySetter
    public void setAny(String name, Object value) {
        any.put(name, value);
    }
}
...
String json = "{\"a\":1,\"b\":2,\"c\":3,\"d\":4,\"e\":5,\"f\":6}";
Bean bean = new ObjectMapper().readValue(json, Bean.class);
assertEquals(2, bean.b);
assertEquals(4, bean.d);
assertEquals(Map.of("a", 1, "c", 3, "e", 5, "f", 6), bean.any);
// failed with:
// Expected :{f=6, e=5, c=3, a=1}
// Actual   :{e=5, f=6}

@JooHyukKim
Copy link
Member

JooHyukKim commented Nov 8, 2024

Hello @bsa01, thank you for reporting!
I confirmed it worked in 2.17 and failing in 2.18.
I am sorry to it happened.
Let me see what we can do about it.
And I am really hoping that it would be as straight forward as #4738

@cowtowncoder
Copy link
Member

@bsa01 We don't usually re-open closed issues (too confusing for release notes), so I was to suggest a new issue. But since @JooHyukKim created PR, I think we are ok.

@cowtowncoder cowtowncoder changed the title @JsonAnySetter on field ignoring unrecognized properties if they are declared before the last recognized properties in JSON @JsonAnySetter on field ignoring unrecognized properties if they are declared before the last recognized properties in JSON Nov 8, 2024
@davsclaus
Copy link

Thanks as the 2.8.1 did not fix our issue in Apache Camel, so good to hear that an additional fix is on the way for 2.8.2

@JooHyukKim
Copy link
Member

we are talking about 2.18 not 2.8. Just to clear things up 😉 @davsclaus

@davsclaus
Copy link

we are talking about 2.18 not 2.8. Just to clear things up 😉 @davsclaus

Oh thanks yeah so many version numbers - Again thanks for all the hard work as OSS maintainer. I know the feeling and how much it takes to care and sit and do this year after year.

@cowtowncoder
Copy link
Member

Thank you @davsclaus.

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

Successfully merging a pull request may close this issue.

6 participants