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.

Sign up for GitHub

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

Jump to bottom

Unify standard and ASM AnnotatedTypeMetadata retention behavior #22886

Closed
philwebb opened this issue May 4, 2019 · 3 comments
Closed

Unify standard and ASM AnnotatedTypeMetadata retention behavior #22886

philwebb opened this issue May 4, 2019 · 3 comments
Assignees
@philwebb
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug

Comments

@philwebb
Copy link
Member

philwebb commented May 4, 2019

Reflection based AnnotatedTypeMetadata implementations only support @RetentionPolicy(RetentionPolicy.RUNTIME) annotations. ASM implementations however currently ignore the visible attribute and will read both RetentionPolicy.CLASS and RetentionPolicy.RUNTIME annotations.

I would like to unify the two implementations if possible.
@philwebb philwebb added the type: enhancement A general enhancement label May 4, 2019
@philwebb philwebb self-assigned this May 4, 2019
@philwebb
Copy link
Member Author

philwebb commented May 4, 2019

@jhoeller any problem with this? Is the current behavior intentional?

We have a few tests that fail when I make such a change, but I believe that they have just forgotten to use @RetentionPolicy(RetentionPolicy.RUNTIME).

@jhoeller
Copy link
Contributor

jhoeller commented May 4, 2019

Indeed, this was only ever meant to work with @RetentionPolicy(RetentionPolicy.RUNTIME)...

@jhoeller jhoeller added the in: core Issues in core modules (aop, beans, core, context, expression) label May 4, 2019
@philwebb
Copy link
Member Author

philwebb commented May 4, 2019

Thanks, I'll fix this when I look at #22884

philwebb added a commit to philwebb/spring-framework that referenced this issue May 4, 2019
@philwebb
Don't expose RetentionPolicy.CLASS annotations
070b95a 
Update ASM based metadata readers so that only RetentionPolicy.RUNTIME
annotations are exposed. This aligned behavior with the reflection based
implementation.

Closes spring-projectsgh-22886
philwebb added a commit to philwebb/spring-framework that referenced this issue May 6, 2019
@philwebb
Don't expose RetentionPolicy.CLASS annotations
cf3b333 
Update ASM based metadata readers so that only RetentionPolicy.RUNTIME
annotations are exposed. This aligned behavior with the reflection based
implementation.

Closes spring-projectsgh-22886
philwebb added a commit to philwebb/spring-framework that referenced this issue May 6, 2019
@philwebb
Don't expose RetentionPolicy.CLASS annotations
f4519b4 
Update ASM based metadata readers so that only RetentionPolicy.RUNTIME
annotations are exposed. This aligned behavior with the reflection based
implementation.

Closes spring-projectsgh-22886
@sbrannen sbrannen added type: bug A general bug and removed type: enhancement A general enhancement labels Nov 3, 2019
@sbrannen sbrannen mentioned this issue Nov 3, 2019
Closed
@wilkinsona wilkinsona mentioned this issue Feb 4, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@philwebb philwebb

Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@sbrannen @philwebb @jhoeller