Narrow selection criterial for "notify idle devices with messages" ex…

…periment
signalapp · Sep 23, 2024 · 3e51366 · 3e51366
1 parent 2d184b1
commit 3e51366
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 13 deletions.
diff --git a/.../org/whispersystems/textsecuregcm/experiment/NotifyIdleDevicesWithMessagesExperiment.java b/.../org/whispersystems/textsecuregcm/experiment/NotifyIdleDevicesWithMessagesExperiment.java
@@ -1,11 +1,13 @@
 package org.whispersystems.textsecuregcm.experiment;
 
 import com.google.common.annotations.VisibleForTesting;
+import org.whispersystems.textsecuregcm.entities.MessageProtos;
 import org.whispersystems.textsecuregcm.identity.IdentityType;
 import org.whispersystems.textsecuregcm.push.IdleDeviceNotificationScheduler;
 import org.whispersystems.textsecuregcm.storage.Account;
 import org.whispersystems.textsecuregcm.storage.Device;
 import org.whispersystems.textsecuregcm.storage.MessagesManager;
+import reactor.core.publisher.Flux;
 import java.time.Clock;
 import java.time.Duration;
 import java.time.LocalTime;
@@ -53,6 +55,10 @@ public String getExperimentName() {
   @Override
   public CompletableFuture<Boolean> isDeviceEligible(final Account account, final Device device) {
 
+    if (!device.isPrimary()) {
+      return CompletableFuture.completedFuture(false);
+    }
+
     if (!hasPushToken(device)) {
       return CompletableFuture.completedFuture(false);
     }
@@ -61,7 +67,9 @@ public CompletableFuture<Boolean> isDeviceEligible(final Account account, final
       return CompletableFuture.completedFuture(false);
     }
 
-    return messagesManager.mayHavePersistedMessages(account.getIdentifier(IdentityType.ACI), device);
+    return Flux.from(messagesManager.getMessagesForDeviceReactive(account.getIdentifier(IdentityType.ACI), device, false))
+        .any(MessageProtos.Envelope::getUrgent)
+        .toFuture();
   }
 
   @Override

diff --git a/.../whispersystems/textsecuregcm/experiment/NotifyIdleDevicesWithMessagesExperimentTest.java b/.../whispersystems/textsecuregcm/experiment/NotifyIdleDevicesWithMessagesExperimentTest.java
@@ -12,17 +12,18 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.UUID;
-import java.util.concurrent.CompletableFuture;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
+import org.whispersystems.textsecuregcm.entities.MessageProtos;
 import org.whispersystems.textsecuregcm.identity.IdentityType;
 import org.whispersystems.textsecuregcm.push.IdleDeviceNotificationScheduler;
 import org.whispersystems.textsecuregcm.storage.Account;
 import org.whispersystems.textsecuregcm.storage.Device;
 import org.whispersystems.textsecuregcm.storage.MessagesManager;
+import reactor.core.publisher.Flux;
 
 class NotifyIdleDevicesWithMessagesExperimentTest extends IdleDevicePushNotificationExperimentTest {
 
@@ -50,11 +51,11 @@ protected IdleDevicePushNotificationExperiment getExperiment() {
   @MethodSource
   void isDeviceEligible(final Account account,
       final Device device,
-      final boolean mayHaveMessages,
+      final boolean hasUrgentMessage,
       final boolean expectEligible) {
 
-    when(messagesManager.mayHavePersistedMessages(account.getIdentifier(IdentityType.ACI), device))
-        .thenReturn(CompletableFuture.completedFuture(mayHaveMessages));
+    when(messagesManager.getMessagesForDeviceReactive(account.getIdentifier(IdentityType.ACI), device, false))
+        .thenReturn(Flux.just(MessageProtos.Envelope.newBuilder().setUrgent(hasUrgentMessage).build()));
 
     assertEquals(expectEligible, experiment.isDeviceEligible(account, device).join());
   }
@@ -68,67 +69,85 @@ private static List<Arguments> isDeviceEligible() {
         PhoneNumberUtil.getInstance().getExampleNumber("US"), PhoneNumberUtil.PhoneNumberFormat.E164));
 
     {
-      // Idle device with push token and messages
+      // Idle primary device with push token and urgent messages
       final Device device = mock(Device.class);
+      when(device.isPrimary()).thenReturn(true);
       when(device.getApnId()).thenReturn("apns-token");
       when(device.getLastSeen()).thenReturn(CURRENT_TIME.minus(NotifyIdleDevicesWithMessagesExperiment.MIN_IDLE_DURATION).toEpochMilli());
 
       arguments.add(Arguments.of(account, device, true, true));
     }
 
     {
-      // Idle device missing push token, but with messages
+      // Idle non-primary device with push token and urgent messages
       final Device device = mock(Device.class);
+      when(device.isPrimary()).thenReturn(false);
+      when(device.getApnId()).thenReturn("apns-token");
+      when(device.getLastSeen()).thenReturn(CURRENT_TIME.minus(NotifyIdleDevicesWithMessagesExperiment.MIN_IDLE_DURATION).toEpochMilli());
+
+      arguments.add(Arguments.of(account, device, true, false));
+    }
+
+    {
+      // Idle primary device missing push token, but with messages
+      final Device device = mock(Device.class);
+      when(device.isPrimary()).thenReturn(true);
       when(device.getLastSeen()).thenReturn(CURRENT_TIME.minus(NotifyIdleDevicesWithMessagesExperiment.MIN_IDLE_DURATION).toEpochMilli());
 
       arguments.add(Arguments.of(account, device, true, false));
     }
 
     {
-      // Idle device missing push token and messages
+      // Idle primary device missing push token and with no urgent messages
       final Device device = mock(Device.class);
+      when(device.isPrimary()).thenReturn(true);
       when(device.getLastSeen()).thenReturn(CURRENT_TIME.minus(NotifyIdleDevicesWithMessagesExperiment.MIN_IDLE_DURATION).toEpochMilli());
 
       arguments.add(Arguments.of(account, device, false, false));
     }
 
     {
-      // Idle device with push token, but no messages
+      // Idle primary device with push token, but no urgent messages
       final Device device = mock(Device.class);
+      when(device.isPrimary()).thenReturn(true);
       when(device.getLastSeen()).thenReturn(CURRENT_TIME.minus(NotifyIdleDevicesWithMessagesExperiment.MIN_IDLE_DURATION).toEpochMilli());
       when(device.getApnId()).thenReturn("apns-token");
 
       arguments.add(Arguments.of(account, device, false, false));
     }
 
     {
-      // Active device with push token and messages
+      // Active primary device with push token and urgent messages
       final Device device = mock(Device.class);
+      when(device.isPrimary()).thenReturn(true);
       when(device.getLastSeen()).thenReturn(CURRENT_TIME.toEpochMilli());
       when(device.getApnId()).thenReturn("apns-token");
 
       arguments.add(Arguments.of(account, device, true, false));
     }
 
     {
-      // Active device missing push token, but with messages
+      // Active primary device missing push token, but with urgent messages
       final Device device = mock(Device.class);
+      when(device.isPrimary()).thenReturn(true);
       when(device.getLastSeen()).thenReturn(CURRENT_TIME.toEpochMilli());
 
       arguments.add(Arguments.of(account, device, true, false));
     }
 
     {
-      // Active device missing push token and messages
+      // Active primary device missing push token and with no urgent messages
       final Device device = mock(Device.class);
+      when(device.isPrimary()).thenReturn(true);
       when(device.getLastSeen()).thenReturn(CURRENT_TIME.toEpochMilli());
 
       arguments.add(Arguments.of(account, device, false, false));
     }
 
     {
-      // Active device with push token, but no messages
+      // Active primary device with push token, but no urgent messages
       final Device device = mock(Device.class);
+      when(device.isPrimary()).thenReturn(true);
       when(device.getLastSeen()).thenReturn(CURRENT_TIME.toEpochMilli());
       when(device.getApnId()).thenReturn("apns-token");