diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index 9a4cc18f6..f1538fdac 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -251,11 +251,8 @@ import org.whispersystems.textsecuregcm.workers.BackupMetricsCommand; import org.whispersystems.textsecuregcm.workers.CertificateCommand; import org.whispersystems.textsecuregcm.workers.CheckDynamicConfigurationCommand; import org.whispersystems.textsecuregcm.workers.DeleteUserCommand; -import org.whispersystems.textsecuregcm.workers.DiscardPushNotificationExperimentSamplesCommand; -import org.whispersystems.textsecuregcm.workers.FinishPushNotificationExperimentCommand; import org.whispersystems.textsecuregcm.workers.IdleDeviceNotificationSchedulerFactory; import org.whispersystems.textsecuregcm.workers.MessagePersisterServiceCommand; -import org.whispersystems.textsecuregcm.workers.NotifyIdleDevicesWithoutMessagesExperimentFactory; import org.whispersystems.textsecuregcm.workers.ProcessScheduledJobsServiceCommand; import org.whispersystems.textsecuregcm.workers.RemoveExpiredAccountsCommand; import org.whispersystems.textsecuregcm.workers.RemoveExpiredBackupsCommand; @@ -265,7 +262,6 @@ import org.whispersystems.textsecuregcm.workers.ScheduledApnPushNotificationSend import org.whispersystems.textsecuregcm.workers.ServerVersionCommand; import org.whispersystems.textsecuregcm.workers.SetRequestLoggingEnabledTask; import org.whispersystems.textsecuregcm.workers.SetUserDiscoverabilityCommand; -import org.whispersystems.textsecuregcm.workers.StartPushNotificationExperimentCommand; import org.whispersystems.textsecuregcm.workers.UnlinkDeviceCommand; import org.whispersystems.textsecuregcm.workers.ZkParamsCommand; import org.whispersystems.websocket.WebSocketResourceProviderFactory; @@ -322,21 +318,6 @@ public class WhisperServerService extends Application("start-notify-idle-devices-without-messages-experiment", - "Start an experiment to send push notifications to idle devices with empty message queues", - new NotifyIdleDevicesWithoutMessagesExperimentFactory())); - - bootstrap.addCommand( - new FinishPushNotificationExperimentCommand<>("finish-notify-idle-devices-without-messages-experiment", - "Finish an experiment to send push notifications to idle devices with empty message queues", - new NotifyIdleDevicesWithoutMessagesExperimentFactory())); - - bootstrap.addCommand( - new DiscardPushNotificationExperimentSamplesCommand("discard-notify-idle-devices-without-messages-samples", - "Discard samples from the \"notify idle devices without messages\" experiment", - new NotifyIdleDevicesWithoutMessagesExperimentFactory())); } @Override diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/experiment/NotifyIdleDevicesWithoutMessagesPushNotificationExperiment.java b/service/src/main/java/org/whispersystems/textsecuregcm/experiment/NotifyIdleDevicesWithoutMessagesPushNotificationExperiment.java deleted file mode 100644 index 581b9855e..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/experiment/NotifyIdleDevicesWithoutMessagesPushNotificationExperiment.java +++ /dev/null @@ -1,164 +0,0 @@ -package org.whispersystems.textsecuregcm.experiment; - -import com.google.common.annotations.VisibleForTesting; -import org.apache.commons.lang3.StringUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -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 javax.annotation.Nullable; -import java.util.EnumMap; -import java.util.Map; -import java.time.LocalTime; -import java.util.concurrent.CompletableFuture; - -public class NotifyIdleDevicesWithoutMessagesPushNotificationExperiment implements PushNotificationExperiment { - - private final MessagesManager messagesManager; - private final IdleDeviceNotificationScheduler idleDeviceNotificationScheduler; - - private static final LocalTime PREFERRED_NOTIFICATION_TIME = LocalTime.of(14, 0); - - private static final Logger log = LoggerFactory.getLogger(NotifyIdleDevicesWithoutMessagesPushNotificationExperiment.class); - - @VisibleForTesting - enum Population { - APNS_CONTROL, - APNS_EXPERIMENT, - FCM_CONTROL, - FCM_EXPERIMENT - } - - @VisibleForTesting - enum Outcome { - DELETED, - UNINSTALLED, - REACTIVATED, - UNCHANGED - } - - public NotifyIdleDevicesWithoutMessagesPushNotificationExperiment(final MessagesManager messagesManager, - final IdleDeviceNotificationScheduler idleDeviceNotificationScheduler) { - - this.messagesManager = messagesManager; - this.idleDeviceNotificationScheduler = idleDeviceNotificationScheduler; - } - - @Override - public String getExperimentName() { - return "notify-idle-devices-without-messages"; - } - - @Override - public CompletableFuture isDeviceEligible(final Account account, final Device device) { - - if (!hasPushToken(device)) { - return CompletableFuture.completedFuture(false); - } - - if (!idleDeviceNotificationScheduler.isIdle(device)) { - return CompletableFuture.completedFuture(false); - } - - return messagesManager.mayHavePersistedMessages(account.getIdentifier(IdentityType.ACI), device) - .thenApply(mayHavePersistedMessages -> !mayHavePersistedMessages); - } - - @Override - public Class getStateClass() { - return DeviceLastSeenState.class; - } - - @VisibleForTesting - static boolean hasPushToken(final Device device) { - // Exclude VOIP tokens since they have their own, distinct delivery mechanism - return !StringUtils.isAllBlank(device.getApnId(), device.getGcmId()) && StringUtils.isBlank(device.getVoipApnId()); - } - - @Override - public DeviceLastSeenState getState(@Nullable final Account account, @Nullable final Device device) { - if (account != null && device != null) { - final DeviceLastSeenState.PushTokenType pushTokenType = StringUtils.isNotBlank(device.getApnId()) - ? DeviceLastSeenState.PushTokenType.APNS - : DeviceLastSeenState.PushTokenType.FCM; - - return new DeviceLastSeenState(true, device.getCreated(), hasPushToken(device), device.getLastSeen(), pushTokenType); - } else { - return DeviceLastSeenState.MISSING_DEVICE_STATE; - } - } - - @Override - public CompletableFuture applyExperimentTreatment(final Account account, final Device device) { - return idleDeviceNotificationScheduler.scheduleNotification(account, device.getId(), PREFERRED_NOTIFICATION_TIME); - } - - @Override - public void analyzeResults(final Flux> samples) { - final Map> contingencyTable = new EnumMap<>(Population.class); - - for (final Population population : Population.values()) { - final Map countsByOutcome = new EnumMap<>(Outcome.class); - - for (final Outcome outcome : Outcome.values()) { - countsByOutcome.put(outcome, 0); - } - - contingencyTable.put(population, countsByOutcome); - } - - samples.doOnNext(sample -> contingencyTable.get(getPopulation(sample)).merge(getOutcome(sample), 1, Integer::sum)) - .then() - .block(); - - final StringBuilder reportBuilder = new StringBuilder("population,deleted,uninstalled,reactivated,unchanged\n"); - - for (final Population population : Population.values()) { - final Map countsByOutcome = contingencyTable.get(population); - - reportBuilder.append(population.name()); - reportBuilder.append(","); - reportBuilder.append(countsByOutcome.getOrDefault(Outcome.DELETED, 0)); - reportBuilder.append(","); - reportBuilder.append(countsByOutcome.getOrDefault(Outcome.UNINSTALLED, 0)); - reportBuilder.append(","); - reportBuilder.append(countsByOutcome.getOrDefault(Outcome.REACTIVATED, 0)); - reportBuilder.append(","); - reportBuilder.append(countsByOutcome.getOrDefault(Outcome.UNCHANGED, 0)); - reportBuilder.append("\n"); - } - - log.info(reportBuilder.toString()); - } - - @VisibleForTesting - static Population getPopulation(final PushNotificationExperimentSample sample) { - assert sample.initialState() != null && sample.initialState().pushTokenType() != null; - - return switch (sample.initialState().pushTokenType()) { - case APNS -> sample.inExperimentGroup() ? Population.APNS_EXPERIMENT : Population.APNS_CONTROL; - case FCM -> sample.inExperimentGroup() ? Population.FCM_EXPERIMENT : Population.FCM_CONTROL; - }; - } - - @VisibleForTesting - static Outcome getOutcome(final PushNotificationExperimentSample sample) { - final Outcome outcome; - - if (!sample.finalState().deviceExists() || sample.initialState().createdAtMillis() != sample.finalState().createdAtMillis()) { - outcome = Outcome.DELETED; - } else if (!sample.finalState().hasPushToken()) { - outcome = Outcome.UNINSTALLED; - } else if (sample.initialState().lastSeenMillis() != sample.finalState().lastSeenMillis()) { - outcome = Outcome.REACTIVATED; - } else { - outcome = Outcome.UNCHANGED; - } - - return outcome; - } -} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/workers/NotifyIdleDevicesWithoutMessagesExperimentFactory.java b/service/src/main/java/org/whispersystems/textsecuregcm/workers/NotifyIdleDevicesWithoutMessagesExperimentFactory.java deleted file mode 100644 index 64bdfd2d7..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/NotifyIdleDevicesWithoutMessagesExperimentFactory.java +++ /dev/null @@ -1,28 +0,0 @@ -package org.whispersystems.textsecuregcm.workers; - -import org.whispersystems.textsecuregcm.WhisperServerConfiguration; -import org.whispersystems.textsecuregcm.configuration.DynamoDbTables; -import org.whispersystems.textsecuregcm.experiment.DeviceLastSeenState; -import org.whispersystems.textsecuregcm.experiment.NotifyIdleDevicesWithoutMessagesPushNotificationExperiment; -import org.whispersystems.textsecuregcm.experiment.PushNotificationExperiment; -import org.whispersystems.textsecuregcm.push.IdleDeviceNotificationScheduler; -import java.time.Clock; - -public class NotifyIdleDevicesWithoutMessagesExperimentFactory implements PushNotificationExperimentFactory { - - @Override - public PushNotificationExperiment buildExperiment(final CommandDependencies commandDependencies, - final WhisperServerConfiguration configuration) { - - final DynamoDbTables.TableWithExpiration tableConfiguration = configuration.getDynamoDbTables().getScheduledJobs(); - - return new NotifyIdleDevicesWithoutMessagesPushNotificationExperiment(commandDependencies.messagesManager(), - new IdleDeviceNotificationScheduler( - commandDependencies.accountsManager(), - commandDependencies.pushNotificationManager(), - commandDependencies.dynamoDbAsyncClient(), - tableConfiguration.getTableName(), - tableConfiguration.getExpiration(), - Clock.systemUTC())); - } -} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/experiment/NotifyIdleDevicesWithoutMessagesPushNotificationExperimentTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/experiment/NotifyIdleDevicesWithoutMessagesPushNotificationExperimentTest.java deleted file mode 100644 index 67641a37d..000000000 --- a/service/src/test/java/org/whispersystems/textsecuregcm/experiment/NotifyIdleDevicesWithoutMessagesPushNotificationExperimentTest.java +++ /dev/null @@ -1,280 +0,0 @@ -package org.whispersystems.textsecuregcm.experiment; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyByte; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -import com.google.i18n.phonenumbers.PhoneNumberUtil; -import java.time.Duration; -import java.time.Instant; -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.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; - -class NotifyIdleDevicesWithoutMessagesPushNotificationExperimentTest { - - private MessagesManager messagesManager; - private IdleDeviceNotificationScheduler idleDeviceNotificationScheduler; - - private NotifyIdleDevicesWithoutMessagesPushNotificationExperiment experiment; - - private static final Instant CURRENT_TIME = Instant.now(); - - @BeforeEach - void setUp() { - messagesManager = mock(MessagesManager.class); - - idleDeviceNotificationScheduler = mock(IdleDeviceNotificationScheduler.class); - when(idleDeviceNotificationScheduler.scheduleNotification(any(), anyByte(), any())) - .thenReturn(CompletableFuture.completedFuture(null)); - - experiment = new NotifyIdleDevicesWithoutMessagesPushNotificationExperiment(messagesManager, - idleDeviceNotificationScheduler); - } - - @ParameterizedTest - @MethodSource - void isDeviceEligible(final Account account, - final Device device, - final boolean isDeviceIdle, - final boolean mayHaveMessages, - final boolean expectEligible) { - - when(messagesManager.mayHavePersistedMessages(account.getIdentifier(IdentityType.ACI), device)) - .thenReturn(CompletableFuture.completedFuture(mayHaveMessages)); - - when(idleDeviceNotificationScheduler.isIdle(device)).thenReturn(isDeviceIdle); - - assertEquals(expectEligible, experiment.isDeviceEligible(account, device).join()); - } - - private static List isDeviceEligible() { - final List arguments = new ArrayList<>(); - - final Account account = mock(Account.class); - when(account.getIdentifier(IdentityType.ACI)).thenReturn(UUID.randomUUID()); - when(account.getNumber()).thenReturn(PhoneNumberUtil.getInstance().format( - PhoneNumberUtil.getInstance().getExampleNumber("US"), PhoneNumberUtil.PhoneNumberFormat.E164)); - - { - // Idle device with push token and messages - final Device device = mock(Device.class); - when(device.getApnId()).thenReturn("apns-token"); - - arguments.add(Arguments.of(account, device, true, true, false)); - } - - { - // Idle device missing push token, but with messages - arguments.add(Arguments.of(account, mock(Device.class), true, true, false)); - } - - { - // Idle device missing push token and messages - arguments.add(Arguments.of(account, mock(Device.class), true, false, false)); - } - - { - // Idle device with push token, but no messages - final Device device = mock(Device.class); - when(device.getApnId()).thenReturn("apns-token"); - - arguments.add(Arguments.of(account, device, true, false, true)); - } - - { - // Active device with push token and messages - final Device device = mock(Device.class); - when(device.getApnId()).thenReturn("apns-token"); - - arguments.add(Arguments.of(account, device, false, true, false)); - } - - { - // Active device missing push token, but with messages - arguments.add(Arguments.of(account, mock(Device.class), false, true, false)); - } - - { - // Active device missing push token and messages - arguments.add(Arguments.of(account, mock(Device.class), false, false, false)); - } - - { - // Active device with push token, but no messages - final Device device = mock(Device.class); - when(device.getApnId()).thenReturn("apns-token"); - - arguments.add(Arguments.of(account, device, false, false, false)); - } - - return arguments; - } - - @ParameterizedTest - @MethodSource - void hasPushToken(final Device device, final boolean expectHasPushToken) { - assertEquals(expectHasPushToken, NotifyIdleDevicesWithoutMessagesPushNotificationExperiment.hasPushToken(device)); - } - - private static List hasPushToken() { - final List arguments = new ArrayList<>(); - - { - // No token at all - final Device device = mock(Device.class); - - arguments.add(Arguments.of(device, false)); - } - - { - // FCM token - final Device device = mock(Device.class); - when(device.getGcmId()).thenReturn("fcm-token"); - - arguments.add(Arguments.of(device, true)); - } - - { - // APNs token - final Device device = mock(Device.class); - when(device.getApnId()).thenReturn("apns-token"); - - arguments.add(Arguments.of(device, true)); - } - - { - // APNs VOIP token - final Device device = mock(Device.class); - when(device.getApnId()).thenReturn("apns-token"); - when(device.getVoipApnId()).thenReturn("apns-voip-token"); - - arguments.add(Arguments.of(device, false)); - } - - return arguments; - } - - @Test - void getState() { - assertEquals(DeviceLastSeenState.MISSING_DEVICE_STATE, experiment.getState(null, null)); - assertEquals(DeviceLastSeenState.MISSING_DEVICE_STATE, experiment.getState(mock(Account.class), null)); - - final long createdAtMillis = CURRENT_TIME.minus(Duration.ofDays(14)).toEpochMilli(); - - { - final Device apnsDevice = mock(Device.class); - when(apnsDevice.getApnId()).thenReturn("apns-token"); - when(apnsDevice.getCreated()).thenReturn(createdAtMillis); - when(apnsDevice.getLastSeen()).thenReturn(CURRENT_TIME.toEpochMilli()); - - assertEquals( - new DeviceLastSeenState(true, createdAtMillis, true, CURRENT_TIME.toEpochMilli(), DeviceLastSeenState.PushTokenType.APNS), - experiment.getState(mock(Account.class), apnsDevice)); - } - - { - final Device fcmDevice = mock(Device.class); - when(fcmDevice.getGcmId()).thenReturn("fcm-token"); - when(fcmDevice.getCreated()).thenReturn(createdAtMillis); - when(fcmDevice.getLastSeen()).thenReturn(CURRENT_TIME.toEpochMilli()); - - assertEquals( - new DeviceLastSeenState(true, createdAtMillis, true, CURRENT_TIME.toEpochMilli(), DeviceLastSeenState.PushTokenType.FCM), - experiment.getState(mock(Account.class), fcmDevice)); - } - } - - @ParameterizedTest - @MethodSource - void getPopulation(final boolean inExperimentGroup, - final DeviceLastSeenState.PushTokenType tokenType, - final NotifyIdleDevicesWithoutMessagesPushNotificationExperiment.Population expectedPopulation) { - - final DeviceLastSeenState state = new DeviceLastSeenState(true, 0, true, 0, tokenType); - final PushNotificationExperimentSample sample = - new PushNotificationExperimentSample<>(UUID.randomUUID(), Device.PRIMARY_ID, inExperimentGroup, state, state); - - assertEquals(expectedPopulation, NotifyIdleDevicesWithoutMessagesPushNotificationExperiment.getPopulation(sample)); - } - - private static List getPopulation() { - return List.of( - Arguments.of(true, DeviceLastSeenState.PushTokenType.APNS, - NotifyIdleDevicesWithoutMessagesPushNotificationExperiment.Population.APNS_EXPERIMENT), - - Arguments.of(false, DeviceLastSeenState.PushTokenType.APNS, - NotifyIdleDevicesWithoutMessagesPushNotificationExperiment.Population.APNS_CONTROL), - - Arguments.of(true, DeviceLastSeenState.PushTokenType.FCM, - NotifyIdleDevicesWithoutMessagesPushNotificationExperiment.Population.FCM_EXPERIMENT), - - Arguments.of(false, DeviceLastSeenState.PushTokenType.FCM, - NotifyIdleDevicesWithoutMessagesPushNotificationExperiment.Population.FCM_CONTROL) - ); - } - - @ParameterizedTest - @MethodSource - void getOutcome(final DeviceLastSeenState initialState, - final DeviceLastSeenState finalState, - final NotifyIdleDevicesWithoutMessagesPushNotificationExperiment.Outcome expectedOutcome) { - - final PushNotificationExperimentSample sample = - new PushNotificationExperimentSample<>(UUID.randomUUID(), Device.PRIMARY_ID, true, initialState, finalState); - - assertEquals(expectedOutcome, NotifyIdleDevicesWithoutMessagesPushNotificationExperiment.getOutcome(sample)); - } - - private static List getOutcome() { - return List.of( - // Device no longer exists - Arguments.of( - new DeviceLastSeenState(true, 0, true, 0, DeviceLastSeenState.PushTokenType.APNS), - DeviceLastSeenState.MISSING_DEVICE_STATE, - NotifyIdleDevicesWithoutMessagesPushNotificationExperiment.Outcome.DELETED - ), - - // Device re-registered (i.e. "created" timestamp changed) - Arguments.of( - new DeviceLastSeenState(true, 0, true, 0, DeviceLastSeenState.PushTokenType.APNS), - new DeviceLastSeenState(true, 1, true, 1, DeviceLastSeenState.PushTokenType.APNS), - NotifyIdleDevicesWithoutMessagesPushNotificationExperiment.Outcome.DELETED - ), - - // Device has lost push tokens - Arguments.of( - new DeviceLastSeenState(true, 0, true, 0, DeviceLastSeenState.PushTokenType.APNS), - new DeviceLastSeenState(true, 0, false, 0, DeviceLastSeenState.PushTokenType.APNS), - NotifyIdleDevicesWithoutMessagesPushNotificationExperiment.Outcome.UNINSTALLED - ), - - // Device reactivated - Arguments.of( - new DeviceLastSeenState(true, 0, true, 0, DeviceLastSeenState.PushTokenType.APNS), - new DeviceLastSeenState(true, 0, true, 1, DeviceLastSeenState.PushTokenType.APNS), - NotifyIdleDevicesWithoutMessagesPushNotificationExperiment.Outcome.REACTIVATED - ), - - // No change - Arguments.of( - new DeviceLastSeenState(true, 0, true, 0, DeviceLastSeenState.PushTokenType.APNS), - new DeviceLastSeenState(true, 0, true, 0, DeviceLastSeenState.PushTokenType.APNS), - NotifyIdleDevicesWithoutMessagesPushNotificationExperiment.Outcome.UNCHANGED - ) - ); - } -}