Canonical discoverability writing.

This commit is contained in:
Graeme Connell 2021-09-03 17:21:04 -06:00 committed by gram-signal
parent 92f035bc2a
commit b4aabd799b
6 changed files with 117 additions and 14 deletions

View File

@ -156,6 +156,7 @@ import org.whispersystems.textsecuregcm.storage.Accounts;
import org.whispersystems.textsecuregcm.storage.AccountsDynamoDb;
import org.whispersystems.textsecuregcm.storage.AccountsDynamoDbMigrator;
import org.whispersystems.textsecuregcm.storage.AccountsManager;
import org.whispersystems.textsecuregcm.storage.ContactDiscoveryWriter;
import org.whispersystems.textsecuregcm.storage.DeletedAccounts;
import org.whispersystems.textsecuregcm.storage.DeletedAccountsDirectoryReconciler;
import org.whispersystems.textsecuregcm.storage.DeletedAccountsManager;
@ -482,6 +483,7 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
directoryServerConfiguration.getReplicationName(), directoryReconciliationClient);
deletedAccountsDirectoryReconcilers.add(deletedAccountsDirectoryReconciler);
}
accountDatabaseCrawlerListeners.add(new ContactDiscoveryWriter(accounts));
// PushFeedbackProcessor may update device properties
accountDatabaseCrawlerListeners.add(new PushFeedbackProcessor(accountsManager));
// delete accounts last

View File

@ -66,6 +66,9 @@ public class Account {
@JsonIgnore
private boolean stale;
@JsonIgnore
private boolean canonicallyDiscoverable;
public Account() {}
@VisibleForTesting
@ -93,6 +96,12 @@ public class Account {
this.number = number;
}
public void setCanonicallyDiscoverable(boolean canonicallyDiscoverable) {
requireNotStale();
this.canonicallyDiscoverable = canonicallyDiscoverable;
}
public String getNumber() {
requireNotStale();
@ -228,6 +237,12 @@ public class Account {
return true;
}
public boolean isCanonicallyDiscoverable() {
requireNotStale();
return canonicallyDiscoverable;
}
public Optional<String> getRelay() {
requireNotStale();

View File

@ -55,6 +55,8 @@ public class AccountsDynamoDb extends AbstractDynamoDbStore implements AccountSt
static final String ATTR_ACCOUNT_DATA = "D";
// internal version for optimistic locking
static final String ATTR_VERSION = "V";
// canonically discoverable
static final String ATTR_CANONICALLY_DISCOVERABLE = "C";
private final DynamoDbClient client;
private final DynamoDbAsyncClient asyncClient;
@ -160,7 +162,8 @@ public class AccountsDynamoDb extends AbstractDynamoDbStore implements AccountSt
KEY_ACCOUNT_UUID, AttributeValues.fromUUID(uuid),
ATTR_ACCOUNT_E164, AttributeValues.fromString(account.getNumber()),
ATTR_ACCOUNT_DATA, AttributeValues.fromByteArray(SystemMapper.getMapper().writeValueAsBytes(account)),
ATTR_VERSION, AttributeValues.fromInt(account.getVersion())))
ATTR_VERSION, AttributeValues.fromInt(account.getVersion()),
ATTR_CANONICALLY_DISCOVERABLE, AttributeValues.fromBool(account.shouldBeVisibleInDirectory())))
.build())
.build();
}
@ -193,13 +196,15 @@ public class AccountsDynamoDb extends AbstractDynamoDbStore implements AccountSt
updateItemRequest = UpdateItemRequest.builder()
.tableName(accountsTableName)
.key(Map.of(KEY_ACCOUNT_UUID, AttributeValues.fromUUID(account.getUuid())))
.updateExpression("SET #data = :data ADD #version :version_increment")
.updateExpression("SET #data = :data, #cds = :cds ADD #version :version_increment")
.conditionExpression("attribute_exists(#number) AND #version = :version")
.expressionAttributeNames(Map.of("#number", ATTR_ACCOUNT_E164,
"#data", ATTR_ACCOUNT_DATA,
"#cds", ATTR_CANONICALLY_DISCOVERABLE,
"#version", ATTR_VERSION))
.expressionAttributeValues(Map.of(
":data", AttributeValues.fromByteArray(SystemMapper.getMapper().writeValueAsBytes(account)),
":cds", AttributeValues.fromBool(account.shouldBeVisibleInDirectory()),
":version", AttributeValues.fromInt(account.getVersion()),
":version_increment", AttributeValues.fromInt(1)))
.returnValues(ReturnValue.UPDATED_NEW)
@ -428,6 +433,7 @@ public class AccountsDynamoDb extends AbstractDynamoDbStore implements AccountSt
static Account fromItem(Map<String, AttributeValue> item) {
if (!item.containsKey(ATTR_ACCOUNT_DATA) ||
!item.containsKey(ATTR_ACCOUNT_E164) ||
// TODO: eventually require ATTR_CANONICALLY_DISCOVERABLE
!item.containsKey(KEY_ACCOUNT_UUID)) {
throw new RuntimeException("item missing values");
}
@ -436,6 +442,7 @@ public class AccountsDynamoDb extends AbstractDynamoDbStore implements AccountSt
account.setNumber(item.get(ATTR_ACCOUNT_E164).s());
account.setUuid(UUIDUtil.fromByteBuffer(item.get(KEY_ACCOUNT_UUID).b().asByteBuffer()));
account.setVersion(Integer.parseInt(item.get(ATTR_VERSION).n()));
account.setCanonicallyDiscoverable(Optional.ofNullable(item.get(ATTR_CANONICALLY_DISCOVERABLE)).map(av -> av.bool()).orElse(false));
return account;

View File

@ -0,0 +1,34 @@
package org.whispersystems.textsecuregcm.storage;
import java.util.List;
import java.util.Optional;
import java.util.UUID;
public class ContactDiscoveryWriter extends AccountDatabaseCrawlerListener {
private final AccountStore accounts;
public ContactDiscoveryWriter(final AccountStore accounts) {
this.accounts = accounts;
}
@Override
public void onCrawlStart() {
// nothing
}
@Override
public void onCrawlEnd(final Optional<UUID> fromUuid) {
// nothing
}
@Override
protected void onCrawlChunk(final Optional<UUID> fromUuid, final List<Account> chunkAccounts)
throws AccountDatabaseCrawlerRestartException {
for (Account account : chunkAccounts) {
if (account.isCanonicallyDiscoverable() != account.shouldBeVisibleInDirectory()) {
accounts.update(account);
}
}
}
}

View File

@ -23,6 +23,8 @@ public class AttributeValues {
return AttributeValue.builder().n(Long.toString(value)).build();
}
public static AttributeValue fromBool(boolean value) { return AttributeValue.builder().bool(value).build(); }
public static AttributeValue fromInt(int value) {
return AttributeValue.builder().n(Integer.toString(value)).build();
}
@ -43,6 +45,10 @@ public class AttributeValues {
return AttributeValue.builder().b(value).build();
}
private static boolean toBool(AttributeValue av) {
return av.bool();
}
private static int toInt(AttributeValue av) {
return Integer.parseInt(av.n());
}
@ -67,6 +73,10 @@ public class AttributeValues {
return Optional.ofNullable(item.get(key));
}
public static boolean getBool(Map<String, AttributeValue> item, String key, boolean defaultValue) {
return AttributeValues.get(item, key).map(AttributeValues::toBool).orElse(defaultValue);
}
public static int getInt(Map<String, AttributeValue> item, String key, int defaultValue) {
return AttributeValues.get(item, key).map(AttributeValues::toInt).orElse(defaultValue);
}

View File

@ -143,11 +143,11 @@ class AccountsDynamoDbTest {
boolean freshUser = accountsDynamoDb.create(account);
assertThat(freshUser).isTrue();
verifyStoredState("+14151112222", account.getUuid(), account);
verifyStoredState("+14151112222", account.getUuid(), account, true);
freshUser = accountsDynamoDb.create(account);
assertThat(freshUser).isTrue();
verifyStoredState("+14151112222", account.getUuid(), account);
verifyStoredState("+14151112222", account.getUuid(), account, true);
}
@ -161,7 +161,7 @@ class AccountsDynamoDbTest {
accountsDynamoDb.create(account);
verifyStoredState("+14151112222", account.getUuid(), account);
verifyStoredState("+14151112222", account.getUuid(), account, true);
}
@Test
@ -210,7 +210,7 @@ class AccountsDynamoDbTest {
accountsDynamoDb.create(account);
verifyStoredState("+14151112222", account.getUuid(), account);
verifyStoredState("+14151112222", account.getUuid(), account, true);
account.setProfileName("name");
@ -223,7 +223,7 @@ class AccountsDynamoDbTest {
final boolean freshUser = accountsDynamoDb.create(account);
assertThat(freshUser).isFalse();
verifyStoredState("+14151112222", firstUuid, account);
verifyStoredState("+14151112222", firstUuid, account, true);
device = generateDevice(1);
Account invalidAccount = generateAccount("+14151113333", firstUuid, Collections.singleton(device));
@ -250,7 +250,7 @@ class AccountsDynamoDbTest {
retrieved = accountsDynamoDb.get(account.getUuid());
assertThat(retrieved.isPresent()).isTrue();
verifyStoredState("+14151112222", account.getUuid(), account);
verifyStoredState("+14151112222", account.getUuid(), account, true);
device = generateDevice(1);
Account unknownAccount = generateAccount("+14151113333", UUID.randomUUID(), Collections.singleton(device));
@ -263,7 +263,7 @@ class AccountsDynamoDbTest {
assertThat(account.getVersion()).isEqualTo(2);
verifyStoredState("+14151112222", account.getUuid(), account);
verifyStoredState("+14151112222", account.getUuid(), account, true);
account.setVersion(1);
@ -274,7 +274,7 @@ class AccountsDynamoDbTest {
accountsDynamoDb.update(account);
verifyStoredState("+14151112222", account.getUuid(), account);
verifyStoredState("+14151112222", account.getUuid(), account, true);
}
@Test
@ -481,13 +481,13 @@ class AccountsDynamoDbTest {
assertThat(migrated).isTrue();
verifyStoredState("+14151112222", account.getUuid(), account);
verifyStoredState("+14151112222", account.getUuid(), account, true);
migrated = accountsDynamoDb.migrate(account).get();
assertThat(migrated).isFalse();
verifyStoredState("+14151112222", account.getUuid(), account);
verifyStoredState("+14151112222", account.getUuid(), account, true);
UUID secondUuid = UUID.randomUUID();
@ -497,7 +497,7 @@ class AccountsDynamoDbTest {
migrated = accountsDynamoDb.migrate(accountRemigrationWithDifferentUuid).get();
assertThat(migrated).isFalse();
verifyStoredState("+14151112222", firstUuid, account);
verifyStoredState("+14151112222", firstUuid, account, true);
account.setVersion(account.getVersion() + 1);
@ -506,6 +506,39 @@ class AccountsDynamoDbTest {
assertThat(migrated).isTrue();
}
@Test
void testCanonicallyDiscoverableSet() {
Device device = generateDevice(1);
UUID uuid = UUID.randomUUID();
Account account = generateAccount("+14151112222", uuid, Collections.singleton(device));
account.setDiscoverableByPhoneNumber(false);
accountsDynamoDb.create(account);
verifyStoredState("+14151112222", account.getUuid(), account, false);
account.setDiscoverableByPhoneNumber(true);
accountsDynamoDb.update(account);
verifyStoredState("+14151112222", account.getUuid(), account, true);
account.setDiscoverableByPhoneNumber(false);
accountsDynamoDb.update(account);
verifyStoredState("+14151112222", account.getUuid(), account, false);
}
@Test
void testContactDiscoveryWriter() throws Exception {
Device device = generateDevice(1);
UUID uuid = UUID.randomUUID();
Account account = generateAccount("+14151112222", uuid, Collections.singleton(device));
accountsDynamoDb.create(account);
verifyStoredState("+14151112222", account.getUuid(), account, true);
ContactDiscoveryWriter writer = new ContactDiscoveryWriter(accountsDynamoDb);
account.setCanonicallyDiscoverable(false);
writer.onCrawlChunk(null, List.of(account));
verifyStoredState("+14151112222", account.getUuid(), account, true);
account.setCanonicallyDiscoverable(true);
account.setDiscoverableByPhoneNumber(false);
writer.onCrawlChunk(null, List.of(account));
verifyStoredState("+14151112222", account.getUuid(), account, false);
}
private Device generateDevice(long id) {
Random random = new Random(System.currentTimeMillis());
SignedPreKey signedPreKey = new SignedPreKey(random.nextInt(), "testPublicKey-" + random.nextInt(), "testSignature-" + random.nextInt());
@ -527,7 +560,7 @@ class AccountsDynamoDbTest {
return new Account(number, uuid, devices, unidentifiedAccessKey);
}
private void verifyStoredState(String number, UUID uuid, Account expecting) {
private void verifyStoredState(String number, UUID uuid, Account expecting, boolean canonicallyDiscoverable) {
final DynamoDbClient db = dynamoDbExtension.getDynamoDbClient();
final GetItemResponse get = db.getItem(GetItemRequest.builder()
@ -543,6 +576,8 @@ class AccountsDynamoDbTest {
assertThat(AttributeValues.getInt(get.item(), AccountsDynamoDb.ATTR_VERSION, -1))
.isEqualTo(expecting.getVersion());
assertThat(AttributeValues.getBool(get.item(), AccountsDynamoDb.ATTR_CANONICALLY_DISCOVERABLE, !canonicallyDiscoverable)).isEqualTo(canonicallyDiscoverable);
Account result = AccountsDynamoDb.fromItem(get.item());
verifyStoredState(number, uuid, result, expecting);
} else {