diff --git a/src/device-sync/__tests__/engine/group-listing.test.ts b/src/device-sync/__tests__/engine/group-listing.test.ts index 16e47c49..5dadacb4 100644 --- a/src/device-sync/__tests__/engine/group-listing.test.ts +++ b/src/device-sync/__tests__/engine/group-listing.test.ts @@ -5,6 +5,7 @@ // row it inserted but never listed hides a later hard delete. import { describe, expect, it } from 'vitest'; +import { MimeType } from '../../android-columns'; import { calendarPlanner } from '../../calendar/planner'; import { contactsPlanner } from '../../contacts/planner'; import { serializeSyncState } from '../../engine/sync-state'; @@ -175,6 +176,34 @@ describe('device sync engine: the groups a SyncState lists', () => { expect(h.server.get('ContactCard', 'a', group)).toBeUndefined(); }); + it("moves a group's membership on the device even when time ran out after the group's chunk", async () => { + const h = real(); + const [ada, grace] = addServerCards(h, ['Ada Lovelace', 'Grace Hopper']); + const uid = (id: string) => h.server.get('ContactCard', 'a', id)!.uid as string; + const group = h.server.addCard('a', { uid: 'g1', kind: 'group', name: { full: 'Friends' }, members: { [uid(ada)]: true } }); + await h.run(); + const members = () => + h.device.rows('data') + .filter((d) => d.mimetype === MimeType.GROUP_MEMBERSHIP) + .map((d) => h.contacts().find((c) => c.id === Number(d.raw_contact_id))?.name) + .sort(); + expect(members()).toEqual(['Ada Lovelace']); + // Another client moves the membership from Ada to Grace: only the group card changes. + h.server.serverUpdate('ContactCard', 'a', group, { members: { [uid(grace)]: true } }); + // 'changes', 'download' (the group's chunk), then out of time before the members' chunk. + h.checkpoints.count = 0; + h.checkpoints.onCheckpoint = (n) => { + if (n === 2) h.clock.now += RUN_BUDGET; + }; + expect((await h.run()).outcome).toBe('cancelled'); + h.checkpoints.onCheckpoint = undefined; + + expect((await h.run()).outcome).toBe('ok'); + + expect(members()).toEqual(['Grace Hopper']); + expect(h.state()?.accounts.a.stale).toEqual([]); + }); + it('never destroys a listed group card that is in no synced book or no longer a group, and forgets it', async () => { const h = real(); const archive = h.server.addAddressBook('a', { name: 'Archive' }); diff --git a/src/device-sync/engine/contacts-sync.ts b/src/device-sync/engine/contacts-sync.ts index 0567e7bc..4642c817 100644 --- a/src/device-sync/engine/contacts-sync.ts +++ b/src/device-sync/engine/contacts-sync.ts @@ -123,6 +123,8 @@ export class ContactsSync extends ItemSync { private uidRefs: Map | null = null; /** Per account: raw contacts the upload phases looked at, for their memberships. */ private readonly uploadRows = new Map>(); + /** Group ref → ids of the contacts whose membership in it flips with the version being downloaded. */ + private readonly refreshes = new Map(); constructor(env: RunEnv) { super(env); @@ -409,27 +411,42 @@ export class ContactsSync extends ItemSync { }; } - /** Contacts of `acct` whose membership in one of these group cards flipped since the group's shadow. */ + /** + * Contacts of `acct` whose membership in one of these group cards flipped + * since the group's shadow; kept per group for `dependents`. + */ private async membersToRefresh(acct: string, groups: Array<{ id: string; members?: Record }>): Promise { if (!groups.length) return []; const locals = await this.groupKind.loadByRefs(groups.map((g) => refOf(acct, g.id))); - const flipped = new Set(); + const flippedBy = new Map>(); for (const group of groups) { - const before = new Set(onIds(locals.get(refOf(acct, group.id))?.shadow?.members as Record | undefined)); + const ref = refOf(acct, group.id); + const before = new Set(onIds(locals.get(ref)?.shadow?.members as Record | undefined)); const after = new Set(onIds(group.members)); - for (const uid of before) if (!after.has(uid)) flipped.add(uid); - for (const uid of after) if (!before.has(uid)) flipped.add(uid); + const flipped = new Set([...before].filter((uid) => !after.has(uid)).concat([...after].filter((uid) => !before.has(uid)))); + this.refreshes.delete(ref); + if (flipped.size) flippedBy.set(ref, flipped); } - if (!flipped.size) return []; + if (!flippedBy.size) return []; await this.ensureShadowIndex(); - const out: string[] = []; - for (const uid of flipped) { - for (const ref of this.uidRefs?.get(uid) ?? []) { - const id = idInAccount(ref, acct); - if (id) out.push(id); + const out = new Set(); + for (const [ref, flipped] of flippedBy) { + const ids: string[] = []; + for (const uid of flipped) { + for (const member of this.uidRefs?.get(uid) ?? []) { + const id = idInAccount(member, acct); + if (id) ids.push(id); + } } + this.refreshes.set(ref, ids); + for (const id of ids) out.add(id); } - return out; + return [...out]; + } + + /** The members a group card gained or lost: refreshed in this page, or first thing next run. */ + protected dependents(kind: Kind, acct: string, object: ServerObject): string[] { + return kind === this.groupKind ? this.refreshes.get(refOf(acct, object.id)) ?? [] : []; } protected async beforePlanning(acct: string, objects: ServerObject[]): Promise { diff --git a/src/device-sync/engine/item-sync.ts b/src/device-sync/engine/item-sync.ts index af3880b6..88c0bbe0 100644 --- a/src/device-sync/engine/item-sync.ts +++ b/src/device-sync/engine/item-sync.ts @@ -155,6 +155,23 @@ export abstract class ItemSync { protected listing(_kind: Kind, _acct: string, _id: string, _present: boolean): StateChange | undefined { return undefined; } + /** + * Objects whose rows a download of `object` makes outdated (contacts: the + * members a group card gained or lost). They go on `stale` in the batch + * that writes it, so a stop before their own chunk leaves them to the next + * run's first fetch. + */ + 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): 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 { @@ -658,7 +675,7 @@ export abstract class ItemSync { const group = local ? plan.ops : prependOps(plan.ops, [insertGuard(kind, ref)]); return { group, - state: this.listing(kind, acct, object.id, true), + state: both(this.listing(kind, acct, object.id, true), this.staleWith(acct, this.dependents(kind, acct, object))), fresh: true, applied: () => { if (plan.effect === 'insert') this.env.report.stats.downloaded.created++; @@ -694,7 +711,10 @@ export abstract class ItemSync { const plan = kind.planDownload(object, null, acct); return { group: concatGroups(ref, found.kind.planLocalDelete(found.local), plan.ops), - state: both(this.listing(found.kind, acct, object.id, false), this.listing(kind, acct, object.id, true)), + state: both( + both(this.listing(found.kind, acct, object.id, false), this.listing(kind, acct, object.id, true)), + this.staleWith(acct, this.dependents(kind, acct, object)), + ), applied: () => { this.env.report.stats.downloaded.updated++; this.markResolved(acct, object.id);