jmix-framework / jmix

Jmix framework
https://www.jmix.io
Apache License 2.0
693 stars 124 forks source link

UiEventListeners is typed ApplicationEvent and not a specific event type #3232

Open KremnevDmitry opened 6 months ago

KremnevDmitry commented 6 months ago

Environment

Jmix version: 2.2.999-SNAPSHOT

Bug Description

Look at the following methods:

  1. io.jmix.flowui.component.main.JmixUserIndicator#onApplicationEvent
  2. io.jmix.notificationsflowui.component.notificationsindicator.NotificationsIndicator#onApplicationEvent
  3. io.jmix.quartzflowui.view.jobs.JobModelListView#onApplicationEvent

All of them are typed on ApplicationEvent. Thus, when a UI event is published, all three listeners are triggered. In the implementation of these listeners, a type check occurs, and if the current event type matches the listener, then the listener processes it.

If you change method signatures to a specific event type, you will get an error.

E.g.:

 protected void onApplicationEvent(VaadinSessionNotificationEvent event) {
     // some code
 }

 protected void onApplicationEvent(UiUserSubstitutionsChangedEvent event) {
     // some code
 }

When publishing any UI event, filtering occurs by event type, but it is not correct. See:

  1. io.jmix.flowui.sys.event.UiEventsManager#filterComponentListeners
  2. io.jmix.flowui.sys.event.UiEventsManager#supportsEvent

According to this logic, any event time is suitable. Therefore, when publishing an event in the following method io.jmix.flowui.sys.event.UiEventsManager#invokeListener a java.lang.ClassCastException error occurs.

We need to add proper filtering and change the signature of the listener methods so that they are parameterized by a specific event type.

Sample Project

  1. Apply the following patch to the master branch: UiEventsManager_with_ApplicationEvent.patch
  2. Publish modules flowui, quartz-flowui, notifications-flowui to maven local
  3. Open the following project: ui-event-listeners-bug.zip
  4. Log in as admin/admin
  5. Open UserListView
  6. Add or remove UserSubstitutions for admin to trigger UiUserSubstitutionsChangedEvent
  7. An error occurs
KremnevDmitry commented 5 months ago

Currently, it is not possible to use the definition of a specific event type. Because ui event listeners are passed to the UiEventsManager as lambda functions. Such functions will not be able to resolve the type of generic parameter.

GenericApplicationListenerAdapter won't be able to do this either. See: https://github.com/spring-projects/spring-framework/issues/15303

So the current workaround is the only way.