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

AnnotatedElementUtils fails to find annotations on abstract, bridge, or interface methods [SPR-12738] #17335

Closed
spring-issuemaster opened this issue Feb 20, 2015 · 11 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Feb 20, 2015

Rossen Stoyanchev opened SPR-12738 and commented

Overview

This was encountered while adding support for @RequestMapping as meta annotation. Following a change from AnnotationUtils.findAnnotation to AnnotatedElementUtils.getAnnotationAttributes, there are failing integration tests with controller hierarchies.


Analysis / Known Issues

  1. AnnotatedElementUtils does not currently support interfaces or bridge methods in its search algorithms.

Deliverables

  1. Support abstract methods in AnnotatedElementUtils.
  2. Support interface methods in AnnotatedElementUtils.
  3. Support bridge methods in AnnotatedElementUtils.
  4. Support bridged methods in AnnotatedElementUtils.
  5. Search for TODOs in AnnotatedElementUtilsTests and re-instate @Ignore'd tests.
  6. Search for TODOs in TransactionalEventListenerTests and re-instate @Ignore'd tests.

Affects: 4.1.5

Issue Links:

  • #16901 Custom @RequestMapping annotations ("is depended on by")
  • #17300 AnnotatedElementUtils does not find annotations on methods in dynamic proxies
  • #15734 Support composed annotations declared on interfaces
  • #16221 Favor more locally declared composed annotations over inherited annotations
  • #17537 AnnotatedElementUtils fails to find type-level annotations on interfaces
  • #16331 Consider not overriding meta-annotation attributes if empty

Referenced from: commits 7f0f04d, 91e46cf, bccd59e, 8ece1b1, ad6bea1, e0d2dbd, b94c6fd

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 20, 2015

Rossen Stoyanchev commented

Tests added in b94c6fd.

For example, ServletAnnotationControllerHandlerMethodTests has a number of failures (that's in the temporary branch for @RequestMapping meta-annotation support).

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 19, 2015

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 22, 2015

Sam Brannen commented

FYI: support for finding annotations at the type level on interfaces is addressed by #17537.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 23, 2015

Sam Brannen commented

Completed as described in GitHub commit ad6bea1:

Support abstract, bridge, & interface methods in AnnotatedElementUtils

This commit introduces support for finding annotations on abstract,
bridge, and interface methods in AnnotatedElementUtils.

  • Introduced dedicated findAnnotationAttributes() methods in
    AnnotatedElementUtils that provide first-class support for
    processing methods, class hierarchies, interfaces, bridge methods,
    etc.

  • Introduced find/get search algorithm dichotomy in
    AnnotatedElementUtils which is visible in the public API as well as
    in the internal implementation. This was necessary in order to
    maintain backwards compatibility with the existing API (even though
    it was undocumented).

  • Reverted all recent changes made to the "get semantics" search
    algorithm in AnnotatedElementUtils in order to ensure backwards
    compatibility, and reverted recent changes to
    JtaTransactionAnnotationParser and SpringTransactionAnnotationParser
    accordingly.

  • Documented internal AnnotatedElementUtils.Processor<T> interface.

  • Enabled failing tests and introduced
    findAnnotationAttributesFromBridgeMethod() test in
    AnnotatedElementUtilsTests.

  • Refactored ApplicationListenerMethodAdapter.getCondition() and
    enabled failing test in TransactionalEventListenerTests.

  • AnnotationUtils.isInterfaceWithAnnotatedMethods() is now package
    private.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 23, 2015

Sam Brannen commented

Important

To utilize these features, you will need to invoke AnnotatedElementUtils.findAnnotationAttributes(), not AnnotatedElementUtils.getAnnotationAttributes().

The existing getAnnotationAttributes() methods have been left in place and reworked so that they remain backwards compatible.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 23, 2015

Sam Brannen commented

Rossen Stoyanchev,

Note the (x) in this issue's description.

Basically, AnnotatedElementUtils supports finding annotations on a bridge method but not on a bridged method.

The test you introduced in AnnotatedElementUtilsTests (i.e., findAnnotationAttributesInheritedFromBridgedMethod()) is still @Ignore'd because it actually attempts to find an annotation on a bridged method. Note, however, that the newly introduced findAnnotationAttributesFromBridgeMethod() test works as expected.

Thus, I think your test might not adequately mimic what occurs at runtime. For example, in RequestMappingHandlerMapping it appears to me that all of the methods in a given candidate class will be iterated over reflectively. Thus annotated bridge methods should still be handled appropriately (I hope).

Can you please rebase your temporary branch against master and let me know if the new AnnotatedElementUtils.findAnnotationAttributes() method suits your needs?

Thanks!

Sam

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 24, 2015

Rossen Stoyanchev commented

Sure I'll give it a try.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 24, 2015

Rossen Stoyanchev commented

Sam Brannen, I rebased my branch but still seeing the same failures. Would you take a look? Run the spring-webmvc tests and the failures are in MvcUriComponentsBuilderTests, ServletAnnotationControllerHandlerMethodTests, and HandlerMethodAnnotationDetectionTests.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 24, 2015

Sam Brannen commented

Rossen Stoyanchev, I have not checked out your branch, but just by looking at the code on GitHub, I see that you are still invoking AnnotatedElementUtils.getAnnotationAttributes() which does not support the new search features.

Please switch to AnnotatedElementUtils.findAnnotationAttributes() (note the find) and let me know if that solves your problems!

Thanks,

Sam

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 24, 2015

Rossen Stoyanchev commented

All green!

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 24, 2015

Sam Brannen commented

Awesome! :)

Thanks for letting me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.