Skip to content

Commit

Permalink
Fix ApplicationListenerMethodAdapter#supportsEventType check
Browse files Browse the repository at this point in the history
This commit fixes the check by avoiding a fallback to eventType's
hasUnresolvableGenerics(). This could previously lead to checking a
generic event type `A<T>` against a listener which accepts unrelated
`B` and return `true` despite the inconsistency.

Note that this wouldn't necessarily surface to the user because there is
a `catch (ClassCastException e)` down the line, which was primarily put
in place to deal with lambda-based listeners but happens to catch an
exception thrown due to the bad result of `supportsEventType`.

The `supportsEventType` now matches generic `PayloadApplicationEvent`
types with a raw counterpart, using the above fallback only in that case
and otherwise ultimately returning `false`.

Closes gh-30399
  • Loading branch information
simonbasle committed May 10, 2023
1 parent aefcb9d commit c733ae0
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -173,12 +173,12 @@ public boolean supportsEventType(ResolvableType eventType) {
}
if (PayloadApplicationEvent.class.isAssignableFrom(eventType.toClass())) {
ResolvableType payloadType = eventType.as(PayloadApplicationEvent.class).getGeneric();
if (declaredEventType.isAssignableFrom(payloadType)) {
if (declaredEventType.isAssignableFrom(payloadType) || eventType.hasUnresolvableGenerics()) {
return true;
}
}
}
return eventType.hasUnresolvableGenerics();
return false;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.io.IOException;
import java.lang.reflect.Method;
import java.lang.reflect.UndeclaredThrowableException;
import java.util.List;

import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -325,6 +326,77 @@ public void beanInstanceRetrievedAtEveryInvocation() {
verify(this.context, times(2)).getBean("testBean");
}

// see https://github.com/spring-projects/spring-framework/issues/30399
@Test
void simplePayloadDoesNotSupportArbitraryGenericEventType() throws Exception {
var method = SampleEvents.class.getDeclaredMethod("handleString", String.class);
var adapter = new ApplicationListenerMethodAdapter(null, ApplicationListenerMethodAdapterTests.class, method);

assertThat(adapter.supportsEventType(ResolvableType.forClassWithGenerics(EntityWrapper.class, Integer.class)))
.as("handleString(String) with EntityWrapper<Integer>").isFalse();
assertThat(adapter.supportsEventType(ResolvableType.forClass(EntityWrapper.class)))
.as("handleString(String) with EntityWrapper<?>").isFalse();
assertThat(adapter.supportsEventType(ResolvableType.forClass(String.class)))
.as("handleString(String) with String").isTrue();
}

// see https://github.com/spring-projects/spring-framework/issues/30399
@Test
void genericPayloadDoesNotSupportArbitraryGenericEventType() throws Exception {
var method = SampleEvents.class.getDeclaredMethod("handleGenericStringPayload", EntityWrapper.class);
var adapter = new ApplicationListenerMethodAdapter(null, ApplicationListenerMethodAdapterTests.class, method);

assertThat(adapter.supportsEventType(ResolvableType.forClass(EntityWrapper.class)))
.as("handleGenericStringPayload(EntityWrapper<String>) with EntityWrapper<?>").isFalse();
assertThat(adapter.supportsEventType(ResolvableType.forClassWithGenerics(EntityWrapper.class, Integer.class)))
.as("handleGenericStringPayload(EntityWrapper<String>) with EntityWrapper<Integer>").isFalse();
assertThat(adapter.supportsEventType(ResolvableType.forClassWithGenerics(EntityWrapper.class, String.class)))
.as("handleGenericStringPayload(EntityWrapper<String>) with EntityWrapper<String>").isTrue();
}

// see https://github.com/spring-projects/spring-framework/issues/30399
@Test
void rawGenericPayloadDoesNotSupportArbitraryGenericEventType() throws Exception {
var method = SampleEvents.class.getDeclaredMethod("handleGenericAnyPayload", EntityWrapper.class);
var adapter = new ApplicationListenerMethodAdapter(null, ApplicationListenerMethodAdapterTests.class, method);

assertThat(adapter.supportsEventType(ResolvableType.forClass(EntityWrapper.class)))
.as("handleGenericAnyPayload(EntityWrapper<?>) with EntityWrapper<?>").isTrue();
assertThat(adapter.supportsEventType(ResolvableType.forClassWithGenerics(EntityWrapper.class, Integer.class)))
.as("handleGenericAnyPayload(EntityWrapper<?>) with EntityWrapper<Integer>").isTrue();
assertThat(adapter.supportsEventType(ResolvableType.forClassWithGenerics(EntityWrapper.class, String.class)))
.as("handleGenericAnyPayload(EntityWrapper<?>) with EntityWrapper<String>").isTrue();
assertThat(adapter.supportsEventType(ResolvableType.forClass(List.class)))
.as("handleGenericAnyPayload(EntityWrapper<?>) with List<?>").isFalse();
assertThat(adapter.supportsEventType(ResolvableType.forClassWithGenerics(List.class, String.class)))
.as("handleGenericAnyPayload(EntityWrapper<?>) with List<String>").isFalse();
}

@Test
void genericApplicationEventSupportsSpecificType() throws Exception {
var method = SampleEvents.class.getDeclaredMethod("handleGenericString", GenericTestEvent.class);
var adapter = new ApplicationListenerMethodAdapter(null, ApplicationListenerMethodAdapterTests.class, method);

assertThat(adapter.supportsEventType(ResolvableType.forClass(GenericTestEvent.class)))
.as("handleGenericString(GenericTestEvent<String>) with GenericTestEvent<?>").isFalse();
assertThat(adapter.supportsEventType(ResolvableType.forClassWithGenerics(GenericTestEvent.class, Integer.class)))
.as("handleGenericString(GenericTestEvent<String>) with GenericTestEvent<Integer>").isFalse();
assertThat(adapter.supportsEventType(ResolvableType.forClassWithGenerics(GenericTestEvent.class, String.class)))
.as("handleGenericString(GenericTestEvent<String>) with GenericTestEvent<String>").isTrue();
}

@Test
void genericRawApplicationEventSupportsRawTypeAndAnySpecificType() throws Exception {
var method = SampleEvents.class.getDeclaredMethod("handleGenericRaw", GenericTestEvent.class);
var adapter = new ApplicationListenerMethodAdapter(null, ApplicationListenerMethodAdapterTests.class, method);

assertThat(adapter.supportsEventType(ResolvableType.forClass(GenericTestEvent.class)))
.as("handleGenericRaw(GenericTestEvent<?>) with GenericTestEvent<?>").isTrue();
assertThat(adapter.supportsEventType(ResolvableType.forClassWithGenerics(GenericTestEvent.class, String.class)))
.as("handleGenericRaw(GenericTestEvent<?>) with GenericTestEvent<String>").isTrue();
assertThat(adapter.supportsEventType(ResolvableType.forClassWithGenerics(GenericTestEvent.class, Integer.class)))
.as("handleGenericRaw(GenericTestEvent<?>) with GenericTestEvent<Integer>").isTrue();
}

private void supportsEventType(boolean match, Method method, ResolvableType eventType) {
ApplicationListenerMethodAdapter adapter = createTestInstance(method);
Expand Down Expand Up @@ -373,6 +445,10 @@ public void handleRaw(ApplicationEvent event) {
public void handleGenericString(GenericTestEvent<String> event) {
}

@EventListener
public void handleGenericRaw(GenericTestEvent<?> event) {
}

@EventListener
public void handleString(String payload) {
}
Expand Down

0 comments on commit c733ae0

Please sign in to comment.