diff --git a/spring-web-reactive/src/main/java/org/springframework/core/codec/support/AbstractDecoder.java b/spring-web-reactive/src/main/java/org/springframework/core/codec/support/AbstractDecoder.java index 9f4b8b5705e..fb00faab0cb 100644 --- a/spring-web-reactive/src/main/java/org/springframework/core/codec/support/AbstractDecoder.java +++ b/spring-web-reactive/src/main/java/org/springframework/core/codec/support/AbstractDecoder.java @@ -44,6 +44,9 @@ public abstract class AbstractDecoder implements Decoder { @Override public boolean canDecode(ResolvableType type, MimeType mimeType, Object... hints) { + if (mimeType == null) { + return true; + } for (MimeType supportedMimeType : this.supportedMimeTypes) { if (supportedMimeType.isCompatibleWith(mimeType)) { return true; diff --git a/spring-web-reactive/src/main/java/org/springframework/core/codec/support/AbstractEncoder.java b/spring-web-reactive/src/main/java/org/springframework/core/codec/support/AbstractEncoder.java index 80100421581..d0d1920194b 100644 --- a/spring-web-reactive/src/main/java/org/springframework/core/codec/support/AbstractEncoder.java +++ b/spring-web-reactive/src/main/java/org/springframework/core/codec/support/AbstractEncoder.java @@ -44,6 +44,9 @@ public abstract class AbstractEncoder implements Encoder { @Override public boolean canEncode(ResolvableType type, MimeType mimeType, Object... hints) { + if (mimeType == null) { + return true; + } for (MimeType supportedMimeType : this.supportedMimeTypes) { if (supportedMimeType.isCompatibleWith(mimeType)) { return true; diff --git a/spring-web-reactive/src/main/java/org/springframework/web/reactive/DispatcherHandler.java b/spring-web-reactive/src/main/java/org/springframework/web/reactive/DispatcherHandler.java index 56c950a5e82..444f14a7c91 100644 --- a/spring-web-reactive/src/main/java/org/springframework/web/reactive/DispatcherHandler.java +++ b/spring-web-reactive/src/main/java/org/springframework/web/reactive/DispatcherHandler.java @@ -128,7 +128,7 @@ public class DispatcherHandler implements HttpHandler, ApplicationContextAware { return resultHandler; } } - throw new IllegalStateException("No HandlerResultHandler: " + handlerResult.getValue()); + throw new IllegalStateException("No HandlerResultHandler for " + handlerResult.getValue()); } diff --git a/spring-web-reactive/src/main/java/org/springframework/web/reactive/method/annotation/RequestMappingHandlerAdapter.java b/spring-web-reactive/src/main/java/org/springframework/web/reactive/method/annotation/RequestMappingHandlerAdapter.java index 215718afa80..d997913e7de 100644 --- a/spring-web-reactive/src/main/java/org/springframework/web/reactive/method/annotation/RequestMappingHandlerAdapter.java +++ b/spring-web-reactive/src/main/java/org/springframework/web/reactive/method/annotation/RequestMappingHandlerAdapter.java @@ -26,6 +26,7 @@ import reactor.Publishers; import org.springframework.beans.factory.InitializingBean; import org.springframework.core.ResolvableType; import org.springframework.core.convert.ConversionService; +import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.core.codec.support.ByteBufferDecoder; @@ -33,6 +34,7 @@ import org.springframework.core.codec.Decoder; import org.springframework.core.codec.support.JacksonJsonDecoder; import org.springframework.core.codec.support.JsonObjectDecoder; import org.springframework.core.codec.support.StringDecoder; +import org.springframework.util.ObjectUtils; import org.springframework.web.reactive.HandlerAdapter; import org.springframework.web.reactive.HandlerResult; import org.springframework.web.reactive.method.HandlerMethodArgumentResolver; @@ -45,9 +47,9 @@ import org.springframework.web.method.HandlerMethod; */ public class RequestMappingHandlerAdapter implements HandlerAdapter, InitializingBean { - private List argumentResolvers; + private final List argumentResolvers = new ArrayList<>(); - private ConversionService conversionService; + private ConversionService conversionService = new DefaultConversionService(); public void setArgumentResolvers(List resolvers) { @@ -70,12 +72,11 @@ public class RequestMappingHandlerAdapter implements HandlerAdapter, Initializin @Override public void afterPropertiesSet() throws Exception { - if (this.argumentResolvers == null) { + if (ObjectUtils.isEmpty(this.argumentResolvers)) { List> decoders = Arrays.asList(new ByteBufferDecoder(), new StringDecoder(), new JacksonJsonDecoder(new JsonObjectDecoder())); - this.argumentResolvers = new ArrayList<>(); this.argumentResolvers.add(new RequestParamArgumentResolver()); this.argumentResolvers.add(new RequestBodyArgumentResolver(decoders, this.conversionService)); } diff --git a/spring-web-reactive/src/main/java/org/springframework/web/reactive/method/annotation/ResponseBodyResultHandler.java b/spring-web-reactive/src/main/java/org/springframework/web/reactive/method/annotation/ResponseBodyResultHandler.java index d0061d35c81..d0d4ab63408 100644 --- a/spring-web-reactive/src/main/java/org/springframework/web/reactive/method/annotation/ResponseBodyResultHandler.java +++ b/spring-web-reactive/src/main/java/org/springframework/web/reactive/method/annotation/ResponseBodyResultHandler.java @@ -135,10 +135,20 @@ public class ResponseBodyResultHandler implements HandlerResultHandler, Ordered return Publishers.empty(); } + Publisher publisher; + ResolvableType elementType; ResolvableType returnType = result.getValueType(); + if (this.conversionService.canConvert(returnType.getRawClass(), Publisher.class)) { + publisher = this.conversionService.convert(value, Publisher.class); + elementType = returnType.getGeneric(0); + } + else { + publisher = Publishers.just(value); + elementType = returnType; + } List requestedMediaTypes = getAcceptableMediaTypes(request); - List producibleMediaTypes = getProducibleMediaTypes(returnType); + List producibleMediaTypes = getProducibleMediaTypes(elementType); if (producibleMediaTypes.isEmpty()) { producibleMediaTypes.add(MediaType.ALL); @@ -172,16 +182,6 @@ public class ResponseBodyResultHandler implements HandlerResultHandler, Ordered } if (selectedMediaType != null) { - Publisher publisher; - ResolvableType elementType; - if (this.conversionService.canConvert(returnType.getRawClass(), Publisher.class)) { - publisher = this.conversionService.convert(value, Publisher.class); - elementType = returnType.getGeneric(0); - } - else { - publisher = Publishers.just(value); - elementType = returnType; - } Encoder encoder = resolveEncoder(elementType, selectedMediaType); if (encoder != null) { response.getHeaders().setContentType(selectedMediaType); diff --git a/spring-web-reactive/src/main/resources/log4j.properties b/spring-web-reactive/src/main/resources/log4j.properties index b5bf0d4dfb1..34659ab78bf 100644 --- a/spring-web-reactive/src/main/resources/log4j.properties +++ b/spring-web-reactive/src/main/resources/log4j.properties @@ -1,6 +1,6 @@ log4j.rootCategory=WARN, stdout -log4j.logger.org.springframework.reactive=DEBUG +log4j.logger.org.springframework.http=DEBUG log4j.logger.org.springframework.web=DEBUG log4j.logger.reactor=INFO diff --git a/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/ErrorHandlingHttpHandlerTests.java b/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/ErrorHandlingHttpHandlerTests.java index f59220ac5ab..fe80a41bccb 100644 --- a/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/ErrorHandlingHttpHandlerTests.java +++ b/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/ErrorHandlingHttpHandlerTests.java @@ -115,25 +115,24 @@ public class ErrorHandlingHttpHandlerTests { private static class TestHttpHandler implements HttpHandler { - private final Throwable exception; + private final RuntimeException exception; private final boolean raise; - public TestHttpHandler(Throwable exception) { + public TestHttpHandler(RuntimeException exception) { this(exception, false); } - public TestHttpHandler(Throwable exception, boolean raise) { + public TestHttpHandler(RuntimeException exception, boolean raise) { this.exception = exception; this.raise = raise; - assertTrue(exception instanceof RuntimeException || !this.raise); } @Override public Publisher handle(ServerHttpRequest request, ServerHttpResponse response) { if (this.raise) { - throw (RuntimeException) exception; + throw this.exception; } return Publishers.error(this.exception); } diff --git a/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/FilterChainHttpHandlerTests.java b/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/FilterChainHttpHandlerTests.java index c53e01395c9..cd3bad68ab4 100644 --- a/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/FilterChainHttpHandlerTests.java +++ b/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/FilterChainHttpHandlerTests.java @@ -16,14 +16,26 @@ package org.springframework.http.server.reactive; +import java.util.Arrays; +import java.util.concurrent.Callable; +import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.log4j.spi.LoggerFactory; import org.junit.Before; import org.junit.Test; import org.reactivestreams.Publisher; import reactor.Publishers; +import reactor.core.publisher.PublisherFactory; +import reactor.fn.timer.Timer; import reactor.rx.Streams; +import org.springframework.scheduling.TaskScheduler; +import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler; +import org.springframework.util.ObjectUtils; + import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; @@ -33,6 +45,9 @@ import static org.mockito.Mockito.mock; */ public class FilterChainHttpHandlerTests { + private static Log logger = LogFactory.getLog(FilterChainHttpHandlerTests.class); + + private ServerHttpRequest request; private ServerHttpResponse response; @@ -89,10 +104,24 @@ public class FilterChainHttpHandlerTests { assertFalse(handler.invoked()); } + @Test + public void asyncFilter() throws Exception { + StubHandler handler = new StubHandler(); + AsyncFilter filter = new AsyncFilter(); + FilterChainHttpHandler filterHandler = new FilterChainHttpHandler(handler, filter); + + Publisher voidPublisher = filterHandler.handle(this.request, this.response); + Streams.wrap(voidPublisher).toList().await(10, TimeUnit.SECONDS); + + assertTrue(filter.invoked()); + assertTrue(handler.invoked()); + } + + private static class TestFilter implements HttpFilter { - private boolean invoked; + private volatile boolean invoked; public boolean invoked() { @@ -124,9 +153,25 @@ public class FilterChainHttpHandlerTests { } } + private static class AsyncFilter extends TestFilter { + + @Override + public Publisher doFilter(ServerHttpRequest req, ServerHttpResponse res, HttpFilterChain chain) { + return Publishers.concatMap(doAsyncWork(), asyncResult -> { + logger.debug("Async result: " + asyncResult); + return chain.filter(req, res); + }); + } + + private Publisher doAsyncWork() { + return Publishers.just("123"); + } + } + + private static class StubHandler implements HttpHandler { - private boolean invoked; + private volatile boolean invoked; public boolean invoked() { return this.invoked; @@ -134,6 +179,7 @@ public class FilterChainHttpHandlerTests { @Override public Publisher handle(ServerHttpRequest req, ServerHttpResponse res) { + logger.trace("StubHandler invoked."); this.invoked = true; return Publishers.empty(); } diff --git a/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/MockServerHttpResponse.java b/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/MockServerHttpResponse.java index 694951ff38d..ba12b8d76d3 100644 --- a/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/MockServerHttpResponse.java +++ b/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/MockServerHttpResponse.java @@ -57,7 +57,7 @@ public class MockServerHttpResponse implements ServerHttpResponse { @Override public Publisher setBody(Publisher body) { this.body = body; - return Publishers.empty(); + return Publishers.concatMap(body, b -> Publishers.empty()); } public Publisher getBody() { diff --git a/spring-web-reactive/src/test/java/org/springframework/web/reactive/DispatcherHandlerErrorTests.java b/spring-web-reactive/src/test/java/org/springframework/web/reactive/DispatcherHandlerErrorTests.java new file mode 100644 index 00000000000..0027fd31d62 --- /dev/null +++ b/spring-web-reactive/src/test/java/org/springframework/web/reactive/DispatcherHandlerErrorTests.java @@ -0,0 +1,282 @@ +/* + * Copyright 2002-2015 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.web.reactive; + +import java.net.URI; +import java.nio.ByteBuffer; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.TimeUnit; + +import org.junit.Before; +import org.junit.Test; +import org.reactivestreams.Publisher; +import reactor.Publishers; +import reactor.rx.Streams; +import reactor.rx.action.Signal; + +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.codec.Encoder; +import org.springframework.core.codec.support.StringEncoder; +import org.springframework.core.convert.support.DefaultConversionService; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; +import org.springframework.http.server.reactive.ErrorHandlingHttpHandler; +import org.springframework.http.server.reactive.FilterChainHttpHandler; +import org.springframework.http.server.reactive.HttpExceptionHandler; +import org.springframework.http.server.reactive.HttpFilter; +import org.springframework.http.server.reactive.HttpFilterChain; +import org.springframework.http.server.reactive.HttpHandler; +import org.springframework.http.server.reactive.MockServerHttpRequest; +import org.springframework.http.server.reactive.MockServerHttpResponse; +import org.springframework.http.server.reactive.ServerHttpRequest; +import org.springframework.http.server.reactive.ServerHttpResponse; +import org.springframework.stereotype.Controller; +import org.springframework.web.HttpMediaTypeNotAcceptableException; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.ResponseBody; +import org.springframework.web.reactive.method.annotation.RequestMappingHandlerAdapter; +import org.springframework.web.reactive.method.annotation.RequestMappingHandlerMapping; +import org.springframework.web.reactive.method.annotation.ResponseBodyResultHandler; + +import static org.hamcrest.CoreMatchers.startsWith; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThat; + +/** + * Test the effect of exceptions at different stages of request processing by + * checking the error signals on the completion publisher. + * + * @author Rossen Stoyanchev + */ +@SuppressWarnings({"ThrowableResultOfMethodCallIgnored", "ThrowableInstanceNeverThrown"}) +public class DispatcherHandlerErrorTests { + + public static final IllegalStateException EXCEPTION = new IllegalStateException("boo"); + + + private DispatcherHandler dispatcherHandler; + + private MockServerHttpRequest request; + + private MockServerHttpResponse response; + + + @Before + public void setUp() throws Exception { + AnnotationConfigApplicationContext appContext = new AnnotationConfigApplicationContext(); + appContext.register(TestConfig.class); + appContext.refresh(); + + this.dispatcherHandler = new DispatcherHandler(); + this.dispatcherHandler.setApplicationContext(appContext); + + this.request = new MockServerHttpRequest(HttpMethod.GET, new URI("/")); + this.response = new MockServerHttpResponse(); + } + + + @Test + public void noHandler() throws Exception { + this.request.setUri(new URI("/does-not-exist")); + + Publisher publisher = this.dispatcherHandler.handle(this.request, this.response); + Throwable ex = awaitErrorSignal(publisher); + + assertEquals(HandlerNotFoundException.class, ex.getClass()); + } + + @Test + public void noResolverForArgument() throws Exception { + this.request.setUri(new URI("/uknown-argument-type")); + + Publisher publisher = this.dispatcherHandler.handle(this.request, this.response); + Throwable ex = awaitErrorSignal(publisher); + + assertEquals(IllegalStateException.class, ex.getClass()); + assertThat(ex.getMessage(), startsWith("No resolver for argument [0]")); + } + + @Test + public void controllerMethodError() throws Exception { + this.request.setUri(new URI("/error-signal")); + + Publisher publisher = this.dispatcherHandler.handle(this.request, this.response); + Throwable ex = awaitErrorSignal(publisher); + + assertSame(EXCEPTION, ex); + } + + @Test + public void controllerMethodWithThrownException() throws Exception { + this.request.setUri(new URI("/raise-exception")); + + Publisher publisher = this.dispatcherHandler.handle(this.request, this.response); + Throwable ex = awaitErrorSignal(publisher); + + assertSame(EXCEPTION, ex); + } + + @Test + public void noHandlerResultHandler() throws Exception { + this.request.setUri(new URI("/unknown-return-type")); + + Publisher publisher = this.dispatcherHandler.handle(this.request, this.response); + Throwable ex = awaitErrorSignal(publisher); + + assertEquals(IllegalStateException.class, ex.getClass()); + assertThat(ex.getMessage(), startsWith("No HandlerResultHandler")); + } + + @Test + public void notAcceptable() throws Exception { + this.request.setUri(new URI("/request-body")); + this.request.getHeaders().setAccept(Collections.singletonList(MediaType.APPLICATION_JSON)); + this.request.setBody(Publishers.just(ByteBuffer.wrap("body".getBytes("UTF-8")))); + + Publisher publisher = this.dispatcherHandler.handle(this.request, this.response); + Throwable ex = awaitErrorSignal(publisher); + + assertEquals(HttpMediaTypeNotAcceptableException.class, ex.getClass()); + } + + @Test + public void requestBodyError() throws Exception { + this.request.setUri(new URI("/request-body")); + this.request.setBody(Publishers.error(EXCEPTION)); + + Publisher publisher = this.dispatcherHandler.handle(this.request, this.response); + Throwable ex = awaitErrorSignal(publisher); + + assertSame(EXCEPTION, ex); + } + + @Test + public void dispatcherHandlerWithHttpExceptionHandler() throws Exception { + this.request.setUri(new URI("/uknown-argument-type")); + + HttpExceptionHandler exceptionHandler = new ServerError500ExceptionHandler(); + HttpHandler httpHandler = new ErrorHandlingHttpHandler(this.dispatcherHandler, exceptionHandler); + Publisher publisher = httpHandler.handle(this.request, this.response); + + Streams.wrap(publisher).toList().await(5, TimeUnit.SECONDS); + assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, this.response.getStatus()); + } + + @Test + public void filterChainWithHttpExceptionHandler() throws Exception { + this.request.setUri(new URI("/uknown-argument-type")); + + HttpHandler httpHandler; + httpHandler = new FilterChainHttpHandler(this.dispatcherHandler, new TestHttpFilter()); + httpHandler = new ErrorHandlingHttpHandler(httpHandler, new ServerError500ExceptionHandler()); + Publisher publisher = httpHandler.handle(this.request, this.response); + + Streams.wrap(publisher).toList().await(5, TimeUnit.SECONDS); + assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, this.response.getStatus()); + } + + + private Throwable awaitErrorSignal(Publisher publisher) throws Exception { + Signal signal = Streams.wrap(publisher).materialize().toList().await(5, TimeUnit.SECONDS).get(0); + assertEquals("Unexpected signal: " + signal, Signal.Type.ERROR, signal.getType()); + return signal.getThrowable(); + } + + + @Configuration + @SuppressWarnings("unused") + static class TestConfig { + + @Bean + public RequestMappingHandlerMapping handlerMapping() { + return new RequestMappingHandlerMapping(); + } + + @Bean + public RequestMappingHandlerAdapter handlerAdapter() { + return new RequestMappingHandlerAdapter(); + } + + @Bean + public ResponseBodyResultHandler resultHandler() { + List> encoders = Collections.singletonList(new StringEncoder()); + return new ResponseBodyResultHandler(encoders, new DefaultConversionService()); + } + + @Bean + public TestController testController() { + return new TestController(); + } + } + + @Controller + @SuppressWarnings("unused") + private static class TestController { + + @RequestMapping("/uknown-argument-type") + public void uknownArgumentType(Foo arg) { + } + + @RequestMapping("/error-signal") + @ResponseBody + public Publisher errorSignal() { + return Publishers.error(EXCEPTION); + } + + @RequestMapping("/raise-exception") + public void raiseException() throws Exception { + throw EXCEPTION; + } + + @RequestMapping("/unknown-return-type") + public Foo unknownReturnType() throws Exception { + return new Foo(); + } + + @RequestMapping("/request-body") + @ResponseBody + public Publisher requestBody(@RequestBody Publisher body) { + return Publishers.map(body, s -> "hello " + s); + } + } + + private static class Foo { + } + + private static class ServerError500ExceptionHandler implements HttpExceptionHandler { + + @Override + public Publisher handle(ServerHttpRequest request, ServerHttpResponse response, Throwable ex) { + response.setStatusCode(HttpStatus.INTERNAL_SERVER_ERROR); + return Publishers.empty(); + } + } + + private static class TestHttpFilter implements HttpFilter { + + @Override + public Publisher filter(ServerHttpRequest req, ServerHttpResponse res, HttpFilterChain chain) { + return chain.filter(req, res); + } + } + +}