From dd9f652f022db9278fd5701aae89858d93f96f94 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Fri, 6 Nov 2026 13:02:42 +0100 Subject: [PATCH] Keep provider dates to whole seconds, test recurrence against a real provider (#135) A DTSTART with milliseconds never matches the provider's first occurrence, so completing it left the series in place. --- .../tasks/ExternalProviderRecurrenceTest.kt | 100 ++++++++++++++++++ .../data/tasks/AndroidTasksDataSource.kt | 2 +- .../agendula/data/tasks/TaskWriteMapper.kt | 12 ++- .../data/tasks/TaskWriteMapperTest.kt | 9 +- 4 files changed, 117 insertions(+), 6 deletions(-) create mode 100644 app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/ExternalProviderRecurrenceTest.kt 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..98bf1f5 --- /dev/null +++ b/app/src/androidTest/java/de/jeanlucmakiola/agendula/data/tasks/ExternalProviderRecurrenceTest.kt @@ -0,0 +1,100 @@ +package de.jeanlucmakiola.agendula.data.tasks + +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 + +/** + * 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() { + assumeTrue("no external tasks provider", resolver.resolve() != null) + 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)).single { it.taskId == id } + 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) + + assertThat(storedRule(id)).isNotNull() + val next = source.tasks(TaskQuery(listId = listId)).filter { it.seriesId == id } + assertThat(next).isNotEmpty() + assertThat(next.single().occurrenceStart).isGreaterThan(task.occurrenceStart) + } +} 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 711161f..5290f56 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 @@ -234,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)) 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/test/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapperTest.kt b/app/src/test/java/de/jeanlucmakiola/agendula/data/tasks/TaskWriteMapperTest.kt index 2ede4d0..1d969a4 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,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