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

@Enable registrars invoked with subclass for annotation placed on superclass (3.2.x) [SPR-11251] #15876

Closed
spring-projects-issues opened this issue Dec 20, 2013 · 4 comments
Assignees
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Dec 20, 2013

Oliver Drotbohm opened SPR-11251 and commented

If you have an @Enable… annotation, itself not being annotated with @Inherited, the related ImportBeanDefinitionRegistrar is still invoked multiple times when used in inheritance scenarios for configuration classes.

@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
@Import(SomeImporBeanDefinitionRegistrar.class)
public @interface MyAnnotation {

}

@Configuration
@MyAnnotation
public class Config {}

@Configuration
public class SubConfig {}

new AnnotationConfigApplicationContext(SubConfig.class);

Here are the erroneous effects:

  1. The ImportBeanDefinitionRegistrar gets invoked for the leaf class (SubConfig of the inheritance hierarchy (not the one the annotation is on)
  2. The AnnotationMetadata does then return null if you're trying to access the annotation attributes of the triggering annotation.

I've prepared more code samples and the log output for the invocations in this Gist. Spring 4 seems to handle this correct already.


Affects: 3.2.6

Issue Links:

  • DATAREST-210 ApplicationContext fails to load with HAL enabled snapshot
  • #15491 @EnableTransactionManagement and co should also get detected on superclasses
  • #14558 Prevent duplicate @Import processing and ImportBeanDefinitionRegistrar invocation
  • SEC-2425 Support EnableGlobalMethodSecurity on superclass
  • #14572 Backport fix for multiple invocations of ImportBeanDefinitionRegistrars
@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Dec 20, 2013

Oliver Drotbohm commented

Apparently this is a bit worse than expected. The linked tickets were dealing with an issue of multiple invocations of the registrar, while this one is about a wrong invocation (for the wrong config class and thus not carrying the annotation attributes). I've tried all release versions back to 3.2 M1 and none of them invokes the registrar correctly if the annotation is on the superclass and the context was created with a subtype of that configuration class.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Dec 20, 2013

Juergen Hoeller commented

Ollie, it seems this is effectively the same issue as #15491 where I've addressed the same kind of arrangement for the 4.0 line.

Indeed, it seems this was never actually supported, and never documented to be supported either. So from that perspective, it arguably doesn't have to be fixed for the 3.2.x line, except maybe for better error reporting there.

Do you see it as critical for 3.2.x or would you be alright with it only being supported as of 4.0 as well?

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Dec 23, 2013

Juergen Hoeller commented

I went forward with this one, since it turned out to be a local enough change - more local than in 4.0, actually. In that sense, it's not really a backport of #15491.

Anyway, it seems to work fine in 3.2.7 now. Would be great to validate that against a 3.2.7 snapshot, of course :-)

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Dec 26, 2013

Oliver Drotbohm commented

Thanks for fixing this in the 3.2.x line. I'd argue the issue is probably more considered a bug as annotations like @EnableTransactionManagement work in this scenario (as they're backend by an ImportSelector instead of an ImportBeanDefinitionRegistrar) and others will not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants