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 79c3a5a5..eb1dd734 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 @@ -159,8 +159,9 @@ class BackupWorker( val selection = prefs.autoBackupSelection.first() val targets = backupTargets(all.filter(selection::selects), combined = prefs.autoBackupCombined.first()) if (targets.isEmpty()) return RunOutcome.NothingSelected + prefs.adoptLegacyBackupFile() val listing = exporter.listFolder(folder) - var ledger = prefs.autoBackupLedger.first() + val ledger = prefs.autoBackupLedger.first() // A listing still loading can't tell a gone file from a late one. val existing = if (listing.complete) listing.files.keys else listing.files.keys + ledger.values.flatten() val plans = planBackupFiles( @@ -169,15 +170,13 @@ class BackupWorker( existing = existing, at = LocalDateTime.now(), keep = prefs.autoBackupKeep.first(), - legacyOwned = prefs.autoBackupLegacyInstall.first(), ) val stamp = Clock.System.now() val liveKeys = all.map { it.id.toString() }.toSet() - suspend fun record(key: String, files: List) { - ledger = withLedgerEntry(ledger, key, files, liveKeys) - prefs.setAutoBackupLedger(ledger) - } + suspend fun record(key: String, files: List) = + prefs.updateAutoBackupLedger { withLedgerEntry(it, key, files, liveKeys) } var written = 0 + val landedLatest = mutableSetOf() for (plan in plans) { val target = plan.target val bytes = try { @@ -202,28 +201,30 @@ class BackupWorker( } catch (e: Exception) { Log.w(TAG, "Backup of one series failed", e) } - // The stable copy is best-effort: it never holds up the dated - // backup, its pruning or the run's status. + // Best-effort: the stable copy never holds up the dated backup, + // its pruning or the run's status. An old copy goes only once the + // new one has landed. try { + val latest = plan.latestName ?: return@withContext val latestKey = latestLedgerKey(target.key) - val latest = plan.latestName?.let { intended -> - val landed = exporter.writeToFolder(folder, listing, intended, bytes) - // A provider that renamed it leaves a copy to clean up next run. - listOf(landed, intended).distinct() - }.orEmpty() - val owned = ledger[latestKey].orEmpty().filter { it !in plan.staleLatest } - record(latestKey, plan.staleLatest + (latest.ifEmpty { owned })) + // A provider that renamed it leaves a copy to clean up next run. + val landed = exporter.writeToFolder(folder, listing, latest, bytes) + record(latestKey, plan.staleLatest + landed) + if (landed == latest) landedLatest += target.key val staleLeft = exporter.deleteFromFolder(listing, plan.staleLatest) - record(latestKey, plan.staleLatest.filter { it in staleLeft } + latest.ifEmpty { owned }) + record(latestKey, plan.staleLatest.filter { it in staleLeft } + landed) } catch (e: Exception) { Log.w(TAG, "Stable copy of one series failed", e) } } } - replacedLatestCopies(targets, ledger, existing).forEach { (key, names) -> - withContext(NonCancellable) { - val left = exporter.deleteFromFolder(listing, names) - record(latestLedgerKey(key), names.filter { it in left }) + // A layout switch retires the other layout's copies once every new one is there. + if (landedLatest.size == plans.size) { + replacedLatestCopies(targets, prefs.autoBackupLedger.first(), existing).forEach { (key, names) -> + withContext(NonCancellable) { + val left = exporter.deleteFromFolder(listing, names) + record(latestLedgerKey(key), names.filter { it in left }) + } } } return when (written) { 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 1fa01e6a..223e3cfa 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 @@ -354,6 +354,9 @@ class SettingsPrefs @Inject constructor( suspend fun setAutoBackupFolderUri(uri: String?) { prefs.edit { it.pinBackupLayout() + // The ledger says which files in the old folder are ours; in a new + // one a file of the same name may be the user's. + if (it[SettingsKeys.autoBackupFolderUri] != uri) it[SettingsKeys.autoBackupLedger] = emptyMap() it[SettingsKeys.autoBackupFolderUri] = uri } } @@ -367,7 +370,15 @@ class SettingsPrefs @Inject constructor( suspend fun setAutoBackupKeep(keep: Int) = prefs.set(SettingsKeys.autoBackupKeep, keep) - suspend fun setAutoBackupLedger(ledger: BackupLedger) = prefs.set(SettingsKeys.autoBackupLedger, ledger) + /** Atomic, so a run's ledger writes never undo one made meanwhile (pinning the layout). */ + suspend fun updateAutoBackupLedger(transform: (BackupLedger) -> BackupLedger) { + prefs.update(SettingsKeys.autoBackupLedger, transform) + } + + /** 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() } + } /** Switch [calendar] in or out of the automatic backup (#147). */ suspend fun setAutoBackupCalendar(calendar: CalendarSource, included: Boolean) { @@ -839,12 +850,14 @@ val Preferences.autoBackupCombined: Boolean private fun MutablePreferences.pinBackupLayout() { if (this[SettingsKeys.autoBackupCombinedStored] != null) return val legacy = autoBackupLegacyInstall + if (legacy) adoptLegacyBackupFile() this[SettingsKeys.autoBackupCombinedStored] = legacy - if (legacy) { - val key = latestLedgerKey(COMBINED_SERIES) - val ledger = this[SettingsKeys.autoBackupLedger] - this[SettingsKeys.autoBackupLedger] = ledger + (key to (ledger[key].orEmpty() + LEGACY_BACKUP_FILE).distinct()) - } +} + +private fun MutablePreferences.adoptLegacyBackupFile() { + val key = latestLedgerKey(COMBINED_SERIES) + val ledger = this[SettingsKeys.autoBackupLedger] + this[SettingsKeys.autoBackupLedger] = ledger + (key to (ledger[key].orEmpty() + LEGACY_BACKUP_FILE).distinct()) } /** An install that set up automatic backup before #411 and hasn't been pinned since. */ 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 d98b9265..9cc56820 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 @@ -158,9 +158,7 @@ fun backupFileName(baseName: String, at: LocalDateTime): String = * files no longer in the folder ([existing]) are forgotten. Each target also * gets its stable copy, which retention never touches. A stable name is only * written when the file is absent or this series owns it — or a series of this - * run that is moving off it (a rename hand-off). [legacyOwned]: this install - * wrote the pre-#411 [LEGACY_BACKUP_FILE], which is the combined series' stable - * copy now. + * run that is moving off it (a rename hand-off). */ fun planBackupFiles( targets: List, @@ -168,7 +166,6 @@ fun planBackupFiles( existing: Set, at: LocalDateTime, keep: Int, - legacyOwned: Boolean = false, ): List { val retain = keep.coerceAtLeast(1) val runKeys = targets.map { it.key }.toSet() @@ -180,13 +177,11 @@ fun planBackupFiles( val fileName = backupFileName(target.baseName, at) val latest = latestFileName(target.baseName) val earlier = ledger[target.key].orEmpty() - .filter { it in existing && it != fileName && it != latest && it != LEGACY_BACKUP_FILE } + .filter { it in existing && it != fileName && it != latest } .distinct() val kept = earlier.takeLast(retain - 1) val owner = owners[latest] - val writable = latest !in existing || - (owner != null && owner in runKeys) || - (latest == LEGACY_BACKUP_FILE && target.key == COMBINED_SERIES && legacyOwned) + val writable = latest !in existing || (owner != null && owner in runKeys) BackupFilePlan( target = target, fileName = fileName, @@ -240,6 +235,12 @@ fun withLedgerEntry( series in liveKeys || series == COMBINED_SERIES }.toMutableMap() next[key] = files.distinct() + // A stable copy has one owner: claiming it takes it off any other series. + if (key.startsWith(LATEST_PREFIX)) { + next.keys.filter { it != key && it.startsWith(LATEST_PREFIX) }.forEach { other -> + next[other] = next.getValue(other) - files.toSet() + } + } return next.filterValues { it.isNotEmpty() } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index fa11f5d7..eac1f5a3 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -912,7 +912,7 @@ Only the latest The latest %d - Counted per file: with one file per calendar, each calendar keeps this many dated copies, plus one undated copy that always holds the latest backup. Older backups Calendula wrote are deleted after each run; a calendar you stop backing up keeps its last files, and nothing else in the folder is touched. + Counted per file: with one file per calendar, each calendar keeps this many dated copies, plus one undated copy of the latest backup unless a file of yours already has that name. Older backups Calendula wrote are deleted after each run; a calendar you stop backing up keeps its last files, and nothing else in the folder is touched. No automatic backup yet Last backup: %1$s Last backup failed: %1$s diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/data/prefs/SettingsPrefsTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/data/prefs/SettingsPrefsTest.kt index 5e5a4360..d1ee8689 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/data/prefs/SettingsPrefsTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/data/prefs/SettingsPrefsTest.kt @@ -733,8 +733,8 @@ class SettingsPrefsTest { prefs.setAutoBackupFolderUri("content://tree/new") assertThat(prefs.autoBackupCombined.first()).isTrue() assertThat(prefs.autoBackupLegacyInstall.first()).isFalse() - assertThat(prefs.autoBackupLedger.first()) - .containsExactly("latest-combined", listOf("calendula-backup.ics")) + // A new folder starts a new ledger: the old file there isn't known to be ours. + assertThat(prefs.autoBackupLedger.first()).isEmpty() prefs.setAutoBackupCombined(false) assertThat(prefs.autoBackupCombined.first()).isFalse() @@ -765,6 +765,20 @@ class SettingsPrefsTest { assertThat(prefs.autoBackupKeep.first()).isEqualTo(10) } + @Test + fun `a pre-411 install adopts its old file when the layout is pinned in place`(@TempDir tempDir: Path) = runTest { + val store = testDataStore(tempDir, "test_settings") + val prefs = SettingsPrefs(store) + store.updateData { p -> + val m = p.toMutablePreferences() + m[stringPreferencesKey("auto_backup_folder_uri")] = "content://tree/old" + m + } + prefs.setAutoBackupCombined(false) + assertThat(prefs.autoBackupLedger.first()) + .containsExactly("latest-combined", listOf("calendula-backup.ics")) + } + private fun backupCalendar(id: Long, local: Boolean) = CalendarSource( id = id, displayName = "C$id", 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 7d6135fa..6734bc99 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 @@ -132,7 +132,8 @@ class BackupPlanTest { @Test fun `the pre-411 file is the combined stable copy only for the install that wrote it`() { - val owned = planBackupFiles(listOf(combined), emptyMap(), setOf(LEGACY_BACKUP_FILE), at, 1, legacyOwned = true) + val adopted = mapOf(latestLedgerKey(COMBINED_SERIES) to listOf(LEGACY_BACKUP_FILE)) + val owned = planBackupFiles(listOf(combined), adopted, setOf(LEGACY_BACKUP_FILE), at, 1) assertThat(owned.single().latestName).isEqualTo(LEGACY_BACKUP_FILE) assertThat(owned.single().prune).isEmpty() @@ -144,7 +145,7 @@ class BackupPlanTest { fun `a stable name recorded as a dated file by an earlier build is never pruned`() { val ledger = mapOf(COMBINED_SERIES to listOf(LEGACY_BACKUP_FILE, "calendula-backup_a.ics")) val plan = planBackupFiles( - listOf(combined), ledger, setOf(LEGACY_BACKUP_FILE, "calendula-backup_a.ics"), at, 1, legacyOwned = true, + listOf(combined), ledger, setOf(LEGACY_BACKUP_FILE, "calendula-backup_a.ics"), at, 1, ).single() assertThat(plan.prune).containsExactly("calendula-backup_a.ics") } @@ -266,4 +267,13 @@ class BackupPlanTest { assertThat(targets.map { it.baseName }).containsExactly("Work-5-5", "Work-6", "Work-5-9").inOrder() } + @Test + fun `claiming a stable copy takes it off its previous owner`() { + val previous = mapOf(latestLedgerKey("1") to listOf("Work.ics", "Job.ics")) + val next = withLedgerEntry(previous, latestLedgerKey("2"), listOf("Work.ics"), setOf("1", "2")) + assertThat(next).containsExactly( + latestLedgerKey("1"), listOf("Job.ics"), + latestLedgerKey("2"), listOf("Work.ics"), + ) + } } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index ebb333c8..49828e1d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -472,7 +472,8 @@ automatic backup writes one `__