diff --git a/CHANGELOG.md b/CHANGELOG.md index a1bf371..4fcd508 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,13 @@ All notable changes to this project are documented here. The format follows see it as done instead of still open. Unticking it moves the series back, as long as the series hasn't moved on or changed its start since (#137). +### Fixed +- With tasks.org as the storage provider, recurring tasks are recognised as + recurring. Before, editing one removed its recurrence and ticking it off + closed the whole series. With tasks.org and OpenTasks alike, the editor now + shows the task's repeat rule instead of none, and an occurrence changed on + its own stays part of its series (#135). + ## [1.3.0] - 2026-11-02 ### Added diff --git a/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/ExternalProviderRecurrenceTest.kt b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/ExternalProviderRecurrenceTest.kt new file mode 100644 index 0000000..f22c196 --- /dev/null +++ b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/ExternalProviderRecurrenceTest.kt @@ -0,0 +1,290 @@ +package de.jeanlucmakiola.agendula.data.tasks + +import android.content.ContentValues +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat +import de.jeanlucmakiola.agendula.data.tasks.TasksContract.Tasks +import de.jeanlucmakiola.agendula.domain.Task +import de.jeanlucmakiola.agendula.domain.TaskForm +import de.jeanlucmakiola.agendula.domain.TaskReminder +import org.junit.After +import org.junit.Assert.assertThrows +import org.junit.Assume.assumeTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import kotlin.time.Clock +import kotlin.time.Duration.Companion.hours +import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.Instant + +/** + * Recurrence against a real external provider (#135). Skipped unless one is + * installed and its permissions are granted, so it only runs where set up: + * `pm grant org.tasks.permission.READ_TASKS` (and WRITE_TASKS). + */ +@RunWith(AndroidJUnit4::class) +class ExternalProviderRecurrenceTest { + + private val context = ApplicationProvider.getApplicationContext() + private val resolver = ProviderResolver(AndroidProviderEnvironment(context)).apply { + storageMode = StorageMode.EXTERNAL + } + private val source = AndroidTasksDataSource(context, resolver) + private var listId = 0L + + @Before + fun setUp() { + val provider = resolver.resolve() + assumeTrue("no external tasks provider, or not granted", provider != null && resolver.hasPermission(provider)) + listId = source.createLocalList("Recurrence test", 0) + } + + @After + fun tearDown() { + if (listId != 0L) source.deleteList(listId) + } + + private fun dailyTask(): Long { + // Milliseconds and all, as the "start today" default gives. + val start = Clock.System.now() + 1.hours + return source.insertTask( + TaskForm(title = "Daily", listId = listId, start = start, due = start + 1.hours, rrule = "FREQ=DAILY"), + reminders = null, + ) + } + + private fun storedRule(taskId: Long): String? = context.contentResolver.query( + TasksContract.tasksUri(resolver.resolve()!!.authority), + arrayOf(Tasks.RRULE), + "${Tasks.ID} = ?", + arrayOf(taskId.toString()), + null, + )?.use { c -> if (c.moveToFirst()) c.getString(0) else null } + + @Test + fun aSeriesReadsAsRecurringWithItsRule() { + val id = dailyTask() + + val listed = source.tasks(TaskQuery(listId = listId)).filter { it.taskId == id }.minBy { it.occurrenceStart!! } + assertThat(listed.isRecurring).isTrue() + assertThat(listed.isOccurrence).isTrue() + assertThat(listed.recurrenceRule).isEqualTo("FREQ=DAILY") + + val opened = source.task(id)!! + assertThat(opened.recurrenceRule).isEqualTo("FREQ=DAILY") + } + + @Test + fun savingTheSeriesKeepsItsRule() { + val id = dailyTask() + val task = source.task(id)!! + + source.updateSeries( + id, + task.occurrenceStart!!, + TaskForm(title = "Daily, renamed", listId = listId, start = task.start, due = task.due, rrule = task.recurrenceRule), + reminders = null, + ) + + assertThat(storedRule(id)).isEqualTo("FREQ=DAILY") + } + + @Test + fun completingAnOccurrenceMovesTheSeriesOn() { + val id = dailyTask() + val task = source.task(id)!! + + source.setCompletedInstance(id, task.occurrenceStart!!, completed = true) + + assertSeriesMovedOn(id, task.occurrenceStart!!) + } + + @Test + fun aSeriesStoredWithMillisecondsStillMovesOn() { + val id = legacySeries() + val task = source.task(id)!! + + source.setCompletedInstance(id, task.occurrenceStart!!, completed = true) + + assertSeriesMovedOn(id, task.occurrenceStart) + } + + @Test + fun anUnknownOccurrenceDoesNotCloseTheSeries() { + val id = dailyTask() + val anchor = source.task(id)!!.occurrenceStart!! + + assertThrows(TaskWriteFailedException::class.java) { + source.setCompletedInstance(id, anchor + 30.hours, completed = true) + } + + assertThat(source.task(id)!!.isClosed).isFalse() + } + + @Test + fun roundingALegacySeriesLeavesItUnmodified() { + val id = legacySeries() + val before = storedColumn(id, Tasks.LAST_MODIFIED) + + source.task(id) + + assertThat(storedStart(id)!! % 1000).isEqualTo(0L) + assertThat(storedColumn(id, Tasks.LAST_MODIFIED)).isEqualTo(before) + } + + /** A daily series as builds before whole-second writes left it, already in sync with its server. */ + private fun legacySeries(): Long { + val start = (Clock.System.now() + 1.hours).toEpochMilliseconds() / 1000 * 1000 + return insertAsSyncAdapter( + Tasks.LIST_ID to listId, + Tasks.TITLE to "Old daily", + Tasks.DTSTART to start + 345, + Tasks.DUE to start + 3_600_345, + Tasks.TZ to "UTC", + Tasks.RRULE to "FREQ=DAILY", + ).also { + // The app's own reads may already have run the one-time rounding. + context.getSharedPreferences("provider_migrations", Context.MODE_PRIVATE).edit().clear().commit() + } + } + + private fun insertAsSyncAdapter(vararg values: Pair): Long { + val uri = TasksContract.asSyncAdapter( + TasksContract.tasksUri(resolver.resolve()!!.authority), + TasksContract.LOCAL_ACCOUNT_NAME, + TasksContract.LOCAL_ACCOUNT_TYPE, + ) + val cv = ContentValues().apply { + values.forEach { (k, v) -> if (v is Long) put(k, v) else put(k, v.toString()) } + } + return context.contentResolver.insert(uri, cv)!!.lastPathSegment!!.toLong() + } + + /** Edits the series' first occurrence on its own and returns the override that makes. */ + private fun overrideOf(id: Long): Task { + val task = source.task(id)!! + source.updateInstance( + id, + task.occurrenceStart!!, + TaskForm(title = "Just this once", listId = listId, start = task.start, due = task.due), + reminders = null, + ) + return source.tasks(TaskQuery(listId = listId)).single { it.seriesId == id && it.taskId != id } + } + + @Test + fun anOverrideBelongsToItsSeries() { + val id = dailyTask() + val anchor = source.task(id)!!.occurrenceStart + + val override = overrideOf(id) + + assertThat(override.title).isEqualTo("Just this once") + assertThat(override.isOccurrence).isTrue() + assertThat(override.occurrenceStart).isEqualTo(anchor) + assertThat(override.recurrenceRule).isEqualTo("FREQ=DAILY") + assertThat(source.task(override.taskId, override.occurrenceStart)?.taskId).isEqualTo(override.taskId) + } + + @Test + fun anAllDayDueAddedToTheSeriesStaysOnItsDay() { + val day = Clock.System.now().toEpochMilliseconds() / 86_400_000 * 86_400_000 + val id = source.insertTask( + TaskForm(title = "All day", listId = listId, start = Instant.fromEpochMilliseconds(day), isAllDay = true, rrule = "FREQ=DAILY"), + reminders = null, + ) + val task = source.task(id)!! + // Local midnight the next day, as the date picker hands it over. + val localMidnight = Instant.fromEpochMilliseconds(day + 86_400_000 - java.util.TimeZone.getDefault().getOffset(day)) + + source.updateSeries( + id, + task.occurrenceStart!!, + TaskForm(title = "All day", listId = listId, start = task.start, due = localMidnight, isAllDay = true, rrule = "FREQ=DAILY"), + reminders = null, + ) + + assertThat(storedColumn(id, Tasks.DUE)!!.toLong() % 86_400_000).isEqualTo(0L) + } + + @Test + fun savingAllFromAnOverrideKeepsTheSeriesWhereItIs() { + val id = dailyTask() + val before = storedStart(id) + val override = overrideOf(id) + + source.updateSeries( + id, + override.occurrenceStart!!, + TaskForm(title = "Renamed", listId = listId, start = override.start, due = override.due, rrule = override.recurrenceRule), + reminders = null, + ) + + assertThat(storedStart(id)).isEqualTo(before) + assertThat(storedRule(id)).isEqualTo("FREQ=DAILY") + } + + @Test + fun deletingAnOverrideExcludesItsOccurrence() { + val id = dailyTask() + val override = overrideOf(id) + + source.deleteOverride(override.taskId, override.occurrenceStart!!) + + // Excluded on the master, not just the override dropped. + assertThat(storedColumn(id, Tasks.EXDATE)).isNotNull() + val open = source.tasks(TaskQuery(listId = listId)).filter { it.seriesId == id } + assertThat(open.map { it.occurrenceStart }).doesNotContain(override.occurrenceStart) + assertThat(storedRule(id)).isEqualTo("FREQ=DAILY") + } + + @Test + fun editingAnOverrideAgainKeepsItsNewTagsAndReminders() { + val id = dailyTask() + val override = overrideOf(id) + val form = TaskForm( + title = override.title, + listId = listId, + start = override.start, + due = override.due, + tags = listOf("urgent"), + reminders = listOf(TaskReminder(minutesBefore = 15)), + ) + + source.updateInstance(override.taskId, override.occurrenceStart!!, form, form.reminders) + + assertThat(source.task(override.taskId)!!.tags).containsExactly("urgent") + assertThat(source.reminders()[override.taskId]).containsExactly(TaskReminder(minutesBefore = 15)) + assertThat(source.task(id)?.tags.orEmpty()).isEmpty() + } + + @Test + fun anOccurrenceIsFoundByItsAnchorToTheSecond() { + val id = dailyTask() + val override = overrideOf(id) + + val found = source.task(id, override.occurrenceStart!! + 345.milliseconds) + + assertThat(found?.taskId).isEqualTo(override.taskId) + } + + private fun storedStart(taskId: Long): Long? = storedColumn(taskId, Tasks.DTSTART)?.toLong() + + private fun storedColumn(taskId: Long, column: String): String? = context.contentResolver.query( + TasksContract.tasksUri(resolver.resolve()!!.authority), + arrayOf(column), + "${Tasks.ID} = ?", + arrayOf(taskId.toString()), + null, + )?.use { c -> if (c.moveToFirst()) c.getString(0) else null } + + private fun assertSeriesMovedOn(id: Long, completed: Instant) { + assertThat(storedRule(id)).isNotNull() + val open = source.tasks(TaskQuery(listId = listId)).filter { it.seriesId == id } + assertThat(open).isNotEmpty() + assertThat(open.minOf { it.occurrenceStart!! }).isGreaterThan(completed) + } +} 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 958a0d5..f653735 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 @@ -7,9 +7,12 @@ import android.content.ContentUris import android.content.ContentValues import android.content.Context import android.database.ContentObserver +import android.database.Cursor +import android.database.sqlite.SQLiteException import android.net.Uri import android.os.Handler import android.os.Looper +import android.util.Log import dagger.hilt.android.qualifiers.ApplicationContext import de.jeanlucmakiola.agendula.data.tasks.TasksContract.Instances import de.jeanlucmakiola.agendula.data.tasks.TasksContract.Lists @@ -73,14 +76,20 @@ class AndroidTasksDataSource @Inject constructor( } override fun task(taskId: Long): Task? { - val rows = queryInstances("${Instances.TASK_ID} = ?", arrayOf(taskId.toString())) - return rows.firstOrNull { it.distanceFromCurrent == 0 } ?: rows.firstOrNull() + roundLegacyAnchors() + val rows = instanceRows(resolver.query(TasksContract.instancesUri(authority()), null, "${Instances.TASK_ID} = ?", arrayOf(taskId.toString()), null)) + return (rows.firstOrNull { it.task.distanceFromCurrent == 0 } ?: rows.firstOrNull())?.let { enrich(listOf(it)).single() } } override fun task(taskId: Long, occurrenceStart: Instant?): Task? { if (occurrenceStart == null) return task(taskId) - val rows = queryInstances("${Instances.TASK_ID} = ?", arrayOf(taskId.toString())) - return rows.firstOrNull { it.occurrenceStart == occurrenceStart } ?: task(taskId) + roundLegacyAnchors() + val rows = instanceRows(queryOccurrences(taskId)) + val chosen = rows.firstOrNull { it.anchor == occurrenceStart } + ?: rows.firstOrNull { it.anchor?.let { a -> sameSecond(a, occurrenceStart) } == true } + ?: rows.firstOrNull { it.task.taskId == taskId && it.task.distanceFromCurrent == 0 } + ?: rows.firstOrNull { it.task.taskId == taskId } + return chosen?.let { enrich(listOf(it)).single() } } override fun subtasks(parentTaskId: Long): List = @@ -107,23 +116,74 @@ class AndroidTasksDataSource @Inject constructor( } } ?: emptyList() + /** + * All columns and no sort: the instances view differs across provider + * versions (tasks.org's has no `is_recurring`), the by-name mapper nulls what + * is missing, and the repository re-sorts. + */ private fun queryInstances(selection: String?, args: Array?): List { - val uri = TasksContract.instancesUri(authority()) - // projection = null (all columns): the instances view differs across - // provider versions (e.g. tasks.org's bundled OpenTasks has no - // `is_recurring` / `distance_from_current`). The by-name mapper reads - // whatever is present and nulls the rest; the repository re-sorts, so we - // don't depend on a provider sort column either. - val tasks = resolver.query(uri, null, selection, args, null) - ?.use { c -> - val reader = CursorColumnReader(c) - buildList { while (c.moveToNext()) add(TaskMapper.task(reader)) } - } ?: emptyList() - if (tasks.isEmpty()) return tasks + roundLegacyAnchors() + return instanceTasks(resolver.query(TasksContract.instancesUri(authority()), null, selection, args, null)) + } + + /** Maps [cursor]'s instance rows, with the rule each series keeps on its `tasks` row, and their tags. */ + private fun instanceTasks(cursor: Cursor?): List = enrich(instanceRows(cursor)) + + /** [cursor]'s instance rows, before their rules and tags are looked up. */ + private fun instanceRows(cursor: Cursor?): List { + var flagged = false + val rows = cursor?.use { c -> + flagged = c.getColumnIndex(Instances.IS_RECURRING) >= 0 + val reader = CursorColumnReader(c) + buildList { while (c.moveToNext()) add(TaskMapper.instance(reader)) } + }.orEmpty() + // Where the provider flags recurrence, only flagged rows need their rule looked up. + return if (flagged) rows.map { if (it.task.isRecurring) it else it.copy(ruleOwner = NO_RULE) } else rows + } + + /** [rows] completed with the rule each series keeps on its `tasks` row, and their tags. */ + private fun enrich(rows: List): List { + if (rows.isEmpty()) return emptyList() + val rules = seriesRules(rows.map { it.ruleOwner }.filter { it != NO_RULE }) + val tasks = rows.map { TaskMapper.withRule(it, rules[it.ruleOwner]) } val tags = categories(tasks.map { it.taskId }) return tasks.map { task -> tags[task.taskId]?.let { task.copy(tags = it) } ?: task } } + /** The recurrence of each of [taskIds] that is a series, keyed by task id. */ + private fun seriesRules(taskIds: Collection): Map = queryByIds( + TasksContract.tasksUri(authority()), + arrayOf(Tasks.ID, Tasks.RRULE, Tasks.RDATE), + Tasks.ID, + taskIds, + filter = "${Tasks.RRULE} IS NOT NULL OR ${Tasks.RDATE} IS NOT NULL", + ) { r -> r.getLong(Tasks.ID)?.let { id -> TaskMapper.seriesRule(r)?.let { id to it } } }.toMap() + + /** + * Rows of [uri] whose [idColumn] is one of [ids], in chunks that stay under + * SQLite's bound-argument limit, each mapped by [read] (null rows dropped). + */ + private fun queryByIds( + uri: Uri, + projection: Array, + idColumn: String, + ids: Collection, + filter: String? = null, + filterArgs: List = emptyList(), + sortOrder: String? = null, + read: (ColumnReader) -> T?, + ): List = ids.distinct().chunked(MAX_SQL_ARGS - filterArgs.size).flatMap { chunk -> + val selection = buildString { + append("$idColumn IN (${chunk.joinToString(",") { "?" }})") + if (filter != null) append(" AND ($filter)") + } + val args = (chunk.map(Long::toString) + filterArgs).toTypedArray() + resolver.query(uri, projection, selection, args, sortOrder)?.use { c -> + val reader = CursorColumnReader(c) + buildList { while (c.moveToNext()) read(reader)?.let(::add) } + } ?: emptyList() + } + override fun exportTasks(listId: Long): List { // projection = null for the same reason queryInstances uses it: the tasks // table's shape varies across provider versions, and the by-name mapper @@ -194,7 +254,7 @@ class AndroidTasksDataSource @Inject constructor( } override fun updateSeries(seriesId: Long, occurrenceStart: Instant, form: TaskForm, reminders: List?) { - val occurrence = task(seriesId, occurrenceStart) + val occurrence = occurrence(seriesId, occurrenceStart) { TaskMapper.instance(it).task } ?: task(seriesId) val anchors = resolver.query( taskUri(authority(), seriesId), arrayOf(Tasks.DTSTART, Tasks.DUE), null, null, null, )?.use { c -> @@ -202,14 +262,14 @@ class AndroidTasksDataSource @Inject constructor( val r = CursorColumnReader(c) r.getLong(Tasks.DTSTART) to r.getLong(Tasks.DUE) } - fun shifted(anchor: Long?, was: Instant?, now: Instant?): Long? = when { - now == null -> null - anchor == null || was == null -> now.toEpochMilliseconds() - else -> anchor + (now - was).inWholeMilliseconds - } - val values = TaskWriteMapper.seriesValues(form, ZoneId.systemDefault().id) + - (Tasks.DTSTART to shifted(anchors?.first, occurrence?.start, form.start)) + - (Tasks.DUE to shifted(anchors?.second, occurrence?.due, form.due)) + val base = TaskWriteMapper.seriesValues(form, ZoneId.systemDefault().id) + // Without an anchor to shift, the form's own value stands — already pinned for an all-day series. + fun shifted(column: String, anchor: Long?, was: Instant?, now: Instant?): Any? = + if (now == null || anchor == null || was == null) base[column] + else providerMillis(anchor + (now - was).inWholeMilliseconds) + val values = base + + (Tasks.DTSTART to shifted(Tasks.DTSTART, anchors?.first, occurrence?.start, form.start)) + + (Tasks.DUE to shifted(Tasks.DUE, anchors?.second, occurrence?.due, form.due)) val rows = resolver.update(taskUri(authority(), seriesId), values.toContentValues(), null, null) if (rows == 0) throw TaskWriteFailedException("update series $seriesId") setCategories(seriesId, form.tags) @@ -253,9 +313,10 @@ class AndroidTasksDataSource @Inject constructor( if (!c.moveToFirst()) return@use null val r = CursorColumnReader(c) val anchor = r.getLong(Tasks.DTSTART) ?: r.getLong(Tasks.DUE) ?: return@use null + val rule = TaskMapper.seriesRule(r) RecurrenceSpec( - rrule = r.getString(Tasks.RRULE)?.ifBlank { null }, - rdate = r.getString(Tasks.RDATE), + rrule = rule?.rrule, + rdate = rule?.rdate, exdate = r.getString(Tasks.EXDATE), anchor = Instant.fromEpochMilliseconds(anchor), isAllDay = r.getBoolean(Tasks.IS_ALLDAY), @@ -290,7 +351,7 @@ class AndroidTasksDataSource @Inject constructor( buildList { while (c.moveToNext()) { val original = r.getLong(Tasks.ORIGINAL_INSTANCE_TIME) ?: continue - if (original >= from.toEpochMilliseconds()) r.getLong(Tasks.ID)?.let(::add) + if (providerMillis(original) >= providerMillis(from.toEpochMilliseconds())) r.getLong(Tasks.ID)?.let(::add) } } } ?: emptyList() @@ -302,7 +363,10 @@ class AndroidTasksDataSource @Inject constructor( } override fun updateInstance(taskId: Long, occurrenceStart: Instant, form: TaskForm, reminders: List?) { - if (reminders == null) return writeInstance(taskId, occurrenceStart, form) + val hit = instanceFor(taskId, occurrenceStart) + ?: throw TaskWriteFailedException("update instance $taskId@$occurrenceStart: no such occurrence") + hit.overrideId?.let { return updateOverride(it, form, reminders) } + if (reminders == null) return writeInstance(taskId, hit.instanceId, 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 @@ -311,15 +375,26 @@ class AndroidTasksDataSource @Inject constructor( val seriesReminders = alarmRows(taskId).map { it.second } writeReminders(taskId, reminders) try { - writeInstance(taskId, occurrenceStart, form) + writeInstance(taskId, hit.instanceId, 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") + /** + * An occurrence that already has its own row is written there directly. The + * fork-and-restore dance in [updateInstance] would put its old tags and + * reminders back over the new ones. + */ + private fun updateOverride(overrideId: Long, form: TaskForm, reminders: List?) { + val values = TaskWriteMapper.instanceValues(form, ZoneId.systemDefault().id) + val rows = resolver.update(taskUri(authority(), overrideId), values.toContentValues(), null, null) + if (rows == 0) throw TaskWriteFailedException("update override $overrideId") + setCategories(overrideId, form.tags) + writeReminders(overrideId, reminders) + } + + private fun writeInstance(taskId: Long, instanceId: Long, form: TaskForm) { val values = TaskWriteMapper.instanceValues(form, ZoneId.systemDefault().id) val uri = TasksContract.instanceUri(authority(), instanceId) // The fork copies the series' properties onto the override, so the tags @@ -346,28 +421,136 @@ class AndroidTasksDataSource @Inject constructor( } } + /** An occurrence's instance row, and the override row behind it if it has one. */ + private class InstanceHit(val instanceId: Long, val overrideId: Long?) + /** - * The provider's instance row id for one occurrence. + * The provider's instance row for one occurrence. * * The seam addresses occurrences by `(taskId, occurrenceStart)`; writing * through the instances URI still needs the row id, so it is looked up here - * rather than carried around above the data layer. Selection is on `task_id` - * only — the anchor is matched in Kotlin because the column that holds it - * (`instance_original_time`) is missing on older provider schemas, where a - * WHERE clause naming it would throw instead of falling back. + * rather than carried around above the data layer. */ - private fun instanceIdFor(taskId: Long, occurrenceStart: Instant): Long? { - val uri = TasksContract.instancesUri(authority()) - val selection = "${Instances.TASK_ID} = ?" - return resolver.query(uri, null, selection, arrayOf(taskId.toString()), null)?.use { c -> + private fun instanceFor(taskId: Long, occurrenceStart: Instant): InstanceHit? { + roundLegacyAnchors() + return occurrence(taskId, occurrenceStart) { r -> + r.getLong(Tasks.ID)?.let { id -> + InstanceHit(id, r.getLong(Instances.TASK_ID)?.takeIf { r.getLong(Tasks.ORIGINAL_INSTANCE_ID) != null }) + } + } + } + + private fun instanceIdFor(taskId: Long, occurrenceStart: Instant): Long? = instanceFor(taskId, occurrenceStart)?.instanceId + + /** + * [read] applied to the instance row of [taskId]'s occurrence at + * [occurrenceStart] — its own, or an override's that replaced it — matched in + * whole seconds, as the provider expands them. The anchor is matched in + * Kotlin because `instance_original_time` is missing on older schemas. + */ + private fun occurrence(taskId: Long, occurrenceStart: Instant, read: (ColumnReader) -> T): T? = + queryOccurrences(taskId)?.use { c -> val reader = CursorColumnReader(c) + // An exact match first: a row of the same second is a fallback, not a peer. + var near: Int? = null while (c.moveToNext()) { - if (TaskMapper.occurrenceAnchor(reader) == occurrenceStart) { - return@use reader.getLong(Tasks.ID) + val anchor = TaskMapper.occurrenceAnchor(reader) ?: continue + if (anchor == occurrenceStart) return@use read(reader) + if (near == null && sameSecond(anchor, occurrenceStart)) near = c.position + } + near?.let { c.moveToPosition(it); read(reader) } + } + + /** + * Instance rows of [taskId] and of the overrides standing in for its + * occurrences. A provider whose view lacks `original_instance_id` gets the + * task's own rows only, rather than a failed query. + */ + private fun queryOccurrences(taskId: Long): Cursor? { + val authority = authority() + val uri = TasksContract.instancesUri(authority) + val id = taskId.toString() + if (ownRowsOnlyFor != authority) { + try { + return resolver.query(uri, null, OCCURRENCE_OF, arrayOf(id, id), null) + } catch (_: SQLiteException) { + ownRowsOnlyFor = authority + } catch (_: IllegalArgumentException) { + ownRowsOnlyFor = authority + } + } + return resolver.query(uri, null, "${Instances.TASK_ID} = ?", arrayOf(id), null) + } + + /** The provider whose instances view turned out to lack `original_instance_id`. */ + @Volatile + private var ownRowsOnlyFor: String? = null + + @Volatile + private var anchorsRoundedFor: String? = null + + private val roundingLock = Any() + + /** + * Rounds the DTSTART/DUE a build before [providerMillis] stored with + * milliseconds, which the provider cannot move past a completed occurrence. + * Once per provider, before the first read: concurrent readers wait for it. + * Written as the rows' sync adapter, since the server only ever had whole + * seconds, so nothing is dirtied or re-uploaded. Overrides need nothing: the + * provider forks them at its own whole-second instance times. + * + * Never fails a read. A row the provider refuses is left as it was and tried + * again on the next start; only a pass that rounded everything is remembered. + */ + private fun roundLegacyAnchors() { + val authority = authority() + if (anchorsRoundedFor == authority) return + synchronized(roundingLock) { + if (anchorsRoundedFor == authority) return + val prefs = context.getSharedPreferences(MIGRATIONS, Context.MODE_PRIVATE) + val key = "anchors_rounded:$authority" + val done = prefs.getBoolean(key, false) || runCatching { roundAll(authority) } + .onFailure { Log.w(TAG, "Rounding legacy anchors failed", it) } + .getOrDefault(false) + if (done) prefs.edit().putBoolean(key, true).apply() + anchorsRoundedFor = authority + } + } + + /** True when every stale row was rounded. */ + private fun roundAll(authority: String): Boolean { + val columns = listOf(Tasks.DTSTART, Tasks.DUE) + val ops = resolver.query( + TasksContract.tasksUri(authority), + arrayOf(Tasks.ID, Tasks.ACCOUNT_NAME, Tasks.ACCOUNT_TYPE) + columns, + columns.joinToString(" OR ") { "$it % 1000 != 0" }, + null, + null, + )?.use { c -> + val r = CursorColumnReader(c) + buildList { + while (c.moveToNext()) { + val id = r.getLong(Tasks.ID) ?: continue + val values = buildMap { + for (column in columns) { + val millis = r.getLong(column) ?: continue + if (providerMillis(millis) != millis) put(column, providerMillis(millis)) + } + } + val name = r.getString(Tasks.ACCOUNT_NAME) + val type = r.getString(Tasks.ACCOUNT_TYPE) + val uri = if (name.isNullOrEmpty() || type.isNullOrEmpty()) taskUri(authority, id) + else TasksContract.asSyncAdapter(taskUri(authority, id), name, type) + add(ContentProviderOperation.newUpdate(uri).withValues(values.toContentValues()).build()) } } - null - } + } ?: return false + if (ops.isEmpty()) return true + val batch = runCatching { resolver.applyBatch(authority, ArrayList(ops)) }.getOrNull() + if (batch != null) return batch.all { (it.count ?: 0) > 0 } + // One refused row fails the whole batch; row by row, the rest still go. + return ops.map { op -> runCatching { resolver.applyBatch(authority, arrayListOf(op)).single().count ?: 0 }.getOrDefault(0) > 0 } + .all { it } } override fun setReminders(taskId: Long, reminders: List) { @@ -430,29 +613,19 @@ class AndroidTasksDataSource @Inject constructor( } /** Tags by task id for [taskIds], in stored order. */ - private fun categories(taskIds: Collection): Map> { - val rows = taskIds.distinct().chunked(MAX_SQL_ARGS - 1).flatMap { chunk -> - val selection = "${Properties.MIMETYPE} = ? AND ${Properties.TASK_ID} IN (${chunk.joinToString(",") { "?" }})" - val args = (listOf(TasksContract.Category.MIMETYPE) + chunk.map(Long::toString)).toTypedArray() - resolver.query( - TasksContract.propertiesUri(authority()), - arrayOf(Properties.TASK_ID, TasksContract.Category.NAME), - selection, - args, - Properties.PROPERTY_ID, - )?.use { c -> - val reader = CursorColumnReader(c) - buildList { - while (c.moveToNext()) { - val id = reader.getLong(Properties.TASK_ID) ?: continue - val name = reader.getString(TasksContract.Category.NAME) ?: continue - add(id to name) - } - } - } ?: emptyList() - } - return rows.groupBy({ it.first }, { it.second }).mapValues { normalizeTags(it.value) } - } + private fun categories(taskIds: Collection): Map> = queryByIds( + TasksContract.propertiesUri(authority()), + arrayOf(Properties.TASK_ID, TasksContract.Category.NAME), + Properties.TASK_ID, + taskIds, + filter = "${Properties.MIMETYPE} = ?", + filterArgs = listOf(TasksContract.Category.MIMETYPE), + sortOrder = Properties.PROPERTY_ID, + ) { r -> + val id = r.getLong(Properties.TASK_ID) + val name = r.getString(TasksContract.Category.NAME) + if (id == null || name == null) null else id to name + }.groupBy({ it.first }, { it.second }).mapValues { normalizeTags(it.value) } /** * Replaces [taskId]'s category rows in one batch, so a refused insert cannot @@ -489,7 +662,7 @@ class AndroidTasksDataSource @Inject constructor( /** Through the instances URI, like [setCompletedInstance]. */ override fun setCancelledInstance(taskId: Long, occurrenceStart: Instant, cancelled: Boolean) { val instanceId = instanceIdFor(taskId, occurrenceStart) - ?: return setCancelled(taskId, cancelled) + ?: return plainUnlessSeries(taskId, occurrenceStart) { setCancelled(taskId, cancelled) } val uri = TasksContract.instanceUri(authority(), instanceId) val rows = resolver.update(uri, cancelValues(cancelled).toContentValues(), null, null) if (rows == 0) throw TaskWriteFailedException("cancel instance $instanceId") @@ -508,22 +681,42 @@ class AndroidTasksDataSource @Inject constructor( /** * Through the instances URI, which is what makes the provider fork an override - * rather than close the series. No instance row for the anchor means the task - * is not a series after all — the plain write is then the right one. + * rather than close the series. No instance row for the anchor on a task that + * is not a series means the plain write is the right one. */ override fun setCompletedInstance(taskId: Long, occurrenceStart: Instant, completed: Boolean) { val instanceId = instanceIdFor(taskId, occurrenceStart) - ?: return setCompleted(taskId, completed) + ?: return plainUnlessSeries(taskId, occurrenceStart) { setCompleted(taskId, completed) } val values = TaskWriteMapper.completionValues(completed, System.currentTimeMillis()) val uri = TasksContract.instanceUri(authority(), instanceId) val rows = resolver.update(uri, values.toContentValues(), null, null) if (rows == 0) throw TaskWriteFailedException("complete instance $instanceId") } + /** + * [write] for a task that turned out not to be a series. A series whose + * occurrence was not found fails instead: the plain write would close all of it. + */ + private fun plainUnlessSeries(taskId: Long, occurrenceStart: Instant, write: () -> Unit) { + if (seriesRules(listOf(taskId)).isNotEmpty()) { + throw TaskWriteFailedException("instance $taskId@$occurrenceStart: no such occurrence") + } + write() + } + override fun deleteTask(taskId: Long) { resolver.delete(taskUri(authority(), taskId), null, null) } + /** + * Through its instance, which the provider turns into an EXDATE on the master, + * as the Room store does. Deleting the row alone leaves the master without one. + */ + override fun deleteOverride(taskId: Long, occurrenceStart: Instant) { + val instanceId = instanceIdFor(taskId, occurrenceStart) ?: return deleteTask(taskId) + resolver.delete(TasksContract.instanceUri(authority(), instanceId), null, null) + } + override fun createLocalList(name: String, color: Int): Long { val uri = TasksContract.asSyncAdapter( TasksContract.listsUri(authority()), @@ -594,5 +787,18 @@ class AndroidTasksDataSource @Inject constructor( } } +/** An instance row of the task, or of an override standing in for one of its occurrences. */ +private const val OCCURRENCE_OF = "${Instances.TASK_ID} = ? OR ${Tasks.ORIGINAL_INSTANCE_ID} = ?" + +private fun sameSecond(a: Instant, b: Instant): Boolean = + providerMillis(a.toEpochMilliseconds()) == providerMillis(b.toEpochMilliseconds()) + +private const val MIGRATIONS = "provider_migrations" + +private const val TAG = "AndroidTasksDataSource" + +/** No rule to look up: the provider says the row does not recur. */ +private const val NO_RULE = -1L + /** SQLite's default bound-argument limit on older Android releases. */ private const val MAX_SQL_ARGS = 999 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 cb0ef65..7f3eb5a 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 @@ -67,6 +67,7 @@ class ModeRoutingTasksDataSource( active().setCancelledInstance(taskId, occurrenceStart, cancelled) override fun deleteTask(taskId: Long) = active().deleteTask(taskId) + override fun deleteOverride(taskId: Long, occurrenceStart: Instant) = active().deleteOverride(taskId, occurrenceStart) override fun createLocalList(name: String, color: Int): Long = active().createLocalList(name, color) override fun updateList(listId: Long, name: String, color: Int) = active().updateList(listId, name, color) override fun deleteList(listId: Long) = active().deleteList(listId) diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskMapper.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskMapper.kt index e5d810f..6dfed86 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskMapper.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskMapper.kt @@ -13,20 +13,23 @@ import kotlin.time.Instant /** Maps a provider row (via [ColumnReader]) to a domain model. Pure + testable. */ object TaskMapper { - fun task(r: ColumnReader): Task { + /** + * Maps an `instances` row, without its series' rule: the dmfs instances view + * (DB 22 and 23) masks `rrule`/`rdate` with `NULL`, so the row alone cannot + * tell a series from a one-off on tasks.org's bundled provider (DB 22), which + * also lacks `is_recurring`. [withRule] completes it with the rule read from + * the master's `tasks` row. An override names its master in + * `original_instance_id`. + */ + fun instance(r: ColumnReader): InstanceRow { fun instant(name: String): Instant? = r.getLong(name)?.let { Instant.fromEpochMilliseconds(it) } val rowId = r.getLong(Tasks.ID) ?: 0L - // Derived from the rule columns rather than the `is_recurring` column - // alone: that column only exists from OpenTasks 1.4.0 (DB 23) and is - // absent on tasks.org's bundled provider (DB 22), where reading it - // would silently report every recurring task as one-off — and route - // its edits onto the series anchor. - val recurring = r.getString(Tasks.RRULE) != null || - r.getString(Tasks.RDATE) != null || - r.getBoolean(Instances.IS_RECURRING) - return Task( + val masterId = r.getLong(Tasks.ORIGINAL_INSTANCE_ID) + val recurring = masterId != null || r.getBoolean(Instances.IS_RECURRING) + val anchor = occurrenceAnchor(r) + val task = Task( taskId = r.getLong(Instances.TASK_ID) ?: rowId, listId = r.getLong(Tasks.LIST_ID) ?: 0L, title = r.getString(Tasks.TITLE).orEmpty(), @@ -47,13 +50,31 @@ object TaskMapper { accountName = r.getString(Tasks.ACCOUNT_NAME), parentId = r.getLong(Tasks.PARENT_ID), isRecurring = recurring, - occurrenceStart = if (recurring) occurrenceAnchor(r) else null, + occurrenceStart = if (recurring) anchor else null, distanceFromCurrent = r.getInt(Instances.DISTANCE_FROM_CURRENT), created = instant(Tasks.CREATED), lastModified = instant(Tasks.LAST_MODIFIED), - seriesId = if (recurring) r.getLong(Instances.TASK_ID) ?: rowId else null, - recurrenceRule = r.getString(Tasks.RRULE), + seriesId = if (recurring) masterId ?: r.getLong(Instances.TASK_ID) ?: rowId else null, ) + return InstanceRow(task, ruleOwner = masterId ?: task.taskId, anchor = anchor) + } + + /** [row]'s task as an occurrence of the series [rule] belongs to, or as it is without one. */ + fun withRule(row: InstanceRow, rule: SeriesRule?): Task { + if (rule == null) return row.task + return row.task.copy( + isRecurring = true, + occurrenceStart = row.task.occurrenceStart ?: row.anchor, + seriesId = row.task.seriesId ?: row.ruleOwner, + recurrenceRule = rule.rrule, + ) + } + + /** The rule columns of a `tasks` row, or null when it does not recur. */ + fun seriesRule(r: ColumnReader): SeriesRule? { + val rrule = r.getString(Tasks.RRULE)?.ifBlank { null } + val rdate = r.getString(Tasks.RDATE)?.ifBlank { null } + return if (rrule == null && rdate == null) null else SeriesRule(rrule, rdate) } /** @@ -119,3 +140,12 @@ object TaskMapper { owner = r.getString(Lists.OWNER), ) } + +/** A series' recurrence as stored on its master row. */ +data class SeriesRule(val rrule: String?, val rdate: String?) + +/** + * An instance row's task before its series' rule is applied. [ruleOwner] is the + * task whose `tasks` row carries that rule: the master, for an override. + */ +data class InstanceRow(val task: Task, val ruleOwner: Long, val anchor: Instant?) diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapper.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapper.kt index 7413e12..a6f106e 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapper.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapper.kt @@ -17,6 +17,14 @@ private fun Instant.forAllDay(allDay: Boolean): Instant = Math.floorDiv(toEpochMilliseconds(), MILLIS_PER_DAY) * MILLIS_PER_DAY, ) +/** + * Whole seconds, as iCalendar stores times. The provider expands a series from + * second-precision instants, so a DTSTART carrying milliseconds never matches its + * own first occurrence: completing it detaches a copy but leaves the series where + * it was, and the same occurrence comes straight back. + */ +fun providerMillis(epochMillis: Long): Long = Math.floorDiv(epochMillis, 1000L) * 1000L + /** * Turns a [TaskForm] / mutation into a name→value map. Pure (no ContentValues), * so it unit-tests on the JVM; [AndroidTasksDataSource] converts the map to @@ -57,8 +65,8 @@ object TaskWriteMapper { // through DateTime.toAllDay(), which drops the time-of-day and resolves the // remaining date against UTC — so a local-midnight instant lands on the // previous day for anyone west of UTC. Pin all-day values to UTC midnight. - put(Tasks.DTSTART, form.start?.forAllDay(form.isAllDay)?.toEpochMilliseconds()) - put(Tasks.DUE, form.due?.forAllDay(form.isAllDay)?.toEpochMilliseconds()) + put(Tasks.DTSTART, form.start?.forAllDay(form.isAllDay)?.toEpochMilliseconds()?.let(::providerMillis)) + put(Tasks.DUE, form.due?.forAllDay(form.isAllDay)?.toEpochMilliseconds()?.let(::providerMillis)) // DUE and DURATION are mutually exclusive. The provider's Validating // processor evaluates the *merged* row (supplied values over the stored // ones), so writing DUE onto a task that already carries a DURATION throws 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 e516b93..f70b51a 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 @@ -124,13 +124,17 @@ interface TasksDataSource { * * Implementations fall back to [setCompleted] when the row turns out not to * be a series master — an override, or a plain task the caller happened to - * hand an anchor for — so the routing above cannot get this wrong. + * hand an anchor for. A series whose occurrence cannot be found fails with + * [TaskWriteFailedException] instead: the plain write would close all of it. */ fun setCompletedInstance(taskId: Long, occurrenceStart: Instant, completed: Boolean) /** Cancel (or restore) one occurrence of a series; the counterpart of [setCompletedInstance]. */ fun setCancelledInstance(taskId: Long, occurrenceStart: Instant, cancelled: Boolean) fun deleteTask(taskId: Long) + + /** Delete the override [taskId] standing for one occurrence, excluding that occurrence from its series. */ + fun deleteOverride(taskId: Long, occurrenceStart: Instant) = deleteTask(taskId) fun createLocalList(name: String, color: Int): Long /** Rename and recolour [listId]. */ 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 84b039c..8aa440a 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 @@ -241,7 +241,7 @@ class TasksRepositoryImpl @Inject constructor( RecurringScope.ThisOccurrence -> // An override row deletes itself into an EXDATE; a generated // occurrence has no row to delete. - if (current.taskId != seriesId) dataSource.deleteTask(current.taskId) + if (current.taskId != seriesId) dataSource.deleteOverride(current.taskId, occurrence) else dataSource.deleteOccurrence(seriesId, occurrence) RecurringScope.ThisAndFollowing -> dataSource.deleteFollowing(seriesId, occurrence) RecurringScope.AllOccurrences -> dataSource.deleteWithSubtasks(seriesId) diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/provider/ProviderWriter.kt b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/provider/ProviderWriter.kt index c1b6c70..835ab12 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/provider/ProviderWriter.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/provider/ProviderWriter.kt @@ -189,7 +189,8 @@ class ProviderWriter internal constructor( val current = database.tasks().entity(taskId)?.takeUnless { it.isDeleted } ?: return false requireWritableList(current.listId) // An override deletes itself into an EXDATE on its master. - if (current.masterId != null) store.deleteTask(taskId) else store.deleteWithSubtasks(taskId) + val occurrence = current.recurrenceId + if (current.masterId != null && occurrence != null) store.deleteOverride(taskId, occurrence) else store.deleteWithSubtasks(taskId) return true } diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/TaskMapperTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/TaskMapperTest.kt index 781b02a..4f9ee43 100644 --- a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/TaskMapperTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/TaskMapperTest.kt @@ -7,6 +7,7 @@ import de.jeanlucmakiola.agendula.data.tasks.TasksContract.Tasks import de.jeanlucmakiola.agendula.domain.Priority import de.jeanlucmakiola.agendula.domain.TaskStatus import org.junit.jupiter.api.Test +import kotlin.time.Instant class TaskMapperTest { @@ -34,7 +35,7 @@ class TaskMapperTest { ), ) - val task = TaskMapper.task(reader) + val task = mapTask(reader) assertThat(task.taskId).isEqualTo(7L) assertThat(task.listId).isEqualTo(3L) @@ -52,7 +53,7 @@ class TaskMapperTest { @Test fun `an occurrence is identified by its recurrence-id anchor`() { fun occurrence(columns: Map) = - TaskMapper.task(MapColumnReader(columns + (Tasks.RRULE to "FREQ=DAILY"))) + mapTask(MapColumnReader(columns), SeriesRule(rrule = "FREQ=DAILY", rdate = null)) // instance_original_time is the provider's own RECURRENCE-ID and wins. val anchored = occurrence( @@ -76,7 +77,7 @@ class TaskMapperTest { @Test fun `a non-recurring task has no occurrence anchor and keys by task id`() { - val task = TaskMapper.task( + val task = mapTask( MapColumnReader(mapOf(Tasks.ID to 4L, Instances.INSTANCE_START to 500L)), ) assertThat(task.occurrenceStart).isNull() @@ -84,33 +85,81 @@ class TaskMapperTest { } @Test - fun `recurrence is detected from rrule when is_recurring is absent`() { - // tasks.org's bundled provider is DB 22 and has no `is_recurring` column; - // reading it alone would report the series as one-off and send its edits - // to the master row, re-anchoring the whole thing. - val task = TaskMapper.task( - MapColumnReader(mapOf(Tasks.ID to 1L, Tasks.RRULE to "FREQ=WEEKLY;BYDAY=MO")), + fun `a series is detected from its master rule when the view masks it (#135)`() { + // tasks.org (DB 22): rule masked in the view, no `is_recurring`. + val task = mapTask( + MapColumnReader( + mapOf( + Tasks.ID to 90L, + Instances.TASK_ID to 9L, + Instances.INSTANCE_START to 1_000L, + Instances.INSTANCE_ORIGINAL_TIME to 1_000L, + ), + ), + SeriesRule(rrule = "FREQ=DAILY", rdate = null), ) assertThat(task.isRecurring).isTrue() + assertThat(task.seriesId).isEqualTo(9L) + assertThat(task.occurrenceStart).isEqualTo(Instant.fromEpochMilliseconds(1_000L)) + assertThat(task.isOccurrence).isTrue() + assertThat(task.recurrenceRule).isEqualTo("FREQ=DAILY") + } + + @Test + fun `the master rule fills the editor even where is_recurring exists`() { + // OpenTasks (DB 23) flags the row but masks the rule too. + val task = mapTask( + MapColumnReader(mapOf(Tasks.ID to 90L, Instances.TASK_ID to 9L, Instances.IS_RECURRING to 1)), + SeriesRule(rrule = "FREQ=WEEKLY;BYDAY=MO", rdate = null), + ) + assertThat(task.recurrenceRule).isEqualTo("FREQ=WEEKLY;BYDAY=MO") + } + + @Test + fun `a tasks row without a rule is no series`() { + assertThat(TaskMapper.seriesRule(MapColumnReader(mapOf(Tasks.ID to 1L, Tasks.RRULE to "")))).isNull() + assertThat(TaskMapper.seriesRule(MapColumnReader(mapOf(Tasks.ID to 1L, Tasks.RDATE to "20260720T090000Z")))) + .isEqualTo(SeriesRule(rrule = null, rdate = "20260720T090000Z")) } @Test fun `recurrence is detected from rdate alone`() { - val task = TaskMapper.task( - MapColumnReader(mapOf(Tasks.ID to 1L, Tasks.RDATE to "20260720T090000Z")), + val task = mapTask( + MapColumnReader(mapOf(Tasks.ID to 1L)), + SeriesRule(rrule = null, rdate = "20260720T090000Z"), ) assertThat(task.isRecurring).isTrue() } + @Test + fun `an override belongs to its master's series`() { + // An edited occurrence is its own task row, without a rule of its own. + val task = mapTask( + MapColumnReader( + mapOf( + Tasks.ID to 95L, + Instances.TASK_ID to 12L, + Tasks.ORIGINAL_INSTANCE_ID to 9L, + Instances.INSTANCE_ORIGINAL_TIME to 2_000L, + ), + ), + SeriesRule(rrule = "FREQ=DAILY", rdate = null), + ) + assertThat(task.taskId).isEqualTo(12L) + assertThat(task.seriesId).isEqualTo(9L) + assertThat(task.occurrenceStart).isEqualTo(Instant.fromEpochMilliseconds(2_000L)) + assertThat(task.recurrenceRule).isEqualTo("FREQ=DAILY") + } + @Test fun `a plain task is not recurring`() { - val task = TaskMapper.task(MapColumnReader(mapOf(Tasks.ID to 1L, Tasks.TITLE to "One-off"))) + val task = mapTask(MapColumnReader(mapOf(Tasks.ID to 1L, Tasks.TITLE to "One-off"))) assertThat(task.isRecurring).isFalse() } @Test fun `falls back to instance id when task_id missing, and list color when no task color`() { - val task = TaskMapper.task( + val task = mapTask( MapColumnReader(mapOf(Tasks.ID to 9L, Tasks.LIST_COLOR to 0x111111)), ) assertThat(task.taskId).isEqualTo(9L) @@ -140,3 +189,5 @@ class TaskMapperTest { assertThat(list.isLocal).isFalse() } } + +private fun mapTask(r: ColumnReader, rule: SeriesRule? = null) = TaskMapper.withRule(TaskMapper.instance(r), rule) diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapperTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapperTest.kt index 2ede4d0..4c19139 100644 --- a/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapperTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapperTest.kt @@ -102,14 +102,15 @@ class TaskWriteMapperTest { } @Test - fun `timed timestamps are written untouched`() { + fun `timed timestamps are kept to the second`() { + // The provider expands series in whole seconds. val at = Instant.fromEpochMilliseconds(1_784_412_345_678L) val values = TaskWriteMapper.taskValues( TaskForm(title = "Standup", listId = 1L, start = at, due = at), tzId = "Europe/Berlin", ) - assertThat(values[Tasks.DTSTART]).isEqualTo(1_784_412_345_678L) - assertThat(values[Tasks.DUE]).isEqualTo(1_784_412_345_678L) + assertThat(values[Tasks.DTSTART]).isEqualTo(1_784_412_345_000L) + assertThat(values[Tasks.DUE]).isEqualTo(1_784_412_345_000L) } @Test