GH-1783 - Invoke listeners through SimpleApplicationEventMulticaster - #1784
Open
seonwooj0810 wants to merge 1 commit into
Open
GH-1783 - Invoke listeners through SimpleApplicationEventMulticaster#1784seonwooj0810 wants to merge 1 commit into
seonwooj0810 wants to merge 1 commit into
Conversation
…entMulticaster. PersistentApplicationEventMulticaster called ApplicationListener.onApplicationEvent(…) directly, which bypasses the ClassCastException tolerance that SimpleApplicationEventMulticaster.doInvokeListener(…) applies for lambda-defined listeners whose generic event type cannot be resolved. Such a listener - e.g. one created via ApplicationListener.forPayload(…) - is reported as a candidate for every event, so publishing a non-payload event like ContextRefreshedEvent made the cast to PayloadApplicationEvent fail and the application context refresh blow up as soon as event externalization was enabled. The multicaster now extends SimpleApplicationEventMulticaster and delegates the actual invocation to invokeListener(…), which restores the behavior of the multicaster it replaces and additionally honors a configured ErrorHandler. Signed-off-by: seonwoo_jung <79202163+seonwooj0810@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1783
Root cause
PersistentApplicationEventMulticasterinvokesApplicationListener.onApplicationEvent(…)directly, which bypasses theClassCastExceptiontoleranceSimpleApplicationEventMulticaster.doInvokeListener(…)applies for lambda-defined listeners whose generic event type cannot be resolved by reflection. A listener created viaApplicationListener.forPayload(…)has no resolvable declared event type, sogetApplicationListeners(…)reports it as a candidate for every event; publishing a non-payload event such asContextRefreshedEventthen fails the lambda's implicit cast toPayloadApplicationEvent. Because the multicaster registers itself as theapplicationEventMulticasterbean, merely adding event externalization to an application turns a listener that worked before into a context refresh failure.Change
PersistentApplicationEventMulticasternow extendsSimpleApplicationEventMulticasterand delegates the listener invocation toinvokeListener(…). BothmulticastEvent(…)overloads stay overridden, so the publication-registry logic is unchanged; the fix restores the invocation semantics of the multicaster that is being replaced and, as a side effect, honors a configuredErrorHandler.Tests
Added
PersistentApplicationEventMulticasterUnitTests.doesNotPropagateClassCastExceptionOfNonMatchingLambdaListener(), which registers anApplicationListener.forPayload(…)listener and multicasts a plainApplicationEvent. It fails onmainwith exactly the reported exception and passes with the fix.Verification done: reproduced the reported
ClassCastExceptionwith a failing test onupstream/main(same stack frame as the issue,PersistentApplicationEventMulticaster.multicastEvent);./mvnw test -pl :spring-modulith-events-core— 49 tests, all green;./mvnw test -pl :spring-modulith-events-tests -am— events-core, events-jpa (59) and the integration example all green. Checked thatPersistentApplicationEventMulticasteris not referenced as anAbstractApplicationEventMulticasteranywhere else in the codebase.