From c2be0af9d9a9db9f2912036d5c8613d322e2e943 Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Mon, 8 Aug 2022 12:45:10 -0400 Subject: [PATCH] Refactor `ApnPushNotificationSchedulerTest` to use a `Clock` --- .../push/ApnPushNotificationScheduler.java | 30 +++++++++++-------- .../ApnPushNotificationSchedulerTest.java | 15 ++++++++-- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/push/ApnPushNotificationScheduler.java b/service/src/main/java/org/whispersystems/textsecuregcm/push/ApnPushNotificationScheduler.java index b84bdcf30..e4f686bc1 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/push/ApnPushNotificationScheduler.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/push/ApnPushNotificationScheduler.java @@ -12,6 +12,7 @@ import io.lettuce.core.cluster.SlotHash; import io.micrometer.core.instrument.Counter; import io.micrometer.core.instrument.Metrics; import java.io.IOException; +import java.time.Clock; import java.util.Collections; import java.util.List; import java.util.Optional; @@ -48,6 +49,7 @@ public class ApnPushNotificationScheduler implements Managed { private final APNSender apnSender; private final AccountsManager accountsManager; private final FaultTolerantRedisCluster pushSchedulingCluster; + private final Clock clock; private final ClusterLuaScript getPendingVoipDestinationsScript; private final ClusterLuaScript insertPendingVoipDestinationScript; @@ -109,13 +111,22 @@ public class ApnPushNotificationScheduler implements Managed { } public ApnPushNotificationScheduler(FaultTolerantRedisCluster pushSchedulingCluster, - APNSender apnSender, - AccountsManager accountsManager) - throws IOException - { + APNSender apnSender, + AccountsManager accountsManager) throws IOException { + + this(pushSchedulingCluster, apnSender, accountsManager, Clock.systemUTC()); + } + + @VisibleForTesting + ApnPushNotificationScheduler(FaultTolerantRedisCluster pushSchedulingCluster, + APNSender apnSender, + AccountsManager accountsManager, + Clock clock) throws IOException { + this.apnSender = apnSender; this.accountsManager = accountsManager; this.pushSchedulingCluster = pushSchedulingCluster; + this.clock = clock; this.getPendingVoipDestinationsScript = ClusterLuaScript.fromResource(pushSchedulingCluster, "lua/apn/get.lua", ScriptOutputType.MULTI); this.insertPendingVoipDestinationScript = ClusterLuaScript.fromResource(pushSchedulingCluster, "lua/apn/insert.lua", ScriptOutputType.VALUE); @@ -127,13 +138,8 @@ public class ApnPushNotificationScheduler implements Managed { } public void scheduleRecurringVoipNotification(Account account, Device device) { - scheduleRecurringVoipNotification(account, device, System.currentTimeMillis()); - } - - @VisibleForTesting - void scheduleRecurringVoipNotification(Account account, Device device, long timestamp) { sent.increment(); - insertRecurringVoipNotificationEntry(account, device, timestamp + (15 * 1000), (15 * 1000)); + insertRecurringVoipNotificationEntry(account, device, clock.millis() + (15 * 1000), (15 * 1000)); } public void cancelRecurringVoipNotification(Account account, Device device) { @@ -170,7 +176,7 @@ public class ApnPushNotificationScheduler implements Managed { long deviceLastSeen = device.getLastSeen(); - if (deviceLastSeen < System.currentTimeMillis() - TimeUnit.DAYS.toMillis(7)) { + if (deviceLastSeen < clock.millis() - TimeUnit.DAYS.toMillis(7)) { evicted.increment(); removeRecurringVoipNotificationEntry(account, device); return; @@ -214,7 +220,7 @@ public class ApnPushNotificationScheduler implements Managed { List getPendingDestinationsForRecurringVoipNotifications(final int slot, final int limit) { return (List) getPendingVoipDestinationsScript.execute( List.of(getPendingRecurringVoipNotificationQueueKey(slot)), - List.of(String.valueOf(System.currentTimeMillis()), String.valueOf(limit))); + List.of(String.valueOf(clock.millis()), String.valueOf(limit))); } private void insertRecurringVoipNotificationEntry(final Account account, final Device device, final long timestamp, final long interval) { diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/push/ApnPushNotificationSchedulerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/push/ApnPushNotificationSchedulerTest.java index c79fb3c08..d7e0c04e8 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/push/ApnPushNotificationSchedulerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/push/ApnPushNotificationSchedulerTest.java @@ -12,6 +12,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import io.lettuce.core.cluster.SlotHash; +import java.time.Clock; import java.util.List; import java.util.Optional; import java.util.UUID; @@ -35,6 +36,7 @@ class ApnPushNotificationSchedulerTest { private Device device; private APNSender apnSender; + private Clock clock; private ApnPushNotificationScheduler apnPushNotificationScheduler; @@ -61,19 +63,23 @@ class ApnPushNotificationSchedulerTest { when(accountsManager.getByAccountIdentifier(ACCOUNT_UUID)).thenReturn(Optional.of(account)); apnSender = mock(APNSender.class); + clock = mock(Clock.class); - apnPushNotificationScheduler = new ApnPushNotificationScheduler(REDIS_CLUSTER_EXTENSION.getRedisCluster(), apnSender, accountsManager); + apnPushNotificationScheduler = new ApnPushNotificationScheduler(REDIS_CLUSTER_EXTENSION.getRedisCluster(), apnSender, accountsManager, clock); } @Test void testClusterInsert() { final String endpoint = apnPushNotificationScheduler.getEndpointKey(account, device); + final long currentTimeMillis = System.currentTimeMillis(); assertTrue( apnPushNotificationScheduler.getPendingDestinationsForRecurringVoipNotifications(SlotHash.getSlot(endpoint), 1).isEmpty()); - apnPushNotificationScheduler.scheduleRecurringVoipNotification(account, device, System.currentTimeMillis() - 30_000); + when(clock.millis()).thenReturn(currentTimeMillis - 30_000); + apnPushNotificationScheduler.scheduleRecurringVoipNotification(account, device); + when(clock.millis()).thenReturn(currentTimeMillis); final List pendingDestinations = apnPushNotificationScheduler.getPendingDestinationsForRecurringVoipNotifications(SlotHash.getSlot(endpoint), 2); assertEquals(1, pendingDestinations.size()); @@ -91,12 +97,15 @@ class ApnPushNotificationSchedulerTest { @Test void testProcessNextSlot() { final ApnPushNotificationScheduler.NotificationWorker worker = apnPushNotificationScheduler.new NotificationWorker(); + final long currentTimeMillis = System.currentTimeMillis(); - apnPushNotificationScheduler.scheduleRecurringVoipNotification(account, device, System.currentTimeMillis() - 30_000); + when(clock.millis()).thenReturn(currentTimeMillis - 30_000); + apnPushNotificationScheduler.scheduleRecurringVoipNotification(account, device); final int slot = SlotHash.getSlot(apnPushNotificationScheduler.getEndpointKey(account, device)); final int previousSlot = (slot + SlotHash.SLOT_COUNT - 1) % SlotHash.SLOT_COUNT; + when(clock.millis()).thenReturn(currentTimeMillis); REDIS_CLUSTER_EXTENSION.getRedisCluster().withCluster(connection -> connection.sync() .set(ApnPushNotificationScheduler.NEXT_SLOT_TO_PERSIST_KEY, String.valueOf(previousSlot)));