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

ApplicationEvent should take generics into account when dispatching events [SPR-8201] #12850

Closed
spring-issuemaster opened this issue Apr 4, 2011 · 10 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Apr 4, 2011

Jon Brisbin opened SPR-8201 and commented

The ApplicationEvent dispatcher should take generics information into account when deciding whether to call a particular ApplicationListener. This would allow the user to register an ApplicationListener for their event subclass like this:

public class BeforeSaveEvent<T> extends ApplicationEvent {}
public class MyListener implements ApplicationListener<BeforeSaveEvent<MyDomainObject>>{}

When a BeforeSaveEvent<MyDomainObject> event is dispatched, only those ApplicationListener that have specified MyDomainObject in the generics clause will be dispatched to the configured listener (such as MyListener)


Affects: 3.0.5

Issue Links:

  • #15847 improve ApplicationListener to support generic types ("duplicates")

2 votes, 6 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 6, 2015

Stéphane Nicoll commented

The problem is that (unless I am missing something) we need a Type instance to match the signature of the ApplicationListener implementation against the actual ApplicationEvent. Due to type erasure, the instance Class does not provide that information.

In theory, we could use getGenericSuperClass and getGenericInterfaces to walk the hierarchy tree for our event type (provided that we actually have access to the implementation declaration which is non trivial without breaking backward compatibility since we use wrapper extensively internally).

Back to the basics, we'd probably need something like this:

public interface SmartApplicationEventPublisher extends ApplicationEventPublisher {

	void publishEvent(ApplicationEvent event, Type evenType);

}

We would then need to either break SmartApplicationListener or create a GenericApplicationListener interface to provides the necessary hook points to validate that the evenType matches the expected event type.

We then need to call the smart publishEvent method. In order to build the Type instance, we need a declaration. Here are a couple of proposals:

EventPublisher injection

@Autowired
private EventPublisher<MyEvent<String>> publisher;

where the context would create/provide a generic implementation of such interface that takes the field declaration to extract the generic type of the EventPublisher (that is MyEvent<String>).

Users would then publish an event with publisher.publishEvent(new MyEvent<String>(this, "toto"));

Proxy implementation

@Component // ??
public interface MyEventPublisher {
	
	@EventPublisher
	void publish(MyEvent<String> event);
}

This is very similar to what Spring Data does with its Repository support. There are probably a lot of code in there that we would need to implement that properly.

Note that the interface/annotation names collide (EventPublisher) so we should probably find a different strategy if we want to support both. Besides EventPublisher looks really like ApplicationEventPublisher (except the latter doesn't have a generic type) so we should probably find a way to reconciliate those use cases.

Using generic type with the existing implementation

If one uses the regular mechanism to publish a generic event, we may or may not be able to detect what is necessary. If the event does not implement a common interface that we could use to resolve the generic parameters at runtime, we're probably screwed.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 7, 2015

Stéphane Nicoll commented

514d226c1 is a first cut for this feature. We're probably leaning towards ApplicationEventPublisher smart injection.

There are still two problems though:

  1. Existing code base will have to be refactored to support generic types as the current publishEvent on the multi caster won't work. This isn't obvious at all
  2. If a user injects ApplicationEventPulisher<GenericEvent<?>> and use it to sends a GenericEvent<String>, an ApplicationListener whose definition is ApplicationListener<GenericEvent<String>> will not be called since the Type of the event will be GenericEvent<?> (see multicastGenericEventWildcardSubType)

I'll give it more thoughts as we really need to find something better for this.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 10, 2015

Stéphane Nicoll commented

Turns out that we decided to limit ourselves to a basic support for now with the idea to improve it with community feedback. Generics are now fully supported in ApplicationListener implementation as long as the event type provides a signature that resolve said generics.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 4, 2015

Christian Rudolph commented

Is it wanted that compatibility with prior versions of spring-framework gets broken? For example spring-security (core and config) uses the old SmartApplicationListener and now fails to assign instances of GenericApplicationListenerAdapter to a list of SmartApplicationListener (e. g. in DelegatingApplicationListener).
Has this to be handled by spring-security?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 4, 2015

Stéphane Nicoll commented

Interesting, thanks. This is supposed to be fully backward compatible. Can you give more details about the issue so that I can have a look to it?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 5, 2015

Christian Rudolph commented

I can post further details later today. My setup isn't really special, I used the most current snapshot of spring-framework and the most recent release of spring-security (I think it is 4.0.1 or so).
This change breaks compatibility: 6d6422a#diff-c7ce4f9153e96392cf53a4e9749a2382R35

For example DelegatingApplicationListener of spring-security uses the former interface SmartApplicationListener. The new GenericApplicationListener isn't castable to SmartApplicationListener so the assignment to a variable of type SmartApplicationListener fails.
I dont't know from where the linked line gets called, but I can provide a stacktrace, later.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 5, 2015

Stéphane Nicoll commented

Yeah, I came to the same conclusion as well. Thanks for reporting it.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 5, 2015

Stéphane Nicoll commented

I can see why it can fail but looking at the source code I don't see any reason why it would fail. Could you please provide the stacktrace?

In any case, I've pushed a fix for what you reported. Can you try again with the latest Spring framework 4.2.0.BUILD-SNAPSHOT please? Thanks!

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 6, 2015

Christian Rudolph commented

Following the stacktrace:

2015-05-04 18:40:58,131 ERROR org.springframework.web.context.ContextLoader - Context initialization failed
java.lang.ClassCastException: org.springframework.context.event.GenericApplicationListenerAdapter cannot be cast to org.springframework.context.event.SmartApplicationListener
	at org.springframework.security.context.DelegatingApplicationListener.onApplicationEvent(DelegatingApplicationListener.java:41) ~[spring-security-core-4.0.1.RELEASE.jar:4.0.1.RELEASE]
	at org.springframework.context.event.SimpleApplicationEventMulticaster.invokeListener(SimpleApplicationEventMulticaster.java:163) ~[spring-context-4.2.0.BUILD-SNAPSHOT.jar:4.2.0.BUILD-SNAPSHOT]
	at org.springframework.context.event.SimpleApplicationEventMulticaster.multicastEvent(SimpleApplicationEventMulticaster.java:136) ~[spring-context-4.2.0.BUILD-SNAPSHOT.jar:4.2.0.BUILD-SNAPSHOT]
	at org.springframework.context.support.AbstractApplicationContext.publishEvent(AbstractApplicationContext.java:364) ~[spring-context-4.2.0.BUILD-SNAPSHOT.jar:4.2.0.BUILD-SNAPSHOT]
	at org.springframework.context.support.AbstractApplicationContext.publishEvent(AbstractApplicationContext.java:333) ~[spring-context-4.2.0.BUILD-SNAPSHOT.jar:4.2.0.BUILD-SNAPSHOT]
	at org.springframework.context.support.AbstractApplicationContext.finishRefresh(AbstractApplicationContext.java:828) ~[spring-context-4.2.0.BUILD-SNAPSHOT.jar:4.2.0.BUILD-SNAPSHOT]
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:524) ~[spring-context-4.2.0.BUILD-SNAPSHOT.jar:4.2.0.BUILD-SNAPSHOT]
	at org.springframework.web.context.ContextLoader.configureAndRefreshWebApplicationContext(ContextLoader.java:446) ~[spring-web-4.2.0.BUILD-SNAPSHOT.jar:4.2.0.BUILD-SNAPSHOT]
	at org.springframework.web.context.ContextLoader.initWebApplicationContext(ContextLoader.java:328) [spring-web-4.2.0.BUILD-SNAPSHOT.jar:4.2.0.BUILD-SNAPSHOT]
	at org.springframework.web.context.ContextLoaderListener.contextInitialized(ContextLoaderListener.java:107) [spring-web-4.2.0.BUILD-SNAPSHOT.jar:4.2.0.BUILD-SNAPSHOT]
	at org.apache.catalina.core.StandardContext.listenerStart(StandardContext.java:4770) [catalina.jar:8.0.15]
	at org.apache.catalina.core.StandardContext.startInternal(StandardContext.java:5196) [catalina.jar:8.0.15]
	at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:150) [catalina.jar:8.0.15]
	at org.apache.catalina.core.ContainerBase.addChildInternal(ContainerBase.java:725) [catalina.jar:8.0.15]
	at org.apache.catalina.core.ContainerBase.addChild(ContainerBase.java:701) [catalina.jar:8.0.15]
	at org.apache.catalina.core.StandardHost.addChild(StandardHost.java:714) [catalina.jar:8.0.15]
	at org.apache.catalina.startup.HostConfig.deployWAR(HostConfig.java:917) [catalina.jar:8.0.15]
	at org.apache.catalina.startup.HostConfig.deployApps(HostConfig.java:461) [catalina.jar:8.0.15]
	at org.apache.catalina.startup.HostConfig.check(HostConfig.java:1493) [catalina.jar:8.0.15]
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_31]
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_31]
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_31]
	at java.lang.reflect.Method.invoke(Method.java:483) ~[?:1.8.0_31]
	at org.apache.tomcat.util.modeler.BaseModelMBean.invoke(BaseModelMBean.java:300) [tomcat-coyote.jar:8.0.15]
	at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.invoke(DefaultMBeanServerInterceptor.java:819) [?:1.8.0_31]
	at com.sun.jmx.mbeanserver.JmxMBeanServer.invoke(JmxMBeanServer.java:801) [?:1.8.0_31]
	at org.apache.catalina.manager.ManagerServlet.check(ManagerServlet.java:1432) [catalina.jar:8.0.15]
	at org.apache.catalina.manager.ManagerServlet.deploy(ManagerServlet.java:711) [catalina.jar:8.0.15]
	at org.apache.catalina.manager.ManagerServlet.doPut(ManagerServlet.java:423) [catalina.jar:8.0.15]
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:647) [servlet-api.jar:?]
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:725) [servlet-api.jar:?]
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:291) [catalina.jar:8.0.15]
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) [catalina.jar:8.0.15]
	at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:52) [tomcat-websocket.jar:8.0.15]
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:239) [catalina.jar:8.0.15]
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) [catalina.jar:8.0.15]
	at org.apache.catalina.filters.SetCharacterEncodingFilter.doFilter(SetCharacterEncodingFilter.java:108) [catalina.jar:8.0.15]
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:239) [catalina.jar:8.0.15]
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) [catalina.jar:8.0.15]
	at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:219) [catalina.jar:8.0.15]
	at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:106) [catalina.jar:8.0.15]
	at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:613) [catalina.jar:8.0.15]
	at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:142) [catalina.jar:8.0.15]
	at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:79) [catalina.jar:8.0.15]
	at org.apache.catalina.valves.AbstractAccessLogValve.invoke(AbstractAccessLogValve.java:610) [catalina.jar:8.0.15]
	at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:88) [catalina.jar:8.0.15]
	at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:537) [catalina.jar:8.0.15]
	at org.apache.coyote.http11.AbstractHttp11Processor.process(AbstractHttp11Processor.java:1085) [tomcat-coyote.jar:8.0.15]
	at org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:658) [tomcat-coyote.jar:8.0.15]
	at org.apache.coyote.http11.Http11AprProtocol$Http11ConnectionHandler.process(Http11AprProtocol.java:277) [tomcat-coyote.jar:8.0.15]
	at org.apache.tomcat.util.net.AprEndpoint$SocketProcessor.doRun(AprEndpoint.java:2407) [tomcat-coyote.jar:8.0.15]
	at org.apache.tomcat.util.net.AprEndpoint$SocketProcessor.run(AprEndpoint.java:2396) [tomcat-coyote.jar:8.0.15]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) [?:1.8.0_31]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) [?:1.8.0_31]
	at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61) [tomcat-util.jar:8.0.15]
	at java.lang.Thread.run(Thread.java:745) [?:1.8.0_31]

The GenericApplicationListenerAdapter gets added to the listeners list in DelegatingApplicationListener without any complaints, because the type safety is checked at compile time. But when the onApplicationEvent method gets called, it tries to iterate over the listeners where they must be assigned to the foreach-loop's listener variable.

I just tried to run my code with the most current snapshot: it is fixed now. Thank you!

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 6, 2015

Stéphane Nicoll commented

Thanks for testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.