mirror of
https://github.com/bulwarkmail/native.git
synced 2026-10-31 08:06:36 +00:00
fix: forget a contact created and deleted on the phone between two syncs #34
ContactsProvider soft-deletes every raw contact and group of a sync account, also one that never reached the server (DELETED=1, SOURCE_ID NULL). When a contact or group was created and deleted on the phone before a run claimed it, no upload phase took the row: without an identity or a pending create it belongs to no JMAP account, and the claim step skipped deleted rows. The row stayed for good, and the upload-only check and the teardown counted it as a waiting change: every upload sync the providers schedule became a full run, and turning sync off or signing out always asked about a change that would be lost. The claim step now hands such a row to the planner, whose plan for it is a purge, and `hasLocalWork` and `countPending` leave out deleted rows that have neither an identity nor a pending create. Nothing is uploaded for them. Calendar has no such gap: CalendarProvider hard-deletes an app-deleted event without `_SYNC_ID`, and a claimed event carries `~pending/<uid>`, so it is soft-deleted with its pending create and destroyed by uid as before.
This commit is contained in:
@@ -0,0 +1,97 @@
|
||||
// Items created on the device and deleted before any run claimed them.
|
||||
// ContactsProvider soft-deletes every raw contact and group of a sync account
|
||||
// (DELETED=1, SOURCE_ID NULL), CalendarProvider hard-deletes an event without
|
||||
// `_SYNC_ID` at once. Such an item never reached the server: it is purged,
|
||||
// nothing uploads, and it never counts as a change waiting for an upload.
|
||||
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { calendarPlanner } from '../../calendar/planner';
|
||||
import { contactsPlanner } from '../../contacts/planner';
|
||||
import {
|
||||
addDeviceContact,
|
||||
addServerCards,
|
||||
ANDROID_ACCOUNT,
|
||||
CALENDAR_AUTHORITY,
|
||||
CONTACTS_AUTHORITY,
|
||||
createHarness,
|
||||
type Harness,
|
||||
} from './harness';
|
||||
|
||||
function real(): Harness {
|
||||
const h = createHarness();
|
||||
h.deps.planners = { contacts: contactsPlanner, calendar: calendarPlanner };
|
||||
return h;
|
||||
}
|
||||
|
||||
/** Synced once, then a contact and a group created on the device and deleted again. */
|
||||
async function createdAndDeleted(): Promise<{ h: Harness; contact: number; group: number }> {
|
||||
const h = real();
|
||||
addServerCards(h, ['Ada Lovelace']);
|
||||
expect((await h.run()).outcome).toBe('ok');
|
||||
const contact = addDeviceContact(h, 'Hedy Lamarr');
|
||||
h.device.user.deleteContact(contact);
|
||||
const group = h.device.user.insertGroup(ANDROID_ACCOUNT, 'Friends');
|
||||
h.device.user.deleteGroup(group);
|
||||
return { h, contact, group };
|
||||
}
|
||||
|
||||
function offline(h: Harness): void {
|
||||
h.deps.jmap = async () => {
|
||||
const error = new Error('Network request failed');
|
||||
error.name = 'NetworkError';
|
||||
throw error;
|
||||
};
|
||||
}
|
||||
|
||||
describe('device sync engine: an item deleted on the device before it was claimed', () => {
|
||||
it('purges a contact and a group, and uploads nothing', async () => {
|
||||
const { h, contact, group } = await createdAndDeleted();
|
||||
|
||||
const report = await h.run();
|
||||
|
||||
expect(report).toMatchObject({ outcome: 'ok', itemErrors: [], stats: { uploaded: { created: 0, updated: 0, deleted: 0 } } });
|
||||
expect(h.device.row('raw_contacts', contact)).toBeUndefined();
|
||||
expect(h.device.rows('data', 'raw_contact_id = ?', [contact])).toEqual([]);
|
||||
expect(h.device.row('groups', group)).toBeUndefined();
|
||||
expect(h.server.all('ContactCard', 'a').map((c) => c.name)).toEqual([{ full: 'Ada Lovelace' }]);
|
||||
});
|
||||
|
||||
it('does not turn an upload-only sync into a full one', async () => {
|
||||
const { h } = await createdAndDeleted();
|
||||
const requests = h.server.requests.length;
|
||||
|
||||
expect((await h.run(CONTACTS_AUTHORITY, { upload: true })).outcome).toBe('ok');
|
||||
|
||||
expect(h.server.requests.length).toBe(requests);
|
||||
});
|
||||
|
||||
it('is no change that turning sync off would lose, also while offline', async () => {
|
||||
const { h } = await createdAndDeleted();
|
||||
offline(h);
|
||||
|
||||
expect(await h.teardown(CONTACTS_AUTHORITY)).toEqual({ pending: 0 });
|
||||
|
||||
expect(h.device.rows('raw_contacts')).toEqual([]);
|
||||
expect(h.device.rows('groups')).toEqual([]);
|
||||
});
|
||||
|
||||
it('leaves nothing behind for an event: CalendarProvider removes it at once', async () => {
|
||||
const h = real();
|
||||
expect((await h.run(CALENDAR_AUTHORITY)).outcome).toBe('ok');
|
||||
const calendarRow = Number(h.device.rows('calendars')[0]._id);
|
||||
const event = h.device.user.insertEvent(calendarRow, {
|
||||
title: 'Dentist',
|
||||
dtstart: Date.UTC(2026, 9, 1, 8),
|
||||
dtend: Date.UTC(2026, 9, 1, 9),
|
||||
eventTimezone: 'Europe/Berlin',
|
||||
});
|
||||
h.device.user.deleteEvent(event);
|
||||
expect(h.events()).toEqual([]);
|
||||
const requests = h.server.requests.length;
|
||||
|
||||
expect((await h.run(CALENDAR_AUTHORITY, { upload: true })).outcome).toBe('ok');
|
||||
expect(h.server.requests.length).toBe(requests);
|
||||
offline(h);
|
||||
expect(await h.teardown(CALENDAR_AUTHORITY)).toEqual({ pending: 0 });
|
||||
});
|
||||
});
|
||||
@@ -886,13 +886,15 @@ export abstract class ItemSync {
|
||||
/**
|
||||
* New items get their uid and target written before anything is sent. A
|
||||
* claim whose collection is gone from the server is made again where the
|
||||
* kind allows it (contacts: nothing of it can exist in a deleted book).
|
||||
* kind allows it (contacts: nothing of it can exist in a deleted book). A
|
||||
* new item deleted before it was claimed never reached the server, and no
|
||||
* upload phase takes it (it has no account): its plan is a purge.
|
||||
*/
|
||||
private async claimNewItems(): Promise<void> {
|
||||
const works: Work[] = [];
|
||||
for (const held of await this.loadUploadItems()) {
|
||||
const meta = held.kind.meta(held.local);
|
||||
if (!meta.isNew || meta.deleted || !this.claimable(held)) continue;
|
||||
if (!meta.isNew || (meta.deleted && meta.pending) || !this.claimable(held)) continue;
|
||||
const reclaim = !!meta.pending && !!held.kind.unclaimed && this.collectionGone(meta.pending.target);
|
||||
if (meta.pending && !reclaim) continue;
|
||||
const ref = `row:${meta.rowId}`;
|
||||
|
||||
@@ -17,6 +17,7 @@ import {
|
||||
type RunPayload,
|
||||
type RunReport,
|
||||
type RunStatus,
|
||||
type Row,
|
||||
} from '../types';
|
||||
import { BatchWriter } from './batch';
|
||||
import { CalendarSync } from './calendar-sync';
|
||||
@@ -26,7 +27,7 @@ import type { RunEnv } from './context';
|
||||
import { DEFAULT_TUNING, type EngineDeps, type Tuning } from './deps';
|
||||
import { classifyFailure, RunAbort } from './errors';
|
||||
import { acquireLock, lockKey, stopRequested } from './mutex';
|
||||
import { ProviderReader, str } from './provider';
|
||||
import { flag, ProviderReader, str } from './provider';
|
||||
import { isQuietRun, ReportBuilder, statusOf, statusToRecord } from './report';
|
||||
import { emptySyncState, parseSyncState, StateStore, type SyncState } from './sync-state';
|
||||
import type { ItemSync } from './item-sync';
|
||||
@@ -46,6 +47,15 @@ function itemTypeOf(authority: Authority): string {
|
||||
return authority === CONTACTS_AUTHORITY ? 'ContactCard' : 'CalendarEvent';
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a dirty, deleted or new raw contact or group has anything to
|
||||
* upload. One created and deleted on the device before a run claimed it (no
|
||||
* identity, no pending create) never reached the server: a run purges it.
|
||||
*/
|
||||
export function waitsForUpload(row: Row): boolean {
|
||||
return !!str(row.sourceid) || !!str(row.sync3) || !flag(row.deleted);
|
||||
}
|
||||
|
||||
/** Rows waiting for an upload (dirty, deleted, new), or state work a run must do whatever its extras. */
|
||||
export async function hasLocalWork(reader: ProviderReader, authority: Authority, state: SyncState): Promise<boolean> {
|
||||
for (const account of Object.values(state.accounts)) {
|
||||
@@ -55,11 +65,11 @@ export async function hasLocalWork(reader: ProviderReader, authority: Authority,
|
||||
const rows = await reader.rows('events', ['_id'], "dirty = 1 OR deleted = 1 OR _sync_id IS NULL OR _sync_id LIKE '~pending/%'");
|
||||
return rows.length > 0;
|
||||
}
|
||||
const contacts = await reader.rows('raw_contacts', ['_id'], 'dirty = 1 OR deleted = 1 OR sourceid IS NULL');
|
||||
if (contacts.length) return true;
|
||||
const groups = await reader.rows('groups', ['_id', 'sourceid', 'dirty', 'deleted']);
|
||||
const contacts = await reader.rows('raw_contacts', ['_id', 'sourceid', 'deleted', 'sync3'], 'dirty = 1 OR deleted = 1 OR sourceid IS NULL');
|
||||
if (contacts.some(waitsForUpload)) return true;
|
||||
const groups = await reader.rows('groups', ['_id', 'sourceid', 'dirty', 'deleted', 'sync3']);
|
||||
const present = new Set(groups.map((g) => str(g.sourceid)).filter(Boolean));
|
||||
if (groups.some((g) => !str(g.sourceid) || Number(g.dirty) === 1 || Number(g.deleted) === 1)) return true;
|
||||
if (groups.some((g) => (!str(g.sourceid) || Number(g.dirty) === 1 || Number(g.deleted) === 1) && waitsForUpload(g))) return true;
|
||||
// A group an app hard-deleted is only found by its absence.
|
||||
return Object.values(state.accounts).some((a) => (a.groups ?? []).some((ref) => !present.has(ref)));
|
||||
}
|
||||
|
||||
@@ -12,7 +12,7 @@ import type { EngineDeps } from './deps';
|
||||
import { acquireLock, clearStopRequest, lockKey, requestStop } from './mutex';
|
||||
import { ProviderReader, str } from './provider';
|
||||
import { ReportBuilder } from './report';
|
||||
import { createEnv, tuningOf } from './run';
|
||||
import { createEnv, tuningOf, waitsForUpload } from './run';
|
||||
import { parseSyncState } from './sync-state';
|
||||
|
||||
export interface TeardownOptions {
|
||||
@@ -36,12 +36,12 @@ export async function countPending(reader: ProviderReader, authority: Authority)
|
||||
const masters = new Set(rows.map((r) => str(r.original_id) ?? str(r.original_sync_id) ?? `row:${String(r._id)}`));
|
||||
return masters.size;
|
||||
}
|
||||
const contacts = await reader.rows('raw_contacts', ['_id'], 'dirty = 1 OR deleted = 1 OR sourceid IS NULL');
|
||||
const groups = await reader.rows('groups', ['_id', 'sourceid', 'dirty', 'deleted']);
|
||||
const contacts = await reader.rows('raw_contacts', ['_id', 'sourceid', 'deleted', 'sync3'], 'dirty = 1 OR deleted = 1 OR sourceid IS NULL');
|
||||
const groups = await reader.rows('groups', ['_id', 'sourceid', 'dirty', 'deleted', 'sync3']);
|
||||
const present = new Set(groups.map((g) => str(g.sourceid)).filter(Boolean));
|
||||
const waitingGroups = groups.filter((g) => !str(g.sourceid) || Number(g.dirty) === 1 || Number(g.deleted) === 1).length;
|
||||
const waitingGroups = groups.filter((g) => (!str(g.sourceid) || Number(g.dirty) === 1 || Number(g.deleted) === 1) && waitsForUpload(g)).length;
|
||||
const absent = Object.values(state.accounts).reduce((n, a) => n + (a.groups ?? []).filter((ref) => !present.has(ref)).length, 0);
|
||||
return contacts.length + waitingGroups + absent;
|
||||
return contacts.filter(waitsForUpload).length + waitingGroups + absent;
|
||||
}
|
||||
|
||||
function deleteEverything(authority: Authority): ProviderOp[] {
|
||||
|
||||
Reference in New Issue
Block a user