From 894ca6d29041bdafac6f2ae9f95d2be584d09cb5 Mon Sep 17 00:00:00 2001 From: Ravi Khadiwala Date: Wed, 7 May 2025 13:04:43 -0500 Subject: [PATCH] remove ANDROID_SKIP_LOW_URGENCY_PUSH_EXPERIMENT --- .../textsecuregcm/push/MessageSender.java | 11 +--- .../push/PushNotificationManager.java | 2 +- .../storage/MessagePersister.java | 3 +- .../websocket/WebSocketConnection.java | 2 +- .../textsecuregcm/push/MessageSenderTest.java | 51 ------------------- 5 files changed, 5 insertions(+), 64 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/push/MessageSender.java b/service/src/main/java/org/whispersystems/textsecuregcm/push/MessageSender.java index 51f45aea1..70eb5a6b5 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/push/MessageSender.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/push/MessageSender.java @@ -57,8 +57,6 @@ public class MessageSender { private final PushNotificationManager pushNotificationManager; private final ExperimentEnrollmentManager experimentEnrollmentManager; - public static final String ANDROID_SKIP_LOW_URGENCY_PUSH_EXPERIMENT = "androidSkipLowUrgencyPush"; - // Note that these names deliberately reference `MessageController` for metric continuity private static final String REJECT_OVERSIZE_MESSAGE_COUNTER_NAME = name(MessageController.class, "rejectOversizeMessage"); private static final String CONTENT_SIZE_DISTRIBUTION_NAME = MetricsUtil.name(MessageController.class, "messageContentSize"); @@ -156,7 +154,7 @@ public class MessageSender { .forEach((deviceId, destinationPresent) -> { final Envelope message = messagesByDeviceId.get(deviceId); - if (!destinationPresent && !message.getEphemeral() && !shouldSkipPush(destination, deviceId, message.getUrgent())) { + if (!destinationPresent && !message.getEphemeral()) { try { pushNotificationManager.sendNewMessageNotification(destination, deviceId, message.getUrgent()); } catch (final NotPushRegisteredException ignored) { @@ -177,13 +175,6 @@ public class MessageSender { }); } - private boolean shouldSkipPush(final Account account, byte deviceId, boolean urgent) { - final boolean isAndroidFcm = account.getDevice(deviceId).map(Device::getGcmId).isPresent(); - return !urgent - && isAndroidFcm - && experimentEnrollmentManager.isEnrolled(account.getUuid(), ANDROID_SKIP_LOW_URGENCY_PUSH_EXPERIMENT); - } - /** * Sends messages to a group of recipients. If a destination device has a valid push notification token and does not * have an active connection to a Signal server, then this method will also send a push notification to that device to diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/push/PushNotificationManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/push/PushNotificationManager.java index ea59124e1..d9f923c5a 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/push/PushNotificationManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/push/PushNotificationManager.java @@ -31,7 +31,7 @@ public class PushNotificationManager { private final PushNotificationScheduler pushNotificationScheduler; private final ExperimentEnrollmentManager experimentEnrollmentManager; - private static final String SCHEDULE_LOW_URGENCY_FCM_PUSH_EXPERIMENT = "scheduleLowUregencyFcmPush"; + 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"); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagePersister.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagePersister.java index 0ac62e9f3..8bc94fdca 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagePersister.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/MessagePersister.java @@ -32,6 +32,7 @@ import org.whispersystems.textsecuregcm.experiment.ExperimentEnrollmentManager; import org.whispersystems.textsecuregcm.identity.IdentityType; import org.whispersystems.textsecuregcm.metrics.DevicePlatformUtil; import org.whispersystems.textsecuregcm.push.MessageSender; +import org.whispersystems.textsecuregcm.push.PushNotificationManager; import org.whispersystems.textsecuregcm.util.Util; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -245,7 +246,7 @@ public class MessagePersister implements Managed { final boolean inSkipExperiment = device.getGcmId() != null && experimentEnrollmentManager.isEnrolled( accountUuid, - MessageSender.ANDROID_SKIP_LOW_URGENCY_PUSH_EXPERIMENT); + PushNotificationManager.SCHEDULE_LOW_URGENCY_FCM_PUSH_EXPERIMENT); DistributionSummary.builder(QUEUE_SIZE_DISTRIBUTION_SUMMARY_NAME) .tags(Tags.of(platformTag).and("lowUrgencySkip", Boolean.toString(inSkipExperiment))) .publishPercentileHistogram(true) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnection.java b/service/src/main/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnection.java index e0e2baa2e..4a9931202 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnection.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/websocket/WebSocketConnection.java @@ -339,7 +339,7 @@ public class WebSocketConnection implements WebSocketConnectionEventListener { if (sentInitialQueueEmptyMessage.compareAndSet(false, true)) { final boolean inSkipExperiment = auth.getAuthenticatedDevice().getGcmId() != null && experimentEnrollmentManager.isEnrolled( auth.getAccount().getUuid(), - MessageSender.ANDROID_SKIP_LOW_URGENCY_PUSH_EXPERIMENT); + PushNotificationManager.SCHEDULE_LOW_URGENCY_FCM_PUSH_EXPERIMENT); final Tags tags = Tags .of(UserAgentTagUtil.getPlatformTag(client.getUserAgent())) diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/push/MessageSenderTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/push/MessageSenderTest.java index c3d9231ba..8da5d3f87 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/push/MessageSenderTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/push/MessageSenderTest.java @@ -68,57 +68,6 @@ class MessageSenderTest { } - @CartesianTest - void pushSkippedExperiment( - @CartesianTest.Values(booleans = {true, false}) final boolean hasGcmToken, - @CartesianTest.Values(booleans = {true, false}) final boolean isUrgent, - @CartesianTest.Values(booleans = {true, false}) final boolean inExperiment) throws NotPushRegisteredException { - - final boolean shouldSkip = hasGcmToken && !isUrgent && inExperiment; - - final UUID accountIdentifier = UUID.randomUUID(); - final ServiceIdentifier serviceIdentifier = new AciServiceIdentifier(accountIdentifier); - final byte deviceId = Device.PRIMARY_ID; - final int registrationId = 17; - - final Account account = mock(Account.class); - final Device device = mock(Device.class); - final MessageProtos.Envelope message = MessageProtos.Envelope.newBuilder() - .setEphemeral(false) - .setUrgent(isUrgent) - .build(); - - when(account.getUuid()).thenReturn(accountIdentifier); - when(account.getIdentifier(IdentityType.ACI)).thenReturn(accountIdentifier); - when(account.isIdentifiedBy(serviceIdentifier)).thenReturn(true); - when(account.getDevices()).thenReturn(List.of(device)); - when(account.getDevice(deviceId)).thenReturn(Optional.of(device)); - when(device.getId()).thenReturn(deviceId); - when(device.getRegistrationId()).thenReturn(registrationId); - - if (hasGcmToken) { - when(device.getGcmId()).thenReturn("gcm-token"); - } else { - when(device.getApnId()).thenReturn("apn-token"); - } - when(messagesManager.insert(any(), any())).thenReturn(Map.of(deviceId, false)); - when(experimentEnrollmentManager.isEnrolled(accountIdentifier, MessageSender.ANDROID_SKIP_LOW_URGENCY_PUSH_EXPERIMENT)) - .thenReturn(inExperiment); - - assertDoesNotThrow(() -> messageSender.sendMessages(account, - serviceIdentifier, - Map.of(device.getId(), message), - Map.of(device.getId(), registrationId), - Optional.empty(), - null)); - - if (shouldSkip) { - verifyNoInteractions(pushNotificationManager); - } else { - verify(pushNotificationManager).sendNewMessageNotification(account, deviceId, isUrgent); - } - } - @CartesianTest void sendMessage(@CartesianTest.Values(booleans = {true, false}) final boolean clientPresent, @CartesianTest.Values(booleans = {true, false}) final boolean ephemeral,