From c24246083724265b3e92bde943e8b0c3a3d3d14f Mon Sep 17 00:00:00 2001 From: Dustin Popp Date: Mon, 7 Apr 2025 08:51:20 -0500 Subject: [PATCH 1/3] feat: add helpers to extract processed api from spectral context The Spectral 'context' object contains processed data about the OpenAPI definition being validated, including the resolved and unresolved versions of the definition and information about references. We use these objects in rule functions and they need to be extracted from deeply nested fields in the spectral context. Rather than always needing to remember or look up the fields, this commit allows us to use simple helper functions to get the data we need. Signed-off-by: Dustin Popp --- docs/openapi-ruleset-utilities.md | 33 +++++++++++++++ packages/utilities/src/utils/index.js | 3 +- .../src/utils/spectral-context-utils.js | 40 ++++++++++++++++++ .../test/spectral-context-utils.test.js | 42 +++++++++++++++++++ 4 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 packages/utilities/src/utils/spectral-context-utils.js create mode 100644 packages/utilities/test/spectral-context-utils.test.js diff --git a/docs/openapi-ruleset-utilities.md b/docs/openapi-ruleset-utilities.md index 9c05c96ea..4ca81da86 100644 --- a/docs/openapi-ruleset-utilities.md +++ b/docs/openapi-ruleset-utilities.md @@ -354,6 +354,39 @@ for OpenAPI documents where a `required` property is not defined under the `prop #### Returns `boolean` +### `getResolvedSpec(context)` + +Returns the programmatic representation of an OpenAPI document, stored in the +Spectral-created "context" object, with all non-circular references resolved. + +#### Parameters + +- **`context`** ``: passed as an argument to Spectral-based rule functions + +#### Returns `object`: the resolved version of an OpenAPI document + +### `getUnresolvedSpec(context)` + +Returns the programmatic representation of an OpenAPI document, stored in +the Spectral-created "context" object, with all references still intact. + +#### Parameters + +- **`context`** ``: passed as an argument to Spectral-based rule functions + +#### Returns `object`: the unresolved version of an OpenAPI document + +### `getNodes(context)` + +Returns the graph nodes, with information about references and the locations +they resolve to, that are computed by the Spectral resolver. + +#### Parameters + +- **`context`** ``: passed as an argument to Spectral-based rule functions + +#### Returns `object`: the graph nodes + ### `validateComposedSchemas(schema, path, validate, includeSelf, includeNot)` Performs validation on a schema and all of its composed schemas. diff --git a/packages/utilities/src/utils/index.js b/packages/utilities/src/utils/index.js index 0a2e928a1..df39fd41e 100644 --- a/packages/utilities/src/utils/index.js +++ b/packages/utilities/src/utils/index.js @@ -1,5 +1,5 @@ /** - * Copyright 2017 - 2024 IBM Corporation. + * Copyright 2017 - 2025 IBM Corporation. * SPDX-License-Identifier: Apache2.0 */ @@ -13,6 +13,7 @@ module.exports = { schemaHasProperty: require('./schema-has-property'), schemaLooselyHasConstraint: require('./schema-loosely-has-constraint'), schemaRequiresProperty: require('./schema-requires-property'), + ...require('./spectral-context-utils'), validateComposedSchemas: require('./validate-composed-schemas'), validateNestedSchemas: require('./validate-nested-schemas'), validateSubschemas: require('./validate-subschemas'), diff --git a/packages/utilities/src/utils/spectral-context-utils.js b/packages/utilities/src/utils/spectral-context-utils.js new file mode 100644 index 000000000..89be8297a --- /dev/null +++ b/packages/utilities/src/utils/spectral-context-utils.js @@ -0,0 +1,40 @@ +/** + * Copyright 2025 IBM Corporation. + * SPDX-License-Identifier: Apache2.0 + */ + +/** + * Returns the programmatic representation of an OpenAPI document, stored in the + * Spectral-created "context" object, with all non-circular references resolved. + * @param {object} context passed as an argument to Spectral-based rule functions + * @returns {object} the resolved version of an OpenAPI document + */ +function getResolvedSpec(context) { + return context.documentInventory.resolved; +} + +/** + * Returns the programmatic representation of an OpenAPI document, stored in + * the Spectral-created "context" object, with all references still intact. + * @param {object} context passed as an argument to Spectral-based rule functions + * @returns {object} the unresolved version of an OpenAPI document + */ +function getUnresolvedSpec(context) { + return context.document.parserResult.data; +} + +/** + * Returns the graph nodes, with information about references and the locations + * they resolve to, that are computed by the Spectral resolver. + * @param {object} context passed as an argument to Spectral-based rule functions + * @returns {object} the graph nodes + */ +function getNodes(context) { + return context.documentInventory.graph.nodes; +} + +module.exports = { + getNodes, + getResolvedSpec, + getUnresolvedSpec, +}; diff --git a/packages/utilities/test/spectral-context-utils.test.js b/packages/utilities/test/spectral-context-utils.test.js new file mode 100644 index 000000000..d4b89c80d --- /dev/null +++ b/packages/utilities/test/spectral-context-utils.test.js @@ -0,0 +1,42 @@ +/** + * Copyright 2025 IBM Corporation. + * SPDX-License-Identifier: Apache2.0 + */ + +const { getNodes, getResolvedSpec, getUnresolvedSpec } = require('../src'); + +describe('Utility functions: Spectral Context Utils', () => { + const mockSpectralContextObject = { + documentInventory: { + resolved: 'resolved spec', + graph: { + nodes: 'graph nodes', + }, + }, + document: { + parserResult: { + data: 'unresolved spec', + }, + }, + }; + + describe('getNodes', () => { + it('should return graph nodes from context object', async () => { + expect(getNodes(mockSpectralContextObject)).toBe('graph nodes'); + }); + }); + + describe('getResolvedSpec', () => { + it('should return graph nodes from context object', async () => { + expect(getResolvedSpec(mockSpectralContextObject)).toBe('resolved spec'); + }); + }); + + describe('getUnresolvedSpec', () => { + it('should return graph nodes from context object', async () => { + expect(getUnresolvedSpec(mockSpectralContextObject)).toBe( + 'unresolved spec' + ); + }); + }); +}); From a7ae1d51cb938ae6411048958b86a0d64633296c Mon Sep 17 00:00:00 2001 From: Dustin Popp Date: Mon, 7 Apr 2025 08:58:17 -0500 Subject: [PATCH 2/3] refactor: adjust code to use new spectral context utilities Signed-off-by: Dustin Popp --- packages/ruleset/src/functions/api-symmetry.js | 3 ++- .../src/functions/collection-array-property.js | 3 ++- .../src/functions/request-and-response-content.js | 11 +++++------ .../src/functions/resource-response-consistency.js | 10 +++++++--- .../ruleset/src/functions/response-status-codes.js | 7 ++----- .../ruleset/src/functions/schema-naming-convention.js | 7 +++++-- .../ruleset/src/functions/use-date-based-format.js | 3 ++- 7 files changed, 25 insertions(+), 19 deletions(-) diff --git a/packages/ruleset/src/functions/api-symmetry.js b/packages/ruleset/src/functions/api-symmetry.js index c56210b87..0b19a9b1e 100644 --- a/packages/ruleset/src/functions/api-symmetry.js +++ b/packages/ruleset/src/functions/api-symmetry.js @@ -4,6 +4,7 @@ */ const { + getNodes, getSchemaType, isObject, isArraySchema, @@ -46,7 +47,7 @@ module.exports = function apiSymmetry(apidef, options, context) { ruleId = context.rule.name; logger = LoggerFactory.getInstance().getLogger(ruleId); } - return checkApiForSymmetry(apidef, context.documentInventory.graph.nodes); + return checkApiForSymmetry(apidef, getNodes(context)); }; /** diff --git a/packages/ruleset/src/functions/collection-array-property.js b/packages/ruleset/src/functions/collection-array-property.js index 03b99bf7c..fb1a44d22 100644 --- a/packages/ruleset/src/functions/collection-array-property.js +++ b/packages/ruleset/src/functions/collection-array-property.js @@ -7,6 +7,7 @@ const { schemaHasConstraint, isArraySchema, isObject, + getUnresolvedSpec, } = require('@ibm-cloud/openapi-ruleset-utilities'); const { LoggerFactory } = require('../utils'); @@ -21,7 +22,7 @@ module.exports = function (schema, _opts, context) { return collectionArrayProperty( schema, context.path, - context.document.parserResult.data + getUnresolvedSpec(context) ); }; diff --git a/packages/ruleset/src/functions/request-and-response-content.js b/packages/ruleset/src/functions/request-and-response-content.js index d377bf8d7..f061bd8b1 100644 --- a/packages/ruleset/src/functions/request-and-response-content.js +++ b/packages/ruleset/src/functions/request-and-response-content.js @@ -3,7 +3,10 @@ * SPDX-License-Identifier: Apache2.0 */ -const { isObject } = require('@ibm-cloud/openapi-ruleset-utilities'); +const { + isObject, + getResolvedSpec, +} = require('@ibm-cloud/openapi-ruleset-utilities'); const { LoggerFactory, pathHasMinimallyRepresentedResource, @@ -29,11 +32,7 @@ module.exports = function requestAndResponseContent( ruleId = context.rule.name; logger = LoggerFactory.getInstance().getLogger(ruleId); } - return checkForContent( - operation, - context.path, - context.documentInventory.resolved - ); + return checkForContent(operation, context.path, getResolvedSpec(context)); }; /** diff --git a/packages/ruleset/src/functions/resource-response-consistency.js b/packages/ruleset/src/functions/resource-response-consistency.js index f3694f81a..2305fb8fb 100644 --- a/packages/ruleset/src/functions/resource-response-consistency.js +++ b/packages/ruleset/src/functions/resource-response-consistency.js @@ -4,7 +4,11 @@ */ const { isEqual } = require('lodash'); -const { isObject } = require('@ibm-cloud/openapi-ruleset-utilities'); +const { + isObject, + getResolvedSpec, + getNodes, +} = require('@ibm-cloud/openapi-ruleset-utilities'); const { computeRefsAtPaths, getResourceSpecificSiblingPath, @@ -29,8 +33,8 @@ module.exports = function (operation, _opts, context) { return resourceResponseConsistency( operation, context.path, - context.documentInventory.resolved, - context.documentInventory.graph.nodes + getResolvedSpec(context), + getNodes(context) ); }; diff --git a/packages/ruleset/src/functions/response-status-codes.js b/packages/ruleset/src/functions/response-status-codes.js index 37d11c4c7..28937b97a 100644 --- a/packages/ruleset/src/functions/response-status-codes.js +++ b/packages/ruleset/src/functions/response-status-codes.js @@ -3,6 +3,7 @@ * SPDX-License-Identifier: Apache2.0 */ +const { getResolvedSpec } = require('@ibm-cloud/openapi-ruleset-utilities'); const { LoggerFactory, isCreateOperation, @@ -21,11 +22,7 @@ module.exports = function (operation, _opts, context) { logger = LoggerFactory.getInstance().getLogger(ruleId); } - return responseStatusCodes( - operation, - context.path, - context.documentInventory.resolved - ); + return responseStatusCodes(operation, context.path, getResolvedSpec(context)); }; /** diff --git a/packages/ruleset/src/functions/schema-naming-convention.js b/packages/ruleset/src/functions/schema-naming-convention.js index 7cd42e105..125ff32b3 100644 --- a/packages/ruleset/src/functions/schema-naming-convention.js +++ b/packages/ruleset/src/functions/schema-naming-convention.js @@ -3,7 +3,10 @@ * SPDX-License-Identifier: Apache2.0 */ -const { schemaHasProperty } = require('@ibm-cloud/openapi-ruleset-utilities'); +const { + schemaHasProperty, + getNodes, +} = require('@ibm-cloud/openapi-ruleset-utilities'); const { LoggerFactory, @@ -40,7 +43,7 @@ module.exports = function schemaNames(apidef, options, context) { ruleId = context.rule.name; logger = LoggerFactory.getInstance().getLogger(ruleId); } - return checkSchemaNames(apidef, context.documentInventory.graph.nodes); + return checkSchemaNames(apidef, getNodes(context)); }; /** diff --git a/packages/ruleset/src/functions/use-date-based-format.js b/packages/ruleset/src/functions/use-date-based-format.js index 681de67eb..47ca1f53b 100644 --- a/packages/ruleset/src/functions/use-date-based-format.js +++ b/packages/ruleset/src/functions/use-date-based-format.js @@ -11,6 +11,7 @@ const { isObject, isStringSchema, validateNestedSchemas, + getResolvedSpec, } = require('@ibm-cloud/openapi-ruleset-utilities'); const { @@ -48,7 +49,7 @@ module.exports = function (schema, _opts, context) { return checkForDateBasedFormat( schema, context.path, - context.documentInventory.resolved + getResolvedSpec(context) ); }; From d4709e35878f7a19bb5e63ab1ed530c946240bd0 Mon Sep 17 00:00:00 2001 From: Dustin Popp Date: Mon, 7 Apr 2025 08:58:38 -0500 Subject: [PATCH 3/3] fix(ibm-valid-schema-example): support circular references in schemas This rule would previously fail if it tried to validate the example for a schema that contains a circular reference (i.e. defines a schema cycle) because the ref resolver leaves these schemas with unresolved refs. The JSON Schema validator tries to resolve the references but can't find them, so it fails. This commit unconditionally adds the components from the OpenAPI definition to the schema so that the JSON Schema parser can find references if it needs to. Signed-off-by: Dustin Popp --- .../src/functions/valid-schema-example.js | 23 +++++- .../test/rules/valid-schema-example.test.js | 70 +++++++++++++++++++ 2 files changed, 90 insertions(+), 3 deletions(-) diff --git a/packages/ruleset/src/functions/valid-schema-example.js b/packages/ruleset/src/functions/valid-schema-example.js index 36c5a273b..2a6c42b62 100644 --- a/packages/ruleset/src/functions/valid-schema-example.js +++ b/packages/ruleset/src/functions/valid-schema-example.js @@ -4,11 +4,15 @@ */ const { validate } = require('jsonschema'); -const { validateSubschemas } = require('@ibm-cloud/openapi-ruleset-utilities'); +const { + validateSubschemas, + getResolvedSpec, +} = require('@ibm-cloud/openapi-ruleset-utilities'); const { LoggerFactory } = require('../utils'); let ruleId; let logger; +let openapi; module.exports = function (schema, _opts, context) { if (!logger) { @@ -16,6 +20,8 @@ module.exports = function (schema, _opts, context) { logger = LoggerFactory.getInstance().getLogger(ruleId); } + openapi = getResolvedSpec(context); + return validateSubschemas(schema, context.path, checkSchemaExamples); }; @@ -50,10 +56,21 @@ function checkSchemaExamples(schema, path) { function validateExamples(examples) { return examples .map(({ schema, example, path }) => { + // If the spec includes circular references, there may be unresolved + // references in the schema. The JSON Schema validator needs to be + // able to look those up, so include all of the components in the schema. + const schemaWithComponents = { + ...schema, + components: openapi.components, + }; + // Setting required: true prevents undefined values from passing validation. - const { valid, errors } = validate(example, schema, { required: true }); + const { valid, errors } = validate(example, schemaWithComponents, { + required: true, + }); + if (!valid) { - const message = getMessage(errors, example, schema); + const message = getMessage(errors, example, schemaWithComponents); return { message: `Schema example is not valid: ${message}`, path, diff --git a/packages/ruleset/test/rules/valid-schema-example.test.js b/packages/ruleset/test/rules/valid-schema-example.test.js index 089252beb..eb15f3593 100644 --- a/packages/ruleset/test/rules/valid-schema-example.test.js +++ b/packages/ruleset/test/rules/valid-schema-example.test.js @@ -22,6 +22,39 @@ describe(`Spectral rule: ${ruleId}`, () => { const results = await testRule(ruleId, rule, rootDocument); expect(results).toHaveLength(0); }); + + it('Schema with valid example contains a circular reference', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.Movie.properties.prop = { + type: 'object', + required: ['some_prop'], + examples: [ + { + some_prop: 'example', + other_prop: 'example', + inspiration: { + id: '1234', + name: 'Good Will Hunting', + }, + }, + ], + properties: { + inspiration: { + $ref: '#/components/schemas/Movie', + }, + some_prop: { + type: 'string', + }, + other_prop: { + type: 'string', + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(0); + }); }); describe('Should yield errors', () => { @@ -430,5 +463,42 @@ describe(`Spectral rule: ${ruleId}`, () => { expect(results[i].path.join('.')).toBe(expectedExamplePaths[i]); } }); + + it('Schema contains a circular reference', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.Movie.properties.prop = { + type: 'object', + required: ['some_prop'], + examples: [ + { + other_prop: 'example', + }, + ], + properties: { + inspiration: { + $ref: '#/components/schemas/Movie', + }, + some_prop: { + type: 'string', + }, + other_prop: { + type: 'string', + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(4); + + for (const i in results) { + expect(results[i].code).toBe(ruleId); + expect(results[i].message).toBe( + `${expectedMsgPrefix} requires property "some_prop"` + ); + expect(results[i].severity).toBe(expectedSeverity); + expect(results[i].path.join('.')).toBe(expectedExamplesPaths[i]); + } + }); }); });