diff --git a/README.md b/README.md index bc42926ba..234b141ee 100644 --- a/README.md +++ b/README.md @@ -64,7 +64,7 @@ as well as IBM-defined best practices. ## Getting Started The validator analyzes your API definition and reports any problems within. The validator is highly customizable, and supports both OpenAPI 3.0 and OpenAPI 2.0 (Swagger 2.0) formats. The tool also supports a number of rules from [Spectral](https://stoplight.io/open-source/spectral/). You can easily extend the tool with custom rules to meet your specific needs and ensure compliance to your standards. -The default configuration uses both OpenAPI 3.0 rules as well as Spectral rules. The [default mode](#default-mode) section decscribes these rules. Get started by [installing the tool](#installation), then [run the tool](#usage) on your API definition. +The default configuration uses both OpenAPI 3.0 rules as well as Spectral rules. The [default mode](#default-mode) section describes these rules. Get started by [installing the tool](#installation), then [run the tool](#usage) on your API definition. ### Customization @@ -496,32 +496,32 @@ The default values for each rule are described below. Currently the validator configures Spectral to check the following rules from its [“oas" ruleset](https://meta.stoplight.io/docs/spectral/docs/reference/openapi-rules.md): ``` +oas2-operation-formData-consume-check: true +operation-operationId-unique: true +operation-parameters: true +operation-tag-defined: true no-eval-in-markdown: true no-script-tags-in-markdown: true openapi-tags: true operation-description: true -operation-operationId-unique: true -operation-parameters: true operation-tags: true -operation-tag-defined: true path-keys-no-trailing-slash: true path-not-include-query: true -request-body-object: true typed-enum: true oas2-api-host: true oas2-api-schemes: true oas2-host-trailing-slash: true -oas2-valid-example: true -oas2-valid-definition-example: true +oas2-valid-schema-example: 'warn' oas2-anyOf: true oas2-oneOf: true -oas2-operation-formData-consume-check: true +oas2-unused-definition: true oas3-api-servers: true oas3-examples-value-or-externalValue: true oas3-server-trailing-slash: true +oas3-valid-media-example: 'warn' +oas3-valid-schema-example: 'warn' oas3-schema: true -oas3-valid-example: true -oas3-valid-schema-example: true +oas3-unused-component: true ``` This ruleset has the name `@ibm-cloud/openapi-ruleset`, and you can "extend" this ruleset or specify your own custom ruleset diff --git a/packages/ruleset/src/collections/index.js b/packages/ruleset/src/collections/index.js new file mode 100644 index 000000000..2f32d5b28 --- /dev/null +++ b/packages/ruleset/src/collections/index.js @@ -0,0 +1,19 @@ +// a group of predefined "collections" of OpenAPI locations to validate +// helpful when the same group of locations needs to be used by multiple rules + +// a collection of locations where a JSON Schema object can be *used*. +// +// note that this does not include "components.schemas" to avoid duplication. +// this collection should be used in a rule that has "resolved" set to "true". +// we separately validate that all schemas in "components" need to be used. +const schemas = [ + '$.paths[*][parameters][*].schema', + '$.paths[*][parameters][*].content[*].schema', + '$.paths[*][*][parameters][*].schema', + '$.paths[*][*][parameters,responses][*].content[*].schema', + '$.paths[*][*][requestBody].content[*].schema' +]; + +module.exports = { + schemas +}; diff --git a/packages/ruleset/src/functions/discriminator.js b/packages/ruleset/src/functions/discriminator.js new file mode 100644 index 000000000..a765c4be4 --- /dev/null +++ b/packages/ruleset/src/functions/discriminator.js @@ -0,0 +1,40 @@ +// Assertation 1: +// if discriminator exist inside schema object, it must be of type Object +// enforced by Spectral's oas3-schema rule + +// Assertion 2: +// discriminator object must have a field name propertyName +// enforced by Spectral's oas3-schema rule + +// Assertation 3: +// propertyName is of type string +// enforced by Spectral's oas3-schema rule + +// Assertation 4: +// properties inside a schema object must include propertyName from discriminator object + +const { checkSubschemasForProperty, validateSubschemas } = require('../utils'); + +module.exports = function(schema, _opts, { path }) { + return validateSubschemas(schema, path, validateDiscriminators); +}; + +function validateDiscriminators(schema, path) { + const errors = []; + + const { discriminator } = schema; + if (!discriminator || !typeof discriminator === 'object') { + return errors; + } + + const { propertyName } = discriminator; + if (!checkSubschemasForProperty(schema, propertyName)) { + errors.push({ + message: + 'The discriminator property name used must be defined in this schema', + path: [...path, 'discriminator', 'propertyName'] + }); + } + + return errors; +} diff --git a/packages/ruleset/src/functions/index.js b/packages/ruleset/src/functions/index.js index c154599fe..0f7708d29 100644 --- a/packages/ruleset/src/functions/index.js +++ b/packages/ruleset/src/functions/index.js @@ -1,4 +1,5 @@ const checkMajorVersion = require('./check-major-version'); +const discriminator = require('./discriminator'); const errorResponseSchema = require('./error-response-schema'); const requiredProperty = require('./required-property'); const responseExampleProvided = require('./response-example-provided'); @@ -7,6 +8,7 @@ const stringBoundary = require('./string-boundary'); module.exports = { checkMajorVersion, + discriminator, errorResponseSchema, requiredProperty, responseExampleProvided, diff --git a/packages/ruleset/src/functions/required-property.js b/packages/ruleset/src/functions/required-property.js index 2ed5c2f90..809f79715 100644 --- a/packages/ruleset/src/functions/required-property.js +++ b/packages/ruleset/src/functions/required-property.js @@ -1,35 +1,14 @@ +const { checkSubschemasForProperty, validateSubschemas } = require('../utils'); + module.exports = function(schema, _opts, { path }) { - return getErrorsForMissingRequiredProperties(schema, path); + return validateSubschemas(schema, path, checkRequiredProperties); }; -function getErrorsForMissingRequiredProperties(schema, path) { - const errors = []; - errors.push(...checkRequiredProperties(schema, path)); - if (schema.properties) { - Object.entries(schema.properties).forEach(function(prop) { - const propName = prop[0]; - const propSchema = prop[1]; - errors.push( - ...getErrorsForMissingRequiredProperties(propSchema, [ - ...path, - 'properties', - propName - ]) - ); - }); - } else if (schema.items) { - errors.push( - ...getErrorsForMissingRequiredProperties(schema.items, [...path, 'items']) - ); - } - return errors; -} - function checkRequiredProperties(schema, path) { const errors = []; if (Array.isArray(schema.required)) { schema.required.forEach(function(requiredPropName) { - if (!checkSchemaForProp(requiredPropName, schema)) { + if (!checkSubschemasForProperty(schema, requiredPropName)) { let message; if (schema.allOf) { message = `Required property, ${requiredPropName}, must be defined in at least one of the allOf schemas`; @@ -47,27 +26,3 @@ function checkRequiredProperties(schema, path) { } return errors; } - -function checkSchemaForProp(requiredProp, schema) { - if (schema.properties && schema.properties[requiredProp]) { - return true; - } else if (Array.isArray(schema.allOf)) { - let reqPropDefined = false; - schema.allOf.forEach(childObj => { - if (checkSchemaForProp(requiredProp, childObj)) { - reqPropDefined = true; - } - }); - return reqPropDefined; - } else if (Array.isArray(schema.anyOf) || Array.isArray(schema.oneOf)) { - const childList = schema.anyOf || schema.oneOf; - let reqPropDefined = true; - childList.forEach(childObj => { - if (!checkSchemaForProp(requiredProp, childObj)) { - reqPropDefined = false; - } - }); - return reqPropDefined; - } - return false; -} diff --git a/packages/ruleset/src/functions/string-boundary.js b/packages/ruleset/src/functions/string-boundary.js index 7dcb2869c..9d371b198 100644 --- a/packages/ruleset/src/functions/string-boundary.js +++ b/packages/ruleset/src/functions/string-boundary.js @@ -1,72 +1,40 @@ +const { validateSubschemas } = require('../utils'); + module.exports = function(schema, _opts, { path }) { - return traverseSchema(schema, path); + return validateSubschemas(schema, path, stringBoundaryErrors); }; -function traverseSchema(schema, path) { - if (schema.type === 'string') { - return stringBoundaryErrors(schema, path); - } +function stringBoundaryErrors(schema, path) { const errors = []; - if (schema.properties) { - Object.entries(schema.properties).forEach(function(prop) { - const propName = prop[0]; - const propSchema = prop[1]; - errors.push( - ...traverseSchema(propSchema, [...path, 'properties', propName]) - ); - }); - } else if (schema.items) { - errors.push(...traverseSchema(schema.items, [...path, 'items'])); - } else if (schema.allOf || schema.anyOf || schema.oneOf) { - const whichComposedSchemaType = schema.allOf - ? 'allOf' - : schema.anyOf - ? 'anyOf' - : 'oneOf'; - const composedSchemas = schema[whichComposedSchemaType]; - if (Array.isArray(composedSchemas)) { - composedSchemas.forEach(function(composedSchema, index) { - errors.push( - ...traverseSchema(composedSchema, [ - ...path, - whichComposedSchemaType, - index - ]) - ); - }); - } + if (schema.type !== 'string') { + return errors; } - return errors; -} - -function stringBoundaryErrors(stringSchema, path) { - const errors = []; - if (isUndefinedOrNull(stringSchema.enum)) { + if (isUndefinedOrNull(schema.enum)) { if ( - isUndefinedOrNull(stringSchema.pattern) && - !['binary', 'date', 'date-time'].includes(stringSchema.format) + isUndefinedOrNull(schema.pattern) && + !['binary', 'date', 'date-time'].includes(schema.format) ) { errors.push({ message: 'Should define a pattern for a valid string', path }); } - if (isUndefinedOrNull(stringSchema.minLength)) { + if (isUndefinedOrNull(schema.minLength)) { errors.push({ message: 'Should define a minLength for a valid string', path }); } - if (isUndefinedOrNull(stringSchema.maxLength)) { + if (isUndefinedOrNull(schema.maxLength)) { errors.push({ message: 'Should define a maxLength for a valid string', path }); } if ( - !isUndefinedOrNull(stringSchema.minLength) && - !isUndefinedOrNull(stringSchema.maxLength) && - stringSchema.minLength > stringSchema.maxLength + !isUndefinedOrNull(schema.minLength) && + !isUndefinedOrNull(schema.maxLength) && + schema.minLength > schema.maxLength ) { errors.push({ message: 'minLength must be less than maxLength', diff --git a/packages/ruleset/src/ibm-oas.js b/packages/ruleset/src/ibm-oas.js index 218f5af5c..dbe305fc2 100644 --- a/packages/ruleset/src/ibm-oas.js +++ b/packages/ruleset/src/ibm-oas.js @@ -90,6 +90,7 @@ module.exports = { // IBM Custom Rules 'content-entry-provided': ibmRules.contentEntryProvided, + discriminator: ibmRules.discriminator, 'content-entry-contains-schema': ibmRules.contentEntryContainsSchema, 'ibm-content-type-is-specific': ibmRules.ibmContentTypeIsSpecific, 'ibm-error-content-type-is-json': ibmRules.ibmErrorContentTypeIsJson, diff --git a/packages/ruleset/src/rules/discriminator.js b/packages/ruleset/src/rules/discriminator.js new file mode 100644 index 000000000..1e4aa5081 --- /dev/null +++ b/packages/ruleset/src/rules/discriminator.js @@ -0,0 +1,15 @@ +const { oas3 } = require('@stoplight/spectral-formats'); +const { discriminator } = require('../functions'); +const { schemas } = require('../collections'); + +module.exports = { + description: 'The discriminator property name must be defined in this schema', + message: '{{error}}', + given: schemas, + severity: 'error', + formats: [oas3], + resolved: true, + then: { + function: discriminator + } +}; diff --git a/packages/ruleset/src/rules/index.js b/packages/ruleset/src/rules/index.js index 07da74e62..c72af8444 100644 --- a/packages/ruleset/src/rules/index.js +++ b/packages/ruleset/src/rules/index.js @@ -1,4 +1,5 @@ const contentEntryContainsSchema = require('./content-entry-contains-schema'); +const discriminator = require('./discriminator'); const ibmErrorContentTypeIsJson = require('./ibm-error-content-type-is-json'); const missingRequiredProperty = require('./missing-required-property'); const responseErrorResponseSchema = require('./response-error-response-schema'); @@ -16,6 +17,7 @@ const stringBoundary = require('./string-boundary'); module.exports = { contentEntryContainsSchema, + discriminator, ibmErrorContentTypeIsJson, missingRequiredProperty, responseErrorResponseSchema, diff --git a/packages/ruleset/src/rules/missing-required-property.js b/packages/ruleset/src/rules/missing-required-property.js index 1a2190fef..9660ad50f 100644 --- a/packages/ruleset/src/rules/missing-required-property.js +++ b/packages/ruleset/src/rules/missing-required-property.js @@ -1,15 +1,12 @@ const { oas2, oas3 } = require('@stoplight/spectral-formats'); const { requiredProperty } = require('../functions'); +const { schemas } = require('../collections'); module.exports = { description: 'A required property is not in the schema', message: '{{error}}', formats: [oas2, oas3], - given: [ - '$.paths[*][*][parameters][*].schema', - '$.paths[*][*][parameters,responses][*].content[*].schema', - '$.paths[*][*][requestBody].content[*].schema' - ], + given: schemas, severity: 'error', then: { function: requiredProperty diff --git a/packages/ruleset/src/utils/check-subschemas-for-prop.js b/packages/ruleset/src/utils/check-subschemas-for-prop.js new file mode 100644 index 000000000..c37d30e06 --- /dev/null +++ b/packages/ruleset/src/utils/check-subschemas-for-prop.js @@ -0,0 +1,36 @@ +const checkSubschemasForProperty = (schema, name) => { + if (!schema) { + return false; + } + + let propertyIsDefined = false; + + // first check the properties + if (schema.properties) { + propertyIsDefined = name in schema.properties; + } else if (schema.oneOf || schema.anyOf) { + // every schema in a oneOf or anyOf must contain the property + const subschemas = schema.oneOf || schema.anyOf; + if (Array.isArray(subschemas)) { + propertyIsDefined = true; + for (const s of subschemas) { + if (!checkSubschemasForProperty(s, name)) { + propertyIsDefined = false; + break; + } + } + } + } else if (Array.isArray(schema.allOf)) { + // at least one schema in an allOf must contain the property + for (const s of schema.allOf) { + if (checkSubschemasForProperty(s, name)) { + propertyIsDefined = true; + break; + } + } + } + + return propertyIsDefined; +}; + +module.exports = checkSubschemasForProperty; diff --git a/packages/ruleset/src/utils/index.js b/packages/ruleset/src/utils/index.js new file mode 100644 index 000000000..687310c1f --- /dev/null +++ b/packages/ruleset/src/utils/index.js @@ -0,0 +1,7 @@ +const checkSubschemasForProperty = require('./check-subschemas-for-prop'); +const validateSubschemas = require('./validate-subschemas'); + +module.exports = { + checkSubschemasForProperty, + validateSubschemas +}; diff --git a/packages/ruleset/src/utils/validate-subschemas.js b/packages/ruleset/src/utils/validate-subschemas.js new file mode 100644 index 000000000..41ec599cd --- /dev/null +++ b/packages/ruleset/src/utils/validate-subschemas.js @@ -0,0 +1,59 @@ +// Subschemas include property schemas (for an object schema), item schemas +// (for an array schema), and applicator schemas (such as those in an allOf, +// anyOf, or oneOf property), plus all subschemas of those schemas. + +const validateSubschemas = (schema, path, validate) => { + const errors = []; + // invoke validation function + errors.push(...validate(schema, path)); + + // recursively process subschemas + if (schema.properties) { + for (const property of Object.entries(schema.properties)) { + errors.push( + ...validateSubschemas( + property[1], + [...path, 'properties', property[0]], + validate + ) + ); + } + } + + if (schema.items) { + errors.push( + ...validateSubschemas(schema.items, [...path, 'items'], validate) + ); + } + + if ( + schema.additionalProperties && + typeof schema.additionalProperties === 'object' + ) { + errors.push( + ...validateSubschemas( + schema.additionalProperties, + [...path, 'additionalProperties'], + validate + ) + ); + } + + if (schema.not) { + errors.push(...validateSubschemas(schema.not, [...path, 'not'], validate)); + } + + for (const applicatorType of ['allOf', 'oneOf', 'anyOf']) { + if (Array.isArray(schema[applicatorType])) { + schema[applicatorType].forEach((s, i) => { + errors.push( + ...validateSubschemas(s, [...path, applicatorType, i], validate) + ); + }); + } + } + + return errors; +}; + +module.exports = validateSubschemas; diff --git a/packages/ruleset/test/discriminator.test.js b/packages/ruleset/test/discriminator.test.js new file mode 100644 index 000000000..97e368eab --- /dev/null +++ b/packages/ruleset/test/discriminator.test.js @@ -0,0 +1,342 @@ +const { discriminator } = require('../src/rules'); +const { makeCopy, rootDocument, testRule, severityCodes } = require('./utils'); + +const name = 'discriminator'; + +describe('Spectral rule: discriminator', () => { + it('should not error with a clean spec', async () => { + // tests oneOf with all properties containing the property + const results = await testRule(name, discriminator, rootDocument); + + expect(results).toHaveLength(0); + }); + + it('should not error if property is present in all oneOf nested subschemas', async () => { + const testDocument = makeCopy(rootDocument); + testDocument.components.schemas.Soda = { + oneOf: [ + { + type: 'object', + properties: { + type: { + type: 'string' + } + } + }, + { + type: 'object', + properties: { + type: { + type: 'string' + } + } + } + ] + }; + + const results = await testRule(name, discriminator, testDocument); + + expect(results).toHaveLength(0); + }); + + it('should not error if property is present in all anyOf nested subschemas', async () => { + const testDocument = makeCopy(rootDocument); + testDocument.components.schemas.Soda = { + anyOf: [ + { + type: 'object', + properties: { + type: { + type: 'string' + } + } + }, + { + type: 'object', + properties: { + type: { + type: 'string' + } + } + } + ] + }; + + const results = await testRule(name, discriminator, testDocument); + + expect(results).toHaveLength(0); + }); + + it('should not error if property is present in at least one allOf nested subschemas', async () => { + const testDocument = makeCopy(rootDocument); + testDocument.components.schemas.Soda = { + allOf: [ + { + type: 'object', + properties: { + name: { + type: 'string' + } + } + }, + { + type: 'object', + properties: { + type: { + type: 'string' + } + } + } + ] + }; + + const results = await testRule(name, discriminator, testDocument); + + expect(results).toHaveLength(0); + }); + + it('should not error if all anyOf properties have a schema', async () => { + const testDocument = makeCopy(rootDocument); + testDocument.components.schemas.Drink = { + anyOf: [ + { + $ref: '#/components/schemas/Juice' + }, + { + $ref: '#/components/schemas/Soda' + } + ], + discriminator: { + propertyName: 'type' + } + }; + + const results = await testRule(name, discriminator, testDocument); + + expect(results).toHaveLength(0); + }); + + it('should error if discriminator is not present in all oneOf subschemas', async () => { + const testDocument = makeCopy(rootDocument); + testDocument.components.schemas.Soda = { + type: 'object', + required: ['name'], + properties: { + name: { + $ref: '#/components/schemas/NormalString' + } + } + }; + + const results = await testRule(name, discriminator, testDocument); + + // the spectral path resolution logic is ignored for these tests so the + // request and response instance are both reported + expect(results).toHaveLength(2); + + const validation = results[0]; + expect(validation.code).toBe(name); + expect(validation.message).toBe( + 'The discriminator property name used must be defined in this schema' + ); + expect(validation.path).toStrictEqual([ + 'paths', + '/v1/drinks', + 'post', + 'requestBody', + 'content', + 'application/json', + 'schema', + 'discriminator', + 'propertyName' + ]); + expect(validation.severity).toBe(severityCodes.error); + }); + + it('should error if discriminator is not present in all anyOf subschemas', async () => { + const testDocument = makeCopy(rootDocument); + testDocument.components.schemas.Drink = { + anyOf: [ + { + $ref: '#/components/schemas/Juice' + }, + { + $ref: '#/components/schemas/Soda' + } + ], + discriminator: { + propertyName: 'type' + } + }; + + testDocument.components.schemas.Juice = { + type: 'object', + required: ['fruit'], + properties: { + fruit: { + $ref: '#/components/schemas/NormalString' + } + } + }; + + const results = await testRule(name, discriminator, testDocument); + + expect(results).toHaveLength(2); + + const validation = results[0]; + expect(validation.code).toBe(name); + expect(validation.message).toBe( + 'The discriminator property name used must be defined in this schema' + ); + expect(validation.path).toStrictEqual([ + 'paths', + '/v1/drinks', + 'post', + 'requestBody', + 'content', + 'application/json', + 'schema', + 'discriminator', + 'propertyName' + ]); + expect(validation.severity).toBe(severityCodes.error); + }); + + it('should error if discriminator is not present in all oneOf nested subschemas', async () => { + const testDocument = makeCopy(rootDocument); + testDocument.components.schemas.Soda = { + oneOf: [ + { + type: 'object', + properties: { + brand: { + type: 'string' + } + } + }, + { + type: 'object', + properties: { + type: { + type: 'string' + } + } + } + ] + }; + + const results = await testRule(name, discriminator, testDocument); + + expect(results).toHaveLength(2); + + const validation = results[0]; + expect(validation.code).toBe(name); + expect(validation.message).toBe( + 'The discriminator property name used must be defined in this schema' + ); + expect(validation.path).toStrictEqual([ + 'paths', + '/v1/drinks', + 'post', + 'requestBody', + 'content', + 'application/json', + 'schema', + 'discriminator', + 'propertyName' + ]); + expect(validation.severity).toBe(severityCodes.error); + }); + + it('should error if property is not present in all anyOf nested subschemas', async () => { + const testDocument = makeCopy(rootDocument); + testDocument.components.schemas.Soda = { + anyOf: [ + { + type: 'object', + properties: { + brand: { + type: 'string' + } + } + }, + { + type: 'object', + properties: { + type: { + type: 'string' + } + } + } + ] + }; + + const results = await testRule(name, discriminator, testDocument); + + expect(results).toHaveLength(2); + + const validation = results[0]; + expect(validation.code).toBe(name); + expect(validation.message).toBe( + 'The discriminator property name used must be defined in this schema' + ); + expect(validation.path).toStrictEqual([ + 'paths', + '/v1/drinks', + 'post', + 'requestBody', + 'content', + 'application/json', + 'schema', + 'discriminator', + 'propertyName' + ]); + expect(validation.severity).toBe(severityCodes.error); + }); + + it('should not error if property is present in at least one allOf nested subschemas', async () => { + const testDocument = makeCopy(rootDocument); + testDocument.components.schemas.Soda = { + allOf: [ + { + type: 'object', + properties: { + brand: { + type: 'string' + } + } + }, + { + type: 'object', + properties: { + flavor: { + type: 'string' + } + } + } + ] + }; + + const results = await testRule(name, discriminator, testDocument); + + expect(results).toHaveLength(2); + + const validation = results[0]; + expect(validation.code).toBe(name); + expect(validation.message).toBe( + 'The discriminator property name used must be defined in this schema' + ); + expect(validation.path).toStrictEqual([ + 'paths', + '/v1/drinks', + 'post', + 'requestBody', + 'content', + 'application/json', + 'schema', + 'discriminator', + 'propertyName' + ]); + expect(validation.severity).toBe(severityCodes.error); + }); +}); diff --git a/packages/ruleset/test/major-version-in-path.test.js b/packages/ruleset/test/major-version-in-path.test.js index 7feac186e..8330adc05 100644 --- a/packages/ruleset/test/major-version-in-path.test.js +++ b/packages/ruleset/test/major-version-in-path.test.js @@ -53,8 +53,10 @@ describe('Spectral rule: major-version-in-path', () => { it('should error when no version is indicated anywhere', async () => { const testDocument = makeCopy(rootDocument); - delete testDocument.paths['/v1/movies']; - testDocument.paths['/movies'] = {}; + delete testDocument.paths; + testDocument.paths = { + '/movies': {} + }; const results = await testRule(name, majorVersionInPath, testDocument); diff --git a/packages/ruleset/test/utils/all-schemas-document.js b/packages/ruleset/test/utils/all-schemas-document.js new file mode 100644 index 000000000..71fb0521d --- /dev/null +++ b/packages/ruleset/test/utils/all-schemas-document.js @@ -0,0 +1,210 @@ +module.exports = { + openapi: '3.0.2', + info: { + title: 'Subschema examples', + description: + 'A collection of schemas with various kinds of subschemas for testing.', + version: '0.0.1', + contact: { + email: 'example@example.com' + } + }, + tags: [ + { + name: 'Index' + } + ], + servers: [ + { + url: '/api/v3' + } + ], + paths: { + '/schema': { + get: { + tags: ['Index'], + summary: 'Get the index', + description: 'Get the index.', + operationId: 'get_index', + responses: { + '200': { + description: "Here's the index.", + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Index' + } + } + } + } + } + } + }, + '/every_flavor': { + get: { + tags: ['Index'], + summary: 'Get every flavor', + description: 'Get every flavor.', + operationId: 'get_every_flavor', + responses: { + '200': { + description: "Here's every flavor.", + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/EveryFlavor' + } + } + } + } + } + } + } + }, + components: { + schemas: { + Index: { + type: 'object', + properties: { + schema_with_property_schema: { + $ref: '#/components/schemas/SchemaWithPropertySchema' + }, + schema_with_additional_properties_schema: { + $ref: '#/components/schemas/SchemaWithAdditionalPropertiesSchema' + }, + schema_with_items_schema: { + $ref: '#/components/schemas/SchemaWithItemsSchema' + }, + schema_with_all_of_schema: { + $ref: '#/components/schemas/SchemaWithAllOfSchema' + }, + schema_with_one_of_schema: { + $ref: '#/components/schemas/SchemaWithOneOfSchema' + }, + schema_with_any_of_schema: { + $ref: '#/components/schemas/SchemaWithAnyOfSchema' + }, + schema_with_not_schema: { + $ref: '#/components/schemas/SchemaWithNotSchema' + } + } + }, + EveryFlavor: { + properties: { + property_schema: { + $ref: '#/components/schemas/SchemaWithPropertySchema' + } + }, + additionalProperties: { + $ref: '#/components/schemas/AdditionalPropertiesSchema' + }, + items: { + $ref: '#/components/schemas/ItemsSchema' + }, + allOf: [ + { + $ref: '#/components/schemas/AllOfSchema' + } + ], + oneOf: [ + { + $ref: '#/components/schemas/OneOfSchema' + } + ], + anyOf: [ + { + $ref: '#/components/schemas/AnyOfSchema' + } + ], + not: { + $ref: '#/components/schemas/NotSchema' + } + }, + SchemaWithPropertySchema: { + type: 'object', + properties: { + property_schema: { + $ref: '#/components/schemas/PropertySchema' + } + } + }, + PropertySchema: { + type: 'string', + description: + 'This schema is reachable from `EveryFlavor` and `SchemaWithPropertySchema`.' + }, + SchemaWithAdditionalPropertiesSchema: { + type: 'object', + additionalProperties: { + $ref: '#/components/schemas/AdditionalPropertiesSchema' + } + }, + AdditionalPropertiesSchema: { + type: 'string', + description: + 'This schema is reachable from `EveryFlavor` and `SchemaWithAdditionalPropertiesSchema`.' + }, + SchemaWithItemsSchema: { + type: 'array', + items: { + $ref: '#/components/schemas/ItemsSchema' + } + }, + ItemsSchema: { + type: 'string', + description: + 'This schema is reachable from `EveryFlavor` and `SchemaWithItemsSchema`.' + }, + SchemaWithAllOfSchema: { + type: 'string', + allOf: [ + { + $ref: '#/components/schemas/AllOfSchema' + } + ] + }, + AllOfSchema: { + type: 'string', + description: + 'This schema is reachable from `EveryFlavor` and `SchemaWithAllOfSchema`.' + }, + SchemaWithOneOfSchema: { + type: 'string', + oneOf: [ + { + $ref: '#/components/schemas/OneOfSchema' + } + ] + }, + OneOfSchema: { + type: 'string', + description: + 'This schema is reachable from `EveryFlavor` and `SchemaWithOneOfSchema`.' + }, + SchemaWithAnyOfSchema: { + type: 'string', + anyOf: [ + { + $ref: '#/components/schemas/AnyOfSchema' + } + ] + }, + AnyOfSchema: { + type: 'string', + description: + 'This schema is reachable from `EveryFlavor` and `SchemaWithAnyOfSchema`.' + }, + SchemaWithNotSchema: { + type: 'string', + not: { + $ref: '#/components/schemas/NotSchema' + } + }, + NotSchema: { + type: 'string', + description: + 'This schema is reachable from `EveryFlavor` and `SchemaWithNotSchema`.' + } + } + } +}; diff --git a/packages/ruleset/test/utils/index.js b/packages/ruleset/test/utils/index.js index ae9a46dfe..4c42f8187 100644 --- a/packages/ruleset/test/utils/index.js +++ b/packages/ruleset/test/utils/index.js @@ -1,9 +1,11 @@ +const allSchemasDocument = require('./all-schemas-document'); const makeCopy = require('./make-copy'); const testRule = require('./test-rule'); const rootDocument = require('./root-document'); const severityCodes = require('./severity-codes'); module.exports = { + allSchemasDocument, makeCopy, rootDocument, testRule, diff --git a/packages/ruleset/test/utils/root-document.js b/packages/ruleset/test/utils/root-document.js index 2831b08ed..2d7e45a69 100644 --- a/packages/ruleset/test/utils/root-document.js +++ b/packages/ruleset/test/utils/root-document.js @@ -11,6 +11,52 @@ module.exports = { } ], paths: { + '/v1/drinks': { + post: { + operationId: 'createDrink', + summary: 'Create a drink', + requestBody: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Drink' + } + } + } + }, + responses: { + '201': { + description: 'Success!', + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Drink' + } + } + } + }, + '400': { + description: 'Error!', + content: { + 'application/json': { + schema: { + type: 'object', + properties: { + trace: { + type: 'string', + format: 'uuid' + }, + error: { + $ref: '#/components/schemas/RequestError' + } + } + } + } + } + } + } + } + }, '/v1/movies': { post: { operationId: 'createMovie', @@ -94,23 +140,13 @@ module.exports = { required: ['id', 'name'], properties: { id: { - type: 'string', - readOnly: true, - pattern: '[a-zA-Z0-9]+', - minLength: 1, - maxLength: 30 + $ref: '#/components/schemas/IdString' }, name: { - type: 'string', - pattern: '[a-zA-Z0-9]+', - minLength: 1, - maxLength: 30 + $ref: '#/components/schemas/NormalString' }, director: { - type: 'string', - pattern: '[a-zA-Z0-9]+', - minLength: 1, - maxLength: 30 + $ref: '#/components/schemas/NormalString' } }, example: { @@ -118,6 +154,62 @@ module.exports = { director: 'Peter Jackson' } }, + Drink: { + oneOf: [ + { + $ref: '#/components/schemas/Juice' + }, + { + $ref: '#/components/schemas/Soda' + } + ], + discriminator: { + propertyName: 'type' + }, + example: { + type: 'soda', + name: 'Root Beer' + } + }, + Soda: { + type: 'object', + required: ['type', 'name'], + properties: { + type: { + type: 'string', + enum: ['soda'] + }, + name: { + $ref: '#/components/schemas/NormalString' + } + } + }, + Juice: { + type: 'object', + required: ['type', 'fruit'], + properties: { + type: { + type: 'string', + enum: ['juice'] + }, + fruit: { + $ref: '#/components/schemas/NormalString' + } + } + }, + NormalString: { + type: 'string', + pattern: '[a-zA-Z0-9 ]+', + minLength: 1, + maxLength: 30 + }, + IdString: { + type: 'string', + readOnly: true, + pattern: '[a-zA-Z0-9]+', + minLength: 1, + maxLength: 10 + }, RequestError: { type: 'object', properties: { diff --git a/packages/ruleset/test/validate-subschemas.test.js b/packages/ruleset/test/validate-subschemas.test.js new file mode 100644 index 000000000..57d3e8dd2 --- /dev/null +++ b/packages/ruleset/test/validate-subschemas.test.js @@ -0,0 +1,104 @@ +const { allSchemasDocument, testRule } = require('./utils'); +const { schemas } = require('../src/collections'); +const { validateSubschemas } = require('../src/utils'); + +describe('Utility: validateSubschemas', () => { + const visitedSchemas = []; + + function pathRecorder(schema, path) { + visitedSchemas.push(path.join('.')); + return []; + } + + function ruleFunction(schema, _opts, { path }) { + return validateSubschemas(schema, path, pathRecorder); + } + + // this needs to be executed as a spectral rule to resolve the document + const ruleForTesting = { + given: schemas, + resolved: true, + then: { + function: ruleFunction + } + }; + + it('should find all subschemas', async () => { + await testRule('rule-name', ruleForTesting, allSchemasDocument); + + expect(visitedSchemas.length).toBe(24); + + expect(visitedSchemas[0]).toBe( + 'paths./schema.get.responses.200.content.application/json.schema' + ); + expect(visitedSchemas[1]).toBe( + 'paths./schema.get.responses.200.content.application/json.schema.properties.schema_with_property_schema' + ); + expect(visitedSchemas[2]).toBe( + 'paths./schema.get.responses.200.content.application/json.schema.properties.schema_with_property_schema.properties.property_schema' + ); + expect(visitedSchemas[3]).toBe( + 'paths./schema.get.responses.200.content.application/json.schema.properties.schema_with_additional_properties_schema' + ); + expect(visitedSchemas[4]).toBe( + 'paths./schema.get.responses.200.content.application/json.schema.properties.schema_with_additional_properties_schema.additionalProperties' + ); + expect(visitedSchemas[5]).toBe( + 'paths./schema.get.responses.200.content.application/json.schema.properties.schema_with_items_schema' + ); + expect(visitedSchemas[6]).toBe( + 'paths./schema.get.responses.200.content.application/json.schema.properties.schema_with_items_schema.items' + ); + expect(visitedSchemas[7]).toBe( + 'paths./schema.get.responses.200.content.application/json.schema.properties.schema_with_all_of_schema' + ); + expect(visitedSchemas[8]).toBe( + 'paths./schema.get.responses.200.content.application/json.schema.properties.schema_with_all_of_schema.allOf.0' + ); + expect(visitedSchemas[9]).toBe( + 'paths./schema.get.responses.200.content.application/json.schema.properties.schema_with_one_of_schema' + ); + expect(visitedSchemas[10]).toBe( + 'paths./schema.get.responses.200.content.application/json.schema.properties.schema_with_one_of_schema.oneOf.0' + ); + expect(visitedSchemas[11]).toBe( + 'paths./schema.get.responses.200.content.application/json.schema.properties.schema_with_any_of_schema' + ); + expect(visitedSchemas[12]).toBe( + 'paths./schema.get.responses.200.content.application/json.schema.properties.schema_with_any_of_schema.anyOf.0' + ); + expect(visitedSchemas[13]).toBe( + 'paths./schema.get.responses.200.content.application/json.schema.properties.schema_with_not_schema' + ); + expect(visitedSchemas[14]).toBe( + 'paths./schema.get.responses.200.content.application/json.schema.properties.schema_with_not_schema.not' + ); + expect(visitedSchemas[15]).toBe( + 'paths./every_flavor.get.responses.200.content.application/json.schema' + ); + expect(visitedSchemas[16]).toBe( + 'paths./every_flavor.get.responses.200.content.application/json.schema.properties.property_schema' + ); + expect(visitedSchemas[17]).toBe( + 'paths./every_flavor.get.responses.200.content.application/json.schema.properties.property_schema.properties.property_schema' + ); + expect(visitedSchemas[18]).toBe( + 'paths./every_flavor.get.responses.200.content.application/json.schema.items' + ); + expect(visitedSchemas[19]).toBe( + 'paths./every_flavor.get.responses.200.content.application/json.schema.additionalProperties' + ); + expect(visitedSchemas[20]).toBe( + 'paths./every_flavor.get.responses.200.content.application/json.schema.not' + ); + expect(visitedSchemas[21]).toBe( + 'paths./every_flavor.get.responses.200.content.application/json.schema.allOf.0' + ); + expect(visitedSchemas[22]).toBe( + 'paths./every_flavor.get.responses.200.content.application/json.schema.oneOf.0' + ); + expect(visitedSchemas[23]).toBe( + 'paths./every_flavor.get.responses.200.content.application/json.schema.anyOf.0' + ); + }); +}); diff --git a/packages/validator/src/plugins/validation/oas3/semantic-validators/discriminator.js b/packages/validator/src/plugins/validation/oas3/semantic-validators/discriminator.js deleted file mode 100644 index f5bad23c2..000000000 --- a/packages/validator/src/plugins/validation/oas3/semantic-validators/discriminator.js +++ /dev/null @@ -1,53 +0,0 @@ -// Assertation 1: -// if discriminator exist inside schema object, it must be of type Object -// enforced by Spectral's oas3-schema rule - -// Assertion 2: -// discriminator object must have a field name propertyName -// enforced by Spectral's oas3-schema rule - -// Assertation 3: -// propertyName is of type string -// enforced by Spectral's oas3-schema rule - -// Assertation 4: -// properties inside a schema object must include propertyName from discriminator object - -const each = require('lodash/each'); -const has = require('lodash/has'); -const get = require('lodash/get'); -const isPlainObject = require('lodash/isPlainObject'); -const MessageCarrier = require('../../../utils/message-carrier'); - -module.exports.validate = function({ jsSpec }) { - const messages = new MessageCarrier(); - - const schemas = get(jsSpec, ['components', 'schemas'], []); - - const basePath = ['components', 'schemas']; - - each(schemas, (schema, schemaName) => { - if (has(schema, 'discriminator')) { - const { discriminator } = schema; - - if (!isPlainObject(discriminator)) { - return; - } - - const { propertyName } = discriminator; - // If the schema's property doesn't include propertyName defined in discriminator, add error and return - const { properties } = schema; - if (!has(properties, propertyName)) { - messages.addMessage( - basePath - .concat([schemaName, 'discriminator', 'propertyName']) - .join('.'), - 'The discriminator property name used must be defined in this schema', - 'error' - ); - return; - } - } - }); - return messages; -}; diff --git a/packages/validator/test/plugins/validation/oas3/discriminator.test.js b/packages/validator/test/plugins/validation/oas3/discriminator.test.js deleted file mode 100644 index dfa74cfa3..000000000 --- a/packages/validator/test/plugins/validation/oas3/discriminator.test.js +++ /dev/null @@ -1,71 +0,0 @@ -const expect = require('expect'); -const { - validate -} = require('../../../../src/plugins/validation/oas3/semantic-validators/discriminator'); - -describe('validation plugin - semantic - oas3 discriminator', () => { - it('should not return errors when schema is defined correctly', () => { - const spec = { - components: { - schemas: { - Pet: { - discriminator: { - propertyName: 'petType' - }, - properties: { - petType: { - type: 'string' - } - } - } - } - } - }; - const res = validate({ jsSpec: spec }); - expect(res.errors.length).toEqual(0); - }); - - it('should return an error when propertyName is defined inside discriminator object but not in schema properties', () => { - const spec = { - components: { - schemas: { - Pet: { - discriminator: { - propertyName: 'petType' - }, - properties: { - name: { - type: 'string' - } - } - }, - Food: { - discriminator: { - propertyName: 'expirationDate' - }, - properties: {} - } - } - } - }; - const res = validate({ jsSpec: spec }); - expect(res.errors.length).toEqual(2); - expect(res.errors[0].path).toEqual( - ['components', 'schemas', 'Pet', 'discriminator', 'propertyName'].join( - '.' - ) - ); - expect(res.errors[0].message).toEqual( - 'The discriminator property name used must be defined in this schema' - ); - - expect(res.errors[1].path).toEqual( - ['components', 'schemas', 'Food', 'discriminator', 'propertyName'].join( - '.' - ) - ); - expect(res.errors[1].message).toEqual( - 'The discriminator property name used must be defined in this schema' - ); - }); -});