DT-1206: Bug fixes for support requests (#2455)

DataBiosphere · Feb 5, 2025 · 2544432 · 2544432
1 parent 3746860
commit 2544432
Show file tree

Hide file tree

Showing 7 changed files with 116 additions and 9 deletions.
diff --git a/src/main/java/org/broadinstitute/consent/http/exceptions/UnprocessableEntityException.java b/src/main/java/org/broadinstitute/consent/http/exceptions/UnprocessableEntityException.java
@@ -0,0 +1,12 @@
+package org.broadinstitute.consent.http.exceptions;
+
+import com.google.api.client.http.HttpStatusCodes;
+import jakarta.ws.rs.ClientErrorException;
+
+public class UnprocessableEntityException extends ClientErrorException {
+
+  public UnprocessableEntityException(String message) {
+    super(message, HttpStatusCodes.STATUS_CODE_UNPROCESSABLE_ENTITY);
+  }
+
+}
diff --git a/src/main/java/org/broadinstitute/consent/http/resources/Resource.java b/src/main/java/org/broadinstitute/consent/http/resources/Resource.java
@@ -1,5 +1,6 @@
 package org.broadinstitute.consent.http.resources;
 
+import com.google.api.client.http.HttpStatusCodes;
 import com.google.gson.JsonSyntaxException;
 import com.google.gson.stream.MalformedJsonException;
 import io.sentry.Sentry;
@@ -24,6 +25,7 @@
 import org.broadinstitute.consent.http.enumeration.UserRoles;
 import org.broadinstitute.consent.http.exceptions.ConsentConflictException;
 import org.broadinstitute.consent.http.exceptions.UnknownIdentifierException;
+import org.broadinstitute.consent.http.exceptions.UnprocessableEntityException;
 import org.broadinstitute.consent.http.models.Error;
 import org.broadinstitute.consent.http.models.User;
 import org.broadinstitute.consent.http.util.ConsentLogger;
@@ -124,6 +126,9 @@ private interface ExceptionHandler {
     dispatch.put(ConsentConflictException.class, e ->
         Response.status(Response.Status.CONFLICT).type(MediaType.APPLICATION_JSON)
             .entity(new Error(e.getMessage(), Response.Status.CONFLICT.getStatusCode())).build());
+    dispatch.put(UnprocessableEntityException.class, e ->
+        Response.status(HttpStatusCodes.STATUS_CODE_UNPROCESSABLE_ENTITY).type(MediaType.APPLICATION_JSON)
+            .entity(new Error(e.getMessage(), HttpStatusCodes.STATUS_CODE_UNPROCESSABLE_ENTITY)).build());
     dispatch.put(UnsupportedOperationException.class, e ->
         Response.status(Response.Status.CONFLICT).type(MediaType.APPLICATION_JSON)
             .entity(new Error(e.getMessage(), Response.Status.CONFLICT.getStatusCode())).build());

diff --git a/src/main/java/org/broadinstitute/consent/http/service/SupportRequestService.java b/src/main/java/org/broadinstitute/consent/http/service/SupportRequestService.java
@@ -13,6 +13,7 @@
 import java.nio.charset.StandardCharsets;
 import org.apache.commons.io.IOUtils;
 import org.broadinstitute.consent.http.configurations.ServicesConfiguration;
+import org.broadinstitute.consent.http.exceptions.UnprocessableEntityException;
 import org.broadinstitute.consent.http.models.support.DuosTicket;
 import org.broadinstitute.consent.http.models.support.TicketFactory;
 import org.broadinstitute.consent.http.util.ConsentLogger;
@@ -48,18 +49,23 @@ public JsonObject postAttachmentToSupport(byte[] content) throws Exception {
 
       if (!response.isSuccessStatusCode()) {
         String errorMessage = "Error sending attachment to support: " + response.getStatusMessage();
-        var errorException = new ServerErrorException(response.getStatusMessage(),
-            HttpStatusCodes.STATUS_CODE_SERVER_ERROR);
+        var errorException =
+            response.getStatusCode() == HttpStatusCodes.STATUS_CODE_UNPROCESSABLE_ENTITY ?
+                new UnprocessableEntityException(errorMessage) :
+                new ServerErrorException(response.getStatusMessage(), response.getStatusCode());
         logException(errorMessage, errorException);
         throw errorException;
       }
       String responseContent = IOUtils.toString(response.getContent(), Charset.defaultCharset());
       JsonObject obj = GsonUtil.getInstance().fromJson(responseContent, JsonObject.class);
       if (obj != null && obj.get("upload") != null) {
-        JsonObject uploadObj = obj.get("upload").getAsJsonObject();
-        if (uploadObj != null && uploadObj.get("token") != null) {
-          return uploadObj.get("token").getAsJsonObject();
-        }
+        return obj.get("upload").getAsJsonObject();
+      } else {
+        var errorException = new ServerErrorException(response.getStatusMessage(),
+            HttpStatusCodes.STATUS_CODE_SERVER_ERROR);
+        String errorMessage = "Error reading attachment response content: " + responseContent;
+        logException(errorMessage, errorException);
+        throw errorException;
       }
     }
     throw new BadRequestException("Not configured to send support attachments");
@@ -82,8 +88,10 @@ public Request postTicketToSupport(DuosTicket ticket) throws Exception {
 
       if (!response.isSuccessStatusCode()) {
         String errorMessage = "Error posting ticket to support: " + response.getStatusMessage();
-        var errorException = new ServerErrorException(response.getStatusMessage(),
-            HttpStatusCodes.STATUS_CODE_SERVER_ERROR);
+        var errorException =
+            response.getStatusCode() == HttpStatusCodes.STATUS_CODE_UNPROCESSABLE_ENTITY ?
+                new UnprocessableEntityException(errorMessage) :
+                new ServerErrorException(response.getStatusMessage(), response.getStatusCode());
         logException(errorMessage, errorException);
         throw errorException;
       }

diff --git a/src/main/resources/assets/paths/supportRequest.yaml b/src/main/resources/assets/paths/supportRequest.yaml
@@ -18,5 +18,8 @@ post:
     400:
       description: |
         Bad Request: not configured for support requests
+    422:
+      description: |
+        Unprocessable Entity: provided content is not processable
     500:
       description: Internal Server Error
diff --git a/src/main/resources/assets/paths/supportUpload.yaml b/src/main/resources/assets/paths/supportUpload.yaml
@@ -23,5 +23,8 @@ post:
     400:
       description: | 
         Bad Request: max file upload size is 5 MB or server not configured for uploads
+    422:
+      description: |
+        Unprocessable Entity: provided content is not processable
     500:
       description: Internal Server Error
diff --git a/src/test/java/org/broadinstitute/consent/http/resources/SupportResourceTest.java b/src/test/java/org/broadinstitute/consent/http/resources/SupportResourceTest.java
@@ -2,6 +2,7 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.when;
 
 import com.google.api.client.http.HttpStatusCodes;
@@ -10,6 +11,7 @@
 import jakarta.ws.rs.core.Response;
 import java.util.stream.Stream;
 import org.broadinstitute.consent.http.enumeration.SupportRequestType;
+import org.broadinstitute.consent.http.exceptions.UnprocessableEntityException;
 import org.broadinstitute.consent.http.service.SupportRequestService;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -131,6 +133,25 @@ void testPostRequestInvalidFields(String body) {
     }
   }
 
+  @Test
+  void testUnprocessableTicket() throws Exception {
+    doThrow(new UnprocessableEntityException("Unprocessable")).when(supportRequestService)
+        .postTicketToSupport(any());
+    try (Response response = supportResource.postRequest("""
+        {
+          "name": "Test User",
+          "email": "[email protected]",
+          "subject": "Test Subject",
+          "description": "Test Description",
+          "type": "QUESTION",
+          "url": "https://example.com",
+          "uploads": ["token1", "token2"]
+        }
+        """)) {
+      assertEquals(HttpStatusCodes.STATUS_CODE_UNPROCESSABLE_ENTITY, response.getStatus());
+    }
+  }
+
   @Test
   void testPostUpload() throws Exception {
     JsonObject obj = new JsonObject();
@@ -141,4 +162,13 @@ void testPostUpload() throws Exception {
     }
   }
 
+  @Test
+  void testUnprocessableUpload() throws Exception {
+    doThrow(new UnprocessableEntityException("Unprocessable")).when(supportRequestService)
+        .postAttachmentToSupport(any());
+    try (Response response = supportResource.postUpload("test".getBytes())) {
+      assertEquals(HttpStatusCodes.STATUS_CODE_UNPROCESSABLE_ENTITY, response.getStatus());
+    }
+  }
+
 }
diff --git a/src/test/java/org/broadinstitute/consent/http/service/SupportRequestServiceTest.java b/src/test/java/org/broadinstitute/consent/http/service/SupportRequestServiceTest.java
@@ -10,13 +10,15 @@
 import com.google.api.client.http.HttpStatusCodes;
 import jakarta.ws.rs.BadRequestException;
 import jakarta.ws.rs.ServerErrorException;
+import jakarta.ws.rs.WebApplicationException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.List;
 import org.broadinstitute.consent.http.AbstractTestHelper;
 import org.broadinstitute.consent.http.configurations.ServicesConfiguration;
 import org.broadinstitute.consent.http.enumeration.SupportRequestType;
+import org.broadinstitute.consent.http.exceptions.UnprocessableEntityException;
 import org.broadinstitute.consent.http.models.support.DuosTicket;
 import org.broadinstitute.consent.http.models.support.TicketFactory;
 import org.broadinstitute.consent.http.models.support.TicketFields;
@@ -93,6 +95,19 @@ void testPostTicketToSupportNotificationsNotActivated() {
     assertThrows(BadRequestException.class, () -> service.postTicketToSupport(ticket));
   }
 
+  @Test
+  void testPostTicketToSupportNotificationsUnprocessableEntity() {
+    DuosTicket ticket = generateTicket();
+    when(config.isActivateSupportNotifications()).thenReturn(true);
+    when(config.postSupportRequestUrl()).thenReturn(
+        "http://" + container.getHost() + ":" + container.getServerPort() + "/");
+    mockServerClient.when(request())
+        .respond(response()
+            .withHeader(Header.header("Content-Type", "application/json"))
+            .withStatusCode(HttpStatusCodes.STATUS_CODE_UNPROCESSABLE_ENTITY));
+    assertThrows(UnprocessableEntityException.class, () -> service.postTicketToSupport(ticket));
+  }
+
   @Test
   void testPostTicketToSupportServerError() {
     DuosTicket ticket = generateTicket();
@@ -109,7 +124,7 @@ void testPostTicketToSupportServerError() {
   @Test
   void testPostAttachmentToSupport() throws Exception {
     String expectedBody = """
-        { "upload": { "token": { "token": "token string" } } }
+        { "upload": { "token": "token string" } }
         """;
     when(config.isActivateSupportNotifications()).thenReturn(true);
     when(config.postSupportUploadUrl()).thenReturn(
@@ -126,6 +141,25 @@ void testPostAttachmentToSupport() throws Exception {
     assertEquals(1, requests.length);
   }
 
+  @Test
+  void testPostTicketToSupportUnableToParseResponse() {
+    // This case should never happen, but we do inspect the response for a valid "upload" object.
+    // We need to ensure that the service handles invalid response formats correctly.
+    String expectedBody = """
+        { "invalid": { "missing_token": "token string" } }
+        """;
+    when(config.isActivateSupportNotifications()).thenReturn(true);
+    when(config.postSupportUploadUrl()).thenReturn(
+        "http://" + container.getHost() + ":" + container.getServerPort() + "/");
+    mockServerClient.when(request().withMethod("POST"))
+        .respond(response()
+            .withHeader(Header.header("Content-Type", "application/json"))
+            .withStatusCode(HttpStatusCodes.STATUS_CODE_CREATED)
+            .withBody(expectedBody)
+        );
+    assertThrows(ServerErrorException.class, () -> service.postAttachmentToSupport("Test".getBytes()));
+  }
+
   @Test
   void testPostAttachmentToSupportNotificationsNotActivated() {
     when(config.isActivateSupportNotifications()).thenReturn(false);
@@ -146,6 +180,18 @@ void testPostAttachmentToSupportServerError() {
     assertThrows(ServerErrorException.class, () -> service.postAttachmentToSupport("Test".getBytes()));
   }
 
+  @Test
+  void testPostAttachmentToSupportUnprocessableEntity() {
+    when(config.isActivateSupportNotifications()).thenReturn(true);
+    when(config.postSupportUploadUrl()).thenReturn(
+        "http://" + container.getHost() + ":" + container.getServerPort() + "/");
+    mockServerClient.when(request())
+        .respond(response()
+            .withHeader(Header.header("Content-Type", "application/json"))
+            .withStatusCode(HttpStatusCodes.STATUS_CODE_UNPROCESSABLE_ENTITY));
+    assertThrows(UnprocessableEntityException.class, () -> service.postAttachmentToSupport("Test".getBytes()));
+  }
+
   // Creates support ticket with random values
   private DuosTicket generateTicket() {
     List<SupportRequestType> types = new ArrayList<>(EnumSet.allOf(SupportRequestType.class));