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

No way to explicitly disable wrapping in custom annotation processor #4595

Closed
SimonCockx opened this issue Jun 22, 2024 · 6 comments
Closed

Comments

@SimonCockx
Copy link

When upgrading from Jackson 2.15.0 to 2.17.1, we noticed our custom annotation processor stopped working due to a change in behaviour in the way PropertyNames are merged. See #4364. I would like to know whether this is intended, and if it is, how I should migrate my code.

We have a custom XML annotation introspector which explicitly sets the wrapper name to Property.NO_NAME if our custom annotation is present and the type is a list:

    @Override
    public PropertyName findWrapperName(Annotated ann) {
        if (ann.hasAnnotation(RosettaAttribute.class) && hasCollectionType(ann)) {
            // Disable wrapping of lists.
            return PropertyName.NO_NAME;
        }
        return super.findWrapperName(ann);
    }

This used to work in Jackson 2.15.

Unfortunately, due to the change in #4364, PropertyName.NO_NAME is now merged with PropertyName.DEFAULT from the default Jackson XML annotation introspector, and the new behaviour prefers PropertyName.DEFAULT over PropertyName.NO_NAME. The previous behaviour would prefer PropertyName.NO_NAME.

I have not found a way to fix this. Is there a way to get rid of the default annotation processor?

Originally posted by @SimonCockx in #4364 (comment)

@cowtowncoder
Copy link
Member

Ok, first work-arounds possible...

Logic change is in AnnotationIntrospectorPair, which is used when more than one introspector is to be used. To avoid its use, and merging logic, you might be able to sub-class default AnnotationIntrospector (for plain JSON it'd be JacksonAnnotationIntrospector, but for XML JacksonXmlAnnotationIntrospector), instead of registering your own.

If you do need to combine dynamically multiple AnnotationIntrospectors, it'd be possible to sub-class AnnotationIntrospectorPair but that might get complicated.
Actual merging logic is defined in PropertyName.merg(...)

@cowtowncoder
Copy link
Member

But so, on possible actual fix. I think PropertyName.merge(...) needs to consider 2 special instances (NO_NAME and USE_DEFAULT) and return as-is, if primary value is either one.

I could make this change, although bit hesitant to do in 2.17(.2), instead of 2.18.

But one thing that I think I would need is a test case -- could you create a unit test to show the problem? It would need to go in jackson-dataformat-xml, but I can take care of that part.

In the meantime, I'll create a PR to show what might work.

@cowtowncoder
Copy link
Member

cowtowncoder commented Jun 22, 2024

Actually, I think only NO_NAME should have such special semantics.

Also, there is also the question of proper placement; in some ways, check in AnnotationIntrospectorPair would seem better, avoiding calls to secondary introspector.
But I think I will start with more universal solution of PropertyName.merge() checking (alternative can be added later on if need be).

@cowtowncoder
Copy link
Member

Created #4596 (against 2.17) with proposed fix.

@SimonCockx
Copy link
Author

Thanks for the quick resolution! Is there an ETA for the release by any chance?

To avoid its use, and merging logic, you might be able to sub-class default AnnotationIntrospector (for plain JSON it'd be JacksonAnnotationIntrospector, but for XML JacksonXmlAnnotationIntrospector), instead of registering your own.

How do I register my custom subclass of the default introspector? While I really have dug into the code, I have not found a way to "replace" the default, rather than registering a new one.

But one thing that I think I would need is a test case -- could you create a unit test to show the problem? It would need to go in jackson-dataformat-xml, but I can take care of that part.

Created a test here: FasterXML/jackson-dataformat-xml#659

@cowtowncoder
Copy link
Member

cowtowncoder commented Jun 24, 2024 via email

cowtowncoder added a commit that referenced this issue Jun 25, 2024
LuciferYang pushed a commit to apache/spark that referenced this issue Jul 9, 2024
### What changes were proposed in this pull request?

This PR amis to upgrade `fasterxml.jackson` from 2.17.1 to 2.17.2.

### Why are the changes needed?

There are some bug fixes about [Databind](https://github.com/FasterXML/jackson-databind):
[#4561](FasterXML/jackson-databind#4561): Issues using jackson-databind 2.17.1 with Reactor (wrt DeserializerCache and ReentrantLock)
[#4575](FasterXML/jackson-databind#4575): StdDelegatingSerializer does not consider a Converter that may return null for a non-null input
[#4577](FasterXML/jackson-databind#4577): Cannot deserialize value of type java.math.BigDecimal from String "3." (not a valid representation)
[#4595](FasterXML/jackson-databind#4595): No way to explicitly disable wrapping in custom annotation processor
[#4607](FasterXML/jackson-databind#4607): MismatchedInput: No Object Id found for an instance of X to assign to property 'id'
[#4610](FasterXML/jackson-databind#4610): DeserializationFeature.FAIL_ON_UNRESOLVED_OBJECT_IDS does not work when used with Polymorphic type handling

The full release note of 2.17.2:
https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.17.2

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass GA.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #47241 from wayneguow/upgrade_jackson.

Authored-by: Wei Guo <[email protected]>
Signed-off-by: yangjie01 <[email protected]>
ericm-db pushed a commit to ericm-db/spark that referenced this issue Jul 10, 2024
### What changes were proposed in this pull request?

This PR amis to upgrade `fasterxml.jackson` from 2.17.1 to 2.17.2.

### Why are the changes needed?

There are some bug fixes about [Databind](https://github.com/FasterXML/jackson-databind):
[apache#4561](FasterXML/jackson-databind#4561): Issues using jackson-databind 2.17.1 with Reactor (wrt DeserializerCache and ReentrantLock)
[apache#4575](FasterXML/jackson-databind#4575): StdDelegatingSerializer does not consider a Converter that may return null for a non-null input
[apache#4577](FasterXML/jackson-databind#4577): Cannot deserialize value of type java.math.BigDecimal from String "3." (not a valid representation)
[apache#4595](FasterXML/jackson-databind#4595): No way to explicitly disable wrapping in custom annotation processor
[apache#4607](FasterXML/jackson-databind#4607): MismatchedInput: No Object Id found for an instance of X to assign to property 'id'
[apache#4610](FasterXML/jackson-databind#4610): DeserializationFeature.FAIL_ON_UNRESOLVED_OBJECT_IDS does not work when used with Polymorphic type handling

The full release note of 2.17.2:
https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.17.2

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass GA.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#47241 from wayneguow/upgrade_jackson.

Authored-by: Wei Guo <[email protected]>
Signed-off-by: yangjie01 <[email protected]>
jingz-db pushed a commit to jingz-db/spark that referenced this issue Jul 22, 2024
### What changes were proposed in this pull request?

This PR amis to upgrade `fasterxml.jackson` from 2.17.1 to 2.17.2.

### Why are the changes needed?

There are some bug fixes about [Databind](https://github.com/FasterXML/jackson-databind):
[apache#4561](FasterXML/jackson-databind#4561): Issues using jackson-databind 2.17.1 with Reactor (wrt DeserializerCache and ReentrantLock)
[apache#4575](FasterXML/jackson-databind#4575): StdDelegatingSerializer does not consider a Converter that may return null for a non-null input
[apache#4577](FasterXML/jackson-databind#4577): Cannot deserialize value of type java.math.BigDecimal from String "3." (not a valid representation)
[apache#4595](FasterXML/jackson-databind#4595): No way to explicitly disable wrapping in custom annotation processor
[apache#4607](FasterXML/jackson-databind#4607): MismatchedInput: No Object Id found for an instance of X to assign to property 'id'
[apache#4610](FasterXML/jackson-databind#4610): DeserializationFeature.FAIL_ON_UNRESOLVED_OBJECT_IDS does not work when used with Polymorphic type handling

The full release note of 2.17.2:
https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.17.2

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass GA.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#47241 from wayneguow/upgrade_jackson.

Authored-by: Wei Guo <[email protected]>
Signed-off-by: yangjie01 <[email protected]>
attilapiros pushed a commit to attilapiros/spark that referenced this issue Oct 4, 2024
### What changes were proposed in this pull request?

This PR amis to upgrade `fasterxml.jackson` from 2.17.1 to 2.17.2.

### Why are the changes needed?

There are some bug fixes about [Databind](https://github.com/FasterXML/jackson-databind):
[apache#4561](FasterXML/jackson-databind#4561): Issues using jackson-databind 2.17.1 with Reactor (wrt DeserializerCache and ReentrantLock)
[apache#4575](FasterXML/jackson-databind#4575): StdDelegatingSerializer does not consider a Converter that may return null for a non-null input
[apache#4577](FasterXML/jackson-databind#4577): Cannot deserialize value of type java.math.BigDecimal from String "3." (not a valid representation)
[apache#4595](FasterXML/jackson-databind#4595): No way to explicitly disable wrapping in custom annotation processor
[apache#4607](FasterXML/jackson-databind#4607): MismatchedInput: No Object Id found for an instance of X to assign to property 'id'
[apache#4610](FasterXML/jackson-databind#4610): DeserializationFeature.FAIL_ON_UNRESOLVED_OBJECT_IDS does not work when used with Polymorphic type handling

The full release note of 2.17.2:
https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.17.2

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Pass GA.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#47241 from wayneguow/upgrade_jackson.

Authored-by: Wei Guo <[email protected]>
Signed-off-by: yangjie01 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants