From 9f857c1f16bf634ec2e595d74c0ad39ccaaff98b Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 13 Nov 2018 14:49:23 -0500 Subject: [PATCH] Replace constant exceptions with inlined ones Issue: SRP-17475 --- .../web/reactive/DispatcherHandler.java | 11 +++++++++-- .../reactive/resource/ResourceWebHandler.java | 4 +--- .../reactive/DispatcherHandlerErrorTests.java | 17 +++++++++++------ .../resource/ResourceWebHandlerTests.java | 11 ++++++++++- 4 files changed, 31 insertions(+), 12 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/DispatcherHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/DispatcherHandler.java index 86d3862b92..b047a6f96b 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/DispatcherHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/DispatcherHandler.java @@ -142,16 +142,23 @@ public class DispatcherHandler implements WebHandler, ApplicationContextAware { @Override public Mono handle(ServerWebExchange exchange) { if (this.handlerMappings == null) { - return Mono.error(HANDLER_NOT_FOUND_EXCEPTION); + return createNotFoundError(); } return Flux.fromIterable(this.handlerMappings) .concatMap(mapping -> mapping.getHandler(exchange)) .next() - .switchIfEmpty(Mono.error(HANDLER_NOT_FOUND_EXCEPTION)) + .switchIfEmpty(createNotFoundError()) .flatMap(handler -> invokeHandler(exchange, handler)) .flatMap(result -> handleResult(exchange, result)); } + private Mono createNotFoundError() { + return Mono.defer(() -> { + Exception ex = new ResponseStatusException(HttpStatus.NOT_FOUND, "No matching handler"); + return Mono.error(ex); + }); + } + private Mono invokeHandler(ServerWebExchange exchange, Object handler) { if (this.handlerAdapters != null) { for (HandlerAdapter handlerAdapter : this.handlerAdapters) { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java index de678573a2..48f9bf20d5 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java @@ -88,8 +88,6 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { private static final Set SUPPORTED_METHODS = EnumSet.of(HttpMethod.GET, HttpMethod.HEAD); - private static final Exception NOT_FOUND_EXCEPTION = new ResponseStatusException(HttpStatus.NOT_FOUND); - private static final Log logger = LogFactory.getLog(ResourceWebHandler.class); @@ -324,7 +322,7 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { return getResource(exchange) .switchIfEmpty(Mono.defer(() -> { logger.debug(exchange.getLogPrefix() + "Resource not found"); - return Mono.error(NOT_FOUND_EXCEPTION); + return Mono.error(new ResponseStatusException(HttpStatus.NOT_FOUND)); })) .flatMap(resource -> { try { diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/DispatcherHandlerErrorTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/DispatcherHandlerErrorTests.java index e4d34801a3..f74f30e01c 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/DispatcherHandlerErrorTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/DispatcherHandlerErrorTests.java @@ -19,6 +19,7 @@ package org.springframework.web.reactive; import java.time.Duration; import java.util.Collections; import java.util.List; +import java.util.concurrent.atomic.AtomicReference; import org.junit.Before; import org.junit.Test; @@ -81,15 +82,19 @@ public class DispatcherHandlerErrorTests { @Test public void noHandler() { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/does-not-exist")); - Mono publisher = this.dispatcherHandler.handle(exchange); + Mono mono = this.dispatcherHandler.handle(exchange); - StepVerifier.create(publisher) - .consumeErrorWith(error -> { - assertThat(error, instanceOf(ResponseStatusException.class)); - assertThat(error.getMessage(), - is("404 NOT_FOUND \"No matching handler\"")); + StepVerifier.create(mono) + .consumeErrorWith(ex -> { + assertThat(ex, instanceOf(ResponseStatusException.class)); + assertThat(ex.getMessage(), is("404 NOT_FOUND \"No matching handler\"")); }) .verify(); + + // SPR-17475 + AtomicReference exceptionRef = new AtomicReference<>(); + StepVerifier.create(mono).consumeErrorWith(exceptionRef::set).verify(); + StepVerifier.create(mono).consumeErrorWith(ex -> assertNotSame(exceptionRef.get(), ex)).verify(); } @Test diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java index 387f3077b0..a99edfd028 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java @@ -24,6 +24,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import org.junit.Before; import org.junit.Test; @@ -274,6 +275,7 @@ public class ResourceWebHandlerTests { private void testInvalidPath(String requestPath, ResourceWebHandler handler) { ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("")); setPathWithinHandlerMapping(exchange, requestPath); + StepVerifier.create(handler.handle(exchange)) .expectErrorSatisfies(err -> { assertThat(err, instanceOf(ResponseStatusException.class)); @@ -468,11 +470,18 @@ public class ResourceWebHandlerTests { MockServerHttpRequest request = MockServerHttpRequest.method(httpMethod, "").build(); MockServerWebExchange exchange = MockServerWebExchange.from(request); setPathWithinHandlerMapping(exchange, "not-there.css"); - StepVerifier.create(this.handler.handle(exchange)) + Mono mono = this.handler.handle(exchange); + + StepVerifier.create(mono) .expectErrorSatisfies(err -> { assertThat(err, instanceOf(ResponseStatusException.class)); assertEquals(HttpStatus.NOT_FOUND, ((ResponseStatusException) err).getStatus()); }).verify(TIMEOUT); + + // SPR-17475 + AtomicReference exceptionRef = new AtomicReference<>(); + StepVerifier.create(mono).consumeErrorWith(exceptionRef::set).verify(); + StepVerifier.create(mono).consumeErrorWith(ex -> assertNotSame(exceptionRef.get(), ex)).verify(); } @Test