From 4335f6598052dcffba4ba03d6e477793bbd05440 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Fri, 6 Nov 2026 18:37:27 +0100 Subject: [PATCH] Keep the advance record local and undo only the move (#137) The record of how a series moved now lives in a local table (schema v5) instead of an X- property synced with the copy. Reopening undoes the move alone, keeping later edits; series with RDATEs keep the override model; the series also moves past closed occurrences after the ticked one. --- .../5.json | 675 ++++++++++++++++++ .../data/tasks/provider/ProviderWriterTest.kt | 14 + .../tasks/room/RoomTasksDataSourceTest.kt | 128 ++++ .../tasks/room/TasksDatabaseMigrationTest.kt | 12 + .../agendula/data/di/DataModule.kt | 2 +- .../data/tasks/TasksRepositoryImpl.kt | 5 +- .../data/tasks/provider/ProviderWriter.kt | 57 +- .../data/tasks/room/AdvancedOccurrence.kt | 73 ++ .../data/tasks/room/RoomTaskMapper.kt | 6 + .../data/tasks/room/RoomTasksDataSource.kt | 262 ++++--- .../data/tasks/room/SeriesCopyMark.kt | 115 --- .../agendula/data/tasks/room/TaskDao.kt | 4 - .../agendula/data/tasks/room/TasksDatabase.kt | 26 +- .../data/tasks/room/SeriesCopyMarkTest.kt | 35 - 14 files changed, 1129 insertions(+), 285 deletions(-) create mode 100644 app/schemas/de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase/5.json create mode 100644 app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/AdvancedOccurrence.kt delete mode 100644 app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/SeriesCopyMark.kt delete mode 100644 app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/room/SeriesCopyMarkTest.kt diff --git a/app/schemas/de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase/5.json b/app/schemas/de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase/5.json new file mode 100644 index 0000000..d36aa6d --- /dev/null +++ b/app/schemas/de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase/5.json @@ -0,0 +1,675 @@ +{ + "formatVersion": 1, + "database": { + "version": 5, + "identityHash": "319fd369dabf77ad0743bec42d6aab1d", + "entities": [ + { + "tableName": "accounts", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `display_name` TEXT NOT NULL, `principal_url` TEXT, `home_set_url` TEXT, `username` TEXT, `last_sync_at` INTEGER, `last_sync_error` TEXT, `label` TEXT)", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "displayName", + "columnName": "display_name", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "principalUrl", + "columnName": "principal_url", + "affinity": "TEXT" + }, + { + "fieldPath": "homeSetUrl", + "columnName": "home_set_url", + "affinity": "TEXT" + }, + { + "fieldPath": "username", + "columnName": "username", + "affinity": "TEXT" + }, + { + "fieldPath": "lastSyncAt", + "columnName": "last_sync_at", + "affinity": "INTEGER" + }, + { + "fieldPath": "lastSyncError", + "columnName": "last_sync_error", + "affinity": "TEXT" + }, + { + "fieldPath": "label", + "columnName": "label", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "id" + ] + } + }, + { + "tableName": "task_lists", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `name` TEXT NOT NULL, `color` INTEGER NOT NULL, `account_id` INTEGER, `is_visible` INTEGER NOT NULL DEFAULT 1, `is_synced` INTEGER NOT NULL DEFAULT 1, `owner` TEXT, `is_read_only` INTEGER NOT NULL DEFAULT 0, `sort_order` INTEGER NOT NULL DEFAULT 0, `href` TEXT, `ctag` TEXT, `sync_token` TEXT, `is_dirty` INTEGER NOT NULL DEFAULT 0, FOREIGN KEY(`account_id`) REFERENCES `accounts`(`id`) ON UPDATE NO ACTION ON DELETE SET NULL )", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "name", + "columnName": "name", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "color", + "columnName": "color", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "accountId", + "columnName": "account_id", + "affinity": "INTEGER" + }, + { + "fieldPath": "isVisible", + "columnName": "is_visible", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "1" + }, + { + "fieldPath": "isSynced", + "columnName": "is_synced", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "1" + }, + { + "fieldPath": "owner", + "columnName": "owner", + "affinity": "TEXT" + }, + { + "fieldPath": "isReadOnly", + "columnName": "is_read_only", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "sortOrder", + "columnName": "sort_order", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "href", + "columnName": "href", + "affinity": "TEXT" + }, + { + "fieldPath": "ctag", + "columnName": "ctag", + "affinity": "TEXT" + }, + { + "fieldPath": "syncToken", + "columnName": "sync_token", + "affinity": "TEXT" + }, + { + "fieldPath": "isDirty", + "columnName": "is_dirty", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_task_lists_account_id", + "unique": false, + "columnNames": [ + "account_id" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_task_lists_account_id` ON `${TABLE_NAME}` (`account_id`)" + } + ], + "foreignKeys": [ + { + "table": "accounts", + "onDelete": "SET NULL", + "onUpdate": "NO ACTION", + "columns": [ + "account_id" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "tasks", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `list_id` INTEGER NOT NULL, `uid` TEXT NOT NULL, `href` TEXT, `etag` TEXT, `title` TEXT, `description` TEXT, `location` TEXT, `url` TEXT, `color` INTEGER, `categories` TEXT NOT NULL DEFAULT '', `status` INTEGER NOT NULL DEFAULT 0, `percent_complete` INTEGER, `completed_at` INTEGER, `priority` INTEGER NOT NULL DEFAULT 0, `classification` INTEGER, `dtstart` INTEGER, `due` INTEGER, `duration` TEXT, `is_all_day` INTEGER NOT NULL DEFAULT 0, `timezone` TEXT, `rrule` TEXT, `rdate` TEXT, `exdate` TEXT, `recurrence_id` INTEGER, `master_id` INTEGER, `parent_id` INTEGER, `sort_order` INTEGER NOT NULL DEFAULT 0, `created_at` INTEGER, `last_modified` INTEGER, `sequence` INTEGER NOT NULL DEFAULT 0, `is_dirty` INTEGER NOT NULL DEFAULT 0, `is_deleted` INTEGER NOT NULL DEFAULT 0, `unknown_properties` TEXT, `reminders_off` INTEGER NOT NULL DEFAULT 0, FOREIGN KEY(`list_id`) REFERENCES `task_lists`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE , FOREIGN KEY(`master_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE , FOREIGN KEY(`parent_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE SET NULL )", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "listId", + "columnName": "list_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "uid", + "columnName": "uid", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "href", + "columnName": "href", + "affinity": "TEXT" + }, + { + "fieldPath": "etag", + "columnName": "etag", + "affinity": "TEXT" + }, + { + "fieldPath": "title", + "columnName": "title", + "affinity": "TEXT" + }, + { + "fieldPath": "description", + "columnName": "description", + "affinity": "TEXT" + }, + { + "fieldPath": "location", + "columnName": "location", + "affinity": "TEXT" + }, + { + "fieldPath": "url", + "columnName": "url", + "affinity": "TEXT" + }, + { + "fieldPath": "color", + "columnName": "color", + "affinity": "INTEGER" + }, + { + "fieldPath": "categories", + "columnName": "categories", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + }, + { + "fieldPath": "status", + "columnName": "status", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "percentComplete", + "columnName": "percent_complete", + "affinity": "INTEGER" + }, + { + "fieldPath": "completedAt", + "columnName": "completed_at", + "affinity": "INTEGER" + }, + { + "fieldPath": "priority", + "columnName": "priority", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "classification", + "columnName": "classification", + "affinity": "INTEGER" + }, + { + "fieldPath": "dtstart", + "columnName": "dtstart", + "affinity": "INTEGER" + }, + { + "fieldPath": "due", + "columnName": "due", + "affinity": "INTEGER" + }, + { + "fieldPath": "duration", + "columnName": "duration", + "affinity": "TEXT" + }, + { + "fieldPath": "isAllDay", + "columnName": "is_all_day", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "timezone", + "columnName": "timezone", + "affinity": "TEXT" + }, + { + "fieldPath": "rrule", + "columnName": "rrule", + "affinity": "TEXT" + }, + { + "fieldPath": "rdate", + "columnName": "rdate", + "affinity": "TEXT" + }, + { + "fieldPath": "exdate", + "columnName": "exdate", + "affinity": "TEXT" + }, + { + "fieldPath": "recurrenceId", + "columnName": "recurrence_id", + "affinity": "INTEGER" + }, + { + "fieldPath": "masterId", + "columnName": "master_id", + "affinity": "INTEGER" + }, + { + "fieldPath": "parentId", + "columnName": "parent_id", + "affinity": "INTEGER" + }, + { + "fieldPath": "sortOrder", + "columnName": "sort_order", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "createdAt", + "columnName": "created_at", + "affinity": "INTEGER" + }, + { + "fieldPath": "lastModified", + "columnName": "last_modified", + "affinity": "INTEGER" + }, + { + "fieldPath": "sequence", + "columnName": "sequence", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "isDirty", + "columnName": "is_dirty", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "isDeleted", + "columnName": "is_deleted", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "unknownProperties", + "columnName": "unknown_properties", + "affinity": "TEXT" + }, + { + "fieldPath": "remindersOff", + "columnName": "reminders_off", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_tasks_list_id_is_deleted", + "unique": false, + "columnNames": [ + "list_id", + "is_deleted" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_tasks_list_id_is_deleted` ON `${TABLE_NAME}` (`list_id`, `is_deleted`)" + }, + { + "name": "index_tasks_parent_id", + "unique": false, + "columnNames": [ + "parent_id" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_tasks_parent_id` ON `${TABLE_NAME}` (`parent_id`)" + }, + { + "name": "index_tasks_master_id_recurrence_id", + "unique": false, + "columnNames": [ + "master_id", + "recurrence_id" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_tasks_master_id_recurrence_id` ON `${TABLE_NAME}` (`master_id`, `recurrence_id`)" + }, + { + "name": "index_tasks_is_dirty", + "unique": false, + "columnNames": [ + "is_dirty" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_tasks_is_dirty` ON `${TABLE_NAME}` (`is_dirty`)" + }, + { + "name": "index_tasks_list_id_uid_recurrence_id", + "unique": true, + "columnNames": [ + "list_id", + "uid", + "recurrence_id" + ], + "orders": [], + "createSql": "CREATE UNIQUE INDEX IF NOT EXISTS `index_tasks_list_id_uid_recurrence_id` ON `${TABLE_NAME}` (`list_id`, `uid`, `recurrence_id`)" + } + ], + "foreignKeys": [ + { + "table": "task_lists", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "list_id" + ], + "referencedColumns": [ + "id" + ] + }, + { + "table": "tasks", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "master_id" + ], + "referencedColumns": [ + "id" + ] + }, + { + "table": "tasks", + "onDelete": "SET NULL", + "onUpdate": "NO ACTION", + "columns": [ + "parent_id" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "task_alarms", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `task_id` INTEGER NOT NULL, `minutes_before` INTEGER NOT NULL, `reference` TEXT NOT NULL DEFAULT 'DUE', `message` TEXT, `uid` TEXT NOT NULL DEFAULT '', `raw` TEXT, FOREIGN KEY(`task_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "taskId", + "columnName": "task_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "minutesBefore", + "columnName": "minutes_before", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "reference", + "columnName": "reference", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "'DUE'" + }, + { + "fieldPath": "message", + "columnName": "message", + "affinity": "TEXT" + }, + { + "fieldPath": "uid", + "columnName": "uid", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + }, + { + "fieldPath": "raw", + "columnName": "raw", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_task_alarms_task_id", + "unique": false, + "columnNames": [ + "task_id" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_task_alarms_task_id` ON `${TABLE_NAME}` (`task_id`)" + } + ], + "foreignKeys": [ + { + "table": "tasks", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "task_id" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "advanced_occurrences", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`copy_id` INTEGER NOT NULL, `master_id` INTEGER NOT NULL, `anchor` INTEGER NOT NULL, `advanced_to` INTEGER NOT NULL, `prior_dtstart` INTEGER, `prior_due` INTEGER, `prior_status` INTEGER NOT NULL, `prior_percent_complete` INTEGER, `prior_completed_at` INTEGER, `spent` INTEGER NOT NULL, `dropped_exdate` TEXT, `from_override` INTEGER NOT NULL, `detached` TEXT NOT NULL DEFAULT '', PRIMARY KEY(`copy_id`), FOREIGN KEY(`copy_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE , FOREIGN KEY(`master_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "copyId", + "columnName": "copy_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "masterId", + "columnName": "master_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "anchor", + "columnName": "anchor", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "advancedTo", + "columnName": "advanced_to", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "priorDtstart", + "columnName": "prior_dtstart", + "affinity": "INTEGER" + }, + { + "fieldPath": "priorDue", + "columnName": "prior_due", + "affinity": "INTEGER" + }, + { + "fieldPath": "priorStatus", + "columnName": "prior_status", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "priorPercentComplete", + "columnName": "prior_percent_complete", + "affinity": "INTEGER" + }, + { + "fieldPath": "priorCompletedAt", + "columnName": "prior_completed_at", + "affinity": "INTEGER" + }, + { + "fieldPath": "spent", + "columnName": "spent", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "droppedExdate", + "columnName": "dropped_exdate", + "affinity": "TEXT" + }, + { + "fieldPath": "fromOverride", + "columnName": "from_override", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "detached", + "columnName": "detached", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "copy_id" + ] + }, + "indices": [ + { + "name": "index_advanced_occurrences_master_id_anchor", + "unique": false, + "columnNames": [ + "master_id", + "anchor" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_advanced_occurrences_master_id_anchor` ON `${TABLE_NAME}` (`master_id`, `anchor`)" + } + ], + "foreignKeys": [ + { + "table": "tasks", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "copy_id" + ], + "referencedColumns": [ + "id" + ] + }, + { + "table": "tasks", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "master_id" + ], + "referencedColumns": [ + "id" + ] + } + ] + } + ], + "setupQueries": [ + "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '319fd369dabf77ad0743bec42d6aab1d')" + ] + } +} \ No newline at end of file diff --git a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/provider/ProviderWriterTest.kt b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/provider/ProviderWriterTest.kt index 2bb83e5..0348c59 100644 --- a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/provider/ProviderWriterTest.kt +++ b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/provider/ProviderWriterTest.kt @@ -797,6 +797,20 @@ class ProviderWriterTest { private fun store() = RoomTasksDataSource(db) + @Test + fun reopeningATickedOccurrencesCopyMovesTheSeriesBack() { + val series = db.tasks().insert( + TaskEntity(listId = home, uid = "w", title = "Weekly", dtstart = today, rrule = "FREQ=WEEKLY", timezone = "UTC"), + ) + store().setCompletedInstance(series, today, true) + val copy = db.tasks().allIn(home).single { it.id != series } + + writer.update(ProviderTable.TASKS, mapOf(Tasks.STATUS to 0), byId(ProviderTable.TASKS, copy.id), null) + + assertThat(task(series)!!.dtstart).isEqualTo(today) + assertThat(task(copy.id)).isNull() + } + /** Every column of one published row, as a client that read it would write it back. */ private fun readRow(table: ProviderTable, id: Long): Map { val columns = table.columns.map { it.first } 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 c33e051..d28770d 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 @@ -508,6 +508,134 @@ class RoomTasksDataSourceTest { assertThat(master.dtstart!!).isGreaterThan(at + (366 * 10).days) } + @Test + fun reopeningKeepsWhatChangedOnTheSeriesSince() { + val at = now + val id = source.insertTask(form(title = "Weekly")) + makeRecurring(id, at) + source.setCompletedInstance(id, at, completed = true) + source.deleteOccurrence(id, at + 28.days) + + source.setCompletedInstance(id, at, completed = false) + + val master = db.tasks().entity(id)!! + assertThat(master.dtstart).isEqualTo(at) + assertThat(master.exdate).isNotNull() + assertThat(source.tasks(TaskQuery(listId = listId)).map { it.occurrenceStart?.epochSeconds }) + .doesNotContain((at + 28.days).epochSeconds) + } + + @Test + fun aSeriesWithExtraDatesKeepsTheOverrideModel() { + val at = now + val id = source.insertTask(form(title = "Weekly")) + makeRecurring(id, at) + db.tasks().update(db.tasks().entity(id)!!.copy(rdate = "20991231T090000Z")) + + source.setCompletedInstance(id, at, completed = true) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at) + assertThat(db.tasks().overrides(id).single().status).isEqualTo(TaskStatus.COMPLETED) + } + + @Test + fun aDeletedSeriesIsNotBroughtBackByItsCopy() { + val at = now + val work = syncedList() + val id = source.insertTask(form(title = "Weekly").copy(listId = work)) + makeRecurring(id, at) + source.setCompletedInstance(id, at, completed = true) + val copy = db.tasks().allIn(work).single { it.id != id } + source.deleteWithSubtasks(id) + + source.setCompleted(copy.id, completed = false) + + assertThat(db.tasks().entity(copy.id)!!.status).isEqualTo(TaskStatus.NEEDS_ACTION) + assertThat(db.tasks().entity(copy.id)!!.isDeleted).isFalse() + } + + @Test + fun aSeriesEndedOnItsLastOpenOccurrenceReopens() { + val at = now + val id = source.insertTask(form(title = "Three")) + makeRecurring(id, at, rule = "FREQ=WEEKLY;COUNT=3") + val master = db.tasks().entity(id)!! + listOf(at, at + 7.days).forEach { done -> + db.tasks().insert(master.copy(id = 0, masterId = id, recurrenceId = done, rrule = null, status = TaskStatus.COMPLETED)) + } + + source.setCompletedInstance(id, at + 14.days, completed = true) + assertThat(db.tasks().entity(id)!!.status).isEqualTo(TaskStatus.COMPLETED) + + source.setCompletedInstance(id, at + 14.days, completed = false) + assertThat(db.tasks().entity(id)!!.status).isEqualTo(TaskStatus.NEEDS_ACTION) + } + + @Test + fun reopeningAnOlderCopyReopensItOnItsOwn() { + val at = now + val id = source.insertTask(form(title = "Weekly")) + makeRecurring(id, at) + source.setCompletedInstance(id, at, completed = true) + source.setCompletedInstance(id, at + 7.days, completed = true) + + source.setCompletedInstance(id, at, completed = false) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at + 14.days) + val reopened = db.tasks().allIn(listId).single { it.id != id && it.dtstart == at } + assertThat(reopened.status).isEqualTo(TaskStatus.NEEDS_ACTION) + assertThat(db.advancedOccurrences().forCopy(reopened.id)).isNull() + } + + @Test + fun aSeriesMovesPastOccurrencesAlreadyClosedAfterIt() { + val at = now + val id = source.insertTask(form(title = "Weekly")) + makeRecurring(id, at) + val master = db.tasks().entity(id)!! + db.tasks().insert(master.copy(id = 0, masterId = id, recurrenceId = at + 7.days, rrule = null, status = TaskStatus.COMPLETED)) + + source.setCompletedInstance(id, at, completed = true) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at + 14.days) + assertThat(db.tasks().overrides(id)).isEmpty() + } + + @Test + fun aDetachedOccurrenceDeletedSinceStaysExcluded() { + 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.deleteTask(earlier) + + source.setCompletedInstance(id, at + 7.days, completed = false) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at) + assertThat(source.tasks(TaskQuery(listId = listId)).map { it.occurrenceStart?.epochSeconds }) + .doesNotContain(at.epochSeconds) + } + + @Test + fun aNestedComponentKeepsItsOwnUid() { + val at = now + val id = source.insertTask(form(title = "Weekly")) + makeRecurring(id, at) + val master = db.tasks().entity(id)!! + val override = db.tasks().insert( + master.copy( + id = 0, masterId = id, recurrenceId = at, rrule = null, + unknownProperties = "BEGIN:VALARM\r\nUID:alarm-1\r\nACTION:AUDIO\r\nEND:VALARM", + ), + ) + + source.setCompletedInstance(override, at, completed = true) + + assertThat(db.tasks().entity(override)!!.unknownProperties!!.lines()).contains("UID:alarm-1") + } + @Test fun completingANonRecurringTaskThroughTheInstancePathWritesTheRowItself() { val id = source.insertTask(form(title = "Buy milk", due = now + 1.days)) diff --git a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/TasksDatabaseMigrationTest.kt b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/TasksDatabaseMigrationTest.kt index 9e37f7e..ed7a030 100644 --- a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/TasksDatabaseMigrationTest.kt +++ b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/room/TasksDatabaseMigrationTest.kt @@ -162,6 +162,18 @@ class TasksDatabaseMigrationTest { } } + @Test + fun addsTheAdvancedOccurrencesTable() { + helper.createDatabase(TEST_DB, 4).close() + + helper.runMigrationsAndValidate(TEST_DB, 5, true, TasksDatabase.MIGRATION_4_5).use { db -> + db.query("SELECT COUNT(*) FROM advanced_occurrences").use { c -> + assertThat(c.moveToFirst()).isTrue() + assertThat(c.getInt(0)).isEqualTo(0) + } + } + } + private companion object { const val TEST_DB = "migration-test.db" } 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 7190753..26a4a9e 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 @@ -89,7 +89,7 @@ object DataProvideModule { // backed-up `.db` does not. The backup rules carry all three files and // the app checkpoints on ON_STOP. .setJournalMode(RoomDatabase.JournalMode.WRITE_AHEAD_LOGGING) - .addMigrations(TasksDatabase.MIGRATION_1_2, TasksDatabase.MIGRATION_2_3, TasksDatabase.MIGRATION_3_4) + .addMigrations(TasksDatabase.MIGRATION_1_2, TasksDatabase.MIGRATION_2_3, TasksDatabase.MIGRATION_3_4, TasksDatabase.MIGRATION_4_5) .build() /** 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 5f77c29..e418e09 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 @@ -201,7 +201,8 @@ class TasksRepositoryImpl @Inject constructor( // Going through the occurrence forks an override instead, which takes // the reminders on its own row and leaves the series' alone. val instance = current?.takeIf { it.isRecurring }?.occurrenceStart - if (instance == null) dataSource.updateTask(taskId, form, reminders) + // The row the read resolved: a past occurrence may have become a task of its own. + if (instance == null) dataSource.updateTask(current?.taskId ?: taskId, form, reminders) else dataSource.updateInstance(taskId, instance, form, reminders) } @@ -235,7 +236,7 @@ class TasksRepositoryImpl @Inject constructor( val current = dataSource.task(taskId, occurrenceStart) val seriesId = current?.seriesId val occurrence = current?.occurrenceStart - if (seriesId == null || occurrence == null) return@withContext dataSource.deleteWithSubtasks(taskId) + if (seriesId == null || occurrence == null) return@withContext dataSource.deleteWithSubtasks(current?.taskId ?: taskId) when (scope) { RecurringScope.ThisOccurrence -> // An override row deletes itself into an EXDATE; a generated diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/provider/ProviderWriter.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/provider/ProviderWriter.kt index 8afa7ca..a5e99ef 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/provider/ProviderWriter.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/provider/ProviderWriter.kt @@ -12,6 +12,7 @@ import de.jeanlucmakiola.agendula.data.tasks.TasksContract.Properties import de.jeanlucmakiola.agendula.data.tasks.TasksContract.Tasks import de.jeanlucmakiola.agendula.data.tasks.room.AlarmReference import de.jeanlucmakiola.agendula.data.tasks.room.RoomTasksDataSource +import de.jeanlucmakiola.agendula.data.tasks.room.isClosed import de.jeanlucmakiola.agendula.data.tasks.room.TaskAlarmEntity import de.jeanlucmakiola.agendula.data.tasks.room.TaskEntity import de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase @@ -62,38 +63,33 @@ class ProviderWriter internal constructor( private var batchWrote = false /** Inserts one row and returns its published id. */ - fun insert(table: ProviderTable, values: ProviderValues): Long = writable(table, INSERTABLE_TABLES) { - when (table) { - ProviderTable.TASKS -> insertTask(values) - else -> insertProperty(values) - } + fun insert(table: ProviderTable, values: ProviderValues): Long = when (table) { + ProviderTable.TASKS -> write { insertTask(values) } + ProviderTable.PROPERTIES -> write { insertProperty(values) } + ProviderTable.LISTS, ProviderTable.INSTANCES -> readOnly(table) } /** Updates every row [selection] matches and returns how many changed. */ - fun update(table: ProviderTable, values: ProviderValues, selection: String?, args: Array?): Int = writable(table, WRITABLE_TABLES) { - when (table) { - ProviderTable.TASKS -> { - val writable = ProviderTaskWriter.writable(table, values, WRITABLE_TASK_COLUMNS) - val matched = ids(table, selection, args) - (if (Tasks.LIST_ID in writable) parentsFirst(matched) else matched).count { updateTask(it, writable) } - } - ProviderTable.INSTANCES -> { - val writable = ProviderTaskWriter.writable(table, values, WRITABLE_TASK_COLUMNS) - instances(selection, args).count { updateInstance(it, writable) } - } - ProviderTable.PROPERTIES -> ids(table, selection, args).count { updateProperty(it, values) } - else -> error("unreachable") + fun update(table: ProviderTable, values: ProviderValues, selection: String?, args: Array?): Int = when (table) { + ProviderTable.TASKS -> write { + val writable = ProviderTaskWriter.writable(table, values, WRITABLE_TASK_COLUMNS) + val matched = ids(table, selection, args) + (if (Tasks.LIST_ID in writable) parentsFirst(matched) else matched).count { updateTask(it, writable) } } + ProviderTable.INSTANCES -> write { + val writable = ProviderTaskWriter.writable(table, values, WRITABLE_TASK_COLUMNS) + instances(selection, args).count { updateInstance(it, writable) } + } + ProviderTable.PROPERTIES -> write { ids(table, selection, args).count { updateProperty(it, values) } } + ProviderTable.LISTS -> readOnly(table) } /** Deletes every row [selection] matches and returns how many went. */ - fun delete(table: ProviderTable, selection: String?, args: Array?): Int = writable(table, WRITABLE_TABLES) { - when (table) { - ProviderTable.TASKS -> ids(table, selection, args).count { deleteTask(it) } - ProviderTable.INSTANCES -> instances(selection, args).count { deleteInstance(it) } - ProviderTable.PROPERTIES -> ids(table, selection, args).count { deleteProperty(it) } - else -> error("unreachable") - } + fun delete(table: ProviderTable, selection: String?, args: Array?): Int = when (table) { + ProviderTable.TASKS -> write { ids(table, selection, args).count { deleteTask(it) } } + ProviderTable.INSTANCES -> write { instances(selection, args).count { deleteInstance(it) } } + ProviderTable.PROPERTIES -> write { ids(table, selection, args).count { deleteProperty(it) } } + ProviderTable.LISTS -> readOnly(table) } /** Runs a client's batch in one transaction, re-arming reminders once at the end. */ @@ -113,10 +109,8 @@ class ProviderWriter internal constructor( } /** A read-only table is refused as such, whichever store is active. */ - private fun writable(table: ProviderTable, allowed: Set, block: () -> T): T { - if (table !in allowed) throw UnsupportedOperationException("${table.path} is read-only") - return write(block) - } + /** Refused as read-only whichever store is active, before the store is even looked at. */ + private fun readOnly(table: ProviderTable): Nothing = throw UnsupportedOperationException("${table.path} is read-only") private fun write(block: () -> T): T = source.locked { check(resolver.mode() == StorageMode.OWN) { @@ -155,6 +149,8 @@ class ProviderWriter internal constructor( requireWritableList(current.listId) val edited = ProviderTaskWriter.patch(current, values, clock.now(), zone()) if (edited == current) return true + // Reopening the copy of a ticked occurrence moves its series back, as in the app. + if (current.isClosed && !edited.isClosed && store.reopenAdvanced(taskId)) return true val moved = edited.listId != current.listId if (moved) { requireWritableList(edited.listId) @@ -440,6 +436,3 @@ class ProviderWriter internal constructor( private fun zone(): String = ZoneId.systemDefault().id } - -private val INSERTABLE_TABLES = setOf(ProviderTable.TASKS, ProviderTable.PROPERTIES) -private val WRITABLE_TABLES = INSERTABLE_TABLES + ProviderTable.INSTANCES 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 new file mode 100644 index 0000000..21c09aa --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/AdvancedOccurrence.kt @@ -0,0 +1,73 @@ +package de.jeanlucmakiola.agendula.data.tasks.room + +import androidx.room.ColumnInfo +import androidx.room.Dao +import androidx.room.Entity +import androidx.room.ForeignKey +import androidx.room.Index +import androidx.room.Insert +import androidx.room.OnConflictStrategy +import androidx.room.PrimaryKey +import androidx.room.Query +import de.jeanlucmakiola.agendula.domain.TaskStatus +import kotlin.time.Instant + +/** + * How a series was moved past one of its occurrences, kept so reopening the + * completed copy can move it back (#137). Local only: never part of a `VTODO`. + * Holds the change, not the old series, so edits made since are kept. + */ +@Entity( + tableName = "advanced_occurrences", + foreignKeys = [ + ForeignKey(entity = TaskEntity::class, parentColumns = ["id"], childColumns = ["copy_id"], onDelete = ForeignKey.CASCADE), + ForeignKey(entity = TaskEntity::class, parentColumns = ["id"], childColumns = ["master_id"], onDelete = ForeignKey.CASCADE), + ], + indices = [Index(value = ["master_id", "anchor"])], +) +data class AdvancedOccurrenceEntity( + /** The completed copy of the occurrence. */ + @PrimaryKey @ColumnInfo(name = "copy_id") val copyId: Long, + @ColumnInfo(name = "master_id") val masterId: Long, + @ColumnInfo(name = "anchor") val anchor: Instant, + /** The anchor the series moved to; it is only moved back while it still has it. */ + @ColumnInfo(name = "advanced_to") val advancedTo: Instant, + @ColumnInfo(name = "prior_dtstart") val priorDtstart: Instant?, + @ColumnInfo(name = "prior_due") val priorDue: Instant?, + @ColumnInfo(name = "prior_status") val priorStatus: TaskStatus, + @ColumnInfo(name = "prior_percent_complete") val priorPercentComplete: Int?, + @ColumnInfo(name = "prior_completed_at") val priorCompletedAt: Instant?, + /** Occurrences `COUNT` lost to the move. */ + @ColumnInfo(name = "spent") val spent: Int, + /** `EXDATE` values the move dropped, as they were written. */ + @ColumnInfo(name = "dropped_exdate") val droppedExdate: String?, + /** The copy was an override before, and goes back to being one. */ + @ColumnInfo(name = "from_override") val fromOverride: Boolean, + /** Closed overrides detached with it: `anchor:taskId` pairs, comma-separated. */ + @ColumnInfo(name = "detached", defaultValue = "") val detached: String = "", +) { + val detachedRows: List> + get() = detached.split(',').filter { it.isNotEmpty() }.map { + Instant.fromEpochMilliseconds(it.substringBefore(':').toLong()) to it.substringAfter(':').toLong() + } + + companion object { + fun encodeDetached(rows: List>) = + rows.joinToString(",") { "${it.first.toEpochMilliseconds()}:${it.second}" } + } +} + +@Dao +interface AdvancedOccurrenceDao { + @Insert(onConflict = OnConflictStrategy.REPLACE) + fun insert(row: AdvancedOccurrenceEntity) + + @Query("SELECT * FROM advanced_occurrences WHERE copy_id = :copyId") + fun forCopy(copyId: Long): AdvancedOccurrenceEntity? + + @Query("SELECT * FROM advanced_occurrences WHERE master_id = :masterId AND anchor / 1000 = :anchorSecond") + fun at(masterId: Long, anchorSecond: Long): AdvancedOccurrenceEntity? + + @Query("DELETE FROM advanced_occurrences WHERE copy_id = :copyId") + fun delete(copyId: Long) +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTaskMapper.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTaskMapper.kt index fb67ce8..44be52e 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTaskMapper.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/RoomTaskMapper.kt @@ -1,5 +1,7 @@ package de.jeanlucmakiola.agendula.data.tasks.room +import de.jeanlucmakiola.agendula.domain.TaskStatus + import de.jeanlucmakiola.agendula.domain.LocalAccount import de.jeanlucmakiola.agendula.domain.Task import de.jeanlucmakiola.agendula.domain.TaskList @@ -100,6 +102,10 @@ object RoomTaskMapper { val TaskEntity.isRecurring: Boolean get() = !rrule.isNullOrBlank() || !rdate.isNullOrBlank() +/** Completed or cancelled. */ +val TaskEntity.isClosed: Boolean + get() = status == TaskStatus.COMPLETED || status == TaskStatus.CANCELLED + /** * The series anchor: `DTSTART` when present, else `DUE`. A `VTODO` may carry only * a due date, and RFC 5545 then anchors the recurrence on it — matching how the 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 f3bdb0c..8502443 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 @@ -108,6 +108,7 @@ class RoomTasksDataSource @Inject constructor( private val tasks get() = database.tasks() private val lists get() = database.taskLists() private val alarms get() = database.alarms() + private val advanced get() = database.advancedOccurrences() // --- reads ---------------------------------------------------------------- @@ -137,7 +138,7 @@ class RoomTasksDataSource @Inject constructor( if (row.task.recurrenceId != null || !row.task.isRecurring) return task(taskId) return occurrencesOf(row, tasks.overrides(taskId), clock.now()) .firstOrNull { it.occurrenceStart == occurrenceStart } - ?: copyOf(row.task, occurrenceStart)?.let { copy -> + ?: copyAt(row.task, occurrenceStart)?.let { copy -> tasks.task(copy.id)?.let { RoomTaskMapper.task(it).copy(occurrenceStart = occurrenceStart) } } ?: task(taskId) @@ -331,11 +332,7 @@ class RoomTasksDataSource @Inject constructor( return@tx } val now = clock.now() - // COUNT counts before EXDATE is applied (RFC 5545 §3.8.5.3). - val spent = RecurrenceExpander.expand( - spec.copy(exdate = null), - ExpansionWindow(from = anchor, until = occurrenceStart, maxOccurrences = SPLIT_COUNT_CEILING), - ).size + val spent = spentBefore(spec, occurrenceStart) val (start, due) = resolvedTimes(master, occurrenceStart) tasks.update(TaskFormWriter.endedBefore(master, occurrenceStart, now, zone())) @@ -358,7 +355,7 @@ class RoomTasksDataSource @Inject constructor( percentComplete = null, completedAt = null, // Recurrence lines belong to the old series; everything else carries over. - unknownProperties = seriesFree(master.unknownProperties).joinToString("\r\n").ifBlank { null }, + unknownProperties = seriesFreeText(master.unknownProperties), ) val applied = TaskFormWriter.apply(base, form.copy(percentComplete = null), now, zone()) val delta = applied.dtstart?.let { newStart -> start?.let { newStart - it } } @@ -492,13 +489,13 @@ class RoomTasksDataSource @Inject constructor( /** * Ticking a series' first open occurrence moves the series on to its next - * one and keeps a completed copy, as tasks.org and Apple Reminders do — a + * open one and keeps a completed copy, as tasks.org and Apple Reminders do — a * completed `RECURRENCE-ID` override alone leaves the series showing as open - * in most other clients (#137). Occurrences already closed as overrides ahead - * of it become copies too. Reopening the copy moves the series back. Any other - * occurrence forks an override carrying the completion, the same model - * [updateInstance] writes. Writing the status onto the master instead would - * close the series. + * in most other clients (#137). Occurrences already closed as overrides on + * either side of it become copies too. Reopening the copy moves the series + * back. Any other occurrence forks an override carrying the completion, the + * same model [updateInstance] writes. Writing the status onto the master + * instead would close the series. */ override fun setCompletedInstance(taskId: Long, occurrenceStart: Instant, completed: Boolean) = tx { closeOccurrence(taskId, occurrenceStart, completed, "complete") { TaskFormWriter.completed(it, completed, clock.now()) } @@ -509,6 +506,13 @@ class RoomTasksDataSource @Inject constructor( closeOccurrence(taskId, occurrenceStart, cancelled, "cancel") { TaskFormWriter.cancelled(it, cancelled, clock.now()) } } + /** Moves the series behind the copy [taskId] back onto its occurrence; false when it is no such copy. */ + fun reopenAdvanced(taskId: Long): Boolean { + var reopened = false + tx { reopened = tasks.entity(taskId)?.let(::reopenCopy) == true } + return reopened + } + private fun closeOccurrence( taskId: Long, occurrenceStart: Instant, @@ -520,22 +524,21 @@ class RoomTasksDataSource @Inject constructor( val master = row.masterId?.let { tasks.entity(it) } ?: row if (master.recurrenceSpec() == null) { // A plain task, or the copy a ticked occurrence left: this row is the occurrence. - if (closing || !reopenCopy(row)) { - tasks.update(close(row)) - writes.onWritten(setOf(row.listId)) - } + if (closing || !reopenCopy(row)) writeRow(row, close) return } val override = if (row.masterId != null) row else tasks.override(master.id, occurrenceStart) val anchor = override?.recurrenceId ?: occurrenceStart val seriesStart = master.recurrenceAnchor ?: return if (anchor < seriesStart && !sameSecond(anchor, seriesStart)) { - // The series has already moved past it: a stale tick does nothing, a reopen takes it back. - if (override != null) writeInstance(master, override, anchor, close) - else if (!closing) copyOf(master, anchor)?.let(::reopenCopy) + // The series has moved past it: a stale tick does nothing, a reopen takes its copy back. + when { + override != null -> writeInstance(master, override, anchor, close) + !closing -> copyAt(master, anchor)?.let { copy -> if (!reopenCopy(copy)) writeRow(copy, close) } + } return } - if (isClosed(master)) { + if (master.isClosed) { // Only its last occurrence closes a series; reopening that one reopens it. if (!closing && sameSecond(anchor, seriesStart)) closeSeries(master, override, close) else writeInstance(master, override, anchor, close) @@ -544,7 +547,12 @@ class RoomTasksDataSource @Inject constructor( if (!closing || !advance(master, anchor, override, close)) writeInstance(master, override, anchor, close) } - private fun isClosed(task: TaskEntity) = task.status == TaskStatus.COMPLETED || task.status == TaskStatus.CANCELLED + private fun writeRow(row: TaskEntity, close: (TaskEntity) -> TaskEntity) { + tasks.update(close(row)) + // Reopened on its own, it no longer stands for the series' occurrence. + if (!close(row).isClosed) advanced.delete(row.id) + writes.onWritten(setOf(row.listId)) + } /** [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) { @@ -553,65 +561,99 @@ class RoomTasksDataSource @Inject constructor( writes.onWritten(setOf(master.listId)) } - /** Moves [master] past [anchor] when that is its first open occurrence; false when it is not. */ + /** + * Moves [master] past [anchor] when that is its first open occurrence; false + * when it is not. A series with `RDATE`s is left to the override model: + * anchoring on an `RDATE` would re-align its `RRULE`. + */ private fun advance(master: TaskEntity, anchor: Instant, override: TaskEntity?, close: (TaskEntity) -> TaskEntity): Boolean { + if (master.rrule.isNullOrBlank() || !master.rdate.isNullOrBlank()) return false val spec = master.recurrenceSpec() ?: return false val overrides = tasks.overrides(master.id).associateBy { it.recurrenceId?.epochSeconds } - val closedAhead = overrides.values.count(::isClosed) - val leading = expandFrom(spec, maxOccurrences = closedAhead + 2) + fun closedAt(at: Instant) = overrides[at.epochSeconds]?.isClosed == true + val leading = expandFrom(spec, maxOccurrences = overrides.values.count { it.isClosed } + 2) val index = leading.indexOfFirst { sameSecond(it, anchor) } - if (index < 0 || leading.take(index).any { overrides[it.epochSeconds]?.let(::isClosed) != true }) return false - val next = leading.getOrNull(index + 1) ?: return true.also { closeSeries(master, override, close) } + if (index < 0 || leading.take(index).any { !closedAt(it) }) return false + var nextIndex = index + 1 + while (nextIndex < leading.size && closedAt(leading[nextIndex])) nextIndex++ + val next = leading.getOrNull(nextIndex) val now = clock.now() - val detached = leading.take(index).map { earlier -> earlier to detach(overrides.getValue(earlier.epochSeconds), now) } - val prior = Prior(master, fromOverride = override != null, detached = detached) + val zone = master.timezone ?: zone() + val detached = (leading.take(index) + leading.subList(index + 1, nextIndex)).map { at -> + at to detach(overrides.getValue(at.epochSeconds), now) + } + // Expansion drops the anchor's sub-second part; the series keeps it. + val offset = spec.anchor - floorSecond(spec.anchor) + val newAnchor = (next ?: leading[index]) + offset + val spent = spentBefore(spec, next ?: leading[index]) + val (start, due) = occurrenceTimes(master, newAnchor) + val moved = master.copy( + dtstart = start, + due = due, + rrule = master.rrule.let { TaskFormWriter.remainingCount(it, spent) }, + exdate = TaskFormWriter.valuesFrom(master.exdate, newAnchor, zone), + lastModified = now, + isDirty = true, + ) + if (next == null) { + // Its last occurrence: the series ends there, and reopening it reopens the series. + closeSeries(moved, override, close) + return true + } val copyId = if (override != null) { detach(override, now, close) } else { - val (start, due) = occurrenceTimes(master, anchor) - val id = tasks.insert(close(asCopy(master, now).copy(dtstart = start, due = due))) + val (copyStart, copyDue) = occurrenceTimes(master, anchor) + val id = tasks.insert(close(asCopy(master, now).copy(dtstart = copyStart, due = copyDue))) alarms.copyToTask(master.id, id, sameResource = false) id } - // Keeps the anchor's own sub-second part, which expansion drops. - val newAnchor = next + (spec.anchor - Instant.fromEpochSeconds(spec.anchor.epochSeconds)) - val spent = RecurrenceExpander.expand( - spec.copy(exdate = null), - ExpansionWindow(from = floorSecond(spec.anchor), until = next, maxOccurrences = SPLIT_COUNT_CEILING), - ).size - val (start, due) = occurrenceTimes(master, newAnchor) - val zone = master.timezone ?: zone() - tasks.update( - master.copy( - dtstart = start, - due = due, - rrule = master.rrule?.let { TaskFormWriter.remainingCount(it, spent) }, - exdate = TaskFormWriter.valuesFrom(master.exdate, newAnchor, zone), - rdate = TaskFormWriter.valuesFrom(master.rdate, newAnchor, zone), - status = TaskStatus.NEEDS_ACTION, - percentComplete = null, - completedAt = null, - lastModified = now, - isDirty = true, + tasks.update(moved.copy(status = TaskStatus.NEEDS_ACTION, percentComplete = null, completedAt = null)) + advanced.insert( + AdvancedOccurrenceEntity( + copyId = copyId, + masterId = master.id, + anchor = anchor, + advancedTo = newAnchor, + priorDtstart = master.dtstart, + priorDue = master.due, + priorStatus = master.status, + priorPercentComplete = master.percentComplete, + priorCompletedAt = master.completedAt, + spent = spent, + droppedExdate = TaskFormWriter.valuesBefore(master.exdate, newAnchor, zone), + fromOverride = override != null, + detached = AdvancedOccurrenceEntity.encodeDetached(detached), ), ) - val copy = tasks.entity(copyId)!! - val mark = Mark(master.id, master.uid, anchor, newAnchor.toEpochMilliseconds(), prior) - tasks.update(copy.copy(unknownProperties = (seriesFree(copy.unknownProperties) + mark.line()).joinToString("\r\n"))) writes.onWritten(setOf(master.listId)) return true } + /** 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), + ).size + /** * Turns the override [row] into a task of its own, in place so its id keeps - * working: a new UID, unlinked from the series, and a resource of its own. + * working: a new UID, unlinked from the series, a resource of its own — and + * so its alarms are fresh copies, not the series' ones. */ private fun detach(row: TaskEntity, now: Instant, close: (TaskEntity) -> TaskEntity = { it }): Long { - tasks.update(close(asCopy(row, now).copy(id = row.id, unknownProperties = seriesFree(row.unknownProperties).joinToString("\r\n").ifBlank { null }))) + tasks.update(close(asCopy(row, now).copy(id = row.id))) + refreshAlarms(row.id) return row.id } + private fun refreshAlarms(taskId: Long) { + val current = alarms.forTask(taskId) + alarms.deleteForTask(taskId) + current.forEach { alarms.insert(it.freshCopy(taskId)) } + } + /** [source] as a standalone task: no series fields, never uploaded. */ private fun asCopy(source: TaskEntity, now: Instant): TaskEntity = source.copy( id = 0, @@ -627,62 +669,82 @@ class RoomTasksDataSource @Inject constructor( sequence = 0, isDeleted = false, isDirty = true, - unknownProperties = seriesFree(source.unknownProperties).joinToString("\r\n").ifBlank { null }, + unknownProperties = seriesFreeText(source.unknownProperties), ) /** - * Undoes [advance] for the copy [row], when the series has not changed since: - * the master takes back its anchor and rule, and the occurrences it moved past - * become its overrides or generated occurrences again. False when [row] is no - * such copy, so the caller reopens it as a task of its own. + * Undoes [advance] for the copy [row] while the series still sits where it + * moved it: the anchor goes back, `COUNT` gets back what it spent and the + * dropped `EXDATE`s return; any other edit made since stays. The occurrences + * it moved past become overrides again — or, where their copy is gone, are + * excluded. False when [row] is no such copy, so the caller reopens it as a + * task of its own. */ private fun reopenCopy(row: TaskEntity): Boolean { - val mark = Mark.of(row) ?: return false - val master = tasks.entity(mark.masterId)?.takeIf { it.uid == mark.masterUid } ?: return false - if (master.recurrenceAnchor?.toEpochMilliseconds() != mark.advancedTo) return false - val now = clock.now() - val prior = mark.prior - tasks.update(prior.restore(master, now)) - prior.detached.forEach { (anchor, id) -> tasks.entity(id)?.let { toOverride(it, master, anchor, now) { r -> r } } } - if (prior.fromOverride) { - toOverride(row, master, mark.anchor, now) { TaskFormWriter.completed(it, false, now) } - } else { - deleteTask(row.id) + val record = advanced.forCopy(row.id) ?: return false + val master = tasks.entity(record.masterId) + ?.takeIf { !it.isDeleted && !it.isClosed && it.recurrenceAnchor?.let { a -> sameSecond(a, record.advancedTo) } == true } + if (master == null) { + advanced.delete(row.id) + return false } + val now = clock.now() + val zone = master.timezone ?: zone() + val (start, due) = occurrenceTimes(master, record.priorDtstart ?: record.priorDue ?: record.anchor) + var back = master.copy( + dtstart = start, + due = due, + rrule = master.rrule?.let { TaskFormWriter.remainingCount(it, -record.spent) }, + exdate = listOfNotNull(record.droppedExdate, master.exdate).joinToString(",").ifEmpty { null }, + status = if (master.status == TaskStatus.NEEDS_ACTION) record.priorStatus else master.status, + percentComplete = master.percentComplete ?: record.priorPercentComplete, + completedAt = master.completedAt ?: record.priorCompletedAt, + lastModified = now, + isDirty = true, + ) + val synced = lists.entity(master.listId)?.accountId != null + record.detachedRows.forEach { (anchor, id) -> + val detached = tasks.entity(id)?.takeIf { !it.isDeleted && it.listId == master.listId } + if (detached == null) back = TaskFormWriter.excepting(back, anchor, now, zone, dirty = synced) + } + tasks.update(back) + record.detachedRows.forEach { (anchor, id) -> + tasks.entity(id)?.takeIf { !it.isDeleted && it.listId == master.listId }?.let { toOverride(it, back, anchor, now) } + } + advanced.delete(row.id) + if (record.fromOverride) toOverride(TaskFormWriter.completed(row, false, now), back, record.anchor, now) + else deleteTask(row.id) writes.onWritten(setOf(master.listId, row.listId)) return true } - /** [row], a detached occurrence, back as [master]'s override for [anchor] — reopened by [reopen]. */ - private fun toOverride(row: TaskEntity, master: TaskEntity, anchor: Instant, now: Instant, reopen: (TaskEntity) -> TaskEntity) { - val back = reopen( - row.copy( - uid = master.uid, - masterId = master.id, - recurrenceId = anchor, - listId = master.listId, - href = null, - etag = null, - isDirty = true, - lastModified = now, - unknownProperties = seriesFree(row.unknownProperties).joinToString("\r\n").ifBlank { null }, - ), + /** [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( + uid = master.uid, + masterId = master.id, + recurrenceId = anchor, + listId = master.listId, + href = null, + etag = null, + isDirty = true, + lastModified = now, + unknownProperties = seriesFreeText(row.unknownProperties), ) if (row.href == null) { tasks.update(back) + refreshAlarms(row.id) } else { // Already uploaded as a resource of its own: that one goes, a fresh override comes. val id = tasks.insert(back.copy(id = 0)) - alarms.copyToTask(row.id, id, sameResource = true) + alarms.copyToTask(row.id, id, sameResource = false) deleteTask(row.id) } } /** The copy [advance] left for [master]'s occurrence at [anchor], if it is still there. */ - private fun copyOf(master: TaskEntity, anchor: Instant): TaskEntity? = - tasks.withUnknownLike("%$OCCURRENCE_MARK${master.id};%").firstOrNull { copy -> - Mark.of(copy)?.let { it.masterUid == master.uid && sameSecond(it.anchor, anchor) } == true - } + private fun copyAt(master: TaskEntity, anchor: Instant): TaskEntity? = + advanced.at(master.id, anchor.epochSeconds)?.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 { @@ -692,9 +754,21 @@ class RoomTasksDataSource @Inject constructor( private fun floorSecond(at: Instant) = Instant.fromEpochSeconds(at.epochSeconds) - private fun seriesFree(unknown: String?): List = - unknown?.lines()?.filterNot { line -> SERIES_LINES.any { line.startsWith(it, ignoreCase = true) } || line.startsWith(OCCURRENCE_MARK) } - ?.filter { it.isNotBlank() }.orEmpty() + /** + * 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 + * own and stay. + */ + private fun seriesFreeText(unknown: String?): String? { + var depth = 0 + return unknown?.lines()?.filter { line -> + when { + line.startsWith("BEGIN:", ignoreCase = true) -> { depth++; true } + line.startsWith("END:", ignoreCase = true) -> { depth--; true } + else -> depth > 0 || (line.isNotBlank() && SERIES_LINES.none { line.startsWith(it, ignoreCase = true) }) + } + }?.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 diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/SeriesCopyMark.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/SeriesCopyMark.kt deleted file mode 100644 index 5a5eec3..0000000 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/SeriesCopyMark.kt +++ /dev/null @@ -1,115 +0,0 @@ -package de.jeanlucmakiola.agendula.data.tasks.room - -import de.jeanlucmakiola.agendula.domain.TaskStatus -import java.util.Base64 -import kotlin.time.Instant - -internal const val OCCURRENCE_MARK = "X-AGENDULA-OCCURRENCE-OF:" - -/** - * What a series looked like before ticking its first occurrence moved it on, so - * reopening the copy can put it back exactly — `COUNT`, `EXDATE` and `RDATE` - * included — with the occurrences it moved past as overrides again. - */ -internal data class Prior( - val dtstart: Instant?, - val due: Instant?, - val rrule: String?, - val rdate: String?, - val exdate: String?, - val status: TaskStatus, - val percentComplete: Int?, - val completedAt: Instant?, - val fromOverride: Boolean, - /** Earlier closed occurrences detached alongside: their anchor and the row they became. */ - val detached: List>, -) { - constructor(master: TaskEntity, fromOverride: Boolean, detached: List>) : this( - master.dtstart, master.due, master.rrule, master.rdate, master.exdate, - master.status, master.percentComplete, master.completedAt, fromOverride, detached, - ) - - fun restore(master: TaskEntity, now: Instant): TaskEntity = master.copy( - dtstart = dtstart, - due = due, - rrule = rrule, - rdate = rdate, - exdate = exdate, - status = status, - percentComplete = percentComplete, - completedAt = completedAt, - lastModified = now, - isDirty = true, - ) - - fun encode(): String { - val fields = listOf( - dtstart?.toEpochMilliseconds()?.toString(), - due?.toEpochMilliseconds()?.toString(), - rrule, - rdate, - exdate, - status.name, - percentComplete?.toString(), - completedAt?.toEpochMilliseconds()?.toString(), - fromOverride.toString(), - detached.joinToString(",") { "${it.first.toEpochMilliseconds()}:${it.second}" }, - ).joinToString("\n") { it.orEmpty() } - return Base64.getUrlEncoder().withoutPadding().encodeToString(fields.toByteArray()) - } - - companion object { - fun decode(raw: String): Prior? = runCatching { - val f = String(Base64.getUrlDecoder().decode(raw)).split('\n') - fun instant(i: Int) = f[i].toLongOrNull()?.let(Instant::fromEpochMilliseconds) - Prior( - dtstart = instant(0), - due = instant(1), - rrule = f[2].ifEmpty { null }, - rdate = f[3].ifEmpty { null }, - exdate = f[4].ifEmpty { null }, - status = TaskStatus.valueOf(f[5]), - percentComplete = f[6].toIntOrNull(), - completedAt = instant(7), - fromOverride = f[8].toBooleanStrict(), - detached = f[9].split(',').filter { it.isNotEmpty() }.map { pair -> - Instant.fromEpochMilliseconds(pair.substringBefore(':').toLong()) to pair.substringAfter(':').toLong() - }, - ) - }.getOrNull() - } -} - -/** - * The line a completed copy carries back to its series: - * `X-AGENDULA-OCCURRENCE-OF:;;;;`. - * Another device's copy names a master id that is not this one's, so it never matches. - */ -internal data class Mark( - val masterId: Long, - val masterUid: String, - val anchor: Instant, - val advancedTo: Long, - val prior: Prior, -) { - fun line(): String = - "$OCCURRENCE_MARK$masterId;$masterUid;${anchor.toEpochMilliseconds()};$advancedTo;${prior.encode()}" - - companion object { - fun of(task: TaskEntity): Mark? = task.unknownProperties?.lines() - ?.firstOrNull { it.startsWith(OCCURRENCE_MARK) } - ?.let { parse(it.removePrefix(OCCURRENCE_MARK)) } - - fun parse(value: String): Mark? { - val parts = value.split(';') - if (parts.size < 5) return null - return Mark( - masterId = parts.first().toLongOrNull() ?: return null, - masterUid = parts.subList(1, parts.size - 3).joinToString(";"), - anchor = parts[parts.size - 3].toLongOrNull()?.let(Instant::fromEpochMilliseconds) ?: return null, - advancedTo = parts[parts.size - 2].toLongOrNull() ?: return null, - prior = Prior.decode(parts.last()) ?: return 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 58fc44a..747ef2d 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 @@ -122,10 +122,6 @@ interface TaskDao { @Query("SELECT * FROM tasks WHERE list_id = :listId") fun allIn(listId: Long): List - /** Live rows whose unmodelled properties match the `LIKE` [pattern]. */ - @Query("SELECT * FROM tasks WHERE unknown_properties LIKE :pattern AND is_deleted = 0") - fun withUnknownLike(pattern: String): List - // --- writes --------------------------------------------------------------- @Insert diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TasksDatabase.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TasksDatabase.kt index 6e24f41..5a3050f 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TasksDatabase.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TasksDatabase.kt @@ -9,7 +9,7 @@ import java.util.UUID import kotlin.time.Instant /** - * Agendula's own task store. Four tables, designed from + * Agendula's own task store. Five tables, designed from * what the app actually reads and writes plus RFC 5545's `VTODO`. * * Schemas are exported to `app/schemas/` and committed, so a future version can @@ -21,8 +21,9 @@ import kotlin.time.Instant TaskListEntity::class, TaskEntity::class, TaskAlarmEntity::class, + AdvancedOccurrenceEntity::class, ], - version = 4, + version = 5, exportSchema = true, ) @TypeConverters(Converters::class) @@ -31,6 +32,7 @@ abstract class TasksDatabase : RoomDatabase() { abstract fun tasks(): TaskDao abstract fun alarms(): TaskAlarmDao abstract fun accounts(): AccountDao + abstract fun advancedOccurrences(): AdvancedOccurrenceDao companion object { const val NAME = "agendula-tasks.db" @@ -180,5 +182,25 @@ abstract class TasksDatabase : RoomDatabase() { db.execSQL("ALTER TABLE accounts ADD COLUMN label TEXT") } } + + /** v5 remembers how ticking an occurrence moved its series, so reopening it can move it back. */ + val MIGRATION_4_5 = object : Migration(4, 5) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL( + "CREATE TABLE IF NOT EXISTS `advanced_occurrences` (`copy_id` INTEGER NOT NULL, " + + "`master_id` INTEGER NOT NULL, `anchor` INTEGER NOT NULL, `advanced_to` INTEGER NOT NULL, " + + "`prior_dtstart` INTEGER, `prior_due` INTEGER, `prior_status` INTEGER NOT NULL, " + + "`prior_percent_complete` INTEGER, `prior_completed_at` INTEGER, `spent` INTEGER NOT NULL, " + + "`dropped_exdate` TEXT, `from_override` INTEGER NOT NULL, `detached` TEXT NOT NULL DEFAULT '', " + + "PRIMARY KEY(`copy_id`), " + + "FOREIGN KEY(`copy_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE , " + + "FOREIGN KEY(`master_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + ) + db.execSQL( + "CREATE INDEX IF NOT EXISTS `index_advanced_occurrences_master_id_anchor` " + + "ON `advanced_occurrences` (`master_id`, `anchor`)", + ) + } + } } } diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/room/SeriesCopyMarkTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/room/SeriesCopyMarkTest.kt deleted file mode 100644 index 96b9f63..0000000 --- a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/room/SeriesCopyMarkTest.kt +++ /dev/null @@ -1,35 +0,0 @@ -package de.jeanlucmakiola.agendula.data.tasks.room - -import com.google.common.truth.Truth.assertThat -import de.jeanlucmakiola.agendula.domain.TaskStatus -import org.junit.jupiter.api.Test -import kotlin.time.Instant - -class SeriesCopyMarkTest { - - private val prior = Prior( - dtstart = Instant.fromEpochMilliseconds(1_000), - due = null, - rrule = "FREQ=DAILY;COUNT=4", - rdate = null, - exdate = "20261107T090000Z,20261108T090000Z", - status = TaskStatus.IN_PROCESS, - percentComplete = 40, - completedAt = null, - fromOverride = true, - detached = listOf(Instant.fromEpochMilliseconds(500) to 12L), - ) - - @Test - fun `a mark survives its own line`() { - val mark = Mark(7, "uid;with;semicolons", Instant.fromEpochMilliseconds(1_000), 2_000, prior) - assertThat(Mark.parse(mark.line().removePrefix(OCCURRENCE_MARK))).isEqualTo(mark) - } - - @Test - fun `a damaged mark reads as none`() { - assertThat(Mark.parse("7;uid;1000;2000;!!!")).isNull() - assertThat(Mark.parse("x;uid;1000;2000;")).isNull() - assertThat(Mark.parse("7;uid")).isNull() - } -}