diff --git a/web/src/locales/de.ts b/web/src/locales/de.ts index 57d7184..ba7daaf 100644 --- a/web/src/locales/de.ts +++ b/web/src/locales/de.ts @@ -1080,10 +1080,11 @@ export const catalog: Catalog = { "Nothing unread here": "Hier ist nichts ungelesen", }, plurals: { + "Updated {n} contacts, nothing new": { one: "{n} Kontakt aktualisiert, nichts Neues", other: "{n} Kontakte aktualisiert, nichts Neues" }, + "{n} updated": { one: "{n} aktualisiert", other: "{n} aktualisiert" }, + "Updated {n} contacts you already had": { one: "Vorhandenen Kontakt aktualisiert", other: "{n} vorhandene Kontakte aktualisiert" }, "{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" }, - "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" }, "Exported {n} events": { one: "{n} Termin exportiert", other: "{n} Termine exportiert" }, "Imported {n} events": { one: "{n} Termin importiert", other: "{n} Termine importiert" }, "Already here: {n} events, nothing imported": { one: "Bereits vorhanden: {n} Termin, nichts importiert", other: "Bereits vorhanden: {n} Termine, nichts importiert" }, diff --git a/web/src/locales/es.ts b/web/src/locales/es.ts index e35f8cb..3557050 100644 --- a/web/src/locales/es.ts +++ b/web/src/locales/es.ts @@ -1053,10 +1053,11 @@ export const catalog: Catalog = { "Nothing unread here": "Aquí no hay nada sin leer", }, plurals: { + "Updated {n} contacts, nothing new": { one: "{n} contacto actualizado, nada nuevo", other: "{n} contactos actualizados, nada nuevo" }, + "{n} updated": { one: "{n} actualizado", other: "{n} actualizados" }, + "Updated {n} contacts you already had": { one: "Se actualizó el contacto que ya tenías", other: "Se actualizaron {n} contactos que ya tenías" }, "{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" }, - "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" }, "Exported {n} events": { one: "{n} evento exportado", other: "{n} eventos exportados" }, "Imported {n} events": { one: "{n} evento importado", other: "{n} eventos importados" }, "Already here: {n} events, nothing imported": { one: "Ya estaba aquí: {n} evento, no se importó nada", other: "Ya estaban aquí: {n} eventos, no se importó nada" }, diff --git a/web/src/locales/fr.ts b/web/src/locales/fr.ts index 1513613..345395b 100644 --- a/web/src/locales/fr.ts +++ b/web/src/locales/fr.ts @@ -1058,10 +1058,11 @@ export const catalog: Catalog = { "Nothing unread here": "Rien de non lu ici", }, plurals: { + "Updated {n} contacts, nothing new": { one: "{n} contact mis à jour, rien de nouveau", other: "{n} contacts mis à jour, rien de nouveau" }, + "{n} updated": { one: "{n} mis à jour", other: "{n} mis à jour" }, + "Updated {n} contacts you already had": { one: "Contact déjà présent mis à jour", other: "{n} contacts déjà présents mis à jour" }, "{n} of them look like contacts you already had": { one: "{n} d’entre eux ressemble à un contact que vous aviez déjà", other: "{n} d’entre 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" }, - "Already here: {n} contacts, nothing imported": { one: "Déjà présent : {n} contact, rien d’importé", other: "Déjà présents : {n} contacts, rien d’importé" }, - "All {n} are already in your contacts": { one: "Déjà dans vos contacts", other: "Les {n} sont déjà dans vos contacts" }, "Exported {n} events": { one: "{n} événement exporté", other: "{n} événements exportés" }, "Imported {n} events": { one: "{n} événement importé", other: "{n} événements importés" }, "Already here: {n} events, nothing imported": { one: "Déjà présent : {n} événement, rien d’importé", other: "Déjà présents : {n} événements, rien d’importé" }, diff --git a/web/src/locales/ja.ts b/web/src/locales/ja.ts index 29bf9ca..c12ec39 100644 --- a/web/src/locales/ja.ts +++ b/web/src/locales/ja.ts @@ -1061,10 +1061,11 @@ export const catalog: Catalog = { "Nothing unread here": "ここに未読はありません", }, plurals: { + "Updated {n} contacts, nothing new": { other: "{n} 件の連絡先を更新しました。新規はありません" }, + "{n} updated": { other: "{n} 件を更新" }, + "Updated {n} contacts you already had": { other: "すでにある連絡先 {n} 件を更新しました" }, "{n} of them look like contacts you already had": { other: "うち {n} 件はすでにある連絡先に似ています" }, "Your administrator changed {n} settings": { other: "管理者が {n} 件の設定を変更しました" }, - "Already here: {n} contacts, nothing imported": { other: "すでに存在: {n} 件、インポートなし" }, - "All {n} are already in your contacts": { other: "{n} 件はすでに連絡先にあります" }, "Exported {n} events": { other: "{n} 件の予定をエクスポートしました" }, "Imported {n} events": { other: "{n} 件の予定をインポートしました" }, "Already here: {n} events, nothing imported": { other: "すでに存在: {n} 件、インポートなし" }, diff --git a/web/src/locales/nl.ts b/web/src/locales/nl.ts index 4412d79..041e4d0 100644 --- a/web/src/locales/nl.ts +++ b/web/src/locales/nl.ts @@ -1049,10 +1049,11 @@ export const catalog: Catalog = { "Nothing unread here": "Hier is niets ongelezen", }, plurals: { + "Updated {n} contacts, nothing new": { one: "{n} contact bijgewerkt, niets nieuws", other: "{n} contacten bijgewerkt, niets nieuws" }, + "{n} updated": { one: "{n} bijgewerkt", other: "{n} bijgewerkt" }, + "Updated {n} contacts you already had": { one: "Bestaand contact bijgewerkt", other: "{n} bestaande contacten bijgewerkt" }, "{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" }, - "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" }, "Exported {n} events": { one: "{n} afspraak geëxporteerd", other: "{n} afspraken geëxporteerd" }, "Imported {n} events": { one: "{n} afspraak geïmporteerd", other: "{n} afspraken geïmporteerd" }, "Already here: {n} events, nothing imported": { one: "Al aanwezig: {n} afspraak, niets geïmporteerd", other: "Al aanwezig: {n} afspraken, niets geïmporteerd" }, diff --git a/web/src/locales/pt-BR.ts b/web/src/locales/pt-BR.ts index d9908b2..30d8163 100644 --- a/web/src/locales/pt-BR.ts +++ b/web/src/locales/pt-BR.ts @@ -1056,10 +1056,11 @@ export const catalog: Catalog = { "Nothing unread here": "Não há nada não lido aqui", }, plurals: { + "Updated {n} contacts, nothing new": { one: "{n} contato atualizado, nada novo", other: "{n} contatos atualizados, nada novo" }, + "{n} updated": { one: "{n} atualizado", other: "{n} atualizados" }, + "Updated {n} contacts you already had": { one: "Contato que você já tinha atualizado", other: "{n} contatos que você já tinha atualizados" }, "{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" }, - "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" }, "Exported {n} events": { one: "{n} evento exportado", other: "{n} eventos exportados" }, "Imported {n} events": { one: "{n} evento importado", other: "{n} eventos importados" }, "Already here: {n} events, nothing imported": { one: "Já estava aqui: {n} evento, nada importado", other: "Já estavam aqui: {n} eventos, nada importado" }, diff --git a/web/src/locales/ru.ts b/web/src/locales/ru.ts index 97102ee..2b68a36 100644 --- a/web/src/locales/ru.ts +++ b/web/src/locales/ru.ts @@ -1055,10 +1055,11 @@ export const catalog: Catalog = { "Nothing unread here": "Здесь нет непрочитанного", }, plurals: { + "Updated {n} contacts, nothing new": { one: "Обновлён {n} контакт, новых нет", few: "Обновлено {n} контакта, новых нет", many: "Обновлено {n} контактов, новых нет", other: "Обновлено {n} контакта, новых нет" }, + "{n} updated": { one: "{n} обновлён", few: "{n} обновлено", many: "{n} обновлено", other: "{n} обновлено" }, + "Updated {n} contacts you already had": { one: "Обновлён контакт, который уже был", few: "Обновлено {n} контакта, которые уже были", many: "Обновлено {n} контактов, которые уже были", other: "Обновлено {n} контакта, которые уже были" }, "{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} настройки" }, - "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} уже в ваших контактах" }, "Exported {n} events": { one: "Экспортировано {n} событие", few: "Экспортировано {n} события", many: "Экспортировано {n} событий", other: "Экспортировано {n} события" }, "Imported {n} events": { one: "Импортировано {n} событие", few: "Импортировано {n} события", many: "Импортировано {n} событий", other: "Импортировано {n} события" }, "Already here: {n} events, nothing imported": { one: "Уже есть: {n} событие, ничего не импортировано", few: "Уже есть: {n} события, ничего не импортировано", many: "Уже есть: {n} событий, ничего не импортировано", other: "Уже есть: {n} события, ничего не импортировано" }, diff --git a/web/src/locales/uk.ts b/web/src/locales/uk.ts index d7694c1..14ca825 100644 --- a/web/src/locales/uk.ts +++ b/web/src/locales/uk.ts @@ -1049,10 +1049,11 @@ export const catalog: Catalog = { "Nothing unread here": "Тут немає непрочитаного", }, plurals: { + "Updated {n} contacts, nothing new": { one: "Оновлено {n} контакт, нових немає", few: "Оновлено {n} контакти, нових немає", many: "Оновлено {n} контактів, нових немає", other: "Оновлено {n} контакти, нових немає" }, + "{n} updated": { one: "{n} оновлено", few: "{n} оновлено", many: "{n} оновлено", other: "{n} оновлено" }, + "Updated {n} contacts you already had": { one: "Оновлено контакт, який уже був", few: "Оновлено {n} контакти, які вже були", many: "Оновлено {n} контактів, які вже були", other: "Оновлено {n} контакти, які вже були" }, "{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} налаштування" }, - "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} уже у ваших контактах" }, "Exported {n} events": { one: "Експортовано {n} подію", few: "Експортовано {n} події", many: "Експортовано {n} подій", other: "Експортовано {n} події" }, "Imported {n} events": { one: "Імпортовано {n} подію", few: "Імпортовано {n} події", many: "Імпортовано {n} подій", other: "Імпортовано {n} події" }, "Already here: {n} events, nothing imported": { one: "Уже є: {n} подія, нічого не імпортовано", few: "Уже є: {n} події, нічого не імпортовано", many: "Уже є: {n} подій, нічого не імпортовано", other: "Уже є: {n} події, нічого не імпортовано" }, diff --git a/web/src/locales/zh-Hans.ts b/web/src/locales/zh-Hans.ts index 1db4637..2a7dbe3 100644 --- a/web/src/locales/zh-Hans.ts +++ b/web/src/locales/zh-Hans.ts @@ -1060,10 +1060,11 @@ export const catalog: Catalog = { "Nothing unread here": "这里没有未读邮件", }, plurals: { + "Updated {n} contacts, nothing new": { other: "已更新 {n} 个联系人,无新增" }, + "{n} updated": { other: "已更新 {n} 个" }, + "Updated {n} contacts you already had": { other: "已更新您已有的 {n} 个联系人" }, "{n} of them look like contacts you already had": { other: "其中 {n} 个与您已有的联系人相似" }, "Your administrator changed {n} settings": { other: "管理员更改了 {n} 项设置" }, - "Already here: {n} contacts, nothing imported": { other: "已存在 {n} 个,未导入" }, - "All {n} are already in your contacts": { other: "这 {n} 个已在您的联系人中" }, "Exported {n} events": { other: "已导出 {n} 个日程" }, "Imported {n} events": { other: "已导入 {n} 个日程" }, "Already here: {n} events, nothing imported": { other: "已存在 {n} 个,未导入" }, diff --git a/web/src/store/__tests__/contact-set-batching.test.ts b/web/src/store/__tests__/contact-set-batching.test.ts index 9c536cf..130c3ff 100644 --- a/web/src/store/__tests__/contact-set-batching.test.ts +++ b/web/src/store/__tests__/contact-set-batching.test.ts @@ -18,7 +18,7 @@ import type { ContactCard, Id, JmapSession, UploadResponse } from "@/jmap/types" const MAX = 500; -interface SetArgs { create?: Record>; destroy?: Id[] } +interface SetArgs { create?: Record>; update?: Record>; destroy?: Id[] } /** * @param max the ceiling on objects in one call, refused whole the way Stalwart @@ -39,9 +39,12 @@ function server(opts: { max?: number; failOn?: number; parsed?: unknown[]; notCr if (name === "ContactCard/set") { const nth = sets.length; const create = args.create as Record> | undefined; + const update = args.update as Record> | undefined; const destroy = args.destroy as Id[] | undefined; - sets.push({ create, destroy }); - const n = Object.keys(create ?? {}).length + (destroy?.length ?? 0); + sets.push({ create, update, destroy }); + /* Everything in the call counts against the ceiling, the way Stalwart + counts it -- creates and updates share one budget. */ + const n = Object.keys(create ?? {}).length + Object.keys(update ?? {}).length + (destroy?.length ?? 0); if (opts.max != null && n > opts.max) { return [ "error", @@ -54,7 +57,8 @@ function server(opts: { max?: number; failOn?: number; parsed?: unknown[]; notCr return [name, { accountId: "a1", oldState: "1", newState: "2", created: Object.fromEntries(Object.keys(create ?? {}).filter((k) => !(k in notCreated)).map((k) => [k, { id: `new-${k}` }])), - notCreated, + updated: Object.fromEntries(Object.keys(update ?? {}).map((k) => [k, null])), + notCreated, notUpdated: {}, destroyed: destroy ?? [], }, id]; } @@ -81,7 +85,8 @@ const vcardsOf = (n: number) => const cardsInState = (n: number) => Object.fromEntries(Array.from({ length: n }, (_, i) => [`c${i}`, { id: `c${i}`, name: { full: `Person ${i}` } }])) as unknown as Record; -const sizes = (sets: SetArgs[]) => sets.map((s) => Object.keys(s.create ?? {}).length + (s.destroy?.length ?? 0)); +const sizes = (sets: SetArgs[]) => + sets.map((s) => Object.keys(s.create ?? {}).length + Object.keys(s.update ?? {}).length + (s.destroy?.length ?? 0)); beforeEach(() => { client.session = { @@ -100,14 +105,14 @@ afterEach(() => { 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 () => { const sets = server({ max: MAX }); - await expect(useContacts.getState().importLdif(ldifOf(1200), "book1")).resolves.toEqual({ created: 1200, skipped: 0, alike: 0 }); + await expect(useContacts.getState().importLdif(ldifOf(1200), "book1")).resolves.toEqual({ created: 1200, updated: 0, alike: 0 }); expect(sizes(sets)).toEqual([500, 500, 200]); }); it("splits by what the session advertises, not by a number of its own", async () => { client.session!.capabilities[CAP.core] = { maxObjectsInGet: 40, maxObjectsInSet: 40 }; const sets = server({ max: 40 }); - await expect(useContacts.getState().importLdif(ldifOf(100), "book1")).resolves.toEqual({ created: 100, skipped: 0, alike: 0 }); + await expect(useContacts.getState().importLdif(ldifOf(100), "book1")).resolves.toEqual({ created: 100, updated: 0, alike: 0 }); expect(sizes(sets)).toEqual([40, 40, 20]); }); @@ -134,7 +139,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", () => { it("splits it into calls the server will accept, and files all of it", async () => { const sets = server({ max: MAX, parsed: vcardsOf(1200) }); - await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1200, skipped: 0, alike: 0 }); + await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1200, updated: 0, alike: 0 }); expect(sizes(sets)).toEqual([500, 500, 200]); }); @@ -155,7 +160,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 () => { server({ max: MAX, parsed: vcardsOf(2), notCreated: { c1: { type: "invalidProperties" } } }); - await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, skipped: 0, alike: 0 }); + await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, updated: 0, alike: 0 }); }); }); diff --git a/web/src/store/__tests__/ldif-import.test.ts b/web/src/store/__tests__/ldif-import.test.ts index 3be9da6..cfddd45 100644 --- a/web/src/store/__tests__/ldif-import.test.ts +++ b/web/src/store/__tests__/ldif-import.test.ts @@ -63,7 +63,7 @@ describe("importing an LDIF address book", () => { it("creates every entry in one call, not one call each", async () => { const sets = server(); const n = await useContacts.getState().importLdif(TWO, "book1"); - expect(n).toEqual({ created: 2, skipped: 0, alike: 0 }); + expect(n).toEqual({ created: 2, updated: 0, alike: 0 }); expect(sets).toHaveLength(1); 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 () => { const sets = server(); const n = await useContacts.getState().importLdif(`${TWO}\ndn: cn=Nobody\nobjectClass: top\n`, "book1"); - expect(n).toEqual({ created: 2, skipped: 0, alike: 0 }); + expect(n).toEqual({ created: 2, updated: 0, alike: 0 }); 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 () => { server({ notCreated: { c1: { type: "invalidProperties" } } }); - await expect(useContacts.getState().importLdif(TWO, "book1")).resolves.toEqual({ created: 1, skipped: 0, alike: 0 }); + await expect(useContacts.getState().importLdif(TWO, "book1")).resolves.toEqual({ created: 1, updated: 0, alike: 0 }); }); }); diff --git a/web/src/store/__tests__/ldif-likeness.test.ts b/web/src/store/__tests__/ldif-likeness.test.ts index ab102e1..75a5b01 100644 --- a/web/src/store/__tests__/ldif-likeness.test.ts +++ b/web/src/store/__tests__/ldif-likeness.test.ts @@ -72,7 +72,7 @@ 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", "jane@example.com")]); const r = await useContacts.getState().importLdif(entry("Jane Doe", "jane@example.com"), "book1"); - expect(r).toEqual({ created: 1, skipped: 0, alike: 1 }); + expect(r).toEqual({ created: 1, updated: 0, alike: 1 }); }); it("imports it anyway, which is the whole point of counting rather than matching", async () => { @@ -124,7 +124,7 @@ describe("telling somebody what an LDIF re-import duplicated", () => { it("counts nothing against an empty book", async () => { server([]); const r = await useContacts.getState().importLdif(entry("Jane Doe", "jane@example.com"), "book1"); - expect(r).toEqual({ created: 1, skipped: 0, alike: 0 }); + expect(r).toEqual({ created: 1, updated: 0, alike: 0 }); }); it("does not count the file against itself", async () => { @@ -133,6 +133,6 @@ describe("telling somebody what an LDIF re-import duplicated", () => { server([]); const twice = entry("Jane Doe", "jane@example.com") + "\n" + entry("Jane Doe", "jane@example.com"); const r = await useContacts.getState().importLdif(twice, "book1"); - expect(r).toEqual({ created: 2, skipped: 0, alike: 0 }); + expect(r).toEqual({ created: 2, updated: 0, alike: 0 }); }); }); diff --git a/web/src/store/__tests__/vcard-dedupe.test.ts b/web/src/store/__tests__/vcard-dedupe.test.ts index 4bfb1ab..e9733b2 100644 --- a/web/src/store/__tests__/vcard-dedupe.test.ts +++ b/web/src/store/__tests__/vcard-dedupe.test.ts @@ -15,9 +15,9 @@ import type { ContactCard, JmapSession, UploadResponse } from "@/jmap/types"; const MAX = 500; -interface SetArgs { create?: Record> } +interface SetArgs { create?: Record>; update?: Record> } -function server(opts: { parsed?: unknown[]; existing?: Array<{ id: string; uid: string; addressBookIds: Record }> } = {}) { +function server(opts: { parsed?: unknown[]; existing?: Array<{ id: string; uid: string; addressBookIds: Record }>; max?: number } = {}) { const sets: SetArgs[] = []; const existing = opts.existing ?? []; const fetchMock = vi.fn(async (_url: string, init: RequestInit) => { @@ -36,11 +36,21 @@ function server(opts: { parsed?: unknown[]; existing?: Array<{ id: string; uid: 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> }); + sets.push({ + create: args.create as Record>, + update: args.update as Record>, + }); + /* Refused whole over the ceiling, the way Stalwart refuses it, and + counting creates and updates together the way Stalwart counts. */ + const n = Object.keys((args.create ?? {}) as object).length + Object.keys((args.update ?? {}) as object).length; + if (opts.max != null && n > opts.max) { + return ["error", { type: "requestTooLarge", description: "too many objects" }, id]; + } return [name, { accountId: "a1", oldState: "1", newState: "2", created: Object.fromEntries(Object.keys((args.create ?? {}) as object).map((k) => [k, { id: `new-${k}` }])), - notCreated: {}, + 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]; @@ -71,36 +81,66 @@ afterEach(() => { }); describe("re-importing vCards the book already has", () => { - it("skips a card whose uid is already in this book", async () => { + it("updates a card whose uid is already in this book, and creates the rest", async () => { + /* + * It used to skip. The reporter asked for the opposite on #174 and he is + * right: the reason to import a file twice is usually that the first one + * was wrong, and skipping means a corrected export corrects nothing. + */ 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, alike: 0 }); + await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, updated: 1, alike: 0 }); expect(Object.values(sets[0]!.create!).map((c) => (c.name as { full: string }).full)).toEqual(["Alan"]); + // Addressed by the id already here, not by a client-side key. + expect(Object.keys(sets[0]!.update!)).toEqual(["srv-ada@x"]); + }); + + it("does not move an updated card into the book it is being imported into", async () => { + // The card is already in this book; sending addressBookIds again would say + // nothing, and sending it on a card shared into another book would move it. + const sets = server({ parsed: [card("ada@x", "Ada")], existing: [here("ada@x")] }); + await useContacts.getState().importVCard("BEGIN:VCARD", "book1"); + expect(Object.values(sets[0]!.update!)[0]).not.toHaveProperty("addressBookIds"); + }); + + it("leaves properties the file does not mention alone", async () => { + /* + * A merge rather than a replacement: a phone number added in ihasmail after + * the first import survives a re-import of the original file. The cost is + * that a field deleted at the source stays here, which is the better way to + * be wrong. + */ + const sets = server({ parsed: [card("ada@x", "Ada")], existing: [here("ada@x")] }); + await useContacts.getState().importVCard("BEGIN:VCARD", "book1"); + const patch = Object.values(sets[0]!.update!)[0]!; + expect(patch).not.toHaveProperty("phones"); + expect(patch.name).toEqual({ full: "Ada" }); }); it("imports a card whose uid is in a different book", async () => { // The same person legitimately filed in two address books is not a // duplicate, any more than the same event in two calendars is. 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, alike: 0 }); + await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, updated: 0, alike: 0 }); expect(Object.keys(sets[0]!.create!)).toHaveLength(1); }); 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 sets = server({ parsed: [noUid], existing: [here("ada@x")] }); - await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, skipped: 0, alike: 0 }); + await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 1, updated: 0, alike: 0 }); 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("updates the lot when the whole file is already here, creating none", async () => { 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, alike: 0 }); - expect(sets).toHaveLength(0); + await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 0, updated: 2, alike: 0 }); + expect(Object.keys(sets[0]!.create ?? {})).toHaveLength(0); + expect(Object.keys(sets[0]!.update!)).toHaveLength(2); }); it("imports everything into an empty book", async () => { 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, alike: 0 }); + await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 2, updated: 0, alike: 0 }); expect(Object.keys(sets[0]!.create!)).toHaveLength(2); }); }); @@ -116,6 +156,32 @@ describe("LDIF, which has nothing to match on", () => { * name and an address instead is the open question on #223. */ server({ existing: [here("anything")] }); - await expect(useContacts.getState().importLdif(TWO, "book1")).resolves.toEqual({ created: 2, skipped: 0, alike: 0 }); + await expect(useContacts.getState().importLdif(TWO, "book1")).resolves.toEqual({ created: 2, updated: 0, alike: 0 }); + }); +}); + +/* + * Creates and updates share the ceiling. + * + * Now that a re-import updates rather than skips, one file can carry both. + * Stalwart counts every object in a `/set` against `maxObjectsInSet` together, + * so batching the halves separately would send 300 new and 300 changed as two + * calls of 300 and be refused for a limit of 500 that neither half exceeds. + */ +describe("a file that both creates and updates", () => { + it("counts them against one budget, not one each", async () => { + const MAX = 500; + const existing = Array.from({ length: 300 }, (_, i) => here(`old-${i}@x`)); + const parsed = [ + ...Array.from({ length: 300 }, (_, i) => card(`old-${i}@x`, `Old ${i}`)), + ...Array.from({ length: 300 }, (_, i) => card(`new-${i}@x`, `New ${i}`)), + ]; + const sets = server({ max: MAX, parsed, existing }); + await expect(useContacts.getState().importVCard("BEGIN:VCARD", "book1")).resolves.toEqual({ created: 300, updated: 300, alike: 0 }); + for (const s of sets) { + const n = Object.keys(s.create ?? {}).length + Object.keys(s.update ?? {}).length; + expect(n).toBeLessThanOrEqual(MAX); + } + expect(sets).toHaveLength(2); }); }); diff --git a/web/src/store/contacts.ts b/web/src/store/contacts.ts index 2f08567..c1f9183 100644 --- a/web/src/store/contacts.ts +++ b/web/src/store/contacts.ts @@ -41,8 +41,10 @@ import { useMail } from "./mail"; * happens to be holding costs one pass over a list nobody imports into twice a * day. */ -async function scanBook(accountId: Id, addressBookId: Id): Promise<{ uids: Set; likeness: Set }> { - const uids = new Set(); +async function scanBook(accountId: Id, addressBookId: Id): Promise<{ byUid: Map; likeness: Set }> { + /* The id as well as the UID, because a card that is already here is now + updated rather than skipped, and updating needs something to address. */ + const byUid = new Map(); const likeness = new Set(); const page = client.maxObjectsInGet; for (let position = 0; ; ) { @@ -53,7 +55,7 @@ async function scanBook(accountId: Id, addressBookId: Id): Promise<{ uids: Set>("ContactCard/get", { accountId, ids: part, properties: ["uid", "addressBookIds", "name", "emails"] }); for (const c of g.list) { if (!c.addressBookIds?.[addressBookId]) continue; - if (c.uid) uids.add(c.uid); + if (c.uid && !byUid.has(c.uid)) byUid.set(c.uid, c.id); for (const key of likenessKeys(c)) likeness.add(key); } } @@ -61,7 +63,7 @@ async function scanBook(accountId: Id, addressBookId: Id): Promise<{ uids: Set= q.total) break; } - return { uids, likeness }; + return { byUid, likeness }; } /** @@ -90,27 +92,47 @@ function likenessKeys(c: Partial): string[] { return addresses.map((a) => `${name}\u0000${a}`); } -async function createCards(accountId: Id, create: Record): Promise<{ created: number; refused?: SetError }> { - const keys = Object.keys(create); +async function writeCards( + accountId: Id, + create: Record, + update: Record = {}, +): Promise<{ created: number; updated: number; refused?: SetError }> { + /* + * Creates and updates share one budget. Stalwart counts every object in a + * `/set` against `maxObjectsInSet` -- creates, updates and destroys together + * -- so batching them separately would let a file of 300 new and 300 changed + * cards through as two calls of 300 and be refused for a limit of 500 that + * neither half exceeds. + */ + const keys = [ + ...Object.keys(create).map((k) => ["create", k] as const), + ...Object.keys(update).map((k) => ["update", k] as const), + ]; let created = 0; + let updated = 0; let refused: SetError | undefined; for (const part of chunk(keys, client.maxObjectsInSet)) { - const sub: Record = {}; - for (const k of part) sub[k] = create[k]; + const subCreate: Record = {}; + const subUpdate: Record = {}; + for (const [kind, k] of part) { + if (kind === "create") subCreate[k] = create[k]; + else subUpdate[k] = update[k]; + } let res: SetResponse; try { - res = await client.call>("ContactCard/set", { accountId, create: sub }); + res = await client.call>("ContactCard/set", { accountId, create: subCreate, update: subUpdate }); } catch (err) { // A batch that failed with earlier ones already filed: those contacts are // in the address book, and an error saying only that the import failed // sends someone looking for contacts that are already there. - if (!created) throw err; - throw new Error(`${created} of ${keys.length} contacts were imported before this happened: ${(err as Error).message}`); + if (!created && !updated) throw err; + throw new Error(`${created + updated} of ${keys.length} contacts were imported before this happened: ${(err as Error).message}`); } created += Object.keys(res.created ?? {}).length; - refused ??= Object.values(res.notCreated ?? {})[0]; + updated += Object.keys(res.updated ?? {}).length; + refused ??= Object.values(res.notCreated ?? {})[0] ?? Object.values(res.notUpdated ?? {})[0]; } - return { created, refused }; + return { created, updated, refused }; } export interface Suggestion { @@ -184,16 +206,17 @@ interface ContactsState { createBook(name: string): Promise; updateBook(id: Id, patch: Partial): Promise; destroyBook(id: Id): Promise; - /** Import vCards, skipping any whose UID this book already holds. */ - importVCard(text: string, addressBookId: Id): Promise<{ created: number; skipped: number; alike: number }>; + /** Import vCards, updating any whose UID this book already holds rather than duplicating it. */ + importVCard(text: string, addressBookId: Id): Promise<{ created: number; updated: number; alike: number }>; /** * Import an address book in LDIF, read against Mozilla's schema. * - * `skipped` is always 0: Mozilla's schema has no UID, so there is nothing to - * 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. + * `updated` is always 0: Mozilla's schema has no UID, so there is nothing to + * recognise a re-import by and everything arrives as new. `alike` says how + * many look like cards already here without acting on it. Answered in the + * same shape as the vCard import so the caller need not know which it called. */ - importLdif(text: string, addressBookId: Id): Promise<{ created: number; skipped: number; alike: number }>; + importLdif(text: string, addressBookId: Id): Promise<{ created: number; updated: number; alike: number }>; loadPrincipals(): Promise; suggest(query: string, limit?: number): Promise; addRecent(addrs: EmailAddress[]): void; @@ -491,39 +514,44 @@ export const useContacts = create((set, get) => ({ const entry = parsed.parsed?.[up.blobId]; const cards: ContactCard[] = entry ? (Array.isArray(entry) ? entry : [entry]) : []; if (!cards.length) throw new Error("No contacts found in file"); - const already = (await scanBook(accountId, addressBookId)).uids; + const { byUid } = await scanBook(accountId, addressBookId); const create: Record = {}; - let skipped = 0; + const update: Record = {}; cards.forEach((c, i) => { const { id: _id, addressBookIds: _ab, ...rest } = c as ContactCard & { id?: Id }; /* * A vCard UID is an identity its author meant, so a card whose UID this - * book already holds is the same card and re-importing an export used to - * leave a second copy of every one of them. Asked for on #174 after the - * reporter's colleague hit it, and decided on #173 for events: skip on a - * UID that is already here, import what arrives without one, since - * nothing can be matched on an identity that is not there. + * book already holds is that card -- and the newer version of it wins. + * + * It used to be skipped. The reporter asked for the opposite on #174 and + * he is right: the reason to import a file a second time is usually that + * the first one was not right, and skipping means a corrected export + * corrects nothing. + * + * A merge, not a replacement. Properties the file carries overwrite what + * is here; properties it does not mention are left alone, so a phone + * number somebody added in ihasmail after the first import survives a + * re-import of the original file. The cost is that a field genuinely + * deleted at the source stays here -- worth it, because the other way + * round loses work nobody asked to lose. */ - if (rest.uid && already.has(rest.uid)) { - skipped++; + const existing = rest.uid ? byUid.get(rest.uid) : undefined; + if (existing) { + update[existing] = { ...rest, addressBookIds: undefined }; + delete (update[existing] as Record).addressBookIds; return; } create[`c${i}`] = { ...rest, uid: rest.uid || crypto.randomUUID(), addressBookIds: { [addressBookId]: true } }; }); - // The whole file was already here. Nothing to send, and nothing wrong. - if (!Object.keys(create).length) { - await get().loadAll(); - return { created: 0, skipped, alike: 0 }; - } try { - const { created, refused } = await createCards(accountId, create); + const { created, updated, refused } = await writeCards(accountId, create, update); // Nothing at all got in: say why rather than report importing none as // though the file had been empty. The LDIF import said this already; a // 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"); - /* 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 }; + if (!created && !updated) throw new Error(refused ? setErrorMessage(refused) : "the server did not accept any of its contacts"); + /* No likeness count: a vCard carries a UID, so anything already here was + matched on it rather than guessed at. */ + return { created, updated, alike: 0 }; } finally { await get().loadAll(); } @@ -559,7 +587,7 @@ export const useContacts = create((set, get) => ({ }); let created: number; try { - const r = await createCards(accountId, create); + const r = await writeCards(accountId, create); created = r.created; if (!created) throw new Error(r.refused ? setErrorMessage(r.refused) : "the server did not accept any of its contacts"); } finally { @@ -576,7 +604,7 @@ export const useContacts = create((set, get) => ({ * card was imported -- and it is the confusion rather than the duplication * that was reported as the harm. */ - return { created, skipped: 0, alike }; + return { created, updated: 0, alike }; }, async loadPrincipals() { diff --git a/web/src/views/contacts/ContactsView.tsx b/web/src/views/contacts/ContactsView.tsx index a35bddb..d4600e1 100644 --- a/web/src/views/contacts/ContactsView.tsx +++ b/web/src/views/contacts/ContactsView.tsx @@ -136,22 +136,24 @@ export function ContactsView({ id }: { id?: string }) { * LDIF may arrive as .ldif, .ldi, .txt or with no extension at all, and * the name is the least reliable thing about it. */ - const { created, skipped, alike } = /^\s*BEGIN:VCARD/im.test(text) + const { created, updated, alike } = /^\s*BEGIN:VCARD/im.test(text) ? await contacts.importVCard(text, book.id) : await contacts.importLdif(text, book.id); /* - * The two counts kept apart, as the calendar import keeps them: "Imported - * 3 contacts" over a file of two hundred reads as a failure when the rest - * were simply already here, and a re-import of an unchanged export would - * otherwise report importing nothing at all. + * The counts kept apart, as the calendar import keeps them. "Imported 3 + * contacts" over a file of two hundred reads as a failure when the other + * hundred and ninety-seven were updated, and a re-import of a corrected + * export -- the reason for doing this at all -- creates nothing and would + * otherwise report importing nothing. */ const imported = plural(created, { one: "Imported {n} contact", other: "Imported {n} contacts" }); - 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" })}`); + const refreshed = plural(updated, { one: "{n} updated", other: "{n} updated" }); + if (!created) toast.success(plural(updated, { one: "Updated {n} contact, nothing new", other: "Updated {n} contacts, nothing new" })); + else if (updated) toast.success(`${imported} · ${refreshed}`); 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 + * LDIF has no UID to match on, so nothing was updated 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). */ diff --git a/web/src/views/mail/VCardCard.tsx b/web/src/views/mail/VCardCard.tsx index 7ee84fb..f600270 100644 --- a/web/src/views/mail/VCardCard.tsx +++ b/web/src/views/mail/VCardCard.tsx @@ -17,14 +17,15 @@ export function VCardCard({ part, accountId }: { part: EmailBodyPart; accountId: const text = await client.fetchBlobText(accountId, part.blobId!, "text/vcard"); const book = Object.values(contacts.books).find((b) => b.isDefault) ?? Object.values(contacts.books)[0]; if (!book) throw new Error("No address book available"); - const { created, skipped } = await contacts.importVCard(text, book.id); + const { created, updated } = await contacts.importVCard(text, book.id); setDone(true); /* - * A card attached to a message is usually one you have already been sent - * once. Saying "Added 0 contacts" for that would read as a failure; it is - * the opposite -- there was nothing to do. + * A card attached to a message is usually one you already have, and now + * the newer copy wins rather than being dropped -- so the message says it + * was brought up to date. "Added 0 contacts" would read as a failure when + * the opposite happened. */ - if (!created && skipped) toast.success(plural(skipped, { one: "Already in your contacts", other: "All {n} are already in your contacts" })); + if (!created && updated) toast.success(plural(updated, { one: "Updated the contact you already had", other: "Updated {n} contacts you already had" })); else toast.success(plural(created, { one: "Added {n} contact", other: "Added {n} contacts" })); } catch (err) { toast.error((err as Error).message);