Match an LDIF re-import on the entry's dn

Reported again by the submitter's colleague at LINET after #223 was
closed: duplicate checking was implemented for vCard and never for LDIF,
so re-importing an address book still leaves a second copy of everything.
That was deliberate at the time -- the matching key was an open question
I did not want to answer alone -- but the answer had already been given
on #174 and I closed the issue without acting on it.

The answer, in the submitter's words: an attribute that *can* change is
fine, because it will not have changed between two imports minutes apart.
An import is not a sync. That makes the `dn` usable -- it is the only
identity the file carries, and Mozilla's schema defines no UID -- and it
needs no guessing at all, unlike the name-plus-email fallback I had been
weighing.

So `uidFromDn` derives a namespaced, stable uid from the distinguished
name, normalised for the case and spacing two exports of one directory
differ in. A card the book already holds under that uid is updated rather
than duplicated, merged the way the vCard import merges: what the file
carries wins, what it does not mention is left alone. Reported as created
and updated, which is the pair that was asked for.

Three things worth knowing:

Matching is per address book, so two customer directories that each hold
a `cn=John Smith` stay two people as long as they are filed separately.
Imported into one book they would merge, which is the one way this can be
wrong and the reason the escape hatch is worth naming.

The look-alike count stays, and now means something narrower: entries
that `dn` matching could not catch -- one whose `dn` moved between
exports, and anything imported before there was a `dn` to match on. Those
are still only counted, never merged.

A file holding two entries under one `dn` is malformed, since a directory
cannot, and now becomes one card instead of two sharing an identity.

FEATURES gains the re-import behaviour for both formats; it documented
neither.

web/src/store/__tests__/ldif-dedupe.test.ts

@@ -0,0 +1,154 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { CAP, client } from "@/jmap/client";
+import { useContacts } from "@/store/contacts";
+import type { ContactCard, JmapSession } from "@/jmap/types";
+
+/*
+ * Re-importing an LDIF address book you already have.
+ *
+ * Mozilla's schema has no UID, so for a long time an LDIF re-import duplicated
+ * everything -- reported on #174, tracked on #223, and reported again once the
+ * vCard half shipped without it. The entry's `dn` is the identity the file
+ * actually carries: not durable enough to be anyone's identity forever, and
+ * unchanged across the ten minutes between importing a migration, spotting a
+ * mistake, correcting the export and importing again, which is the only
+ * interval an import has to survive.
+ *
+ * Updated rather than skipped, because the reason to import a file twice is
+ * that the first attempt was not right.
+ */
+
+interface SetArgs { create?: Record<string, Record<string, unknown>>; update?: Record<string, Record<string, unknown>> }
+
+/** A card as the server holds it: what `scanBook` asks for, and nothing else. */
+const here = (id: string, uid: string, full: string, bookId = "book1") => ({
+  id, uid, addressBookIds: { [bookId]: true }, name: { full }, emails: {},
+}) as unknown as Partial<ContactCard> & { id: string };
+
+/** What `uidFromDn` makes of a `dn`, spelled out rather than imported, so a
+    change to the scheme has to be a deliberate one. */
+const uidFor = (dn: string) => `urn:x-ihasmail:ldif:${encodeURIComponent(dn)}`;
+
+function server(existing: Array<Partial<ContactCard> & { id: string }> = []) {
+  const sets: SetArgs[] = [];
+  const fetchMock = vi.fn(async (_url: string, init: RequestInit) => {
+    const body = JSON.parse(init.body as string) as { methodCalls: [string, Record<string, unknown>, string][] };
+    const methodResponses = body.methodCalls.map(([name, args, id]) => {
+      if (name === "ContactCard/query") {
+        const position = (args.position as number) ?? 0;
+        return [name, { accountId: "a1", queryState: "1", canCalculateChanges: false, position, ids: position ? [] : existing.map((c) => c.id), total: existing.length }, id];
+      }
+      if (name === "ContactCard/get") {
+        const want = new Set((args.ids as string[]) ?? []);
+        return [name, { accountId: "a1", state: "1", list: existing.filter((c) => want.has(c.id)), notFound: [] }, id];
+      }
+      if (name === "ContactCard/set") {
+        sets.push({
+          create: args.create as Record<string, Record<string, unknown>>,
+          update: args.update as Record<string, Record<string, unknown>>,
+        });
+        return [name, {
+          accountId: "a1", oldState: "1", newState: "2",
+          created: Object.fromEntries(Object.keys((args.create ?? {}) as object).map((k) => [k, { id: `new-${k}` }])),
+          updated: Object.fromEntries(Object.keys((args.update ?? {}) as object).map((k) => [k, null])),
+          notCreated: {}, notUpdated: {},
+        }, id];
+      }
+      return [name, { accountId: "a1", state: "1", list: [], notFound: [], ids: [], total: 0, queryState: "q", position: 0, canCalculateChanges: false }, id];
+    });
+    return { ok: true, status: 200, json: async () => ({ methodResponses, sessionState: "1" }) } as Response;
+  });
+  vi.stubGlobal("fetch", fetchMock);
+  return sets;
+}
+
+/** The same card, carrying the address likeness is read from. */
+const withEmail = (c: Partial<ContactCard> & { id: string }, address = "[email protected]") =>
+  ({ ...c, emails: { e0: { address } } }) as unknown as Partial<ContactCard> & { id: string };
+
+const JANE = "dn: cn=Jane Doe,ou=People\ngivenName: Jane\nsn: Doe\ncn: Jane Doe\nmail: [email protected]\n";
+
+beforeEach(() => {
+  client.session = {
+    capabilities: { [CAP.core]: { maxObjectsInGet: 500, maxObjectsInSet: 500 }, [CAP.contacts]: {} },
+    accounts: {}, primaryAccounts: {}, state: "s1",
+  } as unknown as JmapSession;
+  useContacts.setState({ accountId: "a1", available: true, books: {}, cards: {} as Record<string, ContactCard> });
+});
+
+afterEach(() => {
+  vi.unstubAllGlobals();
+  vi.restoreAllMocks();
+});
+
+describe("re-importing an LDIF address book", () => {
+  it("updates the card an entry's dn already names, rather than adding a second", async () => {
+    const sets = server([here("c1", uidFor("cn=jane doe,ou=people"), "Jane Doe")]);
+    const r = await useContacts.getState().importLdif(JANE, "book1");
+    expect(r).toEqual({ created: 0, updated: 1, alike: 0 });
+    expect(sets[0]!.create).toEqual({});
+    expect(Object.keys(sets[0]!.update!)).toEqual(["c1"]);
+  });
+
+  it("carries the file's version of the entry into the existing card", async () => {
+    const sets = server([here("c1", uidFor("cn=jane doe,ou=people"), "Jane Doe")]);
+    await useContacts.getState().importLdif(JANE.replace("sn: Doe", "sn: Doe-Smith").replace("cn: Jane Doe", "cn: Jane Doe-Smith"), "book1");
+    expect(sets[0]!.update!.c1!.name).toMatchObject({ full: "Jane Doe-Smith" });
+  });
+
+  it("does not move the card into the book being imported into", async () => {
+    // A contact filed in two books stays filed in both. Naming the target book
+    // in an update would quietly refile it.
+    const sets = server([here("c1", uidFor("cn=jane doe,ou=people"), "Jane Doe")]);
+    await useContacts.getState().importLdif(JANE, "book1");
+    expect(sets[0]!.update!.c1!).not.toHaveProperty("addressBookIds");
+  });
+
+  it("creates an entry whose dn is not here yet", async () => {
+    const sets = server([here("c1", uidFor("cn=someone else,ou=people"), "Someone Else")]);
+    const r = await useContacts.getState().importLdif(JANE, "book1");
+    expect(r).toEqual({ created: 1, updated: 0, alike: 0 });
+    expect(Object.keys(sets[0]!.create!)).toHaveLength(1);
+  });
+
+  it("ignores the case and spacing two exports of one directory differ in", async () => {
+    const sets = server([here("c1", uidFor("cn=jane doe,ou=people"), "Jane Doe")]);
+    const spaced = JANE.replace("dn: cn=Jane Doe,ou=People", "dn: CN = Jane Doe , OU = People");
+    await useContacts.getState().importLdif(spaced, "book1");
+    expect(Object.keys(sets[0]!.update!)).toEqual(["c1"]);
+  });
+
+  it("matches only inside the book being imported into", async () => {
+    // Two customers' directories can each hold a cn=John Smith. Filed in two
+    // books they stay two people; this is the escape hatch for that.
+    const sets = server([here("c1", uidFor("cn=jane doe,ou=people"), "Jane Doe", "book2")]);
+    const r = await useContacts.getState().importLdif(JANE, "book1");
+    expect(r.created).toBe(1);
+    expect(sets[0]!.update).toEqual({});
+  });
+
+  it("still counts a look-alike whose dn moved, and imports it anyway", async () => {
+    // The same person under a different branch of the directory: nothing to
+    // match on, so it arrives as a new card. Reported, never merged -- name
+    // plus address is a guess, and a merge made on a guess cannot be undone.
+    server([withEmail(here("c1", uidFor("cn=jane doe,ou=staff"), "Jane Doe"))]);
+    const r = await useContacts.getState().importLdif(JANE, "book1");
+    expect(r).toEqual({ created: 1, updated: 0, alike: 1 });
+  });
+
+  it("does not also count an entry it matched as a look-alike", async () => {
+    // It is not a card that looks like this one; it is this one.
+    server([withEmail(here("c1", uidFor("cn=jane doe,ou=people"), "Jane Doe"))]);
+    const r = await useContacts.getState().importLdif(JANE, "book1");
+    expect(r).toEqual({ created: 0, updated: 1, alike: 0 });
+  });
+
+  it("reports created and updated apart when a file holds both", async () => {
+    const sets = server([here("c1", uidFor("cn=jane doe,ou=people"), "Jane Doe")]);
+    const both = `${JANE}\ndn: cn=Alan Turing,ou=People\ncn: Alan Turing\nsn: Turing\nmail: [email protected]\n`;
+    const r = await useContacts.getState().importLdif(both, "book1");
+    expect(r).toEqual({ created: 1, updated: 1, alike: 0 });
+    // One call, not one per kind: creates and updates share Stalwart's budget.
+    expect(sets).toHaveLength(1);
+  });
+});

web/src/store/__tests__/ldif-import.test.ts

@@ -86,15 +86,22 @@ describe("importing an LDIF address book", () => {
     expect(first.name).toMatchObject({ full: "Jane Doe" });
   });
 
-  it("gives each contact an identity of its own, not the entry's directory name", async () => {
+  it("gives each contact an identity derived from its entry, and a distinct one", async () => {
     const sets = server();
     await useContacts.getState().importLdif(TWO, "book1");
     const uids = Object.values(sets[0]!.create!).map((c) => c.uid as string);
-    expect(uids.every((u) => typeof u === "string" && u.length > 0)).toBe(true);
     expect(new Set(uids).size).toBe(2);
-    // A distinguished name says where an entry sat in somebody else's
-    // directory, and must not become the contact's identity here.
-    expect(uids.some((u) => u.includes("cn="))).toBe(false);
+    // Namespaced, so it is never mistaken for a UID a vCard author meant, and
+    // stable, so importing the same file again recognises these.
+    expect(uids.every((u) => u.startsWith("urn:x-ihasmail:ldif:"))).toBe(true);
+  });
+
+  it("gives an entry with no usable dn an identity of its own", async () => {
+    const sets = server();
+    await useContacts.getState().importLdif("dn:\ncn: Nameless Place\nmail: [email protected]\n", "book1");
+    const uid = Object.values(sets[0]!.create!)[0]!.uid as string;
+    expect(uid).not.toContain("urn:x-ihasmail:ldif:");
+    expect(uid.length).toBeGreaterThan(0);
   });
 
   it("says a file held no contacts rather than reporting none imported", async () => {

web/src/store/__tests__/ldif-likeness.test.ts

@@ -128,11 +128,23 @@ describe("telling somebody what an LDIF re-import duplicated", () => {
   });
 
   it("does not count the file against itself", async () => {
-    // Two of the same person in one file are two new cards, not a duplicate of
+    // Two people in one file are two new cards, neither a duplicate of
     // something that was already here. The scan is read before anything lands.
     server([]);
-    const twice = entry("Jane Doe", "[email protected]") + "\n" + entry("Jane Doe", "jane@example.com");
-    const r = await useContacts.getState().importLdif(twice, "book1");
+    const two = entry("Jane Doe", "[email protected]") + "\n" + entry("Alan Turing", "alan@example.org");
+    const r = await useContacts.getState().importLdif(two, "book1");
     expect(r).toEqual({ created: 2, updated: 0, alike: 0 });
   });
+
+  it("makes one card of two entries in a file that share a dn", async () => {
+    // A directory cannot hold two entries under one name, so a file that does
+    // is malformed -- and must not produce two cards sharing an identity,
+    // which is the duplication this all exists to prevent. The later wins.
+    const sets = server([]);
+    const twice = entry("Jane Doe", "[email protected]") + "\n" + entry("Jane Doe", "[email protected]");
+    const r = await useContacts.getState().importLdif(twice, "book1");
+    expect(r).toEqual({ created: 1, updated: 0, alike: 0 });
+    const only = Object.values(sets[0]!.create!)[0]!;
+    expect(Object.values(only.emails as Record<string, { address: string }>)[0]!.address).toBe("[email protected]");
+  });
 });

web/src/store/__tests__/vcard-dedupe.test.ts

@@ -9,8 +9,8 @@ import type { ContactCard, JmapSession, UploadResponse } from "@/jmap/types";
  * A vCard UID is an identity its author meant, so a card whose UID a book
  * already holds is that card and importing it again used to leave a second
  * copy. Reported on #174 by the reporter's colleague, decided on #173 for
- * events, tracked as #223. The LDIF half is deliberately absent -- Mozilla's
- * schema has no UID, so the import invents one and there is nothing to match.
+ * events, tracked as #223. The LDIF half matches on the entry's `dn` instead,
+ * since Mozilla's schema has no UID; it is tested in `ldif-dedupe.test.ts`.
  */
 
 const MAX = 500;