diff --git a/AGENTS.md b/AGENTS.md index cda2831..1f34c19 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,6 +19,10 @@ feature. Do not trade it away for elegance or convenience. ## Current state (2026-11-04) +2026-11-07: the backup folder pick is **two-phase** (issue #30) -- a picked folder is +pending until the "restore or keep?" question is answered, so an unanswered pick can no +longer redirect automatic backups. See the Backup & restore entry under "Implemented". + 2026-11-07: deleting a medication is a **soft delete** (`medications.deleted`, DB v3) -- a hard DELETE cascaded through `doses` and destroyed the whole history the confirm dialog promised to keep; the row stays, flagged `deleted = 1` + `active = 0`, hidden from user-facing lists. @@ -372,6 +376,15 @@ the user's phone. `master` is branch-protected — see "Git workflow" below. that already has backups offers to restore first; restore is always confirmed, is one SQLite transaction, re-ids everything, and first drains native outcomes and cancels every armed alarm (incl. snoozes) so no orphaned alarm survives it. + - Two-phase folder pick (2026-11-07, issue #30): a picked folder is stored *pending* + (pref `pending_tree_uri`), not active, until Dart's "restore or keep?" is answered + and `confirmBackupFolder` moves it to `tree_uri`; every other exit calls + `discardBackupFolder`. Before this, the picker made the folder active immediately, + and leaving the dialog unanswered (killed app, closed page, unreadable folder) could + let the next automatic backup write into a folder the user never confirmed. + Automatic backups only ever use `tree_uri`; a leftover pending folder is discarded + on the next pick. The old Dart-side `holds`/`backupsIdle` guards are gone -- the + pending folder is invisible to automatic backups natively, so no hold is needed. - Today screen warns "Backups are off" (no folder / permission gone) and "Automatic backup failed: …", same pattern as the alarm-path warnings. - Restore verified on hardware 2026-09-24; see "Earlier on 2026-09-24" for what's left. diff --git a/android/app/src/main/kotlin/dev/susiee/dosekeeper/BackupStore.kt b/android/app/src/main/kotlin/dev/susiee/dosekeeper/BackupStore.kt index 856676b..22db795 100644 --- a/android/app/src/main/kotlin/dev/susiee/dosekeeper/BackupStore.kt +++ b/android/app/src/main/kotlin/dev/susiee/dosekeeper/BackupStore.kt @@ -23,9 +23,13 @@ import java.io.FileNotFoundException object BackupStore { private const val PREFS = "dosekeeper_backup" private const val KEY_TREE_URI = "tree_uri" + private const val KEY_PENDING_TREE_URI = "pending_tree_uri" private const val KEY_LAST_BACKUP_AT = "last_backup_at" private const val MIME_JSON = "application/json" + private const val GRANT_FLAGS = + Intent.FLAG_GRANT_READ_URI_PERMISSION or Intent.FLAG_GRANT_WRITE_URI_PERMISSION + private fun prefs(context: Context) = context.getSharedPreferences(PREFS, Context.MODE_PRIVATE) @@ -43,24 +47,61 @@ object BackupStore { if (enabled) marker.createNewFile() else marker.delete() } - /** Takes a lasting grant on [tree] (survives reboots) and makes it the backup folder. */ - fun setFolder(context: Context, tree: Uri) { - val flags = Intent.FLAG_GRANT_READ_URI_PERMISSION or Intent.FLAG_GRANT_WRITE_URI_PERMISSION - context.contentResolver.takePersistableUriPermission(tree, flags) + /** + * Takes a lasting grant on [tree] (survives reboots) and keeps it as the *pending* + * folder: not the backup folder until [confirmPendingFolder]. Two-phase because the + * picker answers before Dart has asked "restore or keep" -- if answering made the + * folder active right away, leaving the dialog unanswered (app killed, page closed) + * would point automatic backups at a folder the user never confirmed. + * A pending folder left over from a killed app is discarded on the next pick. + */ + fun setPendingFolder(context: Context, tree: Uri): Map { + // Grant first: if the tree is the same as a leftover pending, the discard must + // not release the grant we are about to rely on. + context.contentResolver.takePersistableUriPermission(tree, GRANT_FLAGS) + val leftover = prefs(context).getString(KEY_PENDING_TREE_URI, null) + prefs(context).edit().putString(KEY_PENDING_TREE_URI, tree.toString()).apply() + if (leftover != null && leftover != tree.toString()) { + releaseGrant(context, Uri.parse(leftover)) + } + return mapOf("folderName" to folderName(context, tree)) + } + + /** Makes the pending folder the backup folder. No-op without one. */ + fun confirmPendingFolder(context: Context) { + val pending = prefs(context).getString(KEY_PENDING_TREE_URI, null) ?: return // Let go of the previous folder's grant: Android caps how many an app may hold. - folderUri(context)?.takeIf { it != tree }?.let { - runCatching { context.contentResolver.releasePersistableUriPermission(it, flags) } + val previous = prefs(context).getString(KEY_TREE_URI, null) + prefs(context).edit() + .putString(KEY_TREE_URI, pending) + .remove(KEY_PENDING_TREE_URI) + .apply() + if (previous != null && previous != pending) { + releaseGrant(context, Uri.parse(previous)) } - prefs(context).edit().putString(KEY_TREE_URI, tree.toString()).apply() + } + + /** Drops the pending folder without making it active. */ + fun discardPendingFolder(context: Context) { + val pending = prefs(context).getString(KEY_PENDING_TREE_URI, null) ?: return + prefs(context).edit().remove(KEY_PENDING_TREE_URI).apply() + releaseGrant(context, Uri.parse(pending)) + } + + /** Never releases the active folder's grant: pending may point at the same folder. */ + private fun releaseGrant(context: Context, uri: Uri) { + if (uri.toString() == prefs(context).getString(KEY_TREE_URI, null)) return + runCatching { context.contentResolver.releasePersistableUriPermission(uri, GRANT_FLAGS) } } /** - * The backup folder, or null if none was chosen or the grant is gone (revoked in - * settings, or the folder was deleted) -- the caller must treat that as "backups - * are off", not as silently succeeding. + * The backup folder (or the pending one with [pending]), or null if none was chosen + * or the grant is gone (revoked in settings, or the folder was deleted) -- the caller + * must treat that as "backups are off", not as silently succeeding. */ - fun folderUri(context: Context): Uri? { - val stored = prefs(context).getString(KEY_TREE_URI, null) ?: return null + fun folderUri(context: Context, pending: Boolean = false): Uri? { + val key = if (pending) KEY_PENDING_TREE_URI else KEY_TREE_URI + val stored = prefs(context).getString(key, null) ?: return null val uri = Uri.parse(stored) val granted = context.contentResolver.persistedUriPermissions.any { it.uri == uri && it.isWritePermission && it.isReadPermission @@ -88,8 +129,10 @@ object BackupStore { return name ?: tree.lastPathSegment ?: "Backup folder" } - private fun requireFolder(context: Context): Uri = - folderUri(context) ?: throw FileNotFoundException("No backup folder chosen") + private fun requireFolder(context: Context, pending: Boolean = false): Uri = + folderUri(context, pending) ?: throw FileNotFoundException( + if (pending) "No picked folder waiting to be confirmed" else "No backup folder chosen", + ) /** (documentId, displayName) of every file directly inside the folder. */ private fun children(context: Context, tree: Uri): List> { @@ -116,11 +159,11 @@ object BackupStore { children(context, tree).firstOrNull { it.second == name } ?.let { DocumentsContract.buildDocumentUriUsingTree(tree, it.first) } - fun list(context: Context): List = - children(context, requireFolder(context)).map { it.second } + fun list(context: Context, pending: Boolean = false): List = + children(context, requireFolder(context, pending)).map { it.second } - fun read(context: Context, name: String): String { - val tree = requireFolder(context) + fun read(context: Context, name: String, pending: Boolean = false): String { + val tree = requireFolder(context, pending) val doc = find(context, tree, name) ?: throw FileNotFoundException(name) return readUri(context, doc) } diff --git a/android/app/src/main/kotlin/dev/susiee/dosekeeper/MainActivity.kt b/android/app/src/main/kotlin/dev/susiee/dosekeeper/MainActivity.kt index f434367..f00ef2d 100644 --- a/android/app/src/main/kotlin/dev/susiee/dosekeeper/MainActivity.kt +++ b/android/app/src/main/kotlin/dev/susiee/dosekeeper/MainActivity.kt @@ -241,10 +241,28 @@ class MainActivity : FlutterActivity() { REQ_PICK_BACKUP_FOLDER, ) - "listBackups" -> io(result) { BackupStore.list(this) } + "listBackups" -> io(result) { + BackupStore.list(this, pending = call.argument("pending") == true) + } "readBackup" -> io(result) { - BackupStore.read(this, call.argument("name")!!) + BackupStore.read( + this, + call.argument("name")!!, + pending = call.argument("pending") == true, + ) + } + + /** The user answered "keep" or restored: the pending folder becomes active. */ + "confirmBackupFolder" -> io(result) { + BackupStore.confirmPendingFolder(this) + true + } + + /** Any other exit from the folder pick: the pending folder goes away. */ + "discardBackupFolder" -> io(result) { + BackupStore.discardPendingFolder(this) + true } "writeBackup" -> io(result) { @@ -311,10 +329,9 @@ class MainActivity : FlutterActivity() { return } when (requestCode) { - REQ_PICK_BACKUP_FOLDER -> io(result) { - BackupStore.setFolder(this, uri) - BackupStore.status(this) - } + // Pending, not active: Dart still has to ask "restore or keep?" and only a + // confirmBackupFolder makes this the backup folder (see BackupStore). + REQ_PICK_BACKUP_FOLDER -> io(result) { BackupStore.setPendingFolder(this, uri) } REQ_PICK_BACKUP_FILE -> io(result) { BackupStore.readUri(this, uri) } } } diff --git a/lib/services/backup_bridge.dart b/lib/services/backup_bridge.dart index 838716e..404e5e7 100644 --- a/lib/services/backup_bridge.dart +++ b/lib/services/backup_bridge.dart @@ -9,12 +9,20 @@ class BackupBridge { await _channel.invokeMapMethod('backupStatus') ?? const {}, ); - /// Opens the system folder picker. Null if the user backed out. - static Future pickFolder() async { + /// Opens the system folder picker. The picked folder is *pending*, not active: it + /// only becomes the backup folder via [confirmFolder]. Returns the pending folder's + /// name, or null if the user backed out. + static Future pickFolder() async { final raw = await _channel.invokeMapMethod('pickBackupFolder'); - return raw == null ? null : BackupFolderStatus.fromMap(raw); + return raw?['folderName'] as String?; } + /// Makes the pending folder the backup folder. + static Future confirmFolder() => _channel.invokeMethod('confirmBackupFolder'); + + /// Drops the pending folder without making it active; releases its grant. + static Future discardFolder() => _channel.invokeMethod('discardBackupFolder'); + /// Whether Android's Google backup may copy app data. Off unless the user turns it on. static Future googleBackupEnabled() async => await _channel.invokeMethod('googleBackupEnabled') ?? false; @@ -22,11 +30,14 @@ class BackupBridge { static Future setGoogleBackupEnabled(bool enabled) => _channel.invokeMethod('setGoogleBackupEnabled', {'enabled': enabled}); - static Future> list() async => - await _channel.invokeListMethod('listBackups') ?? const []; + /// Lists the backup folder, or the pending one with [pending] (a folder just picked + /// that has not been confirmed yet). + static Future> list({bool pending = false}) async => + await _channel.invokeListMethod('listBackups', {'pending': pending}) ?? + const []; - static Future read(String name) async => - (await _channel.invokeMethod('readBackup', {'name': name}))!; + static Future read(String name, {bool pending = false}) async => + (await _channel.invokeMethod('readBackup', {'name': name, 'pending': pending}))!; static Future write(String name, String content) => _channel.invokeMethod('writeBackup', {'name': name, 'content': content}); diff --git a/lib/services/backup_service.dart b/lib/services/backup_service.dart index 3f166bc..456cce5 100644 --- a/lib/services/backup_service.dart +++ b/lib/services/backup_service.dart @@ -27,12 +27,6 @@ class BackupService { /// none, for the same reason an alarm that quietly doesn't ring is. String? lastError; - /// Folder choices in progress. While any is open, its resume must not write today's - /// file before the backup page has asked whether to restore what is already there. - /// A count, not a flag: a second choice started while the first one is still - /// waiting must keep the hold when the first one ends. - int holds = 0; - Future snapshot({DateTime? now}) async => Backup( exportedAt: now ?? DateTime.now(), medications: await _db.medications(includeDeleted: true), @@ -55,9 +49,6 @@ class BackupService { Future _running = Future.value(); - /// Completes once every backup already queued has finished. - Future get backupsIdle => _running; - Future _backUp({DateTime? now}) async { final status = await BackupBridge.status(); if (!status.enabled) return status; @@ -90,7 +81,6 @@ class BackupService { /// refresh. Future backUpQuietly() async { try { - if (holds > 0) return await BackupBridge.status(); return await backUp(); } on PlatformException catch (e) { lastError = e.message ?? e.code; diff --git a/lib/ui/backup_page.dart b/lib/ui/backup_page.dart index 7490a22..1944a6d 100644 --- a/lib/ui/backup_page.dart +++ b/lib/ui/backup_page.dart @@ -62,32 +62,43 @@ class _BackupPageState extends State { } Future _chooseFolder() => _run(() async { - _service.holds++; + // The picked folder stays *pending* until an explicit confirm, so any exit + // without an answer (backed out, page closed, error -- the pick itself + // included) leaves the old folder active and automatic backups untouched. + // The finally discards it. + var confirmed = false; try { - // A backup queued just before the hold must not land in the new folder. - await _service.backupsIdle; - if (!mounted) return; // left the page while waiting - final picked = await BackupBridge.pickFolder(); - if (picked == null) return; // backed out - + final name = await BackupBridge.pickFolder(); + if (name == null) return; // backed out of the picker // Picking a folder that already holds backups is what someone does after a - // reinstall. Offer the restore *before* the first automatic backup can - // overwrite today's file with whatever is on the phone now. - final existing = Backup.newestFirst(await BackupBridge.list()); + // reinstall. Offer the restore *before* the first backup can overwrite + // today's file with whatever is on the phone now. + final existing = + Backup.newestFirst(await BackupBridge.list(pending: true)); // Asked until answered: cancelling the restore's own confirmation comes back // here instead of counting as "keep". while (existing.isNotEmpty) { if (!mounted) return; // nobody to ask: don't write over what is there final restore = await _askRestoreFromFolder(existing.first); if (restore != true) break; // keep this phone's data - if (await _restore(Backup.decode(await BackupBridge.read(existing.first)))) { - return; - } + final backup = + Backup.decode(await BackupBridge.read(existing.first, pending: true)); + // Confirmed once the data is replaced but before the restore's own backup: + // that backup lands in this folder, the old folder's file for today keeps + // the pre-restore data, and a failed restore leaves this folder unconfirmed + // so no automatic backup can overwrite the file being restored. + final restored = await _restore(backup, onReplaced: () async { + await BackupBridge.confirmFolder(); + confirmed = true; + }); + if (restored) return; } + await BackupBridge.confirmFolder(); + confirmed = true; await _service.backUp(); - _toast('Backups will be saved to ${picked.folderName}'); + _toast('Backups will be saved to $name'); } finally { - _service.holds--; + if (!confirmed) await BackupBridge.discardFolder(); } }); @@ -138,7 +149,7 @@ class _BackupPageState extends State { /// Confirms, then replaces everything. Always confirmed: a restore overwrites every /// medication and all history currently on the phone. True once restored. - Future _restore(Backup backup) async { + Future _restore(Backup backup, {Future Function()? onReplaced}) async { final taken = backup.doses.where((d) => d.status == DoseStatus.taken).length; if (!mounted) return false; final confirmed = await showDialog( @@ -166,6 +177,7 @@ class _BackupPageState extends State { if (confirmed != true) return false; await widget.scheduler.restore(backup); + await onReplaced?.call(); await _service.backUpQuietly(); _toast('Restored ${backup.medications.length} medication(s). Alarms re-armed.'); return true;