diff --git a/integration-tests/src/main/java/org/signal/integration/IntegrationTools.java b/integration-tests/src/main/java/org/signal/integration/IntegrationTools.java index ac043864a..0b61f9b01 100644 --- a/integration-tests/src/main/java/org/signal/integration/IntegrationTools.java +++ b/integration-tests/src/main/java/org/signal/integration/IntegrationTools.java @@ -53,7 +53,7 @@ public class IntegrationTools { new RegistrationRecoveryPasswordsManager(registrationRecoveryPasswords), new VerificationSessionManager(verificationSessions), new PhoneNumberIdentifiers(dynamoDbAsyncClient, config.dynamoDbTables().phoneNumberIdentifiers()), - new ChangeNumberWaitingPeriods(config.dynamoDbTables().changeNumberWaitingPeriods(), dynamoDbAsyncClient, dynamoDbClient) + new ChangeNumberWaitingPeriods(config.dynamoDbTables().changeNumberWaitingPeriods(), dynamoDbClient) ); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index 6d6ec36ea..97db1264c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -708,7 +708,7 @@ public class WhisperServerService extends Application implemen return CompletableFuture.allOf(keysManager.deleteSingleUsePreKeys(aci), keysManager.deleteSingleUsePreKeys(pni), messagesManager.clear(aci), - profilesManager.deleteAll(aci, false), - changeNumberWaitingPeriodManager.handleAccountCreated(aci, clock.instant())); + profilesManager.deleteAll(aci, false)); }) .join(); } @@ -419,6 +417,7 @@ public class AccountsManager extends RedisPubSubAdapter implemen .join()) .orElse(false); + changeNumberWaitingPeriodManager.handleAccountCreated(account.getUuid(), clock.instant()); Tags tags = Tags.of(UserAgentTagUtil.getPlatformTag(userAgent), Tag.of("type", accountCreationType), diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriodManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriodManager.java index e0a3fdf28..40e260523 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriodManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriodManager.java @@ -11,7 +11,6 @@ import java.time.Duration; import java.time.Instant; import java.util.Optional; import java.util.UUID; -import java.util.concurrent.CompletableFuture; /// Manages post-registration change number waiting period expiration data public class ChangeNumberWaitingPeriodManager { @@ -29,8 +28,8 @@ public class ChangeNumberWaitingPeriodManager { /// Must be called when an account is created, including re-registration @VisibleForTesting - public CompletableFuture handleAccountCreated(final UUID aci, final Instant created) { - return changeNumberWaitingPeriods.setExpiration(aci, created.plus(waitingPeriod)); + public void handleAccountCreated(final UUID aci, final Instant created) { + changeNumberWaitingPeriods.setExpiration(aci, created.plus(waitingPeriod)); } /// Returns the waiting period duration remaining, if any. If present, {@code duration} will always be positive. diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriods.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriods.java index 08729289a..77078e834 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriods.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriods.java @@ -10,10 +10,7 @@ import java.time.Instant; import java.util.Map; import java.util.Optional; import java.util.UUID; -import java.util.concurrent.CompletableFuture; import org.whispersystems.textsecuregcm.util.AttributeValues; -import org.whispersystems.textsecuregcm.util.Util; -import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient; import software.amazon.awssdk.services.dynamodb.DynamoDbClient; import software.amazon.awssdk.services.dynamodb.model.AttributeValue; import software.amazon.awssdk.services.dynamodb.model.DeleteItemRequest; @@ -29,23 +26,20 @@ public class ChangeNumberWaitingPeriods { static final String ATTR_TTL = "E"; private final String tableName; - private final DynamoDbAsyncClient dynamoDbAsyncClient; private final DynamoDbClient dynamoDbClient; - public ChangeNumberWaitingPeriods(final String tableName, final DynamoDbAsyncClient dynamoDbAsyncClient, final DynamoDbClient dynamoDbClient) { + public ChangeNumberWaitingPeriods(final String tableName, final DynamoDbClient dynamoDbClient) { this.tableName = tableName; - this.dynamoDbAsyncClient = dynamoDbAsyncClient; this.dynamoDbClient = dynamoDbClient; } - public CompletableFuture setExpiration(final UUID aci, final Instant expiration) { - return dynamoDbAsyncClient.putItem(PutItemRequest.builder() + public void setExpiration(final UUID aci, final Instant expiration) { + dynamoDbClient.putItem(PutItemRequest.builder() .tableName(tableName) .item(Map.of( KEY_ACCOUNT_UUID, AttributeValues.fromUUID(aci), ATTR_TTL, AttributeValues.fromLong(expiration.getEpochSecond()))) - .build()) - .thenRun(Util.NOOP); + .build()); } public Optional getExpiration(final UUID aci) { diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/workers/CommandDependencies.java b/service/src/main/java/org/whispersystems/textsecuregcm/workers/CommandDependencies.java index 6ded4ccee..f4a293935 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/CommandDependencies.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/CommandDependencies.java @@ -284,7 +284,7 @@ public record CommandDependencies( RegistrationRecoveryPasswordsManager registrationRecoveryPasswordsManager = new RegistrationRecoveryPasswordsManager(registrationRecoveryPasswords); final ChangeNumberWaitingPeriods changeNumberWaitingPeriods = new ChangeNumberWaitingPeriods( - configuration.getDynamoDbTables().getChangeNumberWaitingPeriods().getTableName(), dynamoDbAsyncClient, dynamoDbClient); + configuration.getDynamoDbTables().getChangeNumberWaitingPeriods().getTableName(), dynamoDbClient); final ChangeNumberWaitingPeriodManager changeNumberWaitingPeriodManager = new ChangeNumberWaitingPeriodManager( changeNumberWaitingPeriods, configuration.getChangeNumber().postRegistrationWaitingPeriod(), clock); AccountsManager accountsManager = new AccountsManager(accounts, phoneNumberIdentifiers, cacheCluster, diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCreationDeletionIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCreationDeletionIntegrationTest.java index 26f2d9554..af524b50e 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCreationDeletionIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCreationDeletionIntegrationTest.java @@ -140,10 +140,6 @@ public class AccountCreationDeletionIntegrationTest { disconnectionRequestManager = mock(DisconnectionRequestManager.class); when(disconnectionRequestManager.requestDisconnection(any())).thenReturn(CompletableFuture.completedFuture(null)); - final ChangeNumberWaitingPeriodManager changeNumberWaitingPeriodManager = mock(ChangeNumberWaitingPeriodManager.class); - when(changeNumberWaitingPeriodManager.handleAccountCreated(any(UUID.class), any(Instant.class))) - .thenReturn(CompletableFuture.completedFuture(null)); - accountsManager = new AccountsManager( accounts, phoneNumberIdentifiers, @@ -153,7 +149,7 @@ public class AccountCreationDeletionIntegrationTest { keysManager, messagesManager, profilesManager, - changeNumberWaitingPeriodManager, + mock(ChangeNumberWaitingPeriodManager.class), secureStorageClient, svr2Client, disconnectionRequestManager, 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 ae2aab0f6..c1aa3e0f8 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java @@ -49,7 +49,6 @@ import java.util.Base64; import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.HexFormat; import java.util.List; import java.util.Map; import java.util.Objects; @@ -123,6 +122,7 @@ class AccountsManagerTest { private MessagesManager messagesManager; private ProfilesManager profilesManager; private DisconnectionRequestManager disconnectionRequestManager; + private ChangeNumberWaitingPeriodManager changeNumberWaitingPeriodManager; private Map phoneNumberIdentifiersByE164; @@ -147,7 +147,7 @@ class AccountsManagerTest { messagesManager = mock(MessagesManager.class); profilesManager = mock(ProfilesManager.class); disconnectionRequestManager = mock(DisconnectionRequestManager.class); - final ChangeNumberWaitingPeriodManager changeNumberWaitingPeriodManager = mock(ChangeNumberWaitingPeriodManager.class); + changeNumberWaitingPeriodManager = mock(ChangeNumberWaitingPeriodManager.class); //noinspection unchecked asyncCommands = mock(RedisAsyncCommands.class); @@ -215,8 +215,6 @@ class AccountsManagerTest { .build(); when(disconnectionRequestManager.requestDisconnection(any())).thenReturn(CompletableFuture.completedFuture(null)); - when(changeNumberWaitingPeriodManager.handleAccountCreated(any(UUID.class), any(Instant.class))) - .thenReturn(CompletableFuture.completedFuture(null)); accountsManager = new AccountsManager( accounts, @@ -725,6 +723,8 @@ class AccountsManagerTest { notNull(), notNull()); + verify(changeNumberWaitingPeriodManager).handleAccountCreated(eq(createdAccount.getUuid()), any(Instant.class)); + verifyNoInteractions(messagesManager); verifyNoInteractions(profilesManager); } @@ -782,6 +782,7 @@ class AccountsManagerTest { verify(profilesManager, times(2)).deleteAll(existingUuid, false); verify(disconnectionRequestManager).requestDisconnection(argThat(account -> account.getIdentifier(IdentityType.ACI).equals(existingUuid) && account != reregisteredAccount)); + verify(changeNumberWaitingPeriodManager).handleAccountCreated(eq(existingUuid), any(Instant.class)); } @Test 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 8886510ac..b9febb3fc 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java @@ -134,10 +134,6 @@ class AccountsManagerUsernameIntegrationTest { final DisconnectionRequestManager disconnectionRequestManager = mock(DisconnectionRequestManager.class); when(disconnectionRequestManager.requestDisconnection(any())).thenReturn(CompletableFuture.completedFuture(null)); - final ChangeNumberWaitingPeriodManager changeNumberWaitingPeriodManager = mock(ChangeNumberWaitingPeriodManager.class); - when(changeNumberWaitingPeriodManager.handleAccountCreated(any(UUID.class), any(Instant.class))) - .thenReturn(CompletableFuture.completedFuture(null)); - accountsManager = new AccountsManager( accounts, phoneNumberIdentifiers, @@ -147,7 +143,7 @@ class AccountsManagerUsernameIntegrationTest { keysManager, messageManager, profileManager, - changeNumberWaitingPeriodManager, + mock(ChangeNumberWaitingPeriodManager.class), mock(SecureStorageClient.class), mock(SecureValueRecoveryClient.class), disconnectionRequestManager, diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriodManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriodManagerTest.java index 653f02010..74219553b 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriodManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriodManagerTest.java @@ -11,7 +11,6 @@ import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.UUID; -import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -31,31 +30,27 @@ class ChangeNumberWaitingPeriodManagerTest { void setUp() { changeNumberWaitingPeriodManager = new ChangeNumberWaitingPeriodManager( new ChangeNumberWaitingPeriods(Tables.CHANGE_NUMBER_WAITING_PERIODS.tableName(), - DYNAMO_DB_EXTENSION.getDynamoDbAsyncClient(), DYNAMO_DB_EXTENSION.getDynamoDbClient()), WAITING_PERIOD, Clock.systemUTC()); } @Test - void testNewAccount() throws Exception { + void testNewAccount() { final UUID aci = UUID.randomUUID(); assertTrue(changeNumberWaitingPeriodManager.getWaitingPeriodRemaining(aci).isEmpty()); - changeNumberWaitingPeriodManager.handleAccountCreated(aci, Instant.now()) - .get(5, TimeUnit.SECONDS); + changeNumberWaitingPeriodManager.handleAccountCreated(aci, Instant.now()); assertTrue(changeNumberWaitingPeriodManager.getWaitingPeriodRemaining(aci).isPresent()); } @Test - void testOldAccount() throws Exception { + void testOldAccount() { final UUID aci = UUID.randomUUID(); - changeNumberWaitingPeriodManager.handleAccountCreated(aci, - Instant.now().minus(WAITING_PERIOD).minus(Duration.ofHours(1))) - .get(5, TimeUnit.SECONDS); + changeNumberWaitingPeriodManager.handleAccountCreated(aci, Instant.now().minus(WAITING_PERIOD).minus(Duration.ofHours(1))); assertTrue(changeNumberWaitingPeriodManager.getWaitingPeriodRemaining(aci).isEmpty()); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriodsTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriodsTest.java index 8c8e48ca5..c9be7b721 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriodsTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberWaitingPeriodsTest.java @@ -30,22 +30,21 @@ class ChangeNumberWaitingPeriodsTest { void setUp() { changeNumberWaitingPeriods = new ChangeNumberWaitingPeriods( Tables.CHANGE_NUMBER_WAITING_PERIODS.tableName(), - DYNAMO_DB_EXTENSION.getDynamoDbAsyncClient(), DYNAMO_DB_EXTENSION.getDynamoDbClient()); } @Test - void getExpiration_unknownAci() throws Exception { + void getExpiration_unknownAci() { assertTrue(changeNumberWaitingPeriods.getExpiration(UUID.randomUUID()).isEmpty()); } @Test - void setAndGetExpiration() throws Exception { + void setAndGetExpiration() { final UUID aci = UUID.randomUUID(); // truncate to seconds because the TTL attribute stores epoch seconds final Instant expiration = Instant.now().plusSeconds(3600).truncatedTo(ChronoUnit.SECONDS); - changeNumberWaitingPeriods.setExpiration(aci, expiration).get(); + changeNumberWaitingPeriods.setExpiration(aci, expiration); final Optional result = changeNumberWaitingPeriods.getExpiration(aci); assertTrue(result.isPresent()); @@ -55,12 +54,12 @@ class ChangeNumberWaitingPeriodsTest { } @Test - void setExpiration_overwritesExistingEntry() throws Exception { + void setExpiration_overwritesExistingEntry() { final UUID aci = UUID.randomUUID(); final Instant first = Instant.now().plusSeconds(3600).truncatedTo(ChronoUnit.SECONDS); final Instant second = Instant.now().plusSeconds(7200).truncatedTo(ChronoUnit.SECONDS); - changeNumberWaitingPeriods.setExpiration(aci, first).get(); + changeNumberWaitingPeriods.setExpiration(aci, first); { final Optional result = changeNumberWaitingPeriods.getExpiration(aci); @@ -68,7 +67,7 @@ class ChangeNumberWaitingPeriodsTest { assertEquals(first, result.get()); } - changeNumberWaitingPeriods.setExpiration(aci, second).get(); + changeNumberWaitingPeriods.setExpiration(aci, second); { final Optional result = changeNumberWaitingPeriods.getExpiration(aci); @@ -78,9 +77,9 @@ class ChangeNumberWaitingPeriodsTest { } @Test - void delete_removesExisting() throws Exception { + void delete_removesExisting() { final UUID aci = UUID.randomUUID(); - changeNumberWaitingPeriods.setExpiration(aci, Instant.now().plusSeconds(3600)).get(); + changeNumberWaitingPeriods.setExpiration(aci, Instant.now().plusSeconds(3600)); assertTrue( changeNumberWaitingPeriods.getExpiration(aci).isPresent());