diff --git a/.github/workflows/sync-api.yml b/.github/workflows/sync-api.yml index d2ee224..63e7294 100644 --- a/.github/workflows/sync-api.yml +++ b/.github/workflows/sync-api.yml @@ -106,7 +106,9 @@ jobs: - name: Check for changes id: changes run: | - # Only count actual spec yaml changes, not summary.json or formatting + # Count substantive changes only: spec yaml, generated API/type files, + # and the operation-identity manifest (scripts/api-surface.yaml), not + # summary.json or pure formatting. SPEC_CHANGES=$(git diff --name-only -- 'specs/raw/*.yaml' 'specs/fixed/*.yaml' | wc -l) API_CHANGES=$(git diff --name-only -- 'src/api/*.ts' | wc -l) TYPE_CHANGES=$(git diff --name-only -- 'src/types/generated/*.ts' | grep -v 'index.ts' | wc -l) @@ -287,6 +289,8 @@ jobs: echo "### ✅ Forced Release Complete" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY echo "Released version: **v${{ steps.version.outputs.new_version }}**" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "Classification: **$CLASSIFICATION**" >> $GITHUB_STEP_SUMMARY else echo "### ℹ️ No Changes Detected" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY @@ -304,7 +308,7 @@ jobs: if [[ "$CLASSIFICATION" == "breaking" ]]; then MISSING_OPS=$(jq -r '.missingOperations[] | "- `" + (.spec | gsub("\\r|\\n"; " ")) + "`: " + .method + " " + (.path | gsub("\\r|\\n"; " ")) + " (as `" + (.name | gsub("\\r|\\n"; " ")) + "`)"' sync-report.json 2>/dev/null || echo "") - echo "### Breaking API changes detected" >> $GITHUB_STEP_SUMMARY || true + echo "### ⚠️ Breaking API changes detected" >> $GITHUB_STEP_SUMMARY || true echo "" >> $GITHUB_STEP_SUMMARY || true echo "Manifest operations missing upstream:" >> $GITHUB_STEP_SUMMARY || true echo "" >> $GITHUB_STEP_SUMMARY || true diff --git a/scripts/fetch-specs.ts b/scripts/fetch-specs.ts index 6d8909d..4eb39fb 100644 --- a/scripts/fetch-specs.ts +++ b/scripts/fetch-specs.ts @@ -25,6 +25,7 @@ import * as yaml from 'yaml'; import { type ValidatedFetchedDoc, validateFetchedSpecDocs } from './lib/fetched-spec-utils.js'; import { canonicalizeSpec, stringifySpec } from './lib/spec-canonicalize.js'; import { type FetchedDoc, mergeSpecDocs } from './lib/spec-merge.js'; +import { isRecord } from './lib/spec-utils.js'; const BASE_URL = 'https://developer.deere.com/devDoc/apiDetails'; const OUTPUT_DIR = join(process.cwd(), 'specs', 'raw'); @@ -116,10 +117,6 @@ function parseSlugDocs(slug: string, docs: ValidatedFetchedDoc[]): FetchedDoc[] } } -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} - /** * Read back the merge order mergeSpecDocs stamped onto the merged document's * `x-source-documents` extension field (primary document first, then the diff --git a/scripts/generate-sdk.ts b/scripts/generate-sdk.ts index 7ee0e8c..c976d61 100644 --- a/scripts/generate-sdk.ts +++ b/scripts/generate-sdk.ts @@ -15,6 +15,7 @@ import { basename, join } from 'node:path'; import * as yaml from 'yaml'; import { buildSyncReport, + displayOp, loadApiSurface, normalizePathPattern, resolveMethodNames, @@ -540,7 +541,8 @@ function generateApiClass(api: GeneratedApi, names: Map): string .map((op) => { // resolveMethodNames keys its map by each op's raw display string // (`METHOD /path` with real param names), NOT the normalized opKey. - const key = `${op.method.toUpperCase()} ${op.path}`; + // displayOp is the shared builder of that exact key. + const key = displayOp(op); const methodName = names.get(key); if (!methodName) { throw new Error( diff --git a/scripts/lib/api-surface.ts b/scripts/lib/api-surface.ts index 6d5868b..1608c31 100644 --- a/scripts/lib/api-surface.ts +++ b/scripts/lib/api-surface.ts @@ -16,6 +16,7 @@ import { existsSync, readFileSync } from 'node:fs'; import { join } from 'node:path'; import * as yaml from 'yaml'; +import { isRecord } from './spec-utils.js'; // ============================================================================ // Types @@ -37,7 +38,7 @@ export interface ApiSurface { /** What naming needs to know about a single spec operation. */ export interface SurfaceOp { - /** Synthesized upstream when the spec omits one; see extractOps. */ + /** Synthesized when the spec omits one; see parseSpec in generate-sdk.ts. */ operationId: string; method: 'get' | 'post' | 'put' | 'patch' | 'delete'; path: string; @@ -78,16 +79,11 @@ const OP_PATTERN = /^(GET|POST|PUT|PATCH|DELETE) (\/\S*)$/; const NAME_PATTERN = /^[a-z][a-zA-Z0-9]*$/; /** Generated-class field names a method name must never shadow. */ const RESERVED_NAMES = new Set(['constructor', 'spec', 'client']); -const METHODS: readonly SurfaceOp['method'][] = ['get', 'post', 'put', 'patch', 'delete']; function fail(detail: string): never { throw new Error(`api-surface: ${detail}`); } -function isObject(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} - function describeType(value: unknown): string { if (value === null) return 'null'; if (Array.isArray(value)) return 'array'; @@ -139,7 +135,7 @@ export function loadApiSurface(filePath: string = DEFAULT_SURFACE_PATH): ApiSurf } function validateSurface(parsed: unknown, filePath: string): ApiSurface { - if (!isObject(parsed)) { + if (!isRecord(parsed)) { fail( `top-level value in ${filePath} must be a mapping with "version" and "specs", got ${describeType(parsed)}` ); @@ -149,7 +145,7 @@ function validateSurface(parsed: unknown, filePath: string): ApiSurface { `unsupported version ${JSON.stringify(parsed.version)} in ${filePath}; expected version: 1` ); } - if (!isObject(parsed.specs)) { + if (!isRecord(parsed.specs)) { fail( `"specs" in ${filePath} must be a mapping of specName to entries, got ${describeType(parsed.specs)}` ); @@ -172,7 +168,7 @@ function validateSpecEntries(specName: string, entries: unknown): SurfaceEntry[] for (let i = 0; i < entries.length; i++) { const entry: unknown = entries[i]; const where = `spec "${specName}" entry[${i}]`; - if (!isObject(entry)) { + if (!isRecord(entry)) { fail(`${where} must be a mapping with "op" and "name", got ${describeType(entry)}`); } if (typeof entry.op !== 'string' || entry.op.length === 0) { @@ -311,42 +307,6 @@ function compareEntries(a: SurfaceEntry, b: SurfaceEntry): number { return compareStrings(a.name, b.name); } -// ============================================================================ -// Op extraction (shared by the seed script and the generator) -// ============================================================================ - -/** - * Walk a parsed spec's `paths` and return one SurfaceOp per operation. The - * operationId synthesis and isCollection rule replicate parseSpec in - * generate-sdk.ts exactly, so the seed script and the generator see identical - * operations. Missing / empty / malformed `paths` yield []. - */ -export function extractOps(spec: unknown): SurfaceOp[] { - const ops: SurfaceOp[] = []; - if (!isObject(spec) || !isObject(spec.paths)) return ops; - - for (const [path, pathItem] of Object.entries(spec.paths)) { - if (!isObject(pathItem)) continue; - for (const method of METHODS) { - const operation = pathItem[method]; - if (!operation) continue; - const rawId = isObject(operation) ? operation.operationId : undefined; - const operationId = - typeof rawId === 'string' && rawId.length > 0 - ? rawId - : `${method}${path.replace(/[^a-zA-Z]/g, '')}`; - ops.push({ operationId, method, path, isCollection: isCollectionEndpoint(path, method) }); - } - } - return ops; -} - -function isCollectionEndpoint(path: string, method: string): boolean { - if (method !== 'get') return false; - const lastSegment = path.split('/').pop() || ''; - return !lastSegment.startsWith('{'); -} - // ============================================================================ // Name proposal for NEW operations (deterministic, no positional counters) // ============================================================================ @@ -386,7 +346,9 @@ function paramNames(path: string): string[] { /** * Deterministic name for a NEW operation. Verb by method (collection GETs use - * `list`), then capHump of the last non-param segment; never a bare verb. + * `list`), then capHump of the last non-param segment; never a bare verb (a + * segment that is all punctuation strips to empty under capHump and falls back + * to the `Item` noun, so the result is never just the verb). * Tiebreak chain, first free wins: prepend preceding non-param segments * nearest-first up to the whole path, then append `By` to the * full-path candidate. If every candidate is taken, throw (a human picks a @@ -404,7 +366,11 @@ export function proposeName(op: SurfaceOp, takenNames: ReadonlySet): str candidates.push(`${verb}Item`); } else { for (let i = nonParam.length - 1; i >= 0; i--) { - candidates.push(`${verb}${nonParam.slice(i).map(capHump).join('')}`); + // capHump strips a segment with no alphanumerics to '', which would leave + // a bare verb like "get". Fall back to the same "Item" noun the + // no-segment branch uses so a proposed name is never just a verb. + const suffix = nonParam.slice(i).map(capHump).join(''); + candidates.push(suffix.length > 0 ? `${verb}${suffix}` : `${verb}Item`); } } if (params.length > 0) { @@ -432,7 +398,16 @@ function addImpliedTwin(taken: Set, op: SurfaceOp, name: string): void { } } -function displayOp(op: SurfaceOp): string { +/** + * The raw display string for an operation: "METHOD /path" with real param + * names (e.g. "GET /organizations/{orgId}/fields") and an uppercased method. + * This is the exact key `resolveMethodNames` stores its returned `names` map + * under, so generate-sdk must build the identical string to look a name up. + * Sharing this one function keeps the resolver and the generator in lockstep + * instead of reconstructing the format inline in two places. Distinct from + * `opKey`, which normalizes param names to `{_}` for identity. + */ +export function displayOp(op: SurfaceOp): string { return `${op.method.toUpperCase()} ${op.path}`; } diff --git a/scripts/lib/fetched-spec-utils.ts b/scripts/lib/fetched-spec-utils.ts index 30df38f..ec76bff 100644 --- a/scripts/lib/fetched-spec-utils.ts +++ b/scripts/lib/fetched-spec-utils.ts @@ -1,5 +1,6 @@ import * as yaml from 'yaml'; import { redactSpecContent } from './spec-redactor.js'; +import { isRecord } from './spec-utils.js'; export interface ValidatedFetchedDoc { slug: string; @@ -9,10 +10,6 @@ export interface ValidatedFetchedDoc { ymlContent: string; } -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} - function isOpenApiDocument(content: string): boolean { try { const document = yaml.parseDocument(content); diff --git a/scripts/lib/spec-canonicalize.ts b/scripts/lib/spec-canonicalize.ts index 413ca85..c9e13d1 100644 --- a/scripts/lib/spec-canonicalize.ts +++ b/scripts/lib/spec-canonicalize.ts @@ -12,10 +12,7 @@ */ import * as yaml from 'yaml'; - -function isPlainObject(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} +import { isRecord } from './spec-utils.js'; function compareStrings(a: string, b: string): number { return a < b ? -1 : a > b ? 1 : 0; @@ -43,7 +40,7 @@ function canonicalizeComponents(components: Record): Record = {}; for (const category of Object.keys(components).sort(compareStrings)) { const value = components[category]; - result[category] = isPlainObject(value) ? sortKeys(value) : value; + result[category] = isRecord(value) ? sortKeys(value) : value; } return result; } @@ -66,15 +63,15 @@ function canonicalizeComponents(components: Record): Record = { ...doc }; - if (isPlainObject(doc.paths)) { + if (isRecord(doc.paths)) { result.paths = sortKeys(doc.paths); } - if (isPlainObject(doc.components)) { + if (isRecord(doc.components)) { result.components = canonicalizeComponents(doc.components); } diff --git a/scripts/lib/spec-merge.ts b/scripts/lib/spec-merge.ts index e9981c6..c6a3428 100644 --- a/scripts/lib/spec-merge.ts +++ b/scripts/lib/spec-merge.ts @@ -15,7 +15,7 @@ * silently. */ -import { toPascalCase } from './spec-utils.js'; +import { isRecord, toPascalCase } from './spec-utils.js'; const PRIMARY_ENDPOINT_NAME: Record = { 'field-operations-api': 'field-operation', @@ -58,11 +58,13 @@ export interface MergeOptions { */ maxRenameIterations?: number; /** - * Invoked once per declaring document whose servers block carries a - * non-deere.com placeholder URL (a documentation-editor default such as - * `https://server.com`). The message names the slug, the document, and the - * offending URL(s). fetch-specs wires this to the console so CI logs surface - * the spec-quality defect; unit tests that do not assert on it leave it unset. + * Invoked once per declaring document whose servers block carries an entry + * that is not a usable deere.com https URL: a documentation-editor placeholder + * (`https://server.com`), a non-https or non-deere host, an unparseable url, + * or a url-less/malformed entry. The message names the slug, the document, and + * each offending entry with the reason it was rejected. fetch-specs wires this + * to the console so CI logs surface the spec-quality defect; unit tests that + * do not assert on it leave it unset. */ onWarning?: (message: string) => void; } @@ -71,12 +73,8 @@ export interface MergeOptions { // Small structural helpers // --------------------------------------------------------------------------- -function isPlainObject(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} - function asObject(value: unknown): Record | undefined { - return isPlainObject(value) ? value : undefined; + return isRecord(value) ? value : undefined; } function compareStrings(a: string, b: string): number { @@ -98,7 +96,7 @@ function deepEqualUnordered(a: unknown, b: unknown): boolean { } return true; } - if (isPlainObject(a) && isPlainObject(b)) { + if (isRecord(a) && isRecord(b)) { const aKeys = Object.keys(a); const bKeys = Object.keys(b); if (aKeys.length !== bKeys.length) return false; @@ -172,7 +170,7 @@ function rewriteRefStrings(node: unknown, oldRef: string, newRef: string): void for (const item of node) rewriteRefStrings(item, oldRef, newRef); return; } - if (!isPlainObject(node)) return; + if (!isRecord(node)) return; for (const key of Object.keys(node)) { const value = node[key]; if (key === '$ref' && value === oldRef) { @@ -378,9 +376,10 @@ function mergePaths( if (deepEqualUnordered(mergedItem[key], incomingValue)) continue; const firstOwner = owner.get(pathMethodKey(path, key)) ?? '(unknown)'; throw new Error( - `mergeSpecDocs: slug "${slug}": conflicting definitions for ${key.toUpperCase()} ` + - `${path} between documents "${firstOwner}" and "${entry.endPointName}". The same ` + - `path and method is defined differently in two documents; a human must reconcile them.` + `mergeSpecDocs: slug "${slug}": conflicting definitions for path item member ` + + `"${key}" on "${path}" between documents "${firstOwner}" and "${entry.endPointName}". ` + + `The same path item member (an HTTP method, or a shared key such as parameters or ` + + `description) is defined differently in two documents; a human must reconcile them.` ); } } @@ -449,7 +448,12 @@ function serverUrl(entry: unknown): string | undefined { return typeof url === 'string' ? url : undefined; } -/** Parses to an https URL whose host ends in `.deere.com` (case-insensitive). */ +/** + * Parses to an https URL whose host is `deere.com` itself or any subdomain of + * it (case-insensitive). The match is on a label boundary (host equals + * `deere.com` OR ends with `.deere.com`), so the apex host counts while a + * lookalike like `notdeere.com` does not. + */ function isDeereHttpsUrl(url: string): boolean { let parsed: URL; try { @@ -457,7 +461,9 @@ function isDeereHttpsUrl(url: string): boolean { } catch { return false; } - return parsed.protocol === 'https:' && parsed.hostname.toLowerCase().endsWith('.deere.com'); + if (parsed.protocol !== 'https:') return false; + const host = parsed.hostname.toLowerCase(); + return host === 'deere.com' || host.endsWith('.deere.com'); } /** @@ -475,30 +481,56 @@ function isPlatformFamilyUrl(url: string): boolean { interface ServerClassification { family: ServerFamily; - /** URLs that do not resolve to a deere.com host (editor placeholders). */ - placeholders: string[]; + /** + * One human-readable descriptor per server entry that is not a usable + * deere.com https URL: a url-less/malformed entry, or a url that is + * unparseable, non-https, or on a non-deere host. Empty when every entry is a + * usable deere URL. Drives the onWarning message; never affects the family. + */ + rejected: string[]; +} + +/** + * Describe why one server entry is not a usable deere.com https URL, for the + * onWarning message. Called only on entries classifyServers has already + * rejected, so it names the concrete reason: a url-less/malformed entry, or a + * url that is unparseable, non-https, or points at a non-deere host. It never + * claims "no deere.com host" for an `http://x.deere.com` entry whose real + * defect is the non-https scheme. + */ +function describeRejection(entry: unknown): string { + const url = serverUrl(entry); + if (url === undefined) return 'an entry with no url'; + let parsed: URL; + try { + parsed = new URL(url); + } catch { + return `${url} (unparseable)`; + } + if (parsed.protocol !== 'https:') return `${url} (non-https)`; + return `${url} (non-deere host)`; } /** - * Classify one document's servers list. Non-deere URLs are documentation-editor - * placeholders (`https://server.com`): they are collected as `placeholders` and - * ignored for the family decision, so a stray placeholder never forces an - * otherwise-platform document into a conflict. A list whose only URLs are - * placeholders declares nothing usable (`junk`). Otherwise the deere URLs + * Classify one document's servers list. An entry that is not a usable deere.com + * https URL (a documentation-editor placeholder like `https://server.com`, a + * non-https scheme, an unparseable url, or a url-less/malformed entry) is + * described in `rejected` and ignored for the family decision, so a stray bad + * entry never forces an otherwise-platform document into a conflict. A list + * with no usable deere URL declares nothing (`junk`). Otherwise the deere URLs * decide: all on the platform family -> `platform`, any deere host on a * non-platform path -> `other`. */ function classifyServers(servers: readonly unknown[]): ServerClassification { - const placeholders: string[] = []; + const rejected: string[] = []; const deereUrls: string[] = []; for (const entry of servers) { const url = serverUrl(entry); - if (url === undefined) continue; - if (isDeereHttpsUrl(url)) deereUrls.push(url); - else placeholders.push(url); + if (url !== undefined && isDeereHttpsUrl(url)) deereUrls.push(url); + else rejected.push(describeRejection(entry)); } - if (deereUrls.length === 0) return { family: 'junk', placeholders }; - return { family: deereUrls.every(isPlatformFamilyUrl) ? 'platform' : 'other', placeholders }; + if (deereUrls.length === 0) return { family: 'junk', rejected }; + return { family: deereUrls.every(isPlatformFamilyUrl) ? 'platform' : 'other', rejected }; } function applyServers( @@ -515,22 +547,25 @@ function applyServers( ); if (rawDeclared.length === 0) return; - // Classify each declaring document. A placeholder-only block declares nothing - // usable and is dropped to non-declaring (it inherits the merged block); every - // placeholder, dropped or merely ignored, is surfaced through onWarning so CI - // logs the spec-quality defect. + // Classify each declaring document. A block with no usable deere URL declares + // nothing and is dropped to non-declaring (it inherits the merged block). + // Every rejected entry (a placeholder, a non-https or non-deere url, an + // unparseable url, or a url-less/malformed entry), whether it made the block + // junk or was merely ignored alongside good urls, is surfaced through + // onWarning so CI logs the spec-quality defect. const declared: Array<{ endPointName: string; servers: unknown[]; family: ServerFamily }> = []; for (const entry of rawDeclared) { - const { family, placeholders } = classifyServers(entry.servers); - if (placeholders.length > 0) { - const urls = placeholders.join(', '); + const { family, rejected } = classifyServers(entry.servers); + if (rejected.length > 0) { + const detail = rejected.join(', '); onWarning?.( family === 'junk' - ? `mergeSpecDocs: slug "${slug}": document "${entry.endPointName}" declares only ` + - `placeholder server URL(s) with no deere.com host (${urls}); treating it as ` + - `non-declaring so it inherits the merged servers.` - : `mergeSpecDocs: slug "${slug}": document "${entry.endPointName}" declares placeholder ` + - `server URL(s) with no deere.com host (${urls}); ignoring them for servers reconciliation.` + ? `mergeSpecDocs: slug "${slug}": document "${entry.endPointName}" declares no usable ` + + `deere.com https server URL (${detail}); treating it as non-declaring so it ` + + `inherits the merged servers.` + : `mergeSpecDocs: slug "${slug}": document "${entry.endPointName}" declares server ` + + `entries that are not usable deere.com https URLs (${detail}); ignoring them for ` + + `servers reconciliation.` ); } if (family === 'junk') continue; @@ -569,7 +604,7 @@ function applyTags(merged: Record, docs: readonly OrderedDoc[]) if (!Array.isArray(tags)) continue; sawTags = true; for (const tag of tags) { - const name = isPlainObject(tag) && typeof tag.name === 'string' ? tag.name : undefined; + const name = isRecord(tag) && typeof tag.name === 'string' ? tag.name : undefined; if (name !== undefined) { if (seen.has(name)) continue; seen.add(name); diff --git a/scripts/lib/spec-utils.ts b/scripts/lib/spec-utils.ts index 19c70ec..beac882 100644 --- a/scripts/lib/spec-utils.ts +++ b/scripts/lib/spec-utils.ts @@ -3,6 +3,19 @@ * Extracted for testability. */ +/** + * True when `value` is a non-null, plain object (a JSON record), excluding + * `null` (which is `typeof 'object'`) and arrays. Narrows to + * `Record` so callers can index keys safely. This is the one + * structural guard shared across the spec pipeline's tree walks: the + * multi-doc merge, the canonicalizer, fetch-time validation, and the + * api-surface loader all classified objects identically, so they share this + * definition rather than each keeping a private copy. + */ +export function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + /** * Last path segment of a JSON `$ref`. * e.g. "#/components/schemas/Foo" -> "Foo" diff --git a/tests/api-surface-manifest.test.ts b/tests/api-surface-manifest.test.ts index a0efcbd..90f6bde 100644 --- a/tests/api-surface-manifest.test.ts +++ b/tests/api-surface-manifest.test.ts @@ -7,9 +7,10 @@ * rebind if an upstream spec ever reorders its paths block. * * Tests run from the repo root, so the default cwd-based path in - * loadApiSurface() resolves to the real scripts/api-surface.yaml. The - * manifest itself is produced by scripts/seed-api-surface.ts (a one-time - * entrypoint, not unit tested); see that file for provenance. + * loadApiSurface() resolves to the real scripts/api-surface.yaml. The manifest + * was originally seeded by a one-time script (since removed) and is now + * maintained by generate-sdk's additive auto-append; see the file's git history + * for its provenance. * * Pinned pairs were verified against the "@generated from" JSDoc lines in * the committed src/api/field-operations-api.ts, src/api/equipment.ts, diff --git a/tests/api-surface.test.ts b/tests/api-surface.test.ts index 1d1b253..c790521 100644 --- a/tests/api-surface.test.ts +++ b/tests/api-surface.test.ts @@ -4,8 +4,8 @@ * The library is the committed operation-identity manifest: it maps * (HTTP method, normalized path) to public method names so an upstream spec * reorder cannot silently rebind a name. These tests cover loader validation, - * deterministic serialization + round-trip, shared op extraction, the - * deterministic name proposer, order-independent resolution, and run + * deterministic serialization + round-trip, the deterministic name proposer, + * order-independent resolution, and run * classification. Style follows tests/fix-specs-embed.test.ts (node:test + * node:assert + mkdtempSync for the file-backed cases) and tests/fuzz.test.ts * (fast-check properties). @@ -21,7 +21,6 @@ import { type ApiSurface, buildSyncReport, classifyRun, - extractOps, loadApiSurface, normalizePathPattern, opKey, @@ -347,51 +346,6 @@ describe('serializeApiSurface', () => { }); }); -// --------------------------------------------------------------------------- -// extractOps -// --------------------------------------------------------------------------- - -describe('extractOps', () => { - it('synthesizes operationIds, flags isCollection, and honors explicit ids', () => { - const spec = { - paths: { - '/widgets': { - get: {}, // no operationId -> synthesized; collection GET - post: { operationId: 'createWidget' }, // POST -> not a collection - }, - '/widgets/{id}': { - get: { operationId: 'getWidgetById' }, // item GET -> not a collection - }, - }, - }; - const ops = extractOps(spec); - const byKey = new Map(ops.map((o) => [`${o.method.toUpperCase()} ${o.path}`, o])); - - const list = byKey.get('GET /widgets'); - assert.ok(list); - assert.strictEqual(list.operationId, 'getwidgets'); - assert.strictEqual(list.isCollection, true); - - const create = byKey.get('POST /widgets'); - assert.ok(create); - assert.strictEqual(create.operationId, 'createWidget'); - assert.strictEqual(create.isCollection, false); - - const item = byKey.get('GET /widgets/{id}'); - assert.ok(item); - assert.strictEqual(item.operationId, 'getWidgetById'); - assert.strictEqual(item.isCollection, false); - }); - - it('tolerates missing / empty / malformed paths', () => { - assert.deepStrictEqual(extractOps({}), []); - assert.deepStrictEqual(extractOps({ paths: {} }), []); - assert.deepStrictEqual(extractOps({ paths: null }), []); - assert.deepStrictEqual(extractOps(null), []); - assert.deepStrictEqual(extractOps('nope'), []); - }); -}); - // --------------------------------------------------------------------------- // proposeName // --------------------------------------------------------------------------- @@ -480,6 +434,22 @@ describe('proposeName', () => { assert.strictEqual(proposeName(op, new Set(['getItem'])), 'getItemById'); }); + it('falls back to Item for an all-punctuation segment instead of a bare verb', () => { + // A non-param segment with no alphanumerics strips to '' under capHump, + // which would otherwise leave a bare "delete"; the suffix falls back to Item. + const op: SurfaceOp = { operationId: '', method: 'delete', path: '/!!!', isCollection: false }; + assert.strictEqual(proposeName(op, new Set()), 'deleteItem'); + // The By tiebreak still chains off the Item fallback. + const withParam: SurfaceOp = { + operationId: '', + method: 'delete', + path: '/!!!/{id}', + isCollection: false, + }; + assert.strictEqual(proposeName(withParam, new Set()), 'deleteItem'); + assert.strictEqual(proposeName(withParam, new Set(['deleteItem'])), 'deleteItemById'); + }); + it('throws when every candidate is taken, naming the op and the candidates', () => { const op: SurfaceOp = { operationId: '', diff --git a/tests/fix-specs.test.ts b/tests/fix-specs.test.ts index 3f0d050..0ba7255 100644 --- a/tests/fix-specs.test.ts +++ b/tests/fix-specs.test.ts @@ -200,6 +200,39 @@ describe('fix-specs utilities', () => { }); }); + it('restores through the vnd.deere.axiom media type, preferring it over application/json', () => { + // The generator resolves a response's schema from the vnd media type first + // (extractSchemaFromContent prefers application/vnd.deere.axiom.v3+json), + // so restoreEquipmentItemRefs must land the ref there. Declare BOTH media + // types and prove only the preferred vnd envelope is touched. + const vndValues: ValuesEnvelope = { type: 'array' }; + const jsonValues: ValuesEnvelope = { type: 'array' }; + const getEquipment = { + content: { + 'application/vnd.deere.axiom.v3+json': { + schema: { type: 'object', properties: { values: vndValues } }, + }, + 'application/json': { + schema: { type: 'object', properties: { values: jsonValues } }, + }, + }, + }; + const spec = { + components: { + schemas: { equipmentForList: { type: 'object' } }, + responses: { GetEquipment: getEquipment }, + }, + }; + + const restored = restoreEquipmentItemRefs(spec); + + assert.strictEqual(restored, 1); + // The preferred vnd envelope gets the ref... + assert.deepStrictEqual(vndValues.items, { $ref: '#/components/schemas/equipmentForList' }); + // ...and the application/json envelope is left untouched. + assert.strictEqual(jsonValues.items, undefined); + }); + it('no-ops when the item ref is already present (does not overwrite a repaired doc)', () => { const getEquipment = envelopeResponse({ items: { $ref: '#/components/schemas/equipmentForList' }, diff --git a/tests/spec-canonicalize.test.ts b/tests/spec-canonicalize.test.ts index 489aef7..cc7c3d5 100644 --- a/tests/spec-canonicalize.test.ts +++ b/tests/spec-canonicalize.test.ts @@ -97,7 +97,13 @@ const categoryNameArb = fc.constantFrom( 'responses', 'requestBodies', 'headers', - 'securitySchemes' + 'securitySchemes', + // A synthetic non-OpenAPI category: canonicalizeComponents is whitelist-free, + // so an unknown extension category must sort by name and have its members + // sorted exactly like a standard one. Including it here exercises that + // tolerance in the order-independence and idempotence properties, not just the + // deterministic test below. + 'x-futureCategory' ); const memberNameArb = fc.constantFrom('Alpha', 'Beta', 'Gamma', 'Delta', 'Epsilon', 'Zeta'); @@ -281,6 +287,29 @@ describe('canonicalizeSpec: preserves non-target ordering', () => { }); }); +// --------------------------------------------------------------------------- +// Unknown (non-OpenAPI) component categories: the sorter has no whitelist +// --------------------------------------------------------------------------- + +describe('canonicalizeSpec: tolerates unknown component categories', () => { + it('sorts an x- extension category and its members like any standard category', () => { + const doc = { + components: { + schemas: { B: {}, A: {} }, + 'x-futureCategory': { Zeta: { type: 'object' }, Alpha: { type: 'object' } }, + }, + }; + const result = canonicalizeSpec(doc) as { + components: Record>; + }; + // Category names sort, so the unknown x- category lands after schemas... + assert.deepStrictEqual(Object.keys(result.components), ['schemas', 'x-futureCategory']); + // ...and its members are sorted too, with no special-casing of known names. + assert.deepStrictEqual(Object.keys(result.components['x-futureCategory']), ['Alpha', 'Zeta']); + assert.deepStrictEqual(Object.keys(result.components.schemas), ['A', 'B']); + }); +}); + // --------------------------------------------------------------------------- // Missing / absent / malformed sections // --------------------------------------------------------------------------- diff --git a/tests/spec-merge.test.ts b/tests/spec-merge.test.ts index 31cf1ac..3835fcc 100644 --- a/tests/spec-merge.test.ts +++ b/tests/spec-merge.test.ts @@ -172,6 +172,37 @@ describe('mergeSpecDocs: paths', () => { ); }); + it('reports a differing non-method path-item member as a member, not a "method"', () => { + // A path item can carry non-method members (description, parameters, ...). + // When one differs across docs the conflict must name it as a "path item + // member", not mislabel it a method with an uppercased key. + const primary = { + info: {}, + paths: { '/x': { get: { operationId: 'getX' }, description: 'one' } }, + }; + const secondary = { + info: {}, + paths: { '/x': { description: 'two' } }, + }; + assert.throws( + () => + mergeSpecDocs('products', [ + { endPointName: 'varieties', id: 1, doc: primary }, + { endPointName: 'chemicals', id: 2, doc: secondary }, + ]), + (error: unknown) => { + assert.ok(error instanceof Error); + assert.match(error.message, /path item member/); + assert.match(error.message, /"description"/); + // Guard against regressing to the old wording, which uppercased the key + // and called every conflicting member a "method". + assert.doesNotMatch(error.message, /DESCRIPTION/); + assert.doesNotMatch(error.message, /path and method/); + return true; + } + ); + }); + it('keeps normalized-pattern siblings as two distinct paths without error', () => { // /x/{name} and /x/{id} share a normalized pattern but are different literal // paths; the manifest's ambiguous-group matching resolves them downstream. @@ -487,6 +518,31 @@ describe('mergeSpecDocs: servers', () => { assert.deepStrictEqual(merged.servers, [{ url: 'https://sandboxapi.deere.com' }]); }); + it('treats an apex deere.com host (no subdomain) as platform-family', () => { + // A server url whose host is exactly deere.com (no subdomain label) must + // count as platform-family via the label-boundary check, so pairing it with + // a platform block does not throw, the primary (apex) wins, and nothing is + // rejected as junk. + const primary = { info: {}, paths: {}, servers: [{ url: 'https://deere.com/platform' }] }; + const secondary = { + info: {}, + paths: {}, + servers: [{ url: 'https://api.deere.com/platform' }], + }; + const warnings: string[] = []; + const merged = mergeSpecDocs( + 'products', + [ + { endPointName: 'varieties', id: 1, doc: primary }, + { endPointName: 'chemicals', id: 2, doc: secondary }, + ], + { onWarning: (message: string) => warnings.push(message) } + ) as MergedSpec; + + assert.deepStrictEqual(merged.servers, [{ url: 'https://deere.com/platform' }]); + assert.deepStrictEqual(warnings, [], 'an apex deere host is not rejected, so no warning'); + }); + it('resolves a templated + static platform mix to the primary block', () => { const primary = { info: {}, @@ -549,6 +605,104 @@ describe('mergeSpecDocs: servers', () => { assert.ok(/server\.com/.test(warnings[0]), 'warning names the placeholder url'); }); + it('warns when a doc declares only url-less/malformed server entries', () => { + // A servers block whose entries carry no usable url string declares nothing, + // exactly like a placeholder-only block, so it must warn rather than be + // dropped to non-declaring silently. + const primary = { + info: {}, + paths: {}, + servers: [{ url: 'https://api.deere.com/platform' }], + }; + const secondary = { + info: {}, + paths: {}, + servers: [{ description: 'no url here' }, {}], + }; + const warnings: string[] = []; + const merged = mergeSpecDocs( + 'products', + [ + { endPointName: 'varieties', id: 1, doc: primary }, + { endPointName: 'documents', id: 2, doc: secondary }, + ], + { onWarning: (message: string) => warnings.push(message) } + ) as MergedSpec; + + // The url-less block is non-declaring; the primary block wins. + assert.deepStrictEqual(merged.servers, [{ url: 'https://api.deere.com/platform' }]); + // But it is surfaced, naming the slug and the offending document. + assert.strictEqual(warnings.length, 1); + assert.ok(/products/.test(warnings[0]), 'warning names the slug'); + assert.ok(/documents/.test(warnings[0]), 'warning names the endPointName'); + }); + + it('labels an http:// deere host as non-https, not "no deere.com host"', () => { + // An http (non-https) deere host is rejected for its scheme, not its host. + // The warning must name the offending url with the real reason and must not + // misstate that there is no deere.com host. + const primary = { + info: {}, + paths: {}, + servers: [{ url: 'https://api.deere.com/platform' }], + }; + const secondary = { + info: {}, + paths: {}, + servers: [{ url: 'http://sandboxapi.deere.com/platform' }], + }; + const warnings: string[] = []; + mergeSpecDocs( + 'products', + [ + { endPointName: 'varieties', id: 1, doc: primary }, + { endPointName: 'documents', id: 2, doc: secondary }, + ], + { onWarning: (message: string) => warnings.push(message) } + ); + + assert.strictEqual(warnings.length, 1); + assert.ok(/sandboxapi\.deere\.com/.test(warnings[0]), 'names the offending url'); + assert.ok(/non-https/.test(warnings[0]), 'states the real reason'); + assert.ok(!/no deere\.com host/.test(warnings[0]), 'does not misstate the reason'); + }); + + it('warns once for a mixed block that pairs a good platform url with a url-less entry', () => { + // A block with both a usable platform-family url and a url-less entry must + // still classify platform (the good url decides the family, so the trust + // decision is unaffected), but the url-less entry must not be silently + // dropped: it is a genuine spec defect distinct from the all-bad blocks the + // other warning tests above cover, which collapse to junk classification. + const primary = { + info: {}, + paths: {}, + servers: [{ url: 'https://api.deere.com/platform' }], + }; + const secondary = { + info: {}, + paths: {}, + servers: [{ url: 'https://api.deere.com/platform' }, { description: 'broken' }], + }; + const warnings: string[] = []; + const merged = mergeSpecDocs( + 'products', + [ + { endPointName: 'varieties', id: 1, doc: primary }, + { endPointName: 'documents', id: 2, doc: secondary }, + ], + { onWarning: (message: string) => warnings.push(message) } + ) as MergedSpec; + + // Still platform-family, so the merge succeeds with the primary's block. + assert.deepStrictEqual(merged.servers, [{ url: 'https://api.deere.com/platform' }]); + // The url-less entry is still surfaced, naming the slug, the offending + // document, and the rejected entry. + assert.strictEqual(warnings.length, 1); + assert.ok(/products/.test(warnings[0]), 'warning names the slug'); + assert.ok(/documents/.test(warnings[0]), 'warning names the endPointName'); + assert.ok(/an entry with no url/.test(warnings[0]), 'warning names the rejected entry'); + }); + it('keeps two identical OTHER-family blocks without throwing', () => { const primary = { info: {},