diff --git a/CHANGELOG.md b/CHANGELOG.md index afb1971..6ec6c85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,20 @@ All notable changes to this project are documented here. The format follows ## [Unreleased] +### Changed +- Reminders now sync. A reminder set on a task is uploaded as a standard alarm + (`VALARM`), so it also notifies in macOS and iOS Reminders, Thunderbird, + tasks.org and jtx Board, and alarms set in those apps now fire in Agendula + instead of the default reminder. Removing a task's alarm in another app + turns it off here too, rather than falling back to the default (#52). +- A new task with a due date, or one that gets its first due date, takes the + default reminder as its own, so it travels with the task to the server. + Changing the default later no longer changes those tasks; tasks that rely on + the default today are left as they are. +- On upgrade, reminders on open, synced tasks that so far stayed on this + device are uploaded once. +- Alarms imported from an `.ics` file become reminders on the task. + ## [1.1.0] - 2026-10-10 ### Added diff --git a/README.md b/README.md index 0343709..e8c4da4 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,8 @@ many small *florets* — so the two apps are florets of one bloom.) this and the following ones, or the whole series. - **Reminders** that fire at the exact time: several per task, separate defaults for timed and all-day tasks (globally and per list), Done and Snooze right on - the notification, and they survive reboots. + the notification, and they survive reboots. They sync as standard `VALARM`s, + so they also ring in other CalDAV clients, and theirs ring here. - **iCalendar import and export** — open an `.ics` or a zip of them, or write any list out as standard `.ics` files. - A **home-screen widget**, launcher shortcuts, a Quick Settings tile, and diff --git a/app/schemas/de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase/3.json b/app/schemas/de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase/3.json new file mode 100644 index 0000000..4bc7c6a --- /dev/null +++ b/app/schemas/de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase/3.json @@ -0,0 +1,548 @@ +{ + "formatVersion": 1, + "database": { + "version": 3, + "identityHash": "725292b38bce25ce9032c4650573ae85", + "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)", + "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" + } + ], + "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" + ] + } + ] + } + ], + "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, '725292b38bce25ce9032c4650573ae85')" + ] + } +} \ No newline at end of file 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 e54c1ea..e4d3d3b 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 @@ -411,6 +411,134 @@ class RoomTasksDataSourceTest { assertThat(db.alarms().forTask(override.id).single().minutesBefore).isEqualTo(30) } + @Test + fun aSaveKeepsTheIdentityOfAReminderItDidNotChange() { + val id = source.insertTask(form(due = now + 1.days), listOf(TaskReminder(15))) + val original = db.alarms().forTask(id).single() + db.alarms().update(original.copy(raw = "BEGIN:VALARM\r\nX-VENDOR:keep\r\nEND:VALARM\r\n")) + + source.updateTask(id, form(title = "renamed", due = now + 2.days), dueReminders = listOf(15, 60)) + + val rows = db.alarms().forTask(id) + val kept = rows.single { it.minutesBefore == 15 } + // Same row, same identity on the server, same original text. + assertThat(kept.id).isEqualTo(original.id) + assertThat(kept.uid).isEqualTo(original.uid) + assertThat(kept.raw).contains("X-VENDOR:keep") + assertThat(rows.single { it.minutesBefore == 60 }.uid).isNotEqualTo(original.uid) + } + + @Test + fun aSaveKeepsABeforeStartReminderTheEditorNeverShows() { + val id = source.insertTask(form(due = now + 1.days)) + source.setReminders(id, listOf(TaskReminder(10, fromStart = true), TaskReminder(30))) + + source.updateTask(id, form(due = now + 1.days), dueReminders = listOf(45)) + + assertThat(source.reminders()[id]).containsExactly(TaskReminder(10, fromStart = true), TaskReminder(45)) + } + + @Test + fun aReminderForOneOccurrenceLeavesTheSeriesUntouched() { + val work = syncedList() + val id = source.insertTask(form(due = now + 1.days).copy(listId = work), listOf(TaskReminder(30))) + makeRecurring(id, now) + db.tasks().update(db.tasks().entity(id)!!.copy(isDirty = false)) + val target = source.tasks(TaskQuery(listId = work)) + .filter { it.taskId == id } + .first { it.distanceFromCurrent == 1 } + + source.updateInstance(id, target.occurrenceStart!!, form(title = "moved").copy(listId = work), listOf(60)) + + // ⚠️ The master is never written, not even briefly: a sync landing in + // between would upload the occurrence's reminder for the whole series. + assertThat(db.alarms().forTask(id).map { it.minutesBefore }).containsExactly(30) + assertThat(db.tasks().entity(id)!!.isDirty).isFalse() + val override = db.tasks().override(id, target.occurrenceStart)!! + assertThat(db.alarms().forTask(override.id).map { it.minutesBefore }).containsExactly(60) + } + + @Test + fun splittingASeriesGivesTheNewSeriesItsOwnReminderIdentities() { + val id = source.insertTask(form(due = now + 1.days), listOf(TaskReminder(30))) + makeRecurring(id, now - 14.days) + val master = db.alarms().forTask(id).single() + val target = source.tasks(TaskQuery(listId = listId)) + .filter { it.taskId == id } + .first { it.distanceFromCurrent == 1 } + + val created = source.splitSeries(id, target.occurrenceStart!!, form(title = "later"), listOf(30)) + + assertThat(created).isNotEqualTo(id) + val copy = db.alarms().forTask(created).single() + assertThat(copy.minutesBefore).isEqualTo(30) + // A different UID on the server, so the alarm must not share one either. + assertThat(copy.uid).isNotEqualTo(master.uid) + assertThat(db.alarms().forTask(id).single().uid).isEqualTo(master.uid) + } + + @Test + fun changingOnlyTheRemindersOwesTheServerAPut() { + val work = syncedList() + val id = source.insertTask(form(due = now + 1.days).copy(listId = work)) + db.tasks().update(db.tasks().entity(id)!!.copy(isDirty = false)) + + source.setReminders(id, listOf(TaskReminder(15))) + assertThat(db.tasks().entity(id)!!.isDirty).isTrue() + + db.tasks().update(db.tasks().entity(id)!!.copy(isDirty = false)) + source.setReminders(id, listOf(TaskReminder(15))) + // Nothing changed, so nothing is owed. + assertThat(db.tasks().entity(id)!!.isDirty).isFalse() + } + + @Test + fun aSeriesReminderEditReachesTheOccurrencesThatStillHadTheOldOne() { + val id = source.insertTask(form(due = now + 1.days), listOf(TaskReminder(15))) + makeRecurring(id, now) + val occurrences = source.tasks(TaskQuery(listId = listId)).filter { it.taskId == id } + val completed = occurrences.first { it.distanceFromCurrent == 1 }.occurrenceStart!! + val own = occurrences.first { it.distanceFromCurrent == 2 }.occurrenceStart!! + source.setCompletedInstance(id, completed, true) + source.updateInstance(id, own, form(title = "own"), listOf(5)) + + // Same times and rule, so the series is not reshaped — that drops its overrides. + val first = occurrences.first() + val unchanged = form().copy(start = first.start, due = first.due, rrule = "FREQ=WEEKLY") + source.updateSeries(id, first.occurrenceStart!!, unchanged, listOf(60)) + + fun leads(anchor: Instant) = db.alarms().forTask(db.tasks().override(id, anchor)!!.id).map { it.minutesBefore } + assertThat(db.alarms().forTask(id).map { it.minutesBefore }).containsExactly(60) + // A snapshot of the series' reminder follows it… + assertThat(leads(completed)).containsExactly(60) + // …one set for that occurrence alone does not. + assertThat(leads(own)).containsExactly(5) + } + + @Test + fun aTaskWhoseAlarmsWereTurnedOffElsewhereReportsNone() { + val id = source.insertTask(form(due = now + 1.days)) + db.alarms().setRemindersOff(id, true) + + assertThat(source.reminders()[id]).isEmpty() + assertThat(source.reminders()).containsKey(id) + + // A reminder of its own again: the flag goes. + source.setReminders(id, listOf(TaskReminder(15))) + assertThat(db.tasks().entity(id)!!.remindersOff).isFalse() + } + + @Test + fun settingTheEditableReminderOwesTheServerAPut() { + val work = syncedList() + val id = source.insertTask(form(due = now + 1.days).copy(listId = work)) + db.tasks().update(db.tasks().entity(id)!!.copy(isDirty = false)) + + source.setAlarm(id, 30) + + assertThat(db.tasks().entity(id)!!.isDirty).isTrue() + } + @Test fun deletingATaskInALocalListRemovesItOutright() { val id = source.insertTask(form()) @@ -447,7 +575,7 @@ class RoomTasksDataSourceTest { source.updateInstance(id, target.occurrenceStart!!, form(title = "moved")) // The fork copies the master's properties, which is what carries the - // reminder across — the repository is what puts the series' own back. + // reminder across. val override = db.tasks().override(id, target.occurrenceStart)!! assertThat(db.alarms().forTask(override.id).single().minutesBefore).isEqualTo(30) } 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 ca0d824..a6fa376 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 @@ -75,6 +75,78 @@ class TasksDatabaseMigrationTest { } } + @Test + fun liftsClaimableAlarmsAndPushesRemindersThatNeverLeftTheDevice() { + val future = System.currentTimeMillis() + 7L * 24 * 60 * 60 * 1000 + val past = System.currentTimeMillis() - 7L * 24 * 60 * 60 * 1000 + val claimable = "X-FOO:bar\r\nBEGIN:VALARM\r\nACTION:DISPLAY\r\nX-WR-ALARMUID:srv\r\n" + + "TRIGGER;RELATED=END:-PT15M\r\nEND:VALARM\r\n" + val appleDefault = "BEGIN:VALARM\r\nACTION:DISPLAY\r\nX-APPLE-DEFAULT-ALARM:TRUE\r\n" + + "TRIGGER;RELATED=END:-PT30M\r\nEND:VALARM\r\n" + helper.createDatabase(TEST_DB, 2).use { db -> + db.execSQL("INSERT INTO accounts (id, display_name) VALUES (1, 'me')") + db.execSQL("INSERT INTO task_lists (id, name, color, account_id) VALUES (1, 'Synced', 0, 1)") + db.execSQL("INSERT INTO task_lists (id, name, color) VALUES (2, 'Local', 0)") + db.execSQL("INSERT INTO task_lists (id, name, color, account_id, is_read_only) VALUES (3, 'Shared', 0, 1, 1)") + fun task(id: Int, list: Int, due: Long, residue: String? = null) = db.execSQL( + "INSERT INTO tasks (id, list_id, uid, title, due, unknown_properties) VALUES (?, ?, ?, ?, ?, ?)", + arrayOf(id, list, "uid-$id", "Task $id", due, residue), + ) + fun alarm(task: Int, minutes: Int) = + db.execSQL("INSERT INTO task_alarms (task_id, minutes_before) VALUES (?, ?)", arrayOf(task, minutes)) + // A server alarm, and the same lead copied locally by a fork. + task(1, 1, future, claimable) + alarm(1, 15) + // Set in Agendula, never uploaded — still to fire, already past, device-only. + task(2, 1, future) + alarm(2, 30) + task(3, 1, past) + alarm(3, 30) + task(4, 2, future) + alarm(4, 30) + // Somebody else's default alarm: not ours to claim. + task(5, 1, future, appleDefault) + // A read-only share: dirty would freeze it, never sent and never refreshed. + task(6, 3, future) + alarm(6, 30) + } + + helper.runMigrationsAndValidate(TEST_DB, 3, true, TasksDatabase.MIGRATION_2_3).use { db -> + db.query("SELECT task_id, minutes_before, uid, raw FROM task_alarms ORDER BY task_id").use { c -> + val rows = buildList { while (c.moveToNext()) add(listOf(c.getLong(0), c.getInt(1), c.getString(2), c.getString(3))) } + // One for task 1: the lifted copy won and the local duplicate went. + assertThat(rows.map { it[0] }).containsExactly(1L, 2L, 3L, 4L, 6L).inOrder() + assertThat(rows[0][2]).isEqualTo("srv") + assertThat(rows[0][3] as String).contains("TRIGGER;RELATED=END:-PT15M") + // Every pre-existing row got an identity of its own, shaped as a UUID. + assertThat(rows.drop(1).map { it[2] as String }.toSet()).hasSize(4) + rows.drop(1).forEach { + assertThat(it[2] as String).matches("[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}") + } + assertThat(rows.drop(1).all { (it[2] as String).isNotEmpty() && it[3] == null }).isTrue() + } + db.query("SELECT id, unknown_properties, is_dirty, last_modified, reminders_off FROM tasks ORDER BY id").use { c -> + c.moveToNext() + assertThat(c.getString(1)).isEqualTo("X-FOO:bar\r\n") + assertThat(c.getInt(2)).isEqualTo(0) + assertThat(c.getInt(4)).isEqualTo(0) + c.moveToNext() + assertThat(c.getInt(2)).isEqualTo(1) + // The body changes, so its modification stamp does too. + assertThat(c.isNull(3)).isFalse() + c.moveToNext() + assertThat(c.getInt(2)).isEqualTo(0) + c.moveToNext() + assertThat(c.getInt(2)).isEqualTo(0) + c.moveToNext() + assertThat(c.getString(1)).contains("X-APPLE-DEFAULT-ALARM:TRUE") + assertThat(c.getInt(2)).isEqualTo(0) + c.moveToNext() + assertThat(c.getInt(2)).isEqualTo(0) + } + } + } + private companion object { const val TEST_DB = "migration-test.db" } diff --git a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/transfer/ExternalImportTest.kt b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/transfer/ExternalImportTest.kt index 8bc1eb3..0fe2d57 100644 --- a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/transfer/ExternalImportTest.kt +++ b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/transfer/ExternalImportTest.kt @@ -299,9 +299,10 @@ private class FakeExternalStore : TasksDataSource { override fun tasks(query: TaskQuery): List = unused() override fun task(taskId: Long): Task? = unused() override fun subtasks(parentTaskId: Long): List = unused() - override fun insertTask(form: TaskForm): Long = unused() - override fun updateTask(taskId: Long, form: TaskForm) = unused() - override fun updateInstance(taskId: Long, occurrenceStart: Instant, form: TaskForm) = unused() + override fun insertTask(form: TaskForm, reminders: List?): Long = unused() + override fun updateTask(taskId: Long, form: TaskForm, dueReminders: List?) = unused() + override fun updateInstance(taskId: Long, occurrenceStart: Instant, form: TaskForm, dueReminders: List?) = + unused() override fun setAlarm(taskId: Long, minutesBeforeDue: Int?) = unused() override fun setReminders(taskId: Long, reminders: List) = unused() override fun setCompleted(taskId: Long, completed: Boolean) = unused() @@ -309,8 +310,10 @@ private class FakeExternalStore : TasksDataSource { override fun deleteTask(taskId: Long) = unused() override fun setCancelled(taskId: Long, cancelled: Boolean) = unused() override fun setCancelledInstance(taskId: Long, occurrenceStart: Instant, cancelled: Boolean) = unused() - override fun updateSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm) = unused() - override fun splitSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm): Long = unused() + override fun updateSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm, dueReminders: List?) = + unused() + override fun splitSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm, dueReminders: List?): Long = + unused() override fun deleteOccurrence(seriesId: Long, occurrenceStart: Instant) = unused() override fun deleteFollowing(seriesId: Long, occurrenceStart: Instant) = unused() override fun createLocalList(name: String, color: Int): Long = unused() 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 4b46d42..abc7dbf 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 @@ -1,8 +1,10 @@ package de.jeanlucmakiola.agendula.data.sync +import de.jeanlucmakiola.agendula.data.tasks.ical.AlarmCodec import de.jeanlucmakiola.agendula.data.tasks.ical.CalendarResource import de.jeanlucmakiola.agendula.data.tasks.ical.ResourceValidator import de.jeanlucmakiola.agendula.data.tasks.ical.VTodoMapper +import de.jeanlucmakiola.agendula.data.tasks.room.TaskAlarmEntity import de.jeanlucmakiola.agendula.data.tasks.room.TaskEntity import de.jeanlucmakiola.agendula.data.tasks.room.TaskListEntity import at.bitfire.dav4jvm.exception.UnauthorizedException @@ -422,8 +424,9 @@ class CollectionSyncer( /** Null when the resource was rejected before it left the device. */ private fun serialize(local: LocalResource): String? { + val alarms = store.alarmsFor(local.live.map { it.id }) val todos = local.live.map { row -> - VTodoMapper.write(row, parentUidOf(row), now()) + VTodoMapper.write(row, parentUidOf(row), now(), alarms[row.id].orEmpty()) } val calendar = CalendarResource.build(todos) @@ -778,14 +781,17 @@ class CollectionSyncer( val losing = existing.values.firstOrNull { it.isDirty } val master = mapped.firstOrNull { it.entity.recurrenceId == null } ?: mapped.first() - val masterRow = upsert(master.entity, existing[master.entity.recurrenceId], null, href, eTag) + val masterRow = upsert(master, existing[master.entity.recurrenceId], null, href, eTag) val masterId = masterRow.id parents[uid] = master.parentUid val written = mutableListOf(masterRow) mapped.filter { it !== master }.forEach { override -> + // ⚠️ An override's alarms are its own: RFC 5545 has an override + // replace the component, so one the server sends without a + // VALARM has none, whatever its master carries. written += upsert( - override.entity, existing[override.entity.recurrenceId], masterId, href, eTag, + override, existing[override.entity.recurrenceId], masterId, href, eTag, ) } val kept = written.map { it.recurrenceId }.toSet() @@ -832,12 +838,13 @@ class CollectionSyncer( /** @return the row as it now stands, so the caller can index it. */ private fun upsert( - incoming: TaskEntity, + mapped: VTodoMapper.Mapped, existing: TaskEntity?, masterId: Long?, href: String, eTag: String?, ): TaskEntity { + val incoming = mapped.entity // Local-only columns the server has no opinion about. Taking the // mapper's defaults here would silently reset the user's ordering and // per-task colour on every download. @@ -854,12 +861,42 @@ class CollectionSyncer( // set uploads what was just downloaded. isDirty = false, isDeleted = false, + // The store's to change, from what the alarms do. + remindersOff = existing?.remindersOff ?: false, ) - return if (existing == null) { - row.copy(id = store.insert(row)) - } else { - store.update(row) - row + // The server's alarms replace ours, as every other field does: a + // dirty row is never downloaded except to discard its edit. + val alarms = existing?.let { keepLocalPrecision(mapped.alarms, it, row) } ?: mapped.alarms + return row.copy(id = store.upsertWithAlarms(row, alarms)) + } + + /** + * [incoming] with each alarm we uploaded ourselves put back to the lead + * we hold for it. + * + * ⚠️ The wire is coarser than the row: an all-day lead goes out in whole + * days, and a before-start reminder on a task without a start goes out + * before-due. Taking the round trip at its word would quietly turn a + * 15-minute all-day reminder into "on the day", or a before-start one + * into a before-due one, the first time the task is downloaded again. + * An alarm that comes back as exactly what we would have sent has not + * been changed, so the row's own lead stands. + */ + private fun keepLocalPrecision( + incoming: List, + existing: TaskEntity, + task: TaskEntity, + ): List { + val local = store.alarmsFor(listOf(existing.id))[existing.id].orEmpty() + if (local.isEmpty()) return incoming + return incoming.map { alarm -> + val mine = local.firstOrNull { it.uid == alarm.uid } ?: return@map alarm + val sent = AlarmCodec.wireLead(mine, task) + if (sent == AlarmCodec.Lead(alarm.minutesBefore, alarm.reference)) { + alarm.copy(minutesBefore = mine.minutesBefore, reference = mine.reference) + } else { + alarm + } } } 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 b3940c5..0cb0544 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 @@ -1,5 +1,6 @@ package de.jeanlucmakiola.agendula.data.sync +import de.jeanlucmakiola.agendula.data.tasks.room.TaskAlarmEntity import de.jeanlucmakiola.agendula.data.tasks.room.TaskEntity import de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase import javax.inject.Inject @@ -11,17 +12,33 @@ import javax.inject.Inject * discovery: the reconciliation above this interface is where local edits are * discarded, tombstones swept and conflicts resolved, and every one of those is * a decision that should be provable without a device. Room's test double is - * Robolectric plus an in-memory database; this is eight methods. + * Robolectric plus an in-memory database; this is a dozen methods. */ interface SyncStore { /** Every row in a list, **tombstones included**. */ fun rowsIn(listId: Long): List - fun insert(row: TaskEntity): Long - fun update(row: TaskEntity) + /** The reminders of [taskIds] — one resource's rows — by task id, for the body that uploads them. */ + fun alarmsFor(taskIds: List): Map> + + /** + * Writes a downloaded row — inserted when its id is 0 — and makes its + * reminders exactly [alarms], in one transaction. + * + * ⚠️ One transaction, not an [update] and a second call. The row lands clean + * with the server's ETag, so if the process died between the two, nothing + * would ever download it again and the reminders would stay stale for good. + * + * A row that is gone by the time it is written — deleted on the device + * while the sync ran — stays gone, reminders and all. + * + * @return the row's id. + */ + fun upsertWithAlarms(row: TaskEntity, alarms: List): Long + fun deleteAll(taskIds: List) /** @@ -56,12 +73,26 @@ class RoomSyncStore @Inject constructor( override fun rowsIn(listId: Long) = database.tasks().allIn(listId) - override fun insert(row: TaskEntity) = database.tasks().insert(row) - override fun update(row: TaskEntity) { database.tasks().update(row) } + override fun alarmsFor(taskIds: List) = + if (taskIds.isEmpty()) emptyMap() else database.alarms().forTasks(taskIds).groupBy { it.taskId } + + override fun upsertWithAlarms(row: TaskEntity, alarms: List): Long = + database.runInTransaction { + if (row.id == 0L) { + database.tasks().insert(row).also { database.alarms().replaceFromServer(it, alarms) } + } else { + // ⚠️ Zero rows means the row was deleted under us. Its alarms + // would then name a task that does not exist, and the foreign + // key would fail the whole transaction. + if (database.tasks().update(row) > 0) database.alarms().replaceFromServer(row.id, alarms) + row.id + } + } + override fun deleteAll(taskIds: List) { if (taskIds.isNotEmpty()) database.tasks().deleteAll(taskIds) } 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 65a744b..5d93f08 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) + .addMigrations(TasksDatabase.MIGRATION_1_2, TasksDatabase.MIGRATION_2_3) .build() /** diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/prefs/SettingsPrefs.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/prefs/SettingsPrefs.kt index 02a0c6c..332fe0a 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/prefs/SettingsPrefs.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/prefs/SettingsPrefs.kt @@ -115,6 +115,10 @@ data class Settings( /** The lead times for an all-day task in [listId]. */ fun allDayReminderLeadsFor(listId: Long): List = perListAllDayReminderOverride.reminderLeadsFor(listId, defaultAllDayReminderMinutes) + + /** The default leads a task in [listId] takes — the one rule for which default applies. */ + fun defaultLeadsFor(listId: Long, allDay: Boolean): List = + if (allDay) allDayReminderLeadsFor(listId) else reminderLeadsFor(listId) } /** App preferences, backed by DataStore. Mirrors Calendula's prefs shape. */ diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/reminders/ReminderPlanner.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/reminders/ReminderPlanner.kt index d341a72..30f787c 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/reminders/ReminderPlanner.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/reminders/ReminderPlanner.kt @@ -42,25 +42,41 @@ internal object ReminderPlanner { // the one the editor shows; otherwise the task's list may override the // global lead, or opt out entirely (override = null). All-day tasks // have their own day-scale defaults. - val reminders = perTask[task.taskId].orEmpty().ifEmpty { - val leads = if (task.isAllDay) settings.allDayReminderLeadsFor(task.listId) - else settings.reminderLeadsFor(task.listId) - leads.map { TaskReminder(it) } - } - reminders.map { reminder -> + // + // ⚠️ Present-but-empty is "none", not "the default": the store's + // answer for a task whose alarms another client turned off. A + // default here would keep ringing for what the user switched off. + val reminders = perTask[task.taskId] + ?: settings.defaultLeadsFor(task.listId, task.isAllDay).map { TaskReminder(it) } + reminders.mapNotNull { reminder -> // A stored reminder says what it counts back from. Ours are always // before due, but an imported dmfs alarm or another client's can be // before *start*. val anchor = if (reminder.fromStart) task.start ?: task.due!! else task.due!! + val trigger = triggerAt( + anchor, + task.isAllDay, + reminder.minutesBefore, + settings.allDayReminderMinuteOfDay, + zone, + ) + // Dismissed on another client already: firing it here within + // the missed-reminder grace would ring for something done with. + // + // ⚠️ An all-day reminder is compared from the start of its day, + // not from this device's time of day: other clients ring it at + // midnight, or at their own hour, so a dismissal at 07:00 must + // still silence the 09:00 this device would ring it at. + val earliest = if (task.isAllDay) { + triggerAt(anchor, true, reminder.minutesBefore, 0, zone) + } else { + trigger + } + val acknowledged = reminder.acknowledgedAt?.let { earliest <= it.toEpochMilliseconds() } == true + if (acknowledged) return@mapNotNull null ScheduledReminder( taskId = task.taskId, - triggerAt = triggerAt( - anchor, - task.isAllDay, - reminder.minutesBefore, - settings.allDayReminderMinuteOfDay, - zone, - ), + triggerAt = trigger, occurrenceStart = task.occurrenceStart?.toEpochMilliseconds() ?: ScheduledReminder.NO_OCCURRENCE, ) } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/AndroidTasksDataSource.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/AndroidTasksDataSource.kt index b87069c..5bcfc2d 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/AndroidTasksDataSource.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/AndroidTasksDataSource.kt @@ -127,9 +127,10 @@ class AndroidTasksDataSource @Inject constructor( // --- writes --------------------------------------------------------------- - override fun insertTask(form: TaskForm): Long { + override fun insertTask(form: TaskForm, reminders: List?): Long { val values = TaskWriteMapper.seriesValues(form, ZoneId.systemDefault().id) - // One batch, so a refused tag leaves no task behind for a retry to duplicate. + // One batch, so a refused tag or alarm leaves no task behind for a retry + // to duplicate. val authority = authority() val ops = arrayListOf( ContentProviderOperation.newInsert(TasksContract.tasksUri(authority)) @@ -142,20 +143,37 @@ class AndroidTasksDataSource @Inject constructor( .withValueBackReference(Properties.TASK_ID, 0) .build() } + reminders.orEmpty().forEach { reminder -> + ops += ContentProviderOperation.newInsert(TasksContract.propertiesUri(authority)) + .withValues( + TaskWriteMapper.alarmValues(0, reminder.minutesBefore, reminder.fromStart) + .toContentValues().apply { remove(Properties.TASK_ID) }, + ) + .withValueBackReference(Properties.TASK_ID, 0) + .build() + } val results = runCatching { resolver.applyBatch(authority, ops) } .getOrElse { throw TaskWriteFailedException("insert task", it) } return results.firstOrNull()?.uri?.lastPathSegment?.toLongOrNull() ?: throw TaskWriteFailedException("insert task: no id") } - override fun updateTask(taskId: Long, form: TaskForm) { + override fun updateTask(taskId: Long, form: TaskForm, dueReminders: List?) { val values = TaskWriteMapper.seriesValues(form, ZoneId.systemDefault().id) val rows = resolver.update(taskUri(authority(), taskId), values.toContentValues(), null, null) if (rows == 0) throw TaskWriteFailedException("update task $taskId") setCategories(taskId, form.tags) + writeDueReminders(taskId, dueReminders) } - override fun updateSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm) { + /** [leads] as the task's before-due reminders; a start-relative one from elsewhere is kept. */ + private fun writeDueReminders(taskId: Long, leads: List?) { + if (leads == null) return + val kept = alarmRows(taskId).map { it.second }.filter { it.fromStart } + setReminders(taskId, kept + leads.distinct().sorted().map { TaskReminder(it) }) + } + + override fun updateSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm, dueReminders: List?) { val occurrence = task(seriesId, occurrenceStart) val anchors = resolver.query( taskUri(authority(), seriesId), arrayOf(Tasks.DTSTART, Tasks.DUE), null, null, null, @@ -175,6 +193,7 @@ class AndroidTasksDataSource @Inject constructor( val rows = resolver.update(taskUri(authority(), seriesId), values.toContentValues(), null, null) if (rows == 0) throw TaskWriteFailedException("update series $seriesId") setCategories(seriesId, form.tags) + writeDueReminders(seriesId, dueReminders) } /** @@ -183,10 +202,10 @@ class AndroidTasksDataSource @Inject constructor( * task. Per-occurrence edits past the split point are dropped rather than * carried over — the provider offers no way to re-parent an exception. */ - override fun splitSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm): Long { + override fun splitSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm, dueReminders: List?): Long { val spec = seriesSpec(seriesId) ?: throw TaskWriteFailedException("split series $seriesId") if (spec.rrule == null || occurrenceStart <= spec.anchor) { - updateSeries(seriesId, occurrenceStart, form) + updateSeries(seriesId, occurrenceStart, form, dueReminders) return seriesId } // COUNT counts before EXDATE is applied (RFC 5545 §3.8.5.3). @@ -196,7 +215,8 @@ class AndroidTasksDataSource @Inject constructor( ).size endSeriesBefore(seriesId, spec, occurrenceStart) val rule = form.rrule?.let { if (it == spec.rrule) TaskFormWriter.remainingCount(it, spent) else it } - return insertTask(form.copy(rrule = rule, percentComplete = null)) + val reminders = dueReminders?.distinct()?.sorted()?.map { TaskReminder(it) } + return insertTask(form.copy(rrule = rule, percentComplete = null), reminders) } override fun deleteFollowing(seriesId: Long, occurrenceStart: Instant) { @@ -262,7 +282,23 @@ class AndroidTasksDataSource @Inject constructor( resolver.delete(TasksContract.instanceUri(authority(), instanceId), null, null) } - override fun updateInstance(taskId: Long, occurrenceStart: Instant, form: TaskForm) { + override fun updateInstance(taskId: Long, occurrenceStart: Instant, form: TaskForm, dueReminders: List?) { + if (dueReminders == null) return writeInstance(taskId, occurrenceStart, form) + // ⚠️ Written to the series, and then put back. Forking an occurrence + // copies the task's *current* properties onto the new override row, so + // setting the alarm beforehand is the only way to carry it across — and + // leaving it there would silently give every other occurrence in the + // series the reminder the user set on one of them. + val seriesReminders = alarmRows(taskId).map { it.second } + writeDueReminders(taskId, dueReminders) + try { + writeInstance(taskId, occurrenceStart, form) + } finally { + setReminders(taskId, seriesReminders) + } + } + + private fun writeInstance(taskId: Long, occurrenceStart: Instant, form: TaskForm) { val instanceId = instanceIdFor(taskId, occurrenceStart) ?: throw TaskWriteFailedException("update instance $taskId@$occurrenceStart: no such occurrence") val values = TaskWriteMapper.instanceValues(form, ZoneId.systemDefault().id) diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ModeRoutingTasksDataSource.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ModeRoutingTasksDataSource.kt index 1f25caf..e6596b1 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ModeRoutingTasksDataSource.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ModeRoutingTasksDataSource.kt @@ -34,20 +34,21 @@ class ModeRoutingTasksDataSource( override fun tasks(query: TaskQuery): List = active().tasks(query) override fun task(taskId: Long): Task? = active().task(taskId) override fun task(taskId: Long, occurrenceStart: Instant?): Task? = active().task(taskId, occurrenceStart) - override fun updateSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm) = - active().updateSeries(seriesId, occurrenceStart, form) - override fun splitSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm): Long = - active().splitSeries(seriesId, occurrenceStart, form) + override fun updateSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm, dueReminders: List?) = + active().updateSeries(seriesId, occurrenceStart, form, dueReminders) + override fun splitSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm, dueReminders: List?): Long = + active().splitSeries(seriesId, occurrenceStart, form, dueReminders) override fun deleteOccurrence(seriesId: Long, occurrenceStart: Instant) = active().deleteOccurrence(seriesId, occurrenceStart) override fun deleteFollowing(seriesId: Long, occurrenceStart: Instant) = active().deleteFollowing(seriesId, occurrenceStart) override fun subtasks(parentTaskId: Long): List = active().subtasks(parentTaskId) - override fun insertTask(form: TaskForm): Long = active().insertTask(form) - override fun updateTask(taskId: Long, form: TaskForm) = active().updateTask(taskId, form) + override fun insertTask(form: TaskForm, reminders: List?): Long = active().insertTask(form, reminders) + override fun updateTask(taskId: Long, form: TaskForm, dueReminders: List?) = + active().updateTask(taskId, form, dueReminders) - override fun updateInstance(taskId: Long, occurrenceStart: Instant, form: TaskForm) = - active().updateInstance(taskId, occurrenceStart, form) + override fun updateInstance(taskId: Long, occurrenceStart: Instant, form: TaskForm, dueReminders: List?) = + active().updateInstance(taskId, occurrenceStart, form, dueReminders) override fun setAlarm(taskId: Long, minutesBeforeDue: Int?) = active().setAlarm(taskId, minutesBeforeDue) override fun alarms(): Map = active().alarms() diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksDataSource.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksDataSource.kt index 92f5eef..3a3987a 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksDataSource.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksDataSource.kt @@ -13,7 +13,19 @@ import kotlin.time.Instant * them — while Agendula's own UI only ever sets a before-due lead. Collapsing it * to a number here is what silently fired those reminders off the wrong anchor. */ -data class TaskReminder(val minutesBefore: Int, val fromStart: Boolean = false) +data class TaskReminder( + val minutesBefore: Int, + val fromStart: Boolean = false, + /** + * When another client last dismissed this alarm (RFC 9074 `ACKNOWLEDGED`): + * a trigger at or before it has been dealt with and is not fired here. + * Never part of what a reminder *is* — see [lead]. + */ + val acknowledgedAt: Instant? = null, +) { + /** The reminder without its bookkeeping, for comparing two of them. */ + val lead: TaskReminder get() = if (acknowledgedAt == null) this else copy(acknowledgedAt = null) +} /** * The one reminder Agendula's single-reminder editor shows and writes: the last @@ -46,8 +58,17 @@ interface TasksDataSource { fun task(taskId: Long, occurrenceStart: Instant?): Task? = task(taskId) fun subtasks(parentTaskId: Long): List - fun insertTask(form: TaskForm): Long - fun updateTask(taskId: Long, form: TaskForm) + /** + * The write paths take the task's reminders in the same write as the task + * itself, so a sync can never upload one without the other. + * + * [insertTask] takes the new task's [reminders] whole. The updates take the + * before-due leads as `dueReminders`, and `null` leaves the reminders + * alone; a start-relative reminder the task already carries is kept — the + * editor never shows one, so it never sends one. + */ + fun insertTask(form: TaskForm, reminders: List? = null): Long + fun updateTask(taskId: Long, form: TaskForm, dueReminders: List? = null) /** * Update a single occurrence of a recurring task, addressed by the task row and @@ -60,7 +81,7 @@ interface TasksDataSource { * instance row id keeps this seam independent of any one store's row * numbering; External mode maps it back to an instance row itself. */ - fun updateInstance(taskId: Long, occurrenceStart: Instant, form: TaskForm) + fun updateInstance(taskId: Long, occurrenceStart: Instant, form: TaskForm, dueReminders: List? = null) /** * Set (or clear, with `null`) the task's [editable] reminder lead, stored as an * Alarm property row. Every other reminder on the task is left untouched. The @@ -74,13 +95,13 @@ interface TasksDataSource { * Edit the whole series [seriesId] through its occurrence at [occurrenceStart]: * content from [form], and the series moved by however far that occurrence moved. */ - fun updateSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm) + fun updateSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm, dueReminders: List? = null) /** * "This and following": end [seriesId] before [occurrenceStart] and continue * from there as a new series built from [form]. Returns the new series' id. */ - fun splitSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm): Long + fun splitSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm, dueReminders: List? = null): Long /** Delete one generated occurrence of [seriesId] (an `EXDATE`). */ fun deleteOccurrence(seriesId: Long, occurrenceStart: Instant) @@ -89,10 +110,14 @@ interface TasksDataSource { fun deleteFollowing(seriesId: Long, occurrenceStart: Instant) - /** Replace every reminder on [taskId] with [reminders], in order. */ + /** Make [taskId]'s reminders exactly [reminders]. A synced task is owed a PUT for it. */ fun setReminders(taskId: Long, reminders: List) - /** Every task's reminders, by task id, in stored order. One query, for the scheduler. */ + /** + * Every task's reminders, by task id, in stored order. One query, for the + * scheduler. A task missing from the map takes the default; one present + * with an empty list has none (see `TaskEntity.remindersOff`). + */ fun reminders(): Map> = alarms().mapValues { listOf(it.value) } /** Every task's [editable] reminder, by task id. */ 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 1a519ce..70991c5 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 @@ -21,6 +21,7 @@ import kotlinx.coroutines.channels.consumeEach import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.callbackFlow +import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.flowOn @@ -109,11 +110,41 @@ class TasksRepositoryImpl @Inject constructor( override suspend fun createTask(form: TaskForm): Long = withContext(io) { - val id = dataSource.insertTask(form) - writeReminders(id, form) - id + dataSource.insertTask(form, dueRemindersFor(form, current = null).distinct().sorted().map { TaskReminder(it) }) } + /** + * The before-due leads to store for [form], given the task as it was. + * + * A task that gets its due date in this write and names no reminder of its + * own takes the list's or the app's default *as its own reminders*. The + * default is this device's setting, which nothing else can see; stored on + * the task, it travels with it to the server and fires on every client. + * The cost is tasks.org's: changing the default later does not reach the + * tasks that already have it. + * + * A task that already had a due date keeps what the form says, so an + * edit that clears every reminder means "use the default here" as before — + * with one exception. Switching between timed and all-day swaps the + * default it was given for the other kind's, as long as the reminders are + * still exactly that default: a timed 15 minutes is no lead for an all-day + * task, which the planner used to sort out by itself while the default + * stayed unstored. + */ + private suspend fun dueRemindersFor(form: TaskForm, current: Task?): List { + if (form.due == null) return form.reminders + val hadDue = current?.due != null + if (!hadDue) { + if (form.reminders.isNotEmpty()) return form.reminders + return settingsPrefs.settings.first().defaultLeadsFor(form.listId, form.isAllDay) + } + if (current.isAllDay == form.isAllDay || form.reminders.isEmpty()) return form.reminders + val settings = settingsPrefs.settings.first() + val previous = settings.defaultLeadsFor(current.listId, current.isAllDay) + return if (form.reminders.sorted() == previous.sorted()) settings.defaultLeadsFor(form.listId, form.isAllDay) + else form.reminders + } + override suspend fun duplicateTask(taskId: Long, occurrenceStart: Instant?): Long = withContext(io) { val task = checkNotNull(dataSource.task(taskId, occurrenceStart)) { "Task $taskId is gone" } @@ -121,6 +152,9 @@ class TasksRepositoryImpl @Inject constructor( } private fun copyOf(task: Task, parentId: Long?, depth: Int): Long { + // In the same write as the copy: inserted first and given its reminders + // after, a sync landing in between uploads the copy without them. + val reminders = dataSource.reminders()[task.taskId].orEmpty().map { it.lead } val id = dataSource.insertTask( TaskForm( title = task.title, @@ -135,9 +169,8 @@ class TasksRepositoryImpl @Inject constructor( url = task.url, tags = task.tags, ), + reminders, ) - val reminders = dataSource.reminders()[task.taskId].orEmpty() - if (reminders.isNotEmpty()) dataSource.setReminders(id, reminders) if (depth < MAX_COPY_DEPTH) { SeriesCollapse.visible(dataSource.subtasks(task.taskId)) .filter { it.taskId != task.taskId && it.status != TaskStatus.CANCELLED } @@ -172,18 +205,17 @@ class TasksRepositoryImpl @Inject constructor( val seen = current?.lastModified if (seen != null && seen != expectedLastModified) throw TaskConflictException(taskId) } + val reminders = dueRemindersFor(form, current) val seriesId = current?.seriesId val occurrence = current?.occurrenceStart if (seriesId != null && occurrence != null) { when (scope) { RecurringScope.AllOccurrences -> { - writeReminders(seriesId, form) - dataSource.updateSeries(seriesId, occurrence, form) + dataSource.updateSeries(seriesId, occurrence, form, reminders) return@withContext } RecurringScope.ThisAndFollowing -> { - val created = dataSource.splitSeries(seriesId, occurrence, form) - writeReminders(created, form) + dataSource.splitSeries(seriesId, occurrence, form, reminders) return@withContext } RecurringScope.ThisOccurrence -> Unit @@ -191,31 +223,11 @@ class TasksRepositoryImpl @Inject constructor( } // A recurring task's start/due are one occurrence's resolved times, so // writing them back to the task row would re-anchor the whole series. - // Going through the occurrence forks an override instead. + // 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) { - writeReminders(taskId, form) - dataSource.updateTask(taskId, form) - return@withContext - } - // ⚠️ Written to the series, and then put back. Forking an occurrence - // copies the task's *current* properties onto the new override row, - // so setting the alarm beforehand is the only way to carry it - // across — and leaving it there would silently give every other - // occurrence in the series the reminder the user set on one of them. - val seriesReminders = dataSource.reminders()[taskId].orEmpty() - writeReminders(taskId, form) - try { - dataSource.updateInstance(taskId, instance, form) - } finally { - dataSource.setReminders(taskId, seriesReminders) - } - } - - /** The form's before-due reminders; a start-relative one from elsewhere is kept as it was. */ - private fun writeReminders(taskId: Long, form: TaskForm) { - val kept = dataSource.reminders()[taskId].orEmpty().filter { it.fromStart } - dataSource.setReminders(taskId, kept + form.reminders.distinct().sorted().map { TaskReminder(it) }) + if (instance == null) dataSource.updateTask(taskId, form, reminders) + else dataSource.updateInstance(taskId, instance, form, reminders) } override suspend fun setCancelled(taskId: Long, occurrenceStart: Instant?, cancelled: Boolean) = diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ical/AlarmCodec.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ical/AlarmCodec.kt new file mode 100644 index 0000000..80de961 --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ical/AlarmCodec.kt @@ -0,0 +1,215 @@ +package de.jeanlucmakiola.agendula.data.tasks.ical + +import de.jeanlucmakiola.agendula.data.reminders.ReminderPlanner +import de.jeanlucmakiola.agendula.data.tasks.room.AlarmReference +import de.jeanlucmakiola.agendula.data.tasks.room.TaskAlarmEntity +import de.jeanlucmakiola.agendula.data.tasks.room.TaskEntity +import de.jeanlucmakiola.agendula.domain.ical.ICalComponent +import de.jeanlucmakiola.agendula.domain.ical.ICalParam +import de.jeanlucmakiola.agendula.domain.ical.ICalParser +import de.jeanlucmakiola.agendula.domain.ical.ICalProperty +import de.jeanlucmakiola.agendula.domain.ical.ICalSerializer +import de.jeanlucmakiola.agendula.domain.ical.ICalValues +import kotlin.math.roundToLong +import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.Duration.Companion.seconds +import kotlin.time.Instant + +/** + * `VALARM` ↔ [TaskAlarmEntity]. + * + * ## What is claimed + * + * Only an alarm Agendula can fire exactly as written becomes a reminder row. + * Everything else stays in the task's residue and round-trips untouched — it + * still fires in the client that wrote it, just not here: + * + * - `ACTION` other than `DISPLAY`/`AUDIO`; + * - Apple's default alarms (`X-APPLE-DEFAULT-ALARM:TRUE`). They are that Mac's + * preference, not task data: claimed, they would come back to the Mac as an + * explicit alarm without the flag after the first edit here; + * - location alarms (`X-APPLE-PROXIMITY`, RFC 9074 `PROXIMITY`, a nested + * component) and Apple's `19760401T005545Z` sentinel trigger they carry; + * - repeating alarms (`REPEAT`/`DURATION`) and RFC 9074 snoozes (`RELATED-TO`); + * - a trigger *after* its anchor. The planner coerces a negative lead to 0, so + * it would fire at the wrong time; + * - an absolute trigger on a recurring task, or on one with no `DUE` to count + * back from. + * + * ## What "unchanged" means + * + * A claimed row keeps the `VALARM` it was read from in [TaskAlarmEntity.raw]. + * On write, the row is "unchanged" when that original still [resolve]s to the + * row's lead against the task *as it is now* — so `-PT15M` and `-P0DT0H15M0S` + * are the same, a relative alarm survives a due-date edit, and an absolute one + * stops matching once `DUE` moves. An unchanged row is re-emitted verbatim; a + * changed one is authored fresh under its existing uid. + * + * ## Anchors + * + * `RELATED=END` counts from `DUE`. `RELATED=START` (the default) counts from + * `DTSTART` — except when `DTSTART` equals `DUE`, or when there is no + * `DTSTART` but there is a `DUE`. Apple writes the latter shape (against §3.8.6.3, + * which wants `DTSTART` present), and reading it as a before-due reminder is + * what makes it editable here: the editor only shows before-due rows. + * + * ## All-day tasks + * + * The local fire time of day is a device preference this mapper cannot see, so + * an all-day lead is whole days, written `-PD` against `DUE`'s midnight. Read + * back, any trigger on an all-day task rounds to the nearest whole day. + */ +object AlarmCodec { + + private const val DAY_MINUTES = 24 * 60 + + /** Apple's placeholder trigger for location alarms. Never a real time. */ + private const val APPLE_SENTINEL = "19760401T005545Z" + + /** What a `VALARM` means to the scheduler. */ + data class Lead(val minutesBefore: Int, val reference: AlarmReference) + + /** A `VALARM` as a reminder row — `taskId` is the caller's — or null if it is not ours to claim. */ + fun fromVAlarm(alarm: ICalComponent, task: TaskEntity): TaskAlarmEntity? { + val lead = resolve(alarm, task) ?: return null + val raw = ICalSerializer.serialize(alarm) + // Too large to keep verbatim means it could not be re-emitted, and an + // alarm that is authored fresh on every write loses what made it large. + if (raw.toByteArray(Charsets.UTF_8).size > VTodoMapper.MAX_RESIDUE_BYTES) return null + val uid = (alarm.property("X-WR-ALARMUID") ?: alarm.property("UID"))?.value?.trim()?.takeIf { it.isNotEmpty() } + val base = TaskAlarmEntity( + taskId = 0, + minutesBefore = lead.minutesBefore, + reference = lead.reference, + raw = raw, + ) + return if (uid == null) base else base.copy(uid = uid) + } + + /** The lead [alarm] stands for on [task], or null when it is not claimable. */ + fun resolve(alarm: ICalComponent, task: TaskEntity): Lead? { + val action = alarm.property("ACTION")?.value?.trim()?.uppercase() + if (action != "DISPLAY" && action != "AUDIO") return null + if (alarm.property("X-APPLE-DEFAULT-ALARM")?.value?.trim().equals("TRUE", ignoreCase = true)) return null + val excluded = listOf("X-APPLE-PROXIMITY", "PROXIMITY", "REPEAT", "DURATION", "RELATED-TO") + if (excluded.any { alarm.property(it) != null }) return null + if (alarm.components.isNotEmpty()) return null + + val trigger = alarm.properties("TRIGGER").singleOrNull() ?: return null + val value = trigger.value.trim() + if (value.startsWith(APPLE_SENTINEL)) return null + + val absolute = trigger.param("VALUE").equals("DATE-TIME", ignoreCase = true) || + (!value.startsWith("P", ignoreCase = true) && !value.startsWith("-") && !value.startsWith("+")) + return if (absolute) absoluteLead(trigger, task) else relativeLead(trigger, task) + } + + private fun relativeLead(trigger: ICalProperty, task: TaskEntity): Lead? { + val seconds = ICalValues.parseDuration(trigger.value) ?: return null + val reference = when { + trigger.param("RELATED").equals("END", ignoreCase = true) -> + if (task.due != null) AlarmReference.DUE else return null + task.dtstart != null && task.dtstart != task.due -> AlarmReference.START + task.due != null -> AlarmReference.DUE + else -> return null + } + val before = -seconds + val minutes = if (task.isAllDay) { + // Rounded before the sign check: `PT9H` on a date-only DUE is + // "09:00 on the day", which is this device's "on the day". + roundToDays(before) + } else { + // A lead in seconds is not one the planner can reproduce. + if (before % 60 != 0L) return null + before / 60 + } + return minutes.toIntOrNull()?.let { Lead(it, reference) } + } + + private fun absoluteLead(trigger: ICalProperty, task: TaskEntity): Lead? { + if (task.rrule != null || task.rdate != null) return null + val due = task.due ?: return null + val at = (ICalValues.parseTime(trigger) as? ICalValues.TimeValue.Timed) + ?.takeIf { it.tzid == null } // §3.8.6.3: an absolute trigger MUST be UTC + ?.instant ?: return null + val before = (due - at).inWholeSeconds + val minutes = if (task.isAllDay) { + // An all-day DUE is stored as UTC midnight of its date, which is + // the calendar date the trigger counts back from. + roundToDays(before) + } else { + Math.floorDiv(before, 60L) + } + return minutes.toIntOrNull()?.let { Lead(it, AlarmReference.DUE) } + } + + /** Nearest whole day, in minutes. */ + private fun roundToDays(seconds: Long): Long = + (seconds / 86_400.0).roundToLong() * DAY_MINUTES + + /** Negative is after the anchor, which is not claimed. */ + private fun Long.toIntOrNull(): Int? = takeIf { it in 0..Int.MAX_VALUE }?.toInt() + + /** + * [row] as a `VALARM` on [task], or null when it cannot be written: its + * anchor is missing. That row stays on the device — the planner ignores it + * too — because `RELATED=END` without `DUE` is a resource the server + * refuses ([VTodoMapper.validate]). + * + * ⚠️ A before-start row on a task that has lost its start is written + * against `DUE`, which is what the planner fires it from. Leaving it out + * would drop it from every other client, and the next download would then + * take it off this device too. + * + * [now] marks an authored alarm `ACKNOWLEDGED` once it is past even the + * planner's missed-reminder grace, so a client that honours RFC 9074 does + * not greet it as a missed alarm. Not sooner: the stamp comes back on the + * next download and would silence the late reminder this device still owes. + */ + fun toVAlarm(row: TaskAlarmEntity, task: TaskEntity, now: Instant): ICalComponent? { + val reference = if (row.reference == AlarmReference.START && task.dtstart == null) AlarmReference.DUE + else row.reference + val lead = Lead(row.minutesBefore, reference) + row.raw?.let(::parse)?.let { original -> + if (resolve(original, task) == lead) return original + } + + val anchor = when (reference) { + AlarmReference.DUE -> task.due + AlarmReference.START -> task.dtstart + } ?: return null + val related = if (reference == AlarmReference.DUE) "END" else "START" + val minutes = row.minutesBefore.coerceAtLeast(0).toLong() + val seconds = if (task.isAllDay) { + -((minutes + DAY_MINUTES / 2) / DAY_MINUTES) * 86_400 + } else { + -minutes * 60 + } + + val properties = mutableListOf( + ICalProperty("UID", value = row.uid), + ICalProperty("X-WR-ALARMUID", value = row.uid), + ICalProperty("ACTION", value = "DISPLAY"), + ICalProperty("DESCRIPTION", value = ICalValues.escapeText(row.message ?: "Reminder")), + ICalProperty("TRIGGER", listOf(ICalParam("RELATED", related)), ICalValues.formatDuration(seconds)), + ) + val recurring = task.rrule != null || task.rdate != null + if (!recurring && anchor + seconds.seconds + ReminderPlanner.MISSED_GRACE_MS.milliseconds < now) { + properties += ICalProperty("ACKNOWLEDGED", value = ICalValues.formatDateTime(now, null)) + } + return ICalComponent("VALARM", properties) + } + + /** + * What [row] reads back as once it has been to the server and back — not + * always the row itself, because the wire is coarser: an all-day lead is + * whole days, and a before-start row on a task without a start goes out + * before-due. A download that brings back exactly this has not changed the + * reminder, and the row's own lead must be kept rather than taken from it. + */ + fun wireLead(row: TaskAlarmEntity, task: TaskEntity): Lead? = + toVAlarm(row, task, Instant.DISTANT_PAST)?.let { resolve(it, task) } + + private fun parse(raw: String): ICalComponent? = + runCatching { ICalParser.parse(raw) }.getOrNull()?.takeIf { it.name.equals("VALARM", ignoreCase = true) } +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapper.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapper.kt index 084d84b..313bf7b 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapper.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapper.kt @@ -1,5 +1,6 @@ package de.jeanlucmakiola.agendula.data.tasks.ical +import de.jeanlucmakiola.agendula.data.tasks.room.TaskAlarmEntity import de.jeanlucmakiola.agendula.data.tasks.room.TaskEntity import de.jeanlucmakiola.agendula.domain.PRIORITY_NONE import de.jeanlucmakiola.agendula.domain.TaskStatus @@ -52,10 +53,11 @@ import kotlin.time.Instant * * ## Alarms * - * `VALARM`s round-trip in the residue and are never authored here. Local - * reminders live in `task_alarms` and are never serialised. The two stores are - * disjoint, so neither can destroy the other. Merging them is a later - * decision, not a silent one. + * A `VALARM` [AlarmCodec] can fire exactly as written is claimed into + * [Mapped.alarms] — rows for `task_alarms`, carrying the original verbatim — + * and [write] authors the task's rows back. Any other `VALARM` (Apple's default + * and location alarms, repeating ones, snoozes) stays in the residue and is + * re-emitted untouched after the authored ones. */ object VTodoMapper { @@ -95,6 +97,8 @@ object VTodoMapper { val uidWasMissing: Boolean, /** Residue that exceeded [MAX_RESIDUE_BYTES] and had to be dropped. */ val droppedResidue: Boolean, + /** The claimed `VALARM`s, `taskId` unset. Not counted against the residue cap. */ + val alarms: List = emptyList(), ) // ---------------------------------------------------------------- read @@ -263,8 +267,18 @@ object VTodoMapper { .filterIndexed { index, _ -> index !in claimed } .filterNot { it.name.equals("DTSTAMP", ignoreCase = true) } + // Claimed against the entity as read, so the anchors are the ones the + // rows will be written back against. + val alarms = mutableListOf() + val residueComponents = vtodo.components.filter { component -> + if (!component.name.equals("VALARM", ignoreCase = true)) return@filter true + val row = AlarmCodec.fromVAlarm(component, entity) ?: return@filter true + alarms += row + false + } + val residueText = ICalSerializer.serializeProperties(residueProperties) + - ICalSerializer.serializeAll(vtodo.components) + ICalSerializer.serializeAll(residueComponents) val tooLarge = residueText.toByteArray(Charsets.UTF_8).size > MAX_RESIDUE_BYTES return Mapped( @@ -274,6 +288,7 @@ object VTodoMapper { parentUid = parentUid, uidWasMissing = uid.isEmpty(), droppedResidue = tooLarge, + alarms = alarms, ) } @@ -283,12 +298,14 @@ object VTodoMapper { * Serialises [entity] back to a VTODO. * * [parentUid] is the parent's `UID`, which the entity holds only as a row id. - * [now] is the `DTSTAMP`. + * [now] is the `DTSTAMP`. [alarms] are the task's reminder rows; one whose + * anchor the task no longer has is left out (see [AlarmCodec.toVAlarm]). */ fun write( entity: TaskEntity, parentUid: String? = null, now: Instant = Clock.System.now(), + alarms: List = emptyList(), ): ICalComponent { val residue = parseResidue(entity.unknownProperties) val residueCategories = normalizeTags(residue.properties("CATEGORIES").flatMap(::categoryValues)) @@ -349,7 +366,7 @@ object VTodoMapper { return ICalComponent( name = "VTODO", properties = properties + keptResidue, - components = residue.components, + components = alarms.mapNotNull { AlarmCodec.toVAlarm(it, entity, now) } + residue.components, ) } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/AlarmLift.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/AlarmLift.kt new file mode 100644 index 0000000..a43ce8f --- /dev/null +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/AlarmLift.kt @@ -0,0 +1,101 @@ +package de.jeanlucmakiola.agendula.data.tasks.room + +import de.jeanlucmakiola.agendula.domain.ical.ICalComponent +import de.jeanlucmakiola.agendula.domain.ical.ICalParser +import de.jeanlucmakiola.agendula.domain.ical.ICalProperty +import de.jeanlucmakiola.agendula.domain.ical.ICalSerializer +import de.jeanlucmakiola.agendula.domain.ical.ICalValues +import kotlin.math.roundToLong +import kotlin.time.Instant + +/** + * Lifts claimable `VALARM`s out of a residue written before v3 synced them. + * + * ⚠️ Frozen with [TasksDatabase.MIGRATION_2_3]: a copy of `AlarmCodec`'s claim + * rule as it stood then, not a call into it, so a later change to what the + * codec claims cannot change what this migration produced for an earlier + * upgrader. + */ +internal object AlarmLift { + + /** The task columns the claim rule reads. */ + data class Task( + val dtstart: Instant?, + val due: Instant?, + val isAllDay: Boolean, + val recurring: Boolean, + ) + + data class Alarm(val minutesBefore: Int, val reference: AlarmReference, val uid: String?, val raw: String) + + /** The lifted alarms and the residue without them, or `null` residue when nothing is left. */ + data class Result(val alarms: List, val residue: String?) + + private const val DAY_MINUTES = 24 * 60 + private const val MAX_RAW_BYTES = 25 * 1024 + + fun lift(residue: String?, task: Task): Result { + val component = parse(residue) ?: return Result(emptyList(), residue) + val alarms = mutableListOf() + val kept = component.components.filter { sub -> + if (!sub.name.equals("VALARM", ignoreCase = true)) return@filter true + val alarm = claim(sub, task) ?: return@filter true + alarms += alarm + false + } + if (alarms.isEmpty()) return Result(emptyList(), residue) + val rest = ICalSerializer.serializeProperties(component.properties) + ICalSerializer.serializeAll(kept) + return Result(alarms, rest.ifEmpty { null }) + } + + private fun claim(alarm: ICalComponent, task: Task): Alarm? { + val action = alarm.property("ACTION")?.value?.trim()?.uppercase() + if (action != "DISPLAY" && action != "AUDIO") return null + if (alarm.property("X-APPLE-DEFAULT-ALARM")?.value?.trim().equals("TRUE", ignoreCase = true)) return null + if (listOf("X-APPLE-PROXIMITY", "PROXIMITY", "REPEAT", "DURATION", "RELATED-TO").any { alarm.property(it) != null }) { + return null + } + if (alarm.components.isNotEmpty()) return null + val trigger = alarm.properties("TRIGGER").singleOrNull() ?: return null + val value = trigger.value.trim() + if (value.startsWith("19760401T005545Z")) return null + val absolute = trigger.param("VALUE").equals("DATE-TIME", ignoreCase = true) || + (!value.startsWith("P", ignoreCase = true) && !value.startsWith("-") && !value.startsWith("+")) + val (minutes, reference) = (if (absolute) absolute(trigger, task) else relative(trigger, task)) ?: return null + val raw = ICalSerializer.serialize(alarm) + if (raw.toByteArray(Charsets.UTF_8).size > MAX_RAW_BYTES) return null + val uid = (alarm.property("X-WR-ALARMUID") ?: alarm.property("UID"))?.value?.trim()?.takeIf { it.isNotEmpty() } + return Alarm(minutes, reference, uid, raw) + } + + private fun relative(trigger: ICalProperty, task: Task): Pair? { + val seconds = ICalValues.parseDuration(trigger.value) ?: return null + val reference = when { + trigger.param("RELATED").equals("END", ignoreCase = true) -> + if (task.due != null) AlarmReference.DUE else return null + task.dtstart != null && task.dtstart != task.due -> AlarmReference.START + task.due != null -> AlarmReference.DUE + else -> return null + } + val before = -seconds + val minutes = if (task.isAllDay) days(before) else if (before % 60 != 0L) return null else before / 60 + return minutes.takeIf { it in 0..Int.MAX_VALUE }?.let { it.toInt() to reference } + } + + private fun absolute(trigger: ICalProperty, task: Task): Pair? { + if (task.recurring) return null + val due = task.due ?: return null + val at = (ICalValues.parseTime(trigger) as? ICalValues.TimeValue.Timed) + ?.takeIf { it.tzid == null }?.instant ?: return null + val before = (due - at).inWholeSeconds + val minutes = if (task.isAllDay) days(before) else Math.floorDiv(before, 60L) + return minutes.takeIf { it in 0..Int.MAX_VALUE }?.let { it.toInt() to AlarmReference.DUE } + } + + private fun days(seconds: Long): Long = (seconds / 86_400.0).roundToLong() * DAY_MINUTES + + private fun parse(text: String?): ICalComponent? { + if (text.isNullOrEmpty()) return null + return runCatching { ICalParser.parse("BEGIN:VTODO\r\n$text\r\nEND:VTODO\r\n") }.getOrNull() + } +} diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/Entities.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/Entities.kt index 513efc1..cad74ee 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/Entities.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/Entities.kt @@ -8,6 +8,7 @@ import androidx.room.PrimaryKey import androidx.room.TypeConverters import de.jeanlucmakiola.agendula.domain.PRIORITY_NONE import de.jeanlucmakiola.agendula.domain.TaskStatus +import java.util.UUID import kotlin.time.Instant /** @@ -199,12 +200,31 @@ data class TaskEntity( * verbatim on write so a round-trip cannot silently lose a field. */ @ColumnInfo(name = "unknown_properties") val unknownProperties: String? = null, + + // reminders + /** + * Another client removed this task's last alarm, so it has none — not "use + * the default". A task without reminder rows normally takes the list's or + * the app's default, which is right for one that never had any (a client + * that does not write alarms), and wrong for one whose alarms were turned + * off elsewhere: it would keep ringing here. Set by a download that takes + * the last row away; cleared as soon as the task has a reminder again. + */ + @ColumnInfo(name = "reminders_off", defaultValue = "0") val remindersOff: Boolean = false, ) /** What [TaskAlarmEntity.minutesBefore] counts back from. */ enum class AlarmReference { DUE, START } -/** A reminder lead on a task. Positive [minutesBefore] is *before* [reference]. */ +/** + * A reminder lead on a task. Positive [minutesBefore] is *before* [reference]. + * + * Synced as a `VALARM` (see `AlarmCodec`). [uid] is the alarm's identity on the + * wire — `X-WR-ALARMUID` / RFC 9074 `UID` — minted when the row is created, so + * every upload of the same alarm names it the same way. [raw] is the `VALARM` + * this row was read from, re-emitted verbatim while the row still means what + * it did, so another client's `ACKNOWLEDGED`, `X-*` and trigger form survive. + */ @Entity( tableName = "task_alarms", foreignKeys = [ @@ -224,4 +244,6 @@ data class TaskAlarmEntity( @ColumnInfo(name = "minutes_before") val minutesBefore: Int, @ColumnInfo(name = "reference", defaultValue = "DUE") val reference: AlarmReference = AlarmReference.DUE, @ColumnInfo(name = "message") val message: String? = null, + @ColumnInfo(name = "uid", defaultValue = "") val uid: String = UUID.randomUUID().toString(), + @ColumnInfo(name = "raw") val raw: String? = null, ) 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 b759ea2..34a9b37 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 @@ -33,6 +33,9 @@ internal const val SPLIT_COUNT_CEILING = 100_000 private val OBSERVED_TABLES = arrayOf("tasks", "task_lists", "task_alarms", "accounts") +/** Distinct alarm texts whose `ACKNOWLEDGED` is remembered between scheduler passes. */ +private const val ACK_CACHE_SIZE = 512 + /** * [TasksDataSource] over Agendula's own Room store. * @@ -104,13 +107,23 @@ class RoomTasksDataSource @Inject constructor( override fun exportTasks(listId: Long): List = tasks.exportTasks(listId).map(RoomTaskMapper::exportTask) - override fun reminders(): Map> = - alarms.all().groupBy({ it.taskId }) { - TaskReminder( - minutesBefore = it.minutesBefore, - fromStart = it.reference == AlarmReference.START, - ) + override fun reminders(): Map> { + val stored = alarms.all().groupBy({ it.taskId }) { it.asReminder().copy(acknowledgedAt = acknowledged(it)) } + // Turned off elsewhere: present and empty, so nothing falls back to a default. + return stored + alarms.remindersOff().filterNot(stored::containsKey).associateWith { emptyList() } + } + + /** `raw` → its `ACKNOWLEDGED`, so a scheduler pass does not re-parse every alarm. */ + private val acknowledgements = object : LinkedHashMap(64, 0.75f, true) { + override fun removeEldestEntry(eldest: MutableMap.MutableEntry) = size > ACK_CACHE_SIZE + } + + private fun acknowledged(row: TaskAlarmEntity): Instant? { + val raw = row.raw ?: return null + return synchronized(acknowledgements) { + if (raw in acknowledgements) acknowledgements[raw] else row.acknowledgedAt().also { acknowledgements[raw] = it } } + } override fun alarms(): Map = reminders().mapNotNull { (id, list) -> list.editable()?.let { id to it } }.toMap() @@ -173,20 +186,44 @@ class RoomTasksDataSource @Inject constructor( // --- writes --------------------------------------------------------------- - override fun insertTask(form: TaskForm): Long { + override fun insertTask(form: TaskForm, reminders: List?): Long { if (lists.exists(form.listId) == 0) throw TaskWriteFailedException("insert task: no list ${form.listId}") val entity = TaskFormWriter.newTask(form, uid = UUID.randomUUID().toString(), now = clock.now(), tzId = zone()) - return tasks.insert(TaskFormWriter.withRule(entity, form.rrule)).also { writes.onWritten(setOf(form.listId)) } + var id = 0L + tx { + id = tasks.insert(TaskFormWriter.withRule(entity, form.rrule)) + reminders?.let { alarms.syncForTask(id, it) } + writes.onWritten(setOf(form.listId)) + } + return id } - override fun updateTask(taskId: Long, form: TaskForm) { + /** + * [leads] as [taskId]'s before-due reminders, inside the caller's + * transaction. A start-relative row is kept: the editor never shows one. + * + * ⚠️ Part of the task write rather than a second call after it. Once + * reminders sync, a sync landing between the two would upload the task + * without them, or with the ones it is about to lose. + */ + private fun dueLeads(taskId: Long): List = + alarms.forTask(taskId).filter { it.reference == AlarmReference.DUE }.map { it.minutesBefore }.sorted() + + private fun writeDueReminders(taskId: Long, leads: List?) { + if (leads == null) return + val kept = alarms.forTask(taskId).filter { it.reference == AlarmReference.START }.map { it.asReminder() } + alarms.syncForTask(taskId, kept + leads.distinct().sorted().map { TaskReminder(it) }) + } + + override fun updateTask(taskId: Long, form: TaskForm, dueReminders: List?) = tx { val current = tasks.entity(taskId) ?: throw TaskWriteFailedException("update task $taskId") val edited = TaskFormWriter.apply(current, form, clock.now(), zone()) + writeDueReminders(taskId, dueReminders) if (current.masterId == null && edited.listId != current.listId) { if (lists.exists(edited.listId) == 0) throw TaskWriteFailedException("move task: no list ${edited.listId}") - database.runInTransaction { move(current, TaskFormWriter.withRule(edited, form.rrule)) } + move(current, TaskFormWriter.withRule(edited, form.rrule)) writes.onWritten(setOf(current.listId, edited.listId)) - return + return@tx } // ⚠️ An override's list and parent are the master's, the rule // [updateInstance] states and this path has to keep. An overridden @@ -220,8 +257,20 @@ class RoomTasksDataSource @Inject constructor( plan.tombstone?.let { tasks.insert(it) } } - override fun updateSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm) = tx { + override fun updateSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm, dueReminders: List?) = tx { val master = seriesMaster(seriesId) + // ⚠️ The overrides too, where they still carry the series' reminders. + // A fork copies them as a snapshot — completing one occurrence is + // enough — and each override uploads its own VALARMs, so leaving them + // would keep ringing those occurrences at the old lead everywhere. One + // whose reminders were set for that occurrence alone keeps them. + if (dueReminders != null) { + val before = dueLeads(master.id) + tasks.overrides(master.id) + .filter { dueLeads(it.id) == before } + .forEach { writeDueReminders(it.id, dueReminders) } + } + writeDueReminders(master.id, dueReminders) val (start, due) = resolvedTimes(master, occurrenceStart) val edited = TaskFormWriter.seriesEdit(master, start, due, form, clock.now(), zone()) // A moved or re-ruled series no longer generates the anchors its @@ -237,14 +286,14 @@ class RoomTasksDataSource @Inject constructor( writes.onWritten(setOf(master.listId, edited.listId)) } - override fun splitSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm): Long { + override fun splitSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm, dueReminders: List?): Long { var created = seriesId tx { val master = seriesMaster(seriesId) val spec = master.recurrenceSpec() val anchor = master.recurrenceAnchor if (spec == null || anchor == null || occurrenceStart <= anchor) { - updateSeries(seriesId, occurrenceStart, form) + updateSeries(seriesId, occurrenceStart, form, dueReminders) return@tx } val now = clock.now() @@ -287,7 +336,9 @@ class RoomTasksDataSource @Inject constructor( else TaskFormWriter.shiftValues(laterExceptions, delta, master.timezone ?: zone()) val fresh = TaskFormWriter.withRule(applied, rule).let { if (it.rrule != null) it.copy(exdate = exceptions) else it } val newId = tasks.insert(fresh) - alarms.forTask(master.id).forEach { alarms.insert(it.copy(id = 0, taskId = newId)) } + // A new UID, so a new resource: the alarms are copies, not the same ones. + alarms.copyToTask(master.id, newId, sameResource = false) + writeDueReminders(newId, dueReminders) val unmoved = fresh.dtstart == start && fresh.due == due && sameRule tasks.overrides(master.id) @@ -352,7 +403,7 @@ class RoomTasksDataSource @Inject constructor( * which is the model least compatible with sync; the override shares its * master's UID, which is exactly what makes it an override. */ - override fun updateInstance(taskId: Long, occurrenceStart: Instant, form: TaskForm) { + override fun updateInstance(taskId: Long, occurrenceStart: Instant, form: TaskForm, dueReminders: List?) = tx { val master = tasks.entity(taskId) ?: throw TaskWriteFailedException("update instance $taskId") val now = clock.now() val existing = tasks.override(taskId, occurrenceStart) @@ -361,8 +412,9 @@ class RoomTasksDataSource @Inject constructor( // master's, whatever the form was carrying. val edited = TaskFormWriter.apply(existing, form, now, zone()) tasks.update(edited.copy(listId = master.listId, parentId = master.parentId)) + writeDueReminders(existing.id, dueReminders) writes.onWritten(setOf(master.listId)) - return + return@tx } val (start, due) = occurrenceTimes(master, occurrenceStart) val fork = TaskFormWriter.apply( @@ -374,28 +426,27 @@ class RoomTasksDataSource @Inject constructor( // The list and parent come from the master: moving one occurrence between // lists or parents is not something the override model expresses. val id = tasks.insert(fork.copy(listId = master.listId, parentId = master.parentId)) - alarms.replaceAllForTask(id, alarms.forTask(taskId)) + // The occurrence starts out with the series' reminders and then takes + // the form's — the master's own are never touched. + alarms.copyToTask(taskId, id, sameResource = true) + writeDueReminders(id, dueReminders) writes.onWritten(setOf(master.listId)) } + /** Through [setReminders], so a synced task is owed the PUT and the others keep their identity. */ override fun setAlarm(taskId: Long, minutesBeforeDue: Int?) { - alarms.replaceEditable( - taskId, - minutesBeforeDue?.let { TaskAlarmEntity(taskId = taskId, minutesBefore = it) }, - ) + val current = alarms.forTask(taskId).map { it.asReminder() } + val editable = current.indexOfLast { !it.fromStart } + val others = if (editable < 0) current else current.filterIndexed { index, _ -> index != editable } + setReminders(taskId, others + listOfNotNull(minutesBeforeDue?.let { TaskReminder(it) })) } - override fun setReminders(taskId: Long, reminders: List) { - alarms.replaceAllForTask( - taskId, - reminders.map { - TaskAlarmEntity( - taskId = taskId, - minutesBefore = it.minutesBefore, - reference = if (it.fromStart) AlarmReference.START else AlarmReference.DUE, - ) - }, - ) + override fun setReminders(taskId: Long, reminders: List) = tx { + if (!alarms.syncForTask(taskId, reminders)) return@tx + // The reminders are part of the VTODO now, so changing them is an edit. + val current = tasks.entity(taskId) ?: return@tx + tasks.update(current.copy(isDirty = true, lastModified = clock.now())) + writes.onWritten(setOf(current.listId)) } override fun setCancelled(taskId: Long, cancelled: Boolean) { @@ -447,7 +498,7 @@ class RoomTasksDataSource @Inject constructor( } val (start, due) = occurrenceTimes(master, occurrenceStart) val id = tasks.insert(write(newOverride(master, taskId, occurrenceStart, start, due))) - alarms.replaceAllForTask(id, alarms.forTask(taskId)) + alarms.copyToTask(taskId, id, sameResource = true) writes.onWritten(setOf(master.listId)) } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskAlarmDao.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskAlarmDao.kt index b9091e1..a7a5539 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskAlarmDao.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/room/TaskAlarmDao.kt @@ -4,6 +4,11 @@ import androidx.room.Dao import androidx.room.Insert import androidx.room.Query import androidx.room.Transaction +import androidx.room.Update +import de.jeanlucmakiola.agendula.data.tasks.TaskReminder +import de.jeanlucmakiola.agendula.domain.ical.ICalParser +import de.jeanlucmakiola.agendula.domain.ical.ICalValues +import kotlin.time.Instant /** Reads and writes over `task_alarms`. */ @Dao @@ -16,6 +21,10 @@ interface TaskAlarmDao { @Query("SELECT * FROM task_alarms WHERE task_id = :taskId ORDER BY id") fun forTask(taskId: Long): List + /** The reminders of several tasks in one query — one resource's rows, for an upload. */ + @Query("SELECT * FROM task_alarms WHERE task_id IN (:taskIds) ORDER BY id") + fun forTasks(taskIds: List): List + @Insert fun insert(alarm: TaskAlarmEntity): Long @@ -25,28 +34,121 @@ interface TaskAlarmDao { @Query("DELETE FROM task_alarms WHERE task_id = :taskId") fun deleteForTask(taskId: Long): Int + @Update + fun update(alarm: TaskAlarmEntity) + + /** See [TaskEntity.remindersOff]. */ + @Query("UPDATE tasks SET reminders_off = :off WHERE id = :taskId") + fun setRemindersOff(taskId: Long, off: Boolean): Int + + /** The tasks whose alarms were turned off elsewhere — "none", not "the default". */ + @Query("SELECT id FROM tasks WHERE reminders_off = 1") + fun remindersOff(): List + /** Set the task's only reminder, or clear every one with `null`. */ @Transaction - fun replaceForTask(taskId: Long, alarm: TaskAlarmEntity?) = replaceAllForTask(taskId, listOfNotNull(alarm)) - - /** - * Replace every reminder on the task. The rows that land are always new ones — - * ids are cleared so alarms lifted off another task (forking an occurrence - * copies the master's) insert instead of colliding. - */ - @Transaction - fun replaceAllForTask(taskId: Long, alarms: List) { + fun replaceForTask(taskId: Long, alarm: TaskAlarmEntity?) { deleteForTask(taskId) - alarms.forEach { insert(it.copy(id = 0, taskId = taskId)) } - } - - /** - * Set the task's editable reminder — its last before-due one — or clear it - * with `null`. Start-relative and additional reminders stay as they are. - */ - @Transaction - fun replaceEditable(taskId: Long, alarm: TaskAlarmEntity?) { - forTask(taskId).lastOrNull { it.reference == AlarmReference.DUE }?.let { delete(it.id) } alarm?.let { insert(it.copy(id = 0, taskId = taskId)) } } + + /** + * Make the task's reminders [wanted], touching only what differs. + * + * ⚠️ A diff, not a delete-and-reinsert. A row that is still wanted keeps its + * [TaskAlarmEntity.uid] and its [TaskAlarmEntity.raw] — which [TaskReminder] + * carries neither of, so a rewrite from it would give every save a new + * alarm identity on the server and throw away another client's `VALARM` the + * user never touched. + * + * Duplicates count: two identical leads are two rows. + * + * @return whether anything changed. + */ + @Transaction + fun syncForTask(taskId: Long, wanted: List): Boolean { + val unmatched = wanted.map { it.lead }.toMutableList() + var changed = false + forTask(taskId).forEach { row -> + if (!unmatched.remove(row.asReminder())) { + delete(row.id) + changed = true + } + } + unmatched.forEach { + insert(it.toAlarmEntity(taskId)) + changed = true + } + // A reminder of its own again, so the "turned off elsewhere" no longer holds. + if (wanted.isNotEmpty()) setRemindersOff(taskId, false) + return changed + } + + /** + * Replace the task's reminders with what a download carried. + * + * Incoming alarms are matched to the rows already here by uid first, then by + * lead, so a reminder the server merely re-described keeps its row rather + * than being churned into a new one. + * + * A download that takes away every alarm the task had marks it + * [TaskEntity.remindersOff]; one that carries alarms clears that. + */ + @Transaction + fun replaceFromServer(taskId: Long, incoming: List) { + val existing = forTask(taskId).toMutableList() + val hadAny = existing.isNotEmpty() + val placed = incoming.map { alarm -> + val match = existing.firstOrNull { it.uid == alarm.uid } + ?: existing.firstOrNull { it.asReminder() == alarm.asReminder() } + match?.let(existing::remove) + alarm.copy(id = match?.id ?: 0, taskId = taskId) + } + existing.forEach { delete(it.id) } + placed.forEach { if (it.id == 0L) insert(it) else update(it) } + when { + incoming.isNotEmpty() -> setRemindersOff(taskId, false) + hadAny -> setRemindersOff(taskId, true) + } + } + + /** + * Copy [fromTaskId]'s reminders onto [toTaskId]. + * + * [sameResource] is for an override forked inside the same series — the + * same `VTODO` UID, so the alarms keep their identity. Anything else is a + * different task on the server and gets fresh uids: two tasks naming one + * alarm is an identity no other client expects. + */ + @Transaction + fun copyToTask(fromTaskId: Long, toTaskId: Long, sameResource: Boolean) { + forTask(fromTaskId).forEach { insert(if (sameResource) it.copy(id = 0, taskId = toTaskId) else it.freshCopy(toTaskId)) } + } +} + +/** What the row means to the scheduler and the editor, without its identity. */ +fun TaskAlarmEntity.asReminder() = TaskReminder(minutesBefore, reference == AlarmReference.START) + +/** The inverse of [asReminder]: a new row, with an identity of its own. */ +fun TaskReminder.toAlarmEntity(taskId: Long) = TaskAlarmEntity( + taskId = taskId, + minutesBefore = minutesBefore, + reference = if (fromStart) AlarmReference.START else AlarmReference.DUE, +) + +/** The same reminder on another task: what it means, never who it is on the server. */ +fun TaskAlarmEntity.freshCopy(taskId: Long) = + TaskAlarmEntity(taskId = taskId, minutesBefore = minutesBefore, reference = reference, message = message) + +/** + * The `ACKNOWLEDGED` stamp of the `VALARM` the row was read from, if any. + * + * Read off [TaskAlarmEntity.raw] rather than kept in a column: it is the other + * client's bookkeeping, and it changes only when a download replaces the row. + */ +fun TaskAlarmEntity.acknowledgedAt(): Instant? { + val text = raw ?: return null + if (!text.contains("ACKNOWLEDGED", ignoreCase = true)) return null + val property = runCatching { ICalParser.parse(text) }.getOrNull()?.property("ACKNOWLEDGED") ?: return null + return (ICalValues.parseTime(property) as? ICalValues.TimeValue.Timed)?.instant } 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 a203d11..9f9ac7e 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 @@ -5,6 +5,8 @@ import androidx.room.RoomDatabase import androidx.room.TypeConverters import androidx.room.migration.Migration import androidx.sqlite.db.SupportSQLiteDatabase +import java.util.UUID +import kotlin.time.Instant /** * Agendula's own task store. Four tables, designed from @@ -20,7 +22,7 @@ import androidx.sqlite.db.SupportSQLiteDatabase TaskEntity::class, TaskAlarmEntity::class, ], - version = 2, + version = 3, exportSchema = true, ) @TypeConverters(Converters::class) @@ -63,5 +65,113 @@ abstract class TasksDatabase : RoomDatabase() { } } } + + /** + * v3 syncs reminders as `VALARM`s, which takes two things the rows did not + * have: an identity on the wire (`uid`) and the original text (`raw`). + * + * Claimable `VALARM`s are lifted out of the residue for every row, synced + * or not — an unchanged task is never downloaded again, and an imported + * one never was. Not marked dirty: the server already has them. Where a + * task held the same lead in both stores (an override forked before v3 + * copies the master's alarms *and* carries its `VALARM`s), the lifted + * one wins: it is the one with an identity. + * + * Reminders that so far lived only on this device are pushed once, by + * marking their tasks dirty — bounded to open tasks in synced lists whose + * reminder can still fire, so the upgrade is not a burst of PUTs landing + * on other clients as long-missed alarms. + * + * `tasks.reminders_off` starts at 0 everywhere: until now nothing could + * have turned a synced task's alarms off. + */ + val MIGRATION_2_3 = object : Migration(2, 3) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL("ALTER TABLE task_alarms ADD COLUMN uid TEXT NOT NULL DEFAULT ''") + db.execSQL("ALTER TABLE task_alarms ADD COLUMN raw TEXT") + db.execSQL("ALTER TABLE tasks ADD COLUMN reminders_off INTEGER NOT NULL DEFAULT 0") + // A version-4 UUID per row, in one statement: `randomblob` is + // evaluated per row, and the upgrade runs on app start. + db.execSQL( + """ + UPDATE task_alarms SET uid = + lower(hex(randomblob(4))) || '-' || lower(hex(randomblob(2))) || '-4' || + substr(lower(hex(randomblob(2))), 2) || '-' || + substr('89ab', 1 + (abs(random()) % 4), 1) || substr(lower(hex(randomblob(2))), 2) || '-' || + lower(hex(randomblob(6))) + """.trimIndent(), + ) + + data class Lifted(val taskId: Long, val result: AlarmLift.Result) + val lifted = mutableListOf() + db.query( + "SELECT id, dtstart, due, is_all_day, rrule, rdate, unknown_properties FROM tasks " + + "WHERE unknown_properties LIKE '%VALARM%'", + ).use { c -> + while (c.moveToNext()) { + val task = AlarmLift.Task( + dtstart = if (c.isNull(1)) null else Instant.fromEpochMilliseconds(c.getLong(1)), + due = if (c.isNull(2)) null else Instant.fromEpochMilliseconds(c.getLong(2)), + isAllDay = c.getInt(3) != 0, + recurring = !c.isNull(4) || !c.isNull(5), + ) + val result = AlarmLift.lift(c.getString(6), task) + if (result.alarms.isNotEmpty()) lifted += Lifted(c.getLong(0), result) + } + } + lifted.forEach { (taskId, result) -> + val local = mutableListOf>() + db.query( + "SELECT id, minutes_before, reference FROM task_alarms WHERE task_id = ?", + arrayOf(taskId), + ).use { c -> while (c.moveToNext()) local += Triple(c.getLong(0), c.getInt(1), c.getString(2)) } + result.alarms.forEach { alarm -> + local.firstOrNull { it.second == alarm.minutesBefore && it.third == alarm.reference.name } + ?.let { duplicate -> + local.remove(duplicate) + db.execSQL("DELETE FROM task_alarms WHERE id = ?", arrayOf(duplicate.first)) + } + db.execSQL( + "INSERT INTO task_alarms (task_id, minutes_before, reference, uid, raw) VALUES (?, ?, ?, ?, ?)", + arrayOf( + taskId, + alarm.minutesBefore, + alarm.reference.name, + alarm.uid ?: UUID.randomUUID().toString(), + alarm.raw, + ), + ) + } + db.execSQL("UPDATE tasks SET unknown_properties = ? WHERE id = ?", arrayOf(result.residue, taskId)) + } + + // ⚠️ `raw IS NULL` is "never on the server": everything lifted + // above carries its original. Status 0/1 are NEEDS-ACTION and + // IN-PROCESS (`ICalStatus`). + // + // ⚠️ Never a list the upload cannot write to. A dirty row is + // also one the download will not overwrite, so marking a task in + // a read-only share — or a confidential one, which the upload + // skips in a shared collection — would freeze it for good: never + // sent, and the owner's edits never taken. Classification 2 is + // CONFIDENTIAL. + // + // LAST-MODIFIED moves with it: the body changes, and a client + // reconciling by that stamp would otherwise keep its stale copy. + val now = System.currentTimeMillis() + db.execSQL( + """ + UPDATE tasks SET is_dirty = 1, last_modified = ? + WHERE is_deleted = 0 + AND status IN (0, 1) + AND (classification IS NULL OR classification != 2) + AND list_id IN (SELECT id FROM task_lists WHERE account_id IS NOT NULL AND is_read_only = 0) + AND (rrule IS NOT NULL OR rdate IS NOT NULL OR due > ?) + AND id IN (SELECT task_id FROM task_alarms WHERE raw IS NULL) + """.trimIndent(), + arrayOf(now, now), + ) + } + } } } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/transfer/ExternalImport.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/transfer/ExternalImport.kt index 1585bb0..e50e558 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/transfer/ExternalImport.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/transfer/ExternalImport.kt @@ -9,8 +9,7 @@ import de.jeanlucmakiola.agendula.data.di.IoDispatcher import de.jeanlucmakiola.agendula.data.tasks.ProviderResolver import de.jeanlucmakiola.agendula.data.tasks.TaskReminder import de.jeanlucmakiola.agendula.data.tasks.TasksDataSource -import de.jeanlucmakiola.agendula.data.tasks.room.AlarmReference -import de.jeanlucmakiola.agendula.data.tasks.room.TaskAlarmEntity +import de.jeanlucmakiola.agendula.data.tasks.room.toAlarmEntity import de.jeanlucmakiola.agendula.data.tasks.room.TaskEntity import de.jeanlucmakiola.agendula.data.tasks.room.TaskListEntity import de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase @@ -200,7 +199,7 @@ class ExternalImport @Inject constructor( var alarmCount = 0 for ((sourceTaskId, reminders) in snapshot.alarms) { val taskId = taskIds[sourceTaskId] ?: continue - reminders.forEach { alarmDao.insert(it.toEntity(taskId)) } + reminders.forEach { alarmDao.insert(it.toAlarmEntity(taskId)) } alarmCount += reminders.size } @@ -228,12 +227,6 @@ class ExternalImport @Inject constructor( lastModified = lastModified, ) - private fun TaskReminder.toEntity(taskId: Long) = TaskAlarmEntity( - taskId = taskId, - minutesBefore = minutesBefore, - reference = if (fromStart) AlarmReference.START else AlarmReference.DUE, - ) - private fun verify(written: TransferCounts, before: TransferCounts) { val after = tableCounts() check(after.lists - before.lists == written.lists) { diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/transfer/IcsImport.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/transfer/IcsImport.kt index 41c4929..18bbc93 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/transfer/IcsImport.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/transfer/IcsImport.kt @@ -7,7 +7,9 @@ import dagger.hilt.android.qualifiers.ApplicationContext import de.jeanlucmakiola.agendula.data.di.IoDispatcher import de.jeanlucmakiola.agendula.data.tasks.ical.CalendarResource import de.jeanlucmakiola.agendula.data.tasks.ical.VTodoMapper +import de.jeanlucmakiola.agendula.data.tasks.room.TaskAlarmEntity import de.jeanlucmakiola.agendula.data.tasks.room.TaskEntity +import de.jeanlucmakiola.agendula.data.tasks.room.freshCopy import de.jeanlucmakiola.agendula.data.tasks.room.TaskListEntity import de.jeanlucmakiola.agendula.data.tasks.room.TasksDatabase import kotlinx.coroutines.CoroutineDispatcher @@ -65,7 +67,11 @@ interface IcsImportStore { fun transaction(block: () -> T): T fun createList(name: String, color: Int): Long fun hasUid(listId: Long, uid: String): Boolean + + /** Whether any list holds [uid] — the same file imported somewhere else before. */ + fun hasUidAnywhere(uid: String): Boolean fun insert(row: TaskEntity): Long + fun insertAlarms(taskId: Long, alarms: List) fun masterByUid(listId: Long, uid: String): TaskEntity? fun setParent(taskId: Long, parentId: Long?) } @@ -76,9 +82,9 @@ interface IcsImportStore { * * Goes through the same [CalendarResource] / [VTodoMapper] path CalDAV sync * uses, so recurrence, `RECURRENCE-ID` overrides, `RELATED-TO` parents and every - * property the mapper does not claim (`VALARM`s included) survive as they would - * on a download. `VALARM`s stay in the residue: like sync, the import does not - * turn them into local reminders. + * property the mapper does not claim survive as they would on a download. A + * `VALARM` the mapper claims becomes a reminder on the task, exactly as a + * download does; the rest stay in the residue. * * Tasks whose UID the target list already holds are skipped, so importing the * same file twice is harmless. @@ -200,8 +206,17 @@ class IcsImport @Inject constructor( skipped++ continue } + // ⚠️ The same file in a second list is a second task carrying + // the first one's alarm identities. Those are what clients + // track a dismissal by, so the copy's alarms are new ones. + val copied = store.hasUidAnywhere(task.uid) + fun alarmsOf(mapped: VTodoMapper.Mapped) = + if (copied) mapped.alarms.map { it.freshCopy(0) } else mapped.alarms val masterId = store.insert(task.master.entity.forImport(listId, masterId = null)) - task.overrides.forEach { store.insert(it.entity.forImport(listId, masterId)) } + store.insertAlarms(masterId, alarmsOf(task.master)) + task.overrides.forEach { + store.insertAlarms(store.insert(it.entity.forImport(listId, masterId)), alarmsOf(it)) + } task.master.parentUid?.let { parents += masterId to it } imported++ } @@ -270,8 +285,15 @@ class RoomIcsImportStore(private val database: TasksDatabase) : IcsImportStore { arrayOf(listId, uid), ).use { it.moveToFirst() } + override fun hasUidAnywhere(uid: String): Boolean = + database.query("SELECT 1 FROM tasks WHERE uid = ? LIMIT 1", arrayOf(uid)).use { it.moveToFirst() } + override fun insert(row: TaskEntity): Long = database.tasks().insert(row) + override fun insertAlarms(taskId: Long, alarms: List) { + alarms.forEach { database.alarms().insert(it.copy(id = 0, taskId = taskId)) } + } + override fun masterByUid(listId: Long, uid: String): TaskEntity? = database.tasks().byUid(listId, uid) override fun setParent(taskId: Long, parentId: Long?) { diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/domain/ical/ICalValues.kt b/app/src/main/java/de/jeanlucmakiola/agendula/domain/ical/ICalValues.kt index 32422f8..f5bc0e1 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/domain/ical/ICalValues.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/domain/ical/ICalValues.kt @@ -161,6 +161,57 @@ object ICalValues { } } + private val DURATION = Regex( + "([+-])?P(?:(\\d+)W|(?:(\\d+)D)?(?:T(?:(\\d+)H)?(?:(\\d+)M)?(?:(\\d+)S)?)?)", + RegexOption.IGNORE_CASE, + ) + + /** + * RFC 5545 §3.3.6 `dur-value` in signed seconds: `-PT15M` → -900, `P1W` → + * 604800, `-P1DT2H` → -93600. Null when it does not parse. + * + * A little more lenient than the grammar — `PT1H5S` skips the minutes the + * grammar would require — because a stricter reader would leave another + * client's alarm unread over a form nobody misreads. + */ + fun parseDuration(value: String): Long? { + val match = DURATION.matchEntire(value.trim()) ?: return null + val (sign, weeks, days, hours, minutes, seconds) = match.destructured + // "P" and "PT" alone match the pattern and mean nothing. + if (listOf(weeks, days, hours, minutes, seconds).all(String::isEmpty)) return null + if (value.trim().endsWith("T", ignoreCase = true)) return null + fun part(text: String, scale: Long): Long = + if (text.isEmpty()) 0L else Math.multiplyExact(text.toLong(), scale) + val total = runCatching { + listOf( + part(weeks, 7 * 86_400L), part(days, 86_400L), + part(hours, 3_600L), part(minutes, 60L), part(seconds, 1L), + ).reduce(Math::addExact) + }.getOrNull() ?: return null + return if (sign == "-") -total else total + } + + /** The inverse of [parseDuration], in days and time, never weeks: `-PT15M`, `-P1D`, `PT0S`. */ + fun formatDuration(seconds: Long): String { + if (seconds == 0L) return "PT0S" + val magnitude = kotlin.math.abs(seconds) + val days = magnitude / 86_400 + val hours = magnitude % 86_400 / 3_600 + val minutes = magnitude % 3_600 / 60 + val rest = magnitude % 60 + return buildString { + if (seconds < 0) append('-') + append('P') + if (days > 0) append(days).append('D') + if (hours > 0 || minutes > 0 || rest > 0) { + append('T') + if (hours > 0) append(hours).append('H') + if (minutes > 0) append(minutes).append('M') + if (rest > 0) append(rest).append('S') + } + } + } + private fun java.time.Instant.toKotlin(): Instant = Instant.fromEpochMilliseconds(toEpochMilli()) } diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/data/reminders/ReminderPlannerTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/data/reminders/ReminderPlannerTest.kt index 7e761ba..7ddf389 100644 --- a/app/src/test/java/de/jeanlucmakiola/agendula/data/reminders/ReminderPlannerTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/agendula/data/reminders/ReminderPlannerTest.kt @@ -60,6 +60,59 @@ class ReminderPlannerTest { assertThat(plan).containsExactly(ScheduledReminder(1, millis(date.atTime(7, 0), berlin))) } + @Test + fun `a reminder another client already dismissed is not fired again`() { + val due = Instant.parse("2026-07-20T10:00:00Z") + val task = testTask(id = 3, due = due) + val trigger = due.toEpochMilliseconds() - 15 * 60_000L + // Inside the missed-reminder grace, which is where a dismissal elsewhere matters. + val now = trigger + 60_000L + fun plan(acknowledged: Instant?) = ReminderPlanner.plan( + tasks = listOf(task), + perTask = mapOf(3L to listOf(TaskReminder(15, acknowledgedAt = acknowledged))), + settings = Settings(), + now = now, + zone = berlin, + ) + + assertThat(plan(acknowledged = Instant.fromEpochMilliseconds(trigger + 1))).isEmpty() + // Dismissed before this trigger came due: an earlier one, still to fire. + assertThat(plan(acknowledged = Instant.fromEpochMilliseconds(trigger - 1))) + .containsExactly(ScheduledReminder(3, trigger)) + assertThat(plan(acknowledged = null)).containsExactly(ScheduledReminder(3, trigger)) + } + + @Test + fun `a task whose alarms were turned off elsewhere has none, not the default`() { + val due = Instant.parse("2026-07-20T10:00:00Z") + val now = due.toEpochMilliseconds() - 60 * 60_000L + fun plan(perTask: Map>) = ReminderPlanner.plan( + tasks = listOf(testTask(id = 4, due = due)), + perTask = perTask, + settings = Settings(defaultReminderMinutes = listOf(15)), + now = now, + zone = berlin, + ) + + assertThat(plan(mapOf(4L to emptyList()))).isEmpty() + assertThat(plan(emptyMap())).hasSize(1) + } + + @Test + fun `an all-day reminder dismissed elsewhere before this device's hour stays dismissed`() { + val task = testTask(id = 5, due = allDayInstantOf(date)).copy(isAllDay = true) + // Rung at midnight on the Mac, dismissed at 07:00; this device rings at 09:00. + val dismissed = date.atTime(7, 0).atZone(berlin).toInstant() + val plan = ReminderPlanner.plan( + tasks = listOf(task), + perTask = mapOf(5L to listOf(TaskReminder(0, acknowledgedAt = Instant.fromEpochMilliseconds(dismissed.toEpochMilli())))), + settings = Settings(allDayReminderMinuteOfDay = 9 * 60), + now = millis(date.atTime(8, 0), berlin), + zone = berlin, + ) + assertThat(plan).isEmpty() + } + @Test fun `an all-day task takes the all-day defaults, not the timed ones`() { val task = testTask(id = 6, due = allDayInstantOf(date)).copy(isAllDay = true, listId = 7) diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/AlarmCodecTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/AlarmCodecTest.kt new file mode 100644 index 0000000..b324d89 --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/AlarmCodecTest.kt @@ -0,0 +1,267 @@ +package de.jeanlucmakiola.agendula.data.tasks.ical + +import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.agendula.data.tasks.room.AlarmReference +import de.jeanlucmakiola.agendula.data.tasks.room.TaskAlarmEntity +import de.jeanlucmakiola.agendula.data.tasks.room.TaskEntity +import de.jeanlucmakiola.agendula.domain.allDayInstantOf +import de.jeanlucmakiola.agendula.domain.ical.ICalComponent +import de.jeanlucmakiola.agendula.domain.ical.ICalParser +import de.jeanlucmakiola.agendula.domain.ical.ICalSerializer +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import java.time.LocalDate +import kotlin.time.Duration.Companion.hours +import kotlin.time.Instant + +class AlarmCodecTest { + + private val due = Instant.parse("2026-10-01T15:00:00Z") + private val start = Instant.parse("2026-10-01T09:00:00Z") + private val timed = TaskEntity(listId = 1, uid = "t", due = due) + private val withStart = timed.copy(dtstart = start) + private val allDay = TaskEntity( + listId = 1, + uid = "t", + due = allDayInstantOf(LocalDate.of(2026, 10, 1)), + isAllDay = true, + ) + + private fun alarm(vararg lines: String): ICalComponent = + ICalParser.parse((listOf("BEGIN:VALARM") + lines + "END:VALARM").joinToString("\r\n")) + + private fun lead(minutes: Int, reference: AlarmReference = AlarmReference.DUE) = AlarmCodec.Lead(minutes, reference) + + @Nested + inner class Claiming { + + @Test + fun `a display alarm before due is claimed`() { + assertThat(AlarmCodec.resolve(alarm("ACTION:DISPLAY", "TRIGGER;RELATED=END:-PT15M"), timed)) + .isEqualTo(lead(15)) + } + + @Test + fun `an audio alarm is claimed too, an email one is not`() { + assertThat(AlarmCodec.resolve(alarm("ACTION:AUDIO", "TRIGGER;RELATED=END:-PT5M"), timed)).isEqualTo(lead(5)) + assertThat(AlarmCodec.resolve(alarm("ACTION:EMAIL", "TRIGGER;RELATED=END:-PT5M"), timed)).isNull() + assertThat(AlarmCodec.resolve(alarm("TRIGGER;RELATED=END:-PT5M"), timed)).isNull() + } + + @Test + fun `alarms that are somebody else's to fire stay in the residue`() { + val unclaimable = listOf( + alarm("ACTION:DISPLAY", "X-APPLE-DEFAULT-ALARM:TRUE", "TRIGGER;RELATED=END:-PT15M"), + alarm("ACTION:DISPLAY", "X-APPLE-PROXIMITY:ARRIVE", "TRIGGER;VALUE=DATE-TIME:19760401T005545Z"), + alarm("ACTION:DISPLAY", "PROXIMITY:DEPART", "TRIGGER;RELATED=END:-PT15M"), + alarm("ACTION:DISPLAY", "REPEAT:2", "DURATION:PT5M", "TRIGGER;RELATED=END:-PT15M"), + alarm("ACTION:DISPLAY", "RELATED-TO;RELTYPE=SNOOZE:other", "TRIGGER;RELATED=END:-PT15M"), + alarm("ACTION:DISPLAY", "TRIGGER;VALUE=DATE-TIME:19760401T005545Z"), + ) + unclaimable.forEach { assertThat(AlarmCodec.resolve(it, timed)).isNull() } + } + + @Test + fun `a trigger after its anchor is not claimed, because the planner would fire it early`() { + assertThat(AlarmCodec.resolve(alarm("ACTION:DISPLAY", "TRIGGER;RELATED=END:PT10M"), timed)).isNull() + } + + @Test + fun `a lead in seconds is not one the planner can reproduce`() { + assertThat(AlarmCodec.resolve(alarm("ACTION:DISPLAY", "TRIGGER;RELATED=END:-PT90S"), timed)).isNull() + } + + @Test + fun `RELATED=END without a DUE has nothing to count from`() { + val noDue = timed.copy(due = null, dtstart = start) + assertThat(AlarmCodec.resolve(alarm("ACTION:DISPLAY", "TRIGGER;RELATED=END:-PT15M"), noDue)).isNull() + } + + @Test + fun `an absolute UTC trigger becomes a lead before due`() { + assertThat(AlarmCodec.resolve(alarm("ACTION:DISPLAY", "TRIGGER;VALUE=DATE-TIME:20261001T143000Z"), timed)) + .isEqualTo(lead(30)) + } + + @Test + fun `an absolute trigger on a recurring task has no single lead`() { + val recurring = timed.copy(rrule = "FREQ=DAILY") + assertThat(AlarmCodec.resolve(alarm("ACTION:DISPLAY", "TRIGGER;VALUE=DATE-TIME:20261001T143000Z"), recurring)) + .isNull() + } + + @Test + fun `the identity is taken from X-WR-ALARMUID, then UID`() { + val apple = AlarmCodec.fromVAlarm(alarm("ACTION:DISPLAY", "X-WR-ALARMUID:apple", "UID:rfc", "TRIGGER:-PT5M"), timed) + val rfc = AlarmCodec.fromVAlarm(alarm("ACTION:DISPLAY", "UID:rfc", "TRIGGER:-PT5M"), timed) + assertThat(apple?.uid).isEqualTo("apple") + assertThat(rfc?.uid).isEqualTo("rfc") + } + } + + @Nested + inner class Anchors { + + @Test + fun `RELATED=START with a distinct DTSTART is a before-start reminder`() { + assertThat(AlarmCodec.resolve(alarm("ACTION:DISPLAY", "TRIGGER;RELATED=START:-PT10M"), withStart)) + .isEqualTo(lead(10, AlarmReference.START)) + } + + @Test + fun `RELATED is START by default`() { + assertThat(AlarmCodec.resolve(alarm("ACTION:DISPLAY", "TRIGGER:-PT10M"), withStart)) + .isEqualTo(lead(10, AlarmReference.START)) + } + + @Test + fun `START on a task whose start is its due is a before-due reminder`() { + val same = timed.copy(dtstart = due) + assertThat(AlarmCodec.resolve(alarm("ACTION:DISPLAY", "TRIGGER:-PT10M"), same)).isEqualTo(lead(10)) + } + + @Test + fun `START on a task with only a DUE is Apple's shape, and reads as before-due`() { + assertThat(AlarmCodec.resolve(alarm("ACTION:DISPLAY", "TRIGGER;RELATED=START:-PT30M"), timed)) + .isEqualTo(lead(30)) + } + } + + @Nested + inner class AllDay { + + @Test + fun `a relative trigger rounds to whole days`() { + assertThat(AlarmCodec.resolve(alarm("ACTION:DISPLAY", "TRIGGER;RELATED=END:-P1D"), allDay)) + .isEqualTo(lead(24 * 60)) + assertThat(AlarmCodec.resolve(alarm("ACTION:DISPLAY", "TRIGGER;RELATED=END:-PT15H"), allDay)) + .isEqualTo(lead(24 * 60)) + } + + @Test + fun `a time on the day itself is "on the day", not an after-due alarm`() { + assertThat(AlarmCodec.resolve(alarm("ACTION:DISPLAY", "TRIGGER;RELATED=END:PT9H"), allDay)) + .isEqualTo(lead(0)) + } + + @Test + fun `an absolute trigger counts back from the due date`() { + // 09:00 Berlin the day before, as Apple writes it. + assertThat(AlarmCodec.resolve(alarm("ACTION:DISPLAY", "TRIGGER;VALUE=DATE-TIME:20260930T070000Z"), allDay)) + .isEqualTo(lead(24 * 60)) + } + + @Test + fun `an authored all-day alarm is written in days`() { + val row = TaskAlarmEntity(taskId = 1, minutesBefore = 2 * 24 * 60, uid = "a") + assertThat(AlarmCodec.toVAlarm(row, allDay, due)?.property("TRIGGER")?.value).isEqualTo("-P2D") + val onTheDay = row.copy(minutesBefore = 0) + assertThat(AlarmCodec.toVAlarm(onTheDay, allDay, due)?.property("TRIGGER")?.value).isEqualTo("PT0S") + } + } + + @Nested + inner class Writing { + + private val earlier = due - 48.hours + + @Test + fun `an authored alarm names itself both ways and counts back from due`() { + val written = AlarmCodec.toVAlarm(TaskAlarmEntity(taskId = 1, minutesBefore = 15, uid = "a-1"), timed, earlier)!! + assertThat(ICalSerializer.serialize(written)).isEqualTo( + "BEGIN:VALARM\r\n" + + "UID:a-1\r\n" + + "X-WR-ALARMUID:a-1\r\n" + + "ACTION:DISPLAY\r\n" + + "DESCRIPTION:Reminder\r\n" + + "TRIGGER;RELATED=END:-PT15M\r\n" + + "END:VALARM\r\n", + ) + } + + @Test + fun `a before-start row is written against START`() { + val row = TaskAlarmEntity(taskId = 1, minutesBefore = 10, reference = AlarmReference.START, uid = "a") + val trigger = AlarmCodec.toVAlarm(row, withStart, earlier)?.property("TRIGGER") + assertThat(trigger?.param("RELATED")).isEqualTo("START") + assertThat(trigger?.value).isEqualTo("-PT10M") + } + + @Test + fun `a row whose anchor is gone is not written, so the resource stays valid`() { + val row = TaskAlarmEntity(taskId = 1, minutesBefore = 15, uid = "a") + assertThat(AlarmCodec.toVAlarm(row, timed.copy(due = null), earlier)).isNull() + } + + @Test + fun `an alarm long past is written acknowledged`() { + val row = TaskAlarmEntity(taskId = 1, minutesBefore = 15, uid = "a") + val muchLater = due + 7.hours + assertThat(AlarmCodec.toVAlarm(row, timed, muchLater)?.property("ACKNOWLEDGED")?.value) + .isEqualTo("20261001T220000Z") + assertThat(AlarmCodec.toVAlarm(row, timed, earlier)?.property("ACKNOWLEDGED")).isNull() + } + + @Test + fun `one still inside the missed-reminder grace is not, or it would silence the late reminder here`() { + val row = TaskAlarmEntity(taskId = 1, minutesBefore = 15, uid = "a") + assertThat(AlarmCodec.toVAlarm(row, timed, due + 1.hours)?.property("ACKNOWLEDGED")).isNull() + } + + @Test + fun `a before-start row on a task that lost its start goes out before due`() { + val row = TaskAlarmEntity(taskId = 1, minutesBefore = 10, reference = AlarmReference.START, uid = "a") + val trigger = AlarmCodec.toVAlarm(row, timed, earlier)?.property("TRIGGER") + assertThat(trigger?.param("RELATED")).isEqualTo("END") + assertThat(trigger?.value).isEqualTo("-PT10M") + } + + @Test + fun `the wire lead is what the row reads back as`() { + val fifteen = TaskAlarmEntity(taskId = 1, minutesBefore = 15, uid = "a") + assertThat(AlarmCodec.wireLead(fifteen, allDay)).isEqualTo(lead(0)) + assertThat(AlarmCodec.wireLead(fifteen, timed)).isEqualTo(lead(15)) + } + } + + @Nested + inner class Unchanged { + + private fun claimed(text: String, task: TaskEntity) = + checkNotNull(AlarmCodec.fromVAlarm(alarm(*text.split("|").toTypedArray()), task)) + + @Test + fun `an unchanged row is written back verbatim, whatever spelling it arrived in`() { + val row = claimed("ACTION:DISPLAY|TRIGGER;RELATED=END:-P0DT0H15M0S|X-VENDOR:keep", timed) + assertThat(row.minutesBefore).isEqualTo(15) + val written = AlarmCodec.toVAlarm(row, timed, due)!! + assertThat(written.property("TRIGGER")?.value).isEqualTo("-P0DT0H15M0S") + assertThat(written.property("X-VENDOR")?.value).isEqualTo("keep") + } + + @Test + fun `a relative original survives a due-date edit`() { + val row = claimed("ACTION:DISPLAY|TRIGGER;RELATED=END:-PT15M|X-VENDOR:keep", timed) + val moved = timed.copy(due = due + 24.hours) + assertThat(AlarmCodec.toVAlarm(row, moved, due)?.property("X-VENDOR")).isNotNull() + } + + @Test + fun `an absolute original stops matching once due moves, and the row is authored fresh`() { + val row = claimed("ACTION:DISPLAY|TRIGGER;VALUE=DATE-TIME:20261001T143000Z|X-WR-ALARMUID:abs", timed) + val moved = timed.copy(due = due + 24.hours) + val written = AlarmCodec.toVAlarm(row, moved, due)!! + assertThat(written.property("TRIGGER")?.value).isEqualTo("-PT30M") + // Under its old identity, so other clients see it change, not appear. + assertThat(written.property("X-WR-ALARMUID")?.value).isEqualTo("abs") + } + + @Test + fun `an edited lead is authored fresh`() { + val row = claimed("ACTION:DISPLAY|TRIGGER;RELATED=END:-PT15M|X-VENDOR:keep", timed).copy(minutesBefore = 60) + val written = AlarmCodec.toVAlarm(row, timed, due)!! + assertThat(written.property("TRIGGER")?.value).isEqualTo("-PT1H") + assertThat(written.property("X-VENDOR")).isNull() + } + } +} diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapperTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapperTest.kt index b2e1c5e..03461b4 100644 --- a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapperTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoMapperTest.kt @@ -1,6 +1,8 @@ package de.jeanlucmakiola.agendula.data.tasks.ical import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.agendula.data.tasks.room.AlarmReference +import de.jeanlucmakiola.agendula.data.tasks.room.TaskAlarmEntity import de.jeanlucmakiola.agendula.data.tasks.room.TaskEntity import de.jeanlucmakiola.agendula.domain.PRIORITY_NONE import de.jeanlucmakiola.agendula.domain.TaskStatus @@ -8,6 +10,7 @@ import de.jeanlucmakiola.agendula.domain.ical.ICalComponent import de.jeanlucmakiola.agendula.domain.ical.ICalParser import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test +import kotlin.time.Duration.Companion.days import kotlin.time.Instant class VTodoMapperTest { @@ -442,4 +445,64 @@ class VTodoMapperTest { timezone = timezone, unknownProperties = unknownProperties, ) + + @Nested + inner class Alarms { + + private val due = Instant.parse("2026-10-01T15:00:00Z") + private val task = TaskEntity(listId = 1, uid = "t-1", title = "Pay rent", due = due) + + @Test + fun `claimed alarms leave the residue and unclaimed ones stay`() { + val mapped = VTodoMapper.read( + vtodo( + "DUE:20261001T150000Z", + "BEGIN:VALARM", "ACTION:DISPLAY", "TRIGGER;RELATED=END:-PT15M", "END:VALARM", + "BEGIN:VALARM", "ACTION:EMAIL", "TRIGGER;RELATED=END:-PT1H", "END:VALARM", + ), + ) + assertThat(mapped.alarms.map { it.minutesBefore }).containsExactly(15) + assertThat(mapped.entity.unknownProperties).contains("ACTION:EMAIL") + assertThat(mapped.entity.unknownProperties).doesNotContain("ACTION:DISPLAY") + } + + @Test + fun `the task's reminders are written ahead of the residue's alarms`() { + val withResidue = task.copy( + unknownProperties = "BEGIN:VALARM\r\nACTION:EMAIL\r\nTRIGGER;RELATED=END:-PT1H\r\nEND:VALARM\r\n", + ) + val written = VTodoMapper.write( + withResidue, + now = due - 1.days, + alarms = listOf(TaskAlarmEntity(taskId = 1, minutesBefore = 15, uid = "a-1")), + ) + val alarms = written.components("VALARM") + assertThat(alarms.map { it.property("ACTION")?.value }).containsExactly("DISPLAY", "EMAIL").inOrder() + assertThat(alarms.first().property("TRIGGER")?.value).isEqualTo("-PT15M") + } + + @Test + fun `a reminder left behind by a cleared due date does not make the task unuploadable`() { + val cleared = task.copy(due = null) + val written = VTodoMapper.write( + cleared, + alarms = listOf(TaskAlarmEntity(taskId = 1, minutesBefore = 15, uid = "a-1")), + ) + assertThat(written.components("VALARM")).isEmpty() + assertThat(VTodoMapper.validate(written)).isEmpty() + } + + @Test + fun `a reminder survives write and read with its identity`() { + val written = VTodoMapper.write( + task, + now = due - 1.days, + alarms = listOf(TaskAlarmEntity(taskId = 1, minutesBefore = 30, uid = "a-1")), + ) + val back = VTodoMapper.read(written).alarms.single() + assertThat(back.minutesBefore).isEqualTo(30) + assertThat(back.reference).isEqualTo(AlarmReference.DUE) + assertThat(back.uid).isEqualTo("a-1") + } + } } diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoRoundTripTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoRoundTripTest.kt index 55e7c6b..eb17eda 100644 --- a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoRoundTripTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/ical/VTodoRoundTripTest.kt @@ -1,6 +1,7 @@ package de.jeanlucmakiola.agendula.data.tasks.ical import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.agendula.data.tasks.room.AlarmReference import de.jeanlucmakiola.agendula.domain.ical.ICalComponent import de.jeanlucmakiola.agendula.domain.ical.ICalParser import de.jeanlucmakiola.agendula.domain.ical.ICalSerializer @@ -23,6 +24,7 @@ class VTodoRoundTripTest { "nextcloud-shared-confidential", "recurring-master-overrides", "valarm-unknown-nested", + "valarm-claimed-apple", "unknown-component", "unknown-tzid", "uid-special-chars", @@ -70,6 +72,29 @@ class VTodoRoundTripTest { assertThat(alarm.property("ACKNOWLEDGED")?.value).isEqualTo("20260901T113000Z") } + @Test + fun `a claimed alarm becomes a reminder and comes back exactly as it arrived`() { + val original = vtodosOf("valarm-claimed-apple").single() + val mapped = VTodoMapper.read(original) + + val alarm = mapped.alarms.single() + // RELATED=START with only a DUE: Apple's shape, read as before-due. + assertThat(alarm.minutesBefore).isEqualTo(30) + assertThat(alarm.reference).isEqualTo(AlarmReference.DUE) + assertThat(alarm.uid).isEqualTo("8E6C4A1E-0000-4C1B-9B1A-3F5F4C2D9A11") + assertThat(mapped.entity.unknownProperties.orEmpty()).doesNotContain("VALARM") + + val written = roundTrip(original).components("VALARM").single() + assertThat(written).isEqualTo(original.components("VALARM").single()) + } + + @Test + fun `an Apple default alarm is left in the residue`() { + val mapped = VTodoMapper.read(vtodosOf("valarm-unknown-nested").single()) + assertThat(mapped.alarms).isEmpty() + assertThat(mapped.entity.unknownProperties).contains("X-APPLE-DEFAULT-ALARM:TRUE") + } + @Test fun `an entirely unknown sub-component survives`() { val roundTripped = roundTrip(vtodosOf("unknown-component").single()) @@ -145,6 +170,7 @@ class VTodoRoundTripTest { entity = mapped.entity, parentUid = mapped.parentUid, now = Instant.fromEpochMilliseconds(0), + alarms = mapped.alarms, ) // Serialise and re-parse rather than comparing the tree directly: folding // and parameter quoting are where a serialiser loses data, and skipping diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/transfer/IcsImportTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/transfer/IcsImportTest.kt index 70819b4..cc0d78e 100644 --- a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/transfer/IcsImportTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/transfer/IcsImportTest.kt @@ -1,6 +1,8 @@ package de.jeanlucmakiola.agendula.data.tasks.transfer import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.agendula.data.tasks.room.AlarmReference +import de.jeanlucmakiola.agendula.data.tasks.room.TaskAlarmEntity import de.jeanlucmakiola.agendula.data.tasks.room.TaskEntity import org.junit.jupiter.api.Test import java.io.ByteArrayOutputStream @@ -24,12 +26,20 @@ class IcsImportTest { override fun hasUid(listId: Long, uid: String) = rows.any { it.listId == listId && it.uid == uid } + override fun hasUidAnywhere(uid: String) = rows.any { it.uid == uid } + override fun insert(row: TaskEntity): Long { val id = nextId++ rows += row.copy(id = id) return id } + val alarms = mutableListOf() + + override fun insertAlarms(taskId: Long, alarms: List) { + this.alarms += alarms.map { it.copy(taskId = taskId) } + } + override fun masterByUid(listId: Long, uid: String) = rows.firstOrNull { it.listId == listId && it.uid == uid && it.recurrenceId == null } @@ -87,12 +97,55 @@ class IcsImportTest { val child = rows.single { it.recurrenceId != null } assertThat(master.rrule).isEqualTo("FREQ=WEEKLY;BYDAY=MO,TH") assertThat(master.unknownProperties).contains("X-APPLE-SORT-ORDER:7") - assertThat(master.unknownProperties).contains("BEGIN:VALARM") + // The alarm is a reminder now, carrying the original for the upload. + assertThat(master.unknownProperties).doesNotContain("BEGIN:VALARM") + val alarm = store.alarms.single() + assertThat(alarm.taskId).isEqualTo(master.id) + assertThat(alarm.minutesBefore).isEqualTo(15) + assertThat(alarm.reference).isEqualTo(AlarmReference.START) + assertThat(alarm.raw).contains("DESCRIPTION:Water") assertThat(child.masterId).isEqualTo(master.id) assertThat(child.title).isEqualTo("Water plants (balcony)") assertThat(rows.all { it.listId == 1L && it.isDirty && it.href == null }).isTrue() } + @Test + fun `an override keeps its own alarms and an Apple default alarm stays in the residue`() { + val withAlarm = vtodo( + "series", "Water plants (balcony)", + "RECURRENCE-ID:20260604T080000Z", + "DTSTART:20260604T100000Z", + "DUE:20260604T110000Z", + "BEGIN:VALARM\r\nACTION:DISPLAY\r\nTRIGGER;RELATED=END:-PT1H\r\nEND:VALARM", + "BEGIN:VALARM\r\nACTION:DISPLAY\r\nX-APPLE-DEFAULT-ALARM:TRUE\r\nTRIGGER;RELATED=END:-PT5M\r\nEND:VALARM", + ) + val parsed = IcsImport.parse(listOf(IcsSource("plants.ics", calendar(series, withAlarm)))) + val store = FakeImportStore() + + IcsImport.write(parsed, ImportTarget.Existing(1L), store) + + val child = store.byUid("series").single { it.recurrenceId != null } + val own = store.alarms.filter { it.taskId == child.id } + assertThat(own.map { it.minutesBefore to it.reference }).containsExactly(60 to AlarmReference.DUE) + assertThat(child.unknownProperties).contains("X-APPLE-DEFAULT-ALARM:TRUE") + } + + @Test + fun `the same file in a second list gives the copy alarm identities of its own`() { + val parsed = IcsImport.parse(listOf(IcsSource("plants.ics", calendar(series)))) + val store = FakeImportStore() + + IcsImport.write(parsed, ImportTarget.Existing(1L), store) + IcsImport.write(parsed, ImportTarget.Existing(2L), store) + + val (first, second) = store.alarms + assertThat(first.raw).isNotNull() + assertThat(second.uid).isNotEqualTo(first.uid) + // Fresh, so an untouched copy is not re-sent as the original text either. + assertThat(second.raw).isNull() + assertThat(second.minutesBefore).isEqualTo(first.minutesBefore) + } + @Test fun `a parent later in the file is still linked`() { val child = vtodo("child", "Buy soil", "RELATED-TO;RELTYPE=PARENT:parent") diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/domain/ical/ICalDurationTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/domain/ical/ICalDurationTest.kt new file mode 100644 index 0000000..ce50d3e --- /dev/null +++ b/app/src/test/java/de/jeanlucmakiola/agendula/domain/ical/ICalDurationTest.kt @@ -0,0 +1,36 @@ +package de.jeanlucmakiola.agendula.domain.ical + +import com.google.common.truth.Truth.assertThat +import org.junit.jupiter.api.Test + +/** RFC 5545 §3.3.6 `dur-value`, as alarm triggers use it. */ +class ICalDurationTest { + + @Test + fun `the forms other clients write`() { + assertThat(ICalValues.parseDuration("-PT15M")).isEqualTo(-900L) + assertThat(ICalValues.parseDuration("+PT5M")).isEqualTo(300L) + assertThat(ICalValues.parseDuration("PT0S")).isEqualTo(0L) + assertThat(ICalValues.parseDuration("P1W")).isEqualTo(7 * 86_400L) + assertThat(ICalValues.parseDuration("-P1DT2H")).isEqualTo(-(86_400L + 7_200L)) + assertThat(ICalValues.parseDuration("-P0DT0H15M0S")).isEqualTo(-900L) + assertThat(ICalValues.parseDuration("-PT90S")).isEqualTo(-90L) + } + + @Test + fun `what is not a duration is null`() { + listOf("", "P", "PT", "-P1DT", "15M", "PT15", "P1H", "20261001T090000Z", "P99999999999999999999D") + .forEach { assertThat(ICalValues.parseDuration(it)).isNull() } + } + + @Test + fun `written in days and time, and read back the same`() { + assertThat(ICalValues.formatDuration(-900)).isEqualTo("-PT15M") + assertThat(ICalValues.formatDuration(0)).isEqualTo("PT0S") + assertThat(ICalValues.formatDuration(-86_400)).isEqualTo("-P1D") + assertThat(ICalValues.formatDuration(-(86_400L + 5_400L))).isEqualTo("-P1DT1H30M") + listOf(-900L, 0L, -86_400L, -93_784L, 3_600L).forEach { + assertThat(ICalValues.parseDuration(ICalValues.formatDuration(it))).isEqualTo(it) + } + } +} diff --git a/app/src/test/resources/vtodo/README.md b/app/src/test/resources/vtodo/README.md index cf4ee53..dd603a6 100644 --- a/app/src/test/resources/vtodo/README.md +++ b/app/src/test/resources/vtodo/README.md @@ -13,7 +13,8 @@ VTIMEZONE bodies, fold positions, parameter quoting). |---|---| | `nextcloud-shared-confidential.ics` | A `CLASS:CONFIDENTIAL` task as Nextcloud rewrites it on GET from a **shared** calendar — `DUE`, `STATUS`, `COMPLETED`, `PERCENT-COMPLETE`, `PRIORITY` and `RELATED-TO` already stripped, ETag left untouched. Re-PUTting this destroys the owner's task | | `recurring-master-overrides.ics` | Master plus two `RECURRENCE-ID` overrides in one resource, sharing a UID | -| `valarm-unknown-nested.ics` | Unknown properties **inside** a `VALARM` — the case a flat property-per-row model cannot represent | +| `valarm-unknown-nested.ics` | Unknown properties **inside** a `VALARM` — the case a flat property-per-row model cannot represent. An Apple *default* alarm, so it stays in the residue | +| `valarm-claimed-apple.ics` | The same Apple alarm without `X-APPLE-DEFAULT-ALARM`, so it is **claimed** as a reminder: `RELATED=START` on a task with only `DUE` (Apple's non-conformant shape) reads as a before-due lead, and the original must come back verbatim through the row's `raw` | | `unknown-component.ics` | An entirely unknown sub-component | | `unknown-tzid.ics` | A `TZID` no device tzdb knows, with its own `VTIMEZONE` | | `uid-special-chars.ics` | A UID containing `/` and `@` — the href sanitiser must not assume it is filename-safe | diff --git a/app/src/test/resources/vtodo/valarm-claimed-apple.ics b/app/src/test/resources/vtodo/valarm-claimed-apple.ics new file mode 100644 index 0000000..9c658fd --- /dev/null +++ b/app/src/test/resources/vtodo/valarm-claimed-apple.ics @@ -0,0 +1,17 @@ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Apple Inc.//iOS 18.2//EN +BEGIN:VTODO +UID:alarm-claimed-001 +DTSTAMP:20260901T120000Z +SUMMARY:Renew passport +DUE;TZID=Europe/Berlin:20261001T170000 +BEGIN:VALARM +ACTION:DISPLAY +DESCRIPTION:Reminder +TRIGGER;RELATED=START:-PT30M +ACKNOWLEDGED:20260901T113000Z +X-WR-ALARMUID:8E6C4A1E-0000-4C1B-9B1A-3F5F4C2D9A11 +END:VALARM +END:VTODO +END:VCALENDAR diff --git a/app/src/testFull/java/de/jeanlucmakiola/agendula/data/sync/CollectionSyncerTest.kt b/app/src/testFull/java/de/jeanlucmakiola/agendula/data/sync/CollectionSyncerTest.kt index ee5a5b7..04ea3f6 100644 --- a/app/src/testFull/java/de/jeanlucmakiola/agendula/data/sync/CollectionSyncerTest.kt +++ b/app/src/testFull/java/de/jeanlucmakiola/agendula/data/sync/CollectionSyncerTest.kt @@ -1,6 +1,8 @@ package de.jeanlucmakiola.agendula.data.sync import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.agendula.data.tasks.room.AlarmReference +import de.jeanlucmakiola.agendula.data.tasks.room.TaskAlarmEntity import de.jeanlucmakiola.agendula.data.tasks.room.TaskEntity import de.jeanlucmakiola.agendula.data.tasks.room.TaskListEntity import de.jeanlucmakiola.agendula.data.tasks.room.TaskMove @@ -970,6 +972,137 @@ class CollectionSyncerTest { assertThat(report.discardedEdits.single().cause).isEqualTo(DiscardedEdit.Cause.DELETE_LOST) } + // ------------------------------------------------------------- reminders + + private val due = Instant.parse("2026-03-05T09:00:00Z") + private val alarm15 = "BEGIN:VALARM\r\nACTION:DISPLAY\r\nX-WR-ALARMUID:srv-1\r\n" + + "TRIGGER;RELATED=END:-PT15M\r\nEND:VALARM" + + private fun alarm(taskId: Long, minutes: Int, uid: String = "a-$minutes") = + TaskAlarmEntity(taskId = taskId, minutesBefore = minutes, uid = uid) + + @Test fun `an upload carries the task's reminders as VALARMs`() { + store.rows += task(id = 1, uid = "a", title = "Pay rent").copy(due = due, isDirty = true) + store.alarms += alarm(1, 15, uid = "a-1") + + sync() + + val body = remote.resources.values.single().body + assertThat(body).contains("BEGIN:VALARM") + assertThat(body).contains("TRIGGER;RELATED=END:-PT15M") + assertThat(body).contains("X-WR-ALARMUID:a-1") + } + + @Test fun `the alarm keeps its identity across uploads`() { + store.rows += task(id = 1, uid = "a", title = "Pay rent").copy(due = due, isDirty = true) + store.alarms += alarm(1, 15, uid = "a-1") + sync() + val first = remote.resources.values.single().body + store.rows[0] = store.rows[0].copy(title = "Pay the rent", isDirty = true) + + sync() + + val second = remote.resources.values.single().body + assertThat(second).isNotEqualTo(first) + assertThat(second).contains("X-WR-ALARMUID:a-1") + } + + @Test fun `a downloaded alarm becomes a reminder and leaves the residue`() { + remote.put("one.ics", vtodo("a", "Pay rent", extra = "DUE:20260305T090000Z\r\n$alarm15")) + + sync() + + val row = store.rows.single() + val reminder = store.alarms.single() + assertThat(reminder.taskId).isEqualTo(row.id) + assertThat(reminder.minutesBefore).isEqualTo(15) + assertThat(reminder.uid).isEqualTo("srv-1") + assertThat(row.unknownProperties.orEmpty()).doesNotContain("VALARM") + assertThat(row.isDirty).isFalse() + } + + @Test fun `a re-download keeps the reminder's row, and a removal on the server clears it`() { + remote.put("one.ics", vtodo("a", "Pay rent", extra = "DUE:20260305T090000Z\r\n$alarm15")) + sync() + val id = store.alarms.single().id + remote.put("one.ics", vtodo("a", "Pay the rent", extra = "DUE:20260305T090000Z\r\n$alarm15"), eTag = "e-2") + + sync() + + // Matched by identity, so the row is updated in place rather than churned. + assertThat(store.alarms.single().id).isEqualTo(id) + + remote.put("one.ics", vtodo("a", "Pay the rent", extra = "DUE:20260305T090000Z"), eTag = "e-3") + sync() + + assertThat(store.alarms).isEmpty() + // Turned off elsewhere: "none", so the device default does not take over. + assertThat(store.rows.single().remindersOff).isTrue() + } + + @Test fun `a task that never had alarms still takes the default`() { + remote.put("one.ics", vtodo("a", "Pay rent", extra = "DUE:20260305T090000Z")) + + sync() + + assertThat(store.rows.single().remindersOff).isFalse() + } + + @Test fun `an alarm coming back as we sent it keeps the lead the wire could not carry`() { + // A 15-minute lead on an all-day task goes out as "on the day". + store.rows += task(id = 1, uid = "a", title = "Pay rent") + .copy(due = Instant.parse("2026-03-05T00:00:00Z"), isAllDay = true, isDirty = true) + store.alarms += alarm(1, 15, uid = "a-1") + sync() + val sent = remote.resources.values.single() + assertThat(sent.body).contains("TRIGGER;RELATED=END:PT0S") + + // The server hands it back under a new ETag, unchanged. + sent.eTag = de.jeanlucmakiola.caldav.ETag("e-2", weak = false) + sync() + + assertThat(store.alarms.single().minutesBefore).isEqualTo(15) + } + + @Test fun `a before-start reminder on a task without a start survives the round trip`() { + store.rows += task(id = 1, uid = "a", title = "Pay rent").copy(due = due, isDirty = true) + store.alarms += alarm(1, 10, uid = "s-1").copy(reference = AlarmReference.START) + sync() + val sent = remote.resources.values.single() + // Sent from DUE, which is what the planner fires it from. + assertThat(sent.body).contains("TRIGGER;RELATED=END:-PT10M") + + sent.eTag = de.jeanlucmakiola.caldav.ETag("e-2", weak = false) + sync() + + assertThat(store.alarms.single().reference).isEqualTo(AlarmReference.START) + } + + @Test fun `an override without alarms has none, whatever its master carries`() { + val body = "BEGIN:VCALENDAR\r\nVERSION:2.0\r\n" + + "BEGIN:VTODO\r\nUID:s\r\nSUMMARY:Water\r\nDUE:20260305T090000Z\r\nRRULE:FREQ=DAILY\r\n" + + "$alarm15\r\nEND:VTODO\r\n" + + "BEGIN:VTODO\r\nUID:s\r\nSUMMARY:Water (late)\r\nRECURRENCE-ID:20260306T090000Z\r\n" + + "DUE:20260306T120000Z\r\nEND:VTODO\r\nEND:VCALENDAR\r\n" + remote.put("s.ics", body) + + sync() + + val master = store.rows.single { it.recurrenceId == null } + assertThat(store.alarms.map { it.taskId }).containsExactly(master.id) + } + + @Test fun `a 412 discards a local reminder edit along with the rest`() { + remote.put("one.ics", vtodo("a", "Pay rent", extra = "DUE:20260305T090000Z\r\n$alarm15"), eTag = "newer") + store.rows += task(id = 1, uid = "a", title = "Pay rent") + .copy(href = "http://server/dav/tasks/one.ics", etag = "stale", due = due, isDirty = true) + store.alarms += alarm(1, 60) + + sync() + + assertThat(store.alarms.map { it.minutesBefore }).containsExactly(15) + } + private fun task( id: Long, uid: String, diff --git a/app/src/testFull/java/de/jeanlucmakiola/agendula/data/sync/SyncFakes.kt b/app/src/testFull/java/de/jeanlucmakiola/agendula/data/sync/SyncFakes.kt index c59b3af..0b18304 100644 --- a/app/src/testFull/java/de/jeanlucmakiola/agendula/data/sync/SyncFakes.kt +++ b/app/src/testFull/java/de/jeanlucmakiola/agendula/data/sync/SyncFakes.kt @@ -1,5 +1,6 @@ package de.jeanlucmakiola.agendula.data.sync +import de.jeanlucmakiola.agendula.data.tasks.room.TaskAlarmEntity import de.jeanlucmakiola.agendula.data.tasks.room.TaskEntity import de.jeanlucmakiola.caldav.CalendarCollection import de.jeanlucmakiola.caldav.ChangeSet @@ -19,6 +20,10 @@ import okhttp3.HttpUrl.Companion.toHttpUrl class FakeStore(rows: List = emptyList()) : SyncStore { val rows = rows.toMutableList() + + /** `task_alarms`, cascading with [rows] as the foreign key does. */ + val alarms = mutableListOf() + private var nextAlarmId = 1L val readOnlyWrites = mutableListOf>() /** In order, so a test can prove the token was stored *after* its bodies. */ @@ -27,7 +32,7 @@ class FakeStore(rows: List = emptyList()) : SyncStore { override fun rowsIn(listId: Long) = this.rows.filter { it.listId == listId } - override fun insert(row: TaskEntity): Long { + fun insert(row: TaskEntity): Long { // Computed against the rows as they stand, not at construction: tests // seed `rows` directly after building this, and an id fixed up front // collides with the first seeded row — which then makes a delete of one @@ -45,6 +50,30 @@ class FakeStore(rows: List = emptyList()) : SyncStore { override fun deleteAll(taskIds: List) { rows.removeAll { it.id in taskIds } + alarms.removeAll { it.taskId in taskIds } + } + + override fun alarmsFor(taskIds: List) = alarms.filter { it.taskId in taskIds }.groupBy { it.taskId } + + override fun upsertWithAlarms(row: TaskEntity, alarms: List): Long { + if (row.id != 0L && rows.none { it.id == row.id }) return row.id + val id = if (row.id == 0L) insert(row) else row.id.also { update(row) } + val hadAny = this.alarms.any { it.taskId == id } + // Matched by uid, then lead, as `TaskAlarmDao.replaceFromServer` does. + val existing = this.alarms.filter { it.taskId == id }.toMutableList() + this.alarms.removeAll { it.taskId == id } + alarms.forEach { alarm -> + val match = existing.firstOrNull { it.uid == alarm.uid } + ?: existing.firstOrNull { it.minutesBefore == alarm.minutesBefore && it.reference == alarm.reference } + match?.let(existing::remove) + this.alarms += alarm.copy(id = match?.id ?: nextAlarmId++, taskId = id) + } + val index = rows.indexOfFirst { it.id == id } + when { + alarms.isNotEmpty() -> rows[index] = rows[index].copy(remindersOff = false) + hadAny -> rows[index] = rows[index].copy(remindersOff = true) + } + return id } override fun markSynced(taskIds: List, href: String?, eTag: String?) { diff --git a/docs/PRIVACY.md b/docs/PRIVACY.md index 60a91df..a09b504 100644 --- a/docs/PRIVACY.md +++ b/docs/PRIVACY.md @@ -1,7 +1,7 @@ --- title: Privacy Policy — Agendula description: What Agendula does with your data. No servers, no account, no analytics — your tasks stay on your device unless you add a CalDAV server yourself. -updated: 2026-09-21 +updated: 2026-10-19 --- -**Last updated:** 21 September 2026 +**Last updated:** 19 October 2026 Applies to the Android app **Agendula** (package `de.jeanlucmakiola.agendula`), all versions and all distribution channels. @@ -76,8 +76,9 @@ exported from the device. ### What is transmitted, and to whom -- The tasks in the synchronised lists, as standard iCalendar (`VTODO`) data, - and the credentials needed to authenticate. +- The tasks in the synchronised lists, as standard iCalendar (`VTODO`) data — + including their reminders, as `VALARM`s — and the credentials needed to + authenticate. - Requests carry the user agent `Agendula (Android)` — a fixed string, so that you can recognise and revoke the session on your server. No device identifier is sent.