diff --git a/src/device-sync/__tests__/engine/real-planners.test.ts b/src/device-sync/__tests__/engine/real-planners.test.ts index a493d065..9a040a0d 100644 --- a/src/device-sync/__tests__/engine/real-planners.test.ts +++ b/src/device-sync/__tests__/engine/real-planners.test.ts @@ -235,6 +235,55 @@ describe('device sync engine with the real planners', () => { }); }); + describe('a create whose target the server deleted after the claim', () => { + it('claims a new contact again for a book that exists', async () => { + const h = real(); + const work = h.server.addAddressBook('a', { name: 'Work' }); + h.prefs.newContactsAddressBook = `a/${work}`; + await h.run(); + const id = addDeviceContact(h, 'Hedy Lamarr'); + // The claim names Work, the create is lost; then another client deletes Work. + h.server.failNextRequest('network', { match: 'ContactCard/set' }); + expect((await h.run()).outcome).toBe('io'); + h.server.serverDestroy('AddressBook', 'a', work); + + const report = await h.run(); + + expect(report).toMatchObject({ outcome: 'ok', itemErrors: [], stats: { uploaded: { created: 1 } } }); + const hedy = h.server.all('ContactCard', 'a').find((c) => (c.name as { full?: string }).full === 'Hedy Lamarr')!; + expect(hedy.addressBookIds).toEqual({ [h.book]: true }); + expect(h.contacts().find((c) => c.id === id)).toMatchObject({ sourceId: `a/${hedy.id}`, dirty: false }); + expect(h.contacts().find((c) => c.id === id)!.row.sync4).toBeNull(); + }); + + it('keeps a new event whose calendar is gone, without poisoning it, until the user moves it', async () => { + const h = real(); + const work = h.server.addCalendar('a', { name: 'Work' }); + await h.run(CALENDAR_AUTHORITY); + const rowOf = (calendarId: string) => Number(h.device.rows('calendars').find((c) => c._sync_id === `a/${calendarId}`)!._id); + const workRow = rowOf(work); + const dentist = { title: 'Dentist', dtstart: Date.UTC(2026, 9, 1, 8), dtend: Date.UTC(2026, 9, 1, 9), eventTimezone: 'Europe/Berlin' }; + const id = h.device.user.insertEvent(workRow, dentist); + h.server.failNextRequest('network', { match: 'CalendarEvent/set' }); + expect((await h.run(CALENDAR_AUTHORITY)).outcome).toBe('io'); + h.server.serverDestroy('Calendar', 'a', work); + + const report = await h.run(CALENDAR_AUTHORITY); + + expect(report.itemErrors).toEqual([expect.objectContaining({ ref: `row:${id}`, type: 'collectionGone' })]); + expect(h.server.all('CalendarEvent', 'a')).toEqual([]); + expect(h.events().find((e) => Number(e._id) === id)).toMatchObject({ title: 'Dentist', sync_data5: null }); + + // The user moves it to the personal calendar (Etar: delete + insert). + h.device.user.deleteEvent(id); + h.device.user.insertEvent(rowOf(h.calendar), dentist); + expect(await h.run(CALENDAR_AUTHORITY)).toMatchObject({ outcome: 'ok', itemErrors: [] }); + + expect(h.server.all('CalendarEvent', 'a').map((e) => [e.title, e.calendarIds])).toEqual([['Dentist', { [h.calendar]: true }]]); + expect(h.device.rows('calendars').map((c) => c._sync_id)).toEqual([`a/${h.calendar}`]); + }); + }); + describe('a device edit of an object the server moved out of every synced collection', () => { /** Ada with two numbers in the synced book; the device deletes the second in place (AOSP). */ async function adaWithDeletedNumber() { diff --git a/src/device-sync/engine/calendar-sync.ts b/src/device-sync/engine/calendar-sync.ts index e14d8264..fab6327c 100644 --- a/src/device-sync/engine/calendar-sync.ts +++ b/src/device-sync/engine/calendar-sync.ts @@ -257,6 +257,12 @@ export class CalendarSync extends ItemSync { await this.refreshCalendarRows(); } + protected collectionGone(key: string): boolean { + const parsed = parseCollectionKey(key); + const list = parsed ? this.calendars.get(parsed.accountId) : undefined; + return !!list && !list.some((c) => c.id === parsed!.id); + } + protected syncedCollections(acct: string): string[] { return (this.calendars.get(acct) ?? []) .filter((c) => this.syncedKeys.has(collectionKey(acct, c.id))) diff --git a/src/device-sync/engine/contacts-sync.ts b/src/device-sync/engine/contacts-sync.ts index 3c8660ba..228443e8 100644 --- a/src/device-sync/engine/contacts-sync.ts +++ b/src/device-sync/engine/contacts-sync.ts @@ -172,6 +172,7 @@ export class ContactsSync extends ItemSync { planBaselineHeal: (c) => planner.planBaselineHeal(c), planUpload: (c, acct) => planner.planUpload(c, this.ctx(acct)), planAccepted: (c, card, acct) => planner.planAccepted(c, card, this.ctx(acct)), + unclaimed: (c) => ({ ...c, pending: null }), }; } @@ -205,6 +206,7 @@ export class ContactsSync extends ItemSync { planBaselineHeal: () => null, planUpload: (g, acct) => planner.planGroupUpload(g, this.ctx(acct)), planAccepted: (g, card, acct) => planner.planGroupAccepted(g, card, this.ctx(acct)), + unclaimed: (g) => ({ ...g, pending: null }), }; } @@ -284,6 +286,12 @@ export class ContactsSync extends ItemSync { return !!book && book.myRights?.mayWrite !== false; } + protected collectionGone(key: string): boolean { + const parsed = parseCollectionKey(key); + const books = parsed ? this.books.get(parsed.accountId) : undefined; + return !!books && !books.some((b) => b.id === parsed!.id); + } + protected async collections(write: boolean): Promise { this.syncedKeys.clear(); for (const account of this.env.accounts) { diff --git a/src/device-sync/engine/item-sync.ts b/src/device-sync/engine/item-sync.ts index 84e9c254..4bda226a 100644 --- a/src/device-sync/engine/item-sync.ts +++ b/src/device-sync/engine/item-sync.ts @@ -155,6 +155,10 @@ export abstract class ItemSync { protected claimable(_held: Held): boolean { return true; } + /** Whether a collection key names a collection its account no longer has (listed this run). */ + protected collectionGone(_key: string): boolean { + return false; + } /** The JMAP account an item uploads to. */ protected accountOfItem(held: Held): string | null { const meta = held.kind.meta(held.local); @@ -792,12 +796,18 @@ export abstract class ItemSync { return this.setStates.get(acct) ?? null; } - /** New items get their uid and target written before anything is sent. */ + /** + * New items get their uid and target written before anything is sent. A + * claim whose collection is gone from the server is made again where the + * kind allows it (contacts: nothing of it can exist in a deleted book). + */ private async claimNewItems(): Promise { const works: Work[] = []; for (const held of await this.loadUploadItems()) { const meta = held.kind.meta(held.local); - if (!meta.isNew || meta.pending || meta.deleted || !this.claimable(held)) continue; + if (!meta.isNew || meta.deleted || !this.claimable(held)) continue; + const reclaim = !!meta.pending && !!held.kind.unclaimed && this.collectionGone(meta.pending.target); + if (meta.pending && !reclaim) continue; const ref = `row:${meta.rowId}`; const acct = this.claimAccount(held); if (!acct) { @@ -807,7 +817,7 @@ export abstract class ItemSync { } let plan; try { - plan = held.kind.planUpload(held.local, acct); + plan = held.kind.planUpload(reclaim ? held.kind.unclaimed!(held.local) : held.local, acct); } catch (error) { this.plannerFailed(acct, ref, 'upload', error); continue; @@ -915,6 +925,12 @@ export abstract class ItemSync { report.itemError({ ref, side: 'upload', type: 'stale', description: 'Waits until the server version could be stored' }); return null; } + if (meta.isNew && !meta.deleted && meta.pending && this.collectionGone(meta.pending.target)) { + // Nothing can be created there (an event stays in its calendar until the user moves it). + report.stats.skipped++; + report.itemError({ ref, side: 'upload', type: 'collectionGone', description: `${meta.pending.target} no longer exists on the server` }); + return null; + } const fingerprint = kind.fingerprint(local); if (meta.poison && isBackedOff(meta.poison, fingerprint, this.env.now())) { report.stats.skipped++; diff --git a/src/device-sync/engine/kinds.ts b/src/device-sync/engine/kinds.ts index 7ef6538f..10881a05 100644 --- a/src/device-sync/engine/kinds.ts +++ b/src/device-sync/engine/kinds.ts @@ -57,6 +57,8 @@ export interface Kind { planBaselineHeal(local: L): OpGroup | null; planUpload(local: L, jmapAccountId: string): UploadPlan; planAccepted(local: L, server: O, jmapAccountId: string): AcceptedPlan; + /** The item without its pending create, so `planUpload` claims it again; absent when the target can't change. */ + unclaimed?(local: L): L; } export function refOf(jmapAccountId: string, id: string): string {