sync: three store-side defects, and a launch loop
An override could be moved out of its series' list. updateTask applied form.listId and form.parentId to any row, though updateInstance states the opposite rule ten lines below — and an overridden occurrence maps with isRecurring = false, so the repository routes it here and the edit screen offers its list picker. list_id = B with master_id in list A is invisible in both, since the task query skips a non-null master_id and the override query finds no master in B, while still uploading as part of A's resource. Both paths keep the master's list and parent now. DatabaseCheckpoint never ran its pragma. `query` hands back a lazy cursor and the statement is stepped on the first fill, so closing it unread made the whole class a no-op: the -wal sidecar kept growing and the .db stayed stale, which is exactly the restore case its KDoc says it narrows. And a truncated preferences_pb was a crash at every launch, in all three stores: DataStore.data throws on collection and the collectors are root coroutines in a scope with no handler. They replace a corrupt file with an empty one now — settings fall back to defaults, sync state to "never reconciled", credentials to an account asking to be signed in again, all states the app knows how to be in. StorageModeHolder needs its own guard either way, and specifically has to release the startup gate when it gives up: failing quietly without it parks every observing flow on awaitReady for ever, which is a blank app instead of a crashing one.
This commit is contained in:
+20
@@ -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()
|
||||
|
||||
@@ -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<Preferences> by preferencesDataStore(
|
||||
name = "agendula_prefs",
|
||||
corruptionHandler = replaceCorrupted(),
|
||||
)
|
||||
|
||||
/** See [CredentialsDataStore] for why this is a separate file. */
|
||||
private val Context.credentialsDataStore: DataStore<Preferences> by preferencesDataStore(
|
||||
name = CREDENTIALS_DATASTORE,
|
||||
corruptionHandler = replaceCorrupted(),
|
||||
)
|
||||
|
||||
/** See [SyncStateDataStore] for why this is a separate file. */
|
||||
private val Context.syncStateDataStore: DataStore<Preferences> by preferencesDataStore(
|
||||
name = SYNC_STATE_DATASTORE,
|
||||
corruptionHandler = replaceCorrupted(),
|
||||
)
|
||||
|
||||
/**
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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() }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+20
-2
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user