mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-11-04 08:15:41 +00:00
fix: keep an account's contacts when only another account could answer
This commit is contained in:
+22
-14
@@ -19,6 +19,7 @@ import { isTaskLikeObject, type ScannedCalendarObject } from "@/lib/calendar-com
|
||||
import { DEFAULT_CALENDAR_COMPONENTS, mkCalendarCollection, newCalendarCollectionName } from "@/lib/webdav/calendar-collection";
|
||||
import { sanitizeDisplayName, splitMailbox } from "@/lib/rfc5322-mailbox";
|
||||
import { decodeFileNodeName } from "./filenode-name";
|
||||
import { PartialAccountsError } from "./partial-accounts";
|
||||
import { fileNodeKind, fromDraftFileNode, toDraftFileNodeCreate } from "./filenode-kind";
|
||||
import { fileNodeDepth } from "./filenode-depth";
|
||||
import { contactFromWire, contactToWire } from "./contact-wire";
|
||||
@@ -7327,13 +7328,16 @@ export class JMAPClient implements IJMAPClient {
|
||||
}
|
||||
}
|
||||
|
||||
/** Throws when no account could answer, so a failure doesn't read as having no address books. */
|
||||
/**
|
||||
* Throws PartialAccountsError when an account couldn't answer, with what the
|
||||
* others gave, so a failure doesn't read as that account having no books.
|
||||
*/
|
||||
async getAllAddressBooks(): Promise<AddressBook[]> {
|
||||
let answered = 0;
|
||||
const failed: string[] = [];
|
||||
let failure: unknown;
|
||||
const allBooks: AddressBook[] = [];
|
||||
const primaryId = this.getContactsAccountId();
|
||||
try {
|
||||
const allBooks: AddressBook[] = [];
|
||||
const primaryId = this.getContactsAccountId();
|
||||
const accountIds = this.getContactCapableAccountIds();
|
||||
|
||||
for (const accountId of accountIds) {
|
||||
@@ -7356,23 +7360,23 @@ export class JMAPClient implements IJMAPClient {
|
||||
isShared: !isPrimary,
|
||||
}));
|
||||
allBooks.push(...books);
|
||||
answered += 1;
|
||||
} else if (response.methodResponses?.[0]?.[0] === "error") {
|
||||
const refused = response.methodResponses[0][1];
|
||||
throw new Error(refused?.description || refused?.type || "AddressBook/get failed");
|
||||
}
|
||||
} catch (error) {
|
||||
console.error(`Failed to fetch address books for account ${accountId}:`, error);
|
||||
failed.push(accountId);
|
||||
failure ??= error;
|
||||
}
|
||||
}
|
||||
|
||||
if (answered > 0 || !failure) return allBooks;
|
||||
if (failed.length === 0) return allBooks;
|
||||
} catch (error) {
|
||||
console.error('Failed to fetch all address books:', error);
|
||||
return this.getAddressBooks();
|
||||
}
|
||||
throw failure;
|
||||
throw new PartialAccountsError(allBooks, failed, primaryId, failure);
|
||||
}
|
||||
|
||||
async createAddressBook(name: string): Promise<AddressBook> {
|
||||
@@ -8286,13 +8290,17 @@ export class JMAPClient implements IJMAPClient {
|
||||
}
|
||||
}
|
||||
|
||||
/** Throws when no account could answer, so a failure doesn't read as an empty address book. */
|
||||
/**
|
||||
* Throws PartialAccountsError when an account couldn't answer, with what the
|
||||
* others gave, so a failure doesn't read as that account having no cards.
|
||||
*/
|
||||
async getAllContacts(): Promise<ContactCard[]> {
|
||||
let answered = 0;
|
||||
const failed: string[] = [];
|
||||
let failure: unknown;
|
||||
let allContacts: ContactCard[] = [];
|
||||
const primaryId = this.getContactsAccountId();
|
||||
try {
|
||||
const allContacts: ContactCard[] = [];
|
||||
const primaryId = this.getContactsAccountId();
|
||||
allContacts = [];
|
||||
const accountIds = this.getContactCapableAccountIds();
|
||||
|
||||
for (const accountId of accountIds) {
|
||||
@@ -8313,19 +8321,19 @@ export class JMAPClient implements IJMAPClient {
|
||||
isShared: !isPrimary,
|
||||
}));
|
||||
allContacts.push(...contacts);
|
||||
answered += 1;
|
||||
} catch (error) {
|
||||
console.error(`Failed to fetch contacts for account ${accountId}:`, error);
|
||||
failed.push(accountId);
|
||||
failure ??= error;
|
||||
}
|
||||
}
|
||||
|
||||
if (answered > 0 || !failure) return allContacts;
|
||||
if (failed.length === 0) return allContacts;
|
||||
} catch (error) {
|
||||
console.error('Failed to fetch all contacts:', error);
|
||||
return this.getContacts();
|
||||
}
|
||||
throw failure;
|
||||
throw new PartialAccountsError(allContacts, failed, primaryId, failure);
|
||||
}
|
||||
|
||||
async getContact(contactId: string, accountId?: string): Promise<ContactCard | null> {
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
/**
|
||||
* A listing across JMAP accounts where at least one account couldn't answer.
|
||||
* It carries what the others gave. An account that failed isn't an account
|
||||
* with nothing in it, so the caller keeps what it had for that one.
|
||||
*/
|
||||
export class PartialAccountsError<T> extends Error {
|
||||
constructor(
|
||||
readonly items: T[],
|
||||
readonly failedAccountIds: string[],
|
||||
readonly primaryAccountId: string,
|
||||
cause: unknown,
|
||||
) {
|
||||
super(cause instanceof Error ? cause.message : 'An account could not answer');
|
||||
this.name = 'PartialAccountsError';
|
||||
}
|
||||
}
|
||||
|
||||
/** By name: `instanceof` fails across Next's bundle copies. */
|
||||
export function partialAccounts<T>(error: unknown): PartialAccountsError<T> | null {
|
||||
return error instanceof Error && error.name === 'PartialAccountsError' ? error as PartialAccountsError<T> : null;
|
||||
}
|
||||
|
||||
/** What to show after a partial listing: the answers, and what's held for the accounts that failed. */
|
||||
export function withFailedAccountsKept<T extends { accountId?: string }>(partial: PartialAccountsError<T>, held: readonly T[]): T[] {
|
||||
const failed = new Set(partial.failedAccountIds);
|
||||
return [...partial.items, ...held.filter((item) => failed.has(item.accountId ?? partial.primaryAccountId))];
|
||||
}
|
||||
@@ -6,6 +6,8 @@
|
||||
import { afterEach, describe, expect, it } from 'vitest';
|
||||
import { JMAPClient } from '@/lib/jmap/client';
|
||||
import { setOfflineHooks, type OfflineHooks } from '@/lib/jmap/offline-hooks';
|
||||
import { partialAccounts } from '@/lib/jmap/partial-accounts';
|
||||
import type { AddressBook, ContactCard } from '@/lib/jmap/types';
|
||||
import { DEFAULT_SCOPE } from '../area';
|
||||
import { contactsArea } from '../contacts';
|
||||
import { AccountEngine } from '../engine/account-engine';
|
||||
@@ -80,6 +82,27 @@ describe('the client over the store', () => {
|
||||
expect(books.map((b) => b.id).sort()).toEqual(['book1', `${SHARED_ACCOUNT}:teambook`]);
|
||||
});
|
||||
|
||||
it('says which account failed, with what the others gave', async () => {
|
||||
const { client, engine } = await offlineClient(seeded());
|
||||
const failPrimary: Partial<OfflineHooks> = {
|
||||
skipNetwork: () => true,
|
||||
answer: async (_client, calls) => (calls[0][1].accountId === JMAP_ACCOUNT
|
||||
? { methodResponses: calls.map(([, , id]) => ['error', { type: 'serverUnavailable' }, id]) }
|
||||
: engine.answer(calls)),
|
||||
record: () => {},
|
||||
noteNetwork: () => {},
|
||||
};
|
||||
setOfflineHooks(failPrimary as OfflineHooks);
|
||||
const contacts = await client.getAllContacts().catch((error: unknown) => error);
|
||||
const partial = partialAccounts<ContactCard>(contacts);
|
||||
expect(partial?.failedAccountIds).toEqual([JMAP_ACCOUNT]);
|
||||
expect(partial?.primaryAccountId).toBe(JMAP_ACCOUNT);
|
||||
expect(partial?.items.map((c) => c.name?.full)).toEqual(['Dora Team']);
|
||||
const books = partialAccounts<AddressBook>(await client.getAllAddressBooks().catch((error: unknown) => error));
|
||||
expect(books?.failedAccountIds).toEqual([JMAP_ACCOUNT]);
|
||||
expect(books?.items.map((b) => b.originalId)).toEqual(['teambook']);
|
||||
});
|
||||
|
||||
it('searches with its own filter', async () => {
|
||||
const { client } = await offlineClient(seeded());
|
||||
expect((await client.searchContacts('müll')).map((c) => c.name?.full)).toEqual(['Carla Müller']);
|
||||
|
||||
@@ -13,6 +13,7 @@ import type { AddressBook, ContactCard, ContactMedia } from '@/lib/jmap/types';
|
||||
import type { IJMAPClient } from '@/lib/jmap/client-interface';
|
||||
import { clearContactPhotoCache } from '@/lib/contact-photo-cache';
|
||||
import { PrincipalsRequestError } from '@/lib/jmap/principals-error';
|
||||
import { PartialAccountsError } from '@/lib/jmap/partial-accounts';
|
||||
import { setClientLookup } from '../client-registry';
|
||||
|
||||
vi.stubGlobal('crypto', { randomUUID: () => '00000000-0000-0000-0000-000000000000' });
|
||||
@@ -1387,6 +1388,46 @@ describe('contact-store', () => {
|
||||
expect(state.addressBooks.map((b) => b.id)).toEqual(['acc-a::booka', 'acc-b::bookb']);
|
||||
});
|
||||
|
||||
it('keeps the primary account\'s cards when only a shared account answered', async () => {
|
||||
const refusal = new PartialAccountsError<ContactCard>([makeContact({ id: 'shared:s2', accountId: 'shared', isShared: true })], ['me'], 'me', new Error('offline'));
|
||||
const failing = { getAllContacts: vi.fn().mockRejectedValue(refusal) } as unknown as IJMAPClient;
|
||||
useContactStore.setState({
|
||||
contacts: [
|
||||
makeContact({ id: 'p1', accountId: 'me' }),
|
||||
// A card just created has no accountId yet: it's the primary account's.
|
||||
makeContact({ id: 'p2' }),
|
||||
makeContact({ id: 'shared:s1', accountId: 'shared', isShared: true }),
|
||||
],
|
||||
});
|
||||
await useContactStore.getState().fetchContacts(failing);
|
||||
const state = useContactStore.getState();
|
||||
expect(state.contacts.map((c) => c.id).sort()).toEqual(['p1', 'p2', 'shared:s2']);
|
||||
expect(state.error).toBe('Failed to fetch contacts');
|
||||
expect(state.isLoading).toBe(false);
|
||||
});
|
||||
|
||||
it('keeps the primary account\'s books when only a shared account answered', async () => {
|
||||
const refusal = new PartialAccountsError<AddressBook>([{ id: 'shared:b', name: 'B', accountId: 'shared' } as AddressBook], ['me'], 'me', new Error('offline'));
|
||||
const failing = { getAllAddressBooks: vi.fn().mockRejectedValue(refusal) } as unknown as IJMAPClient;
|
||||
useContactStore.setState({ addressBooks: [{ id: 'mine', name: 'Mine', accountId: 'me' } as AddressBook] });
|
||||
await useContactStore.getState().fetchAddressBooks(failing);
|
||||
expect(useContactStore.getState().addressBooks.map((b) => b.id).sort()).toEqual(['mine', 'shared:b']);
|
||||
});
|
||||
|
||||
it('keeps a failed JMAP account of one login while its others refresh', async () => {
|
||||
const refusal = new PartialAccountsError<ContactCard>([makeContact({ id: 's2', accountId: 'shared' })], ['me'], 'me', new Error('offline'));
|
||||
const failing = { getAllContacts: vi.fn().mockRejectedValue(refusal) } as unknown as IJMAPClient;
|
||||
useContactStore.setState({
|
||||
contacts: [
|
||||
makeContact({ id: 'acc-a::p1', accountId: 'me', localAccountId: 'acc-a' }),
|
||||
makeContact({ id: 'acc-a::s1', accountId: 'shared', localAccountId: 'acc-a' }),
|
||||
makeContact({ id: 'acc-b::x', accountId: 'me', localAccountId: 'acc-b' }),
|
||||
],
|
||||
});
|
||||
await useContactStore.getState().fetchAllAccountsContacts([{ client: failing, localAccountId: 'acc-a' }]);
|
||||
expect(useContactStore.getState().contacts.map((c) => c.id).sort()).toEqual(['acc-a::p1', 'acc-a::s2']);
|
||||
});
|
||||
|
||||
it('keeps the list when the only account fails', async () => {
|
||||
const failing = { getAllContacts: vi.fn().mockRejectedValue(new Error('offline')) } as unknown as IJMAPClient;
|
||||
useContactStore.setState({ contacts: [makeContact()] });
|
||||
|
||||
+25
-5
@@ -13,6 +13,7 @@ import { sanitizeDisplayName, splitMailbox } from '@/lib/rfc5322-mailbox';
|
||||
import { inlineContactMedia, writeContactWithNewPhoto, type ContactSaveOptions } from '@/lib/contact-media';
|
||||
import { contactToView } from '@/lib/plugin-projection';
|
||||
import { PrincipalsRequestError } from '@/lib/jmap/principals-error';
|
||||
import { PartialAccountsError, partialAccounts, withFailedAccountsKept } from '@/lib/jmap/partial-accounts';
|
||||
import type { Principal } from '@/lib/jmap/types';
|
||||
import { currentStoreEpoch } from '@/lib/store-epoch';
|
||||
|
||||
@@ -614,7 +615,14 @@ export const useContactStore = create<ContactStore>()(
|
||||
} catch (error) {
|
||||
if (currentStoreEpoch() !== epoch) return;
|
||||
console.error('Failed to fetch contacts:', error);
|
||||
set({ error: 'Failed to fetch contacts', isLoading: false, contactsLoaded: true });
|
||||
// An account that couldn't answer keeps its cards; the others are fresh.
|
||||
const partial = partialAccounts<ContactCard>(error);
|
||||
set((state) => ({
|
||||
error: 'Failed to fetch contacts',
|
||||
isLoading: false,
|
||||
contactsLoaded: true,
|
||||
contacts: partial ? withFailedAccountsKept(partial, state.contacts) : state.contacts,
|
||||
}));
|
||||
}
|
||||
},
|
||||
|
||||
@@ -656,7 +664,11 @@ export const useContactStore = create<ContactStore>()(
|
||||
} catch (error) {
|
||||
if (currentStoreEpoch() !== epoch) return;
|
||||
console.error('Failed to fetch address books:', error);
|
||||
set({ error: 'Failed to fetch address books' });
|
||||
const partial = partialAccounts<AddressBook>(error);
|
||||
set((state) => ({
|
||||
error: 'Failed to fetch address books',
|
||||
addressBooks: partial ? withFailedAccountsKept(partial, state.addressBooks) : state.addressBooks,
|
||||
}));
|
||||
}
|
||||
},
|
||||
|
||||
@@ -671,8 +683,12 @@ export const useContactStore = create<ContactStore>()(
|
||||
return prefixContactsWithLocalAccount(list, localAccountId);
|
||||
} catch (error) {
|
||||
debug.error(`Failed to fetch contacts for account ${localAccountId}:`, error);
|
||||
// Keep what this account's cards were: it just couldn't answer.
|
||||
return get().contacts.filter((contact) => contact.localAccountId === localAccountId);
|
||||
// Keep what this login's cards were: it just couldn't answer.
|
||||
const held = get().contacts.filter((contact) => contact.localAccountId === localAccountId);
|
||||
const partial = partialAccounts<ContactCard>(error);
|
||||
if (!partial) return held;
|
||||
const fresh = prefixContactsWithLocalAccount(partial.items, localAccountId);
|
||||
return withFailedAccountsKept(new PartialAccountsError(fresh, partial.failedAccountIds, partial.primaryAccountId, error), held);
|
||||
}
|
||||
}),
|
||||
);
|
||||
@@ -695,7 +711,11 @@ export const useContactStore = create<ContactStore>()(
|
||||
return prefixAddressBooksWithLocalAccount(list, localAccountId);
|
||||
} catch (error) {
|
||||
debug.error(`Failed to fetch address books for account ${localAccountId}:`, error);
|
||||
return get().addressBooks.filter((book) => book.localAccountId === localAccountId);
|
||||
const held = get().addressBooks.filter((book) => book.localAccountId === localAccountId);
|
||||
const partial = partialAccounts<AddressBook>(error);
|
||||
if (!partial) return held;
|
||||
const fresh = prefixAddressBooksWithLocalAccount(partial.items, localAccountId);
|
||||
return withFailedAccountsKept(new PartialAccountsError(fresh, partial.failedAccountIds, partial.primaryAccountId, error), held);
|
||||
}
|
||||
}),
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user