diff --git a/src/device-sync/__tests__/engine/real-planners.test.ts b/src/device-sync/__tests__/engine/real-planners.test.ts index 7c3c00c6..d44e3e06 100644 --- a/src/device-sync/__tests__/engine/real-planners.test.ts +++ b/src/device-sync/__tests__/engine/real-planners.test.ts @@ -3,9 +3,10 @@ // the planners' own tests; these check what the engine guarantees. import { describe, expect, it } from 'vitest'; -import { Data, MimeType } from '../../android-columns'; +import { Attendees, Data, Events, MimeType } from '../../android-columns'; import { calendarPlanner } from '../../calendar/planner'; import { contactsPlanner } from '../../contacts/planner'; +import type { Row } from '../../types'; import { addDeviceContact, addServerCards, @@ -21,6 +22,36 @@ function real(options: Parameters[0] = {}): Harness { return h; } +/** The event columns Etar copies into the row it inserts when it moves an event or ends its recurrence. */ +const ETAR_COPY = [ + Events.TITLE, Events.DESCRIPTION, Events.EVENT_LOCATION, Events.STATUS, Events.AVAILABILITY, Events.ACCESS_LEVEL, + Events.EVENT_COLOR, Events.DTSTART, Events.DTEND, Events.DURATION, Events.EVENT_TIMEZONE, Events.ALL_DAY, Events.RRULE, +]; + +/** Etar's move or "series to single event": the row is deleted and a copy (with its attendees) inserted. */ +function etarReinsert(h: Harness, old: Row, calendarRowId: number, changes: Row = {}): number { + const values: Row = {}; + for (const c of ETAR_COPY) if (old[c] !== undefined) values[c] = old[c]; + const attendees = h.device.rows('attendees') + .filter((a) => Number(a.event_id) === Number(old._id)) + .map((a) => ({ + [Attendees.ATTENDEE_EMAIL]: a[Attendees.ATTENDEE_EMAIL], + [Attendees.ATTENDEE_NAME]: a[Attendees.ATTENDEE_NAME], + [Attendees.ATTENDEE_RELATIONSHIP]: a[Attendees.ATTENDEE_RELATIONSHIP], + [Attendees.ATTENDEE_TYPE]: a[Attendees.ATTENDEE_TYPE], + [Attendees.ATTENDEE_STATUS]: a[Attendees.ATTENDEE_STATUS], + })); + h.device.user.deleteEvent(Number(old._id)); + return h.device.user.insertEvent(calendarRowId, { ...values, ...changes }, { attendees }); +} + +/** Masters (no ORIGINAL_*) that share a `_SYNC_ID`: a split the next run would resolve by destroy + create. */ +function sharedIdentities(h: Harness): string[] { + const masters = h.events().filter((e) => e[Events.ORIGINAL_ID] === null && e[Events.ORIGINAL_SYNC_ID] === null && e._sync_id); + const ids = masters.map((e) => String(e._sync_id)); + return ids.filter((id, i) => ids.indexOf(id) !== i); +} + /** A device edit of the display name only, as AOSP Contacts makes it (the provider fills the components). */ function retypeName(h: Harness, rawContactId: number, name: string): void { const row = h.nameDataRow(rawContactId)!; @@ -192,6 +223,153 @@ describe('device sync engine with the real planners', () => { expect(h.events().every((e) => Number(e.dirty ?? 0) === 0)).toBe(true); }); + it("uploads Etar's move to another calendar as a patch of the same event, keeping what the device can't show", async () => { + const h = real(); + const work = h.server.addCalendar('a', { name: 'Work' }); + const lunch = h.server.addEvent('a', { + uid: 'lunch-uid', + title: 'Lunch', + start: '2026-10-06T12:00:00', + duration: 'PT1H', + timeZone: 'Europe/Berlin', + keywords: { important: true }, + virtualLocations: { v1: { '@type': 'VirtualLocation', uri: 'https://meet.example.com/lunch' } }, + organizerCalendarAddress: 'mailto:alice@example.com', + participants: { + me: { '@type': 'Participant', calendarAddress: 'mailto:alice@example.com', roles: { owner: true, attendee: true }, participationStatus: 'accepted' }, + bob: { '@type': 'Participant', calendarAddress: 'mailto:bob@example.com', name: 'Bob', roles: { attendee: true }, participationStatus: 'accepted' }, + }, + calendarIds: { [h.calendar]: true }, + }); + await h.run(CALENDAR_AUTHORITY); + const old = h.events().find((e) => e._sync_id === `a/${lunch}`)!; + const workRow = Number(h.device.rows('calendars').find((c) => c._sync_id === `a/${work}`)!._id); + + const moved = etarReinsert(h, old, workRow); + const report = await h.run(CALENDAR_AUTHORITY); + + expect(report).toMatchObject({ outcome: 'ok', itemErrors: [], stats: { uploaded: { created: 0, updated: 1, deleted: 0 } } }); + expect(h.server.all('CalendarEvent', 'a')).toHaveLength(1); + const event = h.server.get('CalendarEvent', 'a', lunch)!; + expect(event.calendarIds).toEqual({ [work]: true }); + expect(event).toMatchObject({ + uid: 'lunch-uid', + keywords: { important: true }, + virtualLocations: { v1: { uri: 'https://meet.example.com/lunch' } }, + participants: { bob: { participationStatus: 'accepted' } }, + }); + // A move tells nobody. + expect(h.server.scheduling).toEqual([]); + expect(h.events()).toEqual([expect.objectContaining({ _id: moved, _sync_id: `a/${lunch}`, uid2445: 'lunch-uid', calendar_id: workRow, dirty: 0 })]); + + const before = h.batches.log.length; + expect((await h.run(CALENDAR_AUTHORITY)).outcome).toBe('ok'); + expect(rowWrites(h.batches.log.slice(before))).toEqual([]); + }); + + it('uploads a series Etar turned into a single event as the removal of its rule', async () => { + const h = real(); + const series = h.server.addEvent('a', { + uid: 'series-uid', + title: 'Standup', + start: '2026-09-28T09:00:00', + duration: 'PT15M', + timeZone: 'Europe/Berlin', + keywords: { team: true }, + recurrenceRule: { '@type': 'RecurrenceRule', frequency: 'daily' }, + calendarIds: { [h.calendar]: true }, + }); + await h.run(CALENDAR_AUTHORITY); + const old = h.events().find((e) => e._sync_id === `a/${series}`)!; + + const single = etarReinsert(h, old, Number(old.calendar_id), { + [Events.RRULE]: null, + [Events.DURATION]: null, + [Events.DTEND]: Number(old[Events.DTSTART]) + 15 * 60_000, + }); + const report = await h.run(CALENDAR_AUTHORITY); + + expect(report).toMatchObject({ outcome: 'ok', itemErrors: [], stats: { uploaded: { created: 0, updated: 1, deleted: 0 } } }); + expect(h.server.all('CalendarEvent', 'a')).toHaveLength(1); + const event = h.server.get('CalendarEvent', 'a', series)!; + expect(event).toMatchObject({ uid: 'series-uid', title: 'Standup', keywords: { team: true } }); + expect(event.recurrenceRule).toBeUndefined(); + expect(h.events()).toEqual([expect.objectContaining({ _id: single, _sync_id: `a/${series}`, rrule: null, dirty: 0 })]); + }); + + it('never leaves two rows with one identity when the moved event is edited while its move uploads', async () => { + const h = real(); + const work = h.server.addCalendar('a', { name: 'Work' }); + const lunch = h.server.addEvent('a', { + uid: 'lunch-uid', + title: 'Lunch', + start: '2026-10-06T12:00:00', + duration: 'PT1H', + timeZone: 'Europe/Berlin', + calendarIds: { [h.calendar]: true }, + }); + await h.run(CALENDAR_AUTHORITY); + const old = h.events().find((e) => e._sync_id === `a/${lunch}`)!; + const workRow = Number(h.device.rows('calendars').find((c) => c._sync_id === `a/${work}`)!._id); + const moved = etarReinsert(h, old, workRow); + let edited = false; + const stop = h.server.onAfterRequest((request) => { + if (edited || !request.methods.includes('CalendarEvent/set')) return; + edited = true; + h.device.user.updateEvent(moved, { [Events.TITLE]: 'Team lunch' }); + }); + + await h.run(CALENDAR_AUTHORITY); + stop(); + + expect(edited).toBe(true); + expect(sharedIdentities(h)).toEqual([]); + expect(h.events().find((e) => Number(e._id) === moved)).toMatchObject({ title: 'Team lunch' }); + + expect((await h.run(CALENDAR_AUTHORITY)).outcome).toBe('ok'); + expect(sharedIdentities(h)).toEqual([]); + const server = h.server.all('CalendarEvent', 'a'); + expect(server.map((e) => [e.title, e.calendarIds])).toEqual([['Team lunch', { [work]: true }]]); + expect(h.events()).toEqual([expect.objectContaining({ _id: moved, _sync_id: `a/${server[0].id}`, dirty: 0 })]); + }); + + it('pairs the rows again when an app rewrote the moved event unchanged while its move uploaded', async () => { + const h = real(); + const work = h.server.addCalendar('a', { name: 'Work' }); + const lunch = h.server.addEvent('a', { + uid: 'lunch-uid', + title: 'Lunch', + start: '2026-10-06T12:00:00', + duration: 'PT1H', + timeZone: 'Europe/Berlin', + keywords: { important: true }, + organizerCalendarAddress: 'mailto:alice@example.com', + participants: { + me: { '@type': 'Participant', calendarAddress: 'mailto:alice@example.com', roles: { owner: true, attendee: true }, participationStatus: 'accepted' }, + bob: { '@type': 'Participant', calendarAddress: 'mailto:bob@example.com', name: 'Bob', roles: { attendee: true }, participationStatus: 'accepted' }, + }, + calendarIds: { [h.calendar]: true }, + }); + await h.run(CALENDAR_AUTHORITY); + const old = h.events().find((e) => e._sync_id === `a/${lunch}`)!; + const workRow = Number(h.device.rows('calendars').find((c) => c._sync_id === `a/${work}`)!._id); + const moved = etarReinsert(h, old, workRow); + const stop = h.server.onAfterRequest((request) => { + if (!request.methods.includes('CalendarEvent/set')) return; + stop(); + // Fossify's save: the same attendees, deleted and inserted again. + const attendees = h.device.rows('attendees').filter((a) => Number(a.event_id) === moved).map(({ _id, event_id, ...a }) => a); + h.device.user.fossifySaveEvent(moved, {}, attendees, []); + }); + + const report = await h.run(CALENDAR_AUTHORITY); + + expect(report).toMatchObject({ outcome: 'ok', itemErrors: [], stats: { uploaded: { created: 0, updated: 1, deleted: 0 } } }); + expect(h.server.all('CalendarEvent', 'a')).toHaveLength(1); + expect(h.server.get('CalendarEvent', 'a', lunch)).toMatchObject({ uid: 'lunch-uid', calendarIds: { [work]: true }, keywords: { important: true } }); + expect(h.events()).toEqual([expect.objectContaining({ _id: moved, _sync_id: `a/${lunch}`, dirty: 0 })]); + }); + it('syncs events both ways and writes nothing for its own echo', async () => { const h = real(); const standup = h.server.addEvent('a', { diff --git a/src/device-sync/engine/calendar-sync.ts b/src/device-sync/engine/calendar-sync.ts index 1f78fecf..e14d8264 100644 --- a/src/device-sync/engine/calendar-sync.ts +++ b/src/device-sync/engine/calendar-sync.ts @@ -5,6 +5,7 @@ */ import { findTasksOnlyCalendarIds, isTaskLikeObject, SCAN_PROPERTIES, type ScannedCalendarObject } from '../../lib/calendar-component-detection'; import { collectionKey, parseCollectionKey, parseObjectRef, pendingUidOf } from '../common/ids'; +import { deepEqual } from '../common/json'; import type { CalendarContext, CalendarEventWire, CalendarLike, CalendarPlanner, LocalCalendar, LocalEvent, OpGroup } from '../planner'; import { CALENDAR_AUTHORITY, type ReminderOwner, type Row } from '../types'; import { CALENDAR_EVENT_PROPERTIES, CALENDAR_PROPERTIES } from '../wire'; @@ -15,7 +16,7 @@ import type { SubscriptionCalendar } from './deps'; import { ItemSync, type Pending } from './item-sync'; import { accountOfRef, idInAccount, refOf, type Held, type Kind, type ServerObject } from './kinds'; import { poisonFingerprint } from './poison'; -import { flag, num, str } from './provider'; +import { flag, hasWrites, num, str } from './provider'; import { isCollectionSelected } from './selection'; import { accountOf, type SyncState } from './sync-state'; @@ -24,6 +25,8 @@ const WITH_IDENTITY = "_sync_id IS NOT NULL AND _sync_id NOT LIKE '~pending/%'"; /** CalendarProvider leaves DIRTY (and may leave DELETED) NULL on sync-adapter inserts: NULL is clean. */ const CLEAN = '(dirty IS NULL OR dirty = 0) AND (deleted IS NULL OR deleted = 0)'; +type PairPlan = ReturnType[number]; + function onIds(map: Record | undefined | null): string[] { return Object.entries(map ?? {}) .filter(([, on]) => on === true) @@ -411,41 +414,47 @@ export class CalendarSync extends ItemSync { /** * Before the creates: a split's source uploads its rule while the split is - * still visible, then its clone is claimed as a new event; and deleted + - * new masters of the run that are one edit (a move, a series turned - * single) upload as patches of the existing object. + * still visible, then its clone is claimed as a new event. */ protected async prePhase(acct: string): Promise { - let items = (await this.loadUploadItems()).filter((h) => this.accountOfItem(h) === acct); + const items = (await this.loadUploadItems()).filter((h) => this.accountOfItem(h) === acct); const sources = items.filter((h) => (h.local as LocalEvent).split === 'source'); const clones = items.filter((h) => (h.local as LocalEvent).split === 'clone'); - if (sources.length || clones.length) { - const works: Work[] = []; - const ready: Pending[] = []; - for (const held of sources) { - const pending = await this.planItem(acct, held, works); - if (pending) ready.push(pending); - } - // A clone (of a split, or a new master whose uid another row carries) needs no source: - // it claims a fresh uid here and is created with the other new events. - for (const held of clones) { - const pending = await this.planItem(acct, held, works); - if (pending) ready.push(pending); - } - await this.env.writer.write(works); - if (ready.length) await this.send(acct, ready); - items = (await this.loadUploadItems()).filter((h) => this.accountOfItem(h) === acct); + if (!sources.length && !clones.length) return; + const works: Work[] = []; + const ready: Pending[] = []; + for (const held of sources) { + const pending = await this.planItem(acct, held, works); + if (pending) ready.push(pending); } + // A clone (of a split, or a new master whose uid another row carries) needs no source: + // it claims a fresh uid here and is created with the other new events. + for (const held of clones) { + const pending = await this.planItem(acct, held, works); + if (pending) ready.push(pending); + } + await this.env.writer.write(works); + if (ready.length) await this.send(acct, ready); + } + + /** + * Before new rows are claimed: deleted and new masters of the run that are + * one edit (a move, a series turned single) upload as patches of the + * existing object, and the new row takes the old one's identity. The + * planner pairs only a new row that has not claimed a uid of its own. + */ + protected async pairPhase(acct: string): Promise { + const items = (await this.loadUploadItems()).filter((h) => this.accountOfItem(h) === acct); const deleted = items.map((h) => h.local as LocalEvent).filter((e) => { const m = this.eventKind.meta(e); return m.deleted && !!m.sourceId; }); const fresh = items.map((h) => h.local as LocalEvent).filter((e) => { const m = this.eventKind.meta(e); - return m.isNew && !m.deleted && !!m.pending; + return m.isNew && !m.deleted && !m.pending; }); if (!deleted.length || !fresh.length) return; - let pairs: ReturnType; + let pairs: PairPlan[]; try { pairs = this.planner.planPairs(deleted, fresh, this.ctx(acct)); } catch (error) { @@ -465,16 +474,25 @@ export class CalendarSync extends ItemSync { actions: pair.actions as Pending['actions'], accept: async (objects) => { // The new row takes the identity and exceptions of the old one, which is purged. - await this.env.writer.write([{ group: pair.ops }]); - const [moved] = await this.eventKind.loadByRowIds([pair.fresh.eventId]); + let paired = false; + await this.env.writer.write([this.pairWork(acct, pair, () => { + paired = true; + })]); + const [moved] = paired ? await this.eventKind.loadByRowIds([pair.fresh.eventId]) : []; const server = ownId ? objects.get(ownId) : undefined; if (!moved || !server) { + // Fetched again first next run. Rows that no longer pair (the new one was edited + // meanwhile) upload as a create and a deletion; neither takes the identity here. if (ownId) this.markStale(acct, ownId); return []; } const held: Held = { kind: this.eventKind, local: moved }; try { - return [this.acceptWork(acct, { held, meta: this.eventKind.meta(moved), fingerprint: '', actions: [] }, server, false)]; + const work = this.acceptWork(acct, { held, meta: this.eventKind.meta(moved), fingerprint: '', actions: [] }, server, false); + if (hasWrites(work.group.ops)) return [work]; + // The pair's ops already wrote what the server holds. + this.env.report.stats.uploaded.updated++; + return []; } catch (error) { this.plannerFailed(acct, refOf(acct, server.id), 'upload', error, server.id); return []; @@ -485,6 +503,32 @@ export class CalendarSync extends ItemSync { await this.send(acct, pending); } + /** + * A pair's rows after the server took its patch. When their assert fails + * (an app changed them during the upload), both rows are read and paired + * again; only a pair that uploads the same patch is written. + */ + private pairWork(acct: string, pair: PairPlan, applied: () => void): Work { + return { + group: pair.ops, + applied, + replan: async () => { + const rows = await this.eventKind.loadByRowIds([pair.deleted.eventId, pair.fresh.eventId]); + const deleted = rows.find((e) => e.eventId === pair.deleted.eventId); + const fresh = rows.find((e) => e.eventId === pair.fresh.eventId); + if (!deleted || !fresh) return null; + let again: PairPlan | undefined; + try { + [again] = this.planner.planPairs([deleted], [fresh], this.ctx(acct)); + } catch (error) { + this.plannerFailed(acct, pair.deleted.syncId ?? `pairs:${acct}`, 'upload', error); + return null; + } + return again && deepEqual(again.actions, pair.actions) ? this.pairWork(acct, again, applied) : null; + }, + }; + } + // ── After the upload ── /** A master with nothing waiting for an upload. */ diff --git a/src/device-sync/engine/item-sync.ts b/src/device-sync/engine/item-sync.ts index e4c96911..f390a57f 100644 --- a/src/device-sync/engine/item-sync.ts +++ b/src/device-sync/engine/item-sync.ts @@ -142,7 +142,9 @@ export abstract class ItemSync { const meta = held.kind.meta(held.local); return accountOfRef(meta.sourceId) ?? accountOfRef(meta.pending?.target ?? null); } - /** Before the creates (calendar: split series and pairs). */ + /** Before new rows are claimed (calendar: deleted and new rows that are one edit, uploaded as patches). */ + protected async pairPhase(_acct: string): Promise {} + /** Before the creates (calendar: split series). */ protected async prePhase(_acct: string): Promise {} /** After the deletions (contacts: groups deleted by absence, memberships). */ protected async extraPhase(_acct: string, _deletionsAllowed: boolean): Promise {} @@ -691,16 +693,20 @@ export abstract class ItemSync { // ── Uploads ── /** - * Local changes to the server, per phase across the accounts: pairs and - * splits, creates, updates, deletions, then memberships. Creates of every - * account come before deletions of any, so a move between accounts never - * destroys first. A `stateMismatch` downloads the account again and - * repeats the phase, at most three times. + * Local changes to the server, per phase across the accounts: pairs, the + * claims of new rows, splits, creates, updates, deletions, then + * memberships. Pairs go first: the new row of a pair takes the deleted + * row's identity, so it must not have claimed a uid of its own, and a + * paired deletion is a move, not a deletion. Creates of every account come + * before deletions of any, so a move between accounts never destroys + * first. A `stateMismatch` downloads the account again and repeats the + * phase, at most three times. */ protected async uploadAll(): Promise { + const accounts = this.accountIds(); + for (const acct of accounts) await this.withRetries(acct, () => this.pairPhase(acct)); await this.claimNewItems(); const deletionsAllowed = await this.deletionPolicy(await this.loadUploadItems()); - const accounts = this.accountIds(); for (const acct of accounts) await this.withRetries(acct, () => this.prePhase(acct)); for (const phase of ['create', 'update', 'delete'] as const) { for (const acct of accounts) await this.withRetries(acct, () => this.phase(acct, phase, deletionsAllowed));