diff --git a/lib/global-search/__tests__/providers.test.ts b/lib/global-search/__tests__/providers.test.ts index 111bf736c..fe54a5f7e 100644 --- a/lib/global-search/__tests__/providers.test.ts +++ b/lib/global-search/__tests__/providers.test.ts @@ -163,7 +163,7 @@ describe('mail provider', () => { const result = await mailProvider.remote(parseSearchQuery('zebra'), account('login-a'), { limit: 25, signal }); expect(buildUnifiedAccountClients).toHaveBeenCalledWith({ includeGroup: true, scopeToClientAccountId: 'login-a' }); // The stub login has no trash/junk role folders, so no exclusion is added. - expect(advancedSearchEmails).toHaveBeenCalledWith({ text: 'zebra' }, undefined, 25, 0); + expect(advancedSearchEmails).toHaveBeenCalledWith({ text: 'zebra' }, undefined, 25, 0, { palette: true }); expect(result.hasMore).toBe(true); expect(result.hits[0]).toMatchObject({ id: 'm9', localAccountId: 'login-a', jmapAccountId: 'jmap-a', source: 'remote', subtitle: 'Inbox', diff --git a/lib/global-search/providers/mail.ts b/lib/global-search/providers/mail.ts index 14850795c..d52db2eab 100644 --- a/lib/global-search/providers/mail.ts +++ b/lib/global-search/providers/mail.ts @@ -119,6 +119,7 @@ export const mailProvider: SearchProvider = { entry.isShared ? entry.jmapAccountId : undefined, limit, position, + { palette: true }, ); if (result.hasMore) hasMore = true; for (const raw of result.emails) { diff --git a/lib/jmap/__tests__/search-merge.test.ts b/lib/jmap/__tests__/search-merge.test.ts index 4bfa9135b..4ef295281 100644 --- a/lib/jmap/__tests__/search-merge.test.ts +++ b/lib/jmap/__tests__/search-merge.test.ts @@ -8,7 +8,7 @@ import { openMemoryStore } from '@/lib/store/__tests__/helpers/node-adapter'; import { JMAPClient } from '../client'; import type { SearchEmailsResult } from '../client-interface'; import { setOfflineHooks, type OfflineHooks } from '../offline-hooks'; -import { filterHasTextTerms, LocalSearchMerge } from '../search-merge'; +import { filterHasTextTerms, LocalSearchMerge, mailboxesFitFilter } from '../search-merge'; import type { Email } from '../types'; function mail(id: string, minute: number, extra: Partial = {}): Email { @@ -16,10 +16,11 @@ function mail(id: string, minute: number, extra: Partial = {}): Email { } /** A side of the search: the whole result list, answered a page at a time, newest first. */ -function side(rows: Email[], total: number = rows.length) { +function side(rows: Email[], fixedTotal?: number) { const asked: number[] = []; const answer = async (position: number, limit: number): Promise => { asked.push(position); + const total = fixedTotal ?? rows.length; const emails = rows.slice(position, position + limit); return { emails, total, hasMore: position + emails.length < total }; }; @@ -199,12 +200,138 @@ describe('paging', () => { expect(ids(await page(0))).toEqual(['l3', 's2']); }); - it('sends a position it has no cursor for to the server alone', async () => { + it('walks a search again when it has no cursor, so no server row is skipped', async () => { + const server = side([mail('s4', 40), mail('s3', 30), mail('s2', 20), mail('s1', 10)]); + const local = side([mail('l5', 50), mail('l25', 25)]); + const merge = new LocalSearchMerge(); + const ask = (key: string, position: number) => merge.run({ + key, limit: 2, position, + server: (at) => server.answer(at, 2), + local: (at) => local.answer(at, 2), + }); + // The caller holds l5, s4, s3. As a merged position 3 would skip s2 on the server. + const result = await ask('evicted', 3); + expect(ids(result)).toEqual(['l25', 's2']); + }); + + it('sends a position with no cursor straight to the server when the store is off', async () => { const server = side([mail('s2', 20), mail('s1', 10)]); - const local = side([mail('l3', 30)]); - const result = await pager(server, local, 2)(1); + const result = await pager(server, null, 2)(1); expect(ids(result)).toEqual(['s1']); - expect(local.asked).toEqual([]); + expect(server.asked).toEqual([0, 1]); + }); + + it('ends the list, and never guesses, past how far it will walk again', async () => { + const rows = Array.from({ length: 100 }, (_, i) => mail(`s${i}`, 1000 - i)); + const server = side(rows); + const local = side([mail('l', 2000)]); + const result = await pager(server, local, 2)(90); + expect(result.emails).toEqual([]); + expect(result.hasMore).toBe(false); + expect(server.asked).not.toContain(90); + }); + + describe('a refresh of a longer list', () => { + function paged() { + const serverRows = [mail('s6', 60), mail('s5', 50), mail('s4', 40), mail('s3', 30), mail('s2', 20), mail('s1', 10)]; + const server = side(serverRows); + const local = side([mail('l7', 70)]); + return { server, local, page: pager(server, local, 2) }; + } + + it('leaves the next load-more where the list had got to', async () => { + const { page } = paged(); + await page(0); + await page(2); + const refreshed = await page(0); + expect(ids(refreshed)).toEqual(['l7', 's6']); + const more = await page(4); + expect(ids(more)).toEqual(['s3', 's2']); + }); + + it('keeps total the same when nothing changed', async () => { + const { page } = paged(); + const first = await page(0); + expect(first.total).toBe(7); + await page(2); + expect((await page(0)).total).toBe(7); + }); + + it('does not forget a local row that was on a later page', async () => { + const server = side([mail('s6', 60), mail('s5', 50), mail('s4', 40), mail('s3', 30), mail('s2', 20), mail('s1', 10)]); + const local = side([mail('l45', 45)]); + const page = pager(server, local, 2); + expect(ids(await page(0))).toEqual(['s6', 's5']); + expect(ids(await page(2))).toEqual(['l45', 's4']); + // This first page has no local row, so it knows nothing of l45 by itself. + const refreshed = await page(0); + expect(ids(refreshed)).toEqual(['s6', 's5']); + expect(refreshed.total).toBe(7); + // The list holds four rows, three of them the server's: s3 is next. + const more = await page(4); + expect(ids(more)).toEqual(['s3', 's2']); + expect(more.total).toBe(7); + }); + + it('still lets a new search of the same words page from its own start', async () => { + const { page } = paged(); + await page(0); + await page(2); + await page(0); + // The list was replaced by that page, so the next one starts right after it. + expect(ids(await page(2))).toEqual(['s5', 's4']); + }); + + it('moves on past a new row the refresh brought in', async () => { + const serverRows = [mail('s4', 40), mail('s3', 30), mail('s2', 20), mail('s1', 10)]; + const server = side(serverRows); + const local = side([mail('l5', 50)]); + const page = pager(server, local, 2); + await page(0); + await page(2); + serverRows.unshift(mail('new', 60)); + const refreshed = await page(0); + expect(refreshed.total).toBe(6); + // The caller's list grew by one: l5 s4 s3 s2 plus the new row. + expect(ids(await page(5))).toEqual(['s1']); + }); + }); + + describe('how long a cursor lasts', () => { + function search(merge: LocalSearchMerge, key: string, position: number, rows: ReturnType, local: ReturnType) { + return merge.run({ key, limit: 1, position, server: (at) => rows.answer(at, 1), local: (at) => local.answer(at, 1) }); + } + const other = () => [side([mail('o', 1)]), side([])] as const; + + it('keeps the search in use when many others come and go', async () => { + const merge = new LocalSearchMerge(); + const server = side([mail('s2', 20), mail('s1', 10)]); + const local = side([mail('l3', 30)]); + await search(merge, 'mine', 0, server, local); + for (let i = 0; i < 60; i += 1) await search(merge, `other-${i}`, 0, ...other()); + await search(merge, 'mine', 1, server, local); + for (let i = 60; i < 120; i += 1) await search(merge, `other-${i}`, 0, ...other()); + server.asked.length = 0; + expect(ids(await search(merge, 'mine', 2, server, local))).toEqual(['s1']); + // The cursor was there: no walking from the start again. + expect(server.asked).toEqual([1]); + }); + + it('still returns the right rows once a cursor is gone', async () => { + const merge = new LocalSearchMerge(); + const server = side([mail('s2', 20), mail('s1', 10)]); + const local = side([mail('l3', 30)]); + await search(merge, 'mine', 0, server, local); + for (let i = 0; i < 70; i += 1) await search(merge, `other-${i}`, 0, ...other()); + expect(ids(await search(merge, 'mine', 1, server, local))).toEqual(['s2']); + }); + }); + + it('counts a row both sides have once in total', async () => { + const server = side([mail('s1', 10)]); + const local = side([mail('s1', 10)]); + const result = await pager(server, local, 5)(0); + expect(result.total).toBe(1); }); it('keeps searches apart by key', async () => { @@ -218,7 +345,142 @@ describe('paging', () => { }); }); +describe('rows only the store has', () => { + function run( + serverRows: Email[], + localRows: Email[], + verify: (rows: Email[]) => Promise, + limit = 5, + ) { + const server = side(serverRows); + const local = side(localRows); + return new LocalSearchMerge().run({ + key: 'k', limit, position: 0, + server: (at) => server.answer(at, limit), + local: (at) => local.answer(at, limit), + verify, + }); + } + + it('drops what the server no longer has, and fills the page with the rest', async () => { + const verify = vi.fn(async (rows: Email[]) => rows.filter((row) => row.id !== 'deleted')); + const result = await run([mail('s1', 10)], [mail('deleted', 30), mail('kept', 20)], verify); + expect(ids(result)).toEqual(['kept', 's1']); + expect(result.total).toBe(2); + expect(verify.mock.calls[0][0].map((row) => row.id)).toEqual(['deleted', 'kept']); + }); + + it('shows the current copy of what the server still has', async () => { + const result = await run([], [mail('kept', 20, { subject: 'old', keywords: {} })], async () => [mail('kept', 20, { subject: 'current', keywords: { $seen: true } })]); + expect(result.emails[0].subject).toBe('current'); + expect(result.emails[0].keywords).toEqual({ $seen: true }); + }); + + it('marks them, so nothing takes them for new mail', async () => { + const result = await run([mail('s', 30)], [mail('s', 30), mail('kept', 20)], async (rows) => rows); + expect(result.emails.map((email) => email.foundLocally ?? false)).toEqual([false, true]); + }); + + it('does not ask when the store has nothing of its own', async () => { + const verify = vi.fn(async (rows: Email[]) => rows); + await run([mail('both', 10)], [mail('both', 10)], verify); + expect(verify).not.toHaveBeenCalled(); + }); + + it('does not ask about a row too old for this page', async () => { + const verify = vi.fn(async (rows: Email[]) => rows); + await run([mail('s3', 30), mail('s2', 20), mail('s1', 10)], [mail('old', 5)], verify, 2); + expect(verify).not.toHaveBeenCalled(); + }); + + it('keeps them when the check fails', async () => { + const result = await run([mail('s1', 10)], [mail('kept', 20)], async () => { throw new Error('offline again'); }); + expect(ids(result)).toEqual(['kept', 's1']); + }); + + it('keeps them, unchecked, when the search itself failed', async () => { + const verify = vi.fn(async () => []); + const result = await new LocalSearchMerge().run({ + key: 'k', limit: 5, position: 0, + server: async () => { throw new Error('server broke'); }, + local: async () => ({ emails: [mail('kept', 20)], total: 1, hasMore: false }), + verify, + }); + expect(ids(result)).toEqual(['kept']); + expect(verify).not.toHaveBeenCalled(); + }); +}); + +describe('mailboxesFitFilter', () => { + const inbox = { inbox: true }; + it.each([ + [{ inMailbox: 'inbox' }, inbox, true], + [{ inMailbox: 'inbox' }, { trash: true }, false], + [{ inMailbox: 'inbox' }, { inbox: false }, false], + [{ inMailbox: 'inbox' }, undefined, false], + [{ inMailboxOtherThan: ['trash', 'junk'] }, inbox, true], + [{ inMailboxOtherThan: ['trash', 'junk'] }, { trash: true }, false], + [{ inMailboxOtherThan: ['trash', 'junk'] }, { trash: true, inbox: true }, true], + [{ text: 'x' }, { trash: true }, true], + [{ operator: 'AND', conditions: [{ inMailbox: 'inbox' }, { text: 'x' }] }, { trash: true }, false], + [{ operator: 'AND', conditions: [{ inMailbox: 'inbox' }, { text: 'x' }] }, inbox, true], + [{ operator: 'OR', conditions: [{ inMailbox: 'a' }, { inMailbox: 'b' }] }, { b: true }, true], + [{ operator: 'OR', conditions: [{ inMailbox: 'a' }, { inMailbox: 'b' }] }, inbox, false], + [{ operator: 'OR', conditions: [{ inMailbox: 'a' }, { text: 'x' }] }, inbox, true], + [{ operator: 'NOT', conditions: [{ inMailbox: 'trash' }] }, inbox, true], + [{ operator: 'NOT', conditions: [{ inMailbox: 'trash' }] }, { trash: true }, false], + [{ operator: 'NOT', conditions: [{ text: 'x' }] }, inbox, true], + [{ inMailbox: 'trash', text: 'x' }, inbox, false], + ])('%j with %j is %s', (filter, boxes, expected) => { + expect(mailboxesFitFilter(filter, boxes as Record | undefined)).toBe(expected); + }); +}); + describe('errors', () => { + it('leaves no unhandled rejection when the store fails while the server is still working', async () => { + const unhandled: unknown[] = []; + const note = (reason: unknown) => { unhandled.push(reason); }; + process.on('unhandledRejection', note); + try { + const result = await new LocalSearchMerge().run({ + key: 'k', limit: 10, position: 0, + server: async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + return { emails: [mail('s', 1)], total: 1, hasMore: false }; + }, + local: async () => { throw new Error('store broke'); }, + }); + await new Promise((resolve) => setTimeout(resolve, 10)); + expect(ids(result)).toEqual(['s']); + expect(unhandled).toEqual([]); + } finally { + process.off('unhandledRejection', note); + } + }); + + it('keeps the list open when the server fails on a later page', async () => { + let fail = false; + const rows = side([mail('s4', 40), mail('s3', 30), mail('s2', 20), mail('s1', 10)]); + const local = side([mail('l5', 50), mail('l25', 25)]); + const merge = new LocalSearchMerge(); + const run = (position: number) => merge.run({ + key: 'k', limit: 2, position, + server: async (at) => { + if (fail) throw new Error('server broke'); + return rows.answer(at, 2); + }, + local: (at) => local.answer(at, 2), + }); + expect((await run(0)).hasMore).toBe(true); + fail = true; + const during = await run(2); + expect(ids(during)).toEqual(['l25']); + // Local rows ran out, but the server said it had more: the list isn't over. + expect(during.hasMore).toBe(true); + fail = false; + expect(ids(await run(3))).toEqual(['s3', 's2']); + }); + it('keeps the server result when the store fails', async () => { const merge = new LocalSearchMerge(); const result = await merge.run({ @@ -303,6 +565,41 @@ describe('errors', () => { beforeEach(() => { vi.useFakeTimers(); }); afterEach(() => { vi.useRealTimers(); }); + it('counts the grace period from when the server answered', async () => { + const merge = new LocalSearchMerge(); + let local: (value: SearchEmailsResult) => void = () => {}; + const answer = merge.run({ + key: 'k', limit: 10, position: 0, + server: async () => { + await new Promise((resolve) => setTimeout(resolve, 5_000)); + return { emails: [mail('s', 1)], total: 1, hasMore: false }; + }, + local: () => new Promise((resolve) => { local = resolve; }), + }); + await vi.advanceTimersByTimeAsync(5_000); + // The store answers soon after a slow server, and is still in time. + await vi.advanceTimersByTimeAsync(500); + local({ emails: [mail('l', 2)], total: 1, hasMore: false }); + expect(ids(await answer)).toEqual(['l', 's']); + }); + + it('waits only as long as it was told to', async () => { + const merge = new LocalSearchMerge({ graceMs: 200 }); + let done = false; + const answer = merge.run({ + key: 'k', limit: 10, position: 0, + server: async () => ({ emails: [mail('s', 1)], total: 1, hasMore: false }), + local: () => new Promise(() => {}), + }).then((result) => { + done = true; + return result; + }); + await vi.advanceTimersByTimeAsync(199); + expect(done).toBe(false); + await vi.advanceTimersByTimeAsync(1); + expect(ids(await answer)).toEqual(['s']); + }); + it('does not hold the server result back for long', async () => { const merge = new LocalSearchMerge(); const answer = merge.run({ @@ -363,6 +660,8 @@ describe('through the client, with a real store', () => { let serverRows: Array>; let serverFails: boolean; let requests: Array, string]>>; + /** What Email/get by id finds on the server, past the search's own rows. */ + let serverKnows: Array>; function fakeServerFetch() { return vi.fn(async (url: string, init?: RequestInit) => { @@ -370,6 +669,14 @@ describe('through the client, with a real store', () => { const { methodCalls } = JSON.parse(String(init?.body)) as { methodCalls: Array<[string, Record, string]> }; requests.push(methodCalls); if (serverFails) return new Response('broken', { status: 500 }); + const [firstName, firstArgs, firstId] = methodCalls[0]; + if (firstName === 'Email/get' && Array.isArray(firstArgs.ids)) { + const wanted = firstArgs.ids as string[]; + const list = [...serverRows, ...serverKnows].filter((row) => wanted.includes(row.id as string)); + return new Response(JSON.stringify({ + methodResponses: [[firstName, { list, notFound: wanted.filter((id) => !list.some((row) => row.id === id)) }, firstId]], + }), { status: 200 }); + } const position = (methodCalls[0][1].position as number) ?? 0; const limit = methodCalls[0][1].limit as number; const page = serverRows.slice(position, position + limit); @@ -425,6 +732,9 @@ describe('through the client, with a real store', () => { beforeEach(() => { serverFails = false; requests = []; + serverKnows = [ + { id: 'only-local', subject: 'Zahlungserinnerungsrechnung', receivedAt: '2026-10-03T00:00:00Z', mailboxIds: { inbox: true } }, + ]; serverRows = [ { id: 'newer', subject: 'Rechnung', receivedAt: '2026-10-06T00:00:00Z', mailboxIds: { inbox: true } }, { id: 'both', subject: 'Stromrechnung, server copy', receivedAt: '2026-10-05T00:00:00Z', mailboxIds: { inbox: true } }, @@ -451,6 +761,76 @@ describe('through the client, with a real store', () => { expect(result.hasMore).toBe(false); }); + it('drops a row deleted on the server since the store last synced', async () => { + const engine = await storeEngine(); + setOfflineHooks(hooksWith((_client, calls) => engine.answer(calls))); + serverKnows = []; + const client = await connected(); + const result = await client.searchEmails('rechnung'); + expect(result.emails.map((email) => email.id)).toEqual(['newer', 'both']); + expect(result.total).toBe(2); + }); + + it('drops a row moved out of the folder searched', async () => { + const engine = await storeEngine(); + setOfflineHooks(hooksWith((_client, calls) => engine.answer(calls))); + serverKnows = [{ id: 'only-local', subject: 'x', receivedAt: '2026-10-03T00:00:00Z', mailboxIds: { trash: true } }]; + const client = await connected(); + const inFolder = await client.searchEmails('rechnung', 'inbox'); + expect(inFolder.emails.map((email) => email.id)).toEqual(['newer', 'both']); + const everywhere = await client.searchEmails('rechnung'); + expect(everywhere.emails.map((email) => email.id)).toEqual(['newer', 'both', 'only-local']); + const notInTrash = await client.advancedSearchEmails({ operator: 'AND', conditions: [{ text: 'rechnung' }, { inMailboxOtherThan: ['trash'] }] }); + expect(notInTrash.emails.map((email) => email.id)).toEqual(['newer', 'both']); + }); + + it('searches the store without snippets, and asks only for those of its own rows', async () => { + const engine = await storeEngine(); + const asked: string[][] = []; + const snippetIds: unknown[] = []; + setOfflineHooks(hooksWith((_client, calls) => { + asked.push(calls.map(([name]) => name)); + for (const [name, args] of calls) if (name === 'SearchSnippet/get') snippetIds.push(args.emailIds); + return engine.answer(calls); + })); + const client = await connected(); + await client.searchEmails('rechnung'); + expect(asked).toEqual([['Email/query', 'Email/get'], ['SearchSnippet/get']]); + expect(snippetIds).toEqual([['only-local']]); + }); + + it('adds the snippets of its own rows to the deferred ones', async () => { + const engine = await storeEngine(); + setOfflineHooks(hooksWith((_client, calls) => engine.answer(calls))); + const client = await connected(); + const result = await client.searchEmails('rechnung', undefined, undefined, 50, 0, { deferSnippets: true }); + expect(result.emails.every((email) => !email.searchSnippet)).toBe(true); + const snippets = await result.snippets; + expect(snippets?.find((s) => s.emailId === 'only-local')?.subject).toBe('Zahlungserinnerungsrechnung'); + }); + + it('keeps the palette from pushing out the paging of the mail list', async () => { + const engine = await storeEngine(); + setOfflineHooks(hooksWith((_client, calls) => engine.answer(calls))); + const client = await connected(); + await client.searchEmails('rechnung', undefined, undefined, 2); + for (let i = 0; i < 100; i += 1) await client.searchEmails(`wort${i}`, undefined, undefined, 2, 0, { palette: true }); + requests = []; + await client.searchEmails('rechnung', undefined, undefined, 2, 2); + // With its cursor, the next page starts where the server left off, and nothing is walked again. + const queries = requests.flatMap((calls) => calls.filter(([name]) => name === 'Email/query')); + expect(queries.map(([, args]) => args.position)).toEqual([2]); + }); + + it('waits less for the store in the palette', async () => { + setOfflineHooks(hooksWith(() => new Promise(() => {}))); + const client = await connected(); + const started = Date.now(); + const result = await client.searchEmails('rechnung', undefined, undefined, 50, 0, { palette: true }); + expect(result.emails.length).toBe(2); + expect(Date.now() - started).toBeLessThan(900); + }); + it('merges into advancedSearchEmails too, and leaves the server request as it was', async () => { const engine = await storeEngine(); setOfflineHooks(hooksWith((_client, calls) => engine.answer(calls))); diff --git a/lib/jmap/client-interface.ts b/lib/jmap/client-interface.ts index 377e61525..fd3e936f3 100644 --- a/lib/jmap/client-interface.ts +++ b/lib/jmap/client-interface.ts @@ -94,6 +94,11 @@ export interface SearchRequestOptions { * where the list must not wait for the highlighting. */ deferSnippets?: boolean; + /** + * For the search palette. It searches on every few keystrokes, so it keeps its + * paging apart from the mail list's, and waits less for the local store. + */ + palette?: boolean; } export interface SearchEmailsResult { diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 6563320f0..7243c10d9 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -9,7 +9,7 @@ import type { SieveScript, SieveCapabilities } from "./sieve-types"; import type { BlobBatch, CalendarEventUpdateOptions, CalendarQueryOptions, FileNodeListings, IJMAPClient, KeywordDiscoveryResult, KeywordInfo, KeywordMigration, SearchEmailsResult, SearchRequestOptions, UploadedEmailImport, UploadedEmailImportResult } from "./client-interface"; import { attachSearchSnippets, filterHasSnippetTerms, snippetFilterFor, type SearchSnippetResult } from "@/lib/search-snippet"; import { textConditions, textFilter } from "./search-utils"; -import { filterHasTextTerms, LocalSearchMerge } from "./search-merge"; +import { filterHasTextTerms, LocalSearchMerge, mailboxesFitFilter } from "./search-merge"; import { batched, batchedBySize, itemsPerRequest } from "./request-limits"; import { keywordPointer, pointerToken } from "./patch-pointer"; import { FirstTouchGate } from "./first-touch-gate"; @@ -1640,6 +1640,7 @@ export class JMAPClient implements IJMAPClient { */ private offlineOnly = false; private readonly searchMerge = new LocalSearchMerge(); + private readonly paletteSearchMerge = new LocalSearchMerge({ graceMs: 200 }); constructor(serverUrl: string, username: string, password: string) { this.serverUrl = serverUrl.replace(/\/$/, ''); @@ -4422,19 +4423,52 @@ export class JMAPClient implements IJMAPClient { if (!searchLocal || this.offlineOnly || hooks.skipNetwork(this) || !filterHasTextTerms(filter)) { return server(position); } - return this.searchMerge.run({ + const targetAccountId = accountId || this.accountId; + const merge = options.palette ? this.paletteSearchMerge : this.searchMerge; + return merge.run({ key: JSON.stringify([accountId ?? null, filter]), limit, position, server, local: async (at) => { - const targetAccountId = accountId || this.accountId; - const answer = await searchLocal.call(hooks, this, this.searchCalls(targetAccountId, filter, limit, at, false)); + // No snippets here: excerpting every hit is the slow part, and most of + // them are the server's to show. See localSnippets. + const answer = await searchLocal.call(hooks, this, this.searchCalls(targetAccountId, filter, limit, at, true)); return answer ? this.searchResultFrom(answer as JMAPResponse, accountId, limit, at) : null; }, + verify: (rows) => this.currentSearchRows(rows, filter, accountId), + localSnippets: async (ids) => { + if (!filterHasSnippetTerms(filter)) return null; + const answer = await searchLocal.call(hooks, this, [ + ["SearchSnippet/get", { accountId: targetAccountId, filter: snippetFilterFor(filter), emailIds: ids }, "snippets"], + ]); + return answer ? this.searchSnippetsFrom(answer as JMAPResponse) ?? null : null; + }, + deferSnippets: options.deferSnippets, }); } + /** + * The server's copies of rows only the local store found, without those it + * no longer has or has outside the filter's folders. The filter is checked + * before the ids get their shared-account namespace, since it has bare ones. + */ + private async currentSearchRows(rows: Email[], filter: Record, accountId: string | undefined): Promise { + const response = await this.request([ + ["Email/get", { + accountId: accountId || this.accountId, + ids: rows.map((row) => row.id), + properties: [...EMAIL_LIST_PROPERTIES], + }, "0"], + ]); + const [name, result] = response.methodResponses?.[0] ?? []; + if (name !== "Email/get" || !result) throw new Error(methodErrorMessage(response, 'Failed to get emails')); + const list = applyListAuthenticationResults((result.list || []) as Email[]) + .filter((email) => mailboxesFitFilter(filter, email.mailboxIds)); + if (accountId && accountId !== this.accountId) namespaceMailboxIds(list, accountId); + return list; + } + private searchCalls( targetAccountId: string, filter: Record, diff --git a/lib/jmap/search-merge.ts b/lib/jmap/search-merge.ts index a38cb53d1..a7200b0f9 100644 --- a/lib/jmap/search-merge.ts +++ b/lib/jmap/search-merge.ts @@ -7,14 +7,28 @@ * have a message, the server's copy is kept. A page never holds more than * `limit` rows, so a local row can push a server row to the next page. * + * A row only the store has may be stale: deleted or moved since the store last + * synced. So the server is asked for those rows before they're shown. What it + * no longer has, or has outside the filter's folders, is dropped, and what it + * still has is shown as its current copy. It takes a request of its own, since + * these ids are only known once both answers are in. If that request fails, or + * the search itself did, the rows stay: an error must not lose results. + * * Callers page by counting the rows they have, so `position` counts merged * rows and means nothing to the server once a local row is among them. A cursor - * for each search remembers how far each side was read. A request past 0 with - * no cursor goes to the server alone, exactly as it would without a store. + * for each search remembers how far each side was read: + * - Position 0 can be a new search or a refresh of a longer list, and only the + * next page tells which. So it runs on a cursor of its own next to the one + * already there. The next page takes whichever ended closest to its position. + * - With no cursor (it was evicted), the search is walked again from the start + * up to the position. A merged position never goes to the server as it is. + * - When the store has nothing to say for this search, no merged row exists and + * positions are the server's own, so requests pass straight through. * * `hasMore` is true while either side has rows this search hasn't returned. * A local list made only of rows the server also has can therefore cost one - * extra page that comes back empty, and then it's false. + * extra page that comes back empty, and then it's false. When the server fails + * on a later page, what it last said about having more still counts. * * `total` is the server's total plus the local-only rows returned so far, and * never less than the local total or the rows already returned. It can grow @@ -26,17 +40,22 @@ * * An error on one side keeps the other side's rows. When the server fails and * the store has nothing, the server's error stands, since an empty list would - * read as "no results". A slow store never holds up the server's answer for - * longer than LOCAL_GRACE_MS. + * read as "no results". A slow store holds up the server's answer for at most + * the grace period, counted from when the server answered. Snippets for local + * rows are asked for separately, so excerpting bodies in the worker doesn't + * hold up the list either. */ import { debug } from '@/lib/debug'; +import { attachSearchSnippets, type SearchSnippetResult } from '@/lib/search-snippet'; import type { SearchEmailsResult } from './client-interface'; import type { Email } from './types'; const LOCAL_GRACE_MS = 1_000; const LOCAL_ALONE_MS = 5_000; -const MAX_CURSORS = 8; +const MAX_CURSORS = 64; +/** How far back an evicted search is walked again. Past this, the list ends. */ +const MAX_WALK_PAGES = 20; const TEXT_FILTER_KEYS = ['text', 'from', 'to', 'cc', 'bcc', 'subject', 'body']; @@ -53,16 +72,62 @@ export function filterHasTextTerms(filter: Record): boolean { return TEXT_FILTER_KEYS.some((key) => typeof filter[key] === 'string' && (filter[key] as string).trim().length > 0); } +/** True, false, or null when the filter's answer depends on more than the folders. */ +function mailboxAnswer(filter: Record, boxes: readonly string[]): boolean | null { + if (typeof filter.operator === 'string') { + const answers = (Array.isArray(filter.conditions) ? filter.conditions as Array> : []) + .map((condition) => mailboxAnswer(condition, boxes)); + const unknown = answers.includes(null); + if (filter.operator === 'AND') return answers.includes(false) ? false : unknown ? null : true; + const any = answers.includes(true) ? true : unknown ? null : false; + if (filter.operator === 'OR') return any; + if (filter.operator === 'NOT') return any === null ? null : !any; + return null; + } + const answers: boolean[] = []; + if (typeof filter.inMailbox === 'string') answers.push(boxes.includes(filter.inMailbox)); + if (Array.isArray(filter.inMailboxOtherThan)) { + const excluded = filter.inMailboxOtherThan as string[]; + answers.push(boxes.some((box) => !excluded.includes(box))); + } + if (answers.includes(false)) return false; + return answers.length > 0 && Object.keys(filter).length === answers.length ? true : null; +} + +/** Whether a message's folders still fit the filter. Conditions on anything else don't count against it. */ +export function mailboxesFitFilter(filter: Record, mailboxIds: Record | undefined): boolean { + const boxes = Object.entries(mailboxIds ?? {}).filter(([, on]) => on).map(([id]) => id); + return mailboxAnswer(filter, boxes) !== false; +} + interface MergeCursor { /** Merged rows returned so far, which is where the next page should ask from. */ next: number; serverPosition: number; localPosition: number; serverTotal: number; + serverHasMore: boolean; /** Local-only rows returned so far. */ extras: number; + /** Local rows the server said are gone. */ + rejected: number; /** Every row returned so far, so none appears twice. */ shown: Set; + /** The store has nothing for this search: positions are the server's. */ + passthrough: boolean; +} + +function newCursor(): MergeCursor { + return { + next: 0, serverPosition: 0, localPosition: 0, serverTotal: 0, serverHasMore: false, + extras: 0, rejected: 0, shown: new Set(), passthrough: false, + }; +} + +interface CursorEntry { + cursor: MergeCursor; + /** A position 0 that came after later pages: a new search or a refresh. */ + restarted: MergeCursor | null; } function receivedTime(email: Email): number { @@ -73,7 +138,7 @@ interface PageMerge { emails: Email[]; serverRead: number; localRead: number; - extras: number; + extraIds: string[]; } /** One merged page. Rows are added to `shown`. */ @@ -86,9 +151,9 @@ export function mergeSearchPages( ): PageMerge { const serverIds = new Set(server.map((email) => email.id)); const emails: Email[] = []; + const extraIds: string[] = []; let s = 0; let l = 0; - let extras = 0; while (emails.length < limit) { const fromServer = server[s]; const fromLocal = local[l]; @@ -106,30 +171,31 @@ export function mergeSearchPages( shown.add(fromServer.id); s += 1; } else { - emails.push(fromLocal); + emails.push({ ...fromLocal, foundLocally: true }); shown.add(fromLocal.id); + extraIds.push(fromLocal.id); l += 1; - extras += 1; } } - return { emails, serverRead: s, localRead: l, extras }; + return { emails, serverRead: s, localRead: l, extraIds }; } -/** The promise's value, or null if it fails or takes longer than `ms`. */ +/** The promise's value, or null if it takes longer than `ms`. The promise must not reject. */ function within(promise: Promise, ms: number): Promise { return new Promise((resolve) => { const timer = setTimeout(() => resolve(null), ms); - promise.then( - (value) => { - clearTimeout(timer); - resolve(value); - }, - (error) => { - clearTimeout(timer); - debug.warn('jmap', 'Local search failed:', error); - resolve(null); - }, - ); + void promise.then((value) => { + clearTimeout(timer); + resolve(value); + }); + }); +} + +/** Runs a store call with its failure handled right away, so it can't go unhandled while the server is awaited. */ +function safely(what: string, call: () => Promise): Promise { + return Promise.resolve().then(call).catch((error: unknown) => { + debug.warn('jmap', `${what} failed:`, error); + return null; }); } @@ -141,28 +207,92 @@ export interface MergedSearchRequest { server(position: number): Promise; /** Null when the store doesn't cover this account. */ local(position: number): Promise; + /** The server's current copies of these rows, leaving out what's gone or no longer fits the search. */ + verify?(rows: Email[]): Promise; + /** Highlights for rows only the store has. */ + localSnippets?(ids: string[]): Promise; + /** Hand local snippets back as `snippets` instead of waiting for them. */ + deferSnippets?: boolean; +} + +export interface LocalSearchMergeOptions { + /** How long a slow store may hold up the server's answer. */ + graceMs?: number; } export class LocalSearchMerge { - private readonly cursors = new Map(); + private readonly cursors = new Map(); + private readonly graceMs: number; - private startCursor(key: string): MergeCursor { - const cursor: MergeCursor = { next: 0, serverPosition: 0, localPosition: 0, serverTotal: 0, extras: 0, shown: new Set() }; + constructor(options: LocalSearchMergeOptions = {}) { + this.graceMs = options.graceMs ?? LOCAL_GRACE_MS; + } + + /** Least recently used goes first, so a search being paged through stays. */ + private remember(key: string, entry: CursorEntry): void { this.cursors.delete(key); - this.cursors.set(key, cursor); + this.cursors.set(key, entry); if (this.cursors.size > MAX_CURSORS) this.cursors.delete(this.cursors.keys().next().value as string); - return cursor; } async run(request: MergedSearchRequest): Promise { const { key, limit, position } = request; - const cursor = position === 0 ? this.startCursor(key) : this.cursors.get(key); - if (!cursor) return request.server(position); + const entry = this.cursors.get(key); + if (position === 0) { + const fresh = newCursor(); + const alongside = entry && entry.cursor.next > 0 ? entry.cursor : null; + this.remember(key, alongside ? { cursor: alongside, restarted: fresh } : { cursor: fresh, restarted: null }); + return this.page(fresh, request, 0, limit, { alongside }); + } + if (!entry) return this.walkTo(request); + this.remember(key, entry); + const cursor = this.choose(entry, position); + if (cursor.passthrough) return request.server(position); + return this.page(cursor, request, position, limit, {}); + } + /** Which cursor the page after a position 0 continues from. Only that one is kept from then on. */ + private choose(entry: CursorEntry, position: number): MergeCursor { + const { cursor, restarted } = entry; + if (!restarted) return cursor; + entry.restarted = null; + const restartedIsCloser = Math.abs(position - restarted.next) <= Math.abs(position - cursor.next); + if (cursor.passthrough || restartedIsCloser) { + entry.cursor = restarted; + return restarted; + } + for (const id of restarted.shown) cursor.shown.add(id); + return cursor; + } + + /** No cursor: read the search again from the start, so the rows the caller holds are accounted for. */ + private async walkTo(request: MergedSearchRequest): Promise { + const { key, limit, position } = request; + const cursor = newCursor(); + let last: SearchEmailsResult | null = null; + for (let page = 0; cursor.next < position && !cursor.passthrough; page += 1) { + if (page === MAX_WALK_PAGES) return { emails: [], hasMore: false, total: last?.total ?? position }; + last = await this.page(cursor, request, cursor.next, Math.min(limit, position - cursor.next), { walking: true }); + if (last.emails.length === 0 || !last.hasMore) break; + } + this.remember(key, { cursor, restarted: null }); + if (cursor.passthrough) return request.server(position); + // The search ended before the position: the caller holds rows that are gone. + cursor.next = position; + return this.page(cursor, request, position, limit, {}); + } + + private async page( + cursor: MergeCursor, + request: MergedSearchRequest, + position: number, + limit: number, + { alongside = null, walking = false }: { alongside?: MergeCursor | null; walking?: boolean }, + ): Promise { // A caller that removed rows or added its own asks from a little off // where the last page ended, and the server's side moves with it. const serverPosition = Math.max(0, cursor.serverPosition + position - cursor.next); - const localAnswer = Promise.resolve().then(() => request.local(cursor.localPosition)); + const localAnswer = safely('Local search', () => request.local(cursor.localPosition)); let serverPage: SearchEmailsResult | null = null; let serverError: unknown = null; @@ -171,33 +301,82 @@ export class LocalSearchMerge { } catch (error) { serverError = error; } - const localPage = await within(localAnswer, serverPage ? LOCAL_GRACE_MS : LOCAL_ALONE_MS); + const localPage = await within(localAnswer, serverPage ? this.graceMs : LOCAL_ALONE_MS); if (!serverPage && !localPage?.emails.length) throw serverError; if (serverPage && !localPage && cursor.extras === 0) { // Nothing local in this search: the server's answer, untouched. - this.cursors.delete(key); + cursor.passthrough = true; + cursor.next = position + serverPage.emails.length; return serverPage; } const server = serverPage?.emails ?? []; const serverHasMore = serverPage?.hasMore ?? false; - const local = localPage?.emails ?? []; + const local = serverPage + ? await this.verified(request, cursor, server, serverHasMore, localPage?.emails ?? []) + : localPage?.emails ?? []; const merged = mergeSearchPages(server, serverHasMore, local, limit, cursor.shown); cursor.serverPosition = serverPosition + merged.serverRead; cursor.localPosition += merged.localRead; cursor.next = position + merged.emails.length; - cursor.extras += merged.extras; - if (serverPage) cursor.serverTotal = serverPage.total; + cursor.extras += merged.extraIds.length; + if (serverPage) { + cursor.serverTotal = serverPage.total; + cursor.serverHasMore = merged.serverRead < server.length || serverHasMore; + } - const serverLeft = merged.serverRead < server.length || serverHasMore; const localLeft = localPage ? merged.localRead < local.length || localPage.hasMore : false; - return { + // A refresh of a longer list counts the local rows already in that list. + const extras = Math.max(cursor.extras, alongside?.extras ?? 0); + const rejected = Math.max(cursor.rejected, alongside?.rejected ?? 0); + const result: SearchEmailsResult = { ...serverPage, emails: merged.emails, - hasMore: serverLeft || localLeft, - total: Math.max(cursor.serverTotal + cursor.extras, localPage?.total ?? 0, cursor.next), + hasMore: cursor.serverHasMore || localLeft, + total: Math.max(cursor.serverTotal + extras, (localPage?.total ?? 0) - rejected, cursor.next), }; + if (!walking && merged.extraIds.length > 0 && request.localSnippets) await this.addLocalSnippets(request, merged, result); + return result; + } + + /** The local rows, with what the server no longer has marked as read past, and the rest as its current copies. */ + private async verified( + request: MergedSearchRequest, + cursor: MergeCursor, + server: readonly Email[], + serverHasMore: boolean, + local: Email[], + ): Promise { + if (!request.verify) return local; + const serverIds = new Set(server.map((email) => email.id)); + const oldestPlaceable = serverHasMore && server.length > 0 ? receivedTime(server[server.length - 1]) : -Infinity; + const candidates = local.filter((row) => !serverIds.has(row.id) && !cursor.shown.has(row.id) && receivedTime(row) >= oldestPlaceable); + if (candidates.length === 0) return local; + let current: Email[]; + try { + current = await request.verify(candidates); + } catch (error) { + debug.warn('jmap', 'Checking local search results failed:', error); + return local; + } + const byId = new Map(current.map((row) => [row.id, row])); + for (const row of candidates) { + if (byId.has(row.id)) continue; + cursor.shown.add(row.id); + cursor.rejected += 1; + } + return local.map((row) => byId.get(row.id) ?? row); + } + + private async addLocalSnippets(request: MergedSearchRequest, merged: PageMerge, result: SearchEmailsResult): Promise { + const asked = safely('Local snippets', () => (request.localSnippets as NonNullable)(merged.extraIds)); + if (!request.deferSnippets) { + attachSearchSnippets(result.emails, (await within(asked, this.graceMs)) ?? undefined); + return; + } + const fromServer = result.snippets; + result.snippets = Promise.all([fromServer, asked]).then(([a, b]) => (a || b ? [...(a ?? []), ...(b ?? [])] : undefined)); } } diff --git a/lib/jmap/types.ts b/lib/jmap/types.ts index 647a57309..6a19f659f 100644 --- a/lib/jmap/types.ts +++ b/lib/jmap/types.ts @@ -51,6 +51,8 @@ export interface Email { // subject / body excerpt with the matched terms in , set on search // hits only. See lib/search-snippet.ts. searchSnippet?: { subject: string | null; preview: string | null }; + /** A search row only the local store found. It's no sign of new mail, whatever its date (lib/jmap/search-merge.ts). */ + foundLocally?: boolean; textBody?: EmailBodyPart[]; htmlBody?: EmailBodyPart[]; bodyValues?: Record; diff --git a/stores/__tests__/email-store-search-refresh-toast.test.ts b/stores/__tests__/email-store-search-refresh-toast.test.ts new file mode 100644 index 000000000..7eba711f7 --- /dev/null +++ b/stores/__tests__/email-store-search-refresh-toast.test.ts @@ -0,0 +1,60 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { useEmailStore } from '../email-store'; +import { useSettingsStore } from '../settings-store'; +import { useAuthStore } from '../auth-store'; +import type { Email, Mailbox } from '@/lib/jmap/types'; +import type { IJMAPClient } from '@/lib/jmap/client-interface'; + +// A refresh during a search in the Inbox can bring a row only the local store +// found, for example once an old message's body has arrived. It isn't new mail, +// even when it's the newest row on the page and not in the list yet. + +const inbox = { id: 'inbox', role: 'inbox', isShared: false } as Mailbox; + +function email(id: string, receivedAt: string, extra: Partial = {}): Email { + return { + id, threadId: `thread-${id}`, mailboxIds: { inbox: true }, keywords: {}, + subject: id, receivedAt, from: [{ email: 'sender@example.com' }], to: [], + preview: '', size: 1, hasAttachment: false, ...extra, + } as Email; +} + +function client(rows: Email[]) { + return { + advancedSearchEmails: vi.fn(async () => ({ emails: rows, total: rows.length, hasMore: false })), + getThreads: vi.fn(async () => []), + getAccountId: () => 'me', + } as unknown as IJMAPClient; +} + +async function refreshWith(row: Email) { + const listed = email('listed', '2026-10-01T00:00:00Z'); + useEmailStore.setState({ emails: [listed], totalEmails: 1 }); + await useEmailStore.getState().refreshCurrentMailbox(client([row, listed])); + return useEmailStore.getState(); +} + +describe('the new-mail notification during a search', () => { + beforeEach(() => { + useAuthStore.setState(useAuthStore.getInitialState()); + useSettingsStore.setState({ + emailsPerPage: 25, messageListOrder: [], messageListOrderScope: 'inbox', + notificationMutes: { accounts: [], mailboxes: {} }, + }); + useEmailStore.setState({ + ...useEmailStore.getInitialState(), + selectedMailbox: 'inbox', mailboxes: [inbox], searchQuery: 'rechnung', searchMailboxId: 'inbox', + }); + }); + + it('stays quiet for a row only the local store found, and lists it', async () => { + const state = await refreshWith(email('old-find', '2026-10-15T00:00:00Z', { foundLocally: true })); + expect(state.newEmailNotification).toBeNull(); + expect(state.emails.map((row) => row.id)).toContain('old-find'); + }); + + it('still notifies for a new row from the server', async () => { + const state = await refreshWith(email('new', '2026-10-15T00:00:00Z')); + expect(state.newEmailNotification?.id).toBe('new'); + }); +}); diff --git a/stores/email-store.ts b/stores/email-store.ts index 97d75caca..95d245d2f 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -7054,6 +7054,7 @@ export const useEmailStore = create((set, get) => ({ !selectedKeyword && mailbox?.role === 'inbox' && !currentEmails.some(e => e.id === newFirst.id) && + !newFirst.foundLocally && (!keepOpenThread || insertedCount > 0) ) { notifyNewInboxMail(client, mailbox, newFirst, get);