Say how much an LDIF re-import duplicated, without acting on it

The half of #223 that can move while the matching question is still open.

Mozilla's schema defines no UID, so the import invents one and a re-import
duplicates everything. Whether to guess an identity from a name and an address
instead is the reporter's call and he has not made it -- but the harm that was
actually reported was confusion rather than duplication: somebody imports a
file twice and cannot tell what happened.

So the import now counts how many of the entries look like contacts the book
already held, and says so in a second message. Every card is still imported.
Nothing is skipped and nothing is merged, which is the point: counting is a
different act from matching, and it takes no decision away from the person who
still owes us one.

The likeness key is name plus one address, and it is wrong in both directions
by design -- two colleagues sharing a name and an alias collapse, somebody
whose address changed since the last export looks like a stranger. That is
tolerable for a number on a toast and would not be tolerable for a merge,
which is exactly why the number is all it does.

The scan the vCard import already makes for UIDs now collects names and
addresses on the same request, so this costs no extra round trip. It is read
before anything is created, so a file that repeats a person twice counts as
two new cards rather than as a duplicate of itself.

If the answer comes back "match on name and email", the matching is written
and becomes a skip instead of a count.

Refs #223.
This commit is contained in:
2026-09-02 12:45:30 -07:00
parent 0d5975d427
commit 9622875659
15 changed files with 229 additions and 24 deletions
+1
View File
@@ -1079,6 +1079,7 @@ export const catalog: Catalog = {
"Nothing unread here": "Hier ist nichts ungelesen", "Nothing unread here": "Hier ist nichts ungelesen",
}, },
plurals: { plurals: {
"{n} of them look like contacts you already had": { one: "{n} davon sieht aus wie ein Kontakt, den Sie schon hatten", other: "{n} davon sehen aus wie Kontakte, die Sie schon hatten" },
"Your administrator changed {n} settings": { one: "Ihre Administration hat {n} Einstellung geändert", other: "Ihre Administration hat {n} Einstellungen geändert" }, "Your administrator changed {n} settings": { one: "Ihre Administration hat {n} Einstellung geändert", other: "Ihre Administration hat {n} Einstellungen geändert" },
"Already here: {n} contacts, nothing imported": { one: "Bereits vorhanden: {n} Kontakt, nichts importiert", other: "Bereits vorhanden: {n} Kontakte, nichts importiert" }, "Already here: {n} contacts, nothing imported": { one: "Bereits vorhanden: {n} Kontakt, nichts importiert", other: "Bereits vorhanden: {n} Kontakte, nichts importiert" },
"All {n} are already in your contacts": { one: "Bereits in Ihren Kontakten", other: "Alle {n} sind bereits in Ihren Kontakten" }, "All {n} are already in your contacts": { one: "Bereits in Ihren Kontakten", other: "Alle {n} sind bereits in Ihren Kontakten" },
+1
View File
@@ -1052,6 +1052,7 @@ export const catalog: Catalog = {
"Nothing unread here": "Aquí no hay nada sin leer", "Nothing unread here": "Aquí no hay nada sin leer",
}, },
plurals: { plurals: {
"{n} of them look like contacts you already had": { one: "{n} de ellos se parece a un contacto que ya tenías", other: "{n} de ellos se parecen a contactos que ya tenías" },
"Your administrator changed {n} settings": { one: "Tu administración cambió {n} ajuste", other: "Tu administración cambió {n} ajustes" }, "Your administrator changed {n} settings": { one: "Tu administración cambió {n} ajuste", other: "Tu administración cambió {n} ajustes" },
"Already here: {n} contacts, nothing imported": { one: "Ya estaba aquí: {n} contacto, no se importó nada", other: "Ya estaban aquí: {n} contactos, no se importó nada" }, "Already here: {n} contacts, nothing imported": { one: "Ya estaba aquí: {n} contacto, no se importó nada", other: "Ya estaban aquí: {n} contactos, no se importó nada" },
"All {n} are already in your contacts": { one: "Ya está en tus contactos", other: "Los {n} ya están en tus contactos" }, "All {n} are already in your contacts": { one: "Ya está en tus contactos", other: "Los {n} ya están en tus contactos" },
+1
View File
@@ -1057,6 +1057,7 @@ export const catalog: Catalog = {
"Nothing unread here": "Rien de non lu ici", "Nothing unread here": "Rien de non lu ici",
}, },
plurals: { plurals: {
"{n} of them look like contacts you already had": { one: "{n} dentre eux ressemble à un contact que vous aviez déjà", other: "{n} dentre eux ressemblent à des contacts que vous aviez déjà" },
"Your administrator changed {n} settings": { one: "Votre administration a modifié {n} paramètre", other: "Votre administration a modifié {n} paramètres" }, "Your administrator changed {n} settings": { one: "Votre administration a modifié {n} paramètre", other: "Votre administration a modifié {n} paramètres" },
"Already here: {n} contacts, nothing imported": { one: "Déjà présent : {n} contact, rien dimporté", other: "Déjà présents : {n} contacts, rien dimporté" }, "Already here: {n} contacts, nothing imported": { one: "Déjà présent : {n} contact, rien dimporté", other: "Déjà présents : {n} contacts, rien dimporté" },
"All {n} are already in your contacts": { one: "Déjà dans vos contacts", other: "Les {n} sont déjà dans vos contacts" }, "All {n} are already in your contacts": { one: "Déjà dans vos contacts", other: "Les {n} sont déjà dans vos contacts" },
+1
View File
@@ -1060,6 +1060,7 @@ export const catalog: Catalog = {
"Nothing unread here": "ここに未読はありません", "Nothing unread here": "ここに未読はありません",
}, },
plurals: { plurals: {
"{n} of them look like contacts you already had": { other: "うち {n} 件はすでにある連絡先に似ています" },
"Your administrator changed {n} settings": { other: "管理者が {n} 件の設定を変更しました" }, "Your administrator changed {n} settings": { other: "管理者が {n} 件の設定を変更しました" },
"Already here: {n} contacts, nothing imported": { other: "すでに存在: {n} 件、インポートなし" }, "Already here: {n} contacts, nothing imported": { other: "すでに存在: {n} 件、インポートなし" },
"All {n} are already in your contacts": { other: "{n} 件はすでに連絡先にあります" }, "All {n} are already in your contacts": { other: "{n} 件はすでに連絡先にあります" },
+1
View File
@@ -1048,6 +1048,7 @@ export const catalog: Catalog = {
"Nothing unread here": "Hier is niets ongelezen", "Nothing unread here": "Hier is niets ongelezen",
}, },
plurals: { plurals: {
"{n} of them look like contacts you already had": { one: "{n} daarvan lijkt op een contact dat u al had", other: "{n} daarvan lijken op contacten die u al had" },
"Your administrator changed {n} settings": { one: "Uw beheerder heeft {n} instelling gewijzigd", other: "Uw beheerder heeft {n} instellingen gewijzigd" }, "Your administrator changed {n} settings": { one: "Uw beheerder heeft {n} instelling gewijzigd", other: "Uw beheerder heeft {n} instellingen gewijzigd" },
"Already here: {n} contacts, nothing imported": { one: "Al aanwezig: {n} contact, niets geïmporteerd", other: "Al aanwezig: {n} contacten, niets geïmporteerd" }, "Already here: {n} contacts, nothing imported": { one: "Al aanwezig: {n} contact, niets geïmporteerd", other: "Al aanwezig: {n} contacten, niets geïmporteerd" },
"All {n} are already in your contacts": { one: "Staat al in uw contacten", other: "Alle {n} staan al in uw contacten" }, "All {n} are already in your contacts": { one: "Staat al in uw contacten", other: "Alle {n} staan al in uw contacten" },
+1
View File
@@ -1055,6 +1055,7 @@ export const catalog: Catalog = {
"Nothing unread here": "Não há nada não lido aqui", "Nothing unread here": "Não há nada não lido aqui",
}, },
plurals: { plurals: {
"{n} of them look like contacts you already had": { one: "{n} deles parece um contato que você já tinha", other: "{n} deles parecem contatos que você já tinha" },
"Your administrator changed {n} settings": { one: "Sua administração alterou {n} configuração", other: "Sua administração alterou {n} configurações" }, "Your administrator changed {n} settings": { one: "Sua administração alterou {n} configuração", other: "Sua administração alterou {n} configurações" },
"Already here: {n} contacts, nothing imported": { one: "Já estava aqui: {n} contato, nada importado", other: "Já estavam aqui: {n} contatos, nada importado" }, "Already here: {n} contacts, nothing imported": { one: "Já estava aqui: {n} contato, nada importado", other: "Já estavam aqui: {n} contatos, nada importado" },
"All {n} are already in your contacts": { one: "Já está nos seus contatos", other: "Todos os {n} já estão nos seus contatos" }, "All {n} are already in your contacts": { one: "Já está nos seus contatos", other: "Todos os {n} já estão nos seus contatos" },
+1
View File
@@ -1054,6 +1054,7 @@ export const catalog: Catalog = {
"Nothing unread here": "Здесь нет непрочитанного", "Nothing unread here": "Здесь нет непрочитанного",
}, },
plurals: { plurals: {
"{n} of them look like contacts you already had": { one: "{n} из них похож на контакт, который уже был", few: "{n} из них похожи на контакты, которые уже были", many: "{n} из них похожи на контакты, которые уже были", other: "{n} из них похожи на контакты, которые уже были" },
"Your administrator changed {n} settings": { one: "Администратор изменил {n} настройку", few: "Администратор изменил {n} настройки", many: "Администратор изменил {n} настроек", other: "Администратор изменил {n} настройки" }, "Your administrator changed {n} settings": { one: "Администратор изменил {n} настройку", few: "Администратор изменил {n} настройки", many: "Администратор изменил {n} настроек", other: "Администратор изменил {n} настройки" },
"Already here: {n} contacts, nothing imported": { one: "Уже есть: {n} контакт, ничего не импортировано", few: "Уже есть: {n} контакта, ничего не импортировано", many: "Уже есть: {n} контактов, ничего не импортировано", other: "Уже есть: {n} контакта, ничего не импортировано" }, "Already here: {n} contacts, nothing imported": { one: "Уже есть: {n} контакт, ничего не импортировано", few: "Уже есть: {n} контакта, ничего не импортировано", many: "Уже есть: {n} контактов, ничего не импортировано", other: "Уже есть: {n} контакта, ничего не импортировано" },
"All {n} are already in your contacts": { one: "Уже в ваших контактах", few: "Все {n} уже в ваших контактах", many: "Все {n} уже в ваших контактах", other: "Все {n} уже в ваших контактах" }, "All {n} are already in your contacts": { one: "Уже в ваших контактах", few: "Все {n} уже в ваших контактах", many: "Все {n} уже в ваших контактах", other: "Все {n} уже в ваших контактах" },
+1
View File
@@ -1048,6 +1048,7 @@ export const catalog: Catalog = {
"Nothing unread here": "Тут немає непрочитаного", "Nothing unread here": "Тут немає непрочитаного",
}, },
plurals: { plurals: {
"{n} of them look like contacts you already had": { one: "{n} з них схожий на контакт, який уже був", few: "{n} з них схожі на контакти, які вже були", many: "{n} з них схожі на контакти, які вже були", other: "{n} з них схожі на контакти, які вже були" },
"Your administrator changed {n} settings": { one: "Адміністратор змінив {n} налаштування", few: "Адміністратор змінив {n} налаштування", many: "Адміністратор змінив {n} налаштувань", other: "Адміністратор змінив {n} налаштування" }, "Your administrator changed {n} settings": { one: "Адміністратор змінив {n} налаштування", few: "Адміністратор змінив {n} налаштування", many: "Адміністратор змінив {n} налаштувань", other: "Адміністратор змінив {n} налаштування" },
"Already here: {n} contacts, nothing imported": { one: "Уже є: {n} контакт, нічого не імпортовано", few: "Уже є: {n} контакти, нічого не імпортовано", many: "Уже є: {n} контактів, нічого не імпортовано", other: "Уже є: {n} контакти, нічого не імпортовано" }, "Already here: {n} contacts, nothing imported": { one: "Уже є: {n} контакт, нічого не імпортовано", few: "Уже є: {n} контакти, нічого не імпортовано", many: "Уже є: {n} контактів, нічого не імпортовано", other: "Уже є: {n} контакти, нічого не імпортовано" },
"All {n} are already in your contacts": { one: "Уже у ваших контактах", few: "Усі {n} уже у ваших контактах", many: "Усі {n} уже у ваших контактах", other: "Усі {n} уже у ваших контактах" }, "All {n} are already in your contacts": { one: "Уже у ваших контактах", few: "Усі {n} уже у ваших контактах", many: "Усі {n} уже у ваших контактах", other: "Усі {n} уже у ваших контактах" },
+1
View File
@@ -1059,6 +1059,7 @@ export const catalog: Catalog = {
"Nothing unread here": "这里没有未读邮件", "Nothing unread here": "这里没有未读邮件",
}, },
plurals: { plurals: {
"{n} of them look like contacts you already had": { other: "其中 {n} 个与您已有的联系人相似" },
"Your administrator changed {n} settings": { other: "管理员更改了 {n} 项设置" }, "Your administrator changed {n} settings": { other: "管理员更改了 {n} 项设置" },
"Already here: {n} contacts, nothing imported": { other: "已存在 {n} 个,未导入" }, "Already here: {n} contacts, nothing imported": { other: "已存在 {n} 个,未导入" },
"All {n} are already in your contacts": { other: "这 {n} 个已在您的联系人中" }, "All {n} are already in your contacts": { other: "这 {n} 个已在您的联系人中" },
@@ -100,14 +100,14 @@ afterEach(() => {
describe("importing an LDIF bigger than the server will take at once", () => { describe("importing an LDIF bigger than the server will take at once", () => {
it("splits it into calls the server will accept, and files all of it", async () => { it("splits it into calls the server will accept, and files all of it", async () => {
const sets = server({ max: MAX }); const sets = server({ max: MAX });
await expect(useContacts.getState().importLdif(ldifOf(1200), "book1")).resolves.toEqual({ created: 1200, skipped: 0 }); await expect(useContacts.getState().importLdif(ldifOf(1200), "book1")).resolves.toEqual({ created: 1200, skipped: 0, alike: 0 });
expect(sizes(sets)).toEqual([500, 500, 200]); expect(sizes(sets)).toEqual([500, 500, 200]);
}); });
it("splits by what the session advertises, not by a number of its own", async () => { it("splits by what the session advertises, not by a number of its own", async () => {
client.session!.capabilities[CAP.core] = { maxObjectsInGet: 40, maxObjectsInSet: 40 }; client.session!.capabilities[CAP.core] = { maxObjectsInGet: 40, maxObjectsInSet: 40 };
const sets = server({ max: 40 }); const sets = server({ max: 40 });
await expect(useContacts.getState().importLdif(ldifOf(100), "book1")).resolves.toEqual({ created: 100, skipped: 0 }); await expect(useContacts.getState().importLdif(ldifOf(100), "book1")).resolves.toEqual({ created: 100, skipped: 0, alike: 0 });
expect(sizes(sets)).toEqual([40, 40, 20]); expect(sizes(sets)).toEqual([40, 40, 20]);
}); });
@@ -134,7 +134,7 @@ describe("importing an LDIF bigger than the server will take at once", () => {
describe("importing a vCard file bigger than the server will take at once", () => { describe("importing a vCard file bigger than the server will take at once", () => {
it("splits it into calls the server will accept, and files all of it", async () => { it("splits it into calls the server will accept, and files all of it", async () => {
const sets = server({ max: MAX, parsed: vcardsOf(1200) }); const sets = server({ max: MAX, parsed: vcardsOf(1200) });
await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1200, skipped: 0 }); await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1200, skipped: 0, alike: 0 });
expect(sizes(sets)).toEqual([500, 500, 200]); expect(sizes(sets)).toEqual([500, 500, 200]);
}); });
@@ -155,7 +155,7 @@ describe("importing a vCard file bigger than the server will take at once", () =
it("counts what got in when only some of it did", async () => { it("counts what got in when only some of it did", async () => {
server({ max: MAX, parsed: vcardsOf(2), notCreated: { c1: { type: "invalidProperties" } } }); server({ max: MAX, parsed: vcardsOf(2), notCreated: { c1: { type: "invalidProperties" } } });
await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, skipped: 0 }); await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, skipped: 0, alike: 0 });
}); });
}); });
+3 -3
View File
@@ -63,7 +63,7 @@ describe("importing an LDIF address book", () => {
it("creates every entry in one call, not one call each", async () => { it("creates every entry in one call, not one call each", async () => {
const sets = server(); const sets = server();
const n = await useContacts.getState().importLdif(TWO, "book1"); const n = await useContacts.getState().importLdif(TWO, "book1");
expect(n).toEqual({ created: 2, skipped: 0 }); expect(n).toEqual({ created: 2, skipped: 0, alike: 0 });
expect(sets).toHaveLength(1); expect(sets).toHaveLength(1);
expect(Object.keys(sets[0]!.create!)).toEqual(["c0", "c1"]); expect(Object.keys(sets[0]!.create!)).toEqual(["c0", "c1"]);
}); });
@@ -105,7 +105,7 @@ describe("importing an LDIF address book", () => {
it("skips entries too empty to be a person, and imports the rest", async () => { it("skips entries too empty to be a person, and imports the rest", async () => {
const sets = server(); const sets = server();
const n = await useContacts.getState().importLdif(`${TWO}\ndn: cn=Nobody\nobjectClass: top\n`, "book1"); const n = await useContacts.getState().importLdif(`${TWO}\ndn: cn=Nobody\nobjectClass: top\n`, "book1");
expect(n).toEqual({ created: 2, skipped: 0 }); expect(n).toEqual({ created: 2, skipped: 0, alike: 0 });
expect(Object.keys(sets[0]!.create!)).toHaveLength(2); expect(Object.keys(sets[0]!.create!)).toHaveLength(2);
}); });
@@ -116,6 +116,6 @@ describe("importing an LDIF address book", () => {
it("counts what got in when only some of it did", async () => { it("counts what got in when only some of it did", async () => {
server({ notCreated: { c1: { type: "invalidProperties" } } }); server({ notCreated: { c1: { type: "invalidProperties" } } });
await expect(useContacts.getState().importLdif(TWO, "book1")).resolves.toEqual({ created: 1, skipped: 0 }); await expect(useContacts.getState().importLdif(TWO, "book1")).resolves.toEqual({ created: 1, skipped: 0, alike: 0 });
}); });
}); });
@@ -0,0 +1,138 @@
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";
/*
* Counting look-alikes on an LDIF import, without acting on them.
*
* Mozilla's schema has no UID, so the import invents one and a re-import
* duplicates everything. Whether to guess an identity from a name and an
* address is still open on #223 -- and the harm that was actually reported was
* confusion rather than duplication: somebody imports a file twice and cannot
* tell what happened. So this counts and says so, and imports every card
* regardless. Reporting is not matching.
*/
interface SetArgs { create?: Record<string, Record<string, unknown>> }
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>> });
return [name, {
accountId: "a1", oldState: "1", newState: "2",
created: Object.fromEntries(Object.keys((args.create ?? {}) as object).map((k) => [k, { id: `new-${k}` }])),
notCreated: {},
}, 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;
}
/** A card already in the book, with the two fields likeness is read from. */
const card = (id: string, full: string, ...emails: string[]) => ({
id, uid: `uid-${id}`, addressBookIds: { book1: true },
name: { full },
emails: Object.fromEntries(emails.map((address, i) => [`e${i}`, { address }])),
}) as unknown as Partial<ContactCard> & { id: string };
const entry = (cn: string, mail: string) =>
`dn: cn=${cn}\ngivenName: ${cn.split(" ")[0]}\nsn: ${cn.split(" ").slice(-1)[0]}\ncn: ${cn}\nmail: ${mail}\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("telling somebody what an LDIF re-import duplicated", () => {
it("counts an entry that matches an existing card on name and address", async () => {
server([card("c1", "Jane Doe", "[email protected]")]);
const r = await useContacts.getState().importLdif(entry("Jane Doe", "[email protected]"), "book1");
expect(r).toEqual({ created: 1, skipped: 0, alike: 1 });
});
it("imports it anyway, which is the whole point of counting rather than matching", async () => {
const sets = server([card("c1", "Jane Doe", "[email protected]")]);
await useContacts.getState().importLdif(entry("Jane Doe", "[email protected]"), "book1");
expect(Object.keys(sets[0]!.create!)).toHaveLength(1);
});
it("does not count a name match with a different address", async () => {
// Two people who share a name are two people. This is exactly the guess
// the counting refuses to make on anyone's behalf.
server([card("c1", "Jane Doe", "[email protected]")]);
const r = await useContacts.getState().importLdif(entry("Jane Doe", "[email protected]"), "book1");
expect(r.alike).toBe(0);
});
it("does not count an address match under a different name", async () => {
server([card("c1", "Someone Else", "[email protected]")]);
const r = await useContacts.getState().importLdif(entry("Jane Doe", "[email protected]"), "book1");
expect(r.alike).toBe(0);
});
it("recognises a match on a second address", async () => {
server([card("c1", "Jane Doe", "[email protected]", "[email protected]")]);
const r = await useContacts.getState().importLdif(entry("Jane Doe", "[email protected]"), "book1");
expect(r.alike).toBe(1);
});
it("ignores case and spacing, which an export and a hand-typed card differ in", async () => {
server([card("c1", " JANE DOE ", "[email protected]")]);
const r = await useContacts.getState().importLdif(entry("Jane Doe", "[email protected]"), "book1");
expect(r.alike).toBe(1);
});
it("counts each entry once however many of its addresses match", async () => {
server([card("c1", "Jane Doe", "[email protected]", "[email protected]")]);
const two = `dn: cn=Jane Doe\ncn: Jane Doe\nmail: [email protected]\nmozillaSecondEmail: [email protected]\n`;
const r = await useContacts.getState().importLdif(two, "book1");
expect(r.alike).toBe(1);
});
it("looks only at the book being imported into", async () => {
const elsewhere = { ...card("c1", "Jane Doe", "[email protected]"), addressBookIds: { book2: true } };
server([elsewhere]);
const r = await useContacts.getState().importLdif(entry("Jane Doe", "[email protected]"), "book1");
expect(r.alike).toBe(0);
});
it("counts nothing against an empty book", async () => {
server([]);
const r = await useContacts.getState().importLdif(entry("Jane Doe", "[email protected]"), "book1");
expect(r).toEqual({ created: 1, skipped: 0, alike: 0 });
});
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
// something that was already here. The scan is read before anything lands.
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: 2, skipped: 0, alike: 0 });
});
});
+6 -6
View File
@@ -73,7 +73,7 @@ afterEach(() => {
describe("re-importing vCards the book already has", () => { describe("re-importing vCards the book already has", () => {
it("skips a card whose uid is already in this book", async () => { it("skips a card whose uid is already in this book", async () => {
const sets = server({ parsed: [card("ada@x", "Ada"), card("alan@x", "Alan")], existing: [here("ada@x")] }); const sets = server({ parsed: [card("ada@x", "Ada"), card("alan@x", "Alan")], existing: [here("ada@x")] });
await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, skipped: 1 }); await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, skipped: 1, alike: 0 });
expect(Object.values(sets[0]!.create!).map((c) => (c.name as { full: string }).full)).toEqual(["Alan"]); expect(Object.values(sets[0]!.create!).map((c) => (c.name as { full: string }).full)).toEqual(["Alan"]);
}); });
@@ -81,26 +81,26 @@ describe("re-importing vCards the book already has", () => {
// The same person legitimately filed in two address books is not a // The same person legitimately filed in two address books is not a
// duplicate, any more than the same event in two calendars is. // duplicate, any more than the same event in two calendars is.
const sets = server({ parsed: [card("ada@x", "Ada")], existing: [here("ada@x", "book2")] }); const sets = server({ parsed: [card("ada@x", "Ada")], existing: [here("ada@x", "book2")] });
await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, skipped: 0 }); await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, skipped: 0, alike: 0 });
expect(Object.keys(sets[0]!.create!)).toHaveLength(1); expect(Object.keys(sets[0]!.create!)).toHaveLength(1);
}); });
it("imports a card that arrived with no uid, rather than guessing at one", async () => { it("imports a card that arrived with no uid, rather than guessing at one", async () => {
const noUid = { "@type": "Card", version: "1.0", kind: "individual", name: { full: "Anon" } }; const noUid = { "@type": "Card", version: "1.0", kind: "individual", name: { full: "Anon" } };
const sets = server({ parsed: [noUid], existing: [here("ada@x")] }); const sets = server({ parsed: [noUid], existing: [here("ada@x")] });
await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, skipped: 0 }); await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, skipped: 0, alike: 0 });
expect(Object.values(sets[0]!.create!)[0]!.uid).toEqual(expect.any(String)); expect(Object.values(sets[0]!.create!)[0]!.uid).toEqual(expect.any(String));
}); });
it("sends nothing at all when the whole file is already here", async () => { it("sends nothing at all when the whole file is already here", async () => {
const sets = server({ parsed: [card("ada@x", "Ada"), card("alan@x", "Alan")], existing: [here("ada@x"), here("alan@x")] }); const sets = server({ parsed: [card("ada@x", "Ada"), card("alan@x", "Alan")], existing: [here("ada@x"), here("alan@x")] });
await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 0, skipped: 2 }); await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 0, skipped: 2, alike: 0 });
expect(sets).toHaveLength(0); expect(sets).toHaveLength(0);
}); });
it("imports everything into an empty book", async () => { it("imports everything into an empty book", async () => {
const sets = server({ parsed: [card("ada@x", "Ada"), card("alan@x", "Alan")] }); const sets = server({ parsed: [card("ada@x", "Ada"), card("alan@x", "Alan")] });
await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 2, skipped: 0 }); await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 2, skipped: 0, alike: 0 });
expect(Object.keys(sets[0]!.create!)).toHaveLength(2); expect(Object.keys(sets[0]!.create!)).toHaveLength(2);
}); });
}); });
@@ -116,6 +116,6 @@ describe("LDIF, which has nothing to match on", () => {
* name and an address instead is the open question on #223. * name and an address instead is the open question on #223.
*/ */
server({ existing: [here("anything")] }); server({ existing: [here("anything")] });
await expect(useContacts.getState().importLdif(TWO, "book1")).resolves.toEqual({ created: 2, skipped: 0 }); await expect(useContacts.getState().importLdif(TWO, "book1")).resolves.toEqual({ created: 2, skipped: 0, alike: 0 });
}); });
}); });
+56 -10
View File
@@ -41,22 +41,53 @@ import { useMail } from "./mail";
* happens to be holding costs one pass over a list nobody imports into twice a * happens to be holding costs one pass over a list nobody imports into twice a
* day. * day.
*/ */
async function uidsInBook(accountId: Id, addressBookId: Id): Promise<Set<string>> { async function scanBook(accountId: Id, addressBookId: Id): Promise<{ uids: Set<string>; likeness: Set<string> }> {
const uids = new Set<string>(); const uids = new Set<string>();
const likeness = new Set<string>();
const page = client.maxObjectsInGet; const page = client.maxObjectsInGet;
for (let position = 0; ; ) { for (let position = 0; ; ) {
const q = await client.call<QueryResponse>("ContactCard/query", { accountId, position, limit: page, calculateTotal: true }); const q = await client.call<QueryResponse>("ContactCard/query", { accountId, position, limit: page, calculateTotal: true });
const ids = q.ids ?? []; const ids = q.ids ?? [];
if (!ids.length) break; if (!ids.length) break;
for (const part of chunk(ids, page)) { for (const part of chunk(ids, page)) {
const g = await client.call<GetResponse<ContactCard>>("ContactCard/get", { accountId, ids: part, properties: ["uid", "addressBookIds"] }); const g = await client.call<GetResponse<ContactCard>>("ContactCard/get", { accountId, ids: part, properties: ["uid", "addressBookIds", "name", "emails"] });
for (const c of g.list) if (c.uid && c.addressBookIds?.[addressBookId]) uids.add(c.uid); for (const c of g.list) {
if (!c.addressBookIds?.[addressBookId]) continue;
if (c.uid) uids.add(c.uid);
for (const key of likenessKeys(c)) likeness.add(key);
}
} }
position += ids.length; position += ids.length;
// `total` is optional, so the empty page above is what actually ends this. // `total` is optional, so the empty page above is what actually ends this.
if (q.total != null && position >= q.total) break; if (q.total != null && position >= q.total) break;
} }
return uids; return { uids, likeness };
}
/**
* What makes two cards *look* like the same person -- name and one address.
*
* Deliberately not used to skip or merge anything. It is a guess, and it is
* wrong in both directions: two colleagues who share a name and a shared alias
* collapse into one, and somebody whose address changed since the last export
* looks like a stranger. Either mistake is silent and one of them is
* unrecoverable, which is why #223 leaves the decision open.
*
* Counting is a different act from acting. An LDIF re-import duplicates
* everything -- Mozilla's schema has no UID, so the import invents one and
* nothing can match -- and the reported harm was confusion rather than data
* loss: somebody imports a file twice and cannot tell what happened. Being told
* "40 of these look like contacts you already had" answers that without
* touching a single card.
*
* One key per address, so a person whose second address matches is still
* recognised.
*/
function likenessKeys(c: Partial<ContactCard>): string[] {
const name = contactDisplayName(c as ContactCard).trim().toLowerCase();
if (!name) return [];
const addresses = Object.values(c.emails ?? {}).map((e) => e.address?.trim().toLowerCase()).filter(Boolean);
return addresses.map((a) => `${name}\u0000${a}`);
} }
async function createCards(accountId: Id, create: Record<string, unknown>): Promise<{ created: number; refused?: SetError }> { async function createCards(accountId: Id, create: Record<string, unknown>): Promise<{ created: number; refused?: SetError }> {
@@ -154,7 +185,7 @@ interface ContactsState {
updateBook(id: Id, patch: Partial<AddressBook>): Promise<void>; updateBook(id: Id, patch: Partial<AddressBook>): Promise<void>;
destroyBook(id: Id): Promise<void>; destroyBook(id: Id): Promise<void>;
/** Import vCards, skipping any whose UID this book already holds. */ /** Import vCards, skipping any whose UID this book already holds. */
importVCard(text: string, addressBookId: Id): Promise<{ created: number; skipped: number }>; importVCard(text: string, addressBookId: Id): Promise<{ created: number; skipped: number; alike: number }>;
/** /**
* Import an address book in LDIF, read against Mozilla's schema. * Import an address book in LDIF, read against Mozilla's schema.
* *
@@ -162,7 +193,7 @@ interface ContactsState {
* recognise a re-import by. Answered in the same shape as the vCard import so * recognise a re-import by. Answered in the same shape as the vCard import so
* the caller does not have to know which one it called. * the caller does not have to know which one it called.
*/ */
importLdif(text: string, addressBookId: Id): Promise<{ created: number; skipped: number }>; importLdif(text: string, addressBookId: Id): Promise<{ created: number; skipped: number; alike: number }>;
loadPrincipals(): Promise<void>; loadPrincipals(): Promise<void>;
suggest(query: string, limit?: number): Promise<Suggestion[]>; suggest(query: string, limit?: number): Promise<Suggestion[]>;
addRecent(addrs: EmailAddress[]): void; addRecent(addrs: EmailAddress[]): void;
@@ -460,7 +491,7 @@ export const useContacts = create<ContactsState>((set, get) => ({
const entry = parsed.parsed?.[up.blobId]; const entry = parsed.parsed?.[up.blobId];
const cards: ContactCard[] = entry ? (Array.isArray(entry) ? entry : [entry]) : []; const cards: ContactCard[] = entry ? (Array.isArray(entry) ? entry : [entry]) : [];
if (!cards.length) throw new Error("No contacts found in file"); if (!cards.length) throw new Error("No contacts found in file");
const already = await uidsInBook(accountId, addressBookId); const already = (await scanBook(accountId, addressBookId)).uids;
const create: Record<string, unknown> = {}; const create: Record<string, unknown> = {};
let skipped = 0; let skipped = 0;
cards.forEach((c, i) => { cards.forEach((c, i) => {
@@ -482,7 +513,7 @@ export const useContacts = create<ContactsState>((set, get) => ({
// The whole file was already here. Nothing to send, and nothing wrong. // The whole file was already here. Nothing to send, and nothing wrong.
if (!Object.keys(create).length) { if (!Object.keys(create).length) {
await get().loadAll(); await get().loadAll();
return { created: 0, skipped }; return { created: 0, skipped, alike: 0 };
} }
try { try {
const { created, refused } = await createCards(accountId, create); const { created, refused } = await createCards(accountId, create);
@@ -490,7 +521,9 @@ export const useContacts = create<ContactsState>((set, get) => ({
// though the file had been empty. The LDIF import said this already; a // though the file had been empty. The LDIF import said this already; a
// vCard import that quietly returned 0 was the odd one out. // vCard import that quietly returned 0 was the odd one out.
if (!created) throw new Error(refused ? setErrorMessage(refused) : "the server did not accept any of its contacts"); if (!created) throw new Error(refused ? setErrorMessage(refused) : "the server did not accept any of its contacts");
return { created, skipped }; /* No likeness count here: a vCard carries a UID, so anything that was
already present was skipped by name above rather than guessed at. */
return { created, skipped, alike: 0 };
} finally { } finally {
await get().loadAll(); await get().loadAll();
} }
@@ -509,8 +542,16 @@ export const useContacts = create<ContactsState>((set, get) => ({
const accountId = get().accountId!; const accountId = get().accountId!;
const cards = parseLdif(text).map(cardFromLdif).filter((c): c is Partial<ContactCard> => c !== null); const cards = parseLdif(text).map(cardFromLdif).filter((c): c is Partial<ContactCard> => c !== null);
if (!cards.length) throw new Error("it has no contacts in it"); if (!cards.length) throw new Error("it has no contacts in it");
/*
* Read before anything is created, so "already had" means before this
* import rather than including it. Every card here is imported either way;
* this only counts.
*/
const before = await scanBook(accountId, addressBookId);
let alike = 0;
const create: Record<string, unknown> = {}; const create: Record<string, unknown> = {};
cards.forEach((c, i) => { cards.forEach((c, i) => {
if (likenessKeys(c).some((k) => before.likeness.has(k))) alike++;
// Built here rather than read from the file: LDIF identifies an entry by // Built here rather than read from the file: LDIF identifies an entry by
// its distinguished name, which says where it sat in somebody's // its distinguished name, which says where it sat in somebody's
// directory and is no use as a contact's identity anywhere else. // directory and is no use as a contact's identity anywhere else.
@@ -529,8 +570,13 @@ export const useContacts = create<ContactsState>((set, get) => ({
* here because Mozilla's schema does not define one, so a re-import has no * here because Mozilla's schema does not define one, so a re-import has no
* identity to be recognised by -- see #223, where whether to guess at one * identity to be recognised by -- see #223, where whether to guess at one
* from a name and an address is still an open question. * from a name and an address is still an open question.
*
* `alike` is what can be said without answering it: how many of these look
* like contacts that were already here. Reporting is not matching -- every
* card was imported -- and it is the confusion rather than the duplication
* that was reported as the harm.
*/ */
return { created, skipped: 0 }; return { created, skipped: 0, alike };
}, },
async loadPrincipals() { async loadPrincipals() {
+13 -1
View File
@@ -136,7 +136,7 @@ export function ContactsView({ id }: { id?: string }) {
* LDIF may arrive as .ldif, .ldi, .txt or with no extension at all, and * LDIF may arrive as .ldif, .ldi, .txt or with no extension at all, and
* the name is the least reliable thing about it. * the name is the least reliable thing about it.
*/ */
const { created, skipped } = /^\s*BEGIN:VCARD/im.test(text) const { created, skipped, alike } = /^\s*BEGIN:VCARD/im.test(text)
? await contacts.importVCard(text, book.id) ? await contacts.importVCard(text, book.id)
: await contacts.importLdif(text, book.id); : await contacts.importLdif(text, book.id);
/* /*
@@ -149,6 +149,18 @@ export function ContactsView({ id }: { id?: string }) {
if (!created) toast.success(plural(skipped, { one: "Already here: {n} contact, nothing imported", other: "Already here: {n} contacts, nothing imported" })); if (!created) toast.success(plural(skipped, { one: "Already here: {n} contact, nothing imported", other: "Already here: {n} contacts, nothing imported" }));
else if (skipped) toast.success(`${imported} · ${plural(skipped, { one: "{n} was already here", other: "{n} were already here" })}`); else if (skipped) toast.success(`${imported} · ${plural(skipped, { one: "{n} was already here", other: "{n} were already here" })}`);
else toast.success(imported); else toast.success(imported);
/*
* Said separately, and after, because it is a different kind of fact.
* LDIF has no UID to match on, so nothing was skipped and nothing was
* merged -- these are simply here twice now, and saying so is the whole
* of what can honestly be said without guessing (#223).
*/
if (alike) {
toast.show(plural(alike, {
one: "{n} of them looks like a contact you already had",
other: "{n} of them look like contacts you already had",
}), { duration: 9000 });
}
} catch (err) { } catch (err) {
toast.error(translate("Could not import this file: {error}", { error: (err as Error).message })); toast.error(translate("Could not import this file: {error}", { error: (err as Error).message }));
} }