Improve API of ErrorAttributes and DefaultErrorAttributes

This commit improves the backward-compatibility of the ErrorAttributes interfaces by providing a default implementation of a new method. It also encapsulates several parameters that control the inclusion or exclusion of error attributes into a new ErrorAttributeOptions type to make it easier and less intrusive to add additional options in the future. This encapsulation also makes the handling of the includeException option more similar to other options. Fixes gh-21324
spring-projects · May 11, 2020 · 158933c · 158933c
1 parent c3c7fc0
commit 158933c
Show file tree

Hide file tree

Showing 20 changed files with 586 additions and 175 deletions.
diff --git a/...a/org/springframework/boot/actuate/autoconfigure/web/servlet/ManagementErrorEndpoint.java b/...a/org/springframework/boot/actuate/autoconfigure/web/servlet/ManagementErrorEndpoint.java
@@ -19,6 +19,7 @@
 import java.util.Map;
 
 import org.springframework.boot.autoconfigure.web.ErrorProperties;
+import org.springframework.boot.web.error.ErrorAttributeOptions;
 import org.springframework.boot.web.servlet.error.ErrorAttributes;
 import org.springframework.boot.web.servlet.error.ErrorController;
 import org.springframework.stereotype.Controller;
@@ -53,10 +54,27 @@ public ManagementErrorEndpoint(ErrorAttributes errorAttributes, ErrorProperties
 	@RequestMapping("${server.error.path:${error.path:/error}}")
 	@ResponseBody
 	public Map<String, Object> invoke(ServletWebRequest request) {
-		return this.errorAttributes.getErrorAttributes(request, includeStackTrace(request), includeMessage(request),
-				includeBindingErrors(request));
+		return this.errorAttributes.getErrorAttributes(request, getErrorAttributeOptions(request));
 	}
 
+	private ErrorAttributeOptions getErrorAttributeOptions(ServletWebRequest request) {
+		ErrorAttributeOptions options = ErrorAttributeOptions.defaults();
+		if (this.errorProperties.isIncludeException()) {
+			options = options.including(ErrorAttributeOptions.Include.EXCEPTION);
+		}
+		if (includeStackTrace(request)) {
+			options = options.including(ErrorAttributeOptions.Include.STACK_TRACE);
+		}
+		if (includeMessage(request)) {
+			options = options.including(ErrorAttributeOptions.Include.MESSAGE);
+		}
+		if (includeBindingErrors(request)) {
+			options = options.including(ErrorAttributeOptions.Include.BINDING_ERRORS);
+		}
+		return options;
+	}
+
+	@SuppressWarnings("deprecation")
 	private boolean includeStackTrace(ServletWebRequest request) {
 		switch (this.errorProperties.getIncludeStacktrace()) {
 		case ALWAYS:

diff --git a/.../springframework/boot/actuate/autoconfigure/web/servlet/ManagementErrorEndpointTests.java b/.../springframework/boot/actuate/autoconfigure/web/servlet/ManagementErrorEndpointTests.java
@@ -16,6 +16,7 @@
 
 package org.springframework.boot.actuate.autoconfigure.web.servlet;
 
+import java.util.HashMap;
 import java.util.Map;
 
 import org.junit.jupiter.api.BeforeEach;
@@ -26,6 +27,7 @@
 import org.springframework.boot.web.servlet.error.ErrorAttributes;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.web.context.request.ServletWebRequest;
+import org.springframework.web.context.request.WebRequest;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -103,4 +105,63 @@ void errorResponseParamsFalse() {
 		assertThat(response).doesNotContainKey("trace");
 	}
 
+	@Test
+	void errorResponseWithCustomErrorAttributesUsingDeprecatedApi() {
+		ErrorAttributes attributes = new ErrorAttributes() {
+			@Override
+			public Map<String, Object> getErrorAttributes(WebRequest webRequest, boolean includeStackTrace) {
+				Map<String, Object> response = new HashMap<>();
+				response.put("message", "An error occurred");
+				return response;
+			}
+
+			@Override
+			public Throwable getError(WebRequest webRequest) {
+				return null;
+			}
+		};
+		ManagementErrorEndpoint endpoint = new ManagementErrorEndpoint(attributes, this.errorProperties);
+		Map<String, Object> response = endpoint.invoke(new ServletWebRequest(new MockHttpServletRequest()));
+		assertThat(response).hasSize(1);
+		assertThat(response).containsEntry("message", "An error occurred");
+	}
+
+	@Test
+	void errorResponseWithDefaultErrorAttributesSubclassUsingDeprecatedApiAndDelegation() {
+		ErrorAttributes attributes = new DefaultErrorAttributes() {
+			@Override
+			@SuppressWarnings("deprecation")
+			public Map<String, Object> getErrorAttributes(WebRequest webRequest, boolean includeStackTrace) {
+				Map<String, Object> response = super.getErrorAttributes(webRequest, includeStackTrace);
+				response.put("error", "custom error");
+				response.put("custom", "value");
+				response.remove("path");
+				return response;
+			}
+		};
+		ManagementErrorEndpoint endpoint = new ManagementErrorEndpoint(attributes, this.errorProperties);
+		Map<String, Object> response = endpoint.invoke(new ServletWebRequest(new MockHttpServletRequest()));
+		assertThat(response).containsEntry("error", "custom error");
+		assertThat(response).containsEntry("custom", "value");
+		assertThat(response).doesNotContainKey("path");
+		assertThat(response).containsKey("timestamp");
+	}
+
+	@Test
+	void errorResponseWithDefaultErrorAttributesSubclassUsingDeprecatedApiWithoutDelegation() {
+		ErrorAttributes attributes = new DefaultErrorAttributes() {
+			@Override
+			@SuppressWarnings("deprecation")
+			public Map<String, Object> getErrorAttributes(WebRequest webRequest, boolean includeStackTrace) {
+				Map<String, Object> response = new HashMap<>();
+				response.put("error", "custom error");
+				return response;
+			}
+		};
+		ManagementErrorEndpoint endpoint = new ManagementErrorEndpoint(attributes, this.errorProperties);
+		Map<String, Object> response = endpoint.invoke(new ServletWebRequest(new MockHttpServletRequest()));
+		assertThat(response).hasSize(1);
+		assertThat(response).containsEntry("error", "custom error");
+	}
+
 }
diff --git a/...ringframework/boot/autoconfigure/web/reactive/error/AbstractErrorWebExceptionHandler.java b/...ringframework/boot/autoconfigure/web/reactive/error/AbstractErrorWebExceptionHandler.java
@@ -29,6 +29,8 @@
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.boot.autoconfigure.template.TemplateAvailabilityProviders;
 import org.springframework.boot.autoconfigure.web.ResourceProperties;
+import org.springframework.boot.web.error.ErrorAttributeOptions;
+import org.springframework.boot.web.error.ErrorAttributeOptions.Include;
 import org.springframework.boot.web.reactive.error.ErrorAttributes;
 import org.springframework.boot.web.reactive.error.ErrorWebExceptionHandler;
 import org.springframework.context.ApplicationContext;
@@ -134,26 +136,23 @@ public void setViewResolvers(List<ViewResolver> viewResolvers) {
 	 * @param includeStackTrace whether to include the error stacktrace information
 	 * @return the error attributes as a Map
 	 * @deprecated since 2.3.0 in favor of
-	 * {@link #getErrorAttributes(ServerRequest, boolean, boolean, boolean)}
+	 * {@link #getErrorAttributes(ServerRequest, ErrorAttributeOptions)}
 	 */
 	@Deprecated
 	protected Map<String, Object> getErrorAttributes(ServerRequest request, boolean includeStackTrace) {
-		return this.errorAttributes.getErrorAttributes(request, includeStackTrace, false, false);
+		return getErrorAttributes(request,
+				(includeStackTrace) ? ErrorAttributeOptions.of(Include.STACK_TRACE) : ErrorAttributeOptions.defaults());
 	}
 
 	/**
 	 * Extract the error attributes from the current request, to be used to populate error
 	 * views or JSON payloads.
 	 * @param request the source request
-	 * @param includeStackTrace whether to include the stacktrace attribute
-	 * @param includeMessage whether to include the message attribute
-	 * @param includeBindingErrors whether to include the errors attribute
+	 * @param options options to control error attributes
 	 * @return the error attributes as a Map
 	 */
-	protected Map<String, Object> getErrorAttributes(ServerRequest request, boolean includeStackTrace,
-			boolean includeMessage, boolean includeBindingErrors) {
-		return this.errorAttributes.getErrorAttributes(request, includeStackTrace, includeMessage,
-				includeBindingErrors);
+	protected Map<String, Object> getErrorAttributes(ServerRequest request, ErrorAttributeOptions options) {
+		return this.errorAttributes.getErrorAttributes(request, options);
 	}
 
 	/**

diff --git a/...pringframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandler.java b/...pringframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandler.java
@@ -28,6 +28,8 @@
 
 import org.springframework.boot.autoconfigure.web.ErrorProperties;
 import org.springframework.boot.autoconfigure.web.ResourceProperties;
+import org.springframework.boot.web.error.ErrorAttributeOptions;
+import org.springframework.boot.web.error.ErrorAttributeOptions.Include;
 import org.springframework.boot.web.reactive.error.ErrorAttributes;
 import org.springframework.context.ApplicationContext;
 import org.springframework.http.HttpStatus;
@@ -113,11 +115,7 @@ protected RouterFunction<ServerResponse> getRoutingFunction(ErrorAttributes erro
 	 * @return a {@code Publisher} of the HTTP response
 	 */
 	protected Mono<ServerResponse> renderErrorView(ServerRequest request) {
-		boolean includeStackTrace = isIncludeStackTrace(request, MediaType.TEXT_HTML);
-		boolean includeMessage = isIncludeMessage(request, MediaType.TEXT_HTML);
-		boolean includeBindingErrors = isIncludeBindingErrors(request, MediaType.TEXT_HTML);
-		Map<String, Object> error = getErrorAttributes(request, includeStackTrace, includeMessage,
-				includeBindingErrors);
+		Map<String, Object> error = getErrorAttributes(request, getErrorAttributeOptions(request, MediaType.TEXT_HTML));
 		int errorStatus = getHttpStatus(error);
 		ServerResponse.BodyBuilder responseBody = ServerResponse.status(errorStatus).contentType(TEXT_HTML_UTF8);
 		return Flux.just(getData(errorStatus).toArray(new String[] {}))
@@ -144,15 +142,28 @@ private List<String> getData(int errorStatus) {
 	 * @return a {@code Publisher} of the HTTP response
 	 */
 	protected Mono<ServerResponse> renderErrorResponse(ServerRequest request) {
-		boolean includeStackTrace = isIncludeStackTrace(request, MediaType.ALL);
-		boolean includeMessage = isIncludeMessage(request, MediaType.ALL);
-		boolean includeBindingErrors = isIncludeBindingErrors(request, MediaType.ALL);
-		Map<String, Object> error = getErrorAttributes(request, includeStackTrace, includeMessage,
-				includeBindingErrors);
+		Map<String, Object> error = getErrorAttributes(request, getErrorAttributeOptions(request, MediaType.ALL));
 		return ServerResponse.status(getHttpStatus(error)).contentType(MediaType.APPLICATION_JSON)
 				.body(BodyInserters.fromValue(error));
 	}
 
+	protected ErrorAttributeOptions getErrorAttributeOptions(ServerRequest request, MediaType mediaType) {
+		ErrorAttributeOptions options = ErrorAttributeOptions.defaults();
+		if (this.errorProperties.isIncludeException()) {
+			options = options.including(Include.EXCEPTION);
+		}
+		if (isIncludeStackTrace(request, mediaType)) {
+			options = options.including(Include.STACK_TRACE);
+		}
+		if (isIncludeMessage(request, mediaType)) {
+			options = options.including(Include.MESSAGE);
+		}
+		if (isIncludeBindingErrors(request, mediaType)) {
+			options = options.including(Include.BINDING_ERRORS);
+		}
+		return options;
+	}
+
 	/**
 	 * Determine if the stacktrace attribute should be included.
 	 * @param request the source request

diff --git a/.../springframework/boot/autoconfigure/web/reactive/error/ErrorWebFluxAutoConfiguration.java b/.../springframework/boot/autoconfigure/web/reactive/error/ErrorWebFluxAutoConfiguration.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2019 the original author or authors.
+ * Copyright 2012-2020 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.
@@ -45,6 +45,7 @@
  * {@link org.springframework.web.server.WebExceptionHandler}.
  *
  * @author Brian Clozel
+ * @author Scott Frederick
  * @since 2.0.0
  */
 @Configuration(proxyBeanMethods = false)
@@ -77,7 +78,7 @@ public ErrorWebExceptionHandler errorWebExceptionHandler(ErrorAttributes errorAt
 	@Bean
 	@ConditionalOnMissingBean(value = ErrorAttributes.class, search = SearchStrategy.CURRENT)
 	public DefaultErrorAttributes errorAttributes() {
-		return new DefaultErrorAttributes(this.serverProperties.getError().isIncludeException());
+		return new DefaultErrorAttributes();
 	}
 
 }
diff --git a/...ava/org/springframework/boot/autoconfigure/web/servlet/error/AbstractErrorController.java b/...ava/org/springframework/boot/autoconfigure/web/servlet/error/AbstractErrorController.java
@@ -24,6 +24,8 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.springframework.boot.web.error.ErrorAttributeOptions;
+import org.springframework.boot.web.error.ErrorAttributeOptions.Include;
 import org.springframework.boot.web.servlet.error.ErrorAttributes;
 import org.springframework.boot.web.servlet.error.ErrorController;
 import org.springframework.core.annotation.AnnotationAwareOrderComparator;
@@ -74,18 +76,17 @@ private List<ErrorViewResolver> sortErrorViewResolvers(List<ErrorViewResolver> r
 	 * @param includeStackTrace if stack trace elements should be included
 	 * @return the error attributes
 	 * @deprecated since 2.3.0 in favor of
-	 * {@link #getErrorAttributes(HttpServletRequest, boolean, boolean, boolean)}
+	 * {@link #getErrorAttributes(HttpServletRequest, ErrorAttributeOptions)}
 	 */
 	@Deprecated
 	protected Map<String, Object> getErrorAttributes(HttpServletRequest request, boolean includeStackTrace) {
-		return getErrorAttributes(request, includeStackTrace, false, false);
+		return getErrorAttributes(request,
+				(includeStackTrace) ? ErrorAttributeOptions.of(Include.STACK_TRACE) : ErrorAttributeOptions.defaults());
 	}
 
-	protected Map<String, Object> getErrorAttributes(HttpServletRequest request, boolean includeStackTrace,
-			boolean includeMessage, boolean includeBindingErrors) {
+	protected Map<String, Object> getErrorAttributes(HttpServletRequest request, ErrorAttributeOptions options) {
 		WebRequest webRequest = new ServletWebRequest(request);
-		return this.errorAttributes.getErrorAttributes(webRequest, includeStackTrace, includeMessage,
-				includeBindingErrors);
+		return this.errorAttributes.getErrorAttributes(webRequest, options);
 	}
 
 	protected boolean getTraceParameter(HttpServletRequest request) {

diff --git a/...n/java/org/springframework/boot/autoconfigure/web/servlet/error/BasicErrorController.java b/...n/java/org/springframework/boot/autoconfigure/web/servlet/error/BasicErrorController.java
@@ -24,6 +24,8 @@
 import javax.servlet.http.HttpServletResponse;
 
 import org.springframework.boot.autoconfigure.web.ErrorProperties;
+import org.springframework.boot.web.error.ErrorAttributeOptions;
+import org.springframework.boot.web.error.ErrorAttributeOptions.Include;
 import org.springframework.boot.web.servlet.error.ErrorAttributes;
 import org.springframework.boot.web.servlet.server.AbstractServletWebServerFactory;
 import org.springframework.http.HttpStatus;
@@ -87,9 +89,8 @@ public String getErrorPath() {
 	@RequestMapping(produces = MediaType.TEXT_HTML_VALUE)
 	public ModelAndView errorHtml(HttpServletRequest request, HttpServletResponse response) {
 		HttpStatus status = getStatus(request);
-		Map<String, Object> model = Collections.unmodifiableMap(getErrorAttributes(request,
-				isIncludeStackTrace(request, MediaType.TEXT_HTML), isIncludeMessage(request, MediaType.TEXT_HTML),
-				isIncludeBindingErrors(request, MediaType.TEXT_HTML)));
+		Map<String, Object> model = Collections
+				.unmodifiableMap(getErrorAttributes(request, getErrorAttributeOptions(request, MediaType.TEXT_HTML)));
 		response.setStatus(status.value());
 		ModelAndView modelAndView = resolveErrorView(request, response, status, model);
 		return (modelAndView != null) ? modelAndView : new ModelAndView("error", model);
@@ -101,8 +102,7 @@ public ResponseEntity<Map<String, Object>> error(HttpServletRequest request) {
 		if (status == HttpStatus.NO_CONTENT) {
 			return new ResponseEntity<>(status);
 		}
-		Map<String, Object> body = getErrorAttributes(request, isIncludeStackTrace(request, MediaType.ALL),
-				isIncludeMessage(request, MediaType.ALL), isIncludeBindingErrors(request, MediaType.TEXT_HTML));
+		Map<String, Object> body = getErrorAttributes(request, getErrorAttributeOptions(request, MediaType.ALL));
 		return new ResponseEntity<>(body, status);
 	}
 
@@ -112,6 +112,23 @@ public ResponseEntity<String> mediaTypeNotAcceptable(HttpServletRequest request)
 		return ResponseEntity.status(status).build();
 	}
 
+	protected ErrorAttributeOptions getErrorAttributeOptions(HttpServletRequest request, MediaType mediaType) {
+		ErrorAttributeOptions options = ErrorAttributeOptions.defaults();
+		if (this.errorProperties.isIncludeException()) {
+			options = options.including(Include.EXCEPTION);
+		}
+		if (isIncludeStackTrace(request, mediaType)) {
+			options = options.including(Include.STACK_TRACE);
+		}
+		if (isIncludeMessage(request, mediaType)) {
+			options = options.including(Include.MESSAGE);
+		}
+		if (isIncludeBindingErrors(request, mediaType)) {
+			options = options.including(Include.BINDING_ERRORS);
+		}
+		return options;
+	}
+
 	/**
 	 * Determine if the stacktrace attribute should be included.
 	 * @param request the source request

diff --git a/...a/org/springframework/boot/autoconfigure/web/servlet/error/ErrorMvcAutoConfiguration.java b/...a/org/springframework/boot/autoconfigure/web/servlet/error/ErrorMvcAutoConfiguration.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2019 the original author or authors.
+ * Copyright 2012-2020 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.
@@ -81,6 +81,7 @@
  * @author Andy Wilkinson
  * @author Stephane Nicoll
  * @author Brian Clozel
+ * @author Scott Frederick
  * @since 1.0.0
  */
 @Configuration(proxyBeanMethods = false)
@@ -100,7 +101,7 @@ public ErrorMvcAutoConfiguration(ServerProperties serverProperties) {
 	@Bean
 	@ConditionalOnMissingBean(value = ErrorAttributes.class, search = SearchStrategy.CURRENT)
 	public DefaultErrorAttributes errorAttributes() {
-		return new DefaultErrorAttributes(this.serverProperties.getError().isIncludeException());
+		return new DefaultErrorAttributes();
 	}
 
 	@Bean