diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index cb4aabc54..77a7fc0ab 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -4,8 +4,8 @@ */ package org.whispersystems.textsecuregcm; -import static org.whispersystems.textsecuregcm.metrics.MetricsUtil.name; import static java.util.Objects.requireNonNull; +import static org.whispersystems.textsecuregcm.metrics.MetricsUtil.name; import com.google.common.collect.Lists; import com.webauthn4j.appattest.DeviceCheckManager; @@ -1085,7 +1085,7 @@ public class WhisperServerService extends Application removePendingMessage(@Auth AuthenticatedDevice auth, @PathParam("uuid") UUID uuid) { - final Account account = accountsManager.getByAccountIdentifier(auth.accountIdentifier()) - .orElseThrow(() -> new WebApplicationException(Status.UNAUTHORIZED)); - - final Device device = account.getDevice(auth.deviceId()) - .orElseThrow(() -> new WebApplicationException(Status.UNAUTHORIZED)); - - return messagesManager.delete(auth.accountIdentifier(), device, uuid, null) - .thenAccept(maybeRemovedMessage -> maybeRemovedMessage.ifPresent(removedMessage -> { - if (removedMessage.sourceServiceId().isPresent() - && removedMessage.envelopeType() != Type.SERVER_DELIVERY_RECEIPT) { - if (removedMessage.sourceServiceId().get() instanceof AciServiceIdentifier aciServiceIdentifier) { - try { - receiptSender.sendReceipt(removedMessage.destinationServiceId(), auth.deviceId(), - aciServiceIdentifier, removedMessage.clientTimestamp()); - } catch (Exception e) { - logger.warn("Failed to send delivery receipt", e); - } - } else { - // If source service ID is present and the envelope type is not a server delivery receipt, then - // the source service ID *should always* be an ACI -- PNIs are receive-only, so they can only be the - // "source" via server delivery receipts - logger.warn("Source service ID unexpectedly a PNI service ID"); - } - } - })) - .thenApply(Util.ASYNC_EMPTY_RESPONSE); - } - @POST @Consumes(MediaType.APPLICATION_JSON) @Path("/report/{source}/{messageGuid}") diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagesManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagesManager.java index 16bdf5ba9..1979783b6 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagesManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagesManager.java @@ -253,26 +253,17 @@ public class MessagesManager { return messagesCache.clear(destinationUuid, deviceId); } - public CompletableFuture> delete(UUID destinationUuid, Device destinationDevice, UUID guid, - @Nullable Long serverTimestamp) { + public CompletableFuture> delete(final UUID destinationUuid, + final Device destinationDevice, + final UUID guid, + final long serverTimestamp) { + return messagesCache.remove(destinationUuid, destinationDevice.getId(), guid) - .thenComposeAsync(removed -> { - - if (removed.isPresent()) { - return CompletableFuture.completedFuture(removed); - } - - final CompletableFuture> maybeDeletedEnvelope; - if (serverTimestamp == null) { - maybeDeletedEnvelope = messagesDynamoDb.deleteMessageByDestinationAndGuid(destinationUuid, - destinationDevice, guid); - } else { - maybeDeletedEnvelope = messagesDynamoDb.deleteMessage(destinationUuid, destinationDevice, guid, - serverTimestamp); - } - - return maybeDeletedEnvelope.thenApply(maybeEnvelope -> maybeEnvelope.map(RemovedMessage::fromEnvelope)); - }, messageDeletionExecutor); + .thenComposeAsync(removed -> removed + .map(_ -> CompletableFuture.completedFuture(removed)) + .orElseGet(() -> messagesDynamoDb.deleteMessage(destinationUuid, destinationDevice, guid, serverTimestamp) + .thenApply(maybeEnvelope -> maybeEnvelope.map(RemovedMessage::fromEnvelope)) + ), messageDeletionExecutor); } /** diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java index 381cfd2df..932b8b324 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java @@ -100,7 +100,6 @@ import org.whispersystems.textsecuregcm.push.MessageSender; import org.whispersystems.textsecuregcm.push.MessageTooLargeException; import org.whispersystems.textsecuregcm.push.PushNotificationManager; import org.whispersystems.textsecuregcm.push.PushNotificationScheduler; -import org.whispersystems.textsecuregcm.push.ReceiptSender; import org.whispersystems.textsecuregcm.spam.SpamChecker; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; @@ -108,13 +107,11 @@ import org.whispersystems.textsecuregcm.storage.ClientReleaseManager; import org.whispersystems.textsecuregcm.storage.Device; import org.whispersystems.textsecuregcm.storage.MessagesManager; import org.whispersystems.textsecuregcm.storage.PhoneNumberIdentifiers; -import org.whispersystems.textsecuregcm.storage.RemovedMessage; import org.whispersystems.textsecuregcm.storage.ReportMessageManager; import org.whispersystems.textsecuregcm.tests.util.AccountsHelper; import org.whispersystems.textsecuregcm.tests.util.AuthHelper; import org.whispersystems.textsecuregcm.tests.util.MultiRecipientMessageHelper; import org.whispersystems.textsecuregcm.tests.util.TestRecipient; -import org.whispersystems.textsecuregcm.util.CompletableFutureTestUtil; import org.whispersystems.textsecuregcm.util.HeaderUtils; import org.whispersystems.textsecuregcm.util.Pair; import org.whispersystems.textsecuregcm.util.SystemMapper; @@ -165,7 +162,6 @@ class MessageControllerTest { private static final RedisAdvancedClusterCommands redisCommands = mock(RedisAdvancedClusterCommands.class); private static final MessageSender messageSender = mock(MessageSender.class); - private static final ReceiptSender receiptSender = mock(ReceiptSender.class); private static final AccountsManager accountsManager = mock(AccountsManager.class); private static final MessagesManager messagesManager = mock(MessagesManager.class); private static final RateLimiters rateLimiters = mock(RateLimiters.class); @@ -192,7 +188,7 @@ class MessageControllerTest { .addProvider(MultiRecipientMessageProvider.class) .setTestContainerFactory(new GrizzlyWebTestContainerFactory()) .addResource( - new MessageController(rateLimiters, cardinalityEstimator, messageSender, receiptSender, accountsManager, + new MessageController(rateLimiters, cardinalityEstimator, messageSender, accountsManager, messagesManager, phoneNumberIdentifiers, pushNotificationManager, pushNotificationScheduler, reportMessageManager, messageDeliveryScheduler, mock(ClientReleaseManager.class), serverSecretParams, SpamChecker.noop(), new MessageMetrics(), mock(MessageDeliveryLoopMonitor.class), @@ -269,7 +265,6 @@ class MessageControllerTest { reset( redisCommands, messageSender, - receiptSender, accountsManager, messagesManager, rateLimiters, @@ -798,80 +793,6 @@ class MessageControllerTest { assertThat("Unauthorized response", response.getStatus(), is(equalTo(401))); } - @Test - void testDeleteMessages() { - long clientTimestamp = System.currentTimeMillis(); - - UUID sourceUuid = UUID.randomUUID(); - - UUID uuid1 = UUID.randomUUID(); - - final long serverTimestamp = 0; - when(messagesManager.delete(AuthHelper.VALID_UUID, AuthHelper.VALID_DEVICE, uuid1, null)) - .thenReturn( - CompletableFutureTestUtil.almostCompletedFuture(Optional.of( - new RemovedMessage(Optional.of(new AciServiceIdentifier(sourceUuid)), - new AciServiceIdentifier(AuthHelper.VALID_UUID), uuid1, serverTimestamp, clientTimestamp, - Envelope.Type.CIPHERTEXT)))); - - UUID uuid2 = UUID.randomUUID(); - when(messagesManager.delete(AuthHelper.VALID_UUID, AuthHelper.VALID_DEVICE, uuid2, null)) - .thenReturn( - CompletableFutureTestUtil.almostCompletedFuture(Optional.of( - new RemovedMessage(Optional.of(new AciServiceIdentifier(sourceUuid)), - new AciServiceIdentifier(AuthHelper.VALID_UUID), uuid2, serverTimestamp, clientTimestamp, - Envelope.Type.SERVER_DELIVERY_RECEIPT)))); - - UUID uuid3 = UUID.randomUUID(); - when(messagesManager.delete(AuthHelper.VALID_UUID, AuthHelper.VALID_DEVICE, uuid3, null)) - .thenReturn(CompletableFutureTestUtil.almostCompletedFuture(Optional.empty())); - - UUID uuid4 = UUID.randomUUID(); - when(messagesManager.delete(AuthHelper.VALID_UUID, AuthHelper.VALID_DEVICE, uuid4, null)) - .thenReturn(CompletableFuture.failedFuture(new RuntimeException("Oh No"))); - - try (final Response response = resources.getJerseyTest() - .target(String.format("/v1/messages/uuid/%s", uuid1)) - .request() - .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .delete()) { - - assertThat("Good Response Code", response.getStatus(), is(equalTo(204))); - verify(receiptSender).sendReceipt(eq(new AciServiceIdentifier(AuthHelper.VALID_UUID)), eq((byte) 1), - eq(new AciServiceIdentifier(sourceUuid)), eq(clientTimestamp)); - } - - try (final Response response = resources.getJerseyTest() - .target(String.format("/v1/messages/uuid/%s", uuid2)) - .request() - .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .delete()) { - - assertThat("Good Response Code", response.getStatus(), is(equalTo(204))); - verifyNoMoreInteractions(receiptSender); - } - - try (final Response response = resources.getJerseyTest() - .target(String.format("/v1/messages/uuid/%s", uuid3)) - .request() - .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .delete()) { - - assertThat("Good Response Code", response.getStatus(), is(equalTo(204))); - verifyNoMoreInteractions(receiptSender); - } - - try (final Response response = resources.getJerseyTest() - .target(String.format("/v1/messages/uuid/%s", uuid4)) - .request() - .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) - .delete()) { - - assertThat("Bad Response Code", response.getStatus(), is(equalTo(500))); - verifyNoMoreInteractions(receiptSender); - } - } - @Test void testReportMessageByE164() { final String senderNumber = "+12125550001";