diff --git a/docs/ibm-cloud-rules.md b/docs/ibm-cloud-rules.md index 368a828b8..7c166b80e 100644 --- a/docs/ibm-cloud-rules.md +++ b/docs/ibm-cloud-rules.md @@ -91,6 +91,7 @@ which is delivered in the `@ibm-cloud/openapi-ruleset` NPM package. * [ibm-response-status-codes](#ibm-response-status-codes) * [ibm-schema-description](#ibm-schema-description) * [ibm-schema-keywords](#ibm-schema-keywords) + * [ibm-schema-naming-convention](#ibm-schema-naming-convention) * [ibm-schema-type](#ibm-schema-type) * [ibm-schema-type-format](#ibm-schema-type-format) * [ibm-sdk-operations](#ibm-sdk-operations) @@ -518,6 +519,12 @@ specific "allow-listed" keywords. oas3_1 +ibm-schema-naming-convention +warn +Schemas should follow the API Handbook naming conventions. +oas3 + + ibm-schema-type off Schemas and schema properties should have a non-empty type field. This rule is disabled by default. @@ -5493,6 +5500,77 @@ components: +### ibm-schema-naming-convention + + + + + + + + + + + + + + + + + + + + + + + + + +
Rule id:ibm-schema-naming-convention
Description: +The name of each schema should follow the IBM Cloud API Handbook schema naming conventions. + +The rule checks the names of collection schemas, resource collection element schemas, creation/replacement schemas, and patch schemas against the name of the associated canonical schema to ensure the names follow the guidelines. +
Severity:warn
OAS Versions:oas3
Non-compliant example: +
+paths:
+  /v1/things:
+    post:
+      requestBody:
+        content:
+          'application/json':
+            schema:
+              $ref: '#/components/schemas/ThingCreator' # Should be ThingPrototype
+  /v1/things/{id}:
+    get:
+      responses:
+        200:
+          content:
+            'application/json':
+              schema:
+                $ref: '#/components/schemas/Thing' # Canonical schema
+
+
Compliant example: +
+paths:
+  /v1/things:
+    post:
+      requestBody:
+        content:
+          'application/json':
+            schema:
+              $ref: '#/components/schemas/ThingPrototype'
+  /v1/things/{id}:
+    get:
+      responses:
+        200:
+          content:
+            'application/json':
+              schema:
+                $ref: '#/components/schemas/Thing'
+
+
+ + ### ibm-schema-type diff --git a/packages/ruleset/src/functions/index.js b/packages/ruleset/src/functions/index.js index dda82a26f..933cd35bf 100644 --- a/packages/ruleset/src/functions/index.js +++ b/packages/ruleset/src/functions/index.js @@ -55,6 +55,7 @@ module.exports = { responseExampleExists: require('./response-example-exists'), responseStatusCodes: require('./response-status-codes'), schemaDescriptionExists: require('./schema-description-exists'), + schemaNamingConvention: require('./schema-naming-convention'), schemaOrContentProvided: require('./schema-or-content-provided'), schemaTypeExists: require('./schema-type-exists'), schemaTypeFormat: require('./schema-type-format'), diff --git a/packages/ruleset/src/functions/resource-response-consistency.js b/packages/ruleset/src/functions/resource-response-consistency.js index 92bb0885c..1760764fa 100644 --- a/packages/ruleset/src/functions/resource-response-consistency.js +++ b/packages/ruleset/src/functions/resource-response-consistency.js @@ -8,6 +8,7 @@ const { isObject } = require('@ibm-cloud/openapi-ruleset-utilities'); const { getResourceSpecificSiblingPath, getResponseCodes, + getSuccessResponseSchemaForOperation, isCreateOperation, isJsonMimeType, isOperationOfType, @@ -167,9 +168,9 @@ function resourceResponseConsistency(operation, path, apidef) { function getCanonicalSchema(path, apidef) { const resourceSpecificPath = isOperationOfType('post', path) - ? getResourceSpecificSiblingPath(path, apidef) + ? getResourceSpecificSiblingPath(path.at(-2), apidef) : // This is a PUT or PATCH and should already be on the path we need - path[path.length - 2].toString().trim(); + path.at(-2).trim(); logger.debug( `${ruleId}: calculated resource-specific path to be "${resourceSpecificPath}"` @@ -189,62 +190,11 @@ function getCanonicalSchema(path, apidef) { ); return; } - if (!resourceGetOperation.responses) { - logger.debug( - `${ruleId}: no responses defined on GET operation at path "${resourceSpecificPath}"` - ); - return; - } - const [, getOpSuccessCodes] = getResponseCodes( - resourceGetOperation.responses - ); - - logger.debug( - `${ruleId}: corresponding GET operation has the following status codes: ${getOpSuccessCodes.join( - ', ' - )}` - ); - - if ( - !getOpSuccessCodes || - !getOpSuccessCodes.length || - !getOpSuccessCodes.includes('200') - ) { - logger.debug( - `${ruleId}: no 200 success code found in responses defined on GET operation at path "${resourceSpecificPath}"` - ); - return; - } - - const successResponse = resourceGetOperation.responses['200']; - if (!successResponse.content || !isObject(successResponse.content)) { - return; - } - - // Find the first content object determined to be JSON - - // we are only interested in JSON content. - const jsonMimeType = Object.keys(successResponse.content).find(mimeType => - isJsonMimeType(mimeType) - ); - if (!jsonMimeType) { - logger.debug( - `${ruleId}: no JSON content found on 200 response defined on GET operation at path "${resourceSpecificPath}"` - ); - return; - } - - const jsonContent = successResponse.content[jsonMimeType]; - if (!isObject(jsonContent) || !jsonContent.schema) { - logger.debug( - `${ruleId}: no JSON content schema found in 200 response defined on GET operation at path "${resourceSpecificPath}"` - ); - return; - } - - logger.debug( - `${ruleId}: found schema in ${jsonMimeType} content object in 200 response defined on GET operation at path "${resourceSpecificPath}"` + const { schemaObject } = getSuccessResponseSchemaForOperation( + resourceGetOperation, + path ); - return jsonContent.schema; + return schemaObject; } diff --git a/packages/ruleset/src/functions/response-status-codes.js b/packages/ruleset/src/functions/response-status-codes.js index f5e79994b..a33aae18f 100644 --- a/packages/ruleset/src/functions/response-status-codes.js +++ b/packages/ruleset/src/functions/response-status-codes.js @@ -168,7 +168,10 @@ function responseStatusCodes(operation, path, apidef) { } function hasBodyRepresentation(path, apidef) { - const resourceSpecificPath = getResourceSpecificSiblingPath(path, apidef); + const resourceSpecificPath = getResourceSpecificSiblingPath( + path.at(-2), + apidef + ); logger.debug( `${ruleId}: calculated resource-specific path to be "${resourceSpecificPath}"` diff --git a/packages/ruleset/src/functions/schema-naming-convention.js b/packages/ruleset/src/functions/schema-naming-convention.js new file mode 100644 index 000000000..627993c3c --- /dev/null +++ b/packages/ruleset/src/functions/schema-naming-convention.js @@ -0,0 +1,476 @@ +/** + * Copyright 2023 IBM Corporation. + * SPDX-License-Identifier: Apache2.0 + */ + +const { schemaHasProperty } = require('@ibm-cloud/openapi-ruleset-utilities'); + +const { + LoggerFactory, + getRequestBodySchemaForOperation, + getResourceSpecificSiblingPath, + getSuccessResponseSchemaForOperation, +} = require('../utils'); + +let ruleId; +let logger; + +/** + * The implementation for this rule makes assumptions that are dependent on the + * presence of the following other rules: + * + * - ibm-request-and-response-content: all request bodies and relevant success + * responses define content objects + * + * - ibm-content-contains-schema: the content objects define schemas + * + * - ibm-avoid-inline-schemas: all schemas are named (defined with references) + * + * - ibm-collection-array-property: the presence and correct name of the + * property in a collection schema that holds the resource list + * + * - (yet to be developed): schema names use upper camel case + */ + +module.exports = function schemaNames(apidef, options, context) { + if (!logger) { + ruleId = context.rule.name; + logger = LoggerFactory.getInstance().getLogger(ruleId); + } + return checkSchemaNames(apidef, context.documentInventory.graph.nodes); +}; + +/** + * This function checks for most of the API Handbook's schema naming conventions. + * For each pair of resource-oriented paths, it determines the name of the + * canonical schema, then checks the name of the collection schema, collection + * schema resource list property items schema, creation schemas, and patch schema + * against the canonical schema to ensure they are appropriately named. + * + * The conventions we are unable to cover in this rule are: + * - The name of the canonical schema relative to the path segments (pluralization is + * too difficult of a problem to solve for in a robust way) + * - The names of Identity or Reference schemas (no solid heuristic for determining these) + * + * @param {*} apidef the entire, resolved API definition as an object + * @param {*} nodes the spectral-computed graph nodes mapping paths to referenced schemas + * @returns an array containing the violations found or [] if no violations + */ +function checkSchemaNames(apidef, nodes) { + const pathToReferencesMap = computeRefsAtPaths(nodes); + const resourceOrientedPaths = collectResourceOrientedPaths(apidef); + + if (Object.keys(resourceOrientedPaths).length === 0) { + logger.debug(`${ruleId}: no resource-oriented paths found, skipping rule`); + } + + const errors = []; + + for (const [genericPath, specificPath] of Object.entries( + resourceOrientedPaths + )) { + logger.debug( + `${ruleId}: found resource path pair: "${genericPath}" and "${specificPath}"` + ); + + // Look for the canonical schema by checking the GET operation on the + // resource-specific path. If we can't find it, we'll exit because we'll + // have no basis of comparison for the other schema names. + const canonicalSchemaInfo = getSuccessResponseSchemaForOperation( + apidef.paths[specificPath].get, + `paths.${specificPath}.get` + ); + + const canonicalSchemaPath = canonicalSchemaInfo.schemaPath; + logger.debug( + `${ruleId}: found the path to the canonical schema to be ${canonicalSchemaPath}` + ); + + const canonicalSchemaName = getSchemaNameAtPath( + canonicalSchemaPath, + pathToReferencesMap + ); + logger.debug( + `${ruleId}: found the name of the canonical schema to be ${canonicalSchemaName}` + ); + + // If we can't find the canonical schema, + // don't perform the rest of the checks. + if (!canonicalSchemaName) { + continue; + } + + // 2. Collection schemas + const collectionSchemaInfo = getSuccessResponseSchemaForOperation( + apidef.paths[genericPath].get, + `paths.${genericPath}.get` + ); + + const collectionSchemaPath = collectionSchemaInfo.schemaPath; + logger.debug( + `${ruleId}: found the path to the collection schema to be ${collectionSchemaPath}` + ); + + if (collectionSchemaPath) { + const collectionSchemaName = getSchemaNameAtPath( + collectionSchemaPath, + pathToReferencesMap + ); + logger.debug( + `${ruleId}: found the name of the collection schema to be ${collectionSchemaName}` + ); + + // 2a) Check the name of the collection schema itself + if ( + collectionSchemaName && + collectionSchemaName !== `${canonicalSchemaName}Collection` + ) { + logger.debug( + `${ruleId}: error! collection schema does not follow conventions` + ); + + errors.push({ + message: `Collection schema for path '${genericPath}' should use the name: ${canonicalSchemaName}Collection`, + path: collectionSchemaPath.split('.'), + }); + } + + // Locate the actual schema in order to check its list property. + const collectionSchema = collectionSchemaPath + .split('.') + .reduce( + (openApiArtifact, pathElement) => openApiArtifact[pathElement], + apidef + ); + + if (collectionSchema) { + logger.debug(`${ruleId}: found collection schema object`); + + // The name of the array property should match the last segment of the + // generic path. We already check for this in `ibm-collection-array-property`. + const resourceListPropName = genericPath.split('/').at(-1); + logger.debug( + `${ruleId}: expecting the resource list property to be called ${resourceListPropName}` + ); + + if (schemaHasProperty(collectionSchema, resourceListPropName)) { + logger.debug( + `${ruleId}: found resource list property named ${resourceListPropName}` + ); + + const resourceListSchemaPath = computePathToProperty( + resourceListPropName, + collectionSchemaPath, + collectionSchema + ); + logger.debug( + `${ruleId}: found the path to the resource list property to be ${resourceListSchemaPath}` + ); + + const listPropItemsSchemaPath = `${resourceListSchemaPath}.items`; + logger.debug( + `${ruleId}: found the path to the resource list property items to be ${listPropItemsSchemaPath}` + ); + + const listPropItemsSchemaName = getSchemaNameAtPath( + listPropItemsSchemaPath, + pathToReferencesMap + ); + logger.debug( + `${ruleId}: found the name of the resource list property items schema to be ${listPropItemsSchemaName}` + ); + + if ( + listPropItemsSchemaName && + listPropItemsSchemaName !== canonicalSchemaName && + listPropItemsSchemaName !== `${canonicalSchemaName}Summary` + ) { + logger.debug( + `${ruleId}: reporting error! list prop in collection is wrong` + ); + errors.push({ + message: `Items schema for collection resource list property '${resourceListPropName}' should use the name: ${canonicalSchemaName} or ${canonicalSchemaName}Summary`, + path: listPropItemsSchemaPath.split('.'), + }); + } + } + } + } + + // 3. Prototype schema + // 3a) post + const postRequestSchemaInfo = getRequestBodySchemaForOperation( + apidef.paths[genericPath].post, + `paths.${genericPath}.post` + ); + + const postRequestSchemaPath = postRequestSchemaInfo.schemaPath; + logger.debug( + `${ruleId}: found the path to the prototype schema (for post) to be ${postRequestSchemaPath}` + ); + + if (postRequestSchemaPath) { + const postRequestSchemaName = getSchemaNameAtPath( + postRequestSchemaPath, + pathToReferencesMap + ); + logger.debug( + `${ruleId}: found the name of the prototype schema (for post) to be ${postRequestSchemaName}` + ); + + if ( + postRequestSchemaName && + postRequestSchemaName !== `${canonicalSchemaName}Prototype` && + // The API Handbook makes an exception for cases where the canonical + // schema itself should be used for creation + postRequestSchemaName !== canonicalSchemaName + ) { + logger.debug(`${ruleId}: reporting error! post prototype is wrong`); + errors.push({ + message: `Prototype schema (POST request body) for path '${genericPath}' should use the name: ${canonicalSchemaName}Prototype`, + path: postRequestSchemaPath.split('.'), + }); + } + } + + // 3a) put + const putRequestSchemaInfo = getRequestBodySchemaForOperation( + apidef.paths[specificPath].put, + `paths.${specificPath}.put` + ); + + const putRequestSchemaPath = putRequestSchemaInfo.schemaPath; + logger.debug( + `${ruleId}: found the path to the prototype schema (for put) to be ${putRequestSchemaPath}` + ); + + if (putRequestSchemaPath) { + const putRequestSchemaName = getSchemaNameAtPath( + putRequestSchemaPath, + pathToReferencesMap + ); + logger.debug( + `${ruleId}: found the name of the prototype schema (for put) to be ${putRequestSchemaName}` + ); + + if ( + putRequestSchemaName && + putRequestSchemaName !== `${canonicalSchemaName}Prototype` && + // The API Handbook makes an exception for cases where the canonical + // schema itself should be used for creation + putRequestSchemaName !== canonicalSchemaName + ) { + logger.debug(`${ruleId}: reporting error! put prototype is wrong`); + errors.push({ + message: `Prototype schema (PUT request body) for path '${specificPath}' should use the name: ${canonicalSchemaName}Prototype`, + path: putRequestSchemaPath.split('.'), + }); + } + } + + // 4. Patch schema + const patchRequestSchemaInfo = getRequestBodySchemaForOperation( + apidef.paths[specificPath].patch, + `paths.${specificPath}.patch` + ); + + const patchRequestSchemaPath = patchRequestSchemaInfo.schemaPath; + logger.debug( + `${ruleId}: found the path to the patch schema to be ${patchRequestSchemaPath}` + ); + + if (patchRequestSchemaPath) { + const patchRequestSchemaName = getSchemaNameAtPath( + patchRequestSchemaPath, + pathToReferencesMap + ); + logger.debug( + `${ruleId}: found the name of the patch schema to be ${patchRequestSchemaName}` + ); + + if ( + patchRequestSchemaName && + patchRequestSchemaName !== `${canonicalSchemaName}Patch` + ) { + logger.debug(`${ruleId}: reporting error! patch schema is wrong`); + errors.push({ + message: `Patch schema for path '${specificPath}' should use the name: ${canonicalSchemaName}Patch`, + path: patchRequestSchemaPath.split('.'), + }); + } + } + } + + return errors; +} + +function collectResourceOrientedPaths(apidef) { + const paths = Object.keys(apidef.paths); + const pathStore = {}; + paths.forEach(p => { + // Skip paths that have already been discovered + if (pathStore[p]) { + return; + } + + // This can only receive a value for resource generic paths + const sibling = getResourceSpecificSiblingPath(p, apidef); + if (sibling) { + pathStore[p] = sibling; + } + }); + + return pathStore; +} + +/** + * Takes the graph nodes object computed by the Spectral resolver and converts + * it to a new format that is better suited for our purposes. The nodes have + * extra info we don't need and all of the paths are encoded in a unique way. + * We need to cut out the fluff, decode the paths, and convert the paths + * to use the dot-separated standard we employ for paths in our rule functions. + * + * @param {object} nodes - graph nodes object computed by the Spectral resolver + * @returns {object} - the re-formatted object + */ +function computeRefsAtPaths(nodes) { + const resultMap = {}; + Object.keys(nodes).forEach(source => { + // We need to ensure our source is a file (except when running tests) + if ( + source.toLowerCase().endsWith('yaml') || + source.toLowerCase().endsWith('yml') || + source.toLowerCase().endsWith('json') || + source === 'root' // This is necessary for running the tests + ) { + const refMap = nodes[source].refMap; + + // Each resolved path to a schema is stored with a path to its referenced + // schema in 'components'. Sub-schemas within components also have their + // paths stored with the path to the schema they reference. This gathers + // the paths, transforming them from Spectral's internal format, and maps + // them to the name of the schema they reference. + Object.keys(refMap).forEach(pathWithRef => { + const path = pathWithRef + .split('/') + .map(p => decodeURIComponent(p.replaceAll('~1', '/'))) + .join('.') + .slice(2); + resultMap[path] = refMap[pathWithRef].slice(2).replaceAll('/', '.'); + }); + } + }); + + return resultMap; +} + +/** + * Takes an unresolved path to a schema and un-resolves it to the format in which + * it will be stored in the graph nodes map by Spectral. The nodes provide a + * map from resolved path locations to the locations in 'components' they + * reference (if there is a reference at that location). Each path will be + * resolved to their nearest parent. For example if a request body schema + * references a schema in components, there will be an entry mapping the path + * to the request body schema to the schema path in components - but if that + * referenced schema has a property defined by a reference to another schema, + * there will not be an entry in the map including the path leading through + * the request body to the sub-property, there will be an entry mapping from + * the location of the schema in components. Here are example entries: + * - paths./v1/things.post.requestBody.content.application/json.schema: + * components.schemas.ThingPrototype + * - components.schemas.ThingPrototype.properties.data: + * components.schemas.DataObject + * + * The purpose of this function is to take the fully resolved path (in the + * example above, something like 'paths./v1/things.post.requestBody.content. + * application/json.schema.properties.data') and un-resolve enough that it + * will match the format Spectral uses in nodes (e.g. 'components.schemas. + * ThingPrototype.properties.data'). + * + * @param {string} path - the resolved JSON path to a schema, as a dot-separated string + * @param {object} pathToReferencesMap - the graph nodes map from Spectral, which has + * been sanitzed for our purposes. it maps referenced + * schema locations to their reference location. + * @returns {string} - the name of the referenced schema at a given path, or undefined + */ +function getSchemaNameAtPath(path, pathToReferencesMap) { + if (!path || typeof path !== 'string') { + return; + } + + // Build the path, replacing each path that resolves to a reference with the + // referenced path in order to match the expected format in the + // pathToReferencesMap (which comes from graph nodes that Spectral gives us). + // See the function documentation above for more info. + let pathBuilder = ''; + for (const pathSegment of path.split('.')) { + if (pathBuilder) { + pathBuilder += '.'; + } + pathBuilder += `${pathSegment}`; + const schemaReference = pathToReferencesMap[pathBuilder]; + + // If it is the last time through the loop, we should definitely + // find a schema reference - but we don't want to throw away the + // path. We'll find the reference again at the end of this function. + if (schemaReference && pathSegment !== path.split('.').at(-1)) { + pathBuilder = schemaReference; + } + } + + if (path !== pathBuilder) { + logger.debug(`${ruleId}: resolved path to be ${pathBuilder}`); + } + + return getSchemaNameFromReference(pathToReferencesMap[pathBuilder]); +} + +/** + * Takes a path to a referenced schema (as a string) and extracts the last element, + * which will be the name of the schema. + * + * @param {string} reference - the ref value as a dot-separated string (e.g. 'components.schemas.Thing') + * @returns {string} - the name of the schema (e.g. 'Thing') + */ +function getSchemaNameFromReference(reference) { + if (!reference || typeof reference !== 'string') { + return; + } + + logger.debug(`${ruleId}: getting name from reference ${reference}`); + return reference.split('.').at(-1); +} + +/** + * Given a property name, a path to a given schema, and that schema object, + * compute the path to the property definition while handling the potential for + * composed models (and nested composed models). Returns the first instance it + * finds. + * + * @param {string} name - the name of the property to look for + * @param {string} path - the resolved JSON path to the schema, as a dot-separated string + * @param {object} schema - the resolved schema object + * @returns {string} - the resolved JSON path to the property, as a dot-separated string, + * or `undefined` if the property is not found. + */ +function computePathToProperty(name, path, schema) { + if (schema.properties && schema.properties[name]) { + return `${path}.properties.${name}`; + } + + for (const applicator of ['allOf', 'oneOf', 'anyOf']) { + if (Array.isArray(schema[applicator])) { + for (let i = 0; i < schema[applicator].length; i++) { + const subschema = schema[applicator][i]; + const checkPath = computePathToProperty( + name, + `${path}.${applicator}.${i.toString()}`, + subschema + ); + if (checkPath && checkPath.endsWith(`properties.${name}`)) { + return checkPath; + } + } + } + } +} diff --git a/packages/ruleset/src/ibm-oas.js b/packages/ruleset/src/ibm-oas.js index be0f36def..0376781e8 100644 --- a/packages/ruleset/src/ibm-oas.js +++ b/packages/ruleset/src/ibm-oas.js @@ -168,6 +168,7 @@ module.exports = { 'ibm-response-status-codes': ibmRules.responseStatusCodes, 'ibm-schema-description': ibmRules.schemaDescriptionExists, 'ibm-schema-keywords': ibmRules.schemaKeywords, + 'ibm-schema-naming-convention': ibmRules.schemaNamingConvention, 'ibm-schema-type': ibmRules.schemaTypeExists, 'ibm-schema-type-format': ibmRules.schemaTypeFormat, 'ibm-sdk-operations': ibmRules.ibmSdkOperations, diff --git a/packages/ruleset/src/rules/index.js b/packages/ruleset/src/rules/index.js index 1686e234a..1f58fdf20 100644 --- a/packages/ruleset/src/rules/index.js +++ b/packages/ruleset/src/rules/index.js @@ -68,6 +68,7 @@ module.exports = { responseStatusCodes: require('./response-status-codes'), schemaDescriptionExists: require('./schema-description-exists'), schemaKeywords: require('./schema-keywords'), + schemaNamingConvention: require('./schema-naming-convention'), schemaTypeExists: require('./schema-type-exists'), schemaTypeFormat: require('./schema-type-format'), securitySchemes: require('./securityschemes'), diff --git a/packages/ruleset/src/rules/schema-naming-convention.js b/packages/ruleset/src/rules/schema-naming-convention.js new file mode 100644 index 000000000..42ced061b --- /dev/null +++ b/packages/ruleset/src/rules/schema-naming-convention.js @@ -0,0 +1,19 @@ +/** + * Copyright 2023 IBM Corporation. + * SPDX-License-Identifier: Apache2.0 + */ + +const { oas3 } = require('@stoplight/spectral-formats'); +const { schemaNamingConvention } = require('../functions'); + +module.exports = { + description: 'Schemas should follow naming conventions in the API Handbook', + message: '{{error}}', + given: ['$'], + severity: 'warn', + formats: [oas3], + resolved: true, + then: { + function: schemaNamingConvention, + }, +}; diff --git a/packages/ruleset/src/utils/get-resource-specific-sibling-path.js b/packages/ruleset/src/utils/get-resource-specific-sibling-path.js index 5b6927946..388d3e978 100644 --- a/packages/ruleset/src/utils/get-resource-specific-sibling-path.js +++ b/packages/ruleset/src/utils/get-resource-specific-sibling-path.js @@ -6,30 +6,27 @@ const { isObject } = require('@ibm-cloud/openapi-ruleset-utilities'); /** - * Given an operation on a generic resource path (e.g. /foo) to an operation, - * look for a specific resource path that is a sibling (e.g. /foo/{id}); if - * found, return the sibling path as a string. + * Given a "generic" resource path string (e.g. '/foo'), look for a + * "specific" resource path that is a sibling (e.g. /foo/{id}); if + * found, return the sibling path string. * - * Note: the given path MUST be to an operation object. - * - * @param {*} path the array of path segments indicating the "location" of an operation within the API definition + * @param {*} path the path string (for a generic path e.g. '/foo') * @param {*} apidef the resolved API spec, as an object * @returns the specific resource path, as a string */ function getResourceSpecificSiblingPath(path, apidef) { // Paths are expected to be arrays, API def is expected to be an object - if (!Array.isArray(path) || !isObject(apidef)) { + if (typeof path !== 'string' || !isObject(apidef)) { return; } // If this path already ends with a path parameter, return 'undefined'. // This function should only find a path if it is a sibling of the current path. - if (path[path.length - 2].toString().trim().endsWith('}')) { + if (path.trim().endsWith('}')) { return; } - const thisPath = path[path.length - 2].toString().trim(); - const siblingPathRE = new RegExp(`^${thisPath}/{[^{}/]+}$`); + const siblingPathRE = new RegExp(`^${path}/{[^{}/]+}$`); return Object.keys(apidef.paths).find(p => siblingPathRE.test(p)); } diff --git a/packages/ruleset/src/utils/index.js b/packages/ruleset/src/utils/index.js index aea6de9e4..3e4d01275 100644 --- a/packages/ruleset/src/utils/index.js +++ b/packages/ruleset/src/utils/index.js @@ -19,4 +19,5 @@ module.exports = { pathMatchesRegexp: require('./path-matches-regexp'), ...require('./mimetype-utils'), ...require('./pagination-utils'), + ...require('./schema-finding-utils'), }; diff --git a/packages/ruleset/src/utils/is-create-operation.js b/packages/ruleset/src/utils/is-create-operation.js index a3b90c0c7..84134d770 100644 --- a/packages/ruleset/src/utils/is-create-operation.js +++ b/packages/ruleset/src/utils/is-create-operation.js @@ -35,7 +35,7 @@ function isCreateOperation(operation, path, apidef) { } // 3. Does this operation's path have a sibling path with a trailing path param reference? - const siblingPath = getResourceSpecificSiblingPath(path, apidef); + const siblingPath = getResourceSpecificSiblingPath(path.at(-2), apidef); return !!siblingPath; } diff --git a/packages/ruleset/src/utils/schema-finding-utils.js b/packages/ruleset/src/utils/schema-finding-utils.js new file mode 100644 index 000000000..21f39ea8a --- /dev/null +++ b/packages/ruleset/src/utils/schema-finding-utils.js @@ -0,0 +1,153 @@ +/** + * Copyright 2023 IBM Corporation. + * SPDX-License-Identifier: Apache2.0 + */ + +const { isObject } = require('@ibm-cloud/openapi-ruleset-utilities'); +const getResponseCodes = require('./get-response-codes'); + +const { + isJsonMimeType, + isJsonPatchMimeType, + isMergePatchMimeType, +} = require('./mimetype-utils'); + +/** + * Takes an operation object/path combo and finds the JSON success + * response (200) schema, if it exists. We're only interested in + * JSON content and this is to be used for exclusively GET operations + * (individual "gets" or "lists") so we only look at 200 status codes. + * + * @param {object} operation - an operation object + * @param {string} pathToOp - the json path leading to the operation + * (as a dot-separated string) + * @returns {object} a map containing the schema object (with key + * 'schemaObject') and path (as a dot-separated string) (with key + * 'schemaPath'). + */ +function getSuccessResponseSchemaForOperation(operation, pathToOp) { + const info = {}; + + if (!operation) { + return info; + } + + let pathToSchema = pathToOp; + + if (!operation.responses) { + return info; + } + + pathToSchema += '.responses'; + + const [, successCodes] = getResponseCodes(operation.responses); + + if (!successCodes || !successCodes.length) { + return info; + } + + // Prefer 200 responses if present. If not, find the first response with a JSON schema. + const code = successCodes.includes('200') + ? '200' + : successCodes.find(c => responseHasJsonSchema(operation.responses[c])); + if (!code) { + return info; + } + + const successResponse = operation.responses[code]; + if (!successResponse.content || !isObject(successResponse.content)) { + return info; + } + + pathToSchema += `.${code}.content`; + + // Find the first content object determined to be JSON - + // we are only interested in JSON content. + const jsonMimeType = Object.keys(successResponse.content).find(mimeType => + isJsonMimeType(mimeType) + ); + + if (!jsonMimeType) { + return info; + } + + const jsonContent = successResponse.content[jsonMimeType]; + if (!isObject(jsonContent) || !jsonContent.schema) { + return info; + } + + info.schemaObject = jsonContent.schema; + info.schemaPath = pathToSchema + `.${jsonMimeType}.schema`; + + return info; +} + +function responseHasJsonSchema(response) { + return ( + response.content && + Object.keys(response.content).some( + c => isJsonMimeType(c) && response.content[c].schema + ) + ); +} + +/** + * Takes an operation object/path combo and finds the JSON request body + * schema, if it exists. We're only interested in JSON content. + * + * @param {object} operation - an operation object + * @param {string} pathToOp - the json path leading to the operation + * (as a dot-separated string) + * @returns {object} a map containing the schema object (with key + * 'schemaObject') and path (as a dot-separated string) (with key + * 'schemaPath'). + */ +function getRequestBodySchemaForOperation(operation, pathToOp) { + const info = {}; + + if (!operation) { + return info; + } + + let pathToSchema = pathToOp; + + if (!operation.requestBody) { + return info; + } + + const body = operation.requestBody; + pathToSchema += '.requestBody'; + + if (!body.content || !isObject(body.content)) { + return info; + } + + pathToSchema += '.content'; + + // Find the first content object determined to be JSON - + // we are only interested in JSON content. + const isPatch = pathToOp.split('.').at(-1).toLowerCase() === 'patch'; + const mimeType = Object.keys(body.content).find(mimeType => + isPatch + ? isJsonPatchMimeType(mimeType) || isMergePatchMimeType(mimeType) + : isJsonMimeType(mimeType) + ); + if (!mimeType) { + return info; + } + + const jsonContent = body.content[mimeType]; + if (!isObject(jsonContent) || !jsonContent.schema) { + return info; + } + + info.schemaObject = jsonContent.schema; + info.schemaPath = pathToSchema + `.${mimeType}.schema`; + + return info; +} + +module.exports = { + getSuccessResponseSchemaForOperation, + getRequestBodySchemaForOperation, +}; diff --git a/packages/ruleset/test/array-attributes.test.js b/packages/ruleset/test/array-attributes.test.js index 391162d83..75310d7df 100644 --- a/packages/ruleset/test/array-attributes.test.js +++ b/packages/ruleset/test/array-attributes.test.js @@ -500,13 +500,11 @@ describe(`Spectral rule: ${ruleId}`, () => { }; const results = await testRule(ruleId, rule, testDocument); - expect(results).toHaveLength(6); + expect(results).toHaveLength(4); const expectedPaths = [ - 'paths./v1/movies.post.requestBody.content.application/json.schema.additionalProperties', 'paths./v1/movies.post.responses.201.content.application/json.schema.additionalProperties', 'paths./v1/movies.get.responses.200.content.application/json.schema.allOf.1.properties.movies.items.additionalProperties', 'paths./v1/movies/{movie_id}.get.responses.200.content.application/json.schema.additionalProperties', - 'paths./v1/movies/{movie_id}.put.requestBody.content.application/json.schema.additionalProperties', 'paths./v1/movies/{movie_id}.put.responses.200.content.application/json.schema.additionalProperties', ]; for (let i = 0; i < results.length; i++) { @@ -530,9 +528,8 @@ describe(`Spectral rule: ${ruleId}`, () => { }; const results = await testRule(ruleId, rule, testDocument); - expect(results).toHaveLength(4); + expect(results).toHaveLength(3); const expectedPaths = [ - 'paths./v1/cars.post.requestBody.content.application/json.schema.properties.wheel_count', 'paths./v1/cars.post.responses.201.content.application/json.schema.properties.wheel_count', 'paths./v1/cars/{car_id}.get.responses.200.content.application/json.schema.properties.wheel_count', 'paths./v1/cars/{car_id}.patch.responses.200.content.application/json.schema.properties.wheel_count', @@ -555,9 +552,8 @@ describe(`Spectral rule: ${ruleId}`, () => { }; const results = await testRule(ruleId, rule, testDocument); - expect(results).toHaveLength(4); + expect(results).toHaveLength(3); const expectedPaths = [ - 'paths./v1/cars.post.requestBody.content.application/json.schema.properties.wheel_count.minItems', 'paths./v1/cars.post.responses.201.content.application/json.schema.properties.wheel_count.minItems', 'paths./v1/cars/{car_id}.get.responses.200.content.application/json.schema.properties.wheel_count.minItems', 'paths./v1/cars/{car_id}.patch.responses.200.content.application/json.schema.properties.wheel_count.minItems', @@ -580,9 +576,8 @@ describe(`Spectral rule: ${ruleId}`, () => { }; const results = await testRule(ruleId, rule, testDocument); - expect(results).toHaveLength(4); + expect(results).toHaveLength(3); const expectedPaths = [ - 'paths./v1/cars.post.requestBody.content.application/json.schema.properties.wheel_count.maxItems', 'paths./v1/cars.post.responses.201.content.application/json.schema.properties.wheel_count.maxItems', 'paths./v1/cars/{car_id}.get.responses.200.content.application/json.schema.properties.wheel_count.maxItems', 'paths./v1/cars/{car_id}.patch.responses.200.content.application/json.schema.properties.wheel_count.maxItems', diff --git a/packages/ruleset/test/array-of-arrays.test.js b/packages/ruleset/test/array-of-arrays.test.js index 68a8f19b6..bc7df596c 100644 --- a/packages/ruleset/test/array-of-arrays.test.js +++ b/packages/ruleset/test/array-of-arrays.test.js @@ -143,7 +143,7 @@ describe(`Spectral rule: ${ruleId}`, () => { }; const results = await testRule(ruleId, rule, testDocument); - expect(results).toHaveLength(6); + expect(results).toHaveLength(4); for (const result of results) { expect(result.code).toBe(ruleId); expect(result.message).toBe(expectedMessage); @@ -151,18 +151,6 @@ describe(`Spectral rule: ${ruleId}`, () => { } expect(results[0].path).toStrictEqual([ - 'paths', - '/v1/movies', - 'post', - 'requestBody', - 'content', - 'application/json', - 'schema', - 'properties', - 'array_prop', - ]); - - expect(results[1].path).toStrictEqual([ 'paths', '/v1/movies', 'post', @@ -175,7 +163,7 @@ describe(`Spectral rule: ${ruleId}`, () => { 'array_prop', ]); - expect(results[2].path).toStrictEqual([ + expect(results[1].path).toStrictEqual([ 'paths', '/v1/movies', 'get', @@ -193,7 +181,7 @@ describe(`Spectral rule: ${ruleId}`, () => { 'array_prop', ]); - expect(results[3].path).toStrictEqual([ + expect(results[2].path).toStrictEqual([ 'paths', '/v1/movies/{movie_id}', 'get', @@ -206,11 +194,12 @@ describe(`Spectral rule: ${ruleId}`, () => { 'array_prop', ]); - expect(results[4].path).toStrictEqual([ + expect(results[3].path).toStrictEqual([ 'paths', '/v1/movies/{movie_id}', 'put', - 'requestBody', + 'responses', + '200', 'content', 'application/json', 'schema', diff --git a/packages/ruleset/test/circular-refs.test.js b/packages/ruleset/test/circular-refs.test.js index 2d2c0f48a..270495104 100644 --- a/packages/ruleset/test/circular-refs.test.js +++ b/packages/ruleset/test/circular-refs.test.js @@ -43,7 +43,7 @@ describe(`Spectral rule: ${ruleId}`, () => { expect(result.severity).toBe(expectedSeverity); } expect(results[0].path.join('.')).toBe( - 'paths./v1/movies.post.requestBody.content.application/json.schema.$ref' + 'paths./v1/movies.post.responses.201.content.application/json.schema.$ref' ); }); it('Circular ref in schema property', async () => { @@ -61,7 +61,7 @@ describe(`Spectral rule: ${ruleId}`, () => { expect(result.severity).toBe(expectedSeverity); } expect(results[0].path.join('.')).toBe( - 'paths./v1/movies.post.requestBody.content.application/json.schema.properties.related_movie.$ref' + 'paths./v1/movies.post.responses.201.content.application/json.schema.properties.related_movie.$ref' ); }); @@ -80,7 +80,7 @@ describe(`Spectral rule: ${ruleId}`, () => { expect(result.severity).toBe(expectedSeverity); } expect(results[0].path.join('.')).toBe( - 'paths./v1/movies.post.requestBody.content.application/json.schema.properties.related_movies.$ref' + 'paths./v1/movies.post.responses.201.content.application/json.schema.properties.related_movies.$ref' ); expect(results[1].path.join('.')).toBe( 'paths./v1/movies.get.responses.200.content.application/json.schema.allOf.1.properties.movies.items.$ref' @@ -105,7 +105,7 @@ describe(`Spectral rule: ${ruleId}`, () => { expect(result.severity).toBe(expectedSeverity); } expect(results[0].path.join('.')).toBe( - 'paths./v1/movies.post.requestBody.content.application/json.schema.properties.related_movies.items.$ref' + 'paths./v1/movies.post.responses.201.content.application/json.schema.properties.related_movies.items.$ref' ); }); @@ -124,7 +124,7 @@ describe(`Spectral rule: ${ruleId}`, () => { expect(result.severity).toBe(expectedSeverity); } expect(results[0].path.join('.')).toBe( - 'paths./v1/movies.post.requestBody.content.application/json.schema.additionalProperties.$ref' + 'paths./v1/movies.post.responses.201.content.application/json.schema.additionalProperties.$ref' ); }); @@ -166,7 +166,7 @@ describe(`Spectral rule: ${ruleId}`, () => { expect(result.severity).toBe(expectedSeverity); } expect(results[0].path.join('.')).toBe( - 'paths./v1/drinks.post.requestBody.content.application/json.schema.anyOf.0.$ref' + 'paths./v1/drinks.post.responses.201.content.application/json.schema.anyOf.0.$ref' ); expect(results[1].path.join('.')).toBe( 'paths./v1/drinks.get.responses.200.content.application/json.schema.allOf.1.properties.drinks.items.$ref' @@ -188,10 +188,10 @@ describe(`Spectral rule: ${ruleId}`, () => { expect(result.severity).toBe(expectedSeverity); } expect(results[0].path.join('.')).toBe( - 'paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.1.$ref' + 'paths./v1/drinks.post.requestBody.content.application/json.schema.oneOf.1.properties.next_drink.$ref' ); expect(results[1].path.join('.')).toBe( - 'components.schemas.Soda.properties.next_drink.$ref' + 'paths./v1/drinks.post.responses.201.content.application/json.schema.oneOf.1.$ref' ); }); diff --git a/packages/ruleset/test/get-resource-specific-sibling-path.test.js b/packages/ruleset/test/get-resource-specific-sibling-path.test.js index f70ecf684..af0c22e1e 100644 --- a/packages/ruleset/test/get-resource-specific-sibling-path.test.js +++ b/packages/ruleset/test/get-resource-specific-sibling-path.test.js @@ -7,7 +7,7 @@ const { getResourceSpecificSiblingPath } = require('../src/utils'); describe('Utility function: getResourceSpecificSiblingPath', () => { it('should find sibling path when present', () => { - const path = ['paths', '/v1/things', 'post']; + const path = '/v1/things'; const apidef = { paths: { '/v1/things': { @@ -28,7 +28,7 @@ describe('Utility function: getResourceSpecificSiblingPath', () => { }); it('should return undefined when sibling path is not present', () => { - const path = ['paths', '/v1/things', 'post']; + const path = '/v1/things'; const apidef = { paths: { '/v1/things': { @@ -44,7 +44,7 @@ describe('Utility function: getResourceSpecificSiblingPath', () => { }); it('should return undefined when there are no other paths', () => { - const path = ['paths', '/v1/things', 'post']; + const path = '/v1/things'; const apidef = { paths: { '/v1/things': { @@ -57,7 +57,7 @@ describe('Utility function: getResourceSpecificSiblingPath', () => { }); it('should return undefined when given path already ends in a path parameter', () => { - const path = ['paths', '/v1/things/{id}', 'get']; + const path = '/v1/things/{id}'; const apidef = { paths: { '/v1/things/{id}': { @@ -69,8 +69,8 @@ describe('Utility function: getResourceSpecificSiblingPath', () => { expect(getResourceSpecificSiblingPath(path, apidef)).toBeUndefined(); }); - it('should return undefined when path is not an array', () => { - const path = 'wrong'; + it('should return undefined when path is not a string', () => { + const path = ['paths', '/v1/things', 'post']; const apidef = { paths: { '/v1/things': { @@ -83,7 +83,7 @@ describe('Utility function: getResourceSpecificSiblingPath', () => { }); it('should return undefined when apidef is not an object', () => { - const path = ['paths', '/v1/things', 'post']; + const path = '/v1/things'; const apidef = 'wrong'; expect(getResourceSpecificSiblingPath(path, apidef)).toBeUndefined(); }); diff --git a/packages/ruleset/test/pattern-properties.test.js b/packages/ruleset/test/pattern-properties.test.js index 0bf7ec935..d35f401c6 100644 --- a/packages/ruleset/test/pattern-properties.test.js +++ b/packages/ruleset/test/pattern-properties.test.js @@ -59,14 +59,12 @@ describe(`Spectral rule: ${ruleId}`, () => { testDocument.components.schemas['Movie'].additionalProperties = true; const results = await testRule(ruleId, rule, testDocument); - expect(results).toHaveLength(6); + expect(results).toHaveLength(4); const expectedPaths = [ - 'paths./v1/movies.post.requestBody.content.application/json.schema', 'paths./v1/movies.post.responses.201.content.application/json.schema', 'paths./v1/movies.get.responses.200.content.application/json.schema.allOf.1.properties.movies.items', 'paths./v1/movies/{movie_id}.get.responses.200.content.application/json.schema', - 'paths./v1/movies/{movie_id}.put.requestBody.content.application/json.schema', 'paths./v1/movies/{movie_id}.put.responses.200.content.application/json.schema', ]; for (let i = 0; i < results.length; i++) { @@ -82,14 +80,12 @@ describe(`Spectral rule: ${ruleId}`, () => { testDocument.components.schemas['Movie'].patternProperties = 'foo'; const results = await testRule(ruleId, rule, testDocument); - expect(results).toHaveLength(6); + expect(results).toHaveLength(4); const expectedPaths = [ - 'paths./v1/movies.post.requestBody.content.application/json.schema.patternProperties', 'paths./v1/movies.post.responses.201.content.application/json.schema.patternProperties', 'paths./v1/movies.get.responses.200.content.application/json.schema.allOf.1.properties.movies.items.patternProperties', 'paths./v1/movies/{movie_id}.get.responses.200.content.application/json.schema.patternProperties', - 'paths./v1/movies/{movie_id}.put.requestBody.content.application/json.schema.patternProperties', 'paths./v1/movies/{movie_id}.put.responses.200.content.application/json.schema.patternProperties', ]; for (let i = 0; i < results.length; i++) { @@ -105,14 +101,12 @@ describe(`Spectral rule: ${ruleId}`, () => { testDocument.components.schemas['Movie'].patternProperties = {}; const results = await testRule(ruleId, rule, testDocument); - expect(results).toHaveLength(6); + expect(results).toHaveLength(4); const expectedPaths = [ - 'paths./v1/movies.post.requestBody.content.application/json.schema.patternProperties', 'paths./v1/movies.post.responses.201.content.application/json.schema.patternProperties', 'paths./v1/movies.get.responses.200.content.application/json.schema.allOf.1.properties.movies.items.patternProperties', 'paths./v1/movies/{movie_id}.get.responses.200.content.application/json.schema.patternProperties', - 'paths./v1/movies/{movie_id}.put.requestBody.content.application/json.schema.patternProperties', 'paths./v1/movies/{movie_id}.put.responses.200.content.application/json.schema.patternProperties', ]; for (let i = 0; i < results.length; i++) { @@ -135,14 +129,12 @@ describe(`Spectral rule: ${ruleId}`, () => { }; const results = await testRule(ruleId, rule, testDocument); - expect(results).toHaveLength(6); + expect(results).toHaveLength(4); const expectedPaths = [ - 'paths./v1/movies.post.requestBody.content.application/json.schema.patternProperties', 'paths./v1/movies.post.responses.201.content.application/json.schema.patternProperties', 'paths./v1/movies.get.responses.200.content.application/json.schema.allOf.1.properties.movies.items.patternProperties', 'paths./v1/movies/{movie_id}.get.responses.200.content.application/json.schema.patternProperties', - 'paths./v1/movies/{movie_id}.put.requestBody.content.application/json.schema.patternProperties', 'paths./v1/movies/{movie_id}.put.responses.200.content.application/json.schema.patternProperties', ]; for (let i = 0; i < results.length; i++) { diff --git a/packages/ruleset/test/property-attributes.test.js b/packages/ruleset/test/property-attributes.test.js index 6b5b58d34..f54ccf509 100644 --- a/packages/ruleset/test/property-attributes.test.js +++ b/packages/ruleset/test/property-attributes.test.js @@ -104,9 +104,8 @@ describe(`Spectral rule: ${ruleId}`, () => { }; const results = await testRule(ruleId, rule, testDocument); - expect(results).toHaveLength(4); + expect(results).toHaveLength(3); const expectedPaths = [ - 'paths./v1/cars.post.requestBody.content.application/json.schema.properties.wheel_count.minimum', 'paths./v1/cars.post.responses.201.content.application/json.schema.properties.wheel_count.minimum', 'paths./v1/cars/{car_id}.get.responses.200.content.application/json.schema.properties.wheel_count.minimum', 'paths./v1/cars/{car_id}.patch.responses.200.content.application/json.schema.properties.wheel_count.minimum', @@ -129,9 +128,8 @@ describe(`Spectral rule: ${ruleId}`, () => { }; const results = await testRule(ruleId, rule, testDocument); - expect(results).toHaveLength(4); + expect(results).toHaveLength(3); const expectedPaths = [ - 'paths./v1/cars.post.requestBody.content.application/json.schema.properties.wheel_count.minimum', 'paths./v1/cars.post.responses.201.content.application/json.schema.properties.wheel_count.minimum', 'paths./v1/cars/{car_id}.get.responses.200.content.application/json.schema.properties.wheel_count.minimum', 'paths./v1/cars/{car_id}.patch.responses.200.content.application/json.schema.properties.wheel_count.minimum', @@ -154,9 +152,8 @@ describe(`Spectral rule: ${ruleId}`, () => { }; const results = await testRule(ruleId, rule, testDocument); - expect(results).toHaveLength(4); + expect(results).toHaveLength(3); const expectedPaths = [ - 'paths./v1/cars.post.requestBody.content.application/json.schema.properties.wheel_count.maximum', 'paths./v1/cars.post.responses.201.content.application/json.schema.properties.wheel_count.maximum', 'paths./v1/cars/{car_id}.get.responses.200.content.application/json.schema.properties.wheel_count.maximum', 'paths./v1/cars/{car_id}.patch.responses.200.content.application/json.schema.properties.wheel_count.maximum', @@ -183,9 +180,8 @@ describe(`Spectral rule: ${ruleId}`, () => { }; const results = await testRule(ruleId, rule, testDocument); - expect(results).toHaveLength(4); + expect(results).toHaveLength(3); const expectedPaths = [ - 'paths./v1/cars.post.requestBody.content.application/json.schema.properties.wheel_count.minProperties', 'paths./v1/cars.post.responses.201.content.application/json.schema.properties.wheel_count.minProperties', 'paths./v1/cars/{car_id}.get.responses.200.content.application/json.schema.properties.wheel_count.minProperties', 'paths./v1/cars/{car_id}.patch.responses.200.content.application/json.schema.properties.wheel_count.minProperties', @@ -208,9 +204,8 @@ describe(`Spectral rule: ${ruleId}`, () => { }; const results = await testRule(ruleId, rule, testDocument); - expect(results).toHaveLength(4); + expect(results).toHaveLength(3); const expectedPaths = [ - 'paths./v1/cars.post.requestBody.content.application/json.schema.properties.wheel_count.minProperties', 'paths./v1/cars.post.responses.201.content.application/json.schema.properties.wheel_count.minProperties', 'paths./v1/cars/{car_id}.get.responses.200.content.application/json.schema.properties.wheel_count.minProperties', 'paths./v1/cars/{car_id}.patch.responses.200.content.application/json.schema.properties.wheel_count.minProperties', @@ -234,9 +229,8 @@ describe(`Spectral rule: ${ruleId}`, () => { }; const results = await testRule(ruleId, rule, testDocument); - expect(results).toHaveLength(4); + expect(results).toHaveLength(3); const expectedPaths = [ - 'paths./v1/cars.post.requestBody.content.application/json.schema.properties.wheel_count.maxProperties', 'paths./v1/cars.post.responses.201.content.application/json.schema.properties.wheel_count.maxProperties', 'paths./v1/cars/{car_id}.get.responses.200.content.application/json.schema.properties.wheel_count.maxProperties', 'paths./v1/cars/{car_id}.patch.responses.200.content.application/json.schema.properties.wheel_count.maxProperties', diff --git a/packages/ruleset/test/property-description.test.js b/packages/ruleset/test/property-description.test.js index 244601a7d..d42fd5a06 100644 --- a/packages/ruleset/test/property-description.test.js +++ b/packages/ruleset/test/property-description.test.js @@ -176,26 +176,32 @@ describe(`Spectral rule: ${ruleId}`, () => { testDocument.components.schemas['IdString'].description = ''; const results = await testRule(ruleId, rule, testDocument); - expect(results).toHaveLength(6); + expect(results).toHaveLength(7); for (const result of results) { expect(result.code).toBe(ruleId); expect(result.message).toBe(expectedMsg); expect(result.severity).toBe(expectedSeverity); } expect(results[0].path.join('.')).toBe( - 'paths./v1/movies.post.requestBody.content.application/json.schema.properties.id' + 'paths./v1/drinks.post.responses.201.content.application/json.schema.properties.id' ); expect(results[1].path.join('.')).toBe( - 'paths./v1/movies.post.responses.201.content.application/json.schema.properties.id' + 'paths./v1/drinks.get.responses.200.content.application/json.schema.allOf.1.properties.drinks.items.properties.id' ); expect(results[2].path.join('.')).toBe( - 'paths./v1/movies.get.responses.200.content.application/json.schema.allOf.1.properties.movies.items.properties.id' + 'paths./v1/drinks/{drink_id}.get.responses.200.content.application/json.schema.properties.id' ); expect(results[3].path.join('.')).toBe( - 'paths./v1/movies/{movie_id}.get.responses.200.content.application/json.schema.properties.id' + 'paths./v1/movies.post.responses.201.content.application/json.schema.properties.id' ); expect(results[4].path.join('.')).toBe( - 'paths./v1/movies/{movie_id}.put.requestBody.content.application/json.schema.properties.id' + 'paths./v1/movies.get.responses.200.content.application/json.schema.allOf.1.properties.movies.items.properties.id' + ); + expect(results[5].path.join('.')).toBe( + 'paths./v1/movies/{movie_id}.get.responses.200.content.application/json.schema.properties.id' + ); + expect(results[6].path.join('.')).toBe( + 'paths./v1/movies/{movie_id}.put.responses.200.content.application/json.schema.properties.id' ); }); diff --git a/packages/ruleset/test/property-name-collision.test.js b/packages/ruleset/test/property-name-collision.test.js index 787f6ab33..2e15e20f1 100644 --- a/packages/ruleset/test/property-name-collision.test.js +++ b/packages/ruleset/test/property-name-collision.test.js @@ -44,7 +44,7 @@ describe(`Spectral rule: ${ruleId}`, () => { const results = await testRule(ruleId, rule, testDocument); - expect(results).toHaveLength(6); + expect(results).toHaveLength(4); const validation = results[0]; expect(validation.code).toBe(ruleId); @@ -55,7 +55,8 @@ describe(`Spectral rule: ${ruleId}`, () => { 'paths', '/v1/movies', 'post', - 'requestBody', + 'responses', + '201', 'content', 'application/json', 'schema', diff --git a/packages/ruleset/test/requestbody-name.test.js b/packages/ruleset/test/requestbody-name.test.js index ddbf3ceb9..2c0d95628 100644 --- a/packages/ruleset/test/requestbody-name.test.js +++ b/packages/ruleset/test/requestbody-name.test.js @@ -168,7 +168,7 @@ describe(`Spectral rule: ${ruleId}`, () => { it('Request body schema is dynamic, extension missing', async () => { const testDocument = makeCopy(rootDocument); - testDocument.components.schemas.Movie.additionalProperties = true; + testDocument.components.schemas.MoviePrototype.additionalProperties = true; const results = await testRule(ruleId, rule, testDocument); expect(results).toHaveLength(1); @@ -181,7 +181,7 @@ describe(`Spectral rule: ${ruleId}`, () => { it('Request body schema has discriminator, extension missing', async () => { const testDocument = makeCopy(rootDocument); - testDocument.components.schemas.Movie.discriminator = { + testDocument.components.schemas.MoviePrototype.discriminator = { propertyName: 'type', }; diff --git a/packages/ruleset/test/schema-naming-convention.test.js b/packages/ruleset/test/schema-naming-convention.test.js new file mode 100644 index 000000000..65394cfbd --- /dev/null +++ b/packages/ruleset/test/schema-naming-convention.test.js @@ -0,0 +1,935 @@ +/** + * Copyright 2023 IBM Corporation. + * SPDX-License-Identifier: Apache2.0 + */ + +const { schemaNamingConvention } = require('../src/rules'); +const { makeCopy, rootDocument, testRule, severityCodes } = require('./utils'); + +const rule = schemaNamingConvention; +const ruleId = 'ibm-schema-naming-convention'; +const expectedSeverity = severityCodes.warning; + +function collectionSchemaMessage(genericPath, canonicalSchemaName) { + return `Collection schema for path '${genericPath}' should use the name: ${canonicalSchemaName}Collection`; +} + +function resourceListItemsSchemaMessage( + resourceListPropName, + canonicalSchemaName +) { + return `Items schema for collection resource list property '${resourceListPropName}' should use the name: ${canonicalSchemaName} or ${canonicalSchemaName}Summary`; +} + +function postPrototypeSchemaMessage(genericPath, canonicalSchemaName) { + return `Prototype schema (POST request body) for path '${genericPath}' should use the name: ${canonicalSchemaName}Prototype`; +} + +function putPrototypeSchemaMessage(specificPath, canonicalSchemaName) { + return `Prototype schema (PUT request body) for path '${specificPath}' should use the name: ${canonicalSchemaName}Prototype`; +} + +function patchSchemaMessage(specificPath, canonicalSchemaName) { + return `Patch schema for path '${specificPath}' should use the name: ${canonicalSchemaName}Patch`; +} + +describe(`Spectral rule: ${ruleId}`, () => { + describe('Should not yield errors', () => { + it('Clean spec', async () => { + const results = await testRule(ruleId, rule, rootDocument); + expect(results).toHaveLength(0); + }); + + it('Collection schema and list property items schema are correctly named', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.Coin = {}; + testDocument.components.schemas.CoinCollection = { + properties: { + coins: { + type: 'array', + items: { + $ref: '#/components/schemas/Coin', + }, + }, + }, + }; + testDocument.paths['/v1/coins'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/CoinCollection', + }, + }, + }, + }, + }, + }, + }; + testDocument.paths['/v1/coins/{id}'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Coin', + }, + }, + }, + }, + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(0); + }); + + it('Collection schema is correctly named and resource list prop items use summary schema', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.Coin = {}; + testDocument.components.schemas.CoinSummary = {}; + testDocument.components.schemas.CoinCollection = { + properties: { + coins: { + type: 'array', + items: { + $ref: '#/components/schemas/CoinSummary', + }, + }, + }, + }; + testDocument.paths['/v1/coins'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/CoinCollection', + }, + }, + }, + }, + }, + }, + }; + testDocument.paths['/v1/coins/{id}'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Coin', + }, + }, + }, + }, + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(0); + }); + + it('Unidentifiable resource list property items schema is incorrectly named', async () => { + // Note that the list property having the correct name is validated + // for in a separate rule (ibm-collection-array-property) + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.Coin = {}; + testDocument.components.schemas.CoinLike = {}; + testDocument.components.schemas.CoinCollection = { + properties: { + list_o_coins: { + // would be detected if this was named 'coins' + type: 'array', + items: { + $ref: '#/components/schemas/CoinLike', + }, + }, + }, + }; + testDocument.paths['/v1/coins'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/CoinCollection', + }, + }, + }, + }, + }, + }, + }; + testDocument.paths['/v1/coins/{id}'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Coin', + }, + }, + }, + }, + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(0); + }); + + it('Collection schema would be incorrectly named but there is no canonical schema for reference', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.CoinBunch = {}; + testDocument.paths['/v1/coins'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/CoinBunch', + }, + }, + }, + }, + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(0); + }); + + it('Prototype schema in POST request body is correctly named', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.Mess = {}; + testDocument.components.schemas.MessPrototype = {}; + testDocument.paths['/v1/messes'] = { + post: { + requestBody: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/MessPrototype', + }, + }, + }, + }, + }, + }; + testDocument.paths['/v1/messes/{id}'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Mess', + }, + }, + }, + }, + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(0); + }); + + it('Prototype schema in POST request body uses canonical schema', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.Mess = {}; + testDocument.components.schemas.MessPrototype = {}; + testDocument.paths['/v1/messes'] = { + post: { + requestBody: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Mess', + }, + }, + }, + }, + }, + }; + testDocument.paths['/v1/messes/{id}'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Mess', + }, + }, + }, + }, + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(0); + }); + + it('Prototype schema in PUT request body is correctly named', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.Mess = {}; + testDocument.components.schemas.MessPrototype = {}; + testDocument.paths['/v1/messes'] = {}; + testDocument.paths['/v1/messes/{id}'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Mess', + }, + }, + }, + }, + }, + }, + put: { + requestBody: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/MessPrototype', + }, + }, + }, + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(0); + }); + + it('Prototype schema in PUT request body uses canonical schema', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.Mess = {}; + testDocument.components.schemas.MessPrototype = {}; + testDocument.paths['/v1/messes'] = {}; + testDocument.paths['/v1/messes/{id}'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Mess', + }, + }, + }, + }, + }, + }, + put: { + requestBody: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Mess', + }, + }, + }, + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(0); + }); + + it('Prototype schema in PUT request body is incorrectly named but the path is not resource-oriented', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.Mess = {}; + testDocument.components.schemas.MessMaker = {}; + testDocument.paths['/v1/messes/{id}'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Mess', + }, + }, + }, + }, + }, + }, + put: { + requestBody: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/MessMaker', + }, + }, + }, + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(0); + }); + + it('Patch schema is correctly named', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.Jeans = {}; + testDocument.components.schemas.JeansPatch = {}; + testDocument.paths['/v1/jeans'] = {}; + testDocument.paths['/v1/jeans/{id}'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Jeans', + }, + }, + }, + }, + }, + }, + patch: { + requestBody: { + content: { + 'application/merge-patch+json; charset=utf-8': { + schema: { + $ref: '#/components/schemas/JeansPatch', + }, + }, + }, + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(0); + }); + }); + + describe('Should yield errors', () => { + it('Collection schema is incorrectly named', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.Coin = {}; + testDocument.components.schemas.Coins = { + properties: { + coins: { + type: 'array', + items: { + $ref: '#/components/schemas/Coin', + }, + }, + }, + }; + testDocument.paths['/v1/coins'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Coins', + }, + }, + }, + }, + }, + }, + }; + testDocument.paths['/v1/coins/{id}'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Coin', + }, + }, + }, + }, + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(1); + const r = results[0]; + expect(r.code).toBe(ruleId); + expect(r.severity).toBe(expectedSeverity); + expect(r.message).toBe(collectionSchemaMessage('/v1/coins', 'Coin')); + expect(r.path.join('.')).toBe( + 'paths./v1/coins.get.responses.200.content.application/json.schema' + ); + }); + + it('Collection resource list property items schema is incorrectly named', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.Coin = {}; + testDocument.components.schemas.SingleCoin = {}; + testDocument.components.schemas.CoinCollection = { + properties: { + coins: { + type: 'array', + items: { + $ref: '#/components/schemas/SingleCoin', + }, + }, + }, + }; + testDocument.paths['/v1/coins'] = { + get: { + responses: { + 200: { + content: { + 'application/json; charset=utf-8': { + schema: { + $ref: '#/components/schemas/CoinCollection', + }, + }, + }, + }, + }, + }, + }; + testDocument.paths['/v1/coins/{id}'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Coin', + }, + }, + }, + }, + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(1); + const r = results[0]; + expect(r.code).toBe(ruleId); + expect(r.severity).toBe(expectedSeverity); + expect(r.message).toBe(resourceListItemsSchemaMessage('coins', 'Coin')); + expect(r.path.join('.')).toBe( + 'paths./v1/coins.get.responses.200.content.application/json; charset=utf-8.schema.properties.coins.items' + ); + }); + + it('Collection resource list property items schema (referenced) is incorrectly named', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.Coin = {}; + testDocument.components.schemas.SingleCoin = {}; + testDocument.components.schemas.CoinList = { + type: 'array', + items: { + $ref: '#/components/schemas/SingleCoin', + }, + }; + testDocument.components.schemas.CoinCollection = { + properties: { + coins: { + $ref: '#/components/schemas/CoinList', + }, + }, + }; + testDocument.paths['/v1/coins'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/CoinCollection', + }, + }, + }, + }, + }, + }, + }; + testDocument.paths['/v1/coins/{id}'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Coin', + }, + }, + }, + }, + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(1); + const r = results[0]; + expect(r.code).toBe(ruleId); + expect(r.severity).toBe(expectedSeverity); + expect(r.message).toBe(resourceListItemsSchemaMessage('coins', 'Coin')); + expect(r.path.join('.')).toBe( + 'paths./v1/coins.get.responses.200.content.application/json.schema.properties.coins.items' + ); + }); + + it('Collection resource list property items schema (in composed model) is incorrectly named', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.Coin = {}; + testDocument.components.schemas.CoinsOne = { + properties: { + coins: { + $ref: '#/components/schemas/CoinList', + }, + other_data: { + type: 'string', + }, + }, + }; + testDocument.components.schemas.CoinsTwo = { + properties: { + coins: { + $ref: '#/components/schemas/CoinList', + }, + extra_data: { + type: 'string', + }, + }, + }; + testDocument.components.schemas.CollectionBase = {}; + testDocument.components.schemas.SingleCoin = {}; + testDocument.components.schemas.CoinList = { + type: 'array', + items: { + $ref: '#/components/schemas/SingleCoin', + }, + }; + testDocument.components.schemas.CoinCollection = { + allOf: [ + { + $ref: '#/components/schemas/CollectionBase', + }, + { + oneOf: [ + { + $ref: '#/components/schemas/CoinsOne', + }, + { + $ref: '#/components/schemas/CoinsTwo', + }, + ], + }, + ], + }; + testDocument.paths['/v1/coins'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/CoinCollection', + }, + }, + }, + }, + }, + }, + }; + testDocument.paths['/v1/coins/{id}'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Coin', + }, + }, + }, + }, + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(1); + const r = results[0]; + expect(r.code).toBe(ruleId); + expect(r.severity).toBe(expectedSeverity); + expect(r.message).toBe(resourceListItemsSchemaMessage('coins', 'Coin')); + expect(r.path.join('.')).toBe( + 'paths./v1/coins.get.responses.200.content.application/json.schema.allOf.1.oneOf.0.properties.coins.items' + ); + }); + + it('Prototype schema in POST request body is incorrectly named', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.Mess = {}; + testDocument.components.schemas.MessMaker = {}; + testDocument.paths['/v1/messes'] = { + post: { + requestBody: { + content: { + 'application/json; charset=utf-8': { + schema: { + $ref: '#/components/schemas/MessMaker', + }, + }, + }, + }, + }, + }; + testDocument.paths['/v1/messes/{id}'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Mess', + }, + }, + }, + }, + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(1); + const r = results[0]; + expect(r.code).toBe(ruleId); + expect(r.severity).toBe(expectedSeverity); + expect(r.message).toBe(postPrototypeSchemaMessage('/v1/messes', 'Mess')); + expect(r.path.join('.')).toBe( + 'paths./v1/messes.post.requestBody.content.application/json; charset=utf-8.schema' + ); + }); + + it('Prototype schema in referenced POST request body is incorrectly named', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.Mess = {}; + testDocument.components.schemas.MessMaker = {}; + testDocument.components.requestBodies.MessCreation = { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/MessMaker', + }, + }, + }, + }; + testDocument.paths['/v1/messes'] = { + post: { + requestBody: { + $ref: '#/components/requestBodies/MessCreation', + }, + }, + }; + testDocument.paths['/v1/messes/{id}'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Mess', + }, + }, + }, + }, + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(1); + const r = results[0]; + expect(r.code).toBe(ruleId); + expect(r.severity).toBe(expectedSeverity); + expect(r.message).toBe(postPrototypeSchemaMessage('/v1/messes', 'Mess')); + expect(r.path.join('.')).toBe( + 'paths./v1/messes.post.requestBody.content.application/json.schema' + ); + }); + + it('Prototype schema in PUT request body is incorrectly named', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.Mess = {}; + testDocument.components.schemas.MessMaker = {}; + testDocument.paths['/v1/messes'] = {}; + testDocument.paths['/v1/messes/{id}'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Mess', + }, + }, + }, + }, + }, + }, + put: { + requestBody: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/MessMaker', + }, + }, + }, + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(1); + const r = results[0]; + expect(r.code).toBe(ruleId); + expect(r.severity).toBe(expectedSeverity); + expect(r.message).toBe( + putPrototypeSchemaMessage('/v1/messes/{id}', 'Mess') + ); + expect(r.path.join('.')).toBe( + 'paths./v1/messes/{id}.put.requestBody.content.application/json.schema' + ); + }); + + it('Patch schema (merge patch) is incorrectly named', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.Jeans = {}; + testDocument.components.schemas.JeansUpdater = {}; + testDocument.paths['/v1/jeans'] = {}; + testDocument.paths['/v1/jeans/{id}'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Jeans', + }, + }, + }, + }, + }, + }, + patch: { + requestBody: { + content: { + 'application/merge-patch+json; charset=utf-8': { + schema: { + $ref: '#/components/schemas/JeansUpdater', + }, + }, + }, + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(1); + const r = results[0]; + expect(r.code).toBe(ruleId); + expect(r.severity).toBe(expectedSeverity); + expect(r.message).toBe(patchSchemaMessage('/v1/jeans/{id}', 'Jeans')); + expect(r.path.join('.')).toBe( + 'paths./v1/jeans/{id}.patch.requestBody.content.application/merge-patch+json; charset=utf-8.schema' + ); + }); + + it('Patch schema (json patch) is incorrectly named', async () => { + const testDocument = makeCopy(rootDocument); + + testDocument.components.schemas.Jeans = {}; + testDocument.components.schemas.JeansUpdater = {}; + testDocument.paths['/v1/jeans'] = {}; + testDocument.paths['/v1/jeans/{id}'] = { + get: { + responses: { + 200: { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Jeans', + }, + }, + }, + }, + }, + }, + patch: { + requestBody: { + content: { + 'application/json-patch+json': { + schema: { + $ref: '#/components/schemas/JeansUpdater', + }, + }, + }, + }, + }, + }; + + const results = await testRule(ruleId, rule, testDocument); + expect(results).toHaveLength(1); + const r = results[0]; + expect(r.code).toBe(ruleId); + expect(r.severity).toBe(expectedSeverity); + expect(r.message).toBe(patchSchemaMessage('/v1/jeans/{id}', 'Jeans')); + expect(r.path.join('.')).toBe( + 'paths./v1/jeans/{id}.patch.requestBody.content.application/json-patch+json.schema' + ); + }); + }); +}); diff --git a/packages/ruleset/test/schema-type-format.test.js b/packages/ruleset/test/schema-type-format.test.js index 3eb70c28c..12d8a2496 100644 --- a/packages/ruleset/test/schema-type-format.test.js +++ b/packages/ruleset/test/schema-type-format.test.js @@ -424,26 +424,23 @@ describe(`Spectral rule: ${ruleId}`, () => { }; const results = await testRule(ruleId, rule, testDocument); - expect(results).toHaveLength(6); + expect(results).toHaveLength(4); for (const result of results) { expect(result.code).toBe(ruleId); expect(result.message).toMatch(errorMsgIntegerFormat); expect(result.severity).toBe(expectedSeverity); } expect(results[0].path.join('.')).toBe( - 'paths./v1/movies.post.requestBody.content.application/json.schema.properties.bad_int_prop' - ); - expect(results[1].path.join('.')).toBe( 'paths./v1/movies.post.responses.201.content.application/json.schema.properties.bad_int_prop' ); - expect(results[2].path.join('.')).toBe( + expect(results[1].path.join('.')).toBe( 'paths./v1/movies.get.responses.200.content.application/json.schema.allOf.1.properties.movies.items.properties.bad_int_prop' ); - expect(results[3].path.join('.')).toBe( + expect(results[2].path.join('.')).toBe( 'paths./v1/movies/{movie_id}.get.responses.200.content.application/json.schema.properties.bad_int_prop' ); - expect(results[4].path.join('.')).toBe( - 'paths./v1/movies/{movie_id}.put.requestBody.content.application/json.schema.properties.bad_int_prop' + expect(results[3].path.join('.')).toBe( + 'paths./v1/movies/{movie_id}.put.responses.200.content.application/json.schema.properties.bad_int_prop' ); }); it('Number schema with invalid format - inline response schema', async () => { diff --git a/packages/ruleset/test/unevaluated-properties.test.js b/packages/ruleset/test/unevaluated-properties.test.js index 7e329814f..f6ce5d77e 100644 --- a/packages/ruleset/test/unevaluated-properties.test.js +++ b/packages/ruleset/test/unevaluated-properties.test.js @@ -39,14 +39,12 @@ describe(`Spectral rule: ${ruleId}`, () => { testDocument.components.schemas['Movie'].unevaluatedProperties = true; const results = await testRule(ruleId, rule, testDocument); - expect(results).toHaveLength(6); + expect(results).toHaveLength(4); const expectedPaths = [ - 'paths./v1/movies.post.requestBody.content.application/json.schema.unevaluatedProperties', 'paths./v1/movies.post.responses.201.content.application/json.schema.unevaluatedProperties', 'paths./v1/movies.get.responses.200.content.application/json.schema.allOf.1.properties.movies.items.unevaluatedProperties', 'paths./v1/movies/{movie_id}.get.responses.200.content.application/json.schema.unevaluatedProperties', - 'paths./v1/movies/{movie_id}.put.requestBody.content.application/json.schema.unevaluatedProperties', 'paths./v1/movies/{movie_id}.put.responses.200.content.application/json.schema.unevaluatedProperties', ]; for (let i = 0; i < results.length; i++) { diff --git a/packages/ruleset/test/unique-parameter-request-property-names.test.js b/packages/ruleset/test/unique-parameter-request-property-names.test.js index faa4757d8..b0c3018e9 100644 --- a/packages/ruleset/test/unique-parameter-request-property-names.test.js +++ b/packages/ruleset/test/unique-parameter-request-property-names.test.js @@ -37,7 +37,7 @@ describe(`Spectral rule: ${ruleId}`, () => { }, }, ]; - testDocument.components.schemas.Drink = { + testDocument.components.schemas.DrinkPrototype = { description: 'Fruit juice', type: 'object', required: ['type', 'fruit'], @@ -71,7 +71,7 @@ describe(`Spectral rule: ${ruleId}`, () => { }, }, ]; - testDocument.components.schemas.Drink = { + testDocument.components.schemas.DrinkPrototype = { description: 'Fruit juice', type: 'object', required: ['type', 'fruit'], @@ -116,7 +116,7 @@ describe(`Spectral rule: ${ruleId}`, () => { }, }, ]; - testDocument.components.schemas.Drink = { + testDocument.components.schemas.DrinkPrototype = { description: 'Fruit juice', type: 'object', required: ['type', 'fruit'], @@ -150,7 +150,7 @@ describe(`Spectral rule: ${ruleId}`, () => { }, }, ]; - testDocument.components.schemas.Drink = { + testDocument.components.schemas.DrinkPrototype = { type: 'array', items: { description: 'Fruit juice', @@ -196,7 +196,7 @@ describe(`Spectral rule: ${ruleId}`, () => { }, }, ]; - testDocument.components.schemas.Drink = { + testDocument.components.schemas.DrinkPrototype = { description: 'Fruit juice', type: 'object', required: ['type', 'fruit'], @@ -232,7 +232,7 @@ describe(`Spectral rule: ${ruleId}`, () => { }, }, ]; - testDocument.components.schemas.Drink = { + testDocument.components.schemas.DrinkPrototype = { description: 'Fruit juice', oneOf: [ { @@ -300,7 +300,7 @@ describe(`Spectral rule: ${ruleId}`, () => { }, }, ]; - testDocument.components.schemas.Drink = { + testDocument.components.schemas.DrinkPrototype = { description: 'Fruit juice', type: 'object', required: ['type', 'fruit'], @@ -352,7 +352,7 @@ describe(`Spectral rule: ${ruleId}`, () => { }, }, ]; - testDocument.components.schemas.Drink = { + testDocument.components.schemas.DrinkPrototype = { description: 'Fruit juice', type: 'object', required: ['type', 'fruit'], @@ -406,7 +406,7 @@ describe(`Spectral rule: ${ruleId}`, () => { }, }, ]; - testDocument.components.schemas.Drink = { + testDocument.components.schemas.DrinkPrototype = { description: 'Fruit juice', type: 'object', required: ['type', 'fruit'], @@ -460,7 +460,7 @@ describe(`Spectral rule: ${ruleId}`, () => { }, }, ]; - testDocument.components.schemas.Drink = { + testDocument.components.schemas.DrinkPrototype = { description: 'Fruit juice', type: 'object', allOf: [ @@ -510,7 +510,7 @@ describe(`Spectral rule: ${ruleId}`, () => { }, }, ]; - testDocument.components.schemas.Drink = { + testDocument.components.schemas.DrinkPrototype = { description: 'Fruit juice', oneOf: [ { @@ -570,7 +570,7 @@ describe(`Spectral rule: ${ruleId}`, () => { }, }, ]; - testDocument.components.schemas.Drink = { + testDocument.components.schemas.DrinkPrototype = { description: 'Fruit juice', type: 'object', required: ['type', 'fruit'], @@ -610,7 +610,7 @@ describe(`Spectral rule: ${ruleId}`, () => { }, }, ]; - testDocument.components.schemas.Drink = { + testDocument.components.schemas.DrinkPrototype = { description: 'Fruit juice', type: 'object', required: ['type', 'fruit'], @@ -656,7 +656,7 @@ describe(`Spectral rule: ${ruleId}`, () => { }, }, ]; - testDocument.components.schemas.Drink = { + testDocument.components.schemas.DrinkPrototype = { description: 'Fruit juice', type: 'object', required: ['type', 'fruit'], diff --git a/packages/ruleset/test/utils/root-document.js b/packages/ruleset/test/utils/root-document.js index a0ccae906..1e1a91b32 100644 --- a/packages/ruleset/test/utils/root-document.js +++ b/packages/ruleset/test/utils/root-document.js @@ -47,7 +47,7 @@ module.exports = { content: { 'application/json': { schema: { - $ref: '#/components/schemas/Drink', + $ref: '#/components/schemas/DrinkPrototype', }, }, }, @@ -271,7 +271,7 @@ module.exports = { content: { 'application/json': { schema: { - $ref: '#/components/schemas/Movie', + $ref: '#/components/schemas/MoviePrototype', }, }, }, @@ -428,7 +428,7 @@ module.exports = { content: { 'application/json': { schema: { - $ref: '#/components/schemas/Movie', + $ref: '#/components/schemas/MoviePrototype', }, }, }, @@ -669,6 +669,40 @@ module.exports = { maxLength: 1024, }, }, + example: { + id: 'acb123', + name: 'The Two Towers', + director: 'Peter Jackson', + running_time: 179, + }, + }, + MoviePrototype: { + description: 'This is the Movie creation schema.', + type: 'object', + required: ['name'], + properties: { + name: { + $ref: '#/components/schemas/NormalString', + }, + director: { + $ref: '#/components/schemas/NormalString', + }, + running_time: { + type: 'integer', + format: 'int32', + description: 'The length of the movie, in minutes.', + }, + imdb_url: { + $ref: '#/components/schemas/UrlString', + }, + trailer: { + type: 'string', + format: 'byte', + description: 'A short trailer for the movie.', + minLength: 0, + maxLength: 1024, + }, + }, example: { name: 'The Two Towers', director: 'Peter Jackson', @@ -676,6 +710,32 @@ module.exports = { }, }, Drink: { + type: 'object', + description: + 'A Drink can be either a Juice or Soda instance. Sorry, no Beer or Whisky allowed.', + properties: { + id: { + $ref: '#/components/schemas/IdString', + }, + }, + oneOf: [ + { + $ref: '#/components/schemas/Juice', + }, + { + $ref: '#/components/schemas/Soda', + }, + ], + discriminator: { + propertyName: 'type', + }, + example: { + id: 'acb123', + type: 'soda', + name: 'Root Beer', + }, + }, + DrinkPrototype: { type: 'object', description: 'A Drink can be either a Juice or Soda instance. Sorry, no Beer or Whisky allowed.', @@ -834,6 +894,7 @@ module.exports = { }, movies: [ { + id: 'acb123', name: 'The Two Towers', director: 'Peter Jackson', running_time: 179, @@ -865,6 +926,23 @@ module.exports = { }, }, }, + CarPrototype: { + description: 'Information about a car.', + type: 'object', + required: ['make', 'model'], + properties: { + make: { + description: 'The car make.', + type: 'string', + minLength: 1, + maxLength: 32, + pattern: '.*', + }, + model: { + $ref: '#/components/schemas/CarModelType', + }, + }, + }, CarPatch: { description: 'Information about a car.', type: 'object', @@ -1155,7 +1233,7 @@ module.exports = { content: { 'application/json': { schema: { - $ref: '#/components/schemas/Car', + $ref: '#/components/schemas/CarPrototype', }, examples: { RequestExample: { diff --git a/packages/validator/test/cli-validator/mock-files/oas3/clean-with-tabs.yml b/packages/validator/test/cli-validator/mock-files/oas3/clean-with-tabs.yml index ae7156e44..d512062fa 100644 --- a/packages/validator/test/cli-validator/mock-files/oas3/clean-with-tabs.yml +++ b/packages/validator/test/cli-validator/mock-files/oas3/clean-with-tabs.yml @@ -51,7 +51,7 @@ paths: content: application/json: schema: - $ref: "#/components/schemas/Pets" + $ref: "#/components/schemas/PetCollection" default: description: unexpected error content: @@ -70,7 +70,7 @@ paths: content: application/json: schema: - $ref: "#/components/schemas/Pets" + $ref: "#/components/schemas/Pet" default: description: unexpected error content: @@ -100,7 +100,7 @@ paths: content: application/json: schema: - $ref: "#/components/schemas/Pets" + $ref: "#/components/schemas/Pet" default: description: unexpected error content: @@ -137,7 +137,7 @@ components: id: 1 name: doggie tag: dog - Pets: + PetCollection: type: object description: A list of pets diff --git a/packages/validator/test/cli-validator/mock-files/oas3/clean.yml b/packages/validator/test/cli-validator/mock-files/oas3/clean.yml index 6f9b0683d..35000577e 100644 --- a/packages/validator/test/cli-validator/mock-files/oas3/clean.yml +++ b/packages/validator/test/cli-validator/mock-files/oas3/clean.yml @@ -51,7 +51,7 @@ paths: content: application/json: schema: - $ref: "#/components/schemas/Pets" + $ref: "#/components/schemas/PetCollection" default: description: unexpected error content: @@ -70,7 +70,7 @@ paths: content: application/json: schema: - $ref: "#/components/schemas/Pets" + $ref: "#/components/schemas/Pet" default: description: unexpected error content: @@ -100,7 +100,7 @@ paths: content: application/json: schema: - $ref: "#/components/schemas/Pets" + $ref: "#/components/schemas/Pet" default: description: unexpected error content: @@ -137,7 +137,7 @@ components: id: 1 name: doggie tag: dog - Pets: + PetCollection: type: object description: A list of pets diff --git a/packages/validator/test/cli-validator/mock-files/oas31/clean.yml b/packages/validator/test/cli-validator/mock-files/oas31/clean.yml index 30aecef13..32a4874a7 100644 --- a/packages/validator/test/cli-validator/mock-files/oas31/clean.yml +++ b/packages/validator/test/cli-validator/mock-files/oas31/clean.yml @@ -51,7 +51,7 @@ paths: content: application/json: schema: - $ref: "#/components/schemas/Pets" + $ref: "#/components/schemas/PetCollection" default: description: unexpected error content: @@ -70,7 +70,7 @@ paths: content: application/json: schema: - $ref: "#/components/schemas/Pets" + $ref: "#/components/schemas/Pet" default: description: unexpected error content: @@ -100,7 +100,7 @@ paths: content: application/json: schema: - $ref: "#/components/schemas/Pets" + $ref: "#/components/schemas/Pet" default: description: unexpected error content: @@ -137,7 +137,7 @@ components: id: 1 name: doggie tag: dog - Pets: + PetCollection: type: object description: A list of pets