diff --git a/tools/spectral/ipa/__tests__/updateResponseCodeShouldBe200OK.test.js b/tools/spectral/ipa/__tests__/updateResponseCodeShouldBe200OK.test.js new file mode 100644 index 0000000000..c472f17707 --- /dev/null +++ b/tools/spectral/ipa/__tests__/updateResponseCodeShouldBe200OK.test.js @@ -0,0 +1,292 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +testRule('xgen-IPA-107-put-method-response-code-is-200', [ + { + name: 'valid methods', + document: { + paths: { + '/resource/{id}': { + put: { + responses: { + 200: {}, + 400: {}, + 500: {}, + }, + }, + }, + '/resource/{id}/singleton': { + put: { + responses: { + 200: {}, + 400: {}, + 500: {}, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid methods', + document: { + paths: { + '/resourceOne/{id}': { + put: { + responses: { + 201: {}, + 400: {}, + 500: {}, + }, + }, + }, + '/resourceTwo/{id}': { + put: { + responses: { + 400: {}, + 500: {}, + }, + }, + }, + '/resourceThree/{id}': { + put: { + responses: { + 200: {}, + 201: {}, + 400: {}, + 500: {}, + }, + }, + }, + '/resource/{id}/singleton': { + put: { + responses: { + 202: {}, + 400: {}, + 500: {}, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-107-put-method-response-code-is-200', + message: + 'The Update method response status code should be 200 OK. This method either lacks a 200 OK response or defines a different 2xx status code.', + path: ['paths', '/resourceOne/{id}', 'put'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-107-put-method-response-code-is-200', + message: + 'The Update method response status code should be 200 OK. This method either lacks a 200 OK response or defines a different 2xx status code.', + path: ['paths', '/resourceTwo/{id}', 'put'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-107-put-method-response-code-is-200', + message: + 'The Update method response status code should be 200 OK. This method either lacks a 200 OK response or defines a different 2xx status code.', + path: ['paths', '/resourceThree/{id}', 'put'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-107-put-method-response-code-is-200', + message: + 'The Update method response status code should be 200 OK. This method either lacks a 200 OK response or defines a different 2xx status code.', + path: ['paths', '/resource/{id}/singleton', 'put'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid method with exception', + document: { + paths: { + '/resourceOne/{id}': { + put: { + responses: { + 201: {}, + 400: {}, + 500: {}, + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-107-put-method-response-code-is-200': 'reason', + }, + }, + }, + '/resourceTwo/{id}': { + put: { + responses: { + 400: {}, + 500: {}, + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-107-put-method-response-code-is-200': 'reason', + }, + }, + }, + '/resource/{id}/singleton': { + put: { + responses: { + 202: {}, + 400: {}, + 500: {}, + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-107-put-method-response-code-is-200': 'reason', + }, + }, + }, + }, + }, + errors: [], + }, +]); + +testRule('xgen-IPA-107-patch-method-response-code-is-200', [ + { + name: 'valid methods', + document: { + paths: { + '/resource/{id}': { + patch: { + responses: { + 200: {}, + 400: {}, + 500: {}, + }, + }, + }, + '/resource/{id}/singleton': { + patch: { + responses: { + 200: {}, + 400: {}, + 500: {}, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid methods', + document: { + paths: { + '/resourceOne/{id}': { + patch: { + responses: { + 201: {}, + 400: {}, + 500: {}, + }, + }, + }, + '/resourceTwo/{id}': { + patch: { + responses: { + 400: {}, + 500: {}, + }, + }, + }, + '/resourceThree/{id}': { + patch: { + responses: { + 200: {}, + 201: {}, + 400: {}, + 500: {}, + }, + }, + }, + '/resource/{id}/singleton': { + patch: { + responses: { + 202: {}, + 400: {}, + 500: {}, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-107-patch-method-response-code-is-200', + message: + 'The Update method response status code should be 200 OK. This method either lacks a 200 OK response or defines a different 2xx status code.', + path: ['paths', '/resourceOne/{id}', 'patch'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-107-patch-method-response-code-is-200', + message: + 'The Update method response status code should be 200 OK. This method either lacks a 200 OK response or defines a different 2xx status code.', + path: ['paths', '/resourceTwo/{id}', 'patch'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-107-patch-method-response-code-is-200', + message: + 'The Update method response status code should be 200 OK. This method either lacks a 200 OK response or defines a different 2xx status code.', + path: ['paths', '/resourceThree/{id}', 'patch'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-107-patch-method-response-code-is-200', + message: + 'The Update method response status code should be 200 OK. This method either lacks a 200 OK response or defines a different 2xx status code.', + path: ['paths', '/resource/{id}/singleton', 'patch'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid method with exception', + document: { + paths: { + '/resourceOne/{id}': { + patch: { + responses: { + 201: {}, + 400: {}, + 500: {}, + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-107-patch-method-response-code-is-200': 'reason', + }, + }, + }, + '/resourceTwo/{id}': { + patch: { + responses: { + 400: {}, + 500: {}, + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-107-patch-method-response-code-is-200': 'reason', + }, + }, + }, + '/resource/{id}/singleton': { + patch: { + responses: { + 202: {}, + 400: {}, + 500: {}, + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-107-patch-method-response-code-is-200': 'reason', + }, + }, + }, + }, + }, + errors: [], + }, +]); diff --git a/tools/spectral/ipa/rulesets/IPA-107.yaml b/tools/spectral/ipa/rulesets/IPA-107.yaml index 3cc42b093b..896ee78f64 100644 --- a/tools/spectral/ipa/rulesets/IPA-107.yaml +++ b/tools/spectral/ipa/rulesets/IPA-107.yaml @@ -3,6 +3,7 @@ functions: - updateMethodMustNotHaveQueryParams + - updateResponseCodeShouldBe200OK rules: xgen-IPA-107-put-must-not-have-query-params: @@ -24,7 +25,7 @@ rules: ignoredValues: ['pretty', 'envelope'] xgen-IPA-107-patch-must-not-have-query-params: description: >- - Update operations must not accept query parameters. + Update operations must not accept query parameters. ##### Implementation details @@ -39,3 +40,31 @@ rules: function: 'updateMethodMustNotHaveQueryParams' functionOptions: ignoredValues: ['pretty', 'envelope'] + xgen-IPA-107-put-method-response-code-is-200: + description: >- + The Update method response status code should be 200 OK. + + ##### Implementation details + + Validation checks the PUT method for single resource paths and [singleton resources](https://go/ipa/113). + + - Operation objects with `x-xgen-IPA-exception` for this rule are excluded from validation + message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-107-put-method-response-code-is-200' + severity: warn + given: '$.paths[*].put' + then: + function: 'updateResponseCodeShouldBe200OK' + xgen-IPA-107-patch-method-response-code-is-200: + description: >- + The Update method response status code should be 200 OK. + + ##### Implementation details + + Validation checks the PATCH method for single resource paths and [singleton resources](https://go/ipa/113). + + - Operation objects with `x-xgen-IPA-exception` for this rule are excluded from validation + message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-107-patch-method-response-code-is-200' + severity: warn + given: '$.paths[*].patch' + then: + function: 'updateResponseCodeShouldBe200OK' diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 41c008c0b2..394c5651ba 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -170,12 +170,28 @@ Validation checks the PUT method for single resource paths and singleton resourc #### xgen-IPA-107-patch-must-not-have-query-params ![warn](https://img.shields.io/badge/warning-yellow) -Update operations must not accept query parameters. +Update operations must not accept query parameters. ##### Implementation details Validation checks the PATCH method for single resource paths and singleton resources. - Query parameters `envelope` and `pretty` are exempt from this rule - Operation objects with `x-xgen-IPA-exception` for this rule are excluded from validation +#### xgen-IPA-107-put-method-response-code-is-200 + + ![warn](https://img.shields.io/badge/warning-yellow) +The Update method response status code should be 200 OK. +##### Implementation details +Validation checks the PUT method for single resource paths and [singleton resources](https://go/ipa/113). + + - Operation objects with `x-xgen-IPA-exception` for this rule are excluded from validation +#### xgen-IPA-107-patch-method-response-code-is-200 + + ![warn](https://img.shields.io/badge/warning-yellow) +The Update method response status code should be 200 OK. +##### Implementation details +Validation checks the PATCH method for single resource paths and [singleton resources](https://go/ipa/113). + + - Operation objects with `x-xgen-IPA-exception` for this rule are excluded from validation ### IPA-108 diff --git a/tools/spectral/ipa/rulesets/functions/createMethodResponseCodeIs201Created.js b/tools/spectral/ipa/rulesets/functions/createMethodResponseCodeIs201Created.js index b079469536..33328e01e2 100644 --- a/tools/spectral/ipa/rulesets/functions/createMethodResponseCodeIs201Created.js +++ b/tools/spectral/ipa/rulesets/functions/createMethodResponseCodeIs201Created.js @@ -5,12 +5,8 @@ import { isSingletonResource, } from './utils/resourceEvaluation.js'; import { hasException } from './utils/exceptions.js'; -import { - collectAdoption, - collectAndReturnViolation, - collectException, - handleInternalError, -} from './utils/collectionUtils.js'; +import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; +import { checkResponseCodeAndReturnErrors } from './utils/validations.js'; const RULE_NAME = 'xgen-IPA-106-create-method-response-code-is-201'; const ERROR_MESSAGE = @@ -31,29 +27,9 @@ export default (input, _, { path, documentInventory }) => { return; } - const errors = checkViolationsAndReturnErrors(input, path); + const errors = checkResponseCodeAndReturnErrors(input, '201', path, RULE_NAME, ERROR_MESSAGE); if (errors.length !== 0) { return collectAndReturnViolation(path, RULE_NAME, errors); } collectAdoption(path, RULE_NAME); }; - -function checkViolationsAndReturnErrors(input, path) { - try { - const responses = input.responses; - - // If there is no 201 response, return a violation - if (!responses || !responses['201']) { - return [{ path, message: ERROR_MESSAGE }]; - } - - // If there are other 2xx responses that are not 201, return a violation - if (Object.keys(responses).some((key) => key.startsWith('2') && key !== '201')) { - return [{ path, message: ERROR_MESSAGE }]; - } - - return []; - } catch (e) { - handleInternalError(RULE_NAME, path, e); - } -} diff --git a/tools/spectral/ipa/rulesets/functions/getResponseCodeShouldBe200OK.js b/tools/spectral/ipa/rulesets/functions/getResponseCodeShouldBe200OK.js index 5a65dc4d02..f85c002211 100644 --- a/tools/spectral/ipa/rulesets/functions/getResponseCodeShouldBe200OK.js +++ b/tools/spectral/ipa/rulesets/functions/getResponseCodeShouldBe200OK.js @@ -1,16 +1,12 @@ import { hasException } from './utils/exceptions.js'; -import { - collectAdoption, - collectAndReturnViolation, - collectException, - handleInternalError, -} from './utils/collectionUtils.js'; +import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; import { getResourcePathItems, isResourceCollectionIdentifier, isSingleResourceIdentifier, isSingletonResource, } from './utils/resourceEvaluation.js'; +import { checkResponseCodeAndReturnErrors } from './utils/validations.js'; const RULE_NAME = 'xgen-IPA-104-get-method-response-code-is-200'; const ERROR_MESSAGE = @@ -33,27 +29,9 @@ export default (input, _, { path, documentInventory }) => { return; } - const errors = checkViolationsAndReturnErrors(input, path); + const errors = checkResponseCodeAndReturnErrors(input, '200', path, RULE_NAME, ERROR_MESSAGE); if (errors.length !== 0) { return collectAndReturnViolation(path, RULE_NAME, errors); } collectAdoption(path, RULE_NAME); }; - -function checkViolationsAndReturnErrors(input, path) { - try { - const responses = input.responses; - // If there is no 200 response, return a violation - if (!responses || !responses['200']) { - return [{ path, message: ERROR_MESSAGE }]; - } - - // If there are other 2xx responses that are not 200, return a violation - if (Object.keys(responses).some((key) => key.startsWith('2') && key !== '200')) { - return [{ path, message: ERROR_MESSAGE }]; - } - return []; - } catch (e) { - handleInternalError(RULE_NAME, path, e); - } -} diff --git a/tools/spectral/ipa/rulesets/functions/listResponseCodeShouldBe200OK.js b/tools/spectral/ipa/rulesets/functions/listResponseCodeShouldBe200OK.js index 1cc3b52c48..57ff71f4f6 100644 --- a/tools/spectral/ipa/rulesets/functions/listResponseCodeShouldBe200OK.js +++ b/tools/spectral/ipa/rulesets/functions/listResponseCodeShouldBe200OK.js @@ -5,6 +5,7 @@ import { isResourceCollectionIdentifier, isSingletonResource, } from './utils/resourceEvaluation.js'; +import { checkResponseCodeAndReturnErrors } from './utils/validations.js'; const RULE_NAME = 'xgen-IPA-105-list-method-response-code-is-200'; const ERROR_MESSAGE = @@ -25,27 +26,10 @@ export default (input, _, { path, documentInventory }) => { return; } - const errors = checkViolationsAndReturnErrors(input, path); + const errors = checkResponseCodeAndReturnErrors(input, '200', path, RULE_NAME, ERROR_MESSAGE); if (errors.length !== 0) { return collectAndReturnViolation(path, RULE_NAME, errors); } return collectAdoption(path, RULE_NAME); }; - -function checkViolationsAndReturnErrors(input, path) { - if (input['responses']) { - const responses = input['responses']; - - // If there is no 200 response, return a violation - if (!responses['200']) { - return [{ path, message: ERROR_MESSAGE }]; - } - - // If there are other 2xx responses that are not 200, return a violation - if (Object.keys(responses).some((key) => key.startsWith('2') && key !== '200')) { - return [{ path, message: ERROR_MESSAGE }]; - } - } - return []; -} diff --git a/tools/spectral/ipa/rulesets/functions/updateResponseCodeShouldBe200OK.js b/tools/spectral/ipa/rulesets/functions/updateResponseCodeShouldBe200OK.js new file mode 100644 index 0000000000..cf992845ae --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/updateResponseCodeShouldBe200OK.js @@ -0,0 +1,37 @@ +import { hasException } from './utils/exceptions.js'; +import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; +import { + getResourcePathItems, + isResourceCollectionIdentifier, + isSingleResourceIdentifier, + isSingletonResource, +} from './utils/resourceEvaluation.js'; +import { checkResponseCodeAndReturnErrors } from './utils/validations.js'; + +const ERROR_MESSAGE = + 'The Update method response status code should be 200 OK. This method either lacks a 200 OK response or defines a different 2xx status code.'; + +export default (input, _, { path, documentInventory, rule }) => { + const resourcePath = path[1]; + const oas = documentInventory.resolved; + const resourcePaths = getResourcePathItems(resourcePath, oas.paths); + const ruleName = rule.name; + + if ( + !isSingleResourceIdentifier(resourcePath) && + !(isResourceCollectionIdentifier(resourcePath) && isSingletonResource(resourcePaths)) + ) { + return; + } + + if (hasException(input, ruleName)) { + collectException(input, ruleName, path); + return; + } + + const errors = checkResponseCodeAndReturnErrors(input, '200', path, ruleName, ERROR_MESSAGE); + if (errors.length !== 0) { + return collectAndReturnViolation(path, ruleName, errors); + } + collectAdoption(path, ruleName); +}; diff --git a/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js b/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js index ab8431a0d5..9feb2cbd6e 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js +++ b/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js @@ -49,8 +49,9 @@ export function collectException(object, ruleName, jsonPath) { /** * Creates internal rule error entry for the collector in order to not fail validation process. - * @param {Array} jsonPathArray - The JSON path for the object where the rule exception occurred. * @param {string} ruleName - The name of the rule that was adopted. + * @param {Array} jsonPathArray - The JSON path for the object where the rule exception occurred. + * @param {string} error - The error message */ export function handleInternalError(ruleName, jsonPathArray, error) { return [ diff --git a/tools/spectral/ipa/rulesets/functions/utils/validations.js b/tools/spectral/ipa/rulesets/functions/utils/validations.js new file mode 100644 index 0000000000..403cc0a52b --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/utils/validations.js @@ -0,0 +1,32 @@ +import { handleInternalError } from './collectionUtils.js'; + +/** + * Common validation function for checking that responses have the expected status code. + * Returns errors in case of violations, ready to be used in a custom validation function. + * + * @param {Object} operationObject the operation object to evaluate + * @param {string} expectedStatusCode the expected status code to validate for + * @param {string[]} path the path to the operation object being evaluated + * @param {string} ruleName the rule name + * @param errorMessage the error message + * @returns {*[]|[{path, message}]} the errors found, or an empty array in case of no errors + */ +export function checkResponseCodeAndReturnErrors(operationObject, expectedStatusCode, path, ruleName, errorMessage) { + try { + const responses = operationObject.responses; + // If the expected status code is not present, return a violation + if (!responses || !responses[expectedStatusCode]) { + return [{ path, message: errorMessage }]; + } + + // If there are other responses within the same status code group (hundreds), return a violation + if ( + Object.keys(responses).some((key) => key.startsWith(expectedStatusCode.charAt(0)) && key !== expectedStatusCode) + ) { + return [{ path, message: errorMessage }]; + } + return []; + } catch (e) { + handleInternalError(ruleName, path, e); + } +}