Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import org.gradle.kotlin.dsl.configure
* tag matches this string — so the version is a reviewed change in a commit, not a property of
* whoever typed the tag. Bump this, commit, then tag the commit.
*/
private const val VERSION_NAME = "0.4.2"
private const val VERSION_NAME = "0.4.3"

/**
* versionCode, computed from versionName rather than stored beside it.
Expand Down
69 changes: 47 additions & 22 deletions core/data/src/main/kotlin/de/plmail/core/data/CalendarRepository.kt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import de.plmail.core.database.PlMailDatabase
import de.plmail.core.database.StoreKey
import de.plmail.core.datastore.CredentialStore
import de.plmail.jmap.calendar.Calendar
import de.plmail.jmap.calendar.CalendarDialect
import de.plmail.jmap.calendar.CalendarEvent
import de.plmail.jmap.calendar.CalendarEventFilter
import de.plmail.jmap.calendar.RecurrenceRule
Expand All @@ -28,7 +29,6 @@ import de.plmail.jmap.methods.SetError
import de.plmail.jmap.protocol.AccountId
import de.plmail.jmap.protocol.CalendarEventId
import de.plmail.jmap.protocol.CalendarId
import de.plmail.jmap.protocol.Capability
import de.plmail.jmap.protocol.JmapError
import de.plmail.jmap.protocol.MaterialisedHorizon
import de.plmail.jmap.protocol.RequestBuilder
Expand Down Expand Up @@ -229,7 +229,7 @@ data class EventDraft(
val description: String? = null,
val status: String = NewCalendarEvent.STATUS_CONFIRMED,
/**
* **Create only.** An update deliberately never sends `recurrenceRules`.
* **Create only.** An update deliberately never sends `recurrenceRule`.
*
* The cache stores *whether* an event recurs but not the rule it recurs by — no screen in this
* app draws a rule — so an update built from a draft would send one derived from nothing, and
Expand Down Expand Up @@ -282,8 +282,8 @@ interface EventEditing {
*
* An occurrence id is **opaque** — `42_20260304T090000Z` is the series id and the occurrence's
* original start, and reading either half back out is the same client-side expansion by a quieter
* route. Which series an occurrence belongs to is its object's `seriesId`, and where it goes is its
* object's `start`.
* route. Which series an occurrence belongs to is its object's `baseEventId`, and where it goes is
* its object's `start`.
*/
@Singleton
class CalendarRepository
Expand Down Expand Up @@ -539,7 +539,7 @@ constructor(

place(
occurrence = occurrence,
// Read from `seriesId`, never from the occurrence id. The
// Read from `baseEventId`, never from the occurrence id. The
// two agree on this server and the id is documented opaque,
// which is exactly the combination that lets a shortcut go
// unnoticed until an id shape changes.
Expand Down Expand Up @@ -634,7 +634,7 @@ constructor(
* **Both are needed, and the second is not the first with extra rows.** `expandRecurrences`
* answers one id per occurrence, and the object behind an occurrence id is the series with that
* occurrence's override merged into it — its `start` is that Tuesday's, its `title` may be that
* Tuesday's, and `recurrenceRules` comes back null. Those are exactly the properties an editor
* Tuesday's, and `recurrenceRule` comes back null. Those are exactly the properties an editor
* must *not* see: a form seeded from one of them would send that occurrence's date as the
* series' start and drag a whole standup onto the day somebody happened to be looking at. So
* the collapsed query answers the series, the expanded one answers the days, and the cache
Expand Down Expand Up @@ -758,7 +758,10 @@ constructor(
val q =
batch.add(query(accountId, after, before, paging.position, pageSize))

q to batch.add(hydrate(accountId, q.reference("/ids"), CACHED_PROPERTIES))
q to
batch.add(
hydrate(session, accountId, q.reference("/ids"), CACHED_PROPERTIES)
)
}

val occurrenceHandles =
Expand All @@ -780,6 +783,7 @@ constructor(
q to
batch.add(
hydrate(
session,
accountId,
q.reference("/ids"),
CalendarEventGet.OCCURRENCE_PROPERTIES,
Expand Down Expand Up @@ -910,6 +914,7 @@ constructor(
mapOf(
CREATION_ID to stored.toNewEvent(CalendarId(calendar.calendarId))
),
dialect = session.dialect,
)
)

Expand All @@ -926,6 +931,13 @@ constructor(
"The server created no event and gave no reason.",
)

// Read from the answer where there is one: a rule the server
// cannot convert is stored verbatim and expands to one
// occurrence, so a create carrying a recurrence can come back
// not recurring. Only a server that answers neither name
// leaves the rule that was sent as all there is to go on.
val recurs = created.isRecurring ?: (stored.recurrenceRule != null)

val eventKey = StoreKey.objectKey(calendar.accountKey, created.id.value)
val row =
stored.toEntity(
Expand All @@ -943,11 +955,7 @@ constructor(
// `uid` therefore keeps the ordinary id-only reconcile until
// the first refresh fills the uid in.
eventUid = created.uid?.takeIf { it.isNotBlank() },
// Read from the answer, never inferred from the rule that
// was sent: a rule the server cannot convert is stored
// verbatim and expands to one occurrence, so a create
// carrying a recurrence can come back not recurring.
isRecurring = created.isRecurring,
isRecurring = recurs,
sequence = created.sequence,
)

Expand All @@ -958,12 +966,12 @@ constructor(
// appear without a round trip. A recurring one cannot be: where
// its occurrences land is the server's answer, and the refresh
// below is how that is asked.
if (!created.isRecurring) {
if (!recurs) {
database.calendarEvents().upsertOccurrences(row.placeFromItself(zone))
}
}

if (created.isRecurring) refreshLastWindow()
if (recurs) refreshLastWindow()

CalendarWriteResult.Applied(eventKey, created.id)
}
Expand Down Expand Up @@ -1043,6 +1051,7 @@ constructor(
CalendarEventSet(
accountId = accountId,
update = mapOf(CalendarEventId(existing.eventId) to stored.toPatch()),
dialect = session.dialect,
)
)

Expand Down Expand Up @@ -1102,6 +1111,7 @@ constructor(
CalendarEventSet(
accountId = accountId,
destroy = listOf(CalendarEventId(existing.eventId)),
dialect = session.dialect,
)
)

Expand Down Expand Up @@ -1190,9 +1200,9 @@ constructor(
* Which days those are is read off the object the server answered for this occurrence id and
* from nothing else: its `start` is already resolved against whatever override moved it, so
* there is no rule to expand, no override map to interpret, and no timestamp to lift out of an
* id. [series] is the row it hangs off — [CalendarEvent.seriesId]'s object, never a prefix of
* the occurrence id — and an occurrence whose series did not come back is dropped rather than
* orphaned, because nothing joins to a series row that is not there.
* id. [series] is the row it hangs off — [CalendarEvent.baseEventId]'s object, never a prefix
* of the occurrence id — and an occurrence whose series did not come back is dropped rather
* than orphaned, because nothing joins to a series row that is not there.
*
* The day an occurrence lands on is the reader's: an event that names a zone is an instant, and
* is placed where that instant falls on the device's clock — see [toDeviceClock]. An all-day or
Expand Down Expand Up @@ -1296,28 +1306,41 @@ constructor(
expandRecurrences = expandRecurrences,
)

private fun hydrate(accountId: AccountId, ids: ResultReference, properties: List<String>) =
private fun hydrate(
session: Session,
accountId: AccountId,
ids: ResultReference,
properties: List<String>,
) =
CalendarEventGet.byReference(
accountId = accountId,
queryReference = ids,
properties = properties,
dialect = session.dialect,
)

/**
* A request declaring the calendars capability, and nothing else.
*
* `USING_CALENDARS` behaves **oppositely** to the push URN: push is advertised in the session
* The calendars URN behaves **oppositely** to the push URN: push is advertised in the session
* and fails the whole request if it is declared, calendars is advertised and refuses the call
* if it is not. Mail is deliberately absent — a calendar request needs nothing from it, and
* declaring a capability a request does not use is how a client breaks on an instance where
* that one is switched off.
*
* Which calendars URN is the session's to say — see [Session.calendarDialect]. Every caller has
* already found a calendar account, so the session names one.
*/
private fun request(session: Session): RequestBuilder =
RequestBuilder(
using = Capability.USING_CALENDARS,
using = session.dialect.using,
maxCallsInRequest = session.core.maxCallsInRequest,
)

/** See [request]: only ever read once the session has named a calendar account. */
private val Session.dialect: CalendarDialect
get() = calendarDialect ?: CalendarDialect.IETF

private companion object {
const val CREATION_ID = "c1"

Expand Down Expand Up @@ -1347,15 +1370,17 @@ constructor(
listOf(
"id",
"uid",
"calendarId",
"calendarIds",
"title",
"description",
"start",
"duration",
"timeZone",
"showWithoutTime",
"status",
"isRecurring",
// Asked for with whether it recurs beside it, which the
// server answers better; see CalendarEvent.isRecurring.
"recurrenceRule",
"sequence",
"locations",
"recurrenceOverrides",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package de.plmail.core.data
import app.cash.turbine.test
import de.plmail.core.database.PlMailDatabase
import de.plmail.core.database.StoreKey
import de.plmail.jmap.calendar.CalendarDialect
import de.plmail.jmap.client.HttpRequest
import de.plmail.jmap.protocol.CalendarEventId
import de.plmail.jmap.testing.RecordingTransport
Expand Down Expand Up @@ -457,7 +458,7 @@ class CalendarRepositoryTest {
* of the same, has nothing to parse here and draws nothing.
*
* The dates asserted are therefore only reachable through the object's own `start`, and the
* series row is only reachable through its `seriesId`.
* series row is only reachable through its `baseEventId`.
*/
@Test
fun `occurrences are placed from their objects, never from their ids`() = runTest {
Expand All @@ -474,17 +475,71 @@ class CalendarRepositoryTest {
assertEquals(
"10867",
database.calendarEvents().byUid(standupKey)?.eventId,
"and the series row is keyed by seriesId, which is the id a write is addressed to",
"and the series row is keyed by baseEventId, which is the id a write is addressed to",
)
}

/**
* A server from before the IETF calendars URN is read just the same.
*
* It advertises only plMail's vendor URN and answers in plMail's earlier shape — `calendarId`,
* `recurrenceRules`, `seriesId`, `isRecurring` — and refuses a request that declares the IETF
* one. Every plMail released before that URN arrived is such a server, and a phone updated
* ahead of its server must not lose its calendar to the order the two were upgraded in.
*/
@Test
fun `a server that speaks only the vendor calendars URN is still read`() = runTest {
val server = FakeCalendarServer(events = seededWeek(), dialect = CalendarDialect.PLMAIL)
val repository = calendarStack(database, calendarTransport(server))

val result = repository.refresh(week) as CalendarRefresh.Refreshed

assertEquals(1, result.requests)
assertEquals(
listOf("2026-08-03", "2026-08-05", "2026-08-07"),
database.calendarEvents().occurrencesOf(standupKey).map { it.date },
)
assertEquals(true, database.calendarEvents().byUid(standupKey)?.isRecurring)
assertEquals(testCalendarKey, database.calendarEvents().byUid(standupKey)?.calendarKey)
}

@Test
fun `a write to a vendor-only server is sent in its names`() = runTest {
val server = FakeCalendarServer(events = seededWeek(), dialect = CalendarDialect.PLMAIL)
var sent: JsonObject? = null

server.onSet = { arguments ->
sent = arguments["create"]!!.jsonObject.getValue("c1").jsonObject
createdEvent(id = "10999", dialect = CalendarDialect.PLMAIL)
}

val repository = calendarStack(database, calendarTransport(server))

repository.refresh(week)

val result =
repository.create(
calendarKey = testCalendarKey,
draft =
EventDraft(
title = "Zahnarzt",
start = LocalDateTime.parse("2026-08-06T09:00:00"),
duration = Duration.ofMinutes(30),
),
)

assertTrue(result is CalendarWriteResult.Applied, "was $result")
assertEquals(TEST_CALENDAR_ID, sent?.get("calendarId")?.jsonPrimitive?.content)
assertFalse(sent!!.containsKey("calendarIds"))
}

/**
* An edit of a recurring series goes to the series id, never to an occurrence's.
*
* `CalendarEvent/set` refuses an occurrence id by name — `invalidArguments`, pointing at
* `seriesId` — so a cache that had filed a standup under `o2` would have an editor that cannot
* save. The occurrence ids never reach a row at all, which is what this asserts from the other
* end: the id on the wire is the plain one.
* `baseEventId` — so a cache that had filed a standup under `o2` would have an editor that
* cannot save. The occurrence ids never reach a row at all, which is what this asserts from the
* other end: the id on the wire is the plain one.
*/
@Test
fun `an edit of a recurring series is addressed to the series id`() = runTest {
Expand Down Expand Up @@ -769,7 +824,10 @@ class CalendarRepositoryTest {
*/
@Test
fun `a window past the horizon is clamped and reported as possibly incomplete`() = runTest {
val server = FakeCalendarServer(events = seededWeek())
// Vendor-only, because only that URN publishes the horizon asserted on
// below; the IETF one has nothing to say it with, and the clamp is the
// client's own line either way.
val server = FakeCalendarServer(events = seededWeek(), dialect = CalendarDialect.PLMAIL)
val repository = calendarStack(database, calendarTransport(server))

val far = CalendarWindow(LocalDate.of(2027, 7, 1), LocalDate.of(2027, 9, 1))
Expand Down Expand Up @@ -1557,7 +1615,10 @@ class CalendarRepositoryTest {
val days = (0 until 120).map { start.plusDays(it.toLong()) }.toSet()

val server =
// Vendor-only: a hundred a page is that URN's maxEventsInGet. Under
// the IETF one a get takes core's 500, and these would fit on one.
FakeCalendarServer(
dialect = CalendarDialect.PLMAIL,
events =
mutableListOf(
recurring(
Expand All @@ -1566,7 +1627,7 @@ class CalendarRepositoryTest {
start = "2026-08-03T09:00:00",
days = days,
)
)
),
)

val repository = calendarStack(database, calendarTransport(server))
Expand All @@ -1588,8 +1649,9 @@ class CalendarRepositoryTest {
assertEquals(days.map { it.toString() }.sorted(), cached.map { it.date }.sorted())
}

/** Under the IETF URN a get takes core's `maxObjectsInGet`, so the same quarter is one page. */
@Test
fun `the second page asks from where the first stopped, and only for what is left`() = runTest {
fun `under the IETF URN a get is held to core's limit rather than a hundred`() = runTest {
val start = LocalDate.of(2026, 8, 3)
val days = (0 until 120).map { start.plusDays(it.toLong()) }.toSet()

Expand All @@ -1605,6 +1667,36 @@ class CalendarRepositoryTest {
)
)
)
val repository = calendarStack(database, calendarTransport(server))

val result =
repository.refresh(CalendarWindow(start, start.plusDays(120)))
as CalendarRefresh.Refreshed

assertEquals(1, result.requests)
assertEquals(120, result.occurrences)
}

@Test
fun `the second page asks from where the first stopped, and only for what is left`() = runTest {
val start = LocalDate.of(2026, 8, 3)
val days = (0 until 120).map { start.plusDays(it.toLong()) }.toSet()

val server =
// Vendor-only: a hundred a page is that URN's maxEventsInGet. Under
// the IETF one a get takes core's 500, and these would fit on one.
FakeCalendarServer(
dialect = CalendarDialect.PLMAIL,
events =
mutableListOf(
recurring(
id = "10867",
title = "Standup",
start = "2026-08-03T09:00:00",
days = days,
)
),
)

val repository = calendarStack(database, calendarTransport(server))

Expand Down
Loading
Loading