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

Regression: custom composed @Profile annotation without runtime retention no longer supported with component scanning #23901

Closed
pavelspicak opened this issue Oct 31, 2019 · 10 comments
Labels
status: declined A suggestion or change that we don't feel we should currently apply status: feedback-provided Feedback has been provided type: regression A bug that is also a regression

Comments

@pavelspicak
Copy link

pavelspicak commented Oct 31, 2019

Affects: Spring 5.2


If I create custom @Profile annotation such as this:

@Profile("sapConnector")
public @interface SapConnectorProfile {
}

and then annotate some bean with @SapConnectorProfile and set the property spring.profiles.active=demoConnector, the bean is still loaded.

If I use @Profile("sapConnector") directly on the bean, the bean is not loaded.

My original question on stack overflow: https://stackoverflow.com/questions/58554912/custom-spring-profile-annotation

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 31, 2019
@sbrannen sbrannen changed the title Custom @Profile annotation Regression: custom composed @Profile annotation no longer supported Oct 31, 2019
@sbrannen
Copy link
Member

@pavelspicak, did you declare your custom annotation with runtime retention as follows?

@Retention(RetentionPolicy.RUNTIME)
@Profile("sapConnector")
public @interface SapConnectorProfile {
}

@sbrannen sbrannen added the status: waiting-for-feedback We need additional information before we can continue label Oct 31, 2019
@sbrannen
Copy link
Member

Also, how is the component annotated with @SapConnectorProfile registered in the ApplicationContext?

Is that via component scanning, @Bean method registration, or some other means?

@kingtran2112
Copy link

kingtran2112 commented Nov 1, 2019

Hi, I get a similar problem with my composed annotation contains @Component and @javax.enterprise.inject.Stereotype. But Spring cannot scan it anymore. I just saw it was mentioned in the upgrading document as a side effect but is my problem on purpose?

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 1, 2019
@pavelspicak
Copy link
Author

pavelspicak commented Nov 1, 2019

Hello,

you are right @sbrannen. I was missing @Retention(RetentionPolicy.RUNTIME). It was not necessary in Spring 4.

With @Retention(RetentionPolicy.RUNTIME) it works again.

Thank you very much.

@kingtran2112
Copy link

It works for me also but I curious why it needs the annotation for version 5.2 since before it wasn't?

@sbrannen
Copy link
Member

sbrannen commented Nov 1, 2019

@pavelspicak and @kingtran2112, thanks for the feedback. Glad to hear that it works now for you.

Please note, however, that @Retention(RetentionPolicy.RUNTIME) is required for any annotation in Java if the annotation needs to be found at runtime. Spring always evaluates annotations at runtime. Thus it has always been required for use with Spring.

In light of that, it is not possible that it worked before Spring Framework 5.2 without @Retention(RetentionPolicy.RUNTIME).

@sbrannen sbrannen closed this as completed Nov 1, 2019
@sbrannen sbrannen added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 1, 2019
@pavelspicak
Copy link
Author

pavelspicak commented Nov 1, 2019

I am sorry but you are not right with "In light of that, it is not possible that it worked before Spring Framework 5.2 without @Retention(RetentionPolicy.RUNTIME)."

I have created a small app with Spring Boot version 1.5.22.RELEASE and even when @Retention(RetentionPolicy.RUNTIME) is commented then DevComponent class is not created, when spring.profiles.active=test is set. But in the Spring Boot 2.2.0.RELEASE it loads both DevComponent and TestComponent.

You can take a look here: https://github.com/pavelspicak/customProfileAnnotation

If you run ./gradlew bootRun you will see log message:

  1. with Spring Boot version '1.5.22.RELEASE' (only TestComponent is created):

2019-11-01 13:27:21.945 INFO 44112 --- [ main] c.s.c.CustomProfileAnnotationApplication : The following profiles are active: test
2019-11-01 13:27:22.034 INFO 44112 --- [ main] s.c.a.AnnotationConfigApplicationContext : Refreshing org.springframework.context.annotation.AnnotationConfigApplicationContext@1da51a35: startup date [Fri Nov 01 13:27:22 CET 2019]; root of context hierarchy
2019-11-01 13:27:22.570 INFO 44112 --- [ main] c.s.c.test.TestComponent : test component created.
2019-11-01 13:27:22.805 INFO 44112 --- [ main] o.s.j.e.a.AnnotationMBeanExporter : Registering beans for JMX exposure on startup
2019-11-01 13:27:22.828 INFO 44112 --- [ main] c.s.c.CustomProfileAnnotationApplication : Started CustomProfileAnnotationApplication in 1.362 seconds (JVM running for 2.127)

  1. with Spring Boot version '2.2.0.RELEASE' (TestComponent and DevComponent are created):

2019-11-01 13:28:12.612 INFO 44142 --- [ main] c.s.c.CustomProfileAnnotationApplication : The following profiles are active: test
2019-11-01 13:28:13.056 INFO 44142 --- [ main] c.s.c.dev.DevComponent : dev component created.
2019-11-01 13:28:13.057 INFO 44142 --- [ main] c.s.c.test.TestComponent : test component created.
2019-11-01 13:28:13.118 INFO 44142 --- [ main] c.s.c.CustomProfileAnnotationApplication : Started CustomProfileAnnotationApplication in 0.929 seconds (JVM running for 1.287)

@sbrannen
Copy link
Member

sbrannen commented Nov 1, 2019

@pavelspicak, thanks for providing the example project. I'll take a look.

@sbrannen sbrannen changed the title Regression: custom composed @Profile annotation no longer supported Regression: custom composed @Profile annotation no longer supported with component scanning Nov 1, 2019
@sbrannen sbrannen changed the title Regression: custom composed @Profile annotation no longer supported with component scanning Regression: custom composed @Profile annotation without runtime retention no longer supported with component scanning Nov 1, 2019
@sbrannen sbrannen added status: declined A suggestion or change that we don't feel we should currently apply type: regression A bug that is also a regression and removed status: invalid An issue that we don't feel is valid labels Nov 3, 2019
@sbrannen
Copy link
Member

sbrannen commented Nov 3, 2019

Hi @pavelspicak,

I am sorry but you are not right with "In light of that, it is not possible that it worked before Spring Framework 5.2 without @Retention(RetentionPolicy.RUNTIME)."

You're correct!

It turns out that my statement was only partially true. So I apologize for that.

My statement was true for any annotations looked up by Spring using reflection. However, as your example project demonstrates, the same was not true for annotations looked up via ASM (i.e., by analyzing the compiled byte code directly).

So, thanks for providing the example project, because that helped me get to the bottom of this mystery! 👍

Speaking of which, I executed javap -v DevComponent.class which revealed the following.

SourceFile: "DevComponent.java"
RuntimeVisibleAnnotations:
  0: #20()
    org.springframework.stereotype.Component
RuntimeInvisibleAnnotations:
  0: #22()
    com.spicak.customProfileAnnotation.dev.DevProfile

That showed that something must have been reading the "invisible" @DevProfile annotation, even though it should not have been doing so. Some further investigation lead me to the following conclusion...

Prior to Spring Framework 5.2, our ASM-based annotation processing also looked up class retention annotations by accident. This bug was fixed in #22886, and the new MergedAnnotations API internals also implement the same behavior (see #22884).

As of Spring Framework 5.2, all annotations are now required to be annotated with @Retention(RetentionPolicy.RUNTIME) in order for Spring to see them.

In addition, please note that Spring only uses ASM to look up annotations during component scanning (e.g., for annotations on component-scanned classes such as @Component, @Service, @Configuration, @Profile, @Import, etc.), JPA @Entity scanning, and JAXB2 mapped class scanning, as well as for annotation metadata supplied to implementations of ImportSelector and ImportBeanDefinitionRegistrar. Otherwise, Spring uses reflection to look up annotations.

In other words, most user code should not be affected by this change in Spring Framework 5.2. I'll add a note to the Core Container upgrade notes to point this out as well.

I hope this clarifies everything, and thanks again for sticking your ground and providing a failing example project!

Cheers,

Sam

@pavelspicak
Copy link
Author

Hi @sbrannen,

thank you very much for the clarification.
Best regards,
Pavel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply status: feedback-provided Feedback has been provided type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

4 participants