diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java index 87158be88..e8cddb6f3 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java @@ -4,6 +4,8 @@ */ package org.whispersystems.textsecuregcm.controllers; +import static org.whispersystems.textsecuregcm.metrics.MetricsUtil.name; + import io.dropwizard.auth.Auth; import io.micrometer.core.instrument.Metrics; import io.micrometer.core.instrument.Tag; @@ -67,13 +69,10 @@ import org.whispersystems.textsecuregcm.storage.Device; import org.whispersystems.textsecuregcm.storage.RegistrationRecoveryPasswordsManager; import org.whispersystems.textsecuregcm.storage.UsernameHashNotAvailableException; import org.whispersystems.textsecuregcm.storage.UsernameReservationNotFoundException; -import org.whispersystems.textsecuregcm.util.ExceptionUtils; import org.whispersystems.textsecuregcm.util.HeaderUtils; import org.whispersystems.textsecuregcm.util.UsernameHashZkProofVerifier; import org.whispersystems.textsecuregcm.util.Util; -import static org.whispersystems.textsecuregcm.metrics.MetricsUtil.name; - @SuppressWarnings("OptionalUsedAsFieldOrParameterType") @Path("/v1/accounts") @io.swagger.v3.oas.annotations.tags.Tag(name = "Account") @@ -306,12 +305,11 @@ public class AccountController { ) @ApiResponse(responseCode = "204", description = "Username successfully deleted.", useReturnTypeSchema = true) @ApiResponse(responseCode = "401", description = "Account authentication check failed.") - public CompletableFuture deleteUsernameHash(@Auth final AuthenticatedDevice auth) { + public void deleteUsernameHash(@Auth final AuthenticatedDevice auth) { final Account account = accounts.getByAccountIdentifier(auth.accountIdentifier()) .orElseThrow(() -> new WebApplicationException(Status.UNAUTHORIZED)); - return accounts.clearUsernameHash(account) - .thenApply(Util.ASYNC_EMPTY_RESPONSE); + accounts.clearUsernameHash(account); } @PUT @@ -330,7 +328,7 @@ public class AccountController { @ApiResponse(responseCode = "409", description = "All username hashes from the list are taken.") @ApiResponse(responseCode = "422", description = "Invalid request format.") @ApiResponse(responseCode = "429", description = "Ratelimited.") - public CompletableFuture reserveUsernameHash( + public ReserveUsernameHashResponse reserveUsernameHash( @Auth final AuthenticatedDevice auth, @NotNull @Valid final ReserveUsernameHashRequest usernameRequest) throws RateLimitExceededException { @@ -345,15 +343,14 @@ public class AccountController { } } - return accounts.reserveUsernameHash(account, usernameRequest.usernameHashes()) - .thenApply(reservation -> new ReserveUsernameHashResponse(reservation.reservedUsernameHash())) - .exceptionally(throwable -> { - if (ExceptionUtils.unwrap(throwable) instanceof UsernameHashNotAvailableException) { - throw new WebApplicationException(Status.CONFLICT); - } + try { + final AccountsManager.UsernameReservation reservation = + accounts.reserveUsernameHash(account, usernameRequest.usernameHashes()); - throw ExceptionUtils.wrap(throwable); - }); + return new ReserveUsernameHashResponse(reservation.reservedUsernameHash()); + } catch (final UsernameHashNotAvailableException e) { + throw new WebApplicationException(Status.CONFLICT); + } } @PUT @@ -373,9 +370,9 @@ public class AccountController { @ApiResponse(responseCode = "410", description = "Username hash not available (username can't be used).") @ApiResponse(responseCode = "422", description = "Invalid request format.") @ApiResponse(responseCode = "429", description = "Ratelimited.") - public CompletableFuture confirmUsernameHash( + public UsernameHashResponse confirmUsernameHash( @Auth final AuthenticatedDevice auth, - @NotNull @Valid final ConfirmUsernameHashRequest confirmRequest) { + @NotNull @Valid final ConfirmUsernameHashRequest confirmRequest) throws RateLimitExceededException { final Account account = accounts.getByAccountIdentifier(auth.accountIdentifier()) .orElseThrow(() -> new WebApplicationException(Status.UNAUTHORIZED)); @@ -386,26 +383,22 @@ public class AccountController { throw new WebApplicationException(Response.status(422).build()); } - return rateLimiters.getUsernameSetLimiter().validateAsync(account.getUuid()) - .thenCompose(ignored -> accounts.confirmReservedUsernameHash( - account, - confirmRequest.usernameHash(), - confirmRequest.encryptedUsername())) - .thenApply(updatedAccount -> new UsernameHashResponse(updatedAccount.getUsernameHash() - .orElseThrow(() -> new IllegalStateException("Could not get username after setting")), - updatedAccount.getUsernameLinkHandle())) - .exceptionally(throwable -> { - if (ExceptionUtils.unwrap(throwable) instanceof UsernameReservationNotFoundException) { - throw new WebApplicationException(Status.CONFLICT); - } + rateLimiters.getUsernameSetLimiter().validate(account.getUuid()); - if (ExceptionUtils.unwrap(throwable) instanceof UsernameHashNotAvailableException) { - throw new WebApplicationException(Status.GONE); - } + try { + final Account updatedAccount = accounts.confirmReservedUsernameHash( + account, + confirmRequest.usernameHash(), + confirmRequest.encryptedUsername()); - throw ExceptionUtils.wrap(throwable); - }) - .toCompletableFuture(); + return new UsernameHashResponse(updatedAccount.getUsernameHash() + .orElseThrow(() -> new IllegalStateException("Could not get username after setting")), + updatedAccount.getUsernameLinkHandle()); + } catch (final UsernameReservationNotFoundException e) { + throw new WebApplicationException(Status.CONFLICT); + } catch (UsernameHashNotAvailableException e) { + throw new WebApplicationException(Status.GONE); + } } @GET diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcService.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcService.java index 6119c1189..f894f703f 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcService.java @@ -10,7 +10,6 @@ import java.util.ArrayList; import java.util.HexFormat; import java.util.List; import java.util.UUID; -import java.util.concurrent.CompletionException; import org.signal.chat.account.ClearRegistrationLockRequest; import org.signal.chat.account.ClearRegistrationLockResponse; import org.signal.chat.account.ConfigureUnidentifiedAccessRequest; @@ -160,19 +159,15 @@ public class AccountsGrpcService extends SimpleAccountsGrpc.AccountsImplBase { try { final AccountsManager.UsernameReservation usernameReservation = - accountsManager.reserveUsernameHash(account, usernameHashes).join(); + accountsManager.reserveUsernameHash(account, usernameHashes); return ReserveUsernameHashResponse.newBuilder() .setUsernameHash(ByteString.copyFrom(usernameReservation.reservedUsernameHash())) .build(); - } catch (final CompletionException e) { - if (e.getCause() instanceof UsernameHashNotAvailableException) { + } catch (final UsernameHashNotAvailableException e) { return ReserveUsernameHashResponse.newBuilder() .setUsernameNotAvailable(UsernameNotAvailable.getDefaultInstance()) .build(); - } - - throw e; } } @@ -216,34 +211,29 @@ public class AccountsGrpcService extends SimpleAccountsGrpc.AccountsImplBase { try { final Account updatedAccount = accountsManager.confirmReservedUsernameHash(getAuthenticatedAccount(), request.getUsernameHash().toByteArray(), - request.getUsernameCiphertext().toByteArray()) - .join(); + request.getUsernameCiphertext().toByteArray()); return ConfirmUsernameHashResponse.newBuilder() .setConfirmedUsernameHash(ConfirmUsernameHashResponse.ConfirmedUsernameHash.newBuilder() .setUsernameHash(ByteString.copyFrom(updatedAccount.getUsernameHash().orElseThrow())) .setUsernameLinkHandle(UUIDUtil.toByteString(updatedAccount.getUsernameLinkHandle()))) .build(); - } catch (final CompletionException e) { - if (e.getCause() instanceof UsernameReservationNotFoundException) { - return ConfirmUsernameHashResponse - .newBuilder() - .setReservationNotFound(FailedPrecondition.getDefaultInstance()) - .build(); - } else if (e.getCause() instanceof UsernameHashNotAvailableException) { - return ConfirmUsernameHashResponse - .newBuilder() - .setUsernameNotAvailable(UsernameNotAvailable.getDefaultInstance()) - .build(); - } - - throw e; + } catch (final UsernameHashNotAvailableException e) { + return ConfirmUsernameHashResponse + .newBuilder() + .setUsernameNotAvailable(UsernameNotAvailable.getDefaultInstance()) + .build(); + } catch (final UsernameReservationNotFoundException e) { + return ConfirmUsernameHashResponse + .newBuilder() + .setReservationNotFound(FailedPrecondition.getDefaultInstance()) + .build(); } } @Override public DeleteUsernameHashResponse deleteUsernameHash(final DeleteUsernameHashRequest request) { - accountsManager.clearUsernameHash(getAuthenticatedAccount()).join(); + accountsManager.clearUsernameHash(getAuthenticatedAccount()); return DeleteUsernameHashResponse.getDefaultInstance(); } 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 43c91c370..815cac224 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Accounts.java @@ -376,7 +376,7 @@ public class Accounts { writeItems.addAll(additionalWriteItems); return dynamoDbAsyncClient.transactWriteItems(TransactWriteItemsRequest.builder().transactItems(writeItems).build()) - .thenApply(response -> { + .thenApply(_ -> { accountToCreate.setVersion(accountToCreate.getVersion() + 1); return (Void) null; }) @@ -495,18 +495,13 @@ public class Accounts { }); } - - /** - * Reserve a username hash under the account UUID - * @return a future that completes once the username hash has been reserved; may fail with an - * {@link ContestedOptimisticLockException} if the account has been updated or there are concurrent updates to the - * account or constraint records, and with an - * {@link UsernameHashNotAvailableException} if the username was taken by someone else - */ - public CompletableFuture reserveUsernameHash( - final Account account, - final byte[] reservedUsernameHash, - final Duration ttl) { + /// Reserve a username hash under the account UUID + /// + /// @throws ContestedOptimisticLockException if the account has been updated or there are concurrent updates to the + /// account or constraint records, and with an + /// @throws UsernameHashNotAvailableException if the username was taken by someone else + public void reserveUsernameHash(final Account account, final byte[] reservedUsernameHash, final Duration ttl) + throws ContestedOptimisticLockException, UsernameHashNotAvailableException { final Timer.Sample sample = Timer.start(); @@ -521,19 +516,24 @@ public class Accounts { // What we'd really like to do is set expirationTime = max(oldExpirationTime, now + ttl), but dynamodb doesn't // support that. Instead, we'll set expiration if it's greater than the existing expiration, otherwise retry final long expirationTime = clock.instant().plus(ttl).getEpochSecond(); - return tryReserveUsernameHash(account, reservedUsernameHash, expirationTime) - .exceptionallyCompose(ExceptionUtils.exceptionallyHandler(TtlConflictException.class, ttlConflict -> - // retry (once) with the returned expiration time - tryReserveUsernameHash(account, reservedUsernameHash, ttlConflict.getExistingExpirationSeconds()))) - .whenComplete((response, throwable) -> { - sample.stop(RESERVE_USERNAME_TIMER); + boolean succeeded = false; - if (throwable == null) { - account.setVersion(account.getVersion() + 1); - } else { - account.setReservedUsernameHash(maybeOriginalReservation.orElse(null)); - } - }); + try { + tryReserveUsernameHash(account, reservedUsernameHash, expirationTime); + succeeded = true; + } catch (final TtlConflictException e) { + // retry (once) with the returned expiration time + tryReserveUsernameHash(account, reservedUsernameHash, e.getExistingExpirationSeconds()); + succeeded = true; + } finally { + sample.stop(RESERVE_USERNAME_TIMER); + + if (succeeded) { + account.setVersion(account.getVersion() + 1); + } else { + account.setReservedUsernameHash(maybeOriginalReservation.orElse(null)); + } + } } private static class TtlConflictException extends ContestedOptimisticLockException { @@ -548,20 +548,21 @@ public class Accounts { } } - /** - * Try to reserve the provided usernameHash - * - * @param updatedAccount The account, already updated to reserve the provided usernameHash - * @param reservedUsernameHash The usernameHash to reserve - * @param expirationTimeSeconds When the reservation should expire - * @return A future that completes successfully if the usernameHash was reserved - * @throws TtlConflictException if the usernameHash was already reserved but with a longer TTL. The operation should - * be retried with the returned {@link TtlConflictException#getExistingExpirationSeconds()} - */ - private CompletableFuture tryReserveUsernameHash( + /// Try to reserve the provided usernameHash + /// + /// @param updatedAccount The account, already updated to reserve the provided usernameHash + /// @param reservedUsernameHash The usernameHash to reserve + /// @param expirationTimeSeconds When the reservation should expire + /// + /// @throws ContestedOptimisticLockException in the event of concurrent modifications to the account + /// @throws UsernameHashNotAvailableException if the username hash is already taken + /// @throws TtlConflictException if the usernameHash was already reserved but with a longer TTL. The + /// operation should be retried with the returned + /// {@link TtlConflictException#getExistingExpirationSeconds()} + private void tryReserveUsernameHash( final Account updatedAccount, final byte[] reservedUsernameHash, - final long expirationTimeSeconds) { + final long expirationTimeSeconds) throws ContestedOptimisticLockException, TtlConflictException, UsernameHashNotAvailableException { // Use account UUID as a "reservation token" - by providing this, the client proves ownership of the hash final UUID uuid = updatedAccount.getUuid(); @@ -597,34 +598,31 @@ public class Accounts { writeItems.add(UpdateAccountSpec.forAccount(accountsTableName, updatedAccount).transactItem()); - return dynamoDbAsyncClient - .transactWriteItems(TransactWriteItemsRequest.builder().transactItems(writeItems).build()) - .thenRun(Util.NOOP) - .exceptionally(ExceptionUtils.exceptionallyHandler(TransactionCanceledException.class, e -> { - // If the constraint table update failed the condition check, the username's taken and we should stop - // trying. However, - if (conditionalCheckFailed(e.cancellationReasons().get(0))) { - // The constraint table update failed the condition check. It could be because the username was taken, - // or because we need to retry with a longer TTL - final Map item = e.cancellationReasons().getFirst().item(); - final UUID existingOwner = AttributeValues.getUUID(item, UsernameTable.ATTR_ACCOUNT_UUID, null); - final boolean confirmed = AttributeValues.getBool(item, UsernameTable.ATTR_CONFIRMED, false); - final long existingTtl = AttributeValues.getLong(item, UsernameTable.ATTR_TTL, 0L); - if (uuid.equals(existingOwner) && !confirmed && existingTtl > expirationTimeSeconds) { - // We failed because we provided a shorter TTL than the one that exists on the reservation. The caller - // can retry with updated expiration time. - throw new TtlConflictException(existingTtl); - } - throw ExceptionUtils.wrap(new UsernameHashNotAvailableException()); - } else if (conditionalCheckFailed(e.cancellationReasons().get(1)) || - e.cancellationReasons().stream().anyMatch(Accounts::isTransactionConflict)) { - // The accounts table fails the conditional check or either table was concurrently updated, it's an - // optimistic locking failure and we should try again. - throw new ContestedOptimisticLockException(); - } else { - throw ExceptionUtils.wrap(e); - } - })); + try { + dynamoDbClient.transactWriteItems(TransactWriteItemsRequest.builder().transactItems(writeItems).build()); + } catch (final TransactionCanceledException e) { + if (conditionalCheckFailed(e.cancellationReasons().get(0))) { + // The constraint table update failed the condition check. It could be because the username was taken, + // or because we need to retry with a longer TTL + final Map item = e.cancellationReasons().getFirst().item(); + final UUID existingOwner = AttributeValues.getUUID(item, UsernameTable.ATTR_ACCOUNT_UUID, null); + final boolean confirmed = AttributeValues.getBool(item, UsernameTable.ATTR_CONFIRMED, false); + final long existingTtl = AttributeValues.getLong(item, UsernameTable.ATTR_TTL, 0L); + if (uuid.equals(existingOwner) && !confirmed && existingTtl > expirationTimeSeconds) { + // We failed because we provided a shorter TTL than the one that exists on the reservation. The caller + // can retry with updated expiration time. + throw new TtlConflictException(existingTtl); + } + throw new UsernameHashNotAvailableException(); + } else if (conditionalCheckFailed(e.cancellationReasons().get(1)) || + e.cancellationReasons().stream().anyMatch(Accounts::isTransactionConflict)) { + // The accounts table fails the conditional check or either table was concurrently updated, it's an + // optimistic locking failure and we should try again. + throw new ContestedOptimisticLockException(); + } else { + throw e; + } + } } /** @@ -719,138 +717,133 @@ public class Accounts { .build()).build(); } - /** - * Confirm (set) a previously reserved username hash - * - * @param account to update - * @param usernameHash believed to be available - * @param encryptedUsername the encrypted form of the previously reserved username; used for the username link - * @return a future that completes once the username hash has been confirmed; may fail with an - * {@link ContestedOptimisticLockException} if the account has been updated or there are concurrent updates to the - * account or constraint records, and with an - * {@link UsernameHashNotAvailableException} if the username was taken by someone else - */ - public CompletableFuture confirmUsernameHash(final Account account, final byte[] usernameHash, @Nullable final byte[] encryptedUsername) { + /// Confirm (set) a previously reserved username hash + /// + /// @param account to update + /// @param usernameHash believed to be available + /// @param encryptedUsername the encrypted form of the previously reserved username; used for the username link + /// + /// @throws ContestedOptimisticLockException if the account has been updated or there are concurrent updates to the + /// account or constraint records + /// @throws UsernameHashNotAvailableException if the username was taken by someone else + public void confirmUsernameHash(final Account account, final byte[] usernameHash, @Nullable final byte[] encryptedUsername) + throws ContestedOptimisticLockException, UsernameHashNotAvailableException { + final Timer.Sample sample = Timer.start(); if (usernameHash == null) { throw new IllegalArgumentException("Cannot confirm a null usernameHash"); } - return pickLinkHandle(account, usernameHash) - .thenCompose(linkHandle -> { - final Optional maybeOriginalUsernameHash = account.getUsernameHash(); - final Account updatedAccount = AccountUtil.cloneAccountAsNotStale(account); - updatedAccount.setUsernameHash(usernameHash); - updatedAccount.setReservedUsernameHash(null); - updatedAccount.setUsernameLinkDetails(encryptedUsername == null ? null : linkHandle, encryptedUsername); - final Instant now = clock.instant(); - final Optional holdToRemove = maybeOriginalUsernameHash - .flatMap(hold -> addToHolds(updatedAccount, hold, now)); + final UUID linkHandle = pickLinkHandle(account, usernameHash); - final List writeItems = new ArrayList<>(); + final Optional maybeOriginalUsernameHash = account.getUsernameHash(); + final Account updatedAccount = AccountUtil.cloneAccountAsNotStale(account); + updatedAccount.setUsernameHash(usernameHash); + updatedAccount.setReservedUsernameHash(null); + updatedAccount.setUsernameLinkDetails(encryptedUsername == null ? null : linkHandle, encryptedUsername); - // 0: add the username hash to the constraint table, wiping out the ttl if we had already reserved the hash - writeItems.add(TransactWriteItem.builder().put(Put.builder() - .tableName(usernamesConstraintTableName) - .item(Map.of( - UsernameTable.ATTR_ACCOUNT_UUID, AttributeValues.fromUUID(updatedAccount.getUuid()), - UsernameTable.KEY_USERNAME_HASH, AttributeValues.fromByteArray(usernameHash), - UsernameTable.ATTR_CONFIRMED, AttributeValues.fromBool(true))) - // it's not in the constraint table OR it's expired OR it was reserved by us - .conditionExpression("attribute_not_exists(#username_hash) OR #ttl < :now OR (#aci = :aci AND #confirmed = :confirmed)") - .expressionAttributeNames(Map.of( - "#username_hash", UsernameTable.KEY_USERNAME_HASH, - "#ttl", UsernameTable.ATTR_TTL, - "#aci", UsernameTable.ATTR_ACCOUNT_UUID, - "#confirmed", UsernameTable.ATTR_CONFIRMED)) - .expressionAttributeValues(Map.of( - ":now", AttributeValues.fromLong(clock.instant().getEpochSecond()), - ":aci", AttributeValues.fromUUID(updatedAccount.getUuid()), - ":confirmed", AttributeValues.fromBool(false))) - .returnValuesOnConditionCheckFailure(ReturnValuesOnConditionCheckFailure.ALL_OLD) - .build()) - .build()); + final Instant now = clock.instant(); + final Optional holdToRemove = + maybeOriginalUsernameHash.flatMap(hold -> addToHolds(updatedAccount, hold, now)); - // 1: update the account object (conditioned on the version increment) - writeItems.add(UpdateAccountSpec.forAccount(accountsTableName, updatedAccount).transactItem()); + final List writeItems = new ArrayList<>(); - // 2?: Add a temporary hold for the old username to stop others from claiming it - maybeOriginalUsernameHash.ifPresent(originalUsernameHash -> - writeItems.add(holdUsernameTransactItem(updatedAccount.getUuid(), originalUsernameHash, now))); + // 0: add the username hash to the constraint table, wiping out the ttl if we had already reserved the hash + writeItems.add(TransactWriteItem.builder().put(Put.builder() + .tableName(usernamesConstraintTableName) + .item(Map.of( + UsernameTable.ATTR_ACCOUNT_UUID, AttributeValues.fromUUID(updatedAccount.getUuid()), + UsernameTable.KEY_USERNAME_HASH, AttributeValues.fromByteArray(usernameHash), + UsernameTable.ATTR_CONFIRMED, AttributeValues.fromBool(true))) + // it's not in the constraint table OR it's expired OR it was reserved by us + .conditionExpression("attribute_not_exists(#username_hash) OR #ttl < :now OR (#aci = :aci AND #confirmed = :confirmed)") + .expressionAttributeNames(Map.of( + "#username_hash", UsernameTable.KEY_USERNAME_HASH, + "#ttl", UsernameTable.ATTR_TTL, + "#aci", UsernameTable.ATTR_ACCOUNT_UUID, + "#confirmed", UsernameTable.ATTR_CONFIRMED)) + .expressionAttributeValues(Map.of( + ":now", AttributeValues.fromLong(clock.instant().getEpochSecond()), + ":aci", AttributeValues.fromUUID(updatedAccount.getUuid()), + ":confirmed", AttributeValues.fromBool(false))) + .returnValuesOnConditionCheckFailure(ReturnValuesOnConditionCheckFailure.ALL_OLD) + .build()) + .build()); - // 3?: Adding that hold may have caused our account to exceed our maximum holds. Release an old hold - holdToRemove.ifPresent(oldHold -> - writeItems.add(releaseHoldIfAllowedTransactItem(updatedAccount.getUuid(), oldHold, now))); + // 1: update the account object (conditioned on the version increment) + writeItems.add(UpdateAccountSpec.forAccount(accountsTableName, updatedAccount).transactItem()); - return dynamoDbAsyncClient.transactWriteItems(TransactWriteItemsRequest.builder().transactItems(writeItems).build()) - .thenApply(ignored -> updatedAccount); - }) - .thenApply(updatedAccount -> { - account.setUsernameHash(usernameHash); - account.setReservedUsernameHash(null); - account.setUsernameLinkDetails(updatedAccount.getUsernameLinkHandle(), updatedAccount.getEncryptedUsername().orElse(null)); - account.setUsernameHolds(updatedAccount.getUsernameHolds()); - account.setVersion(account.getVersion() + 1); - return (Void) null; - }) - .exceptionally(ExceptionUtils.exceptionallyHandler(TransactionCanceledException.class, e -> { - // If the constraint table update failed the condition check, the username's taken and we should stop - // trying. However, if the accounts table fails the conditional check or either table was concurrently - // updated, it's an optimistic locking failure and we should try again. - if (conditionalCheckFailed(e.cancellationReasons().get(0))) { - throw ExceptionUtils.wrap(new UsernameHashNotAvailableException()); - } else if (conditionalCheckFailed(e.cancellationReasons().get(1)) // Account version conflict - // When we looked at the holds on our account, we thought we still held the corresponding username - // reservation. But it turned out that someone else has taken the reservation since. This means that the - // TTL on the hold must have just expired, so if we retry we should see that our hold is expired, and we - // won't try to remove it again. - || (e.cancellationReasons().size() > 3 && conditionalCheckFailed(e.cancellationReasons().get(3))) - // concurrent update on any table - || e.cancellationReasons().stream().anyMatch(Accounts::isTransactionConflict)) { - throw new ContestedOptimisticLockException(); - } else { - throw ExceptionUtils.wrap(e); - } - })) - .whenComplete((ignored, throwable) -> sample.stop(SET_USERNAME_TIMER)); + // 2?: Add a temporary hold for the old username to stop others from claiming it + maybeOriginalUsernameHash.ifPresent(originalUsernameHash -> + writeItems.add(holdUsernameTransactItem(updatedAccount.getUuid(), originalUsernameHash, now))); + + // 3?: Adding that hold may have caused our account to exceed our maximum holds. Release an old hold + holdToRemove.ifPresent(oldHold -> + writeItems.add(releaseHoldIfAllowedTransactItem(updatedAccount.getUuid(), oldHold, now))); + + try { + dynamoDbClient.transactWriteItems(TransactWriteItemsRequest.builder().transactItems(writeItems).build()); + + account.setUsernameHash(usernameHash); + account.setReservedUsernameHash(null); + account.setUsernameLinkDetails(updatedAccount.getUsernameLinkHandle(), updatedAccount.getEncryptedUsername().orElse(null)); + account.setUsernameHolds(updatedAccount.getUsernameHolds()); + account.setVersion(account.getVersion() + 1); + } catch (final TransactionCanceledException e) { + if (conditionalCheckFailed(e.cancellationReasons().get(0))) { + throw new UsernameHashNotAvailableException(); + } else if (conditionalCheckFailed(e.cancellationReasons().get(1)) // Account version conflict + // When we looked at the holds on our account, we thought we still held the corresponding username + // reservation. But it turned out that someone else has taken the reservation since. This means that the + // TTL on the hold must have just expired, so if we retry we should see that our hold is expired, and we + // won't try to remove it again. + || (e.cancellationReasons().size() > 3 && conditionalCheckFailed(e.cancellationReasons().get(3))) + // concurrent update on any table + || e.cancellationReasons().stream().anyMatch(Accounts::isTransactionConflict)) { + throw new ContestedOptimisticLockException(); + } else { + throw e; + } + } finally { + sample.stop(SET_USERNAME_TIMER); + } } - private CompletableFuture pickLinkHandle(final Account account, final byte[] usernameHash) { + private UUID pickLinkHandle(final Account account, final byte[] usernameHash) { if (account.getUsernameLinkHandle() == null) { // There's no old link handle, so we can just use a randomly generated link handle - return CompletableFuture.completedFuture(UUID.randomUUID()); + return UUID.randomUUID(); } // Otherwise, there's an existing link handle. If this is the result of an account being re-registered, we should // preserve the link handle. - return dynamoDbAsyncClient.getItem(GetItemRequest.builder() + final GetItemResponse response = dynamoDbClient.getItem(GetItemRequest.builder() .tableName(usernamesConstraintTableName) .key(Map.of(UsernameTable.KEY_USERNAME_HASH, AttributeValues.b(usernameHash))) - .projectionExpression(UsernameTable.ATTR_RECLAIMABLE).build()) - .thenApply(response -> { - if (response.hasItem() && AttributeValues.getBool(response.item(), UsernameTable.ATTR_RECLAIMABLE, false)) { - // this username reservation indicates it's a username waiting to be "reclaimed" - return account.getUsernameLinkHandle(); - } - // There was no existing username reservation, or this was a standard "new" username. Either way, we should - // generate a new link handle. - return UUID.randomUUID(); - }); + .projectionExpression(UsernameTable.ATTR_RECLAIMABLE).build()); + + if (response.hasItem() && AttributeValues.getBool(response.item(), UsernameTable.ATTR_RECLAIMABLE, false)) { + // this username reservation indicates it's a username waiting to be "reclaimed" + return account.getUsernameLinkHandle(); + } + + // There was no existing username reservation, or this was a standard "new" username. Either way, we should + // generate a new link handle. + return UUID.randomUUID(); } - /** - * Clear the username hash and link from the given account - * - * @param account to update - * @return a future that completes once the username data has been cleared; - * it can fail with a {@link ContestedOptimisticLockException} if there are concurrent updates - * to the account or username constraint records. - */ - public CompletableFuture clearUsernameHash(final Account account) { + /// Clear the username hash and link from the given account + /// + /// @param account to update + /// + /// @throws ContestedOptimisticLockException if there are concurrent updates to the account or username constraint + /// records + public void clearUsernameHash(final Account account) throws ContestedOptimisticLockException { if (account.getUsernameHash().isEmpty()) { // no username to clear - return CompletableFuture.completedFuture(null); + return; } + final byte[] usernameHash = account.getUsernameHash().get(); final Timer.Sample sample = Timer.start(); @@ -872,28 +865,29 @@ public class Accounts { // 2?: Adding that hold may have caused our account to exceed our maximum holds. Release an old hold holdToRemove.ifPresent(oldHold -> items.add(releaseHoldIfAllowedTransactItem(updatedAccount.getUuid(), oldHold, now))); - return dynamoDbAsyncClient.transactWriteItems(TransactWriteItemsRequest.builder().transactItems(items).build()) - .thenAccept(ignored -> { - account.setUsernameHash(null); - account.setUsernameLinkDetails(null, null); - account.setVersion(account.getVersion() + 1); - account.setUsernameHolds(updatedAccount.getUsernameHolds()); - }) - .exceptionally(ExceptionUtils.exceptionallyHandler(TransactionCanceledException.class, e -> { - if (conditionalCheckFailed(e.cancellationReasons().get(0)) // Account version conflict - // When we looked at the holds on our account, we thought we still held the corresponding username - // reservation. But it turned out that someone else has taken the reservation since. This means that the - // TTL on the hold must have just expired, so if we retry we should see that our hold is expired, and we - // won't try to remove it again. - || (e.cancellationReasons().size() > 2 && conditionalCheckFailed(e.cancellationReasons().get(2))) - // concurrent update on any table - || e.cancellationReasons().stream().anyMatch(Accounts::isTransactionConflict)) { - throw new ContestedOptimisticLockException(); - } else { - throw ExceptionUtils.wrap(e); - } - })) - .whenComplete((ignored, throwable) -> sample.stop(CLEAR_USERNAME_HASH_TIMER)); + try { + dynamoDbClient.transactWriteItems(TransactWriteItemsRequest.builder().transactItems(items).build()); + + account.setUsernameHash(null); + account.setUsernameLinkDetails(null, null); + account.setVersion(account.getVersion() + 1); + account.setUsernameHolds(updatedAccount.getUsernameHolds()); + } catch (final TransactionCanceledException e) { + if (conditionalCheckFailed(e.cancellationReasons().get(0)) // Account version conflict + // When we looked at the holds on our account, we thought we still held the corresponding username + // reservation. But it turned out that someone else has taken the reservation since. This means that the + // TTL on the hold must have just expired, so if we retry we should see that our hold is expired, and we + // won't try to remove it again. + || (e.cancellationReasons().size() > 2 && conditionalCheckFailed(e.cancellationReasons().get(2))) + // concurrent update on any table + || e.cancellationReasons().stream().anyMatch(Accounts::isTransactionConflict)) { + throw new ContestedOptimisticLockException(); + } else { + throw e; + } + } finally { + sample.stop(CLEAR_USERNAME_HASH_TIMER); + } } /** @@ -1056,7 +1050,7 @@ public class Accounts { return dynamoDbAsyncClient.transactWriteItems(TransactWriteItemsRequest.builder() .transactItems(writeItems) .build()) - .thenApply(response -> { + .thenApply(_ -> { account.setVersion(account.getVersion() + 1); return (Void) null; }) @@ -1142,7 +1136,7 @@ public class Accounts { return itemByGsiKeyAsync(accountsTableName, USERNAME_LINK_TO_UUID_INDEX, ATTR_USERNAME_LINK_UUID, AttributeValues.fromUUID(usernameLinkHandle)) .thenApply(maybeItem -> maybeItem.map(Accounts::fromItem)) - .whenComplete((account, throwable) -> sample.stop(GET_BY_USERNAME_LINK_HANDLE_TIMER)); + .whenComplete((_, _) -> sample.stop(GET_BY_USERNAME_LINK_HANDLE_TIMER)); } @Nonnull @@ -1284,7 +1278,7 @@ public class Accounts { final String tableName, final String keyName, final AttributeValue keyValue) { - return getByIndirectLookup(timer, tableName, keyName, keyValue, i -> true); + return getByIndirectLookup(timer, tableName, keyName, keyValue, _ -> true); } @Nonnull @@ -1294,7 +1288,7 @@ public class Accounts { final String keyName, final AttributeValue keyValue) { - return getByIndirectLookupAsync(timer, tableName, keyName, keyValue, i -> true); + return getByIndirectLookupAsync(timer, tableName, keyName, keyValue, _ -> true); } @Nonnull @@ -1607,7 +1601,7 @@ public class Accounts { sb.append("???"); sb.append(StringUtils.length(phoneNumber) < 3 ? "" - : phoneNumber.substring(phoneNumber.length() - 2, phoneNumber.length())); + : phoneNumber.substring(phoneNumber.length() - 2)); return sb.toString(); } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java index cc21f137a..907e3fe31 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java @@ -88,6 +88,7 @@ import org.whispersystems.textsecuregcm.util.Pair; import org.whispersystems.textsecuregcm.util.RegistrationIdValidator; import org.whispersystems.textsecuregcm.util.ResilienceUtil; import org.whispersystems.textsecuregcm.util.SystemMapper; +import org.whispersystems.textsecuregcm.util.ThrowingConsumer; import org.whispersystems.textsecuregcm.util.Util; import reactor.core.publisher.Flux; import reactor.core.scheduler.Scheduler; @@ -830,119 +831,116 @@ public class AccountsManager extends RedisPubSubAdapter implemen public record UsernameReservation(Account account, byte[] reservedUsernameHash){} - /** - * Reserve a username hash so that no other accounts may take it. - *

- * The reserved hash can later be set with {@link #confirmReservedUsernameHash(Account, byte[], byte[])}. The reservation - * will eventually expire, after which point confirmReservedUsernameHash may fail if another account has taken the - * username hash. - * - * @param account the account to update - * @param requestedUsernameHashes the list of username hashes to attempt to reserve - * @return a future that yields the reserved username hash and an updated Account object on success; may fail with a - * {@link UsernameHashNotAvailableException} if none of the given username hashes are available - */ - public CompletableFuture reserveUsernameHash(final Account account, final List requestedUsernameHashes) { + /// Reserve a username hash so that no other accounts may take it. + /// + /// The reserved hash can later be set with [#confirmReservedUsernameHash(Account, byte\[\], byte\[\])]. The + /// reservation will eventually expire, after which point confirmReservedUsernameHash may fail if another account has + /// taken the username hash. + /// + /// @param account the account to update + /// @param requestedUsernameHashes the list of username hashes to attempt to reserve + /// + /// @return the reserved username hash + /// + /// @throws UsernameHashNotAvailableException if none of the given username hashes are available + public UsernameReservation reserveUsernameHash(final Account account, final List requestedUsernameHashes) + throws UsernameHashNotAvailableException { if (account.getUsernameHash().filter( oldHash -> requestedUsernameHashes.stream().anyMatch(hash -> Arrays.equals(oldHash, hash))) .isPresent()) { + // if we are trying to reserve our already-confirmed username hash, we don't need to do // anything, and can give the client a success response (they may try to confirm it again, // but that's a no-op other than rotaing their username link which they may need to do // anyway). note this is *not* the case for reserving our already-reserved username hash, // which should extend the reservation's TTL. - return CompletableFuture.completedFuture(new UsernameReservation(account, account.getUsernameHash().get())); + return new UsernameReservation(account, account.getUsernameHash().get()); } final AtomicReference reservedUsernameHash = new AtomicReference<>(); - return redisDeleteAsync(account) - .thenCompose(ignored -> updateWithRetriesAsync( - account, - a -> true, - a -> checkAndReserveNextUsernameHash(a, new ArrayDeque<>(requestedUsernameHashes)) - .thenAccept(reservedUsernameHash::set), - () -> accounts.getByAccountIdentifierAsync(account.getUuid()).thenApply(Optional::orElseThrow), - AccountChangeValidator.USERNAME_CHANGE_VALIDATOR, - MAX_UPDATE_ATTEMPTS)) - .whenComplete((updatedAccount, throwable) -> { - if (throwable == null) { - // Make a best effort to clear any stale data that may have been cached while this operation was in progress - redisDeleteAsync(updatedAccount); - } - }) - .thenApply(updatedAccount -> new UsernameReservation(updatedAccount, reservedUsernameHash.get())); + redisDelete(account); + + final Account updatedAccount = updateWithRetries( + account, + _ -> true, + a -> reservedUsernameHash.set( + checkAndReserveNextUsernameHash(a, new ArrayDeque<>(requestedUsernameHashes))), + () -> accounts.getByAccountIdentifier(account.getUuid()).orElseThrow(), + AccountChangeValidator.USERNAME_CHANGE_VALIDATOR); + + redisDelete(updatedAccount); + + return new UsernameReservation(updatedAccount, reservedUsernameHash.get()); } - private CompletableFuture checkAndReserveNextUsernameHash(final Account account, final Queue requestedUsernameHashes) { + private byte[] checkAndReserveNextUsernameHash(final Account account, final Queue requestedUsernameHashes) + throws UsernameHashNotAvailableException { + final byte[] usernameHash = requestedUsernameHashes.remove(); - return accounts.reserveUsernameHash(account, usernameHash, USERNAME_HASH_RESERVATION_TTL_MINUTES) - .thenApply(ignored -> usernameHash) - .exceptionallyComposeAsync( - throwable -> { - if (ExceptionUtils.unwrap(throwable) instanceof UsernameHashNotAvailableException && !requestedUsernameHashes.isEmpty()) { - return checkAndReserveNextUsernameHash(account, requestedUsernameHashes); - } - return CompletableFuture.failedFuture(throwable); - }); + try { + accounts.reserveUsernameHash(account, usernameHash, USERNAME_HASH_RESERVATION_TTL_MINUTES); + return usernameHash; + } catch (final UsernameHashNotAvailableException e) { + if (!requestedUsernameHashes.isEmpty()) { + return checkAndReserveNextUsernameHash(account, requestedUsernameHashes); + } + + throw e; + } } - /** - * Set a username hash previously reserved with {@link #reserveUsernameHash(Account, List)} - * - * @param account the account to update - * @param reservedUsernameHash the previously reserved username hash - * @param encryptedUsername the encrypted form of the previously reserved username for the username link - * @return a future that yields the updated account with the username hash field set; may fail with a - * {@link UsernameHashNotAvailableException} if the reserved username hash has been taken (because the reservation - * expired) or a {@link UsernameReservationNotFoundException} if {@code reservedUsernameHash} was not reserved in the - * account - */ - public CompletableFuture confirmReservedUsernameHash(final Account account, final byte[] reservedUsernameHash, @Nullable final byte[] encryptedUsername) { + /// Set a username hash previously reserved with {@link #reserveUsernameHash(Account, List)} + /// + /// @param account the account to update + /// @param reservedUsernameHash the previously reserved username hash + /// @param encryptedUsername the encrypted form of the previously reserved username for the username link + /// + /// @return the updated account with the username hash field set + /// + /// @throws UsernameHashNotAvailableException if the reserved username hash has been taken (because the reservation + /// expired) + /// @throws UsernameReservationNotFoundException if `reservedUsernameHash` was not reserved for the account + public Account confirmReservedUsernameHash(final Account account, final byte[] reservedUsernameHash, @Nullable final byte[] encryptedUsername) + throws UsernameReservationNotFoundException, UsernameHashNotAvailableException { + if (account.getUsernameHash().map(currentUsernameHash -> Arrays.equals(currentUsernameHash, reservedUsernameHash)).orElse(false)) { // the client likely already succeeded and is retrying - return CompletableFuture.completedFuture(account); + return account; } if (!account.getReservedUsernameHash().map(oldHash -> Arrays.equals(oldHash, reservedUsernameHash)).orElse(false)) { // no such reservation existed, either there was no previous call to reserveUsername // or the reservation changed - return CompletableFuture.failedFuture(new UsernameReservationNotFoundException()); + throw new UsernameReservationNotFoundException(); } - return redisDeleteAsync(account) - .thenCompose(ignored -> updateWithRetriesAsync( - account, - a -> true, - a -> accounts.confirmUsernameHash(a, reservedUsernameHash, encryptedUsername), - () -> accounts.getByAccountIdentifierAsync(account.getUuid()).thenApply(Optional::orElseThrow), - AccountChangeValidator.USERNAME_CHANGE_VALIDATOR, - MAX_UPDATE_ATTEMPTS - )) - .whenComplete((updatedAccount, throwable) -> { - if (throwable == null) { - // Make a best effort to clear any stale data that may have been cached while this operation was in progress - redisDeleteAsync(updatedAccount); - } - }); + redisDelete(account); + + final Account updatedAccount = updateWithRetries(account, + _ -> true, + a -> accounts.confirmUsernameHash(a, reservedUsernameHash, encryptedUsername), + () -> accounts.getByAccountIdentifier(account.getUuid()).orElseThrow(), + AccountChangeValidator.USERNAME_CHANGE_VALIDATOR); + + redisDelete(updatedAccount); + + return updatedAccount; } - public CompletableFuture clearUsernameHash(final Account account) { - return redisDeleteAsync(account) - .thenCompose(ignored -> updateWithRetriesAsync( - account, - a -> true, - accounts::clearUsernameHash, - () -> accounts.getByAccountIdentifierAsync(account.getUuid()).thenApply(Optional::orElseThrow), - AccountChangeValidator.USERNAME_CHANGE_VALIDATOR, - MAX_UPDATE_ATTEMPTS)) - .whenComplete((updatedAccount, throwable) -> { - if (throwable == null) { - // Make a best effort to clear any stale data that may have been cached while this operation was in progress - redisDeleteAsync(updatedAccount); - } - }); + public Account clearUsernameHash(final Account account) { + redisDelete(account); + + final Account updatedAccount = updateWithRetries(account, + _ -> true, + accounts::clearUsernameHash, + () -> accounts.getByAccountIdentifier(account.getUuid()).orElseThrow(), + AccountChangeValidator.USERNAME_CHANGE_VALIDATOR); + + redisDelete(updatedAccount); + + return updatedAccount; } public Account update(Account account, Consumer updater) { @@ -1031,11 +1029,11 @@ public class AccountsManager extends RedisPubSubAdapter implemen .whenComplete((_, _) -> timerSample.stop(updateTimer)); } - private Account updateWithRetries(Account account, + private Account updateWithRetries(Account account, final Function updater, - final Consumer persister, + final ThrowingConsumer persister, final Supplier retriever, - final AccountChangeValidator changeValidator) { + final AccountChangeValidator changeValidator) throws E { Account originalAccount = AccountUtil.cloneAccountAsNotStale(account); @@ -1129,12 +1127,6 @@ public class AccountsManager extends RedisPubSubAdapter implemen return getByNumberTimer.record(() -> accounts.getByE164(number)); } - public CompletableFuture> getByE164Async(final String number) { - Timer.Sample sample = Timer.start(); - return accounts.getByE164Async(number) - .whenComplete((ignoredResult, ignoredThrowable) -> sample.stop(getByNumberTimer)); - } - public Optional getByPhoneNumberIdentifier(final UUID pni) { return checkRedisThenAccounts( getByNumberTimer, @@ -1209,10 +1201,6 @@ public class AccountsManager extends RedisPubSubAdapter implemen return accounts.getAll(segments, scheduler); } - public Flux streamAccountIdentifiersFromDynamo(final int segments, final Scheduler scheduler) { - return accounts.getAllAccountIdentifiers(segments, scheduler); - } - public CompletableFuture delete(final Account account, final DeletionReason deletionReason) { final Timer.Sample sample = Timer.start(); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/util/ThrowingConsumer.java b/service/src/main/java/org/whispersystems/textsecuregcm/util/ThrowingConsumer.java new file mode 100644 index 000000000..f436f2faa --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/util/ThrowingConsumer.java @@ -0,0 +1,19 @@ +/* + * Copyright 2026 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.util; + +/// Represents an operation that accepts a single input argument and returns no result, but may throw a checked +/// exception. Unlike most other functional interfaces, `ThrowingConsumer` is expected to operate via side-effects. +@FunctionalInterface +public interface ThrowingConsumer { + + /// Performs this operation on the given argument. + /// + /// @param t the input argument + /// + /// @throws E at the implementation's discretion + void accept(T t) throws E; +} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java index 5a702a068..092c1a241 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/AccountControllerTest.java @@ -504,9 +504,9 @@ class AccountControllerTest { } @Test - void testReserveUsernameHash() { + void testReserveUsernameHash() throws UsernameHashNotAvailableException { when(accountsManager.reserveUsernameHash(any(), any())) - .thenReturn(CompletableFuture.completedFuture(new AccountsManager.UsernameReservation(null, USERNAME_HASH_1))); + .thenReturn(new AccountsManager.UsernameReservation(null, USERNAME_HASH_1)); try (final Response response = resources.getJerseyTest() .target("/v1/accounts/username_hash/reserve") @@ -521,9 +521,9 @@ class AccountControllerTest { } @Test - void testReserveUsernameHashUnavailable() { + void testReserveUsernameHashUnavailable() throws UsernameHashNotAvailableException { when(accountsManager.reserveUsernameHash(any(), anyList())) - .thenReturn(CompletableFuture.failedFuture(new UsernameHashNotAvailableException())); + .thenThrow(new UsernameHashNotAvailableException()); try (final Response response = resources.getJerseyTest() .target("/v1/accounts/username_hash/reserve") @@ -604,13 +604,14 @@ class AccountControllerTest { } @Test - void testConfirmUsernameHash() throws BaseUsernameException { + void testConfirmUsernameHash() + throws BaseUsernameException, UsernameHashNotAvailableException, UsernameReservationNotFoundException { Account account = mock(Account.class); final UUID uuid = UUID.randomUUID(); when(account.getUsernameHash()).thenReturn(Optional.of(USERNAME_HASH_1)); when(account.getUsernameLinkHandle()).thenReturn(uuid); when(accountsManager.confirmReservedUsernameHash(any(), eq(USERNAME_HASH_1), eq(ENCRYPTED_USERNAME_1))) - .thenReturn(CompletableFuture.completedFuture(account)); + .thenReturn(account); try (final Response response = resources.getJerseyTest() .target("/v1/accounts/username_hash/confirm") @@ -641,12 +642,13 @@ class AccountControllerTest { } @Test - void testConfirmUsernameHashOld() throws BaseUsernameException { + void testConfirmUsernameHashOld() + throws BaseUsernameException, UsernameHashNotAvailableException, UsernameReservationNotFoundException { Account account = mock(Account.class); when(account.getUsernameHash()).thenReturn(Optional.of(USERNAME_HASH_1)); when(account.getUsernameLinkHandle()).thenReturn(null); when(accountsManager.confirmReservedUsernameHash(any(), eq(USERNAME_HASH_1), eq(null))) - .thenReturn(CompletableFuture.completedFuture(account)); + .thenReturn(account); try (final Response response = resources.getJerseyTest() .target("/v1/accounts/username_hash/confirm") @@ -664,9 +666,10 @@ class AccountControllerTest { } @Test - void testConfirmUnreservedUsernameHash() throws BaseUsernameException { + void testConfirmUnreservedUsernameHash() + throws BaseUsernameException, UsernameHashNotAvailableException, UsernameReservationNotFoundException { when(accountsManager.confirmReservedUsernameHash(any(), eq(USERNAME_HASH_1), any())) - .thenReturn(CompletableFuture.failedFuture(new UsernameReservationNotFoundException())); + .thenThrow(new UsernameReservationNotFoundException()); try (final Response response = resources.getJerseyTest() .target("/v1/accounts/username_hash/confirm") @@ -680,9 +683,10 @@ class AccountControllerTest { } @Test - void testConfirmLapsedUsernameHash() throws BaseUsernameException { + void testConfirmLapsedUsernameHash() + throws BaseUsernameException, UsernameHashNotAvailableException, UsernameReservationNotFoundException { when(accountsManager.confirmReservedUsernameHash(any(), eq(USERNAME_HASH_1), any())) - .thenReturn(CompletableFuture.failedFuture(new UsernameHashNotAvailableException())); + .thenThrow(new UsernameHashNotAvailableException()); try (final Response response = resources.getJerseyTest() .target("/v1/accounts/username_hash/confirm") @@ -748,7 +752,7 @@ class AccountControllerTest { @Test void testDeleteUsername() { when(accountsManager.clearUsernameHash(any())) - .thenAnswer(invocation -> CompletableFutureTestUtil.almostCompletedFuture(invocation.getArgument(0))); + .thenAnswer(invocation -> invocation.getArgument(0)); try (final Response response = resources.getJerseyTest() .target("/v1/accounts/username_hash/") @@ -756,7 +760,6 @@ class AccountControllerTest { .header(HttpHeaders.AUTHORIZATION, AuthHelper.getAuthHeader(AuthHelper.VALID_UUID, AuthHelper.VALID_PASSWORD)) .delete()) { - assertThat(response.readEntity(String.class)).isEqualTo(""); assertThat(response.getStatus()).isEqualTo(204); verify(accountsManager).clearUsernameHash(AuthHelper.VALID_ACCOUNT); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcServiceTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcServiceTest.java index eb067dbd0..460777bdc 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcServiceTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/AccountsGrpcServiceTest.java @@ -245,7 +245,7 @@ class AccountsGrpcServiceTest extends SimpleBaseGrpcTest { final List usernameHashes = invocation.getArgument(1); - return CompletableFuture.completedFuture( - new AccountsManager.UsernameReservation(invocation.getArgument(0), usernameHashes.getFirst())); + return new AccountsManager.UsernameReservation(invocation.getArgument(0), usernameHashes.getFirst()); }); final ReserveUsernameHashResponse expectedResponse = ReserveUsernameHashResponse.newBuilder() @@ -272,7 +271,7 @@ class AccountsGrpcServiceTest extends SimpleBaseGrpcTest authenticatedServiceStub().deleteUsernameHash(DeleteUsernameHashRequest.newBuilder().build())); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java index 5aea36233..9702cf778 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java @@ -97,7 +97,6 @@ import org.whispersystems.textsecuregcm.tests.util.KeysHelper; import org.whispersystems.textsecuregcm.tests.util.MockRedisFuture; import org.whispersystems.textsecuregcm.tests.util.RedisClusterHelper; import org.whispersystems.textsecuregcm.tests.util.RedisServerHelper; -import org.whispersystems.textsecuregcm.util.CompletableFutureTestUtil; import org.whispersystems.textsecuregcm.util.Pair; import org.whispersystems.textsecuregcm.util.TestClock; import org.whispersystems.textsecuregcm.util.TestRandomUtil; @@ -1214,53 +1213,56 @@ class AccountsManagerTest { } @Test - void testReserveUsernameHash() { + void testReserveUsernameHash() throws UsernameHashNotAvailableException { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); - when(accounts.getByAccountIdentifierAsync(account.getUuid())).thenReturn(CompletableFuture.completedFuture(Optional.of(account))); + when(accounts.getByAccountIdentifier(account.getUuid())).thenReturn(Optional.of(account)); final List usernameHashes = List.of(TestRandomUtil.nextBytes(32), TestRandomUtil.nextBytes(32)); - when(accounts.reserveUsernameHash(any(), any(), any())).thenReturn(CompletableFuture.completedFuture(null)); - UsernameReservation result = accountsManager.reserveUsernameHash(account, usernameHashes).join(); - assertArrayEquals(usernameHashes.get(0), result.reservedUsernameHash()); + final UsernameReservation result = accountsManager.reserveUsernameHash(account, usernameHashes); + assertArrayEquals(usernameHashes.getFirst(), result.reservedUsernameHash()); verify(accounts, times(1)).reserveUsernameHash(eq(account), any(), eq(Duration.ofMinutes(5))); } @Test - void testReserveOwnUsernameHash() { + void testReserveOwnUsernameHash() throws UsernameHashNotAvailableException { final byte[] oldUsernameHash = TestRandomUtil.nextBytes(32); final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); account.setUsernameHash(oldUsernameHash); - when(accounts.getByAccountIdentifierAsync(account.getUuid())).thenReturn(CompletableFuture.completedFuture(Optional.of(account))); + when(accounts.getByAccountIdentifier(account.getUuid())).thenReturn(Optional.of(account)); final List usernameHashes = List.of(TestRandomUtil.nextBytes(32), oldUsernameHash, TestRandomUtil.nextBytes(32)); - UsernameReservation result = accountsManager.reserveUsernameHash(account, usernameHashes).join(); + final UsernameReservation result = accountsManager.reserveUsernameHash(account, usernameHashes); assertArrayEquals(oldUsernameHash, result.reservedUsernameHash()); verify(accounts, never()).reserveUsernameHash(any(), any(), any()); } @Test - void testReserveUsernameOptimisticLockingFailure() { + void testReserveUsernameOptimisticLockingFailure() throws UsernameHashNotAvailableException { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); - when(accounts.getByAccountIdentifierAsync(account.getUuid())).thenReturn(CompletableFuture.completedFuture(Optional.of(account))); + when(accounts.getByAccountIdentifier(account.getUuid())).thenReturn(Optional.of(account)); final List usernameHashes = List.of(TestRandomUtil.nextBytes(32), TestRandomUtil.nextBytes(32)); - when(accounts.reserveUsernameHash(any(), any(), any())) - .thenReturn(CompletableFuture.failedFuture(new ContestedOptimisticLockException())) - .thenReturn(CompletableFuture.completedFuture(null)); - UsernameReservation result = accountsManager.reserveUsernameHash(account, usernameHashes).join(); - assertArrayEquals(usernameHashes.get(0), result.reservedUsernameHash()); + doThrow(new ContestedOptimisticLockException()) + .doNothing() + .when(accounts).reserveUsernameHash(any(), any(), any()); + + final UsernameReservation result = accountsManager.reserveUsernameHash(account, usernameHashes); + assertArrayEquals(usernameHashes.getFirst(), result.reservedUsernameHash()); verify(accounts, times(2)).reserveUsernameHash(eq(account), any(), eq(Duration.ofMinutes(5))); } @Test - void testReserveUsernameHashNotAvailable() { + void testReserveUsernameHashAsyncNotAvailable() throws UsernameHashNotAvailableException { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); when(accounts.getByAccountIdentifierAsync(account.getUuid())).thenReturn(CompletableFuture.completedFuture(Optional.of(account))); - when(accounts.reserveUsernameHash(any(), any(), any())).thenReturn(CompletableFuture.failedFuture(new UsernameHashNotAvailableException())); - CompletableFutureTestUtil.assertFailsWithCause(UsernameHashNotAvailableException.class, + + doThrow(new UsernameHashNotAvailableException()) + .when(accounts).reserveUsernameHash(any(), any(), any()); + + assertThrows(UsernameHashNotAvailableException.class, () -> accountsManager.reserveUsernameHash(account, List.of(USERNAME_HASH_1, USERNAME_HASH_2))); } @@ -1269,10 +1271,7 @@ class AccountsManagerTest { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); setReservationHash(account, USERNAME_HASH_1); - when(accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)) - .thenReturn(CompletableFuture.completedFuture(null)); - - accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); verify(accounts).confirmUsernameHash(eq(account), eq(USERNAME_HASH_1), eq(ENCRYPTED_USERNAME_1)); } @@ -1280,13 +1279,13 @@ class AccountsManagerTest { void testConfirmReservedUsernameHashOptimisticLockingFailure() throws UsernameHashNotAvailableException, UsernameReservationNotFoundException { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); setReservationHash(account, USERNAME_HASH_1); - when(accounts.getByAccountIdentifierAsync(account.getUuid())).thenReturn(CompletableFuture.completedFuture(Optional.of(account))); + when(accounts.getByAccountIdentifier(account.getUuid())).thenReturn(Optional.of(account)); - when(accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)) - .thenReturn(CompletableFuture.failedFuture(new ContestedOptimisticLockException())) - .thenReturn(CompletableFuture.completedFuture(null)); + doThrow(new ContestedOptimisticLockException()) + .doNothing() + .when(accounts).confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); - accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); verify(accounts, times(2)).confirmUsernameHash(eq(account), eq(USERNAME_HASH_1), eq(ENCRYPTED_USERNAME_1)); } @@ -1294,21 +1293,19 @@ class AccountsManagerTest { void testConfirmReservedHashNameMismatch() { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); setReservationHash(account, USERNAME_HASH_1); - when(accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)) - .thenReturn(CompletableFuture.completedFuture(null)); - CompletableFutureTestUtil.assertFailsWithCause(UsernameReservationNotFoundException.class, - accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_2, ENCRYPTED_USERNAME_2)); + assertThrows(UsernameReservationNotFoundException.class, + () -> accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_2, ENCRYPTED_USERNAME_2)); } @Test - void testConfirmReservedLapsed() { + void testConfirmReservedLapsed() throws UsernameHashNotAvailableException { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); // hash was reserved, but the reservation lapsed and another account took it setReservationHash(account, USERNAME_HASH_1); - when(accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)) - .thenReturn(CompletableFuture.failedFuture(new UsernameHashNotAvailableException())); - CompletableFutureTestUtil.assertFailsWithCause(UsernameHashNotAvailableException.class, - accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); + doThrow(new UsernameHashNotAvailableException()) + .when(accounts).confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); + assertThrows(UsernameHashNotAvailableException.class, + () -> accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); assertTrue(account.getUsernameHash().isEmpty()); } @@ -1318,27 +1315,24 @@ class AccountsManagerTest { account.setUsernameHash(USERNAME_HASH_1); // reserved username already set, should be treated as a replay - accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); verifyNoInteractions(accounts); } @Test - void testConfirmReservedUsernameHashWithNoReservation() { + void testConfirmReservedUsernameHashWithNoReservation() throws UsernameHashNotAvailableException { final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); - CompletableFutureTestUtil.assertFailsWithCause(UsernameReservationNotFoundException.class, - accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); + assertThrows(UsernameReservationNotFoundException.class, + () -> accountsManager.confirmReservedUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); verify(accounts, never()).confirmUsernameHash(any(), any(), any()); } @Test void testClearUsernameHash() { - when(accounts.clearUsernameHash(any())) - .thenReturn(CompletableFuture.completedFuture(null)); - - Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); + final Account account = AccountsHelper.generateTestAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID(), new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); account.setUsernameHash(USERNAME_HASH_1); - accountsManager.clearUsernameHash(account).join(); + accountsManager.clearUsernameHash(account); verify(accounts).clearUsernameHash(eq(account)); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java index 3a93dd3f8..e5f694dba 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java @@ -8,6 +8,7 @@ package org.whispersystems.textsecuregcm.storage; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; @@ -46,7 +47,6 @@ import org.whispersystems.textsecuregcm.securevaluerecovery.SecureValueRecoveryC import org.whispersystems.textsecuregcm.storage.DynamoDbExtensionSchema.Tables; import org.whispersystems.textsecuregcm.tests.util.AccountsHelper; import org.whispersystems.textsecuregcm.util.AttributeValues; -import org.whispersystems.textsecuregcm.util.CompletableFutureTestUtil; import org.whispersystems.textsecuregcm.util.TestRandomUtil; import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient; import software.amazon.awssdk.services.dynamodb.model.AttributeValue; @@ -190,8 +190,8 @@ class AccountsManagerUsernameIntegrationTest { .build()); } - CompletableFutureTestUtil.assertFailsWithCause(UsernameHashNotAvailableException.class, - accountsManager.reserveUsernameHash(account, usernameHashes)); + assertThrows(UsernameHashNotAvailableException.class, + () -> accountsManager.reserveUsernameHash(account, usernameHashes)); assertThat(accountsManager.getByAccountIdentifier(account.getUuid()).orElseThrow().getUsernameHash()).isEmpty(); } @@ -217,19 +217,19 @@ class AccountsManagerUsernameIntegrationTest { final byte[] username = accountsManager .reserveUsernameHash(account, usernameHashes) - .join() .reservedUsernameHash(); assertArrayEquals(username, availableHash); } @Test - public void testReserveConfirmClear() throws InterruptedException { + public void testReserveConfirmClear() + throws InterruptedException, UsernameHashNotAvailableException, UsernameReservationNotFoundException { Account account = AccountsHelper.createAccount(accountsManager, "+18005551111"); // reserve AccountsManager.UsernameReservation reservation = - accountsManager.reserveUsernameHash(account, List.of(USERNAME_HASH_1)).join(); + accountsManager.reserveUsernameHash(account, List.of(USERNAME_HASH_1)); assertArrayEquals(reservation.account().getReservedUsernameHash().orElseThrow(), USERNAME_HASH_1); assertThat(accountsManager.getByUsernameHash(reservation.reservedUsernameHash()).join()).isEmpty(); @@ -238,7 +238,7 @@ class AccountsManagerUsernameIntegrationTest { account = accountsManager.confirmReservedUsernameHash( reservation.account(), reservation.reservedUsernameHash(), - ENCRYPTED_USERNAME_1).join(); + ENCRYPTED_USERNAME_1); assertArrayEquals(account.getUsernameHash().orElseThrow(), USERNAME_HASH_1); assertThat(accountsManager.getByUsernameHash(USERNAME_HASH_1).join().orElseThrow().getUuid()).isEqualTo( account.getUuid()); @@ -247,43 +247,45 @@ class AccountsManagerUsernameIntegrationTest { .isEqualTo(account.getUuid()); // clear - account = accountsManager.clearUsernameHash(account).join(); + account = accountsManager.clearUsernameHash(account); assertThat(accountsManager.getByUsernameHash(USERNAME_HASH_1).join()).isEmpty(); assertThat(accountsManager.getByAccountIdentifier(account.getUuid()).orElseThrow().getUsernameHash()).isEmpty(); } @Test - public void testHold() throws InterruptedException { + public void testHold() + throws InterruptedException, UsernameHashNotAvailableException, UsernameReservationNotFoundException { Account account = AccountsHelper.createAccount(accountsManager, "+18005551111"); AccountsManager.UsernameReservation reservation = - accountsManager.reserveUsernameHash(account, List.of(USERNAME_HASH_1)).join(); + accountsManager.reserveUsernameHash(account, List.of(USERNAME_HASH_1)); // confirm account = accountsManager.confirmReservedUsernameHash( reservation.account(), reservation.reservedUsernameHash(), - ENCRYPTED_USERNAME_1).join(); + ENCRYPTED_USERNAME_1); // clear - account = accountsManager.clearUsernameHash(account).join(); + account = accountsManager.clearUsernameHash(account); assertThat(accountsManager.getByUsernameHash(USERNAME_HASH_1).join()).isEmpty(); assertThat(accountsManager.getByAccountIdentifier(account.getUuid()).orElseThrow().getUsernameHash()).isEmpty(); assertThat(accountsManager.getByUsernameHash(reservation.reservedUsernameHash()).join()).isEmpty(); Account account2 = AccountsHelper.createAccount(accountsManager, "+18005552222"); - CompletableFutureTestUtil.assertFailsWithCause(UsernameHashNotAvailableException.class, - accountsManager.reserveUsernameHash(account2, List.of(USERNAME_HASH_1)), + assertThrows(UsernameHashNotAvailableException.class, + () -> accountsManager.reserveUsernameHash(account2, List.of(USERNAME_HASH_1)), "account2 should not be able to reserve a held hash"); } @Test - public void testReservationLapsed() throws InterruptedException { + public void testReservationLapsed() + throws InterruptedException, UsernameHashNotAvailableException, UsernameReservationNotFoundException { final Account account = AccountsHelper.createAccount(accountsManager, "+18005551111"); AccountsManager.UsernameReservation reservation1 = - accountsManager.reserveUsernameHash(account, List.of(USERNAME_HASH_1)).join(); + accountsManager.reserveUsernameHash(account, List.of(USERNAME_HASH_1)); long past = Instant.now().minus(Duration.ofMinutes(1)).getEpochSecond(); // force expiration @@ -299,29 +301,30 @@ class AccountsManagerUsernameIntegrationTest { Account account2 = AccountsHelper.createAccount(accountsManager, "+18005552222"); final AccountsManager.UsernameReservation reservation2 = - accountsManager.reserveUsernameHash(account2, List.of(USERNAME_HASH_1)).join(); + accountsManager.reserveUsernameHash(account2, List.of(USERNAME_HASH_1)); assertArrayEquals(reservation2.reservedUsernameHash(), USERNAME_HASH_1); - CompletableFutureTestUtil.assertFailsWithCause(UsernameHashNotAvailableException.class, - accountsManager.confirmReservedUsernameHash(reservation1.account(), USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); - account2 = accountsManager.confirmReservedUsernameHash(reservation2.account(), USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + assertThrows(UsernameHashNotAvailableException.class, + () -> accountsManager.confirmReservedUsernameHash(reservation1.account(), USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); + account2 = accountsManager.confirmReservedUsernameHash(reservation2.account(), USERNAME_HASH_1, ENCRYPTED_USERNAME_1); assertEquals(accountsManager.getByUsernameHash(USERNAME_HASH_1).join().orElseThrow().getUuid(), account2.getUuid()); assertArrayEquals(account2.getUsernameHash().orElseThrow(), USERNAME_HASH_1); } @Test - void testUsernameSetReserveAnotherClearSetReserved() throws InterruptedException { + void testUsernameSetReserveAnotherClearSetReserved() + throws InterruptedException, UsernameHashNotAvailableException, UsernameReservationNotFoundException { Account account = AccountsHelper.createAccount(accountsManager, "+18005551111"); // Set username hash final AccountsManager.UsernameReservation reservation1 = - accountsManager.reserveUsernameHash(account, List.of(USERNAME_HASH_1)).join(); + accountsManager.reserveUsernameHash(account, List.of(USERNAME_HASH_1)); - account = accountsManager.confirmReservedUsernameHash(reservation1.account(), USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + account = accountsManager.confirmReservedUsernameHash(reservation1.account(), USERNAME_HASH_1, ENCRYPTED_USERNAME_1); // Reserve another hash on the same account final AccountsManager.UsernameReservation reservation2 = - accountsManager.reserveUsernameHash(account, List.of(USERNAME_HASH_2)).join(); + accountsManager.reserveUsernameHash(account, List.of(USERNAME_HASH_2)); account = reservation2.account(); @@ -330,23 +333,23 @@ class AccountsManagerUsernameIntegrationTest { assertArrayEquals(account.getEncryptedUsername().orElseThrow(), ENCRYPTED_USERNAME_1); // Clear the set username hash but not the reserved one - account = accountsManager.clearUsernameHash(account).join(); + account = accountsManager.clearUsernameHash(account); assertThat(account.getReservedUsernameHash()).isPresent(); assertThat(account.getUsernameHash()).isEmpty(); // Confirm second reservation - account = accountsManager.confirmReservedUsernameHash(account, reservation2.reservedUsernameHash(), ENCRYPTED_USERNAME_2).join(); + account = accountsManager.confirmReservedUsernameHash(account, reservation2.reservedUsernameHash(), ENCRYPTED_USERNAME_2); assertArrayEquals(account.getUsernameHash().orElseThrow(), USERNAME_HASH_2); assertArrayEquals(account.getEncryptedUsername().orElseThrow(), ENCRYPTED_USERNAME_2); } @Test - public void testReclaim() throws InterruptedException { + public void testReclaim() + throws InterruptedException, UsernameHashNotAvailableException, UsernameReservationNotFoundException { Account account = AccountsHelper.createAccount(accountsManager, "+18005551111"); final AccountsManager.UsernameReservation reservation1 = - accountsManager.reserveUsernameHash(account, List.of(USERNAME_HASH_1)).join(); - account = accountsManager.confirmReservedUsernameHash(reservation1.account(), USERNAME_HASH_1, ENCRYPTED_USERNAME_1) - .join(); + accountsManager.reserveUsernameHash(account, List.of(USERNAME_HASH_1)); + account = accountsManager.confirmReservedUsernameHash(reservation1.account(), USERNAME_HASH_1, ENCRYPTED_USERNAME_1); // "reclaim" the account by re-registering Account reclaimed = AccountsHelper.createAccount(accountsManager, "+18005551111"); @@ -358,7 +361,7 @@ class AccountsManagerUsernameIntegrationTest { assertThat(accountsManager.getByUsernameHash(USERNAME_HASH_1).join()).isEmpty(); // confirm it again - accountsManager.confirmReservedUsernameHash(reclaimed, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accountsManager.confirmReservedUsernameHash(reclaimed, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); assertThat(accountsManager.getByUsernameHash(USERNAME_HASH_1).join()).isPresent(); } 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 d712c6989..2362bf351 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsTest.java @@ -65,7 +65,6 @@ import org.whispersystems.textsecuregcm.storage.DynamoDbExtensionSchema.Tables; import org.whispersystems.textsecuregcm.tests.util.AccountsHelper; import org.whispersystems.textsecuregcm.tests.util.DevicesHelper; import org.whispersystems.textsecuregcm.util.AttributeValues; -import org.whispersystems.textsecuregcm.util.CompletableFutureTestUtil; import org.whispersystems.textsecuregcm.util.SystemMapper; import org.whispersystems.textsecuregcm.util.TestClock; import org.whispersystems.textsecuregcm.util.TestRandomUtil; @@ -394,7 +393,7 @@ class AccountsTest { @ParameterizedTest @EnumSource(UsernameStatus.class) - void reclaimAccountWithNoUsername(UsernameStatus usernameStatus) { + void reclaimAccountWithNoUsername(UsernameStatus usernameStatus) throws UsernameHashNotAvailableException { Device device = generateDevice(DEVICE_ID_1); UUID firstUuid = UUID.randomUUID(); UUID firstPni = UUID.randomUUID(); @@ -407,12 +406,12 @@ class AccountsTest { case NONE: break; case RESERVED: - accounts.reserveUsernameHash(account, TestRandomUtil.nextBytes(32), Duration.ofMinutes(1)).join(); + accounts.reserveUsernameHash(account, TestRandomUtil.nextBytes(32), Duration.ofMinutes(1)); break; case RESERVED_WITH_SAVED_LINK: // give the account a username - accounts.reserveUsernameHash(account, usernameHash, Duration.ofMinutes(1)).join(); - accounts.confirmUsernameHash(account, usernameHash, encryptedUsername).join(); + accounts.reserveUsernameHash(account, usernameHash, Duration.ofMinutes(1)); + accounts.confirmUsernameHash(account, usernameHash, encryptedUsername); // simulate a partially-completed re-reg: we give the account a reclaimable username, but we'll try // re-registering again later in the test case @@ -420,8 +419,8 @@ class AccountsTest { reclaimAccount(account); break; case CONFIRMED: - accounts.reserveUsernameHash(account, usernameHash, Duration.ofMinutes(1)).join(); - accounts.confirmUsernameHash(account, usernameHash, encryptedUsername).join(); + accounts.reserveUsernameHash(account, usernameHash, Duration.ofMinutes(1)); + accounts.confirmUsernameHash(account, usernameHash, encryptedUsername); break; } @@ -433,7 +432,7 @@ class AccountsTest { // If we had a username link, or we had previously saved a username link from another re-registration, make sure // we preserve it - accounts.confirmUsernameHash(account, usernameHash, encryptedUsername).join(); + accounts.confirmUsernameHash(account, usernameHash, encryptedUsername); boolean shouldReuseLink = switch (usernameStatus) { case RESERVED_WITH_SAVED_LINK, CONFIRMED -> true; @@ -488,7 +487,7 @@ class AccountsTest { } @Test - void testReclaimAccount() { + void testReclaimAccount() throws UsernameHashNotAvailableException { final String e164 = "+14151112222"; final Device device = generateDevice(DEVICE_ID_1); final UUID existingUuid = UUID.randomUUID(); @@ -505,7 +504,7 @@ class AccountsTest { final byte[] encryptedUsername = TestRandomUtil.nextBytes(16); // Set up the existing account to have a username hash - accounts.confirmUsernameHash(existingAccount, usernameHash, encryptedUsername).join(); + accounts.confirmUsernameHash(existingAccount, usernameHash, encryptedUsername); final UUID usernameLinkHandle = existingAccount.getUsernameLinkHandle(); verifyStoredState(e164, existingAccount.getUuid(), existingAccount.getPhoneNumberIdentifier(), usernameHash, existingAccount, true); @@ -538,7 +537,7 @@ class AccountsTest { assertThat(result.getBackupVoucher()).isEqualTo(bv); // should keep the same usernameLink, now encryptedUsername should be set - accounts.confirmUsernameHash(result, usernameHash, encryptedUsername).join(); + accounts.confirmUsernameHash(result, usernameHash, encryptedUsername); item = readAccount(existingUuid); result = Accounts.fromItem(item); assertThat(AttributeValues.getUUID(item, Accounts.ATTR_USERNAME_LINK_UUID, null)) @@ -1025,14 +1024,14 @@ class AccountsTest { } @Test - void testSwitchUsernameHashes() { + void testSwitchUsernameHashes() throws UsernameHashNotAvailableException { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); createAccount(account); assertThat(accounts.getByUsernameHash(USERNAME_HASH_1).join()).isEmpty(); - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)).join(); - accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); + accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); final UUID oldHandle = account.getUsernameLinkHandle(); { @@ -1043,8 +1042,8 @@ class AccountsTest { verifyStoredState(account.getNumber(), account.getUuid(), account.getPhoneNumberIdentifier(), USERNAME_HASH_1, maybeAccount2.orElseThrow(), account); } - accounts.reserveUsernameHash(account, USERNAME_HASH_2, Duration.ofDays(1)).join(); - accounts.confirmUsernameHash(account, USERNAME_HASH_2, ENCRYPTED_USERNAME_2).join(); + accounts.reserveUsernameHash(account, USERNAME_HASH_2, Duration.ofDays(1)); + accounts.confirmUsernameHash(account, USERNAME_HASH_2, ENCRYPTED_USERNAME_2); final UUID newHandle = account.getUsernameLinkHandle(); // switching usernames should put a hold on our original username @@ -1079,8 +1078,8 @@ class AccountsTest { // first account reserves and confirms username hash assertThatNoException().isThrownBy(() -> { - accounts.reserveUsernameHash(firstAccount, USERNAME_HASH_1, Duration.ofDays(1)).join(); - accounts.confirmUsernameHash(firstAccount, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(firstAccount, USERNAME_HASH_1, Duration.ofDays(1)); + accounts.confirmUsernameHash(firstAccount, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); }); final Optional maybeAccount = accounts.getByUsernameHash(USERNAME_HASH_1).join(); @@ -1089,16 +1088,16 @@ class AccountsTest { verifyStoredState(firstAccount.getNumber(), firstAccount.getUuid(), firstAccount.getPhoneNumberIdentifier(), USERNAME_HASH_1, maybeAccount.get(), firstAccount); // throw an error if second account tries to reserve or confirm the same username hash - CompletableFutureTestUtil.assertFailsWithCause(UsernameHashNotAvailableException.class, - accounts.reserveUsernameHash(secondAccount, USERNAME_HASH_1, Duration.ofDays(1))); - CompletableFutureTestUtil.assertFailsWithCause(UsernameHashNotAvailableException.class, - accounts.confirmUsernameHash(secondAccount, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); + assertThrows(UsernameHashNotAvailableException.class, + () -> accounts.reserveUsernameHash(secondAccount, USERNAME_HASH_1, Duration.ofDays(1))); + assertThrows(UsernameHashNotAvailableException.class, + () -> accounts.confirmUsernameHash(secondAccount, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); // throw an error if first account tries to reserve or confirm the username hash that it has already confirmed - CompletableFutureTestUtil.assertFailsWithCause(UsernameHashNotAvailableException.class, - accounts.reserveUsernameHash(firstAccount, USERNAME_HASH_1, Duration.ofDays(1))); - CompletableFutureTestUtil.assertFailsWithCause(UsernameHashNotAvailableException.class, - accounts.confirmUsernameHash(firstAccount, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); + assertThrows(UsernameHashNotAvailableException.class, + () -> accounts.reserveUsernameHash(firstAccount, USERNAME_HASH_1, Duration.ofDays(1))); + assertThrows(UsernameHashNotAvailableException.class, + () -> accounts.confirmUsernameHash(firstAccount, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); assertThat(secondAccount.getReservedUsernameHash()).isEmpty(); assertThat(secondAccount.getUsernameHash()).isEmpty(); @@ -1110,12 +1109,12 @@ class AccountsTest { void testReserveUsernameHashTransactionConflict(final Optional constraintCancellationString, final Optional accountsCancellationString, final Class expectedException) { - final DynamoDbAsyncClient dbAsyncClient = mock(DynamoDbAsyncClient.class); + final DynamoDbClient dynamoDbClient = mock(DynamoDbClient.class); accounts = new Accounts( clock, - mock(DynamoDbClient.class), - dbAsyncClient, + dynamoDbClient, + mock(DynamoDbAsyncClient.class), Tables.ACCOUNTS.tableName(), Tables.NUMBERS.tableName(), Tables.PNI_ASSIGNMENTS.tableName(), @@ -1133,13 +1132,13 @@ class AccountsTest { reason -> CancellationReason.builder().code(reason).build() ).orElse(CancellationReason.builder().build()); - when(dbAsyncClient.transactWriteItems(any(TransactWriteItemsRequest.class))) - .thenReturn(CompletableFuture.failedFuture(TransactionCanceledException.builder() + when(dynamoDbClient.transactWriteItems(any(TransactWriteItemsRequest.class))) + .thenThrow(TransactionCanceledException.builder() .cancellationReasons(constraintCancellationReason, accountsCancellationReason) - .build())); + .build()); - CompletableFutureTestUtil.assertFailsWithCause(expectedException, - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1))); + assertThrows(expectedException, + () -> accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1))); } private static Stream testReserveUsernameHashTransactionConflict() { @@ -1156,12 +1155,12 @@ class AccountsTest { void testConfirmUsernameHashTransactionConflict(final Optional constraintCancellationString, final Optional accountsCancellationString, final Class expectedException) { - final DynamoDbAsyncClient dbAsyncClient = mock(DynamoDbAsyncClient.class); + final DynamoDbClient dynamoDbClient = mock(DynamoDbClient.class); accounts = new Accounts( clock, - mock(DynamoDbClient.class), - dbAsyncClient, + dynamoDbClient, + mock(DynamoDbAsyncClient.class), Tables.ACCOUNTS.tableName(), Tables.NUMBERS.tableName(), Tables.PNI_ASSIGNMENTS.tableName(), @@ -1179,15 +1178,15 @@ class AccountsTest { reason -> CancellationReason.builder().code(reason).build() ).orElse(CancellationReason.builder().build()); - when(dbAsyncClient.transactWriteItems(any(TransactWriteItemsRequest.class))) - .thenReturn(CompletableFuture.failedFuture(TransactionCanceledException.builder() + when(dynamoDbClient.transactWriteItems(any(TransactWriteItemsRequest.class))) + .thenThrow(TransactionCanceledException.builder() .cancellationReasons(constraintCancellationReason, accountsCancellationReason, CancellationReason.builder().build()) - .build())); + .build()); - CompletableFutureTestUtil.assertFailsWithCause(expectedException, - accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); + assertThrows(expectedException, + () -> accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); } private static Stream testConfirmUsernameHashTransactionConflict() { @@ -1199,28 +1198,28 @@ class AccountsTest { } @Test - void testConfirmUsernameHashVersionMismatch() { + void testConfirmUsernameHashVersionMismatch() throws UsernameHashNotAvailableException { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); createAccount(account); - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)).join(); + accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); account.setVersion(account.getVersion() + 77); - CompletableFutureTestUtil.assertFailsWithCause(ContestedOptimisticLockException.class, - accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); assertThat(account.getUsernameHash()).isEmpty(); } @Test - void testClearUsername() { + void testClearUsername() throws UsernameHashNotAvailableException { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); createAccount(account); - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)).join(); - accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); + accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); assertThat(accounts.getByUsernameHash(USERNAME_HASH_1).join()).isPresent(); - accounts.clearUsernameHash(account).join(); + accounts.clearUsernameHash(account); assertThat(accounts.getByUsernameHash(USERNAME_HASH_1).join()).isEmpty(); assertThat(accounts.getByAccountIdentifier(account.getUuid())) @@ -1236,21 +1235,21 @@ class AccountsTest { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); createAccount(account); - assertThatNoException().isThrownBy(() -> accounts.clearUsernameHash(account).join()); + assertThatNoException().isThrownBy(() -> accounts.clearUsernameHash(account)); } @Test - void testClearUsernameVersionMismatch() { + void testClearUsernameVersionMismatch() throws UsernameHashNotAvailableException { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); createAccount(account); - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)).join(); - accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); + accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); account.setVersion(account.getVersion() + 12); - CompletableFutureTestUtil.assertFailsWithCause(ContestedOptimisticLockException.class, - accounts.clearUsernameHash(account)); + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.clearUsernameHash(account)); assertArrayEquals(USERNAME_HASH_1, account.getUsernameHash().orElseThrow()); } @@ -1259,13 +1258,13 @@ class AccountsTest { @ParameterizedTest @MethodSource void testClearUsernameTransactionConflict(final Optional constraintCancellationString, - final Optional accountsCancellationString) { - final DynamoDbAsyncClient dbAsyncClient = mock(DynamoDbAsyncClient.class); + final Optional accountsCancellationString) throws UsernameHashNotAvailableException { + final DynamoDbClient dynamoDbClient = mock(DynamoDbClient.class); accounts = new Accounts( clock, - mock(DynamoDbClient.class), - dbAsyncClient, + dynamoDbClient, + mock(DynamoDbAsyncClient.class), Tables.ACCOUNTS.tableName(), Tables.NUMBERS.tableName(), Tables.PNI_ASSIGNMENTS.tableName(), @@ -1276,27 +1275,27 @@ class AccountsTest { final Account account = generateAccount("+14155551111", UUID.randomUUID(), UUID.randomUUID()); createAccount(account); - when(dbAsyncClient.transactWriteItems(any(TransactWriteItemsRequest.class))) - .thenReturn(CompletableFuture.completedFuture(mock(TransactWriteItemsResponse.class))); + when(dynamoDbClient.transactWriteItems(any(TransactWriteItemsRequest.class))) + .thenReturn(mock(TransactWriteItemsResponse.class)); - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)).join(); - accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); + accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); - final CancellationReason constraintCancellationReason = constraintCancellationString.map( - reason -> CancellationReason.builder().code(reason).build() - ).orElse(CancellationReason.builder().build()); + final CancellationReason constraintCancellationReason = constraintCancellationString + .map(reason -> CancellationReason.builder().code(reason).build()) + .orElse(CancellationReason.builder().build()); - final CancellationReason accountsCancellationReason = accountsCancellationString.map( - reason -> CancellationReason.builder().code(reason).build() - ).orElse(CancellationReason.builder().build()); + final CancellationReason accountsCancellationReason = accountsCancellationString + .map(reason -> CancellationReason.builder().code(reason).build()) + .orElse(CancellationReason.builder().build()); - when(dbAsyncClient.transactWriteItems(any(TransactWriteItemsRequest.class))) - .thenReturn(CompletableFuture.failedFuture(TransactionCanceledException.builder() + when(dynamoDbClient.transactWriteItems(any(TransactWriteItemsRequest.class))) + .thenThrow(TransactionCanceledException.builder() .cancellationReasons(accountsCancellationReason, constraintCancellationReason) - .build())); + .build()); - CompletableFutureTestUtil.assertFailsWithCause(ContestedOptimisticLockException.class, - accounts.clearUsernameHash(account)); + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.clearUsernameHash(account)); assertArrayEquals(USERNAME_HASH_1, account.getUsernameHash().orElseThrow()); } @@ -1309,24 +1308,24 @@ class AccountsTest { } @Test - void testReservedUsernameHash() { + void testReservedUsernameHash() throws UsernameHashNotAvailableException { final Account account1 = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID()); createAccount(account1); final Account account2 = generateAccount("+18005552222", UUID.randomUUID(), UUID.randomUUID()); createAccount(account2); - accounts.reserveUsernameHash(account1, USERNAME_HASH_1, Duration.ofDays(1)).join(); + accounts.reserveUsernameHash(account1, USERNAME_HASH_1, Duration.ofDays(1)); assertArrayEquals(USERNAME_HASH_1, account1.getReservedUsernameHash().orElseThrow()); assertThat(account1.getUsernameHash()).isEmpty(); // account 2 shouldn't be able to reserve or confirm the same username hash - CompletableFutureTestUtil.assertFailsWithCause(UsernameHashNotAvailableException.class, - accounts.reserveUsernameHash(account2, USERNAME_HASH_1, Duration.ofDays(1))); - CompletableFutureTestUtil.assertFailsWithCause(UsernameHashNotAvailableException.class, - accounts.confirmUsernameHash(account2, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); + assertThrows(UsernameHashNotAvailableException.class, + () -> accounts.reserveUsernameHash(account2, USERNAME_HASH_1, Duration.ofDays(1))); + assertThrows(UsernameHashNotAvailableException.class, + () -> accounts.confirmUsernameHash(account2, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); assertThat(accounts.getByUsernameHash(USERNAME_HASH_1).join()).isEmpty(); - accounts.confirmUsernameHash(account1, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accounts.confirmUsernameHash(account1, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); assertThat(account1.getReservedUsernameHash()).isEmpty(); assertArrayEquals(USERNAME_HASH_1, account1.getUsernameHash().orElseThrow()); assertThat(accounts.getByUsernameHash(USERNAME_HASH_1).join().orElseThrow().getUuid()).isEqualTo(account1.getUuid()); @@ -1338,15 +1337,15 @@ class AccountsTest { } @Test - void switchBetweenReservedUsernameHashes() { + void switchBetweenReservedUsernameHashes() throws UsernameHashNotAvailableException { final Account account = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID()); createAccount(account); - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)).join(); + accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); assertArrayEquals(USERNAME_HASH_1, account.getReservedUsernameHash().orElseThrow()); assertThat(account.getUsernameHash()).isEmpty(); - accounts.reserveUsernameHash(account, USERNAME_HASH_2, Duration.ofDays(1)).join(); + accounts.reserveUsernameHash(account, USERNAME_HASH_2, Duration.ofDays(1)); assertArrayEquals(USERNAME_HASH_2, account.getReservedUsernameHash().orElseThrow()); assertThat(account.getUsernameHash()).isEmpty(); @@ -1359,7 +1358,7 @@ class AccountsTest { clock.pin(Instant.EPOCH.plus(Duration.ofMinutes(1))); - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)).join(); + accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); assertArrayEquals(USERNAME_HASH_1, account.getReservedUsernameHash().orElseThrow()); assertThat(account.getUsernameHash()).isEmpty(); @@ -1371,23 +1370,23 @@ class AccountsTest { } @Test - void reserveOwnConfirmedUsername() { + void reserveOwnConfirmedUsername() throws UsernameHashNotAvailableException { final Account account = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID()); createAccount(account); - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)).join(); + accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); assertArrayEquals(USERNAME_HASH_1, account.getReservedUsernameHash().orElseThrow()); assertThat(account.getUsernameHash()).isEmpty(); assertThat(getUsernameConstraintTableItem(USERNAME_HASH_1)).containsKey(Accounts.UsernameTable.ATTR_TTL); - accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); assertThat(account.getReservedUsernameHash()).isEmpty(); assertArrayEquals(USERNAME_HASH_1, account.getUsernameHash().orElseThrow()); assertThat(getUsernameConstraintTableItem(USERNAME_HASH_1)).doesNotContainKey(Accounts.UsernameTable.ATTR_TTL); - CompletableFutureTestUtil.assertFailsWithCause(UsernameHashNotAvailableException.class, - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1))); + assertThrows(UsernameHashNotAvailableException.class, + () -> accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1))); assertThat(account.getReservedUsernameHash()).isEmpty(); assertArrayEquals(USERNAME_HASH_1, account.getUsernameHash().orElseThrow()); assertThat(getUsernameConstraintTableItem(USERNAME_HASH_1)).containsKey(Accounts.UsernameTable.KEY_USERNAME_HASH); @@ -1395,47 +1394,47 @@ class AccountsTest { } @Test - void testConfirmReservedUsernameHashWrongAccountUuid() { + void testConfirmReservedUsernameHashWrongAccountUuid() throws UsernameHashNotAvailableException { final Account account1 = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID()); createAccount(account1); final Account account2 = generateAccount("+18005552222", UUID.randomUUID(), UUID.randomUUID()); createAccount(account2); - accounts.reserveUsernameHash(account1, USERNAME_HASH_1, Duration.ofDays(1)).join(); + accounts.reserveUsernameHash(account1, USERNAME_HASH_1, Duration.ofDays(1)); assertArrayEquals(USERNAME_HASH_1, account1.getReservedUsernameHash().orElseThrow()); assertThat(account1.getUsernameHash()).isEmpty(); // only account1 should be able to confirm the reserved hash - CompletableFutureTestUtil.assertFailsWithCause(UsernameHashNotAvailableException.class, - accounts.confirmUsernameHash(account2, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); + assertThrows(UsernameHashNotAvailableException.class, + () -> accounts.confirmUsernameHash(account2, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); } @Test - void testConfirmExpiredReservedUsernameHash() { + void testConfirmExpiredReservedUsernameHash() throws UsernameHashNotAvailableException { final Account account1 = generateAccount("+18005551111", UUID.randomUUID(), UUID.randomUUID()); createAccount(account1); final Account account2 = generateAccount("+18005552222", UUID.randomUUID(), UUID.randomUUID()); createAccount(account2); - accounts.reserveUsernameHash(account1, USERNAME_HASH_1, Duration.ofDays(2)).join(); + accounts.reserveUsernameHash(account1, USERNAME_HASH_1, Duration.ofDays(2)); for (int i = 0; i <= 2; i++) { clock.pin(Instant.EPOCH.plus(Duration.ofDays(i))); - CompletableFutureTestUtil.assertFailsWithCause(UsernameHashNotAvailableException.class, - accounts.reserveUsernameHash(account2, USERNAME_HASH_1, Duration.ofDays(1))); + assertThrows(UsernameHashNotAvailableException.class, + () -> accounts.reserveUsernameHash(account2, USERNAME_HASH_1, Duration.ofDays(1))); } // after 2 days, can reserve and confirm the hash clock.pin(Instant.EPOCH.plus(Duration.ofDays(2)).plus(Duration.ofSeconds(1))); - accounts.reserveUsernameHash(account2, USERNAME_HASH_1, Duration.ofDays(1)).join(); + accounts.reserveUsernameHash(account2, USERNAME_HASH_1, Duration.ofDays(1)); assertEquals(USERNAME_HASH_1, account2.getReservedUsernameHash().orElseThrow()); - accounts.confirmUsernameHash(account2, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accounts.confirmUsernameHash(account2, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); - CompletableFutureTestUtil.assertFailsWithCause(UsernameHashNotAvailableException.class, - accounts.reserveUsernameHash(account1, USERNAME_HASH_1, Duration.ofDays(2))); - CompletableFutureTestUtil.assertFailsWithCause(UsernameHashNotAvailableException.class, - accounts.confirmUsernameHash(account1, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); + assertThrows(UsernameHashNotAvailableException.class, + () -> accounts.reserveUsernameHash(account1, USERNAME_HASH_1, Duration.ofDays(2))); + assertThrows(UsernameHashNotAvailableException.class, + () -> accounts.confirmUsernameHash(account1, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); assertThat(accounts.getByUsernameHash(USERNAME_HASH_1).join().orElseThrow().getUuid()).isEqualTo(account2.getUuid()); } @@ -1444,30 +1443,30 @@ class AccountsTest { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); createAccount(account); account.setVersion(account.getVersion() + 12); - CompletableFutureTestUtil.assertFailsWithCause(ContestedOptimisticLockException.class, - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1))); - CompletableFutureTestUtil.assertFailsWithCause(ContestedOptimisticLockException.class, - accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1))); + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1)); assertThat(account.getReservedUsernameHash()).isEmpty(); assertThat(account.getUsernameHash()).isEmpty(); } @ParameterizedTest @ValueSource(booleans = {false, true}) - void testRemoveOldestHold(boolean clearUsername) { + void testRemoveOldestHold(boolean clearUsername) throws UsernameHashNotAvailableException { Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); createAccount(account); - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)).join(); - accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); + accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); - final List usernames = IntStream.range(0, 7).mapToObj(i -> TestRandomUtil.nextBytes(32)).toList(); + final List usernames = IntStream.range(0, 7).mapToObj(_ -> TestRandomUtil.nextBytes(32)).toList(); final ArrayDeque expectedHolds = new ArrayDeque<>(); expectedHolds.add(USERNAME_HASH_1); for (byte[] username : usernames) { - accounts.reserveUsernameHash(account, username, Duration.ofDays(1)).join(); - accounts.confirmUsernameHash(account, username, ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(account, username, Duration.ofDays(1)); + accounts.confirmUsernameHash(account, username, ENCRYPTED_USERNAME_1); assertThat(accounts.getByUsernameHash(username).join()).isPresent(); final Account read = accounts.getByAccountIdentifier(account.getUuid()).orElseThrow(); @@ -1482,7 +1481,7 @@ class AccountsTest { // clearing the username adds a hold, but the subsequent confirm in the next iteration should add the same hold // (should be a noop) so we don't need to touch expectedHolds if (clearUsername) { - accounts.clearUsernameHash(account).join(); + accounts.clearUsernameHash(account); } } @@ -1496,72 +1495,69 @@ class AccountsTest { final List freeUsernames = usernames.subList(0, numFree); final List heldUsernames = usernames.subList(numFree, usernames.size()); for (byte[] username : freeUsernames) { - assertDoesNotThrow(() -> - accounts.reserveUsernameHash(account2, username, Duration.ofDays(2)).join()); + assertDoesNotThrow(() -> accounts.reserveUsernameHash(account2, username, Duration.ofDays(2))); } for (byte[] username : heldUsernames) { - CompletableFutureTestUtil.assertFailsWithCause(UsernameHashNotAvailableException.class, - accounts.reserveUsernameHash(account2, username, Duration.ofDays(2))); + assertThrows(UsernameHashNotAvailableException.class, + () -> accounts.reserveUsernameHash(account2, username, Duration.ofDays(2))); } } @Test - void testHoldUsername() { + void testHoldUsername() throws UsernameHashNotAvailableException { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); createAccount(account); - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)).join(); - accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); + accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); - accounts.clearUsernameHash(account).join(); + accounts.clearUsernameHash(account); Account account2 = generateAccount("+18005554321", UUID.randomUUID(), UUID.randomUUID()); createAccount(account2); - CompletableFutureTestUtil.assertFailsWithCause( - UsernameHashNotAvailableException.class, - accounts.reserveUsernameHash(account2, USERNAME_HASH_1, Duration.ofDays(1)), + assertThrows(UsernameHashNotAvailableException.class, + () -> accounts.reserveUsernameHash(account2, USERNAME_HASH_1, Duration.ofDays(1)), "account2 should not be able reserve username held by account"); // but we should be able to get it back - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)).join(); - accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); + accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); } @Test - void testNoHoldsBarred() { + void testNoHoldsBarred() throws UsernameHashNotAvailableException { // should be able to reserve all MAX_HOLDS usernames final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); createAccount(account); final List usernames = IntStream.range(0, Accounts.MAX_USERNAME_HOLDS + 1) - .mapToObj(i -> TestRandomUtil.nextBytes(32)) + .mapToObj(_ -> TestRandomUtil.nextBytes(32)) .toList(); for (byte[] username : usernames) { - accounts.reserveUsernameHash(account, username, Duration.ofDays(1)).join(); - accounts.confirmUsernameHash(account, username, ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(account, username, Duration.ofDays(1)); + accounts.confirmUsernameHash(account, username, ENCRYPTED_USERNAME_1); } // someone else shouldn't be able to get any of our holds Account account2 = generateAccount("+18005554321", UUID.randomUUID(), UUID.randomUUID()); createAccount(account2); for (byte[] username : usernames) { - CompletableFutureTestUtil.assertFailsWithCause( - UsernameHashNotAvailableException.class, - accounts.reserveUsernameHash(account2, username, Duration.ofDays(1)), + assertThrows(UsernameHashNotAvailableException.class, + () -> accounts.reserveUsernameHash(account2, username, Duration.ofDays(1)), "account2 should not be able reserve username held by account"); } // once the hold expires it's fine though clock.pin(Instant.EPOCH.plus(Accounts.USERNAME_HOLD_DURATION).plus(Duration.ofSeconds(1))); - accounts.reserveUsernameHash(account2, usernames.getFirst(), Duration.ofDays(1)).join(); + accounts.reserveUsernameHash(account2, usernames.getFirst(), Duration.ofDays(1)); // if account1 modifies their username, we should also clear out the old holds, leaving only their newly added hold - accounts.clearUsernameHash(account).join(); + accounts.clearUsernameHash(account); assertThat(account.getUsernameHolds().stream().map(Account.UsernameHold::usernameHash)) .containsExactly(usernames.getLast()); } @Test - public void testCannotRemoveHold() { + public void testCannotRemoveHold() throws UsernameHashNotAvailableException { // Tests the case where we are trying to remove a hold we think we have, but it turns out we've already lost it. // This means that the Account record an account has a hold on a particular username, but that hold is held by // someone else in the username table. This can happen when the hold TTL expires while we are performing the update @@ -1569,44 +1565,44 @@ class AccountsTest { // case, a simple retry should let us check the clock again and notice that our hold in our account has expired. final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); createAccount(account); - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)).join(); - accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); + accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); - accounts.reserveUsernameHash(account, USERNAME_HASH_2, Duration.ofDays(1)).join(); - accounts.confirmUsernameHash(account, USERNAME_HASH_2, ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(account, USERNAME_HASH_2, Duration.ofDays(1)); + accounts.confirmUsernameHash(account, USERNAME_HASH_2, ENCRYPTED_USERNAME_1); // Now we have a hold on username_hash_1. Simulate a race where the TTL on username_hash_1 expires, and someone // else picks up the username by going forward and then back in time Account account2 = generateAccount("+18005554321", UUID.randomUUID(), UUID.randomUUID()); createAccount(account2); clock.pin(Instant.EPOCH.plus(Accounts.USERNAME_HOLD_DURATION).plus(Duration.ofSeconds(1))); - accounts.reserveUsernameHash(account2, USERNAME_HASH_1, Duration.ofDays(1)).join(); - accounts.confirmUsernameHash(account2, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(account2, USERNAME_HASH_1, Duration.ofDays(1)); + accounts.confirmUsernameHash(account2, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); clock.pin(Instant.EPOCH); // already have 1 hold, should be able to get to MAX_HOLDS without a problem for (int i = 1; i < Accounts.MAX_USERNAME_HOLDS; i++) { - accounts.reserveUsernameHash(account, TestRandomUtil.nextBytes(32), Duration.ofDays(1)).join(); - accounts.confirmUsernameHash(account, TestRandomUtil.nextBytes(32), ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(account, TestRandomUtil.nextBytes(32), Duration.ofDays(1)); + accounts.confirmUsernameHash(account, TestRandomUtil.nextBytes(32), ENCRYPTED_USERNAME_1); } - accounts.reserveUsernameHash(account, TestRandomUtil.nextBytes(32), Duration.ofDays(1)).join(); + accounts.reserveUsernameHash(account, TestRandomUtil.nextBytes(32), Duration.ofDays(1)); // Should fail, because we cannot remove our hold on USERNAME_HASH_1 - CompletableFutureTestUtil.assertFailsWithCause(ContestedOptimisticLockException.class, - accounts.confirmUsernameHash(account, TestRandomUtil.nextBytes(32), ENCRYPTED_USERNAME_1)); + assertThrows(ContestedOptimisticLockException.class, + () -> accounts.confirmUsernameHash(account, TestRandomUtil.nextBytes(32), ENCRYPTED_USERNAME_1)); // Should now pass once we realize our hold's TTL is over clock.pin(Instant.EPOCH.plus(Accounts.USERNAME_HOLD_DURATION).plus(Duration.ofSeconds(1))); - accounts.confirmUsernameHash(account, TestRandomUtil.nextBytes(32), ENCRYPTED_USERNAME_1).join(); + accounts.confirmUsernameHash(account, TestRandomUtil.nextBytes(32), ENCRYPTED_USERNAME_1); } @Test - void testDeduplicateHoldsOnSwappedUsernames() { + void testDeduplicateHoldsOnSwappedUsernames() throws UsernameHashNotAvailableException { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); createAccount(account); - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)).join(); - accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); + accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); final Consumer assertSingleHold = (byte[] usernameToCheck) -> { // our account should have exactly one hold for the username @@ -1623,25 +1619,25 @@ class AccountsTest { // Swap back and forth between username 1 and 2. Username hashes shouldn't reappear in our holds if we already have // a hold for (int i = 0; i < 5; i++) { - accounts.reserveUsernameHash(account, USERNAME_HASH_2, Duration.ofSeconds(1)).join(); - accounts.confirmUsernameHash(account, USERNAME_HASH_2, ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(account, USERNAME_HASH_2, Duration.ofSeconds(1)); + accounts.confirmUsernameHash(account, USERNAME_HASH_2, ENCRYPTED_USERNAME_1); assertSingleHold.accept(USERNAME_HASH_1); - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofSeconds(1)).join(); - accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofSeconds(1)); + accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); assertSingleHold.accept(USERNAME_HASH_2); } } @Test - void testRemoveHoldAfterConfirm() { + void testRemoveHoldAfterConfirm() throws UsernameHashNotAvailableException { final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); createAccount(account); final List usernames = IntStream.range(0, Accounts.MAX_USERNAME_HOLDS) - .mapToObj(i -> TestRandomUtil.nextBytes(32)).toList(); + .mapToObj(_ -> TestRandomUtil.nextBytes(32)).toList(); for (byte[] username : usernames) { - accounts.reserveUsernameHash(account, username, Duration.ofDays(1)).join(); - accounts.confirmUsernameHash(account, username, ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(account, username, Duration.ofDays(1)); + accounts.confirmUsernameHash(account, username, ENCRYPTED_USERNAME_1); } int holdToRereserve = (Accounts.MAX_USERNAME_HOLDS / 2) - 1; @@ -1651,8 +1647,8 @@ class AccountsTest { .containsExactlyElementsOf(usernames.subList(0, usernames.size() - 1)); // if we confirm a username we already have held, it should just drop out of the holds list - accounts.reserveUsernameHash(account, usernames.get(holdToRereserve), Duration.ofDays(1)).join(); - accounts.confirmUsernameHash(account, usernames.get(holdToRereserve), ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(account, usernames.get(holdToRereserve), Duration.ofDays(1)); + accounts.confirmUsernameHash(account, usernames.get(holdToRereserve), ENCRYPTED_USERNAME_1); // should have a hold on every username but the one we just confirmed assertThat(account.getUsernameHolds().stream().map(Account.UsernameHold::usernameHash).toList()) @@ -1682,7 +1678,7 @@ class AccountsTest { } @Test - void testGetByUsernameHashAsync() { + void testGetByUsernameHashAsync() throws UsernameHashNotAvailableException { assertThat(accounts.getByUsernameHash(USERNAME_HASH_1).join()).isEmpty(); final Account account = generateAccount("+18005551234", UUID.randomUUID(), UUID.randomUUID()); @@ -1690,8 +1686,8 @@ class AccountsTest { assertThat(accounts.getByUsernameHash(USERNAME_HASH_1).join()).isEmpty(); - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)).join(); - accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(account, USERNAME_HASH_1, Duration.ofDays(1)); + accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); assertThat(accounts.getByUsernameHash(USERNAME_HASH_1).join()).isPresent(); } @@ -1764,20 +1760,16 @@ class AccountsTest { final Account conflictingUsernameAccount = nextRandomAccount(); createAccount(conflictingUsernameAccount); - final CompletionException completionException = assertThrows(CompletionException.class, - () -> accounts.reserveUsernameHash(conflictingUsernameAccount, USERNAME_HASH_1, Accounts.USERNAME_HOLD_DURATION).join()); - - assertInstanceOf(UsernameHashNotAvailableException.class, completionException.getCause()); + assertThrows(UsernameHashNotAvailableException.class, + () -> accounts.reserveUsernameHash(conflictingUsernameAccount, USERNAME_HASH_1, Accounts.USERNAME_HOLD_DURATION)); } { final Account conflictingUsernameHoldAccount = nextRandomAccount(); createAccount(conflictingUsernameHoldAccount); - final CompletionException completionException = assertThrows(CompletionException.class, - () -> accounts.reserveUsernameHash(conflictingUsernameHoldAccount, USERNAME_HASH_2, Accounts.USERNAME_HOLD_DURATION).join()); - - assertInstanceOf(UsernameHashNotAvailableException.class, completionException.getCause()); + assertThrows(UsernameHashNotAvailableException.class, + () -> accounts.reserveUsernameHash(conflictingUsernameHoldAccount, USERNAME_HASH_2, Accounts.USERNAME_HOLD_DURATION)); } // Check that bare constraint records are written as expected @@ -1795,7 +1787,7 @@ class AccountsTest { } @Test - void testRegeneratedConstraintsMatchOriginalConstraints() { + void testRegeneratedConstraintsMatchOriginalConstraints() throws UsernameHashNotAvailableException { final Instant usernameHoldExpiration = clock.instant().plus(Accounts.USERNAME_HOLD_DURATION).truncatedTo(ChronoUnit.SECONDS); final Account account = nextRandomAccount(); @@ -1804,10 +1796,10 @@ class AccountsTest { account.setUsernameHolds(List.of(new Account.UsernameHold(USERNAME_HASH_2, usernameHoldExpiration.getEpochSecond()))); createAccount(account); - accounts.reserveUsernameHash(account, USERNAME_HASH_2, Accounts.USERNAME_HOLD_DURATION).join(); - accounts.confirmUsernameHash(account, USERNAME_HASH_2, ENCRYPTED_USERNAME_2).join(); - accounts.reserveUsernameHash(account, USERNAME_HASH_1, Accounts.USERNAME_HOLD_DURATION).join(); - accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + accounts.reserveUsernameHash(account, USERNAME_HASH_2, Accounts.USERNAME_HOLD_DURATION); + accounts.confirmUsernameHash(account, USERNAME_HASH_2, ENCRYPTED_USERNAME_2); + accounts.reserveUsernameHash(account, USERNAME_HASH_1, Accounts.USERNAME_HOLD_DURATION); + accounts.confirmUsernameHash(account, USERNAME_HASH_1, ENCRYPTED_USERNAME_1); final Map originalE164ConstraintItem = DYNAMO_DB_EXTENSION.getDynamoDbClient().getItem(GetItemRequest.builder()