fix: upload new contacts whose address book was deleted meanwhile #34

A new contact is claimed for an address book before its create is
sent, so a retry after a lost response finds it again. When that book
was deleted on the server before the create went up, every attempt
failed and was retried after up to a day, forever. A book that is gone
can hold nothing of an earlier attempt, so the contact is now claimed
again for the book the settings choose.

An event cannot move on its own to another calendar: a create into a
calendar that is gone is no longer sent, and the report says why, until
the user moves the event in the calendar app.
This commit is contained in:
Linus Rath
2026-09-27 07:53:36 +02:00
parent 34236f677b
commit fad7b30dbf
5 changed files with 84 additions and 3 deletions
@@ -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() {
+6
View File
@@ -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)))
+8
View File
@@ -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<void> {
this.syncedKeys.clear();
for (const account of this.env.accounts) {
+19 -3
View File
@@ -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<void> {
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++;
+2
View File
@@ -57,6 +57,8 @@ export interface Kind<L = any, O extends ServerObject = any> {
planBaselineHeal(local: L): OpGroup | null;
planUpload(local: L, jmapAccountId: string): UploadPlan<O>;
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 {