diff --git a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSourceTest.kt b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSourceTest.kt index 1c922fe..4c9970a 100644 --- a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSourceTest.kt +++ b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSourceTest.kt @@ -369,6 +369,26 @@ class RoomTasksDataSourceTest { assertThat(db.tasks().allOverrides(listId)).isEmpty() } + @Test + fun anOccurrenceCannotBeMovedOutOfItsSeriesList() { + val other = source.createLocalList("Work", 0) + val id = source.insertTask(form()) + makeRecurring(id, now) + val target = source.tasks(TaskQuery(listId = listId)) + .filter { it.taskId == id } + .first { it.distanceFromCurrent == 1 } + source.updateInstance(id, target.occurrenceStart!!, form(title = "moved")) + val override = db.tasks().override(id, target.occurrenceStart)!! + + source.updateTask(override.id, TaskForm(title = "moved", listId = other)) + + // ⚠️ list_id = B with master_id in list A is invisible in both — the task + // query skips non-null master_id, and the override query finds no master + // in B — while still uploading as part of A's resource. + assertThat(db.tasks().entity(override.id)!!.listId).isEqualTo(listId) + assertThat(db.tasks().entity(override.id)!!.title).isEqualTo("moved") + } + @Test fun deletingASyncedSeriesTombstonesItsOverridesToo() { val syncedList = syncedList() diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/di/DataModule.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/di/DataModule.kt index f6b5265..4079208 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/di/DataModule.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/di/DataModule.kt @@ -28,6 +28,8 @@ import de.jeanlucmakiola.agendula.data.tasks.TasksRepository import de.jeanlucmakiola.agendula.data.tasks.TasksRepositoryImpl import de.jeanlucmakiola.agendula.data.tasks.room.RoomTasksDataSource import de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase +import androidx.datastore.core.handlers.ReplaceFileCorruptionHandler +import androidx.datastore.preferences.core.emptyPreferences import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -35,18 +37,35 @@ import kotlinx.coroutines.SupervisorJob import javax.inject.Provider import javax.inject.Singleton +/** + * ⚠️ Every one of these needs a corruption handler, and without one a truncated + * `preferences_pb` is a **crash at every launch**: `DataStore.data` throws + * `CorruptionException` on collection, and the collectors here are root + * coroutines in a scope with no handler. A file half-written by a kill during + * `edit` is the ordinary way to get one. + * + * Starting empty is the only recovery available and it is a mild one: the + * settings store falls back to defaults, the sync-state store to "never + * reconciled", and the credential store to accounts that ask to be signed in + * again — all states the app already knows how to be in, unlike a launch loop. + */ +private fun replaceCorrupted() = ReplaceFileCorruptionHandler { emptyPreferences() } + private val Context.agendulaDataStore: DataStore by preferencesDataStore( name = "agendula_prefs", + corruptionHandler = replaceCorrupted(), ) /** See [CredentialsDataStore] for why this is a separate file. */ private val Context.credentialsDataStore: DataStore by preferencesDataStore( name = CREDENTIALS_DATASTORE, + corruptionHandler = replaceCorrupted(), ) /** See [SyncStateDataStore] for why this is a separate file. */ private val Context.syncStateDataStore: DataStore by preferencesDataStore( name = SYNC_STATE_DATASTORE, + corruptionHandler = replaceCorrupted(), ) /** diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/StorageModeHolder.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/StorageModeHolder.kt index 81b86dc..53b94fc 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/StorageModeHolder.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/StorageModeHolder.kt @@ -2,6 +2,7 @@ package de.jeanlucmakiola.agendula.data.tasks import de.jeanlucmakiola.agendula.data.di.ApplicationScope import de.jeanlucmakiola.agendula.data.prefs.SettingsPrefs +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch @@ -35,8 +36,24 @@ class StorageModeHolder @Inject constructor( /** Starts mirroring. Idempotent in effect; call once, from `Application.onCreate`. */ fun start() { scope.launch { - prefs.storageMode.collect { mode -> - resolver.storageMode = mode + try { + prefs.storageMode.collect { mode -> + resolver.storageMode = mode + firstValue.complete(Unit) + } + } catch (e: CancellationException) { + throw e + } catch (_: Throwable) { + // ⚠️ Two failures in one, and the second is the worse. This is a + // root coroutine in a scope with no exception handler, so a read + // that throws — a corrupt file the handler could not replace, a + // filesystem that will not answer — takes the app down at every + // launch. And releasing the gate is not optional either: + // `awaitReady` is awaited before anything touches the provider, + // so failing quietly without it parks every observing flow for + // ever, which is a blank app rather than a crashing one. + // `autoMode` answers from here on, which is the steady state for + // a user who never chose. firstValue.complete(Unit) } } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/DatabaseCheckpoint.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/DatabaseCheckpoint.kt index 4c3487d..f02144b 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/DatabaseCheckpoint.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/DatabaseCheckpoint.kt @@ -28,7 +28,14 @@ class DatabaseCheckpoint @Inject constructor( override fun onStop(owner: LifecycleOwner) { scope.launch(Dispatchers.IO) { runCatching { - database.openHelper.writableDatabase.query("PRAGMA wal_checkpoint(TRUNCATE)").close() + // ⚠️ Stepped, not merely compiled. `query` hands back a lazy + // cursor and the statement runs on the first fill — closing it + // unread made this whole class a no-op, so the sidecar kept + // growing and the `.db` stayed stale, which is precisely the + // restore case above. + database.openHelper.writableDatabase + .query("PRAGMA wal_checkpoint(TRUNCATE)") + .use { it.moveToFirst() } } } } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSource.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSource.kt index 62889a7..351f5db 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSource.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTasksDataSource.kt @@ -147,7 +147,22 @@ class RoomTasksDataSource @Inject constructor( override fun updateTask(taskId: Long, form: TaskForm) { val current = tasks.entity(taskId) ?: throw TaskWriteFailedException("update task $taskId") - tasks.update(TaskFormWriter.apply(current, form, clock.now(), zone())) + val edited = TaskFormWriter.apply(current, form, clock.now(), zone()) + // ⚠️ An override's list and parent are the master's, the rule + // [updateInstance] states and this path has to keep. An overridden + // occurrence maps with `isRecurring = false`, so the repository routes + // it here and the edit screen offers its list picker — and a row that + // took `list_id = B` while its `master_id` stayed in list A is invisible + // in both (the task query skips non-null `master_id`, and the override + // query finds no master in B) while still uploading as part of A's + // resource. + tasks.update( + if (current.masterId == null) { + edited + } else { + edited.copy(listId = current.listId, parentId = current.parentId) + }, + ) } /** @@ -162,7 +177,10 @@ class RoomTasksDataSource @Inject constructor( val now = clock.now() val existing = tasks.override(taskId, occurrenceStart) if (existing != null) { - tasks.update(TaskFormWriter.apply(existing, form, now, zone())) + // The same rule the fork below applies: the list and parent are the + // master's, whatever the form was carrying. + val edited = TaskFormWriter.apply(existing, form, now, zone()) + tasks.update(edited.copy(listId = master.listId, parentId = master.parentId)) return } val (start, due) = occurrenceTimes(master, occurrenceStart)