From 79bc227c9d976aa7a7fddf07e4815b60e1c27df7 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 27 Jul 2016 15:45:15 -0400 Subject: [PATCH] Remove SimpleResultHandler There is really no need for a result handler dedicated to a void return value and it's actually problematic to have it. Each result handler treats void as necessary. For an @ResponseBody method it means an empty body. For view resolution it means no specific value was returned and we should procede with selecting a default view name. Having a dedicated void result handler can interfere with this especially since view resolution needs to be last in order. At the same time there are cases when no result handling is needed and the response is fully handled within the HandlerAdapter. This is the case with WebHandler and the SimpleHandlerAdapter. For that case we simply return mono.then(aVoid -> Mono.empty()) which effectively returns an empty Mono and no result handling follows. The HandlerAdapter already says you can return no values at all if the response is fully handled. --- .../config/WebReactiveConfiguration.java | 6 - .../reactive/result/SimpleHandlerAdapter.java | 2 +- .../reactive/result/SimpleResultHandler.java | 112 ------------------ .../result/SimpleResultHandlerTests.java | 95 --------------- ...mpleUrlHandlerMappingIntegrationTests.java | 5 - 5 files changed, 1 insertion(+), 219 deletions(-) delete mode 100644 spring-web-reactive/src/main/java/org/springframework/web/reactive/result/SimpleResultHandler.java delete mode 100644 spring-web-reactive/src/test/java/org/springframework/web/reactive/result/SimpleResultHandlerTests.java diff --git a/spring-web-reactive/src/main/java/org/springframework/web/reactive/config/WebReactiveConfiguration.java b/spring-web-reactive/src/main/java/org/springframework/web/reactive/config/WebReactiveConfiguration.java index 4b49ec553b2..d262dfcd906 100644 --- a/spring-web-reactive/src/main/java/org/springframework/web/reactive/config/WebReactiveConfiguration.java +++ b/spring-web-reactive/src/main/java/org/springframework/web/reactive/config/WebReactiveConfiguration.java @@ -55,7 +55,6 @@ import org.springframework.validation.Validator; import org.springframework.web.reactive.accept.RequestedContentTypeResolver; import org.springframework.web.reactive.accept.RequestedContentTypeResolverBuilder; import org.springframework.web.reactive.result.SimpleHandlerAdapter; -import org.springframework.web.reactive.result.SimpleResultHandler; import org.springframework.web.reactive.result.method.HandlerMethodArgumentResolver; import org.springframework.web.reactive.result.method.annotation.RequestMappingHandlerAdapter; import org.springframework.web.reactive.result.method.annotation.RequestMappingHandlerMapping; @@ -321,11 +320,6 @@ public class WebReactiveConfiguration implements ApplicationContextAware { return new SimpleHandlerAdapter(); } - @Bean - public SimpleResultHandler simpleResultHandler() { - return new SimpleResultHandler(); - } - @Bean public ResponseEntityResultHandler responseEntityResultHandler() { return new ResponseEntityResultHandler(getMessageWriters(), mvcContentTypeResolver()); diff --git a/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/SimpleHandlerAdapter.java b/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/SimpleHandlerAdapter.java index 091a6b9d15d..69b48538278 100644 --- a/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/SimpleHandlerAdapter.java +++ b/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/SimpleHandlerAdapter.java @@ -60,7 +60,7 @@ public class SimpleHandlerAdapter implements HandlerAdapter { public Mono handle(ServerWebExchange exchange, Object handler) { WebHandler webHandler = (WebHandler) handler; Mono mono = webHandler.handle(exchange); - return Mono.just(new HandlerResult(webHandler, mono, RETURN_TYPE)); + return mono.then(aVoid -> Mono.empty()); } } diff --git a/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/SimpleResultHandler.java b/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/SimpleResultHandler.java deleted file mode 100644 index 39d795d6f21..00000000000 --- a/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/SimpleResultHandler.java +++ /dev/null @@ -1,112 +0,0 @@ -/* - * Copyright 2002-2016 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.result; - -import java.util.Optional; - -import reactor.core.publisher.Mono; - -import org.springframework.core.Ordered; -import org.springframework.core.ReactiveAdapter; -import org.springframework.core.ReactiveAdapterRegistry; -import org.springframework.core.ResolvableType; -import org.springframework.util.Assert; -import org.springframework.web.reactive.HandlerResult; -import org.springframework.web.reactive.HandlerResultHandler; -import org.springframework.web.server.ServerWebExchange; - - -/** - * A simple handler for return values of type {@code void}, or - * {@code Publisher}, or if a {link ConversionService} is provided, also - * of any other async return value types that can be converted to - * {@code Publisher} such as {@code Observable} or - * {@code CompletableFuture}. - * - * @author Sebastien Deleuze - * @author Rossen Stoyanchev - * @since 5.0 - */ -public class SimpleResultHandler implements Ordered, HandlerResultHandler { - - private ReactiveAdapterRegistry adapterRegistry; - - private int order = Ordered.LOWEST_PRECEDENCE; - - - public SimpleResultHandler() { - this.adapterRegistry = new ReactiveAdapterRegistry(); - } - - public SimpleResultHandler(ReactiveAdapterRegistry adapterRegistry) { - Assert.notNull(adapterRegistry, "'adapterRegistry' is required."); - this.adapterRegistry = adapterRegistry; - } - - - /** - * Return the configured {@link ReactiveAdapterRegistry}. - */ - public ReactiveAdapterRegistry getAdapterRegistry() { - return this.adapterRegistry; - } - - /** - * Set the order for this result handler relative to others. - *

By default this is set to {@link Ordered#LOWEST_PRECEDENCE} and is - * generally safe to use late in the order since it looks specifically for - * {@code void} or async return types parameterized by {@code void}. - * @param order the order - */ - public void setOrder(int order) { - this.order = order; - } - - @Override - public int getOrder() { - return this.order; - } - - - @Override - public boolean supports(HandlerResult result) { - ResolvableType type = result.getReturnType(); - Class rawClass = type.getRawClass(); - if (Void.TYPE.equals(rawClass)) { - return true; - } - ReactiveAdapter adapter = getAdapterRegistry().getAdapterFrom(rawClass, result.getReturnValue()); - if (adapter != null) { - Class clazz = type.getGeneric(0).getRawClass(); - return Void.class.equals(clazz); - } - return false; - } - - @SuppressWarnings("unchecked") - @Override - public Mono handleResult(ServerWebExchange exchange, HandlerResult result) { - Optional optionalValue = result.getReturnValue(); - if (!optionalValue.isPresent()) { - return Mono.empty(); - } - Class returnType = result.getReturnType().getRawClass(); - ReactiveAdapter adapter = getAdapterRegistry().getAdapterFrom(returnType, optionalValue); - return adapter.toMono(optionalValue); - } - -} diff --git a/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/SimpleResultHandlerTests.java b/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/SimpleResultHandlerTests.java deleted file mode 100644 index af660ebdfbf..00000000000 --- a/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/SimpleResultHandlerTests.java +++ /dev/null @@ -1,95 +0,0 @@ -/* - * Copyright 2002-2016 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.result; - -import java.util.List; -import java.util.concurrent.CompletableFuture; - -import org.junit.Before; -import org.junit.Test; -import org.reactivestreams.Publisher; -import reactor.core.publisher.Flux; -import rx.Observable; - -import org.springframework.core.MethodParameter; -import org.springframework.core.ResolvableType; -import org.springframework.web.reactive.HandlerResult; - -import static org.junit.Assert.assertEquals; - -/** - * Unit tests for {@link SimpleResultHandler}. - * @author Sebastien Deleuze - * @author Rossen Stoyanchev - */ -public class SimpleResultHandlerTests { - - private SimpleResultHandler resultHandler; - - - @Before - public void setUp() throws Exception { - this.resultHandler = new SimpleResultHandler(); - } - - - @Test - public void supports() throws NoSuchMethodException { - testSupports(ResolvableType.forClass(void.class), true); - testSupports(ResolvableType.forClassWithGenerics(Publisher.class, Void.class), true); - testSupports(ResolvableType.forClassWithGenerics(Flux.class, Void.class), true); - testSupports(ResolvableType.forClassWithGenerics(Observable.class, Void.class), true); - testSupports(ResolvableType.forClassWithGenerics(CompletableFuture.class, Void.class), true); - - testSupports(ResolvableType.forClass(String.class), false); - testSupports(ResolvableType.forClassWithGenerics(Publisher.class, String.class), false); - } - - @Test - public void supportsUsesGenericTypeInformation() throws Exception { - testSupports(ResolvableType.forClassWithGenerics(List.class, Void.class), false); - } - - private void testSupports(ResolvableType type, boolean result) { - MethodParameter param = ResolvableMethod.onClass(TestController.class).returning(type).resolveReturnType(); - HandlerResult handlerResult = new HandlerResult(new TestController(), null, param); - assertEquals(result, this.resultHandler.supports(handlerResult)); - } - - - @SuppressWarnings("unused") - private static class TestController { - - public void voidReturn() { } - - public Publisher publisherString() { return null; } - - public Flux flux() { return null; } - - public Observable observable() { return null; } - - public CompletableFuture completableFuture() { return null; } - - public String string() { return null; } - - public Publisher publisher() { return null; } - - public List list() { return null; } - - } - -} diff --git a/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/SimpleUrlHandlerMappingIntegrationTests.java b/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/SimpleUrlHandlerMappingIntegrationTests.java index 76142d278eb..46668a574a2 100644 --- a/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/SimpleUrlHandlerMappingIntegrationTests.java +++ b/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/SimpleUrlHandlerMappingIntegrationTests.java @@ -143,11 +143,6 @@ public class SimpleUrlHandlerMappingIntegrationTests extends AbstractHttpHandler public SimpleHandlerAdapter handlerAdapter() { return new SimpleHandlerAdapter(); } - - @Bean - public SimpleResultHandler resultHandler() { - return new SimpleResultHandler(); - } } }