ClientRSocketFactoryConfigurer refactoring

Author: rstoyanchev

spring-messaging/src/main/java/org/springframework/messaging/rsocket/ClientRSocketFactoryConfigurer.java

@@ -18,10 +18,9 @@
 import io.rsocket.RSocketFactory;
 
 /**
- * Strategy to apply some configuration to a
- * {@link io.rsocket.RSocketFactory.ClientRSocketFactory ClientRSocketFactory}.
- * It is given to {@link RSocketRequester.Builder} to initialize the
- * {@code RSocketFactory} that's used to connect.
+ * Strategy to apply configuration to a client side {@code RSocketFactory}.
+ * that's being prepared by {@link RSocketRequester.Builder} to connect
+ * to a server.
  *
  * @author Rossen Stoyanchev
  * @since 5.2
@@ -30,17 +29,7 @@
 public interface ClientRSocketFactoryConfigurer {
 
 	/**
-	 * This method is invoked by {@link RSocketRequester.Builder} immediately
-	 * before the call to {@link #configure}, and can be used by implementations
-	 * of this interface that need access to the configured
-	 * {@code RSocketStrategies}.
-	 */
-	default void configureWithStrategies(RSocketStrategies strategies) {
-	}
-
-	/**
-	 * Configure the given {@code ClientRSocketFactory}.
-	 * @param rsocketFactory the factory to configure
+	 * Apply configuration to the given {@code ClientRSocketFactory}.
 	 */
 	void configure(RSocketFactory.ClientRSocketFactory rsocketFactory);
 

spring-messaging/src/main/java/org/springframework/messaging/rsocket/DefaultRSocketRequesterBuilder.java

@@ -55,7 +55,7 @@ final class DefaultRSocketRequesterBuilder implements RSocketRequester.Builder {
 
 	private List<Consumer<RSocketStrategies.Builder>> strategiesConfigurers = new ArrayList<>();
 
-	private List<ClientRSocketFactoryConfigurer> rsocketFactoryConfigurers = new ArrayList<>();
+	private List<ClientRSocketFactoryConfigurer> rsocketConfigurers = new ArrayList<>();
 
 
 	@Override
@@ -85,7 +85,7 @@ public RSocketRequester.Builder rsocketStrategies(Consumer<RSocketStrategies.Bui
 
 	@Override
 	public RSocketRequester.Builder rsocketFactory(ClientRSocketFactoryConfigurer configurer) {
-		this.rsocketFactoryConfigurers.add(configurer);
+		this.rsocketConfigurers.add(configurer);
 		return this;
 	}
 
@@ -118,10 +118,7 @@ private Mono<RSocketRequester> doConnect(ClientTransport transport) {
 			rsocketFactory.frameDecoder(PayloadDecoder.ZERO_COPY);
 		}
 
-		this.rsocketFactoryConfigurers.forEach(configurer -> {
-			configurer.configureWithStrategies(rsocketStrategies);
-			configurer.configure(rsocketFactory);
-		});
+		this.rsocketConfigurers.forEach(configurer -> configurer.configure(rsocketFactory));
 
 		return rsocketFactory.transport(transport)
 				.start()

spring-messaging/src/main/java/org/springframework/messaging/rsocket/RSocketRequester.java

@@ -164,15 +164,12 @@ interface Builder {
 
 		/**
 		 * Callback to configure the {@code ClientRSocketFactory} directly.
-		 * <p>See static factory method
-		 * {@link RSocketMessageHandler#clientResponder(Object...)} for
-		 * configuring a client side responder with annotated methods.
-		 * <p><strong>Note:</strong> Do not set {@link #dataMimeType(MimeType)}
-		 * and {@link #metadataMimeType(MimeType)} directly on the
-		 * {@code ClientRSocketFactory}. Use the shortcuts on this builder
-		 * instead since the created {@code RSocketRequester} needs to be aware
-		 * of those settings.
-		 * @see RSocketMessageHandler#clientResponder(Object...)
+		 * <p>Do not set {@link #dataMimeType(MimeType)} and
+		 * {@link #metadataMimeType(MimeType)} directly on the
+		 * {@code ClientRSocketFactory}. Use methods on this builder instead
+		 * so the {@code RSocketRequester} will have access to them.
+		 * <p>For configuring client side responding, see
+		 * {@link RSocketMessageHandler#clientResponder(RSocketStrategies, Object...)}.
 		 */
 		RSocketRequester.Builder rsocketFactory(ClientRSocketFactoryConfigurer configurer);
 

spring-messaging/src/main/java/org/springframework/messaging/rsocket/annotation/support/RSocketMessageHandler.java

@@ -24,7 +24,6 @@
 
 import io.rsocket.ConnectionSetupPayload;
 import io.rsocket.RSocket;
-import io.rsocket.RSocketFactory;
 import io.rsocket.SocketAcceptor;
 import io.rsocket.frame.FrameType;
 import reactor.core.publisher.Mono;
@@ -344,18 +343,15 @@ public SocketAcceptor serverResponder() {
 	}
 
 	/**
-	 * Return an adapter for a client side
-	 * {@link io.rsocket.RSocketFactory.ClientRSocketFactory#acceptor(BiFunction)
-	 * acceptor} that delegate to this {@link RSocketMessageHandler} for
-	 * handling.
-	 * <p>The initial {@link ConnectionSetupPayload} can be processed with a
-	 * {@link ConnectMapping @ConnectionMapping} method but, unlike the
-	 * server side, such a method is merely a callback and cannot prevent the
-	 * connection unless the method throws an error immediately. Such a method
-	 * can also start requests to the server but must do so decoupled from
-	 * handling and from the current thread.
-	 * <p>Subsequent stream requests can be handled with
-	 * {@link MessageMapping MessageMapping} methods.
+	 * Return an adapter for a client side responder that can be used to set
+	 * {@link io.rsocket.RSocketFactory.ClientRSocketFactory#acceptor(Function)}.
+	 * The responder delegates requests to this {@code RSocketMessageHandler}
+	 * for handling via {@code @MessageMapping} methods.
+	 * <p>The initial {@link ConnectionSetupPayload} can be accessed through a
+	 * {@link ConnectMapping @ConnectionMapping} method, but such a method is
+	 * only a callback just before the connection is made and cannot "accept"
+	 * or prevent the connection. Such a method can also start requests to the
+	 * server but must do so decoupled from handling and the current thread.
 	 */
 	public BiFunction<ConnectionSetupPayload, RSocket, RSocket> clientResponder() {
 		return (setupPayload, sendingRSocket) -> {
@@ -375,17 +371,13 @@ private MessagingRSocket createResponder(ConnectionSetupPayload setupPayload, RS
 		MimeType metaMimeType = StringUtils.hasText(s) ? MimeTypeUtils.parseMimeType(s) : this.defaultMetadataMimeType;
 		Assert.notNull(metaMimeType, "No `metadataMimeType` in ConnectionSetupPayload and no default value");
 
-		RSocketStrategies strategies = getRSocketStrategies();
-		RSocketRequester requester = RSocketRequester.wrap(rsocket, dataMimeType, metaMimeType, strategies);
-
-		Assert.state(this.metadataExtractor != null,
-				() -> "No MetadataExtractor. Was afterPropertiesSet not called?");
+		RSocketRequester requester = RSocketRequester.wrap(
+				rsocket, dataMimeType, metaMimeType, this.strategies);
 
-		Assert.state(getRouteMatcher() != null,
-				() -> "No RouteMatcher. Was afterPropertiesSet not called?");
+		Assert.state(getRouteMatcher() != null, () -> "No RouteMatcher. Was afterPropertiesSet not called?");
 
-		return new MessagingRSocket(dataMimeType, metaMimeType, this.metadataExtractor, requester,
-				this, getRouteMatcher(), strategies);
+		return new MessagingRSocket(dataMimeType, metaMimeType, this.metadataExtractor,
+				requester, this, getRouteMatcher(), this.strategies);
 	}
 
 	private boolean isDataMimeTypeSupported(MimeType dataMimeType) {
@@ -399,39 +391,39 @@ private boolean isDataMimeTypeSupported(MimeType dataMimeType) {
 		return false;
 	}
 
-	public static ClientRSocketFactoryConfigurer clientResponder(Object... handlers) {
-		return new ResponderConfigurer(handlers);
-	}
-
-
-	private static final class ResponderConfigurer implements ClientRSocketFactoryConfigurer {
-
-		private final List<Object> handlers = new ArrayList<>();
-
-		@Nullable
-		private RSocketStrategies strategies;
-
-
-		private ResponderConfigurer(Object... handlers) {
-			Assert.notEmpty(handlers, "No handlers");
-			for (Object obj : handlers) {
-				this.handlers.add(obj instanceof Class ? BeanUtils.instantiateClass((Class<?>) obj) : obj);
-			}
-		}
+	/**
+	 * Static factory method for a configurer of a client side responder with
+	 * annotated handler methods. This is intended to be passed into
+	 * {@link RSocketRequester.Builder#rsocketFactory(ClientRSocketFactoryConfigurer)}.
+	 * <p>In effect a shortcut to create and initialize
+	 * {@code RSocketMessageHandler} with the given strategies and handlers,
+	 * and use {@link #clientResponder()} to obtain the responder.
+	 * For more advanced scenarios, e.g. discovering handlers through a custom
+	 * stereotype annotation, consider declaring {@code RSocketMessageHandler}
+	 * as a bean, and then obtain the responder from it.
+	 * @param strategies the strategies to set on the created
+	 * {@code RSocketMessageHandler}
+	 * @param candidateHandlers a list of Objects and/or Classes with annotated
+	 * handler methods; used to call {@link #setHandlers(List)} with
+	 * on the created {@code RSocketMessageHandler}
+	 * @return a configurer that may be passed into
+	 * {@link RSocketRequester.Builder#rsocketFactory(ClientRSocketFactoryConfigurer)}
+	 */
+	public static ClientRSocketFactoryConfigurer clientResponder(
+			RSocketStrategies strategies, Object... candidateHandlers) {
 
-		@Override
-		public void configureWithStrategies(RSocketStrategies strategies) {
-			this.strategies = strategies;
+		Assert.notEmpty(candidateHandlers, "No handlers");
+		List<Object> handlers = new ArrayList<>(candidateHandlers.length);
+		for (Object obj : candidateHandlers) {
+			handlers.add(obj instanceof Class ? BeanUtils.instantiateClass((Class<?>) obj) : obj);
 		}
 
-		@Override
-		public void configure(RSocketFactory.ClientRSocketFactory factory) {
+		return rsocketFactory -> {
 			RSocketMessageHandler handler = new RSocketMessageHandler();
-			handler.setHandlers(this.handlers);
-			handler.setRSocketStrategies(this.strategies);
+			handler.setHandlers(handlers);
+			handler.setRSocketStrategies(strategies);
 			handler.afterPropertiesSet();
-			factory.acceptor(handler.clientResponder());
-		}
+			rsocketFactory.acceptor(handler.clientResponder());
+		};
 	}
-
 }

spring-messaging/src/test/java/org/springframework/messaging/rsocket/DefaultRSocketRequesterBuilderTests.java

@@ -99,7 +99,6 @@ public void shouldApplyCustomizations() {
 
 		verify(this.transport).connect(anyInt());
 		verify(rsocketStrategiesConfigurer).accept(any(RSocketStrategies.Builder.class));
-		assertThat(this.rsocketFactoryConfigurer.rsocketStrategies()).isNotNull();
 		assertThat(this.rsocketFactoryConfigurer.rsocketFactory()).isNotNull();
 	}
 
@@ -194,25 +193,14 @@ public void dispose() {
 
 	static class TestRSocketFactoryConfigurer implements ClientRSocketFactoryConfigurer {
 
-		private RSocketStrategies strategies;
-
 		private RSocketFactory.ClientRSocketFactory rsocketFactory;
 
 
-		public RSocketStrategies rsocketStrategies() {
-			return this.strategies;
-		}
-
-		public RSocketFactory.ClientRSocketFactory rsocketFactory() {
+		RSocketFactory.ClientRSocketFactory rsocketFactory() {
 			return this.rsocketFactory;
 		}
 
 
-		@Override
-		public void configureWithStrategies(RSocketStrategies strategies) {
-			this.strategies = strategies;
-		}
-
 		@Override
 		public void configure(RSocketFactory.ClientRSocketFactory rsocketFactory) {
 			this.rsocketFactory = rsocketFactory;

spring-messaging/src/test/java/org/springframework/messaging/rsocket/RSocketServerToClientIntegrationTests.java

@@ -106,15 +106,17 @@ private static void connectAndRunTest(String connectionRoute) {
 		ServerController serverController = context.getBean(ServerController.class);
 		serverController.reset();
 
+		RSocketStrategies strategies = context.getBean(RSocketStrategies.class);
+		ClientRSocketFactoryConfigurer clientResponderConfigurer =
+				RSocketMessageHandler.clientResponder(strategies, new ClientHandler());
+
 		RSocketRequester requester = null;
 		try {
 			requester = RSocketRequester.builder()
-					.rsocketFactory(factory -> {
-						factory.metadataMimeType("text/plain");
-						factory.setupPayload(ByteBufPayload.create("", connectionRoute));
-					})
-					.rsocketFactory(RSocketMessageHandler.clientResponder(new ClientHandler()))
-					.rsocketStrategies(context.getBean(RSocketStrategies.class))
+					.metadataMimeType(MimeTypeUtils.TEXT_PLAIN)
+					.rsocketStrategies(strategies)
+					.rsocketFactory(clientResponderConfigurer)
+					.rsocketFactory(factory -> factory.setupPayload(ByteBufPayload.create("", connectionRoute)))
 					.connectTcp("localhost", server.address().getPort())
 					.block();