From 68495a936c9d379c831faa2bfa1858392980d70c Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Mon, 9 Nov 2026 18:19:57 +0100 Subject: [PATCH] Ticking a repeating task moves the series on (#137) (#144) In Agendula's own storage, ticking off the current occurrence of a repeating task now moves the series on to its next date and keeps a completed copy, as tasks.org and Apple Reminders do. Before, the series stayed open and the completion was a `RECURRENCE-ID` override with `STATUS:COMPLETED` in the same resource. That's valid iCalendar, but Infomaniak's web view and Apple Reminders ignore it and keep showing the task as open, which is what #137 reports. The reporter hasn't confirmed their storage mode yet. With tasks.org/OpenTasks the provider already moves the series on, so this change is about the built-in CalDAV sync. How it works (`RoomTasksDataSource`): - Ticking or cancelling the series' **first open occurrence** moves the master to its next open occurrence. Closed overrides on either side become standalone completed copies (new UID, fresh alarm identities), including the ones 1.3.0 left behind. `COUNT` drops by the occurrences used up (counted before `EXDATE`, shared with `splitSeries`), `EXDATE` values before the new start are dropped, and status and progress are reset. An edited occurrence is converted in place, so its id, edits and own properties survive. Lines inside nested components, such as a `VALARM`'s `UID`, are kept. - Series with `RDATE`s keep the override model, because moving the anchor onto an `RDATE` would re-align the `RRULE`. So do recurring subtasks, whose copies would otherwise pile up under the parent. - How the series moved is recorded in two new local tables, `advanced_occurrences` and `advanced_detached` (schema v5, migration and migration test included). They are never part of the synced `VTODO`. A record keeps mapping the occurrence to its copy (pruned after 30 days), so the detail screen and reminders find the copy even after it was edited. Every save path goes through one check: when the only thing that changes on a copy is that it reopens (compared as the editor writes values back), the move is undone. That covers undo, the completed section, the editor, the detail screen and other apps through Agendula's provider. The anchor goes back, `COUNT` regains what it spent, the dropped `EXDATE`s return in the list's own value shape, other edits made since are kept, and a reopened state such as 40% stays on the occurrence. Detached overrides come back as overrides; one deleted since, or given its own parent or subtasks, is excluded instead. A copy that was edited, moved or given subtasks, or whose series moved on, changed its rule or was deleted, reopens as a task of its own. - If a sync discards the local version of a moved series because the server's is newer, the copies of moves the server never saw are deleted too, and the server's own overrides stand for those occurrences. Moves already uploaded are marked settled and left alone. - `task(series, anchor)` resolves a moved-past occurrence to its copy, so the detail screen and reminder checks see it as done. The repository then acts on the row it resolved, including the detail screen's subtasks, so deleting or saving from a screen opened before the move can't hit the whole series. - A stale tick (an old notification or widget row) for an occurrence the series has already moved past does nothing. - Ticking the last open occurrence closes the series, and reopening any of its occurrences reopens the series. - Any other occurrence (with earlier ones still open) keeps the override model. Also fixed here: `AgendulaTaskProviderTest` failed on `release/v1.3.1` whenever Agendula ran on an external provider, because `ProviderWriter` checked the active store first. Read-only tables are now refused as such whichever store is active (one exhaustive `when` per operation), and the two tests that check value errors skip when the provider takes no writes at all. Tests: `RoomTasksDataSourceTest` covers each case above. There is also a migration test, a `ProviderWriterTest` for reopening through the provider, and `RoomSyncStoreAdvancesTest` (full flavour) for the sync discard. The full on-device suite (223 tests) passes on the emulator, and the unit tests pass for both flavours. Closes #137 Co-authored-by: Jean-Luc Makiola Reviewed-on: https://codeberg.org/jlmakiola/agendula/pulls/144 --- CHANGELOG.md | 7 + .../5.json | 768 ++++++++++++++++++ .../provider/AgendulaTaskProviderTest.kt | 5 + .../data/tasks/provider/ProviderWriterTest.kt | 20 +- .../tasks/room/RoomTasksDataSourceTest.kt | 539 +++++++++++- .../tasks/room/TasksDatabaseMigrationTest.kt | 12 + .../data/sync/RoomSyncStoreAdvancesTest.kt | 57 ++ .../agendula/data/sync/CollectionSyncer.kt | 1 + .../agendula/data/sync/SyncStore.kt | 28 +- .../agendula/data/di/DataModule.kt | 2 +- .../data/tasks/TasksRepositoryImpl.kt | 14 +- .../data/tasks/provider/ProviderWriter.kt | 49 +- .../data/tasks/room/AdvancedOccurrence.kt | 120 +++ .../data/tasks/room/RoomTaskMapper.kt | 5 + .../data/tasks/room/RoomTasksDataSource.kt | 379 +++++++-- .../data/tasks/room/TaskFormWriter.kt | 15 +- .../agendula/data/tasks/room/TasksDatabase.kt | 28 +- .../jeanlucmakiola/agendula/domain/Models.kt | 5 +- 18 files changed, 1963 insertions(+), 91 deletions(-) create mode 100644 app/schemas/de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase/5.json create mode 100644 app/src/androidTestFull/java/de/jeanlucmakiola/agendula/data/sync/RoomSyncStoreAdvancesTest.kt create mode 100644 app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/AdvancedOccurrence.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a65170..a1bf371 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ All notable changes to this project are documented here. The format follows ## [Unreleased] +### Changed +- Ticking off the current occurrence of a repeating task moves the series on + to its next date and keeps a completed copy, as tasks.org and Apple + Reminders do. Other apps and the web views of servers such as Infomaniak now + see it as done instead of still open. Unticking it moves the series back, + as long as the series hasn't moved on or changed its start since (#137). + ## [1.3.0] - 2026-11-02 ### Added 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..30c5361 --- /dev/null +++ b/app/schemas/de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase/5.json @@ -0,0 +1,768 @@ +{ + "formatVersion": 1, + "database": { + "version": 5, + "identityHash": "fcb14ff68d892c72fa7459204276abb6", + "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, `rule_after` TEXT, `dropped_exdates` TEXT NOT NULL DEFAULT '', `from_override` INTEGER NOT NULL, `undoable` INTEGER NOT NULL DEFAULT 1, `settled` INTEGER NOT NULL DEFAULT 0, 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": "ruleAfter", + "columnName": "rule_after", + "affinity": "TEXT" + }, + { + "fieldPath": "droppedExdates", + "columnName": "dropped_exdates", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + }, + { + "fieldPath": "fromOverride", + "columnName": "from_override", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "undoable", + "columnName": "undoable", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "1" + }, + { + "fieldPath": "settled", + "columnName": "settled", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + } + ], + "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" + ] + } + ] + }, + { + "tableName": "advanced_detached", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `copy_id` INTEGER NOT NULL, `anchor` INTEGER NOT NULL, `task_id` INTEGER, FOREIGN KEY(`copy_id`) REFERENCES `advanced_occurrences`(`copy_id`) ON UPDATE NO ACTION ON DELETE CASCADE , FOREIGN KEY(`task_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE SET NULL )", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "copyId", + "columnName": "copy_id", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "anchor", + "columnName": "anchor", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "taskId", + "columnName": "task_id", + "affinity": "INTEGER" + } + ], + "primaryKey": { + "autoGenerate": true, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_advanced_detached_copy_id", + "unique": false, + "columnNames": [ + "copy_id" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_advanced_detached_copy_id` ON `${TABLE_NAME}` (`copy_id`)" + }, + { + "name": "index_advanced_detached_task_id", + "unique": false, + "columnNames": [ + "task_id" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_advanced_detached_task_id` ON `${TABLE_NAME}` (`task_id`)" + } + ], + "foreignKeys": [ + { + "table": "advanced_occurrences", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "copy_id" + ], + "referencedColumns": [ + "copy_id" + ] + }, + { + "table": "tasks", + "onDelete": "SET NULL", + "onUpdate": "NO ACTION", + "columns": [ + "task_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, 'fcb14ff68d892c72fa7459204276abb6')" + ] + } +} \ No newline at end of file diff --git a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/provider/AgendulaTaskProviderTest.kt b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/provider/AgendulaTaskProviderTest.kt index fd0f678..5e82779 100644 --- a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/provider/AgendulaTaskProviderTest.kt +++ b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/provider/AgendulaTaskProviderTest.kt @@ -124,6 +124,7 @@ class AgendulaTaskProviderTest { @Test fun aBadWriteReachesTheClientAsIllegalArgument() { + assumeTrue("Agendula is running on an external provider", ownStore()) // No list has this id, so nothing in the app's real store is touched. val values = ContentValues().apply { put("list_id", Long.MAX_VALUE) @@ -135,6 +136,7 @@ class AgendulaTaskProviderTest { @Test fun aFailingBatchWritesNothing() { + assumeTrue("Agendula is running on an external provider", ownStore()) val missingList = Long.MAX_VALUE val operations = arrayListOf( ContentProviderOperation.newInsert(uri("tasks")).withValue("list_id", missingList).withValue("title", "a").build(), @@ -145,6 +147,9 @@ class AgendulaTaskProviderTest { .use { assertThat(it.count).isEqualTo(0) } } + /** Whether the app's own store is active: only then does the provider take writes at all. */ + private fun ownStore() = runCatching { resolver.delete(uri("tasks"), "_id = -1", null) }.isSuccess + /** stdout and stderr of [command]; the denial lands on stderr. */ private fun shell(command: String): String { val (stdout, stdin, stderr) = InstrumentationRegistry.getInstrumentation().uiAutomation 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 f4383a9..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 @@ -734,9 +734,11 @@ class ProviderWriterTest { val series = db.tasks().insert( TaskEntity(listId = home, uid = "f", title = "Floating", due = today, rrule = "FREQ=DAILY;COUNT=3", timezone = null), ) - val row = readRow(ProviderTable.INSTANCES, instanceId(today)) + // The second occurrence: ticking the first moves the series on instead. + val second = today + 1.days + val row = readRow(ProviderTable.INSTANCES, instanceId(second)) - writer.update(ProviderTable.INSTANCES, row + (Tasks.STATUS to 2), byId(ProviderTable.INSTANCES, instanceId(today)), null) + writer.update(ProviderTable.INSTANCES, row + (Tasks.STATUS to 2), byId(ProviderTable.INSTANCES, instanceId(second)), null) writer.update(ProviderTable.TASKS, mapOf(Tasks.TITLE to "Still floating"), byId(ProviderTable.TASKS, series), null) assertThat(db.tasks().overrides(series).single().status).isEqualTo(TaskStatus.COMPLETED) @@ -795,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 75a3a9e..78d4223 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 @@ -211,7 +211,8 @@ class RoomTasksDataSourceTest { @Test fun completingOneOccurrenceLeavesTheRestOfTheSeriesOpen() { val id = source.insertTask(form(title = "Water the plants")) - makeRecurring(id, now) + // Overdue occurrences before it, so it is not the first and forks an override. + makeRecurring(id, now - 14.days) val open = { source.tasks(TaskQuery(listId = listId)).filter { it.taskId == id } } val before = open() val target = before.first { it.distanceFromCurrent == 0 } @@ -237,7 +238,8 @@ class RoomTasksDataSourceTest { @Test fun reopeningACompletedOccurrenceReusesItsOverride() { val id = source.insertTask(form(title = "Water the plants")) - makeRecurring(id, now) + // Overdue occurrences before it, so it is not the first and forks an override. + makeRecurring(id, now - 14.days) val target = source.tasks(TaskQuery(listId = listId)) .first { it.taskId == id && it.distanceFromCurrent == 0 } @@ -254,7 +256,8 @@ class RoomTasksDataSourceTest { @Test fun cancellingOneOccurrenceLeavesTheRestOfTheSeriesOpen() { val id = source.insertTask(form(title = "Water the plants")) - makeRecurring(id, now) + // Overdue occurrences before it, so it is not the first and forks an override. + makeRecurring(id, now - 14.days) val target = source.tasks(TaskQuery(listId = listId)) .first { it.taskId == id && it.distanceFromCurrent == 0 } @@ -269,6 +272,530 @@ class RoomTasksDataSourceTest { assertThat(db.tasks().override(id, target.occurrenceStart)!!.status).isEqualTo(TaskStatus.NEEDS_ACTION) } + @Test + fun completingTheFirstOccurrenceMovesTheSeriesOnAndKeepsACopy() { + val at = now + val id = source.insertTask(form(title = "Water the plants")) + makeRecurring(id, at, rule = "FREQ=WEEKLY;COUNT=4") + + source.setCompletedInstance(id, at, completed = true) + + val master = db.tasks().entity(id)!! + assertThat(master.status).isEqualTo(TaskStatus.NEEDS_ACTION) + assertThat(master.dtstart).isEqualTo(at + 7.days) + assertThat(master.due).isEqualTo(at + 8.days) + assertThat(master.rrule).isEqualTo("FREQ=WEEKLY;COUNT=3") + assertThat(master.isDirty).isTrue() + assertThat(db.tasks().overrides(id)).isEmpty() + val copy = db.tasks().allIn(listId).single { it.id != id } + assertThat(copy.status).isEqualTo(TaskStatus.COMPLETED) + assertThat(copy.uid).isNotEqualTo(master.uid) + assertThat(copy.rrule).isNull() + assertThat(copy.masterId).isNull() + assertThat(copy.dtstart).isEqualTo(at) + assertThat(copy.title).isEqualTo("Water the plants") + } + + @Test + fun reopeningTheOccurrenceMovesTheSeriesBack() { + val at = now + val id = source.insertTask(form(title = "Water the plants")) + makeRecurring(id, at, rule = "FREQ=WEEKLY;COUNT=4") + source.setCompletedInstance(id, at, completed = true) + + source.setCompletedInstance(id, at, completed = false) + + val master = db.tasks().entity(id)!! + assertThat(master.dtstart).isEqualTo(at) + assertThat(master.rrule).isEqualTo("FREQ=WEEKLY;COUNT=4") + assertThat(db.tasks().allIn(listId).filter { !it.isDeleted }.map { it.id }).containsExactly(id) + } + + @Test + fun completingAnEditedFirstOccurrenceKeepsItsEdits() { + val at = now + val id = source.insertTask(form(title = "Water the plants")) + makeRecurring(id, at) + source.updateInstance(id, at, TaskForm(title = "Water the cactus", listId = listId, start = at, due = at + 1.days), reminders = null) + val override = db.tasks().override(id, at)!! + + source.setCompletedInstance(override.id, at, completed = true) + + assertThat(db.tasks().overrides(id)).isEmpty() + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at + 7.days) + val copy = db.tasks().allIn(listId).single { it.id != id } + assertThat(copy.title).isEqualTo("Water the cactus") + assertThat(copy.status).isEqualTo(TaskStatus.COMPLETED) + } + + @Test + fun completingTheLastOccurrenceClosesTheSeries() { + val at = now + val id = source.insertTask(form(title = "Water the plants")) + makeRecurring(id, at, rule = "FREQ=WEEKLY;COUNT=1") + + source.setCompletedInstance(id, at, completed = true) + + assertThat(db.tasks().entity(id)!!.status).isEqualTo(TaskStatus.COMPLETED) + assertThat(db.tasks().allIn(listId).map { it.id }).containsExactly(id) + + source.setCompletedInstance(id, at, completed = false) + + assertThat(db.tasks().entity(id)!!.status).isEqualTo(TaskStatus.NEEDS_ACTION) + } + + @Test + fun cancellingTheFirstOccurrenceMovesTheSeriesOn() { + val at = now + val id = source.insertTask(form(title = "Water the plants")) + makeRecurring(id, at) + + source.setCancelledInstance(id, at, cancelled = true) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at + 7.days) + assertThat(db.tasks().allIn(listId).single { it.id != id }.status).isEqualTo(TaskStatus.CANCELLED) + } + + @Test + fun aSeriesWhoseStartWasDeletedMovesPastTheTickedOne() { + val at = now + val id = source.insertTask(form(title = "Weekly")) + makeRecurring(id, at) + source.deleteOccurrence(id, at) + + source.setCompletedInstance(id, at + 7.days, completed = true) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at + 14.days) + } + + @Test + fun countSpendsExcludedOccurrencesAndReopeningRestoresThem() { + val at = now + val id = source.insertTask(form(title = "Daily")) + makeRecurring(id, at, rule = "FREQ=DAILY;COUNT=4") + source.deleteOccurrence(id, at + 1.days) + val before = db.tasks().entity(id)!! + + source.setCompletedInstance(id, at, completed = true) + + val moved = db.tasks().entity(id)!! + assertThat(moved.dtstart).isEqualTo(at + 2.days) + assertThat(moved.rrule).isEqualTo("FREQ=DAILY;COUNT=2") + + source.setCompletedInstance(id, at, completed = false) + + val back = db.tasks().entity(id)!! + assertThat(back.dtstart).isEqualTo(before.dtstart) + assertThat(back.rrule).isEqualTo(before.rrule) + assertThat(back.exdate).isEqualTo(before.exdate) + } + + @Test + fun reopeningTheCopyItselfMovesTheSeriesBack() { + 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.setCompleted(copy.id, completed = false) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at) + assertThat(db.tasks().allIn(listId).filter { !it.isDeleted }.map { it.id }).containsExactly(id) + } + + @Test + fun undoingAnEditedOccurrenceBringsItsOverrideBack() { + val at = now + val id = source.insertTask(form(title = "Weekly")) + makeRecurring(id, at) + source.updateInstance(id, at, TaskForm(title = "Just this once", listId = listId, start = at, due = at + 1.days), reminders = null) + val override = db.tasks().override(id, at)!! + source.setCompletedInstance(override.id, at, completed = true) + + source.setCompletedInstance(override.id, at, completed = false) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at) + val back = db.tasks().override(id, at)!! + assertThat(back.title).isEqualTo("Just this once") + assertThat(back.status).isEqualTo(TaskStatus.NEEDS_ACTION) + } + + @Test + fun aStaleTickOfAnOccurrenceMovedPastForksNothing() { + val at = now + val id = source.insertTask(form(title = "Weekly")) + makeRecurring(id, at) + source.setCompletedInstance(id, at, completed = true) + + source.setCompletedInstance(id, at, completed = true) + + assertThat(db.tasks().overrides(id)).isEmpty() + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at + 7.days) + } + + @Test + fun occurrencesClosedBeforeTheUpdateNoLongerHoldTheSeriesBack() { + 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, rrule = null, status = TaskStatus.COMPLETED, completedAt = at), + ) + + source.setCompletedInstance(id, at + 7.days, completed = true) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at + 14.days) + assertThat(db.tasks().overrides(id)).isEmpty() + assertThat(db.tasks().allIn(listId).count { it.id != id && it.status == TaskStatus.COMPLETED }).isEqualTo(2) + } + + @Test + fun theTickedOccurrenceStillResolvesToItsCopy() { + val at = now + val id = source.insertTask(form(title = "Weekly")) + makeRecurring(id, at) + val occurrence = source.task(id, at)!!.occurrenceStart!! + + source.setCompletedInstance(id, occurrence, completed = true) + + val found = source.task(id, occurrence)!! + assertThat(found.taskId).isNotEqualTo(id) + assertThat(found.isCompleted).isTrue() + assertThat(found.occurrenceStart).isEqualTo(occurrence) + } + + @Test + fun theNextOccurrenceStartsWithoutTheLastOnesProgress() { + val at = now + val id = source.insertTask(form(title = "Weekly")) + makeRecurring(id, at) + db.tasks().update(db.tasks().entity(id)!!.copy(status = TaskStatus.IN_PROCESS, percentComplete = 60)) + + source.setCompletedInstance(id, at, completed = true) + + val master = db.tasks().entity(id)!! + assertThat(master.status).isEqualTo(TaskStatus.NEEDS_ACTION) + assertThat(master.percentComplete).isNull() + } + + @Test + fun theCopyOfAnEditedOccurrenceKeepsItsOwnProperties() { + 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 = "COMMENT:only this one"), + ) + + source.setCompletedInstance(override, at, completed = true) + + assertThat(db.tasks().entity(override)!!.unknownProperties!!.lines()).contains("COMMENT:only this one") + } + + @Test + fun aRuleWithAGapOfMoreThanTenYearsKeepsGoing() { + val at = now + val id = source.insertTask(form(title = "Rare")) + makeRecurring(id, at, rule = "FREQ=YEARLY;INTERVAL=11") + + source.setCompletedInstance(id, at, completed = true) + + val master = db.tasks().entity(id)!! + assertThat(master.status).isEqualTo(TaskStatus.NEEDS_ACTION) + 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) + // Nothing to move past: the closed occurrences stay the series' own. + assertThat(db.tasks().overrides(id)).hasSize(2) + + 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)!!.undoable).isFalse() + } + + @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 reopeningTheCopyInTheEditorMovesTheSeriesBack() { + 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 = null, + ) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at) + } + + @Test + fun anEditedCopyReopensAsATaskOfItsOwn() { + 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 } + db.tasks().update(copy.copy(description = "notes")) + source.updateTask(copy.id, TaskForm(title = "Renamed", listId = listId, start = copy.dtstart, due = copy.due, percentComplete = 100), null) + + source.setCompleted(copy.id, completed = false) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at + 7.days) + assertThat(db.tasks().entity(copy.id)!!.title).isEqualTo("Renamed") + assertThat(db.tasks().entity(copy.id)!!.status).isEqualTo(TaskStatus.NEEDS_ACTION) + } + + @Test + fun aCopyWithASubtaskReopensOnItsOwn() { + 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.insertTask(form(title = "Part").copy(parentId = copy.id)) + + source.setCompleted(copy.id, completed = false) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at + 7.days) + assertThat(db.tasks().entity(copy.id)!!.status).isEqualTo(TaskStatus.NEEDS_ACTION) + } + + @Test + fun aRecurringSubtaskKeepsTheOverrideModel() { + val at = now + val parent = source.insertTask(form(title = "Parent")) + val id = source.insertTask(form(title = "Daily part").copy(parentId = parent)) + makeRecurring(id, at, rule = "FREQ=DAILY") + + source.setCompletedInstance(id, at, completed = true) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at) + assertThat(db.tasks().overrides(id)).hasSize(1) + } + + @Test + fun aSeriesReshapedSinceIsNotMovedBack() { + 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 = "FREQ=DAILY;COUNT=5")) + + source.setCompletedInstance(id, at, completed = false) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at + 7.days) + assertThat(db.tasks().entity(id)!!.rrule).isEqualTo("FREQ=DAILY;COUNT=5") + assertThat(db.tasks().allIn(listId).single { it.id != id }.status).isEqualTo(TaskStatus.NEEDS_ACTION) + } + + @Test + fun tickingTheSameOccurrenceAgainKeepsOneRecord() { + val at = now + val id = source.insertTask(form(title = "Weekly")) + makeRecurring(id, at) + source.setCompletedInstance(id, at, completed = true) + source.setCompletedInstance(id, at, completed = false) + + source.setCompletedInstance(id, at, completed = true) + + assertThat(db.advancedOccurrences().forMaster(id)).hasSize(1) + source.setCompletedInstance(id, at, completed = false) + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at) + } + + @Test + fun anEditedCopyStillStandsForItsOccurrence() { + 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 } + + db.tasks().update(copy.copy(title = "Watered")) + + assertThat(source.task(id, at)!!.taskId).isEqualTo(copy.id) + } + + @Test + fun aDetachedOccurrenceWithALifeOfItsOwnStaysOut() { + 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.insertTask(form(title = "Follow-up").copy(parentId = earlier)) + + source.setCompletedInstance(id, at + 7.days, completed = false) + + assertThat(db.tasks().entity(earlier)!!.masterId).isNull() + assertThat(source.tasks(TaskQuery(listId = listId)).map { it.occurrenceStart?.epochSeconds }) + .doesNotContain(at.epochSeconds) + } + + @Test + fun reopeningWithProgressKeepsIt() { + 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 = 40), + reminders = null, + ) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at) + assertThat(db.tasks().override(id, at)!!.percentComplete).isEqualTo(40) + } + + @Test + fun theEditorsTidyingDoesNotCountAsAnEdit() { + val at = now + val id = source.insertTask(form(title = "Weekly")) + makeRecurring(id, at) + db.tasks().update(db.tasks().entity(id)!!.copy(description = "From the server\n")) + 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, description = copy.description, start = copy.dtstart, due = copy.due, percentComplete = 0), + reminders = null, + ) + + assertThat(db.tasks().entity(id)!!.dtstart).isEqualTo(at) + } + @Test fun completingANonRecurringTaskThroughTheInstancePathWritesTheRowItself() { val id = source.insertTask(form(title = "Buy milk", due = now + 1.days)) @@ -657,10 +1184,12 @@ class RoomTasksDataSourceTest { val outOfWindow = db.tasks().insert( TaskEntity(listId = listId, uid = "old", title = "old", dtstart = longAgo, rrule = "FREQ=DAILY;COUNT=2", parentId = parent), ) + val at = now val series = db.tasks().insert( - TaskEntity(listId = listId, uid = "s", title = "s", dtstart = now, rrule = "FREQ=DAILY;COUNT=3", parentId = parent), + TaskEntity(listId = listId, uid = "s", title = "s", dtstart = at, rrule = "FREQ=DAILY;COUNT=3", parentId = parent), ) - source.setCompletedInstance(series, now, true) + // Not the first occurrence, so it forks an override rather than a copy. + source.setCompletedInstance(series, at + 1.days, true) val plain = source.insertTask(form("plain").copy(parentId = parent)) assertThat(source.subtaskIds(parent)).containsExactly(outOfWindow, series, plain) 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/androidTestFull/java/de/jeanlucmakiola/agendula/data/sync/RoomSyncStoreAdvancesTest.kt b/app/src/androidTestFull/java/de/jeanlucmakiola/agendula/data/sync/RoomSyncStoreAdvancesTest.kt new file mode 100644 index 0000000..2551e25 --- /dev/null +++ b/app/src/androidTestFull/java/de/jeanlucmakiola/agendula/data/sync/RoomSyncStoreAdvancesTest.kt @@ -0,0 +1,57 @@ +package de.jeanlucmakiola.agendula.data.sync + +import android.content.Context +import androidx.room.Room +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.agendula.data.tasks.room.RoomTasksDataSource +import de.jeanlucmakiola.agendula.data.tasks.room.TaskEntity +import de.jeanlucmakiola.agendula.data.tasks.room.TaskListEntity +import de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase +import org.junit.After +import org.junit.Test +import org.junit.runner.RunWith +import kotlin.time.Clock +import kotlin.time.Instant + +/** A server version replacing a moved series takes the moves' copies with it (#137). */ +@RunWith(AndroidJUnit4::class) +class RoomSyncStoreAdvancesTest { + + private val db = Room.inMemoryDatabaseBuilder(ApplicationProvider.getApplicationContext(), TasksDatabase::class.java) + .allowMainThreadQueries().build() + + @After + fun tearDown() = db.close() + + @Test + fun droppingAdvancesDeletesTheirCopies() { + 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().update(copy.copy(href = "copy.ics")) + + RoomSyncStore(db).dropAdvances(series) + + assertThat(db.advancedOccurrences().forMaster(series)).isEmpty() + assertThat(db.tasks().entity(copy.id)!!.isDeleted).isTrue() + } + + @Test + fun movesTheServerAlreadyHasStay() { + 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 } + val store = RoomSyncStore(db) + store.markSynced(listOf(series), "s.ics", "1") + + store.dropAdvances(series) + + assertThat(db.tasks().entity(copy.id)!!.isDeleted).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 a54e999..f7d91df 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 @@ -825,6 +825,7 @@ class CollectionSyncer( } pendingDiscard.remove(href)?.let { cause -> + existing[null]?.let { store.dropAdvances(it.id) } 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 0cb0544..d29b918 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 @@ -41,6 +41,13 @@ interface SyncStore { fun deleteAll(taskIds: List) + /** + * 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. + */ + fun dropAdvances(masterId: Long) = Unit + /** * Records href and ETag on a resource's rows, clearing `is_dirty`. * @@ -97,8 +104,27 @@ class RoomSyncStore @Inject constructor( if (taskIds.isNotEmpty()) database.tasks().deleteAll(taskIds) } + override fun dropAdvances(masterId: Long) = database.runInTransaction { + val advanced = database.advancedOccurrences() + // Only moves the server never saw, on copies that are still just that. + advanced.forMaster(masterId).filter { !it.settled && it.undoable }.forEach { record -> + 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)) + } + } + } + override fun markSynced(taskIds: List, href: String?, eTag: String?) { - if (taskIds.isNotEmpty()) database.tasks().markSynced(taskIds, href, eTag) + if (taskIds.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) + } } override fun setParent(taskId: Long, parentId: Long?) { 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..84b039c 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,9 +68,10 @@ 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 TaskDetail( task = task, - subtasks = SeriesCollapse.visible(dataSource.subtasks(taskId)).filter { it.taskId != taskId }, + subtasks = SeriesCollapse.visible(dataSource.subtasks(owner)).filter { it.taskId != owner }, parent = task.parentId?.takeIf { it > 0 }?.let { dataSource.task(it) }, ) } @@ -201,7 +202,7 @@ 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) + if (instance == null) dataSource.updateTask(resolvedRow(current, 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(resolvedRow(current, taskId)) when (scope) { RecurringScope.ThisOccurrence -> // An override row deletes itself into an EXDATE; a generated @@ -291,6 +292,13 @@ class TasksRepositoryImpl @Inject constructor( ticks.close() } }.flowOn(io) + + /** + * The row a read for [taskId] resolved to — a past occurrence the series moved + * past is a task of its own — which is the one a write has to address. + */ + private fun resolvedRow(resolved: Task?, taskId: Long): Long = resolved?.taskId ?: taskId + } /** Subtask levels a duplicate follows; deeper than any real list, short of a cycle. */ 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 b262286..c1b6c70 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 @@ -62,39 +62,33 @@ class ProviderWriter internal constructor( private var batchWrote = false /** Inserts one row and returns its published id. */ - fun insert(table: ProviderTable, values: ProviderValues): Long = write { - when (table) { - ProviderTable.TASKS -> insertTask(values) - ProviderTable.PROPERTIES -> insertProperty(values) - else -> throw UnsupportedOperationException("${table.path} is read-only") - } + 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 = write { - 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 -> throw UnsupportedOperationException("${table.path} is read-only") + 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 = write { - 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 -> throw UnsupportedOperationException("${table.path} is read-only") - } + 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,6 +107,9 @@ class ProviderWriter internal constructor( } } + /** 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) { "Agendula is working on top of an external tasks provider; write to that one instead" 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..836092a --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/AdvancedOccurrence.kt @@ -0,0 +1,120 @@ +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 (#137). It maps the + * occurrence to its completed copy for as long as the copy lives, and while + * [undoable] it also holds what reopening the copy needs to move the series + * back: the change, not the old series, so edits made since are kept. Local + * only: never part of a `VTODO`. + */ +@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, + /** The `RRULE` the move left; once it differs, the series was reshaped and is not moved back. */ + @ColumnInfo(name = "rule_after") val ruleAfter: String?, + /** The `EXDATE` occurrences the move dropped, as epoch millis, comma-separated. */ + @ColumnInfo(name = "dropped_exdates", defaultValue = "") val droppedExdates: String = "", + /** The copy was an override before, and goes back to being one. */ + @ColumnInfo(name = "from_override") val fromOverride: Boolean, + /** Cleared once the copy is edited, moved or reopened on its own: it is then a task of its own. */ + @ColumnInfo(name = "undoable", defaultValue = "1") val undoable: Boolean = true, + /** Set once the moved series reached the server, which then holds the move itself. */ + @ColumnInfo(name = "settled", defaultValue = "0") val settled: Boolean = false, +) { + val dropped: List + get() = droppedExdates.split(',').mapNotNull { it.toLongOrNull()?.let(Instant::fromEpochMilliseconds) } + + companion object { + fun encode(instants: List) = instants.joinToString(",") { it.toEpochMilliseconds().toString() } + } +} + +/** A closed override detached along with an advance; [taskId] is null once that row is gone. */ +@Entity( + tableName = "advanced_detached", + foreignKeys = [ + ForeignKey( + entity = AdvancedOccurrenceEntity::class, + parentColumns = ["copy_id"], + childColumns = ["copy_id"], + onDelete = ForeignKey.CASCADE, + ), + ForeignKey(entity = TaskEntity::class, parentColumns = ["id"], childColumns = ["task_id"], onDelete = ForeignKey.SET_NULL), + ], + indices = [Index(value = ["copy_id"]), Index(value = ["task_id"])], +) +data class AdvancedDetachedEntity( + @PrimaryKey(autoGenerate = true) @ColumnInfo(name = "id") val id: Long = 0, + @ColumnInfo(name = "copy_id") val copyId: Long, + @ColumnInfo(name = "anchor") val anchor: Instant, + @ColumnInfo(name = "task_id") val taskId: Long?, +) + +@Dao +interface AdvancedOccurrenceDao { + @Insert(onConflict = OnConflictStrategy.REPLACE) + fun insert(row: AdvancedOccurrenceEntity) + + @Insert + fun insertDetached(rows: List) + + @Query("SELECT * FROM advanced_occurrences WHERE copy_id = :copyId") + fun forCopy(copyId: Long): AdvancedOccurrenceEntity? + + @Query("SELECT * FROM advanced_detached WHERE copy_id = :copyId") + fun detachedFor(copyId: Long): List + + @Query("SELECT * FROM advanced_occurrences WHERE master_id = :masterId") + fun forMaster(masterId: Long): List + + /** The record for [masterId]'s occurrence within the second starting at [secondStart]. */ + @Query( + "SELECT * FROM advanced_occurrences WHERE master_id = :masterId " + + "AND anchor >= :secondStart AND anchor < :secondStart + 1000", + ) + fun at(masterId: Long, secondStart: Instant): AdvancedOccurrenceEntity? + + @Query("DELETE FROM advanced_occurrences WHERE copy_id = :copyId") + fun delete(copyId: Long) + + @Query("UPDATE advanced_occurrences SET undoable = 0 WHERE copy_id = :copyId") + fun keepAsOwnTask(copyId: Long) + + @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) +} 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..3bf4214 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 @@ -4,6 +4,7 @@ import de.jeanlucmakiola.agendula.domain.LocalAccount import de.jeanlucmakiola.agendula.domain.Task import de.jeanlucmakiola.agendula.domain.TaskList import de.jeanlucmakiola.agendula.domain.export.ExportTask +import de.jeanlucmakiola.agendula.domain.isClosed import de.jeanlucmakiola.agendula.domain.priorityFromICal import de.jeanlucmakiola.agendula.domain.recurrence.RecurrenceSpec import kotlin.time.Instant @@ -100,6 +101,10 @@ object RoomTaskMapper { val TaskEntity.isRecurring: Boolean get() = !rrule.isNullOrBlank() || !rdate.isNullOrBlank() +/** Completed or cancelled. */ +val TaskEntity.isClosed: Boolean + get() = status.isClosed + /** * 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 60edba7..ac21b02 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 @@ -11,8 +11,10 @@ import de.jeanlucmakiola.agendula.domain.TaskForm import de.jeanlucmakiola.agendula.domain.TaskList import de.jeanlucmakiola.agendula.domain.TaskStatus import de.jeanlucmakiola.agendula.domain.export.ExportTask +import de.jeanlucmakiola.agendula.domain.normalizeTags import de.jeanlucmakiola.agendula.domain.recurrence.ExpansionWindow import de.jeanlucmakiola.agendula.domain.recurrence.RecurrenceExpander +import de.jeanlucmakiola.agendula.domain.recurrence.RecurrenceSpec import java.time.ZoneId import java.util.UUID import javax.inject.Inject @@ -28,6 +30,12 @@ private val WINDOW_FORWARD = 730.days /** Residue lines that describe the old series and must not follow the new one. */ private val SERIES_LINES = listOf("EXDATE", "RDATE", "RRULE", "RECURRENCE-ID", "UID") +/** How long a record keeps mapping a ticked occurrence to its copy. */ +private val RECORD_HORIZON = 30.days + +/** Only bounds the search: the occurrence count is what stops expansion, so no real gap is cut off. */ +private val SERIES_LOOKAHEAD = (366 * 1000).days + /** Enough to count every occurrence before a split point without bounding a real series. */ internal const val SPLIT_COUNT_CEILING = 100_000 @@ -103,6 +111,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 ---------------------------------------------------------------- @@ -132,6 +141,9 @@ 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 } + ?: copyAt(row.task, occurrenceStart)?.let { copy -> + tasks.task(copy.id)?.let { RoomTaskMapper.task(it).copy(occurrenceStart = occurrenceStart) } + } ?: task(taskId) } @@ -256,7 +268,8 @@ class RoomTasksDataSource @Inject constructor( // in both (the task query skips non-null `master_id`, and the override // query finds no master in B) while still uploading as part of A's // resource. - tasks.update( + saveRow( + current, if (current.masterId == null) { TaskFormWriter.withRule(edited, form.rrule) } else { @@ -268,6 +281,8 @@ class RoomTasksDataSource @Inject constructor( /** See [TaskMove]: the rows move, and the old collection is left a tombstone. */ private fun move(current: TaskEntity, edited: TaskEntity) { + // A copy moved elsewhere is a task of its own now. + advanced.keepAsOwnTask(current.id) val plan = TaskMove.plan( edited = edited, previous = current, @@ -323,11 +338,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())) @@ -350,9 +361,7 @@ class RoomTasksDataSource @Inject constructor( percentComplete = null, completedAt = null, // Recurrence lines belong to the old series; everything else carries over. - unknownProperties = master.unknownProperties?.lines() - ?.filterNot { line -> SERIES_LINES.any { line.startsWith(it, ignoreCase = true) } } - ?.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 } } @@ -411,9 +420,12 @@ class RoomTasksDataSource @Inject constructor( private inline fun tx(crossinline block: () -> Unit) = database.runInTransaction(Runnable { block() }) - private fun seriesMaster(seriesId: Long): TaskEntity { - val row = tasks.entity(seriesId) ?: throw TaskWriteFailedException("series $seriesId") - return row.masterId?.let { tasks.entity(it) } ?: row + private fun seriesMaster(seriesId: Long): TaskEntity = rowAndMaster(seriesId).second + + /** [taskId]'s row, and its series' master — the row itself unless it is an override. */ + private fun rowAndMaster(taskId: Long): Pair { + val row = tasks.entity(taskId) ?: throw TaskWriteFailedException("series $taskId") + return row to (row.masterId?.let { tasks.entity(it) } ?: row) } /** The occurrence's own times: its override's if it has one, else generated. */ @@ -466,56 +478,331 @@ class RoomTasksDataSource @Inject constructor( writes.onWritten(setOf(current.listId)) } - override fun setCancelled(taskId: Long, cancelled: Boolean) { + /** Reopening the copy a ticked occurrence left behind moves its series back, as in [setCompletedInstance]. */ + override fun setCancelled(taskId: Long, cancelled: Boolean) = tx { val current = tasks.entity(taskId) ?: throw TaskWriteFailedException("cancel task $taskId") - tasks.update(TaskFormWriter.cancelled(current, cancelled, clock.now())) + saveRow(current, TaskFormWriter.cancelled(current, cancelled, clock.now())) writes.onWritten(setOf(current.listId)) } - override fun setCompleted(taskId: Long, completed: Boolean) { + /** Reopening the copy a ticked occurrence left behind moves its series back, as in [setCompletedInstance]. */ + override fun setCompleted(taskId: Long, completed: Boolean) = tx { val current = tasks.entity(taskId) ?: throw TaskWriteFailedException("complete task $taskId") - tasks.update(TaskFormWriter.completed(current, completed, clock.now())) + saveRow(current, TaskFormWriter.completed(current, completed, clock.now())) writes.onWritten(setOf(current.listId)) } /** - * Ticking one occurrence forks a `RECURRENCE-ID` override carrying the - * completion — the same model [updateInstance] writes. Writing the status onto - * the master instead would close the series: the master is what - * [TaskDao.tasks] filters on, so every occurrence, past and future, would - * leave every list at once. + * Ticking a series' first open occurrence moves the series on to its next + * 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). Closed overrides between become copies too. + * Reopening the copy while the series is still where that left it moves it + * 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) = - writeInstance(taskId, occurrenceStart, "complete") { TaskFormWriter.completed(it, completed, clock.now()) } + override fun setCompletedInstance(taskId: Long, occurrenceStart: Instant, completed: Boolean) = tx { + closeOccurrence(taskId, occurrenceStart, completed) { TaskFormWriter.completed(it, completed, clock.now()) } + } - /** Same override model as [setCompletedInstance]. */ - override fun setCancelledInstance(taskId: Long, occurrenceStart: Instant, cancelled: Boolean) = - writeInstance(taskId, occurrenceStart, "cancel") { TaskFormWriter.cancelled(it, cancelled, clock.now()) } + /** Same model as [setCompletedInstance]. */ + override fun setCancelledInstance(taskId: Long, occurrenceStart: Instant, cancelled: Boolean) = tx { + closeOccurrence(taskId, occurrenceStart, cancelled) { TaskFormWriter.cancelled(it, cancelled, clock.now()) } + } - /** Applies [write] to the override for [occurrenceStart], forking one if there is none yet. */ - private fun writeInstance( - taskId: Long, - occurrenceStart: Instant, - what: String, - write: (TaskEntity) -> TaskEntity, - ) { - val master = tasks.entity(taskId) ?: throw TaskWriteFailedException("$what instance $taskId") - // Not a series master — an override, or a plain task the caller handed an - // anchor for. Either way this row *is* the occurrence. + private fun closeOccurrence(taskId: Long, occurrenceStart: Instant, closing: Boolean, close: (TaskEntity) -> TaskEntity) { + val (row, master) = rowAndMaster(taskId) if (master.recurrenceSpec() == null) { - tasks.update(write(master)) - writes.onWritten(setOf(master.listId)) + // A plain task, or the copy a ticked occurrence left: this row is the occurrence. + saveRow(row, close(row)) + writes.onWritten(setOf(row.listId)) 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 moved past it, so a tick has nothing left to close; + // a reopen goes to the copy that occurrence left. + when { + override != null -> writeInstance(master, override, anchor, close) + !closing -> copyAt(master, anchor)?.let { copy -> + saveRow(copy, close(copy)) + writes.onWritten(setOf(copy.listId)) + } + } + 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) + return + } + if (!closing || !advance(master, anchor, override, close)) writeInstance(master, override, anchor, close) + } - tasks.override(taskId, occurrenceStart)?.let { - tasks.update(write(it)) - writes.onWritten(setOf(master.listId)) - return + /** + * 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. + */ + private fun saveRow(current: TaskEntity, edited: TaskEntity) { + val record = advanced.forCopy(current.id)?.takeIf { it.undoable } + if (record != null) { + val unchanged = sameApartFromState(current, edited) + if (unchanged && current.isClosed && !edited.isClosed && reopenCopy(current, edited, record)) return + if (!unchanged || !edited.isClosed) advanced.keepAsOwnTask(current.id) + } + tasks.update(edited) + } + + /** + * Nothing a user sees changed but its state — what saving a reopened copy + * from the editor amounts to. Compared as the editor writes them back: + * trimmed, blank as none, tags normalised. + */ + private fun sameApartFromState(a: TaskEntity, b: TaskEntity): Boolean { + fun text(value: String?) = value?.trim()?.ifEmpty { null } + return text(a.title) == text(b.title) && text(a.description) == text(b.description) && + text(a.location) == text(b.location) && text(a.url) == text(b.url) && + a.priority == b.priority && a.dtstart == b.dtstart && a.due == b.due && a.isAllDay == b.isAllDay && + a.listId == b.listId && a.parentId == b.parentId && a.rrule == b.rrule && a.rdate == b.rdate && + normalizeTags(a.categories) == normalizeTags(b.categories) && a.color == b.color + } + + /** [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)) + override?.let { tasks.update(close(it)) } + writes.onWritten(setOf(master.listId)) + } + + /** + * Moves [master] past [anchor] when that is its first open occurrence; false + * when it is not, so the caller writes an override. Series with `RDATE`s keep + * the override model (anchoring on one would re-align the `RRULE`), and so do + * recurring subtasks, whose copies would pile up under the parent. + */ + private fun advance(master: TaskEntity, anchor: Instant, override: TaskEntity?, close: (TaskEntity) -> TaskEntity): Boolean { + val rule = master.rrule?.takeIf { it.isNotBlank() } ?: return false + if (!master.rdate.isNullOrBlank() || master.parentId != null) return false + val spec = master.recurrenceSpec() ?: return false + val overrides = tasks.overrides(master.id).associateBy { it.recurrenceId?.epochSeconds } + 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 { !closedAt(it) }) return false + var nextIndex = index + 1 + while (nextIndex < leading.size && closedAt(leading[nextIndex])) nextIndex++ + val next = leading.getOrNull(nextIndex) ?: return true.also { closeSeries(master, override, close) } + + val now = clock.now() + 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 newAnchor = next + (spec.anchor - floorSecond(spec.anchor)) + val spent = spentBefore(spec, next) + val (start, due) = occurrenceTimes(master, newAnchor) + val ruleAfter = TaskFormWriter.remainingCount(rule, spent) + tasks.update( + master.copy( + dtstart = start, + due = due, + rrule = ruleAfter, + exdate = TaskFormWriter.valuesFrom(master.exdate, newAnchor, zone), + status = TaskStatus.NEEDS_ACTION, + percentComplete = null, + completedAt = null, + lastModified = now, + isDirty = true, + ), + ) + val copyId = if (override != null) { + detach(override, now, close) + } else { + 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 + } + advanced.at(master.id, floorSecond(anchor))?.let { advanced.delete(it.copyId) } + advanced.pruneBefore(master.id, now - RECORD_HORIZON) + 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, + ruleAfter = ruleAfter, + droppedExdates = AdvancedOccurrenceEntity.encode(TaskFormWriter.instantsBefore(master.exdate, newAnchor, zone)), + fromOverride = override != null, + ), + ) + advanced.insertDetached(detached.map { (at, id) -> AdvancedDetachedEntity(copyId = copyId, anchor = at, taskId = id) }) + 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, 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))) + 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, + uid = UUID.randomUUID().toString(), + masterId = null, + recurrenceId = null, + href = null, + etag = null, + rrule = null, + rdate = null, + exdate = null, + createdAt = now, + sequence = 0, + isDeleted = false, + isDirty = true, + unknownProperties = seriesFreeText(source.unknownProperties), + ) + + /** + * Undoes [advance] for the copy [row] while the series still sits where it + * moved it under the same rule, and the copy is still just that — same list, + * nothing filed under it. The anchor goes back, `COUNT` gets back what it + * spent, and the dropped `EXDATE`s return in the list's own shape; any other + * edit made since stays. The occurrence takes [reopened]'s state: as a plain + * occurrence again, or as an override where that state is its own. The + * occurrences detached with it become overrides again, or are excluded where + * their row is gone or has a life of its own. False when it cannot, so the + * caller reopens the copy as a task of its own. + */ + 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.recurrenceAnchor?.let { a -> sameSecond(a, record.advancedTo) } == true + } + if (master == null || tasks.subtasks(row.id).isNotEmpty()) return false + val now = clock.now() + val zone = master.timezone ?: zone() + val synced = lists.entity(master.listId)?.accountId != null + 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) }, + 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, + ) + record.dropped.forEach { back = TaskFormWriter.excepting(back, it, now, zone, dirty = synced) } + // A detached row given a parent or subtasks since has a life of its own, and stays out. + fun returns(task: TaskEntity?) = task != null && !task.isDeleted && task.listId == master.listId && + task.parentId == master.parentId && tasks.subtasks(task.id).isEmpty() + val detached = advanced.detachedFor(row.id).map { it.anchor to it.taskId?.let(tasks::entity) } + detached.forEach { (at, task) -> if (!returns(task)) back = TaskFormWriter.excepting(back, at, now, zone, dirty = synced) } + tasks.update(back) + detached.forEach { (at, task) -> if (returns(task)) toOverride(task!!, back, at, now) } + advanced.delete(row.id) + val plain = reopened.status == TaskStatus.NEEDS_ACTION && reopened.percentComplete == null + if (record.fromOverride || !plain) toOverride(reopened, back, record.anchor, now) + else deleteTask(row.id) + writes.onWritten(setOf(master.listId)) + return true + } + + /** [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, + 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 = false) + deleteTask(row.id) + } + } + + /** 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 } + + /** The series' occurrences from its anchor on, at most [maxOccurrences]. */ + private fun expandFrom(spec: RecurrenceSpec, maxOccurrences: Int): List { + val from = floorSecond(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 + * 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 + + /** 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) { + tasks.update(write(override)) + } else { + val (start, due) = occurrenceTimes(master, occurrenceStart) + val id = tasks.insert(write(newOverride(master, master.id, occurrenceStart, start, due))) + alarms.copyToTask(master.id, id, sameResource = true) } - val (start, due) = occurrenceTimes(master, occurrenceStart) - val id = tasks.insert(write(newOverride(master, taskId, occurrenceStart, start, due))) - alarms.copyToTask(taskId, id, sameResource = true) writes.onWritten(setOf(master.listId)) } @@ -608,7 +895,7 @@ class RoomTasksDataSource @Inject constructor( if (lists.exists(edited.listId) == 0) throw TaskWriteFailedException("move task: no list ${edited.listId}") move(current, edited) } - else -> tasks.update(edited) + else -> saveRow(current, edited) } } writes.onWritten(setOf(current.listId, edited.listId)) diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskFormWriter.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskFormWriter.kt index a73b2c0..c7743f0 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskFormWriter.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskFormWriter.kt @@ -120,18 +120,25 @@ object TaskFormWriter { fun valuesBefore(values: String?, cut: Instant, zone: String): String? = mapValues(values, zone) { it.takeIf { at -> at < cut } } + /** The instants of an `EXDATE`/`RDATE` list before [cut]; values it cannot read are left out. */ + fun instantsBefore(values: String?, cut: Instant, zone: String): List = + parsed(values, zone).mapNotNull { it.second }.filter { it < cut } + /** The values of an `EXDATE`/`RDATE` list at or after [cut]. */ fun valuesFrom(values: String?, cut: Instant, zone: String): String? = mapValues(values, zone) { it.takeIf { at -> at >= cut } } private fun mapValues(values: String?, zone: String, map: (Instant) -> Instant?): String? { - val list = values?.split(',')?.map(String::trim)?.filter(String::isNotEmpty) ?: return values - return list.mapNotNull { raw -> - val at = readValue(raw, zone) ?: return@mapNotNull raw - map(at)?.let { writeValueLike(raw, it, zone) } + if (values == null) return null + return parsed(values, zone).mapNotNull { (raw, at) -> + if (at == null) raw else map(at)?.let { writeValueLike(raw, it, zone) } }.joinToString(",").ifEmpty { null } } + /** Each value of an `EXDATE`/`RDATE` list with its instant, or null where it cannot be read. */ + private fun parsed(values: String?, zone: String): List> = + values?.split(',')?.map(String::trim)?.filter(String::isNotEmpty)?.map { it to readValue(it, zone) }.orEmpty() + private fun readValue(raw: String, zone: String): Instant? = runCatching { val z = java.time.ZoneId.of(zone) val instant = when { 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..6d81763 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. Six 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,10 @@ import kotlin.time.Instant TaskListEntity::class, TaskEntity::class, TaskAlarmEntity::class, + AdvancedOccurrenceEntity::class, + AdvancedDetachedEntity::class, ], - version = 4, + version = 5, exportSchema = true, ) @TypeConverters(Converters::class) @@ -31,6 +33,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 +183,26 @@ 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, `rule_after` TEXT, `dropped_exdates` TEXT NOT NULL DEFAULT '', `from_override` INTEGER NOT NULL, `undoable` INTEGER NOT NULL DEFAULT 1, `settled` INTEGER NOT NULL DEFAULT 0, 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`)", + ) + db.execSQL( + "CREATE TABLE IF NOT EXISTS `advanced_detached` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `copy_id` INTEGER NOT NULL, `anchor` INTEGER NOT NULL, `task_id` INTEGER, FOREIGN KEY(`copy_id`) REFERENCES `advanced_occurrences`(`copy_id`) ON UPDATE NO ACTION ON DELETE CASCADE , FOREIGN KEY(`task_id`) REFERENCES `tasks`(`id`) ON UPDATE NO ACTION ON DELETE SET NULL )", + ) + db.execSQL( + "CREATE INDEX IF NOT EXISTS `index_advanced_detached_copy_id` ON `advanced_detached` (`copy_id`)", + ) + db.execSQL( + "CREATE INDEX IF NOT EXISTS `index_advanced_detached_task_id` ON `advanced_detached` (`task_id`)", + ) + } + } } } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/domain/Models.kt b/app/src/main/java/de/jeanlucmakiola/agendula/domain/Models.kt index 35de203..55943f9 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/domain/Models.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/domain/Models.kt @@ -37,6 +37,9 @@ data class TaskList( enum class TaskStatus { NEEDS_ACTION, IN_PROCESS, COMPLETED, CANCELLED } +/** Completed or cancelled. */ +val TaskStatus.isClosed: Boolean get() = this == TaskStatus.COMPLETED || this == TaskStatus.CANCELLED + /** iCalendar priority, bucketed for the UI. */ enum class Priority { NONE, LOW, MEDIUM, HIGH } @@ -111,7 +114,7 @@ data class Task( val isOccurrence: Boolean get() = seriesId != null && occurrenceStart != null val isCompleted: Boolean get() = status == TaskStatus.COMPLETED - val isClosed: Boolean get() = status == TaskStatus.COMPLETED || status == TaskStatus.CANCELLED + val isClosed: Boolean get() = status.isClosed val isSubtask: Boolean get() = parentId != null && parentId > 0 /** The task's own colour if set, else the list colour. */ val effectiveColor: Int get() = taskColor ?: listColor