From 7afa2f41f0503384d6d17b5c4587c205ae277222 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Mon, 9 Nov 2026 19:03:41 +0100 Subject: [PATCH] Review fixes for advancing series and provider occurrences (#135, #137) (#147) --- .../tasks/ExternalProviderRecurrenceTest.kt | 71 +++++++++ .../tasks/room/RoomTasksDataSourceTest.kt | 91 +++++++++++ .../data/sync/RoomSyncStoreAdvancesTest.kt | 35 ++++- .../agendula/data/sync/CollectionSyncer.kt | 4 +- .../agendula/data/sync/SyncStore.kt | 46 ++++-- .../data/tasks/AndroidTasksDataSource.kt | 145 ++++++++++++++++-- .../agendula/data/tasks/TaskWriteMapper.kt | 6 + .../data/tasks/TasksRepositoryImpl.kt | 7 +- .../agendula/data/tasks/TickedCopies.kt | 84 ++++++++++ .../data/tasks/room/AdvancedOccurrence.kt | 17 +- .../data/tasks/room/RoomTasksDataSource.kt | 72 +++++---- .../agendula/data/tasks/room/TaskDao.kt | 4 + .../agendula/data/sync/SyncFakes.kt | 9 +- 13 files changed, 526 insertions(+), 65 deletions(-) create mode 100644 app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TickedCopies.kt diff --git a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/ExternalProviderRecurrenceTest.kt b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/ExternalProviderRecurrenceTest.kt index f22c196..bd8e05d 100644 --- a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/ExternalProviderRecurrenceTest.kt +++ b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/ExternalProviderRecurrenceTest.kt @@ -64,6 +64,15 @@ class ExternalProviderRecurrenceTest { null, )?.use { c -> if (c.moveToFirst()) c.getString(0) else null } + private fun setRule(taskId: Long, rule: String) { + context.contentResolver.update( + TasksContract.tasksUri(resolver.resolve()!!.authority), + ContentValues().apply { put(Tasks.RRULE, rule) }, + "${Tasks.ID} = ?", + arrayOf(taskId.toString()), + ) + } + @Test fun aSeriesReadsAsRecurringWithItsRule() { val id = dailyTask() @@ -102,6 +111,68 @@ class ExternalProviderRecurrenceTest { assertSeriesMovedOn(id, task.occurrenceStart!!) } + @Test + fun aTickedOccurrenceReadsAsItsCopy() { + val id = dailyTask() + val anchor = source.task(id)!!.occurrenceStart!! + + source.setCompletedInstance(id, anchor, completed = true) + + val read = source.task(id, anchor)!! + assertThat(read.taskId).isNotEqualTo(id) + assertThat(read.isClosed).isTrue() + } + + @Test + fun undoingATickMovesTheSeriesBack() { + val id = dailyTask() + val anchor = source.task(id)!!.occurrenceStart!! + source.setCompletedInstance(id, anchor, completed = true) + val copy = source.task(id, anchor)!!.taskId + + source.setCompletedInstance(id, anchor, completed = false) + + val read = source.task(id, anchor)!! + assertThat(read.taskId).isEqualTo(id) + assertThat(read.isClosed).isFalse() + assertThat(source.task(copy)).isNull() + } + + @Test + fun reopeningTheCopyItselfMovesTheSeriesBack() { + val id = dailyTask() + val anchor = source.task(id)!!.occurrenceStart!! + source.setCompletedInstance(id, anchor, completed = true) + val copy = source.task(id, anchor)!!.taskId + + source.setCompleted(copy, completed = false) + + assertThat(source.task(id, anchor)!!.taskId).isEqualTo(id) + assertThat(source.task(copy)).isNull() + } + + @Test + fun aSeriesChangedSinceTheTickReopensOnlyTheCopy() { + val id = dailyTask() + val anchor = source.task(id)!!.occurrenceStart!! + source.setCompletedInstance(id, anchor, completed = true) + val copy = source.task(id, anchor)!!.taskId + setRule(id, "FREQ=WEEKLY") + + source.setCompletedInstance(id, anchor, completed = false) + + assertThat(source.task(copy)!!.isClosed).isFalse() + assertThat(storedRule(id)).isEqualTo("FREQ=WEEKLY") + } + + @Test + fun anOccurrenceThatIsGoneReadsAsNothing() { + val id = dailyTask() + val anchor = source.task(id)!!.occurrenceStart!! + + assertThat(source.task(id, anchor + 30.hours)).isNull() + } + @Test fun aSeriesStoredWithMillisecondsStillMovesOn() { val id = legacySeries() 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 78d4223..4dcfedb 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 @@ -760,6 +760,97 @@ class RoomTasksDataSourceTest { .doesNotContain(at.epochSeconds) } + @Test + fun reopeningAnotherOverrideOfAFinishedSeriesLeavesItsLastOccurrenceClosed() { + val at = now + val id = source.insertTask(form(title = "Three")) + makeRecurring(id, at, rule = "FREQ=WEEKLY;COUNT=3") + val master = db.tasks().entity(id)!! + val last = db.tasks().insert(master.copy(id = 0, masterId = id, recurrenceId = at + 14.days, rrule = null, status = TaskStatus.COMPLETED)) + source.setCompletedInstance(id, at, completed = true) + source.setCompletedInstance(id, at + 7.days, completed = true) + assertThat(db.tasks().entity(id)!!.status).isEqualTo(TaskStatus.COMPLETED) + + source.setCompletedInstance(last, at + 14.days, completed = false) + + assertThat(db.tasks().entity(last)!!.status).isEqualTo(TaskStatus.NEEDS_ACTION) + assertThat(db.tasks().entity(id)!!.status).isEqualTo(TaskStatus.COMPLETED) + assertThat(source.task(id, at + 7.days)!!.isClosed).isTrue() + } + + @Test + fun reopeningTheCopyWithNewRemindersKeepsThem() { + val at = now + val id = source.insertTask(form(title = "Weekly")) + makeRecurring(id, at) + source.setCompletedInstance(id, at, completed = true) + val copy = db.tasks().allIn(listId).single { it.id != id } + + source.updateTask( + copy.id, + TaskForm(title = copy.title.orEmpty(), listId = listId, start = copy.dtstart, due = copy.due, percentComplete = 0), + reminders = listOf(TaskReminder(minutesBefore = 30)), + ) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at + 7.days) + assertThat(db.tasks().entity(copy.id)!!.status).isEqualTo(TaskStatus.NEEDS_ACTION) + assertThat(db.alarms().forTask(copy.id)).hasSize(1) + } + + @Test + fun aDetachedOccurrenceReopenedSinceStaysOut() { + val at = now + val id = source.insertTask(form(title = "Weekly")) + makeRecurring(id, at) + val master = db.tasks().entity(id)!! + val earlier = db.tasks().insert(master.copy(id = 0, masterId = id, recurrenceId = at, rrule = null, status = TaskStatus.COMPLETED)) + source.setCompletedInstance(id, at + 7.days, completed = true) + source.setCompleted(earlier, completed = false) + + source.setCompletedInstance(id, at + 7.days, completed = false) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at) + assertThat(db.tasks().entity(earlier)!!.masterId).isNull() + assertThat(source.tasks(TaskQuery(listId = listId)).filter { it.taskId == id }.map { it.occurrenceStart?.epochSeconds }) + .doesNotContain(at.epochSeconds) + } + + @Test + fun anOccurrenceWhoseCopyIsGoneReadsAsNothing() { + val at = now + val id = source.insertTask(form(title = "Weekly")) + makeRecurring(id, at) + source.setCompletedInstance(id, at, completed = true) + source.deleteTask(db.tasks().allIn(listId).single { it.id != id }.id) + + assertThat(source.task(id, at)).isNull() + } + + @Test + fun catchingUpOnAnOverdueSeriesKeepsEachTickUndoable() { + val at = now - 56.days + val id = source.insertTask(form(title = "Weekly")) + makeRecurring(id, at) + source.setCompletedInstance(id, at, completed = true) + val first = db.tasks().allIn(listId).single { it.id != id } + source.setCompletedInstance(id, at + 7.days, completed = true) + + assertThat(db.advancedOccurrences().forCopy(first.id)).isNotNull() + } + + @Test + fun aRuleTheServerReorderedStillMovesBack() { + val at = now + val id = source.insertTask(form(title = "Ten")) + makeRecurring(id, at, rule = "FREQ=WEEKLY;COUNT=10") + source.setCompletedInstance(id, at, completed = true) + db.tasks().update(db.tasks().entity(id)!!.copy(rrule = "count=9;FREQ=WEEKLY")) + + source.setCompletedInstance(id, at, completed = false) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at) + } + @Test fun reopeningWithProgressKeepsIt() { val at = now diff --git a/app/src/androidTestFull/java/de/jeanlucmakiola/agendula/data/sync/RoomSyncStoreAdvancesTest.kt b/app/src/androidTestFull/java/de/jeanlucmakiola/agendula/data/sync/RoomSyncStoreAdvancesTest.kt index 2551e25..fd8f135 100644 --- a/app/src/androidTestFull/java/de/jeanlucmakiola/agendula/data/sync/RoomSyncStoreAdvancesTest.kt +++ b/app/src/androidTestFull/java/de/jeanlucmakiola/agendula/data/sync/RoomSyncStoreAdvancesTest.kt @@ -13,6 +13,7 @@ import org.junit.After import org.junit.Test import org.junit.runner.RunWith import kotlin.time.Clock +import kotlin.time.Duration.Companion.seconds import kotlin.time.Instant /** A server version replacing a moved series takes the moves' copies with it (#137). */ @@ -48,10 +49,42 @@ class RoomSyncStoreAdvancesTest { RoomTasksDataSource(db).setCompletedInstance(series, at, true) val copy = db.tasks().allIn(list).single { it.id != series } val store = RoomSyncStore(db) - store.markSynced(listOf(series), "s.ics", "1") + store.markSynced(listOf(db.tasks().entity(series)!!), "s.ics", "1") store.dropAdvances(series) assertThat(db.tasks().entity(copy.id)!!.isDeleted).isFalse() } + + @Test + fun aCopyGivenASubtaskSurvivesTheServerWinning() { + val list = db.taskLists().insert(TaskListEntity(name = "Home", color = 0)) + val at = Instant.fromEpochSeconds(Clock.System.now().epochSeconds) + val series = db.tasks().insert(TaskEntity(listId = list, uid = "s", title = "Weekly", dtstart = at, rrule = "FREQ=WEEKLY")) + RoomTasksDataSource(db).setCompletedInstance(series, at, true) + val copy = db.tasks().allIn(list).single { it.id != series } + db.tasks().insert(TaskEntity(listId = list, uid = "p", title = "Part", parentId = copy.id)) + + RoomSyncStore(db).dropAdvances(series) + + assertThat(db.tasks().entity(copy.id)).isNotNull() + assertThat(db.tasks().entity(copy.id)!!.isDeleted).isFalse() + } + + @Test + fun aTickDuringTheUploadIsNotSettled() { + val list = db.taskLists().insert(TaskListEntity(name = "Home", color = 0)) + val at = Instant.fromEpochSeconds(Clock.System.now().epochSeconds) + val series = db.tasks().insert( + TaskEntity(listId = list, uid = "s", title = "Weekly", dtstart = at, rrule = "FREQ=WEEKLY", lastModified = at - 1.seconds), + ) + val uploaded = db.tasks().entity(series)!! + RoomTasksDataSource(db).setCompletedInstance(series, at, true) + val store = RoomSyncStore(db) + + store.markSynced(listOf(uploaded), "s.ics", "1") + + assertThat(db.tasks().entity(series)!!.isDirty).isTrue() + assertThat(db.advancedOccurrences().forMaster(series).single().settled).isFalse() + } } diff --git a/app/src/full/java/de/jeanlucmakiola/agendula/data/sync/CollectionSyncer.kt b/app/src/full/java/de/jeanlucmakiola/agendula/data/sync/CollectionSyncer.kt index f7d91df..2815af9 100644 --- a/app/src/full/java/de/jeanlucmakiola/agendula/data/sync/CollectionSyncer.kt +++ b/app/src/full/java/de/jeanlucmakiola/agendula/data/sync/CollectionSyncer.kt @@ -609,7 +609,7 @@ class CollectionSyncer( // via `master_id`. val (buried, kept) = local.rows.partition { it.isDeleted && it.recurrenceId != null } if (buried.isNotEmpty()) store.deleteAll(buried.map { it.id }) - store.markSynced(kept.map { it.id }, key, eTag) + store.markSynced(kept, key, eTag) // Both, because a resource that failed as a create was counted under // its UID and is now counted under its href. Clearing one would leak // the other into the store forever. @@ -825,7 +825,7 @@ class CollectionSyncer( } pendingDiscard.remove(href)?.let { cause -> - existing[null]?.let { store.dropAdvances(it.id) } + store.dropAdvances(masterId) report = report.copy( discardedEdits = report.discardedEdits + DiscardedEdit(uid, losing?.title, cause), diff --git a/app/src/full/java/de/jeanlucmakiola/agendula/data/sync/SyncStore.kt b/app/src/full/java/de/jeanlucmakiola/agendula/data/sync/SyncStore.kt index d29b918..1dd6acb 100644 --- a/app/src/full/java/de/jeanlucmakiola/agendula/data/sync/SyncStore.kt +++ b/app/src/full/java/de/jeanlucmakiola/agendula/data/sync/SyncStore.kt @@ -3,6 +3,9 @@ package de.jeanlucmakiola.agendula.data.sync import de.jeanlucmakiola.agendula.data.tasks.room.TaskAlarmEntity import de.jeanlucmakiola.agendula.data.tasks.room.TaskEntity import de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase +import de.jeanlucmakiola.agendula.data.tasks.room.recurrenceAnchor +import de.jeanlucmakiola.agendula.data.tasks.wholeSecond +import kotlin.time.Duration.Companion.seconds import javax.inject.Inject /** @@ -44,17 +47,19 @@ interface SyncStore { /** * Forgets how ticking occurrences moved [masterId]'s series, and deletes the * copies that left: the server's version replaced the moved series, and its - * own overrides stand for those occurrences again. + * own overrides stand for those occurrences again. A row given a life of its + * own since stays. */ fun dropAdvances(masterId: Long) = Unit /** - * Records href and ETag on a resource's rows, clearing `is_dirty`. + * Records href and ETag on a resource's [rows], as they were serialised, + * clearing `is_dirty` on those that did not change since. * * The caller excludes any row it left out of the body — that row is deleted, * not marked synced. */ - fun markSynced(taskIds: List, href: String?, eTag: String?) + fun markSynced(rows: List, href: String?, eTag: String?) fun setParent(taskId: Long, parentId: Long?) @@ -106,24 +111,39 @@ class RoomSyncStore @Inject constructor( override fun dropAdvances(masterId: Long) = database.runInTransaction { val advanced = database.advancedOccurrences() + val tasks = database.tasks() + val master = tasks.entity(masterId) // Only moves the server never saw, on copies that are still just that. advanced.forMaster(masterId).filter { !it.settled && it.undoable }.forEach { record -> + // An edited detached row was unlinked already, so only untouched ones are named. val rows = advanced.detachedFor(record.copyId).mapNotNull { it.taskId } + record.copyId advanced.delete(record.copyId) - rows.mapNotNull { database.tasks().entity(it) }.forEach { row -> - // Never uploaded: nobody to tell. Uploaded: a tombstone the next run deletes. - if (row.href == null) database.tasks().delete(row.id) - else database.tasks().update(row.copy(isDeleted = true, isDirty = true)) - } + rows.mapNotNull { tasks.entity(it) } + .filter { row -> + !row.isDeleted && tasks.subtasks(row.id).isEmpty() && + (master == null || row.listId == master.listId && row.parentId == master.parentId) + } + .forEach { row -> + // Never uploaded: nobody to tell. Uploaded: a tombstone the next run deletes. + if (row.href == null) tasks.delete(row.id) + else tasks.update(row.copy(isDeleted = true, isDirty = true)) + } } } - override fun markSynced(taskIds: List, href: String?, eTag: String?) { - if (taskIds.isEmpty()) return + override fun markSynced(rows: List, href: String?, eTag: String?) { + if (rows.isEmpty()) return database.runInTransaction { - database.tasks().markSynced(taskIds, href, eTag) - // The server now holds those series as moved: a later conflict on them is not about the moves. - database.advancedOccurrences().settle(taskIds) + database.tasks().markSynced(rows.map { it.id }, href, eTag) + // Changed while the body was on its way, by a tick or an edit: still owed to the server. + rows.forEach { database.tasks().dirtyIfChangedSince(it.id, it.lastModified) } + // The server now holds each series as far as the body moved it; a tick + // since is still the device's own. + rows.filter { it.recurrenceId == null }.forEach { row -> + val anchor = row.recurrenceAnchor + if (anchor == null) database.advancedOccurrences().settle(listOf(row.id)) + else database.advancedOccurrences().settleUpTo(row.id, wholeSecond(anchor) + 1.seconds) + } } } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/AndroidTasksDataSource.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/AndroidTasksDataSource.kt index 03cf612..67fa2b4 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/AndroidTasksDataSource.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/AndroidTasksDataSource.kt @@ -120,9 +120,11 @@ class AndroidTasksDataSource @Inject constructor( val rows = instanceRows(queryOccurrences(taskId)) val chosen = rows.firstOrNull { it.anchor == occurrenceStart } ?: rows.firstOrNull { it.anchor?.let { a -> sameSecond(a, occurrenceStart) } == true } - ?: rows.firstOrNull { it.task.taskId == taskId && it.task.distanceFromCurrent == 0 } - ?: rows.firstOrNull { it.task.taskId == taskId } - return chosen?.let { enrich(listOf(it)).single() } + if (chosen != null) return enrich(listOf(chosen)).single() + tickedCopy(taskId, occurrenceStart)?.let { copy -> return task(copy)?.copy(occurrenceStart = occurrenceStart) } + // A plain task has one occurrence, whatever it was addressed by. A series + // answers null rather than another occurrence a write would then land on. + return if (seriesRules(listOf(taskId)).isEmpty()) task(taskId) else null } override fun subtasks(parentTaskId: Long): List = @@ -506,15 +508,25 @@ class AndroidTasksDataSource @Inject constructor( if (ownRowsOnlyFor != authority) { try { return resolver.query(uri, null, OCCURRENCE_OF, arrayOf(id, id), null) - } catch (_: SQLiteException) { - ownRowsOnlyFor = authority - } catch (_: IllegalArgumentException) { - ownRowsOnlyFor = authority + } catch (e: SQLiteException) { + rememberIfMissingColumn(authority, e) + } catch (e: IllegalArgumentException) { + rememberIfMissingColumn(authority, e) } } return resolver.query(uri, null, "${Instances.TASK_ID} = ?", arrayOf(id), null) } + /** Only a view without the column is remembered; any other failure falls back for this query alone. */ + private fun rememberIfMissingColumn(authority: String, e: Exception) { + val message = e.message.orEmpty() + if (Tasks.ORIGINAL_INSTANCE_ID in message || "no such column" in message || "Invalid column" in message) { + ownRowsOnlyFor = authority + } else { + Log.w(TAG, "Occurrence query failed, reading the task's own rows", e) + } + } + /** The provider whose instances view turned out to lack `original_instance_id`. */ @Volatile private var ownRowsOnlyFor: String? = null @@ -556,7 +568,8 @@ class AndroidTasksDataSource @Inject constructor( val ops = resolver.query( TasksContract.tasksUri(authority), arrayOf(Tasks.ID, Tasks.ACCOUNT_NAME, Tasks.ACCOUNT_TYPE) + columns, - columns.joinToString(" OR ") { "$it % 1000 != 0" }, + // Only series: the provider moves nothing else, so nothing else needs rounding. + "(${columns.joinToString(" OR ") { "$it % 1000 != 0" }}) AND (${Tasks.RRULE} IS NOT NULL OR ${Tasks.RDATE} IS NOT NULL)", null, null, )?.use { c -> @@ -688,16 +701,21 @@ class AndroidTasksDataSource @Inject constructor( } override fun setCancelled(taskId: Long, cancelled: Boolean) { + if (!cancelled && movedBack(taskId, null)) return val rows = resolver.update(taskUri(authority(), taskId), cancelValues(cancelled).toContentValues(), null, null) if (rows == 0) throw TaskWriteFailedException("cancel task $taskId") } /** Through the instances URI, like [setCompletedInstance]. */ override fun setCancelledInstance(taskId: Long, occurrenceStart: Instant, cancelled: Boolean) { + if (!cancelled && movedBack(taskId, occurrenceStart)) return val instanceId = instanceIdFor(taskId, occurrenceStart) - ?: return plainUnlessSeries(taskId, occurrenceStart) { setCancelled(taskId, cancelled) } + ?: return tickedCopy(taskId, occurrenceStart)?.let { setCancelled(it, cancelled) } + ?: plainUnlessSeries(taskId, occurrenceStart) { setCancelled(taskId, cancelled) } val uri = TasksContract.instanceUri(authority(), instanceId) - val rows = resolver.update(uri, cancelValues(cancelled).toContentValues(), null, null) + val rows = rememberingCopy(taskId, occurrenceStart, closing = cancelled) { + resolver.update(uri, cancelValues(cancelled).toContentValues(), null, null) + } if (rows == 0) throw TaskWriteFailedException("cancel instance $instanceId") } @@ -707,6 +725,7 @@ class AndroidTasksDataSource @Inject constructor( ) override fun setCompleted(taskId: Long, completed: Boolean) { + if (!completed && movedBack(taskId, null)) return val values = TaskWriteMapper.completionValues(completed, System.currentTimeMillis()) val rows = resolver.update(taskUri(authority(), taskId), values.toContentValues(), null, null) if (rows == 0) throw TaskWriteFailedException("complete task $taskId") @@ -718,14 +737,105 @@ class AndroidTasksDataSource @Inject constructor( * is not a series means the plain write is the right one. */ override fun setCompletedInstance(taskId: Long, occurrenceStart: Instant, completed: Boolean) { + if (!completed && movedBack(taskId, occurrenceStart)) return val instanceId = instanceIdFor(taskId, occurrenceStart) - ?: return plainUnlessSeries(taskId, occurrenceStart) { setCompleted(taskId, completed) } + ?: return tickedCopy(taskId, occurrenceStart)?.let { setCompleted(it, completed) } + ?: plainUnlessSeries(taskId, occurrenceStart) { setCompleted(taskId, completed) } val values = TaskWriteMapper.completionValues(completed, System.currentTimeMillis()) val uri = TasksContract.instanceUri(authority(), instanceId) - val rows = resolver.update(uri, values.toContentValues(), null, null) + val rows = rememberingCopy(taskId, occurrenceStart, closing = completed) { + resolver.update(uri, values.toContentValues(), null, null) + } if (rows == 0) throw TaskWriteFailedException("complete instance $instanceId") } + /** + * Runs [write], and when it closed an occurrence the provider then detached + * into a task of its own, remembers that copy, so the occurrence can still be + * read and reopened, and how the series moved, so reopening can move it back. + */ + private fun rememberingCopy(taskId: Long, occurrenceStart: Instant, closing: Boolean, write: () -> Int): Int { + if (!closing) return write() + val authority = authority() + val lastId = runCatching { lastTaskId(authority) }.getOrNull() ?: return write() + val before = runCatching { seriesState(authority, taskId) }.getOrNull() + return write().also { rows -> + if (rows > 0) { + runCatching { + detachedSince(authority, lastId)?.let { copy -> + val after = before?.let { seriesState(authority, taskId) }?.takeIf { it != before } + val tick = Tick(copy, taskId.takeIf { after != null }, before.takeIf { after != null }, after, System.currentTimeMillis()) + ticks.put(ticks.key(authority, taskId, occurrenceStart), tick) + } + }.onFailure { Log.w(TAG, "Looking up the detached copy failed", it) } + } + } + } + + /** + * Undoes the tick that left [taskId]'s occurrence at [occurrenceStart] — or, + * with no occurrence, the tick that left the copy [taskId]: the series goes + * back where it was and the copy goes, as long as the series is still where + * the tick moved it and the copy has nothing filed under it. False when it + * cannot, so the caller reopens the copy as a task of its own. + */ + private fun movedBack(taskId: Long, occurrenceStart: Instant?): Boolean = runCatching { + val authority = authority() + val (key, tick) = occurrenceStart?.let { at -> ticks.key(authority, taskId, at).let { k -> ticks.get(k)?.let { k to it } } } + ?: ticks.forCopy(authority, taskId) + ?: return false + val series = tick.seriesId ?: return false + val before = tick.before ?: return false + if (seriesState(authority, series) != tick.after || subtaskIds(tick.copyId).isNotEmpty()) return false + val restore = before.filter { (column, value) -> tick.after?.get(column) != value } + val ops = arrayListOf( + ContentProviderOperation.newUpdate(taskUri(authority, series)).withValues(restore.toContentValues()).build(), + ContentProviderOperation.newDelete(taskUri(authority, tick.copyId)).build(), + ) + resolver.applyBatch(authority, ops) + ticks.remove(key) + true + }.onFailure { Log.w(TAG, "Moving the series back failed", it) }.getOrDefault(false) + + /** The columns a tick moves a series by, or null when [taskId] is no series. */ + private fun seriesState(authority: String, taskId: Long): Map? { + if (seriesRules(listOf(taskId)).isEmpty()) return null + return resolver.query(taskUri(authority, taskId), SERIES_STATE, null, null, null)?.use { c -> + if (!c.moveToFirst()) return@use null + SERIES_STATE.indices.associate { i -> + SERIES_STATE[i] to when (c.getType(i)) { + Cursor.FIELD_TYPE_NULL -> null + Cursor.FIELD_TYPE_INTEGER -> c.getLong(i) + else -> c.getString(i) + } + } + } + } + + private fun lastTaskId(authority: String): Long = + resolver.query(TasksContract.tasksUri(authority), arrayOf(Tasks.ID), null, null, "${Tasks.ID} DESC") + ?.use { c -> if (c.moveToFirst()) c.getLong(0) else 0L } ?: 0L + + /** The one plain task the provider created after [afterId], or null when it was not exactly one. */ + private fun detachedSince(authority: String, afterId: Long): Long? = + resolver.query(TasksContract.tasksUri(authority), null, "${Tasks.ID} > ?", arrayOf(afterId.toString()), null) + ?.use { c -> + val r = CursorColumnReader(c) + buildList { + while (c.moveToNext()) { + val plain = r.getString(Tasks.RRULE) == null && r.getString(Tasks.RDATE) == null && + r.getLong(Tasks.ORIGINAL_INSTANCE_ID) == null + if (plain) r.getLong(Tasks.ID)?.let(::add) + } + } + }?.singleOrNull() + + private val ticks = TickedCopies { context.getSharedPreferences(TICKED_COPIES, Context.MODE_PRIVATE) } + + /** The copy a tick left for [taskId]'s occurrence at [occurrenceStart], if one was remembered. */ + private fun tickedCopy(taskId: Long, occurrenceStart: Instant): Long? = + ticks.get(ticks.key(authority(), taskId, occurrenceStart))?.copyId + /** * [write] for a task that turned out not to be a series. A series whose * occurrence was not found fails instead: the plain write would close all of it. @@ -825,11 +935,16 @@ private const val LIST_ORDER = "provider_list_order" /** An instance row of the task, or of an override standing in for one of its occurrences. */ private const val OCCURRENCE_OF = "${Instances.TASK_ID} = ? OR ${Tasks.ORIGINAL_INSTANCE_ID} = ?" -private fun sameSecond(a: Instant, b: Instant): Boolean = - providerMillis(a.toEpochMilliseconds()) == providerMillis(b.toEpochMilliseconds()) - private const val MIGRATIONS = "provider_migrations" +private const val TICKED_COPIES = "provider_ticked_copies" + +/** What a tick changes on a series, and an undo changes back. */ +private val SERIES_STATE = arrayOf( + Tasks.DTSTART, Tasks.DUE, Tasks.DURATION, Tasks.RRULE, Tasks.RDATE, Tasks.EXDATE, + Tasks.STATUS, Tasks.COMPLETED, Tasks.PERCENT_COMPLETE, +) + private const val TAG = "AndroidTasksDataSource" /** No rule to look up: the provider says the row does not recur. */ diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapper.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapper.kt index a6f106e..e8512f2 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapper.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapper.kt @@ -25,6 +25,12 @@ private fun Instant.forAllDay(allDay: Boolean): Instant = */ fun providerMillis(epochMillis: Long): Long = Math.floorDiv(epochMillis, 1000L) * 1000L +/** [at] cut to its whole second, as [providerMillis] does. */ +fun wholeSecond(at: Instant): Instant = Instant.fromEpochMilliseconds(providerMillis(at.toEpochMilliseconds())) + +/** True when [a] and [b] fall in the same whole second. */ +fun sameSecond(a: Instant, b: Instant): Boolean = wholeSecond(a) == wholeSecond(b) + /** * Turns a [TaskForm] / mutation into a name→value map. Pure (no ContentValues), * so it unit-tests on the JVM; [AndroidTasksDataSource] converts the map to diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepositoryImpl.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepositoryImpl.kt index 8aa440a..f747de2 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepositoryImpl.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepositoryImpl.kt @@ -68,7 +68,8 @@ class TasksRepositoryImpl @Inject constructor( override fun taskDetail(taskId: Long, occurrenceStart: Instant?): Flow = observing { dataSource.task(taskId, occurrenceStart)?.let { task -> - val owner = if (task.seriesId == null) resolvedRow(task, taskId) else taskId + // A series' subtasks hang off its master, also when this occurrence is an override. + val owner = task.seriesId ?: resolvedRow(task, taskId) TaskDetail( task = task, subtasks = SeriesCollapse.visible(dataSource.subtasks(owner)).filter { it.taskId != owner }, @@ -173,7 +174,9 @@ class TasksRepositoryImpl @Inject constructor( ) = withContext(io) { // Re-read just before writing: it settles the conflict check *and* tells // us which URI to write through. + // An occurrence that is gone is not written through to its series. val current = dataSource.task(taskId, occurrenceStart) + ?: if (occurrenceStart != null) throw TaskWriteFailedException("update task $taskId@$occurrenceStart: gone") else null // Conflict-safe overwrite: bail if the provider's last_modified moved // since the form captured it (external sync / another app). A null // baseline means "force / overwrite anyway". @@ -234,6 +237,8 @@ class TasksRepositoryImpl @Inject constructor( override suspend fun deleteTask(taskId: Long, occurrenceStart: Instant?, scope: RecurringScope) = withContext(io) { val current = dataSource.task(taskId, occurrenceStart) + // An occurrence that is gone leaves nothing to delete; its series stays. + if (current == null && occurrenceStart != null) return@withContext val seriesId = current?.seriesId val occurrence = current?.occurrenceStart if (seriesId == null || occurrence == null) return@withContext dataSource.deleteWithSubtasks(resolvedRow(current, taskId)) diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TickedCopies.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TickedCopies.kt new file mode 100644 index 0000000..23092e4 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TickedCopies.kt @@ -0,0 +1,84 @@ +package de.jeanlucmakiola.agendula.data.tasks + +import android.content.SharedPreferences +import org.json.JSONObject +import kotlin.time.Instant + +/** + * What ticking a provider occurrence left: the copy the provider detached, and + * the series' columns before and after the tick, so an undo can move it back. + */ +internal class Tick( + val copyId: Long, + val seriesId: Long?, + val before: Map?, + val after: Map?, + val at: Long, +) + +/** The provider keeps no record of its ticks, so they are kept here for [HORIZON_MS]. */ +internal class TickedCopies(private val prefs: () -> SharedPreferences) { + + fun key(authority: String, taskId: Long, occurrenceStart: Instant) = + "$authority|$taskId|${wholeSecond(occurrenceStart).epochSeconds}" + + fun get(key: String): Tick? = prefs().getString(key, null)?.let(::decode) + + /** The tick that left [copyId], with its key. */ + fun forCopy(authority: String, copyId: Long): Pair? = + prefs().all.entries.firstNotNullOfOrNull { (key, value) -> + if (!key.startsWith("$authority|")) return@firstNotNullOfOrNull null + (value as? String)?.let(::decode)?.takeIf { it.copyId == copyId }?.let { key to it } + } + + fun put(key: String, tick: Tick) { + val stale = prefs().all.filter { (_, value) -> + val at = (value as? String)?.let(::decode)?.at ?: 0L + tick.at - at > HORIZON_MS + }.keys + prefs().edit().apply { + stale.forEach(::remove) + putString(key, encode(tick)) + }.apply() + } + + fun remove(key: String) { + prefs().edit().remove(key).apply() + } + + private fun encode(tick: Tick): String = JSONObject().apply { + put("copy", tick.copyId) + put("at", tick.at) + tick.seriesId?.let { put("series", it) } + tick.before?.let { put("before", columns(it)) } + tick.after?.let { put("after", columns(it)) } + }.toString() + + private fun columns(values: Map) = JSONObject().apply { + values.forEach { (column, value) -> put(column, value ?: JSONObject.NULL) } + } + + private fun decode(raw: String): Tick? = runCatching { + val json = JSONObject(raw) + Tick( + copyId = json.getLong("copy"), + seriesId = if (json.has("series")) json.getLong("series") else null, + before = json.optJSONObject("before")?.let(::values), + after = json.optJSONObject("after")?.let(::values), + at = json.getLong("at"), + ) + }.getOrNull() + + private fun values(json: JSONObject): Map = json.keys().asSequence().associateWith { column -> + when (val value = json.get(column)) { + JSONObject.NULL -> null + is Number -> value.toLong() + else -> value.toString() + } + } + + companion object { + /** As long as the own store keeps a tick undoable. */ + const val HORIZON_MS = 30L * 24 * 60 * 60 * 1000 + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/AdvancedOccurrence.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/AdvancedOccurrence.kt index 836092a..18f1001 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/AdvancedOccurrence.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/AdvancedOccurrence.kt @@ -114,7 +114,18 @@ interface AdvancedOccurrenceDao { @Query("UPDATE advanced_occurrences SET settled = 1 WHERE master_id IN (:masterIds)") fun settle(masterIds: List) - /** Records too old to matter: their occurrence is long gone from every list and reminder. */ - @Query("DELETE FROM advanced_occurrences WHERE master_id = :masterId AND anchor < :before") - fun pruneBefore(masterId: Long, before: Instant) + /** Settles [masterId]'s moves to before [until], the anchor the server now holds. */ + @Query("UPDATE advanced_occurrences SET settled = 1 WHERE master_id = :masterId AND advanced_to < :until") + fun settleUpTo(masterId: Long, until: Instant) + + /** Records of ticks made before [before]: too old to undo. Counted from the tick, not the occurrence, which may be long overdue. */ + @Query( + "DELETE FROM advanced_occurrences WHERE master_id = :masterId " + + "AND copy_id IN (SELECT id FROM tasks WHERE created_at < :before)", + ) + fun pruneTickedBefore(masterId: Long, before: Instant) + + /** [taskId], once detached with a tick, has a life of its own and no longer returns to the series. */ + @Query("UPDATE advanced_detached SET task_id = NULL WHERE task_id = :taskId") + fun unlinkDetached(taskId: Long) } 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 ac21b02..8990084 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 @@ -6,6 +6,8 @@ import de.jeanlucmakiola.agendula.domain.TaskReminder import de.jeanlucmakiola.agendula.domain.TaskReminderOrder import de.jeanlucmakiola.agendula.data.tasks.TaskWriteFailedException import de.jeanlucmakiola.agendula.data.tasks.TasksDataSource +import de.jeanlucmakiola.agendula.data.tasks.sameSecond +import de.jeanlucmakiola.agendula.data.tasks.wholeSecond import de.jeanlucmakiola.agendula.domain.Task import de.jeanlucmakiola.agendula.domain.TaskForm import de.jeanlucmakiola.agendula.domain.TaskList @@ -139,12 +141,13 @@ class RoomTasksDataSource @Inject constructor( if (occurrenceStart == null) return task(taskId) val row = tasks.task(taskId) ?: return null if (row.task.recurrenceId != null || !row.task.isRecurring) return task(taskId) - return occurrencesOf(row, tasks.overrides(taskId), clock.now()) - .firstOrNull { it.occurrenceStart == occurrenceStart } + val occurrences = occurrencesOf(row, tasks.overrides(taskId), clock.now()) + // Null rather than another occurrence: a write addressed to this one must not land on that. + return occurrences.firstOrNull { it.occurrenceStart == occurrenceStart } + ?: occurrences.firstOrNull { it.occurrenceStart?.let { a -> sameSecond(a, occurrenceStart) } == true } ?: copyAt(row.task, occurrenceStart)?.let { copy -> tasks.task(copy.id)?.let { RoomTaskMapper.task(it).copy(occurrenceStart = occurrenceStart) } } - ?: task(taskId) } private fun overrideTask(row: TaskRow, anchor: Instant): Task = @@ -242,10 +245,8 @@ class RoomTasksDataSource @Inject constructor( * reminders sync, a sync landing between the two would upload the task * without them, or with the ones it is about to lose. */ - private fun writeReminders(taskId: Long, reminders: List?) { - if (reminders == null) return - alarms.syncForTask(taskId, reminders) - } + private fun writeReminders(taskId: Long, reminders: List?): Boolean = + reminders != null && alarms.syncForTask(taskId, reminders) private fun storedReminders(taskId: Long): List = alarms.forTask(taskId).map { it.asReminder() }.sortedWith(TaskReminderOrder) @@ -253,7 +254,7 @@ class RoomTasksDataSource @Inject constructor( override fun updateTask(taskId: Long, form: TaskForm, reminders: List?) = tx { val current = tasks.entity(taskId) ?: throw TaskWriteFailedException("update task $taskId") val edited = TaskFormWriter.apply(current, form, clock.now(), zone()) - writeReminders(taskId, reminders) + val remindersChanged = writeReminders(taskId, reminders) if (current.masterId == null && edited.listId != current.listId) { if (lists.exists(edited.listId) == 0) throw TaskWriteFailedException("move task: no list ${edited.listId}") move(current, TaskFormWriter.withRule(edited, form.rrule)) @@ -275,6 +276,7 @@ class RoomTasksDataSource @Inject constructor( } else { edited.copy(listId = current.listId, parentId = current.parentId) }, + changedBesides = remindersChanged, ) writes.onWritten(setOf(current.listId)) } @@ -283,6 +285,7 @@ class RoomTasksDataSource @Inject constructor( private fun move(current: TaskEntity, edited: TaskEntity) { // A copy moved elsewhere is a task of its own now. advanced.keepAsOwnTask(current.id) + advanced.unlinkDetached(current.id) val plan = TaskMove.plan( edited = edited, previous = current, @@ -472,6 +475,8 @@ class RoomTasksDataSource @Inject constructor( override fun setReminders(taskId: Long, reminders: List) = tx { if (!alarms.syncForTask(taskId, reminders)) return@tx + advanced.keepAsOwnTask(taskId) + advanced.unlinkDetached(taskId) // The reminders are part of the VTODO now, so changing them is an edit. val current = tasks.entity(taskId) ?: return@tx tasks.update(current.copy(isDirty = true, lastModified = clock.now())) @@ -535,8 +540,11 @@ class RoomTasksDataSource @Inject constructor( return } if (master.isClosed) { - // A series ends with its last occurrence; reopening any of it reopens the series. - if (!closing) closeSeries(master, override, close) else writeInstance(master, override, anchor, close) + // A series ends with its last occurrence; reopening that one reopens the + // series. An override reopens alone while other occurrences are closed + // only through the master, which reopening it would reopen too. + val reopensSeries = !closing && (override == null || !closedThroughMaster(master, anchor)) + if (reopensSeries) closeSeries(master, override, close) else writeInstance(master, override, anchor, close) return } if (!closing || !advance(master, anchor, override, close)) writeInstance(master, override, anchor, close) @@ -544,13 +552,15 @@ class RoomTasksDataSource @Inject constructor( /** * Writes [edited] over [current]. A copy whose only change is that it - * reopens moves its series back instead; any other edit makes it a task of - * its own, and it no longer stands for the series' occurrence. + * reopens moves its series back instead; any other edit, [changedBesides] + * included, makes it a task of its own, and it no longer stands for the + * series' occurrence. So does any edit to an occurrence detached with it. */ - private fun saveRow(current: TaskEntity, edited: TaskEntity) { + private fun saveRow(current: TaskEntity, edited: TaskEntity, changedBesides: Boolean = false) { + advanced.unlinkDetached(current.id) val record = advanced.forCopy(current.id)?.takeIf { it.undoable } if (record != null) { - val unchanged = sameApartFromState(current, edited) + val unchanged = !changedBesides && sameApartFromState(current, edited) if (unchanged && current.isClosed && !edited.isClosed && reopenCopy(current, edited, record)) return if (!unchanged || !edited.isClosed) advanced.keepAsOwnTask(current.id) } @@ -571,6 +581,14 @@ class RoomTasksDataSource @Inject constructor( normalizeTags(a.categories) == normalizeTags(b.categories) && a.color == b.color } + /** Whether an occurrence of [master] other than [except] has no override, so its state is the master's. */ + private fun closedThroughMaster(master: TaskEntity, except: Instant): Boolean { + val spec = master.recurrenceSpec() ?: return false + val overridden = tasks.overrides(master.id).mapNotNull { it.recurrenceId?.let(::wholeSecond) }.toSet() + return expandFrom(spec, maxOccurrences = overridden.size + 2) + .any { !sameSecond(it, except) && wholeSecond(it) !in overridden } + } + /** [close] applied to the master — and to [override], its last occurrence — which ends the series. */ private fun closeSeries(master: TaskEntity, override: TaskEntity?, close: (TaskEntity) -> TaskEntity) { tasks.update(close(master)) @@ -603,7 +621,7 @@ class RoomTasksDataSource @Inject constructor( at to detach(overrides.getValue(at.epochSeconds), now) } // Expansion drops the anchor's sub-second part; the series keeps it. - val newAnchor = next + (spec.anchor - floorSecond(spec.anchor)) + val newAnchor = next + (spec.anchor - wholeSecond(spec.anchor)) val spent = spentBefore(spec, next) val (start, due) = occurrenceTimes(master, newAnchor) val ruleAfter = TaskFormWriter.remainingCount(rule, spent) @@ -628,8 +646,8 @@ class RoomTasksDataSource @Inject constructor( alarms.copyToTask(master.id, id, sameResource = false) id } - advanced.at(master.id, floorSecond(anchor))?.let { advanced.delete(it.copyId) } - advanced.pruneBefore(master.id, now - RECORD_HORIZON) + advanced.at(master.id, wholeSecond(anchor))?.let { advanced.delete(it.copyId) } + advanced.pruneTickedBefore(master.id, now - RECORD_HORIZON) advanced.insert( AdvancedOccurrenceEntity( copyId = copyId, @@ -655,7 +673,7 @@ class RoomTasksDataSource @Inject constructor( /** Occurrences `COUNT` has spent before [cut]: counted before `EXDATE` (RFC 5545 §3.8.5.3). */ private fun spentBefore(spec: RecurrenceSpec, cut: Instant): Int = RecurrenceExpander.expand( spec.copy(exdate = null), - ExpansionWindow(from = floorSecond(spec.anchor), until = cut, maxOccurrences = SPLIT_COUNT_CEILING), + ExpansionWindow(from = wholeSecond(spec.anchor), until = cut, maxOccurrences = SPLIT_COUNT_CEILING), ).size /** @@ -706,7 +724,7 @@ class RoomTasksDataSource @Inject constructor( */ private fun reopenCopy(row: TaskEntity, reopened: TaskEntity, record: AdvancedOccurrenceEntity): Boolean { val master = tasks.entity(record.masterId)?.takeIf { - !it.isDeleted && !it.isClosed && row.listId == it.listId && it.rrule == record.ruleAfter && + !it.isDeleted && !it.isClosed && row.listId == it.listId && sameRule(it.rrule, record.ruleAfter) && it.recurrenceAnchor?.let { a -> sameSecond(a, record.advancedTo) } == true } if (master == null || tasks.subtasks(row.id).isNotEmpty()) return false @@ -740,6 +758,13 @@ class RoomTasksDataSource @Inject constructor( return true } + /** The same `RRULE` however a server re-serialised it: parts in any order and case. */ + private fun sameRule(a: String?, b: String?): Boolean { + fun parts(rule: String?) = rule?.trim()?.removePrefix("RRULE:")?.split(';') + ?.map { it.trim().uppercase() }?.filter { it.isNotEmpty() }?.toSet() + return parts(a) == parts(b) + } + /** [row], a detached occurrence, back as [master]'s override for [anchor]. */ private fun toOverride(row: TaskEntity, master: TaskEntity, anchor: Instant, now: Instant) { val back = row.copy( @@ -765,16 +790,14 @@ class RoomTasksDataSource @Inject constructor( /** The copy [advance] left for [master]'s occurrence at [anchor], if it is still there. */ private fun copyAt(master: TaskEntity, anchor: Instant): TaskEntity? = - advanced.at(master.id, floorSecond(anchor))?.let { tasks.entity(it.copyId) }?.takeUnless { it.isDeleted } + advanced.at(master.id, wholeSecond(anchor))?.let { tasks.entity(it.copyId) }?.takeUnless { it.isDeleted } /** The series' occurrences from its anchor on, at most [maxOccurrences]. */ private fun expandFrom(spec: RecurrenceSpec, maxOccurrences: Int): List { - val from = floorSecond(spec.anchor) + val from = wholeSecond(spec.anchor) return RecurrenceExpander.expand(spec, ExpansionWindow(from = from, until = from + SERIES_LOOKAHEAD, maxOccurrences = maxOccurrences)) } - private fun floorSecond(at: Instant) = Instant.fromEpochSeconds(at.epochSeconds) - /** * The residue without the series' own lines, as a new resource must not * carry them. Lines inside a nested component (a `VALARM`'s `UID`) are its @@ -791,9 +814,6 @@ class RoomTasksDataSource @Inject constructor( }?.joinToString("\r\n")?.ifBlank { null } } - /** Expansion runs in whole seconds; stored anchors may carry milliseconds. */ - private fun sameSecond(a: Instant, b: Instant) = a.epochSeconds == b.epochSeconds - /** Applies [write] to [override], or to a new one forked from [master] for [occurrenceStart]. */ private fun writeInstance(master: TaskEntity, override: TaskEntity?, occurrenceStart: Instant, write: (TaskEntity) -> TaskEntity) { if (override != null) { diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskDao.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskDao.kt index 747ef2d..8bdffa5 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskDao.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskDao.kt @@ -199,4 +199,8 @@ interface TaskDao { */ @Query("UPDATE tasks SET href = :href, etag = :etag, is_dirty = 0 WHERE id IN (:taskIds)") fun markSynced(taskIds: List, href: String?, etag: String?): Int + + /** Sets `is_dirty` again on [taskId] when it was written after [seen], its `last_modified` when uploaded. */ + @Query("UPDATE tasks SET is_dirty = 1 WHERE id = :taskId AND last_modified IS NOT :seen") + fun dirtyIfChangedSince(taskId: Long, seen: Instant?) } diff --git a/app/src/testFull/java/de/jeanlucmakiola/agendula/data/sync/SyncFakes.kt b/app/src/testFull/java/de/jeanlucmakiola/agendula/data/sync/SyncFakes.kt index 0b18304..a563d46 100644 --- a/app/src/testFull/java/de/jeanlucmakiola/agendula/data/sync/SyncFakes.kt +++ b/app/src/testFull/java/de/jeanlucmakiola/agendula/data/sync/SyncFakes.kt @@ -76,11 +76,12 @@ class FakeStore(rows: List = emptyList()) : SyncStore { return id } - override fun markSynced(taskIds: List, href: String?, eTag: String?) { - taskIds.forEach { id -> - val index = rows.indexOfFirst { it.id == id } + override fun markSynced(rows: List, href: String?, eTag: String?) { + rows.forEach { synced -> + val index = this.rows.indexOfFirst { it.id == synced.id } if (index >= 0) { - rows[index] = rows[index].copy(href = href, etag = eTag, isDirty = false) + val current = this.rows[index] + this.rows[index] = current.copy(href = href, etag = eTag, isDirty = current.lastModified != synced.lastModified) } } }