From 7118620a74ccbabbe5ea35a62bf20f9b970c4e3d Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Tue, 19 May 2026 10:42:09 -0400 Subject: [PATCH 1/2] CS-11121: dedup concurrent same-key store.search calls during prerender MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an in-flight Map on StoreService keyed by (realms, query) so multiple Glimmer components rendering on the same prerender page that fire identical store.search calls share one `_federated-search` round-trip instead of each issuing its own. Mirrors the server-side `RealmIndexQueryEngine.#inFlightSearch` landed in CS-11115 Phase 1, with the same identity-checked finally cleanup. Gated on `__boxelRenderContext` so live user searches stay uncoalesced — write-then-read freshness story unchanged outside prerender. Refactor `fetchSearchData` and `fetchAndHydrateSearchResults` onto a shared `fetchSearchDoc` HTTP+JSON path so both benefit from the dedup. Wire `clearInFlightSearch()` to render-route deactivate so an in-flight entry doesn't leak across visits. The sibling resolved-doc job-scoped cache (CS-11175) stacks on top of this layer. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/host/app/lib/search-in-flight-key.ts | 28 +++ packages/host/app/routes/render.ts | 8 + packages/host/app/services/store.ts | 112 ++++++++---- .../integration/resources/search-test.ts | 173 ++++++++++++++++++ .../unit/lib/search-in-flight-key-test.ts | 108 +++++++++++ 5 files changed, 390 insertions(+), 39 deletions(-) create mode 100644 packages/host/app/lib/search-in-flight-key.ts create mode 100644 packages/host/tests/unit/lib/search-in-flight-key-test.ts diff --git a/packages/host/app/lib/search-in-flight-key.ts b/packages/host/app/lib/search-in-flight-key.ts new file mode 100644 index 00000000000..97c9f028ffa --- /dev/null +++ b/packages/host/app/lib/search-in-flight-key.ts @@ -0,0 +1,28 @@ +import { + normalizeQueryForSignature, + type Query, +} from '@cardstack/runtime-common'; + +// Stable digest key for store-side `_federated-search` in-flight dedup. +// Mirrors `runtime-common/realm-index-query-engine.ts:searchInFlightKey` +// but takes a realms array (the host fires federated searches against +// one or more realms; the realm-server engine is per-realm so its +// version takes a single URL). +// +// Returns undefined if the inputs can't be serialized deterministically — +// caller falls back to running uncoalesced so dedup is best-effort, never +// a correctness boundary. +// +// `realms` order is preserved (not sorted): the realm-server's +// `_federated-search` iterates the array and concatenates results in +// that order, so `[a, b]` and `[b, a]` are different requests. +export function searchInFlightKey( + realms: string[], + query: Query, +): string | undefined { + try { + return JSON.stringify([realms, normalizeQueryForSignature(query)]); + } catch { + return undefined; + } +} diff --git a/packages/host/app/routes/render.ts b/packages/host/app/routes/render.ts index b0d62af3d62..6a97e37b48f 100644 --- a/packages/host/app/routes/render.ts +++ b/packages/host/app/routes/render.ts @@ -154,6 +154,14 @@ export default class RenderRoute extends Route { if (isTesting()) { (globalThis as any).__boxelRenderContext = undefined; } + // Drop any pending `_federated-search` in-flight entries the + // render-context coalescer accumulated during this visit. Entries + // self-clear on settle, but a deactivate while one is still + // in-flight could otherwise let a same-key caller arriving in the + // next render coalesce onto a promise belonging to the previous + // visit. The window is small (typically <1s per search) but the + // cost of an explicit clear is also small. + this.store.clearInFlightSearch(); (globalThis as any).__renderModel = undefined; (globalThis as any).__docsInFlight = undefined; (globalThis as any).__boxelRenderStage = undefined; diff --git a/packages/host/app/services/store.ts b/packages/host/app/services/store.ts index 820ee42c47f..de20e020f47 100644 --- a/packages/host/app/services/store.ts +++ b/packages/host/app/services/store.ts @@ -69,6 +69,7 @@ import { type LooseSingleResourceDocument, type StoreReadType, type CardResource, + type LinkableCollectionDocument, type RealmIdentifier, type RealmResourceIdentifier, type Saved, @@ -88,6 +89,7 @@ import { duringPrerenderHeaders, jobIdHeader, } from '../lib/prerender-fetch-headers'; +import { searchInFlightKey } from '../lib/search-in-flight-key'; import { errorJsonApiToErrorEntry } from '../lib/window-error-handler'; import { getSearch } from '../resources/search'; import { @@ -161,6 +163,14 @@ export default class StoreService extends Service implements StoreInterface { > = new Map(); private inflightCardMutations: Map> = new Map(); private inflightCardLoads: Map> = new Map(); + // Coalesce concurrent same-(realms, query) `_federated-search` HTTP + // calls during a prerender. Mirrors + // `RealmIndexQueryEngine.#inFlightSearch` server-side. Gated on + // `__boxelRenderContext` so live user searches stay uncoalesced — + // write-then-read freshness story unchanged outside prerender. + // Entries self-clear on `.finally()` via identity check. + private inflightSearch: Map> = + new Map(); private store: CardStore; protected isRenderStore = false; @@ -208,6 +218,7 @@ export default class StoreService extends Service implements StoreInterface { this.inflightGetFileMeta = new Map(); this.inflightCardMutations = new Map(); this.inflightCardLoads = new Map(); + this.inflightSearch = new Map(); this.autoSaveQueues = new Map(); this.autoSavePromises = new Map(); this.store = this.createCardStore(); @@ -218,6 +229,16 @@ export default class StoreService extends Service implements StoreInterface { await this.ready; } + // Drop every pending in-flight search entry. Callers awaiting an + // existing promise still get their answer (the underlying HTTP is + // already in motion); only *new* same-key callers after the drop + // miss the map and re-fetch. Wire this to anything the host + // recognizes as an invalidation boundary — render-route deactivate + // is the obvious one inside a prerender tab. + clearInFlightSearch(): void { + this.inflightSearch.clear(); + } + resetCache(opts?: { preserveReferences?: boolean }) { storeLogger.debug('resetting store cache'); if (!opts?.preserveReferences) { @@ -230,6 +251,7 @@ export default class StoreService extends Service implements StoreInterface { this.inflightGetFileMeta = new Map(); this.inflightCardMutations = new Map(); this.inflightCardLoads = new Map(); + this.inflightSearch = new Map(); this.autoSaveQueues = new Map(); this.autoSavePromises = new Map(); this.store = this.createCardStore(); @@ -819,44 +841,7 @@ export default class StoreService extends Service implements StoreInterface { realms: string[], dependencyTrackingContext?: RuntimeDependencyTrackingContext, ): Promise<{ instances: T[]; meta: QueryResultsMeta }> { - let realmServerURLs = this.realmServer.getRealmServersForRealms(realms); - // TODO remove this assertion after multi-realm server/federated identity is supported - this.realmServer.assertOwnRealmServer(realmServerURLs); - let [realmServerURL] = realmServerURLs; - let searchURL = new URL('_federated-search', realmServerURL); - let response = await this.realmServer.maybeAuthedFetchForRealms( - searchURL.href, - realms, - { - method: 'QUERY', - headers: { - Accept: SupportedMimeType.CardJson, - 'Content-Type': 'application/json', - ...duringPrerenderHeaders(), - ...consumingRealmHeader(), - ...jobIdHeader(), - }, - body: JSON.stringify({ ...query, realms }), - }, - ); - if (!response.ok) { - let responseText = await response.text(); - let err = new Error( - `status: ${response.status} - ${response.statusText}. ${responseText}`, - ) as any; - err.status = response.status; - err.responseText = responseText; - err.responseHeaders = response.headers; - throw err; - } - let json = await response.json(); - if (!isLinkableCollectionDocument(json)) { - throw new Error( - `The realm search response was not a valid collection document: - ${JSON.stringify(json, null, 2)}`, - ); - } - let collectionDoc = json; + let collectionDoc = await this.fetchSearchDoc(query, realms); // Hydrate each result into the store let instances = ( @@ -888,6 +873,55 @@ export default class StoreService extends Service implements StoreInterface { resources: (CardResource | FileMetaResource)[]; meta: QueryResultsMeta; }> { + let doc = await this.fetchSearchDoc(query, realms); + return { resources: doc.data, meta: doc.meta }; + } + + // Shared HTTP+JSON path for both `fetchSearchData` (raw resources for + // data-only callers) and `fetchAndHydrateSearchResults` (instances + // hydrated into the store). Sits between `store.search` and + // `_federated-search`. + // + // Inside a prerender tab (`__boxelRenderContext === true`) + // concurrent same-(realms, query) callers share one in-flight + // promise. Outside a prerender, every caller runs uncoalesced so + // live-SPA write-then-read flows keep their current freshness + // semantics. + private async fetchSearchDoc( + query: Query, + realms: string[], + ): Promise { + let key = (globalThis as any).__boxelRenderContext + ? searchInFlightKey(realms, query) + : undefined; + if (key !== undefined) { + let existing = this.inflightSearch.get(key); + if (existing) { + return await existing; + } + let pending = this.fetchSearchDocUncoalesced(query, realms).finally( + () => { + // Identity-check before deletion: a concurrent + // `clearInFlightSearch()` could in principle have removed + // (and a later caller re-set) this slot while we were + // in-flight. Only clean up if the map still points at *this* + // pending promise. Mirrors + // `RealmIndexQueryEngine.searchCards` server-side. + if (this.inflightSearch.get(key) === pending) { + this.inflightSearch.delete(key); + } + }, + ); + this.inflightSearch.set(key, pending); + return await pending; + } + return await this.fetchSearchDocUncoalesced(query, realms); + } + + private async fetchSearchDocUncoalesced( + query: Query, + realms: string[], + ): Promise { let realmServerURLs = this.realmServer.getRealmServersForRealms(realms); // TODO remove this assertion after multi-realm server/federated identity is supported this.realmServer.assertOwnRealmServer(realmServerURLs); @@ -925,7 +959,7 @@ export default class StoreService extends Service implements StoreInterface { ${JSON.stringify(json, null, 2)}`, ); } - return { resources: json.data, meta: json.meta }; + return json; } getSearchResource( diff --git a/packages/host/tests/integration/resources/search-test.ts b/packages/host/tests/integration/resources/search-test.ts index 1fcb901805a..5f90365dd6c 100644 --- a/packages/host/tests/integration/resources/search-test.ts +++ b/packages/host/tests/integration/resources/search-test.ts @@ -9,6 +9,7 @@ import type { Loader, Query } from '@cardstack/runtime-common'; import { baseRealm, baseRRI, + Deferred, isFileDefInstance, type Realm, type LooseSingleCardDocument, @@ -683,4 +684,176 @@ module(`Integration | search resource`, function (hooks) { 'new file is a FileDef instance', ); }); + + module(`in-flight search dedup`, function (innerHooks) { + // Holds outbound `maybeAuthedFetchForRealms` calls until the test + // releases them, so we can deterministically assert that both + // concurrent callers entered the dedup before either fetch + // settled. + let releaseFetch: Deferred; + let fetchCalls: number; + let restoreFetch: (() => void) | undefined; + + innerHooks.beforeEach(function () { + releaseFetch = new Deferred(); + fetchCalls = 0; + let realmServer = getService('realm-server') as RealmServerService; + let original = realmServer.maybeAuthedFetchForRealms.bind(realmServer); + realmServer.maybeAuthedFetchForRealms = (async (...args) => { + fetchCalls++; + await releaseFetch.promise; + return await original(...args); + }) as RealmServerService['maybeAuthedFetchForRealms']; + restoreFetch = () => { + realmServer.maybeAuthedFetchForRealms = original; + }; + }); + + innerHooks.afterEach(function () { + // Always release any held fetches so a failing test doesn't + // leak pending promises into the next test. + try { + releaseFetch.fulfill(); + } catch { + // already settled + } + restoreFetch?.(); + restoreFetch = undefined; + ( + globalThis as unknown as { __boxelRenderContext?: boolean } + ).__boxelRenderContext = undefined; + storeService.clearInFlightSearch(); + }); + + let bookQuery: Query = { + filter: { + on: { + module: testRRI('book'), + name: 'Book', + }, + eq: { 'author.lastName': 'Jones' }, + }, + }; + + test(`concurrent same-key store.search calls share a single fetch when inside a prerender`, async function (assert) { + ( + globalThis as unknown as { __boxelRenderContext?: boolean } + ).__boxelRenderContext = true; + + let p1 = storeService.search(bookQuery, [testRealmURL]); + let p2 = storeService.search(bookQuery, [testRealmURL]); + + // Yield long enough for both calls to enter `fetchSearchDoc` + // and consult the in-flight map; both fetches (if any) are + // still parked on `releaseFetch.promise`. + await new Promise((r) => setTimeout(r, 10)); + + assert.strictEqual( + fetchCalls, + 1, + 'second caller coalesces onto first in-flight fetch', + ); + + releaseFetch.fulfill(); + let [r1, r2] = await Promise.all([p1, p2]); + assert.deepEqual( + (r1 as { id?: string }[]).map((i) => i.id ?? null), + (r2 as { id?: string }[]).map((i) => i.id ?? null), + 'both callers receive the same resolved instances', + ); + }); + + test(`concurrent same-key store.search calls do NOT coalesce outside a prerender`, async function (assert) { + // __boxelRenderContext stays unset — this is the live-SPA path + let p1 = storeService.search(bookQuery, [testRealmURL]); + let p2 = storeService.search(bookQuery, [testRealmURL]); + + await new Promise((r) => setTimeout(r, 10)); + + assert.strictEqual( + fetchCalls, + 2, + 'both callers fire their own fetch outside the prerender gate', + ); + + releaseFetch.fulfill(); + await Promise.all([p1, p2]); + }); + + test(`different queries produce different keys and run independently inside a prerender`, async function (assert) { + ( + globalThis as unknown as { __boxelRenderContext?: boolean } + ).__boxelRenderContext = true; + + let otherQuery: Query = { + filter: { + on: { + module: testRRI('book'), + name: 'Book', + }, + eq: { 'author.lastName': 'Abdel-Rahman' }, + }, + }; + + let p1 = storeService.search(bookQuery, [testRealmURL]); + let p2 = storeService.search(otherQuery, [testRealmURL]); + + await new Promise((r) => setTimeout(r, 10)); + + assert.strictEqual( + fetchCalls, + 2, + 'different filters do not coalesce even inside a prerender', + ); + + releaseFetch.fulfill(); + await Promise.all([p1, p2]); + }); + + test(`sequential same-key calls fall through after the first resolves (in-flight only, no resolved-doc cache)`, async function (assert) { + ( + globalThis as unknown as { __boxelRenderContext?: boolean } + ).__boxelRenderContext = true; + + // First call: release immediately so the map self-clears. + releaseFetch.fulfill(); + await storeService.search(bookQuery, [testRealmURL]); + assert.strictEqual(fetchCalls, 1, 'first call fetches once'); + + // Second call after the in-flight slot is empty: must re-fetch + // because this layer only dedups concurrent calls. The + // resolved-doc cache (sibling ticket) is what closes the + // sequential-repeat window. + await storeService.search(bookQuery, [testRealmURL]); + assert.strictEqual( + fetchCalls, + 2, + 'sequential same-key call re-fetches after the first resolves', + ); + }); + + test(`clearInFlightSearch drops pending entries so new same-key callers re-fetch`, async function (assert) { + ( + globalThis as unknown as { __boxelRenderContext?: boolean } + ).__boxelRenderContext = true; + + let p1 = storeService.search(bookQuery, [testRealmURL]); + await new Promise((r) => setTimeout(r, 10)); + assert.strictEqual(fetchCalls, 1, 'first fetch in-flight'); + + // Simulate an invalidation event (e.g. render-route deactivate). + storeService.clearInFlightSearch(); + + let p2 = storeService.search(bookQuery, [testRealmURL]); + await new Promise((r) => setTimeout(r, 10)); + assert.strictEqual( + fetchCalls, + 2, + 'post-clear same-key caller fires a fresh fetch', + ); + + releaseFetch.fulfill(); + await Promise.all([p1, p2]); + }); + }); }); diff --git a/packages/host/tests/unit/lib/search-in-flight-key-test.ts b/packages/host/tests/unit/lib/search-in-flight-key-test.ts new file mode 100644 index 00000000000..fe5f3e60741 --- /dev/null +++ b/packages/host/tests/unit/lib/search-in-flight-key-test.ts @@ -0,0 +1,108 @@ +import { module, test } from 'qunit'; + +import type { Query, RealmResourceIdentifier } from '@cardstack/runtime-common'; + +import { searchInFlightKey } from '@cardstack/host/lib/search-in-flight-key'; + +const realmA = 'http://localhost:4201/test/'; +const realmB = 'http://localhost:4201/other/'; + +const personRef = { + module: 'http://localhost:4201/test/person' as RealmResourceIdentifier, + name: 'Person', +}; + +const baseQuery: Query = { + filter: { + on: personRef, + eq: { firstName: 'Mango' }, + }, +}; + +module('Unit | Utility | searchInFlightKey (host)', function () { + test('same realms + query produce the same key', function (assert) { + let a = searchInFlightKey([realmA], baseQuery); + let b = searchInFlightKey([realmA], baseQuery); + assert.strictEqual(a, b); + }); + + test('key is invariant under query property order', function (assert) { + let q1: Query = { + filter: { + on: personRef, + eq: { firstName: 'Mango', lastName: 'Abdel-Rahman' }, + }, + }; + let q2: Query = { + filter: { + eq: { lastName: 'Abdel-Rahman', firstName: 'Mango' }, + on: { + name: 'Person', + module: + 'http://localhost:4201/test/person' as RealmResourceIdentifier, + }, + }, + }; + assert.strictEqual( + searchInFlightKey([realmA], q1), + searchInFlightKey([realmA], q2), + ); + }); + + test('different filters produce different keys', function (assert) { + let other: Query = { + filter: { + on: personRef, + eq: { firstName: 'Vango' }, + }, + }; + assert.notStrictEqual( + searchInFlightKey([realmA], baseQuery), + searchInFlightKey([realmA], other), + ); + }); + + test('different realm sets produce different keys', function (assert) { + assert.notStrictEqual( + searchInFlightKey([realmA], baseQuery), + searchInFlightKey([realmB], baseQuery), + ); + }); + + test('realm array order matters (different orders => different keys)', function (assert) { + // The realm-server's `_federated-search` iterates the `realms` + // array and concatenates results in that order, so `[a, b]` and + // `[b, a]` are not semantically equivalent requests. + assert.notStrictEqual( + searchInFlightKey([realmA, realmB], baseQuery), + searchInFlightKey([realmB, realmA], baseQuery), + ); + }); + + test('superset / subset of realms produce different keys', function (assert) { + assert.notStrictEqual( + searchInFlightKey([realmA], baseQuery), + searchInFlightKey([realmA, realmB], baseQuery), + ); + }); + + test('pagination differences produce different keys', function (assert) { + let p1: Query = { ...baseQuery, page: { number: 0, size: 10 } }; + let p2: Query = { ...baseQuery, page: { number: 1, size: 10 } }; + assert.notStrictEqual( + searchInFlightKey([realmA], p1), + searchInFlightKey([realmA], p2), + ); + }); + + test('empty realms array produces a stable key distinct from non-empty', function (assert) { + assert.strictEqual( + searchInFlightKey([], baseQuery), + searchInFlightKey([], baseQuery), + ); + assert.notStrictEqual( + searchInFlightKey([], baseQuery), + searchInFlightKey([realmA], baseQuery), + ); + }); +}); From b8a9b60d8496b9aa0a33a44d8ac2548e7afb5506 Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Tue, 19 May 2026 11:01:34 -0400 Subject: [PATCH 2/2] test: deterministic dedup assertions; drop setTimeout race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address PR feedback: replace `setTimeout(10)` waits with synchronous assertions, and narrow the fetch counter to `_federated-search` URLs. The whole sync portion of `search → fetchSearchData → fetchSearchDoc → maybeAuthedFetchForRealms` runs before each `storeService.search()` expression returns. The wrapped fetch's `fetchCalls++` is the first line before any `await`, so by the time both invocations have returned their Promises the counter reflects every fetch that's been committed. No timeout needed — and a broken dedup would tick the counter immediately too, so the test is strictly more sensitive than the timed variant. The URL filter scopes the counter to the store-search hot path, so unrelated `maybeAuthedFetchForRealms` traffic (auth probes, registry pings, future callers) can't inflate it and turn the dedup assertion flaky. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../integration/resources/search-test.ts | 35 ++++++++++++------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/packages/host/tests/integration/resources/search-test.ts b/packages/host/tests/integration/resources/search-test.ts index 5f90365dd6c..b1500409ef3 100644 --- a/packages/host/tests/integration/resources/search-test.ts +++ b/packages/host/tests/integration/resources/search-test.ts @@ -699,10 +699,19 @@ module(`Integration | search resource`, function (hooks) { fetchCalls = 0; let realmServer = getService('realm-server') as RealmServerService; let original = realmServer.maybeAuthedFetchForRealms.bind(realmServer); - realmServer.maybeAuthedFetchForRealms = (async (...args) => { - fetchCalls++; - await releaseFetch.promise; - return await original(...args); + // Filter to `_federated-search` URLs only. The wrapper is the + // chokepoint for store-side searches, but any future caller + // routed through it (auth probes, registry pings, etc.) would + // otherwise inflate the counter and make the dedup assertion + // depend on what else the test environment happened to do. + realmServer.maybeAuthedFetchForRealms = (async (url, ...args) => { + let isSearch = + typeof url === 'string' && url.includes('_federated-search'); + if (isSearch) { + fetchCalls++; + await releaseFetch.promise; + } + return await original(url, ...args); }) as RealmServerService['maybeAuthedFetchForRealms']; restoreFetch = () => { realmServer.maybeAuthedFetchForRealms = original; @@ -740,14 +749,18 @@ module(`Integration | search resource`, function (hooks) { globalThis as unknown as { __boxelRenderContext?: boolean } ).__boxelRenderContext = true; + // The synchronous portion of the whole call chain — `search` → + // `fetchSearchData` → `fetchSearchDoc` → the wrapped + // `maybeAuthedFetchForRealms` — runs before each + // `storeService.search(...)` expression returns. `fetchCalls++` + // is the first line of the mock before any `await`, so by the + // time both invocations have completed their synchronous + // portions the counter reflects every fetch that has been + // committed (parked on `releaseFetch.promise`). Asserting + // immediately is deterministic — no timeout race. let p1 = storeService.search(bookQuery, [testRealmURL]); let p2 = storeService.search(bookQuery, [testRealmURL]); - // Yield long enough for both calls to enter `fetchSearchDoc` - // and consult the in-flight map; both fetches (if any) are - // still parked on `releaseFetch.promise`. - await new Promise((r) => setTimeout(r, 10)); - assert.strictEqual( fetchCalls, 1, @@ -768,8 +781,6 @@ module(`Integration | search resource`, function (hooks) { let p1 = storeService.search(bookQuery, [testRealmURL]); let p2 = storeService.search(bookQuery, [testRealmURL]); - await new Promise((r) => setTimeout(r, 10)); - assert.strictEqual( fetchCalls, 2, @@ -798,8 +809,6 @@ module(`Integration | search resource`, function (hooks) { let p1 = storeService.search(bookQuery, [testRealmURL]); let p2 = storeService.search(otherQuery, [testRealmURL]); - await new Promise((r) => setTimeout(r, 10)); - assert.strictEqual( fetchCalls, 2,