MirrorMirror
/
Signal-Server
mirror of https://github.com/signalapp/Signal-Server.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove unversioned profile properties from `Account` entities

This commit is contained in:
Jon Chambers 2021-12-06 15:50:27 -05:00 committed by Jon Chambers
parent 2b2e26f14b
commit 4ea7278c6f
6 changed files with 18 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
service/src/main/java/org/whispersystems/textsecuregcm/controllers/ProfileController.java
View File

@ -168,11 +168,6 @@ public class ProfileController {
currentAvatar = Optional.of(currentProfile.get().getAvatar());
}
if (currentAvatar.isEmpty() && auth.getAccount().getAvatar() != null && auth.getAccount().getAvatar()
.startsWith("profiles/")) {
currentAvatar = Optional.of(auth.getAccount().getAvatar());
}
currentAvatar.ifPresent(s -> s3client.deleteObject(DeleteObjectRequest.builder()
.bucket(bucket)
.key(s)
@ -186,8 +181,6 @@ public class ProfileController {
.orElseGet(() -> auth.getAccount().getBadges());
accountsManager.update(auth.getAccount(), a -> {
a.setProfileName(request.getName());
a.setAvatar(avatar);
a.setBadges(clock, updatedBadges);
a.setCurrentProfileVersion(request.getVersion());
});
@ -261,10 +254,10 @@ public class ProfileController {
Optional<String> username = accountProfile.flatMap(Account::getUsername);
Optional<VersionedProfile> profile = profilesManager.get(uuid, version);
String name = profile.map(VersionedProfile::getName).orElse(accountProfile.get().getProfileName());
String name = profile.map(VersionedProfile::getName).orElse(null);
String about = profile.map(VersionedProfile::getAbout).orElse(null);
String aboutEmoji = profile.map(VersionedProfile::getAboutEmoji).orElse(null);
String avatar = profile.map(VersionedProfile::getAvatar).orElse(accountProfile.get().getAvatar());
String avatar = profile.map(VersionedProfile::getAvatar).orElse(null);
Optional<String> currentProfileVersion = accountProfile.get().getCurrentProfileVersion();
// Allow requests where either the version matches the latest version on Account or the latest version on Account
@ -339,10 +332,10 @@ public class ProfileController {
final boolean isSelf = auth.getAccount().getUuid().equals(accountProfile.getUuid());
return new Profile(
accountProfile.getProfileName(),
null,
null,
accountProfile.getAvatar(),
null,
null,
null,
accountProfile.getIdentityKey(),
UnidentifiedAccessChecksum.generateFor(accountProfile.getUnidentifiedAccessKey()),
@ -418,10 +411,10 @@ public class ProfileController {
Optional<String> username = accountProfile.flatMap(Account::getUsername);
return new Profile(
accountProfile.get().getProfileName(),
null,
null,
accountProfile.get().getAvatar(),
null,
null,
null,
accountProfile.get().getIdentityKey(),
UnidentifiedAccessChecksum.generateFor(accountProfile.get().getUnidentifiedAccessKey()),

30
service/src/main/java/org/whispersystems/textsecuregcm/storage/Account.java
View File

@ -55,12 +55,6 @@ public class Account {
@JsonProperty("cpv")
private String currentProfileVersion;
@JsonProperty
private String name;
@JsonProperty
private String avatar;
@JsonProperty
private List<AccountBadge> badges = new ArrayList<>();
@ -340,30 +334,6 @@ public class Account {
this.currentProfileVersion = currentProfileVersion;
}
public String getProfileName() {
requireNotStale();
return name;
}
public void setProfileName(String name) {
requireNotStale();
this.name = name;
}
public String getAvatar() {
requireNotStale();
return avatar;
}
public void setAvatar(String avatar) {
requireNotStale();
this.avatar = avatar;
}
public List<AccountBadge> getBadges() {
requireNotStale();

10
service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerConcurrentModificationIntegrationTest.java
View File

@ -189,8 +189,6 @@ class AccountsManagerConcurrentModificationIntegrationTest {
uuid = account.getUuid();
}
final String profileName = "name";
final String avatar = "avatar";
final boolean discoverableByPhoneNumber = false;
final String currentProfileVersion = "cpv";
final String identityKey = "ikey";
@ -202,8 +200,6 @@ class AccountsManagerConcurrentModificationIntegrationTest {
final long lastSeen = Instant.now().getEpochSecond();
CompletableFuture.allOf(
modifyAccount(uuid, account -> account.setProfileName(profileName)),
modifyAccount(uuid, account -> account.setAvatar(avatar)),
modifyAccount(uuid, account -> account.setDiscoverableByPhoneNumber(discoverableByPhoneNumber)),
modifyAccount(uuid, account -> account.setCurrentProfileVersion(currentProfileVersion)),
modifyAccount(uuid, account -> account.setIdentityKey(identityKey)),
@ -224,7 +220,7 @@ class AccountsManagerConcurrentModificationIntegrationTest {
new Pair<>("dynamo", dynamoAccount),
new Pair<>("redis", redisAccount)
).forEach(pair ->
verifyAccount(pair.first(), pair.second(), profileName, avatar, discoverableByPhoneNumber,
verifyAccount(pair.first(), pair.second(), discoverableByPhoneNumber,
currentProfileVersion, identityKey, unidentifiedAccessKey, pin, registrationLock,
unrestrictedUnidentifiedAccess, lastSeen));
}
@ -237,11 +233,9 @@ class AccountsManagerConcurrentModificationIntegrationTest {
return JsonHelpers.fromJson(redisSetArgumentCapture.getValue(), Account.class);
}
private void verifyAccount(final String name, final Account account, final String profileName, final String avatar, final boolean discoverableByPhoneNumber, final String currentProfileVersion, final String identityKey, final byte[] unidentifiedAccessKey, final String pin, final String clientRegistrationLock, final boolean unrestrictedUnidentifiedAcces, final long lastSeen) {
private void verifyAccount(final String name, final Account account, final boolean discoverableByPhoneNumber, final String currentProfileVersion, final String identityKey, final byte[] unidentifiedAccessKey, final String pin, final String clientRegistrationLock, final boolean unrestrictedUnidentifiedAcces, final long lastSeen) {
assertAll(name,
() -> assertEquals(profileName, account.getProfileName()),
() -> assertEquals(avatar, account.getAvatar()),
() -> assertEquals(discoverableByPhoneNumber, account.isDiscoverableByPhoneNumber()),
() -> assertEquals(currentProfileVersion, account.getCurrentProfileVersion().orElseThrow()),
() -> assertEquals(identityKey, account.getIdentityKey()),

18
service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java
View File

@ -68,8 +68,6 @@ class AccountsManagerTest {
private RedisAdvancedClusterCommands<String, String> commands;
private AccountsManager accountsManager;
private static final UUID RESERVED_FOR_UUID = UUID.randomUUID();
private static final Answer<?> ACCOUNT_UPDATE_ANSWER = (answer) -> {
// it is implicit in the update() contract is that a successful call will
// result in an incremented version
@ -150,13 +148,12 @@ class AccountsManagerTest {
UUID uuid = UUID.randomUUID();
when(commands.get(eq("AccountMap::+14152222222"))).thenReturn(uuid.toString());
when(commands.get(eq("Account3::" + uuid))).thenReturn("{\"number\": \"+14152222222\", \"name\": \"test\", \"pni\": \"de24dc73-fbd8-41be-a7d5-764c70d9da7e\"}");
when(commands.get(eq("Account3::" + uuid))).thenReturn("{\"number\": \"+14152222222\", \"pni\": \"de24dc73-fbd8-41be-a7d5-764c70d9da7e\"}");
Optional<Account> account = accountsManager.getByE164("+14152222222");
assertTrue(account.isPresent());
assertEquals(account.get().getNumber(), "+14152222222");
assertEquals(account.get().getProfileName(), "test");
assertEquals(UUID.fromString("de24dc73-fbd8-41be-a7d5-764c70d9da7e"), account.get().getPhoneNumberIdentifier());
verify(commands, times(1)).get(eq("AccountMap::+14152222222"));
@ -170,14 +167,13 @@ class AccountsManagerTest {
void testGetAccountByUuidInCache() {
UUID uuid = UUID.randomUUID();
when(commands.get(eq("Account3::" + uuid))).thenReturn("{\"number\": \"+14152222222\", \"name\": \"test\", \"pni\": \"de24dc73-fbd8-41be-a7d5-764c70d9da7e\"}");
when(commands.get(eq("Account3::" + uuid))).thenReturn("{\"number\": \"+14152222222\", \"pni\": \"de24dc73-fbd8-41be-a7d5-764c70d9da7e\"}");
Optional<Account> account = accountsManager.getByAccountIdentifier(uuid);
assertTrue(account.isPresent());
assertEquals(account.get().getNumber(), "+14152222222");
assertEquals(account.get().getUuid(), uuid);
assertEquals(account.get().getProfileName(), "test");
assertEquals(UUID.fromString("de24dc73-fbd8-41be-a7d5-764c70d9da7e"), account.get().getPhoneNumberIdentifier());
verify(commands, times(1)).get(eq("Account3::" + uuid));
@ -192,13 +188,12 @@ class AccountsManagerTest {
UUID pni = UUID.randomUUID();
when(commands.get(eq("AccountMap::" + pni))).thenReturn(uuid.toString());
when(commands.get(eq("Account3::" + uuid))).thenReturn("{\"number\": \"+14152222222\", \"name\": \"test\", \"pni\": \"de24dc73-fbd8-41be-a7d5-764c70d9da7e\"}");
when(commands.get(eq("Account3::" + uuid))).thenReturn("{\"number\": \"+14152222222\", \"pni\": \"de24dc73-fbd8-41be-a7d5-764c70d9da7e\"}");
Optional<Account> account = accountsManager.getByPhoneNumberIdentifier(pni);
assertTrue(account.isPresent());
assertEquals(account.get().getNumber(), "+14152222222");
assertEquals(account.get().getProfileName(), "test");
assertEquals(UUID.fromString("de24dc73-fbd8-41be-a7d5-764c70d9da7e"), account.get().getPhoneNumberIdentifier());
verify(commands).get(eq("AccountMap::" + pni));
@ -214,13 +209,12 @@ class AccountsManagerTest {
String username = "test";
when(commands.get(eq("AccountMap::" + username))).thenReturn(uuid.toString());
when(commands.get(eq("Account3::" + uuid))).thenReturn("{\"number\": \"+14152222222\", \"name\": \"test\", \"pni\": \"de24dc73-fbd8-41be-a7d5-764c70d9da7e\", \"username\": \"test\"}");
when(commands.get(eq("Account3::" + uuid))).thenReturn("{\"number\": \"+14152222222\", \"pni\": \"de24dc73-fbd8-41be-a7d5-764c70d9da7e\", \"username\": \"test\"}");
Optional<Account> account = accountsManager.getByUsername(username);
assertTrue(account.isPresent());
assertEquals(account.get().getNumber(), "+14152222222");
assertEquals(account.get().getProfileName(), "test");
assertEquals(UUID.fromString("de24dc73-fbd8-41be-a7d5-764c70d9da7e"), account.get().getPhoneNumberIdentifier());
assertEquals(Optional.of(username), account.get().getUsername());
@ -451,10 +445,10 @@ class AccountsManagerTest {
.doAnswer(ACCOUNT_UPDATE_ANSWER)
.when(accounts).update(any());
account = accountsManager.update(account, a -> a.setProfileName("name"));
account = accountsManager.update(account, a -> a.setIdentityKey("identity-key"));
assertEquals(1, account.getVersion());
assertEquals("name", account.getProfileName());
assertEquals("identity-key", account.getIdentityKey());
verify(accounts, times(1)).getByAccountIdentifier(uuid);
verify(accounts, times(2)).update(any());

5
service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java
View File

@ -294,8 +294,6 @@ class AccountsTest {
assertPhoneNumberConstraintExists("+14151112222", firstUuid);
assertPhoneNumberIdentifierConstraintExists(firstPni, firstUuid);
account.setProfileName("name");
accounts.update(account);
UUID secondUuid = UUID.randomUUID();
@ -348,8 +346,6 @@ class AccountsTest {
assertThatThrownBy(() -> accounts.update(unknownAccount)).isInstanceOfAny(ConditionalCheckFailedException.class);
account.setProfileName("name");
accounts.update(account);
assertThat(account.getVersion()).isEqualTo(2);
@ -361,7 +357,6 @@ class AccountsTest {
assertThatThrownBy(() -> accounts.update(account)).isInstanceOfAny(ContestedOptimisticLockException.class);
account.setVersion(2);
account.setProfileName("name2");
accounts.update(account);

21
service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/ProfileControllerTest.java
View File

@ -164,8 +164,6 @@ class ProfileControllerTest {
profileAccount = mock(Account.class);
when(profileAccount.getIdentityKey()).thenReturn("bar");
when(profileAccount.getProfileName()).thenReturn("baz");
when(profileAccount.getAvatar()).thenReturn("profiles/bang");
when(profileAccount.getUuid()).thenReturn(AuthHelper.VALID_UUID_TWO);
when(profileAccount.isEnabled()).thenReturn(true);
when(profileAccount.isGroupsV2Supported()).thenReturn(false);
@ -179,8 +177,6 @@ class ProfileControllerTest {
Account capabilitiesAccount = mock(Account.class);
when(capabilitiesAccount.getIdentityKey()).thenReturn("barz");
when(capabilitiesAccount.getProfileName()).thenReturn("bazz");
when(capabilitiesAccount.getAvatar()).thenReturn("profiles/bangz");
when(capabilitiesAccount.isEnabled()).thenReturn(true);
when(capabilitiesAccount.isGroupsV2Supported()).thenReturn(true);
when(capabilitiesAccount.isGv1MigrationSupported()).thenReturn(true);
@ -220,8 +216,8 @@ class ProfileControllerTest {
.get(Profile.class);
assertThat(profile.getIdentityKey()).isEqualTo("bar");
assertThat(profile.getName()).isEqualTo("baz");
assertThat(profile.getAvatar()).isEqualTo("profiles/bang");
assertThat(profile.getName()).isNull();
assertThat(profile.getAvatar()).isNull();
assertThat(profile.getUsername()).isEqualTo("n00bkiller");
assertThat(profile.getBadges()).hasSize(1).element(0).has(new Condition<>(
badge -> "Test Badge".equals(badge.getName()), "has badge with expected name"));
@ -239,8 +235,8 @@ class ProfileControllerTest {
.get(Profile.class);
assertThat(profile.getIdentityKey()).isEqualTo("bar");
assertThat(profile.getName()).isEqualTo("baz");
assertThat(profile.getAvatar()).isEqualTo("profiles/bang");
assertThat(profile.getName()).isNull();
assertThat(profile.getAvatar()).isNull();
assertThat(profile.getUsername()).isEqualTo("n00bkiller");
assertThat(profile.getUuid()).isEqualTo(AuthHelper.VALID_UUID_TWO);
assertThat(profile.getBadges()).hasSize(1).element(0).has(new Condition<>(
@ -383,9 +379,6 @@ class ProfileControllerTest {
verify(profilesManager, times(1)).get(eq(AuthHelper.VALID_UUID_TWO), eq("anotherversion"));
verify(profilesManager, times(1)).set(eq(AuthHelper.VALID_UUID_TWO), profileArgumentCaptor.capture());
verify(AuthHelper.VALID_ACCOUNT_TWO).setProfileName("123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678");
verify(AuthHelper.VALID_ACCOUNT_TWO).setAvatar(null);
verifyNoMoreInteractions(s3client);
assertThat(profileArgumentCaptor.getValue().getCommitment()).isEqualTo(commitment.serialize());
@ -472,9 +465,6 @@ class ProfileControllerTest {
verify(profilesManager, times(1)).get(eq(AuthHelper.VALID_UUID_TWO), eq("anotherversion"));
verify(profilesManager, times(1)).set(eq(AuthHelper.VALID_UUID_TWO), profileArgumentCaptor.capture());
verify(AuthHelper.VALID_ACCOUNT_TWO).setProfileName(name);
verify(AuthHelper.VALID_ACCOUNT_TWO).setAvatar(null);
verifyNoMoreInteractions(s3client);
final VersionedProfile profile = profileArgumentCaptor.getValue();
@ -510,9 +500,6 @@ class ProfileControllerTest {
verify(profilesManager).get(eq(AuthHelper.VALID_UUID_TWO), eq("yetanotherversion"));
verify(profilesManager).set(eq(AuthHelper.VALID_UUID_TWO), profileArgumentCaptor.capture());
verify(AuthHelper.VALID_ACCOUNT_TWO).setProfileName(eq(name));
verify(AuthHelper.VALID_ACCOUNT_TWO).setAvatar(null);
verifyNoMoreInteractions(s3client);
final VersionedProfile profile = profileArgumentCaptor.getValue();