From 90140112bb791465e45d77deb56082266c804b22 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 13 Aug 2026 17:23:31 +0200 Subject: [PATCH] fix(store): three defects the instrumented suite found on first run - forking an occurrence copied the master's alarm row id and hit the primary key; replaceForTask now clears it - a sub-second DTSTART made lib-recur emit the anchor and its truncated self, doubling a series' first occurrence; floor to the second, which is all RFC 5545 DATE-TIME carries - room-testing needs kotlinx-serialization 1.8+, but consistent resolution pinned androidTest to the app's 1.7.3 Tests: 52 pass on device. --- app/build.gradle.kts | 8 +++++ .../tasks/room/RoomTasksDataSourceTest.kt | 33 ++++++++++++++++--- .../agendula/data/tasks/room/TaskAlarmDao.kt | 8 +++-- .../domain/recurrence/RecurrenceExpander.kt | 12 +++++-- .../recurrence/RecurrenceExpanderTest.kt | 13 ++++++++ gradle/libs.versions.toml | 2 ++ 6 files changed, 68 insertions(+), 8 deletions(-) diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 732b234..2115c52 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -149,6 +149,14 @@ ksp { } dependencies { + // Not a dependency we use directly — lifecycle already drags it in at 1.7.3. + // AGP's consistent resolution then pins androidTest to the app classpath, and + // room-testing's MigrationTestHelper needs 1.8+ to deserialize the exported + // schema; on 1.7.3 it dies with an AbstractMethodError. Raise it in one place. + constraints { + implementation(libs.kotlinx.serialization.json) + } + implementation(libs.androidx.core.ktx) implementation(libs.androidx.appcompat) implementation(libs.androidx.lifecycle.runtime.ktx) 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 9731019..98ce04d 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 @@ -27,7 +27,8 @@ class RoomTasksDataSourceTest { private lateinit var source: RoomTasksDataSource private var listId = 0L - private val now get() = Clock.System.now() + /** Truncated to the store's granularity: instants are columns of epoch millis. */ + private val now get() = Instant.fromEpochMilliseconds(Clock.System.now().toEpochMilliseconds()) @Before fun setUp() { @@ -146,14 +147,38 @@ class RoomTasksDataSourceTest { fun anOverrideReplacesOnlyItsOwnOccurrence() { val id = source.insertTask(form(title = "Water the plants")) makeRecurring(id, now) - val before = source.tasks(TaskQuery(listId = listId)).filter { it.taskId == id } + // The list holds this series alone, so no filter is needed — and none can + // be written on taskId, since the override reports its own row id. + val before = source.tasks(TaskQuery(listId = listId)) val target = before.first { it.distanceFromCurrent == 1 } source.updateInstance(id, target.occurrenceStart!!, form(title = "Water them twice")) - val after = source.tasks(TaskQuery(listId = listId)).filter { it.taskId == id } + val after = source.tasks(TaskQuery(listId = listId)) assertThat(after).hasSize(before.size) - assertThat(after.filter { it.title == "Water them twice" }).hasSize(1) + val edited = after.single { it.title == "Water them twice" } + assertThat(edited.occurrenceStart).isEqualTo(target.occurrenceStart) + assertThat(after.filter { it.occurrenceStart == target.occurrenceStart }).hasSize(1) + } + + /** + * An edited occurrence addresses its own row, not the master's. That is what + * sends the *next* edit down `updateTask` rather than forking a second time: + * an override carries no rule, so it reads back as non-recurring. + */ + @Test + fun anEditedOccurrenceReportsTheOverridesOwnId() { + val id = source.insertTask(form(title = "Water the plants")) + makeRecurring(id, now) + val target = source.tasks(TaskQuery(listId = listId)).first { it.distanceFromCurrent == 1 } + + source.updateInstance(id, target.occurrenceStart!!, form(title = "Water them twice")) + + val edited = source.tasks(TaskQuery(listId = listId)).single { it.title == "Water them twice" } + val overrideId = db.tasks().override(id, target.occurrenceStart)!!.id + assertThat(edited.taskId).isEqualTo(overrideId) + assertThat(edited.taskId).isNotEqualTo(id) + assertThat(source.task(overrideId)!!.isRecurring).isFalse() } @Test 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 d77d15f..bd52710 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 @@ -22,10 +22,14 @@ interface TaskAlarmDao { @Query("DELETE FROM task_alarms WHERE task_id = :taskId") fun deleteForTask(taskId: Long): Int - /** Set the task's only reminder, or clear it with `null`. */ + /** + * Set the task's only reminder, or clear it with `null`. The row that lands is + * always a new one — the id is cleared so an alarm lifted off another task + * (forking an occurrence copies the master's) inserts instead of colliding. + */ @Transaction fun replaceForTask(taskId: Long, alarm: TaskAlarmEntity?) { deleteForTask(taskId) - alarm?.let { insert(it.copy(taskId = taskId)) } + alarm?.let { insert(it.copy(id = 0, taskId = taskId)) } } } diff --git a/app/src/main/java/de/jeanlucmakiola/agendula/domain/recurrence/RecurrenceExpander.kt b/app/src/main/java/de/jeanlucmakiola/agendula/domain/recurrence/RecurrenceExpander.kt index 1fa97e9..31e9a13 100644 --- a/app/src/main/java/de/jeanlucmakiola/agendula/domain/recurrence/RecurrenceExpander.kt +++ b/app/src/main/java/de/jeanlucmakiola/agendula/domain/recurrence/RecurrenceExpander.kt @@ -9,6 +9,7 @@ import java.time.ZoneId import java.util.TimeZone import kotlin.time.Instant +private const val MILLIS_PER_SECOND = 1000L private const val MILLIS_PER_DAY = 24L * 60 * 60 * 1000 /** The rule set of one task series, as stored. All strings are raw iCalendar values. */ @@ -117,10 +118,17 @@ object RecurrenceExpander { return TimeZone.getTimeZone(stored ?: floatingZone) } - /** All-day series are date-anchored: pin the anchor to UTC midnight, as it is stored. */ + /** + * All-day series are date-anchored: pin the anchor to UTC midnight, as it is + * stored. A timed one is floored to the second, because RFC 5545 DATE-TIME + * has no sub-second field — carrying millis in makes lib-recur emit the raw + * anchor *and* its truncated self, doubling the first occurrence, and mints + * `RECURRENCE-ID`s no other client could address. + */ private fun anchorMillis(spec: RecurrenceSpec): Long { val millis = spec.anchor.toEpochMilliseconds() - return if (!spec.isAllDay) millis else Math.floorDiv(millis, MILLIS_PER_DAY) * MILLIS_PER_DAY + val unit = if (spec.isAllDay) MILLIS_PER_DAY else MILLIS_PER_SECOND + return Math.floorDiv(millis, unit) * unit } private fun ruleOf(value: String, zone: TimeZone): RecurrenceRule? = runCatching { diff --git a/app/src/test/java/de/jeanlucmakiola/agendula/domain/recurrence/RecurrenceExpanderTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/domain/recurrence/RecurrenceExpanderTest.kt index 98baa1e..2c08ac6 100644 --- a/app/src/test/java/de/jeanlucmakiola/agendula/domain/recurrence/RecurrenceExpanderTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/agendula/domain/recurrence/RecurrenceExpanderTest.kt @@ -391,6 +391,19 @@ class RecurrenceExpanderTest { ).inOrder() } + @Test + fun `a sub-second anchor does not double the first occurrence`() { + // RFC 5545 DATE-TIME has second precision, but a task created from + // Clock.now() carries millis. Left un-floored, lib-recur emits the raw + // anchor *and* its truncated self, so the series starts twice. + val result = expand(spec(rrule = "FREQ=DAILY;COUNT=3", anchor = "2025-01-07T08:00:00.081Z")) + assertThat(result).containsExactly( + "2025-01-07T08:00:00Z", + "2025-01-08T08:00:00Z", + "2025-01-09T08:00:00Z", + ).inOrder() + } + @Test fun `a window that ends before the anchor yields nothing`() { val result = expand( diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index cd55c5d..c57fbae 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -15,6 +15,7 @@ material3 = "1.5.0-alpha21" datastore = "1.2.1" # Room — Agendula's own task store (docs/OWN-STORE.md). room = "2.8.4" +kotlinxSerialization = "1.8.1" # SAF directory writing for export/backup (DocumentFile). documentfile = "1.1.0" junit = "6.1.0" @@ -78,6 +79,7 @@ androidx-room-runtime = { group = "androidx.room", name = "room-runtime", versio androidx-room-ktx = { group = "androidx.room", name = "room-ktx", version.ref = "room" } androidx-room-compiler = { group = "androidx.room", name = "room-compiler", version.ref = "room" } androidx-room-testing = { group = "androidx.room", name = "room-testing", version.ref = "room" } +kotlinx-serialization-json = { group = "org.jetbrains.kotlinx", name = "kotlinx-serialization-json", version.ref = "kotlinxSerialization" } # DataStore androidx-datastore-preferences = { group = "androidx.datastore", name = "datastore-preferences", version.ref = "datastore" }