diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index b272d8e9..175dab02 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -304,6 +304,11 @@ wipe pending alarms, and a clock or timezone change moves every reminder relative to the one that is armed. Exported: the system broadcasts arrive from outside the app. --> + + diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/backup/AutoBackup.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/backup/AutoBackup.kt index e3d82cbd..8b1e9d93 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/backup/AutoBackup.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/backup/AutoBackup.kt @@ -7,7 +7,6 @@ import android.app.PendingIntent import android.content.Context import android.net.Uri import android.os.Build -import android.provider.CalendarContract import android.util.Log import androidx.core.app.NotificationCompat import androidx.core.app.NotificationManagerCompat @@ -66,14 +65,8 @@ object BackupScheduler { private const val WORK_NAME = "auto-backup" private const val WORK_NAME_NOW = "auto-backup-now" - private val CHANGE_SLOTS = listOf("auto-backup-changes-a", "auto-backup-changes-b") - internal const val KEY_CHANGE_SLOT = "change_slot" - - /** Quiet time after the last provider change before a change run starts, so a burst makes one run. */ - private const val CHANGE_SETTLE_MINUTES = 2L - - /** Longest a change run waits on a provider that never goes quiet (a long sync). */ - private const val CHANGE_MAX_WAIT_MINUTES = 15L + private const val WORK_NAME_CHANGE = "auto-backup-change" + internal const val KEY_CHANGE_RUN = "change_run" private val reconcileLock = Mutex() @@ -84,67 +77,70 @@ object BackupScheduler { * * The first run is delayed by one interval so it doesn't follow an immediate * [runNow] with a near-identical copy. With backing up after changes on - * (#516), the change trigger is armed unless a slot is already waiting. + * (#516), the change watch ([BackupChangeJob]) is armed unless it already + * waits; it doesn't survive a reboot, and this runs on every process start. * Serialised and not cancellable, so quick toggles settle on the last one. */ suspend fun reconcile(context: Context, prefs: SettingsPrefs) = withContext(NonCancellable) { reconcileLock.withLock { - val workManager = WorkManager.getInstance(context) - suspend fun cancelWatch() = CHANGE_SLOTS.indices - .filter { slot -> slotStates(workManager, slot).any { !it.isFinished } } - .forEach { workManager.cancelUniqueWork(CHANGE_SLOTS[it]).await() } - if (!prefs.autoBackupEnabled.first() || prefs.autoBackupFolderUri.first() == null) { - workManager.cancelUniqueWork(WORK_NAME) - // Also cancel any pending immediate run — otherwise a failing run-now - // keeps retrying after the user has turned backup off. - workManager.cancelUniqueWork(WORK_NAME_NOW) - cancelWatch() - return@withLock + try { + apply(context, prefs) + } catch (e: Exception) { + Log.w(TAG, "Scheduling the automatic backup failed", e) } - // WorkManager's own floor is 15 min; the UI floors the user choice at 30. - val interval = prefs.autoBackupIntervalMinutes.first().coerceAtLeast(15L) - val request = PeriodicWorkRequestBuilder(interval, TimeUnit.MINUTES) - .setInitialDelay(interval, TimeUnit.MINUTES) - .build() - workManager.enqueueUniquePeriodicWork( - WORK_NAME, - ExistingPeriodicWorkPolicy.UPDATE, - request, - ) - if (!prefs.autoBackupOnChange.first()) { - cancelWatch() - return@withLock - } - takeWatchStep(context, workManager, self = null) } } + private suspend fun apply(context: Context, prefs: SettingsPrefs) { + val workManager = WorkManager.getInstance(context) + fun stopWatching() { + BackupChangeJob.cancel(context) + workManager.cancelUniqueWork(WORK_NAME_CHANGE) + } + if (!prefs.autoBackupEnabled.first() || prefs.autoBackupFolderUri.first() == null) { + workManager.cancelUniqueWork(WORK_NAME) + // Also cancel any pending immediate run — otherwise a failing run-now + // keeps retrying after the user has turned backup off. + workManager.cancelUniqueWork(WORK_NAME_NOW) + stopWatching() + return + } + // WorkManager's own floor is 15 min; the UI floors the user choice at 30. + val interval = prefs.autoBackupIntervalMinutes.first().coerceAtLeast(15L) + val request = PeriodicWorkRequestBuilder(interval, TimeUnit.MINUTES) + .setInitialDelay(interval, TimeUnit.MINUTES) + .build() + workManager.enqueueUniquePeriodicWork( + WORK_NAME, + ExistingPeriodicWorkPolicy.UPDATE, + request, + ) + if (prefs.autoBackupOnChange.first()) BackupChangeJob.scheduleIfIdle(context) else stopWatching() + } + /** - * Keep one change watch armed around the change run in [self] (see - * [watchStep]), under the same lock as [reconcile] so it can't outlast a - * switch-off. + * The watch fired: queue a change run and arm the next watch, unless the + * option went off meanwhile. Under [reconcileLock], so it can't re-arm + * after a switch-off. */ - internal suspend fun watchNext(context: Context, prefs: SettingsPrefs, self: Int) = - withContext(NonCancellable) { - reconcileLock.withLock { - if (watchWanted(prefs)) takeWatchStep(context, WorkManager.getInstance(context), self) + internal suspend fun changed(context: Context, prefs: SettingsPrefs) = withContext(NonCancellable) { + reconcileLock.withLock { + val wanted = prefs.autoBackupEnabled.first() && prefs.autoBackupFolderUri.first() != null && + prefs.autoBackupOnChange.first() + if (!wanted) return@withLock + val workManager = WorkManager.getInstance(context) + val states = workManager.getWorkInfosForUniqueWorkFlow(WORK_NAME_CHANGE).first().map { it.state } + changeRunPolicy(states)?.let { policy -> + val run = OneTimeWorkRequestBuilder() + .setConstraints(Constraints.Builder().setRequiresBatteryNotLow(true).build()) + .setInputData(workDataOf(KEY_CHANGE_RUN to true)) + .build() + workManager.enqueueUniqueWork(WORK_NAME_CHANGE, policy, run).await() } - } - - private suspend fun takeWatchStep(context: Context, workManager: WorkManager, self: Int?) { - when (val step = watchStep(self, CHANGE_SLOTS.indices.map { slotStates(workManager, it) })) { - WatchStep.None -> Unit - is WatchStep.Cancel -> workManager.cancelUniqueWork(CHANGE_SLOTS[step.slot]).await() - is WatchStep.Arm -> watchChanges(context, step.slot, step.append) + BackupChangeJob.schedule(context) } } - private suspend fun slotStates(workManager: WorkManager, slot: Int): List = - workManager.getWorkInfosForUniqueWorkFlow(CHANGE_SLOTS[slot]).first().map { it.state } - - private suspend fun watchWanted(prefs: SettingsPrefs): Boolean = - prefs.autoBackupEnabled.first() && prefs.autoBackupFolderUri.first() != null && prefs.autoBackupOnChange.first() - /** * Run one export immediately (e.g. right after enabling or changing the * folder) for instant feedback. Unique + REPLACE so rapid taps coalesce into @@ -158,58 +154,18 @@ object BackupScheduler { ) } - /** - * Arm a one-off run on the next change in the calendar provider (#516) in - * one of two [slot]s. The provider reports every change on its root URI, - * whoever made it (this app, a sync adapter or another app), so that is the - * URI watched. A content trigger fires once and observes only while its - * job waits, so each change run arms the other slot ([watchNext]); [append] - * queues the request behind that slot's running run. - */ - private fun watchChanges(context: Context, slot: Int, append: Boolean) { - val constraints = Constraints.Builder() - .addContentUriTrigger(CalendarContract.CONTENT_URI, true) - .setTriggerContentUpdateDelay(CHANGE_SETTLE_MINUTES, TimeUnit.MINUTES) - .setTriggerContentMaxDelay(CHANGE_MAX_WAIT_MINUTES, TimeUnit.MINUTES) - .setRequiresBatteryNotLow(true) - .build() - val request = OneTimeWorkRequestBuilder() - .setConstraints(constraints) - .setInputData(workDataOf(KEY_CHANGE_SLOT to slot)) - .build() - val policy = if (append) ExistingWorkPolicy.APPEND_OR_REPLACE else ExistingWorkPolicy.KEEP - WorkManager.getInstance(context).enqueueUniqueWork(CHANGE_SLOTS[slot], policy, request) - } -} - -/** What keeps exactly one change watch armed (#516): nothing, a new watch in [slot], or cancelling the extra one there. */ -internal sealed interface WatchStep { - data object None : WatchStep - - /** [append]: queued behind the run still going in [slot], so it starts watching once that ends. */ - data class Arm(val slot: Int, val append: Boolean) : WatchStep - - data class Cancel(val slot: Int) : WatchStep + private const val TAG = "BackupScheduler" } /** - * The step that leaves exactly one watch waiting, from both slots' work - * [states], taken by the change run in [self] as it starts and ends, or by - * [BackupScheduler.reconcile] (null). A waiting watch is enough; two are one - * too many. With none, the other slot is armed, queued behind its run if one - * still shows as running, so a run whose end isn't recorded yet can't leave - * the provider unwatched. + * How a fired change watch queues its run (#516), from the change runs' work + * [states]: a run that hasn't started yet will see the change anyway (null); + * one already going may have read too early, so the new one queues behind it. */ -internal fun watchStep(self: Int?, states: List>): WatchStep { - fun waiting(slot: Int) = states[slot].any { it == WorkInfo.State.ENQUEUED || it == WorkInfo.State.BLOCKED } - val keep = self ?: 0 - val other = 1 - keep - val target = if (self == null) keep else other - return when { - waiting(keep) && waiting(other) -> WatchStep.Cancel(other) - waiting(keep) || waiting(other) -> WatchStep.None - else -> WatchStep.Arm(target, append = WorkInfo.State.RUNNING in states[target]) - } +internal fun changeRunPolicy(states: List): ExistingWorkPolicy? = when { + states.any { it == WorkInfo.State.ENQUEUED || it == WorkInfo.State.BLOCKED } -> null + WorkInfo.State.RUNNING in states -> ExistingWorkPolicy.APPEND_OR_REPLACE + else -> ExistingWorkPolicy.REPLACE } /** @@ -235,24 +191,10 @@ class BackupWorker( override suspend fun doWork(): Result { val deps = EntryPointAccessors.fromApplication(applicationContext, Deps::class.java) - val prefs = deps.settingsPrefs() - val slot = inputData.getInt(BackupScheduler.KEY_CHANGE_SLOT, -1) - if (slot < 0) return backUp(deps, prefs, changeRun = false) - suspend fun watchNext() = runCatching { - BackupScheduler.watchNext(applicationContext, prefs, slot) - }.onFailure { Log.w(TAG, "Arming the next change run failed", it) } - watchNext() - // Never fails or retries: a retry would only wait for another change. - try { - backUp(deps, prefs, changeRun = true) - } catch (e: CancellationException) { - throw e - } catch (e: Exception) { - Log.w(TAG, "Backup after a change failed", e) - } finally { - watchNext() - } - return Result.success() + val changeRun = inputData.getBoolean(BackupScheduler.KEY_CHANGE_RUN, false) + val result = backUp(deps, deps.settingsPrefs(), changeRun) + // A failed change run would fail the one queued behind it too. + return if (changeRun) Result.success() else result } /** diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/backup/BackupChangeJob.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/backup/BackupChangeJob.kt new file mode 100644 index 00000000..a2dbe12b --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/backup/BackupChangeJob.kt @@ -0,0 +1,81 @@ +package de.jeanlucmakiola.calendula.data.backup + +import android.app.job.JobInfo +import android.app.job.JobParameters +import android.app.job.JobScheduler +import android.app.job.JobService +import android.content.ComponentName +import android.content.Context +import android.provider.CalendarContract +import android.util.Log +import dagger.hilt.android.EntryPointAccessors +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel +import kotlinx.coroutines.launch +import java.util.concurrent.TimeUnit + +/** + * The watch behind "Also back up after changes" (#516): a JobScheduler job + * with a content trigger on the provider's root URI, where it reports every + * change, whoever made it (this app, a sync adapter or another app). It waits + * for [SETTLE_MINUTES] of quiet, at most [MAX_WAIT_MINUTES], so a burst makes + * one run. A trigger fires once; each firing queues a change run and schedules + * the next watch under the same id before finishing, which hands over the + * changes seen meanwhile, so exactly one watch exists and none are lost. + */ +class BackupChangeJob : JobService() { + + private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Default) + + override fun onStartJob(params: JobParameters): Boolean { + scope.launch { + try { + val deps = EntryPointAccessors.fromApplication(applicationContext, BackupWorker.Deps::class.java) + BackupScheduler.changed(applicationContext, deps.settingsPrefs()) + } catch (e: Exception) { + Log.w(TAG, "Queueing a backup after a change failed", e) + } finally { + jobFinished(params, false) + } + } + return true + } + + override fun onStopJob(params: JobParameters): Boolean = false + + override fun onDestroy() { + scope.cancel() + super.onDestroy() + } + + companion object { + /** Clear of WorkManager's ids, which count up from 0. */ + private const val JOB_ID = 0x0516_0001 + private const val SETTLE_MINUTES = 2L + private const val MAX_WAIT_MINUTES = 15L + private const val TAG = "BackupChangeJob" + + fun schedule(context: Context) { + val job = JobInfo.Builder(JOB_ID, ComponentName(context, BackupChangeJob::class.java)) + .addTriggerContentUri( + JobInfo.TriggerContentUri( + CalendarContract.CONTENT_URI, + JobInfo.TriggerContentUri.FLAG_NOTIFY_FOR_DESCENDANTS, + ), + ) + .setTriggerContentUpdateDelay(TimeUnit.MINUTES.toMillis(SETTLE_MINUTES)) + .setTriggerContentMaxDelay(TimeUnit.MINUTES.toMillis(MAX_WAIT_MINUTES)) + .build() + context.getSystemService(JobScheduler::class.java).schedule(job) + } + + /** Arm the watch unless it already waits; replacing it would drop the changes it has seen. */ + fun scheduleIfIdle(context: Context) { + if (context.getSystemService(JobScheduler::class.java).getPendingJob(JOB_ID) == null) schedule(context) + } + + fun cancel(context: Context) = context.getSystemService(JobScheduler::class.java).cancel(JOB_ID) + } +} diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/data/backup/ChangeRunPolicyTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/data/backup/ChangeRunPolicyTest.kt new file mode 100644 index 00000000..523608c7 --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/calendula/data/backup/ChangeRunPolicyTest.kt @@ -0,0 +1,30 @@ +package de.jeanlucmakiola.calendula.data.backup + +import androidx.work.ExistingWorkPolicy +import androidx.work.WorkInfo.State.BLOCKED +import androidx.work.WorkInfo.State.CANCELLED +import androidx.work.WorkInfo.State.ENQUEUED +import androidx.work.WorkInfo.State.RUNNING +import androidx.work.WorkInfo.State.SUCCEEDED +import com.google.common.truth.Truth.assertThat +import org.junit.jupiter.api.Test + +class ChangeRunPolicyTest { + + @Test + fun `a run that hasn't started yet covers the change`() { + assertThat(changeRunPolicy(listOf(ENQUEUED))).isNull() + assertThat(changeRunPolicy(listOf(RUNNING, BLOCKED))).isNull() + } + + @Test + fun `a run already going gets the next one queued behind it`() { + assertThat(changeRunPolicy(listOf(RUNNING))).isEqualTo(ExistingWorkPolicy.APPEND_OR_REPLACE) + } + + @Test + fun `with nothing pending a fresh run starts`() { + assertThat(changeRunPolicy(emptyList())).isEqualTo(ExistingWorkPolicy.REPLACE) + assertThat(changeRunPolicy(listOf(SUCCEEDED, CANCELLED))).isEqualTo(ExistingWorkPolicy.REPLACE) + } +} diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/data/backup/WatchStepTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/data/backup/WatchStepTest.kt deleted file mode 100644 index 108b4b0d..00000000 --- a/app/src/test/java/de/jeanlucmakiola/calendula/data/backup/WatchStepTest.kt +++ /dev/null @@ -1,41 +0,0 @@ -package de.jeanlucmakiola.calendula.data.backup - -import androidx.work.WorkInfo.State.BLOCKED -import androidx.work.WorkInfo.State.ENQUEUED -import androidx.work.WorkInfo.State.RUNNING -import androidx.work.WorkInfo.State.SUCCEEDED -import com.google.common.truth.Truth.assertThat -import org.junit.jupiter.api.Test - -class WatchStepTest { - - @Test - fun `a lone run arms the idle slot and leaves a waiting one`() { - assertThat(watchStep(0, listOf(listOf(RUNNING), listOf(SUCCEEDED)))).isEqualTo(WatchStep.Arm(1, append = false)) - assertThat(watchStep(0, listOf(listOf(RUNNING), listOf(ENQUEUED)))).isEqualTo(WatchStep.None) - } - - @Test - fun `a run that still shows as running gets the watch queued behind it`() { - assertThat(watchStep(1, listOf(listOf(RUNNING), listOf(RUNNING)))).isEqualTo(WatchStep.Arm(0, append = true)) - } - - @Test - fun `runs that fired together end with one watch`() { - val afterFirst = listOf(listOf(RUNNING), listOf(RUNNING, BLOCKED)) - assertThat(watchStep(1, afterFirst)).isEqualTo(WatchStep.None) - assertThat(watchStep(0, afterFirst)).isEqualTo(WatchStep.None) - } - - @Test - fun `a second waiting watch is cancelled`() { - assertThat(watchStep(0, listOf(listOf(ENQUEUED), listOf(ENQUEUED)))).isEqualTo(WatchStep.Cancel(1)) - assertThat(watchStep(null, listOf(listOf(ENQUEUED), listOf(ENQUEUED)))).isEqualTo(WatchStep.Cancel(1)) - } - - @Test - fun `reconcile arms slot 0 only when nothing waits`() { - assertThat(watchStep(null, listOf(emptyList(), emptyList()))).isEqualTo(WatchStep.Arm(0, append = false)) - assertThat(watchStep(null, listOf(emptyList(), listOf(ENQUEUED)))).isEqualTo(WatchStep.None) - } -} diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 25e2b206..d5c5b5a6 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -485,29 +485,29 @@ of the backup keeps its last one. The pre-#411 fixed `calendula-backup.ics` is the combined series' stable copy for the install that wrote it, and an install that already had a folder stays on the combined layout. -**Backing up after a change (#516)** is opt-in. `BackupScheduler.watchChanges` -arms a one-off `BackupWorker` with a WorkManager content-URI trigger on the -provider's root URI (the provider notifies every change there, whoever made it: -this app, a sync adapter or another app), a 2-minute settle delay capped at 15 -minutes, and battery-not-low. A trigger fires once and observes only while its -job waits, so there are two slots: a change run arms the other one as it starts -(catching changes made during the run) and checks again as it ends; -`watchStep` keeps exactly one watch waiting, queueing a new one behind a run -that still shows as running and cancelling a second one. Runs share the -scheduled backup's lock. A change run only rewrites the stable copies and never -retries: the dated files and their retention stay on the interval, so a day of -edits can't rotate the history out. Every run that writes something is recorded -on the status line; the failure notification comes once per failure streak, as -it reaches the threshold, and the next success of any run clears it. Since the -trigger fires for any provider change (reminders, other calendars, sync -bookkeeping), each series' file (events in a fixed order) is hashed without its -`DTSTAMP`s (`auto_backup_fingerprints`, reset with the folder and kept only -while the option is on) and a change run -skips the series whose hash (bound to the copy's name) and stable copy are -unchanged, without even listing the folder when no hash moved; the hash is -dropped before a copy is rewritten. A copy that went missing is restored by the -next run that lists the folder; one a still-loading listing doesn't show, or -one edited in place outside the app, is left to the next scheduled run. +**Backing up after a change (#516)** is opt-in. `BackupChangeJob` is a +JobScheduler job with a content trigger on the provider's root URI (the provider +notifies every change there, whoever made it: this app, a sync adapter or +another app), a 2-minute settle delay capped at 15 minutes. A trigger fires +once, so each firing queues a change run (a unique `BackupWorker`, battery not +low, queued behind one already running) and schedules the next watch under the +same id before it finishes, which carries over the changes seen meanwhile. Such +a job can't be persisted; `BackupScheduler.reconcile` re-arms it on every +process start (the boot receivers start one), without replacing a waiting watch. +Runs share the scheduled backup's lock. A change run only rewrites the stable +copies and never retries: the dated files and their retention stay on the +interval, so a day of edits can't rotate the history out. Every run that writes +something is recorded on the status line; the failure notification comes once +per failure streak, as it reaches the threshold, and the next success of any run +clears it. Since the trigger fires for any provider change (reminders, other +calendars, sync bookkeeping), each series' file (events in a fixed order) is +hashed without its `DTSTAMP`s (`auto_backup_fingerprints`, reset with the folder +and kept only while the option is on) and a change run skips the series whose +hash (bound to the copy's name) and stable copy are unchanged, without even +listing the folder when no hash moved; the hash is dropped before a copy is +rewritten. A copy that went missing is restored by the next run that lists the +folder; one a still-loading listing doesn't show, or one edited in place outside +the app, is left to the next scheduled run. **A backup carries the series' deleted and moved occurrences.** `EXDATE` lives on the master row and is part of `EventExportProjection`; `IcsWriter` marks an