diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index f123052cb..c4c127d78 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -647,7 +647,7 @@ public class WhisperServerService extends Application> sendNewMessageNotification(final Account destination, final byte destinationDeviceId, final boolean urgent) throws NotPushRegisteredException { @@ -101,9 +107,9 @@ public class PushNotificationManager { @VisibleForTesting CompletableFuture> sendNotification(final PushNotification pushNotification) { - if (pushNotification.tokenType() == PushNotification.TokenType.APN && !pushNotification.urgent()) { - // APNs imposes a per-device limit on background push notifications; schedule a notification for some time in the - // future (possibly even now!) rather than sending a notification directly + if (shouldScheduleNotification(pushNotification)) { + // Schedule a notification for some time in the future (possibly even now!) rather than sending a notification + // directly return pushNotificationScheduler .scheduleBackgroundNotification(pushNotification.tokenType(), pushNotification.destination(), pushNotification.destinationDevice()) .whenComplete(logErrors()) @@ -150,6 +156,16 @@ public class PushNotificationManager { .thenApply(Optional::of); } + private boolean shouldScheduleNotification(final PushNotification pushNotification) { + return !pushNotification.urgent() && switch (pushNotification.tokenType()) { + // APNs imposes a per-device limit on background push notifications + case APN -> true; + case FCM -> experimentEnrollmentManager.isEnrolled( + pushNotification.destination().getUuid(), + SCHEDULE_LOW_URGENCY_FCM_PUSH_EXPERIMENT); + }; + } + private static BiConsumer logErrors() { return (ignored, throwable) -> { if (throwable != null) { 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 43cdb492b..374d44cc8 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/CommandDependencies.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/CommandDependencies.java @@ -33,6 +33,7 @@ import org.whispersystems.textsecuregcm.backup.Cdn3RemoteStorageManager; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; import org.whispersystems.textsecuregcm.controllers.SecureStorageController; import org.whispersystems.textsecuregcm.controllers.SecureValueRecovery2Controller; +import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.experiment.PushNotificationExperimentSamples; import org.whispersystems.textsecuregcm.limits.RateLimiters; import org.whispersystems.textsecuregcm.metrics.MicrometerAwsSdkMetricPublisher; @@ -113,6 +114,8 @@ record CommandDependencies( new DynamicConfigurationManager<>( configuration.getDynamicConfig().build(awsCredentialsProvider, dynamicConfigurationExecutor), DynamicConfiguration.class); dynamicConfigurationManager.start(); + final ExperimentEnrollmentManager experimentEnrollmentManager = + new ExperimentEnrollmentManager(dynamicConfigurationManager); final ClientResources.Builder redisClientResourcesBuilder = ClientResources.builder(); @@ -271,8 +274,8 @@ record CommandDependencies( FcmSender fcmSender = new FcmSender(fcmSenderExecutor, configuration.getFcmConfiguration().credentials().value()); PushNotificationScheduler pushNotificationScheduler = new PushNotificationScheduler(pushSchedulerCluster, apnSender, fcmSender, accountsManager, 0, 0); - PushNotificationManager pushNotificationManager = - new PushNotificationManager(accountsManager, apnSender, fcmSender, pushNotificationScheduler); + PushNotificationManager pushNotificationManager = new PushNotificationManager(accountsManager, + apnSender, fcmSender, pushNotificationScheduler, experimentEnrollmentManager); PushNotificationExperimentSamples pushNotificationExperimentSamples = new PushNotificationExperimentSamples(dynamoDbAsyncClient, configuration.getDynamoDbTables().getPushNotificationExperimentSamples().getTableName(), diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/push/PushNotificationManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/push/PushNotificationManagerTest.java index cc7469aa2..1aed9f3bc 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/push/PushNotificationManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/push/PushNotificationManagerTest.java @@ -23,6 +23,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +import org.junitpioneer.jupiter.cartesian.CartesianTest; +import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.AccountsManager; import org.whispersystems.textsecuregcm.storage.Device; @@ -34,6 +36,7 @@ class PushNotificationManagerTest { private APNSender apnSender; private FcmSender fcmSender; private PushNotificationScheduler pushNotificationScheduler; + private ExperimentEnrollmentManager experimentEnrollmentManager; private PushNotificationManager pushNotificationManager; @@ -43,11 +46,12 @@ class PushNotificationManagerTest { apnSender = mock(APNSender.class); fcmSender = mock(FcmSender.class); pushNotificationScheduler = mock(PushNotificationScheduler.class); + experimentEnrollmentManager = mock(ExperimentEnrollmentManager.class); AccountsHelper.setupMockUpdate(accountsManager); - pushNotificationManager = - new PushNotificationManager(accountsManager, apnSender, fcmSender, pushNotificationScheduler); + pushNotificationManager = new PushNotificationManager(accountsManager, apnSender, fcmSender, + pushNotificationScheduler, experimentEnrollmentManager); } @ParameterizedTest @@ -155,36 +159,48 @@ class PushNotificationManagerTest { verifyNoInteractions(pushNotificationScheduler); } - @ParameterizedTest - @ValueSource(booleans = {true, false}) - void testSendNotificationApn(final boolean urgent) { + @CartesianTest + void testSendOrScheduleNotification( + @CartesianTest.Enum(PushNotification.TokenType.class) PushNotification.TokenType tokenType, + @CartesianTest.Values(booleans = {false, true}) final boolean urgent, + @CartesianTest.Values(booleans = {false, true}) final boolean inExperiment) { + + final boolean expectSchedule = !urgent && (tokenType == PushNotification.TokenType.APN || inExperiment); + final Account account = mock(Account.class); final Device device = mock(Device.class); + final UUID aci = UUID.randomUUID(); when(device.getId()).thenReturn(Device.PRIMARY_ID); when(account.getDevice(Device.PRIMARY_ID)).thenReturn(Optional.of(device)); + when(account.getUuid()).thenReturn(aci); + + when(experimentEnrollmentManager.isEnrolled(aci, PushNotificationManager.SCHEDULE_LOW_URGENCY_FCM_PUSH_EXPERIMENT)) + .thenReturn(inExperiment); final PushNotification pushNotification = new PushNotification( - "token", PushNotification.TokenType.APN, PushNotification.NotificationType.NOTIFICATION, null, account, device, urgent); + "token", tokenType, PushNotification.NotificationType.NOTIFICATION, null, account, device, urgent); - when(apnSender.sendNotification(pushNotification)) + final PushNotificationSender sender = switch (tokenType) { + case FCM -> fcmSender; + case APN -> apnSender; + }; + when(sender.sendNotification(pushNotification)) .thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, Optional.empty(), false, Optional.empty()))); - if (!urgent) { - when(pushNotificationScheduler.scheduleBackgroundNotification(PushNotification.TokenType.APN, account, device)) + if (expectSchedule) { + when(pushNotificationScheduler.scheduleBackgroundNotification(tokenType, account, device)) .thenReturn(CompletableFuture.completedFuture(null)); } pushNotificationManager.sendNotification(pushNotification); - verifyNoInteractions(fcmSender); - - if (urgent) { - verify(apnSender).sendNotification(pushNotification); + if (!expectSchedule) { + verify(sender).sendNotification(pushNotification); verifyNoInteractions(pushNotificationScheduler); } else { - verifyNoInteractions(apnSender); - verify(pushNotificationScheduler).scheduleBackgroundNotification(PushNotification.TokenType.APN, account, device); + verifyNoInteractions(sender); + verify(pushNotificationScheduler).scheduleBackgroundNotification(tokenType, account, device); } }