MirrorMirror
/
Signal-Server
mirror of https://github.com/signalapp/Signal-Server.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

remove experiment configuration for low urgency pushes

This commit is contained in:
Ravi Khadiwala 2025-06-13 14:33:53 -05:00 committed by ravi-signal
parent 4f1cab407f
commit 295cedc075
6 changed files with 36 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java
View File

@ -641,7 +641,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, experimentEnrollmentManager);
new PushNotificationManager(accountsManager, apnSender, fcmSender, pushNotificationScheduler);
WebSocketConnectionEventManager webSocketConnectionEventManager =
new WebSocketConnectionEventManager(accountsManager, pushNotificationManager, messagesCluster, clientEventExecutor, asyncOperationQueueingExecutor);
RateLimiters rateLimiters = RateLimiters.create(dynamicConfigurationManager, rateLimitersCluster);

20
service/src/main/java/org/whispersystems/textsecuregcm/push/PushNotificationManager.java
View File

@ -17,7 +17,6 @@ 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;
@ -29,9 +28,6 @@ public class PushNotificationManager {
private final APNSender apnSender;
private final FcmSender fcmSender;
private final PushNotificationScheduler pushNotificationScheduler;
private final ExperimentEnrollmentManager experimentEnrollmentManager;
public 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");
@ -42,14 +38,12 @@ public class PushNotificationManager {
public PushNotificationManager(final AccountsManager accountsManager,
final APNSender apnSender,
final FcmSender fcmSender,
final PushNotificationScheduler pushNotificationScheduler,
final ExperimentEnrollmentManager experimentEnrollmentManager) {
final PushNotificationScheduler pushNotificationScheduler) {
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 {
@ -107,7 +101,7 @@ public class PushNotificationManager {
@VisibleForTesting
CompletableFuture<Optional<SendPushNotificationResult>> sendNotification(final PushNotification pushNotification) {
if (shouldScheduleNotification(pushNotification)) {
if (!pushNotification.urgent()) {
// Schedule a notification for some time in the future (possibly even now!) rather than sending a notification
// directly
return pushNotificationScheduler
@ -156,16 +150,6 @@ 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) {

5
service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagePersister.java
View File

@ -244,11 +244,8 @@ public class MessagePersister implements Managed {
} while (!messages.isEmpty());
final boolean inSkipExperiment = device.getGcmId() != null && experimentEnrollmentManager.isEnrolled(
accountUuid,
PushNotificationManager.SCHEDULE_LOW_URGENCY_FCM_PUSH_EXPERIMENT);
DistributionSummary.builder(QUEUE_SIZE_DISTRIBUTION_SUMMARY_NAME)
.tags(Tags.of(platformTag).and("lowUrgencySkip", Boolean.toString(inSkipExperiment)))
.tags(Tags.of(platformTag))
.publishPercentileHistogram(true)
.register(Metrics.globalRegistry)
.record(messageCount);

7
service/src/main/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnection.java
View File

@ -338,13 +338,8 @@ public class WebSocketConnection implements WebSocketConnectionEventListener {
// Cleared the queue! Send a queue empty message if we need to
consecutiveRetries.set(0);
if (sentInitialQueueEmptyMessage.compareAndSet(false, true)) {
final boolean inSkipExperiment = auth.getAuthenticatedDevice().getGcmId() != null && experimentEnrollmentManager.isEnrolled(
auth.getAccount().getUuid(),
PushNotificationManager.SCHEDULE_LOW_URGENCY_FCM_PUSH_EXPERIMENT);
final Tags tags = Tags
.of(UserAgentTagUtil.getPlatformTag(client.getUserAgent()))
.and("lowUrgencySkip", Boolean.toString(inSkipExperiment));
final Tags tags = Tags.of(UserAgentTagUtil.getPlatformTag(client.getUserAgent()));
final long drainDuration = System.currentTimeMillis() - queueDrainStartTime.get();
Metrics.summary(INITIAL_QUEUE_LENGTH_DISTRIBUTION_NAME, tags).record(sentMessageCounter.sum());

6
service/src/main/java/org/whispersystems/textsecuregcm/workers/CommandDependencies.java
View File

@ -33,7 +33,6 @@ 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;
@ -118,9 +117,6 @@ 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();
FaultTolerantRedisClusterClient cacheCluster = configuration.getCacheClusterConfiguration()
@ -286,7 +282,7 @@ record CommandDependencies(
PushNotificationScheduler pushNotificationScheduler = new PushNotificationScheduler(pushSchedulerCluster,
apnSender, fcmSender, accountsManager, 0, 0);
PushNotificationManager pushNotificationManager = new PushNotificationManager(accountsManager,
apnSender, fcmSender, pushNotificationScheduler, experimentEnrollmentManager);
apnSender, fcmSender, pushNotificationScheduler);
PushNotificationExperimentSamples pushNotificationExperimentSamples =
new PushNotificationExperimentSamples(dynamoDbAsyncClient,
configuration.getDynamoDbTables().getPushNotificationExperimentSamples().getTableName(),

52
service/src/test/java/org/whispersystems/textsecuregcm/push/PushNotificationManagerTest.java
View File

@ -24,7 +24,6 @@ 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;
@ -36,7 +35,6 @@ class PushNotificationManagerTest {
private APNSender apnSender;
private FcmSender fcmSender;
private PushNotificationScheduler pushNotificationScheduler;
private ExperimentEnrollmentManager experimentEnrollmentManager;
private PushNotificationManager pushNotificationManager;
@ -46,17 +44,15 @@ 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, experimentEnrollmentManager);
pushNotificationScheduler);
}
@ParameterizedTest
@ValueSource(booleans = {true, false})
void sendNewMessageNotification(final boolean urgent) throws NotPushRegisteredException {
@Test
void sendNewUrgentMessageNotification() throws NotPushRegisteredException {
final Account account = mock(Account.class);
final Device device = mock(Device.class);
@ -66,13 +62,30 @@ class PushNotificationManagerTest {
when(device.getGcmId()).thenReturn(deviceToken);
when(account.getDevice(Device.PRIMARY_ID)).thenReturn(Optional.of(device));
when(fcmSender.sendNotification(any()))
.thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, Optional.empty(), false, Optional.empty())));
pushNotificationManager.sendNewMessageNotification(account, Device.PRIMARY_ID, urgent);
verify(fcmSender).sendNotification(new PushNotification(deviceToken, PushNotification.TokenType.FCM, PushNotification.NotificationType.NOTIFICATION, null, account, device, urgent));
when(fcmSender.sendNotification(any()))
.thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, Optional.empty(), false, Optional.empty())));
pushNotificationManager.sendNewMessageNotification(account, Device.PRIMARY_ID, true);
verify(fcmSender).sendNotification(new PushNotification(deviceToken, PushNotification.TokenType.FCM, PushNotification.NotificationType.NOTIFICATION, null, account, device, true));
}
@Test
void sendNewNonUrgentMessageNotification() throws NotPushRegisteredException {
final Account account = mock(Account.class);
final Device device = mock(Device.class);
final String deviceToken = "token";
when(device.getId()).thenReturn(Device.PRIMARY_ID);
when(device.getGcmId()).thenReturn(deviceToken);
when(account.getDevice(Device.PRIMARY_ID)).thenReturn(Optional.of(device));
when(pushNotificationScheduler.scheduleBackgroundNotification(any(), any(), any()))
.thenReturn(CompletableFuture.completedFuture(null));
pushNotificationManager.sendNewMessageNotification(account, Device.PRIMARY_ID, false);
verify(pushNotificationScheduler).scheduleBackgroundNotification(PushNotification.TokenType.FCM, account, device);
}
@Test
void sendRegistrationChallengeNotification() {
final String deviceToken = "token";
@ -135,9 +148,8 @@ class PushNotificationManagerTest {
}
}
@ParameterizedTest
@ValueSource(booleans = {true, false})
void testSendNotificationFcm(final boolean urgent) {
@Test
void testSendNotificationFcm() {
final Account account = mock(Account.class);
final Device device = mock(Device.class);
@ -145,7 +157,7 @@ class PushNotificationManagerTest {
when(account.getDevice(Device.PRIMARY_ID)).thenReturn(Optional.of(device));
final PushNotification pushNotification = new PushNotification(
"token", PushNotification.TokenType.FCM, PushNotification.NotificationType.NOTIFICATION, null, account, device, urgent);
"token", PushNotification.TokenType.FCM, PushNotification.NotificationType.NOTIFICATION, null, account, device, true);
when(fcmSender.sendNotification(pushNotification))
.thenReturn(CompletableFuture.completedFuture(new SendPushNotificationResult(true, Optional.empty(), false, Optional.empty())));
@ -162,10 +174,9 @@ class PushNotificationManagerTest {
@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) {
@CartesianTest.Values(booleans = {false, true}) final boolean urgent) {
final boolean expectSchedule = !urgent && (tokenType == PushNotification.TokenType.APN || inExperiment);
final boolean expectSchedule = !urgent;
final Account account = mock(Account.class);
final Device device = mock(Device.class);
@ -175,9 +186,6 @@ class PushNotificationManagerTest {
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", tokenType, PushNotification.NotificationType.NOTIFICATION, null, account, device, urgent);