diff --git a/packages/host/app/resources/search.ts b/packages/host/app/resources/search.ts index 262f92e2102..17fcff20f29 100644 --- a/packages/host/app/resources/search.ts +++ b/packages/host/app/resources/search.ts @@ -27,6 +27,9 @@ import { isFileDefInstance, isFileDefCodeRef, isClientEvaluable, + canonicalizeFilterRefs, + identifyCard, + isResolvedCodeRef, matchInstanceAgainstFilter, makeInstanceComparator, logger as runtimeLogger, @@ -44,6 +47,7 @@ import type { Filter, Query } from '@cardstack/runtime-common/query'; import { searchErrorEntry } from '../lib/search-error-entry'; +import type LoaderService from '../services/loader-service'; import type NetworkService from '../services/network'; import type RealmServerService from '../services/realm-server'; import type StoreService from '../services/store'; @@ -162,6 +166,7 @@ export class SearchResource< @service declare private network: NetworkService; @service declare private realmServer: RealmServerService; @service declare private store: StoreService; + @service declare private loaderService: LoaderService; #storeServiceOverride: StoreService | undefined; @tracked private realmsToSearch: RealmIdentifier[] = []; // Resist the urge to expose this property publicly as that may entice @@ -183,6 +188,19 @@ export class SearchResource< // once it lands. @tracked private matchAPI: CardAPIForMatching | undefined; #matchAPILoading = false; + // The active filter with its `type`/`on` refs rewritten to canonical + // (defining-module) form, mirroring the rewrite the query engine applies + // before compiling `types` membership predicates. The matcher compares refs + // against `identifyCard` of an instance's class — the canonical ref — so a + // filter carrying a re-export spelling (e.g. file-api's FileDef) must be + // canonicalized here too or the reconciler would strip the server's correct + // results. `source` ties the rewrite to the exact filter object it was + // computed from; `incomplete` means some ref didn't resolve, in which case + // the search stays a server-only passthrough rather than risking a client + // evaluation the server disagrees with. + @tracked private canonicalizedFilter: + | { source: Filter; filter: Filter; incomplete: boolean } + | undefined; // The query currently driving results, tracked so the client filtering step // re-derives when the filter/sort changes (the server result set also // changes, but reading this keeps the derivation self-contained). @@ -238,6 +256,62 @@ export class SearchResource< }); } + // Canonicalizes the live filter's `type`/`on` refs via the loader (import + // the named module, identify the export's defining-module ref) — the same + // rewrite the query engine performs through its definition lookup. Until + // this settles for the current filter, `isClientFilterEligible` keeps the + // search a server-only passthrough. + private loadCanonicalizedFilter(filter: Filter | undefined): void { + if (!filter) { + this.canonicalizedFilter = undefined; + return; + } + if (this.canonicalizedFilter?.source === filter) { + return; + } + let token = waiter.beginAsync(); + canonicalizeFilterRefs(filter, async (ref) => { + try { + let module = await this.loaderService.loader.import< + Record + >(ref.module); + let canonical = identifyCard( + module[ref.name] as Parameters[0], + ); + return canonical && isResolvedCodeRef(canonical) + ? canonical + : undefined; + } catch (error) { + this.#log.warn( + `could not canonicalize filter ref ${ref.module}/${ref.name}: ${error}`, + ); + return undefined; + } + }) + .then(({ filter: canonical, incomplete }) => { + if (isDestroyed(this) || isDestroying(this)) { + return; + } + // A newer filter may have become the active one while this resolved + // (sibling canonicalizations race, and a cache-warm import resolves + // ahead of a cold one). Drop the stale result so a late resolution + // can't clobber the current filter's result and leave `source` pointing + // at a filter that's no longer active — which would keep the search a + // server-only passthrough until the next query change. + if (this.activeQuery?.filter !== filter) { + return; + } + this.canonicalizedFilter = { + source: filter, + filter: canonical, + incomplete, + }; + }) + .finally(() => { + waiter.endAsync(token); + }); + } + private trackStoreLoad( load: Promise | undefined, source: 'seed' | 'search' | 'live-refresh', @@ -356,6 +430,7 @@ export class SearchResource< // matcher dependencies eagerly so the first eligible result set can be // reconciled without waiting on a Store mutation to trigger it. this.loadMatchAPI(); + this.loadCanonicalizedFilter(query.filter); } this.activeQuery = query; this.#doWhileRefreshing = doWhileRefreshing; @@ -527,7 +602,9 @@ export class SearchResource< return serverInstances; } let api = this.matchAPI!; - let filter = this.activeQuery?.filter; + // Eligibility (below) guarantees the canonicalized form is present and + // complete whenever the active query carries a filter. + let filter = this.canonicalizedFilter?.filter ?? this.activeQuery?.filter; // Reading the mutation-version signal here establishes the dependency that // re-derives on in-place field edits/saves (adds and deletes already flow @@ -636,6 +713,20 @@ export class SearchResource< if (filter && !isClientEvaluable(filter)) { return false; } + // The matcher needs the filter's refs in canonical form (the engine + // rewrites them server-side; see loadCanonicalizedFilter). Until the + // rewrite lands for this exact filter — or if some ref didn't resolve — + // client evaluation could disagree with the server, so pass through. + if (filter) { + let canonicalized = this.canonicalizedFilter; + if ( + !canonicalized || + canonicalized.source !== filter || + canonicalized.incomplete + ) { + return false; + } + } return true; } diff --git a/packages/host/tests/unit/query-canonicalization-test.ts b/packages/host/tests/unit/query-canonicalization-test.ts new file mode 100644 index 00000000000..79c8c320066 --- /dev/null +++ b/packages/host/tests/unit/query-canonicalization-test.ts @@ -0,0 +1,115 @@ +import { module, test } from 'qunit'; + +import { + canonicalizeFilterRefs, + rri, + type ResolvedCodeRef, + type Filter, +} from '@cardstack/runtime-common'; + +const CANONICAL_FILE_DEF: ResolvedCodeRef = { + module: rri('https://cardstack.com/base/card-api'), + name: 'FileDef', +}; +const REEXPORT_FILE_DEF: ResolvedCodeRef = { + module: rri('https://cardstack.com/base/file-api'), + name: 'FileDef', +}; +const PERSON: ResolvedCodeRef = { + module: rri('http://example.com/person'), + name: 'Person', +}; + +// Resolves the file-api re-export spelling to the canonical card-api ref, +// echoes already-canonical refs, and fails everything else. +async function resolve( + ref: ResolvedCodeRef, +): Promise { + if ( + ref.module === REEXPORT_FILE_DEF.module && + ref.name === REEXPORT_FILE_DEF.name + ) { + return CANONICAL_FILE_DEF; + } + if ( + (ref.module === CANONICAL_FILE_DEF.module && + ref.name === CANONICAL_FILE_DEF.name) || + (ref.module === PERSON.module && ref.name === PERSON.name) + ) { + return ref; + } + return undefined; +} + +module('Unit | query-canonicalization', function () { + test('rewrites a top-level `type` ref to its canonical form', async function (assert) { + let { filter, incomplete } = await canonicalizeFilterRefs( + { type: REEXPORT_FILE_DEF }, + resolve, + ); + assert.deepEqual(filter, { type: CANONICAL_FILE_DEF }); + assert.false(incomplete); + }); + + test('rewrites `on` refs nested through any/every/not', async function (assert) { + let input: Filter = { + any: [ + { on: PERSON, eq: { name: 'x' } }, + { + every: [ + { not: { on: REEXPORT_FILE_DEF, eq: { name: 'y' } } }, + { type: REEXPORT_FILE_DEF }, + ], + }, + ], + }; + let { filter, incomplete } = await canonicalizeFilterRefs(input, resolve); + assert.deepEqual(filter, { + any: [ + { on: PERSON, eq: { name: 'x' } }, + { + every: [ + { not: { on: CANONICAL_FILE_DEF, eq: { name: 'y' } } }, + { type: CANONICAL_FILE_DEF }, + ], + }, + ], + }); + assert.false(incomplete); + // the input filter is not mutated + assert.deepEqual( + (input.any![1] as { every: Filter[] }).every[1], + { type: REEXPORT_FILE_DEF }, + 'input tree is left untouched', + ); + }); + + test('an unresolvable ref stays as-given and marks the result incomplete', async function (assert) { + let bogus: ResolvedCodeRef = { + module: rri('http://example.com/nope'), + name: 'Nope', + }; + let { filter, incomplete } = await canonicalizeFilterRefs( + { every: [{ type: bogus }, { on: PERSON, eq: { name: 'x' } }] }, + resolve, + ); + assert.deepEqual(filter, { + every: [{ type: bogus }, { on: PERSON, eq: { name: 'x' } }], + }); + assert.true(incomplete); + }); + + test('duplicate refs resolve once', async function (assert) { + let calls = 0; + await canonicalizeFilterRefs( + { + any: [{ type: REEXPORT_FILE_DEF }, { type: REEXPORT_FILE_DEF }], + }, + async (ref) => { + calls++; + return resolve(ref); + }, + ); + assert.strictEqual(calls, 1); + }); +}); diff --git a/packages/realm-server/tests/search-entries-engine-test.ts b/packages/realm-server/tests/search-entries-engine-test.ts index 276faab4b14..1a0a3d68472 100644 --- a/packages/realm-server/tests/search-entries-engine-test.ts +++ b/packages/realm-server/tests/search-entries-engine-test.ts @@ -991,6 +991,74 @@ module(basename(import.meta.filename), function () { ); }); + test('a pure base-FileDef type filter matches every file row across subtypes', async function (assert) { + // Every file indexes with its full FileDef adoption chain (subtype → + // base FileDef → BaseDef), so a bare base-FileDef anchor enumerates all + // files regardless of concrete subtype — MarkdownDef, GtsFileDef, + // JsonFileDef alike. `scope: 'files'` pins the file rows so the + // dual-indexed card `.json` rows stay in play without their instance + // rows. + let doc = await testRealm.realmIndexQueryEngine.searchEntries( + parseSearchEntryQueryFromPayload({ + scope: 'files', + filter: { + 'item.on': { module: baseRRI('card-api'), name: 'FileDef' }, + }, + fields: { entry: ['item'] }, + }), + ); + let expectedFiles = [ + `${realmHref}hello.md`, + `${realmHref}person.gts`, + `${realmHref}webpage.gts`, + `${johnId}.json`, + `${janeId}.json`, + `${realmHref}home.json`, + `${realmHref}home-slash.json`, + ]; + for (let url of expectedFiles) { + assert.ok(entryFor(doc, url), `${url} matches the base FileDef anchor`); + } + assert.strictEqual( + doc.meta.page.total, + expectedFiles.length, + 'every file row (and nothing else) matches', + ); + assert.notOk( + entryFor(doc, johnId), + 'card instance rows stay out of a files-scoped search', + ); + }); + + test('the file-api re-export spelling of FileDef matches the same rows as the canonical card-api spelling', async function (assert) { + // `@cardstack/base/file-api` re-exports FileDef from + // `@cardstack/base/card-api`, and index rows stamp the canonical + // (defining-module) key. A filter carrying the re-export spelling must + // match the same rows — a code ref that resolves to the class cannot be + // a silent dead end just because it names the re-exporting module. + for (let module of [ + baseRRI('file-api'), + 'https://cardstack.com/base/file-api', + ]) { + let doc = await testRealm.realmIndexQueryEngine.searchEntries( + parseSearchEntryQueryFromPayload({ + scope: 'files', + filter: { 'item.on': { module, name: 'FileDef' } }, + fields: { entry: ['item'] }, + }), + ); + assert.strictEqual( + doc.meta.page.total, + 7, + `the ${module} spelling matches all 7 file rows`, + ); + assert.ok( + entryFor(doc, `${realmHref}hello.md`), + `hello.md matches via the ${module} spelling`, + ); + } + }); + test('A-Z `_title` sort gives file rows real sort values (not a NULL that sinks them)', async function (assert) { // Regression guard for the mixed-search A-Z sort. A file row carries the // synthetic `_title` but not `cardTitle`, so sorting on `cardTitle` diff --git a/packages/runtime-common/expression.ts b/packages/runtime-common/expression.ts index 06ddf698285..63cc9a7b7cb 100644 --- a/packages/runtime-common/expression.ts +++ b/packages/runtime-common/expression.ts @@ -112,6 +112,17 @@ export interface FieldArity { kind: 'field-arity'; } +// A type (`type`/`on`) condition, deferred so pass 1 can resolve the ref's +// definition: rows stamp `types` with the canonical (defining-module) key, so +// a ref that names the type through a re-exporting module only matches once +// the definition's canonical codeRef joins the membership keys. Resolves to +// an `any` of `types-contains` predicates over the union of the as-given +// spelling's keys and the canonical ref's keys. +export interface TypeCondition { + kind: 'type-condition'; + ref: CodeRef; +} + export type CardExpression = ( | string | Param @@ -123,6 +134,7 @@ export type CardExpression = ( | FieldQuery | FieldValue | FieldArity + | TypeCondition )[]; export function addExplicitParens(expression: CardExpression): CardExpression; @@ -230,6 +242,13 @@ export function typesContains(key: string, column = 'i.types'): TypesContains { }; } +export function typeCondition(ref: CodeRef): TypeCondition { + return { + kind: 'type-condition', + ref, + }; +} + export function fieldQuery( path: string, type: CodeRef, diff --git a/packages/runtime-common/index-query-engine.ts b/packages/runtime-common/index-query-engine.ts index f6ebb73e0a1..f0febad186a 100644 --- a/packages/runtime-common/index-query-engine.ts +++ b/packages/runtime-common/index-query-engine.ts @@ -18,10 +18,12 @@ import { type FieldValue, type FieldArity, type JsonContainsQuery, + type TypeCondition, param, isParam, tableValuedTree, typesContains, + typeCondition as typeConditionNode, separatedByCommas, addExplicitParens, any, @@ -628,7 +630,7 @@ export class IndexQueryEngine { if (!isResolvedCodeRef(ref)) { return false; } - let typeKeys = internalKeysFor(ref, undefined, this.#virtualNetwork); + let typeKeys = await this.typeKeysFor(ref); let rows = (await this.#query([ 'SELECT 1', `FROM ${tableFromOpts(opts)} AS i`, @@ -651,7 +653,7 @@ export class IndexQueryEngine { if (!isResolvedCodeRef(ref)) { return false; } - let typeKeys = internalKeysFor(ref, undefined, this.#virtualNetwork); + let typeKeys = await this.typeKeysFor(ref); let rows = (await this.#query([ 'SELECT 1', `FROM ${tableFromOpts(opts)} AS i`, @@ -1118,10 +1120,14 @@ export class IndexQueryEngine { // the type condition only consumes absolute URL card refs. private typeCondition(ref: CodeRef): CardExpression { - // Match any equivalent spelling of the type key (RRI / real-URL / - // virtual-alias), so rows indexed before references were canonicalized to - // RRI still satisfy the filter without a reindex or DB migration. - // + // Deferred to pass 1 (`handleTypeCondition`): the membership keys need the + // ref's definition, which resolves asynchronously. + return [typeConditionNode(ref)]; + } + + private async handleTypeCondition( + condition: TypeCondition, + ): Promise { // Each key is a self-contained `types-contains` membership predicate rather // than a comparison against a shared `jsonb_array_elements_text(types)` // cross-join alias. The cross join gave every type condition in a query @@ -1129,11 +1135,55 @@ export class IndexQueryEngine { // which miscompose: `not: { type: X }` failed to exclude X, and // `every: [{ type: A }, { type: B }]` was unsatisfiable. Per-row membership // makes negation a true exclusion and conjunction a true intersection. - return any( - internalKeysFor(ref, undefined, this.#virtualNetwork).map((typeKey) => [ - typesContains(typeKey), - ]), - ); + let keys = await this.typeKeysFor(condition.ref); + return any(keys.map((typeKey) => [typesContains(typeKey)])) as Expression; + } + + // Every `types` membership key a ref can legitimately match: + // + // - all equivalent spellings of the ref itself (RRI / real-URL / + // virtual-alias), so rows indexed before references were canonicalized to + // RRI still satisfy the filter without a reindex or DB migration; + // - the same spellings of the ref's canonical (defining-module) codeRef. + // Rows stamp `types` via `identifyCard`, which names the module a class is + // defined in — so a ref that names the type through a re-exporting module + // (e.g. file-api's FileDef, re-exported from card-api) only matches + // through its definition's canonical ref. The host's search resource + // applies the same canonicalization (via the loader) before its + // client-side matching, keeping the two evaluations in agreement. + // + // A ref whose definition doesn't resolve keeps only its spelling-based keys + // and matches nothing (unless rows were stamped under that spelling), + // exactly as before. + private async typeKeysFor(ref: CodeRef): Promise { + let keys = internalKeysFor(ref, undefined, this.#virtualNetwork); + if (isResolvedCodeRef(ref)) { + try { + let definition = await this.#definitionLookup.lookupDefinition(ref); + if (isResolvedCodeRef(definition.codeRef)) { + for (let key of internalKeysFor( + definition.codeRef, + undefined, + this.#virtualNetwork, + )) { + if (!keys.includes(key)) { + keys.push(key); + } + } + } + } catch (error) { + // A ref that names no resolvable type falls through to the + // spelling-based keys (matching nothing unless rows were stamped under + // that spelling) — the same way the engine's top-level catch treats a + // nonexistent type as an empty result rather than an error. Any other + // failure is unexpected and propagates rather than silently + // narrowing the match. + if (!isFilterRefersToNonexistentTypeError(error)) { + throw error; + } + } + } + return keys; } // The card's primary `id` and a FileDef's `url` index in URL form, but a @@ -1534,6 +1584,8 @@ export class IndexQueryEngine { return this.handleFieldArity(element); } else if (element.kind === 'json-contains-query') { return this.handleJsonContainsQuery(element); + } else if (element.kind === 'type-condition') { + return this.handleTypeCondition(element); } else { throw assertNever(element); } diff --git a/packages/runtime-common/index.ts b/packages/runtime-common/index.ts index def67a98db0..30ee4b5e0bc 100644 --- a/packages/runtime-common/index.ts +++ b/packages/runtime-common/index.ts @@ -1044,6 +1044,7 @@ export * from './realm-index-card.ts'; export * from './cached-fetch.ts'; export * from './definition-lookup.ts'; export * from './definitions.ts'; +export * from './query-canonicalization.ts'; export * from './searchable-routes.ts'; export * from './catalog.ts'; export * from './commands.ts'; diff --git a/packages/runtime-common/query-canonicalization.ts b/packages/runtime-common/query-canonicalization.ts new file mode 100644 index 00000000000..2ef40bd4b81 --- /dev/null +++ b/packages/runtime-common/query-canonicalization.ts @@ -0,0 +1,95 @@ +import { isResolvedCodeRef } from './code-ref.ts'; + +import type { ResolvedCodeRef } from './code-ref.ts'; +import type { + AnyFilter, + CardTypeFilter, + EveryFilter, + Filter, + NotFilter, + TypedFilter, +} from './query.ts'; + +// Resolves a code ref to the canonical (defining-module) ref for the type it +// names, or undefined when the ref can't be resolved. The server backs this +// with the definition lookup (a re-export spelling's definition entry carries +// the defining module's codeRef); the host backs it with a loader import plus +// `identifyCard` on the loaded export. +export type CanonicalRefResolver = ( + ref: ResolvedCodeRef, +) => Promise; + +export interface CanonicalizedFilter { + filter: Filter; + // True when some `type`/`on` ref could not be resolved; that ref is kept + // as-given in the returned filter. The server treats an unresolvable ref as + // matching nothing (the ref names no known type), while the host's search + // resource uses this flag to fall back to server-only evaluation so the + // client matcher never disagrees with the server about such a ref. + incomplete: boolean; +} + +// Rewrites every `type`/`on` code ref in a filter tree to its canonical +// (defining-module) form. Index rows stamp `types` with the canonical key +// (`identifyCard` of the class), so a filter ref that names the same type +// through a re-exporting module (e.g. `file-api`'s FileDef, re-exported from +// `card-api`) only matches once both sides agree on the canonical spelling. +// URL-form tolerance (RRI / real-URL / virtual-alias) is separate and stays +// in `internalKeysFor`; this handles the module-identity half. +export async function canonicalizeFilterRefs( + filter: Filter, + resolve: CanonicalRefResolver, +): Promise { + let incomplete = false; + // One lookup per distinct ref spelling within a query. The memo holds the + // in-flight promise rather than the settled value: sibling filter nodes are + // walked concurrently, and a value-memo would let both siblings pass the + // has() check before either lookup settles. + let memo = new Map>(); + + async function canonicalRef( + ref: CardTypeFilter['type'], + ): Promise { + if (!isResolvedCodeRef(ref)) { + return ref; + } + let key = `${ref.module}/${ref.name}`; + if (!memo.has(key)) { + memo.set(key, resolve(ref)); + } + let canonical = await memo.get(key); + if (!canonical) { + incomplete = true; + return ref; + } + return canonical; + } + + async function walk(node: Filter): Promise { + let out: Filter = { ...node }; + if ('type' in out && out.type) { + out.type = await canonicalRef(out.type); + } + let typed = out as TypedFilter; + if (typed.on) { + typed.on = await canonicalRef(typed.on); + } + if ('any' in out && Array.isArray(out.any)) { + (out as AnyFilter).any = await Promise.all( + (out as AnyFilter).any.map(walk), + ); + } + if ('every' in out && Array.isArray(out.every)) { + (out as EveryFilter).every = await Promise.all( + (out as EveryFilter).every.map(walk), + ); + } + if ('not' in out && out.not) { + (out as NotFilter).not = await walk((out as NotFilter).not); + } + return out; + } + + let canonicalized = await walk(filter); + return { filter: canonicalized, incomplete }; +}