From 55cf55bbdd5ce4dba350d8315b842d755ced91dc Mon Sep 17 00:00:00 2001 From: saeedjamshaid Date: Wed, 7 Oct 2026 14:58:21 +0500 Subject: [PATCH 1/7] fix(portal): keep serving the preview when a specification fails to load Under `vite dev`, one document saved half written failed the module every route imports, so every page of the preview answered 500 until it was fixed. The preview now leaves that document's reference pages out and serves the rest; a build still fails on it. Co-Authored-By: Claude Opus 5.5 --- portal-template/src/lib/openapi.server.ts | 14 +++++++- test/e2e/portal-serve.test.ts | 44 +++++++++++++++++++++-- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/portal-template/src/lib/openapi.server.ts b/portal-template/src/lib/openapi.server.ts index 3e76ac18e..059b0df05 100644 --- a/portal-template/src/lib/openapi.server.ts +++ b/portal-template/src/lib/openapi.server.ts @@ -5,12 +5,24 @@ import { codeSamplesFile, specs } from './portal.server'; export type OpenApiSource = StaticSource<{ pageData: OpenAPIPageData; metaData: MetaData }>; +type OpenApiSection = Awaited>; + /** * Every document's reference pages as one source. Merged so a reference page has the same * type whichever document it came from, which is what lets the routes tell it apart from a * Markdown page. */ export async function openApiSource(): Promise { - const sections = await Promise.all(Object.entries(specs).map(([slug, file]) => openApiSection(slug, file, codeSamplesFile))); + const loading = Object.entries(specs).map(([slug, file]) => openApiSection(slug, file, codeSamplesFile)); + // Under `vite dev` a document saved half written would fail every page; a build must still fail on it. + const sections = import.meta.env.DEV ? await loaded(loading) : await Promise.all(loading); return { files: sections.flatMap((section) => section.files) }; } + +async function loaded(loading: Promise[]): Promise { + return (await Promise.allSettled(loading)).flatMap((result) => { + if (result.status === 'fulfilled') return [result.value]; + console.error('[OpenAPI] Leaving its reference pages out of the preview:', result.reason); + return []; + }); +} diff --git a/test/e2e/portal-serve.test.ts b/test/e2e/portal-serve.test.ts index 565355a5c..9ca418bde 100644 --- a/test/e2e/portal-serve.test.ts +++ b/test/e2e/portal-serve.test.ts @@ -13,6 +13,7 @@ import { ensurePortalProjectDirectoryBase, removePortalProjectDirectoryBase } from '../../src/infrastructure/tmp-extensions'; +import { sleep } from '../../src/infrastructure/timer-extensions'; // A real Vite dev server takes a minute to compile the portal and needs every runtime // dependency installed, so it stays out of the default run, as the build test does. @@ -24,13 +25,17 @@ const enabled = process.env.APIMATIC_E2E === '1'; const fixture = new DirectoryPath(process.cwd()).join('test/resources/portal-inputs/default/src'); let base: string; let root: string; + let sourceDirectory: DirectoryPath; let server: PortalDevServer | undefined; before(async () => { base = await ensurePortalProjectDirectoryBase(fixture); root = fs.mkdtempSync(path.join(base, 'portal-serve-e2e-')); + // A copy, since a test edits the source while the preview reads it. + sourceDirectory = new DirectoryPath(root).join('src'); + fs.cpSync(fixture.toString(), sourceDirectory.toString(), { recursive: true }); - const source = (await new PortalSourceContext(fixture).resolve())._unsafeUnwrap(); + const source = (await new PortalSourceContext(sourceDirectory).resolve())._unsafeUnwrap(); const delivered = path.join(root, 'delivered'); fs.mkdirSync(delivered, { recursive: true }); fs.writeFileSync(path.join(delivered, 'typescript.zip'), 'PK typescript'); @@ -59,6 +64,24 @@ const enabled = process.env.APIMATIC_E2E === '1'; await removePortalProjectDirectoryBase(base); }); + // Under `vite dev` a page that is not there is answered 200 too, so a Markdown twin is told by its type. + const servesMarkdown = async (address: string): Promise => { + const response = await globalThis.fetch(`${server?.url.toString()}${address}`); + await response.arrayBuffer(); + return response.ok && (response.headers.get('content-type') ?? '').startsWith('text/markdown'); + }; + + // The preview reloads a saved specification in its own time. + const servesMarkdownOnceIt = async (address: string, expected: boolean): Promise => { + const deadline = Date.now() + 60 * 1000; + let served = await servesMarkdown(address); + while (served !== expected && Date.now() < deadline) { + await sleep(250); + served = await servesMarkdown(address); + } + return served; + }; + /** * Safari keeps the entry module cached across previews on the fixed port and executes it * without asking the fresh server, so its first request is the file the entry imports from @@ -77,7 +100,7 @@ const enabled = process.env.APIMATIC_E2E === '1'; // A page imports its images from the static directory, outside the project, by the same kind of address. it('serves an image from the static directory to a browser that asks for it before the page', async () => { const image = fs - .realpathSync(path.join(fixture.toString(), 'static', 'images', 'logo.png')) + .realpathSync(path.join(sourceDirectory.toString(), 'static', 'images', 'logo.png')) .split(path.sep) .join('/'); @@ -85,4 +108,21 @@ const enabled = process.env.APIMATIC_E2E === '1'; expect(response.status).to.equal(200); }); + + it('leaves out the reference of a specification saved half written, serving the rest, until it is fixed', async () => { + const spec = path.join(sourceDirectory.toString(), 'spec', 'Apimatic-Calculator.json'); + const written = fs.readFileSync(spec, 'utf8'); + const reference = '/api/apimatic-calculator/simple-calculator/Calculate.md'; + expect(await servesMarkdown(reference), 'the reference before the save').to.be.true; + + fs.writeFileSync(spec, written.slice(0, written.length / 2)); + try { + expect(await servesMarkdownOnceIt(reference, false), 'the reference while broken').to.be.false; + expect(await servesMarkdown('/authentication.md'), 'a Markdown page while broken').to.be.true; + } finally { + fs.writeFileSync(spec, written); + } + + expect(await servesMarkdownOnceIt(reference, true), 'the reference once fixed').to.be.true; + }); }); From 78a69b88e3fe4f7c4ee8e020324fae50762d85f4 Mon Sep 17 00:00:00 2001 From: saeedjamshaid Date: Wed, 7 Oct 2026 16:26:18 +0500 Subject: [PATCH 2/7] fix(portal): check the files a specification refers to, and say why a document cannot be read Startup read only the documents at the top of `spec/`, so a split specification whose `$ref` named a missing or half-written file passed the check, then failed late in the build or rendered with that reference empty. The files each document refers to are now followed as the portal's bundler follows them, each read once and in parallel, and one that is missing or unreadable is refused, naming the file that refers to it. A document that does not parse is reported with the parser's reason, which carries its line and column. A table of cases holds the check to `bundleSpec`. The preview's dev-only handling in `openApiSource` is inlined. Co-Authored-By: Claude Opus 5.5 --- portal-template/src/lib/openapi.server.ts | 18 +- src/actions/quickstart.ts | 6 +- src/prompts/portal/source.ts | 15 +- src/types/portal-source-context.ts | 92 +++++++-- src/types/portal/openapi-document.ts | 78 ++++++-- src/types/portal/portal-source.ts | 4 +- test/prompts/portal/source.test.ts | 34 ++++ test/types/portal-source-context.test.ts | 211 ++++++++++++++++++++- test/types/portal/openapi-document.test.ts | 80 +++++++- 9 files changed, 484 insertions(+), 54 deletions(-) diff --git a/portal-template/src/lib/openapi.server.ts b/portal-template/src/lib/openapi.server.ts index 059b0df05..cc300c404 100644 --- a/portal-template/src/lib/openapi.server.ts +++ b/portal-template/src/lib/openapi.server.ts @@ -5,8 +5,6 @@ import { codeSamplesFile, specs } from './portal.server'; export type OpenApiSource = StaticSource<{ pageData: OpenAPIPageData; metaData: MetaData }>; -type OpenApiSection = Awaited>; - /** * Every document's reference pages as one source. Merged so a reference page has the same * type whichever document it came from, which is what lets the routes tell it apart from a @@ -15,14 +13,12 @@ type OpenApiSection = Awaited>; export async function openApiSource(): Promise { const loading = Object.entries(specs).map(([slug, file]) => openApiSection(slug, file, codeSamplesFile)); // Under `vite dev` a document saved half written would fail every page; a build must still fail on it. - const sections = import.meta.env.DEV ? await loaded(loading) : await Promise.all(loading); + const sections = import.meta.env.DEV + ? (await Promise.allSettled(loading)).flatMap((result) => { + if (result.status === 'fulfilled') return [result.value]; + console.error('[OpenAPI] Leaving its reference pages out of the preview:', result.reason); + return []; + }) + : await Promise.all(loading); return { files: sections.flatMap((section) => section.files) }; } - -async function loaded(loading: Promise[]): Promise { - return (await Promise.allSettled(loading)).flatMap((result) => { - if (result.status === 'fulfilled') return [result.value]; - console.error('[OpenAPI] Leaving its reference pages out of the preview:', result.reason); - return []; - }); -} diff --git a/src/actions/quickstart.ts b/src/actions/quickstart.ts index f02c80970..506d8131b 100644 --- a/src/actions/quickstart.ts +++ b/src/actions/quickstart.ts @@ -276,8 +276,10 @@ export class QuickstartAction { if (await this.fileService.isZipFile(specPath)) { return { supported: true }; } - const document = OpenApiDocument.parse(specPath.name(), await this.fileService.getContents(specPath)); - return document === undefined ? { supported: true } : document.format(); + return OpenApiDocument.parse(specPath.name(), await this.fileService.getContents(specPath)).match( + (document) => document.format(), + (): SpecFormat => ({ supported: true }) + ); } catch { return { supported: true }; } diff --git a/src/prompts/portal/source.ts b/src/prompts/portal/source.ts index 35f68edee..1eab8d2e7 100644 --- a/src/prompts/portal/source.ts +++ b/src/prompts/portal/source.ts @@ -107,7 +107,20 @@ export function reportSourceProblem( return; } case 'unreadableSpec': { - log.error(`${f.var(problem.fileName.toString())} could not be read as JSON or YAML.`); + const file = relative(problem.file, sourceDirectory); + const named = + problem.referencedBy === null + ? file + : `${file}, which ${relative(problem.referencedBy, sourceDirectory)} refers to,`; + log.error(`${named} could not be read: ${problem.reason}.`); + return; + } + case 'missingSpecReference': { + log.error( + `${relative(problem.referencedBy, sourceDirectory)} refers to ${relative(problem.file, sourceDirectory)}, ` + + 'but there is no such file.' + ); + log.message(`A ${f.var('$ref')} names a file from the folder of the file it is written in.`); return; } case 'missingStaticFiles': { diff --git a/src/types/portal-source-context.ts b/src/types/portal-source-context.ts index 6e1cd702a..7d648176c 100644 --- a/src/types/portal-source-context.ts +++ b/src/types/portal-source-context.ts @@ -36,6 +36,11 @@ import { TRANSFORMATIONS_DIRECTORY_NAME, transformedFileName } from './transform const SPEC_EXTENSIONS = ['.json', '.yaml', '.yml']; +interface SpecFile { + file: FilePath; + document: OpenApiDocument; +} + // Empty on purpose: the portal's own routes under /api/ are files with extensions -- // /api/search.json -- so none can collide with a spec section, which is always a directory. // Pages the user puts under content/api/ share the directory, and `hiddenPages` reports the @@ -386,7 +391,7 @@ export class PortalSourceContext { return PLACEHOLDER_SITE; } const document = await this.readDocument(specPath); - return document === undefined ? PLACEHOLDER_SITE : document.suggestedSite(); + return document?.isOk() ? document.value.suggestedSite() : PLACEHOLDER_SITE; } /** @@ -419,10 +424,15 @@ export class PortalSourceContext { const documentNames = fileNames.filter((name) => SPEC_EXTENSIONS.some((extension) => name.hasExtension(extension))); for (const fileName of documentNames) { const file = new FilePath(this.specDirectory, fileName); - const document = await this.readDocument(file); - if (document === undefined) { - return err({ kind: 'unreadableSpec', fileName }); + const read = await this.readDocument(file); + // Gone since it was listed, as an editor that saves by renaming over the file leaves it for a moment. + if (read === undefined) { + continue; } + if (read.isErr()) { + return err({ kind: 'unreadableSpec', file, referencedBy: null, reason: read.error }); + } + const document = read.value; const format = document.format(); if (!format.supported) { if (format.format !== null) { @@ -431,7 +441,16 @@ export class PortalSourceContext { continue; } - specs.push({ slug: this.uniqueSlug(fileName, usedSlugs), file, endpoints: await this.endpoints(document, file) }); + const spec = { file, document }; + const referenced = await this.referencedDocuments(spec); + if (referenced.isErr()) { + return err(referenced.error); + } + specs.push({ + slug: this.uniqueSlug(fileName, usedSlugs), + file, + endpoints: PortalSourceContext.endpoints(spec, referenced.value) + }); first ??= document; } @@ -468,23 +487,62 @@ export class PortalSourceContext { }; } + // One the bundler cannot read fails the build, or leaves its reference empty, so it is refused. + private async referencedDocuments(spec: SpecFile): Promise> { + const named = new Set([spec.file.toString()]); + const queue: { + file: FilePath; + referencedBy: FilePath; + read: Promise | undefined>; + }[] = []; + // Each file is read once, starting when it is first named, as a split specification runs to thousands. + const enqueue = ({ file: referencedBy, document }: SpecFile) => { + for (const file of document.referencedFiles(referencedBy.directory())) { + if (!named.has(file.toString())) { + named.add(file.toString()); + queue.push({ file, referencedBy, read: this.readDocument(file) }); + } + } + }; + + const walked = [spec]; + enqueue(spec); + // Grows as it is walked: each file read queues the ones it refers to. + for (const { file, referencedBy, read } of queue) { + const parsed = await read; + if (parsed === undefined) { + return err({ kind: 'missingSpecReference', file, referencedBy }); + } + if (parsed.isErr()) { + return err({ kind: 'unreadableSpec', file, referencedBy, reason: parsed.error }); + } + const referenced = { file, document: parsed.value }; + walked.push(referenced); + enqueue(referenced); + } + return ok(walked); + } + // A path item in another file is read from it; one that file refers on to again is not followed. - private async endpoints(document: OpenApiDocument, file: FilePath): Promise { - const referenced = await Promise.all( - document.pathItemReferences(file.directory()).map(async ({ path, file: target, pointer }) => { - const targetDocument = await this.readDocument(target); - return targetDocument?.endpointsAt(path, pointer) ?? []; - }) - ); - return [...document.endpoints(), ...referenced.flat()]; + private static endpoints(spec: SpecFile, referenced: SpecFile[]): Endpoint[] { + const mounted = spec.document + .pathItemReferences(spec.file.directory()) + .flatMap( + ({ path, file, pointer }) => + referenced.find((each) => each.file.isEqual(file))?.document.endpointsAt(path, pointer) ?? [] + ); + return [...spec.document.endpoints(), ...mounted]; } - private async readDocument(file: FilePath): Promise { + /** Undefined when there is no such file; otherwise the document, or why it cannot be read. */ + private async readDocument(file: FilePath): Promise | undefined> { + let contents: string; try { - return OpenApiDocument.parse(file.name(), await this.fileService.getContents(file)); - } catch { - return undefined; + contents = await this.fileService.getContents(file); + } catch (error) { + return (await this.fileService.fileExists(file)) ? err(errorMessage(error)) : undefined; } + return OpenApiDocument.parse(file.name(), contents); } private uniqueSlug(fileName: FileName, used: Set): string { diff --git a/src/types/portal/openapi-document.ts b/src/types/portal/openapi-document.ts index ec0fffcfd..e6410ab98 100644 --- a/src/types/portal/openapi-document.ts +++ b/src/types/portal/openapi-document.ts @@ -1,8 +1,9 @@ -import { basename, dirname } from 'node:path'; -import { parse as parseYaml } from 'yaml'; +import { err, ok, Result } from 'neverthrow'; +import { parse as parseYaml, YAMLParseError } from 'yaml'; import { DirectoryPath } from '../file/directoryPath.js'; import { FileName } from '../file/fileName.js'; import { FilePath } from '../file/filePath.js'; +import { errorMessage } from '../../utils/error-utils.js'; import { isJsonObject, JsonObject } from '../../utils/json-utils.js'; import { stripByteOrderMark } from '../../utils/string-utils.js'; import { PLACEHOLDER_SITE, SuggestedSite } from './config/site-config.js'; @@ -31,26 +32,40 @@ const URL_SCHEME = /^[a-z][a-z\d+.-]+:/i; // The options @scalar/json-magic bundles with, so a spec reads here as it does in the portal. const YAML_OPTIONS = { merge: true, maxAliasCount: 10000 }; +// The keys @scalar/json-magic writes what it has embedded under, which it does not read again. +const BUNDLER_KEYS = new Set(['x-ext', 'x-ext-urls']); + /** A specification as written to disk, read the one way the wizard and the build agree on. */ export class OpenApiDocument { private constructor(private readonly document: JsonObject) {} /** - * Undefined when neither parser accepts the text. A document that parses to something other - * than an object is kept, and reports itself as no specification at all. + * Fails with what the parser said when it does not accept the text. A document that parses to + * something other than an object is kept, and reports itself as no specification at all. */ - public static parse(fileName: FileName, contents: string): OpenApiDocument | undefined { + public static parse(fileName: FileName, contents: string): Result { try { // JSON is valid YAML, but the YAML parser is far slower and specs run to megabytes, // so each extension gets the parser built for it. const text = stripByteOrderMark(contents); const document: unknown = fileName.hasExtension('.json') ? JSON.parse(text) : parseYaml(text, YAML_OPTIONS); - return new OpenApiDocument(isJsonObject(document) ? document : {}); - } catch { - return undefined; + return ok(new OpenApiDocument(isJsonObject(document) ? document : {})); + } catch (error) { + // A YAML error goes on to quote the lines around it, and V8 quotes the JSON it stopped at across line breaks. + const message = errorMessage(error); + const reason = error instanceof YAMLParseError ? message.split('\n')[0].replace(/:$/, '') : message; + return err(reason.replace(/\s+/g, ' ').trim()); } } + /** The local files this document's `$ref`s name, as the portal's bundler finds them, except by `$id`. */ + public referencedFiles(directory: DirectoryPath): FilePath[] { + const ids = idsIn(this.document, new WeakSet(), new Set()); + const references = referencesIn(this.document, new WeakSet(), new Set()); + const names = new Set([...references].map((reference) => reference.split('#')[0])); + return [...names].filter((name) => !ids.has(name)).flatMap((name) => localFile(directory, name) ?? []); + } + /** Every operation declared inline or behind a `$ref` into this document; see `pathItemReferences` for the rest. */ public endpoints(): Endpoint[] { return Object.entries(this.paths()).flatMap(([path, pathItem]) => this.endpointsOf(path, pathItem)); @@ -60,11 +75,9 @@ export class OpenApiDocument { public pathItemReferences(directory: DirectoryPath): PathItemReference[] { return Object.entries(this.paths()).flatMap(([path, pathItem]) => { const reference = isJsonObject(pathItem) && typeof pathItem.$ref === 'string' ? pathItem.$ref : ''; - const [file, pointer = ''] = reference.split('#'); - if (file === '' || URL_SCHEME.test(file)) { - return []; - } - return [{ path, file: new FilePath(directory.resolve(dirname(file)), new FileName(basename(file))), pointer }]; + const [name, pointer = ''] = reference.split('#'); + const file = localFile(directory, name); + return file === undefined ? [] : [{ path, file, pointer }]; }); } @@ -124,6 +137,45 @@ export class OpenApiDocument { } } +/** The file a `$ref` names before its `#`, resolved as the bundler resolves it; undefined for none, or a URL. */ +function localFile(directory: DirectoryPath, name: string): FilePath | undefined { + return name === '' || URL_SCHEME.test(name) ? undefined : FilePath.resolve(directory, name.replaceAll('\\', '/')); +} + +// Both walks add to `found` rather than return arrays to merge, as a specification runs to megabytes. +function referencesIn(node: unknown, seen: WeakSet, found: Set): Set { + if (typeof node !== 'object' || node === null || seen.has(node)) { + return found; + } + seen.add(node); + if (isJsonObject(node) && typeof node.$id === 'string') { + return found; + } + if (isJsonObject(node) && typeof node.$ref === 'string') { + return found.add(node.$ref); + } + for (const [key, value] of Object.entries(node)) { + if (!BUNDLER_KEYS.has(key)) { + referencesIn(value, seen, found); + } + } + return found; +} + +function idsIn(node: unknown, seen: WeakSet, found: Set): Set { + if (typeof node !== 'object' || node === null || seen.has(node)) { + return found; + } + seen.add(node); + if (isJsonObject(node) && typeof node.$id === 'string') { + found.add(node.$id); + } + for (const value of Object.values(node)) { + idsIn(value, seen, found); + } + return found; +} + function valueAt(document: JsonObject, pointer: string): unknown { try { const segments = pointer diff --git a/src/types/portal/portal-source.ts b/src/types/portal/portal-source.ts index 7929e5cc8..37bac2761 100644 --- a/src/types/portal/portal-source.ts +++ b/src/types/portal/portal-source.ts @@ -107,7 +107,9 @@ export type PortalSourceProblem = | { kind: 'invalidConfig'; errors: string[]; missingPortal: boolean } // Every problem found in `content/`, so that one run lists all that a build would refuse. | { kind: 'invalidContent'; problems: ContentProblem[] } - | { kind: 'unreadableSpec'; fileName: FileName } + // `referencedBy`: the file whose `$ref` names it; null for a document at the top of `spec/`. + | { kind: 'unreadableSpec'; file: FilePath; referencedBy: FilePath | null; reason: string } + | { kind: 'missingSpecReference'; file: FilePath; referencedBy: FilePath } // Its `folders` are not read, and `api transform` writes into one of its own. | { kind: 'emptySpecDirectory'; folders: DirectoryPath[] } | { kind: 'noOpenApiSpec'; conversion: SpecConversion } diff --git a/test/prompts/portal/source.test.ts b/test/prompts/portal/source.test.ts index 5ce4cbbd3..b49cd0e74 100644 --- a/test/prompts/portal/source.test.ts +++ b/test/prompts/portal/source.test.ts @@ -191,6 +191,40 @@ describe('reportSourceProblem', () => { }); }); + describe('a file a specification is read from', () => { + const spec = source.join('spec'); + const api = new FilePath(spec, new FileName('api.yaml')); + const pet = new FilePath(spec.join('schemas'), new FileName('Pet.json')); + + it('names a document it cannot read, and why', () => { + const reason = + 'Flow sequence in block collection must be sufficiently indented and end with a ] at line 3, column 1'; + + reportSourceProblem({ kind: 'unreadableSpec', file: api, referencedBy: null, reason }, source); + + expect(printed()).to.equal(`'spec/api.yaml' could not be read: ${reason}.`); + }); + + it('names the file that refers to one it cannot read', () => { + const reason = "Expected ',' or '}' after property value in JSON at position 13 (line 3 column 3)"; + + reportSourceProblem({ kind: 'unreadableSpec', file: pet, referencedBy: api, reason }, source); + + expect(printed()).to.equal( + `'spec/schemas/Pet.json', which 'spec/api.yaml' refers to, could not be read: ${reason}.` + ); + }); + + it('names a file referred to that is not there, and where a reference is resolved from', () => { + reportSourceProblem({ kind: 'missingSpecReference', file: pet, referencedBy: api }, source); + + expect(printed()).to.equal( + "'spec/api.yaml' refers to 'spec/schemas/Pet.json', but there is no such file.\n" + + "A '$ref' names a file from the folder of the file it is written in." + ); + }); + }); + describe('a page at an address kept for the generated pages', () => { const content = source.join('content'); diff --git a/test/types/portal-source-context.test.ts b/test/types/portal-source-context.test.ts index f30c1c288..10f6e10d4 100644 --- a/test/types/portal-source-context.test.ts +++ b/test/types/portal-source-context.test.ts @@ -24,6 +24,7 @@ import { FilePath } from '../../src/types/file/filePath'; import { PLUGIN_SECTION, SDK_SECTION } from '../../src/types/portal/generated-pages'; import { TabEntry } from '../../src/types/portal/portal-navigation'; import { ZipService } from '../../src/infrastructure/zip-service'; +import { bundleSpec } from '../../portal-template/src/lib/openapi-bundle.server'; const OPENAPI = JSON.stringify({ openapi: '3.0.0', info: { title: 'Calc', version: '1' }, paths: {} }); @@ -379,6 +380,19 @@ describe('PortalSourceContext', () => { }); }); + /** A file the specifications could not be read from: the kind, the file, and the file that refers to it. */ + const specProblem = (problem: PortalSourceProblem): [string, string, string | null] => { + if (problem.kind !== 'unreadableSpec' && problem.kind !== 'missingSpecReference') { + throw new Error(`expected a file the specifications are read from, got ${JSON.stringify(problem)}`); + } + const relative = (file: FilePath) => file.relativeTo(new DirectoryPath(root)); + return [ + problem.kind, + relative(problem.file), + problem.referencedBy === null ? null : relative(problem.referencedBy) + ]; + }; + describe('spec discovery', () => { beforeEach(() => writeConfig({ site: { name: 'Calc' } })); @@ -466,10 +480,13 @@ describe('PortalSourceContext', () => { ]); }); - it('reports a document it cannot parse', async () => { + it('reports a document it cannot parse, with what the parser said', async () => { write('spec/broken.json', '{ not json'); - expect((await resolve())._unsafeUnwrapErr().kind).to.equal('unreadableSpec'); + const problem = (await resolve())._unsafeUnwrapErr(); + + expect(specProblem(problem)).to.deep.equal(['unreadableSpec', 'spec/broken.json', null]); + expect(problem.kind === 'unreadableSpec' && problem.reason).to.contain('JSON'); }); it('reports an empty or absent spec directory', async () => { @@ -518,8 +535,7 @@ describe('PortalSourceContext', () => { paths: { '/health': { get: {} }, '/pets': { $ref: './paths/pets.yaml' }, - '/owners': { $ref: 'shared.json#/items/owners' }, - '/missing': { $ref: './nowhere.yaml' } + '/owners': { $ref: 'shared.json#/items/owners' } } }) ); @@ -530,6 +546,193 @@ describe('PortalSourceContext', () => { }); }); + describe('the files a specification refers to', () => { + beforeEach(() => writeConfig({ site: { name: 'Calc' } })); + afterEach(() => sinon.restore()); + + /** `spec/api.json`, whose schemas are the given ones. */ + const writeSpec = (schemas: Record, extra: Record = {}) => + write( + 'spec/api.json', + JSON.stringify({ + openapi: '3.0.0', + info: { title: 'Calc', version: '1' }, + paths: {}, + components: { schemas }, + ...extra + }) + ); + + it('refuses a file a specification refers to that is not there, naming the file that refers to it', async () => { + writeSpec({ Pet: { $ref: './schemas/Pet.yaml' } }); + + expect(specProblem((await resolve())._unsafeUnwrapErr())).to.deep.equal([ + 'missingSpecReference', + 'spec/schemas/Pet.yaml', + 'spec/api.json' + ]); + }); + + it('refuses a file referred to in turn that does not parse, naming the file that refers to it', async () => { + writeSpec({ Pet: { $ref: './schemas/Pet.yaml' } }); + write('spec/schemas/Pet.yaml', 'type: object\nproperties:\n owner:\n $ref: ./Owner.yaml\n'); + write('spec/schemas/Owner.yaml', 'type: object\nrequired: [name\n'); + + const problem = (await resolve())._unsafeUnwrapErr(); + + expect(specProblem(problem)).to.deep.equal([ + 'unreadableSpec', + 'spec/schemas/Owner.yaml', + 'spec/schemas/Pet.yaml' + ]); + expect(problem.kind === 'unreadableSpec' && problem.reason).to.match(/line \d+, column \d+$/); + }); + + it('follows a reference out of the spec directory, as the build does', async () => { + writeSpec({ Pet: { $ref: '../shared/Pet.yaml' } }); + + expect(specProblem((await resolve())._unsafeUnwrapErr())).to.deep.equal([ + 'missingSpecReference', + 'shared/Pet.yaml', + 'spec/api.json' + ]); + + write('shared/Pet.yaml', 'type: object\n'); + expect((await resolve()).isOk()).to.be.true; + }); + + it('reads each file once, however often it is referred to, and ends at a cycle', async () => { + writeSpec({ Pet: { $ref: './schemas/Pet.yaml' }, Pets: { items: { $ref: './schemas/Pet.yaml#/Pet' } } }); + write('spec/schemas/Pet.yaml', 'Pet:\n properties:\n owner:\n $ref: ./Owner.yaml\n'); + write('spec/schemas/Owner.yaml', 'properties:\n pets:\n $ref: ./Pet.yaml\n'); + const reads = sinon.spy(FileService.prototype, 'getContents'); + + expect((await resolve()).isOk()).to.be.true; + const specFiles = reads.args + .map(([file]) => file.relativeTo(new DirectoryPath(root))) + .filter((file) => file.startsWith('spec/')); + expect(specFiles.sort()).to.deep.equal(['spec/api.json', 'spec/schemas/Owner.yaml', 'spec/schemas/Pet.yaml']); + }); + + // Held to the bundler itself, since a file refused here would otherwise have built. + describe('as the portal bundles it', () => { + beforeEach(() => sinon.stub(globalThis.console, 'warn')); + + const bundles = () => + bundleSpec(path.join(root, 'spec', 'api.json')).then( + () => true, + () => false + ); + + const cases: [string, () => void, boolean][] = [ + [ + 'a file that is there', + () => { + writeSpec({ Pet: { $ref: './schemas/Pet.yaml' } }); + write('spec/schemas/Pet.yaml', 'type: object\n'); + }, + true + ], + ['a file that is not there', () => writeSpec({ Pet: { $ref: './schemas/Pet.yaml' } }), false], + [ + 'a file of half-written YAML', + () => { + writeSpec({ Pet: { $ref: './schemas/Pet.yaml' } }); + write('spec/schemas/Pet.yaml', 'type: object\nrequired: [name\n'); + }, + false + ], + [ + 'a file named in example data', + () => writeSpec({ Pet: { type: 'object', example: { $ref: './examples/pet.json' } } }), + false + ], + [ + 'a file named beside a reference, which is not read', + () => { + writeSpec({ Pet: { $ref: './Pet.yaml', properties: { owner: { $ref: './Owner.yaml' } } } }); + write('spec/Pet.yaml', 'type: object\n'); + }, + true + ], + [ + 'two files that refer to each other', + () => { + writeSpec({ Pet: { $ref: './Pet.yaml' } }); + write('spec/Pet.yaml', 'properties:\n owner:\n $ref: ./Owner.yaml\n'); + write('spec/Owner.yaml', 'properties:\n pet:\n $ref: ./Pet.yaml\n'); + }, + true + ], + [ + 'a file named from the folder of the file that refers to it', + () => { + writeSpec({ Pet: { $ref: './schemas/Pet.yaml' } }); + write('spec/schemas/Pet.yaml', 'properties:\n owner:\n $ref: ./Owner.yaml\n'); + write('spec/schemas/Owner.yaml', 'type: object\n'); + }, + true + ], + [ + 'a file named in a part of a file that no reference points into', + () => { + writeSpec({ Pet: { $ref: './schemas/models.yaml#/Pet' } }); + write('spec/schemas/models.yaml', 'Pet:\n type: object\nOwner:\n $ref: ./Owner.yaml\n'); + }, + false + ], + [ + 'a file outside the spec directory', + () => { + writeSpec({ Pet: { $ref: '../shared/Pet.yaml' } }); + write('shared/Pet.yaml', 'type: object\n'); + }, + true + ], + [ + 'a name written with a percent escape, which is not decoded', + () => { + writeSpec({ Pet: { $ref: './my%20pet.yaml' } }); + write('spec/my pet.yaml', 'type: object\n'); + }, + false + ], + [ + 'a schema named by its $id', + () => writeSpec({ Pet: { $id: 'pet.json', type: 'object' }, Pets: { items: { $ref: 'pet.json' } } }), + true + ], + [ + 'a file named only under the keys the bundler writes', + () => writeSpec({}, { 'x-ext': { abc123: { $ref: './embedded.yaml' } } }), + true + ] + ]; + + cases.forEach(([label, arrange, builds]) => { + it(`${builds ? 'accepts' : 'refuses'} ${label}, as the bundler does`, async () => { + arrange(); + + expect(await bundles(), 'the bundler').to.equal(builds); + expect((await resolve()).isOk(), 'the check').to.equal(builds); + }); + }); + + // The bundler leaves the reference pointing at nothing, and the page renders without it. + it('refuses a referenced JSON file it reads as nothing, where the bundler goes on without it', async () => { + writeSpec({ Pet: { $ref: './schemas/Pet.json' } }); + write('spec/schemas/Pet.json', '{ "type": "obj'); + + expect(await bundles(), 'the bundler').to.be.true; + expect(specProblem((await resolve())._unsafeUnwrapErr())).to.deep.equal([ + 'unreadableSpec', + 'spec/schemas/Pet.json', + 'spec/api.json' + ]); + }); + }); + }); + describe('optional directories', () => { beforeEach(() => { writeConfig({ site: { name: 'Calc' } }); diff --git a/test/types/portal/openapi-document.test.ts b/test/types/portal/openapi-document.test.ts index fb4a450f7..f20a886d6 100644 --- a/test/types/portal/openapi-document.test.ts +++ b/test/types/portal/openapi-document.test.ts @@ -11,8 +11,8 @@ const BYTE_ORDER_MARK = ''; describe('OpenApiDocument', () => { const read = (contents: string, fileName = JSON_FILE): OpenApiDocument => { const document = OpenApiDocument.parse(fileName, contents); - expect(document, `unparsable: ${contents}`).to.not.be.undefined; - return document as OpenApiDocument; + expect(document.isOk(), `unparsable: ${contents}`).to.be.true; + return document._unsafeUnwrap(); }; const readJson = (document: unknown, prefix = '') => read(prefix + JSON.stringify(document)); @@ -24,9 +24,15 @@ describe('OpenApiDocument', () => { expect(read('openapi: 3.1.0\n', YAML_FILE).format()).to.deep.equal({ supported: true }); }); - it('is undefined for text neither parser accepts', () => { - expect(OpenApiDocument.parse(JSON_FILE, '{ not json')).to.be.undefined; - expect(OpenApiDocument.parse(YAML_FILE, 'key: [unclosed')).to.be.undefined; + // Matched loosely: the words are the parser's own, and change with its version. + it('fails with what the parser said, on one line, saying where it stopped', () => { + const yaml = OpenApiDocument.parse(YAML_FILE, 'openapi: 3.0.0\ninfo:\n title: [unclosed\n')._unsafeUnwrapErr(); + const json = OpenApiDocument.parse(JSON_FILE, '{\n "a": 1\n "b": 2\n}')._unsafeUnwrapErr(); + const quoting = OpenApiDocument.parse(JSON_FILE, '[1,\n2,\n,3]')._unsafeUnwrapErr(); + + expect(yaml).to.match(/at line 4, column 1$/); + expect(json).to.contain('line 3 column 3'); + [yaml, json, quoting].forEach((reason) => expect(reason).to.not.contain('\n')); }); it('treats a document that is not an object as no specification', () => { @@ -197,6 +203,70 @@ describe('OpenApiDocument', () => { }); }); + describe('referencedFiles', () => { + const specDirectory = new DirectoryPath('/project/src/spec'); + const referenced = (document: Record) => + readJson(document).referencedFiles(specDirectory).map(String); + const inSpec = (file: string) => path.resolve('/project/src/spec', file); + + it('finds a $ref on any object, in data, extensions and lists too, against the folder of the document', () => { + const files = referenced({ + openapi: '3.0.0', + paths: { '/pets': { $ref: './paths/pets.yaml' } }, + components: { + schemas: { Pet: { allOf: [{ $ref: 'schemas/Base.yaml' }] } }, + examples: { Pet: { value: { $ref: 'examples/pet.json' } } } + }, + 'x-shared': { $ref: '../shared/owners.yaml#/Owner' } + }); + + expect(files).to.deep.equal([ + inSpec('paths/pets.yaml'), + inSpec('schemas/Base.yaml'), + inSpec('examples/pet.json'), + path.resolve('/project/src/shared/owners.yaml') + ]); + }); + + it('reads no further into an object with a $ref, as the bundler does not', () => { + const files = referenced({ Pet: { $ref: './pet.yaml', properties: { owner: { $ref: './owner.yaml' } } } }); + + expect(files).to.deep.equal([inSpec('pet.yaml')]); + }); + + it('leaves out references into the document, URLs, and what the bundler has embedded', () => { + const files = referenced({ + local: { $ref: '#/components/schemas/Pet' }, + remote: { $ref: 'https://example.com/pet.yaml' }, + 'x-ext': { abc123: { $ref: './embedded.yaml' } }, + 'x-ext-urls': { abc123: { $ref: './mapped.yaml' } } + }); + + expect(files).to.be.empty; + }); + + it('leaves out a reference below an $id, or naming one, which the bundler resolves against the $id', () => { + const files = referenced({ + Pet: { $id: 'pet.json', properties: { tag: { $ref: './tag.yaml' } } }, + owner: { $ref: 'pet.json#/properties/tag' } + }); + + expect(files).to.be.empty; + }); + + it('reads a backslash as a slash, and leaves a percent escape as written', () => { + const files = referenced({ a: { $ref: '.\\schemas\\Pet.yaml' }, b: { $ref: './my%20pet.yaml' } }); + + expect(files).to.deep.equal([inSpec('schemas/Pet.yaml'), inSpec('my%20pet.yaml')]); + }); + + it('survives an alias that holds itself', () => { + const document = read('openapi: 3.0.0\nloop: &loop\n again: *loop\n next: { $ref: ./next.yaml }\n', YAML_FILE); + + expect(document.referencedFiles(specDirectory).map(String)).to.deep.equal([inSpec('next.yaml')]); + }); + }); + describe('endpointsAt', () => { it('names the operations of the path item a pointer locates after the path it is mounted at', () => { const file = readJson({ get: {}, 'x-library': { owners: { post: {} } } }); From 20935446c78a29e4b9b7f75c967de4549efadd0c Mon Sep 17 00:00:00 2001 From: saeedjamshaid Date: Wed, 7 Oct 2026 19:02:19 +0500 Subject: [PATCH 3/7] fix(portal): check each save in the spec directory while the preview runs A specification saved with a mistake left its reference out of the preview and said nothing. Each save in `spec/` is now checked as `portal generate` checks it: a mistake is reported with the reason, the next good save says it is fixed, and a document added or removed is warned of once, since the preview serves only the documents it started with. The live preview note says so. The "refused, then fixed once" state the content and config previews each kept is one type now, and the sentences the watch messages shared live once in the prompts. Co-Authored-By: Claude Opus 5.5 --- README.md | 4 +- src/actions/portal/serve.ts | 41 +++++ src/commands/portal/serve.ts | 2 +- src/prompts/portal/serve.ts | 86 +++++++-- src/types/portal-source-context.ts | 6 + src/types/portal/portal-source.ts | 1 + src/types/portal/preview-config.ts | 11 +- src/types/portal/preview-content.ts | 8 +- src/types/portal/preview-refusal.ts | 15 ++ src/types/portal/preview-specs.ts | 43 +++++ test/actions/portal/serve.test.ts | 171 +++++++++++++++++- .../portal-project-service.test.ts | 1 + test/prompts/portal/serve.test.ts | 45 +++++ test/types/portal-source-context.test.ts | 13 ++ test/types/portal/preview-refusal.test.ts | 15 ++ test/types/portal/preview-specs.test.ts | 53 ++++++ 16 files changed, 485 insertions(+), 30 deletions(-) create mode 100644 src/types/portal/preview-refusal.ts create mode 100644 src/types/portal/preview-specs.ts create mode 100644 test/types/portal/preview-refusal.test.ts create mode 100644 test/types/portal/preview-specs.test.ts diff --git a/README.md b/README.md index fade86dca..977cd867d 100644 --- a/README.md +++ b/README.md @@ -394,8 +394,8 @@ DESCRIPTION Preview your API Documentation Portal with live reload. Serves the portal described by 'src/apimatic.json' from your machine, reloading the browser as you edit the Markdown - pages in 'src/content', reorder them in a 'nav.json', or change the 'portal', 'languages' or 'plugin' block of - 'apimatic.json'. + pages in 'src/content', reorder them in a 'nav.json', edit the documents in 'src/spec', or change the 'portal', + 'languages' or 'plugin' block of 'apimatic.json'. Adding a language or a 'plugin' block, adding or removing a page in 'src/content', creating 'src/static', or changing which documents are in 'src/spec', needs the preview restarted. diff --git a/src/actions/portal/serve.ts b/src/actions/portal/serve.ts index 7a5e9de8d..62c21a75b 100644 --- a/src/actions/portal/serve.ts +++ b/src/actions/portal/serve.ts @@ -12,6 +12,7 @@ import { PortalArtifacts } from '../../types/portal/portal-artifacts.js'; import { PortalSettings, PortalSource } from '../../types/portal/portal-source.js'; import { PreviewConfig } from '../../types/portal/preview-config.js'; import { PreviewContent } from '../../types/portal/preview-content.js'; +import { PreviewSpecs } from '../../types/portal/preview-specs.js'; import { FileWatch, FileWatchService } from '../../infrastructure/file-watch-service.js'; import { NetworkService } from '../../infrastructure/network-service.js'; import { LauncherService } from '../../infrastructure/launcher-service.js'; @@ -78,7 +79,9 @@ export class PortalServeAction { contentWatch?.recheck(); } }); + const specWatch = this.watchSpecs(project, source); const closeWatches = async () => { + await specWatch?.close(); await configWatch?.close(); await contentWatch?.close(); }; @@ -224,6 +227,44 @@ export class PortalServeAction { ); } + /** Held to a build's rules, as `content/` is, but the preview itself leaves out a reference it cannot read. */ + private watchSpecs(project: ProjectContext, source: PortalSource): FileWatch | undefined { + const sourceDirectory = project.sourceDirectory(); + const sourceContext = project.portalSource(); + const preview = new PreviewSpecs(source.specs); + + const check = async () => { + const checked = await sourceContext.resolveSpecs(); + if (checked.isErr()) { + preview.refuse(); + this.prompts.specRejected(checked.error, sourceDirectory); + return; + } + const shown = preview.show(checked.value); + if (shown.fixed) { + this.prompts.specAccepted(sourceDirectory); + } + if (shown.change !== null) { + this.prompts.specsNeedRestart(shown.change, sourceDirectory); + } + }; + + return this.startWatch( + (onChange) => + this.fileWatchService.watchTree( + source.specDirectory, + onChange, + (reason) => this.prompts.specWatchFailed(reason, sourceDirectory), + isSkippedByGlob + ), + check, + { + notWatched: (reason) => this.prompts.specNotWatched(reason, sourceDirectory), + thrown: (reason) => this.prompts.specNotChecked(reason, sourceDirectory) + } + ); + } + /** * Runs `handle` on each save the watch reports, and once at the start: what it handles was read * before the preview started, which can take a minute, and a save made meanwhile reached no watch. diff --git a/src/commands/portal/serve.ts b/src/commands/portal/serve.ts index a3e580561..9fa8d254a 100644 --- a/src/commands/portal/serve.ts +++ b/src/commands/portal/serve.ts @@ -11,7 +11,7 @@ const defaultPortText = String(DEFAULT_PORTAL_PORT); export default class PortalServe extends Command { static readonly summary = 'Preview your API Documentation Portal with live reload.'; - static readonly description = `Serves the portal described by 'src/apimatic.json' from your machine, reloading the browser as you edit the Markdown pages in 'src/content', reorder them in a 'nav.json', or change the 'portal', 'languages' or 'plugin' block of 'apimatic.json'. + static readonly description = `Serves the portal described by 'src/apimatic.json' from your machine, reloading the browser as you edit the Markdown pages in 'src/content', reorder them in a 'nav.json', edit the documents in 'src/spec', or change the 'portal', 'languages' or 'plugin' block of 'apimatic.json'. Adding a language or a 'plugin' block, adding or removing a page in 'src/content', creating 'src/static', or changing which documents are in 'src/spec', needs the preview restarted. diff --git a/src/prompts/portal/serve.ts b/src/prompts/portal/serve.ts index b610fe4a4..8b3c87454 100644 --- a/src/prompts/portal/serve.ts +++ b/src/prompts/portal/serve.ts @@ -2,12 +2,15 @@ import { log } from '@clack/prompts'; import { once } from 'node:events'; import { APIMATIC_CONFIG_FILE_NAME } from '../../types/apimatic-config/document.js'; import { DirectoryPath } from '../../types/file/directoryPath.js'; +import { FilePath } from '../../types/file/filePath.js'; import { UrlPath } from '../../types/file/urlPath.js'; import { ContentNotices } from '../../types/portal/content-notices.js'; import { MissingArtifacts } from '../../types/portal/generated-pages.js'; import { NAVIGATION_FILE_NAME } from '../../types/portal/portal-navigation.js'; import { ContentProblem, PortalSourceProblem } from '../../types/portal/portal-source.js'; +import { SpecsChange } from '../../types/portal/preview-specs.js'; import { PortalDevServer, PortalDevServerFailure } from '../../infrastructure/portal-dev-server-service.js'; +import { listedInProse } from '../../utils/string-utils.js'; import { Result } from 'neverthrow'; import { format as f } from '../format.js'; import { logTail, noteWrapped, withSpinner } from '../prompt.js'; @@ -53,6 +56,8 @@ export class PortalServePrompts { `So does removing a language from ${f.var('languages')} (updates the SDK pages) or the ` + `${f.var('plugin')} block (removes the Context Plugin pages).`, 'Mistakes in these files are reported on save, and the preview keeps what it last accepted.', + `Saving a document in ${specPath(sourceDirectory)} updates its API reference; one with a mistake is ` + + 'reported, and left out until it is fixed.', '', `Restart the preview after adding a language or a ${f.var('plugin')} block (its SDK or plugin is ` + `fetched at startup), adding or removing a page, creating ${staticPath(sourceDirectory)}, or ` + @@ -96,16 +101,11 @@ export class PortalServePrompts { } public configNotWatched(reason: string) { - log.warn( - `${f.var(APIMATIC_CONFIG_FILE_NAME)} cannot be watched (${reason}), so edits to it need the preview restarted.` - ); + this.notWatched(f.var(APIMATIC_CONFIG_FILE_NAME), reason, 'edits to it need the preview restarted'); } public configWatchFailed(reason: string) { - log.warn( - `${f.var(APIMATIC_CONFIG_FILE_NAME)} is no longer watched (${reason}), so further edits to it need the ` + - `preview restarted.` - ); + this.watchFailed(f.var(APIMATIC_CONFIG_FILE_NAME), reason, 'further edits to it need the preview restarted'); } /** Explained as `portal generate` would explain it, since the same rules refused it. */ @@ -122,11 +122,11 @@ export class PortalServePrompts { } public contentNotChecked(reason: string, sourceDirectory: DirectoryPath) { - log.warn(`The changes to ${contentPath(sourceDirectory)} could not be checked: ${reason}`); + this.notChecked(contentPath(sourceDirectory), reason); } public contentAccepted(sourceDirectory: DirectoryPath) { - log.success(`${contentPath(sourceDirectory)} is fixed; a build would accept it again.`); + this.fixed(contentPath(sourceDirectory)); } public contentNotices(notices: ContentNotices, sourceDirectory: DirectoryPath) { @@ -134,17 +134,48 @@ export class PortalServePrompts { } public contentNotWatched(reason: string, sourceDirectory: DirectoryPath) { - log.warn( - `${contentPath(sourceDirectory)} cannot be watched (${reason}), so a mistake in a page or a ` + - `${f.var(NAVIGATION_FILE_NAME)} is only reported when the preview is restarted.` - ); + const until = reportedOnRestart(`a page or a ${f.var(NAVIGATION_FILE_NAME)}`); + this.notWatched(contentPath(sourceDirectory), reason, until); } public contentWatchFailed(reason: string, sourceDirectory: DirectoryPath) { + const until = reportedOnRestart(`a page or a ${f.var(NAVIGATION_FILE_NAME)}`); + this.watchFailed(contentPath(sourceDirectory), reason, until); + } + + /** Explained as `portal generate` would explain it, since the same rules refused it. */ + public specRejected(problem: PortalSourceProblem, sourceDirectory: DirectoryPath) { + reportSourceProblem(problem, sourceDirectory, { offerQuickstart: false }); + log.message( + `Until ${specPath(sourceDirectory)} is fixed, the preview leaves out the API reference it cannot read; ` + + 'a build would stop here.' + ); + } + + public specAccepted(sourceDirectory: DirectoryPath) { + this.fixed(specPath(sourceDirectory)); + } + + // The preview lists its documents once, when it starts. + public specsNeedRestart(change: SpecsChange, sourceDirectory: DirectoryPath) { + const changes = [...filesWere(change.added, 'added'), ...filesWere(change.removed, 'removed')]; log.warn( - `${contentPath(sourceDirectory)} is no longer watched (${reason}), so a mistake in a page or ` + - `a ${f.var(NAVIGATION_FILE_NAME)} is only reported when the preview is restarted.` + `The documents in ${specPath(sourceDirectory)} are not the ones the preview started with: ` + + `${changes.join(', and ')}.` ); + log.message('Restart the preview to show them; until then it shows those it started with that are still there.'); + } + + public specNotChecked(reason: string, sourceDirectory: DirectoryPath) { + this.notChecked(specPath(sourceDirectory), reason); + } + + public specNotWatched(reason: string, sourceDirectory: DirectoryPath) { + this.notWatched(specPath(sourceDirectory), reason, reportedOnRestart('a specification')); + } + + public specWatchFailed(reason: string, sourceDirectory: DirectoryPath) { + this.watchFailed(specPath(sourceDirectory), reason, reportedOnRestart('a specification')); } public stopping() { @@ -163,4 +194,29 @@ export class PortalServePrompts { public async blockExecution() { await Promise.race([once(process, 'SIGINT'), once(process, 'SIGTERM')]); } + + private fixed(directory: string) { + log.success(`${directory} is fixed; a build would accept it again.`); + } + + private notChecked(directory: string, reason: string) { + log.warn(`The changes to ${directory} could not be checked: ${reason}`); + } + + private notWatched(watched: string, reason: string, until: string) { + log.warn(`${watched} cannot be watched (${reason}), so ${until}.`); + } + + private watchFailed(watched: string, reason: string, until: string) { + log.warn(`${watched} is no longer watched (${reason}), so ${until}.`); + } +} + +const reportedOnRestart = (mistakes: string) => + `a mistake in ${mistakes} is only reported when the preview is restarted`; + +// `'a.yaml' was added`, or `'a.yaml' and 'b.yaml' were removed`; nothing for no files. +function filesWere(files: FilePath[], done: string): string[] { + const names = listedInProse(files.map((file) => f.var(file.name().toString()))); + return files.length === 0 ? [] : [`${names} ${files.length === 1 ? 'was' : 'were'} ${done}`]; } diff --git a/src/types/portal-source-context.ts b/src/types/portal-source-context.ts index 7d648176c..817f5bf9f 100644 --- a/src/types/portal-source-context.ts +++ b/src/types/portal-source-context.ts @@ -112,6 +112,7 @@ export class PortalSourceContext { return ok({ ...settings.value, suggestedSite: suggested, + specDirectory: this.specDirectory, specs, contentDirectory, staticDirectory, @@ -131,6 +132,11 @@ export class PortalSourceContext { return await this.content(await this.existingContentDirectory(), specs, generatedPages); } + /** The `spec/` half of `resolve`, for `portal serve` to run on each save. */ + public async resolveSpecs(): Promise> { + return (await this.specs()).map(({ specs }) => specs); + } + private async existingContentDirectory(): Promise { return (await this.fileService.directoryExists(this.contentDirectory)) ? this.contentDirectory : null; } diff --git a/src/types/portal/portal-source.ts b/src/types/portal/portal-source.ts index 37bac2761..1d0dd3c91 100644 --- a/src/types/portal/portal-source.ts +++ b/src/types/portal/portal-source.ts @@ -28,6 +28,7 @@ export interface PortalSource extends PortalSettings { * can judge an edited config without reading the specifications again. */ suggestedSite: SuggestedSite | null; + specDirectory: DirectoryPath; specs: PortalSpec[]; contentDirectory: DirectoryPath | null; staticDirectory: DirectoryPath | null; diff --git a/src/types/portal/preview-config.ts b/src/types/portal/preview-config.ts index 8d2436011..ece9fd68c 100644 --- a/src/types/portal/preview-config.ts +++ b/src/types/portal/preview-config.ts @@ -1,9 +1,10 @@ import { PortalConfig } from './portal-config.js'; +import { PreviewRefusal } from './preview-refusal.js'; /** The notice is given once, on the save that brings it about, rather than on every save after it. */ export class PreviewConfig { private shown: PortalConfig; - private refused = false; + private readonly refusal = new PreviewRefusal(); /** * `servesStatic`: the dev server serves `static/` only if it was there at startup, and a @@ -14,7 +15,7 @@ export class PreviewConfig { } public refuse(): void { - this.refused = true; + this.refusal.refuse(); } public staticDirectoryNotServed(config: PortalConfig): boolean { @@ -27,9 +28,9 @@ export class PreviewConfig { * it is accepted again. */ public show(config: PortalConfig, written: boolean): boolean { - const announce = written || this.refused; - this.refused = false; + // Not after `written ||`, which would skip it, and so say it is fixed again after the next save. + const fixed = this.refusal.accept(); this.shown = config; - return announce; + return written || fixed; } } diff --git a/src/types/portal/preview-content.ts b/src/types/portal/preview-content.ts index 7ec18ffc2..e491201f0 100644 --- a/src/types/portal/preview-content.ts +++ b/src/types/portal/preview-content.ts @@ -2,11 +2,12 @@ import { FilePath } from '../file/filePath.js'; import { ContentNotices, NoTabsListed } from './content-notices.js'; import { isSameTabEntry } from './portal-navigation.js'; import { SharedTabName, TabOwner } from './portal-tabs.js'; +import { PreviewRefusal } from './preview-refusal.js'; /** A notice is given once, on the save that brings it about, rather than on every save after it. */ export class PreviewContent { private shown: ContentNotices; - private refused = false; + private readonly refusal = new PreviewRefusal(); /** `startup`: what the checks before the preview started reported. */ constructor(startup: ContentNotices) { @@ -14,7 +15,7 @@ export class PreviewContent { } public refuse(): void { - this.refused = true; + this.refusal.refuse(); } /** @@ -22,8 +23,7 @@ export class PreviewContent { * and which notices to give: each kind in full when it holds something new, and none otherwise. */ public show(notices: ContentNotices): { fixed: boolean; notices: ContentNotices } { - const shown = { fixed: this.refused, notices: PreviewContent.since(notices, this.shown) }; - this.refused = false; + const shown = { fixed: this.refusal.accept(), notices: PreviewContent.since(notices, this.shown) }; this.shown = notices; return shown; } diff --git a/src/types/portal/preview-refusal.ts b/src/types/portal/preview-refusal.ts new file mode 100644 index 000000000..fb1790cac --- /dev/null +++ b/src/types/portal/preview-refusal.ts @@ -0,0 +1,15 @@ +/** A save the preview refused, so that the first one it accepts after it, and only that one, is said to fix it. */ +export class PreviewRefusal { + private refused = false; + + public refuse(): void { + this.refused = true; + } + + /** Whether this accepted save fixes one refused before it. */ + public accept(): boolean { + const fixed = this.refused; + this.refused = false; + return fixed; + } +} diff --git a/src/types/portal/preview-specs.ts b/src/types/portal/preview-specs.ts new file mode 100644 index 000000000..46e766362 --- /dev/null +++ b/src/types/portal/preview-specs.ts @@ -0,0 +1,43 @@ +import { FilePath } from '../file/filePath.js'; +import { PortalSpec } from './portal-source.js'; +import { PreviewRefusal } from './preview-refusal.js'; + +/** The documents put in `spec/`, or taken out of it, since the preview started. */ +export interface SpecsChange { + added: FilePath[]; + removed: FilePath[]; +} + +/** The preview serves only the documents it started with, so a change to which are in `spec/` is said once. */ +export class PreviewSpecs { + private readonly served: FilePath[]; + private said: FilePath[]; + private readonly refusal = new PreviewRefusal(); + + /** `startup`: the documents the preview started with. */ + constructor(startup: PortalSpec[]) { + this.served = startup.map(({ file }) => file); + this.said = this.served; + } + + public refuse(): void { + this.refusal.refuse(); + } + + /** Records the documents as shown: whether `spec/` is fixed, having been refused, and a change not yet warned of. */ + public show(specs: PortalSpec[]): { fixed: boolean; change: SpecsChange | null } { + const files = specs.map(({ file }) => file); + const unchanged = isSameSet(files, this.said) || isSameSet(files, this.served); + this.said = files; + const change = { added: notIn(files, this.served), removed: notIn(this.served, files) }; + return { fixed: this.refusal.accept(), change: unchanged ? null : change }; + } +} + +function notIn(files: FilePath[], others: FilePath[]): FilePath[] { + return files.filter((file) => !others.some((other) => other.isEqual(file))); +} + +function isSameSet(left: FilePath[], right: FilePath[]): boolean { + return notIn(left, right).length === 0 && notIn(right, left).length === 0; +} diff --git a/test/actions/portal/serve.test.ts b/test/actions/portal/serve.test.ts index f29e5d5c8..d717231bd 100644 --- a/test/actions/portal/serve.test.ts +++ b/test/actions/portal/serve.test.ts @@ -230,7 +230,10 @@ describe('PortalServeAction', () => { watched = new Promise((resolve) => { watchTree.callsFake( (directory: DirectoryPath, onChange: () => Promise, onFailed: (reason: string) => void) => { - expect(directory.toString()).to.equal(source.join('content').toString()); + // `spec/` is watched as well, and these tests leave its watch alone. + if (!directory.isEqual(source.join('content'))) { + return ok({ close: sinon.stub().resolves(), recheck: sinon.stub() }); + } resolve({ onChange, onFailed }); return ok({ close: closeWatch, recheck }); } @@ -408,13 +411,175 @@ describe('PortalServeAction', () => { expect(closeWatch.calledBefore(prompts.previewStopped)).to.be.true; }); - it('watches nothing when there is no content directory', async () => { + it('watches no content when there is no content directory', async () => { fs.rmSync(path.join(source.toString(), 'content'), { recursive: true }); interrupt(); await execute(new DirectoryPath(root)); - expect(watchTree.called).to.be.false; + const watchedDirectories = watchTree.getCalls().map(({ args: [directory] }) => directory.toString()); + expect(watchedDirectories).to.deep.equal([source.join('spec').toString()]); + }); + }); + + // The preview reloads a saved specification itself; the CLI's part is saying what a build would refuse. + describe('checking a save in the spec directory', () => { + let source: DirectoryPath; + let watched: Promise<{ onChange: () => Promise; onFailed: (reason: string) => void }>; + let closeWatch: sinon.SinonStub; + let recheck: sinon.SinonStub; + let written: string; + + const specFile = (name: string) => path.join(source.toString(), 'spec', name); + const writeSpec = (name: string, contents: string) => fs.writeFileSync(specFile(name), contents); + const halfWritten = () => writeSpec('Apimatic-Calculator.json', written.slice(0, written.length / 2)); + const restored = () => writeSpec('Apimatic-Calculator.json', written); + + /** Runs the preview until `body` is done with it, then stops it as CTRL+C would. */ + const whileServing = async (body: (saved: () => Promise) => Promise) => { + const running = execute(new DirectoryPath(root)); + const { onChange } = await watched; + try { + await body(onChange); + } finally { + interrupt(); + await running; + } + }; + + beforeEach(() => { + source = new DirectoryPath(root).join('src'); + fs.cpSync(FIXTURE_SOURCE.toString(), source.toString(), { recursive: true }); + written = fs.readFileSync(specFile('Apimatic-Calculator.json'), 'utf8'); + + closeWatch = sinon.stub().resolves(); + recheck = sinon.stub(); + watched = new Promise((resolve) => { + watchTree.callsFake( + (directory: DirectoryPath, onChange: () => Promise, onFailed: (reason: string) => void) => { + // `content/` is watched as well, and these tests leave its watch alone. + if (!directory.isEqual(source.join('spec'))) { + return ok({ close: sinon.stub().resolves(), recheck: sinon.stub() }); + } + resolve({ onChange, onFailed }); + return ok({ close: closeWatch, recheck }); + } + ); + }); + }); + + // The preview itself leaves the document's reference out, and says nothing. + it('reports a document saved half written, as a build would', async () => { + await whileServing(async (saved) => { + halfWritten(); + await saved(); + + const [problem, sourceDirectory] = prompts.specRejected.firstCall.args; + expect(problem.kind === 'unreadableSpec' && problem.file.relativeTo(source)).to.equal( + 'spec/Apimatic-Calculator.json' + ); + expect(sourceDirectory.isEqual(source)).to.be.true; + }); + }); + + it('reports a file a document refers to that is not there', async () => { + const document = JSON.parse(written); + document.components.schemas.Pet = { $ref: './schemas/Pet.yaml' }; + + await whileServing(async (saved) => { + writeSpec('Apimatic-Calculator.json', JSON.stringify(document)); + await saved(); + + const [problem] = prompts.specRejected.firstCall.args; + expect(problem.kind === 'missingSpecReference' && problem.file.relativeTo(source)).to.equal( + 'spec/schemas/Pet.yaml' + ); + }); + }); + + it('says once that the specifications are fixed, and nothing for a save a build accepts', async () => { + await whileServing(async (saved) => { + await saved(); + expect(prompts.specRejected.called || prompts.specAccepted.called).to.be.false; + + halfWritten(); + await saved(); + restored(); + await saved(); + await saved(); + + expect(prompts.specRejected.calledOnce).to.be.true; + expect(prompts.specAccepted.calledOnce).to.be.true; + }); + }); + + it('warns once that a document added needs a restart, and not when it is taken out again', async () => { + await whileServing(async (saved) => { + writeSpec( + 'orders.json', + JSON.stringify({ openapi: '3.0.0', info: { title: 'Orders', version: '1' }, paths: {} }) + ); + await saved(); + await saved(); + + expect(prompts.specsNeedRestart.calledOnce).to.be.true; + const [change] = prompts.specsNeedRestart.firstCall.args; + expect(change.added.map((file) => file.relativeTo(source))).to.deep.equal(['spec/orders.json']); + expect(change.removed).to.be.empty; + + fs.rmSync(specFile('orders.json')); + await saved(); + + expect(prompts.specsNeedRestart.calledOnce).to.be.true; + }); + }); + + // An editor's swap file would only have the specifications checked again for nothing. + it('leaves out of the watch what the build never reads', async () => { + await whileServing(async () => { + const call = watchTree.getCalls().find(({ args: [directory] }) => directory.isEqual(source.join('spec'))); + const isIgnored: (name: string) => boolean = call?.args[3]; + + expect(['.Apimatic-Calculator.json.swp', 'node_modules'].every(isIgnored)).to.be.true; + expect(isIgnored('Apimatic-Calculator.json')).to.be.false; + }); + }); + + // The specifications are read before the preview starts, which can take a minute. + it('checks the specifications again once they are watched, and stops watching when the preview stops', async () => { + await whileServing(async () => { + expect(recheck.calledOnce).to.be.true; + }); + + expect(closeWatch.calledBefore(prompts.stopping)).to.be.true; + }); + + it('says when the specifications cannot be watched, and serves regardless', async () => { + watchTree.returns(err('EMFILE: too many open files')); + interrupt(); + + const result = await execute(new DirectoryPath(root)); + + expect(prompts.specNotWatched.calledOnceWith('EMFILE: too many open files')).to.be.true; + expect(result.isCancelled()).to.be.true; + }); + + it('tells the user when the specifications stop being watched', async () => { + await whileServing(async () => { + (await watched).onFailed('EPERM: operation not permitted'); + + expect(prompts.specWatchFailed.calledOnceWith('EPERM: operation not permitted')).to.be.true; + }); + }); + + it('says when a save could not be checked, rather than dropping it', async () => { + sinon.stub(PortalSourceContext.prototype, 'resolveSpecs').rejects(new Error('EIO: i/o error')); + + await whileServing(async (saved) => { + await saved(); + + expect(prompts.specNotChecked.calledOnceWith('EIO: i/o error')).to.be.true; + }); }); }); diff --git a/test/infrastructure/portal-project-service.test.ts b/test/infrastructure/portal-project-service.test.ts index a1972d4c8..726884845 100644 --- a/test/infrastructure/portal-project-service.test.ts +++ b/test/infrastructure/portal-project-service.test.ts @@ -52,6 +52,7 @@ describe('PortalProjectService', () => { config: configFor({ site: { name: 'My API' } }), generatedPages: pagesFor(), suggestedSite: null, + specDirectory: new DirectoryPath(root).join('spec'), specs: [ { slug: 'calculator', diff --git a/test/prompts/portal/serve.test.ts b/test/prompts/portal/serve.test.ts index ef5e580b1..a55f269fc 100644 --- a/test/prompts/portal/serve.test.ts +++ b/test/prompts/portal/serve.test.ts @@ -4,6 +4,8 @@ import sinon from 'sinon'; import { log } from '@clack/prompts'; import { PortalServePrompts } from '../../../src/prompts/portal/serve.js'; import { DirectoryPath } from '../../../src/types/file/directoryPath.js'; +import { FileName } from '../../../src/types/file/fileName.js'; +import { FilePath } from '../../../src/types/file/filePath.js'; import { UrlPath } from '../../../src/types/file/urlPath.js'; import { Language } from '../../../src/types/sdk/generate.js'; @@ -49,6 +51,49 @@ describe('PortalServePrompts', () => { 'Mistakes in these files are reported on save, and the preview keeps what it last accepted.' ); }); + + it('says a saved specification updates its reference, and one with a mistake is reported and left out', () => { + expect(printed()).to.match( + /Saving a document in '.*spec' updates its API reference; one with a mistake is reported, and left out until it is fixed\./ + ); + }); + }); + + describe('a change to which documents are in the spec directory', () => { + const document = (name: string) => new FilePath(new DirectoryPath('src', 'spec'), new FileName(name)); + + /** The warning, then the advice that follows it. */ + const said = (added: string[], removed: string[]) => { + const warn = sinon.stub(log, 'warn'); + const message = sinon.stub(log, 'message'); + try { + new PortalServePrompts().specsNeedRestart( + { added: added.map(document), removed: removed.map(document) }, + new DirectoryPath('src') + ); + return [warn, message].map((stub) => stripVTControlCharacters(String(stub.firstCall.args[0]))); + } finally { + sinon.restore(); + } + }; + + it('names each document added and removed, and that a restart shows them', () => { + const [warning, advice] = said(['orders.json'], ['pets.json']); + + expect(warning).to.match( + /^The documents in '.*spec' are not the ones the preview started with: 'orders\.json' was added, and 'pets\.json' was removed\.$/ + ); + expect(advice).to.equal( + 'Restart the preview to show them; until then it shows those it started with that are still there.' + ); + }); + + it('lists several documents, and says only what was done', () => { + expect(said(['a.json', 'b.json'], [])[0]).to.match(/: 'a\.json' and 'b\.json' were added\.$/); + expect(said([], ['a.json', 'b.json', 'c.json'])[0]).to.match( + /: 'a\.json', 'b\.json' and 'c\.json' were removed\.$/ + ); + }); }); describe('an edit that needs artifacts the preview was started without', () => { diff --git a/test/types/portal-source-context.test.ts b/test/types/portal-source-context.test.ts index 10f6e10d4..cfabdbf18 100644 --- a/test/types/portal-source-context.test.ts +++ b/test/types/portal-source-context.test.ts @@ -404,6 +404,19 @@ describe('PortalSourceContext', () => { const source = (await resolve())._unsafeUnwrap(); expect(source.specs.map((spec) => spec.slug)).to.deep.equal(['a', 'b', 'c']); + expect(source.specDirectory.toString()).to.equal(path.join(root, 'spec')); + }); + + // What `portal serve` reads again on each save. + it('finds the same documents, and the same problems, on their own', async () => { + write('spec/b.json', OPENAPI); + write('spec/a.yaml', 'openapi: 3.0.0\ninfo:\n title: A\n version: "1"\npaths: {}\n'); + const specs = () => new PortalSourceContext(new DirectoryPath(root)).resolveSpecs(); + + expect((await specs())._unsafeUnwrap()).to.deep.equal((await resolve())._unsafeUnwrap().specs); + + write('spec/b.json', '{ "openapi": '); + expect((await specs())._unsafeUnwrapErr()).to.deep.equal((await resolve())._unsafeUnwrapErr()); }); it('ignores documents that carry no version key', async () => { diff --git a/test/types/portal/preview-refusal.test.ts b/test/types/portal/preview-refusal.test.ts new file mode 100644 index 000000000..7ad277b10 --- /dev/null +++ b/test/types/portal/preview-refusal.test.ts @@ -0,0 +1,15 @@ +import { expect } from 'chai'; +import { PreviewRefusal } from '../../../src/types/portal/preview-refusal'; + +describe('PreviewRefusal', () => { + it('is fixed by the first save accepted after a refusal, and by no other', () => { + const refusal = new PreviewRefusal(); + expect(refusal.accept()).to.be.false; + + refusal.refuse(); + refusal.refuse(); + + expect(refusal.accept()).to.be.true; + expect(refusal.accept()).to.be.false; + }); +}); diff --git a/test/types/portal/preview-specs.test.ts b/test/types/portal/preview-specs.test.ts new file mode 100644 index 000000000..8ea151abb --- /dev/null +++ b/test/types/portal/preview-specs.test.ts @@ -0,0 +1,53 @@ +import { expect } from 'chai'; +import { DirectoryPath } from '../../../src/types/file/directoryPath'; +import { FileName } from '../../../src/types/file/fileName'; +import { FilePath } from '../../../src/types/file/filePath'; +import { PortalSpec } from '../../../src/types/portal/portal-source'; +import { PreviewSpecs } from '../../../src/types/portal/preview-specs'; + +describe('PreviewSpecs', () => { + const spec = new DirectoryPath('src', 'spec'); + const document = (name: string): PortalSpec => ({ + slug: name, + file: new FilePath(spec, new FileName(name)), + endpoints: [] + }); + const named = (files: FilePath[]) => files.map((file) => file.name().toString()); + + /** The change each save warns of, by the names added and removed, for a preview started with `startup`. */ + const warnings = (startup: string[], saves: string[][]) => { + const preview = new PreviewSpecs(startup.map(document)); + return saves.map((names) => { + const { change } = preview.show(names.map(document)); + return change === null ? null : { added: named(change.added), removed: named(change.removed) }; + }); + }; + + it('warns of nothing while the documents are the ones it started with', () => { + expect(warnings(['pets.json'], [['pets.json'], ['pets.json']])).to.deep.equal([null, null]); + }); + + it('warns of a document added or removed since it started, on the save that does it and not after', () => { + expect( + warnings(['pets.json'], [['orders.json', 'pets.json'], ['orders.json', 'pets.json'], ['orders.json']]) + ).to.deep.equal([ + { added: ['orders.json'], removed: [] }, + null, + { added: ['orders.json'], removed: ['pets.json'] } + ]); + }); + + it('says nothing on a return to the documents it started with, and warns again of the change made anew', () => { + expect( + warnings(['pets.json'], [['orders.json', 'pets.json'], ['pets.json'], ['orders.json', 'pets.json']]) + ).to.deep.equal([{ added: ['orders.json'], removed: [] }, null, { added: ['orders.json'], removed: [] }]); + }); + + it('says the specifications are fixed once, on the first save it accepts after refusing one', () => { + const preview = new PreviewSpecs([document('pets.json')]); + preview.refuse(); + + expect(preview.show([document('pets.json')]).fixed).to.be.true; + expect(preview.show([document('pets.json')]).fixed).to.be.false; + }); +}); From 8df4a90be075f617d43c4adb7ff01cac1f2340a9 Mon Sep 17 00:00:00 2001 From: saeedjamshaid Date: Fri, 9 Oct 2026 15:33:34 +0500 Subject: [PATCH 4/7] fix(portal): name the static directory by its real path in the preview's allow list Vite resolves a symlink before comparing a requested file against `server.fs.allow`, and the static directory was listed as written. A project reached through a symlink, as anything under macOS's `/tmp` or `/var` is, had a static file asked for before its page refused with 403, which is the request apimatic-io#2287 is about. The directory is now named by its real path, as the dependency directories beside it already are. Co-Authored-By: Claude Opus 5.5 --- src/infrastructure/portal-project-service.ts | 6 +++++- test/infrastructure/portal-project-service.test.ts | 14 ++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/infrastructure/portal-project-service.ts b/src/infrastructure/portal-project-service.ts index 6a53f3b0f..5e8be39d5 100644 --- a/src/infrastructure/portal-project-service.ts +++ b/src/infrastructure/portal-project-service.ts @@ -284,7 +284,11 @@ export class PortalProjectService { codeSamples: codeSamples === null ? null : codeSamples.toPosix(), contentDir: contentDirectory.toPosix(), generatedDir: projectDirectory.join(GENERATED_DIRECTORY_NAME).toPosix(), - staticDir: source.staticDirectory === null ? null : source.staticDirectory.toPosix(), + // The dev server's allow list names it, and Vite resolves a symlink before comparing. + staticDir: + source.staticDirectory === null + ? null + : new DirectoryPath(canonical(source.staticDirectory.toString())).toPosix(), downloadsDir: downloads === null ? null : downloads.toPosix() }; diff --git a/test/infrastructure/portal-project-service.test.ts b/test/infrastructure/portal-project-service.test.ts index cf744bac3..4151919a2 100644 --- a/test/infrastructure/portal-project-service.test.ts +++ b/test/infrastructure/portal-project-service.test.ts @@ -259,6 +259,20 @@ describe('PortalProjectService', () => { expect(module).to.contain("from '../../portal.identity.json'"); }); + // The dev server compares a file's real path against its allow list, which names this directory. + it('names the static directory by its real path, when the source is reached through a link', async () => { + const real = path.join(root, 'real'); + fs.mkdirSync(path.join(real, 'static'), { recursive: true }); + fs.symlinkSync(real, path.join(root, 'linked'), 'junction'); + const staticDirectory = new DirectoryPath(root).join('linked', 'static'); + + (await service.prepare(project, sourceFor({ staticDirectory }), NO_ARTIFACTS))._unsafeUnwrap(); + + expect(readConfig().staticDir).to.equal( + path.join(fs.realpathSync.native(real), 'static').split(path.sep).join('/') + ); + }); + it('reports no static directory when the project has none', async () => { (await service.prepare(project, sourceFor(), NO_ARTIFACTS))._unsafeUnwrap(); From 865ac8241e9c022ebd536a57fd2f669cbca3478a Mon Sep 17 00:00:00 2001 From: saeedjamshaid Date: Fri, 9 Oct 2026 17:13:22 +0500 Subject: [PATCH 5/7] fix(portal): read every spec file as the bundler does, and follow up the review The check read a file by its extension, and took the `$id`s a `$ref` may name from the file the `$ref` sat in, so a referenced `.json` holding YAML, or a `$ref` naming an `$id` of the root document, was refused although the portal builds it. Every file is now read as the bundler reads it, by what it holds, and one the bundler fails on or reads as nothing is refused with the reason. The `$id`s a reference may name are the root document's, as in the bundler. A document gone from `spec/` mid-save is reported as unreadable rather than left out, which had the preview warn that it was removed. `specs()` is back within Sonar's cognitive complexity limit, the watch messages say what the preview leaves out, and a test replaces the comment on the order in which `PreviewConfig.show` accepts a refusal. Co-Authored-By: Claude Opus 5.5 --- src/actions/quickstart.ts | 2 +- src/infrastructure/portal-project-service.ts | 7 +- src/prompts/portal/serve.ts | 36 +++--- src/types/portal-source-context.ts | 79 ++++++------ src/types/portal/openapi-document.ts | 116 ++++++++++------- src/types/portal/portal-source.ts | 1 - src/types/portal/preview-config.ts | 1 - src/types/portal/preview-specs.ts | 8 +- test/actions/portal/serve.test.ts | 42 +++---- test/prompts/portal/serve.test.ts | 71 ++++++++++- test/types/portal-source-context.test.ts | 125 +++++++++++++------ test/types/portal/openapi-document.test.ts | 71 +++++++---- test/types/portal/preview-config.test.ts | 8 ++ 13 files changed, 366 insertions(+), 201 deletions(-) diff --git a/src/actions/quickstart.ts b/src/actions/quickstart.ts index d7d5087da..1c620ffe7 100644 --- a/src/actions/quickstart.ts +++ b/src/actions/quickstart.ts @@ -280,7 +280,7 @@ export class QuickstartAction { if (await this.fileService.isZipFile(specPath)) { return { supported: true }; } - return OpenApiDocument.parse(specPath.name(), await this.fileService.getContents(specPath)).match( + return OpenApiDocument.parse(await this.fileService.getContents(specPath)).match( (document) => document.format(), (): SpecFormat => ({ supported: true }) ); diff --git a/src/infrastructure/portal-project-service.ts b/src/infrastructure/portal-project-service.ts index 5e8be39d5..8c3081cd1 100644 --- a/src/infrastructure/portal-project-service.ts +++ b/src/infrastructure/portal-project-service.ts @@ -2,7 +2,7 @@ import { createRequire } from 'node:module'; import path from 'node:path'; import fsExtra from 'fs-extra'; import { err, ok, Result } from 'neverthrow'; -import { DirectoryPath } from '../types/file/directoryPath.js'; +import { DirectoryPath, posix } from '../types/file/directoryPath.js'; import { FileName } from '../types/file/fileName.js'; import { FilePath } from '../types/file/filePath.js'; import { CodeSampleCatalogs } from '../types/portal/code-samples.js'; @@ -285,10 +285,7 @@ export class PortalProjectService { contentDir: contentDirectory.toPosix(), generatedDir: projectDirectory.join(GENERATED_DIRECTORY_NAME).toPosix(), // The dev server's allow list names it, and Vite resolves a symlink before comparing. - staticDir: - source.staticDirectory === null - ? null - : new DirectoryPath(canonical(source.staticDirectory.toString())).toPosix(), + staticDir: source.staticDirectory === null ? null : posix(canonical(source.staticDirectory.toString())), downloadsDir: downloads === null ? null : downloads.toPosix() }; diff --git a/src/prompts/portal/serve.ts b/src/prompts/portal/serve.ts index 7840034e0..9e79bf3d9 100644 --- a/src/prompts/portal/serve.ts +++ b/src/prompts/portal/serve.ts @@ -56,8 +56,8 @@ export class PortalServePrompts { `So does removing a language from ${f.var('languages')} (updates the SDK pages) or the ` + `${f.var('plugin')} block (removes the Context Plugin pages).`, 'Mistakes in these files are reported on save, and the preview keeps what it last accepted.', - `Saving a document in ${specPath(sourceDirectory)} updates its API reference; one with a mistake is ` + - 'reported, and left out until it is fixed.', + `Saving a document in ${specPath(sourceDirectory)} updates its API reference; a mistake is reported on ` + + 'save, and the preview leaves what it cannot read out of the reference until it is fixed.', '', `Restart the preview after adding a language or a ${f.var('plugin')} block (its SDK or plugin is ` + `fetched at startup), adding or removing a page, creating ${staticPath(sourceDirectory)}, ` + @@ -135,21 +135,19 @@ export class PortalServePrompts { } public contentNotWatched(reason: string, sourceDirectory: DirectoryPath) { - const until = reportedOnRestart(`a page or a ${f.var(NAVIGATION_FILE_NAME)}`); - this.notWatched(contentPath(sourceDirectory), reason, until); + this.notWatched(contentPath(sourceDirectory), reason, CONTENT_UNTIL_RESTART); } public contentWatchFailed(reason: string, sourceDirectory: DirectoryPath) { - const until = reportedOnRestart(`a page or a ${f.var(NAVIGATION_FILE_NAME)}`); - this.watchFailed(contentPath(sourceDirectory), reason, until); + this.watchFailed(contentPath(sourceDirectory), reason, CONTENT_UNTIL_RESTART); } /** Explained as `portal generate` would explain it, since the same rules refused it. */ public specRejected(problem: PortalSourceProblem, sourceDirectory: DirectoryPath) { reportSourceProblem(problem, sourceDirectory); log.message( - `Until ${specPath(sourceDirectory)} is fixed, the preview leaves out the API reference it cannot read; ` + - 'a build would stop here.' + `The preview leaves what it cannot read out of the API reference until ${specPath(sourceDirectory)} is ` + + 'fixed; a build would stop here.' ); } @@ -159,12 +157,16 @@ export class PortalServePrompts { // The preview lists its documents once, when it starts. public specsNeedRestart(change: SpecsChange, sourceDirectory: DirectoryPath) { - const changes = [...filesWere(change.added, 'added'), ...filesWere(change.removed, 'removed')]; + const changes = [filesWere(change.added, 'added'), filesWere(change.removed, 'removed')].filter( + (each) => each !== null + ); log.warn( `The documents in ${specPath(sourceDirectory)} are not the ones the preview started with: ` + `${changes.join(', and ')}.` ); - log.message('Restart the preview to show them; until then it shows those it started with that are still there.'); + log.message( + 'Restart the preview to match them; until then it shows the ones it started with that are still there.' + ); } public specNotChecked(reason: string, sourceDirectory: DirectoryPath) { @@ -172,11 +174,11 @@ export class PortalServePrompts { } public specNotWatched(reason: string, sourceDirectory: DirectoryPath) { - this.notWatched(specPath(sourceDirectory), reason, reportedOnRestart('a specification')); + this.notWatched(specPath(sourceDirectory), reason, SPEC_UNTIL_RESTART); } public specWatchFailed(reason: string, sourceDirectory: DirectoryPath) { - this.watchFailed(specPath(sourceDirectory), reason, reportedOnRestart('a specification')); + this.watchFailed(specPath(sourceDirectory), reason, SPEC_UNTIL_RESTART); } public stopping() { @@ -215,9 +217,13 @@ export class PortalServePrompts { const reportedOnRestart = (mistakes: string) => `a mistake in ${mistakes} is only reported when the preview is restarted`; +const CONTENT_UNTIL_RESTART = reportedOnRestart(`a page or a ${f.var(NAVIGATION_FILE_NAME)}`); +const SPEC_UNTIL_RESTART = reportedOnRestart('a specification'); -// `'a.yaml' was added`, or `'a.yaml' and 'b.yaml' were removed`; nothing for no files. -function filesWere(files: FilePath[], done: string): string[] { +function filesWere(files: FilePath[], done: string): string | null { + if (files.length === 0) { + return null; + } const names = listedInProse(files.map((file) => f.var(file.name().toString()))); - return files.length === 0 ? [] : [`${names} ${files.length === 1 ? 'was' : 'were'} ${done}`]; + return `${names} ${files.length === 1 ? 'was' : 'were'} ${done}`; } diff --git a/src/types/portal-source-context.ts b/src/types/portal-source-context.ts index e6e08095a..00766bc47 100644 --- a/src/types/portal-source-context.ts +++ b/src/types/portal-source-context.ts @@ -414,8 +414,10 @@ export class PortalSourceContext { if (await this.fileService.isZipFile(specPath)) { return PLACEHOLDER_SITE; } - const document = await this.readDocument(specPath); - return document?.isOk() ? document.value.suggestedSite() : PLACEHOLDER_SITE; + return (await this.readDocument(specPath, null)).match( + (document) => document.suggestedSite(), + () => PLACEHOLDER_SITE + ); } /** @@ -448,13 +450,9 @@ export class PortalSourceContext { const documentNames = fileNames.filter((name) => SPEC_EXTENSIONS.some((extension) => name.hasExtension(extension))); for (const fileName of documentNames) { const file = new FilePath(this.specDirectory, fileName); - const read = await this.readDocument(file); - // Gone since it was listed, as an editor that saves by renaming over the file leaves it for a moment. - if (read === undefined) { - continue; - } + const read = await this.readDocument(file, null); if (read.isErr()) { - return err({ kind: 'unreadableSpec', file, referencedBy: null, reason: read.error }); + return err(read.error); } const document = read.value; const format = document.format(); @@ -511,62 +509,65 @@ export class PortalSourceContext { }; } - // One the bundler cannot read fails the build, or leaves its reference empty, so it is refused. - private async referencedDocuments(spec: SpecFile): Promise> { - const named = new Set([spec.file.toString()]); - const queue: { - file: FilePath; - referencedBy: FilePath; - read: Promise | undefined>; - }[] = []; + /** + * Every file `spec` is bundled from, by its path. One the bundler cannot read fails the build, + * or leaves its reference empty, so it is refused. + */ + private async referencedDocuments( + spec: SpecFile + ): Promise, PortalSourceProblem>> { + const rootIds = spec.document.schemaIds(); + const documents = new Map([[spec.file.toString(), spec.document]]); + const named = new Set(documents.keys()); + const queue: { file: FilePath; read: Promise> }[] = []; // Each file is read once, starting when it is first named, as a split specification runs to thousands. const enqueue = ({ file: referencedBy, document }: SpecFile) => { - for (const file of document.referencedFiles(referencedBy.directory())) { + for (const file of document.referencedFiles(referencedBy.directory(), rootIds)) { if (!named.has(file.toString())) { named.add(file.toString()); - queue.push({ file, referencedBy, read: this.readDocument(file) }); + queue.push({ file, read: this.readDocument(file, referencedBy) }); } } }; - const walked = [spec]; enqueue(spec); // Grows as it is walked: each file read queues the ones it refers to. - for (const { file, referencedBy, read } of queue) { - const parsed = await read; - if (parsed === undefined) { - return err({ kind: 'missingSpecReference', file, referencedBy }); - } - if (parsed.isErr()) { - return err({ kind: 'unreadableSpec', file, referencedBy, reason: parsed.error }); + for (const { file, read } of queue) { + const referenced = await read; + if (referenced.isErr()) { + return err(referenced.error); } - const referenced = { file, document: parsed.value }; - walked.push(referenced); - enqueue(referenced); + documents.set(file.toString(), referenced.value); + enqueue({ file, document: referenced.value }); } - return ok(walked); + return ok(documents); } // A path item in another file is read from it; one that file refers on to again is not followed. - private static endpoints(spec: SpecFile, referenced: SpecFile[]): Endpoint[] { + private static endpoints(spec: SpecFile, documents: Map): Endpoint[] { const mounted = spec.document .pathItemReferences(spec.file.directory()) - .flatMap( - ({ path, file, pointer }) => - referenced.find((each) => each.file.isEqual(file))?.document.endpointsAt(path, pointer) ?? [] - ); + .flatMap(({ path, file, pointer }) => documents.get(file.toString())?.endpointsAt(path, pointer) ?? []); return [...spec.document.endpoints(), ...mounted]; } - /** Undefined when there is no such file; otherwise the document, or why it cannot be read. */ - private async readDocument(file: FilePath): Promise | undefined> { + private async readDocument( + file: FilePath, + referencedBy: FilePath | null + ): Promise> { let contents: string; try { contents = await this.fileService.getContents(file); } catch (error) { - return (await this.fileService.fileExists(file)) ? err(errorMessage(error)) : undefined; + if (referencedBy !== null && !(await this.fileService.fileExists(file))) { + return err({ kind: 'missingSpecReference', file, referencedBy }); + } + // Also a listed document gone mid-save, which the next save fixes, as it would a half-written one. + return err({ kind: 'unreadableSpec', file, referencedBy, reason: errorMessage(error) }); } - return OpenApiDocument.parse(file.name(), contents); + return OpenApiDocument.parse(contents).mapErr( + (reason): PortalSourceProblem => ({ kind: 'unreadableSpec', file, referencedBy, reason }) + ); } private uniqueSlug(fileName: FileName, used: Set): string { diff --git a/src/types/portal/openapi-document.ts b/src/types/portal/openapi-document.ts index e6410ab98..fd8e7382e 100644 --- a/src/types/portal/openapi-document.ts +++ b/src/types/portal/openapi-document.ts @@ -1,7 +1,6 @@ import { err, ok, Result } from 'neverthrow'; import { parse as parseYaml, YAMLParseError } from 'yaml'; import { DirectoryPath } from '../file/directoryPath.js'; -import { FileName } from '../file/fileName.js'; import { FilePath } from '../file/filePath.js'; import { errorMessage } from '../../utils/error-utils.js'; import { isJsonObject, JsonObject } from '../../utils/json-utils.js'; @@ -29,41 +28,75 @@ const HTTP_METHODS = new Set(['get', 'put', 'post', 'delete', 'options', 'head', const URL_SCHEME = /^[a-z][a-z\d+.-]+:/i; -// The options @scalar/json-magic bundles with, so a spec reads here as it does in the portal. +// How @scalar/json-magic reads every file it bundles: JSON, else YAML, with these options, only +// when the text holds a key and does not open as JSON does. const YAML_OPTIONS = { merge: true, maxAliasCount: 10000 }; +const YAML_KEY = /^[^:]+:/; +const JSON_OPENING = /^\s*[[{]/; // The keys @scalar/json-magic writes what it has embedded under, which it does not read again. const BUNDLER_KEYS = new Set(['x-ext', 'x-ext-urls']); /** A specification as written to disk, read the one way the wizard and the build agree on. */ export class OpenApiDocument { - private constructor(private readonly document: JsonObject) {} + private constructor(private readonly value: unknown) {} /** - * Fails with what the parser said when it does not accept the text. A document that parses to - * something other than an object is kept, and reports itself as no specification at all. + * Fails with the reason where the portal's bundler fails, or reads nothing. A document that + * reads as something other than an object is kept, and reports itself as no specification at all. */ - public static parse(fileName: FileName, contents: string): Result { + public static parse(contents: string): Result { + const text = stripByteOrderMark(contents); + if (text.trim() === '') { + return err('it is empty'); + } + let value: unknown; try { - // JSON is valid YAML, but the YAML parser is far slower and specs run to megabytes, - // so each extension gets the parser built for it. - const text = stripByteOrderMark(contents); - const document: unknown = fileName.hasExtension('.json') ? JSON.parse(text) : parseYaml(text, YAML_OPTIONS); - return ok(new OpenApiDocument(isJsonObject(document) ? document : {})); + value = JSON.parse(text); } catch (error) { - // A YAML error goes on to quote the lines around it, and V8 quotes the JSON it stopped at across line breaks. - const message = errorMessage(error); - const reason = error instanceof YAMLParseError ? message.split('\n')[0].replace(/:$/, '') : message; - return err(reason.replace(/\s+/g, ' ').trim()); + // Only the first 50 characters, as the bundler looks no further. + if (JSON_OPENING.test(text.slice(0, 50))) { + return err(parseFailure(error)); + } + if (!YAML_KEY.test(text)) { + return err('it is not JSON, and the portal reads YAML only when it holds a key'); + } + try { + value = parseYaml(text, YAML_OPTIONS); + } catch (yamlError) { + return err(parseFailure(yamlError)); + } } + return value === null ? err('it holds no value') : ok(new OpenApiDocument(value)); } - /** The local files this document's `$ref`s name, as the portal's bundler finds them, except by `$id`. */ - public referencedFiles(directory: DirectoryPath): FilePath[] { - const ids = idsIn(this.document, new WeakSet(), new Set()); - const references = referencesIn(this.document, new WeakSet(), new Set()); - const names = new Set([...references].map((reference) => reference.split('#')[0])); - return [...names].filter((name) => !ids.has(name)).flatMap((name) => localFile(directory, name) ?? []); + /** The `$id`s this document declares, which a `$ref` in any file bundled into it names a schema by. */ + public schemaIds(): Set { + const ids = new Set(); + walk(this.value, (node) => { + if (isJsonObject(node) && typeof node.$id === 'string') { + ids.add(node.$id); + } + return true; + }); + return ids; + } + + /** The local files this document's `$ref`s name, as the portal's bundler finds them, given the root's `$id`s. */ + public referencedFiles(directory: DirectoryPath, rootIds: ReadonlySet): FilePath[] { + const names = new Set(); + // Not below an `$id`: the bundler resolves what is under one against the `$id`, not the file. + walk(this.value, (node) => { + if (isJsonObject(node) && typeof node.$id === 'string') { + return false; + } + if (isJsonObject(node) && typeof node.$ref === 'string') { + names.add(node.$ref.split('#')[0]); + return false; + } + return true; + }); + return [...names].filter((name) => !rootIds.has(name)).flatMap((name) => localFile(directory, name) ?? []); } /** Every operation declared inline or behind a `$ref` into this document; see `pathItemReferences` for the rest. */ @@ -113,6 +146,10 @@ export class OpenApiDocument { return { name, description: description === null ? null : cap(description, DESCRIPTION_LIMIT) }; } + private get document(): JsonObject { + return isJsonObject(this.value) ? this.value : {}; + } + private paths(): JsonObject { return isJsonObject(this.document.paths) ? this.document.paths : {}; } @@ -142,38 +179,27 @@ function localFile(directory: DirectoryPath, name: string): FilePath | undefined return name === '' || URL_SCHEME.test(name) ? undefined : FilePath.resolve(directory, name.replaceAll('\\', '/')); } -// Both walks add to `found` rather than return arrays to merge, as a specification runs to megabytes. -function referencesIn(node: unknown, seen: WeakSet, found: Set): Set { +// A YAML error goes on to quote the lines around it, and V8 quotes the JSON it stopped at across line breaks. +function parseFailure(error: unknown): string { + const message = errorMessage(error); + const reason = error instanceof YAMLParseError ? message.split('\n')[0].replace(/:$/, '') : message; + return reason.replace(/\s+/g, ' ').trim(); +} + +/** Each object and list once, going into one only when `visit` says so, and never into the keys the bundler writes. */ +function walk(node: unknown, visit: (node: object) => boolean, seen = new WeakSet()): void { if (typeof node !== 'object' || node === null || seen.has(node)) { - return found; + return; } seen.add(node); - if (isJsonObject(node) && typeof node.$id === 'string') { - return found; - } - if (isJsonObject(node) && typeof node.$ref === 'string') { - return found.add(node.$ref); + if (!visit(node)) { + return; } for (const [key, value] of Object.entries(node)) { if (!BUNDLER_KEYS.has(key)) { - referencesIn(value, seen, found); + walk(value, visit, seen); } } - return found; -} - -function idsIn(node: unknown, seen: WeakSet, found: Set): Set { - if (typeof node !== 'object' || node === null || seen.has(node)) { - return found; - } - seen.add(node); - if (isJsonObject(node) && typeof node.$id === 'string') { - found.add(node.$id); - } - for (const value of Object.values(node)) { - idsIn(value, seen, found); - } - return found; } function valueAt(document: JsonObject, pointer: string): unknown { diff --git a/src/types/portal/portal-source.ts b/src/types/portal/portal-source.ts index afcb77d02..83e4498e3 100644 --- a/src/types/portal/portal-source.ts +++ b/src/types/portal/portal-source.ts @@ -115,7 +115,6 @@ export type PortalSourceProblem = | { kind: 'invalidConfig'; errors: string[]; missingPortal: boolean } // Every problem found in `content/`, so that one run lists all that a build would refuse. | { kind: 'invalidContent'; problems: ContentProblem[] } - // `referencedBy`: the file whose `$ref` names it; null for a document at the top of `spec/`. | { kind: 'unreadableSpec'; file: FilePath; referencedBy: FilePath | null; reason: string } | { kind: 'missingSpecReference'; file: FilePath; referencedBy: FilePath } // Its `folders` are not read, and `api transform` writes into one of its own. diff --git a/src/types/portal/preview-config.ts b/src/types/portal/preview-config.ts index ece9fd68c..27fd6eb32 100644 --- a/src/types/portal/preview-config.ts +++ b/src/types/portal/preview-config.ts @@ -28,7 +28,6 @@ export class PreviewConfig { * it is accepted again. */ public show(config: PortalConfig, written: boolean): boolean { - // Not after `written ||`, which would skip it, and so say it is fixed again after the next save. const fixed = this.refusal.accept(); this.shown = config; return written || fixed; diff --git a/src/types/portal/preview-specs.ts b/src/types/portal/preview-specs.ts index 46e766362..bde0092be 100644 --- a/src/types/portal/preview-specs.ts +++ b/src/types/portal/preview-specs.ts @@ -11,13 +11,13 @@ export interface SpecsChange { /** The preview serves only the documents it started with, so a change to which are in `spec/` is said once. */ export class PreviewSpecs { private readonly served: FilePath[]; - private said: FilePath[]; + private shown: FilePath[]; private readonly refusal = new PreviewRefusal(); /** `startup`: the documents the preview started with. */ constructor(startup: PortalSpec[]) { this.served = startup.map(({ file }) => file); - this.said = this.served; + this.shown = this.served; } public refuse(): void { @@ -27,8 +27,8 @@ export class PreviewSpecs { /** Records the documents as shown: whether `spec/` is fixed, having been refused, and a change not yet warned of. */ public show(specs: PortalSpec[]): { fixed: boolean; change: SpecsChange | null } { const files = specs.map(({ file }) => file); - const unchanged = isSameSet(files, this.said) || isSameSet(files, this.served); - this.said = files; + const unchanged = isSameSet(files, this.shown) || isSameSet(files, this.served); + this.shown = files; const change = { added: notIn(files, this.served), removed: notIn(this.served, files) }; return { fixed: this.refusal.accept(), change: unchanged ? null : change }; } diff --git a/test/actions/portal/serve.test.ts b/test/actions/portal/serve.test.ts index d717231bd..229087dd3 100644 --- a/test/actions/portal/serve.test.ts +++ b/test/actions/portal/serve.test.ts @@ -52,6 +52,20 @@ describe('PortalServeAction', () => { openInBrowser ); + /** Stands in for the watch on `directory`, answered by `watching`; any other tree's watch is left alone. */ + const watchTreeAt = (directory: DirectoryPath, watching: { close: sinon.SinonStub; recheck: sinon.SinonStub }) => + new Promise<{ onChange: () => Promise; onFailed: (reason: string) => void }>((resolve) => { + watchTree.callsFake( + (watched: DirectoryPath, onChange: () => Promise, onFailed: (reason: string) => void) => { + if (!watched.isEqual(directory)) { + return ok({ close: sinon.stub().resolves(), recheck: sinon.stub() }); + } + resolve({ onChange, onFailed }); + return ok(watching); + } + ); + }); + beforeEach(() => { root = fs.mkdtempSync(path.join(os.tmpdir(), 'portal-serve-')); @@ -227,18 +241,7 @@ describe('PortalServeAction', () => { closeWatch = sinon.stub().resolves(); recheck = sinon.stub(); applyContent = sinon.stub(PortalProjectService.prototype, 'applyContent').resolves(ok(undefined)); - watched = new Promise((resolve) => { - watchTree.callsFake( - (directory: DirectoryPath, onChange: () => Promise, onFailed: (reason: string) => void) => { - // `spec/` is watched as well, and these tests leave its watch alone. - if (!directory.isEqual(source.join('content'))) { - return ok({ close: sinon.stub().resolves(), recheck: sinon.stub() }); - } - resolve({ onChange, onFailed }); - return ok({ close: closeWatch, recheck }); - } - ); - }); + watched = watchTreeAt(source.join('content'), { close: closeWatch, recheck }); }); // The preview itself turns every page into an HTTP 500 and says nothing. @@ -454,18 +457,7 @@ describe('PortalServeAction', () => { closeWatch = sinon.stub().resolves(); recheck = sinon.stub(); - watched = new Promise((resolve) => { - watchTree.callsFake( - (directory: DirectoryPath, onChange: () => Promise, onFailed: (reason: string) => void) => { - // `content/` is watched as well, and these tests leave its watch alone. - if (!directory.isEqual(source.join('spec'))) { - return ok({ close: sinon.stub().resolves(), recheck: sinon.stub() }); - } - resolve({ onChange, onFailed }); - return ok({ close: closeWatch, recheck }); - } - ); - }); + watched = watchTreeAt(source.join('spec'), { close: closeWatch, recheck }); }); // The preview itself leaves the document's reference out, and says nothing. @@ -535,7 +527,7 @@ describe('PortalServeAction', () => { }); // An editor's swap file would only have the specifications checked again for nothing. - it('leaves out of the watch what the build never reads', async () => { + it("leaves dot files, as an editor's swap file is, and node_modules out of the watch", async () => { await whileServing(async () => { const call = watchTree.getCalls().find(({ args: [directory] }) => directory.isEqual(source.join('spec'))); const isIgnored: (name: string) => boolean = call?.args[3]; diff --git a/test/prompts/portal/serve.test.ts b/test/prompts/portal/serve.test.ts index 2eebb9077..dfb614a1a 100644 --- a/test/prompts/portal/serve.test.ts +++ b/test/prompts/portal/serve.test.ts @@ -7,6 +7,7 @@ import { DirectoryPath } from '../../../src/types/file/directoryPath.js'; import { FileName } from '../../../src/types/file/fileName.js'; import { FilePath } from '../../../src/types/file/filePath.js'; import { UrlPath } from '../../../src/types/file/urlPath.js'; +import { PortalSourceProblem } from '../../../src/types/portal/portal-source.js'; import { Language } from '../../../src/types/sdk/generate.js'; describe('PortalServePrompts', () => { @@ -52,13 +53,75 @@ describe('PortalServePrompts', () => { ); }); - it('says a saved specification updates its reference, and one with a mistake is reported and left out', () => { + it('says a saved specification updates its reference, and what it cannot read is reported and left out', () => { expect(printed()).to.match( - /Saving a document in '.*spec' updates its API reference; one with a mistake is reported, and left out until it is fixed\./ + /Saving a document in '.*spec' updates its API reference; a mistake is reported on save, and the preview leaves what it cannot read out of the reference until it is fixed\./ ); }); }); + /** The last line `log[level]` printed for `say`, without its colours. */ + const lastLine = (level: 'warn' | 'message', say: (prompts: PortalServePrompts) => void) => { + const stubs = { + warn: sinon.stub(log, 'warn'), + message: sinon.stub(log, 'message'), + error: sinon.stub(log, 'error') + }; + say(new PortalServePrompts()); + return stripVTControlCharacters(String(stubs[level].lastCall.args[0])); + }; + + // Content keeps what it last accepted; a specification has no earlier version to keep. + it('says a refused specification leaves what it cannot read out of the reference, and would stop a build', () => { + const problem: PortalSourceProblem = { kind: 'emptySpecDirectory', folders: [] }; + + expect(lastLine('message', (prompts) => prompts.specRejected(problem, new DirectoryPath('src')))).to.match( + /^The preview leaves what it cannot read out of the API reference until '.*spec' is fixed; a build would stop here\.$/ + ); + }); + + describe('a watch that cannot start, or stops', () => { + const source = new DirectoryPath('src'); + const cases: [string, (prompts: PortalServePrompts) => void, RegExp][] = [ + [ + 'apimatic.json, at the start', + (prompts) => prompts.configNotWatched('EMFILE'), + /^'apimatic\.json' cannot be watched \(EMFILE\), so edits to it need the preview restarted\.$/ + ], + [ + 'apimatic.json, while serving', + (prompts) => prompts.configWatchFailed('EPERM'), + /^'apimatic\.json' is no longer watched \(EPERM\), so further edits to it need the preview restarted\.$/ + ], + [ + 'the content, at the start', + (prompts) => prompts.contentNotWatched('EMFILE', source), + /^'.*content' cannot be watched \(EMFILE\), so a mistake in a page or a 'nav\.json' is only reported when the preview is restarted\.$/ + ], + [ + 'the content, while serving', + (prompts) => prompts.contentWatchFailed('EPERM', source), + /^'.*content' is no longer watched \(EPERM\), so a mistake in a page or a 'nav\.json' is only reported when the preview is restarted\.$/ + ], + [ + 'the specifications, at the start', + (prompts) => prompts.specNotWatched('EMFILE', source), + /^'.*spec' cannot be watched \(EMFILE\), so a mistake in a specification is only reported when the preview is restarted\.$/ + ], + [ + 'the specifications, while serving', + (prompts) => prompts.specWatchFailed('EPERM', source), + /^'.*spec' is no longer watched \(EPERM\), so a mistake in a specification is only reported when the preview is restarted\.$/ + ] + ]; + + cases.forEach(([label, say, expected]) => { + it(`says what is no longer reported on save for ${label}`, () => { + expect(lastLine('warn', say)).to.match(expected); + }); + }); + }); + describe('a change to which documents are in the spec directory', () => { const document = (name: string) => new FilePath(new DirectoryPath('src', 'spec'), new FileName(name)); @@ -77,14 +140,14 @@ describe('PortalServePrompts', () => { } }; - it('names each document added and removed, and that a restart shows them', () => { + it('names each document added and removed, and that a restart matches them', () => { const [warning, advice] = said(['orders.json'], ['pets.json']); expect(warning).to.match( /^The documents in '.*spec' are not the ones the preview started with: 'orders\.json' was added, and 'pets\.json' was removed\.$/ ); expect(advice).to.equal( - 'Restart the preview to show them; until then it shows those it started with that are still there.' + 'Restart the preview to match them; until then it shows the ones it started with that are still there.' ); }); diff --git a/test/types/portal-source-context.test.ts b/test/types/portal-source-context.test.ts index 775a3d08a..ea517f524 100644 --- a/test/types/portal-source-context.test.ts +++ b/test/types/portal-source-context.test.ts @@ -507,6 +507,39 @@ describe('PortalSourceContext', () => { expect(problem.kind === 'unreadableSpec' && problem.reason).to.contain('JSON'); }); + // As the portal's bundler reads it: by what it holds. + it('reads a document whatever its extension says', async () => { + write('spec/api.json', 'openapi: 3.0.0\ninfo:\n title: Calc\n version: "1"\npaths: {}\n'); + + expect((await resolve())._unsafeUnwrap().specs.map((spec) => spec.slug)).to.deep.equal(['api']); + }); + + it('reports a document left empty, as it does one half written', async () => { + write('spec/api.json', OPENAPI); + write('spec/orders.yaml', ''); + + expect(specProblem((await resolve())._unsafeUnwrapErr())).to.deep.equal([ + 'unreadableSpec', + 'spec/orders.yaml', + null + ]); + }); + + // As an editor that saves by renaming over the file leaves it for a moment. + it('reports a document gone since it was listed as one it cannot read, rather than leaving it out', async () => { + write('spec/api.json', OPENAPI); + sinon.stub(FileService.prototype, 'getContents').rejects(new Error('ENOENT: no such file or directory')); + sinon.stub(FileService.prototype, 'fileExists').resolves(false); + + try { + const problem = (await new PortalSourceContext(new DirectoryPath(root)).resolveSpecs())._unsafeUnwrapErr(); + + expect(specProblem(problem)).to.deep.equal(['unreadableSpec', 'spec/api.json', null]); + } finally { + sinon.restore(); + } + }); + it('reports an empty or absent spec directory', async () => { expect((await resolve())._unsafeUnwrapErr()).to.deep.equal({ kind: 'emptySpecDirectory', folders: [] }); @@ -642,28 +675,40 @@ describe('PortalSourceContext', () => { () => false ); - const cases: [string, () => void, boolean][] = [ - [ - 'a file that is there', - () => { - writeSpec({ Pet: { $ref: './schemas/Pet.yaml' } }); - write('spec/schemas/Pet.yaml', 'type: object\n'); - }, - true - ], - ['a file that is not there', () => writeSpec({ Pet: { $ref: './schemas/Pet.yaml' } }), false], + /** `spec/api.json`, whose schema `Pet` is the file `schemas/` holding `contents`. */ + const writeReferenced = (name: string, contents: string) => { + writeSpec({ Pet: { $ref: `./schemas/${name}` } }); + write(`spec/schemas/${name}`, contents); + }; + + // `empty`: the bundler builds, with the reference pointing at nothing, so the page renders without it. + const BUNDLED = { + builds: 'as the bundler builds it', + fails: 'as the bundler fails on it', + empty: 'which the bundler reads as nothing' + }; + + const cases: [string, () => void, keyof typeof BUNDLED][] = [ + ['a file that is there', () => writeReferenced('Pet.yaml', 'type: object\n'), 'builds'], + ['a file that is not there', () => writeSpec({ Pet: { $ref: './schemas/Pet.yaml' } }), 'fails'], + ['a file of half-written YAML', () => writeReferenced('Pet.yaml', 'type: object\nrequired: [name\n'), 'fails'], + ['a file of half-written JSON', () => writeReferenced('Pet.json', '{ "type": "obj'), 'empty'], + ['a JSON file holding YAML', () => writeReferenced('Pet.json', 'type: object\n'), 'builds'], + ['a YAML file that opens as JSON does', () => writeReferenced('Pet.yaml', '{ type: object }\n'), 'empty'], + ['a file left empty', () => writeReferenced('Pet.yaml', ''), 'empty'], + ['a file holding only a comment', () => writeReferenced('Pet.yaml', '# type: object\n'), 'empty'], [ - 'a file of half-written YAML', + 'a file named in a file that is a list', () => { - writeSpec({ Pet: { $ref: './schemas/Pet.yaml' } }); - write('spec/schemas/Pet.yaml', 'type: object\nrequired: [name\n'); + writeSpec({ Pet: { allOf: { $ref: './schemas/parts.yaml' } } }); + write('spec/schemas/parts.yaml', '- $ref: ./Base.yaml\n'); }, - false + 'fails' ], [ 'a file named in example data', () => writeSpec({ Pet: { type: 'object', example: { $ref: './examples/pet.json' } } }), - false + 'fails' ], [ 'a file named beside a reference, which is not read', @@ -671,7 +716,7 @@ describe('PortalSourceContext', () => { writeSpec({ Pet: { $ref: './Pet.yaml', properties: { owner: { $ref: './Owner.yaml' } } } }); write('spec/Pet.yaml', 'type: object\n'); }, - true + 'builds' ], [ 'two files that refer to each other', @@ -680,16 +725,15 @@ describe('PortalSourceContext', () => { write('spec/Pet.yaml', 'properties:\n owner:\n $ref: ./Owner.yaml\n'); write('spec/Owner.yaml', 'properties:\n pet:\n $ref: ./Pet.yaml\n'); }, - true + 'builds' ], [ 'a file named from the folder of the file that refers to it', () => { - writeSpec({ Pet: { $ref: './schemas/Pet.yaml' } }); - write('spec/schemas/Pet.yaml', 'properties:\n owner:\n $ref: ./Owner.yaml\n'); + writeReferenced('Pet.yaml', 'properties:\n owner:\n $ref: ./Owner.yaml\n'); write('spec/schemas/Owner.yaml', 'type: object\n'); }, - true + 'builds' ], [ 'a file named in a part of a file that no reference points into', @@ -697,7 +741,7 @@ describe('PortalSourceContext', () => { writeSpec({ Pet: { $ref: './schemas/models.yaml#/Pet' } }); write('spec/schemas/models.yaml', 'Pet:\n type: object\nOwner:\n $ref: ./Owner.yaml\n'); }, - false + 'fails' ], [ 'a file outside the spec directory', @@ -705,7 +749,7 @@ describe('PortalSourceContext', () => { writeSpec({ Pet: { $ref: '../shared/Pet.yaml' } }); write('shared/Pet.yaml', 'type: object\n'); }, - true + 'builds' ], [ 'a name written with a percent escape, which is not decoded', @@ -713,40 +757,43 @@ describe('PortalSourceContext', () => { writeSpec({ Pet: { $ref: './my%20pet.yaml' } }); write('spec/my pet.yaml', 'type: object\n'); }, - false + 'fails' ], [ 'a schema named by its $id', () => writeSpec({ Pet: { $id: 'pet.json', type: 'object' }, Pets: { items: { $ref: 'pet.json' } } }), - true + 'builds' + ], + [ + 'a schema named by its $id from another file', + () => { + writeSpec({ Pet: { $id: 'pet.json', type: 'object' }, Owner: { $ref: './schemas/Owner.yaml' } }); + write('spec/schemas/Owner.yaml', 'properties:\n pet:\n $ref: pet.json\n'); + }, + 'builds' ], [ 'a file named only under the keys the bundler writes', () => writeSpec({}, { 'x-ext': { abc123: { $ref: './embedded.yaml' } } }), - true + 'builds' ] ]; - cases.forEach(([label, arrange, builds]) => { - it(`${builds ? 'accepts' : 'refuses'} ${label}, as the bundler does`, async () => { + cases.forEach(([label, arrange, bundled]) => { + it(`${bundled === 'builds' ? 'accepts' : 'refuses'} ${label}, ${BUNDLED[bundled]}`, async () => { arrange(); - expect(await bundles(), 'the bundler').to.equal(builds); - expect((await resolve()).isOk(), 'the check').to.equal(builds); + expect(await bundles(), 'the bundler').to.equal(bundled !== 'fails'); + expect((await resolve()).isOk(), 'the check').to.equal(bundled === 'builds'); }); }); - // The bundler leaves the reference pointing at nothing, and the page renders without it. - it('refuses a referenced JSON file it reads as nothing, where the bundler goes on without it', async () => { - writeSpec({ Pet: { $ref: './schemas/Pet.json' } }); - write('spec/schemas/Pet.json', '{ "type": "obj'); + // Resolved against the $id, which names no folder of the specification, so the check leaves it to the build. + it('accepts a file named below an $id, which the bundler fails on', async () => { + writeSpec({ Pet: { $id: 'pet.json', properties: { tag: { $ref: './tag.yaml' } } } }); - expect(await bundles(), 'the bundler').to.be.true; - expect(specProblem((await resolve())._unsafeUnwrapErr())).to.deep.equal([ - 'unreadableSpec', - 'spec/schemas/Pet.json', - 'spec/api.json' - ]); + expect(await bundles(), 'the bundler').to.be.false; + expect((await resolve()).isOk(), 'the check').to.be.true; }); }); }); diff --git a/test/types/portal/openapi-document.test.ts b/test/types/portal/openapi-document.test.ts index f20a886d6..b985cf9df 100644 --- a/test/types/portal/openapi-document.test.ts +++ b/test/types/portal/openapi-document.test.ts @@ -1,40 +1,48 @@ import { expect } from 'chai'; import path from 'path'; import { OpenApiDocument, SpecFormat } from '../../../src/types/portal/openapi-document'; -import { FileName } from '../../../src/types/file/fileName'; import { DirectoryPath } from '../../../src/types/file/directoryPath'; -const JSON_FILE = new FileName('spec.json'); -const YAML_FILE = new FileName('spec.yaml'); const BYTE_ORDER_MARK = ''; describe('OpenApiDocument', () => { - const read = (contents: string, fileName = JSON_FILE): OpenApiDocument => { - const document = OpenApiDocument.parse(fileName, contents); + const read = (contents: string): OpenApiDocument => { + const document = OpenApiDocument.parse(contents); expect(document.isOk(), `unparsable: ${contents}`).to.be.true; return document._unsafeUnwrap(); }; const readJson = (document: unknown, prefix = '') => read(prefix + JSON.stringify(document)); const withInfo = (info: Record) => readJson({ openapi: '3.0.0', info, paths: {} }); + const refused = (contents: string) => OpenApiDocument.parse(contents)._unsafeUnwrapErr(); describe('parse', () => { - it('reads JSON by the .json extension and YAML by any other', () => { + it('reads JSON, and YAML, by what the text holds', () => { expect(readJson({ openapi: '3.1.0' }).format()).to.deep.equal({ supported: true }); - expect(read('openapi: 3.1.0\n', YAML_FILE).format()).to.deep.equal({ supported: true }); + expect(read('openapi: 3.1.0\n').format()).to.deep.equal({ supported: true }); }); // Matched loosely: the words are the parser's own, and change with its version. it('fails with what the parser said, on one line, saying where it stopped', () => { - const yaml = OpenApiDocument.parse(YAML_FILE, 'openapi: 3.0.0\ninfo:\n title: [unclosed\n')._unsafeUnwrapErr(); - const json = OpenApiDocument.parse(JSON_FILE, '{\n "a": 1\n "b": 2\n}')._unsafeUnwrapErr(); - const quoting = OpenApiDocument.parse(JSON_FILE, '[1,\n2,\n,3]')._unsafeUnwrapErr(); + const yaml = refused('openapi: 3.0.0\ninfo:\n title: [unclosed\n'); + const json = refused('{\n "a": 1\n "b": 2\n}'); + const quoting = refused('[1,\n2,\n,3]'); expect(yaml).to.match(/at line 4, column 1$/); expect(json).to.contain('line 3 column 3'); [yaml, json, quoting].forEach((reason) => expect(reason).to.not.contain('\n')); }); + // The portal's bundler reads these as nothing, so a reference to one would be empty. + it('fails on text the bundler reads as nothing, saying why', () => { + expect(refused(' \n')).to.equal('it is empty'); + expect(refused('# only a comment: here\n')).to.equal('it holds no value'); + expect(refused('{ type: object }')).to.contain('JSON'); + expect(refused('- active\n- inactive\n')).to.equal( + 'it is not JSON, and the portal reads YAML only when it holds a key' + ); + }); + it('treats a document that is not an object as no specification', () => { expect(readJson([1, 2]).format()).to.deep.equal({ supported: false, format: null }); expect(readJson('text').format()).to.deep.equal({ supported: false, format: null }); @@ -42,7 +50,7 @@ describe('OpenApiDocument', () => { it('reads a document written with a byte-order mark', () => { expect(readJson({ openapi: '3.0.0' }, BYTE_ORDER_MARK).format()).to.deep.equal({ supported: true }); - expect(read(`${BYTE_ORDER_MARK}openapi: 3.0.0\n`, YAML_FILE).format()).to.deep.equal({ supported: true }); + expect(read(`${BYTE_ORDER_MARK}openapi: 3.0.0\n`).format()).to.deep.equal({ supported: true }); }); }); @@ -143,7 +151,7 @@ describe('OpenApiDocument', () => { ...aliases ].join('\n'); - expect(read(yaml, YAML_FILE).endpoints().map(String)).to.deep.equal(['GET /merged']); + expect(read(yaml).endpoints().map(String)).to.deep.equal(['GET /merged']); }); it('lists the inline operations of every path, leaving a path item in another file to its reference', () => { @@ -203,10 +211,18 @@ describe('OpenApiDocument', () => { }); }); + describe('schemaIds', () => { + it('names each $id the document declares, however deep', () => { + const document = readJson({ Pet: { $id: 'pet.json', properties: { tag: { $id: 'tag.json' } } } }); + + expect([...document.schemaIds()]).to.deep.equal(['pet.json', 'tag.json']); + }); + }); + describe('referencedFiles', () => { const specDirectory = new DirectoryPath('/project/src/spec'); - const referenced = (document: Record) => - readJson(document).referencedFiles(specDirectory).map(String); + const referenced = (document: unknown, rootIds = new Set()) => + readJson(document).referencedFiles(specDirectory, rootIds).map(String); const inSpec = (file: string) => path.resolve('/project/src/spec', file); it('finds a $ref on any object, in data, extensions and lists too, against the folder of the document', () => { @@ -245,13 +261,24 @@ describe('OpenApiDocument', () => { expect(files).to.be.empty; }); - it('leaves out a reference below an $id, or naming one, which the bundler resolves against the $id', () => { - const files = referenced({ - Pet: { $id: 'pet.json', properties: { tag: { $ref: './tag.yaml' } } }, - owner: { $ref: 'pet.json#/properties/tag' } - }); + it('reads no further below an $id, against which the bundler resolves what is under it', () => { + expect(referenced({ Pet: { $id: 'pet.json', properties: { tag: { $ref: './tag.yaml' } } } })).to.be.empty; + }); - expect(files).to.be.empty; + // The bundler takes the $ids of the document it starts from, and only those, as names of schemas. + it('leaves out a reference naming an $id of the root document, as a schema rather than a file', () => { + const files = referenced( + { owner: { $ref: 'pet.json#/properties/tag' }, tag: { $ref: './tag.yaml' } }, + new Set(['pet.json']) + ); + + expect(files).to.deep.equal([inSpec('tag.yaml')]); + }); + + it('follows a reference in a document that is a list', () => { + expect(referenced([{ name: 'limit' }, { $ref: './parameters/offset.yaml' }])).to.deep.equal([ + inSpec('parameters/offset.yaml') + ]); }); it('reads a backslash as a slash, and leaves a percent escape as written', () => { @@ -261,9 +288,9 @@ describe('OpenApiDocument', () => { }); it('survives an alias that holds itself', () => { - const document = read('openapi: 3.0.0\nloop: &loop\n again: *loop\n next: { $ref: ./next.yaml }\n', YAML_FILE); + const document = read('openapi: 3.0.0\nloop: &loop\n again: *loop\n next: { $ref: ./next.yaml }\n'); - expect(document.referencedFiles(specDirectory).map(String)).to.deep.equal([inSpec('next.yaml')]); + expect(document.referencedFiles(specDirectory, new Set()).map(String)).to.deep.equal([inSpec('next.yaml')]); }); }); diff --git a/test/types/portal/preview-config.test.ts b/test/types/portal/preview-config.test.ts index a0b2eb771..e87a87ff9 100644 --- a/test/types/portal/preview-config.test.ts +++ b/test/types/portal/preview-config.test.ts @@ -41,5 +41,13 @@ describe('PreviewConfig', () => { expect(preview.show(named('Pets'), false)).to.equal(true); expect(preview.show(named('Pets'), false)).to.equal(false); }); + + it('takes a refusal as fixed by a save that also wrote something, and not again after it', () => { + const preview = new PreviewConfig(named('Pets'), false); + preview.refuse(); + + expect(preview.show(named('Cats'), true)).to.equal(true); + expect(preview.show(named('Cats'), false)).to.equal(false); + }); }); }); From ecd4b0ecff5c72caada89ce07edf9cfdc711c3c9 Mon Sep 17 00:00:00 2001 From: saeedjamshaid Date: Mon, 12 Oct 2026 11:44:41 +0500 Subject: [PATCH 6/7] refactor(portal): gather a specification's files in one type, and key the preview's messages The `$ref` walk leaves the source context for `SpecBundle`, a plain type that records each file read and lists the ones still to read, while the context reads them a level at a time. A spec problem's reason is a kind the prompt words, so a document gone mid-save is "no longer there" rather than an ENOENT message, and a missing referenced file is a `MissingFile`, which names its spelling on disk in another case. The 50 characters the bundler looks at for JSON are a named constant, held to the bundler by a row of its table. The preview's watch messages are one prompt each, keyed by what is watched, and each preview type's answer goes straight to its prompt. A save of `apimatic.json` that throws now says its changes could not be checked, and a lost watch on it no longer says "further edits". Co-Authored-By: Claude Opus 5.5 --- src/actions/portal/serve.ts | 80 ++++---------- src/prompts/portal/serve.ts | 123 +++++++++------------ src/prompts/portal/source.ts | 40 +++++-- src/types/portal-source-context.ts | 95 ++++++---------- src/types/portal/openapi-document.ts | 89 ++++++++------- src/types/portal/portal-source.ts | 11 +- src/types/portal/preview-content.ts | 13 ++- src/types/portal/preview-specs.ts | 10 +- src/types/portal/spec-bundle.ts | 49 ++++++++ test/actions/portal/serve.test.ts | 85 +++++++------- test/prompts/portal/serve.test.ts | 84 ++++++++++++-- test/prompts/portal/source.test.ts | 56 ++++++++-- test/types/portal-source-context.test.ts | 57 ++++++++-- test/types/portal/openapi-document.test.ts | 24 ++-- test/types/portal/spec-bundle.test.ts | 51 +++++++++ 15 files changed, 547 insertions(+), 320 deletions(-) create mode 100644 src/types/portal/spec-bundle.ts create mode 100644 test/types/portal/spec-bundle.test.ts diff --git a/src/actions/portal/serve.ts b/src/actions/portal/serve.ts index 62c21a75b..6c48ffd7e 100644 --- a/src/actions/portal/serve.ts +++ b/src/actions/portal/serve.ts @@ -9,7 +9,7 @@ import { ProjectContext } from '../../types/project-context.js'; import { isSkippedByGlob } from '../../types/portal/content-tree.js'; import { GeneratedPages } from '../../types/portal/generated-pages.js'; import { PortalArtifacts } from '../../types/portal/portal-artifacts.js'; -import { PortalSettings, PortalSource } from '../../types/portal/portal-source.js'; +import { PortalSettings, PortalSource, WatchedSource } from '../../types/portal/portal-source.js'; import { PreviewConfig } from '../../types/portal/preview-config.js'; import { PreviewContent } from '../../types/portal/preview-content.js'; import { PreviewSpecs } from '../../types/portal/preview-specs.js'; @@ -150,25 +150,15 @@ export class PortalServeAction { const applied = await this.projectService.applyConfig(portalProjectDirectory, settings); if (applied.isErr()) { - this.prompts.configNotApplied(applied.error); + this.prompts.notApplied('config', applied.error, sourceDirectory); return; } - if (preview.show(config, applied.value)) { - this.prompts.configApplied(); - } + this.prompts.configShown(preview.show(config, applied.value)); onApplied(settings); }; - return this.startWatch( - (onChange) => - this.fileWatchService.watch(sourceDirectory, new FileName(APIMATIC_CONFIG_FILE_NAME), onChange, (reason) => - this.prompts.configWatchFailed(reason) - ), - applyEdit, - { - notWatched: (reason) => this.prompts.configNotWatched(reason), - thrown: (reason) => this.prompts.configNotApplied(reason) - } + return this.startWatch('config', sourceDirectory, applyEdit, (onChange, onFailed) => + this.fileWatchService.watch(sourceDirectory, new FileName(APIMATIC_CONFIG_FILE_NAME), onChange, onFailed) ); } @@ -200,30 +190,15 @@ export class PortalServeAction { checked.value.files ); if (applied.isErr()) { - this.prompts.contentNotApplied(applied.error, sourceDirectory); + this.prompts.notApplied('content', applied.error, sourceDirectory); return; } - const shown = preview.show(checked.value.notices); - if (shown.fixed) { - this.prompts.contentAccepted(sourceDirectory); - } - this.prompts.contentNotices(shown.notices, sourceDirectory); + this.prompts.contentShown(preview.show(checked.value.notices), sourceDirectory); }; - return this.startWatch( - (onChange) => - this.fileWatchService.watchTree( - contentDirectory, - onChange, - (reason) => this.prompts.contentWatchFailed(reason, sourceDirectory), - // What the build never reads, as an editor's swap file, would only have the check run again. - isSkippedByGlob - ), - check, - { - notWatched: (reason) => this.prompts.contentNotWatched(reason, sourceDirectory), - thrown: (reason) => this.prompts.contentNotChecked(reason, sourceDirectory) - } + // What the build never reads, as an editor's swap file, would only have the check run again. + return this.startWatch('content', sourceDirectory, check, (onChange, onFailed) => + this.fileWatchService.watchTree(contentDirectory, onChange, onFailed, isSkippedByGlob) ); } @@ -240,28 +215,12 @@ export class PortalServeAction { this.prompts.specRejected(checked.error, sourceDirectory); return; } - const shown = preview.show(checked.value); - if (shown.fixed) { - this.prompts.specAccepted(sourceDirectory); - } - if (shown.change !== null) { - this.prompts.specsNeedRestart(shown.change, sourceDirectory); - } + this.prompts.specsShown(preview.show(checked.value), sourceDirectory); }; - return this.startWatch( - (onChange) => - this.fileWatchService.watchTree( - source.specDirectory, - onChange, - (reason) => this.prompts.specWatchFailed(reason, sourceDirectory), - isSkippedByGlob - ), - check, - { - notWatched: (reason) => this.prompts.specNotWatched(reason, sourceDirectory), - thrown: (reason) => this.prompts.specNotChecked(reason, sourceDirectory) - } + // As for content: an editor's swap file, saved while a document is refused, would only repeat the refusal. + return this.startWatch('spec', sourceDirectory, check, (onChange, onFailed) => + this.fileWatchService.watchTree(source.specDirectory, onChange, onFailed, isSkippedByGlob) ); } @@ -270,21 +229,22 @@ export class PortalServeAction { * before the preview started, which can take a minute, and a save made meanwhile reached no watch. */ private startWatch( - watch: (onChange: () => Promise) => Result, + watched: WatchedSource, + sourceDirectory: DirectoryPath, handle: () => Promise, - report: { notWatched: (reason: string) => void; thrown: (reason: string) => void } + watch: (onChange: () => Promise, onFailed: (reason: string) => void) => Result ): FileWatch | undefined { // The watch drops whatever its handler throws, which would leave the preview stale without a word. const onChange = async () => { try { await handle(); } catch (error) { - report.thrown(errorMessage(error)); + this.prompts.notChecked(watched, errorMessage(error), sourceDirectory); } }; - const started = watch(onChange); + const started = watch(onChange, (reason) => this.prompts.watchFailed(watched, reason, sourceDirectory)); if (started.isErr()) { - report.notWatched(started.error); + this.prompts.notWatched(watched, started.error, sourceDirectory); return undefined; } started.value.recheck(); diff --git a/src/prompts/portal/serve.ts b/src/prompts/portal/serve.ts index 9e79bf3d9..4b0f78079 100644 --- a/src/prompts/portal/serve.ts +++ b/src/prompts/portal/serve.ts @@ -4,11 +4,11 @@ import { APIMATIC_CONFIG_FILE_NAME } from '../../types/apimatic-config/document. import { DirectoryPath } from '../../types/file/directoryPath.js'; import { FilePath } from '../../types/file/filePath.js'; import { UrlPath } from '../../types/file/urlPath.js'; -import { ContentNotices } from '../../types/portal/content-notices.js'; import { MissingArtifacts } from '../../types/portal/generated-pages.js'; import { NAVIGATION_FILE_NAME } from '../../types/portal/portal-navigation.js'; -import { ContentProblem, PortalSourceProblem } from '../../types/portal/portal-source.js'; -import { SpecsChange } from '../../types/portal/preview-specs.js'; +import { ContentProblem, PortalSourceProblem, WatchedSource } from '../../types/portal/portal-source.js'; +import { ContentShown } from '../../types/portal/preview-content.js'; +import { SpecsShown } from '../../types/portal/preview-specs.js'; import { PortalDevServer, PortalDevServerFailure } from '../../infrastructure/portal-dev-server-service.js'; import { listedInProse } from '../../utils/string-utils.js'; import { Result } from 'neverthrow'; @@ -24,6 +24,9 @@ import { staticPath } from './source.js'; +const reportedOnRestart = (mistakes: string) => + `a mistake in ${mistakes} is only reported when the preview is restarted`; + export class PortalServePrompts { public usingFallbackPort(requestedPort: number, availablePort: number) { const message = @@ -70,11 +73,12 @@ export class PortalServePrompts { ); } - public configApplied() { - log.success(`Applied the changes to ${f.var(APIMATIC_CONFIG_FILE_NAME)}.`); + public configShown(applied: boolean) { + if (applied) { + log.success(`Applied the changes to ${f.var(APIMATIC_CONFIG_FILE_NAME)}.`); + } } - /** Explained as `portal generate` would explain it, since the same rules refused it. */ public configRejected(problem: PortalSourceProblem, sourceDirectory: DirectoryPath) { reportSourceProblem(problem, sourceDirectory); log.message('The preview keeps showing what it last accepted until the file is fixed.'); @@ -89,10 +93,6 @@ export class PortalServePrompts { ); } - public configNotApplied(reason: string) { - log.warn(`The changes to ${f.var(APIMATIC_CONFIG_FILE_NAME)} could not be applied to the preview: ${reason}`); - } - /** Vite reads its public directory once, and a missing one is served as none. */ public staticDirectoryNotServed(sourceDirectory: DirectoryPath) { const message = @@ -101,15 +101,6 @@ export class PortalServePrompts { log.warn(message); } - public configNotWatched(reason: string) { - this.notWatched(f.var(APIMATIC_CONFIG_FILE_NAME), reason, 'edits to it need the preview restarted'); - } - - public configWatchFailed(reason: string) { - this.watchFailed(f.var(APIMATIC_CONFIG_FILE_NAME), reason, 'further edits to it need the preview restarted'); - } - - /** Explained as `portal generate` would explain it, since the same rules refused it. */ public contentRejected(problems: ContentProblem[], sourceDirectory: DirectoryPath) { reportContentProblems(problems, sourceDirectory); log.message( @@ -118,31 +109,13 @@ export class PortalServePrompts { ); } - public contentNotApplied(reason: string, sourceDirectory: DirectoryPath) { - log.warn(`The changes to ${contentPath(sourceDirectory)} could not be applied to the preview: ${reason}`); - } - - public contentNotChecked(reason: string, sourceDirectory: DirectoryPath) { - this.notChecked(contentPath(sourceDirectory), reason); - } - - public contentAccepted(sourceDirectory: DirectoryPath) { - this.fixed(contentPath(sourceDirectory)); - } - - public contentNotices(notices: ContentNotices, sourceDirectory: DirectoryPath) { + public contentShown({ fixed, notices }: ContentShown, sourceDirectory: DirectoryPath) { + if (fixed) { + this.fixed('content', sourceDirectory); + } reportContentNotices(notices, sourceDirectory); } - public contentNotWatched(reason: string, sourceDirectory: DirectoryPath) { - this.notWatched(contentPath(sourceDirectory), reason, CONTENT_UNTIL_RESTART); - } - - public contentWatchFailed(reason: string, sourceDirectory: DirectoryPath) { - this.watchFailed(contentPath(sourceDirectory), reason, CONTENT_UNTIL_RESTART); - } - - /** Explained as `portal generate` would explain it, since the same rules refused it. */ public specRejected(problem: PortalSourceProblem, sourceDirectory: DirectoryPath) { reportSourceProblem(problem, sourceDirectory); log.message( @@ -151,12 +124,13 @@ export class PortalServePrompts { ); } - public specAccepted(sourceDirectory: DirectoryPath) { - this.fixed(specPath(sourceDirectory)); - } - - // The preview lists its documents once, when it starts. - public specsNeedRestart(change: SpecsChange, sourceDirectory: DirectoryPath) { + public specsShown({ fixed, change }: SpecsShown, sourceDirectory: DirectoryPath) { + if (fixed) { + this.fixed('spec', sourceDirectory); + } + if (change === null) { + return; + } const changes = [filesWere(change.added, 'added'), filesWere(change.removed, 'removed')].filter( (each) => each !== null ); @@ -169,16 +143,24 @@ export class PortalServePrompts { ); } - public specNotChecked(reason: string, sourceDirectory: DirectoryPath) { - this.notChecked(specPath(sourceDirectory), reason); + public notWatched(watched: WatchedSource, reason: string, sourceDirectory: DirectoryPath) { + const { name, untilRestart } = describeWatched(watched, sourceDirectory); + log.warn(`${name} cannot be watched (${reason}), so ${untilRestart}.`); + } + + public watchFailed(watched: WatchedSource, reason: string, sourceDirectory: DirectoryPath) { + const { name, untilRestart } = describeWatched(watched, sourceDirectory); + log.warn(`${name} is no longer watched (${reason}), so ${untilRestart}.`); } - public specNotWatched(reason: string, sourceDirectory: DirectoryPath) { - this.notWatched(specPath(sourceDirectory), reason, SPEC_UNTIL_RESTART); + public notChecked(watched: WatchedSource, reason: string, sourceDirectory: DirectoryPath) { + const { name } = describeWatched(watched, sourceDirectory); + log.warn(`The changes to ${name} could not be checked: ${reason}`); } - public specWatchFailed(reason: string, sourceDirectory: DirectoryPath) { - this.watchFailed(specPath(sourceDirectory), reason, SPEC_UNTIL_RESTART); + public notApplied(watched: WatchedSource, reason: string, sourceDirectory: DirectoryPath) { + const { name } = describeWatched(watched, sourceDirectory); + log.warn(`The changes to ${name} could not be applied to the preview: ${reason}`); } public stopping() { @@ -198,28 +180,29 @@ export class PortalServePrompts { await Promise.race([once(process, 'SIGINT'), once(process, 'SIGTERM')]); } - private fixed(directory: string) { - log.success(`${directory} is fixed; a build would accept it again.`); - } - - private notChecked(directory: string, reason: string) { - log.warn(`The changes to ${directory} could not be checked: ${reason}`); - } - - private notWatched(watched: string, reason: string, until: string) { - log.warn(`${watched} cannot be watched (${reason}), so ${until}.`); + private fixed(watched: WatchedSource, sourceDirectory: DirectoryPath) { + log.success(`${describeWatched(watched, sourceDirectory).name} is fixed; a build would accept it again.`); } +} - private watchFailed(watched: string, reason: string, until: string) { - log.warn(`${watched} is no longer watched (${reason}), so ${until}.`); +/** How a message names what is watched, and what is lost until a restart while it is not. */ +function describeWatched( + watched: WatchedSource, + sourceDirectory: DirectoryPath +): { name: string; untilRestart: string } { + switch (watched) { + case 'config': + return { name: f.var(APIMATIC_CONFIG_FILE_NAME), untilRestart: 'edits to it need the preview restarted' }; + case 'content': + return { + name: contentPath(sourceDirectory), + untilRestart: reportedOnRestart(`a page or a ${f.var(NAVIGATION_FILE_NAME)}`) + }; + case 'spec': + return { name: specPath(sourceDirectory), untilRestart: reportedOnRestart('a specification') }; } } -const reportedOnRestart = (mistakes: string) => - `a mistake in ${mistakes} is only reported when the preview is restarted`; -const CONTENT_UNTIL_RESTART = reportedOnRestart(`a page or a ${f.var(NAVIGATION_FILE_NAME)}`); -const SPEC_UNTIL_RESTART = reportedOnRestart('a specification'); - function filesWere(files: FilePath[], done: string): string | null { if (files.length === 0) { return null; diff --git a/src/prompts/portal/source.ts b/src/prompts/portal/source.ts index 51ba0813b..725892708 100644 --- a/src/prompts/portal/source.ts +++ b/src/prompts/portal/source.ts @@ -11,7 +11,8 @@ import { PortalSourceProblem, ReservedAddressPage, SharedAddress, - SpecConversion + SpecConversion, + UnreadableSpecReason } from '../../types/portal/portal-source.js'; import { TRANSFORMATIONS_DIRECTORY_NAME } from '../../types/transform-context.js'; import { PortalTab, SharedTabName } from '../../types/portal/portal-tabs.js'; @@ -74,6 +75,22 @@ function reportSpecConversion({ file, format, converted, others }: SpecConversio ); } +function unreadableBecause(reason: UnreadableSpecReason): string { + switch (reason.kind) { + case 'empty': + return 'it is empty'; + case 'noValue': + return 'it holds no value'; + case 'noKey': + return 'it is not JSON, and the portal reads YAML only when it holds a key'; + case 'gone': + return 'it is no longer there'; + case 'invalid': + case 'unopened': + return reason.message; + } +} + /** * Shared by `portal generate` and `portal serve`: both read the same source directory, so * a broken one has to be explained the same way in both. @@ -95,18 +112,23 @@ export function reportSourceProblem(problem: PortalSourceProblem, sourceDirector } case 'unreadableSpec': { const file = relative(problem.file, sourceDirectory); - const named = + const why = unreadableBecause(problem.reason); + log.error( problem.referencedBy === null - ? file - : `${file}, which ${relative(problem.referencedBy, sourceDirectory)} refers to,`; - log.error(`${named} could not be read: ${problem.reason}.`); + ? `${file} could not be read: ${why}.` + : `${file}, which ${relative(problem.referencedBy, sourceDirectory)} refers to, could not be read: ${why}.` + ); return; } case 'missingSpecReference': { - log.error( - `${relative(problem.referencedBy, sourceDirectory)} refers to ${relative(problem.file, sourceDirectory)}, ` + - 'but there is no such file.' - ); + const { file, foundAs } = problem.missing; + const referencedBy = relative(problem.referencedBy, sourceDirectory); + const refersTo = `${referencedBy} refers to ${relative(file, sourceDirectory)}`; + if (foundAs !== null) { + log.error(`${refersTo}, but the file is ${spelt(foundAs, sourceDirectory)}.`); + return; + } + log.error(`${refersTo}, but there is no such file.`); log.message(`A ${f.var('$ref')} names a file from the folder of the file it is written in.`); return; } diff --git a/src/types/portal-source-context.ts b/src/types/portal-source-context.ts index 00766bc47..2a13f8ce6 100644 --- a/src/types/portal-source-context.ts +++ b/src/types/portal-source-context.ts @@ -13,7 +13,6 @@ import { NOT_FOUND_FILE_NAME, SHELL_FILE_NAME } from './portal-context.js'; import { CONTENT_DIRECTORY_NAME, SPEC_DIRECTORY_NAME, STATIC_DIRECTORY_NAME } from './project-layout.js'; import { PLACEHOLDER_SITE, SiteConfig, SuggestedSite } from './portal/config/site-config.js'; import { AcceptedContent, ContentFile, ContentTree, ReadPage } from './portal/content-tree.js'; -import { Endpoint } from './portal/endpoint.js'; import { GeneratedPages, PluginSource } from './portal/generated-pages.js'; import { OpenApiDocument } from './portal/openapi-document.js'; import { parsePage } from './portal/page.js'; @@ -30,18 +29,15 @@ import { PortalSource, PortalSourceProblem, PortalSpec, - SpecConversion + SpecConversion, + UnreadableSpecReason } from './portal/portal-source.js'; +import { SpecBundle } from './portal/spec-bundle.js'; import { SpecContext } from './spec-context.js'; import { TRANSFORMATIONS_DIRECTORY_NAME, transformedFileName } from './transform-context.js'; const SPEC_EXTENSIONS = ['.json', '.yaml', '.yml']; -interface SpecFile { - file: FilePath; - document: OpenApiDocument; -} - // Empty on purpose: the portal's own routes under /api/ are files with extensions -- // /api/search.json -- so none can collide with a spec section, which is always a directory. // Pages the user puts under content/api/ share the directory, and `hiddenPages` reports the @@ -414,7 +410,7 @@ export class PortalSourceContext { if (await this.fileService.isZipFile(specPath)) { return PLACEHOLDER_SITE; } - return (await this.readDocument(specPath, null)).match( + return (await this.readDocument(specPath)).match( (document) => document.suggestedSite(), () => PLACEHOLDER_SITE ); @@ -450,7 +446,7 @@ export class PortalSourceContext { const documentNames = fileNames.filter((name) => SPEC_EXTENSIONS.some((extension) => name.hasExtension(extension))); for (const fileName of documentNames) { const file = new FilePath(this.specDirectory, fileName); - const read = await this.readDocument(file, null); + const read = await this.readDocument(file); if (read.isErr()) { return err(read.error); } @@ -463,16 +459,11 @@ export class PortalSourceContext { continue; } - const spec = { file, document }; - const referenced = await this.referencedDocuments(spec); - if (referenced.isErr()) { - return err(referenced.error); + const bundle = await this.readBundle(file, document); + if (bundle.isErr()) { + return err(bundle.error); } - specs.push({ - slug: this.uniqueSlug(fileName, usedSlugs), - file, - endpoints: PortalSourceContext.endpoints(spec, referenced.value) - }); + specs.push({ slug: this.uniqueSlug(fileName, usedSlugs), file, endpoints: bundle.value.endpoints() }); first ??= document; } @@ -509,61 +500,43 @@ export class PortalSourceContext { }; } - /** - * Every file `spec` is bundled from, by its path. One the bundler cannot read fails the build, - * or leaves its reference empty, so it is refused. - */ - private async referencedDocuments( - spec: SpecFile - ): Promise, PortalSourceProblem>> { - const rootIds = spec.document.schemaIds(); - const documents = new Map([[spec.file.toString(), spec.document]]); - const named = new Set(documents.keys()); - const queue: { file: FilePath; read: Promise> }[] = []; - // Each file is read once, starting when it is first named, as a split specification runs to thousands. - const enqueue = ({ file: referencedBy, document }: SpecFile) => { - for (const file of document.referencedFiles(referencedBy.directory(), rootIds)) { - if (!named.has(file.toString())) { - named.add(file.toString()); - queue.push({ file, read: this.readDocument(file, referencedBy) }); - } - } - }; - - enqueue(spec); - // Grows as it is walked: each file read queues the ones it refers to. - for (const { file, read } of queue) { - const referenced = await read; - if (referenced.isErr()) { - return err(referenced.error); + /** Reads every file the bundler would embed into `document`, a level of `$ref`s at a time. */ + private async readBundle( + file: FilePath, + document: OpenApiDocument + ): Promise> { + const bundle = new SpecBundle(file, document); + for (let unread = bundle.unread(); unread.length > 0; unread = bundle.unread()) { + const read = Result.combine( + await Promise.all(unread.map((next) => this.readDocument(next.file, next.referencedBy))) + ); + if (read.isErr()) { + return err(read.error); } - documents.set(file.toString(), referenced.value); - enqueue({ file, document: referenced.value }); + unread.forEach((next, index) => bundle.add(next.file, read.value[index])); } - return ok(documents); - } - - // A path item in another file is read from it; one that file refers on to again is not followed. - private static endpoints(spec: SpecFile, documents: Map): Endpoint[] { - const mounted = spec.document - .pathItemReferences(spec.file.directory()) - .flatMap(({ path, file, pointer }) => documents.get(file.toString())?.endpointsAt(path, pointer) ?? []); - return [...spec.document.endpoints(), ...mounted]; + return ok(bundle); } private async readDocument( file: FilePath, - referencedBy: FilePath | null + referencedBy: FilePath | null = null ): Promise> { let contents: string; try { contents = await this.fileService.getContents(file); } catch (error) { - if (referencedBy !== null && !(await this.fileService.fileExists(file))) { - return err({ kind: 'missingSpecReference', file, referencedBy }); + const there = await this.fileService.fileExists(file); + if (!there && referencedBy !== null) { + // Null only for a name listed but not there as a file, as a dangling link is. + const missing = (await this.missingFile(file)) ?? { file, foundAs: null }; + return err({ kind: 'missingSpecReference', missing, referencedBy }); } - // Also a listed document gone mid-save, which the next save fixes, as it would a half-written one. - return err({ kind: 'unreadableSpec', file, referencedBy, reason: errorMessage(error) }); + // A listed document gone is one caught mid-save, which the next save fixes. + const reason: UnreadableSpecReason = there + ? { kind: 'unopened', message: errorMessage(error) } + : { kind: 'gone' }; + return err({ kind: 'unreadableSpec', file, referencedBy, reason }); } return OpenApiDocument.parse(contents).mapErr( (reason): PortalSourceProblem => ({ kind: 'unreadableSpec', file, referencedBy, reason }) diff --git a/src/types/portal/openapi-document.ts b/src/types/portal/openapi-document.ts index fd8e7382e..29db448a2 100644 --- a/src/types/portal/openapi-document.ts +++ b/src/types/portal/openapi-document.ts @@ -15,6 +15,13 @@ import { Endpoint } from './endpoint.js'; */ export type SpecFormat = { supported: true } | { supported: false; format: string | null }; +/** Why the portal's bundler would fail on a text, or read nothing from it; `invalid` carries what the parser said. */ +export type ParseFailure = + | { kind: 'empty' } + | { kind: 'noValue' } + | { kind: 'noKey' } + | { kind: 'invalid'; message: string }; + /** A path item kept in another file, where `pointer` locates it; empty for the whole file. */ export interface PathItemReference { path: string; @@ -28,53 +35,54 @@ const HTTP_METHODS = new Set(['get', 'put', 'post', 'delete', 'options', 'head', const URL_SCHEME = /^[a-z][a-z\d+.-]+:/i; -// How @scalar/json-magic reads every file it bundles: JSON, else YAML, with these options, only -// when the text holds a key and does not open as JSON does. +// How @scalar/json-magic reads each file it bundles: as JSON, else as YAML with these options when it holds a key. const YAML_OPTIONS = { merge: true, maxAliasCount: 10000 }; const YAML_KEY = /^[^:]+:/; +// A text that opens as JSON does within this many characters is taken for JSON, and never read as YAML. const JSON_OPENING = /^\s*[[{]/; +const JSON_OPENING_WITHIN = 50; // The keys @scalar/json-magic writes what it has embedded under, which it does not read again. const BUNDLER_KEYS = new Set(['x-ext', 'x-ext-urls']); -/** A specification as written to disk, read the one way the wizard and the build agree on. */ +/** A specification's document, or a file its `$ref`s name, read the one way the wizard and the build agree on. */ export class OpenApiDocument { - private constructor(private readonly value: unknown) {} + private readonly document: JsonObject; - /** - * Fails with the reason where the portal's bundler fails, or reads nothing. A document that - * reads as something other than an object is kept, and reports itself as no specification at all. - */ - public static parse(contents: string): Result { + private constructor(private readonly value: unknown) { + this.document = isJsonObject(value) ? value : {}; + } + + /** Fails where the portal's bundler fails or reads nothing; a value other than an object is no specification. */ + public static parse(contents: string): Result { const text = stripByteOrderMark(contents); if (text.trim() === '') { - return err('it is empty'); + return err({ kind: 'empty' }); } let value: unknown; try { value = JSON.parse(text); } catch (error) { - // Only the first 50 characters, as the bundler looks no further. - if (JSON_OPENING.test(text.slice(0, 50))) { - return err(parseFailure(error)); + if (JSON_OPENING.test(text.slice(0, JSON_OPENING_WITHIN))) { + return err(invalid(error)); } if (!YAML_KEY.test(text)) { - return err('it is not JSON, and the portal reads YAML only when it holds a key'); + return err({ kind: 'noKey' }); } try { value = parseYaml(text, YAML_OPTIONS); } catch (yamlError) { - return err(parseFailure(yamlError)); + return err(invalid(yamlError)); } } - return value === null ? err('it holds no value') : ok(new OpenApiDocument(value)); + return value === null ? err({ kind: 'noValue' }) : ok(new OpenApiDocument(value)); } /** The `$id`s this document declares, which a `$ref` in any file bundled into it names a schema by. */ public schemaIds(): Set { const ids = new Set(); walk(this.value, (node) => { - if (isJsonObject(node) && typeof node.$id === 'string') { + if (typeof node.$id === 'string') { ids.add(node.$id); } return true; @@ -85,16 +93,17 @@ export class OpenApiDocument { /** The local files this document's `$ref`s name, as the portal's bundler finds them, given the root's `$id`s. */ public referencedFiles(directory: DirectoryPath, rootIds: ReadonlySet): FilePath[] { const names = new Set(); - // Not below an `$id`: the bundler resolves what is under one against the `$id`, not the file. walk(this.value, (node) => { - if (isJsonObject(node) && typeof node.$id === 'string') { + // The bundler resolves what is below an `$id` against the `$id`, not the file. + if (typeof node.$id === 'string') { return false; } - if (isJsonObject(node) && typeof node.$ref === 'string') { - names.add(node.$ref.split('#')[0]); - return false; + const target = reference(node); + if (target === undefined) { + return true; } - return true; + names.add(target.name); + return false; }); return [...names].filter((name) => !rootIds.has(name)).flatMap((name) => localFile(directory, name) ?? []); } @@ -107,10 +116,9 @@ export class OpenApiDocument { /** The path items kept in other local files, resolved against the `directory` this document sits in. */ public pathItemReferences(directory: DirectoryPath): PathItemReference[] { return Object.entries(this.paths()).flatMap(([path, pathItem]) => { - const reference = isJsonObject(pathItem) && typeof pathItem.$ref === 'string' ? pathItem.$ref : ''; - const [name, pointer = ''] = reference.split('#'); - const file = localFile(directory, name); - return file === undefined ? [] : [{ path, file, pointer }]; + const target = reference(pathItem); + const file = target && localFile(directory, target.name); + return target && file ? [{ path, file, pointer: target.pointer }] : []; }); } @@ -146,10 +154,6 @@ export class OpenApiDocument { return { name, description: description === null ? null : cap(description, DESCRIPTION_LIMIT) }; } - private get document(): JsonObject { - return isJsonObject(this.value) ? this.value : {}; - } - private paths(): JsonObject { return isJsonObject(this.document.paths) ? this.document.paths : {}; } @@ -174,25 +178,34 @@ export class OpenApiDocument { } } -/** The file a `$ref` names before its `#`, resolved as the bundler resolves it; undefined for none, or a URL. */ +/** A node's `$ref`, split as the bundler splits it: the file it names, empty for its own, and the pointer into it. */ +function reference(node: unknown): { name: string; pointer: string } | undefined { + if (!isJsonObject(node) || typeof node.$ref !== 'string') { + return undefined; + } + const [name, pointer = ''] = node.$ref.split('#'); + return { name, pointer }; +} + +/** The file a `$ref` names, resolved as the bundler resolves it; undefined for none, or a URL. */ function localFile(directory: DirectoryPath, name: string): FilePath | undefined { return name === '' || URL_SCHEME.test(name) ? undefined : FilePath.resolve(directory, name.replaceAll('\\', '/')); } // A YAML error goes on to quote the lines around it, and V8 quotes the JSON it stopped at across line breaks. -function parseFailure(error: unknown): string { +function invalid(error: unknown): ParseFailure { const message = errorMessage(error); - const reason = error instanceof YAMLParseError ? message.split('\n')[0].replace(/:$/, '') : message; - return reason.replace(/\s+/g, ' ').trim(); + const firstLine = error instanceof YAMLParseError ? message.split('\n')[0].replace(/:$/, '') : message; + return { kind: 'invalid', message: firstLine.replace(/\s+/g, ' ').trim() }; } -/** Each object and list once, going into one only when `visit` says so, and never into the keys the bundler writes. */ -function walk(node: unknown, visit: (node: object) => boolean, seen = new WeakSet()): void { +/** Each object and list once; into an object only when `visit` says so, and never into the keys the bundler writes. */ +function walk(node: unknown, visit: (node: JsonObject) => boolean, seen = new WeakSet()): void { if (typeof node !== 'object' || node === null || seen.has(node)) { return; } seen.add(node); - if (!visit(node)) { + if (isJsonObject(node) && !visit(node)) { return; } for (const [key, value] of Object.entries(node)) { diff --git a/src/types/portal/portal-source.ts b/src/types/portal/portal-source.ts index 83e4498e3..9bea1ae6e 100644 --- a/src/types/portal/portal-source.ts +++ b/src/types/portal/portal-source.ts @@ -5,6 +5,7 @@ import { Endpoint } from './endpoint.js'; import { SuggestedSite } from './config/site-config.js'; import { ContentNotices } from './content-notices.js'; import { GeneratedPages } from './generated-pages.js'; +import { ParseFailure } from './openapi-document.js'; import { PortalConfig } from './portal-config.js'; /** An OpenAPI document found in `src/spec/`, with the slug its section is mounted at. */ @@ -15,6 +16,9 @@ export interface PortalSpec { endpoints: Endpoint[]; } +/** A part of the source directory that `portal serve` watches, and checks on each save. */ +export type WatchedSource = 'config' | 'content' | 'spec'; + /** What `apimatic.json` decides about a portal, which `portal serve` reads again on every edit. */ export interface PortalSettings { config: PortalConfig; @@ -108,6 +112,9 @@ export interface SpecConversion { others: number; } +/** Why a file a specification is read from could not be read; `unopened` carries what the file system said. */ +export type UnreadableSpecReason = ParseFailure | { kind: 'gone' } | { kind: 'unopened'; message: string }; + /** Why a source directory cannot be built; each variant maps to its own message. */ export type PortalSourceProblem = | { kind: 'missingConfig' } @@ -115,8 +122,8 @@ export type PortalSourceProblem = | { kind: 'invalidConfig'; errors: string[]; missingPortal: boolean } // Every problem found in `content/`, so that one run lists all that a build would refuse. | { kind: 'invalidContent'; problems: ContentProblem[] } - | { kind: 'unreadableSpec'; file: FilePath; referencedBy: FilePath | null; reason: string } - | { kind: 'missingSpecReference'; file: FilePath; referencedBy: FilePath } + | { kind: 'unreadableSpec'; file: FilePath; referencedBy: FilePath | null; reason: UnreadableSpecReason } + | { kind: 'missingSpecReference'; missing: MissingFile; referencedBy: FilePath } // Its `folders` are not read, and `api transform` writes into one of its own. | { kind: 'emptySpecDirectory'; folders: DirectoryPath[] } | { kind: 'noOpenApiSpec'; conversion: SpecConversion } diff --git a/src/types/portal/preview-content.ts b/src/types/portal/preview-content.ts index e491201f0..a1a5ab280 100644 --- a/src/types/portal/preview-content.ts +++ b/src/types/portal/preview-content.ts @@ -4,6 +4,12 @@ import { isSameTabEntry } from './portal-navigation.js'; import { SharedTabName, TabOwner } from './portal-tabs.js'; import { PreviewRefusal } from './preview-refusal.js'; +/** What to say of a save the preview accepts: whether it fixes a refused one, and the notices it brings. */ +export interface ContentShown { + fixed: boolean; + notices: ContentNotices; +} + /** A notice is given once, on the save that brings it about, rather than on every save after it. */ export class PreviewContent { private shown: ContentNotices; @@ -18,11 +24,8 @@ export class PreviewContent { this.refusal.refuse(); } - /** - * Records the content as shown, and answers whether to say it is fixed, having been refused, - * and which notices to give: each kind in full when it holds something new, and none otherwise. - */ - public show(notices: ContentNotices): { fixed: boolean; notices: ContentNotices } { + /** Records the content as shown, and answers what to say: each kind of notice in full when any of it is new. */ + public show(notices: ContentNotices): ContentShown { const shown = { fixed: this.refusal.accept(), notices: PreviewContent.since(notices, this.shown) }; this.shown = notices; return shown; diff --git a/src/types/portal/preview-specs.ts b/src/types/portal/preview-specs.ts index bde0092be..3b52958e5 100644 --- a/src/types/portal/preview-specs.ts +++ b/src/types/portal/preview-specs.ts @@ -8,6 +8,12 @@ export interface SpecsChange { removed: FilePath[]; } +/** What to say of a save the preview accepts: whether it fixes a refused one, and a change not yet warned of. */ +export interface SpecsShown { + fixed: boolean; + change: SpecsChange | null; +} + /** The preview serves only the documents it started with, so a change to which are in `spec/` is said once. */ export class PreviewSpecs { private readonly served: FilePath[]; @@ -24,8 +30,8 @@ export class PreviewSpecs { this.refusal.refuse(); } - /** Records the documents as shown: whether `spec/` is fixed, having been refused, and a change not yet warned of. */ - public show(specs: PortalSpec[]): { fixed: boolean; change: SpecsChange | null } { + /** Records the documents as shown, and answers what to say of them. */ + public show(specs: PortalSpec[]): SpecsShown { const files = specs.map(({ file }) => file); const unchanged = isSameSet(files, this.shown) || isSameSet(files, this.served); this.shown = files; diff --git a/src/types/portal/spec-bundle.ts b/src/types/portal/spec-bundle.ts new file mode 100644 index 000000000..46f36bba2 --- /dev/null +++ b/src/types/portal/spec-bundle.ts @@ -0,0 +1,49 @@ +import { FilePath } from '../file/filePath.js'; +import { Endpoint } from './endpoint.js'; +import { OpenApiDocument } from './openapi-document.js'; + +/** A file a `$ref` names, with the file the `$ref` is written in. */ +export interface ReferencedFile { + file: FilePath; + referencedBy: FilePath; +} + +/** A specification and every file the portal's bundler embeds into it, added as each is read. */ +export class SpecBundle { + private readonly rootIds: ReadonlySet; + private readonly documents = new Map(); + private readonly named = new Map(); + + constructor(private readonly file: FilePath, private readonly document: OpenApiDocument) { + this.rootIds = document.schemaIds(); + this.add(file, document); + } + + /** Records `document` as read from `file`, and names the files it refers to. */ + public add(file: FilePath, document: OpenApiDocument): void { + this.documents.set(key(file), document); + for (const referenced of document.referencedFiles(file.directory(), this.rootIds)) { + if (!this.named.has(key(referenced))) { + this.named.set(key(referenced), { file: referenced, referencedBy: file }); + } + } + } + + /** The files named but not read yet, each with the first file to name it. */ + public unread(): ReferencedFile[] { + return [...this.named].flatMap(([name, referenced]) => (this.documents.has(name) ? [] : [referenced])); + } + + /** Its operations; a path item in another file is read from it, but not one that file refers on to. */ + public endpoints(): Endpoint[] { + const mounted = this.document + .pathItemReferences(this.file.directory()) + .flatMap(({ path, file, pointer }) => this.documents.get(key(file))?.endpointsAt(path, pointer) ?? []); + return [...this.document.endpoints(), ...mounted]; + } +} + +// By path rather than by `isEqual`: a split specification runs to thousands of files. +function key(file: FilePath): string { + return file.toString(); +} diff --git a/test/actions/portal/serve.test.ts b/test/actions/portal/serve.test.ts index 229087dd3..3e7b3375d 100644 --- a/test/actions/portal/serve.test.ts +++ b/test/actions/portal/serve.test.ts @@ -17,7 +17,7 @@ import { FileName } from '../../../src/types/file/fileName'; import { FilePath } from '../../../src/types/file/filePath'; import { UrlPath } from '../../../src/types/file/urlPath'; import { CommandMetadata } from '../../../src/types/common/command-metadata'; -import { PortalSource } from '../../../src/types/portal/portal-source'; +import { PortalSource, WatchedSource } from '../../../src/types/portal/portal-source'; import { Language } from '../../../src/types/sdk/generate'; import { PortalSourceContext } from '../../../src/types/portal-source-context'; import { ProjectContext } from '../../../src/types/project-context'; @@ -66,6 +66,16 @@ describe('PortalServeAction', () => { ); }); + /** The reason `report` gave each time it was called for `watched`. */ + const reasonsFor = (report: sinon.SinonStub, watched: WatchedSource) => + report.getCalls().flatMap(({ args: [about, reason] }) => (about === watched ? [reason] : [])); + + /** How many of the saves `shown` was called for it said fix one refused. */ + const fixesSaid = (shown: sinon.SinonStub) => shown.getCalls().filter(({ args: [{ fixed }] }) => fixed).length; + + /** How many saves of `apimatic.json` it said were applied. */ + const appliedSaid = () => prompts.configShown.getCalls().filter(({ args: [applied] }) => applied).length; + beforeEach(() => { root = fs.mkdtempSync(path.join(os.tmpdir(), 'portal-serve-')); @@ -270,14 +280,14 @@ describe('PortalServeAction', () => { it('says once that the content is fixed, and nothing for a save a build accepts', async () => { await whileServing(async (save) => { await save(JSON.stringify({ pages: ['intro'] })); - expect(prompts.contentRejected.called || prompts.contentAccepted.called).to.be.false; + expect(prompts.contentRejected.called || fixesSaid(prompts.contentShown) > 0).to.be.false; await save('{'); await save(JSON.stringify({ pages: ['intro'] })); await save(JSON.stringify({ pages: ['intro', '...'] })); expect(prompts.contentRejected.calledOnce).to.be.true; - expect(prompts.contentAccepted.calledOnce).to.be.true; + expect(fixesSaid(prompts.contentShown)).to.equal(1); }); }); @@ -305,8 +315,8 @@ describe('PortalServeAction', () => { await whileServing(async (save) => { await save(JSON.stringify({ title: 'Overview', pages: ['intro'] })); - expect(prompts.contentNotApplied.calledOnceWith('EACCES: permission denied')).to.be.true; - expect(prompts.contentNotices.called).to.be.false; + expect(reasonsFor(prompts.notApplied, 'content')).to.deep.equal(['EACCES: permission denied']); + expect(prompts.contentShown.called).to.be.false; }); }); @@ -316,9 +326,9 @@ describe('PortalServeAction', () => { await save(JSON.stringify({ title: 'Overview', pages: ['intro'] })); await save(JSON.stringify({ title: 'Overview', pages: ['intro', '...'] })); - const names = prompts.contentNotices + const names = prompts.contentShown .getCalls() - .map(({ args: [notices] }) => notices.sharedTabNames.map(({ name }) => name)); + .map(({ args: [{ notices }] }) => notices.sharedTabNames.map(({ name }) => name)); expect(names).to.deep.equal([['Overview'], []]); }); }); @@ -348,7 +358,7 @@ describe('PortalServeAction', () => { // What the watch does on a recheck: check the content as though it had just been saved. await (await watched).onChange(); - const [notices] = prompts.contentNotices.lastCall.args; + const [{ notices }] = prompts.contentShown.lastCall.args; expect(notices.sharedTabNames.map(({ name }) => name)).to.deep.equal(['Context Plugin']); await saveConfig(config); @@ -382,7 +392,7 @@ describe('PortalServeAction', () => { const result = await execute(new DirectoryPath(root)); - expect(prompts.contentNotWatched.calledOnceWith('EMFILE: too many open files')).to.be.true; + expect(reasonsFor(prompts.notWatched, 'content')).to.deep.equal(['EMFILE: too many open files']); expect(result.isCancelled()).to.be.true; }); @@ -390,7 +400,7 @@ describe('PortalServeAction', () => { await whileServing(async () => { (await watched).onFailed('EPERM: operation not permitted'); - expect(prompts.contentWatchFailed.calledOnceWith('EPERM: operation not permitted')).to.be.true; + expect(reasonsFor(prompts.watchFailed, 'content')).to.deep.equal(['EPERM: operation not permitted']); }); }); @@ -400,7 +410,7 @@ describe('PortalServeAction', () => { await whileServing(async (save) => { await save(JSON.stringify({ pages: ['intro'] })); - expect(prompts.contentNotChecked.calledOnceWith('EIO: i/o error')).to.be.true; + expect(reasonsFor(prompts.notChecked, 'content')).to.deep.equal(['EIO: i/o error']); }); }); @@ -483,7 +493,7 @@ describe('PortalServeAction', () => { await saved(); const [problem] = prompts.specRejected.firstCall.args; - expect(problem.kind === 'missingSpecReference' && problem.file.relativeTo(source)).to.equal( + expect(problem.kind === 'missingSpecReference' && problem.missing.file.relativeTo(source)).to.equal( 'spec/schemas/Pet.yaml' ); }); @@ -492,7 +502,7 @@ describe('PortalServeAction', () => { it('says once that the specifications are fixed, and nothing for a save a build accepts', async () => { await whileServing(async (saved) => { await saved(); - expect(prompts.specRejected.called || prompts.specAccepted.called).to.be.false; + expect(prompts.specRejected.called || fixesSaid(prompts.specsShown) > 0).to.be.false; halfWritten(); await saved(); @@ -501,7 +511,7 @@ describe('PortalServeAction', () => { await saved(); expect(prompts.specRejected.calledOnce).to.be.true; - expect(prompts.specAccepted.calledOnce).to.be.true; + expect(fixesSaid(prompts.specsShown)).to.equal(1); }); }); @@ -514,15 +524,16 @@ describe('PortalServeAction', () => { await saved(); await saved(); - expect(prompts.specsNeedRestart.calledOnce).to.be.true; - const [change] = prompts.specsNeedRestart.firstCall.args; + const changes = () => prompts.specsShown.getCalls().flatMap(({ args: [{ change }] }) => change ?? []); + expect(changes()).to.have.length(1); + const [change] = changes(); expect(change.added.map((file) => file.relativeTo(source))).to.deep.equal(['spec/orders.json']); expect(change.removed).to.be.empty; fs.rmSync(specFile('orders.json')); await saved(); - expect(prompts.specsNeedRestart.calledOnce).to.be.true; + expect(changes()).to.have.length(1); }); }); @@ -552,7 +563,7 @@ describe('PortalServeAction', () => { const result = await execute(new DirectoryPath(root)); - expect(prompts.specNotWatched.calledOnceWith('EMFILE: too many open files')).to.be.true; + expect(reasonsFor(prompts.notWatched, 'spec')).to.deep.equal(['EMFILE: too many open files']); expect(result.isCancelled()).to.be.true; }); @@ -560,7 +571,7 @@ describe('PortalServeAction', () => { await whileServing(async () => { (await watched).onFailed('EPERM: operation not permitted'); - expect(prompts.specWatchFailed.calledOnceWith('EPERM: operation not permitted')).to.be.true; + expect(reasonsFor(prompts.watchFailed, 'spec')).to.deep.equal(['EPERM: operation not permitted']); }); }); @@ -570,7 +581,7 @@ describe('PortalServeAction', () => { await whileServing(async (saved) => { await saved(); - expect(prompts.specNotChecked.calledOnceWith('EIO: i/o error')).to.be.true; + expect(reasonsFor(prompts.notChecked, 'spec')).to.deep.equal(['EIO: i/o error']); }); }); }); @@ -673,7 +684,7 @@ describe('PortalServeAction', () => { await (await watched).onChange(); expect(readProject('src/styles/theme.css')).to.contain('--color-fd-primary: #1d4ed8;'); - expect(prompts.configApplied.calledOnce).to.be.true; + expect(appliedSaid()).to.equal(1); }); }); @@ -687,7 +698,7 @@ describe('PortalServeAction', () => { expect(readProject('src/styles/theme.css')).to.contain('--color-fd-primary: #1d4ed8;'); expect(JSON.parse(readProject('portal.identity.json')).name).to.equal('Renamed API'); - expect(prompts.configApplied.calledOnce).to.be.true; + expect(appliedSaid()).to.equal(1); }); }); @@ -702,7 +713,7 @@ describe('PortalServeAction', () => { expect([readProject('portal.identity.json'), readProject('generated/sdks/typescript.mdx')]).to.deep.equal( before ); - expect(prompts.configApplied.called).to.be.false; + expect(appliedSaid()).to.equal(0); expect(prompts.configRejected.called).to.be.false; }); }); @@ -725,7 +736,7 @@ describe('PortalServeAction', () => { 'packageUrl="https://www.npmjs.com/package/calc"' ); expect(readProject('generated/sdks/index.mdx')).to.contain('install="npm install calc"'); - expect(prompts.configApplied.calledOnce).to.be.true; + expect(appliedSaid()).to.equal(1); }); }); @@ -750,11 +761,11 @@ describe('PortalServeAction', () => { }) ).to.be.true; expect(projectHas('generated/sdks/python.mdx')).to.be.false; - expect(prompts.configApplied.called).to.be.false; + expect(appliedSaid()).to.equal(0); await save(originalConfig()); - expect(prompts.configApplied.calledOnce).to.be.true; + expect(appliedSaid()).to.equal(1); }); }); @@ -776,7 +787,7 @@ describe('PortalServeAction', () => { expect(readProject('generated/context-plugin/index.mdx')).to.contain('Context Plugin'); expect(prompts.editNeedsRestart.called).to.be.false; - expect(prompts.configApplied.calledOnce).to.be.true; + expect(appliedSaid()).to.equal(1); }); }); @@ -790,7 +801,7 @@ describe('PortalServeAction', () => { await save(originalConfig()); expect(projectHas('generated/sdks/python.mdx')).to.be.false; - expect(prompts.configApplied.calledOnce).to.be.true; + expect(appliedSaid()).to.equal(1); }); }); }); @@ -823,11 +834,11 @@ describe('PortalServeAction', () => { await save(broken); expect(prompts.configRejected.firstCall.args[0].kind).to.equal('invalidConfig'); - expect(prompts.configApplied.called).to.be.false; + expect(appliedSaid()).to.equal(0); expect([readProject('portal.identity.json'), readProject('src/styles/theme.css')]).to.deep.equal(before); await save(originalConfig()); - expect(prompts.configApplied.calledOnce).to.be.true; + expect(appliedSaid()).to.equal(1); }); }); @@ -873,7 +884,7 @@ describe('PortalServeAction', () => { await save(config); expect(prompts.staticDirectoryNotServed.called).to.be.false; - expect(prompts.configApplied.calledOnce).to.be.true; + expect(appliedSaid()).to.equal(1); }); }); @@ -885,8 +896,8 @@ describe('PortalServeAction', () => { await save(config); - expect(prompts.configNotApplied.calledOnceWith('EACCES: permission denied')).to.be.true; - expect(prompts.configApplied.called).to.be.false; + expect(reasonsFor(prompts.notApplied, 'config')).to.deep.equal(['EACCES: permission denied']); + expect(appliedSaid()).to.equal(0); }); }); @@ -900,10 +911,10 @@ describe('PortalServeAction', () => { await save(config); - expect(prompts.configNotApplied.calledOnceWith('EBUSY: resource busy or locked')).to.be.true; + expect(reasonsFor(prompts.notChecked, 'config')).to.deep.equal(['EBUSY: resource busy or locked']); resolveSettings.restore(); await save(config); - expect(prompts.configApplied.calledOnce).to.be.true; + expect(appliedSaid()).to.equal(1); }); }); @@ -911,7 +922,7 @@ describe('PortalServeAction', () => { await whileServing(async () => { (await watched).onFailed('EPERM: operation not permitted'); - expect(prompts.configWatchFailed.calledOnceWith('EPERM: operation not permitted')).to.be.true; + expect(reasonsFor(prompts.watchFailed, 'config')).to.deep.equal(['EPERM: operation not permitted']); }); }); @@ -921,7 +932,7 @@ describe('PortalServeAction', () => { const result = await execute(new DirectoryPath(root)); - expect(prompts.configNotWatched.calledOnceWith('EMFILE: too many open files')).to.be.true; + expect(reasonsFor(prompts.notWatched, 'config')).to.deep.equal(['EMFILE: too many open files']); expect(result.isCancelled()).to.be.true; }); }); diff --git a/test/prompts/portal/serve.test.ts b/test/prompts/portal/serve.test.ts index dfb614a1a..379e79e7d 100644 --- a/test/prompts/portal/serve.test.ts +++ b/test/prompts/portal/serve.test.ts @@ -7,6 +7,7 @@ import { DirectoryPath } from '../../../src/types/file/directoryPath.js'; import { FileName } from '../../../src/types/file/fileName.js'; import { FilePath } from '../../../src/types/file/filePath.js'; import { UrlPath } from '../../../src/types/file/urlPath.js'; +import { ContentNotices } from '../../../src/types/portal/content-notices.js'; import { PortalSourceProblem } from '../../../src/types/portal/portal-source.js'; import { Language } from '../../../src/types/sdk/generate.js'; @@ -85,32 +86,32 @@ describe('PortalServePrompts', () => { const cases: [string, (prompts: PortalServePrompts) => void, RegExp][] = [ [ 'apimatic.json, at the start', - (prompts) => prompts.configNotWatched('EMFILE'), + (prompts) => prompts.notWatched('config', 'EMFILE', source), /^'apimatic\.json' cannot be watched \(EMFILE\), so edits to it need the preview restarted\.$/ ], [ 'apimatic.json, while serving', - (prompts) => prompts.configWatchFailed('EPERM'), - /^'apimatic\.json' is no longer watched \(EPERM\), so further edits to it need the preview restarted\.$/ + (prompts) => prompts.watchFailed('config', 'EPERM', source), + /^'apimatic\.json' is no longer watched \(EPERM\), so edits to it need the preview restarted\.$/ ], [ 'the content, at the start', - (prompts) => prompts.contentNotWatched('EMFILE', source), + (prompts) => prompts.notWatched('content', 'EMFILE', source), /^'.*content' cannot be watched \(EMFILE\), so a mistake in a page or a 'nav\.json' is only reported when the preview is restarted\.$/ ], [ 'the content, while serving', - (prompts) => prompts.contentWatchFailed('EPERM', source), + (prompts) => prompts.watchFailed('content', 'EPERM', source), /^'.*content' is no longer watched \(EPERM\), so a mistake in a page or a 'nav\.json' is only reported when the preview is restarted\.$/ ], [ 'the specifications, at the start', - (prompts) => prompts.specNotWatched('EMFILE', source), + (prompts) => prompts.notWatched('spec', 'EMFILE', source), /^'.*spec' cannot be watched \(EMFILE\), so a mistake in a specification is only reported when the preview is restarted\.$/ ], [ 'the specifications, while serving', - (prompts) => prompts.specWatchFailed('EPERM', source), + (prompts) => prompts.watchFailed('spec', 'EPERM', source), /^'.*spec' is no longer watched \(EPERM\), so a mistake in a specification is only reported when the preview is restarted\.$/ ] ]; @@ -122,6 +123,22 @@ describe('PortalServePrompts', () => { }); }); + describe('a change it accepted that could not be applied to the preview', () => { + const source = new DirectoryPath('src'); + + it('names apimatic.json, and why', () => { + expect(lastLine('warn', (prompts) => prompts.notApplied('config', 'EACCES', source))).to.equal( + "The changes to 'apimatic.json' could not be applied to the preview: EACCES" + ); + }); + + it('names the content directory, and why', () => { + expect(lastLine('warn', (prompts) => prompts.notApplied('content', 'EBUSY', source))).to.match( + /^The changes to '.*content' could not be applied to the preview: EBUSY$/ + ); + }); + }); + describe('a change to which documents are in the spec directory', () => { const document = (name: string) => new FilePath(new DirectoryPath('src', 'spec'), new FileName(name)); @@ -130,8 +147,8 @@ describe('PortalServePrompts', () => { const warn = sinon.stub(log, 'warn'); const message = sinon.stub(log, 'message'); try { - new PortalServePrompts().specsNeedRestart( - { added: added.map(document), removed: removed.map(document) }, + new PortalServePrompts().specsShown( + { fixed: false, change: { added: added.map(document), removed: removed.map(document) } }, new DirectoryPath('src') ); return [warn, message].map((stub) => stripVTControlCharacters(String(stub.firstCall.args[0]))); @@ -159,6 +176,55 @@ describe('PortalServePrompts', () => { }); }); + describe('a save the preview accepts', () => { + const NO_NOTICES: ContentNotices = { + hiddenPages: [], + ignoredNavigationFiles: [], + sharedTabNames: [], + noTabsListed: undefined, + unseenHomeTitle: undefined + }; + + /** What `show` says succeeded, without its colours. */ + const succeeded = (show: (prompts: PortalServePrompts, source: DirectoryPath) => void) => { + const success = sinon.stub(log, 'success'); + try { + show(new PortalServePrompts(), new DirectoryPath('src')); + return success.getCalls().map((call) => stripVTControlCharacters(String(call.args[0]))); + } finally { + success.restore(); + } + }; + + it('says the content or the specifications are fixed, when the save fixes one refused', () => { + const content = succeeded((prompts, source) => + prompts.contentShown({ fixed: true, notices: NO_NOTICES }, source) + ); + const specs = succeeded((prompts, source) => prompts.specsShown({ fixed: true, change: null }, source)); + + expect(content).to.have.length(1); + expect(content[0]).to.match(/^'.*content' is fixed; a build would accept it again\.$/); + expect(specs).to.have.length(1); + expect(specs[0]).to.match(/^'.*spec' is fixed; a build would accept it again\.$/); + }); + + it('says nothing of a save that fixes nothing', () => { + const content = succeeded((prompts, source) => + prompts.contentShown({ fixed: false, notices: NO_NOTICES }, source) + ); + const specs = succeeded((prompts, source) => prompts.specsShown({ fixed: false, change: null }, source)); + + expect([...content, ...specs]).to.be.empty; + }); + + it('says the changes to apimatic.json were applied only when the preview says to', () => { + expect(succeeded((prompts) => prompts.configShown(true))).to.deep.equal([ + "Applied the changes to 'apimatic.json'." + ]); + expect(succeeded((prompts) => prompts.configShown(false))).to.be.empty; + }); + }); + describe('an edit that needs artifacts the preview was started without', () => { const warned = (sdks: Language[], sdkDocs: Language[], plugin: boolean) => { const warn = sinon.stub(log, 'warn'); diff --git a/test/prompts/portal/source.test.ts b/test/prompts/portal/source.test.ts index eb0b10dc7..37f2d34d7 100644 --- a/test/prompts/portal/source.test.ts +++ b/test/prompts/portal/source.test.ts @@ -10,7 +10,7 @@ import { reportSourceProblem, reportUnseenHomeTitle } from '../../../src/prompts/portal/source.js'; -import { ContentProblem } from '../../../src/types/portal/portal-source.js'; +import { ContentProblem, UnreadableSpecReason } from '../../../src/types/portal/portal-source.js'; import { DirectoryPath } from '../../../src/types/file/directoryPath.js'; import { FileName } from '../../../src/types/file/fileName.js'; import { FilePath } from '../../../src/types/file/filePath.js'; @@ -191,33 +191,71 @@ describe('reportSourceProblem', () => { const api = new FilePath(spec, new FileName('api.yaml')); const pet = new FilePath(spec.join('schemas'), new FileName('Pet.json')); - it('names a document it cannot read, and why', () => { - const reason = + it('names a document it cannot read, and why, in the words of the parser that refused it', () => { + const message = 'Flow sequence in block collection must be sufficiently indented and end with a ] at line 3, column 1'; - reportSourceProblem({ kind: 'unreadableSpec', file: api, referencedBy: null, reason }, source); + reportSourceProblem( + { kind: 'unreadableSpec', file: api, referencedBy: null, reason: { kind: 'invalid', message } }, + source + ); - expect(printed()).to.equal(`'spec/api.yaml' could not be read: ${reason}.`); + expect(printed()).to.equal(`'spec/api.yaml' could not be read: ${message}.`); }); it('names the file that refers to one it cannot read', () => { - const reason = "Expected ',' or '}' after property value in JSON at position 13 (line 3 column 3)"; + const message = "Expected ',' or '}' after property value in JSON at position 13 (line 3 column 3)"; - reportSourceProblem({ kind: 'unreadableSpec', file: pet, referencedBy: api, reason }, source); + reportSourceProblem( + { kind: 'unreadableSpec', file: pet, referencedBy: api, reason: { kind: 'invalid', message } }, + source + ); expect(printed()).to.equal( - `'spec/schemas/Pet.json', which 'spec/api.yaml' refers to, could not be read: ${reason}.` + `'spec/schemas/Pet.json', which 'spec/api.yaml' refers to, could not be read: ${message}.` ); }); + const reasons: [UnreadableSpecReason, string][] = [ + [{ kind: 'empty' }, 'it is empty'], + [{ kind: 'noValue' }, 'it holds no value'], + [{ kind: 'noKey' }, 'it is not JSON, and the portal reads YAML only when it holds a key'], + [{ kind: 'gone' }, 'it is no longer there'], + [{ kind: 'unopened', message: 'EACCES: permission denied' }, 'EACCES: permission denied'] + ]; + + reasons.forEach(([reason, said]) => { + it(`says why a file could not be read, when it is ${reason.kind}`, () => { + reportSourceProblem({ kind: 'unreadableSpec', file: api, referencedBy: null, reason }, source); + + expect(printed()).to.equal(`'spec/api.yaml' could not be read: ${said}.`); + }); + }); + it('names a file referred to that is not there, and where a reference is resolved from', () => { - reportSourceProblem({ kind: 'missingSpecReference', file: pet, referencedBy: api }, source); + reportSourceProblem( + { kind: 'missingSpecReference', missing: { file: pet, foundAs: null }, referencedBy: api }, + source + ); expect(printed()).to.equal( "'spec/api.yaml' refers to 'spec/schemas/Pet.json', but there is no such file.\n" + "A '$ref' names a file from the folder of the file it is written in." ); }); + + it('names a file referred to that is there in another case, as it is spelt', () => { + const lowerCase = new FilePath(spec.join('schemas'), new FileName('pet.json')); + + reportSourceProblem( + { kind: 'missingSpecReference', missing: { file: lowerCase, foundAs: pet }, referencedBy: api }, + source + ); + + expect(printed()).to.equal( + "'spec/api.yaml' refers to 'spec/schemas/pet.json', but the file is spelt 'spec/schemas/Pet.json' on disk." + ); + }); }); describe("a page at an address kept for the portal's own pages", () => { diff --git a/test/types/portal-source-context.test.ts b/test/types/portal-source-context.test.ts index ea517f524..9c1843aff 100644 --- a/test/types/portal-source-context.test.ts +++ b/test/types/portal-source-context.test.ts @@ -386,11 +386,8 @@ describe('PortalSourceContext', () => { throw new Error(`expected a file the specifications are read from, got ${JSON.stringify(problem)}`); } const relative = (file: FilePath) => file.relativeTo(new DirectoryPath(root)); - return [ - problem.kind, - relative(problem.file), - problem.referencedBy === null ? null : relative(problem.referencedBy) - ]; + const file = problem.kind === 'missingSpecReference' ? problem.missing.file : problem.file; + return [problem.kind, relative(file), problem.referencedBy === null ? null : relative(problem.referencedBy)]; }; describe('resolveSpecs', () => { @@ -504,7 +501,9 @@ describe('PortalSourceContext', () => { const problem = (await resolve())._unsafeUnwrapErr(); expect(specProblem(problem)).to.deep.equal(['unreadableSpec', 'spec/broken.json', null]); - expect(problem.kind === 'unreadableSpec' && problem.reason).to.contain('JSON'); + expect( + problem.kind === 'unreadableSpec' && problem.reason.kind === 'invalid' && problem.reason.message + ).to.contain('JSON'); }); // As the portal's bundler reads it: by what it holds. @@ -535,6 +534,23 @@ describe('PortalSourceContext', () => { const problem = (await new PortalSourceContext(new DirectoryPath(root)).resolveSpecs())._unsafeUnwrapErr(); expect(specProblem(problem)).to.deep.equal(['unreadableSpec', 'spec/api.json', null]); + expect(problem.kind === 'unreadableSpec' && problem.reason).to.deep.equal({ kind: 'gone' }); + } finally { + sinon.restore(); + } + }); + + it('reports a document it cannot open with what the file system said', async () => { + write('spec/api.json', OPENAPI); + sinon.stub(FileService.prototype, 'getContents').rejects(new Error('EACCES: permission denied')); + + try { + const problem = (await new PortalSourceContext(new DirectoryPath(root)).resolveSpecs())._unsafeUnwrapErr(); + + expect(problem.kind === 'unreadableSpec' && problem.reason).to.deep.equal({ + kind: 'unopened', + message: 'EACCES: permission denied' + }); } finally { sinon.restore(); } @@ -624,6 +640,26 @@ describe('PortalSourceContext', () => { ]); }); + // As a file system that matches case, which the build may run on, would miss it. + it('names the file a missing reference is spelt as on disk, in another case', async () => { + writeSpec({ Pet: { $ref: './schemas/pet.yaml' } }); + write('spec/schemas/Pet.yaml', 'type: object\n'); + const named = sinon.match((file: FilePath) => file.toString() === path.join(root, 'spec', 'schemas', 'pet.yaml')); + sinon + .stub(FileService.prototype, 'getContents') + .callThrough() + .withArgs(named) + .rejects(new Error('ENOENT: no such file or directory')); + sinon.stub(FileService.prototype, 'fileExists').callThrough().withArgs(named).resolves(false); + + const problem = (await resolve())._unsafeUnwrapErr(); + + expect(specProblem(problem)).to.deep.equal(['missingSpecReference', 'spec/schemas/pet.yaml', 'spec/api.json']); + expect( + problem.kind === 'missingSpecReference' && problem.missing.foundAs?.relativeTo(new DirectoryPath(root)) + ).to.equal('spec/schemas/Pet.yaml'); + }); + it('refuses a file referred to in turn that does not parse, naming the file that refers to it', async () => { writeSpec({ Pet: { $ref: './schemas/Pet.yaml' } }); write('spec/schemas/Pet.yaml', 'type: object\nproperties:\n owner:\n $ref: ./Owner.yaml\n'); @@ -636,7 +672,9 @@ describe('PortalSourceContext', () => { 'spec/schemas/Owner.yaml', 'spec/schemas/Pet.yaml' ]); - expect(problem.kind === 'unreadableSpec' && problem.reason).to.match(/line \d+, column \d+$/); + expect(problem.kind === 'unreadableSpec' && problem.reason.kind === 'invalid' && problem.reason.message).to.match( + /line \d+, column \d+$/ + ); }); it('follows a reference out of the spec directory, as the build does', async () => { @@ -695,6 +733,11 @@ describe('PortalSourceContext', () => { ['a file of half-written JSON', () => writeReferenced('Pet.json', '{ "type": "obj'), 'empty'], ['a JSON file holding YAML', () => writeReferenced('Pet.json', 'type: object\n'), 'builds'], ['a YAML file that opens as JSON does', () => writeReferenced('Pet.yaml', '{ type: object }\n'), 'empty'], + [ + 'a YAML file that opens as JSON does only past its first 50 characters', + () => writeReferenced('Pet.yaml', `${' '.repeat(50)}{ type: object }\n`), + 'builds' + ], ['a file left empty', () => writeReferenced('Pet.yaml', ''), 'empty'], ['a file holding only a comment', () => writeReferenced('Pet.yaml', '# type: object\n'), 'empty'], [ diff --git a/test/types/portal/openapi-document.test.ts b/test/types/portal/openapi-document.test.ts index b985cf9df..1fdf6eb07 100644 --- a/test/types/portal/openapi-document.test.ts +++ b/test/types/portal/openapi-document.test.ts @@ -24,23 +24,25 @@ describe('OpenApiDocument', () => { // Matched loosely: the words are the parser's own, and change with its version. it('fails with what the parser said, on one line, saying where it stopped', () => { - const yaml = refused('openapi: 3.0.0\ninfo:\n title: [unclosed\n'); - const json = refused('{\n "a": 1\n "b": 2\n}'); - const quoting = refused('[1,\n2,\n,3]'); + const said = (contents: string) => { + const failure = refused(contents); + return failure.kind === 'invalid' ? failure.message : `not invalid but ${failure.kind}`; + }; + const yaml = said('openapi: 3.0.0\ninfo:\n title: [unclosed\n'); + const json = said('{\n "a": 1\n "b": 2\n}'); + const quoting = said('[1,\n2,\n,3]'); expect(yaml).to.match(/at line 4, column 1$/); expect(json).to.contain('line 3 column 3'); - [yaml, json, quoting].forEach((reason) => expect(reason).to.not.contain('\n')); + [yaml, json, quoting].forEach((message) => expect(message).to.not.contain('\n')); }); // The portal's bundler reads these as nothing, so a reference to one would be empty. - it('fails on text the bundler reads as nothing, saying why', () => { - expect(refused(' \n')).to.equal('it is empty'); - expect(refused('# only a comment: here\n')).to.equal('it holds no value'); - expect(refused('{ type: object }')).to.contain('JSON'); - expect(refused('- active\n- inactive\n')).to.equal( - 'it is not JSON, and the portal reads YAML only when it holds a key' - ); + it('fails on text the bundler reads as nothing, saying which kind it is', () => { + expect(refused(' \n')).to.deep.equal({ kind: 'empty' }); + expect(refused('# only a comment: here\n')).to.deep.equal({ kind: 'noValue' }); + expect(refused('{ type: object }').kind).to.equal('invalid'); + expect(refused('- active\n- inactive\n')).to.deep.equal({ kind: 'noKey' }); }); it('treats a document that is not an object as no specification', () => { diff --git a/test/types/portal/spec-bundle.test.ts b/test/types/portal/spec-bundle.test.ts new file mode 100644 index 000000000..56f41d15e --- /dev/null +++ b/test/types/portal/spec-bundle.test.ts @@ -0,0 +1,51 @@ +import { expect } from 'chai'; +import path from 'path'; +import { DirectoryPath } from '../../../src/types/file/directoryPath'; +import { FilePath } from '../../../src/types/file/filePath'; +import { OpenApiDocument } from '../../../src/types/portal/openapi-document'; +import { SpecBundle } from '../../../src/types/portal/spec-bundle'; + +describe('SpecBundle', () => { + const spec = new DirectoryPath(path.resolve('/project/src/spec')); + const at = (name: string) => FilePath.resolve(spec, name); + const parsed = (value: unknown) => OpenApiDocument.parse(JSON.stringify(value))._unsafeUnwrap(); + + /** Each file still to read, and the file that names it. */ + const unread = (bundle: SpecBundle) => + bundle + .unread() + .map(({ file, referencedBy }) => `${file.relativeTo(spec)}, named in ${referencedBy.relativeTo(spec)}`); + + it('lists each file named and not yet read once, with the first file to name it, and ends at a cycle', () => { + const bundle = new SpecBundle( + at('api.json'), + parsed({ Pet: { $ref: './Pet.json' }, Pets: { items: { $ref: './Pet.json#/Pet' } } }) + ); + expect(unread(bundle)).to.deep.equal(['Pet.json, named in api.json']); + + bundle.add(at('Pet.json'), parsed({ owner: { $ref: './Owner.json' }, root: { $ref: './api.json' } })); + expect(unread(bundle)).to.deep.equal(['Owner.json, named in Pet.json']); + + bundle.add(at('Owner.json'), parsed({ pet: { $ref: './Pet.json' } })); + expect(unread(bundle)).to.be.empty; + }); + + it('names what a file refers to from its own folder', () => { + const bundle = new SpecBundle(at('api.json'), parsed({ Pet: { $ref: './schemas/Pet.json' } })); + + bundle.add(at('schemas/Pet.json'), parsed({ owner: { $ref: './Owner.json' } })); + + expect(unread(bundle)).to.deep.equal(['schemas/Owner.json, named in schemas/Pet.json']); + }); + + it('lists the operations of a path item kept in another file after its own', () => { + const bundle = new SpecBundle( + at('api.json'), + parsed({ openapi: '3.0.0', paths: { '/pets': { $ref: './paths.json#/pets' }, '/health': { get: {} } } }) + ); + + bundle.add(at('paths.json'), parsed({ pets: { get: {}, post: {} } })); + + expect(bundle.endpoints().map(String)).to.deep.equal(['GET /health', 'GET /pets', 'POST /pets']); + }); +}); From 92910286216aa1275ce5dc58c5a4cbb7574dc7e0 Mon Sep 17 00:00:00 2001 From: saeedjamshaid Date: Fri, 16 Oct 2026 19:47:56 +0500 Subject: [PATCH 7/7] fix(portal): hold the spec check to the bundler as the #434 review found Fixes from mehnoorsiddiqui's review: - Every file a $ref names below src is checked against its spelling on disk, each folder listed once, so a $ref in the wrong case is refused on Windows and macOS as it is on Linux. - A file at the top of spec/ or of a merged folder that holds nothing, only comments or no key is skipped again, since the build never reads it. When every document there holds nothing, the first is reported as empty rather than pointed at a conversion. - Only http and https $refs are taken for URLs, as the bundler fetches only those. An empty $ref, and a bare # or an anchor in the document bundled, are refused as unresolvableSpecReference, and a $ref that names a folder says so. - While serving, a return to the documents the preview started with says no restart is needed. - SpecBundle keeps the files still to read, rather than scanning every file named at each level. - vite dev no longer lists the pages to prerender: the list bundled every spec at startup, so one the bundler fails on stopped the preview from starting. Co-Authored-By: Claude Opus 5.5 --- portal-template/vite.config.ts | 5 +- src/infrastructure/file-service.ts | 56 ++++++-- src/prompts/portal/serve.ts | 7 + src/prompts/portal/source.ts | 16 +++ src/types/portal-source-context.ts | 109 +++++++++++---- src/types/portal/openapi-document.ts | 33 +++-- src/types/portal/portal-source.ts | 8 +- src/types/portal/preview-specs.ts | 9 +- src/types/portal/spec-bundle.ts | 34 ++++- test/actions/portal/serve.test.ts | 6 +- test/e2e/portal-serve.test.ts | 52 +++++--- test/infrastructure/file-service.test.ts | 23 ++++ test/prompts/portal/serve.test.ts | 15 +++ test/prompts/portal/source.test.ts | 19 +++ test/types/portal-source-context.test.ts | 146 ++++++++++++++++++++- test/types/portal/openapi-document.test.ts | 31 ++++- test/types/portal/preview-specs.test.ts | 15 ++- test/types/portal/spec-bundle.test.ts | 15 +++ 18 files changed, 513 insertions(+), 86 deletions(-) diff --git a/portal-template/vite.config.ts b/portal-template/vite.config.ts index 27025c9d4..7209f2292 100644 --- a/portal-template/vite.config.ts +++ b/portal-template/vite.config.ts @@ -13,9 +13,10 @@ import { specReload } from './spec-reload.ts'; import { remarkImageReferences } from './src/lib/remark-image-references.ts'; import { staticFunctionsBase } from './static-functions-base.ts'; -export default defineConfig(async () => { +export default defineConfig(async ({ command, isPreview }) => { const [paths, identity] = await Promise.all([readBuildPaths(), readPortalIdentity()]); - const pages = await prerenderPages(paths, identity.siteUrl); + // `vite dev` prerenders nothing, and listing the pages bundles every spec: one that fails would stop it starting. + const pages = command === 'serve' && !isPreview ? [] : await prerenderPages(paths, identity.siteUrl); const publicDir: string | false = paths.staticDir ?? false; const portalProjectDirectory = fileURLToPath(new URL('.', import.meta.url)); diff --git a/src/infrastructure/file-service.ts b/src/infrastructure/file-service.ts index 011dfee32..d0a3650a2 100644 --- a/src/infrastructure/file-service.ts +++ b/src/infrastructure/file-service.ts @@ -18,6 +18,14 @@ function isDanglingLink(error: unknown): boolean { return code === 'ENOENT' || code === 'ENOTDIR'; } +interface DirectoryEntries { + fileNames: FileName[]; + subDirectories: DirectoryPath[]; +} + +/** Each folder's entries by its path, read once for the files of one call. */ +type Listings = Map>; + function spelling(name: string, entries: T[], nameOf: (entry: T) => string): T | undefined { return ( entries.find((entry) => nameOf(entry) === name) ?? @@ -165,21 +173,43 @@ export class FileService { // By code point first: Windows and macOS open `logo.png` for `Logo.PNG`, where a web server would not. public async spelledOnDisk(root: DirectoryPath, file: FilePath): Promise { - const directory = await this.directorySpelledOnDisk(root, file.directory()); - if (directory === null) { - return null; - } - const { fileNames } = await this.listEntries(directory); - const match = spelling(file.name().toString(), fileNames, (candidate) => candidate.toString()); - return match === undefined ? null : new FilePath(directory, match); + const [found] = await this.spellingsOnDisk(root, [file]); + return found; } - /** `directory`, at or below `root`, as each folder below `root` is spelt on disk; null when one is not there. */ + /** As `spelledOnDisk` for each of `files`, listing each folder once: a split specification runs to thousands. */ + public async spellingsOnDisk(root: DirectoryPath, files: FilePath[]): Promise<(FilePath | null)[]> { + const listings: Listings = new Map(); + return await Promise.all( + files.map(async (file) => { + const directory = await this.spelledDirectory(root, file.directory(), listings); + if (directory === null) { + return null; + } + const { fileNames } = await this.listOnce(directory, listings); + const match = spelling(file.name().toString(), fileNames, (candidate) => candidate.toString()); + return match === undefined ? null : new FilePath(directory, match); + }) + ); + } + + /** `directory` as each folder below `root` is spelt on disk; null when one is not there, or it is not below `root`. */ public async directorySpelledOnDisk(root: DirectoryPath, directory: DirectoryPath): Promise { + return await this.spelledDirectory(root, directory, new Map()); + } + + private async spelledDirectory( + root: DirectoryPath, + directory: DirectoryPath, + listings: Listings + ): Promise { + if (!root.contains(directory)) { + return null; + } const relative = path.relative(root.toString(), directory.toString()); let found = root; for (const name of relative === '' ? [] : relative.split(path.sep)) { - const { subDirectories } = await this.listEntries(found); + const { subDirectories } = await this.listOnce(found, listings); const match = spelling(name, subDirectories, (subDirectory) => path.basename(subDirectory.toString())); if (match === undefined) { return null; @@ -189,10 +219,16 @@ export class FileService { return found; } + private listOnce(dir: DirectoryPath, listings: Listings): Promise { + const listing = listings.get(dir.toString()) ?? this.listEntries(dir); + listings.set(dir.toString(), listing); + return listing; + } + // The direct children of `dir`, split into files and directories; both empty when it cannot // be read. A symlink counts as what it points at: skipping it silently drops an entry the // user did put there. - private async listEntries(dir: DirectoryPath): Promise<{ fileNames: FileName[]; subDirectories: DirectoryPath[] }> { + private async listEntries(dir: DirectoryPath): Promise { const fileNames: FileName[] = []; const subDirectories: DirectoryPath[] = []; try { diff --git a/src/prompts/portal/serve.ts b/src/prompts/portal/serve.ts index 2fb5290b0..ed074ddd0 100644 --- a/src/prompts/portal/serve.ts +++ b/src/prompts/portal/serve.ts @@ -138,6 +138,13 @@ export class PortalServePrompts { filesAre(change.added, 'now read', specDirectory), filesAre(change.removed, 'no longer read', specDirectory) ].filter((each) => each !== null); + if (changes.length === 0) { + log.success( + `The documents read from ${specPath(sourceDirectory)} are the ones the preview started with again; ` + + 'it no longer needs a restart.' + ); + return; + } log.warn( `The documents read from ${specPath(sourceDirectory)} are not the ones the preview started with: ` + `${changes.join(', and ')}.` diff --git a/src/prompts/portal/source.ts b/src/prompts/portal/source.ts index e785107d6..b0136e51a 100644 --- a/src/prompts/portal/source.ts +++ b/src/prompts/portal/source.ts @@ -109,6 +109,8 @@ function unreadableBecause(reason: UnreadableSpecReason): string { return 'it is not JSON, and the portal reads YAML only when it holds a key'; case 'gone': return 'it is no longer there'; + case 'folder': + return 'it is a folder'; case 'invalid': case 'unopened': return reason.message; @@ -182,6 +184,20 @@ export function reportSourceProblem(problem: PortalSourceProblem, sourceDirector log.message(`A ${f.var('$ref')} names a file from the folder of the file it is written in.`); return; } + case 'unresolvableSpecReference': { + const { file, ref } = problem.reference; + const writtenIn = relative(file, sourceDirectory); + log.error( + ref === '' + ? `${writtenIn} has an empty ${f.var('$ref')}.` + : `${writtenIn} has the ${f.var('$ref')} ${f.var(ref)}, which the portal cannot resolve.` + ); + log.message( + `A ${f.var('$ref')} names a part of the file it is in as ${f.var('#/components/schemas/Pet')}, ` + + `and a part of another as ${f.var('./pets.yaml#/Pet')}.` + ); + return; + } case 'unreadableMetadata': { log.error(`${relative(problem.file, sourceDirectory)} could not be read as JSON.`); return; diff --git a/src/types/portal-source-context.ts b/src/types/portal-source-context.ts index e31f88a10..8230a3cea 100644 --- a/src/types/portal-source-context.ts +++ b/src/types/portal-source-context.ts @@ -39,7 +39,7 @@ import { UnreadableSpecReason } from './portal/portal-source.js'; import { shared } from './portal/shared.js'; -import { SpecBundle } from './portal/spec-bundle.js'; +import { ReferencedFile, SpecBundle } from './portal/spec-bundle.js'; import { SpecMetadata } from './portal/spec-metadata.js'; import { SpecContext } from './spec-context.js'; import { TRANSFORMATIONS_DIRECTORY_NAME, transformedFileName } from './transform-context.js'; @@ -51,6 +51,12 @@ const definitionNames = (fileNames: FileName[]): FileName[] => const isDocumentName = (name: FileName): boolean => SPEC_EXTENSIONS.some((extension) => name.hasExtension(extension)); +const NO_DOCUMENT = new Set(['empty', 'noValue', 'noKey']); + +/** A file at the top of a folder that holds no document, which the build reads past as it does any file that is no API. */ +const holdsNoDocument = (problem: PortalSourceProblem): boolean => + problem.kind === 'unreadableSpec' && NO_DOCUMENT.has(problem.reason.kind); + type DiscoveredSpecs = Pick; /** An OpenAPI 3.x document at the top of a directory, read as a spec yet to be given its slug, and the site it suggests. */ @@ -65,6 +71,12 @@ interface ReadDirectory { definitions: FilePath[]; } +/** A document at the top of a directory in a format the portal does not read, which `api transform` converts. */ +interface OtherFormat { + file: FilePath; + format: string; +} + /** The specifications read from `spec/`, and the site the first of them suggests. */ interface ReadSpecs { specs: PortalSpec[]; @@ -616,11 +628,16 @@ export class PortalSourceContext { const definitions: FilePath[] = []; // What each one is, for the conversion a directory without an OpenAPI 3.x document is pointed to. - const otherFormats: { file: FilePath; format: string }[] = []; + const otherFormats: OtherFormat[] = []; + const noDocument: PortalSourceProblem[] = []; const documentNames = fileNames.filter(isDocumentName); for (const fileName of documentNames) { const file = new FilePath(directory, fileName); const read = await this.readDocument(file); + if (read.isErr() && holdsNoDocument(read.error)) { + noDocument.push(read.error); + continue; + } if (read.isErr()) { return err(read.error); } @@ -629,30 +646,41 @@ export class PortalSourceContext { definitions.push(file); } const format = document.format(); - if (!format.supported) { - if (format.format !== null) { - otherFormats.push({ file, format: format.format }); + if (format.supported) { + const bundle = await this.readBundle(file, document); + if (bundle.isErr()) { + return err(bundle.error); } - continue; - } - - const bundle = await this.readBundle(file, document); - if (bundle.isErr()) { - return err(bundle.error); + documents.push({ spec: { file, endpoints: bundle.value.endpoints() }, site: document.suggestedSite() }); + } else if (format.format !== null) { + otherFormats.push({ file, format: format.format }); } - documents.push({ spec: { file, endpoints: bundle.value.endpoints() }, site: document.suggestedSite() }); } - if (documents.length === 0) { - const [convertible] = otherFormats; - // No document declares its format: one the portal would read, by extension, is named before any other file. - const conversion = - convertible === undefined - ? this.conversion(new FilePath(directory, documentNames[0] ?? fileNames[0]), null, 0) - : this.conversion(convertible.file, convertible.format, otherFormats.length - 1); - return err({ kind: 'noOpenApiSpec', conversion }); - } - return ok({ documents, definitions }); + return documents.length > 0 + ? ok({ documents, definitions }) + : err(this.withoutOpenApi(directory, fileNames, documentNames, otherFormats, noDocument)); + } + + /** Why a directory has no OpenAPI 3.x document: one to convert, or the first that holds nothing when they all do. */ + private withoutOpenApi( + directory: DirectoryPath, + fileNames: FileName[], + documentNames: FileName[], + otherFormats: OtherFormat[], + noDocument: PortalSourceProblem[] + ): PortalSourceProblem { + // As a document caught empty mid-save is, rather than as one to convert. + if (noDocument.length > 0 && noDocument.length === documentNames.length) { + return noDocument[0]; + } + const [convertible] = otherFormats; + // No document declares its format: one the portal would read, by extension, is named before any other file. + const conversion = + convertible === undefined + ? this.conversion(new FilePath(directory, documentNames[0] ?? fileNames[0]), null, 0) + : this.conversion(convertible.file, convertible.format, otherFormats.length - 1); + return { kind: 'noOpenApiSpec', conversion }; } private conversion(file: FilePath, format: string | null, others: number): SpecConversion { @@ -694,7 +722,25 @@ export class PortalSourceContext { } unread.forEach((next, index) => bundle.add(next.file, read.value[index])); } - return ok(bundle); + const [unresolvable] = bundle.unresolvable(); + if (unresolvable !== undefined) { + return err({ kind: 'unresolvableSpecReference', reference: unresolvable }); + } + const misspelt = await this.misspeltReference(bundle.referenced()); + return misspelt === undefined ? ok(bundle) : err(misspelt); + } + + /** The first file read only as this file system ignores case, which a build where case matters would miss. */ + private async misspeltReference(referenced: ReferencedFile[]): Promise { + const files = referenced.map(({ file }) => file); + const spelt = await this.fileService.spellingsOnDisk(this.sourceDirectory, files); + // Null outside `src/`, whose folders are not the user's, or in no listing, as a name in another Unicode form is. + const index = spelt.findIndex((foundAs, at) => foundAs !== null && !foundAs.isEqual(files[at])); + if (index === -1) { + return undefined; + } + const { file, referencedBy } = referenced[index]; + return { kind: 'missingSpecReference', missing: { file, foundAs: spelt[index] }, referencedBy }; } private async readDocument( @@ -705,16 +751,12 @@ export class PortalSourceContext { try { contents = await this.fileService.getContents(file); } catch (error) { - const there = await this.fileService.fileExists(file); - if (!there && referencedBy !== null) { + const reason = await this.whyUnread(file, error); + if (reason.kind === 'gone' && referencedBy !== null) { // Null only for a name listed but not there as a file, as a dangling link is. const missing = (await this.missingFile(file)) ?? { file, foundAs: null }; return err({ kind: 'missingSpecReference', missing, referencedBy }); } - // A listed document gone is one caught mid-save, which the next save fixes. - const reason: UnreadableSpecReason = there - ? { kind: 'unopened', message: errorMessage(error) } - : { kind: 'gone' }; return err({ kind: 'unreadableSpec', file, referencedBy, reason }); } return OpenApiDocument.parse(contents).mapErr( @@ -722,6 +764,15 @@ export class PortalSourceContext { ); } + /** A listed document gone is one caught mid-save, which the next save fixes; a `$ref` may name a folder. */ + private async whyUnread(file: FilePath, error: unknown): Promise { + if (await this.fileService.fileExists(file)) { + return { kind: 'unopened', message: errorMessage(error) }; + } + const folder = file.directory().join(file.name().toString()); + return (await this.fileService.directoryExists(folder)) ? { kind: 'folder' } : { kind: 'gone' }; + } + private async readMetadata(file: FilePath): Promise { try { return SpecMetadata.parse(await this.fileService.getContents(file)); diff --git a/src/types/portal/openapi-document.ts b/src/types/portal/openapi-document.ts index 195c462db..9d2a2cc18 100644 --- a/src/types/portal/openapi-document.ts +++ b/src/types/portal/openapi-document.ts @@ -22,6 +22,13 @@ export type ParseFailure = | { kind: 'noKey' } | { kind: 'invalid'; message: string }; +/** What a document's `$ref`s name, as the portal's bundler reads them. */ +export interface DocumentReferences { + files: FilePath[]; + /** Each one, as written, that names no file and no part of a document by a JSON pointer: `''`, `'#'` or `'#pet'`. */ + unnamed: string[]; +} + /** A path item kept in another file, where `pointer` locates it; empty for the whole file. */ export interface PathItemReference { path: string; @@ -33,7 +40,8 @@ const DESCRIPTION_LIMIT = 300; const HTTP_METHODS = new Set(['get', 'put', 'post', 'delete', 'options', 'head', 'patch', 'trace']); -const URL_SCHEME = /^[a-z][a-z\d+.-]+:/i; +// The bundler fetches these, and reads any other `$ref`, `file:` and `urn:` included, as a path. +const FETCHED_PROTOCOLS = new Set(['http:', 'https:']); // How @scalar/json-magic reads each file it bundles: as JSON, else as YAML with these options when it holds a key. const YAML_OPTIONS = { merge: true, maxAliasCount: 10000 }; @@ -90,9 +98,10 @@ export class OpenApiDocument { return ids; } - /** The local files this document's `$ref`s name, as the portal's bundler finds them, given the root's `$id`s. */ - public referencedFiles(directory: DirectoryPath, rootIds: ReadonlySet): FilePath[] { + /** What this document's `$ref`s name, as the portal's bundler finds them, given the root's `$id`s. */ + public references(directory: DirectoryPath, rootIds: ReadonlySet): DocumentReferences { const names = new Set(); + const unnamed: string[] = []; walk(this.value, (node) => { // The bundler resolves what is below an `$id` against the `$id`, not the file. if (typeof node.$id === 'string') { @@ -102,10 +111,15 @@ export class OpenApiDocument { if (target === undefined) { return true; } - names.add(target.name); + if (target.name === '' && !target.pointer.startsWith('/')) { + unnamed.push(target.written); + } else { + names.add(target.name); + } return false; }); - return [...names].filter((name) => !rootIds.has(name)).flatMap((name) => localFile(directory, name) ?? []); + const files = [...names].filter((name) => !rootIds.has(name)).flatMap((name) => localFile(directory, name) ?? []); + return { files, unnamed }; } /** Every operation declared inline or behind a `$ref` into this document; see `pathItemReferences` for the rest. */ @@ -193,17 +207,18 @@ export class OpenApiDocument { } /** A node's `$ref`, split as the bundler splits it: the file it names, empty for its own, and the pointer into it. */ -function reference(node: unknown): { name: string; pointer: string } | undefined { +function reference(node: unknown): { written: string; name: string; pointer: string } | undefined { if (!isJsonObject(node) || typeof node.$ref !== 'string') { return undefined; } const [name, pointer = ''] = node.$ref.split('#'); - return { name, pointer }; + return { written: node.$ref, name, pointer }; } -/** The file a `$ref` names, resolved as the bundler resolves it; undefined for none, or a URL. */ +/** The file a `$ref` names, resolved as the bundler resolves it; undefined for none, or a URL it fetches. */ function localFile(directory: DirectoryPath, name: string): FilePath | undefined { - return name === '' || URL_SCHEME.test(name) ? undefined : FilePath.resolve(directory, name.replaceAll('\\', '/')); + const fetched = FETCHED_PROTOCOLS.has(URL.parse(name)?.protocol ?? ''); + return name === '' || fetched ? undefined : FilePath.resolve(directory, name.replaceAll('\\', '/')); } // A YAML error goes on to quote the lines around it, and V8 quotes the JSON it stopped at across line breaks. diff --git a/src/types/portal/portal-source.ts b/src/types/portal/portal-source.ts index ef419bc63..064b524f8 100644 --- a/src/types/portal/portal-source.ts +++ b/src/types/portal/portal-source.ts @@ -8,6 +8,7 @@ import { GeneratedPages } from './generated-pages.js'; import { ParseFailure } from './openapi-document.js'; import { PortalConfig } from './portal-config.js'; import { Shared } from './shared.js'; +import { UnresolvableReference } from './spec-bundle.js'; /** An OpenAPI document found in `src/spec/` or in a folder merged there, with the slug its section is mounted at. */ export interface PortalSpec { @@ -121,7 +122,11 @@ export interface SpecConversion { } /** Why a file a specification is read from could not be read; `unopened` carries what the file system said. */ -export type UnreadableSpecReason = ParseFailure | { kind: 'gone' } | { kind: 'unopened'; message: string }; +export type UnreadableSpecReason = + | ParseFailure + | { kind: 'gone' } + | { kind: 'folder' } + | { kind: 'unopened'; message: string }; /** Why a source directory cannot be built; each variant maps to its own message. */ export type PortalSourceProblem = @@ -132,6 +137,7 @@ export type PortalSourceProblem = | { kind: 'invalidContent'; problems: ContentProblem[] } | { kind: 'unreadableSpec'; file: FilePath; referencedBy: FilePath | null; reason: UnreadableSpecReason } | { kind: 'missingSpecReference'; missing: MissingFile; referencedBy: FilePath } + | { kind: 'unresolvableSpecReference'; reference: UnresolvableReference } | { kind: 'unreadableMetadata'; file: FilePath } // Its `folders` are not read unless a metadata file merges them, and `api transform` writes into one of its own. | { kind: 'emptySpecDirectory'; folders: DirectoryPath[] } diff --git a/src/types/portal/preview-specs.ts b/src/types/portal/preview-specs.ts index 5c249b1b7..bf3e19406 100644 --- a/src/types/portal/preview-specs.ts +++ b/src/types/portal/preview-specs.ts @@ -8,7 +8,10 @@ export interface SpecsChange { removed: FilePath[]; } -/** What to say of a save the preview accepts: whether it fixes a refused one, and a change not yet warned of. */ +/** + * What to say of a save the preview accepts: whether it fixes a refused one, and a change not yet said, which + * adds and removes nothing once the documents are the ones the preview started with again. + */ export interface SpecsShown { fixed: boolean; change: SpecsChange | null; @@ -33,10 +36,10 @@ export class PreviewSpecs { /** Records the documents as shown, and answers what to say of them. */ public show(specs: PortalSpec[]): SpecsShown { const files = specs.map(({ file }) => file); - const unchanged = isSameSet(files, this.shown) || isSameSet(files, this.served); + const said = isSameSet(files, this.shown); this.shown = files; const change = { added: notIn(files, this.served), removed: notIn(this.served, files) }; - return { fixed: this.refusal.accept(), change: unchanged ? null : change }; + return { fixed: this.refusal.accept(), change: said ? null : change }; } } diff --git a/src/types/portal/spec-bundle.ts b/src/types/portal/spec-bundle.ts index 68e72d2d4..585e4e290 100644 --- a/src/types/portal/spec-bundle.ts +++ b/src/types/portal/spec-bundle.ts @@ -8,11 +8,20 @@ export interface ReferencedFile { referencedBy: FilePath; } +/** A `$ref` the portal's bundler fails on, as written, with the file it is written in. */ +export interface UnresolvableReference { + file: FilePath; + ref: string; +} + /** A specification and every file the portal's bundler embeds into it, added as each is read. */ export class SpecBundle { private readonly rootIds: ReadonlySet; private readonly documents = new Map(); private readonly named = new Map(); + // Those of `named` not yet read, so a chain of files a level apiece is not scanned whole at each level. + private pending: ReferencedFile[] = []; + private readonly unresolved: UnresolvableReference[] = []; constructor(private readonly file: FilePath, private readonly document: OpenApiDocument) { this.rootIds = document.schemaIds(); @@ -22,16 +31,35 @@ export class SpecBundle { /** Records `document` as read from `file`, and names the files it refers to. */ public add(file: FilePath, document: OpenApiDocument): void { this.documents.set(key(file), document); - for (const referenced of document.referencedFiles(file.directory(), this.rootIds)) { + const { files, unnamed } = document.references(file.directory(), this.rootIds); + for (const referenced of files) { if (!this.named.has(key(referenced))) { - this.named.set(key(referenced), { file: referenced, referencedBy: file }); + const named = { file: referenced, referencedBy: file }; + this.named.set(key(referenced), named); + this.pending.push(named); } } + // An empty `$ref` fails anywhere; `#` and `#pet` only in the root, as the bundler makes an embedded file's pointers. + const isRoot = file.isEqual(this.file); + for (const ref of unnamed.filter((written) => isRoot || written === '')) { + this.unresolved.push({ file, ref }); + } + } + + /** The `$ref`s read so far that name no file the bundler can read, in the order read. */ + public unresolvable(): UnresolvableReference[] { + return [...this.unresolved]; } /** The files named but not read yet, each with the first file to name it. */ public unread(): ReferencedFile[] { - return [...this.named].flatMap(([name, referenced]) => (this.documents.has(name) ? [] : [referenced])); + this.pending = this.pending.filter(({ file }) => !this.documents.has(key(file))); + return [...this.pending]; + } + + /** Every file named, each with the first file to name it. */ + public referenced(): ReferencedFile[] { + return [...this.named.values()]; } /** Its operations; a path item in another file is read from it, but not one that file refers on to. */ diff --git a/test/actions/portal/serve.test.ts b/test/actions/portal/serve.test.ts index 3e7b3375d..60f2ba9fb 100644 --- a/test/actions/portal/serve.test.ts +++ b/test/actions/portal/serve.test.ts @@ -515,7 +515,7 @@ describe('PortalServeAction', () => { }); }); - it('warns once that a document added needs a restart, and not when it is taken out again', async () => { + it('warns once that a document added needs a restart, and says once that nothing differs when it is taken out', async () => { await whileServing(async (saved) => { writeSpec( 'orders.json', @@ -532,8 +532,10 @@ describe('PortalServeAction', () => { fs.rmSync(specFile('orders.json')); await saved(); + await saved(); - expect(changes()).to.have.length(1); + expect(changes()).to.have.length(2); + expect(changes()[1]).to.deep.equal({ added: [], removed: [] }); }); }); diff --git a/test/e2e/portal-serve.test.ts b/test/e2e/portal-serve.test.ts index 5858c28a7..d9838bf97 100644 --- a/test/e2e/portal-serve.test.ts +++ b/test/e2e/portal-serve.test.ts @@ -22,19 +22,22 @@ const enabled = process.env.APIMATIC_E2E === '1'; this.timeout(10 * 60 * 1000); const fixture = new DirectoryPath(process.cwd()).join('test/resources/portal-inputs/default/src'); + const specFile = (source: DirectoryPath) => path.join(source.toString(), 'spec', 'Apimatic-Calculator.json'); + const reference = '/api/apimatic-calculator/simple-calculator/Calculate.md'; let base: string | undefined; - let root: string | undefined; + const roots: string[] = []; let sourceDirectory: DirectoryPath; let server: PortalDevServer | undefined; - before(async () => { - base = await ensurePortalProjectDirectoryBase(fixture); - root = fs.mkdtempSync(path.join(base, 'portal-serve-e2e-')); - // A copy, since a test edits the source while the preview reads it. - sourceDirectory = new DirectoryPath(root).join('src'); - fs.cpSync(fixture.toString(), sourceDirectory.toString(), { recursive: true }); + /** A preview of a copy of the fixture as `edit` leaves it: a copy, since a test edits the source while it is read. */ + const startPreview = async (edit: (source: DirectoryPath) => void = () => undefined) => { + const root = fs.mkdtempSync(path.join(base!, 'portal-serve-e2e-')); + roots.push(root); + const copy = new DirectoryPath(root).join('src'); + fs.cpSync(fixture.toString(), copy.toString(), { recursive: true }); + edit(copy); - const source = (await new PortalSourceContext(sourceDirectory).resolve())._unsafeUnwrap(); + const source = (await new PortalSourceContext(copy).resolve())._unsafeUnwrap(); const delivered = path.join(root, 'delivered'); fs.mkdirSync(delivered, { recursive: true }); fs.writeFileSync(path.join(delivered, 'typescript.zip'), 'PK typescript'); @@ -54,18 +57,23 @@ const enabled = process.env.APIMATIC_E2E === '1'; if (started.isErr()) { throw new Error(`${started.error.message}\n${started.error.log.split('\n').slice(-20).join('\n')}`); } - server = started.value; + return { sourceDirectory: copy, server: started.value }; + }; + + before(async () => { + base = await ensurePortalProjectDirectoryBase(fixture); + ({ sourceDirectory, server } = await startPreview()); }); after(async () => { await server?.stop(); - if (root !== undefined) fs.rmSync(root, { recursive: true, force: true }); + roots.forEach((root) => fs.rmSync(root, { recursive: true, force: true })); if (base !== undefined) await removePortalProjectDirectoryBase(base); }); // Under `vite dev` a page that is not there is answered 200 too, so a Markdown twin is told by its type. - const servesMarkdown = async (address: string): Promise => { - const response = await globalThis.fetch(`${server?.url.toString()}${address}`); + const servesMarkdown = async (address: string, from = server): Promise => { + const response = await globalThis.fetch(`${from?.url.toString()}${address}`); await response.arrayBuffer(); return response.ok && (response.headers.get('content-type') ?? '').startsWith('text/markdown'); }; @@ -83,7 +91,7 @@ const enabled = process.env.APIMATIC_E2E === '1'; // A missing file answers 200 too, with the fallback page, so the type says what came back. it('serves a dependency file asked for before its importer (apimatic-io#2287)', async () => { - const installed = fs.realpathSync(path.join(root!, 'build', 'node_modules', '@tanstack', 'react-start')); + const installed = fs.realpathSync(path.join(roots[0], 'build', 'node_modules', '@tanstack', 'react-start')); const entry = path.join(installed, 'dist', 'plugin', 'default-entry', 'client.tsx').split(path.sep).join('/'); const response = await globalThis.fetch(`${server?.url.toString()}/@fs/${entry}`); @@ -105,9 +113,8 @@ const enabled = process.env.APIMATIC_E2E === '1'; }); it('leaves out the reference of a specification saved half written, serving the rest, until it is fixed', async () => { - const spec = path.join(sourceDirectory.toString(), 'spec', 'Apimatic-Calculator.json'); + const spec = specFile(sourceDirectory); const written = fs.readFileSync(spec, 'utf8'); - const reference = '/api/apimatic-calculator/simple-calculator/Calculate.md'; expect(await servesMarkdown(reference), 'the reference before the save').to.be.true; fs.writeFileSync(spec, written.slice(0, written.length / 2)); @@ -120,4 +127,19 @@ const enabled = process.env.APIMATIC_E2E === '1'; expect(await servesMarkdownOnceIt(reference, true), 'the reference once fixed').to.be.true; }); + + // A `$ref` below an `$id` is resolved against the `$id`, so the check leaves it to the bundler, which fails on it. + it('starts with a specification the bundler fails on, serving the rest without its reference', async () => { + const preview = await startPreview((source) => { + const document = JSON.parse(fs.readFileSync(specFile(source), 'utf8')); + document.components.schemas.Tagged = { $id: 'tagged.json', properties: { tag: { $ref: './tag.yaml' } } }; + fs.writeFileSync(specFile(source), JSON.stringify(document)); + }); + try { + expect(await servesMarkdown('/authentication.md', preview.server), 'a Markdown page').to.be.true; + expect(await servesMarkdown(reference, preview.server), 'the reference').to.be.false; + } finally { + await preview.server.stop(); + } + }); }); diff --git a/test/infrastructure/file-service.test.ts b/test/infrastructure/file-service.test.ts index 81aa79ec0..67944017b 100644 --- a/test/infrastructure/file-service.test.ts +++ b/test/infrastructure/file-service.test.ts @@ -116,6 +116,7 @@ describe('FileService', () => { }); afterEach(() => { + sinon.restore(); fs.rmSync(root.toString(), { recursive: true, force: true }); }); @@ -151,6 +152,28 @@ describe('FileService', () => { expect((await fileService.directorySpelledOnDisk(root, root))?.isEqual(root)).to.be.true; expect(await fileService.directorySpelledOnDisk(root, root.join('static', 'logos'))).to.be.null; }); + + it('answers null for a directory that is not below the root', async () => { + expect(await fileService.directorySpelledOnDisk(root.join('static'), root)).to.be.null; + }); + + it('answers for several files, reading each folder once', async () => { + fs.writeFileSync(path.join(root.toString(), 'static', 'images', 'icon.png'), ''); + const readdir = sinon.spy(fsExtra, 'readdir'); + + const found = await fileService.spellingsOnDisk(root, [ + fileAt('static', 'images', 'Logo.png'), + fileAt('static', 'images', 'icon.png'), + fileAt('static', 'images', 'banner.png') + ]); + + expect(found.map((file) => file?.relativeTo(root) ?? null)).to.deep.equal([ + 'static/images/logo.png', + 'static/images/icon.png', + null + ]); + expect(readdir.callCount).to.equal(3); + }); }); describe('findSymlinks', () => { diff --git a/test/prompts/portal/serve.test.ts b/test/prompts/portal/serve.test.ts index 301d009f6..80fc1f294 100644 --- a/test/prompts/portal/serve.test.ts +++ b/test/prompts/portal/serve.test.ts @@ -175,6 +175,21 @@ describe('PortalServePrompts', () => { ); }); + it('says when the documents are the ones it started with again, which needs no restart', () => { + const success = sinon.stub(log, 'success'); + const warn = sinon.stub(log, 'warn'); + + new PortalServePrompts().specsShown( + { fixed: false, change: { added: [], removed: [] } }, + new DirectoryPath('src') + ); + + expect(warn.called).to.be.false; + expect(stripVTControlCharacters(String(success.firstCall.args[0]))).to.match( + /^The documents read from '.*spec' are the ones the preview started with again; it no longer needs a restart\.$/ + ); + }); + // Each folder `APIMATIC-META.json` merges may hold an `openapi.yaml`. it('names a document in a folder by its path from the spec directory', () => { expect(said(['payments/openapi.yaml'], ['orders/openapi.yaml'])[0]).to.match( diff --git a/test/prompts/portal/source.test.ts b/test/prompts/portal/source.test.ts index 8fbef99fe..62fe5069f 100644 --- a/test/prompts/portal/source.test.ts +++ b/test/prompts/portal/source.test.ts @@ -349,6 +349,7 @@ describe('reportSourceProblem', () => { [{ kind: 'noValue' }, 'it holds no value'], [{ kind: 'noKey' }, 'it is not JSON, and the portal reads YAML only when it holds a key'], [{ kind: 'gone' }, 'it is no longer there'], + [{ kind: 'folder' }, 'it is a folder'], [{ kind: 'unopened', message: 'EACCES: permission denied' }, 'EACCES: permission denied'] ]; @@ -384,6 +385,24 @@ describe('reportSourceProblem', () => { "'spec/api.yaml' refers to 'spec/schemas/pet.json', but the file is spelt 'spec/schemas/Pet.json' on disk." ); }); + + const PART_NAMED = + "A '$ref' names a part of the file it is in as '#/components/schemas/Pet', and a part of another as " + + "'./pets.yaml#/Pet'."; + + it('names the file a $ref the build cannot resolve is in, and how to name a part of a file', () => { + reportSourceProblem({ kind: 'unresolvableSpecReference', reference: { file: api, ref: '#pet' } }, source); + + expect(printed()).to.equal( + `'spec/api.yaml' has the '$ref' '#pet', which the portal cannot resolve.\n${PART_NAMED}` + ); + }); + + it('says a $ref is empty rather than quoting it', () => { + reportSourceProblem({ kind: 'unresolvableSpecReference', reference: { file: pet, ref: '' } }, source); + + expect(printed()).to.equal(`'spec/schemas/Pet.json' has an empty '$ref'.\n${PART_NAMED}`); + }); }); describe("a page at an address kept for the portal's own pages", () => { diff --git a/test/types/portal-source-context.test.ts b/test/types/portal-source-context.test.ts index 3d0f75a49..7b55d3e59 100644 --- a/test/types/portal-source-context.test.ts +++ b/test/types/portal-source-context.test.ts @@ -2,6 +2,7 @@ import fs from 'fs'; import os from 'os'; import path from 'path'; import { Buffer } from 'node:buffer'; +import { pathToFileURL } from 'node:url'; import Ajv from 'ajv'; import { expect } from 'chai'; import { Result } from 'neverthrow'; @@ -519,15 +520,25 @@ describe('PortalSourceContext', () => { expect((await resolve())._unsafeUnwrap().specs.map((spec) => spec.slug)).to.deep.equal(['api']); }); - it('reports a document left empty, as it does one half written', async () => { + // The build reads past a file that is no API, and never bundles it. + it('skips a file holding nothing, only comments, or no key beside a document', async () => { write('spec/api.json', OPENAPI); write('spec/orders.yaml', ''); + write('spec/notes.yaml', '# orders: to come\n'); + write('spec/states.yaml', '- active\n- inactive\n'); - expect(specProblem((await resolve())._unsafeUnwrapErr())).to.deep.equal([ - 'unreadableSpec', - 'spec/orders.yaml', - null - ]); + expect((await resolve())._unsafeUnwrap().specs.map((spec) => spec.slug)).to.deep.equal(['api']); + }); + + // As a document caught empty mid-save is; with nothing else to build from, it is not one to convert. + it('reports a document left empty when every document there holds nothing', async () => { + write('spec/api.yaml', ''); + write('spec/notes.yaml', '# to come\n'); + + const problem = (await resolve())._unsafeUnwrapErr(); + + expect(specProblem(problem)).to.deep.equal(['unreadableSpec', 'spec/api.yaml', null]); + expect(problem.kind === 'unreadableSpec' && problem.reason).to.deep.equal({ kind: 'empty' }); }); // As an editor that saves by renaming over the file leaves it for a moment. @@ -896,6 +907,22 @@ describe('PortalSourceContext', () => { ]); }); + it('skips a file holding nothing beside the definition of a folder, and reports one alone', async () => { + merging(); + write('spec/alpha/openapi.json', OPENAPI); + write('spec/alpha/notes.yaml', ''); + + expect(await read()).to.deep.equal([['alpha', 'spec/alpha/openapi.json']]); + + fs.rmSync(path.join(root, 'spec', 'alpha', 'openapi.json')); + + expect(specProblem((await resolve())._unsafeUnwrapErr())).to.deep.equal([ + 'unreadableSpec', + 'spec/alpha/notes.yaml', + null + ]); + }); + it('reports a metadata file it cannot parse', async () => { write('spec/APIMATIC-META.json', '{ not json'); write('spec/alpha/openapi.json', OPENAPI); @@ -1011,6 +1038,48 @@ describe('PortalSourceContext', () => { ).to.equal('spec/schemas/Pet.yaml'); }); + // Read here on a file system that ignores case, as Windows and macOS do; a build on Linux would miss it. + it('refuses a file or a folder named in another case than on disk, wherever it reads', async () => { + write('spec/schemas/Pet.yaml', 'type: object\n'); + const foundAs = async () => { + const problem = (await resolve())._unsafeUnwrapErr(); + expect(specProblem(problem)[0]).to.equal('missingSpecReference'); + return problem.kind === 'missingSpecReference' + ? problem.missing.foundAs?.relativeTo(new DirectoryPath(root)) + : null; + }; + + writeSpec({ Pet: { $ref: './schemas/pet.yaml' } }); + expect(await foundAs()).to.equal('spec/schemas/Pet.yaml'); + + writeSpec({ Pet: { $ref: './Schemas/Pet.yaml' } }); + expect(await foundAs()).to.equal('spec/schemas/Pet.yaml'); + }); + + // As a name stored in another Unicode form, or a folder that cannot be listed, is. + it('accepts a file it reads but finds in no listing, rather than calling it misspelt', async () => { + writeSpec({ Pet: { $ref: './schemas/Pet.yaml' } }); + write('spec/schemas/Pet.yaml', 'type: object\n'); + sinon + .stub(FileService.prototype, 'spellingsOnDisk') + .callsFake(async (_root: DirectoryPath, files: FilePath[]) => files.map(() => null)); + + expect((await resolve()).isOk()).to.be.true; + }); + + // Its folders above the source directory are where the build runs, not the user's to spell. + it('accepts a file outside the source directory', async () => { + const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'portal-shared-')); + try { + fs.writeFileSync(path.join(outside, 'Pet.yaml'), 'type: object\n'); + writeSpec({ Pet: { $ref: path.relative(path.join(root, 'spec'), path.join(outside, 'Pet.yaml')) } }); + + expect((await resolve()).isOk()).to.be.true; + } finally { + fs.rmSync(outside, { recursive: true, force: true }); + } + }); + it('refuses a file referred to in turn that does not parse, naming the file that refers to it', async () => { writeSpec({ Pet: { $ref: './schemas/Pet.yaml' } }); write('spec/schemas/Pet.yaml', 'type: object\nproperties:\n owner:\n $ref: ./Owner.yaml\n'); @@ -1028,6 +1097,29 @@ describe('PortalSourceContext', () => { ); }); + it('says a reference that names a folder names one', async () => { + writeSpec({ Pet: { $ref: './schemas/' } }); + write('spec/schemas/Pet.yaml', 'type: object\n'); + + const problem = (await resolve())._unsafeUnwrapErr(); + + expect(specProblem(problem)).to.deep.equal(['unreadableSpec', 'spec/schemas', 'spec/api.json']); + expect(problem.kind === 'unreadableSpec' && problem.reason).to.deep.equal({ kind: 'folder' }); + }); + + it('refuses an empty reference, naming the file it is written in', async () => { + writeSpec({ Pet: { $ref: './schemas/Pet.yaml' } }); + write('spec/schemas/Pet.yaml', 'properties:\n owner:\n $ref: ""\n'); + + const problem = (await resolve())._unsafeUnwrapErr(); + if (problem.kind !== 'unresolvableSpecReference') { + expect.fail(`expected a $ref the build cannot resolve, got '${problem.kind}'`); + } + + const { file, ref } = problem.reference; + expect([file.relativeTo(new DirectoryPath(root)), ref]).to.deep.equal(['spec/schemas/Pet.yaml', '']); + }); + it('follows a reference out of the spec directory, as the build does', async () => { writeSpec({ Pet: { $ref: '../shared/Pet.yaml' } }); @@ -1170,6 +1262,48 @@ describe('PortalSourceContext', () => { 'a file named only under the keys the bundler writes', () => writeSpec({}, { 'x-ext': { abc123: { $ref: './embedded.yaml' } } }), 'builds' + ], + ['an empty reference', () => writeSpec({ Pet: { $ref: '' } }), 'fails'], + [ + 'an empty reference in a file named', + () => writeReferenced('Pet.yaml', 'properties:\n owner:\n $ref: ""\n'), + 'fails' + ], + ['a bare # in the document bundled', () => writeSpec({ Pet: { $ref: '#' } }), 'fails'], + [ + 'a schema named by its $anchor in the document bundled', + () => writeSpec({ Pet: { $anchor: 'pet', type: 'object' }, Pets: { items: { $ref: '#pet' } } }), + 'fails' + ], + [ + 'a bare # or an anchor in a file named, which the bundler turns into pointers', + () => writeReferenced('Pet.yaml', 'properties:\n self:\n $ref: "#"\n tag:\n $ref: "#tag"\n'), + 'builds' + ], + [ + 'a file named by a file: URL, which the bundler reads as a path', + () => { + writeSpec({ Pet: { $ref: pathToFileURL(path.join(root, 'spec', 'Pet.yaml')).href } }); + write('spec/Pet.yaml', 'type: object\n'); + }, + 'fails' + ], + ['a name with another scheme', () => writeSpec({ Pet: { $ref: 'urn:pet' } }), 'fails'], + [ + 'a folder', + () => { + writeSpec({ Pet: { $ref: './schemas' } }); + write('spec/schemas/Pet.yaml', 'type: object\n'); + }, + 'fails' + ], + [ + 'a part of a file that is not there, which the page renders without', + () => { + writeSpec({ Pet: { $ref: './Pet.yaml#/nope' } }); + write('spec/Pet.yaml', 'type: object\n'); + }, + 'builds' ] ]; diff --git a/test/types/portal/openapi-document.test.ts b/test/types/portal/openapi-document.test.ts index 772ca9e42..f259fb78e 100644 --- a/test/types/portal/openapi-document.test.ts +++ b/test/types/portal/openapi-document.test.ts @@ -241,10 +241,10 @@ describe('OpenApiDocument', () => { }); }); - describe('referencedFiles', () => { + describe('references', () => { const specDirectory = new DirectoryPath('/project/src/spec'); const referenced = (document: unknown, rootIds = new Set()) => - readJson(document).referencedFiles(specDirectory, rootIds).map(String); + readJson(document).references(specDirectory, rootIds).files.map(String); const inSpec = (file: string) => path.resolve('/project/src/spec', file); it('finds a $ref on any object, in data, extensions and lists too, against the folder of the document', () => { @@ -283,6 +283,31 @@ describe('OpenApiDocument', () => { expect(files).to.be.empty; }); + // The bundler fetches only these, and reads `C:/shared/pet.yaml` as a path, as it does any other scheme. + it('takes only http and https for URLs, and any other scheme for the name of a file', () => { + const files = referenced({ + a: { $ref: 'file:///shared/pet.yaml' }, + b: { $ref: 'urn:pet' }, + c: { $ref: 'ab:c.yaml' }, + d: { $ref: 'http://example.com/pet.yaml' } + }); + + expect(files).to.deep.equal([inSpec('file:///shared/pet.yaml'), inSpec('urn:pet'), inSpec('ab:c.yaml')]); + }); + + it('sets apart, as written, each $ref that names no file and no part of a document by a pointer', () => { + const { files, unnamed } = readJson({ + empty: { $ref: '' }, + whole: { $ref: '#' }, + anchor: { $ref: '#pet' }, + pointer: { $ref: '#/components/schemas/Pet' }, + root: { $ref: '#/' } + }).references(specDirectory, new Set()); + + expect(files).to.be.empty; + expect(unnamed).to.deep.equal(['', '#', '#pet']); + }); + it('reads no further below an $id, against which the bundler resolves what is under it', () => { expect(referenced({ Pet: { $id: 'pet.json', properties: { tag: { $ref: './tag.yaml' } } } })).to.be.empty; }); @@ -312,7 +337,7 @@ describe('OpenApiDocument', () => { it('survives an alias that holds itself', () => { const document = read('openapi: 3.0.0\nloop: &loop\n again: *loop\n next: { $ref: ./next.yaml }\n'); - expect(document.referencedFiles(specDirectory, new Set()).map(String)).to.deep.equal([inSpec('next.yaml')]); + expect(document.references(specDirectory, new Set()).files.map(String)).to.deep.equal([inSpec('next.yaml')]); }); }); diff --git a/test/types/portal/preview-specs.test.ts b/test/types/portal/preview-specs.test.ts index 8ea151abb..63cf8a02b 100644 --- a/test/types/portal/preview-specs.test.ts +++ b/test/types/portal/preview-specs.test.ts @@ -37,10 +37,19 @@ describe('PreviewSpecs', () => { ]); }); - it('says nothing on a return to the documents it started with, and warns again of the change made anew', () => { + // The restart it was told it needs is no longer needed. + it('says once that nothing differs on a return to the documents it started with, and warns of the change made anew', () => { expect( - warnings(['pets.json'], [['orders.json', 'pets.json'], ['pets.json'], ['orders.json', 'pets.json']]) - ).to.deep.equal([{ added: ['orders.json'], removed: [] }, null, { added: ['orders.json'], removed: [] }]); + warnings( + ['pets.json'], + [['orders.json', 'pets.json'], ['pets.json'], ['pets.json'], ['orders.json', 'pets.json']] + ) + ).to.deep.equal([ + { added: ['orders.json'], removed: [] }, + { added: [], removed: [] }, + null, + { added: ['orders.json'], removed: [] } + ]); }); it('says the specifications are fixed once, on the first save it accepts after refusing one', () => { diff --git a/test/types/portal/spec-bundle.test.ts b/test/types/portal/spec-bundle.test.ts index d05885b8a..fdb724994 100644 --- a/test/types/portal/spec-bundle.test.ts +++ b/test/types/portal/spec-bundle.test.ts @@ -30,6 +30,21 @@ describe('SpecBundle', () => { expect(unread(bundle)).to.be.empty; }); + // The bundler turns `#` and `#pet` in a file it embeds into pointers to where it put the file. + it('keeps each $ref the bundler fails on: an empty one anywhere, and a bare # or an anchor in the root only', () => { + const bundle = new SpecBundle( + at('api.json'), + parsed({ whole: { $ref: '#' }, anchor: { $ref: '#pet' }, Pet: { $ref: './Pet.json' } }) + ); + bundle.add(at('Pet.json'), parsed({ whole: { $ref: '#' }, anchor: { $ref: '#pet' }, empty: { $ref: '' } })); + + expect(bundle.unresolvable().map(({ file, ref }) => [file.relativeTo(spec), ref])).to.deep.equal([ + ['api.json', '#'], + ['api.json', '#pet'], + ['Pet.json', ''] + ]); + }); + it('names what a file refers to from its own folder', () => { const bundle = new SpecBundle(at('api.json'), parsed({ Pet: { $ref: './schemas/Pet.json' } }));