diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java index fab835349..2fbe8c46f 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java @@ -429,6 +429,24 @@ public class Accounts extends AbstractDynamoDbStore { succeeded = true; } catch (final JsonProcessingException e) { throw new IllegalArgumentException(e); + } catch (final TransactionCanceledException e) { + if (e.hasCancellationReasons() && e.cancellationReasons().size() == 6) { + // the cancellation reasons map to the write items: + // 0. phone number constraint delete - no conditions + // 1. phone number constraint put - conditional on the key not existing (it is deleted by 0) + // 2. pni constraint delete - no conditions + // 3. pni constraint put - conditional on the key not existing (it is deleted by 2) + // 4. deleted accounts delete - no conditions + // 5. account update - conditional on #version = :version + if (CONDITIONAL_CHECK_FAILED.equals(e.cancellationReasons().get(5).code())) { + // the #version = :version condition failed, which indicates a concurrent update + throw new ContestedOptimisticLockException(); + } + } else { + log.warn("Unexpected cancellation reasons: {}", e.cancellationReasons()); + + } + throw e; } finally { if (!succeeded) { account.setNumber(originalNumber, originalPni); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java index 8ef8e1356..7cc18ea96 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java @@ -885,6 +885,41 @@ class AccountsTest { assertThrows(TransactionCanceledException.class, () -> accounts.changeNumber(account, targetNumber, existingPhoneNumberIdentifier, Optional.empty(), Collections.emptyList())); } + @Test + public void testChangeNumberContestedOptimisticLock() { + final String originalNumber = "+14151112222"; + final String targetNumber = "+14151113333"; + + final UUID originalPni = UUID.randomUUID(); + final UUID targetPni = UUID.randomUUID(); + + final Device device = generateDevice(DEVICE_ID_1); + final Account firstAccountInstance = generateAccount(originalNumber, UUID.randomUUID(), originalPni, + List.of(device)); + + createAccount(firstAccountInstance); + + final Account secondAccountInstance = accounts.getByAccountIdentifier(firstAccountInstance.getUuid()).orElseThrow(); + + // update via the first instance, which will update the version + firstAccountInstance.setCurrentProfileVersion("1"); + accounts.update(firstAccountInstance); + + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.changeNumber(secondAccountInstance, targetNumber, targetPni, Optional.empty(), + Collections.emptyList()), "Second account instance has stale version"); + + final Account refreshedAccountInstance = accounts.getByAccountIdentifier(firstAccountInstance.getUuid()) + .orElseThrow(); + accounts.changeNumber(refreshedAccountInstance, targetNumber, targetPni, Optional.empty(), + Collections.emptyList()); + + assertPhoneNumberConstraintDoesNotExist(originalNumber); + assertPhoneNumberIdentifierConstraintDoesNotExist(originalPni); + assertPhoneNumberConstraintExists(targetNumber, firstAccountInstance.getUuid()); + assertPhoneNumberIdentifierConstraintExists(targetPni, firstAccountInstance.getUuid()); + } + @Test void testSwitchUsernameHashes() { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID());