Round series stored with milliseconds before writing an occurrence (#135)

Also only fetch rows that recur, and skip the provider test without permission.

CHANGELOG.md

@@ -7,6 +7,12 @@ All notable changes to this project are documented here. The format follows
 
 ## [Unreleased]
 
+### 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 (#135).
+
 ## [1.3.0] - 2026-11-02
 
 ### Added

app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/ExternalProviderRecurrenceTest.kt

@@ -0,0 +1,129 @@
+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.TaskForm
+import org.junit.After
+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.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 <app> org.tasks.permission.READ_TASKS` (and WRITE_TASKS).
+ */
+@RunWith(AndroidJUnit4::class)
+class ExternalProviderRecurrenceTest {
+
+    private val context = ApplicationProvider.getApplicationContext<Context>()
+    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() {
+        // As builds before whole-second writes left it.
+        val start = Clock.System.now() + 1.hours
+        val id = context.contentResolver.insert(
+            TasksContract.tasksUri(resolver.resolve()!!.authority),
+            ContentValues().apply {
+                put(Tasks.LIST_ID, listId)
+                put(Tasks.TITLE, "Old daily")
+                put(Tasks.DTSTART, start.toEpochMilliseconds() / 1000 * 1000 + 345)
+                put(Tasks.DUE, (start + 1.hours).toEpochMilliseconds() / 1000 * 1000 + 345)
+                put(Tasks.TZ, "UTC")
+                put(Tasks.RRULE, "FREQ=DAILY")
+            },
+        )!!.lastPathSegment!!.toLong()
+        val task = source.task(id)!!
+
+        source.setCompletedInstance(id, task.occurrenceStart!!, completed = true)
+
+        assertSeriesMovedOn(id, task.occurrenceStart)
+    }
+
+    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)
+    }
+}

app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/AndroidTasksDataSource.kt

@@ -111,19 +111,47 @@ class AndroidTasksDataSource @Inject constructor(
         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.
+        // `is_recurring`). 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)) }
+                val taskIds = buildSet {
+                    while (c.moveToNext()) reader.getLong(Instances.TASK_ID)?.let(::add)
+                }
+                // The view hides the rule (`null AS rrule`), so it is read from
+                // the tasks table and handed to the mapper.
+                val rules = seriesRules(taskIds)
+                c.moveToPosition(-1)
+                buildList {
+                    while (c.moveToNext()) add(TaskMapper.task(reader, reader.getLong(Instances.TASK_ID)?.let(rules::get)))
+                }
             } ?: emptyList()
         if (tasks.isEmpty()) return tasks
         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<Long>): Map<Long, SeriesRule> = taskIds.chunked(MAX_SQL_ARGS).flatMap { chunk ->
+        resolver.query(
+            TasksContract.tasksUri(authority()),
+            arrayOf(Tasks.ID, Tasks.RRULE, Tasks.RDATE),
+            "${Tasks.ID} IN (${chunk.joinToString(",") { "?" }}) AND (${Tasks.RRULE} IS NOT NULL OR ${Tasks.RDATE} IS NOT NULL)",
+            chunk.map(Long::toString).toTypedArray(),
+            null,
+        )?.use { c ->
+            val reader = CursorColumnReader(c)
+            buildList {
+                while (c.moveToNext()) {
+                    val id = reader.getLong(Tasks.ID) ?: continue
+                    TaskMapper.seriesRule(reader)?.let { add(id to it) }
+                }
+            }
+        } ?: emptyList()
+    }.toMap()
+
     override fun exportTasks(listId: Long): List<ExportTask> {
         // projection = null for the same reason queryInstances uses it: the tasks
         // table's shape varies across provider versions, and the by-name mapper
@@ -206,7 +234,7 @@ class AndroidTasksDataSource @Inject constructor(
             now == null -> null
             anchor == null || was == null -> now.toEpochMilliseconds()
             else -> anchor + (now - was).inWholeMilliseconds
-        }
+        }?.let(::providerMillis)
         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))
@@ -357,12 +385,15 @@ class AndroidTasksDataSource @Inject constructor(
      * WHERE clause naming it would throw instead of falling back.
      */
     private fun instanceIdFor(taskId: Long, occurrenceStart: Instant): Long? {
+        roundAnchors(taskId)
         val uri = TasksContract.instancesUri(authority())
         val selection = "${Instances.TASK_ID} = ?"
+        val wanted = providerMillis(occurrenceStart.toEpochMilliseconds())
         return resolver.query(uri, null, selection, arrayOf(taskId.toString()), null)?.use { c ->
             val reader = CursorColumnReader(c)
             while (c.moveToNext()) {
-                if (TaskMapper.occurrenceAnchor(reader) == occurrenceStart) {
+                val anchor = TaskMapper.occurrenceAnchor(reader) ?: continue
+                if (providerMillis(anchor.toEpochMilliseconds()) == wanted) {
                     return@use reader.getLong(Tasks.ID)
                 }
             }
@@ -370,6 +401,27 @@ class AndroidTasksDataSource @Inject constructor(
         }
     }
 
+    /**
+     * Rounds a task's DTSTART/DUE down to whole seconds if a build before
+     * [providerMillis] stored them with milliseconds — otherwise the provider
+     * cannot move the series past an occurrence written through its instance.
+     */
+    private fun roundAnchors(taskId: Long) {
+        val uri = taskUri(authority(), taskId)
+        val values = resolver.query(uri, arrayOf(Tasks.DTSTART, Tasks.DUE), null, null, null)?.use { c ->
+            if (!c.moveToFirst()) return@use null
+            val r = CursorColumnReader(c)
+            buildMap<String, Any?> {
+                for (column in listOf(Tasks.DTSTART, Tasks.DUE)) {
+                    val millis = r.getLong(column) ?: continue
+                    if (providerMillis(millis) != millis) put(column, providerMillis(millis))
+                }
+            }
+        }
+        if (values.isNullOrEmpty()) return
+        if (resolver.update(uri, values.toContentValues(), null, null) == 0) throw TaskWriteFailedException("round task $taskId")
+    }
+
     override fun setReminders(taskId: Long, reminders: List<TaskReminder>) {
         val uri = TasksContract.propertiesUri(authority())
         resolver.delete(

app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TaskMapper.kt

@@ -13,17 +13,19 @@ 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. [rule] is the series' rule set as stored on its
+     * `tasks` row: 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`.
+     */
+    fun task(r: ColumnReader, rule: SeriesRule? = null): Task {
         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 ||
+        val recurring = rule != null ||
+            r.getString(Tasks.RRULE) != null ||
             r.getString(Tasks.RDATE) != null ||
             r.getBoolean(Instances.IS_RECURRING)
         return Task(
@@ -52,10 +54,17 @@ object TaskMapper {
             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),
+            recurrenceRule = rule?.rrule ?: r.getString(Tasks.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)
+    }
+
     /**
      * The occurrence's `RECURRENCE-ID` anchor.
      *
@@ -119,3 +128,6 @@ 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?)

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

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 {
 
@@ -85,15 +86,52 @@ 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")),
         )
         assertThat(task.isRecurring).isTrue()
     }
 
+    @Test
+    fun `a series is detected from its master rule when the view masks it (#135)`() {
+        // tasks.org's bundled provider (DB 22): `null AS rrule` in the instances
+        // view and no `is_recurring`, so only the tasks row knows it recurs.
+        val task = TaskMapper.task(
+            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 all the same; without
+        // it the editor opens empty and saving the series drops the RRULE.
+        val task = TaskMapper.task(
+            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(

app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapperTest.kt

@@ -102,14 +102,17 @@ class TaskWriteMapperTest {
     }
 
     @Test
-    fun `timed timestamps are written untouched`() {
+    fun `timed timestamps are kept to the second`() {
+        // A default start of "now" carries milliseconds; the provider expands a
+        // series in whole seconds, and a DTSTART between them stops completing an
+        // occurrence from moving the series on.
         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