diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index 031f23024..9320f6c3d 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -188,7 +188,7 @@ import org.whispersystems.textsecuregcm.storage.MessagesDynamoDb; import org.whispersystems.textsecuregcm.storage.MessagesManager; import org.whispersystems.textsecuregcm.storage.NonNormalizedAccountCrawlerListener; import org.whispersystems.textsecuregcm.storage.PhoneNumberIdentifiers; -import org.whispersystems.textsecuregcm.storage.ProfilesDynamoDb; +import org.whispersystems.textsecuregcm.storage.Profiles; import org.whispersystems.textsecuregcm.storage.ProfilesManager; import org.whispersystems.textsecuregcm.storage.PubSubManager; import org.whispersystems.textsecuregcm.storage.PushChallengeDynamoDb; @@ -385,7 +385,7 @@ public class WhisperServerService extends Application { dynamoDbClient.updateItem(UpdateItemRequest.builder() @@ -187,7 +186,6 @@ public class ProfilesDynamoDb implements ProfilesStore { return expressionValues; } - @Override public Optional get(final UUID uuid, final String version) { return GET_PROFILE_TIMER.record(() -> { final GetItemResponse response = dynamoDbClient.getItem(GetItemRequest.builder() @@ -211,7 +209,6 @@ public class ProfilesDynamoDb implements ProfilesStore { AttributeValues.getByteArray(item, ATTR_COMMITMENT, null)); } - @Override public void deleteAll(final UUID uuid) { DELETE_PROFILES_TIMER.record(() -> { final AttributeValue uuidAttributeValue = AttributeValues.fromUUID(uuid); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ProfilesManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ProfilesManager.java index 4fac73405..cf993c084 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ProfilesManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ProfilesManager.java @@ -22,31 +22,32 @@ public class ProfilesManager { private static final String CACHE_PREFIX = "profiles::"; - private final ProfilesDynamoDb profilesDynamoDb; + private final Profiles profiles; private final FaultTolerantRedisCluster cacheCluster; private final ObjectMapper mapper; - public ProfilesManager(final ProfilesDynamoDb profilesDynamoDb, final FaultTolerantRedisCluster cacheCluster) { - this.profilesDynamoDb = profilesDynamoDb; + public ProfilesManager(final Profiles profiles, + final FaultTolerantRedisCluster cacheCluster) { + this.profiles = profiles; this.cacheCluster = cacheCluster; this.mapper = SystemMapper.getMapper(); } public void set(UUID uuid, VersionedProfile versionedProfile) { memcacheSet(uuid, versionedProfile); - profilesDynamoDb.set(uuid, versionedProfile); + profiles.set(uuid, versionedProfile); } public void deleteAll(UUID uuid) { memcacheDelete(uuid); - profilesDynamoDb.deleteAll(uuid); + profiles.deleteAll(uuid); } public Optional get(UUID uuid, String version) { Optional profile = memcacheGet(uuid, version); if (profile.isEmpty()) { - profile = profilesDynamoDb.get(uuid, version); + profile = profiles.get(uuid, version); profile.ifPresent(versionedProfile -> memcacheSet(uuid, versionedProfile)); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ProfilesStore.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ProfilesStore.java deleted file mode 100644 index b7f513889..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ProfilesStore.java +++ /dev/null @@ -1,18 +0,0 @@ -/* - * Copyright 2013-2021 Signal Messenger, LLC - * SPDX-License-Identifier: AGPL-3.0-only - */ - -package org.whispersystems.textsecuregcm.storage; - -import java.util.Optional; -import java.util.UUID; - -public interface ProfilesStore { - - void set(UUID uuid, VersionedProfile profile); - - Optional get(UUID uuid, String version); - - void deleteAll(UUID uuid); -} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java b/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java index 0fbf896d1..d25be362f 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/DeleteUserCommand.java @@ -49,7 +49,7 @@ import org.whispersystems.textsecuregcm.storage.MessagesCache; import org.whispersystems.textsecuregcm.storage.MessagesDynamoDb; import org.whispersystems.textsecuregcm.storage.MessagesManager; import org.whispersystems.textsecuregcm.storage.PhoneNumberIdentifiers; -import org.whispersystems.textsecuregcm.storage.ProfilesDynamoDb; +import org.whispersystems.textsecuregcm.storage.Profiles; import org.whispersystems.textsecuregcm.storage.ProfilesManager; import org.whispersystems.textsecuregcm.storage.ReportMessageDynamoDb; import org.whispersystems.textsecuregcm.storage.ReportMessageManager; @@ -176,7 +176,7 @@ public class DeleteUserCommand extends EnvironmentCommand buildUpdateExpression() { - final byte[] commitment = "commitment".getBytes(StandardCharsets.UTF_8); - - return Stream.of( - Arguments.of( - new VersionedProfile("version", "name", "avatar", "emoji", "about", "paymentAddress", commitment), - "SET #commitment = if_not_exists(#commitment, :commitment), #name = :name, #avatar = :avatar, #about = :about, #aboutEmoji = :aboutEmoji, #paymentAddress = :paymentAddress"), - - Arguments.of( - new VersionedProfile("version", "name", "avatar", "emoji", "about", null, commitment), - "SET #commitment = if_not_exists(#commitment, :commitment), #name = :name, #avatar = :avatar, #about = :about, #aboutEmoji = :aboutEmoji REMOVE #paymentAddress"), - - Arguments.of( - new VersionedProfile("version", "name", "avatar", "emoji", null, null, commitment), - "SET #commitment = if_not_exists(#commitment, :commitment), #name = :name, #avatar = :avatar, #aboutEmoji = :aboutEmoji REMOVE #about, #paymentAddress"), - - Arguments.of( - new VersionedProfile("version", "name", "avatar", null, null, null, commitment), - "SET #commitment = if_not_exists(#commitment, :commitment), #name = :name, #avatar = :avatar REMOVE #about, #aboutEmoji, #paymentAddress"), - - Arguments.of( - new VersionedProfile("version", "name", null, null, null, null, commitment), - "SET #commitment = if_not_exists(#commitment, :commitment), #name = :name REMOVE #avatar, #about, #aboutEmoji, #paymentAddress"), - - Arguments.of( - new VersionedProfile("version", null, null, null, null, null, commitment), - "SET #commitment = if_not_exists(#commitment, :commitment) REMOVE #name, #avatar, #about, #aboutEmoji, #paymentAddress") - ); - } - - @ParameterizedTest - @MethodSource - void buildUpdateExpressionAttributeValues(final VersionedProfile profile, final Map expectedAttributeValues) { - assertEquals(expectedAttributeValues, ProfilesDynamoDb.buildUpdateExpressionAttributeValues(profile)); - } - - private static Stream buildUpdateExpressionAttributeValues() { - final byte[] commitment = "commitment".getBytes(StandardCharsets.UTF_8); - - return Stream.of( - Arguments.of( - new VersionedProfile("version", "name", "avatar", "emoji", "about", "paymentAddress", commitment), - Map.of( - ":commitment", AttributeValues.fromByteArray(commitment), - ":name", AttributeValues.fromString("name"), - ":avatar", AttributeValues.fromString("avatar"), - ":aboutEmoji", AttributeValues.fromString("emoji"), - ":about", AttributeValues.fromString("about"), - ":paymentAddress", AttributeValues.fromString("paymentAddress"))), - - Arguments.of( - new VersionedProfile("version", "name", "avatar", "emoji", "about", null, commitment), - Map.of( - ":commitment", AttributeValues.fromByteArray(commitment), - ":name", AttributeValues.fromString("name"), - ":avatar", AttributeValues.fromString("avatar"), - ":aboutEmoji", AttributeValues.fromString("emoji"), - ":about", AttributeValues.fromString("about"))), - - Arguments.of( - new VersionedProfile("version", "name", "avatar", "emoji", null, null, commitment), - Map.of( - ":commitment", AttributeValues.fromByteArray(commitment), - ":name", AttributeValues.fromString("name"), - ":avatar", AttributeValues.fromString("avatar"), - ":aboutEmoji", AttributeValues.fromString("emoji"))), - - Arguments.of( - new VersionedProfile("version", "name", "avatar", null, null, null, commitment), - Map.of( - ":commitment", AttributeValues.fromByteArray(commitment), - ":name", AttributeValues.fromString("name"), - ":avatar", AttributeValues.fromString("avatar"))), - - Arguments.of( - new VersionedProfile("version", "name", null, null, null, null, commitment), - Map.of( - ":commitment", AttributeValues.fromByteArray(commitment), - ":name", AttributeValues.fromString("name"))), - - Arguments.of( - new VersionedProfile("version", null, null, null, null, null, commitment), - Map.of(":commitment", AttributeValues.fromByteArray(commitment))) - ); - } -} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ProfilesTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ProfilesTest.java index 6c98970f8..328a24211 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ProfilesTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ProfilesTest.java @@ -5,20 +5,56 @@ package org.whispersystems.textsecuregcm.storage; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.whispersystems.textsecuregcm.util.AttributeValues; +import software.amazon.awssdk.services.dynamodb.model.AttributeDefinition; +import software.amazon.awssdk.services.dynamodb.model.AttributeValue; +import software.amazon.awssdk.services.dynamodb.model.ScalarAttributeType; + import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; import java.nio.charset.StandardCharsets; +import java.util.Map; import java.util.Optional; import java.util.UUID; -import org.junit.jupiter.api.Test; +import java.util.stream.Stream; public abstract class ProfilesTest { - protected abstract ProfilesStore getProfilesStore(); + private static final String PROFILES_TABLE_NAME = "profiles_test"; + + @RegisterExtension + static DynamoDbExtension dynamoDbExtension = DynamoDbExtension.builder() + .tableName(PROFILES_TABLE_NAME) + .hashKey(Profiles.KEY_ACCOUNT_UUID) + .rangeKey(Profiles.ATTR_VERSION) + .attributeDefinition(AttributeDefinition.builder() + .attributeName(Profiles.KEY_ACCOUNT_UUID) + .attributeType(ScalarAttributeType.B) + .build()) + .attributeDefinition(AttributeDefinition.builder() + .attributeName(Profiles.ATTR_VERSION) + .attributeType(ScalarAttributeType.S) + .build()) + .build(); + + private Profiles profiles; + + @BeforeEach + void setUp() { + profiles = new Profiles(dynamoDbExtension.getDynamoDbClient(), + dynamoDbExtension.getDynamoDbAsyncClient(), + PROFILES_TABLE_NAME); + } @Test void testSetGet() { - ProfilesStore profiles = getProfilesStore(); UUID uuid = UUID.randomUUID(); VersionedProfile profile = new VersionedProfile("123", "foo", "avatarLocation", "emoji", "the very model of a modern major general", @@ -37,7 +73,6 @@ public abstract class ProfilesTest { @Test void testDeleteReset() { - ProfilesStore profiles = getProfilesStore(); UUID uuid = UUID.randomUUID(); profiles.set(uuid, new VersionedProfile("123", "foo", "avatarLocation", "emoji", "the very model of a modern major general", @@ -62,7 +97,6 @@ public abstract class ProfilesTest { @Test void testSetGetNullOptionalFields() { - ProfilesStore profiles = getProfilesStore(); UUID uuid = UUID.randomUUID(); VersionedProfile profile = new VersionedProfile("123", "foo", null, null, null, null, "acommitment".getBytes()); @@ -80,7 +114,6 @@ public abstract class ProfilesTest { @Test void testSetReplace() { - ProfilesStore profiles = getProfilesStore(); UUID uuid = UUID.randomUUID(); VersionedProfile profile = new VersionedProfile("123", "foo", "avatarLocation", null, null, "paymentAddress", "acommitment".getBytes()); @@ -113,7 +146,6 @@ public abstract class ProfilesTest { @Test void testMultipleVersions() { - ProfilesStore profiles = getProfilesStore(); UUID uuid = UUID.randomUUID(); VersionedProfile profileOne = new VersionedProfile("123", "foo", "avatarLocation", null, null, null, "acommitmnet".getBytes()); @@ -145,7 +177,6 @@ public abstract class ProfilesTest { @Test void testMissing() { - ProfilesStore profiles = getProfilesStore(); UUID uuid = UUID.randomUUID(); VersionedProfile profile = new VersionedProfile("123", "foo", "avatarLocation", null, null, null, "aDigest".getBytes()); @@ -158,7 +189,6 @@ public abstract class ProfilesTest { @Test void testDelete() { - ProfilesStore profiles = getProfilesStore(); UUID uuid = UUID.randomUUID(); VersionedProfile profileOne = new VersionedProfile("123", "foo", "avatarLocation", null, null, null, "aDigest".getBytes()); @@ -177,4 +207,96 @@ public abstract class ProfilesTest { assertThat(retrieved.isPresent()).isFalse(); } + + @ParameterizedTest + @MethodSource + void buildUpdateExpression(final VersionedProfile profile, final String expectedUpdateExpression) { + assertEquals(expectedUpdateExpression, Profiles.buildUpdateExpression(profile)); + } + + private static Stream buildUpdateExpression() { + final byte[] commitment = "commitment".getBytes(StandardCharsets.UTF_8); + + return Stream.of( + Arguments.of( + new VersionedProfile("version", "name", "avatar", "emoji", "about", "paymentAddress", commitment), + "SET #commitment = if_not_exists(#commitment, :commitment), #name = :name, #avatar = :avatar, #about = :about, #aboutEmoji = :aboutEmoji, #paymentAddress = :paymentAddress"), + + Arguments.of( + new VersionedProfile("version", "name", "avatar", "emoji", "about", null, commitment), + "SET #commitment = if_not_exists(#commitment, :commitment), #name = :name, #avatar = :avatar, #about = :about, #aboutEmoji = :aboutEmoji REMOVE #paymentAddress"), + + Arguments.of( + new VersionedProfile("version", "name", "avatar", "emoji", null, null, commitment), + "SET #commitment = if_not_exists(#commitment, :commitment), #name = :name, #avatar = :avatar, #aboutEmoji = :aboutEmoji REMOVE #about, #paymentAddress"), + + Arguments.of( + new VersionedProfile("version", "name", "avatar", null, null, null, commitment), + "SET #commitment = if_not_exists(#commitment, :commitment), #name = :name, #avatar = :avatar REMOVE #about, #aboutEmoji, #paymentAddress"), + + Arguments.of( + new VersionedProfile("version", "name", null, null, null, null, commitment), + "SET #commitment = if_not_exists(#commitment, :commitment), #name = :name REMOVE #avatar, #about, #aboutEmoji, #paymentAddress"), + + Arguments.of( + new VersionedProfile("version", null, null, null, null, null, commitment), + "SET #commitment = if_not_exists(#commitment, :commitment) REMOVE #name, #avatar, #about, #aboutEmoji, #paymentAddress") + ); + } + + @ParameterizedTest + @MethodSource + void buildUpdateExpressionAttributeValues(final VersionedProfile profile, final Map expectedAttributeValues) { + assertEquals(expectedAttributeValues, Profiles.buildUpdateExpressionAttributeValues(profile)); + } + + private static Stream buildUpdateExpressionAttributeValues() { + final byte[] commitment = "commitment".getBytes(StandardCharsets.UTF_8); + + return Stream.of( + Arguments.of( + new VersionedProfile("version", "name", "avatar", "emoji", "about", "paymentAddress", commitment), + Map.of( + ":commitment", AttributeValues.fromByteArray(commitment), + ":name", AttributeValues.fromString("name"), + ":avatar", AttributeValues.fromString("avatar"), + ":aboutEmoji", AttributeValues.fromString("emoji"), + ":about", AttributeValues.fromString("about"), + ":paymentAddress", AttributeValues.fromString("paymentAddress"))), + + Arguments.of( + new VersionedProfile("version", "name", "avatar", "emoji", "about", null, commitment), + Map.of( + ":commitment", AttributeValues.fromByteArray(commitment), + ":name", AttributeValues.fromString("name"), + ":avatar", AttributeValues.fromString("avatar"), + ":aboutEmoji", AttributeValues.fromString("emoji"), + ":about", AttributeValues.fromString("about"))), + + Arguments.of( + new VersionedProfile("version", "name", "avatar", "emoji", null, null, commitment), + Map.of( + ":commitment", AttributeValues.fromByteArray(commitment), + ":name", AttributeValues.fromString("name"), + ":avatar", AttributeValues.fromString("avatar"), + ":aboutEmoji", AttributeValues.fromString("emoji"))), + + Arguments.of( + new VersionedProfile("version", "name", "avatar", null, null, null, commitment), + Map.of( + ":commitment", AttributeValues.fromByteArray(commitment), + ":name", AttributeValues.fromString("name"), + ":avatar", AttributeValues.fromString("avatar"))), + + Arguments.of( + new VersionedProfile("version", "name", null, null, null, null, commitment), + Map.of( + ":commitment", AttributeValues.fromByteArray(commitment), + ":name", AttributeValues.fromString("name"))), + + Arguments.of( + new VersionedProfile("version", null, null, null, null, null, commitment), + Map.of(":commitment", AttributeValues.fromByteArray(commitment))) + ); + } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/ProfilesManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/ProfilesManagerTest.java index aed7518e8..e3bc5ed44 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/ProfilesManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/storage/ProfilesManagerTest.java @@ -25,14 +25,14 @@ import java.util.UUID; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisCluster; -import org.whispersystems.textsecuregcm.storage.ProfilesDynamoDb; +import org.whispersystems.textsecuregcm.storage.Profiles; import org.whispersystems.textsecuregcm.storage.ProfilesManager; import org.whispersystems.textsecuregcm.storage.VersionedProfile; import org.whispersystems.textsecuregcm.tests.util.RedisClusterHelper; public class ProfilesManagerTest { - private ProfilesDynamoDb profiles; + private Profiles profiles; private RedisAdvancedClusterCommands commands; private ProfilesManager profilesManager; @@ -43,7 +43,7 @@ public class ProfilesManagerTest { commands = mock(RedisAdvancedClusterCommands.class); final FaultTolerantRedisCluster cacheCluster = RedisClusterHelper.buildMockRedisCluster(commands); - profiles = mock(ProfilesDynamoDb.class); + profiles = mock(Profiles.class); profilesManager = new ProfilesManager(profiles, cacheCluster); }