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 not working when annotated on both constructor parameter & field #4634

Closed
1 task done
yihtserns opened this issue Jul 22, 2024 · 6 comments · Fixed by #4641
Closed
1 task done

@JsonAnySetter not working when annotated on both constructor parameter & field #4634

yihtserns opened this issue Jul 22, 2024 · 6 comments · Fixed by #4641

Comments

@yihtserns
Copy link
Contributor

Search before asking

  • I searched in the issues and found nothing similar.

Describe the bug

When both a field & constructor parameter has @JsonAnySetter, I'm getting null value.

I changed the constructor parameter to be assigned to another field to see if maybe the injecting for field vs constructor parameter are overwriting each other e.g.:

@JsonAnySetter
Map<String,Object> stuffFromField;
Map<String,Object> stuffFromConstructor;

@JsonCreator
public TheConstructor(@JsonProperty("a") String a, @JsonAnySetter Map<String, Object> leftovers) {
    this.a = a;
    stuffFromConstructor = leftovers;
}

...but both stuffFromField & stuffFromConstructor have null value.

Version Information

2.18.0

Reproduction

static class POJO562WithAnnotationOnBothCtorParamAndField
{
    String a;
    @JsonAnySetter
    Map<String,Object> stuff;

    @JsonCreator
    public POJO562WithAnnotationOnBothCtorParamAndField(@JsonProperty("a") String a,
                                                        @JsonAnySetter Map<String, Object> leftovers
    ) {
        this.a = a;
        stuff = leftovers;
    }
}

Map<String, Object> expected = new HashMap<>();
expected.put("b", Integer.valueOf(42));
expected.put("c", Integer.valueOf(111));

POJO562WithAnnotationOnBothCtorParamAndField pojo = MAPPER.readValue(a2q(
        "{'a':'value', 'b':42, 'c': 111}"
        ),
        POJO562WithAnnotationOnBothCtorParamAndField.class);

assertEquals("value", pojo.a);
// failed with:
// org.opentest4j.AssertionFailedError: 
// Expected :{b=42, c=111}
// Actual   :null
assertEquals(expected, pojo.stuff);

Expected behavior

No response

Additional context

While this won't normally happen, it is possible with Records:

  1. @JsonAnySetter's @Target allows for ElementType.FIELD & ElementType.PARAMETER.
  2. Which means when @JsonAnySetter is annotated on a Record component, the annotation will be propagated to both field & constructor parameter.
  3. Record fields was previously removed by Avoid mutator-inference for Records (to avoid pulling in Fields as mutators) #3737, so Jackson only sees @JsonAnySetter on the constructor parameter.
  4. But when I tried to revert Avoid mutator-inference for Records (to avoid pulling in Fields as mutators) #3737 via Ignore Records' immutable fields in BeanDeserializerFactory instead of ignoring it in POJOPropertiesCollector, to preserve any annotation info the fields may be carrying. #4627 to fix some bugs, Jackson now sees @JsonAnySetter in both field & constructor parameter, hence this issue.
@yihtserns yihtserns added the to-evaluate Issue that has been received but not yet evaluated label Jul 22, 2024
@yihtserns
Copy link
Contributor Author

yihtserns commented Jul 22, 2024

Right now, if @JsonAnySetter is annotated on both a field & a constructor parameter, the former will "win":

// Find the regular method/field level any-setter
AnnotatedMember anySetter = beanDesc.findAnySetterAccessor();
if (anySetter != null) {
return constructAnySetter(ctxt, beanDesc, anySetter);
}
// else look for any-setter via @JsonCreator
if (creatorProps != null) {
for (SettableBeanProperty prop : creatorProps) {
AnnotatedMember member = prop.getMember();
if (member != null && Boolean.TRUE.equals(ctxt.getAnnotationIntrospector().hasAnySetter(member))) {
return constructAnySetter(ctxt, beanDesc, member);
}
}
}

The weird thing is it will then proceed to inject unrecognized properties into NEITHER the field NOR the constructor argument.

It works when I reverse the priority/precedence (i.e. allowing constructor parameter to "win"), but I've yet to create a PR to propose for that change because I want to understand what's causing the current bizarre behaviour.

@cowtowncoder
Copy link
Member

Ok; with regular accessors there is clearly define precedence so Constructor parameter has precedence over Setter which has precedence over Field. Something similar maybe should probably be done for any-setter (and as necessary any-getter but that's separate), which I think you are suggesting.

But as to how injection may fail altogether... I don't know. Does sound weird.

@cowtowncoder
Copy link
Member

cowtowncoder commented Jul 23, 2024

Actually, now that I bump into the same issue trying to work on #4626, looking at POJOPropertiesCollector, I think the real problem is that @JsonAnySetter is not really detected for Constructor (creator) parameters at all! So everything relies on there being a Field and that Field being used, then. Instead of making things work the proper way (which is much more difficult TBH).

This is why my initial thinking wrt detecting Record Fields in POJOPropertiesCollector for a bit, until dropping at the end (after merging annotations) didn't quite work.
Although looking at code, I actually don't see how dropping fields of POJOPropertyBuilder manages to remove Any-setter field from POJOPropertiesCollector. :-(

@cowtowncoder
Copy link
Member

Hmmh. Actually, Field linkage is not added for @JsonAnySetter annotated constructor parameters. But if so, why does dropping other Fields of record have any effect.
Very confusing.

@yihtserns
Copy link
Contributor Author

Apparently the path for processing @JsonAnySetter on field vs constructor parameter split off in BeanDeserializer._deserializeUsingPropertyBased:

@JsonAnySetter on field

            /---➤---➤---➤ SettableAnyProperty.MapFieldAnyProperty
--➤---➤---
            \--------------
@JsonAnySetter on constructor parameter

            /--------------
--➤---➤---
            \---➤---➤---➤ SettableAnyProperty.MapParameterAnyProperty

For the case of @JsonAnySetter on BOTH field & constructor parameter:

  1. It creates a SettableAnyProperty.MapFieldAnyProperty for the field...
  2. ...but goes down the path for constructor parameter (not due to any special handling for @JsonAnySetter, but just an accident due to the existing design of dealing with constructor parameters).
            /-------------- SettableAnyProperty.MapFieldAnyProperty
--➤---➤---
            \---➤---➤---➤

yihtserns added a commit to yihtserns/jackson-databind that referenced this issue Jul 24, 2024
@cowtowncoder cowtowncoder removed the to-evaluate Issue that has been received but not yet evaluated label Jul 24, 2024
@cowtowncoder cowtowncoder changed the title JsonAnySetter not working when annotated on both constructor parameter & field @JsonAnySetter not working when annotated on both constructor parameter & field Jul 24, 2024
cowtowncoder added a commit that referenced this issue Jul 24, 2024
@cowtowncoder
Copy link
Member

Yeah, I think I was not a big fan of all plumbing needed to fix #562, but it is a very useful feature.
Glad that simple change of ordering resolves at least one problem.

We still have a few other problems left, fwtw.

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