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

Support late detection of ApplicationListener interface in objects returned from @Bean methods [SPR-6060] #10728

Closed
spring-projects-issues opened this issue Sep 1, 2009 · 7 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

Thomas opened SPR-6060* and commented

Issue attempting to directly implement multiple unrelated interfaces (business interface plus ApplicationListener) in a JavaConfig managed bean (JavaConfig 1.0.0.M4).

Either the generated proxy and or behavior of the container with regard to ApplicationListener support are not as expected.

Attached is a single class test case to demonstrate the issue. Switch return type in configuration and observe the change in behavior:

  • For return type EchoService, JdkDynamicAopProxy exposed will implement ApplicationListener but bean not receive events (wrong?!).
  • For return type EchoServiceImpl, JdkDynamicAopProxy is returned instead (wrong?!), and the bean receives events!

Affects: 2.5.6

Reference URL: http://forum.springsource.org/showthread.php?t=76853

Attachments:

@spring-projects-issues
Copy link
Collaborator Author

Chris Beams commented

Attaching updated version of original repro class. This is up-to-date with Spring 3 support for @Configuration classes. Note the use of @ImportResource to pull in an XML snippet that enables tx:annotation-driven/ and context:mbean-export/. The @AnnotationDrivenTx and @MBeanExport annotations are no longer supported as of the move to Spring 3. @AnnotationDrivenConfig is not necessary, as it's on by default when using @Configuration classes.

@spring-projects-issues
Copy link
Collaborator Author

Chris Beams commented

Attaching SPR6060Tests and aop-config.xml - a more concise reproduction of the issues in question.

@spring-projects-issues
Copy link
Collaborator Author

Chris Beams commented

After reviewing this issue, the behavior in question is not actually a bug, but rather simply the way the Spring container works.

When a bean class is declared in XML, the class is evaluated to see if it implements any Spring-specific interfaces, such as ApplicationListener. This works quite well, given that in XML, one always declares a concrete type as the bean class.

However, in @Bean methods, it becomes desirable to declare the return type as the most abstract type available, hiding the implementation details within the method. Unfortunately, it is this same return type that is regarded as the bean class. Thus, in the case of your original example, because the EchoService does not directly extend the ApplicationListener interface, the container has no way of knowing that the actual EchoServiceImpl instance returned actually does.

As a workaround, you could modify the EchoService interface to extend ApplicationListener, but this is not ideal.

It is at least conceivable that the framework could evaluate bean instances after creation (as opposed to bean classes before creation) to determine whether ApplicationListener and other such interfaces are implemented. This, however, is not a trivial change and will require some deliberation. Given that we're on the eve of the RC2 release, I am a) changing this issue from a Bug to an Improvement, and b) slating it for 3.0.1, which will give us some time to think about it.

In the meantime, you have two options:

  1. as mentioned above, modify the EchoService interface to implement ApplicationListener (not recommended)
  2. simply return the concrete type from the @Bean method (recommended)

@spring-projects-issues
Copy link
Collaborator Author

Thomas commented

Returning the concrete type is equivalent to what is happening when declaring the bean in XML, even though it is not intuitive when using the "nice" Java based configuration approach.

It initially appears rather strange when the type returned from the application context is not the type declared on the method (JdkDynamicAopProxy which erases concrete type). Cases where the caller would expect and cast to the concrete class will fail. Again, this is just what would happen with XML, but seems rather counter intuitive when doing everything in Java.

I had to use option (1.) in the actual application I work on, just returning the concrete class implementing all interfaces (2.) would not work. I was not able to reproduce it in a simple test case. If I ever find out why I will update this ticket, perhaps the issue will magically go away when moving to Spring 3.x..

I read now there is the need to use @ImportResource and go back to XML just for tx:annotation-driven/ and context:mbean-export/? Isn't that a step backwards given in 2.x that was all working with plain Java code?

@spring-projects-issues
Copy link
Collaborator Author

Chris Beams commented

Hi Thomas,

Returning the concrete type is equivalent to what is happening when declaring the bean in XML, even though it is not intuitive when using the "nice" Java based configuration approach.

Agreed. Ideally, it would be possible to return an abstract type or interface and still have container-specific interfaces recognized against the concrete bean type. This issue remains open to prompt further looking into that possibility

It initially appears rather strange when the type returned from the application context is not the type declared on the method (JdkDynamicAopProxy which erases concrete type). Cases where the caller would expect and cast to the concrete class will fail. Again, this is just what would happen with XML, but seems rather counter intuitive when doing everything in Java.

As you mention, this is exactly the way it works in XML, and is usually not a problem because folks preferentially assign beans to an interface rather than to the concrete bean type. Keep in mind, however, that if you need/want to assign to the concrete type, you can always switch away from using JDK dynamic proxies and go with the CGLIB-based subclass proxy approach. This is done through the proxy-target-class attribute of namespace elements such as <tx:annotation-driven/>

I read now there is the need to use @ImportResource and go back to XML just for tx:annotation-driven/ and context:mbean-export/? Isn't that a step backwards given in 2.x that was all working with plain Java code?

Yes - @ImportResource provides a single, consistent way to access all namespace-related features without creating a proliferation of annotations such as @AnnotationDrivenTx. While I agree that this is not ideal (because it's not pure Java), it is probably the right 'conservative' approach for the 3.0 roll-out. The rationale is two-fold:

  1. Annotations are not a proper fit for all of the functionality exposed via namespaces in Spring XML. Some are more or less straight-across (such as <tx:annotation-driven/> and {{@AnnotationDrivenTx), but others are not
  2. With the benefit of some refactoring, we may decide to expose the APIs underneath the XML namespaces for direct usage within @Configuration classes. Today, these APIs are rather fused to XML-processing and cannot be used conveniently in a standalone fashion (see the NamespaceHandler hierarchy for details).

There are a number of decisions to be made here, but we do intend to shore up this gap. In the meantime, @ImportResource provides a simple, flexible 'escape hatch' to XML (or any other bean definition source), and buys some time in which we can get more of this kind of valuable user feedback.

Thank you, and please keep the comments coming. It's rather important to know how @Configuration classes are working out, and whether they provide what you need.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

After a bit of research, this seems to affect the ApplicationListener interface only: All other common container interfaces are detected against the instantiated object, independent from the declared type.

ApplicationListeners get detected through by-type retrieval against bean definitions. As a special case, we also check inner bean instances (since Spring 3.0). So it seems we just need to add an instance check for top-level beans as well, for cases where haven't detected the ApplicationListener interface against the declared type.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Fixed as of 3.0.1: ApplicationListeners will get detected lazily as well now, through a revision of ApplicationContextAwareProcessor's listener detection.

Juergen

@spring-projects-issues spring-projects-issues added type: enhancement A general enhancement in: core Issues in core modules (aop, beans, core, context, expression) labels Jan 11, 2019
@spring-projects-issues spring-projects-issues added this to the 3.0.1 milestone Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants