diff --git a/graphile/graphile-i18n/src/__tests__/plugin-cache-isolation.test.ts b/graphile/graphile-i18n/src/__tests__/plugin-cache-isolation.test.ts new file mode 100644 index 0000000000..d24b30e4b8 --- /dev/null +++ b/graphile/graphile-i18n/src/__tests__/plugin-cache-isolation.test.ts @@ -0,0 +1,93 @@ +import { createI18nPlugin } from '../plugin'; + +function makeBuild(origin: string, translatedField: string) { + const idCodec = { name: 'uuid' }; + const textCodec = { name: 'text' }; + const baseCodec = { + name: 'posts', + attributes: { + id: { codec: idCodec }, + [translatedField]: { codec: textCodec }, + }, + extensions: { + pg: { schemaName: 'tenant', name: 'posts' }, + tags: { i18n: 'posts_translations' }, + }, + }; + const translationCodec = { + name: 'postsTranslations', + attributes: { + posts_id: { codec: idCodec }, + lang_code: { codec: textCodec }, + [translatedField]: { codec: textCodec }, + }, + extensions: { + pg: { schemaName: 'tenant', name: 'posts_translations' }, + }, + }; + class GraphQLObjectType { + readonly origin = origin; + constructor(readonly config: any) {} + } + class GraphQLNonNull { + constructor(readonly ofType: any) {} + } + const build = { + input: { + pgRegistry: { + pgCodecs: { baseCodec, translationCodec }, + pgResources: { + base: { + codec: baseCodec, + uniques: [{ isPrimary: true, attributes: ['id'] }], + }, + translation: { codec: translationCodec }, + }, + }, + }, + inflection: { + camelCase: (value: string) => value, + tableType: () => 'Post', + }, + graphql: { + GraphQLString: { name: 'String', origin }, + GraphQLObjectType, + GraphQLNonNull, + }, + extend: (base: object, extra: object) => ({ ...base, ...extra }), + }; + return { build, baseCodec }; +} + +describe('I18nPlugin cache ownership', () => { + it('keeps registry and GraphQL types local to the exact build', () => { + const plugin = createI18nPlugin(); + const init = (plugin.schema!.hooks!.init as any).callback; + const fieldsHook = plugin.schema!.hooks!.GraphQLObjectType_fields as any; + const tenantA = makeBuild('tenant-a', 'title'); + const tenantB = makeBuild('tenant-b', 'summary'); + + init({}, tenantA.build); + init({}, tenantB.build); + + const fieldsA = fieldsHook({}, tenantA.build, { + scope: { isPgClassType: true, pgCodec: tenantA.baseCodec }, + }); + const fieldsAAgain = fieldsHook({}, tenantA.build, { + scope: { isPgClassType: true, pgCodec: tenantA.baseCodec }, + }); + const fieldsB = fieldsHook({}, tenantB.build, { + scope: { isPgClassType: true, pgCodec: tenantB.baseCodec }, + }); + const localeTypeA = fieldsA.localeStrings.type.ofType; + const localeTypeB = fieldsB.localeStrings.type.ofType; + + expect(localeTypeA).toBe(fieldsAAgain.localeStrings.type.ofType); + expect(localeTypeA).not.toBe(localeTypeB); + expect(localeTypeA.origin).toBe('tenant-a'); + expect(localeTypeB.origin).toBe('tenant-b'); + expect(localeTypeA.config.fields).toHaveProperty('title'); + expect(localeTypeA.config.fields).not.toHaveProperty('summary'); + expect(localeTypeB.config.fields).toHaveProperty('summary'); + }); +}); diff --git a/graphile/graphile-i18n/src/plugin.ts b/graphile/graphile-i18n/src/plugin.ts index 9226830a7a..055aa6c61c 100644 --- a/graphile/graphile-i18n/src/plugin.ts +++ b/graphile/graphile-i18n/src/plugin.ts @@ -64,6 +64,11 @@ function resolveAttrPgType(codec: any): string { return codec?.name ?? 'text'; } +interface I18nBuildState { + registry: WeakMap; + localeTypeCache: Map; +} + // ─── Plugin Factory ────────────────────────────────────────────────────────── export function createI18nPlugin(options: I18nPluginOptions = {}): GraphileConfig.Plugin { @@ -74,9 +79,9 @@ export function createI18nPlugin(options: I18nPluginOptions = {}): GraphileConfi defaultLanguages = ['en'], } = options; - // Closure-scoped state shared between init and field hooks - let i18nRegistry: Record = {}; - const localeTypeCache: Record = {}; + // A preset/plugin instance may be reused for multiple schema builds. Keep + // discovery and GraphQL type state owned by the exact build that created it. + const stateByBuild = new WeakMap(); return { name: 'I18nPlugin', @@ -86,7 +91,11 @@ export function createI18nPlugin(options: I18nPluginOptions = {}): GraphileConfi hooks: { init: { callback(_, build) { - i18nRegistry = {}; + const state: I18nBuildState = { + registry: new WeakMap(), + localeTypeCache: new Map() + }; + stateByBuild.set(build, state); for (const [, codec] of Object.entries(build.input.pgRegistry.pgCodecs)) { const c = codec as PgCodecWithAttributes; @@ -190,7 +199,7 @@ export function createI18nPlugin(options: I18nPluginOptions = {}): GraphileConfi if (Object.keys(fields).length === 0) continue; - i18nRegistry[c.name] = { + state.registry.set(c, { baseTable: c.name, translationTable: translationTableName, schemaName, @@ -198,7 +207,7 @@ export function createI18nPlugin(options: I18nPluginOptions = {}): GraphileConfi pkColumn, pkType, fields, - }; + }); } return _; @@ -211,8 +220,10 @@ export function createI18nPlugin(options: I18nPluginOptions = {}): GraphileConfi if (!scope.pgCodec || !scope.isPgClassType) return fields; + const state = stateByBuild.get(build); + if (!state) return fields; const codec = scope.pgCodec as PgCodecWithAttributes; - const info = i18nRegistry[codec.name]; + const info = state.registry.get(codec); if (!info) return fields; const localeFieldsConfig: Record = { @@ -226,13 +237,13 @@ export function createI18nPlugin(options: I18nPluginOptions = {}): GraphileConfi } const localeTypeName = `${build.inflection.tableType(codec)}LocaleStrings`; - if (!localeTypeCache[localeTypeName]) { - localeTypeCache[localeTypeName] = new GraphQLObjectType({ + if (!state.localeTypeCache.has(localeTypeName)) { + state.localeTypeCache.set(localeTypeName, new GraphQLObjectType({ name: localeTypeName, fields: localeFieldsConfig, - }); + })); } - const localeType = localeTypeCache[localeTypeName]; + const localeType = state.localeTypeCache.get(localeTypeName); const { schemaName, baseTable, translationTable, fkColumn, pkColumn, pkType, fields: i18nFields } = info; diff --git a/graphile/graphile-llm/README.md b/graphile/graphile-llm/README.md index 6b970927f7..73a9ef3201 100644 --- a/graphile/graphile-llm/README.md +++ b/graphile/graphile-llm/README.md @@ -60,6 +60,15 @@ The preset bundles all plugins listed below. You can also import each plugin ind - **Toggleable** — each capability (`enableTextSearch`, `enableTextMutations`, `enableRag`) can be independently enabled or disabled - **Plugin-conditional** — fields only appear in the schema when the plugin is loaded +### Cache ownership + +Runtime billing and inference-log configuration is cached per database ID and +cache owner. `getLlmBillingConfig(client, databaseId, owner)` accepts the +Graphile build as the owner; the owner is optional for compatibility with the +original two-argument call and then defaults to the client. Cache statistics +require an owner. `invalidateLlmBillingConfig(databaseId, owner)` clears one +owner, while omitting the owner clears all owners. + ## Plugins | Plugin | Description | Toggle | diff --git a/graphile/graphile-llm/__tests__/agent-discovery.test.ts b/graphile/graphile-llm/__tests__/agent-discovery.test.ts index 0ed0b30a5b..66a68ba0d9 100644 --- a/graphile/graphile-llm/__tests__/agent-discovery.test.ts +++ b/graphile/graphile-llm/__tests__/agent-discovery.test.ts @@ -33,6 +33,14 @@ const fakePool = (respond: (values: unknown[]) => { rows: unknown[] }) => { const pgError = (code: string) => Object.assign(new Error(`pg error ${code}`), { code }); +function deferred(): { promise: Promise; resolve: () => void } { + let resolve!: () => void; + const promise = new Promise(res => { + resolve = res; + }); + return { promise, resolve }; +} + beforeEach(() => clearAgentDiscoveryCache()); describe('getAgentDiscovery', () => { @@ -65,6 +73,58 @@ describe('getAgentDiscovery', () => { expect(calls).toHaveLength(2); }); + it('does not share discovery across physical pool identities', async () => { + const first = fakePool(() => ({ rows: [row('physical_a')] })); + const second = fakePool(() => ({ rows: [row('physical_b')] })); + + const fromFirst = await getAgentDiscovery(first.pool, DB_A); + const fromSecond = await getAgentDiscovery(second.pool, DB_A); + + expect(fromFirst?.thread?.schemaName).toBe('physical_a_agent_public'); + expect(fromSecond?.thread?.schemaName).toBe('physical_b_agent_public'); + expect(first.calls).toHaveLength(1); + expect(second.calls).toHaveLength(1); + }); + + it('keeps overlapping same-database discovery isolated by pool identity', async () => { + const gate = deferred(); + const first = fakePool(() => ({ rows: [row('overlap_a')] })); + const second = fakePool(() => ({ rows: [row('overlap_b')] })); + const firstQuery = first.pool.query as jest.Mock; + const secondQuery = second.pool.query as jest.Mock; + firstQuery.mockImplementation(async (text: string, values?: unknown[]) => { + first.calls.push({ text, values }); + await gate.promise; + return { rows: [row('overlap_a')] }; + }); + secondQuery.mockImplementation(async (text: string, values?: unknown[]) => { + second.calls.push({ text, values }); + await gate.promise; + return { rows: [row('overlap_b')] }; + }); + + const pending = Promise.all([ + getAgentDiscovery(first.pool, DB_A), + getAgentDiscovery(second.pool, DB_A), + ]); + expect(first.calls).toHaveLength(1); + expect(second.calls).toHaveLength(1); + gate.resolve(); + + const [fromFirst, fromSecond] = await pending; + const [cachedFirst, cachedSecond] = await Promise.all([ + getAgentDiscovery(first.pool, DB_A), + getAgentDiscovery(second.pool, DB_A), + ]); + + expect(fromFirst?.thread?.schemaName).toBe('overlap_a_agent_public'); + expect(fromSecond?.thread?.schemaName).toBe('overlap_b_agent_public'); + expect(cachedFirst).toBe(fromFirst); + expect(cachedSecond).toBe(fromSecond); + expect(first.calls).toHaveLength(1); + expect(second.calls).toHaveLength(1); + }); + it('treats an absent module as not provisioned', async () => { const { pool } = fakePool(() => { throw pgError('42P01'); diff --git a/graphile/graphile-llm/src/__tests__/config-cache-isolation.test.ts b/graphile/graphile-llm/src/__tests__/config-cache-isolation.test.ts new file mode 100644 index 0000000000..4e2d807aa9 --- /dev/null +++ b/graphile/graphile-llm/src/__tests__/config-cache-isolation.test.ts @@ -0,0 +1,147 @@ +import { + getLlmBillingCacheStats, + getLlmBillingConfig, + invalidateLlmBillingConfig, +} from '../config-cache'; + +function makeClient(privateSchema: string, waitFor?: Promise) { + const query = jest.fn(async (text: string) => { + if (waitFor) await waitFor; + if (text.includes('information_schema.schemata')) + return { rows: [{ exists: 1 }] }; + if (text.includes('billing_module')) { + return { + rows: [ + { + public_schema: `${privateSchema}_public`, + private_schema: privateSchema, + record_usage_function: 'record_usage', + }, + ], + }; + } + if (text.includes('inference_log_module')) { + return { + rows: [{ schema: privateSchema, table_name: 'usage_log_inference' }], + }; + } + throw new Error('unexpected SQL'); + }); + return { query }; +} + +function deferred(): { promise: Promise; resolve: () => void } { + let resolve!: () => void; + const promise = new Promise(res => { + resolve = res; + }); + return { promise, resolve }; +} + +describe('LLM config cache ownership', () => { + beforeEach(() => invalidateLlmBillingConfig()); + + it('isolates the same database UUID by exact build identity', async () => { + const databaseId = '11111111-1111-1111-1111-111111111111'; + const buildA = {}; + const buildB = {}; + const clientA = makeClient('tenant_a_private'); + const clientB = makeClient('tenant_b_private'); + + const firstA = await getLlmBillingConfig(clientA, databaseId, buildA); + const cachedA = await getLlmBillingConfig(clientA, databaseId, buildA); + const firstB = await getLlmBillingConfig(clientB, databaseId, buildB); + + expect(firstA).toBe(cachedA); + expect(firstA.billing?.privateSchema).toBe('tenant_a_private'); + expect(firstB.billing?.privateSchema).toBe('tenant_b_private'); + expect(clientA.query).toHaveBeenCalledTimes(4); + expect(clientB.query).toHaveBeenCalledTimes(4); + expect(getLlmBillingCacheStats(buildA).size).toBe(1); + expect(getLlmBillingCacheStats(buildB).size).toBe(1); + }); + + it('keeps overlapping owners isolated and retains each owner cache', async () => { + const databaseId = '22222222-2222-2222-2222-222222222222'; + const buildA = {}; + const buildB = {}; + const gate = deferred(); + const clientA = makeClient('overlap_a_private', gate.promise); + const clientB = makeClient('overlap_b_private', gate.promise); + + const pending = Promise.all([ + getLlmBillingConfig(clientA, databaseId, buildA), + getLlmBillingConfig(clientB, databaseId, buildB), + ]); + expect(clientA.query).toHaveBeenCalled(); + expect(clientB.query).toHaveBeenCalled(); + gate.resolve(); + + const [firstA, firstB] = await pending; + const [cachedA, cachedB] = await Promise.all([ + getLlmBillingConfig(clientA, databaseId, buildA), + getLlmBillingConfig(clientB, databaseId, buildB), + ]); + + expect(firstA.billing?.privateSchema).toBe('overlap_a_private'); + expect(firstB.billing?.privateSchema).toBe('overlap_b_private'); + expect(cachedA).toBe(firstA); + expect(cachedB).toBe(firstB); + expect(clientA.query).toHaveBeenCalledTimes(4); + expect(clientB.query).toHaveBeenCalledTimes(4); + }); + + it('supports owner-specific and ownerless invalidation', async () => { + const databaseId = '33333333-3333-3333-3333-333333333333'; + const buildA = {}; + const buildB = {}; + const clientA = makeClient('invalidate_a_private'); + const clientB = makeClient('invalidate_b_private'); + + const [firstA, firstB] = await Promise.all([ + getLlmBillingConfig(clientA, databaseId, buildA), + getLlmBillingConfig(clientB, databaseId, buildB), + ]); + + invalidateLlmBillingConfig(databaseId, buildA); + const cachedB = await getLlmBillingConfig(clientB, databaseId, buildB); + const refreshedA = await getLlmBillingConfig(clientA, databaseId, buildA); + expect(cachedB).toBe(firstB); + expect(refreshedA).not.toBe(firstA); + + invalidateLlmBillingConfig(databaseId); + const refreshedB = await getLlmBillingConfig(clientB, databaseId, buildB); + expect(refreshedB).not.toBe(firstB); + expect(clientA.query).toHaveBeenCalledTimes(8); + expect(clientB.query).toHaveBeenCalledTimes(8); + }); + + it('keeps the original two-argument call scoped to its client', async () => { + const databaseId = '44444444-4444-4444-4444-444444444444'; + const client = makeClient('compat_private'); + + const first = await getLlmBillingConfig(client, databaseId); + const cached = await getLlmBillingConfig(client, databaseId); + + expect(cached).toBe(first); + expect(first.billing?.privateSchema).toBe('compat_private'); + expect(client.query).toHaveBeenCalledTimes(4); + expect(getLlmBillingCacheStats(client).size).toBe(1); + }); + + it('rejects an invalid cache owner', async () => { + await expect( + getLlmBillingConfig( + makeClient('tenant_private'), + 'database-a', + null as any + ) + ).rejects.toThrow('LLM_CONFIG_CACHE_SCOPE_UNAVAILABLE'); + expect(() => getLlmBillingCacheStats(null as any)).toThrow( + 'LLM_CONFIG_CACHE_SCOPE_UNAVAILABLE' + ); + expect(() => invalidateLlmBillingConfig('database-a', null as any)).toThrow( + 'LLM_CONFIG_CACHE_SCOPE_UNAVAILABLE' + ); + }); +}); diff --git a/graphile/graphile-llm/src/config-cache.ts b/graphile/graphile-llm/src/config-cache.ts index c3a5ae82fb..98813df637 100644 --- a/graphile/graphile-llm/src/config-cache.ts +++ b/graphile/graphile-llm/src/config-cache.ts @@ -1,7 +1,7 @@ /** * config-cache — Per-database LLM billing configuration cache * - * Caches resolved billing function names per database_id. + * Caches resolved billing function names per owner and database_id. * Uses an LRU cache with TTL so config changes propagate within a bounded window * without requiring a server restart. * @@ -97,11 +97,36 @@ const INFERENCE_LOG_MODULE_SQL = ` `; // ─── Cache ────────────────────────────────────────────────────────────────── -const billingCache = new ModuleConfigCache({ - name: 'billing-config', - ttlMs: 5 * 60 * 1000, // 5 minutes - max: 50 -}); +const BILLING_CACHE_MAX = 50; +let billingCachesByScope = new WeakMap< + object, + ModuleConfigCache +>(); + +function assertValidCacheScope(cacheScope: unknown): asserts cacheScope is object { + if ( + (typeof cacheScope !== 'object' && typeof cacheScope !== 'function') || + cacheScope === null + ) { + throw new Error('LLM_CONFIG_CACHE_SCOPE_UNAVAILABLE'); + } +} + +function getBillingCache( + cacheScope: object +): ModuleConfigCache { + assertValidCacheScope(cacheScope); + let cache = billingCachesByScope.get(cacheScope); + if (!cache) { + cache = new ModuleConfigCache({ + name: 'billing-config', + ttlMs: 5 * 60 * 1000, // 5 minutes + max: BILLING_CACHE_MAX + }); + billingCachesByScope.set(cacheScope, cache); + } + return cache; +} // ─── Resolution Functions ─────────────────────────────────────────────────── @@ -166,15 +191,19 @@ async function resolveBillingConfig( /** * Resolve billing config for a database. - * Results are cached per database_id with a 5-minute TTL. + * Results are cached per owner and database_id with a 5-minute TTL. * * @param pgClient - A client connected to the tenant database (from withPgClient) * @param databaseId - The database UUID + * @param cacheScope - The exact owner of the cached result. Defaults to the + * client for compatibility with the original two-argument API. */ export async function getLlmBillingConfig( pgClient: PgClient, - databaseId: string + databaseId: string, + cacheScope: object = pgClient ): Promise { + const billingCache = getBillingCache(cacheScope); const cached = billingCache.get(databaseId); if (cached) return cached; @@ -189,9 +218,20 @@ export async function getLlmBillingConfig( } /** - * Invalidate the cached config for a specific database (or all). + * Invalidate cached config for one exact owner. Omitting the owner resets all + * weakly owned caches without retaining their build identities. */ -export function invalidateLlmBillingConfig(databaseId?: string): void { +export function invalidateLlmBillingConfig( + databaseId?: string, + cacheScope?: object +): void { + if (cacheScope === undefined) { + billingCachesByScope = new WeakMap(); + return; + } + assertValidCacheScope(cacheScope); + const billingCache = billingCachesByScope.get(cacheScope); + if (!billingCache) return; if (databaseId) { billingCache.delete(databaseId); } else { @@ -200,8 +240,12 @@ export function invalidateLlmBillingConfig(databaseId?: string): void { } /** - * Get cache stats for diagnostics. + * Get cache stats for an exact owner without retaining other build identities. */ -export function getLlmBillingCacheStats(): { size: number; max: number } { - return { size: billingCache.size, max: 50 }; +export function getLlmBillingCacheStats(cacheScope: object): { size: number; max: number } { + assertValidCacheScope(cacheScope); + return { + size: billingCachesByScope.get(cacheScope)?.size ?? 0, + max: BILLING_CACHE_MAX + }; } diff --git a/graphile/graphile-llm/src/plugins/agent-discovery-plugin.ts b/graphile/graphile-llm/src/plugins/agent-discovery-plugin.ts index 15347c94d7..63f4d6bfd8 100644 --- a/graphile/graphile-llm/src/plugins/agent-discovery-plugin.ts +++ b/graphile/graphile-llm/src/plugins/agent-discovery-plugin.ts @@ -34,14 +34,26 @@ export interface AgentDiscovery { // ─── Cache ────────────────────────────────────────────────────────────────── -const agentDiscoveryCache = new ModuleConfigCache({ - name: 'agent-discovery', - ttlMs: 60_000 -}); +let agentDiscoveryCaches = new WeakMap< + object, + ModuleConfigCache +>(); + +function cacheForPool(pool: object): ModuleConfigCache { + let cache = agentDiscoveryCaches.get(pool); + if (!cache) { + cache = new ModuleConfigCache({ + name: 'agent-discovery', + ttlMs: 60_000 + }); + agentDiscoveryCaches.set(pool, cache); + } + return cache; +} /** Clear all cached discovery results (for testing) */ export function clearAgentDiscoveryCache(): void { - agentDiscoveryCache.clear(); + agentDiscoveryCaches = new WeakMap(); } // ─── Discovery Query ──────────────────────────────────────────────────────── @@ -83,6 +95,7 @@ export async function getAgentDiscovery( throw new Error('getAgentDiscovery: databaseId is required'); } + const agentDiscoveryCache = cacheForPool(pool); const cached = agentDiscoveryCache.get(databaseId); if (cached !== undefined) { return cached; diff --git a/graphile/graphile-llm/src/plugins/metering-plugin.ts b/graphile/graphile-llm/src/plugins/metering-plugin.ts index 754f4aabfd..0d297cc936 100644 --- a/graphile/graphile-llm/src/plugins/metering-plugin.ts +++ b/graphile/graphile-llm/src/plugins/metering-plugin.ts @@ -63,7 +63,8 @@ function defaultResolveEntityId(pgSettings: Record): string | nu async function buildMeteringContext( graphqlContext: any, - resolveEntityId: (pgSettings: Record) => string | null + resolveEntityId: (pgSettings: Record) => string | null, + cacheScope: object ): Promise { const pgSettings: Record = graphqlContext?.pgSettings ?? {}; const entityId = resolveEntityId(pgSettings); @@ -79,7 +80,7 @@ async function buildMeteringContext( let inferenceLogConfig = null; try { await withPgClient(pgSettings, async (pgClient: PgClient) => { - const entry = await getLlmBillingConfig(pgClient, databaseId); + const entry = await getLlmBillingConfig(pgClient, databaseId, cacheScope); billingConfig = entry.billing; inferenceLogConfig = entry.inferenceLog; }); @@ -215,7 +216,11 @@ export function createLlmMeteringPlugin( ...rest, async resolve(source: any, args: any, graphqlContext: any, info: any) { // Build the metering context for this request - const ctx = await buildMeteringContext(graphqlContext, resolveEntityId); + const ctx = await buildMeteringContext( + graphqlContext, + resolveEntityId, + build + ); // Run the original resolver within the AsyncLocalStorage scope // so any embedder calls made by downstream plugins pick up the ctx diff --git a/graphile/graphile-search/README.md b/graphile/graphile-search/README.md index ff75b38169..02c49a89ff 100644 --- a/graphile/graphile-search/README.md +++ b/graphile/graphile-search/README.md @@ -58,6 +58,19 @@ const preset = { - **Hybrid search**: Combine multiple algorithms in a single query - **Zero config**: Auto-discovers columns and indexes per adapter +### BM25 discovery and compatibility + +BM25 index discovery is scoped to the current gather and attaches metadata to +that gather's codec attributes. It does not populate a process-wide cache, so +overlapping builds cannot reuse another build's index metadata. + +The deprecated `bm25IndexStore` export remains available for integrations that +need a migration period. It is never populated automatically; pass it +explicitly as `createBm25Adapter({ bm25IndexStore })` if required. New code +should use the normal `UnifiedSearchPreset` and per-build codec metadata. The +old `bm25ExtensionDetected` deep import was removed and is not part of the +compatibility surface. + ## Architecture ``` diff --git a/graphile/graphile-search/src/__tests__/bm25-cache-isolation.test.ts b/graphile/graphile-search/src/__tests__/bm25-cache-isolation.test.ts new file mode 100644 index 0000000000..b083308a73 --- /dev/null +++ b/graphile/graphile-search/src/__tests__/bm25-cache-isolation.test.ts @@ -0,0 +1,117 @@ +import { createBm25Adapter } from '../adapters/bm25'; +import { bm25IndexStore } from '../index'; +import { + Bm25CodecPlugin, + collectBm25Indexes, +} from '../codecs/bm25-codec'; + +const row = (indexName: string) => ({ + class_id: '100', + attribute_number: 2, + schema_name: 'tenant_a', + table_name: 'documents', + column_name: 'body', + index_name: indexName, +}); + +describe('BM25 gather cache ownership', () => { + beforeEach(() => bm25IndexStore.clear()); + afterEach(() => bm25IndexStore.clear()); + + it('does not retain index discovery across gather states', () => { + const first = collectBm25Indexes([row('first_idx')]); + const rebuilt = collectBm25Indexes([]); + + expect(first.size).toBe(1); + expect(rebuilt.size).toBe(0); + }); + + it('binds and consumes only the current gather state', () => { + const first = collectBm25Indexes([row('first_idx')]); + const rebuilt = collectBm25Indexes([row('rebuilt_idx')]); + const attributeHook = (Bm25CodecPlugin.gather as any).hooks + .pgCodecs_attribute; + const attribute: any = { codec: { name: 'text' } }; + + attributeHook( + { state: { indexesByService: new Map([['main', rebuilt]]) } }, + { + serviceName: 'main', + pgClass: { _id: '100' }, + pgAttribute: { attnum: 2 }, + attribute, + } + ); + + expect(attribute.extensions.bm25Index.indexName).toBe('rebuilt_idx'); + expect([...first.values()][0].indexName).toBe('first_idx'); + + const adapter = createBm25Adapter(); + expect( + adapter.detectColumns({ attributes: { body: attribute } }, {}) + ).toEqual([ + { + attributeName: 'body', + adapterData: { + bm25Index: attribute.extensions.bm25Index, + chunksInfo: undefined, + }, + }, + ]); + }); + + it('does not mutate the deprecated public store during gather', () => { + collectBm25Indexes([row('gather_idx')]); + const attributeHook = (Bm25CodecPlugin.gather as any).hooks + .pgCodecs_attribute; + const attribute: any = { codec: { name: 'text' } }; + + attributeHook( + { + state: { + indexesByService: new Map([ + ['main', collectBm25Indexes([row('gather_idx')])], + ]), + }, + }, + { + serviceName: 'main', + pgClass: { _id: '100' }, + pgAttribute: { attnum: 2 }, + attribute, + } + ); + + expect(bm25IndexStore.size).toBe(0); + }); + + it('uses the deprecated store only when explicitly passed to the adapter', () => { + const explicitIndex = { + schemaName: 'tenant_a', + tableName: 'documents', + columnName: 'body', + indexName: 'explicit_idx', + }; + bm25IndexStore.set('tenant_a.documents.body', explicitIndex); + const codec = { + extensions: { pg: { schemaName: 'tenant_a', name: 'documents' } }, + attributes: { body: { codec: { name: 'text' } } }, + }; + + expect(createBm25Adapter().detectColumns(codec, {})).toEqual([]); + expect( + createBm25Adapter().detectColumns(codec, { + pgBm25IndexStore: bm25IndexStore, + }) + ).toEqual([]); + expect(createBm25Adapter({ bm25IndexStore }).detectColumns(codec, {})).toEqual([ + { + attributeName: 'body', + adapterData: { + bm25Index: explicitIndex, + chunksInfo: undefined, + }, + }, + ]); + }); +}); diff --git a/graphile/graphile-search/src/__tests__/plugin-cache-isolation.test.ts b/graphile/graphile-search/src/__tests__/plugin-cache-isolation.test.ts new file mode 100644 index 0000000000..4812e84c65 --- /dev/null +++ b/graphile/graphile-search/src/__tests__/plugin-cache-isolation.test.ts @@ -0,0 +1,62 @@ +import { createUnifiedSearchPlugin } from '../plugin'; +import type { SearchAdapter } from '../types'; + +describe('UnifiedSearchPlugin cache ownership', () => { + it('isolates discovery by exact build and codec identity', () => { + const detectColumns = jest.fn((_codec: any, build: any) => [ + { attributeName: build.tenantColumn }, + ]); + const adapter: SearchAdapter = { + name: 'tenant-test', + filterPrefix: 'tenantTest', + scoreSemantics: { metric: 'score', lowerIsBetter: false, range: null }, + detectColumns, + registerTypes: jest.fn(), + getFilterTypeName: jest.fn(() => 'TenantTestInput'), + buildFilterApply: jest.fn(), + }; + const plugin = createUnifiedSearchPlugin({ + adapters: [adapter], + enableSearchScore: false, + enableUnifiedSearch: false, + }); + const callback = (plugin.schema!.entityBehavior!.pgCodecAttribute as any) + .inferred.callback; + const buildA = { tenantColumn: 'tenant_a_search' }; + const buildB = { tenantColumn: 'tenant_b_search' }; + const codecA = { + name: 'documents', + attributes: { tenant_a_search: {} }, + }; + const codecB = { + name: 'documents', + attributes: { tenant_b_search: {} }, + }; + + expect(callback('default', [codecA, 'tenant_a_search'], buildA)).toContain( + 'unifiedSearch:select' + ); + expect(callback('default', [codecB, 'tenant_b_search'], buildB)).toContain( + 'unifiedSearch:select' + ); + expect(callback('default', [codecB, 'tenant_a_search'], buildB)).toBe( + 'default' + ); + expect(detectColumns).toHaveBeenCalledTimes(2); + + callback('default', [codecB, 'tenant_b_search'], buildB); + expect(detectColumns).toHaveBeenCalledTimes(2); + + const sharedCodec = { + name: 'shared_documents', + attributes: { tenant_a_search: {}, tenant_b_search: {} }, + }; + expect( + callback('default', [sharedCodec, 'tenant_a_search'], buildA) + ).toContain('unifiedSearch:select'); + expect( + callback('default', [sharedCodec, 'tenant_b_search'], buildB) + ).toContain('unifiedSearch:select'); + expect(detectColumns).toHaveBeenCalledTimes(4); + }); +}); diff --git a/graphile/graphile-search/src/adapters/bm25.ts b/graphile/graphile-search/src/adapters/bm25.ts index d5ebd22254..b967bc45e7 100644 --- a/graphile/graphile-search/src/adapters/bm25.ts +++ b/graphile/graphile-search/src/adapters/bm25.ts @@ -5,7 +5,7 @@ * BM25 relevance scoring. Wraps the same SQL logic as graphile-bm25. * * Requires the Bm25CodecPlugin to be loaded first (for index discovery). - * The adapter reads from the bm25IndexStore populated during the gather phase. + * The adapter reads metadata attached to this gather's codec attributes. * * Supports chunk-aware querying via @hasChunks smart tag: when the parent * table has chunks with a BM25 index, the adapter includes a lateral @@ -15,19 +15,11 @@ import type { SQL } from 'pg-sql2'; -import { bm25IndexStore as moduleBm25IndexStore } from '../codecs/bm25-codec'; +import type { Bm25IndexInfo } from '../codecs/bm25-codec'; import type { FilterApplyResult,SearchableColumn, SearchAdapter } from '../types'; import { type ChunksInfo,getChunksInfo } from './chunks'; -/** - * BM25 index info discovered during gather phase. - */ -export interface Bm25IndexInfo { - schemaName: string; - tableName: string; - columnName: string; - indexName: string; -} +export type { Bm25IndexInfo } from '../codecs/bm25-codec'; /** Combined adapter data for a BM25-searchable column */ interface Bm25ColumnData { @@ -48,8 +40,8 @@ export interface Bm25AdapterOptions { filterPrefix?: string; /** - * External BM25 index store. If not provided, the adapter will attempt - * to read from the build object's `pgBm25IndexStore`. + * Explicit BM25 index metadata for this adapter's owner. Automatic discovery + * uses only metadata attached to the current gather's codec attributes. */ bm25IndexStore?: Map; } @@ -59,22 +51,14 @@ export function createBm25Adapter( ): SearchAdapter { const { filterPrefix = 'bm25', bm25IndexStore } = options; - function getIndexStore(build: any): Map | undefined { - if (bm25IndexStore) return bm25IndexStore; - // Try build.pgBm25IndexStore (set by standalone Bm25SearchPlugin's build hook) - const buildStore = build.pgBm25IndexStore as Map | undefined; - if (buildStore && buildStore.size > 0) return buildStore; - // Fall back to module-level store populated by Bm25CodecPlugin's gather phase - if (moduleBm25IndexStore && moduleBm25IndexStore.size > 0) return moduleBm25IndexStore; - return undefined; - } - function getBm25IndexForAttribute( codec: any, attributeName: string, - build: any, ): Bm25IndexInfo | undefined { - const store = getIndexStore(build); + const bound = codec.attributes?.[attributeName]?.extensions?.bm25Index; + if (bound) return bound as Bm25IndexInfo; + + const store = bm25IndexStore; if (!store) return undefined; const pg = codec?.extensions?.pg; @@ -110,7 +94,7 @@ export function createBm25Adapter( codec.attributes as Record )) { if (!isTextCodec(attribute.codec)) continue; - const bm25Index = getBm25IndexForAttribute(codec, attributeName, build); + const bm25Index = getBm25IndexForAttribute(codec, attributeName); if (!bm25Index) continue; // Check for chunk-aware BM25 diff --git a/graphile/graphile-search/src/codecs/bm25-codec.ts b/graphile/graphile-search/src/codecs/bm25-codec.ts index b48beceeda..a230a66c20 100644 --- a/graphile/graphile-search/src/codecs/bm25-codec.ts +++ b/graphile/graphile-search/src/codecs/bm25-codec.ts @@ -8,12 +8,13 @@ * 1. Creates a codec for bm25query via gather.hooks.pgCodecs_findPgCodec * 2. Discovers all BM25 indexes via gather.hooks.pgIntrospection_introspection * by querying pg_index + pg_am + pg_class + pg_attribute - * 3. Stores discovered BM25 index info in a module-level Map for use by - * the BM25 adapter during the schema build phase + * 3. Attaches discovered BM25 index info to codec attributes belonging to + * the current gather state */ import 'graphile-build-pg'; +import { gatherConfig } from 'graphile-build'; import type { GraphileConfig } from 'graphile-config'; import sql from 'pg-sql2'; @@ -32,18 +33,55 @@ export interface Bm25IndexInfo { } /** - * Module-level store for discovered BM25 indexes. - * Populated during the gather phase, read during the schema build phase. + * @deprecated Pass a store explicitly to createBm25Adapter when needed. * - * Key: "schemaName.tableName.columnName" - * Value: Bm25IndexInfo + * This compatibility map is never populated by automatic discovery; gather + * state is bound to codec attributes instead. */ export const bm25IndexStore = new Map(); -/** - * Whether pg_textsearch extension was detected in the database. - */ -export let bm25ExtensionDetected = false; +declare global { + namespace GraphileConfig { + interface GatherHelpers { + bm25Codec: Record; + } + } + + namespace DataplanPg { + interface PgCodecAttributeExtensions { + /** BM25 index discovered for this attribute during this gather. */ + bm25Index?: Bm25IndexInfo; + } + } +} + +interface Bm25IndexRow { + class_id: string; + attribute_number: number; + schema_name: string; + table_name: string; + column_name: string; + index_name: string; +} + +const attributeKey = (classId: string, attributeNumber: number): string => + `${classId}:${attributeNumber}`; + +/** Convert one gather's query result into state owned by that gather only. */ +export function collectBm25Indexes( + rows: readonly Bm25IndexRow[] +): Map { + const indexes = new Map(); + for (const row of rows) { + indexes.set(attributeKey(row.class_id, row.attribute_number), { + schemaName: row.schema_name, + tableName: row.table_name, + columnName: row.column_name, + indexName: row.index_name + }); + } + return indexes; +} /** * The SQL query that discovers BM25 indexes in the database. @@ -52,6 +90,8 @@ export let bm25ExtensionDetected = false; */ const BM25_DISCOVERY_SQL = ` SELECT + c.oid::text AS class_id, + a.attnum AS attribute_number, n.nspname AS schema_name, c.relname AS table_name, a.attname AS column_name, @@ -70,7 +110,12 @@ export const Bm25CodecPlugin: GraphileConfig.Plugin = { version: '1.0.0', description: 'Registers a codec for the pg_textsearch bm25query type and discovers BM25 indexes', - gather: { + gather: gatherConfig({ + namespace: 'bm25Codec', + initialState: () => ({ + indexesByService: new Map>() + }), + helpers: {}, hooks: { /** * Register the bm25query codec when detected during type introspection. @@ -126,9 +171,6 @@ export const Bm25CodecPlugin: GraphileConfig.Plugin = { ); if (!pgService) return; - // Clear previous entries for this introspection run - bm25IndexStore.clear(); - try { const adaptorSettings = (pgService as any).adaptorSettings; if (!adaptorSettings?.connectionString && !adaptorSettings?.pool) { @@ -147,18 +189,10 @@ export const Bm25CodecPlugin: GraphileConfig.Plugin = { try { const result = await pool.query(BM25_DISCOVERY_SQL); - if (result.rows && result.rows.length > 0) { - bm25ExtensionDetected = true; - for (const row of result.rows) { - const key = `${row.schema_name}.${row.table_name}.${row.column_name}`; - bm25IndexStore.set(key, { - schemaName: row.schema_name, - tableName: row.table_name, - columnName: row.column_name, - indexName: row.index_name, - }); - } - } + info.state.indexesByService.set( + serviceName, + collectBm25Indexes(result.rows as Bm25IndexRow[]) + ); } finally { if (isOwnPool) { await pool.end(); @@ -166,11 +200,20 @@ export const Bm25CodecPlugin: GraphileConfig.Plugin = { } } catch { // pg_textsearch not installed or query failed — gracefully skip - bm25ExtensionDetected = false; + info.state.indexesByService.set(serviceName, new Map()); } }, + + pgCodecs_attribute(info, event) { + const index = info.state.indexesByService + .get(event.serviceName) + ?.get(attributeKey(event.pgClass._id, event.pgAttribute.attnum)); + if (!index) return; + event.attribute.extensions ??= Object.create(null); + event.attribute.extensions.bm25Index = index; + }, }, - }, + }), schema: { hooks: { diff --git a/graphile/graphile-search/src/codecs/index.ts b/graphile/graphile-search/src/codecs/index.ts index 41a283597d..0bc197709f 100644 --- a/graphile/graphile-search/src/codecs/index.ts +++ b/graphile/graphile-search/src/codecs/index.ts @@ -10,7 +10,6 @@ export type { Bm25IndexInfo } from './bm25-codec'; export { Bm25CodecPlugin, Bm25CodecPreset, - bm25ExtensionDetected, bm25IndexStore, } from './bm25-codec'; export type { TsvectorCodecPluginOptions } from './tsvector-codec'; diff --git a/graphile/graphile-search/src/plugin.ts b/graphile/graphile-search/src/plugin.ts index c67f796be3..b69e07bdc3 100644 --- a/graphile/graphile-search/src/plugin.ts +++ b/graphile/graphile-search/src/plugin.ts @@ -171,8 +171,13 @@ export function createUnifiedSearchPlugin( ): GraphileConfig.Plugin { const { adapters, enableSearchScore = true, enableUnifiedSearch = true, rrfK = 60 } = options; - // Per-codec cache of discovered columns, keyed by codec name - const codecCache = new Map(); + // Column discovery may depend on the surrounding build registry, not just + // the codec. Weak identity keys keep one build's result out of another and + // allow both build and codec state to be collected after schema creation. + const buildCodecCache = new WeakMap< + object, + WeakMap + >(); // Bridge between orderBy enum apply and filter apply. // The orderBy enum runs on the PgSelectStep while the filter runs on @@ -195,9 +200,14 @@ export function createUnifiedSearchPlugin( * count as intentional search. */ function getAdapterColumns(codec: PgCodecWithAttributes, build: any): AdapterColumnCache[] { - const cacheKey = codec.name; - if (codecCache.has(cacheKey)) { - return codecCache.get(cacheKey)!; + let codecCache = buildCodecCache.get(build); + if (!codecCache) { + codecCache = new WeakMap(); + buildCodecCache.set(build, codecCache); + } + const cached = codecCache.get(codec); + if (cached) { + return cached; } const primaryAdapters = adapters.filter((a) => !a.isSupplementary); @@ -238,7 +248,7 @@ export function createUnifiedSearchPlugin( } } - codecCache.set(cacheKey, results); + codecCache.set(codec, results); return results; }