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

Add @Inherited annotation to all Jackson annotations #198

Open emmersonbonnat-tw opened this issue 3 years ago • 6 comments

We migrated to Micronaut 3.3.3 from 2.x recently and since Micronaut 3.0 there is a breaking change in Annotation Inheritance. Due to this the Jackson annotations that are not annotated with @java.lang.annotation.Inherited are not inherited from parent classes and interfaces.

Can you add @inherited annotation to all Jackson annotations?

emmersonbonnat-tw avatar Mar 04 '22 07:03 emmersonbonnat-tw

This could go in 2.14; not sure how much time I personally have but could help with PRs; meaning "contributions welcome".

Note that there are annotations both in the main annotations package (jackson-annotations) and some here within jackson-databind. Also it's probably worth considering whether there are annotations that should NOT be marked as inheritable -- I suspect some would be.

My only concern is whether there might be regressions due to handling by other frameworks. I don't know if there is any way to check this.

Other than that no objections: Jackson itself does not care about this annotation so behavior would not be changed.

cowtowncoder avatar Mar 04 '22 17:03 cowtowncoder

@cowtowncoder I would think it would have an impact on jackson, because @Inherited affects the behavior of the normal reflection getAnnotation

yawkat avatar Mar 04 '22 18:03 yawkat

@yawkat I may be wrong here but since Jackson handles inheritance on its own it uses "bulk" access methods for Class annotations (for which @Inherited may be considered) which I think do not process @Inherited. Direct getAnnotation() is only used for Members (Fields, Methods, Constructors), and for those I think @Inherited is not processed in any way (which is a major reason why Jackson had to handle inheritance on its own in the first place).

But if this did have an effect the main concern would be mix-in processing which could definitely be broken.

I guess whoever worked on this would need to add non-trivial testing for Class annotations wrt mix-ins to ensure there is no regression.

cowtowncoder avatar Mar 04 '22 18:03 cowtowncoder

Ah that makes sense.

There are definitely some micronaut bugs in this area also, since we introspect the annotations ourselves too, but don't follow jacksons annotation inheritance behavior properly. It may be safer to fix this on the micronaut side of things.

(I don't know about other frameworks)

yawkat avatar Mar 04 '22 18:03 yawkat

What is the status of this issue? Is it still planned for 2.14?

We'd like to see @Inherited at least on @JsonFilter and it would be nice to have it on@JsonIgnore. In general, I think it would make sense to have @Inherited on all annotations possible.

rearl avatar Sep 30 '22 09:09 rearl

@rearl The issue is the fact that Jackson itself has no benefit from this at all (inheritance is implemented explicitly without regards to this annotation, to support field/method/constructor annotation inheritance, not just class ones); but the potential benefits and risks are for other libraries/frameworks... and we don't have good ways to really gauge that part at all.

So no current plans unless someone steps up and:

  1. Proposes changes (PR against jackson-annotations and/or jackson-databind)
  2. Tests with some popular framework/library to show benefits (and explain those in PR)

I do not have either time or itch for doing this: I am not against it, but if this is to be done, someone else needs to be driving the change. My main concern is that there could be potential downsides if some frameworks (say, Swagger generators?) were not counting on getting inheritance.

Timing-wise it ideally would have done before 2.14.0-rc1, but if it was done really quickly now, I am thinking of 2.14.0-rc2 within a week or two.

cowtowncoder avatar Sep 30 '22 16:09 cowtowncoder