From 20685b6d695559f981a1836af5acf2a95c87e749 Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Fri, 6 Dec 2024 14:59:02 -0500 Subject: [PATCH] Clear "canceled at" timestamp when setting a new subscrition ID --- .../storage/SubscriptionManager.java | 2 +- .../textsecuregcm/storage/Subscriptions.java | 23 +++-- .../storage/SubscriptionsTest.java | 94 ++++++++++++++++++- 3 files changed, 105 insertions(+), 14 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/SubscriptionManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/SubscriptionManager.java index dcc88b98c..d76eaad08 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/SubscriptionManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/SubscriptionManager.java @@ -81,7 +81,7 @@ public class SubscriptionManager { .orElseGet(() -> CompletableFuture.completedFuture(null)); }) .thenCompose(unused -> - subscriptions.canceledAt(subscriberCredentials.subscriberUser(), subscriberCredentials.now())); + subscriptions.setCanceledAt(subscriberCredentials.subscriberUser(), subscriberCredentials.now())); } /** diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Subscriptions.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Subscriptions.java index 2f1d49480..e5a851b5a 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/Subscriptions.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/Subscriptions.java @@ -195,7 +195,7 @@ public class Subscriptions { throw new IllegalStateException( "expected invariant of 1-1 subscriber-customer violated for customer " + processorCustomer); } else { - Map result = queryResponse.items().get(0); + Map result = queryResponse.items().getFirst(); return result.get(KEY_USER).b().asByteArray(); } }); @@ -370,15 +370,16 @@ public class Subscriptions { + "#subscription_id = :subscription_id, " + "#subscription_level = :subscription_level, " + "#subscription_created_at = if_not_exists(#subscription_created_at, :subscription_created_at), " - + "#subscription_level_changed_at = :subscription_level_changed_at" - ) + + "#subscription_level_changed_at = :subscription_level_changed_at " + + "REMOVE #canceled_at") .expressionAttributeNames(Map.of( "#processor_customer_id", KEY_PROCESSOR_ID_CUSTOMER_ID, "#accessed_at", KEY_ACCESSED_AT, "#subscription_id", KEY_SUBSCRIPTION_ID, "#subscription_level", KEY_SUBSCRIPTION_LEVEL, "#subscription_created_at", KEY_SUBSCRIPTION_CREATED_AT, - "#subscription_level_changed_at", KEY_SUBSCRIPTION_LEVEL_CHANGED_AT)) + "#subscription_level_changed_at", KEY_SUBSCRIPTION_LEVEL_CHANGED_AT, + "#canceled_at", KEY_CANCELED_AT)) .expressionAttributeValues(Map.of( ":accessed_at", n(updatedAt.getEpochSecond()), ":processor_customer_id", b(processorCustomer.toDynamoBytes()), @@ -414,7 +415,7 @@ public class Subscriptions { return client.updateItem(request).thenApply(updateItemResponse -> null); } - public CompletableFuture canceledAt(byte[] user, Instant canceledAt) { + public CompletableFuture setCanceledAt(byte[] user, Instant canceledAt) { checkUserLength(user); UpdateItemRequest request = UpdateItemRequest.builder() @@ -449,13 +450,15 @@ public class Subscriptions { + "#subscription_id = :subscription_id, " + "#subscription_created_at = :subscription_created_at, " + "#subscription_level = :subscription_level, " - + "#subscription_level_changed_at = :subscription_level_changed_at") + + "#subscription_level_changed_at = :subscription_level_changed_at " + + "REMOVE #canceled_at") .expressionAttributeNames(Map.of( "#accessed_at", KEY_ACCESSED_AT, "#subscription_id", KEY_SUBSCRIPTION_ID, "#subscription_created_at", KEY_SUBSCRIPTION_CREATED_AT, "#subscription_level", KEY_SUBSCRIPTION_LEVEL, - "#subscription_level_changed_at", KEY_SUBSCRIPTION_LEVEL_CHANGED_AT)) + "#subscription_level_changed_at", KEY_SUBSCRIPTION_LEVEL_CHANGED_AT, + "#canceled_at", KEY_CANCELED_AT)) .expressionAttributeValues(Map.of( ":accessed_at", n(subscriptionCreatedAt.getEpochSecond()), ":subscription_id", s(subscriptionId), @@ -478,12 +481,14 @@ public class Subscriptions { + "#accessed_at = :accessed_at, " + "#subscription_id = :subscription_id, " + "#subscription_level = :subscription_level, " - + "#subscription_level_changed_at = :subscription_level_changed_at") + + "#subscription_level_changed_at = :subscription_level_changed_at " + + "REMOVE #canceled_at") .expressionAttributeNames(Map.of( "#accessed_at", KEY_ACCESSED_AT, "#subscription_id", KEY_SUBSCRIPTION_ID, "#subscription_level", KEY_SUBSCRIPTION_LEVEL, - "#subscription_level_changed_at", KEY_SUBSCRIPTION_LEVEL_CHANGED_AT)) + "#subscription_level_changed_at", KEY_SUBSCRIPTION_LEVEL_CHANGED_AT, + "#canceled_at", KEY_CANCELED_AT)) .expressionAttributeValues(Map.of( ":accessed_at", n(subscriptionLevelChangedAt.getEpochSecond()), ":subscription_id", s(subscriptionId), diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/SubscriptionsTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/SubscriptionsTest.java index ca6773fc1..00fad9845 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/SubscriptionsTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/SubscriptionsTest.java @@ -12,7 +12,6 @@ import static org.whispersystems.textsecuregcm.storage.Subscriptions.GetResult.T import static org.whispersystems.textsecuregcm.storage.Subscriptions.GetResult.Type.PASSWORD_MISMATCH; import jakarta.ws.rs.ClientErrorException; -import java.security.SecureRandom; import java.time.Duration; import java.time.Instant; import java.util.Base64; @@ -36,7 +35,6 @@ class SubscriptionsTest { private static final long NOW_EPOCH_SECONDS = 1_500_000_000L; private static final Duration DEFAULT_TIMEOUT = Duration.ofSeconds(3); - private static final SecureRandom SECURE_RANDOM = new SecureRandom(); @RegisterExtension static final DynamoDbExtension DYNAMO_DB_EXTENSION = new DynamoDbExtension(Tables.SUBSCRIPTIONS); @@ -177,10 +175,10 @@ class SubscriptionsTest { } @Test - void testCanceledAt() { + void testSetCanceledAt() { Instant canceled = Instant.ofEpochSecond(NOW_EPOCH_SECONDS + 42); assertThat(subscriptions.create(user, password, created)).succeedsWithin(DEFAULT_TIMEOUT); - assertThat(subscriptions.canceledAt(user, canceled)).succeedsWithin(DEFAULT_TIMEOUT); + assertThat(subscriptions.setCanceledAt(user, canceled)).succeedsWithin(DEFAULT_TIMEOUT); assertThat(subscriptions.get(user, password)).succeedsWithin(DEFAULT_TIMEOUT).satisfies(getResult -> { assertThat(getResult).isNotNull(); assertThat(getResult.type).isEqualTo(FOUND); @@ -213,6 +211,36 @@ class SubscriptionsTest { }); } + @Test + void testSubscriptionCreatedClearCanceledAt() { + String subscriptionId = Base64.getEncoder().encodeToString(TestRandomUtil.nextBytes(16)); + Instant subscriptionCreated = Instant.ofEpochSecond(NOW_EPOCH_SECONDS + 1); + Instant canceledAt = subscriptionCreated.plusSeconds(1); + long level = 42; + assertThat(subscriptions.create(user, password, created)).succeedsWithin(DEFAULT_TIMEOUT); + assertThat(subscriptions.subscriptionCreated(user, subscriptionId, subscriptionCreated, level)) + .succeedsWithin(DEFAULT_TIMEOUT); + + assertThat(subscriptions.setCanceledAt(user, canceledAt)).succeedsWithin(DEFAULT_TIMEOUT); + assertThat(subscriptions.get(user, password).join().record.canceledAt).isEqualTo(canceledAt); + + assertThat(subscriptions.subscriptionCreated(user, subscriptionId, subscriptionCreated, level)) + .succeedsWithin(DEFAULT_TIMEOUT); + + assertThat(subscriptions.get(user, password)).succeedsWithin(DEFAULT_TIMEOUT).satisfies(getResult -> { + assertThat(getResult).isNotNull(); + assertThat(getResult.type).isEqualTo(FOUND); + assertThat(getResult.record).isNotNull().satisfies(record -> { + assertThat(record.accessedAt).isEqualTo(subscriptionCreated); + assertThat(record.subscriptionId).isEqualTo(subscriptionId); + assertThat(record.subscriptionCreatedAt).isEqualTo(subscriptionCreated); + assertThat(record.subscriptionLevel).isEqualTo(level); + assertThat(record.subscriptionLevelChangedAt).isEqualTo(subscriptionCreated); + assertThat(record.canceledAt).isNull(); + }); + }); + } + @Test void testSubscriptionLevelChanged() { Instant at = Instant.ofEpochSecond(NOW_EPOCH_SECONDS + 500); @@ -235,6 +263,34 @@ class SubscriptionsTest { }); } + @Test + void testSubscriptionLevelChangedClearCanceledAt() { + Instant at = Instant.ofEpochSecond(NOW_EPOCH_SECONDS + 500); + Instant canceledAt = at.plusSeconds(100); + long level = 1776; + String updatedSubscriptionId = "new"; + assertThat(subscriptions.create(user, password, created)).succeedsWithin(DEFAULT_TIMEOUT); + assertThat(subscriptions.subscriptionCreated(user, "original", created, level - 1)) + .succeedsWithin(DEFAULT_TIMEOUT); + + assertThat(subscriptions.setCanceledAt(user, canceledAt)).succeedsWithin(DEFAULT_TIMEOUT); + assertThat(subscriptions.get(user, password).join().record.canceledAt).isEqualTo(canceledAt); + + assertThat(subscriptions.subscriptionLevelChanged(user, at, level, updatedSubscriptionId)) + .succeedsWithin(DEFAULT_TIMEOUT); + assertThat(subscriptions.get(user, password)).succeedsWithin(DEFAULT_TIMEOUT).satisfies(getResult -> { + assertThat(getResult).isNotNull(); + assertThat(getResult.type).isEqualTo(FOUND); + assertThat(getResult.record).isNotNull().satisfies(record -> { + assertThat(record.accessedAt).isEqualTo(at); + assertThat(record.subscriptionLevelChangedAt).isEqualTo(at); + assertThat(record.subscriptionLevel).isEqualTo(level); + assertThat(record.subscriptionId).isEqualTo(updatedSubscriptionId); + assertThat(record.canceledAt).isNull(); + }); + }); + } + @Test void testSetIapPurchase() { Instant at = Instant.ofEpochSecond(NOW_EPOCH_SECONDS + 500); @@ -287,6 +343,36 @@ class SubscriptionsTest { assertThat(record.getProcessorCustomer().orElseThrow()).isEqualTo(pc); } + @Test + void testSetIapPurchaseClearCanceledAt() { + Instant at = Instant.ofEpochSecond(NOW_EPOCH_SECONDS + 500); + Instant canceledAt = at.plusSeconds(100); + long level = 100; + + ProcessorCustomer pc = new ProcessorCustomer("customerId", PaymentProvider.GOOGLE_PLAY_BILLING); + Record record = subscriptions.create(user, password, created).join(); + + // Should be able to set a fresh subscription + assertThat(subscriptions.setIapPurchase(record, pc, "subscriptionId", level, at)) + .succeedsWithin(DEFAULT_TIMEOUT); + + assertThat(subscriptions.setCanceledAt(record.user, canceledAt)) + .succeedsWithin(DEFAULT_TIMEOUT); + + record = subscriptions.get(user, password).join().record; + assertThat(record.canceledAt).isEqualTo(canceledAt); + + // should be able to update the level + Instant nextAt = at.plus(Duration.ofSeconds(10)); + long nextLevel = level + 1; + assertThat(subscriptions.setIapPurchase(record, pc, "subscriptionId", nextLevel, nextAt)) + .succeedsWithin(DEFAULT_TIMEOUT); + + // Resetting the level should clear the "canceled at" timestamp + record = subscriptions.get(user, password).join().record; + assertThat(record.canceledAt).isNull(); + } + @Test void testProcessorAndCustomerId() { final ProcessorCustomer processorCustomer =