From a46045d9875c1005cfed07789f4273090efc2e26 Mon Sep 17 00:00:00 2001 From: Ehren Kret Date: Wed, 11 Aug 2021 17:30:39 -0500 Subject: [PATCH] Remove unused methods that delete messages by sender and timestamp --- .../textsecuregcm/storage/MessagesCache.java | 19 ----------- .../storage/MessagesDynamoDb.java | 31 ----------------- .../storage/MessagesCacheTest.java | 13 ------- .../tests/storage/MessagesDynamoDbTest.java | 34 ++++--------------- 4 files changed, 7 insertions(+), 90 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagesCache.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagesCache.java index 3c026f73b..e90c5b54b 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagesCache.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagesCache.java @@ -86,7 +86,6 @@ public class MessagesCache extends RedisClusterPubSubAdapter imp private static final String REMOVE_TIMER_NAME = name(MessagesCache.class, "remove"); private static final String REMOVE_METHOD_TAG = "method"; - private static final String REMOVE_METHOD_SENDER = "sender"; private static final String REMOVE_METHOD_UUID = "uuid"; private static final Logger logger = LoggerFactory.getLogger(MessagesCache.class); @@ -161,24 +160,6 @@ public class MessagesCache extends RedisClusterPubSubAdapter imp }); } - public Optional remove(final UUID destinationUuid, final long destinationDevice, final String sender, final long timestamp) { - try { - final byte[] serialized = (byte[])Metrics.timer(REMOVE_TIMER_NAME, REMOVE_METHOD_TAG, REMOVE_METHOD_SENDER).record(() -> - removeBySenderScript.executeBinary(List.of(getMessageQueueKey(destinationUuid, destinationDevice), - getMessageQueueMetadataKey(destinationUuid, destinationDevice), - getQueueIndexKey(destinationUuid, destinationDevice)), - List.of((sender + "::" + timestamp).getBytes(StandardCharsets.UTF_8)))); - - if (serialized != null) { - return Optional.of(constructEntityFromEnvelope(0, MessageProtos.Envelope.parseFrom(serialized))); - } - } catch (final InvalidProtocolBufferException e) { - logger.warn("Failed to parse envelope", e); - } - - return Optional.empty(); - } - public Optional remove(final UUID destinationUuid, final long destinationDevice, final UUID messageGuid) { return remove(destinationUuid, destinationDevice, List.of(messageGuid)).stream().findFirst(); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagesDynamoDb.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagesDynamoDb.java index 536a758a3..29c66bfb5 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagesDynamoDb.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagesDynamoDb.java @@ -19,7 +19,6 @@ import java.util.Optional; import java.util.UUID; import java.util.stream.Collectors; import javax.annotation.Nonnull; -import org.apache.commons.lang3.StringUtils; import org.whispersystems.textsecuregcm.entities.MessageProtos; import org.whispersystems.textsecuregcm.entities.OutgoingMessageEntity; import org.whispersystems.textsecuregcm.util.AttributeValues; @@ -53,7 +52,6 @@ public class MessagesDynamoDb extends AbstractDynamoDbStore { private final Timer storeTimer = timer(name(getClass(), "store")); private final Timer loadTimer = timer(name(getClass(), "load")); - private final Timer deleteBySourceAndTimestamp = timer(name(getClass(), "delete", "sourceAndTimestamp")); private final Timer deleteByGuid = timer(name(getClass(), "delete", "guid")); private final Timer deleteByAccount = timer(name(getClass(), "delete", "account")); private final Timer deleteByDevice = timer(name(getClass(), "delete", "device")); @@ -138,35 +136,6 @@ public class MessagesDynamoDb extends AbstractDynamoDbStore { }); } - public Optional deleteMessageByDestinationAndSourceAndTimestamp(final UUID destinationAccountUuid, final long destinationDeviceId, final String source, final long timestamp) { - return deleteBySourceAndTimestamp.record(() -> { - if (StringUtils.isEmpty(source)) { - throw new IllegalArgumentException("must specify a source"); - } - - final AttributeValue partitionKey = convertPartitionKey(destinationAccountUuid); - final QueryRequest queryRequest = QueryRequest.builder() - .tableName(tableName) - .projectionExpression(KEY_SORT) - .consistentRead(true) - .keyConditionExpression("#part = :part AND begins_with ( #sort , :sortprefix )") - .filterExpression("#source = :source AND #timestamp = :timestamp") - .expressionAttributeNames(Map.of( - "#part", KEY_PARTITION, - "#sort", KEY_SORT, - "#source", KEY_SOURCE, - "#timestamp", KEY_TIMESTAMP)) - .expressionAttributeValues(Map.of( - ":part", partitionKey, - ":sortprefix", convertDestinationDeviceIdToSortKeyPrefix(destinationDeviceId), - ":source", AttributeValues.fromString(source), - ":timestamp", AttributeValues.fromLong(timestamp))) - .build(); - - return deleteItemsMatchingQueryAndReturnFirstOneActuallyDeleted(partitionKey, queryRequest); - }); - } - public Optional deleteMessageByDestinationAndGuid(final UUID destinationAccountUuid, final long destinationDeviceId, final UUID messageUuid) { return deleteByGuid.record(() -> { final AttributeValue partitionKey = convertPartitionKey(destinationAccountUuid); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/MessagesCacheTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/MessagesCacheTest.java index 66345ac75..b9f1ec3bb 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/MessagesCacheTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/MessagesCacheTest.java @@ -87,19 +87,6 @@ public class MessagesCacheTest extends AbstractRedisClusterTest { assertEquals(firstId, secondId); } - @Test - public void testRemoveBySender() { - final UUID messageGuid = UUID.randomUUID(); - final MessageProtos.Envelope message = generateRandomMessage(messageGuid, false); - - messagesCache.insert(messageGuid, DESTINATION_UUID, DESTINATION_DEVICE_ID, message); - final Optional maybeRemovedMessage = messagesCache.remove(DESTINATION_UUID, DESTINATION_DEVICE_ID, message.getSource(), message.getTimestamp()); - - assertTrue(maybeRemovedMessage.isPresent()); - assertEquals(MessagesCache.constructEntityFromEnvelope(0, message), maybeRemovedMessage.get()); - assertEquals(Optional.empty(), messagesCache.remove(DESTINATION_UUID, DESTINATION_DEVICE_ID, message.getSource(), message.getTimestamp())); - } - @Test @Parameters({"true", "false"}) public void testRemoveByUUID(final boolean sealedSender) { diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/MessagesDynamoDbTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/MessagesDynamoDbTest.java index bb52e10fc..69071c11e 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/MessagesDynamoDbTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/MessagesDynamoDbTest.java @@ -5,7 +5,14 @@ package org.whispersystems.textsecuregcm.tests.storage; +import static org.assertj.core.api.Assertions.assertThat; + import com.google.protobuf.ByteString; +import java.time.Duration; +import java.util.List; +import java.util.Random; +import java.util.UUID; +import java.util.function.Consumer; import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; @@ -14,14 +21,6 @@ import org.whispersystems.textsecuregcm.entities.OutgoingMessageEntity; import org.whispersystems.textsecuregcm.storage.MessagesDynamoDb; import org.whispersystems.textsecuregcm.tests.util.MessagesDynamoDbRule; -import java.time.Duration; -import java.util.List; -import java.util.Random; -import java.util.UUID; -import java.util.function.Consumer; - -import static org.assertj.core.api.Assertions.assertThat; - public class MessagesDynamoDbTest { private static final Random random = new Random(); private static final MessageProtos.Envelope MESSAGE1; @@ -127,25 +126,6 @@ public class MessagesDynamoDbTest { assertThat(messagesDynamoDb.load(secondDestinationUuid, 1, MessagesDynamoDb.RESULT_SET_CHUNK_SIZE)).isNotNull().hasSize(1).element(0).satisfies(verify(MESSAGE2)); } - @Test - public void testDeleteMessageByDestinationAndSourceAndTimestamp() { - final UUID destinationUuid = UUID.randomUUID(); - final UUID secondDestinationUuid = UUID.randomUUID(); - messagesDynamoDb.store(List.of(MESSAGE1), destinationUuid, 1); - messagesDynamoDb.store(List.of(MESSAGE2), secondDestinationUuid, 1); - messagesDynamoDb.store(List.of(MESSAGE3), destinationUuid, 2); - - assertThat(messagesDynamoDb.load(destinationUuid, 1, MessagesDynamoDb.RESULT_SET_CHUNK_SIZE)).isNotNull().hasSize(1).element(0).satisfies(verify(MESSAGE1)); - assertThat(messagesDynamoDb.load(destinationUuid, 2, MessagesDynamoDb.RESULT_SET_CHUNK_SIZE)).isNotNull().hasSize(1).element(0).satisfies(verify(MESSAGE3)); - assertThat(messagesDynamoDb.load(secondDestinationUuid, 1, MessagesDynamoDb.RESULT_SET_CHUNK_SIZE)).isNotNull().hasSize(1).element(0).satisfies(verify(MESSAGE2)); - - messagesDynamoDb.deleteMessageByDestinationAndSourceAndTimestamp(secondDestinationUuid, 1, MESSAGE2.getSource(), MESSAGE2.getTimestamp()); - - assertThat(messagesDynamoDb.load(destinationUuid, 1, MessagesDynamoDb.RESULT_SET_CHUNK_SIZE)).isNotNull().hasSize(1).element(0).satisfies(verify(MESSAGE1)); - assertThat(messagesDynamoDb.load(destinationUuid, 2, MessagesDynamoDb.RESULT_SET_CHUNK_SIZE)).isNotNull().hasSize(1).element(0).satisfies(verify(MESSAGE3)); - assertThat(messagesDynamoDb.load(secondDestinationUuid, 1, MessagesDynamoDb.RESULT_SET_CHUNK_SIZE)).isNotNull().isEmpty(); - } - @Test public void testDeleteMessageByDestinationAndGuid() { final UUID destinationUuid = UUID.randomUUID();