From 067aee666456f4a53aa9a8bf315db8bdc72b4ea9 Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Thu, 11 Nov 2021 13:01:56 -0500 Subject: [PATCH] Remove unused properties from `OutgoingMessageEntity` --- .../entities/OutgoingMessageEntity.java | 33 +++----------- .../textsecuregcm/storage/MessagesCache.java | 8 ++-- .../storage/MessagesDynamoDb.java | 2 +- .../controllers/MessageControllerTest.java | 15 +++---- .../storage/MessagesCacheTest.java | 11 ++--- .../websocket/WebSocketConnectionTest.java | 44 +++++++++---------- 6 files changed, 44 insertions(+), 69 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/OutgoingMessageEntity.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/OutgoingMessageEntity.java index 5e77874b1..005630c8f 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/OutgoingMessageEntity.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/OutgoingMessageEntity.java @@ -6,21 +6,13 @@ package org.whispersystems.textsecuregcm.entities; import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; - import java.util.Arrays; import java.util.Objects; import java.util.UUID; public class OutgoingMessageEntity { - @JsonIgnore - private final long id; - - @JsonIgnore - private final boolean cached; - @JsonProperty private final UUID guid; @@ -55,9 +47,7 @@ public class OutgoingMessageEntity { private final long serverTimestamp; @JsonCreator - public OutgoingMessageEntity(@JsonProperty("id") final long id, - @JsonProperty("cached") final boolean cached, - @JsonProperty("guid") final UUID guid, + public OutgoingMessageEntity(@JsonProperty("guid") final UUID guid, @JsonProperty("type") final int type, @JsonProperty("relay") final String relay, @JsonProperty("timestamp") final long timestamp, @@ -69,8 +59,6 @@ public class OutgoingMessageEntity { @JsonProperty("content") final byte[] content, @JsonProperty("serverTimestamp") final long serverTimestamp) { - this.id = id; - this.cached = cached; this.guid = guid; this.type = type; this.relay = relay; @@ -124,16 +112,6 @@ public class OutgoingMessageEntity { return content; } - @JsonIgnore - public long getId() { - return id; - } - - @JsonIgnore - public boolean isCached() { - return cached; - } - public long getServerTimestamp() { return serverTimestamp; } @@ -143,23 +121,22 @@ public class OutgoingMessageEntity { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; final OutgoingMessageEntity that = (OutgoingMessageEntity)o; - return id == that.id && - cached == that.cached && - type == that.type && + return type == that.type && timestamp == that.timestamp && sourceDevice == that.sourceDevice && serverTimestamp == that.serverTimestamp && - Objects.equals(guid, that.guid) && + guid.equals(that.guid) && Objects.equals(relay, that.relay) && Objects.equals(source, that.source) && Objects.equals(sourceUuid, that.sourceUuid) && + destinationUuid.equals(that.destinationUuid) && Arrays.equals(message, that.message) && Arrays.equals(content, that.content); } @Override public int hashCode() { - int result = Objects.hash(id, cached, guid, type, relay, timestamp, source, sourceUuid, sourceDevice, serverTimestamp); + int result = Objects.hash(guid, type, relay, timestamp, source, sourceUuid, sourceDevice, destinationUuid, serverTimestamp); result = 31 * result + Arrays.hashCode(message); result = 31 * result + Arrays.hashCode(content); return result; 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 3532610c1..a8107b32e 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagesCache.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagesCache.java @@ -167,7 +167,7 @@ public class MessagesCache extends RedisClusterPubSubAdapter imp for (final byte[] bytes : serialized) { try { - removedMessages.add(constructEntityFromEnvelope(0, MessageProtos.Envelope.parseFrom(bytes))); + removedMessages.add(constructEntityFromEnvelope(MessageProtos.Envelope.parseFrom(bytes))); } catch (final InvalidProtocolBufferException e) { logger.warn("Failed to parse envelope", e); } @@ -208,7 +208,7 @@ public class MessagesCache extends RedisClusterPubSubAdapter imp final long id = Long.parseLong(new String(queueItems.get(i + 1), StandardCharsets.UTF_8)); - messageEntities.add(constructEntityFromEnvelope(id, message)); + messageEntities.add(constructEntityFromEnvelope(message)); } catch (InvalidProtocolBufferException e) { logger.warn("Failed to parse envelope", e); } @@ -376,8 +376,8 @@ public class MessagesCache extends RedisClusterPubSubAdapter imp } @VisibleForTesting - static OutgoingMessageEntity constructEntityFromEnvelope(long id, MessageProtos.Envelope envelope) { - return new OutgoingMessageEntity(id, true, + static OutgoingMessageEntity constructEntityFromEnvelope(MessageProtos.Envelope envelope) { + return new OutgoingMessageEntity( envelope.hasServerGuid() ? UUID.fromString(envelope.getServerGuid()) : null, envelope.getType().getNumber(), envelope.getRelay(), 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 37b14c34d..865af0fb0 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagesDynamoDb.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagesDynamoDb.java @@ -228,7 +228,7 @@ public class MessagesDynamoDb extends AbstractDynamoDbStore { final UUID destinationUuid = AttributeValues.getUUID(message, KEY_DESTINATION_UUID, null); final byte[] messageBytes = AttributeValues.getByteArray(message, KEY_MESSAGE, null); final byte[] content = AttributeValues.getByteArray(message, KEY_CONTENT, null); - return new OutgoingMessageEntity(-1L, false, messageUuid, type, relay, timestamp, source, sourceUuid, sourceDevice, destinationUuid, messageBytes, content, sortKey.getServerTimestamp()); + return new OutgoingMessageEntity(messageUuid, type, relay, timestamp, source, sourceUuid, sourceDevice, destinationUuid, messageBytes, content, sortKey.getServerTimestamp()); } private void deleteRowsMatchingQuery(AttributeValue partitionKey, QueryRequest querySpec) { 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 9ccef1376..69e59a7be 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/MessageControllerTest.java @@ -459,8 +459,8 @@ class MessageControllerTest { final UUID sourceUuid = UUID.randomUUID(); List messages = new LinkedList<>() {{ - add(new OutgoingMessageEntity(1L, false, messageGuidOne, Envelope.Type.CIPHERTEXT_VALUE, null, timestampOne, "+14152222222", sourceUuid, 2, AuthHelper.VALID_UUID, "hi there".getBytes(), null, 0)); - add(new OutgoingMessageEntity(2L, false, null, Envelope.Type.SERVER_DELIVERY_RECEIPT_VALUE, null, timestampTwo, "+14152222222", sourceUuid, 2, AuthHelper.VALID_UUID, null, null, 0)); + add(new OutgoingMessageEntity(messageGuidOne, Envelope.Type.CIPHERTEXT_VALUE, null, timestampOne, "+14152222222", sourceUuid, 2, AuthHelper.VALID_UUID, "hi there".getBytes(), null, 0)); + add(new OutgoingMessageEntity(null, Envelope.Type.SERVER_DELIVERY_RECEIPT_VALUE, null, timestampTwo, "+14152222222", sourceUuid, 2, AuthHelper.VALID_UUID, null, null, 0)); }}; OutgoingMessageEntityList messagesList = new OutgoingMessageEntityList(messages, false); @@ -477,9 +477,6 @@ class MessageControllerTest { assertEquals(response.getMessages().size(), 2); - assertEquals(response.getMessages().get(0).getId(), 0); - assertEquals(response.getMessages().get(1).getId(), 0); - assertEquals(response.getMessages().get(0).getTimestamp(), timestampOne); assertEquals(response.getMessages().get(1).getTimestamp(), timestampTwo); @@ -496,8 +493,8 @@ class MessageControllerTest { final long timestampTwo = 313388; List messages = new LinkedList() {{ - add(new OutgoingMessageEntity(1L, false, UUID.randomUUID(), Envelope.Type.CIPHERTEXT_VALUE, null, timestampOne, "+14152222222", UUID.randomUUID(), 2, AuthHelper.VALID_UUID, "hi there".getBytes(), null, 0)); - add(new OutgoingMessageEntity(2L, false, UUID.randomUUID(), Envelope.Type.SERVER_DELIVERY_RECEIPT_VALUE, null, timestampTwo, "+14152222222", UUID.randomUUID(), 2, AuthHelper.VALID_UUID, null, null, 0)); + add(new OutgoingMessageEntity(UUID.randomUUID(), Envelope.Type.CIPHERTEXT_VALUE, null, timestampOne, "+14152222222", UUID.randomUUID(), 2, AuthHelper.VALID_UUID, "hi there".getBytes(), null, 0)); + add(new OutgoingMessageEntity(UUID.randomUUID(), Envelope.Type.SERVER_DELIVERY_RECEIPT_VALUE, null, timestampTwo, "+14152222222", UUID.randomUUID(), 2, AuthHelper.VALID_UUID, null, null, 0)); }}; OutgoingMessageEntityList messagesList = new OutgoingMessageEntityList(messages, false); @@ -522,12 +519,12 @@ class MessageControllerTest { UUID uuid1 = UUID.randomUUID(); when(messagesManager.delete(AuthHelper.VALID_UUID, 1, uuid1)).thenReturn(Optional.of(new OutgoingMessageEntity( - 31337L, true, uuid1, Envelope.Type.CIPHERTEXT_VALUE, + uuid1, Envelope.Type.CIPHERTEXT_VALUE, null, timestamp, "+14152222222", sourceUuid, 1, AuthHelper.VALID_UUID, "hi".getBytes(), 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, + uuid2, Envelope.Type.SERVER_DELIVERY_RECEIPT_VALUE, null, System.currentTimeMillis(), "+14152222222", sourceUuid, 1, AuthHelper.VALID_UUID, null, null, 0))); 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 cd178de67..acc333a35 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/MessagesCacheTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/MessagesCacheTest.java @@ -104,7 +104,7 @@ public class MessagesCacheTest extends AbstractRedisClusterTest { DESTINATION_DEVICE_ID, messageGuid); assertTrue(maybeRemovedMessage.isPresent()); - assertEquals(MessagesCache.constructEntityFromEnvelope(0, message), maybeRemovedMessage.get()); + assertEquals(MessagesCache.constructEntityFromEnvelope(message), maybeRemovedMessage.get()); } @Test @@ -136,7 +136,7 @@ public class MessagesCacheTest extends AbstractRedisClusterTest { messagesToRemove.stream().map(message -> UUID.fromString(message.getServerGuid())) .collect(Collectors.toList())); - assertEquals(messagesToRemove.stream().map(message -> MessagesCache.constructEntityFromEnvelope(0, message)) + assertEquals(messagesToRemove.stream().map(MessagesCache::constructEntityFromEnvelope) .collect(Collectors.toList()), removedMessages); @@ -167,7 +167,7 @@ public class MessagesCacheTest extends AbstractRedisClusterTest { final MessageProtos.Envelope message = generateRandomMessage(messageGuid, sealedSender); final long messageId = messagesCache.insert(messageGuid, DESTINATION_UUID, DESTINATION_DEVICE_ID, message); - expectedMessages.add(MessagesCache.constructEntityFromEnvelope(messageId, message)); + expectedMessages.add(MessagesCache.constructEntityFromEnvelope(message)); } assertEquals(expectedMessages, messagesCache.get(DESTINATION_UUID, DESTINATION_DEVICE_ID, messageCount)); @@ -224,7 +224,8 @@ public class MessagesCacheTest extends AbstractRedisClusterTest { .setServerTimestamp(timestamp) .setContent(ByteString.copyFromUtf8(RandomStringUtils.randomAlphanumeric(256))) .setType(MessageProtos.Envelope.Type.CIPHERTEXT) - .setServerGuid(messageGuid.toString()); + .setServerGuid(messageGuid.toString()) + .setDestinationUuid(UUID.randomUUID().toString()); if (!sealedSender) { envelopeBuilder.setSourceDevice(random.nextInt(256)) @@ -270,7 +271,7 @@ public class MessagesCacheTest extends AbstractRedisClusterTest { messagesCache.insert(messageGuid, DESTINATION_UUID, DESTINATION_DEVICE_ID, generateRandomMessage(messageGuid, sealedSender)); - final int slot = SlotHash.getSlot(DESTINATION_UUID.toString() + "::" + DESTINATION_DEVICE_ID); + final int slot = SlotHash.getSlot(DESTINATION_UUID + "::" + DESTINATION_DEVICE_ID); assertTrue(messagesCache.getQueuesToPersist(slot + 1, Instant.now().plusSeconds(60), 100).isEmpty()); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnectionTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnectionTest.java index 7af836f01..e61f69ad6 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnectionTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnectionTest.java @@ -151,9 +151,9 @@ public class WebSocketConnectionTest { UUID senderTwoUuid = UUID.randomUUID(); List outgoingMessages = new LinkedList () {{ - add(createMessage(1L, false, "sender1", senderOneUuid, UUID.randomUUID(), 1111, false, "first")); - add(createMessage(2L, false, "sender1", senderOneUuid, UUID.randomUUID(), 2222, false, "second")); - add(createMessage(3L, false, "sender2", senderTwoUuid, UUID.randomUUID(), 3333, false, "third")); + add(createMessage("sender1", senderOneUuid, UUID.randomUUID(), 1111, false, "first")); + add(createMessage("sender1", senderOneUuid, UUID.randomUUID(), 2222, false, "second")); + add(createMessage("sender2", senderTwoUuid, UUID.randomUUID(), 3333, false, "third")); }}; OutgoingMessageEntityList outgoingMessagesList = new OutgoingMessageEntityList(outgoingMessages, false); @@ -233,8 +233,8 @@ public class WebSocketConnectionTest { when(messagesManager.getMessagesForDevice(eq(accountUuid), eq(1L), eq("Test-UA"), anyBoolean())) .thenReturn(new OutgoingMessageEntityList(Collections.emptyList(), false)) - .thenReturn(new OutgoingMessageEntityList(List.of(createMessage(1L, false, "sender1", UUID.randomUUID(), UUID.randomUUID(), 1111, false, "first")), false)) - .thenReturn(new OutgoingMessageEntityList(List.of(createMessage(2L, false, "sender1", UUID.randomUUID(), UUID.randomUUID(), 2222, false, "second")), false)); + .thenReturn(new OutgoingMessageEntityList(List.of(createMessage("sender1", UUID.randomUUID(), UUID.randomUUID(), 1111, false, "first")), false)) + .thenReturn(new OutgoingMessageEntityList(List.of(createMessage("sender1", UUID.randomUUID(), UUID.randomUUID(), 2222, false, "second")), false)); final WebSocketResponseMessage successResponse = mock(WebSocketResponseMessage.class); when(successResponse.getStatus()).thenReturn(200); @@ -303,11 +303,11 @@ public class WebSocketConnectionTest { .build(); List pendingMessages = new LinkedList() {{ - add(new OutgoingMessageEntity(1, true, UUID.randomUUID(), firstMessage.getType().getNumber(), firstMessage.getRelay(), + add(new OutgoingMessageEntity(UUID.randomUUID(), firstMessage.getType().getNumber(), firstMessage.getRelay(), firstMessage.getTimestamp(), firstMessage.getSource(), UUID.fromString(firstMessage.getSourceUuid()), firstMessage.getSourceDevice(), UUID.fromString(firstMessage.getDestinationUuid()), firstMessage.getLegacyMessage().toByteArray(), firstMessage.getContent().toByteArray(), 0)); - add(new OutgoingMessageEntity(2, false, UUID.randomUUID(), secondMessage.getType().getNumber(), secondMessage.getRelay(), + add(new OutgoingMessageEntity(UUID.randomUUID(), secondMessage.getType().getNumber(), secondMessage.getRelay(), secondMessage.getTimestamp(), secondMessage.getSource(), UUID.fromString(secondMessage.getSourceUuid()), secondMessage.getSourceDevice(), UUID.fromString(secondMessage.getDestinationUuid()), secondMessage.getLegacyMessage().toByteArray(), secondMessage.getContent().toByteArray(), 0)); @@ -448,11 +448,11 @@ public class WebSocketConnectionTest { when(client.getUserAgent()).thenReturn("Test-UA"); final List firstPageMessages = - List.of(createMessage(1L, false, "sender1", UUID.randomUUID(), UUID.randomUUID(), 1111, false, "first"), - createMessage(2L, false, "sender1", UUID.randomUUID(), UUID.randomUUID(), 2222, false, "second")); + List.of(createMessage("sender1", UUID.randomUUID(), UUID.randomUUID(), 1111, false, "first"), + createMessage("sender1", UUID.randomUUID(), UUID.randomUUID(), 2222, false, "second")); final List secondPageMessages = - List.of(createMessage(3L, false, "sender1", UUID.randomUUID(), UUID.randomUUID(), 3333, false, "third")); + List.of(createMessage("sender1", UUID.randomUUID(), UUID.randomUUID(), 3333, false, "third")); final OutgoingMessageEntityList firstPage = new OutgoingMessageEntityList(firstPageMessages, true); final OutgoingMessageEntityList secondPage = new OutgoingMessageEntityList(secondPageMessages, false); @@ -493,7 +493,7 @@ public class WebSocketConnectionTest { final UUID senderUuid = UUID.randomUUID(); final List messages = List.of( - createMessage(1L, false, "senderE164", senderUuid, UUID.randomUUID(), 1111L, false, "message the first")); + createMessage("senderE164", senderUuid, UUID.randomUUID(), 1111L, false, "message the first")); final OutgoingMessageEntityList firstPage = new OutgoingMessageEntityList(messages, false); when(messagesManager.getMessagesForDevice(account.getUuid(), 1L, client.getUserAgent(), false)).thenReturn(firstPage); @@ -575,11 +575,11 @@ public class WebSocketConnectionTest { when(client.getUserAgent()).thenReturn("Test-UA"); final List firstPageMessages = - List.of(createMessage(1L, false, "sender1", UUID.randomUUID(), UUID.randomUUID(), 1111, false, "first"), - createMessage(2L, false, "sender1", UUID.randomUUID(), UUID.randomUUID(), 2222, false, "second")); + List.of(createMessage("sender1", UUID.randomUUID(), UUID.randomUUID(), 1111, false, "first"), + createMessage("sender1", UUID.randomUUID(), UUID.randomUUID(), 2222, false, "second")); final List secondPageMessages = - List.of(createMessage(3L, false, "sender1", UUID.randomUUID(), UUID.randomUUID(), 3333, false, "third")); + List.of(createMessage("sender1", UUID.randomUUID(), UUID.randomUUID(), 3333, false, "third")); final OutgoingMessageEntityList firstPage = new OutgoingMessageEntityList(firstPageMessages, false); final OutgoingMessageEntityList secondPage = new OutgoingMessageEntityList(secondPageMessages, false); @@ -681,9 +681,9 @@ public class WebSocketConnectionTest { UUID senderTwoUuid = UUID.randomUUID(); List outgoingMessages = new LinkedList () {{ - add(createMessage(1L, false, "sender1", senderOneUuid, UUID.randomUUID(), 1111, false, "first")); - add(createMessage(2L, false, "sender1", senderOneUuid, UUID.randomUUID(), 2222, false, RandomStringUtils.randomAlphanumeric(WebSocketConnection.MAX_DESKTOP_MESSAGE_SIZE + 1))); - add(createMessage(3L, false, "sender2", senderTwoUuid, UUID.randomUUID(), 3333, false, "third")); + add(createMessage("sender1", senderOneUuid, UUID.randomUUID(), 1111, false, "first")); + add(createMessage("sender1", senderOneUuid, UUID.randomUUID(), 2222, false, RandomStringUtils.randomAlphanumeric(WebSocketConnection.MAX_DESKTOP_MESSAGE_SIZE + 1))); + add(createMessage("sender2", senderTwoUuid, UUID.randomUUID(), 3333, false, "third")); }}; OutgoingMessageEntityList outgoingMessagesList = new OutgoingMessageEntityList(outgoingMessages, false); @@ -757,9 +757,9 @@ public class WebSocketConnectionTest { UUID senderTwoUuid = UUID.randomUUID(); List outgoingMessages = new LinkedList () {{ - add(createMessage(1L, false, "sender1", senderOneUuid, UUID.randomUUID(), 1111, false, "first")); - add(createMessage(2L, false, "sender1", senderOneUuid, UUID.randomUUID(), 2222, false, RandomStringUtils.randomAlphanumeric(WebSocketConnection.MAX_DESKTOP_MESSAGE_SIZE + 1))); - add(createMessage(3L, false, "sender2", senderTwoUuid, UUID.randomUUID(), 3333, false, "third")); + add(createMessage("sender1", senderOneUuid, UUID.randomUUID(), 1111, false, "first")); + add(createMessage("sender1", senderOneUuid, UUID.randomUUID(), 2222, false, RandomStringUtils.randomAlphanumeric(WebSocketConnection.MAX_DESKTOP_MESSAGE_SIZE + 1))); + add(createMessage("sender2", senderTwoUuid, UUID.randomUUID(), 3333, false, "third")); }}; OutgoingMessageEntityList outgoingMessagesList = new OutgoingMessageEntityList(outgoingMessages, false); @@ -884,8 +884,8 @@ public class WebSocketConnectionTest { verify(client, never()).close(anyInt(), anyString()); } - private OutgoingMessageEntity createMessage(long id, boolean cached, String sender, UUID senderUuid, UUID destinationUuid, long timestamp, boolean receipt, String content) { - return new OutgoingMessageEntity(id, cached, UUID.randomUUID(), receipt ? Envelope.Type.SERVER_DELIVERY_RECEIPT_VALUE : Envelope.Type.CIPHERTEXT_VALUE, + private OutgoingMessageEntity createMessage(String sender, UUID senderUuid, UUID destinationUuid, long timestamp, boolean receipt, String content) { + return new OutgoingMessageEntity(UUID.randomUUID(), receipt ? Envelope.Type.SERVER_DELIVERY_RECEIPT_VALUE : Envelope.Type.CIPHERTEXT_VALUE, null, timestamp, sender, senderUuid, 1, destinationUuid, content.getBytes(), null, 0); }