From 98c7d8100d3d570916465f769c8457c8b9e593d1 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Fri, 24 Jun 2022 11:08:32 +0100 Subject: [PATCH] Remove "with" methods in ProblemDetail ProblemDetail is intended to be extended with additional fields. This commit removes its "with" methods for chained initialization to keep it as plain as possible and avoid imposing a particular style on subclasses. See gh-27052 --- .../springframework/http/ProblemDetail.java | 91 ++++--------------- ...ttpRequestMethodNotSupportedException.java | 2 +- .../bind/MethodArgumentNotValidException.java | 2 +- .../server/MissingRequestValueException.java | 2 +- .../UnsatisfiedRequestParameterException.java | 2 +- .../web/servlet/NoHandlerFoundException.java | 2 +- ...questResponseBodyMethodProcessorTests.java | 3 +- 7 files changed, 25 insertions(+), 79 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/ProblemDetail.java b/spring-web/src/main/java/org/springframework/http/ProblemDetail.java index 6796c6368a3..d78a4fc13ba 100644 --- a/spring-web/src/main/java/org/springframework/http/ProblemDetail.java +++ b/spring-web/src/main/java/org/springframework/http/ProblemDetail.java @@ -90,75 +90,10 @@ public class ProblemDetail { } - /** - * Variant of {@link #setType(URI)} for chained initialization. - * @param type the problem type - * @return the same instance - */ - public ProblemDetail withType(URI type) { - setType(type); - return this; - } - - /** - * Variant of {@link #setTitle(String)} for chained initialization. - * @param title the problem title - * @return the same instance - */ - public ProblemDetail withTitle(@Nullable String title) { - setTitle(title); - return this; - } - - /** - * Variant of {@link #setStatus(int)} for chained initialization. - * @param statusCode the response status for the problem - * @return the same instance - */ - public ProblemDetail withStatus(HttpStatusCode statusCode) { - Assert.notNull(statusCode, "HttpStatus is required"); - setStatus(statusCode.value()); - return this; - } - - /** - * Variant of {@link #setStatus(int)} for chained initialization. - * @param status the response status value for the problem - * @return the same instance - */ - public ProblemDetail withStatus(int status) { - setStatus(status); - return this; - } - - /** - * Variant of {@link #setDetail(String)} for chained initialization. - * @param detail the problem detail - * @return the same instance - */ - public ProblemDetail withDetail(@Nullable String detail) { - setDetail(detail); - return this; - } - - /** - * Variant of {@link #setInstance(URI)} for chained initialization. - * @param instance the problem instance URI - * @return the same instance - */ - public ProblemDetail withInstance(@Nullable URI instance) { - setInstance(instance); - return this; - } - - - // Setters for deserialization - /** * Setter for the {@link #getType() problem type}. *

By default, this is {@link #BLANK_TYPE}. * @param type the problem type - * @see #withType(URI) */ public void setType(URI type) { Assert.notNull(type, "'type' is required"); @@ -170,17 +105,22 @@ public class ProblemDetail { *

By default, if not explicitly set and the status is well-known, this * is sourced from the {@link HttpStatus#getReasonPhrase()}. * @param title the problem title - * @see #withTitle(String) */ public void setTitle(@Nullable String title) { this.title = title; } + /** + * Setter for the {@link #getStatus() problem status}. + * @param httpStatus the problem status + */ + public void setStatus(HttpStatus httpStatus) { + this.status = httpStatus.value(); + } + /** * Setter for the {@link #getStatus() problem status}. * @param status the problem status - * @see #withStatus(HttpStatusCode) - * @see #withStatus(int) */ public void setStatus(int status) { this.status = status; @@ -190,7 +130,6 @@ public class ProblemDetail { * Setter for the {@link #getDetail() problem detail}. *

By default, this is not set. * @param detail the problem detail - * @see #withDetail(String) */ public void setDetail(@Nullable String detail) { this.detail = detail; @@ -201,7 +140,6 @@ public class ProblemDetail { *

By default, when {@code ProblemDetail} is returned from an * {@code @ExceptionHandler} method, this is initialized to the request path. * @param instance the problem instance - * @see #withInstance(URI) */ public void setInstance(@Nullable URI instance) { this.instance = instance; @@ -219,9 +157,6 @@ public class ProblemDetail { } - - // Getters - /** * Return the configured {@link #setType(URI) problem type}. */ @@ -312,4 +247,14 @@ public class ProblemDetail { return new ProblemDetail(status); } + /** + * Create a {@code ProblemDetail} instance with the given status and detail. + */ + public static ProblemDetail forStatusAndDetail(HttpStatusCode status, String detail) { + Assert.notNull(status, "HttpStatusCode is required"); + ProblemDetail problemDetail = forStatus(status.value()); + problemDetail.setDetail(detail); + return problemDetail; + } + } diff --git a/spring-web/src/main/java/org/springframework/web/HttpRequestMethodNotSupportedException.java b/spring-web/src/main/java/org/springframework/web/HttpRequestMethodNotSupportedException.java index f9abbf683f8..5a28d87d448 100644 --- a/spring-web/src/main/java/org/springframework/web/HttpRequestMethodNotSupportedException.java +++ b/spring-web/src/main/java/org/springframework/web/HttpRequestMethodNotSupportedException.java @@ -96,7 +96,7 @@ public class HttpRequestMethodNotSupportedException extends ServletException imp this.supportedMethods = supportedMethods; String detail = "Method '" + method + "' is not supported."; - this.body = ProblemDetail.forStatus(getStatusCode()).withDetail(detail); + this.body = ProblemDetail.forStatusAndDetail(getStatusCode(), detail); } diff --git a/spring-web/src/main/java/org/springframework/web/bind/MethodArgumentNotValidException.java b/spring-web/src/main/java/org/springframework/web/bind/MethodArgumentNotValidException.java index 00e5db684fc..1f43c7be888 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/MethodArgumentNotValidException.java +++ b/spring-web/src/main/java/org/springframework/web/bind/MethodArgumentNotValidException.java @@ -49,7 +49,7 @@ public class MethodArgumentNotValidException extends BindException implements Er public MethodArgumentNotValidException(MethodParameter parameter, BindingResult bindingResult) { super(bindingResult); this.parameter = parameter; - this.body = ProblemDetail.forStatus(getStatusCode()).withDetail("Invalid request content."); + this.body = ProblemDetail.forStatusAndDetail(getStatusCode(), "Invalid request content."); } @Override diff --git a/spring-web/src/main/java/org/springframework/web/server/MissingRequestValueException.java b/spring-web/src/main/java/org/springframework/web/server/MissingRequestValueException.java index 0f4ffc9ab10..0f4380b19da 100644 --- a/spring-web/src/main/java/org/springframework/web/server/MissingRequestValueException.java +++ b/spring-web/src/main/java/org/springframework/web/server/MissingRequestValueException.java @@ -40,7 +40,7 @@ public class MissingRequestValueException extends ServerWebInputException { this.name = name; this.type = type; this.label = label; - getBody().withDetail(getReason()); + getBody().setDetail(getReason()); } diff --git a/spring-web/src/main/java/org/springframework/web/server/UnsatisfiedRequestParameterException.java b/spring-web/src/main/java/org/springframework/web/server/UnsatisfiedRequestParameterException.java index 077d0bf087b..9ed5be422db 100644 --- a/spring-web/src/main/java/org/springframework/web/server/UnsatisfiedRequestParameterException.java +++ b/spring-web/src/main/java/org/springframework/web/server/UnsatisfiedRequestParameterException.java @@ -42,7 +42,7 @@ public class UnsatisfiedRequestParameterException extends ServerWebInputExceptio super(initReason(conditions, requestParams)); this.conditions = conditions; this.requestParams = requestParams; - getBody().withDetail("Invalid request parameters."); + getBody().setDetail("Invalid request parameters."); } private static String initReason(List conditions, MultiValueMap queryParams) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/NoHandlerFoundException.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/NoHandlerFoundException.java index 3efd0bf9c0e..e7d45c4e2d4 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/NoHandlerFoundException.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/NoHandlerFoundException.java @@ -60,7 +60,7 @@ public class NoHandlerFoundException extends ServletException implements ErrorRe this.httpMethod = httpMethod; this.requestURL = requestURL; this.headers = headers; - this.body = ProblemDetail.forStatus(getStatusCode()).withDetail(getMessage()); + this.body = ProblemDetail.forStatusAndDetail(getStatusCode(), getMessage()); } @Override diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java index 84a3e2d1522..58e7be0df6c 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java @@ -434,7 +434,8 @@ public class RequestResponseBodyMethodProcessorTests { "\"title\":\"Bad Request\"," + "\"status\":400," + "\"detail\":null," + - "\"instance\":\"/path\"}"); + "\"instance\":\"/path\"," + + "\"properties\":null}"); } @Test // SPR-13135