diff --git a/src/device-sync/__tests__/contacts/groups.test.ts b/src/device-sync/__tests__/contacts/groups.test.ts index 74ffb7dd..f29efe9b 100644 --- a/src/device-sync/__tests__/contacts/groups.test.ts +++ b/src/device-sync/__tests__/contacts/groups.test.ts @@ -183,6 +183,21 @@ describe('contacts groups: device edits of groups', () => { }); }); + it('only leaves the synced book when the server filed the group in an unsynced one after the device deleted it', async () => { + const { h, gid } = await withGroup(); + const archive = h.server.addAddressBook(JMAP, { name: 'Archive' }); + h.unselected.add(`${JMAP}/${archive}`); + h.device.user.deleteGroup((await localGroup(h, gid)).groupId); + h.server.serverUpdate('ContactCard', JMAP, gid, { [`addressBookIds/${archive}`]: true, 'name/full': 'Old friends' }); + const plan = h.planner.planGroupDownload(h.card(gid), await localGroup(h, gid), h.ctx); + await h.applyOk(plan.ops); + // The delete wins over the rename; only the shadow follows the card. + expect(await localGroup(h, gid)).toMatchObject({ deleted: true, title: 'Friends' }); + expect(h.planner.planGroupUpload(await localGroup(h, gid), h.ctx)).toEqual({ + kind: 'upload', actions: [{ kind: 'update', id: gid, patch: { [`addressBookIds/${h.book}`]: null } }], + }); + }); + it('reverts a rename of a read-only group', async () => { const { h, gid } = await withGroup(); h.readOnlyBooks.add(h.book); diff --git a/src/device-sync/__tests__/contacts/merge.test.ts b/src/device-sync/__tests__/contacts/merge.test.ts index 709f3d92..2d07db5a 100644 --- a/src/device-sync/__tests__/contacts/merge.test.ts +++ b/src/device-sync/__tests__/contacts/merge.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { Data, MimeType } from '../../android-columns'; +import { Data, MimeType, RawContacts } from '../../android-columns'; import type { ContactCardWire } from '../../planner'; import { appleCard, googleCard } from './fixtures'; import { Harness, JMAP } from './harness'; @@ -139,13 +139,20 @@ describe('contacts merge: a dirty contact meets a new server version', () => { expect(typeof patch === 'object' && Object.keys(patch).some((k) => k.startsWith('nicknames'))).toBe(false); }); - it('leaves a contact deleted on the device alone: the delete wins and uploads', async () => { + it('leaves the rows of a contact deleted on the device alone: the delete wins and uploads', async () => { const h = new Harness(); const { id, rawId } = await h.seed(appleCard()); h.device.user.deleteContact(rawId); h.server.serverUpdate('ContactCard', JMAP, id, { 'emails/k1/address': 'x@example.com' }); const plan = h.planner.planDownload(h.card(id), await h.contact(rawId), h.ctx); - expect(plan).toMatchObject({ effect: 'none', stillDirty: true, ops: { ops: [] } }); + // Only the shadow follows the card, for the deletion's upload. + expect(plan).toMatchObject({ effect: 'update', stillDirty: true, conflicts: 0 }); + expect(plan.ops.ops.filter((o) => o.op !== 'assert')).toEqual([{ + op: 'update', table: 'raw_contacts', id: rawId, expectCount: 1, + values: { [RawContacts.SYNC2]: expect.any(String) }, + }]); + await h.applyOk(plan.ops); + expect(h.planner.planDownload(h.card(id), await h.contact(rawId), h.ctx)).toMatchObject({ effect: 'none', ops: { ops: [] } }); expect(h.planner.planUpload(await h.contact(rawId), h.ctx)).toMatchObject({ kind: 'upload', actions: [{ kind: 'destroy', id }] }); }); diff --git a/src/device-sync/__tests__/contacts/real-engine.test.ts b/src/device-sync/__tests__/contacts/real-engine.test.ts new file mode 100644 index 00000000..f42ba6ae --- /dev/null +++ b/src/device-sync/__tests__/contacts/real-engine.test.ts @@ -0,0 +1,54 @@ +// The contacts planner driven by the engine (fake providers, fake Stalwart): +// cases that only show end to end, over several runs. The planner's rules +// themselves are tested in the other files of this folder. + +import { describe, expect, it } from 'vitest'; +import { calendarPlanner } from '../../calendar/planner'; +import { contactsPlanner } from '../../contacts/planner'; +import { addServerCards, createHarness, type Harness } from '../engine/harness'; + +function real(): Harness { + const h = createHarness(); + h.deps.planners = { contacts: contactsPlanner, calendar: calendarPlanner }; + return h; +} + +/** An address book of the personal account that this device does not sync. */ +function unsyncedBook(h: Harness, name = 'Archive'): string { + const id = h.server.addAddressBook('a', { name }); + h.prefs.contactsSelection[`a/${id}`] = false; + return id; +} + +describe('contacts through the engine', () => { + it('keeps a card deleted on the device that the server meanwhile also filed in an unsynced book', async () => { + const h = real(); + const archive = unsyncedBook(h); + const [ada] = addServerCards(h, ['Ada Lovelace']); + expect((await h.run()).outcome).toBe('ok'); + + h.device.user.deleteContact(h.contactNamed('Ada Lovelace')!.id); + h.server.serverUpdate('ContactCard', 'a', ada, { [`addressBookIds/${archive}`]: true }); + const report = await h.run(); + + expect(report).toMatchObject({ outcome: 'ok', itemErrors: [], stats: { uploaded: { deleted: 1 } } }); + expect(h.server.get('ContactCard', 'a', ada)).toMatchObject({ addressBookIds: { [archive]: true } }); + expect(h.contacts()).toEqual([]); + }); + + it('keeps a group deleted on the device that the server meanwhile also filed in an unsynced book', async () => { + const h = real(); + const archive = unsyncedBook(h); + const group = h.server.addCard('a', { uid: 'g1', kind: 'group', name: { full: 'Friends' }, addressBookIds: { [h.book]: true } }); + expect((await h.run()).outcome).toBe('ok'); + + const row = h.device.rows('groups').find((g) => g.sourceid === `a/${group}`)!; + h.device.user.deleteGroup(Number(row._id)); + h.server.serverUpdate('ContactCard', 'a', group, { [`addressBookIds/${archive}`]: true }); + const report = await h.run(); + + expect(report).toMatchObject({ outcome: 'ok', itemErrors: [], stats: { uploaded: { deleted: 1 } } }); + expect(h.server.get('ContactCard', 'a', group)).toMatchObject({ addressBookIds: { [archive]: true } }); + expect(h.device.rows('groups')).toEqual([]); + }); +}); diff --git a/src/device-sync/__tests__/contacts/upload-editors.test.ts b/src/device-sync/__tests__/contacts/upload-editors.test.ts index 9bf3fe16..0fcc6a4c 100644 --- a/src/device-sync/__tests__/contacts/upload-editors.test.ts +++ b/src/device-sync/__tests__/contacts/upload-editors.test.ts @@ -309,6 +309,36 @@ describe('contacts upload: deleted contacts', () => { expect(await patchOf(h, rawId)).toEqual({ [`addressBookIds/${h.book}`]: null }); }); + it('only leaves the synced book when the server filed the card in an unsynced one after the device deleted it', async () => { + const h = new Harness(); + const archive = h.server.addAddressBook(JMAP, { name: 'Archive' }); + h.unselected.add(`${JMAP}/${archive}`); + const { id, rawId } = await h.seed(appleCard()); + h.device.user.deleteContact(rawId); + h.server.serverUpdate('ContactCard', JMAP, id, { [`addressBookIds/${archive}`]: true, 'emails/k1/address': 'new@example.com' }); + const rows = JSON.stringify(h.device.rows('data', 'raw_contact_id = ?', [rawId])); + await h.applyOk(h.planner.planDownload(h.card(id), await h.contact(rawId), h.ctx).ops); + // The delete wins over the server's edit: the rows stay as they were, only the shadow follows. + expect(JSON.stringify(h.device.rows('data', 'raw_contact_id = ?', [rawId]))).toBe(rows); + expect(await uploadPlan(h, rawId)).toEqual({ kind: 'upload', actions: [{ kind: 'update', id, patch: { [`addressBookIds/${h.book}`]: null } }] }); + await h.upload(rawId); + expect(h.server.get('ContactCard', JMAP, id)).toMatchObject({ addressBookIds: { [archive]: true } }); + expect(h.device.row('raw_contacts', rawId)).toBeUndefined(); + }); + + it('deletes its own create by id once the download found it, not by a uid lookup', async () => { + const h = new Harness(); + const rawId = h.device.user.insertContact(ACCOUNT, [{ [Data.MIMETYPE]: MimeType.NOTE, data1: 'y' }]); + const plan = await h.planUpload(rawId); + if (plan.kind !== 'upload') throw new Error(plan.kind); + // The create went through but its response was lost; then the user deleted the contact. + const id = h.send(plan.actions)!; + h.device.user.deleteContact(rawId); + await h.applyOk(h.planner.planDownload(h.card(id), await h.contact(rawId), h.ctx).ops); + expect(await h.contact(rawId)).toMatchObject({ sourceId: `${JMAP}/${id}`, pending: null, deleted: true }); + expect(await uploadPlan(h, rawId)).toEqual({ kind: 'upload', actions: [{ kind: 'destroy', id, uid: h.card(id).uid }] }); + }); + it('purges a new contact that never reached the server, and destroys a claimed one by uid', async () => { const h = new Harness(); const fresh = h.device.user.insertContact(ACCOUNT, [{ [Data.MIMETYPE]: MimeType.NOTE, data1: 'x' }]); diff --git a/src/device-sync/contacts/download.ts b/src/device-sync/contacts/download.ts index e6c0eafb..066605f6 100644 --- a/src/device-sync/contacts/download.ts +++ b/src/device-sync/contacts/download.ts @@ -288,8 +288,19 @@ export function planContactDownload(card: ContactCardWire, local: LocalContact | const identity = objectRef(ctx.jmapAccountId, server.id); if (!local) return insertPlan(server, ctx); const none = (stillDirty: boolean): DownloadPlan => ({ ops: { ref: identity, ops: [] }, conflicts: 0, stillDirty, effect: 'none', writes: 0 }); - // Deleted on the device: the delete wins and uploads as a destroy. - if (local.deleted) return none(true); + if (local.deleted) { + // Deleted on the device: the delete wins over a server edit, so the rows stay as they are. + // Only the shadow (and an identity never written) follows the card: the deletion uploads + // against the card as it is now, which may also sit in books this device does not sync. + const shadow = withMemberOf(shadowOf(server), shadowMemberOf(local.shadow)); + const raw = changedRawColumns(local, { + ...(local.sourceId ? {} : { [RawContacts.SOURCE_ID]: identity, [RawContacts.SYNC3]: null }), + [RawContacts.SYNC2]: JSON.stringify(shadow), + }); + if (!Object.keys(raw).length) return none(true); + const ops = [assertContact(local, local.dirty), updateRawContact(local.rawContactId, raw)]; + return { ops: { ref: identity, ops }, conflicts: 0, stillDirty: true, effect: 'update', writes: 1 }; + } if (!local.sourceId) { // Our own create whose identity was never written: adopt it. The rows stay diff --git a/src/device-sync/contacts/groups.ts b/src/device-sync/contacts/groups.ts index 6e9d8dae..c9d0b3f7 100644 --- a/src/device-sync/contacts/groups.ts +++ b/src/device-sync/contacts/groups.ts @@ -94,7 +94,13 @@ export function planGroupDownload(card: ContactCardWire, local: LocalGroup | nul values: { [Groups.SOURCE_ID]: identity, [Groups.TITLE]: remoteTitle ?? '', [Groups.GROUP_VISIBLE]: 1, [Groups.DIRTY]: 0, ...cardColumns(card, ctx) }, }], 'insert'); } - if (local.deleted) return plan([], 'none', 0, true); + if (local.deleted) { + // Deleted on the device: the delete wins over a server edit. Only the shadow (and an identity + // never written) follows the card, so the deletion uploads against the card as it is now. + const diff = changed(local, { ...(local.sourceId ? {} : { [Groups.SOURCE_ID]: identity, [Groups.SYNC3]: null }), ...cardColumns(card, ctx) }); + if (!Object.keys(diff).length) return plan([], 'none', 0, true); + return plan([assertGroup(local, local.dirty), updateGroup(local, diff)], 'update', 0, true); + } if (!local.sourceId) { // Our own create whose identity was never written. return plan([