From 2198ebcef3baf3ca06e1bdd88adaf2a4005274ba Mon Sep 17 00:00:00 2001 From: Wim Deblauwe Date: Sun, 12 Apr 2020 17:36:24 +0200 Subject: [PATCH 1/7] This commit replaces the Set based approach for storing the error codes with a List. This allows for duplicates of the error codes to be present. This is important to support validation errors that might have the same error code on multiple fields. The refactoring introduced the ErrorWithArguments class which combines the previous errorCodes field with the arguments field. This refactoring avoids the potential mismatch between what is the errorCodes field and the map's keys. --- .../me/alidg/errors/ErrorWithArguments.java | 98 ++++++++++++ .../me/alidg/errors/HandledException.java | 92 ++++------- .../me/alidg/errors/WebErrorHandlers.java | 6 +- .../handlers/AnnotatedWebErrorHandler.java | 4 +- .../ConstraintViolationWebErrorHandler.java | 44 +----- .../handlers/LastResortWebErrorHandler.java | 5 +- ...ssingRequestParametersWebErrorHandler.java | 4 +- .../handlers/MultipartWebErrorHandler.java | 8 +- .../ResponseStatusWebErrorHandler.java | 29 ++-- .../handlers/ServletWebErrorHandler.java | 38 +++-- .../SpringSecurityWebErrorHandler.java | 21 +-- .../SpringValidationWebErrorHandler.java | 28 +--- .../handlers/TypeMismatchWebErrorHandler.java | 4 +- .../me/alidg/errors/HandledExceptionTest.java | 36 ++--- .../conf/ErrorsAutoConfigurationIT.java | 7 +- .../AnnotatedWebErrorHandlerTest.java | 2 +- ...onstraintViolationWebErrorHandlerTest.java | 5 +- .../LastResortWebErrorHandlerTest.java | 11 +- ...gRequestParametersWebErrorHandlerTest.java | 2 +- .../ResponseStatusWebErrorHandlerTest.java | 2 + .../handlers/ServletWebErrorHandlerTest.java | 2 +- .../SpringSecurityWebErrorHandlerTest.java | 9 +- .../SpringValidationWebErrorHandlerTest.java | 145 ++++++++++++++---- .../TypeMismatchWebErrorHandlerTest.java | 2 +- 24 files changed, 358 insertions(+), 246 deletions(-) create mode 100644 src/main/java/me/alidg/errors/ErrorWithArguments.java diff --git a/src/main/java/me/alidg/errors/ErrorWithArguments.java b/src/main/java/me/alidg/errors/ErrorWithArguments.java new file mode 100644 index 0000000..ba80d0c --- /dev/null +++ b/src/main/java/me/alidg/errors/ErrorWithArguments.java @@ -0,0 +1,98 @@ +package me.alidg.errors; + +import java.util.Collections; +import java.util.List; +import java.util.Objects; + +import static java.util.Collections.emptyList; +import static java.util.Objects.requireNonNull; + +/** + * This object represents an error code with to-be-exposed arguments. + *

+ * For example, suppose + * we have a bean like: + * 

+ *     public class User {
+ *
+ *         @Size(min=1, max=7, message="interests.range_limit")
+ *         private List<String> interests;
+ *         // omitted for the sake of brevity
+ *     }
+ *
+ * If the given interest list wasn't valid, then this object would contain + * {@code interests.range_limit} as the errorCode and {@code List(Argument(min, 1), Argument(max, 7))} + * as the arguments. Later on we can use those exposed values in our message, for example, + * the following error template: + * 
+ *     You should define between {0} and {1} interests.
+ *
+ * Would be translated to: + * 
+ *     You should define between 1 and 7 interests.
+ *
+ */ +public class ErrorWithArguments { + private final String errorCode; + private final List arguments; + + /** + * Constructor + * + * @param errorCode the error code + * @param arguments the arguments to use when interpolating the message of the error code + */ + public ErrorWithArguments(String errorCode, List arguments) { + this.errorCode = errorCode; + this.arguments = arguments == null ? Collections.emptyList() : arguments; + } + + /** + * Factory method when an error code has no arguments. + * + * @param errorCode the error code + * @return a new {@link ErrorWithArguments} instance + */ + public static ErrorWithArguments noArgumentError(String errorCode) { + requireNonNull(errorCode, "The single error code can't be null"); + return new ErrorWithArguments(errorCode, emptyList()); + } + + /** + * The error code that will be looked up in the messages. + * + * @return the error code + */ + public String getErrorCode() { + return errorCode; + } + + /** + * The arguments to use when interpolating the message of the error code. + * + * @return a List of {@link Argument} objects + */ + public List getArguments() { + return arguments; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + ErrorWithArguments that = (ErrorWithArguments) o; + return errorCode.equals(that.errorCode) && + arguments.equals(that.arguments); + } + + @Override + public int hashCode() { + return Objects.hash(errorCode, arguments); + } + + +} diff --git a/src/main/java/me/alidg/errors/HandledException.java b/src/main/java/me/alidg/errors/HandledException.java index d86ab21..7fab7db 100644 --- a/src/main/java/me/alidg/errors/HandledException.java +++ b/src/main/java/me/alidg/errors/HandledException.java @@ -2,22 +2,19 @@ import org.springframework.http.HttpStatus; import org.springframework.lang.NonNull; -import org.springframework.lang.Nullable; -import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; -import static java.util.Collections.singleton; +import static java.util.Collections.singletonList; import static java.util.Objects.requireNonNull; /** * Encapsulates details about a handled exception, including: *
    - *
  • The mapped business level error codes
    • + *
  • The mapped business level error codes and their arguments that can be used for message translation
    • *
  • The corresponding HTTP status code
    • - *
  • A collection of arguments that can be used for message translation
    • *
* * @author Ali Dehghani @@ -29,81 +26,61 @@ public class HandledException { * Collection of error codes corresponding to the handled exception. Usually this collection * contains only one error code but not always, say for validation errors. */ - private final Set errorCodes; + private final List errors; /** * Corresponding status code for the handled exception. */ private final HttpStatus statusCode; - /** - * Collection of to-be-exposed arguments grouped by the error code. This is a mapping - * between the error code and all its to-be-exposed arguments. For example, suppose - * we have a bean like: - * 
-     *     public class User {
-     *
-     *         @Size(min=1, max=7, message="interests.range_limit")
-     *         private List<String> interests;
-     *         // omitted for the sake of brevity
-     *     }
-     *
- * If the given interest list wasn't valid, then this map would contain an entry with the - * {@code interests.range_limit} as the key and {@code List(Argument(min, 1), Argument(max, 7))} - * as the values. Later on we can use those exposed values in our message, for example, - * the following error template: - * 
-     *     You should define between {0} and {1} interests.
-     *
- * Would be translated to: - * 
-     *     You should define between 1 and 7 interests.
-     *
- */ - private final Map> arguments; - /** * Initialize a handled exception with a set of error codes, a HTTP status code and an * optional collection of arguments. * - * @param errorCodes The corresponding error codes for the handled exception. + * @param errors The corresponding error codes for the handled exception. * @param statusCode The corresponding status code for the handled exception. - * @param arguments Arguments to be exposed from the handled exception to the outside world. * @throws NullPointerException When one of the required parameters is null. * @throws IllegalArgumentException At least one error code should be provided. */ - public HandledException(@NonNull Set errorCodes, - @NonNull HttpStatus statusCode, - @Nullable Map> arguments) { - enforcePreconditions(errorCodes, statusCode); - this.errorCodes = errorCodes; + public HandledException(@NonNull List errors, + @NonNull HttpStatus statusCode) { + enforcePreconditions(errors, statusCode); + this.errors = errors; this.statusCode = statusCode; - this.arguments = arguments == null ? Collections.emptyMap() : arguments; } /** * Initialize a handled exception with an error code, a HTTP status code and an * optional collection of arguments. * - * @param errorCode The corresponding error code for the handled exception. + * @param error The corresponding error code for the handled exception. * @param statusCode The corresponding status code for the handled exception. - * @param arguments Arguments to be exposed from the handled exception to the outside world. * @throws NullPointerException When one of the required parameters is null. * @throws IllegalArgumentException At least one error code should be provided. */ - public HandledException(@NonNull String errorCode, - @NonNull HttpStatus statusCode, - @Nullable Map> arguments) { - this(singleton(errorCode), statusCode, arguments); + public HandledException(@NonNull ErrorWithArguments error, + @NonNull HttpStatus statusCode) { + this(singletonList(error), statusCode); + } + + /** + * + * @return Collection of errors + */ + @NonNull + public List getErrors() { + return errors; } /** * @return Collection of mapped error codes. - * @see #errorCodes + * @see #errors */ @NonNull public Set getErrorCodes() { - return errorCodes; + return errors.stream() + .map(ErrorWithArguments::getErrorCode) + .collect(Collectors.toSet()); } /** @@ -115,23 +92,16 @@ public HttpStatus getStatusCode() { return statusCode; } - /** - * @return Collection of to-be-exposed arguments. - * @see #arguments - */ - @NonNull - public Map> getArguments() { - return arguments; - } - - private void enforcePreconditions(Set errorCodes, HttpStatus statusCode) { + private void enforcePreconditions(List errorCodes, HttpStatus statusCode) { requireNonNull(errorCodes, "Error codes is required"); requireNonNull(statusCode, "Status code is required"); - if (errorCodes.isEmpty()) + if (errorCodes.isEmpty()) { throw new IllegalArgumentException("At least one error code should be provided"); + } - if (errorCodes.size() == 1 && errorCodes.contains(null)) + if (errorCodes.size() == 1 && errorCodes.contains(null)) { throw new NullPointerException("The single error code can't be null"); + } } } diff --git a/src/main/java/me/alidg/errors/WebErrorHandlers.java b/src/main/java/me/alidg/errors/WebErrorHandlers.java index 14403b3..550bcb4 100644 --- a/src/main/java/me/alidg/errors/WebErrorHandlers.java +++ b/src/main/java/me/alidg/errors/WebErrorHandlers.java @@ -233,9 +233,9 @@ private Throwable refineIfNeeded(Throwable exception) { private List translateErrors(HandledException handled, Locale locale) { return handled - .getErrorCodes() + .getErrors() .stream() - .map(code -> withMessage(code, getArgumentsFor(handled, code), locale)) + .map(errorWithArguments -> withMessage(errorWithArguments.getErrorCode(), errorWithArguments.getArguments(), locale)) .collect(toList()); } @@ -265,7 +265,9 @@ private String className(Object toInspect) { return toInspect.getClass().getName(); } +/* private List getArgumentsFor(HandledException handled, String errorCode) { return handled.getArguments().getOrDefault(errorCode, emptyList()); } +*/ } diff --git a/src/main/java/me/alidg/errors/handlers/AnnotatedWebErrorHandler.java b/src/main/java/me/alidg/errors/handlers/AnnotatedWebErrorHandler.java index e777768..c2e64c6 100644 --- a/src/main/java/me/alidg/errors/handlers/AnnotatedWebErrorHandler.java +++ b/src/main/java/me/alidg/errors/handlers/AnnotatedWebErrorHandler.java @@ -1,6 +1,7 @@ package me.alidg.errors.handlers; import me.alidg.errors.Argument; +import me.alidg.errors.ErrorWithArguments; import me.alidg.errors.HandledException; import me.alidg.errors.WebErrorHandler; import me.alidg.errors.annotation.ExceptionMapping; @@ -18,7 +19,6 @@ import java.util.Objects; import java.util.stream.Stream; -import static java.util.Collections.singletonMap; import static java.util.stream.Collectors.toList; import static me.alidg.errors.Argument.arg; @@ -72,7 +72,7 @@ public HandledException handle(Throwable exception) { HttpStatus httpStatus = exceptionMapping.statusCode(); List arguments = getExposedValues(exception); - return new HandledException(errorCode, httpStatus, singletonMap(errorCode, arguments)); + return new HandledException(new ErrorWithArguments(errorCode, arguments), httpStatus); } /** diff --git a/src/main/java/me/alidg/errors/handlers/ConstraintViolationWebErrorHandler.java b/src/main/java/me/alidg/errors/handlers/ConstraintViolationWebErrorHandler.java index 4170b8a..a012be6 100644 --- a/src/main/java/me/alidg/errors/handlers/ConstraintViolationWebErrorHandler.java +++ b/src/main/java/me/alidg/errors/handlers/ConstraintViolationWebErrorHandler.java @@ -1,6 +1,6 @@ package me.alidg.errors.handlers; -import me.alidg.errors.Argument; +import me.alidg.errors.ErrorWithArguments; import me.alidg.errors.HandledException; import me.alidg.errors.WebErrorHandler; import org.springframework.http.HttpStatus; @@ -9,11 +9,9 @@ import javax.validation.ConstraintViolation; import javax.validation.ConstraintViolationException; import java.util.List; -import java.util.Map; import java.util.Set; -import static java.util.stream.Collectors.toMap; -import static java.util.stream.Collectors.toSet; +import static java.util.stream.Collectors.*; /** * A {@link WebErrorHandler} implementation responsible for handling {@link ConstraintViolationException}s @@ -54,10 +52,8 @@ public boolean canHandle(Throwable exception) { @Override public HandledException handle(Throwable exception) { ConstraintViolationException violationException = (ConstraintViolationException) exception; - Set errorCodes = extractErrorCodes(violationException); - Map> arguments = extractArguments(violationException); - return new HandledException(errorCodes, HttpStatus.BAD_REQUEST, arguments); + return new HandledException(extractErrorWithArguments(violationException), HttpStatus.BAD_REQUEST); } /** @@ -72,36 +68,12 @@ private boolean hasAtLeastOneViolation(Throwable exception) { return violations != null && !violations.isEmpty(); } - /** - * Extract annotation attributes (except for those three mandatory attributes) and expose them as arguments. - * - * @param violationException The exception to extract the arguments from. - * @return To-be-exposed arguments. - */ - private Map> extractArguments(ConstraintViolationException violationException) { - Map> args = violationException - .getConstraintViolations() - .stream() - .collect(toMap(ConstraintViolations::getErrorCode, ConstraintViolations::getArguments, (v1, v2) -> v1)); - args.entrySet().removeIf(e -> e.getValue().isEmpty()); - return args; - } - - /** - * Extract message templates and use them as error codes. - * - * @param violationException The exception to extract the error codes from. - * @return A set of error codes. - */ - private Set extractErrorCodes(ConstraintViolationException violationException) { - return violationException + private List extractErrorWithArguments(ConstraintViolationException exception) { + return exception .getConstraintViolations() .stream() - .map(this::errorCode) - .collect(toSet()); - } - - private String errorCode(ConstraintViolation violation) { - return violation.getMessageTemplate().replace("{", "").replace("}", ""); + .map(constraintViolation -> new ErrorWithArguments(ConstraintViolations.getErrorCode(constraintViolation), + ConstraintViolations.getArguments(constraintViolation))) + .collect(toList()); } } diff --git a/src/main/java/me/alidg/errors/handlers/LastResortWebErrorHandler.java b/src/main/java/me/alidg/errors/handlers/LastResortWebErrorHandler.java index 1744345..1d33951 100644 --- a/src/main/java/me/alidg/errors/handlers/LastResortWebErrorHandler.java +++ b/src/main/java/me/alidg/errors/handlers/LastResortWebErrorHandler.java @@ -1,11 +1,10 @@ package me.alidg.errors.handlers; +import me.alidg.errors.ErrorWithArguments; import me.alidg.errors.HandledException; import me.alidg.errors.WebErrorHandler; import org.springframework.http.HttpStatus; -import java.util.Collections; - /** * The default fallback {@link WebErrorHandler} which will be used when all * other registered handlers refuse to handle a particular exception. You can @@ -46,6 +45,6 @@ public boolean canHandle(Throwable exception) { */ @Override public HandledException handle(Throwable exception) { - return new HandledException(UNKNOWN_ERROR_CODE, HttpStatus.INTERNAL_SERVER_ERROR, Collections.emptyMap()); + return new HandledException(ErrorWithArguments.noArgumentError(UNKNOWN_ERROR_CODE), HttpStatus.INTERNAL_SERVER_ERROR); } } diff --git a/src/main/java/me/alidg/errors/handlers/MissingRequestParametersWebErrorHandler.java b/src/main/java/me/alidg/errors/handlers/MissingRequestParametersWebErrorHandler.java index e1cc403..f03aff2 100644 --- a/src/main/java/me/alidg/errors/handlers/MissingRequestParametersWebErrorHandler.java +++ b/src/main/java/me/alidg/errors/handlers/MissingRequestParametersWebErrorHandler.java @@ -1,6 +1,7 @@ package me.alidg.errors.handlers; import me.alidg.errors.Argument; +import me.alidg.errors.ErrorWithArguments; import me.alidg.errors.HandledException; import me.alidg.errors.WebErrorHandler; import org.springframework.core.MethodParameter; @@ -12,7 +13,6 @@ import java.util.ArrayList; import java.util.List; -import static java.util.Collections.singletonMap; import static me.alidg.errors.Argument.arg; import static org.springframework.http.HttpStatus.BAD_REQUEST; @@ -91,7 +91,7 @@ public HandledException handle(Throwable exception) { errorCode = MISSING_MATRIX_VARIABLE; } - return new HandledException(errorCode, BAD_REQUEST, singletonMap(errorCode, arguments)); + return new HandledException(new ErrorWithArguments(errorCode, arguments), BAD_REQUEST); } private String getType(MethodParameter parameter) { diff --git a/src/main/java/me/alidg/errors/handlers/MultipartWebErrorHandler.java b/src/main/java/me/alidg/errors/handlers/MultipartWebErrorHandler.java index 4dc4552..51ba58a 100644 --- a/src/main/java/me/alidg/errors/handlers/MultipartWebErrorHandler.java +++ b/src/main/java/me/alidg/errors/handlers/MultipartWebErrorHandler.java @@ -1,6 +1,7 @@ package me.alidg.errors.handlers; import me.alidg.errors.Argument; +import me.alidg.errors.ErrorWithArguments; import me.alidg.errors.HandledException; import me.alidg.errors.WebErrorHandler; import org.springframework.lang.NonNull; @@ -8,7 +9,6 @@ import org.springframework.web.multipart.MultipartException; import java.util.List; -import java.util.Map; import static java.util.Collections.*; import static me.alidg.errors.Argument.arg; @@ -47,14 +47,14 @@ public boolean canHandle(Throwable exception) { @Override public HandledException handle(Throwable exception) { String errorCode = MULTIPART_EXPECTED; - Map> arguments = emptyMap(); + List arguments = emptyList(); if (exception instanceof MaxUploadSizeExceededException) { long maxSize = ((MaxUploadSizeExceededException) exception).getMaxUploadSize(); errorCode = MAX_SIZE; - arguments = singletonMap(MAX_SIZE, singletonList(arg("max_size", maxSize))); + arguments = singletonList(arg("max_size", maxSize)); } - return new HandledException(errorCode, BAD_REQUEST, arguments); + return new HandledException(new ErrorWithArguments(errorCode, arguments), BAD_REQUEST); } } diff --git a/src/main/java/me/alidg/errors/handlers/ResponseStatusWebErrorHandler.java b/src/main/java/me/alidg/errors/handlers/ResponseStatusWebErrorHandler.java index 982a196..79c7025 100644 --- a/src/main/java/me/alidg/errors/handlers/ResponseStatusWebErrorHandler.java +++ b/src/main/java/me/alidg/errors/handlers/ResponseStatusWebErrorHandler.java @@ -1,6 +1,7 @@ package me.alidg.errors.handlers; import me.alidg.errors.Argument; +import me.alidg.errors.ErrorWithArguments; import me.alidg.errors.HandledException; import me.alidg.errors.WebErrorHandler; import org.springframework.beans.TypeMismatchException; @@ -66,25 +67,27 @@ public boolean canHandle(Throwable exception) { public HandledException handle(Throwable exception) { if (exception instanceof MediaTypeNotSupportedStatusException) { Set types = getMediaTypes(((MediaTypeNotSupportedStatusException) exception).getSupportedMediaTypes()); - Map> args = types.isEmpty() ? emptyMap() : argMap(NOT_SUPPORTED, arg("types", types)); - return new HandledException(NOT_SUPPORTED, UNSUPPORTED_MEDIA_TYPE, args); + List args = types.isEmpty() ? emptyList() : singletonList(arg("types", types)); + return new HandledException(new ErrorWithArguments(NOT_SUPPORTED, args), + UNSUPPORTED_MEDIA_TYPE); } if (exception instanceof UnsupportedMediaTypeStatusException) { Set types = getMediaTypes(((UnsupportedMediaTypeStatusException) exception).getSupportedMediaTypes()); - Map> args = types.isEmpty() ? emptyMap() : argMap(NOT_SUPPORTED, arg("types", types)); - return new HandledException(NOT_SUPPORTED, UNSUPPORTED_MEDIA_TYPE, args); + List args = types.isEmpty() ? emptyList() : singletonList(arg("types", types)); + return new HandledException(new ErrorWithArguments(NOT_SUPPORTED, args), UNSUPPORTED_MEDIA_TYPE); } if (exception instanceof NotAcceptableStatusException) { Set types = getMediaTypes(((NotAcceptableStatusException) exception).getSupportedMediaTypes()); - Map> args = types.isEmpty() ? emptyMap() : argMap(NOT_ACCEPTABLE, arg("types", types)); - return new HandledException(NOT_ACCEPTABLE, HttpStatus.NOT_ACCEPTABLE, args); + List args = types.isEmpty() ? emptyList() : singletonList(arg("types", types)); + return new HandledException(new ErrorWithArguments(NOT_ACCEPTABLE, args), HttpStatus.NOT_ACCEPTABLE); } if (exception instanceof MethodNotAllowedException) { String httpMethod = ((MethodNotAllowedException) exception).getHttpMethod(); - return new HandledException(METHOD_NOT_ALLOWED, HttpStatus.METHOD_NOT_ALLOWED, argMap(METHOD_NOT_ALLOWED, arg("method", httpMethod))); + return new HandledException(new ErrorWithArguments(METHOD_NOT_ALLOWED, singletonList(arg("method", httpMethod))), + HttpStatus.METHOD_NOT_ALLOWED); } if (exception instanceof WebExchangeBindException) { @@ -103,17 +106,17 @@ public HandledException handle(Throwable exception) { HandledException handledException = handleMissingParameters(parameter); if (handledException != null) return handledException; - return new HandledException(INVALID_OR_MISSING_BODY, BAD_REQUEST, null); + return new HandledException(ErrorWithArguments.noArgumentError(INVALID_OR_MISSING_BODY), BAD_REQUEST); } if (exception instanceof ResponseStatusException) { HttpStatus status = ((ResponseStatusException) exception).getStatus(); - if (status == NOT_FOUND) return new HandledException(NO_HANDLER, status, null); + if (status == NOT_FOUND) return new HandledException(ErrorWithArguments.noArgumentError(NO_HANDLER), status); - return new HandledException(UNKNOWN_ERROR_CODE, status, null); + return new HandledException(ErrorWithArguments.noArgumentError(UNKNOWN_ERROR_CODE), status); } - return new HandledException(UNKNOWN_ERROR_CODE, INTERNAL_SERVER_ERROR, null); + return new HandledException(ErrorWithArguments.noArgumentError(UNKNOWN_ERROR_CODE), INTERNAL_SERVER_ERROR); } /** @@ -164,8 +167,8 @@ private HandledException handleMissingParameters(MethodParameter parameter) { } if (code != null) { - return new HandledException(code, BAD_REQUEST, - argMap(code, arg("name", parameterName), arg("expected", Classes.getClassName(parameter.getParameterType())))); + List arguments = asList(arg("name", parameterName), arg("expected", Classes.getClassName(parameter.getParameterType()))); + return new HandledException(new ErrorWithArguments(code, arguments), BAD_REQUEST); } return null; diff --git a/src/main/java/me/alidg/errors/handlers/ServletWebErrorHandler.java b/src/main/java/me/alidg/errors/handlers/ServletWebErrorHandler.java index a71d28b..649eab0 100644 --- a/src/main/java/me/alidg/errors/handlers/ServletWebErrorHandler.java +++ b/src/main/java/me/alidg/errors/handlers/ServletWebErrorHandler.java @@ -1,6 +1,7 @@ package me.alidg.errors.handlers; import me.alidg.errors.Argument; +import me.alidg.errors.ErrorWithArguments; import me.alidg.errors.HandledException; import me.alidg.errors.WebErrorHandler; import org.springframework.http.HttpStatus; @@ -16,7 +17,6 @@ import javax.servlet.ServletException; import java.util.List; -import java.util.Map; import java.util.Set; import static java.util.Arrays.asList; @@ -97,14 +97,15 @@ public boolean canHandle(Throwable exception) { @Override public HandledException handle(Throwable exception) { if (exception instanceof HttpMessageNotReadableException) - return new HandledException(INVALID_OR_MISSING_BODY, HttpStatus.BAD_REQUEST, null); + return new HandledException(ErrorWithArguments.noArgumentError(INVALID_OR_MISSING_BODY), HttpStatus.BAD_REQUEST); if (exception instanceof HttpMediaTypeNotAcceptableException) { Set types = getMediaTypes(((HttpMediaTypeNotAcceptableException) exception).getSupportedMediaTypes()); - Map> args = types.isEmpty() ? - emptyMap() : singletonMap(NOT_ACCEPTABLE, singletonList(arg("types", types))); + List args = types.isEmpty() ? + emptyList() : singletonList(arg("types", types)); - return new HandledException(NOT_ACCEPTABLE, HttpStatus.NOT_ACCEPTABLE, args); + return new HandledException(new ErrorWithArguments(NOT_ACCEPTABLE, args), + HttpStatus.NOT_ACCEPTABLE); } if (exception instanceof HttpMediaTypeNotSupportedException) { @@ -112,16 +113,15 @@ public HandledException handle(Throwable exception) { List arguments = null; if (contentType != null) arguments = singletonList(arg("type", contentType.toString())); - return new HandledException(NOT_SUPPORTED, HttpStatus.UNSUPPORTED_MEDIA_TYPE, - arguments == null ? emptyMap() : singletonMap(NOT_SUPPORTED, arguments)); + return new HandledException(new ErrorWithArguments(NOT_SUPPORTED, arguments), + HttpStatus.UNSUPPORTED_MEDIA_TYPE); } if (exception instanceof HttpRequestMethodNotSupportedException) { String method = ((HttpRequestMethodNotSupportedException) exception).getMethod(); - return new HandledException(METHOD_NOT_ALLOWED, HttpStatus.METHOD_NOT_ALLOWED, - singletonMap(METHOD_NOT_ALLOWED, singletonList(arg("method", method))) - ); + return new HandledException(new ErrorWithArguments(METHOD_NOT_ALLOWED, singletonList(arg("method", method))), + HttpStatus.METHOD_NOT_ALLOWED); } if (exception instanceof MissingServletRequestParameterException) { @@ -129,28 +129,26 @@ public HandledException handle(Throwable exception) { String name = actualException.getParameterName(); String type = actualException.getParameterType(); - return new HandledException(MISSING_PARAMETER, HttpStatus.BAD_REQUEST, - singletonMap(MISSING_PARAMETER, asList(arg("name", name), arg("expected", type))) - ); + return new HandledException(new ErrorWithArguments(MISSING_PARAMETER, + asList(arg("name", name), arg("expected", type))), + HttpStatus.BAD_REQUEST); } if (exception instanceof MissingServletRequestPartException) { String name = ((MissingServletRequestPartException) exception).getRequestPartName(); - return new HandledException(MISSING_PART, HttpStatus.BAD_REQUEST, - singletonMap(MISSING_PART, singletonList(arg("name", name))) - ); + return new HandledException(new ErrorWithArguments(MISSING_PART, singletonList(arg("name", name))), + HttpStatus.BAD_REQUEST); } if (exception instanceof NoHandlerFoundException) { String url = ((NoHandlerFoundException) exception).getRequestURL(); - return new HandledException(NO_HANDLER, HttpStatus.NOT_FOUND, - singletonMap(NO_HANDLER, singletonList(arg("path", url))) - ); + return new HandledException(new ErrorWithArguments(NO_HANDLER, singletonList(arg("path", url))), + HttpStatus.NOT_FOUND); } - return new HandledException("unknown_error", HttpStatus.INTERNAL_SERVER_ERROR, null); + return new HandledException(ErrorWithArguments.noArgumentError("unknown_error"), HttpStatus.INTERNAL_SERVER_ERROR); } private Set getMediaTypes(List mediaTypes) { diff --git a/src/main/java/me/alidg/errors/handlers/SpringSecurityWebErrorHandler.java b/src/main/java/me/alidg/errors/handlers/SpringSecurityWebErrorHandler.java index 94750c4..6120217 100644 --- a/src/main/java/me/alidg/errors/handlers/SpringSecurityWebErrorHandler.java +++ b/src/main/java/me/alidg/errors/handlers/SpringSecurityWebErrorHandler.java @@ -1,5 +1,6 @@ package me.alidg.errors.handlers; +import me.alidg.errors.ErrorWithArguments; import me.alidg.errors.HandledException; import me.alidg.errors.WebErrorHandler; import org.springframework.lang.NonNull; @@ -82,29 +83,29 @@ public boolean canHandle(Throwable exception) { @Override public HandledException handle(Throwable exception) { if (exception instanceof AccessDeniedException) - return new HandledException(ACCESS_DENIED, FORBIDDEN, null); + return new HandledException(ErrorWithArguments.noArgumentError(ACCESS_DENIED), FORBIDDEN); if (exception instanceof AccountExpiredException) - return new HandledException(ACCOUNT_EXPIRED, BAD_REQUEST, null); + return new HandledException(ErrorWithArguments.noArgumentError(ACCOUNT_EXPIRED), BAD_REQUEST); if (exception instanceof AuthenticationCredentialsNotFoundException) - return new HandledException(AUTH_REQUIRED, UNAUTHORIZED, null); + return new HandledException(ErrorWithArguments.noArgumentError(AUTH_REQUIRED), UNAUTHORIZED); if (exception instanceof AuthenticationServiceException) - return new HandledException(INTERNAL_ERROR, INTERNAL_SERVER_ERROR, null); + return new HandledException(ErrorWithArguments.noArgumentError(INTERNAL_ERROR), INTERNAL_SERVER_ERROR); if (exception instanceof BadCredentialsException) - return new HandledException(BAD_CREDENTIALS, BAD_REQUEST, null); + return new HandledException(ErrorWithArguments.noArgumentError(BAD_CREDENTIALS), BAD_REQUEST); if (exception instanceof UsernameNotFoundException) - return new HandledException(BAD_CREDENTIALS, BAD_REQUEST, null); + return new HandledException(ErrorWithArguments.noArgumentError(BAD_CREDENTIALS), BAD_REQUEST); if (exception instanceof InsufficientAuthenticationException) - return new HandledException(AUTH_REQUIRED, UNAUTHORIZED, null); + return new HandledException(ErrorWithArguments.noArgumentError(AUTH_REQUIRED), UNAUTHORIZED); - if (exception instanceof LockedException) return new HandledException(USER_LOCKED, BAD_REQUEST, null); - if (exception instanceof DisabledException) return new HandledException(USER_DISABLED, BAD_REQUEST, null); + if (exception instanceof LockedException) return new HandledException(ErrorWithArguments.noArgumentError(USER_LOCKED), BAD_REQUEST); + if (exception instanceof DisabledException) return new HandledException(ErrorWithArguments.noArgumentError(USER_DISABLED), BAD_REQUEST); - return new HandledException("unknown_error", INTERNAL_SERVER_ERROR, null); + return new HandledException(ErrorWithArguments.noArgumentError("unknown_error"), INTERNAL_SERVER_ERROR); } } diff --git a/src/main/java/me/alidg/errors/handlers/SpringValidationWebErrorHandler.java b/src/main/java/me/alidg/errors/handlers/SpringValidationWebErrorHandler.java index d4aa5a6..73aebea 100644 --- a/src/main/java/me/alidg/errors/handlers/SpringValidationWebErrorHandler.java +++ b/src/main/java/me/alidg/errors/handlers/SpringValidationWebErrorHandler.java @@ -1,6 +1,7 @@ package me.alidg.errors.handlers; import me.alidg.errors.Argument; +import me.alidg.errors.ErrorWithArguments; import me.alidg.errors.HandledException; import me.alidg.errors.WebErrorHandler; import org.springframework.beans.TypeMismatchException; @@ -13,10 +14,9 @@ import javax.validation.ConstraintViolation; import java.util.List; -import java.util.Map; +import java.util.stream.Collectors; import static java.util.Collections.emptyList; -import static java.util.stream.Collectors.collectingAndThen; import static java.util.stream.Collectors.toMap; /** @@ -58,12 +58,12 @@ public boolean canHandle(Throwable exception) { @Override public HandledException handle(Throwable exception) { BindingResult bindingResult = getBindingResult(exception); - return bindingResult.getAllErrors() - .stream() - .collect(collectingAndThen( - toMap(this::errorCode, this::arguments, (value1, value2) -> value1), - m -> new HandledException(m.keySet(), HttpStatus.BAD_REQUEST, dropEmptyValues(m)) - )); + List errorWithArguments = bindingResult.getAllErrors() + .stream() + .map(objectError -> new ErrorWithArguments(errorCode(objectError), + arguments(objectError))) + .collect(Collectors.toList()); + return new HandledException(errorWithArguments, HttpStatus.BAD_REQUEST); } /** @@ -126,16 +126,4 @@ private List arguments(ObjectError error) { return emptyList(); } - - /** - * Drops the empty collection of arguments! - * - * @param input The error code to arguments map. - * @return The filtered map. - */ - private Map> dropEmptyValues(Map> input) { - return input.entrySet().stream() - .filter(e -> e.getValue() != null && !e.getValue().isEmpty()) - .collect(toMap(Map.Entry::getKey, Map.Entry::getValue, (v1, v2) -> v2)); - } } diff --git a/src/main/java/me/alidg/errors/handlers/TypeMismatchWebErrorHandler.java b/src/main/java/me/alidg/errors/handlers/TypeMismatchWebErrorHandler.java index eae1e60..28a2363 100644 --- a/src/main/java/me/alidg/errors/handlers/TypeMismatchWebErrorHandler.java +++ b/src/main/java/me/alidg/errors/handlers/TypeMismatchWebErrorHandler.java @@ -1,6 +1,7 @@ package me.alidg.errors.handlers; import me.alidg.errors.Argument; +import me.alidg.errors.ErrorWithArguments; import me.alidg.errors.HandledException; import me.alidg.errors.WebErrorHandler; import org.springframework.beans.TypeMismatchException; @@ -10,7 +11,6 @@ import java.util.ArrayList; import java.util.List; -import static java.util.Collections.singletonMap; import static me.alidg.errors.Argument.arg; import static org.springframework.http.HttpStatus.BAD_REQUEST; @@ -51,7 +51,7 @@ public HandledException handle(Throwable exception) { String errorCode = getErrorCode(mismatchException); List arguments = getArguments(mismatchException); - return new HandledException(errorCode, BAD_REQUEST, singletonMap(errorCode, arguments)); + return new HandledException(new ErrorWithArguments(errorCode, arguments), BAD_REQUEST); } static List getArguments(TypeMismatchException mismatchException) { diff --git a/src/test/java/me/alidg/errors/HandledExceptionTest.java b/src/test/java/me/alidg/errors/HandledExceptionTest.java index fa85a49..e6db2c9 100644 --- a/src/test/java/me/alidg/errors/HandledExceptionTest.java +++ b/src/test/java/me/alidg/errors/HandledExceptionTest.java @@ -6,10 +6,7 @@ import org.junit.runner.RunWith; import org.springframework.http.HttpStatus; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; import static java.util.Arrays.asList; import static java.util.Collections.*; @@ -28,11 +25,11 @@ public class HandledExceptionTest { @Test @Parameters(method = "provideParamsForPrimary") - public void primaryConstructor_ShouldEnforceItsPreconditions(Set errorCodes, + public void primaryConstructor_ShouldEnforceItsPreconditions(List errors, HttpStatus status, Class expected, String message) { - assertThatThrownBy(() -> new HandledException(errorCodes, status, singletonMap("error", emptyList()))) + assertThatThrownBy(() -> new HandledException(errors, status)) .isInstanceOf(expected) .hasMessage(message); } @@ -43,43 +40,32 @@ public void secondConstructor_ShouldEnforceItsPreconditions(String errorCode, HttpStatus status, Class expected, String message) { - assertThatThrownBy(() -> new HandledException(errorCode, status, singletonMap("error", emptyList()))) + assertThatThrownBy(() -> new HandledException(ErrorWithArguments.noArgumentError(errorCode), status)) .isInstanceOf(expected) .hasMessage(message); } @Test - @Parameters(method = "provideMaps") - public void constructors_ShouldSetNullArgumentsAsEmptyMaps(Map> provided, - Map expected) { - assertThat(new HandledException(singleton("error"), BAD_REQUEST, provided).getArguments()) - .isEqualTo(expected); + public void constructors_ShouldSetNullArgumentsAsEmptyMaps() { + // TODO check arguments assertThat(new HandledException(new HandledException.ErrorWithArguments("error", null), BAD_REQUEST).getArguments()) + // .isEqualTo(Collections.emptyList()); - assertThat(new HandledException("error", BAD_REQUEST, provided).getArguments()) - .isEqualTo(expected); } private Object[] provideParamsForPrimary() { return p( p(null, null, NullPointerException.class, "Error codes is required"), - p(new HashSet<>(asList("", "", null)), null, NullPointerException.class, "Status code is required"), - p(singleton(null), BAD_REQUEST, NullPointerException.class, "The single error code can't be null"), - p(emptySet(), BAD_REQUEST, IllegalArgumentException.class, "At least one error code should be provided") + p(asList(ErrorWithArguments.noArgumentError(""), ErrorWithArguments.noArgumentError(""), null), null, NullPointerException.class, "Status code is required"), + p(singletonList(null), BAD_REQUEST, NullPointerException.class, "The single error code can't be null"), + p(emptyList(), BAD_REQUEST, IllegalArgumentException.class, "At least one error code should be provided") ); } private Object[] provideParamsForSecondary() { return p( - p(null, null, NullPointerException.class, "Status code is required"), + p(null, null, NullPointerException.class, "The single error code can't be null"), p("error", null, NullPointerException.class, "Status code is required"), p(null, BAD_REQUEST, NullPointerException.class, "The single error code can't be null") ); } - - private Object[] provideMaps() { - return p( - p(null, emptyMap()), - p(singletonMap("key", emptyList()), singletonMap("key", emptyList())) - ); - } } diff --git a/src/test/java/me/alidg/errors/conf/ErrorsAutoConfigurationIT.java b/src/test/java/me/alidg/errors/conf/ErrorsAutoConfigurationIT.java index c541add..6280714 100644 --- a/src/test/java/me/alidg/errors/conf/ErrorsAutoConfigurationIT.java +++ b/src/test/java/me/alidg/errors/conf/ErrorsAutoConfigurationIT.java @@ -2,6 +2,7 @@ import junitparams.JUnitParamsRunner; import junitparams.Parameters; +import me.alidg.errors.ErrorWithArguments; import me.alidg.errors.HandledException; import me.alidg.errors.WebErrorHandler; import me.alidg.errors.WebErrorHandlers; @@ -294,7 +295,7 @@ public boolean canHandle(Throwable exception) { @Override @NonNull public HandledException handle(Throwable exception) { - return new HandledException("", HttpStatus.BAD_REQUEST, null); + return new HandledException(ErrorWithArguments.noArgumentError(""), HttpStatus.BAD_REQUEST); } } @@ -308,7 +309,7 @@ public boolean canHandle(Throwable exception) { @Override @NonNull public HandledException handle(Throwable exception) { - return new HandledException("", HttpStatus.BAD_REQUEST, null); + return new HandledException(ErrorWithArguments.noArgumentError(""), HttpStatus.BAD_REQUEST); } } @@ -322,7 +323,7 @@ public boolean canHandle(Throwable exception) { @Override @NonNull public HandledException handle(Throwable exception) { - return new HandledException("", HttpStatus.BAD_REQUEST, null); + return new HandledException(ErrorWithArguments.noArgumentError(""), HttpStatus.BAD_REQUEST); } } } diff --git a/src/test/java/me/alidg/errors/handlers/AnnotatedWebErrorHandlerTest.java b/src/test/java/me/alidg/errors/handlers/AnnotatedWebErrorHandlerTest.java index a532df5..d86b976 100644 --- a/src/test/java/me/alidg/errors/handlers/AnnotatedWebErrorHandlerTest.java +++ b/src/test/java/me/alidg/errors/handlers/AnnotatedWebErrorHandlerTest.java @@ -52,7 +52,7 @@ public void handle_ShouldProperlyHandleTheGivenException(Exception exception, assertThat(handled.getErrorCodes()).hasSize(1); assertThat(handled.getErrorCodes()).containsExactly(code); assertThat(handled.getStatusCode()).isEqualTo(status); - assertThat((handled.getArguments().get(code))).isEqualTo(args); + // TODO check arguments assertThat((handled.getArguments().get(code))).isEqualTo(args); } private Object[] provideParamsForCanHandle() { diff --git a/src/test/java/me/alidg/errors/handlers/ConstraintViolationWebErrorHandlerTest.java b/src/test/java/me/alidg/errors/handlers/ConstraintViolationWebErrorHandlerTest.java index 6ff5c60..eddde61 100644 --- a/src/test/java/me/alidg/errors/handlers/ConstraintViolationWebErrorHandlerTest.java +++ b/src/test/java/me/alidg/errors/handlers/ConstraintViolationWebErrorHandlerTest.java @@ -21,8 +21,7 @@ import java.util.stream.Collectors; import static java.util.Arrays.asList; -import static java.util.Collections.singleton; -import static java.util.Collections.singletonMap; +import static java.util.Collections.*; import static me.alidg.Params.p; import static me.alidg.errors.Argument.arg; import static org.assertj.core.api.Assertions.assertThat; @@ -62,7 +61,7 @@ public void handle_ShouldHandleViolationExceptionsProperly(ConstraintViolationEx assertThat(handledException.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); assertThat(handledException.getErrorCodes()).containsAll(errorCodes); - assertThat(handledException.getArguments()).containsAllEntriesOf(arguments); + // TODO check arguments } @SuppressWarnings("unchecked") diff --git a/src/test/java/me/alidg/errors/handlers/LastResortWebErrorHandlerTest.java b/src/test/java/me/alidg/errors/handlers/LastResortWebErrorHandlerTest.java index 9d4edc5..9cace99 100644 --- a/src/test/java/me/alidg/errors/handlers/LastResortWebErrorHandlerTest.java +++ b/src/test/java/me/alidg/errors/handlers/LastResortWebErrorHandlerTest.java @@ -2,17 +2,19 @@ import junitparams.JUnitParamsRunner; import junitparams.Parameters; +import me.alidg.errors.ErrorWithArguments; import me.alidg.errors.HandledException; import me.alidg.errors.WebErrorHandler; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.http.HttpStatus; -import static java.util.Collections.singleton; +import static java.util.Collections.emptyList; import static me.alidg.Params.p; import static me.alidg.errors.handlers.LastResortWebErrorHandler.INSTANCE; import static me.alidg.errors.handlers.LastResortWebErrorHandler.UNKNOWN_ERROR_CODE; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.tuple; /** * Unit tests for {@link LastResortWebErrorHandler} exception handler. @@ -39,9 +41,12 @@ public void canHandle_AlwaysReturnsFalse(Throwable exception) { public void handle_AlwaysReturn500InternalErrorWithStaticErrorCode(Throwable exception) { HandledException handled = handler.handle(exception); - assertThat(handled.getErrorCodes()).isEqualTo(singleton(UNKNOWN_ERROR_CODE)); assertThat(handled.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); - assertThat(handled.getArguments()).isEmpty(); + assertThat(handled.getErrors()).extracting(ErrorWithArguments::getErrorCode, + ErrorWithArguments::getArguments) + .containsOnly(tuple(UNKNOWN_ERROR_CODE, + emptyList())); + } private Object[] provideParams() { diff --git a/src/test/java/me/alidg/errors/handlers/MissingRequestParametersWebErrorHandlerTest.java b/src/test/java/me/alidg/errors/handlers/MissingRequestParametersWebErrorHandlerTest.java index 7013a3d..c6d2ff8 100644 --- a/src/test/java/me/alidg/errors/handlers/MissingRequestParametersWebErrorHandlerTest.java +++ b/src/test/java/me/alidg/errors/handlers/MissingRequestParametersWebErrorHandlerTest.java @@ -56,7 +56,7 @@ public void handle_ShouldHandleMissingRequestParamsErrorsProperly(Throwable exce assertThat(handledException.getErrorCodes()).containsOnly(expectedCode); assertThat(handledException.getStatusCode()).isEqualTo(expectedStatus); - assertThat(handledException.getArguments()).isEqualTo(expectedArgs); + // TODO check arguments assertThat(handledException.getArguments()).isEqualTo(expectedArgs); } private Object[] provideParamsForCanHandle() { diff --git a/src/test/java/me/alidg/errors/handlers/ResponseStatusWebErrorHandlerTest.java b/src/test/java/me/alidg/errors/handlers/ResponseStatusWebErrorHandlerTest.java index ad6e8b3..b5bb2f7 100644 --- a/src/test/java/me/alidg/errors/handlers/ResponseStatusWebErrorHandlerTest.java +++ b/src/test/java/me/alidg/errors/handlers/ResponseStatusWebErrorHandlerTest.java @@ -57,10 +57,12 @@ public void handle_ShouldHandleResponseStatusExceptionsAppropriately(Exception e assertThat(handledException.getErrorCodes()).containsExactly(expectedErrorCode); assertThat(handledException.getStatusCode()).isEqualTo(expectedStatus); +/* TODO check arguments if (expectedArguments == null || expectedArguments.isEmpty()) assertThat(handledException.getArguments()).isEmpty(); else assertThat(handledException.getArguments().get(expectedErrorCode)).containsAll(expectedArguments); +*/ } private Object[] paramsForCanHandle() { diff --git a/src/test/java/me/alidg/errors/handlers/ServletWebErrorHandlerTest.java b/src/test/java/me/alidg/errors/handlers/ServletWebErrorHandlerTest.java index 34abef8..047669c 100644 --- a/src/test/java/me/alidg/errors/handlers/ServletWebErrorHandlerTest.java +++ b/src/test/java/me/alidg/errors/handlers/ServletWebErrorHandlerTest.java @@ -61,7 +61,7 @@ public void handle_ShouldHandleSpringMvcErrorsProperly(Throwable exception, assertThat(handledException.getErrorCodes()).containsOnly(expectedCode); assertThat(handledException.getStatusCode()).isEqualTo(expectedStatus); - assertThat(handledException.getArguments()).isEqualTo(expectedArgs); + // TODO check arguments assertThat(handledException.getArguments()).isEqualTo(expectedArgs); } private Object[] provideParamsForCanHandle() { diff --git a/src/test/java/me/alidg/errors/handlers/SpringSecurityWebErrorHandlerTest.java b/src/test/java/me/alidg/errors/handlers/SpringSecurityWebErrorHandlerTest.java index 6ddbb1f..f10baae 100644 --- a/src/test/java/me/alidg/errors/handlers/SpringSecurityWebErrorHandlerTest.java +++ b/src/test/java/me/alidg/errors/handlers/SpringSecurityWebErrorHandlerTest.java @@ -2,6 +2,7 @@ import junitparams.JUnitParamsRunner; import junitparams.Parameters; +import me.alidg.errors.ErrorWithArguments; import me.alidg.errors.HandledException; import org.junit.Test; import org.junit.runner.RunWith; @@ -11,9 +12,11 @@ import org.springframework.security.core.userdetails.UsernameNotFoundException; import org.springframework.security.web.authentication.rememberme.CookieTheftException; +import static java.util.Collections.emptyList; import static me.alidg.Params.p; import static me.alidg.errors.handlers.SpringSecurityWebErrorHandler.*; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.tuple; import static org.springframework.http.HttpStatus.*; /** @@ -44,8 +47,10 @@ public void handle_ShouldHandleSecurityRelatedExceptionsProperly(Throwable excep HandledException handled = handler.handle(exception); assertThat(handled.getStatusCode()).isEqualTo(expectedStatusCode); - assertThat(handled.getErrorCodes()).containsOnly(expectedErrorCode); - assertThat(handled.getArguments()).isEmpty(); + assertThat(handled.getErrors()).extracting(ErrorWithArguments::getErrorCode, + ErrorWithArguments::getArguments) + .containsOnly(tuple(expectedErrorCode, + emptyList())); } private Object[] provideParamsForCanHandle() { diff --git a/src/test/java/me/alidg/errors/handlers/SpringValidationWebErrorHandlerTest.java b/src/test/java/me/alidg/errors/handlers/SpringValidationWebErrorHandlerTest.java index 136792f..c1e02aa 100644 --- a/src/test/java/me/alidg/errors/handlers/SpringValidationWebErrorHandlerTest.java +++ b/src/test/java/me/alidg/errors/handlers/SpringValidationWebErrorHandlerTest.java @@ -3,6 +3,7 @@ import junitparams.JUnitParamsRunner; import junitparams.Parameters; import me.alidg.errors.Argument; +import me.alidg.errors.ErrorWithArguments; import me.alidg.errors.HandledException; import org.junit.Test; import org.junit.runner.RunWith; @@ -13,10 +14,7 @@ import javax.validation.Valid; import javax.validation.Validation; -import javax.validation.constraints.Max; -import javax.validation.constraints.Min; -import javax.validation.constraints.NotBlank; -import javax.validation.constraints.Size; +import javax.validation.constraints.*; import java.util.*; import static java.util.Arrays.asList; @@ -27,6 +25,7 @@ import static me.alidg.errors.handlers.SpringValidationWebErrorHandlerTest.TBV.tbv; import static me.alidg.errors.handlers.SpringValidationWebErrorHandlerTest.TBVchild.tbvChild; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.tuple; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -60,8 +59,7 @@ public void canHandle_ShouldOnlyReturnTrueForSpringSpecificValidationErrors(Thro @Test @Parameters(method = "provideParamsForHandle") public void handle_ShouldHandleTheValidationErrorsProperly(Object toValidate, - Set errorCodes, - Map> args) { + List errors) { BindingResult result = new BeanPropertyBindingResult(toValidate, "toValidate"); validator.validate(toValidate, result); @@ -70,18 +68,16 @@ public void handle_ShouldHandleTheValidationErrorsProperly(Object toValidate, BindException bindException = new BindException(result); HandledException handledForBind = handler.handle(bindException); - assertThat(handledForBind.getErrorCodes()).containsAll(errorCodes); + assertThat(handledForBind.getErrors()).containsAll(errors); assertThat(handledForBind.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); - assertThat(handledForBind.getArguments()).isEqualTo(args); // Create and assert for MethodArgumentNotValidException MethodArgumentNotValidException exception = new MethodArgumentNotValidException(null, result); HandledException handled = handler.handle(exception); - assertThat(handled.getErrorCodes()).containsAll(errorCodes); + assertThat(handled.getErrors()).containsAll(errors); assertThat(handled.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); - assertThat(handled.getArguments()).isEqualTo(args); } @Test @@ -91,9 +87,43 @@ public void handle_ForUnknownBindingErrorsShouldReturnBindingFailureErrorCode() BindException exception = new BindException(bindingResult); HandledException handledException = handler.handle(exception); - assertThat(handledException.getArguments()).isEmpty(); assertThat(handledException.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); assertThat(handledException.getErrorCodes()).containsOnly(BINDING_FAILURE); + assertThat(handledException.getErrors()).extracting(ErrorWithArguments::getErrorCode, + ErrorWithArguments::getArguments) + .containsOnly(tuple(BINDING_FAILURE, + emptyList())); + } + + @Test + public void testWithMultipleSameErrorCodes() { + UserCreationParameters toValidate = new UserCreationParameters(); + BindingResult result = new BeanPropertyBindingResult(toValidate, "toValidate"); + validator.validate(toValidate, result); + + BindException bindException = new BindException(result); + HandledException handledForBind = handler.handle(bindException); + + assertThat(handledForBind.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(handledForBind.getErrors()).hasSize(4) + .extracting(ErrorWithArguments::getErrorCode, + ErrorWithArguments::getArguments) + .containsExactlyInAnyOrder( + tuple("property.should.not.be.empty", + asList(Argument.arg("invalid", null), + Argument.arg("property", "firstName"))), + tuple("property.should.not.be.empty", + asList(Argument.arg("invalid", null), + Argument.arg("property", "lastName"))), + tuple("property.should.not.be.empty", + asList(Argument.arg("invalid", null), + Argument.arg("property", "email"))), + tuple("property.should.not.be.empty", + asList(Argument.arg("invalid", null), + Argument.arg("property", "password"))) + + ); + } private Object[] provideParamsForCanHandle() { @@ -107,42 +137,41 @@ private Object[] provideParamsForCanHandle() { private Object[] provideParamsForHandle() { return p( - p(tbv("ali", 0, "coding"), e("age.min"), - singletonMap("age.min", asList( + p(tbv("ali", 0, "coding"), + errors(error("age.min", asList( arg("value", 1L), arg("invalid", 0), - arg("property", "age")))), - p(tbv("ali", 29), e("interests.limit"), - singletonMap("interests.limit", asList( + arg("property", "age"))))), + p(tbv("ali", 29), + errors(error("interests.limit", asList( arg("max", 6), arg("min", 1), arg("invalid", emptyList()), - arg("property", "interests")))), - p(tbv("", 29, "coding"), e("name.required"), - singletonMap("name.required", asList( + arg("property", "interests"))))), + p(tbv("", 29, "coding"), + errors(error("name.required", asList( arg("invalid", ""), - arg("property", "name")))), - p(tbv("", 200), e("name.required", "age.max", "interests.limit"), - m( - "age.max", asList( + arg("property", "name"))))), + p(tbv("", 200), + errors( + error("age.max", asList( arg("value", 100L), arg("invalid", 200), - arg("property", "age")), - "interests.limit", asList( + arg("property", "age"))), + error("interests.limit", asList( arg("max", 6), arg("min", 1), arg("invalid", emptyList()), - arg("property", "interests")), - "name.required", asList( + arg("property", "interests"))), + error("name.required", asList( arg("invalid", ""), - arg("property", "name")) + arg("property", "name"))) ) ), p(tbv("ali", 29, singletonList("coding"), asList(tbvChild("given"), tbvChild(""), tbvChild("also given"))), - e("stringField.required"), - singletonMap("stringField.required", asList( + errors(error("stringField.required", asList( arg("invalid", ""), - arg("property", "tbvChildren[1].stringField")))) + arg("property", "tbvChildren[1].stringField"))))) ); } @@ -150,6 +179,14 @@ private Set e(String... errorCodes) { return new HashSet<>(asList(errorCodes)); } + private List errors(ErrorWithArguments... errors) { + return Arrays.asList(errors); + } + + private ErrorWithArguments error(String errorCode, List arguments) { + return new ErrorWithArguments(errorCode, arguments); + } + private Map m(String k1, List v1, String k2, List v2, String k3, List v3) { Map map = new HashMap<>(); map.put(k1, v1); @@ -250,4 +287,50 @@ public void setStringField(String stringField) { this.stringField = stringField; } } + + static class UserCreationParameters { + @NotEmpty(message = "property.should.not.be.empty") + private String email; + + @NotEmpty(message = "property.should.not.be.empty") + private String firstName; + + @NotEmpty(message = "property.should.not.be.empty") + private String lastName; + + @NotEmpty(message = "property.should.not.be.empty") + private String password; + + public String getEmail() { + return email; + } + + public void setEmail(String email) { + this.email = email; + } + + public String getFirstName() { + return firstName; + } + + public void setFirstName(String firstName) { + this.firstName = firstName; + } + + public String getLastName() { + return lastName; + } + + public void setLastName(String lastName) { + this.lastName = lastName; + } + + public String getPassword() { + return password; + } + + public void setPassword(String password) { + this.password = password; + } + } } diff --git a/src/test/java/me/alidg/errors/handlers/TypeMismatchWebErrorHandlerTest.java b/src/test/java/me/alidg/errors/handlers/TypeMismatchWebErrorHandlerTest.java index fa897c7..ad42fcc 100644 --- a/src/test/java/me/alidg/errors/handlers/TypeMismatchWebErrorHandlerTest.java +++ b/src/test/java/me/alidg/errors/handlers/TypeMismatchWebErrorHandlerTest.java @@ -45,7 +45,7 @@ public void handle_ShouldHandleTypeMismatchExceptionsAppropriately(TypeMismatchE HandledException handledException = errorHandler.handle(ex); String errorCode = String.format("%s.%s", TYPE_MISMATCH, ex.getPropertyName()); - assertThat(handledException.getArguments().get(errorCode)).containsAll(arguments); + // TODO check arguments assertThat(handledException.getArguments().get(errorCode)).containsAll(arguments); assertThat(handledException.getErrorCodes()).contains(errorCode); assertThat(handledException.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); } From 5695b2c2095e3ad1bb98c4b6661ba5d1f105ab78 Mon Sep 17 00:00:00 2001 From: Wim Deblauwe Date: Tue, 28 Apr 2020 20:49:15 +0200 Subject: [PATCH 2/7] Add getArguments() again as deprecated method This will allow users of HandledException to smoothly upgrade since the HandledException is a public API exposed via WebErrorHandler. --- .../java/me/alidg/errors/HandledException.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/main/java/me/alidg/errors/HandledException.java b/src/main/java/me/alidg/errors/HandledException.java index 7fab7db..3543565 100644 --- a/src/main/java/me/alidg/errors/HandledException.java +++ b/src/main/java/me/alidg/errors/HandledException.java @@ -4,6 +4,7 @@ import org.springframework.lang.NonNull; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.stream.Collectors; @@ -83,6 +84,20 @@ public Set getErrorCodes() { .collect(Collectors.toSet()); } + /** + * + * @return Collection of to-be-exposed arguments. + * @deprecated This method should no longer be used as it does not allow to support the same error code + * multiple times + */ + @NonNull + @Deprecated + public Map> getArguments() { + return errors.stream() + .collect(Collectors.toMap(ErrorWithArguments::getErrorCode, + ErrorWithArguments::getArguments)); + } + /** * @return The mapped status code. * @see #statusCode From 6ac11cf359871b318d4d5b90e5e2c29e4d33f8a7 Mon Sep 17 00:00:00 2001 From: Wim Deblauwe Date: Tue, 28 Apr 2020 21:02:45 +0200 Subject: [PATCH 3/7] Add tests for deprecated methods on HandledException --- .../me/alidg/errors/HandledException.java | 4 +- .../me/alidg/errors/HandledExceptionTest.java | 64 +++++++++++++++++-- 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/src/main/java/me/alidg/errors/HandledException.java b/src/main/java/me/alidg/errors/HandledException.java index 3543565..2906868 100644 --- a/src/main/java/me/alidg/errors/HandledException.java +++ b/src/main/java/me/alidg/errors/HandledException.java @@ -75,9 +75,11 @@ public List getErrors() { /** * @return Collection of mapped error codes. - * @see #errors + * @deprecated This method should no longer be used as it does not allow to support the same error code + * multiple times */ @NonNull + @Deprecated public Set getErrorCodes() { return errors.stream() .map(ErrorWithArguments::getErrorCode) diff --git a/src/test/java/me/alidg/errors/HandledExceptionTest.java b/src/test/java/me/alidg/errors/HandledExceptionTest.java index e6db2c9..7cb8778 100644 --- a/src/test/java/me/alidg/errors/HandledExceptionTest.java +++ b/src/test/java/me/alidg/errors/HandledExceptionTest.java @@ -6,13 +6,13 @@ import org.junit.runner.RunWith; import org.springframework.http.HttpStatus; -import java.util.*; +import java.util.List; import static java.util.Arrays.asList; -import static java.util.Collections.*; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; import static me.alidg.Params.p; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.Assertions.*; import static org.springframework.http.HttpStatus.BAD_REQUEST; /** @@ -46,12 +46,62 @@ public void secondConstructor_ShouldEnforceItsPreconditions(String errorCode, } @Test - public void constructors_ShouldSetNullArgumentsAsEmptyMaps() { - // TODO check arguments assertThat(new HandledException(new HandledException.ErrorWithArguments("error", null), BAD_REQUEST).getArguments()) - // .isEqualTo(Collections.emptyList()); + public void testGetErrorCodes_singleError() { + HandledException exception = new HandledException(ErrorWithArguments.noArgumentError("error"), BAD_REQUEST); + assertThat(exception.getErrorCodes()).hasSize(1) + .contains("error"); } + @Test + public void testGetErrorCodes_multipleErrors() { + List list = asList(ErrorWithArguments.noArgumentError("error"), + new ErrorWithArguments("error2", singletonList(Argument.arg("argName", "argValue")))); + HandledException exception = new HandledException(list, BAD_REQUEST); + assertThat(exception.getErrorCodes()).hasSize(2) + .contains("error", "error2"); + + } + + @Test + public void testGetErrorCodes_duplicateErrors() { + List list = asList(new ErrorWithArguments("error", singletonList(Argument.arg("argName", "argValue1"))), + new ErrorWithArguments("error", singletonList(Argument.arg("argName", "argValue2")))); + HandledException exception = new HandledException(list, BAD_REQUEST); + assertThat(exception.getErrorCodes()).hasSize(1) + .contains("error"); + + } + + @Test + public void testGetArguments_singleError() { + HandledException exception = new HandledException(ErrorWithArguments.noArgumentError("error"), BAD_REQUEST); + assertThat(exception.getArguments()).hasSize(1) + .hasEntrySatisfying("error", arguments -> assertThat(arguments).isEmpty()); + + } + + @Test + public void testGetArguments_multipleErrors() { + List list = asList(ErrorWithArguments.noArgumentError("error"), + new ErrorWithArguments("error2", singletonList(Argument.arg("argName", "argValue")))); + HandledException exception = new HandledException(list, BAD_REQUEST); + assertThat(exception.getArguments()).hasSize(2) + .hasEntrySatisfying("error", arguments -> assertThat(arguments).isEmpty()) + .hasEntrySatisfying("error2", arguments -> assertThat(arguments).hasSize(1)); + + } + + @Test + public void testGetArguments_duplicateErrors() { + List list = asList(new ErrorWithArguments("error", singletonList(Argument.arg("argName", "argValue1"))), + new ErrorWithArguments("error", singletonList(Argument.arg("argName", "argValue2")))); + HandledException exception = new HandledException(list, BAD_REQUEST); + assertThatExceptionOfType(IllegalStateException.class) + .isThrownBy(exception::getArguments) + .withMessage("Duplicate key error (attempted merging values [argName=argValue1] and [argName=argValue2])"); + } + private Object[] provideParamsForPrimary() { return p( p(null, null, NullPointerException.class, "Error codes is required"), From 7c54a13aee2a26e4f2e768f92adce6a95834834a Mon Sep 17 00:00:00 2001 From: Wim Deblauwe Date: Tue, 28 Apr 2020 21:42:45 +0200 Subject: [PATCH 4/7] Update tests to use ErrorWithArguments --- .../ConstraintViolationWebErrorHandler.java | 1 + .../AnnotatedWebErrorHandlerTest.java | 11 ++++++--- ...gRequestParametersWebErrorHandlerTest.java | 12 +++++++--- .../ResponseStatusWebErrorHandlerTest.java | 23 +++++++++++-------- .../TypeMismatchWebErrorHandlerTest.java | 9 ++++++-- 5 files changed, 38 insertions(+), 18 deletions(-) diff --git a/src/main/java/me/alidg/errors/handlers/ConstraintViolationWebErrorHandler.java b/src/main/java/me/alidg/errors/handlers/ConstraintViolationWebErrorHandler.java index a012be6..59bdc69 100644 --- a/src/main/java/me/alidg/errors/handlers/ConstraintViolationWebErrorHandler.java +++ b/src/main/java/me/alidg/errors/handlers/ConstraintViolationWebErrorHandler.java @@ -74,6 +74,7 @@ private List extractErrorWithArguments(ConstraintViolationEx .stream() .map(constraintViolation -> new ErrorWithArguments(ConstraintViolations.getErrorCode(constraintViolation), ConstraintViolations.getArguments(constraintViolation))) + .filter(errorWithArguments -> !errorWithArguments.getArguments().isEmpty()) .collect(toList()); } } diff --git a/src/test/java/me/alidg/errors/handlers/AnnotatedWebErrorHandlerTest.java b/src/test/java/me/alidg/errors/handlers/AnnotatedWebErrorHandlerTest.java index d86b976..8e44b2b 100644 --- a/src/test/java/me/alidg/errors/handlers/AnnotatedWebErrorHandlerTest.java +++ b/src/test/java/me/alidg/errors/handlers/AnnotatedWebErrorHandlerTest.java @@ -3,6 +3,7 @@ import junitparams.JUnitParamsRunner; import junitparams.Parameters; import me.alidg.errors.Argument; +import me.alidg.errors.ErrorWithArguments; import me.alidg.errors.HandledException; import me.alidg.errors.annotation.ExceptionMapping; import me.alidg.errors.annotation.ExposeAsArg; @@ -49,10 +50,14 @@ public void handle_ShouldProperlyHandleTheGivenException(Exception exception, HandledException handled = handler.handle(exception); assertThat(handled).isNotNull(); - assertThat(handled.getErrorCodes()).hasSize(1); - assertThat(handled.getErrorCodes()).containsExactly(code); + List errors = handled.getErrors(); + assertThat(errors).hasSize(1) + .extracting(ErrorWithArguments::getErrorCode) + .containsExactly(code); + assertThat(errors).hasSize(1) + .extracting(ErrorWithArguments::getArguments) + .containsExactly(args); assertThat(handled.getStatusCode()).isEqualTo(status); - // TODO check arguments assertThat((handled.getArguments().get(code))).isEqualTo(args); } private Object[] provideParamsForCanHandle() { diff --git a/src/test/java/me/alidg/errors/handlers/MissingRequestParametersWebErrorHandlerTest.java b/src/test/java/me/alidg/errors/handlers/MissingRequestParametersWebErrorHandlerTest.java index c6d2ff8..9a9d371 100644 --- a/src/test/java/me/alidg/errors/handlers/MissingRequestParametersWebErrorHandlerTest.java +++ b/src/test/java/me/alidg/errors/handlers/MissingRequestParametersWebErrorHandlerTest.java @@ -2,6 +2,8 @@ import junitparams.JUnitParamsRunner; import junitparams.Parameters; +import me.alidg.errors.Argument; +import me.alidg.errors.ErrorWithArguments; import me.alidg.errors.HandledException; import me.alidg.errors.WebErrorHandler; import org.junit.Test; @@ -51,12 +53,16 @@ public void canHandle_ShouldReturnTrueForMissingRequestParamsErrors(Throwable ex public void handle_ShouldHandleMissingRequestParamsErrorsProperly(Throwable exception, String expectedCode, HttpStatus expectedStatus, - Map> expectedArgs) { + Map> expectedArgs) { HandledException handledException = handler.handle(exception); - assertThat(handledException.getErrorCodes()).containsOnly(expectedCode); + List errors = handledException.getErrors(); + assertThat(errors).extracting(ErrorWithArguments::getErrorCode) + .containsOnly(expectedCode); + assertThat(errors).extracting(ErrorWithArguments::getArguments) + .containsExactlyInAnyOrderElementsOf(expectedArgs.values()); + assertThat(handledException.getStatusCode()).isEqualTo(expectedStatus); - // TODO check arguments assertThat(handledException.getArguments()).isEqualTo(expectedArgs); } private Object[] provideParamsForCanHandle() { diff --git a/src/test/java/me/alidg/errors/handlers/ResponseStatusWebErrorHandlerTest.java b/src/test/java/me/alidg/errors/handlers/ResponseStatusWebErrorHandlerTest.java index b5bb2f7..e08c31a 100644 --- a/src/test/java/me/alidg/errors/handlers/ResponseStatusWebErrorHandlerTest.java +++ b/src/test/java/me/alidg/errors/handlers/ResponseStatusWebErrorHandlerTest.java @@ -3,6 +3,7 @@ import junitparams.JUnitParamsRunner; import junitparams.Parameters; import me.alidg.errors.Argument; +import me.alidg.errors.ErrorWithArguments; import me.alidg.errors.HandledException; import org.junit.Test; import org.junit.runner.RunWith; @@ -15,9 +16,7 @@ import org.springframework.web.server.*; import java.lang.annotation.Annotation; -import java.util.Arrays; -import java.util.HashSet; -import java.util.List; +import java.util.*; import static java.util.Collections.*; import static me.alidg.Params.p; @@ -55,14 +54,18 @@ public void handle_ShouldHandleResponseStatusExceptionsAppropriately(Exception e List expectedArguments) { HandledException handledException = handler.handle(e); - assertThat(handledException.getErrorCodes()).containsExactly(expectedErrorCode); + List errors = handledException.getErrors(); + assertThat(errors).extracting(ErrorWithArguments::getErrorCode) + .containsOnly(expectedErrorCode); + // We need to sort the list of arguments to be able to compare them + Collections.sort(expectedArguments, Comparator.comparing(Argument::getName)); + assertThat(errors).extracting(errorWithArguments -> { + List arguments = errorWithArguments.getArguments(); + Collections.sort(arguments, Comparator.comparing(Argument::getName)); + return arguments; + }).containsExactlyInAnyOrder(expectedArguments); + assertThat(handledException.getStatusCode()).isEqualTo(expectedStatus); -/* TODO check arguments - if (expectedArguments == null || expectedArguments.isEmpty()) - assertThat(handledException.getArguments()).isEmpty(); - else - assertThat(handledException.getArguments().get(expectedErrorCode)).containsAll(expectedArguments); -*/ } private Object[] paramsForCanHandle() { diff --git a/src/test/java/me/alidg/errors/handlers/TypeMismatchWebErrorHandlerTest.java b/src/test/java/me/alidg/errors/handlers/TypeMismatchWebErrorHandlerTest.java index ad42fcc..58e9a90 100644 --- a/src/test/java/me/alidg/errors/handlers/TypeMismatchWebErrorHandlerTest.java +++ b/src/test/java/me/alidg/errors/handlers/TypeMismatchWebErrorHandlerTest.java @@ -3,6 +3,7 @@ import junitparams.JUnitParamsRunner; import junitparams.Parameters; import me.alidg.errors.Argument; +import me.alidg.errors.ErrorWithArguments; import me.alidg.errors.HandledException; import me.alidg.errors.WebErrorHandler; import org.junit.Test; @@ -45,8 +46,12 @@ public void handle_ShouldHandleTypeMismatchExceptionsAppropriately(TypeMismatchE HandledException handledException = errorHandler.handle(ex); String errorCode = String.format("%s.%s", TYPE_MISMATCH, ex.getPropertyName()); - // TODO check arguments assertThat(handledException.getArguments().get(errorCode)).containsAll(arguments); - assertThat(handledException.getErrorCodes()).contains(errorCode); + List errors = handledException.getErrors(); + assertThat(errors).extracting(ErrorWithArguments::getErrorCode) + .containsExactly(errorCode); + assertThat(errors).extracting(ErrorWithArguments::getArguments) + .containsExactlyInAnyOrder(arguments); + assertThat(handledException.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); } From c36ed98f65fdcf842c1db71c2bce1d9870869605 Mon Sep 17 00:00:00 2001 From: Wim Deblauwe Date: Wed, 29 Apr 2020 19:58:12 +0200 Subject: [PATCH 5/7] Restore constructors as deprecated --- .../me/alidg/errors/HandledException.java | 55 ++++++++++++++++++- 1 file changed, 52 insertions(+), 3 deletions(-) diff --git a/src/main/java/me/alidg/errors/HandledException.java b/src/main/java/me/alidg/errors/HandledException.java index 2906868..165e802 100644 --- a/src/main/java/me/alidg/errors/HandledException.java +++ b/src/main/java/me/alidg/errors/HandledException.java @@ -2,12 +2,12 @@ import org.springframework.http.HttpStatus; import org.springframework.lang.NonNull; +import org.springframework.lang.Nullable; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.stream.Collectors; +import static java.util.Collections.singleton; import static java.util.Collections.singletonList; import static java.util.Objects.requireNonNull; @@ -64,6 +64,44 @@ public HandledException(@NonNull ErrorWithArguments error, this(singletonList(error), statusCode); } + /** + * Initialize a handled exception with a set of error codes, a HTTP status code and an + * optional collection of arguments. + * + * @param errorCodes The corresponding error codes for the handled exception. + * @param statusCode The corresponding status code for the handled exception. + * @param arguments Arguments to be exposed from the handled exception to the outside world. + * @throws NullPointerException When one of the required parameters is null. + * @throws IllegalArgumentException At least one error code should be provided. + * @deprecated This constructor should no longer be used as it does not allow to support the same error code + * multiple times + */ + @Deprecated + public HandledException(@NonNull Set errorCodes, + @NonNull HttpStatus statusCode, + @Nullable Map> arguments) { + this(convertToErrors(errorCodes, arguments), statusCode); + } + + /** + * Initialize a handled exception with an error code, a HTTP status code and an + * optional collection of arguments. + * + * @param errorCode The corresponding error code for the handled exception. + * @param statusCode The corresponding status code for the handled exception. + * @param arguments Arguments to be exposed from the handled exception to the outside world. + * @throws NullPointerException When one of the required parameters is null. + * @throws IllegalArgumentException At least one error code should be provided. + * @deprecated This constructor should no longer be used as it does not allow to support the same error code + * multiple times + */ + @Deprecated + public HandledException(@NonNull String errorCode, + @NonNull HttpStatus statusCode, + @Nullable Map> arguments) { + this(singleton(errorCode), statusCode, arguments); + } + /** * * @return Collection of errors @@ -121,4 +159,15 @@ private void enforcePreconditions(List errorCodes, HttpStatu throw new NullPointerException("The single error code can't be null"); } } + + @NonNull + private static List convertToErrors(Set errorCodes, Map> arguments) { + List result = new ArrayList<>(); + for (String errorCode : errorCodes) { + result.add(new ErrorWithArguments(errorCode, arguments.get(errorCode))); + } + + return result; + } + } From f1cdae1010e2331d4e62ee737c9f512c006f45e7 Mon Sep 17 00:00:00 2001 From: Wim Deblauwe Date: Wed, 29 Apr 2020 20:12:46 +0200 Subject: [PATCH 6/7] Fix ServletWebErrorHandlerTest --- .../handlers/ServletWebErrorHandlerTest.java | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/test/java/me/alidg/errors/handlers/ServletWebErrorHandlerTest.java b/src/test/java/me/alidg/errors/handlers/ServletWebErrorHandlerTest.java index 047669c..5a1f546 100644 --- a/src/test/java/me/alidg/errors/handlers/ServletWebErrorHandlerTest.java +++ b/src/test/java/me/alidg/errors/handlers/ServletWebErrorHandlerTest.java @@ -2,6 +2,8 @@ import junitparams.JUnitParamsRunner; import junitparams.Parameters; +import me.alidg.errors.Argument; +import me.alidg.errors.ErrorWithArguments; import me.alidg.errors.HandledException; import org.junit.Test; import org.junit.runner.RunWith; @@ -56,12 +58,22 @@ public void canHandle_ShouldReturnTrueForSpringMvcSpecificErrors(Throwable excep public void handle_ShouldHandleSpringMvcErrorsProperly(Throwable exception, String expectedCode, HttpStatus expectedStatus, - Map> expectedArgs) { + Map> expectedArgs) { HandledException handledException = handler.handle(exception); - assertThat(handledException.getErrorCodes()).containsOnly(expectedCode); + List errors = handledException.getErrors(); + assertThat(errors).extracting(ErrorWithArguments::getErrorCode) + .containsExactly(expectedCode); + if (expectedArgs.isEmpty()) { + assertThat(errors).extracting(ErrorWithArguments::getArguments) + .extracting(List::size) + .containsExactly(0); + } else { + assertThat(errors).extracting(ErrorWithArguments::getArguments) + .containsExactlyInAnyOrderElementsOf(expectedArgs.values()); + } + assertThat(handledException.getStatusCode()).isEqualTo(expectedStatus); - // TODO check arguments assertThat(handledException.getArguments()).isEqualTo(expectedArgs); } private Object[] provideParamsForCanHandle() { From dcffddcf7169e66ebae6dc0a6b1092efc6a1eeed Mon Sep 17 00:00:00 2001 From: Wim Deblauwe Date: Fri, 8 May 2020 08:47:59 +0200 Subject: [PATCH 7/7] Fix ConstraintViolationWebErrorHandlerTest --- ...onstraintViolationWebErrorHandlerTest.java | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/test/java/me/alidg/errors/handlers/ConstraintViolationWebErrorHandlerTest.java b/src/test/java/me/alidg/errors/handlers/ConstraintViolationWebErrorHandlerTest.java index eddde61..3c088fb 100644 --- a/src/test/java/me/alidg/errors/handlers/ConstraintViolationWebErrorHandlerTest.java +++ b/src/test/java/me/alidg/errors/handlers/ConstraintViolationWebErrorHandlerTest.java @@ -3,6 +3,7 @@ import junitparams.JUnitParamsRunner; import junitparams.Parameters; import me.alidg.errors.Argument; +import me.alidg.errors.ErrorWithArguments; import me.alidg.errors.HandledException; import me.alidg.errors.WebErrorHandler; import org.junit.Test; @@ -60,8 +61,11 @@ public void handle_ShouldHandleViolationExceptionsProperly(ConstraintViolationEx HandledException handledException = handler.handle(exception); assertThat(handledException.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); - assertThat(handledException.getErrorCodes()).containsAll(errorCodes); - // TODO check arguments + List errors = handledException.getErrors(); + assertThat(errors).extracting(ErrorWithArguments::getErrorCode) + .containsExactlyInAnyOrderElementsOf(errorCodes); + assertThat(errors).extracting(ErrorWithArguments::getArguments) + .containsExactlyInAnyOrderElementsOf(arguments.values()); } @SuppressWarnings("unchecked") @@ -91,11 +95,16 @@ private Object[] provideParamsForHandle() { p( v(new Person("", 19)), setOf("username.blank", "username.size"), - singletonMap("username.size", asList( - arg("max", 10), - arg("min", 6), - arg("invalid", ""), - arg("property", "username"))) + new HashMap>() {{ + put("username.size", asList( + arg("max", 10), + arg("min", 6), + arg("invalid", ""), + arg("property", "username"))); + put("username.blank", asList( + arg("invalid", ""), + arg("property", "username"))); + }} ), p( v(new Person("ali", 12)),