Polish

spring-messaging/src/test/java/org/springframework/messaging/handler/invocation/InvocableHandlerMethodTests.java

@@ -18,7 +18,6 @@ package org.springframework.messaging.handler.invocation;
 
 import java.lang.reflect.Method;
 
-import org.junit.Before;
 import org.junit.Test;
 
 import org.springframework.core.MethodParameter;
@@ -27,6 +26,7 @@ import org.springframework.util.ClassUtils;
 
 import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
 
 /**
  * Unit tests for {@link InvocableHandlerMethod}.
@@ -35,17 +35,11 @@ import static org.junit.Assert.*;
  */
 public class InvocableHandlerMethodTests {
 
-	private Message<?> message;
+	private final Message<?> message = mock(Message.class);
 
 	private final HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite();
 
 
-	@Before
-	public void setUp() {
-		this.message = null;
-	}
-
-
 	@Test
 	public void resolveArg() throws Exception {
 		this.composite.addResolver(new StubArgumentResolver(99));
@@ -61,7 +55,7 @@ public class InvocableHandlerMethodTests {
 	}
 
 	@Test
-	public void resolveNullArg() throws Exception {
+	public void resolveNoArgValue() throws Exception {
 		this.composite.addResolver(new StubArgumentResolver(Integer.class));
 		this.composite.addResolver(new StubArgumentResolver(String.class));
 
@@ -135,35 +129,33 @@ public class InvocableHandlerMethodTests {
 
 	@Test
 	public void invocationTargetException() throws Exception {
-		Throwable expected = new RuntimeException("error");
+		Throwable expected = null;
 		try {
+			expected = new RuntimeException("error");
 			getInvocable("handleWithException", Throwable.class).invoke(this.message, expected);
 			fail("Expected exception");
 		}
 		catch (RuntimeException actual) {
 			assertSame(expected, actual);
 		}
-
-		expected = new Error("error");
 		try {
+			expected = new Error("error");
 			getInvocable("handleWithException", Throwable.class).invoke(this.message, expected);
 			fail("Expected exception");
 		}
 		catch (Error actual) {
 			assertSame(expected, actual);
 		}
-
-		expected = new Exception("error");
 		try {
+			expected = new Exception("error");
 			getInvocable("handleWithException", Throwable.class).invoke(this.message, expected);
 			fail("Expected exception");
 		}
 		catch (Exception actual) {
 			assertSame(expected, actual);
 		}
-
-		expected = new Throwable("error");
 		try {
+			expected = new Throwable("error");
 			getInvocable("handleWithException", Throwable.class).invoke(this.message, expected);
 			fail("Expected exception");
 		}

spring-web/src/test/java/org/springframework/web/method/support/InvocableHandlerMethodTests.java

@@ -65,7 +65,7 @@ public class InvocableHandlerMethodTests {
 	}
 
 	@Test
-	public void resolveNullArg() throws Exception {
+	public void resolveNoArgValue() throws Exception {
 		this.composite.addResolver(new StubArgumentResolver(Integer.class));
 		this.composite.addResolver(new StubArgumentResolver(String.class));
 

spring-webflux/src/main/java/org/springframework/web/reactive/result/method/HandlerMethodArgumentResolverComposite.java

@@ -59,7 +59,6 @@ class HandlerMethodArgumentResolverComposite implements HandlerMethodArgumentRes
 
 	/**
 	 * Add the given {@link HandlerMethodArgumentResolver HandlerMethodArgumentResolvers}.
-	 * @since 4.3
 	 */
 	public HandlerMethodArgumentResolverComposite addResolvers(@Nullable HandlerMethodArgumentResolver... resolvers) {
 		if (resolvers != null) {
@@ -89,7 +88,6 @@ class HandlerMethodArgumentResolverComposite implements HandlerMethodArgumentRes
 
 	/**
 	 * Clear the list of configured resolvers.
-	 * @since 4.3
 	 */
 	public void clear() {
 		this.argumentResolvers.clear();

spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java

@@ -141,7 +141,7 @@ public class InvocableHandlerMethod extends HandlerMethod {
 			catch (IllegalArgumentException ex) {
 				assertTargetBean(getBridgedMethod(), getBean(), args);
 				String text = (ex.getMessage() != null ? ex.getMessage() : "Illegal argument");
-				throw new IllegalStateException(formatInvokeError(text, args), ex);
+				return Mono.error(new IllegalStateException(formatInvokeError(text, args), ex));
 			}
 			catch (InvocationTargetException ex) {
 				return Mono.error(ex.getTargetException());
@@ -213,11 +213,9 @@ public class InvocableHandlerMethod extends HandlerMethod {
 		}
 	}
 
-	private boolean isAsyncVoidReturnType(MethodParameter returnType,
-			@Nullable ReactiveAdapter reactiveAdapter) {
-
-		if (reactiveAdapter != null && reactiveAdapter.supportsEmpty()) {
-			if (reactiveAdapter.isNoValue()) {
+	private static boolean isAsyncVoidReturnType(MethodParameter returnType, @Nullable ReactiveAdapter adapter) {
+		if (adapter != null && adapter.supportsEmpty()) {
+			if (adapter.isNoValue()) {
 				return true;
 			}
 			Type parameterType = returnType.getGenericParameterType();

spring-webflux/src/test/java/org/springframework/web/reactive/result/method/InvocableHandlerMethodTests.java

@@ -16,11 +16,12 @@
 
 package org.springframework.web.reactive.result.method;
 
-import java.io.UnsupportedEncodingException;
 import java.lang.reflect.Method;
+import java.nio.charset.StandardCharsets;
 import java.time.Duration;
 import java.time.Instant;
-import java.util.Arrays;
+import java.util.ArrayList;
+import java.util.List;
 
 import org.junit.Test;
 import reactor.core.publisher.Flux;
@@ -57,112 +58,71 @@ public class InvocableHandlerMethodTests {
 
 	private final MockServerWebExchange exchange = MockServerWebExchange.from(get("http://localhost:8080/path"));
 
+	private final List<HandlerMethodArgumentResolver> resolvers = new ArrayList<>();
 
-	@Test
-	public void invokeAndHandle_VoidWithResponseStatus() {
-		Method method = ResolvableMethod.on(VoidController.class).mockCall(VoidController::responseStatus).method();
-		HandlerResult result = invokeForResult(new VoidController(), method);
-
-		assertNull("Expected no result (i.e. fully handled)", result);
-		assertEquals(HttpStatus.BAD_REQUEST, this.exchange.getResponse().getStatusCode());
-	}
 
 	@Test
-	public void invokeAndHandle_withResponse() {
-		ServerHttpResponse response = this.exchange.getResponse();
-		Method method = ResolvableMethod.on(VoidController.class).mockCall(c -> c.response(response)).method();
-		HandlerResult result = invokeForResult(new VoidController(), method, stubResolver(response));
-
-		assertNull("Expected no result (i.e. fully handled)", result);
-		assertEquals("bar", this.exchange.getResponse().getHeaders().getFirst("foo"));
-	}
-
-	@Test
-	public void invokeAndHandle_withResponseAndMonoVoid() {
-		ServerHttpResponse response = this.exchange.getResponse();
-		Method method = ResolvableMethod.on(VoidController.class).mockCall(c -> c.responseMonoVoid(response)).method();
-		HandlerResult result = invokeForResult(new VoidController(), method, stubResolver(response));
-
-		assertNull("Expected no result (i.e. fully handled)", result);
-		assertEquals("body", this.exchange.getResponse().getBodyAsString().block(Duration.ZERO));
-	}
-
-	@Test
-	public void invokeAndHandle_withExchange() {
-		Method method = ResolvableMethod.on(VoidController.class).mockCall(c -> c.exchange(exchange)).method();
-		HandlerResult result = invokeForResult(new VoidController(), method, stubResolver(this.exchange));
-
-		assertNull("Expected no result (i.e. fully handled)", result);
-		assertEquals("bar", this.exchange.getResponse().getHeaders().getFirst("foo"));
-	}
-
-	@Test
-	public void invokeAndHandle_withExchangeAndMonoVoid() {
-		Method method = ResolvableMethod.on(VoidController.class).mockCall(c -> c.exchangeMonoVoid(exchange)).method();
-		HandlerResult result = invokeForResult(new VoidController(), method, stubResolver(this.exchange));
+	public void resolveArg() {
+		this.resolvers.add(stubResolver("value1"));
+		Method method = ResolvableMethod.on(TestController.class).mockCall(o -> o.singleArg(null)).method();
+		Mono<HandlerResult> mono = invoke(new TestController(), method);
 
-		assertNull("Expected no result (i.e. fully handled)", result);
-		assertEquals("body", this.exchange.getResponse().getBodyAsString().block(Duration.ZERO));
+		assertHandlerResultValue(mono, "success:value1");
 	}
 
 	@Test
-	public void invokeAndHandle_withNotModified() {
-		ServerWebExchange exchange = MockServerWebExchange.from(
-				MockServerHttpRequest.get("/").ifModifiedSince(10 * 1000 * 1000));
-
-		Method method = ResolvableMethod.on(VoidController.class).mockCall(c -> c.notModified(exchange)).method();
-		HandlerResult result = invokeForResult(new VoidController(), method, stubResolver(exchange));
+	public void resolveNoArgValue() {
+		this.resolvers.add(stubResolver(Mono.empty()));
+		Method method = ResolvableMethod.on(TestController.class).mockCall(o -> o.singleArg(null)).method();
+		Mono<HandlerResult> mono = invoke(new TestController(), method);
 
-		assertNull("Expected no result (i.e. fully handled)", result);
+		assertHandlerResultValue(mono, "success:null");
 	}
 
 	@Test
-	public void invokeMethodWithNoArguments() {
+	public void resolveNoArgs() {
 		Method method = ResolvableMethod.on(TestController.class).mockCall(TestController::noArgs).method();
 		Mono<HandlerResult> mono = invoke(new TestController(), method);
 		assertHandlerResultValue(mono, "success");
 	}
 
 	@Test
-	public void invokeMethodWithNoValue() {
-		Method method = resolveOn().mockCall(o -> o.singleArg(null)).method();
-		Mono<HandlerResult> mono = invoke(new TestController(), method, stubResolver(Mono.empty()));
-
-		assertHandlerResultValue(mono, "success:null");
-	}
+	public void cannotResolveArg() {
+		Method method = ResolvableMethod.on(TestController.class).mockCall(o -> o.singleArg(null)).method();
+		Mono<HandlerResult> mono = invoke(new TestController(), method);
 
-	private ResolvableMethod.Builder<TestController> resolveOn() {
-		return ResolvableMethod.on(TestController.class);
+		try {
+			mono.block();
+			fail("Expected IllegalStateException");
+		}
+		catch (IllegalStateException ex) {
+			assertThat(ex.getMessage(), is("Could not resolve parameter [0] in " +
+					method.toGenericString() + ": No suitable resolver"));
+		}
 	}
 
 	@Test
-	public void invokeMethodWithValue() {
+	public void resolveProvidedArg() {
 		Method method = ResolvableMethod.on(TestController.class).mockCall(o -> o.singleArg(null)).method();
-		Mono<HandlerResult> mono = invoke(new TestController(), method, stubResolver("value1"));
+		Mono<HandlerResult> mono = invoke(new TestController(), method, "value1");
 
 		assertHandlerResultValue(mono, "success:value1");
 	}
 
 	@Test
-	public void noMatchingResolver() {
+	public void resolveProvidedArgFirst() {
+		this.resolvers.add(stubResolver("value1"));
 		Method method = ResolvableMethod.on(TestController.class).mockCall(o -> o.singleArg(null)).method();
-		Mono<HandlerResult> mono = invoke(new TestController(), method);
+		Mono<HandlerResult> mono = invoke(new TestController(), method, "value2");
 
-		try {
-			mono.block();
-			fail("Expected IllegalStateException");
-		}
-		catch (IllegalStateException ex) {
-			assertThat(ex.getMessage(), is("Could not resolve parameter [0] in " +
-					method.toGenericString() + ": No suitable resolver"));
-		}
+		assertHandlerResultValue(mono, "success:value2");
 	}
 
 	@Test
-	public void resolverThrowsException() {
+	public void exceptionInResolvingArg() {
+		this.resolvers.add(stubResolver(Mono.error(new UnsupportedMediaTypeStatusException("boo"))));
 		Method method = ResolvableMethod.on(TestController.class).mockCall(o -> o.singleArg(null)).method();
-		Mono<HandlerResult> mono = invoke(new TestController(), method,
-				stubResolver(Mono.error(new UnsupportedMediaTypeStatusException("boo"))));
+		Mono<HandlerResult> mono = invoke(new TestController(), method);
 
 		try {
 			mono.block();
@@ -175,8 +135,9 @@ public class InvocableHandlerMethodTests {
 
 	@Test
 	public void illegalArgumentException() {
+		this.resolvers.add(stubResolver(1));
 		Method method = ResolvableMethod.on(TestController.class).mockCall(o -> o.singleArg(null)).method();
-		Mono<HandlerResult> mono = invoke(new TestController(), method, stubResolver(1));
+		Mono<HandlerResult> mono = invoke(new TestController(), method);
 
 		try {
 			mono.block();
@@ -193,7 +154,7 @@ public class InvocableHandlerMethodTests {
 	}
 
 	@Test
-	public void invocationTargetExceptionIsUnwrapped() {
+	public void invocationTargetException() {
 		Method method = ResolvableMethod.on(TestController.class).mockCall(TestController::exceptionMethod).method();
 		Mono<HandlerResult> mono = invoke(new TestController(), method);
 
@@ -207,24 +168,77 @@ public class InvocableHandlerMethodTests {
 	}
 
 	@Test
-	public void invokeMethodWithResponseStatus() {
-		Method method = ResolvableMethod.on(TestController.class).annotPresent(ResponseStatus.class).resolveMethod();
+	public void responseStatusAnnotation() {
+		Method method = ResolvableMethod.on(TestController.class).mockCall(TestController::created).method();
 		Mono<HandlerResult> mono = invoke(new TestController(), method);
 
 		assertHandlerResultValue(mono, "created");
 		assertThat(this.exchange.getResponse().getStatusCode(), is(HttpStatus.CREATED));
 	}
 
+	@Test
+	public void voidMethodWithResponseArg() {
+		ServerHttpResponse response = this.exchange.getResponse();
+		this.resolvers.add(stubResolver(response));
+		Method method = ResolvableMethod.on(TestController.class).mockCall(c -> c.response(response)).method();
+		HandlerResult result = invokeForResult(new TestController(), method);
+
+		assertNull("Expected no result (i.e. fully handled)", result);
+		assertEquals("bar", this.exchange.getResponse().getHeaders().getFirst("foo"));
+	}
+
+	@Test
+	public void voidMonoMethodWithResponseArg() {
+		ServerHttpResponse response = this.exchange.getResponse();
+		this.resolvers.add(stubResolver(response));
+		Method method = ResolvableMethod.on(TestController.class).mockCall(c -> c.responseMonoVoid(response)).method();
+		HandlerResult result = invokeForResult(new TestController(), method);
+
+		assertNull("Expected no result (i.e. fully handled)", result);
+		assertEquals("body", this.exchange.getResponse().getBodyAsString().block(Duration.ZERO));
+	}
+
+	@Test
+	public void voidMethodWithExchangeArg() {
+		this.resolvers.add(stubResolver(this.exchange));
+		Method method = ResolvableMethod.on(TestController.class).mockCall(c -> c.exchange(exchange)).method();
+		HandlerResult result = invokeForResult(new TestController(), method);
+
+		assertNull("Expected no result (i.e. fully handled)", result);
+		assertEquals("bar", this.exchange.getResponse().getHeaders().getFirst("foo"));
+	}
+
+	@Test
+	public void voidMonoMethodWithExchangeArg() {
+		this.resolvers.add(stubResolver(this.exchange));
+		Method method = ResolvableMethod.on(TestController.class).mockCall(c -> c.exchangeMonoVoid(exchange)).method();
+		HandlerResult result = invokeForResult(new TestController(), method);
+
+		assertNull("Expected no result (i.e. fully handled)", result);
+		assertEquals("body", this.exchange.getResponse().getBodyAsString().block(Duration.ZERO));
+	}
+
+	@Test
+	public void checkNotModified() {
+		MockServerHttpRequest request = MockServerHttpRequest.get("/").ifModifiedSince(10 * 1000 * 1000).build();
+		ServerWebExchange exchange = MockServerWebExchange.from(request);
+		this.resolvers.add(stubResolver(exchange));
+		Method method = ResolvableMethod.on(TestController.class).mockCall(c -> c.notModified(exchange)).method();
+		HandlerResult result = invokeForResult(new TestController(), method);
+
+		assertNull("Expected no result (i.e. fully handled)", result);
+	}
+
 
 	@Nullable
-	private HandlerResult invokeForResult(Object handler, Method method, HandlerMethodArgumentResolver... resolvers) {
-		return invoke(handler, method, resolvers).block(Duration.ZERO);
+	private HandlerResult invokeForResult(Object handler, Method method, Object... providedArgs) {
+		return invoke(handler, method, providedArgs).block(Duration.ofSeconds(5));
 	}
 
-	private Mono<HandlerResult> invoke(Object handler, Method method, HandlerMethodArgumentResolver... resolvers) {
+	private Mono<HandlerResult> invoke(Object handler, Method method, Object... providedArgs) {
 		InvocableHandlerMethod invocable = new InvocableHandlerMethod(handler, method);
-		invocable.setArgumentResolvers(Arrays.asList(resolvers));
-		return invocable.invoke(this.exchange, new BindingContext());
+		invocable.setArgumentResolvers(this.resolvers);
+		return invocable.invoke(this.exchange, new BindingContext(), providedArgs);
 	}
 
 	private <T> HandlerMethodArgumentResolver stubResolver(Object stubValue) {
@@ -246,53 +260,46 @@ public class InvocableHandlerMethodTests {
 	}
 
 
-	@SuppressWarnings("unused")
+	@SuppressWarnings({"unused", "UnusedReturnValue", "SameParameterValue"})
 	static class TestController {
 
-		public String noArgs() {
-			return "success";
+		String singleArg(String q) {
+			return "success:" + q;
 		}
 
-		public String singleArg(String q) {
-			return "success:" + q;
+		String noArgs() {
+			return "success";
 		}
 
-		public void exceptionMethod() {
+		void exceptionMethod() {
 			throw new IllegalStateException("boo");
 		}
 
 		@ResponseStatus(HttpStatus.CREATED)
-		public String responseStatus() {
+		String created() {
 			return "created";
 		}
-	}
-
-
-	@SuppressWarnings("unused")
-	static class VoidController {
-
-		@ResponseStatus(HttpStatus.BAD_REQUEST)
-		public void responseStatus() {
-		}
 
-		public void response(ServerHttpResponse response) {
+		void response(ServerHttpResponse response) {
 			response.getHeaders().add("foo", "bar");
 		}
 
-		public Mono<Void> responseMonoVoid(ServerHttpResponse response) {
-			return response.writeWith(getBody("body"));
+		Mono<Void> responseMonoVoid(ServerHttpResponse response) {
+			return Mono.delay(Duration.ofMillis(100))
+					.thenEmpty(Mono.defer(() -> response.writeWith(getBody("body"))));
 		}
 
-		public void exchange(ServerWebExchange exchange) {
+		void exchange(ServerWebExchange exchange) {
 			exchange.getResponse().getHeaders().add("foo", "bar");
 		}
 
-		public Mono<Void> exchangeMonoVoid(ServerWebExchange exchange) {
-			return exchange.getResponse().writeWith(getBody("body"));
+		Mono<Void> exchangeMonoVoid(ServerWebExchange exchange) {
+			return Mono.delay(Duration.ofMillis(100))
+					.thenEmpty(Mono.defer(() -> exchange.getResponse().writeWith(getBody("body"))));
 		}
 
 		@Nullable
-		public String notModified(ServerWebExchange exchange) {
+		String notModified(ServerWebExchange exchange) {
 			if (exchange.checkNotModified(Instant.ofEpochMilli(1000 * 1000))) {
 				return null;
 			}
@@ -300,12 +307,7 @@ public class InvocableHandlerMethodTests {
 		}
 
 		private Flux<DataBuffer> getBody(String body) {
-			try {
-				return Flux.just(new DefaultDataBufferFactory().wrap(body.getBytes("UTF-8")));
-			}
-			catch (UnsupportedEncodingException ex) {
-				throw new IllegalStateException(ex);
-			}
+			return Flux.just(new DefaultDataBufferFactory().wrap(body.getBytes(StandardCharsets.UTF_8)));
 		}
 	}