Add experiment to coalesce android notifications
This commit is contained in:
parent
703a05cb15
commit
847b25f695
|
@ -647,7 +647,7 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
|
|||
PushNotificationScheduler pushNotificationScheduler = new PushNotificationScheduler(pushSchedulerCluster,
|
||||
apnSender, fcmSender, accountsManager, 0, 0);
|
||||
PushNotificationManager pushNotificationManager =
|
||||
new PushNotificationManager(accountsManager, apnSender, fcmSender, pushNotificationScheduler);
|
||||
new PushNotificationManager(accountsManager, apnSender, fcmSender, pushNotificationScheduler, experimentEnrollmentManager);
|
||||
WebSocketConnectionEventManager webSocketConnectionEventManager =
|
||||
new WebSocketConnectionEventManager(accountsManager, pushNotificationManager, messagesCluster, clientEventExecutor, asyncOperationQueueingExecutor);
|
||||
RateLimiters rateLimiters = RateLimiters.createAndValidate(config.getLimitsConfiguration(),
|
||||
|
|
|
@ -17,6 +17,7 @@ import java.util.function.BiConsumer;
|
|||
import org.apache.commons.lang3.StringUtils;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager;
|
||||
import org.whispersystems.textsecuregcm.storage.Account;
|
||||
import org.whispersystems.textsecuregcm.storage.AccountsManager;
|
||||
import org.whispersystems.textsecuregcm.storage.Device;
|
||||
|
@ -28,6 +29,9 @@ public class PushNotificationManager {
|
|||
private final APNSender apnSender;
|
||||
private final FcmSender fcmSender;
|
||||
private final PushNotificationScheduler pushNotificationScheduler;
|
||||
private final ExperimentEnrollmentManager experimentEnrollmentManager;
|
||||
|
||||
private static final String SCHEDULE_LOW_URGENCY_FCM_PUSH_EXPERIMENT = "scheduleLowUregencyFcmPush";
|
||||
|
||||
private static final String SENT_NOTIFICATION_COUNTER_NAME = name(PushNotificationManager.class, "sentPushNotification");
|
||||
private static final String FAILED_NOTIFICATION_COUNTER_NAME = name(PushNotificationManager.class, "failedPushNotification");
|
||||
|
@ -38,12 +42,14 @@ public class PushNotificationManager {
|
|||
public PushNotificationManager(final AccountsManager accountsManager,
|
||||
final APNSender apnSender,
|
||||
final FcmSender fcmSender,
|
||||
final PushNotificationScheduler pushNotificationScheduler) {
|
||||
final PushNotificationScheduler pushNotificationScheduler,
|
||||
final ExperimentEnrollmentManager experimentEnrollmentManager) {
|
||||
|
||||
this.accountsManager = accountsManager;
|
||||
this.apnSender = apnSender;
|
||||
this.fcmSender = fcmSender;
|
||||
this.pushNotificationScheduler = pushNotificationScheduler;
|
||||
this.experimentEnrollmentManager = experimentEnrollmentManager;
|
||||
}
|
||||
|
||||
public CompletableFuture<Optional<SendPushNotificationResult>> sendNewMessageNotification(final Account destination, final byte destinationDeviceId, final boolean urgent) throws NotPushRegisteredException {
|
||||
|
@ -101,9 +107,9 @@ public class PushNotificationManager {
|
|||
|
||||
@VisibleForTesting
|
||||
CompletableFuture<Optional<SendPushNotificationResult>> 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 <T> BiConsumer<T, Throwable> logErrors() {
|
||||
return (ignored, throwable) -> {
|
||||
if (throwable != null) {
|
||||
|
|
|
@ -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(),
|
||||
|
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue