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

Annotations on superclasses are detected by StandardAnnotationMetadata [SPR-11595] #16219

Closed
spring-projects-issues opened this issue Mar 24, 2014 · 6 comments
Assignees
Labels
in: core type: regression
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Mar 24, 2014

Phil Webb opened SPR-11595 and commented

Background

The StandardAnnotationMetadata class in Spring Framework 4.0.2 returns false for isAnnotated when a class is not directly annotated with an annotation (even if the annotation is present on a superclass). Furthermore, getAnnotationAttributes and getAllAnnotationAttributes return null when querying for the same annotation.


Analysis

The fix for #16100 changed this logic and is causing problems with Spring Boot conditions. Specifically, @Conditional annotations were never previously considered on super classes (see #15466).


Affects: 4.0.3

Issue Links:

  • #15466 @Conditional on superclass is ignored
  • #16100 Favor more locally declared composed annotations over inherited annotations in AnnotationUtils
  • #16221 Favor more locally declared composed annotations over inherited annotations

Referenced from: commits a2f1169

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 24, 2014

Phil Webb commented

Sam,

Take a look at https://github.com/philwebb/spring-framework/tree/SPR-11595 for a test case and potential fix. Unfortunately the fix breaks AnnotatedElementUtilsTests which explicitly tests for the exact opposite of the problem.

The failing test in Spring Boot that triggered all this is ManagementSecurityAutoConfigurationTests. It returns true for has annotations, but the attribute map doesn't contain a value.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 25, 2014

Sam Brannen commented

This has been resolved as described in the comments for GitHub commit a2f1169:

Don't detect annotations on superclass in StAnMeta

Changes introduced in conjunction with issue #16100 altered the
behavior of StandardAnnotationMetadata such that annotations could be
detected on superclasses, specifically in the case where the
AnnotatedElementUtils.getAllAnnotationAttributes() method is invoked to
obtain multiple annotations of the same type (on the lowest level in the
class hierarchy), as is the case for @Profile and @Conditional.

This commit partially reverts these changes as follows:

  • All methods in AnnotatedElementUtils now set the
    traverseClassHierarchy to false, thereby effectively reverting the
    changes made in commit 1d30bf8.
    Note, however, that the changes made to AnnotationUtils remain in
    place.

  • Introduced tests in AnnotationMetadataTests that verify behavior
    present in Spring Framework 4.0.2 and earlier.

  • Updated tests in AnnotatedElementUtilsTests so that they pass against
    the reverted changes (i.e., align with the behavior present in Spring
    Framework 4.0.2 and earlier).

  • Refined Javadoc in AnnotationMetadata with regard to annotations
    being "present" vs. "defined".

  • Refined Javadoc in AnnotatedTypeMetadata.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 25, 2014

Sam Brannen commented

Phil Webb, please verify the correctness of the changes against the next 4.0.3 snapshot running against Spring Boot.

Thanks!

Sam

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 25, 2014

Phil Webb commented

Sam Brannen, The tests all pass now, so it looks good.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 25, 2014

Sam Brannen commented

Great! :)

Thanks for letting me know so quickly.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 25, 2014

Sam Brannen commented

Note: #16221 is now a follow up for this issue.

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

No branches or pull requests

2 participants