-
Notifications
You must be signed in to change notification settings - Fork 91
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(new-rule): ibm-accept-and-return-models (#680)
This commit introduces a new rule, `ibm-accept-and-return-models`, which enforces Handbook guidance requiring request and response bodies to be represented as "models". It does so by enforcing that a JSON request or response body defines concrete fields through the `properties` attribute of its schema. Signed-off-by: Dustin Popp <[email protected]>
- Loading branch information
Showing
11 changed files
with
320 additions
and
23 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
76 changes: 76 additions & 0 deletions
76
packages/ruleset/src/functions/accept-and-return-models.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| /** | ||
| * Copyright 2024 IBM Corporation. | ||
| * SPDX-License-Identifier: Apache2.0 | ||
| */ | ||
|
|
||
| const { | ||
| isObject, | ||
| schemaHasConstraint, | ||
| } = require('@ibm-cloud/openapi-ruleset-utilities'); | ||
| const { supportsJsonContent, LoggerFactory } = 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-operation-responses - all operations define a response | ||
| * ibm-requestbody-is-object - JSON request bodies are object schemas | ||
| * ibm-request-and-response content - request and response bodies define content | ||
| * ibm-well-defined-dictionaries - additional properties aren't mixed with static properties | ||
| * | ||
| */ | ||
|
|
||
| module.exports = function acceptAndReturnModels(operation, options, context) { | ||
| if (!logger) { | ||
| ruleId = context.rule.name; | ||
| logger = LoggerFactory.getInstance().getLogger(ruleId); | ||
| } | ||
| return checkForProperties(operation, context.path); | ||
| }; | ||
|
|
||
| /** | ||
| * This function checks to ensure a request or response body schema | ||
| * contains statically defined properties (i.e. is a model and not | ||
| * a dictionary). | ||
| * | ||
| * @param {*} schema - request or response body schema | ||
| * @param {*} path - path to current openapi artifact, as a list | ||
| * @returns an array containing the violations found or [] if no violations | ||
| */ | ||
| function checkForProperties(schema, path) { | ||
| logger.debug(`${ruleId}: checking schema at location: ${path.join('.')}`); | ||
|
|
||
| // Content that does not use JSON representation is exempt. | ||
| if (!supportsJsonContent(path.at(-2))) { | ||
| logger.debug( | ||
| `${ruleId}: skipping non-JSON schema at location: ${path.join('.')}` | ||
| ); | ||
| return []; | ||
| } | ||
|
|
||
| if (!schemaHasConstraint(schema, s => schemaDefinesProperties(s))) { | ||
| logger.debug( | ||
| `${ruleId}: No properties found in schema at location: ${path.join('.')}` | ||
| ); | ||
| return [ | ||
| { | ||
| message: | ||
| 'Request and response bodies must include fields defined in `properties`', | ||
| path, | ||
| }, | ||
| ]; | ||
| } | ||
|
|
||
| logger.debug(`${ruleId}: schema at location: ${path.join('.')} passed!`); | ||
| } | ||
|
|
||
| function schemaDefinesProperties(s) { | ||
| return ( | ||
| s.properties && | ||
| isObject(s.properties) && | ||
| Object.entries(s.properties).length > 0 | ||
| ); | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| /** | ||
| * Copyright 2024 IBM Corporation. | ||
| * SPDX-License-Identifier: Apache2.0 | ||
| */ | ||
|
|
||
| const { | ||
| responseSchemas, | ||
| requestBodySchemas, | ||
| } = require('@ibm-cloud/openapi-ruleset-utilities/src/collections'); | ||
| const { oas3 } = require('@stoplight/spectral-formats'); | ||
| const { acceptAndReturnModels } = require('../functions'); | ||
|
|
||
| module.exports = { | ||
| description: 'Request and response bodies must be defined as model instances', | ||
| given: [...responseSchemas, ...requestBodySchemas], | ||
| message: '{{error}}', | ||
| severity: 'error', | ||
| formats: [oas3], | ||
| resolved: true, | ||
| then: { | ||
| function: acceptAndReturnModels, | ||
| }, | ||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| /** | ||
| * Copyright 2024 IBM Corporation. | ||
| * SPDX-License-Identifier: Apache2.0 | ||
| */ | ||
|
|
||
| const { acceptAndReturnModels } = require('../src/rules'); | ||
| const { makeCopy, rootDocument, testRule, severityCodes } = require('./utils'); | ||
|
|
||
| const rule = acceptAndReturnModels; | ||
| const ruleId = 'ibm-accept-and-return-models'; | ||
| const expectedSeverity = severityCodes.error; | ||
| const expectedMessage = | ||
| 'Request and response bodies must include fields defined in `properties`'; | ||
|
|
||
| // To enable debug logging in the rule function, copy this statement to an it() block: | ||
| // LoggerFactory.getInstance().addLoggerSetting(ruleId, 'debug'); | ||
| // and uncomment this import statement: | ||
| // const LoggerFactory = require('../src/utils/logger-factory'); | ||
|
|
||
| describe(`Spectral rule: ${ruleId}`, () => { | ||
| describe('Should not yield errors', () => { | ||
| it('Clean spec', async () => { | ||
| const results = await testRule(ruleId, rule, rootDocument); | ||
| expect(results).toHaveLength(0); | ||
| }); | ||
|
|
||
| // non-JSON content, captured in root doc but might as well be explicit !!! | ||
| it('Content is non-JSON', async () => { | ||
| const testDocument = makeCopy(rootDocument); | ||
|
|
||
| testDocument.paths['/v1/movies'].post.requestBody.content = { | ||
| 'text/plain': { | ||
| schema: { | ||
| type: 'string', | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| const results = await testRule(ruleId, rule, testDocument); | ||
| expect(results).toHaveLength(0); | ||
| }); | ||
| }); | ||
|
|
||
| describe('Should yield errors', () => { | ||
| describe('Request bodies', () => { | ||
| it('Schema has no defined properties - empty', async () => { | ||
| const testDocument = makeCopy(rootDocument); | ||
|
|
||
| testDocument.components.schemas.MoviePrototype = { | ||
| type: 'object', | ||
| }; | ||
|
|
||
| const results = await testRule(ruleId, rule, testDocument); | ||
| expect(results).toHaveLength(2); | ||
|
|
||
| const expectedPaths = [ | ||
| 'paths./v1/movies.post.requestBody.content.application/json.schema', | ||
| 'paths./v1/movies/{movie_id}.put.requestBody.content.application/json.schema', | ||
| ]; | ||
|
|
||
| for (let i = 0; i < results.length; i++) { | ||
| expect(results[i].code).toBe(ruleId); | ||
| expect(results[i].message).toBe(expectedMessage); | ||
| expect(results[i].severity).toBe(expectedSeverity); | ||
| expect(results[i].path.join('.')).toBe(expectedPaths[i]); | ||
| } | ||
| }); | ||
|
|
||
| it('Schema has no defined properties - only additional properties', async () => { | ||
| const testDocument = makeCopy(rootDocument); | ||
|
|
||
| testDocument.components.schemas.MoviePrototype = { | ||
| type: 'object', | ||
| additionalProperties: { | ||
| type: 'string', | ||
| }, | ||
| }; | ||
|
|
||
| const results = await testRule(ruleId, rule, testDocument); | ||
| expect(results).toHaveLength(2); | ||
|
|
||
| const expectedPaths = [ | ||
| 'paths./v1/movies.post.requestBody.content.application/json.schema', | ||
| 'paths./v1/movies/{movie_id}.put.requestBody.content.application/json.schema', | ||
| ]; | ||
|
|
||
| for (let i = 0; i < results.length; i++) { | ||
| expect(results[i].code).toBe(ruleId); | ||
| expect(results[i].message).toBe(expectedMessage); | ||
| expect(results[i].severity).toBe(expectedSeverity); | ||
| expect(results[i].path.join('.')).toBe(expectedPaths[i]); | ||
| } | ||
| }); | ||
|
|
||
| it('Schema has no defined properties - properties entry is empty', async () => { | ||
| const testDocument = makeCopy(rootDocument); | ||
|
|
||
| testDocument.components.schemas.CarPatch = { | ||
| type: 'object', | ||
| properties: {}, | ||
| }; | ||
|
|
||
| const results = await testRule(ruleId, rule, testDocument); | ||
| expect(results).toHaveLength(1); | ||
|
|
||
| const expectedPaths = [ | ||
| 'paths./v1/cars/{car_id}.patch.requestBody.content.application/merge-patch+json; charset=utf-8.schema', | ||
| ]; | ||
|
|
||
| for (let i = 0; i < results.length; i++) { | ||
| expect(results[i].code).toBe(ruleId); | ||
| expect(results[i].message).toBe(expectedMessage); | ||
| expect(results[i].severity).toBe(expectedSeverity); | ||
| expect(results[i].path.join('.')).toBe(expectedPaths[i]); | ||
| } | ||
| }); | ||
| }); | ||
| }); | ||
| }); |
Oops, something went wrong.