From 88a5b2d2701851acd3631b780a6755dc9499ce72 Mon Sep 17 00:00:00 2001 From: Phil Adams Date: Tue, 21 Feb 2023 19:09:31 -0600 Subject: [PATCH 1/3] fix(rulesets): avoid false errors from ajv This commit modifies the oasExample function so that example fields are removed from the schema to be used for validation. This is needed because the presence of an "example" field in a schema confuses ajv in certain scenarios. References: - https://github.com/stoplightio/spectral/issues/2081 - https://github.com/stoplightio/spectral/issues/2140 - https://github.com/ajv-validator/ajv/issues/1426 --- package.json | 3 +- packages/functions/src/schema/index.ts | 5 +- .../functions/__tests__/oasExample.test.ts | 451 ++++++++++++++++++ .../rulesets/src/oas/functions/oasExample.ts | 54 +++ 4 files changed, 511 insertions(+), 2 deletions(-) create mode 100644 packages/rulesets/src/oas/functions/__tests__/oasExample.test.ts diff --git a/package.json b/package.json index a2bee2699..300531ede 100644 --- a/package.json +++ b/package.json @@ -39,7 +39,8 @@ "test.karma": "karma start", "prepare": "husky install", "prerelease": "patch-package", - "release": "yarn prerelease && yarn workspaces foreach run release" + "release": "yarn prerelease && yarn workspaces foreach run release", + "jest": "jest" }, "workspaces": { "packages": [ diff --git a/packages/functions/src/schema/index.ts b/packages/functions/src/schema/index.ts index a7262517d..955c799b9 100644 --- a/packages/functions/src/schema/index.ts +++ b/packages/functions/src/schema/index.ts @@ -69,7 +69,10 @@ export default createRulesetFunction( // let's ignore any $ref errors if schema fn is provided with already resolved content, // if our resolver fails to resolve them, // ajv is unlikely to do it either, since it won't have access to the whole document, but a small portion of it - if (!rule.resolved || !(ex instanceof MissingRefError)) { + // We specifically check that "rule" is truthy below because "rule" might be undefined/null if this + // code is called from testcases. + const ignoreError = rule?.resolved && ex instanceof MissingRefError; + if (!ignoreError) { results.push({ message: ex.message, path, diff --git a/packages/rulesets/src/oas/functions/__tests__/oasExample.test.ts b/packages/rulesets/src/oas/functions/__tests__/oasExample.test.ts new file mode 100644 index 000000000..899e5b517 --- /dev/null +++ b/packages/rulesets/src/oas/functions/__tests__/oasExample.test.ts @@ -0,0 +1,451 @@ +import { oas3, oas3_0 } from '@stoplight/spectral-formats'; +import { DeepPartial } from '@stoplight/types'; +import oasExample, { Options as ExampleOptions } from '../oasExample'; +import { RulesetFunctionContext } from '@stoplight/spectral-core/src'; + +const schemaOpts: ExampleOptions = { + schemaField: '$', + oasVersion: 3, + type: 'schema', +}; +const mediaOpts: ExampleOptions = { + schemaField: 'schema', + oasVersion: 3, + type: 'media', +}; +const docFormats = { + formats: new Set([oas3, oas3_0]), +}; + +/** + * Runs the oasExample() custom rule function to perform a single test. + * @param target the object (media type or schema) containing an example/default value + * @param ruleOptions the options to be passed to oasExample() + * @param context the spectral context object to pass to oasExample() + * @returns an array of errors, or [] if no errors occurred + */ +function runRule(testData: Record, ruleOptions: ExampleOptions) { + const context: DeepPartial = { + path: [], + documentInventory: {}, + document: docFormats, + }; + + return oasExample(testData, ruleOptions, context as RulesetFunctionContext); +} + +describe('oasExample', () => { + describe('should return no errors', () => { + describe('example/default value in schema', () => { + test('valid "example" object', () => { + const schema = { + type: 'object', + properties: { + foo: { + type: 'number', + }, + bar: { + type: 'string', + }, + }, + required: ['foo'], + example: { + foo: 38, + bar: 'foo', + }, + }; + + const results = runRule(schema, schemaOpts); + expect(results).toHaveLength(0); + }); + test('valid "default" string', () => { + const schema = { + type: 'string', + pattern: 'xyz-.*', + minLength: 4, + maxLength: 6, + default: 'xyz-99', + }; + + const results = runRule(schema, schemaOpts); + expect(results).toHaveLength(0); + }); + test('valid "example" integer', () => { + const schema = { + type: 'integer', + example: 74, + }; + + const results = runRule(schema, schemaOpts); + expect(results).toHaveLength(0); + }); + test('scenario: "resolves to more than one schema"', () => { + // This test data is from https://github.com/stoplightio/spectral/issues/2081 and + // demonstrates a scenario in which ajv returns the dreaded + // "reference <...> resolves to more than one schema" false error. + // Without the fix to the oasExample() function, this test will fail. + // The reason that it fails is due to the way in which ajv handles unknown + // properties found in the schema (e.g. "example" - it's not actually part of JSONSchema), + // and the way it gives special treatment to the "id" property. Ajv gets confused by + // the fact that there are multiple example objects that each contain a property named "id" + // with the value 'bf23bc970b78d27691e8' (repeating example values is probably not an uncommon + // use-case for openapi authors if you think about it). + // So, without the fix to oasExample(), the test below will fail with this result: + // [ + // { + // "message": "reference \"bf23bc970b78d27691e8\" resolves to more than one schema", + // "path": ["example"] + // } + // ] + // However, if you rename the "id" properties to something else, the rule returns []. + // Likewise, if you change the value of "id" in one of the examples (so they are no longer equal) + // the rule returns []. + // And of course, with the fix to oasExample() in place, the rule will also return []. + const schema = { + type: 'object', + required: ['items'], + allOf: [ + { + type: 'object', + properties: { + items: { + type: 'array', + items: { + type: 'object', + required: ['id', 'url'], + properties: { + id: { + type: 'string', + }, + url: { + type: 'string', + format: 'uri', + }, + }, + example: { + id: 'bf23bc970b78d27691e8', + url: 'https://api.example.com/banking/accounts/bf23bc970b78d27691e8', + }, + }, + }, + }, + }, + ], + example: { + items: [ + { + id: 'bf23bc970b78d27691e8', + url: 'https://api.example.com/banking/accounts/bf23bc970b78d27691e8', + }, + { + id: '8d27691e8bf23bc970b7', + url: 'https://api.example.com/banking/accounts/8d27691e8bf23bc970b7', + }, + ], + }, + }; + + const results = runRule(schema, schemaOpts); + expect(results).toHaveLength(0); + }); + }); + describe('example/examples value in mediatype', () => { + test('valid "example" object', () => { + const mediaType = { + schema: { + type: 'object', + properties: { + foo: { + type: 'number', + }, + bar: { + type: 'string', + }, + }, + required: ['foo'], + }, + example: { + foo: 38, + bar: 'foo', + }, + }; + + const results = runRule(mediaType, mediaOpts); + expect(results).toHaveLength(0); + }); + test('valid "examples" object', () => { + const mediaType = { + schema: { + type: 'object', + properties: { + foo: { + type: 'number', + }, + bar: { + type: 'string', + }, + }, + required: ['foo'], + }, + examples: { + first: { + value: { + foo: 38, + bar: 'foo', + }, + }, + second: { + value: { + foo: 26, + bar: 'baz', + }, + }, + }, + }; + + const results = runRule(mediaType, mediaOpts); + expect(results).toHaveLength(0); + }); + test('valid "example" string', () => { + const mediaType = { + schema: { + type: 'string', + pattern: 'xyz-.*', + minLength: 4, + maxLength: 8, + }, + example: 'xyz-9999', + }; + + const results = runRule(mediaType, mediaOpts); + expect(results).toHaveLength(0); + }); + test('valid "examples" string', () => { + const mediaType = { + schema: { + type: 'string', + pattern: 'id-.*', + minLength: 4, + maxLength: 8, + }, + examples: { + first: { + value: 'id-1', + }, + second: { + value: 'id-99999', + }, + third: { + value: 'id-38', + }, + }, + }; + + const results = runRule(mediaType, mediaOpts); + expect(results).toHaveLength(0); + }); + test('scenario: "resolves to more than one schema"', () => { + // This test data was adapted from https://github.com/stoplightio/spectral/issues/2140. + const mediaType = { + schema: { + properties: { + bars: { + description: 'Array of bars!', + type: 'array', + items: { + oneOf: [ + { + type: 'object', + description: 'a real bar!', + required: ['id'], + properties: { + id: { + description: 'The ID for this real bar', + type: 'string', + }, + }, + example: { + id: '6d353a0f-aeb1-4ae1-832e-1110d10981bb', + }, + }, + { + description: 'not a real bar!', + not: { + type: 'object', + description: 'a real bar!', + required: ['id'], + properties: { + id: { + description: 'The ID for this real bar', + type: 'string', + }, + }, + example: { + id: '6d353a0f-aeb1-4ae1-832e-1110d10981bb', + }, + }, + }, + ], + }, + }, + }, + }, + example: { + bars: [{ id: '6d353a0f-aeb1-4ae1-832e-1110d10981bb' }], + }, + }; + + const results = runRule(mediaType, mediaOpts); + expect(results).toHaveLength(0); + }); + }); + }); + describe('should return errors', () => { + describe('example/default value in schema', () => { + test('invalid "example" object', () => { + const schema = { + type: 'object', + properties: { + foo: { + type: 'number', + }, + bar: { + type: 'string', + }, + }, + required: ['foo', 'bar'], + example: { + foo: 38, + bar: 26, + }, + }; + + const results = runRule(schema, schemaOpts); + expect(results).toHaveLength(1); + + expect(results[0].path.join('.')).toBe('example.bar'); + expect(results[0].message).toBe(`"bar" property type must be string`); + }); + test('invalid "default" string', () => { + const schema = { + type: 'string', + pattern: 'xyz-.*', + minLength: 4, + maxLength: 8, + default: 'xyz-99999', + }; + + const results = runRule(schema, schemaOpts); + expect(results).toHaveLength(1); + expect(results[0].message).toBe(`"default" property must not have more than 8 characters`); + expect(results[0].path.join('.')).toBe('default'); + }); + }); + describe('example/examples value in mediatype', () => { + test('invalid "example" object', () => { + const mediaType = { + schema: { + type: 'object', + properties: { + foo: { + type: 'number', + }, + bar: { + type: 'string', + }, + }, + required: ['foo', 'bar'], + }, + example: { + foo: 38, + }, + }; + + const results = runRule(mediaType, mediaOpts); + expect(results).toHaveLength(1); + expect(results[0].message).toBe(`"example" property must have required property "bar"`); + expect(results[0].path.join('.')).toBe('example'); + }); + test('invalid "examples" object', () => { + const mediaType = { + schema: { + type: 'object', + properties: { + foo: { + type: 'number', + }, + bar: { + type: 'string', + }, + }, + required: ['foo', 'bar'], + }, + examples: { + first: { + value: { + foo: 38, + }, + }, + second: { + value: { + foo: 'bar', + bar: 'foo', + }, + }, + }, + }; + + const results = runRule(mediaType, mediaOpts); + expect(results).toHaveLength(2); + + expect(results[0].message).toBe(`"value" property must have required property "bar"`); + expect(results[0].path.join('.')).toBe('examples.first.value'); + + expect(results[1].message).toBe(`"foo" property type must be number`); + expect(results[1].path.join('.')).toBe('examples.second.value.foo'); + }); + test('invalid "example" string', () => { + const mediaType = { + schema: { + type: 'string', + pattern: 'xyz-.*', + minLength: 4, + maxLength: 8, + }, + example: 'xyz-99999', + }; + + const results = runRule(mediaType, mediaOpts); + expect(results).toHaveLength(1); + expect(results[0].message).toBe(`"example" property must not have more than 8 characters`); + expect(results[0].path.join('.')).toBe('example'); + }); + test('invalid "examples" string', () => { + const mediaType = { + schema: { + type: 'string', + pattern: 'xyz-.*', + minLength: 4, + maxLength: 8, + default: 'xyz-99', + }, + examples: { + first: { + value: 'xyz-99999', + }, + second: { + value: 38, + }, + }, + }; + + const results = runRule(mediaType, mediaOpts); + expect(results).toHaveLength(2); + expect(results[0].message).toBe(`"value" property must not have more than 8 characters`); + expect(results[0].path.join('.')).toBe('examples.first.value'); + expect(results[1].message).toBe(`"value" property type must be string`); + expect(results[1].path.join('.')).toBe('examples.second.value'); + }); + }); + }); +}); diff --git a/packages/rulesets/src/oas/functions/oasExample.ts b/packages/rulesets/src/oas/functions/oasExample.ts index 1ad1dcd9b..1f0e62cb7 100644 --- a/packages/rulesets/src/oas/functions/oasExample.ts +++ b/packages/rulesets/src/oas/functions/oasExample.ts @@ -110,6 +110,54 @@ function* getSchemaValidationItems( } } +/** + * Modifies 'schema' (and all its sub-schemas) to remove all "example" fields. + * In this context, "sub-schemas" refers to all schemas reachable from 'schema' + * (e.g. properties, additionalProperties, allOf/anyOf/oneOf, not, items, etc.) + * @param schema the schema to be "de-examplified" + * @returns 'schema' with example fields removed + */ +function deExamplify(schema: Record): Record { + if ('items' in schema && typeof schema.items === 'object') { + schema.items = deExamplify(schema.items as Record); + } + + if (Array.isArray(schema.allOf)) { + for (let i = 0; i < schema.allOf.length; i++) { + schema.allOf[i] = deExamplify(schema.allOf[i] as Record); + } + } + if (Array.isArray(schema.anyOf)) { + for (let i = 0; i < schema.anyOf.length; i++) { + schema.anyOf[i] = deExamplify(schema.anyOf[i] as Record); + } + } + if (Array.isArray(schema.oneOf)) { + for (let i = 0; i < schema.oneOf.length; i++) { + schema.oneOf[i] = deExamplify(schema.oneOf[i] as Record); + } + } + if ('not' in schema && typeof schema.not === 'object') { + schema.not = deExamplify(schema.not as Record); + } + + if ('additionalProperties' in schema && typeof schema.additionalProperties === 'object') { + schema.additionalProperties = deExamplify(schema.additionalProperties as Record); + } + + if ('properties' in schema && typeof schema.properties === 'object') { + for (const propName in schema.properties) { + schema.properties[propName] = deExamplify(schema.properties[propName] as Record); + } + } + + if ('example' in schema) { + delete schema.example; + } + + return schema; +} + export default createRulesetFunction, Options>( { input: { @@ -149,6 +197,12 @@ export default createRulesetFunction, Options>( delete schemaOpts.schema.required; } + // Make a deep copy of the schema and then remove all the "example" fields from it. + // This is to avoid problems down in "ajv" which does the actual schema validation. + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + schemaOpts.schema = JSON.parse(JSON.stringify(schemaOpts.schema)); + schemaOpts.schema = deExamplify(schemaOpts.schema); + for (const validationItem of validationItems) { const result = oasSchema(validationItem.value, schemaOpts, { ...context, From 3ea22e8403cd1f9dbe8126e30a4b53364297cf24 Mon Sep 17 00:00:00 2001 From: Phil Adams Date: Wed, 5 Apr 2023 12:32:07 -0500 Subject: [PATCH 2/3] docs(repo): fix lint warning in README Signed-off-by: Phil Adams --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 4718b72dc..cd990c5cd 100644 --- a/README.md +++ b/README.md @@ -101,6 +101,7 @@ There are also rulesets created by many companies to improve their APIs. You can - [Zalando](https://apistylebook.stoplight.io/docs/zalando-restful-api-guidelines) - Based on [Zalando's RESTFUL API Guidelines](https://github.com/zalando/restful-api-guidelines), covers a wide-range of API topics such as versioning standards, property naming standards, the default format for request/response properties, and more. Check out some additional style guides here: + - [Spectral Rulesets by Stoplight](https://github.com/stoplightio/spectral-rulesets) - [API Stylebook by Stoplight](https://apistylebook.stoplight.io) From 48f81a39a2ec4d46f13dd7d11b8bba58ef4a5125 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ro=C5=BCek?= Date: Tue, 25 Apr 2023 10:26:19 +0200 Subject: [PATCH 3/3] chore(rulesets): use traverse --- .../rulesets/src/oas/functions/oasExample.ts | 46 +++---------------- 1 file changed, 7 insertions(+), 39 deletions(-) diff --git a/packages/rulesets/src/oas/functions/oasExample.ts b/packages/rulesets/src/oas/functions/oasExample.ts index 1f0e62cb7..6af1154f5 100644 --- a/packages/rulesets/src/oas/functions/oasExample.ts +++ b/packages/rulesets/src/oas/functions/oasExample.ts @@ -3,6 +3,7 @@ import type { Dictionary, JsonPath, Optional } from '@stoplight/types'; import oasSchema, { Options as SchemaOptions } from './oasSchema'; import { createRulesetFunction, IFunctionResult } from '@stoplight/spectral-core'; import { oas2 } from '@stoplight/spectral-formats'; +import traverse from 'json-schema-traverse'; export type Options = { oasVersion: 2 | 3; @@ -117,45 +118,12 @@ function* getSchemaValidationItems( * @param schema the schema to be "de-examplified" * @returns 'schema' with example fields removed */ -function deExamplify(schema: Record): Record { - if ('items' in schema && typeof schema.items === 'object') { - schema.items = deExamplify(schema.items as Record); - } - - if (Array.isArray(schema.allOf)) { - for (let i = 0; i < schema.allOf.length; i++) { - schema.allOf[i] = deExamplify(schema.allOf[i] as Record); - } - } - if (Array.isArray(schema.anyOf)) { - for (let i = 0; i < schema.anyOf.length; i++) { - schema.anyOf[i] = deExamplify(schema.anyOf[i] as Record); - } - } - if (Array.isArray(schema.oneOf)) { - for (let i = 0; i < schema.oneOf.length; i++) { - schema.oneOf[i] = deExamplify(schema.oneOf[i] as Record); - } - } - if ('not' in schema && typeof schema.not === 'object') { - schema.not = deExamplify(schema.not as Record); - } - - if ('additionalProperties' in schema && typeof schema.additionalProperties === 'object') { - schema.additionalProperties = deExamplify(schema.additionalProperties as Record); - } - - if ('properties' in schema && typeof schema.properties === 'object') { - for (const propName in schema.properties) { - schema.properties[propName] = deExamplify(schema.properties[propName] as Record); +function deExamplify(schema: Record): void { + traverse(schema, (fragment => { + if ('example' in fragment) { + delete fragment.example; } - } - - if ('example' in schema) { - delete schema.example; - } - - return schema; + })); } export default createRulesetFunction, Options>( @@ -201,7 +169,7 @@ export default createRulesetFunction, Options>( // This is to avoid problems down in "ajv" which does the actual schema validation. // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment schemaOpts.schema = JSON.parse(JSON.stringify(schemaOpts.schema)); - schemaOpts.schema = deExamplify(schemaOpts.schema); + deExamplify(schemaOpts.schema); for (const validationItem of validationItems) { const result = oasSchema(validationItem.value, schemaOpts, {