diff --git a/src/device-sync/__tests__/contacts/harness.ts b/src/device-sync/__tests__/contacts/harness.ts index 663dfbd0..27f1a243 100644 --- a/src/device-sync/__tests__/contacts/harness.ts +++ b/src/device-sync/__tests__/contacts/harness.ts @@ -4,7 +4,7 @@ * way the engine applies them (one op group per batch); uploads go through * the fake server's patch handling and come back through its normalisation. */ -import { GroupMembership, Groups, RawContacts } from '../../android-columns'; +import { Data, GroupMembership, Groups, RawContacts } from '../../android-columns'; import { makeKeyMinter, parseObjectRef, uuidFrom } from '../../common/ids'; import type { GroupRights } from '../../contacts/members'; import { contactsPlanner } from '../../contacts/planner'; @@ -17,7 +17,7 @@ import type { UploadAction, UploadPlan, } from '../../planner'; -import { CONTACTS_AUTHORITY, type BatchResult, type ProviderPort, type Row } from '../../types'; +import { CONTACTS_AUTHORITY, type BatchResult, type OpResult, type ProviderOp, type ProviderPort, type Row } from '../../types'; import { FakeJmapServer } from '../fakes/fake-jmap-server'; import { FakeDeviceProviders } from '../fakes/fake-provider'; @@ -35,6 +35,40 @@ function rowsOf(res: { columns: string[]; rows: unknown[][] }): Row[] { return res.rows.map((cells) => Object.fromEntries(res.columns.map((c, i) => [c, cells[i]])) as Row); } +/** + * ContactsProvider keeps a photo no larger than its 96 px thumbnail as the + * thumbnail only: PHOTO_FILE_ID (DATA14) stays NULL, and the thumbnail + * (DATA15) does not read back. The fake stores every photo with a display + * photo; this makes the photo rows a batch wrote thumbnail-only, as a device + * does with the test fixtures' 1×1 JPEGs. + */ +export function keepThumbnailsOnly(device: FakeDeviceProviders, ops: ProviderOp[], results: OpResult[]): void { + ops.forEach((op, i) => { + if ((op.op !== 'insert' && op.op !== 'update') || op.table !== 'data') return; + const bytes = op.values[Data.DATA15]; + if (!bytes || typeof bytes !== 'object') return; + const id = op.op === 'insert' ? results[i]?.id : op.id; + const row = id === undefined ? undefined : device.table('data').get(id); + if (row) row[Data.DATA14] = null; + }); +} + +/** A provider port whose photo writes come out thumbnail-only (see `keepThumbnailsOnly`). */ +export function thumbnailOnlyPort(device: FakeDeviceProviders, port: ProviderPort): ProviderPort { + return { + accountName: port.accountName, + authority: port.authority, + query: (q) => port.query(q), + readSyncState: () => port.readSyncState(), + readPhoto: (id, px) => port.readPhoto(id, px), + applyBatch: async (ops) => { + const res = await port.applyBatch(ops); + if (res.ok) keepThumbnailsOnly(device, ops, res.results); + return res; + }, + }; +} + export class Harness { readonly device = new FakeDeviceProviders(); readonly port: ProviderPort = this.device.port(ACCOUNT, CONTACTS_AUTHORITY); @@ -49,6 +83,8 @@ export class Harness { readonly unselected = new Set(); /** When false the context has no `groupsOf` (memberships not synced). */ groupIndex = true; + /** When true the photos the planner writes come out thumbnail-only (see `keepThumbnailsOnly`). */ + thumbnailPhotos = false; readonly ctx: ContactsContext & GroupRights; constructor() { @@ -98,7 +134,9 @@ export class Harness { async apply(group: OpGroup): Promise { if (!group.ops.length) return { ok: true, results: [] }; - return this.port.applyBatch(group.ops); + const res = await this.port.applyBatch(group.ops); + if (res.ok && this.thumbnailPhotos) keepThumbnailsOnly(this.device, group.ops, res.results); + return res; } async applyOk(group: OpGroup): Promise { diff --git a/src/device-sync/__tests__/contacts/lifecycle.test.ts b/src/device-sync/__tests__/contacts/lifecycle.test.ts index 9e69cd74..8d66eff4 100644 --- a/src/device-sync/__tests__/contacts/lifecycle.test.ts +++ b/src/device-sync/__tests__/contacts/lifecycle.test.ts @@ -181,6 +181,67 @@ describe('contacts lifecycle', () => { }); }); + describe('a small photo the provider keeps as its thumbnail only (no PHOTO_FILE_ID)', () => { + const photo = { ph: { kind: 'photo', uri: `data:image/jpeg;base64,${JPEG}`, mediaType: 'image/jpeg' } }; + const small = () => ({ name: { full: 'Photo Smalltest' }, emails: { e1: { address: 'small@example.org' } }, notes: { n1: { note: 'v1' } }, media: photo }); + const photos = (h: Harness, rawId: number) => h.device.rows('data', 'raw_contact_id = ? AND mimetype = ?', [rawId, MimeType.PHOTO]); + + it('keeps one row through downloads and merges, and never reads it as deleted', async () => { + const h = new Harness(); + h.thumbnailPhotos = true; + const { id, rawId } = await h.seed(small()); + expect(photos(h, rawId)).toMatchObject([{ data14: null, data_sync1: 'media:ph' }]); + await expectSettled(h, id, rawId); + for (const note of ['v2', 'v3']) { + h.server.serverUpdate('ContactCard', JMAP, id, { 'notes/n1/note': note }); + await h.download(id); + expect(photos(h, rawId)).toHaveLength(1); + } + // A merge into an edited contact. + h.device.user.updateData(h.rowsByKey(rawId)['emails:e1']._id as number, { data1: 'small2@example.org' }); + h.server.serverUpdate('ContactCard', JMAP, id, { 'notes/n1/note': 'v4' }); + await h.applyOk(h.planner.planDownload(h.card(id), await h.contact(rawId), h.ctx).ops); + expect(photos(h, rawId)).toHaveLength(1); + // An edit of another field uploads alone. + expect(h.planner.planUpload(await h.contact(rawId), h.ctx)).toEqual({ + kind: 'upload', actions: [{ kind: 'update', id, patch: { 'emails/e1/address': 'small2@example.org' } }], + }); + await h.upload(rawId); + expect(h.card(id).media).toEqual(photo); + expect(photos(h, rawId)).toHaveLength(1); + await expectSettled(h, id, rawId); + // A photo set on the device (it gets a display photo) replaces it. + h.device.user.setPhoto(rawId, JPEG2); + h.devicePhotos.set(rawId, JPEG2); + expect(h.planner.planUpload(await h.contact(rawId), h.ctx)).toEqual({ + kind: 'upload', actions: [{ kind: 'update', id, patch: { 'media/ph/uri': `data:image/jpeg;base64,${JPEG2}`, 'media/ph/mediaType': 'image/jpeg' } }], + }); + }); + + it('heals the photo rows the same photo was inserted as again, keeping one and the server photo', async () => { + const h = new Harness(); + h.thumbnailPhotos = true; + const { id, rawId } = await h.seed(small()); + const [row] = photos(h, rawId); + const again = { [Data.MIMETYPE]: MimeType.PHOTO, [Data.DATA15]: { b64: JPEG }, data_sync1: row.data_sync1, data_sync2: row.data_sync2, data_sync3: row.data_sync3 }; + for (let i = 0; i < 2; i++) await h.applyOk({ ref: 'again', ops: [{ op: 'insert', table: 'data', values: { ...again, [Data.RAW_CONTACT_ID]: rawId } }] }); + expect(photos(h, rawId)).toHaveLength(3); + // An edit on the device uploads without touching the photo, and its accepted write keeps one row. + h.device.user.updateData(h.rowsByKey(rawId)['notes:n1']._id as number, { data1: 'v2 edited on device' }); + expect(h.planner.planUpload(await h.contact(rawId), h.ctx)).toEqual({ + kind: 'upload', actions: [{ kind: 'update', id, patch: { 'notes/n1/note': 'v2 edited on device' } }], + }); + await h.upload(rawId); + expect(photos(h, rawId)).toHaveLength(1); + expect(h.card(id).media).toEqual(photo); + await expectSettled(h, id, rawId); + // An unchanged card downloaded again (a full reconcile) heals too. + for (let i = 0; i < 2; i++) await h.applyOk({ ref: 'again', ops: [{ op: 'insert', table: 'data', values: { ...again, [Data.RAW_CONTACT_ID]: rawId } }] }); + await h.applyOk(h.planner.planDownload(h.card(id), await h.contact(rawId), h.ctx).ops); + expect(photos(h, rawId)).toHaveLength(1); + }); + }); + it('heals a baseline the provider stored differently from the prediction', async () => { const h = new Harness(); const { id, rawId } = await h.seed({ name: { full: 'Dr. Jane Q. Public' } }); diff --git a/src/device-sync/__tests__/contacts/real-engine.test.ts b/src/device-sync/__tests__/contacts/real-engine.test.ts index f6a160a8..df779301 100644 --- a/src/device-sync/__tests__/contacts/real-engine.test.ts +++ b/src/device-sync/__tests__/contacts/real-engine.test.ts @@ -3,11 +3,13 @@ // themselves are tested in the other files of this folder. import { describe, expect, it } from 'vitest'; -import { Groups, MimeType } from '../../android-columns'; +import { Data, Groups, MimeType } from '../../android-columns'; import { calendarPlanner } from '../../calendar/planner'; import { contactsPlanner } from '../../contacts/planner'; import { addServerCards, CONTACTS_AUTHORITY, createHarness, rowWrites, type Harness } from '../engine/harness'; import type { SetTarget } from '../fakes/fake-jmap-server'; +import { JPEG } from './fixtures'; +import { thumbnailOnlyPort } from './harness'; function real(options: Parameters[0] = {}): Harness { const h = createHarness(options); @@ -78,6 +80,28 @@ describe('contacts through the engine', () => { expect(await h.teardown(CONTACTS_AUTHORITY)).toEqual({ pending: 0 }); }); + it('keeps one row for a small photo the provider stores as its thumbnail, and never deletes it on the server', async () => { + const h = real(); + const base = h.deps.provider; + h.deps.provider = (name, authority) => thumbnailOnlyPort(h.device, base(name, authority)); + const media = { ph: { kind: 'photo', uri: `data:image/jpeg;base64,${JPEG}`, mediaType: 'image/jpeg' } }; + const card = h.server.addCard('a', { uid: 'u-small', name: { full: 'Photo Smalltest' }, notes: { n1: { note: 'v1' } }, addressBookIds: { [h.book]: true }, media }); + expect((await h.run()).outcome).toBe('ok'); + const id = h.contactNamed('Photo Smalltest')!.id; + const rowsOf = (mimetype: string) => h.device.rows('data').filter((d) => Number(d.raw_contact_id) === id && d.mimetype === mimetype); + expect(rowsOf(MimeType.PHOTO)).toMatchObject([{ data14: null }]); + + for (const note of ['v2', 'v3']) { + h.server.serverUpdate('ContactCard', 'a', card, { 'notes/n1/note': note }); + expect((await h.run()).outcome).toBe('ok'); + expect(rowsOf(MimeType.PHOTO)).toHaveLength(1); + } + h.device.user.updateData(Number(rowsOf(MimeType.NOTE)[0]._id), { [Data.DATA1]: 'v4 edited on device' }); + expect(await h.run()).toMatchObject({ outcome: 'ok', itemErrors: [], stats: { uploaded: { updated: 1 } } }); + expect(h.server.get('ContactCard', 'a', card)).toMatchObject({ notes: { n1: { note: 'v4 edited on device' } }, media }); + expect(rowsOf(MimeType.PHOTO)).toHaveLength(1); + }); + it('syncs a contact whose photo is too large for a provider batch, without the photo', async () => { const h = real(); // About 900 KB of JPEG as a data: URI (a camera photo set over CardDAV). diff --git a/src/device-sync/contacts/project.ts b/src/device-sync/contacts/project.ts index b548fd13..1ff0a655 100644 --- a/src/device-sync/contacts/project.ts +++ b/src/device-sync/contacts/project.ts @@ -318,11 +318,16 @@ export function rowMatchesUnit(u: Pick, stored: Row, return spec.columns.every((c) => same(u.cells[c], stored[c], c)); } -/** Whether a row has nothing in it (an editor's blank nickname or note). */ +/** + * Whether a row has nothing in it (an editor's blank nickname or note). A + * photo row always holds a photo: ContactsProvider keeps one within its 96 px + * thumbnail as the thumbnail alone, without PHOTO_FILE_ID, and the thumbnail + * (DATA15, a blob) does not read back. + */ export function isEmptyRow(mimetype: string, cells: Row): boolean { const spec = SPECS[mimetype]; if (!spec) return true; - if (mimetype === MimeType.PHOTO) return cellText(cells[Data.DATA14]) === null; + if (mimetype === MimeType.PHOTO) return false; if (mimetype === MimeType.GROUP_MEMBERSHIP) return cellText(cells[GroupMembership.GROUP_SOURCE_ID]) === null && cellText(cells[Data.DATA1]) === null; return spec.valueColumns.every((c) => text(cells[c]) === null); }