diff --git a/docs/isolated-schema-factory.md b/docs/isolated-schema-factory.md new file mode 100644 index 00000000..7c4cd22b --- /dev/null +++ b/docs/isolated-schema-factory.md @@ -0,0 +1,9 @@ +# Isolated schema factory + +`generateSchema` now creates a new GraphQL composer and schema for every build. A returned schema therefore owns the resolver closures for that build’s content snapshot; a later build cannot replace its types, fields, or reads. + +We removed the config-keyed schema cache after applying the deletion test. Deleting it removed more complexity than it exposed: the cache keyed schemas from configuration while resolver closures captured content from the first build, producing stale reads after content changed. It also required validation-before-cache ordering and forced the Next.js watch demo to add a `__demoCacheBust` field solely to avoid a cache hit. + +The process-global composer was a separate shared-state issue. Per-build composers isolate type registration and make schemas independently usable; this is not evidence that a schema cache is needed. Because `graphql-compose-json` registers nested object types on the global composer even when given a composer instance, core now owns a small JSON→type parser that threads the per-build composer through every recursion while reproducing the upstream semantics and type naming exactly. AVA remains configured with concurrency `1` until a follow-up validates parallel safety across the complete test suite. + +We did not re-key the cache by content-snapshot identity. There is no profiling evidence that schema construction is the dominant cost in a relevant workload, while a content-keyed cache would add identity, eviction, and lifecycle complexity without demonstrated leverage. If profiling later proves a need, introduce a measured cache behind an explicit seam with correctness tests for changing content. diff --git a/examples/nextjs/scripts/watch-content-query.mjs b/examples/nextjs/scripts/watch-content-query.mjs index cc4d8f29..a600179b 100644 --- a/examples/nextjs/scripts/watch-content-query.mjs +++ b/examples/nextjs/scripts/watch-content-query.mjs @@ -42,18 +42,7 @@ async function loadFreshProvider() { throw new Error('Flatbread config did not load.'); } - // generateSchema caches by config, but this demo intentionally rebuilds the - // content graph on every file event to show edit -> query update without a - // server restart. - const config = { - ...result.config, - content: result.config.content.map((entry) => ({ - ...entry, - __demoCacheBust: Date.now(), - })), - }; - - return new FlatbreadProvider(config); + return new FlatbreadProvider(result.config); } async function render() { diff --git a/packages/core/src/cache/cache.ts b/packages/core/src/cache/cache.ts deleted file mode 100644 index 6d40d382..00000000 --- a/packages/core/src/cache/cache.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { GraphQLSchema } from 'graphql'; -import LRU from 'lru-cache'; -import { createHash } from 'node:crypto'; -import { LoadedFlatbreadConfig } from '../types'; -import { anyToString } from '../utils/stringUtils'; - -type SchemaCacheKey = string; - -interface FlatbreadCache { - schema: LRU; -} - -/** - * A general cache for computationally heavy operations in Flatbread. - */ -export const cache: FlatbreadCache = { - /** - * An LRU cache for GraphQL schemas generated by Flatbread. - */ - schema: new LRU({ - max: 100, - }), -}; - -/** - * Setter function for caching the GraphQL schema generated by Flatbread. - */ -export function cacheSchema( - config: LoadedFlatbreadConfig, - schema: GraphQLSchema -) { - const schemaHashKey = getSchemaHash(config); - cache.schema.set(schemaHashKey, schema); -} - -/** - * Getter function for retrieving the GraphQL schema generated by Flatbread for a given config. - */ -export function checkCacheForSchema( - config: LoadedFlatbreadConfig -): GraphQLSchema | undefined { - const schemaHashKey = getSchemaHash(config); - return cache.schema.get(schemaHashKey); -} - -/** - * Generates a hash key for a given Flatbread config. - */ -export function getSchemaHash(config: LoadedFlatbreadConfig) { - return createHash('md5').update(anyToString(config)).digest('hex'); -} diff --git a/packages/core/src/generators/composeCollection.ts b/packages/core/src/generators/composeCollection.ts new file mode 100644 index 00000000..a15a3bd5 --- /dev/null +++ b/packages/core/src/generators/composeCollection.ts @@ -0,0 +1,85 @@ +import { + isComposeOutputType, + ObjectTypeComposer, + SchemaComposer, + upperFirst, +} from 'graphql-compose'; +import type { EntryNode } from '../types'; + +type FieldConfig = Parameters< + ObjectTypeComposer['setField'] +>[1]; + +export function composeCollectionTC( + composer: SchemaComposer, + typeName: string, + reducedNode: EntryNode +): ObjectTypeComposer { + if (!reducedNode || typeof reducedNode !== 'object') { + throw new Error( + 'You provide empty object in second arg for `createTC` method.' + ); + } + + const tc = composer.createObjectTC(typeName); + Object.keys(reducedNode).forEach((fieldName) => { + const fieldConfig = getFieldConfig(reducedNode[fieldName], { + typeName, + fieldName, + composer, + }); + tc.setField(fieldName, fieldConfig); + }); + return tc; +} + +interface FieldConfigOptions { + composer: SchemaComposer; + fieldName?: string; + typeName?: string; +} + +function getFieldConfig( + value: unknown, + options: FieldConfigOptions +): FieldConfig { + const typeOf = typeof value; + if (typeOf === 'number') return 'Float'; + if (typeOf === 'string') return 'String'; + if (typeOf === 'boolean') return 'Boolean'; + if (value instanceof Date) return 'Date'; + if (isComposeOutputType(value)) return value; + + if (typeOf === 'object') { + if (value === null) return 'JSON'; + if (Array.isArray(value)) { + if (Array.isArray(value[0])) return ['JSON']; + const firstValue = value[0]; + const mergedValue = + typeof firstValue === 'object' && firstValue !== null + ? Object.assign({}, ...value) + : firstValue; + const nestedOptions = + options.typeName && options.fieldName + ? { + ...options, + typeName: options.typeName, + fieldName: options.fieldName, + } + : { composer: options.composer }; + return [getFieldConfig(mergedValue, nestedOptions)] as FieldConfig; + } + if (options.typeName && options.fieldName) { + return composeCollectionTC( + options.composer, + `${options.typeName}_${upperFirst(options.fieldName)}`, + value as EntryNode + ); + } + } + + if (typeOf === 'function') { + return (value as () => FieldConfig)(); + } + return 'JSON'; +} diff --git a/packages/core/src/generators/schema.ts b/packages/core/src/generators/schema.ts index 0a2bf810..51b1e0f1 100644 --- a/packages/core/src/generators/schema.ts +++ b/packages/core/src/generators/schema.ts @@ -1,8 +1,6 @@ -import { schemaComposer } from 'graphql-compose'; -import { composeWithJson } from 'graphql-compose-json'; +import { SchemaComposer } from 'graphql-compose'; import { merge } from 'lodash-es'; import plur from 'plur'; -import { cacheSchema, checkCacheForSchema } from '../cache/cache'; import { generateArgsForAllItemQuery, generateArgsForManyItemQuery, @@ -17,6 +15,7 @@ import { LoadedFlatbreadConfig, } from '../types'; import { produceRecords, validateRecords } from '../records'; +import { composeCollectionTC } from './composeCollection'; import { generateCollection } from './generateCollection'; interface RootQueries { @@ -37,7 +36,6 @@ interface ResolverPayload { export async function generateSchema( configResult: ConfigResult & { contentGraph?: ContentGraphSnapshot; - useSchemaCache?: boolean; } ) { const { config } = configResult; @@ -58,21 +56,7 @@ export async function generateSchema( contentNodesByCollection = validateRecords(allContentNodesJSON, config); } - // Content validation must run before returning a cached schema because the - // cache key is derived from config, while invalid IDs/refs live in content. - const cachedSchema = - configResult.useSchemaCache === false - ? undefined - : checkCacheForSchema(config); - - if (cachedSchema) { - return cachedSchema; - } - - // graphql-compose's default schemaComposer is process-global. Reset it before - // building a fresh Flatbread schema so prior schemas with the same collection - // names do not leak fields or resolvers into this generation pass. - schemaComposer.clear(); + const composer = new SchemaComposer(); const preknownSchemaFragments = fetchPreknownSchemaFragments(config); const executor = createQueryExecutor({ @@ -96,15 +80,15 @@ export async function generateSchema( const schemaArray = Object.fromEntries( Object.entries(allContentNodesJSON).map(([collection, nodes]) => [ collection, - composeWithJson( + composeCollectionTC( + composer, collection, generateCollection({ collection, nodes, config, preknownSchemaFragments, - }), - { schemaComposer } + }) ), ]) ); @@ -167,7 +151,7 @@ export async function generateSchema( executor.all({ name: type, refs }, rp.args), }); - schemaComposer.Query.addFields({ + composer.Query.addFields({ /** * Add find by ID to each content type */ @@ -188,12 +172,12 @@ export async function generateSchema( // Create map of references on each content node for (const { collection, refs } of config.content) { - const typeTC = schemaComposer.getOTC(collection); + const typeTC = composer.getOTC(collection); if (!refs) continue; Object.entries(refs).forEach(([refField, refType]) => { - const refTypeTC = schemaComposer.getOTC(refType); + const refTypeTC = composer.getOTC(refType); // If the current content type has this valid reference field as declared in the config, we'll add a resolver for this reference if (!typeTC.hasField(refField)) return; @@ -226,11 +210,7 @@ export async function generateSchema( }); } - const schema = schemaComposer.buildSchema(); - - if (configResult.useSchemaCache !== false) cacheSchema(config, schema); - - return schema; + return composer.buildSchema(); } /** diff --git a/packages/core/src/generators/test/schema.test.ts b/packages/core/src/generators/test/schema.test.ts new file mode 100644 index 00000000..66f58ad6 --- /dev/null +++ b/packages/core/src/generators/test/schema.test.ts @@ -0,0 +1,148 @@ +import test from 'ava'; +import { schemaComposer } from 'graphql-compose'; +import { graphql } from 'graphql'; +import { VFile } from 'vfile'; +import { generateSchema } from '../schema'; +import { initializeConfig } from '../../utils/initializeConfig'; +import type { + EntryNode, + LoadedFlatbreadConfig, + Source, + Transformer, +} from '../../types'; + +interface InMemoryProject { + config: LoadedFlatbreadConfig; + setEntries: (entries: EntryNode[]) => void; +} + +function makeProject( + collection: string, + initialEntries: EntryNode[] +): InMemoryProject { + let entries = initialEntries; + const source: Source = { + fetch: async () => ({ + [collection]: entries.map((entry, index) => { + const file = new VFile({ + path: `virtual/${collection}/${index}.json`, + }); + file.data.entry = entry; + return file; + }), + }), + }; + const transformer: Transformer = { + extensions: ['.json'], + inspect: (input) => JSON.stringify(input), + parse: (input) => input.data.entry as EntryNode, + }; + return { + config: initializeConfig({ + source, + transformer, + content: [{ path: `virtual/${collection}`, collection }], + }), + setEntries: (nextEntries) => { + entries = nextEntries; + }, + }; +} + +async function querySchema( + schema: Awaited>, + source: string +): Promise> { + const result = await graphql({ schema, source }); + if (result.errors?.length) { + throw new Error(result.errors.map((error) => error.message).join('\n')); + } + return result.data as Record; +} + +test.serial( + 'builds sequential schemas on isolated composers with independent data', + async (t) => { + const first = makeProject('IsolatedAuthor', [ + { id: 'author', name: 'Author A' }, + ]); + const second = makeProject('IsolatedAuthor', [ + { id: 'author', name: 'Author B' }, + ]); + + const schemaA = await generateSchema({ config: first.config }); + const schemaB = await generateSchema({ config: second.config }); + + t.deepEqual( + (await querySchema(schemaA, `query { allIsolatedAuthors { name } }`)) + .allIsolatedAuthors, + [{ name: 'Author A' }] + ); + t.deepEqual( + (await querySchema(schemaB, `query { allIsolatedAuthors { name } }`)) + .allIsolatedAuthors, + [{ name: 'Author B' }] + ); + } +); + +test.serial( + 'rebuilds from changed content with the same config object', + async (t) => { + const project = makeProject('MutableAuthor', [ + { id: 'author', name: 'Value A' }, + ]); + const schemaA = await generateSchema({ config: project.config }); + + t.deepEqual( + (await querySchema(schemaA, `query { allMutableAuthors { name } }`)) + .allMutableAuthors, + [{ name: 'Value A' }] + ); + + project.setEntries([{ id: 'author', name: 'Value B' }]); + const schemaB = await generateSchema({ config: project.config }); + + t.not(schemaB, schemaA); + t.deepEqual( + (await querySchema(schemaB, `query { allMutableAuthors { name } }`)) + .allMutableAuthors, + [{ name: 'Value B' }] + ); + } +); + +test.serial( + 'isolates nested object types between sequential builds', + async (t) => { + const project = makeProject('NestedAuthor', [ + { id: 'author', meta: { a: 1 } }, + ]); + const schemaA = await generateSchema({ config: project.config }); + + project.setEntries([{ id: 'author', meta: { a: 1, b: 'x' } }]); + const schemaB = await generateSchema({ config: project.config }); + + t.deepEqual( + await querySchema(schemaB, `query { allNestedAuthors { meta { a b } } }`), + { allNestedAuthors: [{ meta: { a: 1, b: 'x' } }] } + ); + const fieldsA = ( + schemaA.getType('NestedAuthor_Meta') as { + getFields: () => Record; + } + ).getFields(); + t.false('b' in fieldsA); + } +); + +test.serial('registers nothing on the process-global composer', async (t) => { + const project = makeProject('IsolatedAuthorGlobal', [ + { id: 'author', meta: { value: 'local' } }, + ]); + + await generateSchema({ config: project.config }); + + t.false(schemaComposer.has('IsolatedAuthorGlobal')); + t.false(schemaComposer.has('IsolatedAuthorGlobal_Meta')); +}); diff --git a/packages/core/src/providers/test/base.test.ts b/packages/core/src/providers/test/base.test.ts index 6d6905f3..acc1d83a 100644 --- a/packages/core/src/providers/test/base.test.ts +++ b/packages/core/src/providers/test/base.test.ts @@ -256,7 +256,7 @@ test.serial('relational filter query', async (t) => { }); test.serial( - 'validates duplicate IDs before returning a cached schema', + 'revalidates duplicate IDs when rebuilding with the same config', async (t) => { let authorEntries: EntryNode[] = [{ id: 'author-one', name: 'Author One' }]; const collection = 'CacheDuplicateAuthor'; @@ -303,3 +303,52 @@ test.serial( t.regex(error?.message ?? '', /virtual\/authors\/ author-one \.json/); } ); + +test.serial( + 'rebuilds providers from changed content with the same config object', + async (t) => { + let authorEntries: EntryNode[] = [{ id: 'author-one', name: 'Author One' }]; + const source = { + fetch: async () => ({ + VirtualAuthor: authorEntries.map((entry) => { + const file = new VFile({ + path: `virtual/authors/${String(entry.id)}.json`, + }); + file.data.entry = entry; + return file; + }), + }), + }; + const config = { + source, + transformer: { + extensions: ['.json'], + inspect: (input: EntryNode) => JSON.stringify(input), + parse: (input: VFile) => input.data.entry as EntryNode, + }, + content: [ + { + path: 'virtual/authors', + collection: 'VirtualAuthor', + }, + ], + }; + + const firstProvider = new FlatbreadProvider(config); + const firstResult = await firstProvider.query({ + source: `query { allVirtualAuthors { name } }`, + }); + t.deepEqual(firstResult.data, { + allVirtualAuthors: [{ name: 'Author One' }], + }); + + authorEntries = [{ id: 'author-one', name: 'Updated Author' }]; + const secondProvider = new FlatbreadProvider(config); + const secondResult = await secondProvider.query({ + source: `query { allVirtualAuthors { name } }`, + }); + t.deepEqual(secondResult.data, { + allVirtualAuthors: [{ name: 'Updated Author' }], + }); + } +); diff --git a/packages/core/src/reload/liveSchema.ts b/packages/core/src/reload/liveSchema.ts index 51e1f3a7..bd6afa36 100644 --- a/packages/core/src/reload/liveSchema.ts +++ b/packages/core/src/reload/liveSchema.ts @@ -37,7 +37,6 @@ export async function createLiveSchemaReloader( const initialSchema = await generateSchema({ config: options.config, contentGraph: initialGraph, - useSchemaCache: false, }); await options.commitSchema({ schema: initialSchema, graph: initialGraph }); snapshot = { schema: initialSchema, graph: initialGraph, generation: 0 }; @@ -50,7 +49,6 @@ export async function createLiveSchemaReloader( const schema = await generateSchema({ config, contentGraph: graph, - useSchemaCache: false, }); await options.commitSchema({ schema, graph }); generation += 1; diff --git a/packages/core/src/reload/test/liveSchema.test.ts b/packages/core/src/reload/test/liveSchema.test.ts index ff52e5db..c7f9abb1 100644 --- a/packages/core/src/reload/test/liveSchema.test.ts +++ b/packages/core/src/reload/test/liveSchema.test.ts @@ -173,8 +173,7 @@ test.serial( // p1 references a1, so it is a ref-affected neighbor and must be re-read. t.true(requestedPaths.includes(postPath)); - // useSchemaCache: false → a fresh schema object per committed generation - // even though the config is identical. + // Each committed generation builds an isolated schema object even when the config is identical. t.not(reloader.getSnapshot().schema, initialSchema); const data = await queryData( diff --git a/packages/core/src/utils/stringUtils.ts b/packages/core/src/utils/stringUtils.ts deleted file mode 100644 index 0b67e602..00000000 --- a/packages/core/src/utils/stringUtils.ts +++ /dev/null @@ -1,22 +0,0 @@ -/** - * Converts any value to a string. Stringifies functions, RegExp, and objects for hashing. - * - * @param valueToConvert The value to convert - */ -export function anyToString(valueToConvert: unknown): string { - return JSON.stringify(valueToConvert, replaceAnyToString); -} - -/** - * Replacer function for `JSON.stringify` that converts functions, RegExp, and objects to strings. - */ -export function replaceAnyToString(_: string, value: unknown) { - if (value === undefined) { - return 'undefined'; - } - - return typeof value === 'function' || - (value instanceof RegExp && value.constructor === RegExp) - ? value.toString() - : value; -}