fix: keep an address's parts in step with a one-line edit on the device #34

AOSP Contacts edits a postal address as one formatted line; the
provider then copies it into the street and empties the other parts.
The upload sent only the new `full`, so the components on the server
kept the old values: CardDAV clients and the app's editor, which show
the parts, still showed the old city.

A changed line now uploads with the parts it implies. They are read
back along the old line: each of its segments must be some of the old
parts, and a segment whose change is confined to one part rewrites
that part like an in-place edit of its field. When the layout changed
or a change spans two parts, the address keeps the whole line as its
street, which is what the device holds, rather than an address
without parts that the app's editor would show empty and drop on its
next save. An unchanged line still uploads nothing, and editors that
write the parts themselves are handled as before.

Checked with planner tests for the edit from the device test, a
street-only edit, the fallback, a re-inserted line and an editor
that keeps the parts, and with unit tests of the read-back.

src/device-sync/__tests__/contacts/units.test.ts

@@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest';
 import { Data, MimeType, PhoneType, RelationType } from '../../android-columns';
 import { contactsPlanner } from '../../contacts/planner';
 import { entryKey, orgKey, parseEntryKey, parseOrgKey } from '../../contacts/keys';
-import { editPostalComponents, postalCells } from '../../contacts/postal';
+import { editPostalComponents, postalCells, resplitPostal } from '../../contacts/postal';
 import { base64ToBytes, dataUriBase64, shadowOf } from '../../contacts/photo';
 import { fullIsDerived, nameCells } from '../../contacts/name';
 import { phoneFlags, phoneType, relationName, relationType } from '../../contacts/types-map';
@@ -118,6 +118,24 @@ describe('contacts name and postal helpers', () => {
     expect(fullIsDerived({ components, full: 'B, A' })).toBe(false);
   });
 
+  it('reads the parts of a rewritten one-line address back along the old line', () => {
+    // Apple's layout: street / city region postcode / country.
+    const old = { data1: '1 Infinite Loop\nCupertino CA 95014\nUSA', data4: '1 Infinite Loop', data7: 'Cupertino', data8: 'CA', data9: '95014', data10: 'USA' };
+    const parts = (line: string) => {
+      const out = resplitPostal(old, line);
+      return out && { street: out.data4, city: out.data7, region: out.data8, postcode: out.data9, country: out.data10 };
+    };
+    expect(parts('1 Apple Park Way\nCupertino CA 95014\nUSA')).toEqual({ street: '1 Apple Park Way', city: 'Cupertino', region: 'CA', postcode: '95014', country: 'USA' });
+    expect(parts('1 Infinite Loop\nSan Jose CA 95014\nUSA')).toMatchObject({ city: 'San Jose', region: 'CA', postcode: '95014' });
+    expect(parts('1 Infinite Loop\nCupertino 95014\nUSA')).toMatchObject({ city: 'Cupertino', region: null, postcode: '95014' });
+    expect(parts('1 Infinite Loop, Cupertino CA 95014, USA')).toMatchObject({ street: '1 Infinite Loop', city: 'Cupertino' });
+    // Two parts of one segment changed, or the layout changed: nothing to pin the words to.
+    expect(parts('1 Infinite Loop\nSan Jose NV 95014\nUSA')).toBeNull();
+    expect(parts('1 Infinite Loop\nCupertino CA 95014\nCalifornia\nUSA')).toBeNull();
+    // An old line that isn't made of the parts (a `full` written apart from its components).
+    expect(resplitPostal({ ...old, data1: 'Apple HQ, Cupertino' }, 'Apple Park, Cupertino')).toBeNull();
+  });
+
   it('keeps separators between street parts and falls back to the legacy flat fields', () => {
     expect(postalCells({ street: 'Old Rd 1', locality: 'Town' })).toMatchObject({ data4: 'Old Rd 1', data7: 'Town' });
     const address = { components: [{ kind: 'number', value: '12' }, { kind: 'separator', value: '-' }, { kind: 'name', value: 'Rue X' }] };

src/device-sync/__tests__/contacts/upload-fields.test.ts

@@ -108,6 +108,113 @@ describe('contacts upload: one edited field, one pointer', () => {
   });
 });
 
+describe('contacts upload: an address edited as one line (AOSP Contacts)', () => {
+  const munich = () => ({
+    name: { full: 'Maria Muster' },
+    addresses: {
+      home: {
+        contexts: { private: true },
+        coordinates: 'geo:48.137,11.575',
+        components: [
+          { kind: 'name', value: 'Marienplatz 1' },
+          { kind: 'postcode', value: '80331' },
+          { kind: 'locality', value: 'Munich' },
+          { kind: 'region', value: 'Bavaria' },
+          { kind: 'country', value: 'Germany' },
+        ],
+      },
+    },
+  });
+  const LINE = 'Marienplatz 1, 80331 Munich, Bavaria, Germany';
+
+  /** AOSP writes the formatted address alone; the provider keeps it in STREET and empties the other parts. */
+  async function editLine(line: string) {
+    const h = new Harness();
+    const { id, rawId } = await h.seed(munich());
+    const row = h.rowsByKey(rawId)['addresses:home'];
+    expect(row.data1).toBe(LINE);
+    h.device.user.updateData(row._id as number, { data1: line, data4: null, data5: null, data6: null, data7: null, data8: null, data9: null, data10: null });
+    expect(h.rowsByKey(rawId)['addresses:home']).toMatchObject({ data1: line, data4: line, data7: null });
+    return { h, id, rawId };
+  }
+
+  async function patchOf(h: Harness, rawId: number): Promise<PatchObject> {
+    const plan = h.planner.planUpload(await h.contact(rawId), h.ctx);
+    if (plan.kind !== 'upload' || plan.actions[0].kind !== 'update') throw new Error(`expected an update, got ${plan.kind}`);
+    return plan.actions[0].patch;
+  }
+
+  const components = (street: string, locality = 'Munich', postcode = '80331') => [
+    { kind: 'name', value: street },
+    { kind: 'postcode', value: postcode },
+    { kind: 'locality', value: locality },
+    { kind: 'region', value: 'Bavaria' },
+    { kind: 'country', value: 'Germany' },
+  ];
+
+  it('reads the parts back from the new line, so `full` and the components agree', async () => {
+    const line = 'Industrieweg 42, 80331 Augsburg, Bavaria, Germany';
+    const { h, id, rawId } = await editLine(line);
+    expect(await patchOf(h, rawId)).toEqual({
+      'addresses/home/components': components('Industrieweg 42', 'Augsburg'),
+      'addresses/home/full': line,
+    });
+    await h.upload(rawId);
+    expect(h.card(id).addresses!.home).toEqual({
+      contexts: { private: true }, coordinates: 'geo:48.137,11.575', isOrdered: true, full: line,
+      components: components('Industrieweg 42', 'Augsburg'),
+    });
+    // The device gets its parts back and settles.
+    expect(h.rowsByKey(rawId)['addresses:home']).toMatchObject({ data1: line, data4: 'Industrieweg 42', data7: 'Augsburg', data9: '80331' });
+    const local = await h.contact(rawId);
+    expect(local.dirty).toBe(false);
+    expect(h.planner.planDownload(h.card(id), local, h.ctx).effect).toBe('none');
+  });
+
+  it('changes only the part whose words changed', async () => {
+    const { h, rawId } = await editLine('Marienplatz 2, 80331 Munich, Bavaria, Germany');
+    expect(await patchOf(h, rawId)).toEqual({
+      'addresses/home/components': components('Marienplatz 2'),
+      'addresses/home/full': 'Marienplatz 2, 80331 Munich, Bavaria, Germany',
+    });
+  });
+
+  it('keeps the whole line as the street when the change can not be pinned to one part', async () => {
+    const line = 'Industrieweg 42, 86150 Augsburg, Bavaria, Germany';
+    const { h, id, rawId } = await editLine(line);
+    expect(await patchOf(h, rawId)).toEqual({ 'addresses/home/components': [{ kind: 'name', value: line }], 'addresses/home/full': line });
+    await h.upload(rawId);
+    expect(h.card(id).addresses!.home).toMatchObject({ contexts: { private: true }, coordinates: 'geo:48.137,11.575', full: line, components: [{ kind: 'name', value: line }] });
+    const local = await h.contact(rawId);
+    expect(local.dirty).toBe(false);
+    expect(h.planner.planDownload(h.card(id), local, h.ctx).effect).toBe('none');
+  });
+
+  it('uploads nothing for the same line saved again', async () => {
+    const { h, rawId } = await editLine(LINE);
+    expect(h.planner.planUpload(await h.contact(rawId), h.ctx).kind).toBe('clean');
+  });
+
+  it('reads the parts back from a line an editor re-inserted without its key', async () => {
+    const line = 'Marienplatz 1, 80331 Munich, Upper Bavaria, Germany';
+    const h = new Harness();
+    const { rawId } = await h.seed(munich());
+    h.device.user.fossifySave(rawId, null, [{ [Data.MIMETYPE]: MimeType.STRUCTURED_POSTAL, data1: line, data2: 1 }]);
+    expect(await patchOf(h, rawId)).toEqual({
+      'addresses/home/components': components('Marienplatz 1').map((c) => (c.kind === 'region' ? { ...c, value: 'Upper Bavaria' } : c)),
+      'addresses/home/full': line,
+    });
+  });
+
+  it('edits the parts an editor kept as parts, whatever its line says', async () => {
+    const line = 'Marienplatz 1, 80331 Augsburg, Bavaria, Germany';
+    const h = new Harness();
+    const { rawId } = await h.seed(munich());
+    h.device.user.updateData(h.rowsByKey(rawId)['addresses:home']._id as number, { data7: 'Augsburg', data1: line });
+    expect(await patchOf(h, rawId)).toEqual({ 'addresses/home/components': components('Marienplatz 1', 'Augsburg'), 'addresses/home/full': line });
+  });
+});
+
 describe('contacts upload: the name', () => {
   const NAME_KEY = 'name';
 

src/device-sync/contacts/diff.ts

@@ -10,29 +10,49 @@ import { CommonKind, Data, MimeType, StructuredPostal as SP } from '../android-c
 import type { Row } from '../types';
 import { changedFromBaseline, sameCell, text } from './cells';
 import type { Match } from './matching';
-import { POSTAL_PART_COLUMNS } from './postal';
+import { POSTAL_PART_COLUMNS, providerSplitPostal, resplitPostal } from './postal';
 import { SPECS, type Unit } from './project';
 
+const isPart = (column: string) => (POSTAL_PART_COLUMNS as readonly string[]).includes(column);
+
+/** A postal row holding the provider's split of a formatted address an editor wrote alone. */
+function isProviderSplit(m: Match): boolean {
+  const { row } = m;
+  if (row.mimetype !== MimeType.STRUCTURED_POSTAL || !providerSplitPostal(row.cells)) return false;
+  // A multi-part formatted address in STREET was the provider's split, not a typed street.
+  return m.how !== 'key' || !row.baseline || /[,\n]/.test(text(row.cells[SP.FORMATTED_ADDRESS]) ?? '');
+}
+
+/** The address as the device showed it before the edit: a keyed row's baseline, else the shadow's. */
+function postalBefore(m: Match): Row {
+  return m.how === 'key' && m.row.baseline ? m.row.baseline : m.unit.cells;
+}
+
 /**
- * A postal row whose parts are only its formatted address copied into STREET:
- * the provider's split of an editor that wrote FORMATTED_ADDRESS alone.
+ * The cells an upload takes a row's values from: its own, except for a
+ * provider split whose line changed. Its parts are read back from the new
+ * line along the old one (`resplitPostal`), else they stay what the device
+ * holds, the whole line as the street: `full` and the components never
+ * contradict each other on the server.
  */
-function providerSplitPostal(cells: Row): boolean {
-  const street = text(cells[SP.STREET]);
-  return street !== null
-    && street === text(cells[SP.FORMATTED_ADDRESS])
-    && POSTAL_PART_COLUMNS.every((c) => c === SP.STREET || text(cells[c]) === null);
+export function uploadCells(m: Match): Row {
+  if (!isProviderSplit(m)) return m.row.cells;
+  const before = postalBefore(m);
+  const line = text(m.row.cells[SP.FORMATTED_ADDRESS]);
+  if (!line || sameCell(line, before[SP.FORMATTED_ADDRESS])) return m.row.cells;
+  return { ...m.row.cells, ...(resplitPostal(before, line) ?? {}) };
 }
 
 export function localChanges(m: Match): string[] {
-  const { row, unit } = m;
+  const { unit } = m;
   const changed = rawChanges(m).filter((c) => !unit.truncated?.includes(c));
-  if (row.mimetype === MimeType.STRUCTURED_POSTAL && providerSplitPostal(row.cells)) {
-    // A multi-part formatted address in STREET was the provider's split, not a typed street.
-    const split = m.how !== 'key' || !row.baseline || /[,\n]/.test(text(row.cells[SP.FORMATTED_ADDRESS]) ?? '');
-    if (split) return changed.filter((c) => !(POSTAL_PART_COLUMNS as readonly string[]).includes(c));
-  }
-  return changed;
+  if (!isProviderSplit(m)) return changed;
+  // The provider's split is no typed street: only a new line changes the parts, to what it says.
+  const others = changed.filter((c) => !isPart(c));
+  if (!others.includes(SP.FORMATTED_ADDRESS)) return others;
+  const before = postalBefore(m);
+  const after = uploadCells(m);
+  return [...others, ...POSTAL_PART_COLUMNS.filter((c) => !sameCell(after[c], before[c]))];
 }
 
 function rawChanges(m: Match): string[] {

src/device-sync/contacts/postal.ts

@@ -90,6 +90,80 @@ export function predictStoredPostal(cells: Row): Row {
   return out;
 }
 
+/**
+ * A postal row whose parts are only its formatted address copied into STREET:
+ * the provider's split of an editor that wrote FORMATTED_ADDRESS alone.
+ */
+export function providerSplitPostal(cells: Row): boolean {
+  const street = text(cells[SP.STREET]);
+  return street !== null
+    && street === text(cells[SP.FORMATTED_ADDRESS])
+    && POSTAL_PART_COLUMNS.every((c) => c === SP.STREET || text(cells[c]) === null);
+}
+
+/** Lines and comma-separated segments of a formatted address. */
+const SEGMENT = /\s*(?:,|\r?\n)\s*/;
+
+const wordsOf = (value: string) => value.split(/\s+/).filter(Boolean);
+
+function wordsAt(words: string[], part: string[], at: number): boolean {
+  return at >= 0 && at + part.length <= words.length && part.every((w, i) => words[at + i] === w);
+}
+
+/** The parts one segment of the old line is made of, in their order there (each part used once). */
+function segmentParts(segment: string[], parts: Map<string, string[]>, used: Set<string>): string[] | null {
+  if (!segment.length) return [];
+  for (const [column, words] of parts) {
+    if (used.has(column) || !wordsAt(segment, words, 0)) continue;
+    used.add(column);
+    const rest = segmentParts(segment.slice(words.length), parts, used);
+    if (rest) return [column, ...rest];
+    used.delete(column);
+  }
+  return null;
+}
+
+/**
+ * The parts of an address whose formatted line an editor rewrote alone (the
+ * provider then empties them on the device), read back along the old line:
+ * each segment of the old line must be the words of some old parts in a row,
+ * and the new line must have as many segments. A changed segment gives its
+ * words to its one part, or to the one part left once the parts at its start
+ * and end are found unchanged there. Null when the layout changed, or a change
+ * can't be pinned to one part.
+ */
+export function resplitPostal(old: Row, line: string): Row | null {
+  const before = text(old[SP.FORMATTED_ADDRESS]);
+  if (!before) return null;
+  const parts = new Map<string, string[]>();
+  for (const c of POSTAL_PART_COLUMNS) {
+    const value = text(old[c]);
+    if (value) parts.set(c, wordsOf(value));
+  }
+  const oldSegments = before.split(SEGMENT).map(wordsOf).filter((w) => w.length);
+  const newSegments = line.split(SEGMENT).map(wordsOf).filter((w) => w.length);
+  if (oldSegments.length !== newSegments.length) return null;
+  const out: Row = { [SP.FORMATTED_ADDRESS]: line };
+  for (const c of POSTAL_PART_COLUMNS) out[c] = text(old[c]);
+  const used = new Set<string>();
+  for (let i = 0; i < oldSegments.length; i++) {
+    const columns = segmentParts(oldSegments[i], parts, used);
+    if (!columns) return null;
+    const words = newSegments[i];
+    if (words.join(' ') === oldSegments[i].join(' ')) continue;
+    const size = (k: number) => parts.get(columns[k])!.length;
+    let first = 0;
+    let start = 0;
+    while (first < columns.length - 1 && wordsAt(words, parts.get(columns[first])!, start)) start += size(first++);
+    let last = columns.length;
+    let end = words.length;
+    while (last - 1 > first && end - size(last - 1) >= start && wordsAt(words, parts.get(columns[last - 1])!, end - size(last - 1))) end -= size(--last);
+    if (last - first !== 1) return null;
+    out[columns[first]] = words.slice(start, end).join(' ') || null;
+  }
+  return out;
+}
+
 /** The address's components with the edited columns applied (a list for `addresses/<k>/components`). */
 export function editPostalComponents(address: ContactAddress, cells: Row, changed: ReadonlySet<string>): Component[] {
   const comps = clone(components(address));

src/device-sync/contacts/upload.ts

@@ -22,7 +22,7 @@ import type {
 } from '../planner';
 import type { ProviderOp } from '../types';
 import { encodeBaseline } from './cells';
-import { localChanges } from './diff';
+import { localChanges, uploadCells } from './diff';
 import { accountOf, cardUnits, cleanWrite } from './download';
 import { newEntries, unitEdits, unitEntries, type Edit, type NewEntry } from './entries';
 import { parseEntryKey } from './keys';
@@ -88,7 +88,7 @@ function collectChanges(local: LocalContact, shadow: ContactCardWire, ctx: Ctx):
       const changed = localChanges(m);
       if (!changed.length) continue;
       const shown = kind.mimetype === MimeType.RELATION ? (m.unit.cells[Data.DATA1] as string | null) : null;
-      const e = unitEdits(kind.mimetype, m.unit.key, shadow, m.row.cells, m.unit.cells, new Set(changed), shown, m.how === 'key');
+      const e = unitEdits(kind.mimetype, m.unit.key, shadow, uploadCells(m), m.unit.cells, new Set(changed), shown, m.how === 'key');
       out.edits.push(...e.edits);
       out.added.push(...e.added);
       out.removed.push(...e.removed);