diff --git a/src/device-sync/__tests__/engine/own-creates.test.ts b/src/device-sync/__tests__/engine/own-creates.test.ts new file mode 100644 index 00000000..76eb8c94 --- /dev/null +++ b/src/device-sync/__tests__/engine/own-creates.test.ts @@ -0,0 +1,77 @@ +// Objects our uploads created. The stored state stays where the download left +// it, and `/changes` omits an object created and destroyed after its +// `sinceState` (Stalwart does, RFC 8620 allows it): such an object must still +// leave the device when another client destroys it before the next download. + +import { describe, expect, it } from 'vitest'; +import { calendarPlanner } from '../../calendar/planner'; +import { contactsPlanner } from '../../contacts/planner'; +import { addDeviceContact, addServerCards, CALENDAR_AUTHORITY, createHarness, type Harness } from './harness'; + +function real(): Harness { + const h = createHarness(); + h.deps.planners = { contacts: contactsPlanner, calendar: calendarPlanner }; + return h; +} + +describe('device sync engine: objects our uploads created', () => { + it('removes a device-created contact that another client destroyed before the next download', async () => { + const h = real(); + addServerCards(h, ['Ada Lovelace']); + await h.run(); + const id = addDeviceContact(h, 'Hedy Lamarr'); + expect((await h.run()).stats.uploaded.created).toBe(1); + const hedy = h.contacts().find((c) => c.id === id)!.sourceId!.split('/')[1]; + h.server.serverDestroy('ContactCard', 'a', hedy); + + const report = await h.run(); + + expect(report).toMatchObject({ outcome: 'ok', stats: { downloaded: { deleted: 1 } } }); + expect(h.contacts().map((c) => c.name)).toEqual(['Ada Lovelace']); + expect(h.state()?.accounts.a.created ?? []).toEqual([]); + }); + + it('removes a device-created event that another client destroyed before the next download', async () => { + const h = real(); + await h.run(CALENDAR_AUTHORITY); + const rowId = Number(h.device.rows('calendars')[0]._id); + const id = h.device.user.insertEvent(rowId, { title: 'Dentist', dtstart: Date.UTC(2026, 9, 1, 8), dtend: Date.UTC(2026, 9, 1, 9), eventTimezone: 'Europe/Berlin' }); + expect((await h.run(CALENDAR_AUTHORITY)).stats.uploaded.created).toBe(1); + const created = String(h.events().find((e) => Number(e._id) === id)!._sync_id).split('/')[1]; + h.server.serverDestroy('CalendarEvent', 'a', created); + + const report = await h.run(CALENDAR_AUTHORITY); + + expect(report).toMatchObject({ outcome: 'ok', stats: { downloaded: { deleted: 1 } } }); + expect(h.events()).toEqual([]); + }); + + it('fetches a created object once, and writes nothing when it is unchanged', async () => { + const h = real(); + await h.run(); + addDeviceContact(h, 'Hedy Lamarr'); + await h.run(); + expect(h.state()?.accounts.a.created).toHaveLength(1); + + const before = h.batches.log.length; + expect((await h.run()).outcome).toBe('ok'); + + expect(h.state()?.accounts.a.created).toEqual([]); + expect(h.batches.log.slice(before).flat().filter((op) => op.op !== 'syncState')).toEqual([]); + expect(h.contacts().map((c) => c.name)).toEqual(['Hedy Lamarr']); + }); + + it('turns an upload-only sync into a full one while a created object waits to be checked', async () => { + const h = real(); + await h.run(); + const id = addDeviceContact(h, 'Hedy Lamarr'); + await h.run(); + const hedy = h.contacts().find((c) => c.id === id)!.sourceId!.split('/')[1]; + h.server.serverDestroy('ContactCard', 'a', hedy); + + // Android's upload sync 30 s after an app wrote anything, with nothing dirty in this account. + expect((await h.run(undefined, { upload: true })).outcome).toBe('ok'); + + expect(h.contacts()).toEqual([]); + }); +}); diff --git a/src/device-sync/engine/item-sync.ts b/src/device-sync/engine/item-sync.ts index 4bda226a..b0eae42d 100644 --- a/src/device-sync/engine/item-sync.ts +++ b/src/device-sync/engine/item-sync.ts @@ -44,6 +44,19 @@ function both(first: StateChange | undefined, second: StateChange | undefined): }; } +/** The work with one more state change, also after a re-plan. */ +function withState(work: Work, change: StateChange): Work { + const replan = work.replan; + return { + ...work, + state: both(work.state, change), + replan: replan && (async () => { + const next = await replan(); + return next && withState(next, change); + }), + }; +} + /** An item's upload, on its way to the server. */ export interface Pending { held: Held; @@ -324,10 +337,20 @@ export abstract class ItemSync { // ── Downloads ── - /** Objects put on `stale` by an earlier run: fetched again first (invariant 10). */ + /** + * Objects put on `stale` by an earlier run (invariant 10), and objects our + * uploads created since the stored state (`/changes` from it omits one that + * was created and destroyed since): fetched again first. + */ protected async refetchStale(acct: string): Promise { - const ids = [...this.env.store.account(acct).stale]; - if (ids.length) await this.processIds(acct, ids, [], () => undefined); + const account = this.env.store.account(acct); + const created = account.created ?? []; + const ids = [...new Set([...account.stale, ...created])]; + if (!ids.length) return; + await this.processIds(acct, ids, [], (next) => { + const a = accountOf(next, acct); + a.created = (a.created ?? []).filter((id) => !created.includes(id)); + }); } /** `/changes` pages from the stored state; a full reconcile without one or after `cannotCalculateChanges`. */ @@ -1070,7 +1093,8 @@ export abstract class ItemSync { } await this.beforePlanning(acct, [object]); const work = this.downloadWork(p.held.kind, acct, object, p.held.local, null); - return work ? [work] : []; + // Our own earlier create: remembered like one of this run. + return work ? [withState(work, this.rememberCreated(acct, id))] : []; } /** Sends the items' actions in `/set` calls and settles each item once all its actions have an answer. */ @@ -1319,7 +1343,7 @@ export abstract class ItemSync { this.markStale(acct, server.id); this.env.report.itemError({ ref: refOf(acct, server.id), side: 'upload', type: reason, description: message }); }; - const state = this.listing(kind, acct, server.id, true); + const state = both(this.listing(kind, acct, server.id, true), created ? this.rememberCreated(acct, server.id) : undefined); return { group: withExtra(plan.ops), state, @@ -1329,6 +1353,14 @@ export abstract class ItemSync { }; } + /** An object our upload created is remembered with its identity until the next run fetched it (see `refetchStale`). */ + private rememberCreated(acct: string, id: string): StateChange { + return (next) => { + const account = accountOf(next, acct); + account.created = [...new Set([...(account.created ?? []), id])]; + }; + } + private poisonWork(p: Pending, error: SetErrorShape): Work { const marker = nextMarker(p.meta.poison, p.fingerprint, error, this.env.now(), { firstMs: this.env.tuning.poisonBackoffMs, diff --git a/src/device-sync/engine/run.ts b/src/device-sync/engine/run.ts index 39c06178..883be9c5 100644 --- a/src/device-sync/engine/run.ts +++ b/src/device-sync/engine/run.ts @@ -48,7 +48,7 @@ function itemTypeOf(authority: Authority): string { /** Rows waiting for an upload (dirty, deleted, new), or state work a run must do whatever its extras. */ export async function hasLocalWork(reader: ProviderReader, authority: Authority, state: SyncState): Promise { for (const account of Object.values(state.accounts)) { - if (account.reconcile || account.stale.length) return true; + if (account.reconcile || account.stale.length || account.created?.length) return true; } if (authority === CALENDAR_AUTHORITY) { const rows = await reader.rows('events', ['_id'], "dirty = 1 OR deleted = 1 OR _sync_id IS NULL OR _sync_id LIKE '~pending/%'"); diff --git a/src/device-sync/engine/sync-state.ts b/src/device-sync/engine/sync-state.ts index 46e1e73a..2d07f9ab 100644 --- a/src/device-sync/engine/sync-state.ts +++ b/src/device-sync/engine/sync-state.ts @@ -29,6 +29,11 @@ export interface AccountState { selected: string[]; /** Object ids whose rows could not be written; fetched again at the start of every run, nothing uploads for them meanwhile. */ stale: string[]; + /** + * Objects our uploads created after `itemsState`: fetched again at the start of the next run, since `/changes` + * omits an object created and destroyed after its `sinceState`. + */ + created?: string[]; reconcile: ReconcileMarker | null; /** Contacts: refs of the group cards present on the device (a hard-deleted group is found by its absence). */ groups?: string[]; @@ -81,6 +86,7 @@ function parseAccount(value: unknown): AccountState { stale: strings(a.stale), reconcile: parseReconcile(a.reconcile), }; + if (a.created !== undefined) out.created = strings(a.created); if (a.groups !== undefined) out.groups = strings(a.groups); if (a.taskOnly !== undefined) out.taskOnly = strings(a.taskOnly); return out;