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

Drop transactional logic from phone number identifier migration

This commit is contained in:
Jon Chambers 2021-11-09 18:21:59 -05:00 committed by Jon Chambers
parent 296f6a7a88
commit 138a2ebbd0
2 changed files with 15 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java
View File

@ -27,6 +27,7 @@ import org.whispersystems.textsecuregcm.util.UUIDUtil;
import software.amazon.awssdk.services.dynamodb.DynamoDbClient;
import software.amazon.awssdk.services.dynamodb.model.AttributeValue;
import software.amazon.awssdk.services.dynamodb.model.CancellationReason;
import software.amazon.awssdk.services.dynamodb.model.ConditionalCheckFailedException;
import software.amazon.awssdk.services.dynamodb.model.Delete;
import software.amazon.awssdk.services.dynamodb.model.GetItemRequest;
import software.amazon.awssdk.services.dynamodb.model.GetItemResponse;
@ -38,6 +39,8 @@ import software.amazon.awssdk.services.dynamodb.model.TransactWriteItemsRequest;
import software.amazon.awssdk.services.dynamodb.model.TransactionCanceledException;
import software.amazon.awssdk.services.dynamodb.model.TransactionConflictException;
import software.amazon.awssdk.services.dynamodb.model.Update;
import software.amazon.awssdk.services.dynamodb.model.UpdateItemRequest;
import software.amazon.awssdk.services.dynamodb.model.UpdateItemResponse;
public class Accounts extends AbstractDynamoDbStore {
@ -302,11 +305,10 @@ public class Accounts extends AbstractDynamoDbStore {
public void update(Account account) throws ContestedOptimisticLockException {
UPDATE_TIMER.record(() -> {
final List<TransactWriteItem> transactWriteItems = new ArrayList<>(2);
final UpdateItemRequest updateItemRequest;
try {
final TransactWriteItem updateAccountWriteItem = TransactWriteItem.builder()
.update(Update.builder()
updateItemRequest = UpdateItemRequest.builder()
.tableName(accountsTableName)
.key(Map.of(KEY_ACCOUNT_UUID, AttributeValues.fromUUID(account.getUuid())))
.updateExpression("SET #data = :data, #cds = :cds, #pni = :pni ADD #version :version_increment")
@ -322,32 +324,21 @@ public class Accounts extends AbstractDynamoDbStore {
":version", AttributeValues.fromInt(account.getVersion()),
":version_increment", AttributeValues.fromInt(1),
":pni", AttributeValues.fromUUID(account.getPhoneNumberIdentifier())))
.build())
.build();;
transactWriteItems.add(updateAccountWriteItem);
.build();
} catch (JsonProcessingException e) {
throw new IllegalArgumentException(e);
}
try {
client.transactWriteItems(TransactWriteItemsRequest.builder()
.transactItems(transactWriteItems)
.build());
account.setVersion(account.getVersion() + 1);
final UpdateItemResponse response = client.updateItem(updateItemRequest);
account.setVersion(AttributeValues.getInt(response.attributes(), "V", account.getVersion() + 1));
} catch (final TransactionConflictException e) {
throw new ContestedOptimisticLockException();
} catch (final TransactionCanceledException e) {
if (e.cancellationReasons().size() > 1 && "ConditionalCheckFailed".equals(e.cancellationReasons().get(1).code())) {
log.error("Conflicting phone number mapping exists for account {}, PNI {}", account.getUuid(), account.getPhoneNumberIdentifier());
throw e;
}
// We can infer an optimistic locking failure if the UUID is known
} catch (final ConditionalCheckFailedException e) {
// the exception doesn't give details about which condition failed,
// but we can infer it was an optimistic locking failure if the UUID is known
throw getByAccountIdentifier(account.getUuid()).isPresent() ? new ContestedOptimisticLockException() : e;
}
});

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

@ -40,6 +40,7 @@ import software.amazon.awssdk.services.dynamodb.DynamoDbClient;
import software.amazon.awssdk.services.dynamodb.model.AttributeDefinition;
import software.amazon.awssdk.services.dynamodb.model.AttributeValue;
import software.amazon.awssdk.services.dynamodb.model.CancellationReason;
import software.amazon.awssdk.services.dynamodb.model.ConditionalCheckFailedException;
import software.amazon.awssdk.services.dynamodb.model.CreateTableRequest;
import software.amazon.awssdk.services.dynamodb.model.GetItemRequest;
import software.amazon.awssdk.services.dynamodb.model.GetItemResponse;
@ -329,7 +330,7 @@ class AccountsTest {
device = generateDevice(1);
Account unknownAccount = generateAccount("+14151113333", UUID.randomUUID(), UUID.randomUUID(), Collections.singleton(device));
assertThatThrownBy(() -> accounts.update(unknownAccount)).isInstanceOfAny(TransactionCanceledException.class);
assertThatThrownBy(() -> accounts.update(unknownAccount)).isInstanceOfAny(ConditionalCheckFailedException.class);
account.setProfileName("name");
@ -358,38 +359,14 @@ class AccountsTest {
accounts = new Accounts(dynamoDbClient,
dynamoDbExtension.getTableName(), NUMBER_CONSTRAINT_TABLE_NAME, PNI_CONSTRAINT_TABLE_NAME, SCAN_PAGE_SIZE);
when(dynamoDbClient.transactWriteItems(any(TransactWriteItemsRequest.class)))
when(dynamoDbClient.updateItem(any(UpdateItemRequest.class)))
.thenThrow(TransactionConflictException.class);
Device device = generateDevice(1);
Account account = generateAccount("+14151112222", UUID.randomUUID(), UUID.randomUUID(), Collections.singleton(device));
Account account = generateAccount("+14151112222", UUID.randomUUID(), UUID.randomUUID());
assertThatThrownBy(() -> accounts.update(account)).isInstanceOfAny(ContestedOptimisticLockException.class);
}
@Test
// TODO Remove after initial migration is complete
void testUpdateWithMockTransactionCancellationException() {
final DynamoDbClient dynamoDbClient = mock(DynamoDbClient.class);
accounts = new Accounts(dynamoDbClient,
dynamoDbExtension.getTableName(), NUMBER_CONSTRAINT_TABLE_NAME, PNI_CONSTRAINT_TABLE_NAME, SCAN_PAGE_SIZE);
when(dynamoDbClient.transactWriteItems(any(TransactWriteItemsRequest.class)))
.thenThrow(TransactionCanceledException.builder()
.cancellationReasons(CancellationReason.builder()
.code("Test")
.build())
.build());
when(dynamoDbClient.getItem(any(GetItemRequest.class))).thenReturn(GetItemResponse.builder().build());
Device device = generateDevice(1);
Account account = generateAccount("+14151112222", UUID.randomUUID(), UUID.randomUUID(), Collections.singleton(device));
assertThatThrownBy(() -> accounts.update(account)).isInstanceOfAny(TransactionCanceledException.class);
}
@Test
void testRetrieveFrom() {
List<Account> users = new ArrayList<>();