From aec0a9951a93aa7b6524f8d18dcb4ce0c6759e87 Mon Sep 17 00:00:00 2001 From: Nicholas Tinsley Date: Mon, 1 Jul 2024 17:01:02 -0400 Subject: [PATCH] Prevent backups from being scheduled twice within the jitter window. Fixes #13559. --- .../securesms/service/LocalBackupListener.java | 2 +- .../securesms/service/MessageBackupListener.kt | 18 ++++++++++-------- .../securesms/service/BackListenerTest.kt | 14 ++++++++++++++ .../securesms/testutil/MockRandom.kt | 18 ++++++++++++++++++ 4 files changed, 43 insertions(+), 9 deletions(-) create mode 100644 app/src/test/java/org/thoughtcrime/securesms/testutil/MockRandom.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/service/LocalBackupListener.java b/app/src/main/java/org/thoughtcrime/securesms/service/LocalBackupListener.java index 0b6eebfc9c..f7888530fb 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/service/LocalBackupListener.java +++ b/app/src/main/java/org/thoughtcrime/securesms/service/LocalBackupListener.java @@ -47,7 +47,7 @@ public class LocalBackupListener extends PersistentAlarmManagerListener { LocalDateTime now = LocalDateTime.now(); int hour = SignalStore.settings().getBackupHour(); int minute = SignalStore.settings().getBackupMinute(); - LocalDateTime next = MessageBackupListener.getNextDailyBackupTimeFromNowWithJitter(now, hour, minute, BACKUP_JITTER_WINDOW_SECONDS); + LocalDateTime next = MessageBackupListener.getNextDailyBackupTimeFromNowWithJitter(now, hour, minute, BACKUP_JITTER_WINDOW_SECONDS, new Random()); long nextTime = JavaTimeExtensionsKt.toMillis(next); diff --git a/app/src/main/java/org/thoughtcrime/securesms/service/MessageBackupListener.kt b/app/src/main/java/org/thoughtcrime/securesms/service/MessageBackupListener.kt index 51f6c2a902..534357a0fa 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/service/MessageBackupListener.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/service/MessageBackupListener.kt @@ -13,7 +13,8 @@ import org.thoughtcrime.securesms.util.RemoteConfig import org.thoughtcrime.securesms.util.toMillis import java.time.LocalDateTime import java.util.Random -import java.util.concurrent.TimeUnit +import kotlin.time.Duration.Companion.days +import kotlin.time.Duration.Companion.minutes class MessageBackupListener : PersistentAlarmManagerListener() { override fun shouldScheduleExact(): Boolean { @@ -33,8 +34,8 @@ class MessageBackupListener : PersistentAlarmManagerListener() { } companion object { - private val BACKUP_JITTER_WINDOW_SECONDS = Math.toIntExact(TimeUnit.MINUTES.toSeconds(10)) - private val BACKUP_MEDIA_SYNC_INTERVAL = TimeUnit.DAYS.toMillis(7) + private val BACKUP_JITTER_WINDOW_SECONDS = 10.minutes.inWholeSeconds.toInt() + private val BACKUP_MEDIA_SYNC_INTERVAL = 7.days.inWholeMilliseconds @JvmStatic fun schedule(context: Context?) { @@ -44,22 +45,23 @@ class MessageBackupListener : PersistentAlarmManagerListener() { } @JvmStatic - fun getNextDailyBackupTimeFromNowWithJitter(now: LocalDateTime, hour: Int, minute: Int, maxJitterSeconds: Int): LocalDateTime { + fun getNextDailyBackupTimeFromNowWithJitter(now: LocalDateTime, hour: Int, minute: Int, maxJitterSeconds: Int, randomSource: Random = Random()): LocalDateTime { var next = now.withHour(hour).withMinute(minute).withSecond(0) - if (!now.plusSeconds(maxJitterSeconds.toLong() / 2).isBefore(next)) { + val endOfJitterWindowForNow = now.plusSeconds(maxJitterSeconds.toLong() / 2) + while (!endOfJitterWindowForNow.isBefore(next)) { next = next.plusDays(1) } - val jitter = Random().nextInt(BACKUP_JITTER_WINDOW_SECONDS) - BACKUP_JITTER_WINDOW_SECONDS / 2 + val jitter = randomSource.nextInt(maxJitterSeconds) - maxJitterSeconds / 2 return next.plusSeconds(jitter.toLong()) } - fun setNextBackupTimeToIntervalFromNow(): Long { + fun setNextBackupTimeToIntervalFromNow(maxJitterSeconds: Int = BACKUP_JITTER_WINDOW_SECONDS): Long { val now = LocalDateTime.now() val hour = SignalStore.settings.backupHour val minute = SignalStore.settings.backupMinute - var next = getNextDailyBackupTimeFromNowWithJitter(now, hour, minute, BACKUP_JITTER_WINDOW_SECONDS) + var next = getNextDailyBackupTimeFromNowWithJitter(now, hour, minute, maxJitterSeconds) next = when (SignalStore.backup.backupFrequency) { BackupFrequency.MANUAL -> next.plusDays(364) BackupFrequency.MONTHLY -> next.plusDays(29) diff --git a/app/src/test/java/org/thoughtcrime/securesms/service/BackListenerTest.kt b/app/src/test/java/org/thoughtcrime/securesms/service/BackListenerTest.kt index b4e0cf8399..2f2400a3a7 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/service/BackListenerTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/service/BackListenerTest.kt @@ -8,8 +8,12 @@ package org.thoughtcrime.securesms.service import org.junit.Assert import org.junit.Test import org.thoughtcrime.securesms.BaseUnitTest +import org.thoughtcrime.securesms.testutil.MockRandom +import java.time.Duration import java.time.LocalDateTime import java.util.concurrent.TimeUnit +import kotlin.time.Duration.Companion.days +import kotlin.time.Duration.Companion.minutes class BackListenerTest : BaseUnitTest() { @@ -44,4 +48,14 @@ class BackListenerTest : BaseUnitTest() { val next = MessageBackupListener.getNextDailyBackupTimeFromNowWithJitter(now, 3, 0, jitterWindowSeconds) Assert.assertEquals(8, next.dayOfMonth) } + + @Test + fun testBackupJitterWhenScheduledForMidnightButJitterMakesItRunJustBefore() { + val mockRandom = MockRandom(listOf(1.minutes.inWholeSeconds.toInt())) + val jitterWindowSeconds = 10.minutes.inWholeSeconds.toInt() + val now: LocalDateTime = LocalDateTime.of(2024, 6, 27, 23, 57, 0) + val next: LocalDateTime = MessageBackupListener.getNextDailyBackupTimeFromNowWithJitter(now, 0, 0, jitterWindowSeconds, mockRandom) + + Assert.assertTrue(Duration.between(now, next).toSeconds() > (1.days.inWholeSeconds - jitterWindowSeconds)) + } } diff --git a/app/src/test/java/org/thoughtcrime/securesms/testutil/MockRandom.kt b/app/src/test/java/org/thoughtcrime/securesms/testutil/MockRandom.kt new file mode 100644 index 0000000000..798fea3d40 --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/testutil/MockRandom.kt @@ -0,0 +1,18 @@ +/* + * Copyright 2024 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.thoughtcrime.securesms.testutil + +import java.util.LinkedList +import java.util.Random + +class MockRandom(initialInts: List) : Random() { + + val nextInts = LinkedList(initialInts) + + override fun nextInt(): Int { + return nextInts.remove() + } +}