Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DT-1206: Bug fixes for support requests #2455

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
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);
}



}
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand All @@ -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;
}
Expand Down
3 changes: 3 additions & 0 deletions src/main/resources/assets/paths/supportRequest.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 3 additions & 0 deletions src/main/resources/assets/paths/supportUpload.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -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());
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -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(
Expand All @@ -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);
Expand All @@ -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));
Expand Down
Loading