fix: let a contact deleted on the phone and moved away meanwhile survive #34

When another client moved a card out of every address book the device
syncs while the device deleted the contact, the engine noted the card
for dropping but never showed its new version to the planner, so the
deletion was decided on the old copy and destroyed the card. A deleted
item is now merged like any changed one first; the contacts planner
then leaves a card that is in no synced book. When the planner cannot
place the object, the deletion stays as it was.
This commit is contained in:
Linus Rath
2026-09-27 07:53:37 +02:00
parent 654675d90b
commit 0faee568cc
2 changed files with 67 additions and 14 deletions
@@ -296,7 +296,7 @@ describe('device sync engine with the real planners', () => {
});
});
describe('a deletion the teardown uploads', () => {
describe('a deletion decided on the server version', () => {
/** The contacts planner as fixed for data finding 4: a deleted contact's shadow follows the server. */
function refreshingDeletedShadows(h: Harness): void {
h.deps.planners = {
@@ -313,7 +313,7 @@ describe('device sync engine with the real planners', () => {
};
}
it('is decided on the server version when the server changed since the last sync', async () => {
it('in a teardown, when the server changed since the last sync', async () => {
const h = real();
const archive = h.server.addAddressBook('a', { name: 'Archive' });
h.prefs.contactsSelection[`a/${archive}`] = false;
@@ -330,7 +330,51 @@ describe('device sync engine with the real planners', () => {
expect((h.server.get('ContactCard', 'a', ada)!.addressBookIds as object)).not.toHaveProperty(h.book);
});
it('waits, with the edits, while no state guards the upload (the first sync never ended)', async () => {
it('lets the planner see the server version of a contact deleted on the device and filed away meanwhile', async () => {
const h = real();
const archive = h.server.addAddressBook('a', { name: 'Archive' });
h.prefs.contactsSelection[`a/${archive}`] = false;
const [ada] = addServerCards(h, ['Ada Lovelace']);
await h.run();
refreshingDeletedShadows(h);
const planned: string[][] = [];
const contacts = h.deps.planners.contacts;
h.deps.planners = {
...h.deps.planners,
contacts: {
...contacts,
planUpload: (local, ctx) => {
if (local.deleted) planned.push(Object.keys(local.shadow?.addressBookIds ?? {}));
return contacts.planUpload(local, ctx);
},
},
};
h.device.user.deleteContact(h.contactNamed('Ada Lovelace')!.id);
// Another client files Ada in Archive only: she left every synced book.
h.server.serverUpdate('ContactCard', 'a', ada, { addressBookIds: { [archive]: true } });
expect((await h.run()).outcome).toBe('ok');
expect(planned).toEqual([[archive]]);
});
it('still uploads the deletion of an event whose calendar the planner cannot place', async () => {
const h = real();
const archive = h.server.addCalendar('a', { name: 'Archive' });
h.prefs.calendarSelection[`a/${archive}`] = false;
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);
h.device.user.deleteEvent(Number(h.events()[0]._id));
h.server.serverUpdate('CalendarEvent', 'a', lunch, { calendarIds: { [archive]: true } });
const report = await h.run(CALENDAR_AUTHORITY);
expect(report).toMatchObject({ outcome: 'ok', itemErrors: [], stats: { uploaded: { deleted: 1 } } });
expect(h.server.get('CalendarEvent', 'a', lunch)).toBeUndefined();
expect(h.events()).toEqual([]);
});
it('waits in a teardown, with the edits, while no state guards the upload (the first sync never ended)', async () => {
const h = real();
const [ada] = addServerCards(h, ['Ada Lovelace', 'Grace Hopper', ...Array.from({ length: 60 }, (_, i) => `P${i}`)]);
// The first sync (a full reconcile) runs out of time after its first chunk (Ada and Grace): no state yet.
+20 -11
View File
@@ -164,14 +164,6 @@ export abstract class ItemSync {
protected dependents(_kind: Kind, _acct: string, _object: ServerObject): string[] {
return [];
}
private staleWith(acct: string, ids: string[]): StateChange | undefined {
if (!ids.length) return undefined;
return (next) => {
const account = accountOf(next, acct);
account.stale = [...new Set([...account.stale, ...ids])];
};
}
protected onDiscard(_next: SyncState, _accounts: Set<string>): void {}
/** Whether a pending create of `target` may adopt this object (contacts: the target book holds it). */
protected adoptionTargetMatches(acct: string, _object: ServerObject, target: string): boolean {
@@ -279,6 +271,15 @@ export abstract class ItemSync {
if (this.marks(acct).has(id) || this.env.store.account(acct).stale.includes(id)) this.marks(acct).set(id, false);
}
/** Puts `ids` on `stale` in the batch whose rows make them outdated (see `dependents`). */
private staleWith(acct: string, ids: string[]): StateChange | undefined {
if (!ids.length) return undefined;
return (next) => {
const account = accountOf(next, acct);
account.stale = [...new Set([...account.stale, ...ids])];
};
}
protected isStale(acct: string, sourceId: string | null): boolean {
const id = idInAccount(sourceId, acct);
if (!id) return false;
@@ -638,10 +639,18 @@ export abstract class ItemSync {
if (!kind || !this.inSelection(acct, object)) {
if (!found) return null;
const meta = found.kind.meta(found.local);
if (found.kind === kind && meta.dirty && !meta.deleted && !meta.isNew) {
// Uploaded first, dropped afterwards: merged like any dirty item, so the upload builds on the server's version.
if (found.kind === kind && (meta.dirty || meta.deleted) && !meta.isNew) {
// Uploaded first, dropped afterwards: merged like any changed item (a deleted one's shadow follows
// too), so the upload builds on the server's version.
this.outsideOf(acct).add(refOf(acct, object.id));
return this.downloadWork(kind, acct, object, found.local, written);
if (!meta.deleted) return this.downloadWork(kind, acct, object, found.local, written);
try {
return this.downloadWork(kind, acct, object, found.local, written);
} catch (error) {
// A planner that cannot place it (no synced calendar) leaves the deletion as it was planned.
this.env.log(`deleted ${refOf(acct, object.id)} outside the selection keeps its shadow`, error);
return null;
}
}
return this.removeWork(acct, object.id, found, 'outside');
}