mirror of
https://github.com/bulwarkmail/native.git
synced 2026-10-31 08:06:36 +00:00
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:
@@ -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.
|
||||
|
||||
@@ -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');
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user