Polish WebHttpHandlerBuilder

spring-web/src/main/java/org/springframework/web/server/adapter/WebHttpHandlerBuilder.java

 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -16,8 +16,6 @@
 package org.springframework.web.server.adapter;
 
 import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
 import java.util.List;
 
 import org.springframework.beans.factory.NoSuchBeanDefinitionException;
@@ -65,7 +63,7 @@ public class WebHttpHandlerBuilder {
 	public static final String WEB_SESSION_MANAGER_BEAN_NAME = "webSessionManager";
 
 
-	private final WebHandler targetHandler;
+	private final WebHandler webHandler;
 
 	private final List<WebFilter> filters = new ArrayList<>();
 
@@ -76,16 +74,15 @@ public class WebHttpHandlerBuilder {
 
 	/**
 	 * Private constructor.
-	 * See factory method {@link #webHandler(WebHandler)}.
 	 */
-	private WebHttpHandlerBuilder(WebHandler targetHandler) {
-		Assert.notNull(targetHandler, "WebHandler must not be null");
-		this.targetHandler = targetHandler;
+	private WebHttpHandlerBuilder(WebHandler webHandler) {
+		Assert.notNull(webHandler, "WebHandler must not be null");
+		this.webHandler = webHandler;
 	}
 
 
 	/**
-	 * Factory method to create a new builder instance.
+	 * Static factory method to create a new builder instance.
 	 * @param webHandler the target handler for the request
 	 * @return the prepared builder
 	 */
@@ -94,8 +91,8 @@ public class WebHttpHandlerBuilder {
 	}
 
 	/**
-	 * Factory method to create a new builder instance by detecting beans in an
-	 * {@link ApplicationContext}. The following are detected:
+	 * Static factory method to create a new builder instance by detecting beans
+	 * in an {@link ApplicationContext}. The following are detected:
 	 * <ul>
 	 *	<li>{@link WebHandler} [1] -- looked up by the name
 	 *	{@link #WEB_HANDLER_BEAN_NAME}.
@@ -111,24 +108,15 @@ public class WebHttpHandlerBuilder {
 	 */
 	public static WebHttpHandlerBuilder applicationContext(ApplicationContext context) {
 
-		// Target WebHandler
-
 		WebHttpHandlerBuilder builder = new WebHttpHandlerBuilder(
 				context.getBean(WEB_HANDLER_BEAN_NAME, WebHandler.class));
 
-		// WebFilter...
-
-		AutowiredFiltersContainer filtersContainer = new AutowiredFiltersContainer();
-		context.getAutowireCapableBeanFactory().autowireBean(filtersContainer);
-		builder.filters(filtersContainer.getFilters());
-
-		// WebExceptionHandler...
-
-		AutowiredExceptionHandlersContainer handlersContainer = new AutowiredExceptionHandlersContainer();
-		context.getAutowireCapableBeanFactory().autowireBean(handlersContainer);
-		builder.exceptionHandlers(handlersContainer.getExceptionHandlers());
+		// Autowire lists for @Bean + @Order
 
-		// WebSessionManager
+		SortedBeanContainer container = new SortedBeanContainer();
+		context.getAutowireCapableBeanFactory().autowireBean(container);
+		builder.filters(container.getFilters());
+		builder.exceptionHandlers(container.getExceptionHandlers());
 
 		try {
 			builder.sessionManager(
@@ -146,15 +134,7 @@ public class WebHttpHandlerBuilder {
 	 * Add the given filters to use for processing requests.
 	 * @param filters the filters to add
 	 */
-	public WebHttpHandlerBuilder filters(WebFilter... filters) {
-		return filters(Arrays.asList(filters));
-	}
-
-	/**
-	 * Add the given filters to use for processing requests.
-	 * @param filters the filters to add
-	 */
-	public WebHttpHandlerBuilder filters(Collection<? extends WebFilter> filters) {
+	public WebHttpHandlerBuilder filters(List<? extends WebFilter> filters) {
 		if (!ObjectUtils.isEmpty(filters)) {
 			this.filters.addAll(filters);
 		}
@@ -163,19 +143,11 @@ public class WebHttpHandlerBuilder {
 
 	/**
 	 * Add the given exception handler to apply at the end of request processing.
-	 * @param exceptionHandlers the exception handlers
-	 */
-	public WebHttpHandlerBuilder exceptionHandlers(WebExceptionHandler... exceptionHandlers) {
-		return exceptionHandlers(Arrays.asList(exceptionHandlers));
-	}
-
-	/**
-	 * Add the given exception handler to apply at the end of request processing.
-	 * @param exceptionHandlers the exception handlers
+	 * @param handlers the exception handlers
 	 */
-	public WebHttpHandlerBuilder exceptionHandlers(List<WebExceptionHandler> exceptionHandlers) {
-		if (!ObjectUtils.isEmpty(exceptionHandlers)) {
-			this.exceptionHandlers.addAll(exceptionHandlers);
+	public WebHttpHandlerBuilder exceptionHandlers(List<WebExceptionHandler> handlers) {
+		if (!ObjectUtils.isEmpty(handlers)) {
+			this.exceptionHandlers.addAll(handlers);
 		}
 		return this;
 	}
@@ -184,38 +156,41 @@ public class WebHttpHandlerBuilder {
 	 * Configure the {@link WebSessionManager} to set on the
 	 * {@link ServerWebExchange WebServerExchange}.
 	 * <p>By default {@link DefaultWebSessionManager} is used.
-	 * @param sessionManager the session manager
+	 * @param manager the session manager
 	 * @see HttpWebHandlerAdapter#setSessionManager(WebSessionManager)
 	 */
-	public WebHttpHandlerBuilder sessionManager(WebSessionManager sessionManager) {
-		this.sessionManager = sessionManager;
+	public WebHttpHandlerBuilder sessionManager(WebSessionManager manager) {
+		this.sessionManager = manager;
 		return this;
 	}
 
+
 	/**
 	 * Build the {@link HttpHandler}.
 	 */
 	public HttpHandler build() {
-		WebHandler webHandler = this.targetHandler;
-		if (!this.filters.isEmpty()) {
-			WebFilter[] array = new WebFilter[this.filters.size()];
-			webHandler = new FilteringWebHandler(webHandler, this.filters.toArray(array));
-		}
-		WebExceptionHandler[] array = new WebExceptionHandler[this.exceptionHandlers.size()];
-		webHandler = new ExceptionHandlingWebHandler(webHandler,  this.exceptionHandlers.toArray(array));
-		// TODO: protected method for further decoration
-		HttpWebHandlerAdapter httpHandler = new HttpWebHandlerAdapter(webHandler);
+
+		WebHandler decorated;
+
+		decorated = new FilteringWebHandler(this.webHandler, this.filters);
+		decorated = new ExceptionHandlingWebHandler(decorated,  this.exceptionHandlers);
+
+		HttpWebHandlerAdapter adapted = new HttpWebHandlerAdapter(decorated);
 		if (this.sessionManager != null) {
-			httpHandler.setSessionManager(this.sessionManager);
+			adapted.setSessionManager(this.sessionManager);
 		}
-		return httpHandler;
+
+		return adapted;
 	}
 
 
-	private static class AutowiredFiltersContainer {
+	private static class SortedBeanContainer {
 
 		private List<WebFilter> filters;
 
+		private List<WebExceptionHandler> exceptionHandlers;
+
+
 		@Autowired(required = false)
 		public void setFilters(List<WebFilter> filters) {
 			this.filters = filters;
@@ -224,11 +199,6 @@ public class WebHttpHandlerBuilder {
 		public List<WebFilter> getFilters() {
 			return this.filters;
 		}
-	}
-
-	private static class AutowiredExceptionHandlersContainer {
-
-		private List<WebExceptionHandler> exceptionHandlers;
 
 		@Autowired(required = false)
 		public void setExceptionHandlers(List<WebExceptionHandler> exceptionHandlers) {

spring-web/src/main/java/org/springframework/web/server/handler/ExceptionHandlingWebHandler.java

 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -16,6 +16,7 @@
 
 package org.springframework.web.server.handler;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
@@ -33,8 +34,8 @@ import org.springframework.web.server.WebExceptionHandler;
 import org.springframework.web.server.WebHandler;
 
 /**
- * WebHandler that can invoke a target {@link WebHandler} and then apply
- * exception handling with one or more {@link WebExceptionHandler} instances.
+ * WebHandler decorator that invokes one or more {@link WebExceptionHandler}s
+ * after the delegate {@link WebHandler}.
  *
  * @author Rossen Stoyanchev
  * @since 5.0
@@ -42,41 +43,39 @@ import org.springframework.web.server.WebHandler;
 public class ExceptionHandlingWebHandler extends WebHandlerDecorator {
 
 	/**
-	 * Log category to use on network IO exceptions after a client has gone away.
-	 * <p>Servlet containers do not expose notifications when a client disconnects;
-	 * see <a href="https://java.net/jira/browse/SERVLET_SPEC-44">SERVLET_SPEC-44</a>.
-	 * Therefore network IO failures may occur simply because a client has gone away,
-	 * and that can fill the logs with unnecessary stack traces.
-	 * <p>We make a best effort to identify such network failures, on a per-server
-	 * basis and log them under a separate log category. A simple one-line message
-	 * is logged at DEBUG level instead while a full stack trace is shown at TRACE.
+	 * Dedicated log category for disconnected client exceptions.
+	 * <p>Servlet containers do not expose a notification when a client disconnects,
+	 * e.g. <a href="https://java.net/jira/browse/SERVLET_SPEC-44">SERVLET_SPEC-44</a>.
+	 * <p>To avoid filling logs with unnecessary stack traces, we make an
+	 * effort to identify such network failures on a per-server basis, and then
+	 * log under a separate log category a simple one-line message at DEBUG level
+	 * or a full stack trace only at TRACE level.
 	 */
 	private static final String DISCONNECTED_CLIENT_LOG_CATEGORY =
 			ExceptionHandlingWebHandler.class.getName() + ".DisconnectedClient";
 
-	private static final Set<String> DISCONNECTED_CLIENT_EXCEPTIONS;
+	private static final Set<String> DISCONNECTED_CLIENT_EXCEPTIONS =
+			new HashSet<>(Arrays.asList("ClientAbortException", "EOFException", "EofException"));
 
-	static {
-		Set<String> set = new HashSet<>(3);
-		set.add("ClientAbortException");  // Tomcat
-		set.add("EOFException");  // Tomcat
-		set.add("EofException");  // Jetty
-		// java.io.IOException("Broken pipe") on WildFly (already covered)
-		DISCONNECTED_CLIENT_EXCEPTIONS = Collections.unmodifiableSet(set);
-	}
 
 
 	private static final Log logger = LogFactory.getLog(ExceptionHandlingWebHandler.class);
 
 	private static final Log disconnectedClientLogger = LogFactory.getLog(DISCONNECTED_CLIENT_LOG_CATEGORY);
 
+
 	private final List<WebExceptionHandler> exceptionHandlers;
 
 
-	public ExceptionHandlingWebHandler(WebHandler delegate, WebExceptionHandler... exceptionHandlers) {
+	public ExceptionHandlingWebHandler(WebHandler delegate, List<WebExceptionHandler> handlers) {
 		super(delegate);
-		this.exceptionHandlers = (exceptionHandlers != null ?
-				Collections.unmodifiableList(Arrays.asList(exceptionHandlers)): Collections.emptyList());
+		this.exceptionHandlers = initHandlers(handlers);
+	}
+
+	private List<WebExceptionHandler> initHandlers(List<WebExceptionHandler> handlers) {
+		List<WebExceptionHandler> result = new ArrayList<>(handlers);
+		result.add(new UnresolvedExceptionHandler());
+		return Collections.unmodifiableList(result);
 	}
 
 
@@ -90,44 +89,53 @@ public class ExceptionHandlingWebHandler extends WebHandlerDecorator {
 
 	@Override
 	public Mono<Void> handle(ServerWebExchange exchange) {
-		Mono<Void> mono;
+
+		Mono<Void> completion;
 		try {
-			mono = getDelegate().handle(exchange);
+			completion = super.handle(exchange);
 		}
 		catch (Throwable ex) {
-			mono = Mono.error(ex);
+			completion = Mono.error(ex);
 		}
-		for (WebExceptionHandler exceptionHandler : this.exceptionHandlers) {
-			mono = mono.otherwise(ex -> exceptionHandler.handle(exchange, ex));
+
+		for (WebExceptionHandler handler : this.exceptionHandlers) {
+			completion = completion.otherwise(ex -> handler.handle(exchange, ex));
 		}
-		return mono.otherwise(ex -> handleUnresolvedException(exchange, ex));
-	}
 
-	private Mono<? extends Void> handleUnresolvedException(ServerWebExchange exchange, Throwable ex) {
-		logException(ex);
-		exchange.getResponse().setStatusCode(HttpStatus.INTERNAL_SERVER_ERROR);
-		return Mono.empty();
+		return completion;
 	}
 
-	private void logException(Throwable ex) {
-		@SuppressWarnings("serial")
-		NestedCheckedException nestedException = new NestedCheckedException("", ex) {};
 
-		if ("Broken pipe".equalsIgnoreCase(nestedException.getMostSpecificCause().getMessage()) ||
-				DISCONNECTED_CLIENT_EXCEPTIONS.contains(ex.getClass().getSimpleName())) {
+	private static class UnresolvedExceptionHandler implements WebExceptionHandler {
+
+
+		@Override
+		public Mono<Void> handle(ServerWebExchange exchange, Throwable ex) {
+			logException(ex);
+			exchange.getResponse().setStatusCode(HttpStatus.INTERNAL_SERVER_ERROR);
+			return exchange.getResponse().setComplete();
+		}
 
-			if (disconnectedClientLogger.isTraceEnabled()) {
-				disconnectedClientLogger.trace("Looks like the client has gone away", ex);
+		@SuppressWarnings("serial")
+		private void logException(Throwable ex) {
+			NestedCheckedException nestedEx = new NestedCheckedException("", ex) {};
+			if ("Broken pipe".equalsIgnoreCase(nestedEx.getMostSpecificCause().getMessage()) ||
+					DISCONNECTED_CLIENT_EXCEPTIONS.contains(ex.getClass().getSimpleName())) {
+
+				if (disconnectedClientLogger.isTraceEnabled()) {
+					disconnectedClientLogger.trace("Looks like the client has gone away", ex);
+				}
+				else if (disconnectedClientLogger.isDebugEnabled()) {
+					disconnectedClientLogger.debug(
+							"The client has gone away: " + nestedEx.getMessage() +
+									" (For a full stack trace, set the log category" +
+									"'" + DISCONNECTED_CLIENT_LOG_CATEGORY + "' to TRACE)");
+				}
 			}
-			else if (disconnectedClientLogger.isDebugEnabled()) {
-				disconnectedClientLogger.debug("Looks like the client has gone away: " +
-						nestedException.getMessage() + " (For full stack trace, set the '" +
-						DISCONNECTED_CLIENT_LOG_CATEGORY + "' log category to TRACE level)");
+			else {
+				logger.error("Could not complete request", ex);
 			}
 		}
-		else {
-			logger.error("Could not complete request", ex);
-		}
 	}
 
 }

spring-web/src/main/java/org/springframework/web/server/handler/FilteringWebHandler.java

 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -16,7 +16,6 @@
 
 package org.springframework.web.server.handler;
 
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
@@ -28,8 +27,8 @@ import org.springframework.web.server.WebFilterChain;
 import org.springframework.web.server.WebHandler;
 
 /**
- * WebHandler that delegates to a chain of {@link WebFilter} instances and then
- * to the target {@link WebHandler}.
+ * WebHandler decorator that invokes a chain of {@link WebFilter}s before the
+ * delegate {@link WebHandler}.
  *
  * @author Rossen Stoyanchev
  * @since 5.0
@@ -39,25 +38,29 @@ public class FilteringWebHandler extends WebHandlerDecorator {
 	private final List<WebFilter> filters;
 
 
-	public FilteringWebHandler(WebHandler targetHandler, WebFilter... filters) {
-		super(targetHandler);
-		this.filters = initList(filters);
-	}
-
-	private static List<WebFilter> initList(WebFilter[] list) {
-		return (list != null ? Collections.unmodifiableList(Arrays.asList(list)) : Collections.emptyList());
+	/**
+	 * Constructor.
+	 * @param filters the chain of filters
+	 */
+	public FilteringWebHandler(WebHandler webHandler, List<WebFilter> filters) {
+		super(webHandler);
+		this.filters = Collections.unmodifiableList(filters);
 	}
 
 
 	/**
-	 * Return a read-only list of the configured filters.
+	 * Return read-only list of the configured filters.
 	 */
 	public List<WebFilter> getFilters() {
 		return this.filters;
 	}
 
+
 	@Override
 	public Mono<Void> handle(ServerWebExchange exchange) {
+		if (this.filters.isEmpty()) {
+			return super.handle(exchange);
+		}
 		return new DefaultWebFilterChain().filter(exchange);
 	}
 

spring-web/src/test/java/org/springframework/web/server/handler/ExceptionHandlingHttpHandlerTests.java

@@ -16,6 +16,8 @@
 
 package org.springframework.web.server.handler;
 
+import java.util.Arrays;
+
 import org.junit.Before;
 import org.junit.Test;
 import reactor.core.publisher.Mono;
@@ -90,7 +92,7 @@ public class ExceptionHandlingHttpHandlerTests {
 	}
 
 	private WebHandler createWebHandler(WebExceptionHandler... handlers) {
-		return new ExceptionHandlingWebHandler(this.targetHandler, handlers);
+		return new ExceptionHandlingWebHandler(this.targetHandler, Arrays.asList(handlers));
 	}
 
 
@@ -101,11 +103,11 @@ public class ExceptionHandlingHttpHandlerTests {
 		private final boolean raise;
 
 
-		public StubWebHandler(RuntimeException exception) {
+		StubWebHandler(RuntimeException exception) {
 			this(exception, false);
 		}
 
-		public StubWebHandler(RuntimeException exception, boolean raise) {
+		StubWebHandler(RuntimeException exception, boolean raise) {
 			this.exception = exception;
 			this.raise = raise;
 		}

spring-web/src/test/java/org/springframework/web/server/handler/FilteringWebHandlerTests.java

@@ -16,6 +16,10 @@
 
 package org.springframework.web.server.handler;
 
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.junit.Before;
@@ -110,9 +114,13 @@ public class FilteringWebHandlerTests {
 	@Test
 	public void handleErrorFromFilter() throws Exception {
 		TestExceptionHandler exceptionHandler = new TestExceptionHandler();
-		HttpHandler handler = WebHttpHandlerBuilder.webHandler(new StubWebHandler())
-				.filters(new ExceptionFilter()).exceptionHandlers(exceptionHandler).build();
-		handler.handle(this.request, this.response).block();
+		List<ExceptionFilter> filters = Collections.singletonList(new ExceptionFilter());
+		List<WebExceptionHandler> exceptionHandlers = Collections.singletonList(exceptionHandler);
+
+		WebHttpHandlerBuilder.webHandler(new StubWebHandler())
+				.filters(filters).exceptionHandlers(exceptionHandlers).build()
+				.handle(this.request, this.response)
+				.block();
 
 		assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, this.response.getStatusCode());
 
@@ -123,7 +131,7 @@ public class FilteringWebHandlerTests {
 
 
 	private HttpHandler createHttpHandler(StubWebHandler webHandler, WebFilter... filters) {
-		return WebHttpHandlerBuilder.webHandler(webHandler).filters(filters).build();
+		return WebHttpHandlerBuilder.webHandler(webHandler).filters(Arrays.asList(filters)).build();
 	}
 
 

spring-webflux/src/test/java/org/springframework/web/reactive/DispatcherHandlerErrorTests.java

@@ -18,6 +18,7 @@ package org.springframework.web.reactive;
 
 import java.time.Duration;
 import java.util.Collections;
+import java.util.List;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -158,8 +159,8 @@ public class DispatcherHandlerErrorTests {
 		this.request = MockServerHttpRequest.get("/unknown-argument-type").build();
 		ServerWebExchange exchange = createExchange();
 
-		WebExceptionHandler exceptionHandler = new ServerError500ExceptionHandler();
-		WebHandler webHandler = new ExceptionHandlingWebHandler(this.dispatcherHandler, exceptionHandler);
+		List<WebExceptionHandler> handlers = Collections.singletonList(new ServerError500ExceptionHandler());
+		WebHandler webHandler = new ExceptionHandlingWebHandler(this.dispatcherHandler, handlers);
 		webHandler.handle(exchange).block(Duration.ofSeconds(5));
 
 		assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, exchange.getResponse().getStatusCode());

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

@@ -18,6 +18,7 @@ package org.springframework.web.reactive.result;
 
 import java.net.URI;
 import java.nio.charset.StandardCharsets;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -62,7 +63,7 @@ public class SimpleUrlHandlerMappingIntegrationTests extends AbstractHttpHandler
 		wac.refresh();
 
 		return WebHttpHandlerBuilder.webHandler(new DispatcherHandler(wac))
-				.exceptionHandlers(new ResponseStatusExceptionHandler())
+				.exceptionHandlers(Collections.singletonList(new ResponseStatusExceptionHandler()))
 				.build();
 	}