Use method signature to refine RSocket @MessageMapping
Before this change an @MessageMapping could be matched to any RSocket interaction type, which is arguably too flexible, makes it difficult to reason what would happen in case of a significant mismatch of cardinality, e.g. request for Fire-And-Forget (1-to-0) mapped to a method that returns Flux, and could result in payloads being ignored, or not seen unintentionally. This commit checks @ConnectMapping method on startup and rejects them if they return any values (sync or async). It also refines each @MessageMapping to match only the RSocket interaction type it fits based on the input and output cardinality of the handler method. Subsequently if a request is not matched, we'll do a second search to identify partial matches (by route only) and raise a helpful error that explains which interaction type is actually supported. The reference docs has been updated to explain the options. Closes gh-23999
This commit is contained in:
@@ -137,22 +137,16 @@ public class RSocketBufferLeakTests {
|
||||
|
||||
@Test
|
||||
public void errorSignalWithExceptionHandler() {
|
||||
Mono<String> result = requester.route("error-signal").data("foo").retrieveMono(String.class);
|
||||
Flux<String> result = requester.route("error-signal").data("foo").retrieveFlux(String.class);
|
||||
StepVerifier.create(result).expectNext("Handled 'bad input'").expectComplete().verify(Duration.ofSeconds(5));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void ignoreInput() {
|
||||
Flux<String> result = requester.route("ignore-input").data("a").retrieveFlux(String.class);
|
||||
Mono<String> result = requester.route("ignore-input").data("a").retrieveMono(String.class);
|
||||
StepVerifier.create(result).expectNext("bar").thenCancel().verify(Duration.ofSeconds(5));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void retrieveMonoFromFluxResponderMethod() {
|
||||
Mono<String> result = requester.route("request-stream").data("foo").retrieveMono(String.class);
|
||||
StepVerifier.create(result).expectNext("foo-1").expectComplete().verify(Duration.ofSeconds(5));
|
||||
}
|
||||
|
||||
|
||||
@Controller
|
||||
static class ServerController {
|
||||
@@ -188,11 +182,6 @@ public class RSocketBufferLeakTests {
|
||||
Mono<String> ignoreInput() {
|
||||
return Mono.delay(Duration.ofMillis(10)).map(l -> "bar");
|
||||
}
|
||||
|
||||
@MessageMapping("request-stream")
|
||||
Flux<String> stream(String payload) {
|
||||
return Flux.range(1,100).delayElements(Duration.ofMillis(10)).map(idx -> payload + "-" + idx);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -159,13 +159,13 @@ public class RSocketClientToServerIntegrationTests {
|
||||
|
||||
@Test
|
||||
public void voidReturnValue() {
|
||||
Flux<String> result = requester.route("void-return-value").data("Hello").retrieveFlux(String.class);
|
||||
Mono<String> result = requester.route("void-return-value").data("Hello").retrieveMono(String.class);
|
||||
StepVerifier.create(result).expectComplete().verify(Duration.ofSeconds(5));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void voidReturnValueFromExceptionHandler() {
|
||||
Flux<String> result = requester.route("void-return-value").data("bad").retrieveFlux(String.class);
|
||||
Mono<String> result = requester.route("void-return-value").data("bad").retrieveMono(String.class);
|
||||
StepVerifier.create(result).expectComplete().verify(Duration.ofSeconds(5));
|
||||
}
|
||||
|
||||
|
||||
@@ -26,6 +26,8 @@ import org.springframework.messaging.Message;
|
||||
import org.springframework.messaging.support.MessageBuilder;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.springframework.messaging.rsocket.annotation.support.RSocketFrameTypeMessageCondition.CONNECT_CONDITION;
|
||||
import static org.springframework.messaging.rsocket.annotation.support.RSocketFrameTypeMessageCondition.EMPTY_CONDITION;
|
||||
|
||||
/**
|
||||
* Unit tests for {@link RSocketFrameTypeMessageCondition}.
|
||||
@@ -33,16 +35,37 @@ import static org.assertj.core.api.Assertions.assertThat;
|
||||
*/
|
||||
public class RSocketFrameTypeMessageConditionTests {
|
||||
|
||||
private static final RSocketFrameTypeMessageCondition FNF_RR_CONDITION =
|
||||
new RSocketFrameTypeMessageCondition(FrameType.REQUEST_FNF, FrameType.REQUEST_RESPONSE);
|
||||
|
||||
|
||||
@Test
|
||||
public void getMatchingCondition() {
|
||||
Message<?> message = message(FrameType.REQUEST_RESPONSE);
|
||||
RSocketFrameTypeMessageCondition condition = condition(FrameType.REQUEST_FNF, FrameType.REQUEST_RESPONSE);
|
||||
RSocketFrameTypeMessageCondition actual = condition.getMatchingCondition(message);
|
||||
RSocketFrameTypeMessageCondition actual = FNF_RR_CONDITION.getMatchingCondition(message);
|
||||
|
||||
assertThat(actual).isNotNull();
|
||||
assertThat(actual.getFrameTypes()).hasSize(1).containsOnly(FrameType.REQUEST_RESPONSE);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void getMatchingConditionEmpty() {
|
||||
Message<?> message = message(FrameType.REQUEST_RESPONSE);
|
||||
RSocketFrameTypeMessageCondition actual = EMPTY_CONDITION.getMatchingCondition(message);
|
||||
|
||||
assertThat(actual).isNull();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void combine() {
|
||||
|
||||
assertThat(EMPTY_CONDITION.combine(CONNECT_CONDITION).getFrameTypes())
|
||||
.containsExactly(FrameType.SETUP, FrameType.METADATA_PUSH);
|
||||
|
||||
assertThat(EMPTY_CONDITION.combine(new RSocketFrameTypeMessageCondition(FrameType.REQUEST_FNF)).getFrameTypes())
|
||||
.containsExactly(FrameType.REQUEST_FNF);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void compareTo() {
|
||||
Message<byte[]> message = message(null);
|
||||
|
||||
@@ -21,6 +21,9 @@ import java.util.Map;
|
||||
|
||||
import io.rsocket.frame.FrameType;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import reactor.core.publisher.Flux;
|
||||
import reactor.core.publisher.Mono;
|
||||
import reactor.test.StepVerifier;
|
||||
|
||||
import org.springframework.core.ReactiveAdapterRegistry;
|
||||
import org.springframework.core.codec.ByteArrayDecoder;
|
||||
@@ -170,6 +173,69 @@ public class RSocketMessageHandlerTests {
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void rejectConnectMappingMethodsThatCanReply() {
|
||||
|
||||
RSocketMessageHandler handler = new RSocketMessageHandler();
|
||||
handler.setHandlers(Collections.singletonList(new InvalidConnectMappingController()));
|
||||
assertThatThrownBy(handler::afterPropertiesSet)
|
||||
.hasMessage("Invalid @ConnectMapping method. " +
|
||||
"Return type must be void or a void async type: " +
|
||||
"public java.lang.String org.springframework.messaging.rsocket.annotation.support." +
|
||||
"RSocketMessageHandlerTests$InvalidConnectMappingController.connectString()");
|
||||
|
||||
handler = new RSocketMessageHandler();
|
||||
handler.setHandlers(Collections.singletonList(new AnotherInvalidConnectMappingController()));
|
||||
assertThatThrownBy(handler::afterPropertiesSet)
|
||||
.hasMessage("Invalid @ConnectMapping method. " +
|
||||
"Return type must be void or a void async type: " +
|
||||
"public reactor.core.publisher.Mono<java.lang.String> " +
|
||||
"org.springframework.messaging.rsocket.annotation.support." +
|
||||
"RSocketMessageHandlerTests$AnotherInvalidConnectMappingController.connectString()");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void ignoreFireAndForgetToHandlerThatCanReply() {
|
||||
|
||||
InteractionMismatchController controller = new InteractionMismatchController();
|
||||
|
||||
RSocketMessageHandler handler = new RSocketMessageHandler();
|
||||
handler.setHandlers(Collections.singletonList(controller));
|
||||
handler.afterPropertiesSet();
|
||||
|
||||
MessageHeaderAccessor headers = new MessageHeaderAccessor();
|
||||
headers.setLeaveMutable(true);
|
||||
RouteMatcher.Route route = handler.getRouteMatcher().parseRoute("mono-string");
|
||||
headers.setHeader(DestinationPatternsMessageCondition.LOOKUP_DESTINATION_HEADER, route);
|
||||
headers.setHeader(RSocketFrameTypeMessageCondition.FRAME_TYPE_HEADER, FrameType.REQUEST_FNF);
|
||||
Message<?> message = MessageBuilder.createMessage(Mono.empty(), headers.getMessageHeaders());
|
||||
|
||||
// Simply dropped and logged (error cannot propagate to client)
|
||||
StepVerifier.create(handler.handleMessage(message)).expectComplete().verify();
|
||||
assertThat(controller.invokeCount).isEqualTo(0);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void rejectRequestResponseToStreamingHandler() {
|
||||
|
||||
RSocketMessageHandler handler = new RSocketMessageHandler();
|
||||
handler.setHandlers(Collections.singletonList(new InteractionMismatchController()));
|
||||
handler.afterPropertiesSet();
|
||||
|
||||
MessageHeaderAccessor headers = new MessageHeaderAccessor();
|
||||
headers.setLeaveMutable(true);
|
||||
RouteMatcher.Route route = handler.getRouteMatcher().parseRoute("flux-string");
|
||||
headers.setHeader(DestinationPatternsMessageCondition.LOOKUP_DESTINATION_HEADER, route);
|
||||
headers.setHeader(RSocketFrameTypeMessageCondition.FRAME_TYPE_HEADER, FrameType.REQUEST_RESPONSE);
|
||||
Message<?> message = MessageBuilder.createMessage(Mono.empty(), headers.getMessageHeaders());
|
||||
|
||||
StepVerifier.create(handler.handleMessage(message))
|
||||
.expectErrorMessage(
|
||||
"Destination 'flux-string' does not support REQUEST_RESPONSE. " +
|
||||
"Supported interaction(s): [REQUEST_STREAM]")
|
||||
.verify();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void handleNoMatch() {
|
||||
|
||||
@@ -222,4 +288,38 @@ public class RSocketMessageHandlerTests {
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
private static class InvalidConnectMappingController {
|
||||
|
||||
@ConnectMapping
|
||||
public String connectString() {
|
||||
return "";
|
||||
}
|
||||
}
|
||||
|
||||
private static class AnotherInvalidConnectMappingController {
|
||||
|
||||
@ConnectMapping
|
||||
public Mono<String> connectString() {
|
||||
return Mono.empty();
|
||||
}
|
||||
}
|
||||
|
||||
private static class InteractionMismatchController {
|
||||
|
||||
private int invokeCount;
|
||||
|
||||
@MessageMapping("mono-string")
|
||||
public Mono<String> messageMonoString() {
|
||||
this.invokeCount++;
|
||||
return Mono.empty();
|
||||
}
|
||||
|
||||
@MessageMapping("flux-string")
|
||||
public Flux<String> messageFluxString() {
|
||||
this.invokeCount++;
|
||||
return Flux.empty();
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -84,13 +84,13 @@ class RSocketClientToServerCoroutinesIntegrationTests {
|
||||
|
||||
@Test
|
||||
fun unitReturnValue() {
|
||||
val result = requester.route("unit-return-value").data("Hello").retrieveFlux(String::class.java)
|
||||
val result = requester.route("unit-return-value").data("Hello").retrieveMono(String::class.java)
|
||||
StepVerifier.create(result).expectComplete().verify(Duration.ofSeconds(5))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun unitReturnValueFromExceptionHandler() {
|
||||
val result = requester.route("unit-return-value").data("bad").retrieveFlux(String::class.java)
|
||||
val result = requester.route("unit-return-value").data("bad").retrieveMono(String::class.java)
|
||||
StepVerifier.create(result).expectComplete().verify(Duration.ofSeconds(5))
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user