makiolaj/agendula
1
0
Fork 0
Code Issues Pull Requests Actions Releases 7 Activity

refactor(data): address occurrences by (taskId, occurrenceStart)

Browse Source 
Phase 0 of docs/OWN-STORE.md. Prepares the seam for the Room store while
the provider is still the store.

Task.id (the materialised instance row id) is gone; Task carries
occurrenceStart, its RECURRENCE-ID anchor, instead. updateInstance takes
(taskId, occurrenceStart, form) and AndroidTasksDataSource maps that back
to an instance row itself, so the provider path exercises the new
signature before Room exists.

Lazy-list keys move to Task.occurrenceKey. Two occurrences of one series
can appear in the same list once expansion is ours, and taskId alone
would collide there.

domain/Models.kt stops importing TasksContract — status, priority and
local-account constants now live in domain. StorageMode gains OWN as a
third value; LOCAL keeps meaning the dmfs provider until it is deleted.

Room 2.8.4 and room.schemaLocation added to the build.
This commit is contained in:
Jean-Luc Makiola
2026-08-13 15:55:24 +02:00
parent 8c3cbcf928
commit 11b20faf82
16 changed files with 221 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -20,6 +20,7 @@ import de.jeanlucmakiola.agendula.domain.export.ExportTask
import java.time.ZoneId
import javax.inject.Inject
import javax.inject.Singleton
import kotlin.time.Instant
/**
* The only class that knows about the ContentResolver, [TasksContract] and the
@@ -120,13 +121,39 @@ class AndroidTasksDataSource @Inject constructor(
if (rows == 0) throw TaskWriteFailedException("update task $taskId")
}
override fun updateInstance(instanceId: Long, form: TaskForm) {
override fun updateInstance(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)
val uri = TasksContract.instanceUri(authority(), instanceId)
val rows = resolver.update(uri, values.toContentValues(), null, null)
if (rows == 0) throw TaskWriteFailedException("update instance $instanceId")
}
/**
* The provider's instance row id 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.
*/
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 ->
val reader = CursorColumnReader(c)
while (c.moveToNext()) {
if (TaskMapper.occurrenceAnchor(reader) == occurrenceStart) {
return@use reader.getLong(Tasks.ID)
}
}
null
}
}
override fun setAlarm(taskId: Long, minutesBeforeDue: Int?) {
val uri = TasksContract.propertiesUri(authority())
// Replace rather than update: the provider's AlarmHandler re-validates the

3
app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/ProviderResolver.kt
View File

@@ -68,6 +68,9 @@ class ProviderResolver @Inject constructor(
/** The active provider, or `null` when [StorageMode.EXTERNAL] is chosen and none is installed. */
fun resolve(): TaskProvider? = when (storageMode ?: autoMode()) {
StorageMode.LOCAL -> own
// Room has no authority, no ContentResolver and nothing to permit. The
// provider entry stands in so ProviderStatus stays READY; nothing queries it.
StorageMode.OWN -> own
StorageMode.EXTERNAL -> resolveExternal()
}

11
app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/StorageMode.kt
View File

@@ -22,9 +22,20 @@ enum class StorageMode {
* Agendula's own bundled provider (the `:provider` module). Always available —
* it ships in the APK — and needs no permission grant at all, because
* same-uid access to your own provider skips the permission check entirely.
*
* On its way out: [OWN] replaces it once the Room store is the default, and
* this constant leaves with the `:provider` module. See `docs/OWN-STORE.md`.
*/
LOCAL,
/**
* Agendula's own Room database. Named as a third value rather than renaming
* [LOCAL] because both stores exist at once while the migration runs — a
* rename now would make `OWN` mean the dmfs provider for several phases and
* Room afterwards.
*/
OWN,
/**
* A tasks provider app already on the device (OpenTasks, tasks.org), synced by
* whatever that provider's engine is — DAVx5 and friends. This is the original

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

@@ -17,10 +17,17 @@ object TaskMapper {
fun instant(name: String): Instant? =
r.getLong(name)?.let { Instant.fromEpochMilliseconds(it) }
val instanceId = r.getLong(Tasks.ID) ?: 0L
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(
id = instanceId,
taskId = r.getLong(Instances.TASK_ID) ?: instanceId,
taskId = r.getLong(Instances.TASK_ID) ?: rowId,
listId = r.getLong(Tasks.LIST_ID) ?: 0L,
title = r.getString(Tasks.TITLE).orEmpty(),
description = r.getString(Tasks.DESCRIPTION),
@@ -39,20 +46,30 @@ object TaskMapper {
listName = r.getString(Tasks.LIST_NAME),
accountName = r.getString(Tasks.ACCOUNT_NAME),
parentId = r.getLong(Tasks.PARENT_ID),
// 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.
isRecurring = r.getString(Tasks.RRULE) != null ||
r.getString(Tasks.RDATE) != null ||
r.getBoolean(Instances.IS_RECURRING),
isRecurring = recurring,
occurrenceStart = if (recurring) occurrenceAnchor(r) else null,
distanceFromCurrent = r.getInt(Instances.DISTANCE_FROM_CURRENT),
created = instant(Tasks.CREATED),
lastModified = instant(Tasks.LAST_MODIFIED),
)
}
/**
* The occurrence's `RECURRENCE-ID` anchor.
*
* `instance_original_time` is the provider's own name for it and is set on
* every occurrence of a recurring task, so it is read first. It is absent on
* older provider schemas, where the fallbacks reconstruct the same value: a
* DTSTART-anchored series instantiates each occurrence at its start, and a
* series carrying only DUE anchors on the due date instead.
*/
fun occurrenceAnchor(r: ColumnReader): Instant? =
(
r.getLong(Instances.INSTANCE_ORIGINAL_TIME)
?: r.getLong(Instances.INSTANCE_START)
?: r.getLong(Instances.INSTANCE_DUE)
)?.let { Instant.fromEpochMilliseconds(it) }
/**
* Maps a row of the **`tasks` table** — a master task, not an occurrence.
*

7
app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksContract.kt
View File

@@ -99,6 +99,13 @@ object TasksContract {
const val INSTANCE_DUE_SORTING = "instance_due_sorting"
const val DISTANCE_FROM_CURRENT = "distance_from_current"
const val IS_RECURRING = "is_recurring"
/**
* The occurrence's `RECURRENCE-ID` — the time this occurrence was
* instantiated at, before any override moved it. Set on every occurrence
* of a recurring task, which is what makes it the occurrence's identity.
*/
const val INSTANCE_ORIGINAL_TIME = "instance_original_time"
}
/** The `properties` table — per-task side rows, discriminated by [Properties.MIMETYPE]. */

16
app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksDataSource.kt
View File

@@ -3,6 +3,7 @@ package de.jeanlucmakiola.agendula.data.tasks
import de.jeanlucmakiola.agendula.domain.Task
import de.jeanlucmakiola.agendula.domain.TaskForm
import de.jeanlucmakiola.agendula.domain.TaskList
import kotlin.time.Instant
/** What to fetch from the provider. Smart-list date logic is applied above this. */
data class TaskQuery(
@@ -25,12 +26,17 @@ interface TasksDataSource {
fun updateTask(taskId: Long, form: TaskForm)
/**
* Update a single occurrence of a recurring task, addressed by its *instance*
* row id. The provider forks an override task rather than moving the series
* anchor — which is what [updateTask] would do, since a recurring task's
* start/due are read from the instances view.
* Update a single occurrence of a recurring task, addressed by the task row and
* the occurrence's `RECURRENCE-ID` anchor ([Task.occurrenceStart]). The store
* forks an override rather than moving the series anchor — which is what
* [updateTask] would do, since a recurring task's start/due are the
* occurrence's resolved times.
*
* Addressing by `(taskId, occurrenceStart)` rather than by a materialised
* 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(instanceId: Long, form: TaskForm)
fun updateInstance(taskId: Long, occurrenceStart: Instant, form: TaskForm)
/**
* Set (or clear, with `null`) the per-task reminder lead, stored as an Alarm
* property row. The provider never fires it — [de.jeanlucmakiola.agendula

11
app/src/main/java/de/jeanlucmakiola/agendula/data/tasks/TasksRepositoryImpl.kt
View File

@@ -105,11 +105,12 @@ class TasksRepositoryImpl @Inject constructor(
// task's properties onto the new override row, so setting the alarm
// beforehand is what carries it across.
dataSource.setAlarm(taskId, form.reminderMinutesBeforeDue)
// A recurring task's start/due come from the instances view, so writing
// them back to tasks/<id> would re-anchor the whole series. Going through
// the occurrence lets the provider fork an override instead.
if (current != null && current.isRecurring) {
dataSource.updateInstance(current.id, form)
// 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.
val occurrence = current?.takeIf { it.isRecurring }?.occurrenceStart
if (occurrence != null) {
dataSource.updateInstance(taskId, occurrence, form)
} else {
dataSource.updateTask(taskId, form)
}

52
app/src/main/java/de/jeanlucmakiola/agendula/domain/Models.kt
View File

@@ -1,6 +1,5 @@
package de.jeanlucmakiola.agendula.domain
import de.jeanlucmakiola.agendula.data.tasks.TasksContract
import kotlin.time.Instant
/** A task list (the `tasklists` table). Lists group under their account. */
@@ -15,7 +14,8 @@ data class TaskList(
val owner: String?,
) {
/** A device-only list Agendula (or another app) created locally, not synced. */
val isLocal: Boolean get() = accountType == TasksContract.LOCAL_ACCOUNT_TYPE
val isLocal: Boolean
get() = accountType == LocalAccount.TYPE || accountType == LocalAccount.DMFS_TYPE
}
enum class TaskStatus { NEEDS_ACTION, IN_PROCESS, COMPLETED, CANCELLED }
@@ -24,11 +24,11 @@ enum class TaskStatus { NEEDS_ACTION, IN_PROCESS, COMPLETED, CANCELLED }
enum class Priority { NONE, LOW, MEDIUM, HIGH }
/**
* A task occurrence as read from the `instances` view. [id] is the instance row
* id; [taskId] is the underlying `tasks._id` and the stable target for edits.
* One occurrence of a task. [taskId] is the underlying task row and the stable
* target for edits and navigation; [occurrenceStart] distinguishes occurrences of
* the same series.
*/
data class Task(
val id: Long,
val taskId: Long,
val listId: Long,
val title: String,
@@ -49,12 +49,21 @@ data class Task(
val accountName: String?,
val parentId: Long?,
/**
* This row carries a recurrence rule, so [id] is one occurrence of a series
* and [start]/[due] are that occurrence's resolved times — *not* the master's
* anchor. Edits must go through the instances URI (see
* [de.jeanlucmakiola.agendula.data.tasks.TasksContract.instanceUri]).
* This row carries a recurrence rule, so it is one occurrence of a series and
* [start]/[due] are that occurrence's resolved times — *not* the master's
* anchor. Edits go through
* [de.jeanlucmakiola.agendula.data.tasks.TasksDataSource.updateInstance], which
* forks a `RECURRENCE-ID` override instead of re-anchoring the series.
*/
val isRecurring: Boolean,
/**
* This occurrence's `RECURRENCE-ID` anchor — what identifies it within its
* series — or `null` when the task does not recur. Together with [taskId] it
* is a stable, collision-free identity for an occurrence, which is what list
* keys and [de.jeanlucmakiola.agendula.data.tasks.TasksDataSource.updateInstance]
* address it by.
*/
val occurrenceStart: Instant? = null,
val distanceFromCurrent: Int?,
val created: Instant?,
val lastModified: Instant?,
@@ -71,6 +80,15 @@ data class Task(
val isSubtask: Boolean get() = parentId != null && parentId > 0
/** The task's own colour if set, else the list colour. */
val effectiveColor: Int get() = taskColor ?: listColor
/**
* Stable identity for a lazy-list key. Two occurrences of one series can show
* up in the same list, so [taskId] alone is not unique — and folding
* `(taskId, occurrenceStart)` into a Long could collide, which as a Compose
* key is a visible bug.
*/
val occurrenceKey: String
get() = if (occurrenceStart == null) "$taskId" else "$taskId@${occurrenceStart.toEpochMilliseconds()}"
}
/** Detail bundle: a task, its parent (if it's a subtask), and its direct children. */
@@ -91,22 +109,22 @@ fun priorityFromICal(value: Int?): Priority = when {
/** Representative iCalendar priority for a bucket (1 high, 5 medium, 9 low). */
fun Priority.toICal(): Int = when (this) {
Priority.NONE -> TasksContract.PRIORITY_NONE
Priority.NONE -> PRIORITY_NONE
Priority.HIGH -> 1
Priority.MEDIUM -> 5
Priority.LOW -> 9
}
fun statusFromInt(value: Int?): TaskStatus = when (value) {
TasksContract.STATUS_IN_PROCESS -> TaskStatus.IN_PROCESS
TasksContract.STATUS_COMPLETED -> TaskStatus.COMPLETED
TasksContract.STATUS_CANCELLED -> TaskStatus.CANCELLED
ICalStatus.IN_PROCESS -> TaskStatus.IN_PROCESS
ICalStatus.COMPLETED -> TaskStatus.COMPLETED
ICalStatus.CANCELLED -> TaskStatus.CANCELLED
else -> TaskStatus.NEEDS_ACTION
}
fun TaskStatus.toInt(): Int = when (this) {
TaskStatus.NEEDS_ACTION -> TasksContract.STATUS_NEEDS_ACTION
TaskStatus.IN_PROCESS -> TasksContract.STATUS_IN_PROCESS
TaskStatus.COMPLETED -> TasksContract.STATUS_COMPLETED
TaskStatus.CANCELLED -> TasksContract.STATUS_CANCELLED
TaskStatus.NEEDS_ACTION -> ICalStatus.NEEDS_ACTION
TaskStatus.IN_PROCESS -> ICalStatus.IN_PROCESS
TaskStatus.COMPLETED -> ICalStatus.COMPLETED
TaskStatus.CANCELLED -> ICalStatus.CANCELLED
}

30
app/src/main/java/de/jeanlucmakiola/agendula/domain/TaskConstants.kt Normal file
View File

@@ -0,0 +1,30 @@
package de.jeanlucmakiola.agendula.domain
/**
* iCalendar `STATUS` values for a `VTODO`, as integers.
*
* These live in `domain` rather than being read out of a provider contract: the
* numbering is Agendula's own storage encoding as much as it is dmfs's, and the
* domain layer must not depend on the data layer to map its own enums.
*/
object ICalStatus {
const val NEEDS_ACTION = 0
const val IN_PROCESS = 1
const val COMPLETED = 2
const val CANCELLED = 3
}
/** Priority 0 means "no priority"; 1 is highest, 9 lowest (RFC 5545 §3.8.1.9). */
const val PRIORITY_NONE = 0
/** How a device-only list identifies its (non-existent) account. */
object LocalAccount {
/** Shown as the section header above device-only lists. */
const val NAME = "Local"
/** What Agendula's own store reports for a list with no account. */
const val TYPE = "local"
/** What a dmfs-derived provider reports in External mode. */
const val DMFS_TYPE = "org.dmfs.account.LOCAL"
}

2
app/src/main/java/de/jeanlucmakiola/agendula/ui/lists/ListsScreen.kt
View File

@@ -419,7 +419,7 @@ private fun SearchResults(
}
} else {
LazyColumn(modifier = Modifier.fillMaxSize()) {
items(results, key = { it.id }) { task ->
items(results, key = { it.occurrenceKey }) { task ->
UpcomingRow(task = task, onClick = { onOpenTask(task.taskId) })
}
}

10
app/src/main/java/de/jeanlucmakiola/agendula/ui/tasklist/TaskListScreen.kt
View File

@@ -773,20 +773,20 @@ private fun SubtaskExpandButton(expanded: Boolean, onToggle: () -> Unit) {
/** A visual row in a flattened section run: a top-level task, or one of its subtasks. */
private sealed interface ListRow {
val key: Long
val key: String
data class Parent(val task: Task, val expandable: Boolean, val expanded: Boolean) : ListRow {
override val key: Long get() = task.taskId
override val key: String get() = task.occurrenceKey
}
data class Sub(val task: Task) : ListRow {
override val key: Long get() = task.taskId
override val key: String get() = task.occurrenceKey
}
/** The inline "add a subtask" row that closes an expanded group. */
data class AddSub(val parent: Task) : ListRow {
// Negative so it never collides with a real (positive) provider task id.
override val key: Long get() = -parent.taskId
// Prefixed so it never collides with the task row it belongs to.
override val key: String get() = "add-${parent.occurrenceKey}"
}
}

35
app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/TaskMapperTest.kt
View File

@@ -36,7 +36,6 @@ class TaskMapperTest {
val task = TaskMapper.task(reader)
assertThat(task.id).isEqualTo(42L)
assertThat(task.taskId).isEqualTo(7L)
assertThat(task.listId).isEqualTo(3L)
assertThat(task.title).isEqualTo("Buy milk")
@@ -50,6 +49,40 @@ class TaskMapperTest {
assertThat(task.isSubtask).isTrue()
}
@Test
fun `an occurrence is identified by its recurrence-id anchor`() {
fun occurrence(columns: Map<String, Any?>) =
TaskMapper.task(MapColumnReader(columns + (Tasks.RRULE to "FREQ=DAILY")))
// instance_original_time is the provider's own RECURRENCE-ID and wins.
val anchored = occurrence(
mapOf(
Instances.TASK_ID to 7L,
Instances.INSTANCE_ORIGINAL_TIME to 500L,
Instances.INSTANCE_START to 900L,
),
)
assertThat(anchored.occurrenceStart?.toEpochMilliseconds()).isEqualTo(500L)
assertThat(anchored.occurrenceKey).isEqualTo("7@500")
// Older provider schemas omit it; the occurrence's start reconstructs it.
val byStart = occurrence(mapOf(Instances.TASK_ID to 7L, Instances.INSTANCE_START to 900L))
assertThat(byStart.occurrenceStart?.toEpochMilliseconds()).isEqualTo(900L)
// A series carrying only DUE anchors on the due date instead.
val byDue = occurrence(mapOf(Instances.TASK_ID to 7L, Instances.INSTANCE_DUE to 1_200L))
assertThat(byDue.occurrenceStart?.toEpochMilliseconds()).isEqualTo(1_200L)
}
@Test
fun `a non-recurring task has no occurrence anchor and keys by task id`() {
val task = TaskMapper.task(
MapColumnReader(mapOf(Tasks.ID to 4L, Instances.INSTANCE_START to 500L)),
)
assertThat(task.occurrenceStart).isNull()
assertThat(task.occurrenceKey).isEqualTo("4")
}
@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;

4
app/src/test/java/de/jeanlucmakiola/agendula/domain/TaskSortingTest.kt
View File

@@ -17,7 +17,7 @@ class TaskSortingTest {
val sorted = listOf(completed, noDate, dueLater, dueSooner).sortedWith(TaskSorting.DEFAULT)
assertThat(sorted.map { it.id }).containsExactly(3L, 2L, 4L, 1L).inOrder()
assertThat(sorted.map { it.taskId }).containsExactly(3L, 2L, 4L, 1L).inOrder()
}
@Test
@@ -27,6 +27,6 @@ class TaskSortingTest {
val sorted = listOf(low, high).sortedWith(TaskSorting.DEFAULT)
assertThat(sorted.map { it.id }).containsExactly(2L, 1L).inOrder()
assertThat(sorted.map { it.taskId }).containsExactly(2L, 1L).inOrder()
}
}

2
app/src/test/java/de/jeanlucmakiola/agendula/domain/TestTasks.kt
View File

@@ -11,7 +11,6 @@ fun testTask(
priority: Priority = Priority.NONE,
due: Instant? = null,
): Task = Task(
id = id,
taskId = id,
listId = listId,
title = title,
@@ -32,6 +31,7 @@ fun testTask(
accountName = null,
parentId = null,
isRecurring = false,
occurrenceStart = null,
distanceFromCurrent = 0,
created = null,
lastModified = null,