From e4f9f949f02b99edc9d3958002feb94321d56e5a Mon Sep 17 00:00:00 2001 From: Ravi Khadiwala Date: Thu, 15 Aug 2024 17:28:19 -0500 Subject: [PATCH] Serialize subscription errors as json --- .../mappers/SubscriptionExceptionMapper.java | 36 +++++++++++-------- .../storage/SubscriptionException.java | 35 ++++++++++++++---- .../storage/SubscriptionManager.java | 2 +- .../SubscriptionControllerTest.java | 26 ++++++++++++++ 4 files changed, 77 insertions(+), 22 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/mappers/SubscriptionExceptionMapper.java b/service/src/main/java/org/whispersystems/textsecuregcm/mappers/SubscriptionExceptionMapper.java index 1ba90f578..711542b4b 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/mappers/SubscriptionExceptionMapper.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/mappers/SubscriptionExceptionMapper.java @@ -5,11 +5,9 @@ package org.whispersystems.textsecuregcm.mappers; -import javax.ws.rs.BadRequestException; -import javax.ws.rs.ClientErrorException; -import javax.ws.rs.ForbiddenException; -import javax.ws.rs.InternalServerErrorException; -import javax.ws.rs.NotFoundException; +import io.dropwizard.jersey.errors.ErrorMessage; +import javax.ws.rs.WebApplicationException; +import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.ext.ExceptionMapper; import org.whispersystems.textsecuregcm.storage.SubscriptionException; @@ -18,14 +16,24 @@ public class SubscriptionExceptionMapper implements ExceptionMapper new NotFoundException(e.getMessage(), e.getCause()).getResponse(); - case SubscriptionException.Forbidden e -> new ForbiddenException(e.getMessage(), e.getCause()).getResponse(); - case SubscriptionException.InvalidArguments e -> - new BadRequestException(e.getMessage(), e.getCause()).getResponse(); - case SubscriptionException.ProcessorConflict e -> - new ClientErrorException("existing processor does not match", Response.Status.CONFLICT).getResponse(); - default -> new InternalServerErrorException(exception.getMessage(), exception.getCause()).getResponse(); - }; + final Response.Status status = (switch (exception) { + case SubscriptionException.NotFound e -> Response.Status.NOT_FOUND; + case SubscriptionException.Forbidden e -> Response.Status.FORBIDDEN; + case SubscriptionException.InvalidArguments e -> Response.Status.BAD_REQUEST; + case SubscriptionException.ProcessorConflict e -> Response.Status.CONFLICT; + default -> Response.Status.INTERNAL_SERVER_ERROR; + }); + + // If the SubscriptionException came with suitable error message, include that in the response body. Otherwise, + // don't provide any message to the WebApplicationException constructor so the response includes the default + // HTTP error message for the status. + final WebApplicationException wae = exception.errorDetail() + .map(errorMessage -> new WebApplicationException(errorMessage, exception, Response.status(status).build())) + .orElseGet(() -> new WebApplicationException(exception, Response.status(status).build())); + + return Response + .fromResponse(wae.getResponse()) + .type(MediaType.APPLICATION_JSON_TYPE) + .entity(new ErrorMessage(wae.getResponse().getStatus(), wae.getLocalizedMessage())).build(); } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/SubscriptionException.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/SubscriptionException.java index 0a69f3fc9..92030fa93 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/SubscriptionException.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/SubscriptionException.java @@ -4,30 +4,51 @@ */ package org.whispersystems.textsecuregcm.storage; +import java.util.Optional; +import javax.annotation.Nullable; + public class SubscriptionException extends Exception { - public SubscriptionException(String message, Exception cause) { - super(message, cause); + + private @Nullable String errorDetail; + + public SubscriptionException(Exception cause) { + this(cause, null); + } + + SubscriptionException(Exception cause, String errorDetail) { + super(cause); + this.errorDetail = errorDetail; + } + + /** + * @return An error message suitable to include in a client response + */ + public Optional errorDetail() { + return Optional.ofNullable(errorDetail); } public static class NotFound extends SubscriptionException { + public NotFound() { - super(null, null); + super(null); } public NotFound(Exception cause) { - super(null, cause); + super(cause); } } public static class Forbidden extends SubscriptionException { + public Forbidden(final String message) { - super(message, null); + super(null, message); } } public static class InvalidArguments extends SubscriptionException { + public InvalidArguments(final String message, final Exception cause) { - super(message, cause); + super(cause, message); } } @@ -45,7 +66,7 @@ public PaymentRequiresAction() { public static class ProcessorConflict extends SubscriptionException { public ProcessorConflict(final String message) { - super(message, null); + super(null, message); } } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/SubscriptionManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/SubscriptionManager.java index d005778e7..0b6a13912 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/SubscriptionManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/SubscriptionManager.java @@ -263,7 +263,7 @@ public CompletableFuture addPayme final String customerId = updatedRecord.getProcessorCustomer() .filter(pc -> pc.processor().equals(subscriptionPaymentProcessor.getProvider())) .orElseThrow(() -> - ExceptionUtils.wrap(new SubscriptionException("record should not be missing customer", null))) + ExceptionUtils.wrap(new SubscriptionException(null, "record should not be missing customer"))) .customerId(); return paymentSetupFunction.apply(subscriptionPaymentProcessor, customerId); }); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/SubscriptionControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/SubscriptionControllerTest.java index d04dabf47..797ce6f51 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/SubscriptionControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/SubscriptionControllerTest.java @@ -457,8 +457,34 @@ Subscriptions.KEY_ACCESSED_AT, n(Instant.now().getEpochSecond()) String.format("/v1/subscription/%s/level/%s/%s/%s", subscriberId, level, currency, idempotencyKey)) .request() .put(Entity.json("")); + assertThat(response.getStatus()).isEqualTo(409); + assertThat(response.readEntity(Map.class)).containsOnlyKeys("code", "message"); + } + + @Test + void wrongProcessor() { + final byte[] subscriberUserAndKey = new byte[32]; + Arrays.fill(subscriberUserAndKey, (byte) 1); + subscriberId = Base64.getEncoder().encodeToString(subscriberUserAndKey); + + final ProcessorCustomer processorCustomer = new ProcessorCustomer("testCustomerId", PaymentProvider.BRAINTREE); + final Map dynamoItem = Map.of(Subscriptions.KEY_PASSWORD, b(new byte[16]), + Subscriptions.KEY_CREATED_AT, n(Instant.now().getEpochSecond()), + Subscriptions.KEY_ACCESSED_AT, n(Instant.now().getEpochSecond()), + Subscriptions.KEY_PROCESSOR_ID_CUSTOMER_ID, b(processorCustomer.toDynamoBytes()) + ); + final Subscriptions.Record record = Subscriptions.Record.from( + Arrays.copyOfRange(subscriberUserAndKey, 0, 16), dynamoItem); + when(SUBSCRIPTIONS.get(eq(Arrays.copyOfRange(subscriberUserAndKey, 0, 16)), any())) + .thenReturn(CompletableFuture.completedFuture(Subscriptions.GetResult.found(record))); + + final Response response = RESOURCE_EXTENSION + .target(String.format("/v1/subscription/%s/create_payment_method", subscriberId)) + .request() + .post(Entity.json("")); assertThat(response.getStatus()).isEqualTo(409); + assertThat(response.readEntity(Map.class)).containsOnlyKeys("code", "message"); } @Test