KiwiStandardResponses#standardErrorResponse should check status code

Modify KiwiStandardResponses#standardErrorResponse to verify the response status is actually a client or server error. Closes #1165
kiwiproject · Aug 9, 2024 · 8edbc5c · 8edbc5c
1 parent 8c48244
commit 8edbc5c
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 3 deletions.
diff --git a/src/main/java/org/kiwiproject/jaxrs/KiwiStandardResponses.java b/src/main/java/org/kiwiproject/jaxrs/KiwiStandardResponses.java
@@ -1,10 +1,12 @@
 package org.kiwiproject.jaxrs;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static java.util.Objects.nonNull;
 import static org.kiwiproject.base.KiwiStrings.f;
 
 import jakarta.ws.rs.core.MediaType;
 import jakarta.ws.rs.core.Response;
+import jakarta.ws.rs.core.Response.Status.Family;
 import lombok.experimental.UtilityClass;
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.kiwiproject.jaxrs.exception.ErrorMessage;
@@ -321,11 +323,12 @@ public static Response.ResponseBuilder standardNotFoundResponseBuilder(String er
      * Returns a response having the given status and an {@link ErrorMessage} entity which uses {@code errorDetails}
      * as the detailed error message.
      * <p>
-     * Does not verify that the given status is actually an error status.
+     * Verifies that the given status is actually an error status (4xx or 5xx).
      *
      * @param status       the error status to use
      * @param errorDetails the error message to use
      * @return a response with the given status code and {@code application/json} content type
+     * @throws IllegalArgumentException if the given status is not a client or server error
      */
     public static Response standardErrorResponse(Response.Status status, String errorDetails) {
         return standardErrorResponseBuilder(status, errorDetails).build();
@@ -335,14 +338,20 @@ public static Response standardErrorResponse(Response.Status status, String erro
      * Returns a response builder having the given status and an {@link ErrorMessage} entity which uses
      * {@code errorDetails} as the detailed error message.
      * <p>
-     * Does not verify that the given status is actually an error status.
+     * Verifies that the given status is actually an error status (4xx or 5xx).
      *
      * @param status       the error status to use
      * @param errorDetails the error message to use
      * @return a response builder with the given status code and {@code application/json} content type
+     * @throws IllegalArgumentException if the given status is not a client or server error
      */
     public static Response.ResponseBuilder standardErrorResponseBuilder(Response.Status status, String errorDetails) {
-        return JaxrsExceptionMapper.buildResponseBuilder(new JaxrsException(errorDetails, status.getStatusCode()));
+        var family = status.getFamily();
+        var statusCode = status.getStatusCode();
+        checkArgument(family == Family.CLIENT_ERROR || family == Family.SERVER_ERROR,
+                "status %s is not a client error (4xx) or server error (5xx)", statusCode);
+
+        return JaxrsExceptionMapper.buildResponseBuilder(new JaxrsException(errorDetails, statusCode));
     }
 
     /**

diff --git a/src/test/java/org/kiwiproject/jaxrs/KiwiStandardResponsesTest.java b/src/test/java/org/kiwiproject/jaxrs/KiwiStandardResponsesTest.java
@@ -1,6 +1,7 @@
 package org.kiwiproject.jaxrs;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 import static org.kiwiproject.jaxrs.JaxrsTestHelper.assertAcceptedResponse;
 import static org.kiwiproject.jaxrs.JaxrsTestHelper.assertCreatedResponseWithLocation;
 import static org.kiwiproject.jaxrs.JaxrsTestHelper.assertJsonResponseType;
@@ -282,6 +283,15 @@ void shouldReturnResponse_WithGivenStatusAndErrorMessageEntity(int statusCode) {
             assertResponseEntityHasOneErrorMessage(response, statusCode, "This is the error message. It is very helpful.");
             assertJsonResponseType(response);
         }
+
+        @ParameterizedTest
+        @ValueSource(ints = {200, 202, 302, 304})
+        void shouldThrowIllegalArgument_WhenStatusCodeIsNotClientOrServerError(int statusCode) {
+            var status = Response.Status.fromStatusCode(statusCode);
+            assertThatIllegalArgumentException()
+                    .isThrownBy(() -> KiwiStandardResponses.standardErrorResponse(status, "This is not actually an error status!"))
+                    .withMessage("status %d is not a client error (4xx) or server error (5xx)", statusCode);
+        }
     }
 
     @Nested