From 662c905b8056ba046f517fafb9975561cb572f1c Mon Sep 17 00:00:00 2001 From: Ehren Kret Date: Wed, 11 Aug 2021 16:14:36 -0500 Subject: [PATCH] Remove deprecated delete messages endpoint DELETE /v1/messages/{source}/{timestamp} has been deprecated a long time and has minimal usage each day at this point. Dropping support for this endpoint to improve message cache storage flexibility. --- .../controllers/MessageController.java | 23 --------------- .../storage/MessagesManager.java | 16 ----------- .../controllers/MessageControllerTest.java | 28 +++++++++---------- 3 files changed, 13 insertions(+), 54 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java index 29c97af51..d94d8e043 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java @@ -558,29 +558,6 @@ public class MessageController { return size; } - @Timed - @DELETE - @Path("/{source}/{timestamp}") - public void removePendingMessage(@Auth AuthenticatedAccount auth, - @PathParam("source") String source, - @PathParam("timestamp") long timestamp) { - try { - WebSocketConnection.recordMessageDeliveryDuration(timestamp, auth.getAuthenticatedDevice()); - Optional message = messagesManager.delete( - auth.getAccount().getUuid(), - auth.getAuthenticatedDevice().getId(), - source, timestamp); - - if (message.isPresent() && message.get().getType() != Envelope.Type.SERVER_DELIVERY_RECEIPT_VALUE) { - receiptSender.sendReceipt(auth, - message.get().getSource(), - message.get().getTimestamp()); - } - } catch (NoSuchUserException e) { - logger.warn("Sending delivery receipt", e); - } - } - @Timed @DELETE @Path("/uuid/{uuid}") 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 5ff1a226d..3fdfa98cb 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagesManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagesManager.java @@ -26,8 +26,6 @@ public class MessagesManager { private static final int RESULT_SET_CHUNK_SIZE = 100; private static final MetricRegistry metricRegistry = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME); - private static final Meter cacheHitByNameMeter = metricRegistry.meter(name(MessagesManager.class, "cacheHitByName" )); - private static final Meter cacheMissByNameMeter = metricRegistry.meter(name(MessagesManager.class, "cacheMissByName")); private static final Meter cacheHitByGuidMeter = metricRegistry.meter(name(MessagesManager.class, "cacheHitByGuid" )); private static final Meter cacheMissByGuidMeter = metricRegistry.meter(name(MessagesManager.class, "cacheMissByGuid")); @@ -95,20 +93,6 @@ public class MessagesManager { messagesDynamoDb.deleteAllMessagesForDevice(destinationUuid, deviceId); } - public Optional delete( - UUID destinationUuid, long destinationDeviceId, String source, long timestamp) { - Optional removed = messagesCache.remove(destinationUuid, destinationDeviceId, source, timestamp); - - if (removed.isEmpty()) { - removed = messagesDynamoDb.deleteMessageByDestinationAndSourceAndTimestamp(destinationUuid, destinationDeviceId, source, timestamp); - cacheMissByNameMeter.mark(); - } else { - cacheHitByNameMeter.mark(); - } - - return removed; - } - public Optional delete(UUID destinationUuid, long destinationDeviceId, UUID guid) { Optional removed = messagesCache.remove(destinationUuid, destinationDeviceId, guid); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/MessageControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/MessageControllerTest.java index 661e5ac88..66e3568c7 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/MessageControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/MessageControllerTest.java @@ -555,24 +555,22 @@ class MessageControllerTest { UUID sourceUuid = UUID.randomUUID(); - when(messagesManager.delete(AuthHelper.VALID_UUID, 1, "+14152222222", 31337)) - .thenReturn(Optional.of(new OutgoingMessageEntity(31337L, true, null, - Envelope.Type.CIPHERTEXT_VALUE, - null, timestamp, - "+14152222222", sourceUuid, 1, "hi".getBytes(), null, 0))); + UUID uuid1 = UUID.randomUUID(); + when(messagesManager.delete(AuthHelper.VALID_UUID, 1, uuid1)).thenReturn(Optional.of(new OutgoingMessageEntity( + 31337L, true, uuid1, Envelope.Type.CIPHERTEXT_VALUE, + null, timestamp, "+14152222222", sourceUuid, 1, "hi".getBytes(), null, 0))); - when(messagesManager.delete(AuthHelper.VALID_UUID, 1, "+14152222222", 31338)) - .thenReturn(Optional.of(new OutgoingMessageEntity(31337L, true, null, - Envelope.Type.SERVER_DELIVERY_RECEIPT_VALUE, - null, System.currentTimeMillis(), - "+14152222222", sourceUuid, 1, null, null, 0))); + UUID uuid2 = UUID.randomUUID(); + when(messagesManager.delete(AuthHelper.VALID_UUID, 1, uuid2)).thenReturn(Optional.of(new OutgoingMessageEntity( + 31337L, true, uuid2, Envelope.Type.SERVER_DELIVERY_RECEIPT_VALUE, + null, System.currentTimeMillis(), "+14152222222", sourceUuid, 1, null, null, 0))); - when(messagesManager.delete(AuthHelper.VALID_UUID, 1, "+14152222222", 31339)) - .thenReturn(Optional.empty()); + UUID uuid3 = UUID.randomUUID(); + when(messagesManager.delete(AuthHelper.VALID_UUID, 1, uuid3)).thenReturn(Optional.empty()); Response response = resources.getJerseyTest() - .target(String.format("/v1/messages/%s/%d", "+14152222222", 31337)) + .target(String.format("/v1/messages/uuid/%s", uuid1)) .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_NUMBER, AuthHelper.VALID_PASSWORD)) .delete(); @@ -581,7 +579,7 @@ class MessageControllerTest { verify(receiptSender).sendReceipt(any(AuthenticatedAccount.class), eq("+14152222222"), eq(timestamp)); response = resources.getJerseyTest() - .target(String.format("/v1/messages/%s/%d", "+14152222222", 31338)) + .target(String.format("/v1/messages/uuid/%s", uuid2)) .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_UUID.toString(), AuthHelper.VALID_PASSWORD)) .delete(); @@ -590,7 +588,7 @@ class MessageControllerTest { verifyNoMoreInteractions(receiptSender); response = resources.getJerseyTest() - .target(String.format("/v1/messages/%s/%d", "+14152222222", 31339)) + .target(String.format("/v1/messages/uuid/%s", uuid3)) .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_NUMBER, AuthHelper.VALID_PASSWORD)) .delete();