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 5589f480..5a5188d5 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 @@ -32,6 +32,7 @@ import de.jeanlucmakiola.calendula.data.prefs.SettingsPrefs import de.jeanlucmakiola.calendula.domain.backup.BackupFilePlan import de.jeanlucmakiola.calendula.domain.backup.backupFingerprint import de.jeanlucmakiola.calendula.domain.backup.backupTargets +import de.jeanlucmakiola.calendula.domain.backup.inBackupOrder import de.jeanlucmakiola.calendula.domain.backup.needsRefresh import de.jeanlucmakiola.calendula.domain.backup.withFingerprint import de.jeanlucmakiola.calendula.domain.backup.planBackupFiles @@ -185,9 +186,9 @@ class BackupWorker( prefs.autoBackupFolderUri.first() != null if (wanted) BackupScheduler.watchChanges(applicationContext, slot = 1 - slot) } + runCatching { watchNext() }.onFailure { Log.w(TAG, "Arming the next change run failed", it) } // Never fails or retries: a retry would only wait for another change. try { - watchNext() backUp(deps, prefs, changeRun = true) } catch (e: CancellationException) { throw e @@ -201,8 +202,8 @@ class BackupWorker( /** * One run under the shared lock. A change run (#516) refreshes only the - * stable copies; it reports a failure, but a success stays off the status - * line, which dates the last dated backup. + * stable copies and stays off the status line and the failure count, which + * report on the scheduled backup; a broken folder fails that one too. */ private suspend fun backUp(deps: Deps, prefs: SettingsPrefs, changeRun: Boolean): Result { val now = System.currentTimeMillis() @@ -223,9 +224,8 @@ class BackupWorker( RunOutcome.Failed } } - if (outcome == RunOutcome.NothingToDo) return Result.success() + if (changeRun || outcome == RunOutcome.NothingToDo) return Result.success() val success = outcome == RunOutcome.Written - if (changeRun && success) return Result.success() prefs.recordAutoBackupRun(success = success, atMillis = now) if (!success && prefs.autoBackupStatus.first().consecutiveFailures >= FAILURE_NOTIFY_THRESHOLD) { notifyFailure(applicationContext) @@ -239,7 +239,8 @@ class BackupWorker( /** * Writes each series and prunes it only once its new file has landed. A * [latestOnly] run (#516) leaves the dated files and their retention to the - * schedule and rewrites only the stable copies whose calendars changed. + * schedule and rewrites only the stable copies whose calendars changed; + * its outcome isn't reported. */ private suspend fun runBackup(deps: Deps, prefs: SettingsPrefs, folder: Uri, latestOnly: Boolean): RunOutcome { val repository = deps.repository() @@ -307,9 +308,7 @@ class BackupWorker( } catch (e: Exception) { Log.w(TAG, "Stable copy of one series failed", e) // The copy may be half-written now: the next change run must not take it as current. - runCatching { - prefs.updateAutoBackupLedger(folder.toString(), fingerprints = { it - plan.target.key }) { it } - } + runCatching { prefs.dropAutoBackupFingerprint(folder.toString(), plan.target.key) } false } @@ -320,7 +319,8 @@ class BackupWorker( val target = plan.target if (latestOnly && plan.latestName == null) continue val text = try { - IcsWriter().writeCalendar(repository.exportEvents(target.calendarIds), stamp, target.calendarName) + val events = repository.exportEvents(target.calendarIds).inBackupOrder() + IcsWriter().writeCalendar(events, stamp, target.calendarName) } catch (e: CancellationException) { throw e } catch (e: Exception) { @@ -330,18 +330,18 @@ class BackupWorker( } val fingerprint = backupFingerprint(text) if (latestOnly && !plan.needsRefresh(fingerprint, fingerprints, listing.files.keys, listing.complete)) { - landedLatest += target.key + if (plan.latestName in listing.files) landedLatest += target.key continue } val bytes = text.toByteArray(Charsets.UTF_8) // Not cancellable once a file exists: it must reach the ledger, or // nothing would ever prune it. withContext(NonCancellable) { - val dated = latestOnly || writeDated(plan, bytes) - // A full run's stable copy is best-effort and never holds up its status. - val latest = plan.latestName != null && writeLatest(plan, bytes, fingerprint) - if (latest) landedLatest += target.key - if (if (latestOnly) latest else dated) written++ else failed++ + if (!latestOnly) { + if (writeDated(plan, bytes)) written++ else failed++ + } + // Best-effort in a full run: it never holds up the run's status. + if (plan.latestName != null && writeLatest(plan, bytes, fingerprint)) landedLatest += target.key } } // A layout switch retires the other layout's copies once every new one is there. diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/prefs/SettingsPrefs.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/prefs/SettingsPrefs.kt index e039b9fe..c09bad53 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/prefs/SettingsPrefs.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/prefs/SettingsPrefs.kt @@ -399,6 +399,15 @@ class SettingsPrefs @Inject constructor( } } + /** Forget [key]'s hash (#516), so the next change run rewrites its stable copy. */ + suspend fun dropAutoBackupFingerprint(folder: String, key: String) { + prefs.edit { + if (it[SettingsKeys.autoBackupFolderUri] == folder) { + it[SettingsKeys.autoBackupFingerprints] = it[SettingsKeys.autoBackupFingerprints] - key + } + } + } + /** Record a pre-#411 install's old file as its combined series' stable copy (see [pinBackupLayout]). */ suspend fun adoptLegacyBackupFile() { prefs.edit { if (it.autoBackupLegacyInstall) it.adoptLegacyBackupFile() } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/backup/BackupPlan.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/backup/BackupPlan.kt index fd73b1fd..5bb2f9b7 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/domain/backup/BackupPlan.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/backup/BackupPlan.kt @@ -1,6 +1,7 @@ package de.jeanlucmakiola.calendula.domain.backup import de.jeanlucmakiola.calendula.domain.CalendarSource +import de.jeanlucmakiola.calendula.domain.ics.IcsEvent import de.jeanlucmakiola.calendula.domain.isNotSynced import de.jeanlucmakiola.floret.android.safeFileName import java.security.MessageDigest @@ -324,8 +325,8 @@ fun parseBackupFingerprints(stored: String?): BackupFingerprints = /** * Whether a run started by a change (#516) rewrites this series' stable copy: - * only when its content moved on from what the copy holds, or the copy is - * missing or mid-rename. Never when the name belongs to a file of the user's, + * only when its content moved on from what the copy holds (a rename changes + * the name written into it), or the copy is missing. Never when the name belongs to a file of the user's, * nor when a [complete]-less listing doesn't show the copy: writing then would * add a duplicate beside it, so that is left to the scheduled run. */ @@ -337,5 +338,13 @@ fun BackupFilePlan.needsRefresh( ): Boolean { val latest = latestName ?: return false if (latest !in listed) return complete - return stored[target.key] != fingerprint || staleLatest.isNotEmpty() || handOff.isNotEmpty() + return stored[target.key] != fingerprint } + +/** + * The events in a fixed order, reminders sorted, so the same content always + * writes the same file and hashes the same, however the provider returned it. + */ +fun List.inBackupOrder(): List = + sortedWith(compareBy({ it.start }, { it.uid }, { it.overrideAt })) + .map { it.copy(reminderMinutes = it.reminderMinutes.sorted()) } diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/domain/backup/BackupPlanTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/domain/backup/BackupPlanTest.kt index 602a24f3..81046f11 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/domain/backup/BackupPlanTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/domain/backup/BackupPlanTest.kt @@ -330,7 +330,6 @@ class BackupPlanTest { assertThat(plan.needsRefresh("h2", stored, listed, complete = true)).isTrue() assertThat(plan.needsRefresh("h1", emptyMap(), listed, complete = true)).isTrue() assertThat(plan.needsRefresh("h1", stored, emptySet(), complete = true)).isTrue() - assertThat(plan.copy(staleLatest = listOf("Job.ics")).needsRefresh("h1", stored, listed, complete = true)).isTrue() } @Test @@ -349,4 +348,16 @@ class BackupPlanTest { assertThat(plan.latestName).isNull() assertThat(plan.needsRefresh("h1", emptyMap(), setOf("Work.ics"), complete = true)).isFalse() } + + @Test + fun `backup order doesn't depend on the provider's`() { + fun event(uid: String, reminders: List) = IcsEvent( + uid = uid, summary = uid, start = Instant.fromEpochSeconds(0), end = Instant.fromEpochSeconds(3600), + isAllDay = false, zoneId = "UTC", reminderMinutes = reminders, + ) + val one = listOf(event("b", listOf(15, 10)), event("a", listOf(5))) + val other = listOf(event("a", listOf(5)), event("b", listOf(10, 15))) + assertThat(one.inBackupOrder()).isEqualTo(other.inBackupOrder()) + assertThat(one.inBackupOrder().map { it.uid }).containsExactly("a", "b").inOrder() + } } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 3c3be32b..bce7cc14 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -492,8 +492,8 @@ 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 again as it ends. Runs share the -scheduled backup's lock. A change run only rewrites the stable copies and shows -on the status line only when it fails: the dated files and their retention stay on the interval, +scheduled backup's lock. A change run only rewrites the stable copies and stays +off the status line: the dated files and their retention stay on the interval, so a day of edits can't rotate the history out. Since the trigger fires for any provider change (reminders, other calendars, sync bookkeeping), each series' file is hashed without its `DTSTAMP`s (`auto_backup_fingerprints`, reset with