From 66b0ed16d1004e295bd194d904c19556f20a51f6 Mon Sep 17 00:00:00 2001 From: Chris Eager Date: Tue, 19 May 2026 17:30:53 -0500 Subject: [PATCH] Convert VerificationSessions to sync DynamoDB client --- .../signal/integration/IntegrationTools.java | 7 +- .../org/signal/integration/Operations.java | 2 +- .../textsecuregcm/WhisperServerService.java | 2 +- .../controllers/VerificationController.java | 21 +-- .../SerializedExpireableJsonDynamoStore.java | 63 +++---- .../storage/VerificationSessionManager.java | 11 +- .../storage/VerificationSessions.java | 4 +- .../VerificationControllerTest.java | 166 ++++++------------ ...rializedExpireableJsonDynamoStoreTest.java | 47 +++-- .../storage/VerificationSessionsTest.java | 21 +-- 10 files changed, 127 insertions(+), 217 deletions(-) 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 0b61f9b01..11b441824 100644 --- a/integration-tests/src/main/java/org/signal/integration/IntegrationTools.java +++ b/integration-tests/src/main/java/org/signal/integration/IntegrationTools.java @@ -47,7 +47,7 @@ public class IntegrationTools { config.dynamoDbTables().registrationRecovery(), Duration.ofDays(1), dynamoDbAsyncClient, Clock.systemUTC()); final VerificationSessions verificationSessions = new VerificationSessions( - dynamoDbAsyncClient, config.dynamoDbTables().verificationSessions(), Clock.systemUTC()); + dynamoDbClient, config.dynamoDbTables().verificationSessions(), Clock.systemUTC()); return new IntegrationTools( new RegistrationRecoveryPasswordsManager(registrationRecoveryPasswords), @@ -75,9 +75,8 @@ public class IntegrationTools { .thenRun(Util.NOOP); } - public CompletableFuture> peekVerificationSessionPushChallenge(final String sessionId) { - return verificationSessionManager.findForId(sessionId) - .thenApply(maybeSession -> maybeSession.map(VerificationSession::pushChallenge)); + public Optional peekVerificationSessionPushChallenge(final String sessionId) { + return verificationSessionManager.findForId(sessionId).map(VerificationSession::pushChallenge); } public void clearChangeNumberWaitingPeriod(TestUser user) { diff --git a/integration-tests/src/main/java/org/signal/integration/Operations.java b/integration-tests/src/main/java/org/signal/integration/Operations.java index d491a878b..04ce4960c 100644 --- a/integration-tests/src/main/java/org/signal/integration/Operations.java +++ b/integration-tests/src/main/java/org/signal/integration/Operations.java @@ -117,7 +117,7 @@ public final class Operations { } public static String peekVerificationSessionPushChallenge(final String sessionId) { - return INTEGRATION_TOOLS.peekVerificationSessionPushChallenge(sessionId).join() + return INTEGRATION_TOOLS.peekVerificationSessionPushChallenge(sessionId) .orElseThrow(() -> new RuntimeException("push challenge not found for the verification session")); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index ac2d23afa..201de29f5 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -519,7 +519,7 @@ public class WhisperServerService extends Application { @@ -34,7 +34,7 @@ public abstract class SerializedExpireableJsonDynamoStore { long getExpirationEpochSeconds(); } - private final DynamoDbAsyncClient dynamoDbClient; + private final DynamoDbClient dynamoDbClient; private final String tableName; private final Clock clock; private final Class deserializationTargetClass; @@ -47,7 +47,7 @@ public abstract class SerializedExpireableJsonDynamoStore { private final Logger log = LoggerFactory.getLogger(getClass()); - public SerializedExpireableJsonDynamoStore(final DynamoDbAsyncClient dynamoDbClient, final String tableName, + public SerializedExpireableJsonDynamoStore(final DynamoDbClient dynamoDbClient, final String tableName, final Clock clock) { this.dynamoDbClient = dynamoDbClient; this.tableName = tableName; @@ -67,18 +67,16 @@ public abstract class SerializedExpireableJsonDynamoStore { } } - public CompletableFuture insert(final String key, final T v) { - return put(key, v, builder -> builder.expressionAttributeNames(Map.of( - "#key", KEY_KEY - )).conditionExpression("attribute_not_exists(#key)")); + public void insert(final String key, final T v) { + put(key, v, builder -> builder.expressionAttributeNames(Map.of("#key", KEY_KEY)) + .conditionExpression("attribute_not_exists(#key)")); } - public CompletableFuture update(final String key, final T v) { - return put(key, v, ignored -> { - }); + public void update(final String key, final T v) { + put(key, v, _ -> {}); } - private CompletableFuture put(final String key, final T v, + private void put(final String key, final T v, final Consumer putRequestCustomizer) { try { final Map attributeValueMap = new HashMap<>(Map.of( @@ -93,9 +91,7 @@ public abstract class SerializedExpireableJsonDynamoStore { .item(attributeValueMap); putRequestCustomizer.accept(builder); - return dynamoDbClient.putItem(builder.build()) - .thenRun(() -> { - }); + dynamoDbClient.putItem(builder.build()); } catch (final JsonProcessingException e) { // This should never happen when writing directly to a string except in cases of serious misconfiguration, which // would be caught by tests. @@ -107,24 +103,23 @@ public abstract class SerializedExpireableJsonDynamoStore { return v.getExpirationEpochSeconds(); } - public CompletableFuture> findForKey(final String key) { - return dynamoDbClient.getItem(GetItemRequest.builder() + public Optional findForKey(final String key) { + final GetItemResponse response = dynamoDbClient.getItem(GetItemRequest.builder() .tableName(tableName) .consistentRead(true) .key(Map.of(KEY_KEY, AttributeValues.fromString(key))) - .build()) - .thenApply(response -> { - try { - return response.hasItem() - ? filterMaybeExpiredValue( - SystemMapper.jsonMapper() - .readValue(response.item().get(ATTR_SERIALIZED_VALUE).s(), deserializationTargetClass)) - : Optional.empty(); - } catch (final JsonProcessingException e) { - log.error("Failed to parse stored value", e); - return Optional.empty(); - } - }); + .build()); + + try { + return response.hasItem() + ? filterMaybeExpiredValue( + SystemMapper.jsonMapper() + .readValue(response.item().get(ATTR_SERIALIZED_VALUE).s(), deserializationTargetClass)) + : Optional.empty(); + } catch (final JsonProcessingException e) { + log.error("Failed to parse stored value", e); + return Optional.empty(); + } } private Optional filterMaybeExpiredValue(T v) { @@ -139,13 +134,11 @@ public abstract class SerializedExpireableJsonDynamoStore { return Optional.of(v); } - public CompletableFuture remove(final String key) { - return dynamoDbClient.deleteItem(DeleteItemRequest.builder() + public void remove(final String key) { + dynamoDbClient.deleteItem(DeleteItemRequest.builder() .tableName(tableName) .key(Map.of(KEY_KEY, AttributeValues.fromString(key))) - .build()) - .thenRun(() -> { - }); + .build()); } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/VerificationSessionManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/VerificationSessionManager.java index 77a6528c7..9065da8e4 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/VerificationSessionManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/VerificationSessionManager.java @@ -6,7 +6,6 @@ package org.whispersystems.textsecuregcm.storage; import java.util.Optional; -import java.util.concurrent.CompletableFuture; import org.whispersystems.textsecuregcm.registration.VerificationSession; public class VerificationSessionManager { @@ -17,15 +16,15 @@ public class VerificationSessionManager { this.verificationSessions = verificationSessions; } - public CompletableFuture insert(final VerificationSession verificationSession) { - return verificationSessions.insert(verificationSession.sessionId(), verificationSession); + public void insert(final VerificationSession verificationSession) { + verificationSessions.insert(verificationSession.sessionId(), verificationSession); } - public CompletableFuture update(final VerificationSession verificationSession) { - return verificationSessions.update(verificationSession.sessionId(), verificationSession); + public void update(final VerificationSession verificationSession) { + verificationSessions.update(verificationSession.sessionId(), verificationSession); } - public CompletableFuture> findForId(final String encodedSessionId) { + public Optional findForId(final String encodedSessionId) { return verificationSessions.findForKey(encodedSessionId); } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/VerificationSessions.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/VerificationSessions.java index 5c543076b..6814b4df4 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/VerificationSessions.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/VerificationSessions.java @@ -7,11 +7,11 @@ package org.whispersystems.textsecuregcm.storage; import java.time.Clock; import org.whispersystems.textsecuregcm.registration.VerificationSession; -import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient; +import software.amazon.awssdk.services.dynamodb.DynamoDbClient; public class VerificationSessions extends SerializedExpireableJsonDynamoStore { - public VerificationSessions(final DynamoDbAsyncClient dynamoDbClient, final String tableName, final Clock clock) { + public VerificationSessions(final DynamoDbClient dynamoDbClient, final String tableName, final Clock clock) { super(dynamoDbClient, tableName, clock); } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/VerificationControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/VerificationControllerTest.java index f785a21fa..d76f9400b 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/VerificationControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/VerificationControllerTest.java @@ -240,8 +240,6 @@ class VerificationControllerTest { CompletableFuture.completedFuture( new RegistrationServiceSession(SESSION_ID, requestedNumber, false, null, null, null, SESSION_EXPIRATION_SECONDS))); - when(verificationSessionManager.insert(any())) - .thenReturn(CompletableFuture.completedFuture(null)); final Invocation.Builder request = resources.getJerseyTest() .target("/v1/verification/session") @@ -281,8 +279,6 @@ class VerificationControllerTest { CompletableFuture.completedFuture( new RegistrationServiceSession(SESSION_ID, NUMBER, false, null, null, null, SESSION_EXPIRATION_SECONDS))); - when(verificationSessionManager.insert(any())) - .thenReturn(CompletableFuture.completedFuture(null)); final Invocation.Builder request = resources.getJerseyTest() .target("/v1/verification/session") @@ -302,8 +298,6 @@ class VerificationControllerTest { CompletableFuture.completedFuture( new RegistrationServiceSession(SESSION_ID, NUMBER, false, null, null, null, SESSION_EXPIRATION_SECONDS))); - when(verificationSessionManager.insert(any())) - .thenReturn(CompletableFuture.completedFuture(null)); final Invocation.Builder request = resources.getJerseyTest() .target("/v1/verification/session") @@ -337,8 +331,6 @@ class VerificationControllerTest { new RegistrationServiceSession(SESSION_ID, NUMBER, false, null, null, null, SESSION_EXPIRATION_SECONDS))); - when(verificationSessionManager.insert(any())) - .thenReturn(CompletableFuture.completedFuture(null)); when(accountsManager.getByE164(NUMBER)) .thenReturn(isReregistration ? Optional.of(mock(Account.class)) : Optional.empty()); @@ -400,13 +392,9 @@ class VerificationControllerTest { Optional.of( registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of( + .thenReturn(Optional.of( new VerificationSession(encodedSessionId, null, null, List.of(VerificationSession.Information.CAPTCHA), Collections.emptyList(), - null, null, false, clock.millis(), clock.millis(), registrationServiceSession.expiration())))); - - when(verificationSessionManager.update(any())) - .thenReturn(CompletableFuture.completedFuture(null)); + null, null, false, clock.millis(), clock.millis(), registrationServiceSession.expiration()))); final Invocation.Builder request = resources.getJerseyTest() .target("/v1/verification/session/" + encodedSessionId) @@ -445,12 +433,8 @@ class VerificationControllerTest { Optional.of( registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, false, - clock.millis(), clock.millis(), registrationServiceSession.expiration())))); - - when(verificationSessionManager.update(any())) - .thenReturn(CompletableFuture.completedFuture(null)); + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, false, + clock.millis(), clock.millis(), registrationServiceSession.expiration()))); doThrow(RateLimitExceededException.class) .when(captchaLimiter).validate(anyString()); @@ -481,12 +465,8 @@ class VerificationControllerTest { Optional.of( registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, false, - clock.millis(), clock.millis(), registrationServiceSession.expiration())))); - - when(verificationSessionManager.update(any())) - .thenReturn(CompletableFuture.completedFuture(null)); + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, false, + clock.millis(), clock.millis(), registrationServiceSession.expiration()))); doThrow(RateLimitExceededException.class) .when(pushChallengeLimiter).validate(anyString()); @@ -517,12 +497,9 @@ class VerificationControllerTest { Optional.of( registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, "challenge", null, List.of(VerificationSession.Information.PUSH_CHALLENGE), + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, "challenge", null, List.of(VerificationSession.Information.PUSH_CHALLENGE), Collections.emptyList(), null, null, false, clock.millis(), clock.millis(), - registrationServiceSession.expiration())))); - when(verificationSessionManager.update(any())) - .thenReturn(CompletableFuture.completedFuture(null)); + registrationServiceSession.expiration()))); final Invocation.Builder request = resources.getJerseyTest() .target("/v1/verification/session/" + encodedSessionId) @@ -551,17 +528,13 @@ class VerificationControllerTest { Optional.of( registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, null, null, List.of(VerificationSession.Information.CAPTCHA), + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, null, null, List.of(VerificationSession.Information.CAPTCHA), Collections.emptyList(), null, null, false, clock.millis(), clock.millis(), - registrationServiceSession.expiration())))); + registrationServiceSession.expiration()))); when(registrationCaptchaManager.assessCaptcha(any(), any(), any(), any())) .thenReturn(Optional.of(AssessmentResult.invalid())); - when(verificationSessionManager.update(any())) - .thenReturn(CompletableFuture.completedFuture(null)); - final Invocation.Builder request = resources.getJerseyTest() .target("/v1/verification/session/" + encodedSessionId) .request() @@ -598,17 +571,14 @@ class VerificationControllerTest { Optional.of( registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, "challenge", null, List.of(VerificationSession.Information.CAPTCHA), List.of(VerificationSession.Information.PUSH_CHALLENGE), null, null, false, - clock.millis(), clock.millis(), registrationServiceSession.expiration())))); - when(verificationSessionManager.update(any())) - .thenReturn(CompletableFuture.completedFuture(null)); - + clock.millis(), clock.millis(), registrationServiceSession.expiration()))); + final Invocation.Builder request = resources.getJerseyTest() .target("/v1/verification/session/" + encodedSessionId) .request() @@ -645,12 +615,9 @@ class VerificationControllerTest { .thenReturn(CompletableFuture.completedFuture( Optional.of(registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, "challenge", null, List.of(), List.of(), null, null, true, - clock.millis(), clock.millis(), registrationServiceSession.expiration())))); + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, "challenge", null, List.of(), List.of(), null, null, true, + clock.millis(), clock.millis(), registrationServiceSession.expiration()))); - when(verificationSessionManager.update(any())) - .thenReturn(CompletableFuture.completedFuture(null)); when(registrationRecoveryPasswordsManager.remove(PNI)) .thenReturn(CompletableFuture.completedFuture(null)); @@ -683,15 +650,12 @@ class VerificationControllerTest { Optional.of( registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, "challenge", null, List.of(VerificationSession.Information.PUSH_CHALLENGE, VerificationSession.Information.CAPTCHA), Collections.emptyList(), null, null, false, clock.millis(), clock.millis(), - registrationServiceSession.expiration())))); - when(verificationSessionManager.update(any())) - .thenReturn(CompletableFuture.completedFuture(null)); + registrationServiceSession.expiration()))); final Invocation.Builder request = resources.getJerseyTest() .target("/v1/verification/session/" + encodedSessionId) @@ -729,16 +693,13 @@ class VerificationControllerTest { Optional.of( registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, null, null, List.of(VerificationSession.Information.CAPTCHA), + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, null, null, List.of(VerificationSession.Information.CAPTCHA), Collections.emptyList(), null, null, false, clock.millis(), clock.millis(), - registrationServiceSession.expiration())))); + registrationServiceSession.expiration()))); when(registrationCaptchaManager.assessCaptcha(any(), any(), any(), any())) .thenReturn(Optional.of(AssessmentResult.alwaysValid())); - when(verificationSessionManager.update(any())) - .thenReturn(CompletableFuture.completedFuture(null)); final Invocation.Builder request = resources.getJerseyTest() .target("/v1/verification/session/" + encodedSessionId) @@ -776,19 +737,16 @@ class VerificationControllerTest { Optional.of( registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, "challenge", null, List.of(VerificationSession.Information.CAPTCHA, VerificationSession.Information.CAPTCHA), Collections.emptyList(), null, null, false, clock.millis(), clock.millis(), - registrationServiceSession.expiration())))); + registrationServiceSession.expiration()))); when(registrationCaptchaManager.assessCaptcha(any(), any(), any(), any())) .thenReturn(Optional.of(AssessmentResult.alwaysValid())); - when(verificationSessionManager.update(any())) - .thenReturn(CompletableFuture.completedFuture(null)); final Invocation.Builder request = resources.getJerseyTest() .target("/v1/verification/session/" + encodedSessionId) @@ -825,19 +783,16 @@ class VerificationControllerTest { when(registrationServiceClient.getSession(any(), any())) .thenReturn(CompletableFuture.completedFuture(Optional.of(registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, null, null, List.of(VerificationSession.Information.CAPTCHA), Collections.emptyList(), null, null, false, clock.millis(), clock.millis(), - registrationServiceSession.expiration())))); + registrationServiceSession.expiration()))); when(registrationCaptchaManager.assessCaptcha(any(), any(), any(), any())) .thenThrow(new IOException("expected service error")); - when(verificationSessionManager.update(any())) - .thenReturn(CompletableFuture.completedFuture(null)); final Invocation.Builder request = resources.getJerseyTest() .target("/v1/verification/session/" + encodedSessionId) @@ -892,7 +847,7 @@ class VerificationControllerTest { when(registrationServiceClient.getSession(any(), any())) .thenReturn(CompletableFuture.completedFuture(Optional.empty())); when(verificationSessionManager.findForId(encodeSessionId(SESSION_ID))) - .thenReturn(CompletableFuture.completedFuture(Optional.empty())); + .thenReturn(Optional.empty()); Invocation.Builder request = resources.getJerseyTest() .target("/v1/verification/session/" + encodeSessionId(SESSION_ID)) @@ -940,7 +895,7 @@ class VerificationControllerTest { new RegistrationServiceSession(SESSION_ID, NUMBER, false, null, null, null, SESSION_EXPIRATION_SECONDS)))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture(Optional.of(mock(VerificationSession.class)))); + .thenReturn(Optional.of(mock(VerificationSession.class))); final Invocation.Builder request = resources.getJerseyTest() .target("/v1/verification/session/" + encodedSessionId) @@ -962,7 +917,7 @@ class VerificationControllerTest { Optional.of( registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture(Optional.of(mock(VerificationSession.class)))); + .thenReturn(Optional.of(mock(VerificationSession.class))); when(registrationRecoveryPasswordsManager.remove(PNI)) .thenReturn(CompletableFuture.completedFuture(null)); @@ -986,9 +941,8 @@ class VerificationControllerTest { when(registrationServiceClient.getSession(any(), any())) .thenReturn(CompletableFuture.completedFuture(Optional.of(registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, - clock.millis(), clock.millis(), registrationServiceSession.expiration())))); + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, + clock.millis(), clock.millis(), registrationServiceSession.expiration()))); when(registrationServiceClient.sendVerificationCode(any(), any(), any(), any(), any(), any())) .thenReturn(CompletableFuture.completedFuture(registrationServiceSession)); when(registrationRecoveryPasswordsManager.remove(PNI)).thenReturn(CompletableFuture.completedFuture(null)); @@ -1018,9 +972,9 @@ class VerificationControllerTest { Optional.of( registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture(Optional.of(new VerificationSession(encodedSessionId, null, null, + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, null, null, List.of(VerificationSession.Information.CAPTCHA), Collections.emptyList(), null, null, false, clock.millis(), clock.millis(), - registrationServiceSession.expiration())))); + registrationServiceSession.expiration()))); final Invocation.Builder request = resources.getJerseyTest() .target("/v1/verification/session/" + encodedSessionId + "/code") @@ -1049,9 +1003,8 @@ class VerificationControllerTest { Optional.of( registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, false, - clock.millis(), clock.millis(), registrationServiceSession.expiration())))); + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, false, + clock.millis(), clock.millis(), registrationServiceSession.expiration()))); final Invocation.Builder request = resources.getJerseyTest() .target("/v1/verification/session/" + encodedSessionId + "/code") @@ -1077,9 +1030,8 @@ class VerificationControllerTest { when(registrationServiceClient.getSession(any(), any())) .thenReturn(CompletableFuture.completedFuture(Optional.of(registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, - clock.millis(), clock.millis(), registrationServiceSession.expiration())))); + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, + clock.millis(), clock.millis(), registrationServiceSession.expiration()))); when(registrationServiceClient.sendVerificationCode(any(), any(), any(), any(), any(), any())) .thenReturn(CompletableFuture.failedFuture( new CompletionException(new VerificationSessionRateLimitExceededException(registrationServiceSession, @@ -1109,9 +1061,8 @@ class VerificationControllerTest { when(registrationServiceClient.getSession(any(), any())) .thenReturn(CompletableFuture.completedFuture(Optional.of(registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, - clock.millis(), clock.millis(), registrationServiceSession.expiration())))); + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, + clock.millis(), clock.millis(), registrationServiceSession.expiration()))); when(registrationServiceClient.sendVerificationCode(any(), any(), any(), any(), any(), any())) .thenReturn(CompletableFuture.failedFuture( new CompletionException(new TransportNotAllowedException(registrationServiceSession)))); @@ -1142,9 +1093,8 @@ class VerificationControllerTest { when(registrationServiceClient.getSession(any(), any())) .thenReturn(CompletableFuture.completedFuture(Optional.of(registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, - clock.millis(), clock.millis(), registrationServiceSession.expiration())))); + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, + clock.millis(), clock.millis(), registrationServiceSession.expiration()))); when(registrationServiceClient.sendVerificationCode(any(), any(), any(), any(), any(), any())) .thenReturn(CompletableFuture.completedFuture(registrationServiceSession)); @@ -1193,9 +1143,8 @@ class VerificationControllerTest { .thenReturn(CompletableFuture.completedFuture( Optional.of(registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, - clock.millis(), clock.millis(), registrationServiceSession.expiration())))); + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, + clock.millis(), clock.millis(), registrationServiceSession.expiration()))); when(registrationServiceClient.sendVerificationCode(any(), any(), any(), any(), any(), any())) .thenReturn( @@ -1237,9 +1186,8 @@ class VerificationControllerTest { when(registrationServiceClient.getSession(any(), any())) .thenReturn(CompletableFuture.completedFuture(Optional.of(registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, - clock.millis(), clock.millis(), registrationServiceSession.expiration())))); + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, + clock.millis(), clock.millis(), registrationServiceSession.expiration()))); when(registrationServiceClient.sendVerificationCode(any(), any(), any(), any(), any(), any())) .thenReturn(CompletableFuture.failedFuture(new CompletionException( @@ -1271,9 +1219,8 @@ class VerificationControllerTest { .thenReturn(CompletableFuture.completedFuture( Optional.of(registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, - clock.millis(), clock.millis(), registrationServiceSession.expiration())))); + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, + clock.millis(), clock.millis(), registrationServiceSession.expiration()))); when(registrationServiceClient.checkVerificationCode(any(), any(), any())) .thenReturn(CompletableFuture.failedFuture(new CompletionException(new RuntimeException()))); @@ -1298,9 +1245,8 @@ class VerificationControllerTest { .thenReturn(CompletableFuture.completedFuture( Optional.of(registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, - clock.millis(), clock.millis(), registrationServiceSession.expiration())))); + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, + clock.millis(), clock.millis(), registrationServiceSession.expiration()))); when(registrationRecoveryPasswordsManager.remove(PNI)) .thenReturn(CompletableFuture.completedFuture(null)); @@ -1335,9 +1281,8 @@ class VerificationControllerTest { .thenReturn(CompletableFuture.completedFuture( Optional.of(registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, - clock.millis(), clock.millis(), registrationServiceSession.expiration())))); + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, + clock.millis(), clock.millis(), registrationServiceSession.expiration()))); // There is no explicit indication in the exception that no code has been sent, but we treat all RegistrationServiceExceptions // in which the response has a session object as conflicted state @@ -1372,9 +1317,8 @@ class VerificationControllerTest { .thenReturn(CompletableFuture.completedFuture( Optional.of(registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, - clock.millis(), clock.millis(), registrationServiceSession.expiration())))); + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, + clock.millis(), clock.millis(), registrationServiceSession.expiration()))); when(registrationServiceClient.checkVerificationCode(any(), any(), any())) .thenReturn(CompletableFuture.failedFuture(new CompletionException(new RegistrationServiceException(null)))); @@ -1398,9 +1342,8 @@ class VerificationControllerTest { .thenReturn(CompletableFuture.completedFuture( Optional.of(registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, - clock.millis(), clock.millis(), registrationServiceSession.expiration())))); + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, + clock.millis(), clock.millis(), registrationServiceSession.expiration()))); when(registrationServiceClient.checkVerificationCode(any(), any(), any())) .thenReturn(CompletableFuture.failedFuture( new CompletionException(new VerificationSessionRateLimitExceededException(registrationServiceSession, @@ -1430,9 +1373,8 @@ class VerificationControllerTest { .thenReturn(CompletableFuture.completedFuture( Optional.of(registrationServiceSession))); when(verificationSessionManager.findForId(any())) - .thenReturn(CompletableFuture.completedFuture( - Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, - clock.millis(), clock.millis(), registrationServiceSession.expiration())))); + .thenReturn(Optional.of(new VerificationSession(encodedSessionId, null, null, Collections.emptyList(), Collections.emptyList(), null, null, true, + clock.millis(), clock.millis(), registrationServiceSession.expiration()))); when(registrationRecoveryPasswordsManager.remove(any())) .thenReturn(CompletableFuture.completedFuture(true)); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/SerializedExpireableJsonDynamoStoreTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/SerializedExpireableJsonDynamoStoreTest.java index 4205e771f..05951a424 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/SerializedExpireableJsonDynamoStoreTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/SerializedExpireableJsonDynamoStoreTest.java @@ -16,12 +16,11 @@ import java.time.Duration; import java.time.Instant; import java.util.List; import java.util.Optional; -import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; -import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient; +import software.amazon.awssdk.services.dynamodb.DynamoDbClient; import software.amazon.awssdk.services.dynamodb.model.AttributeDefinition; import software.amazon.awssdk.services.dynamodb.model.ScalarAttributeType; @@ -54,7 +53,7 @@ class SerializedExpireableJsonDynamoStoreTest { private SerializedExpireableJsonDynamoStore store; - abstract SerializedExpireableJsonDynamoStore getStore(final DynamoDbAsyncClient dynamoDbClient, + abstract SerializedExpireableJsonDynamoStore getStore(final DynamoDbClient dynamoDbClient, final String tableName); abstract T testValue(final String v); @@ -63,29 +62,29 @@ class SerializedExpireableJsonDynamoStoreTest { @BeforeEach void setUp() { - store = getStore(DYNAMO_DB_EXTENSION.getDynamoDbAsyncClient(), TABLE_NAME); + store = getStore(DYNAMO_DB_EXTENSION.getDynamoDbClient(), TABLE_NAME); } @Test - void testStoreAndFind() throws Exception { - assertEquals(Optional.empty(), store.findForKey(KEY).get(1, TimeUnit.SECONDS)); + void testStoreAndFind() { + assertEquals(Optional.empty(), store.findForKey(KEY)); final T original = testValue("1234"); final T second = testValue("5678"); - store.insert(KEY, original).get(1, TimeUnit.SECONDS); + store.insert(KEY, original); { - final Optional maybeValue = store.findForKey(KEY).get(1, TimeUnit.SECONDS); + final Optional maybeValue = store.findForKey(KEY); assertTrue(maybeValue.isPresent()); assertEquals(original, maybeValue.get()); } - assertThrows(Exception.class, () -> store.insert(KEY, second).get(1, TimeUnit.SECONDS)); - - assertDoesNotThrow(() -> store.update(KEY, second).get(1, TimeUnit.SECONDS)); + assertThrows(Exception.class, () -> store.insert(KEY, second)); + + assertDoesNotThrow(() -> store.update(KEY, second)); { - final Optional maybeValue = store.findForKey(KEY).get(1, TimeUnit.SECONDS); + final Optional maybeValue = store.findForKey(KEY); assertTrue(maybeValue.isPresent()); assertEquals(second, maybeValue.get()); @@ -93,20 +92,20 @@ class SerializedExpireableJsonDynamoStoreTest { } @Test - void testRemove() throws Exception { - assertEquals(Optional.empty(), store.findForKey(KEY).get(1, TimeUnit.SECONDS)); + void testRemove() { + assertEquals(Optional.empty(), store.findForKey(KEY)); - store.insert(KEY, testValue("1234")).get(1, TimeUnit.SECONDS); - assertTrue(store.findForKey(KEY).get(1, TimeUnit.SECONDS).isPresent()); + store.insert(KEY, testValue("1234")); + assertTrue(store.findForKey(KEY).isPresent()); - store.remove(KEY).get(1, TimeUnit.SECONDS); - assertFalse(store.findForKey(KEY).get(1, TimeUnit.SECONDS).isPresent()); + store.remove(KEY); + assertFalse(store.findForKey(KEY).isPresent()); final T v = maybeExpiredTestValue("1234"); - store.insert(KEY, v).get(1, TimeUnit.SECONDS); + store.insert(KEY, v); assertEquals(v instanceof SerializedExpireableJsonDynamoStore.Expireable, - store.findForKey(KEY).get(1, TimeUnit.SECONDS).isEmpty()); + store.findForKey(KEY).isEmpty()); } } @@ -126,7 +125,7 @@ class SerializedExpireableJsonDynamoStoreTest { class ExpiresStore extends SerializedExpireableJsonDynamoStore { - public ExpiresStore(final DynamoDbAsyncClient dynamoDbClient, final String tableName) { + public ExpiresStore(final DynamoDbClient dynamoDbClient, final String tableName) { super(dynamoDbClient, tableName, clock); } } @@ -136,7 +135,7 @@ class SerializedExpireableJsonDynamoStoreTest { Duration.ofHours(1)).toEpochMilli(); @Override - SerializedExpireableJsonDynamoStore getStore(final DynamoDbAsyncClient dynamoDbClient, + SerializedExpireableJsonDynamoStore getStore(final DynamoDbClient dynamoDbClient, final String tableName) { return new ExpiresStore(dynamoDbClient, tableName); } @@ -162,13 +161,13 @@ class SerializedExpireableJsonDynamoStoreTest { class DoesNotExpireStore extends SerializedExpireableJsonDynamoStore { - public DoesNotExpireStore(final DynamoDbAsyncClient dynamoDbClient, final String tableName) { + public DoesNotExpireStore(final DynamoDbClient dynamoDbClient, final String tableName) { super(dynamoDbClient, tableName, clock); } } @Override - SerializedExpireableJsonDynamoStore getStore(final DynamoDbAsyncClient dynamoDbClient, + SerializedExpireableJsonDynamoStore getStore(final DynamoDbClient dynamoDbClient, final String tableName) { return new DoesNotExpireStore(dynamoDbClient, tableName); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/VerificationSessionsTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/VerificationSessionsTest.java index a56ec89b5..f992419a3 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/VerificationSessionsTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/VerificationSessionsTest.java @@ -6,7 +6,6 @@ package org.whispersystems.textsecuregcm.storage; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -17,14 +16,12 @@ import java.time.Instant; import java.util.Collections; import java.util.List; import java.util.Optional; -import java.util.concurrent.CompletionException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.whispersystems.textsecuregcm.registration.VerificationSession; import org.whispersystems.textsecuregcm.storage.DynamoDbExtensionSchema.Tables; import org.whispersystems.textsecuregcm.telephony.CarrierData; -import org.whispersystems.textsecuregcm.util.ExceptionUtils; import software.amazon.awssdk.services.dynamodb.model.ConditionalCheckFailedException; class VerificationSessionsTest { @@ -39,7 +36,7 @@ class VerificationSessionsTest { @BeforeEach void setUp() { verificationSessions = new VerificationSessions( - DYNAMO_DB_EXTENSION.getDynamoDbAsyncClient(), Tables.VERIFICATION_SESSIONS.tableName(), clock); + DYNAMO_DB_EXTENSION.getDynamoDbClient(), Tables.VERIFICATION_SESSIONS.tableName(), clock); } @Test @@ -62,30 +59,26 @@ class VerificationSessionsTest { final String sessionId = "sessionId"; - final Optional absentSession = verificationSessions.findForKey(sessionId).join(); + final Optional absentSession = verificationSessions.findForKey(sessionId); assertTrue(absentSession.isEmpty()); final VerificationSession session = new VerificationSession(sessionId, null, new CarrierData("Test", CarrierData.LineType.MOBILE, Optional.of("123"), Optional.empty(), Optional.empty(), Optional.empty()), List.of(VerificationSession.Information.PUSH_CHALLENGE), Collections.emptyList(), null, null, true, clock.millis(), clock.millis(), Duration.ofMinutes(1).toSeconds()); - verificationSessions.insert(sessionId, session).join(); + verificationSessions.insert(sessionId, session); - assertEquals(session, verificationSessions.findForKey(sessionId).join().orElseThrow()); + assertEquals(session, verificationSessions.findForKey(sessionId).orElseThrow()); - final CompletionException ce = assertThrows(CompletionException.class, - () -> verificationSessions.insert(sessionId, session).join()); - - final Throwable t = ExceptionUtils.unwrap(ce); - assertInstanceOf(ConditionalCheckFailedException.class, t, + assertThrows(ConditionalCheckFailedException.class, () -> verificationSessions.insert(sessionId, session), "inserting with the same key should fail conditional checks"); final VerificationSession updatedSession = new VerificationSession(sessionId, null, new CarrierData("Test", CarrierData.LineType.MOBILE, Optional.of("123"), Optional.empty(), Optional.empty(), Optional.empty()), Collections.emptyList(), List.of(VerificationSession.Information.PUSH_CHALLENGE), null, null, true, clock.millis(), clock.millis(), Duration.ofMinutes(2).toSeconds()); - verificationSessions.update(sessionId, updatedSession).join(); + verificationSessions.update(sessionId, updatedSession); - assertEquals(updatedSession, verificationSessions.findForKey(sessionId).join().orElseThrow()); + assertEquals(updatedSession, verificationSessions.findForKey(sessionId).orElseThrow()); }); }