From 9ba83cc03cf22a80986ef0f84f31a09f4da3084f Mon Sep 17 00:00:00 2001 From: wtrocki Date: Tue, 4 Mar 2025 15:44:40 +0100 Subject: [PATCH 1/2] IPA-108: Add rulesets for delete method responses --- .../__tests__/deleteMethod204Response.test.js | 66 +++++++++++++++++ .../__tests__/deleteMethod404Response.test.js | 63 ++++++++++++++++ .../deleteMethodNoRequestBody.test.js | 71 ++++++++++++++++++ ...eMethodResponseShouldNotHaveSchema.test.js | 73 +++++++++++++++++++ tools/spectral/ipa/ipa-spectral.yaml | 1 + tools/spectral/ipa/rulesets/IPA-108.yaml | 41 +++++++++++ tools/spectral/ipa/rulesets/README.md | 11 +++ .../functions/deleteMethod204Response.js | 29 ++++++++ .../functions/deleteMethod404Response.js | 29 ++++++++ .../functions/deleteMethodNoRequestBody.js | 28 +++++++ ...deleteMethodResponseShouldNotHaveSchema.js | 35 +++++++++ .../functions/utils/collectionUtils.js | 14 ++-- 12 files changed, 454 insertions(+), 7 deletions(-) create mode 100644 tools/spectral/ipa/__tests__/deleteMethod204Response.test.js create mode 100644 tools/spectral/ipa/__tests__/deleteMethod404Response.test.js create mode 100644 tools/spectral/ipa/__tests__/deleteMethodNoRequestBody.test.js create mode 100644 tools/spectral/ipa/__tests__/deleteMethodResponseShouldNotHaveSchema.test.js create mode 100644 tools/spectral/ipa/rulesets/IPA-108.yaml create mode 100644 tools/spectral/ipa/rulesets/functions/deleteMethod204Response.js create mode 100644 tools/spectral/ipa/rulesets/functions/deleteMethod404Response.js create mode 100644 tools/spectral/ipa/rulesets/functions/deleteMethodNoRequestBody.js create mode 100644 tools/spectral/ipa/rulesets/functions/deleteMethodResponseShouldNotHaveSchema.js diff --git a/tools/spectral/ipa/__tests__/deleteMethod204Response.test.js b/tools/spectral/ipa/__tests__/deleteMethod204Response.test.js new file mode 100644 index 0000000000..8635c5e982 --- /dev/null +++ b/tools/spectral/ipa/__tests__/deleteMethod204Response.test.js @@ -0,0 +1,66 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +testRule('xgen-IPA-108-delete-method-return-204-response', [ + { + name: 'valid DELETE with 204', + document: { + paths: { + '/resource/{id}': { + delete: { + responses: { + 204: { + description: 'Resource deleted', + }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid DELETE missing 204', + document: { + paths: { + '/resource/{id}': { + delete: { + responses: { + 200: { + description: 'Resource deleted', + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-108-delete-method-return-204-response', + message: 'DELETE method should return 204 No Content status code http://go/ipa/108', + path: ['paths', '/resource/{id}', 'delete'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'valid with exception', + document: { + paths: { + '/resource/{id}': { + delete: { + 'x-xgen-IPA-exception': { + 'xgen-IPA-108-delete-method-return-204-response': 'Legacy API', + }, + responses: { + 200: { + description: 'Resource deleted', + }, + }, + }, + }, + }, + }, + errors: [], + }, +]); diff --git a/tools/spectral/ipa/__tests__/deleteMethod404Response.test.js b/tools/spectral/ipa/__tests__/deleteMethod404Response.test.js new file mode 100644 index 0000000000..5e57f3063c --- /dev/null +++ b/tools/spectral/ipa/__tests__/deleteMethod404Response.test.js @@ -0,0 +1,63 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +testRule('xgen-IPA-108-delete-include-404-response', [ + { + name: 'valid DELETE with 404', + document: { + paths: { + '/resource/{id}': { + delete: { + responses: { + 204: {}, + 404: { + description: 'Resource not found', + }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid DELETE missing 404', + document: { + paths: { + '/resource/{id}': { + delete: { + responses: { + 204: {}, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-108-delete-include-404-response', + message: 'DELETE method should include 404 status code for not found resources http://go/ipa/108', + path: ['paths', '/resource/{id}', 'delete'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'valid with exception', + document: { + paths: { + '/resource/{id}': { + delete: { + 'x-xgen-IPA-exception': { + 'xgen-IPA-108-delete-include-404-response': 'Idempotent delete', + }, + responses: { + 204: {}, + }, + }, + }, + }, + }, + errors: [], + }, +]); diff --git a/tools/spectral/ipa/__tests__/deleteMethodNoRequestBody.test.js b/tools/spectral/ipa/__tests__/deleteMethodNoRequestBody.test.js new file mode 100644 index 0000000000..5e208e847b --- /dev/null +++ b/tools/spectral/ipa/__tests__/deleteMethodNoRequestBody.test.js @@ -0,0 +1,71 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +testRule('xgen-IPA-108-delete-request-no-body', [ + { + name: 'valid DELETE without body', + document: { + paths: { + '/resource/{id}': { + delete: { + responses: { + 204: {}, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid DELETE with body', + document: { + paths: { + '/resource/{id}': { + delete: { + requestBody: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { type: 'object' }, + }, + }, + }, + responses: { + 204: {}, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-108-delete-request-no-body', + message: 'DELETE method should not have a request body http://go/ipa/108', + path: ['paths', '/resource/{id}', 'delete'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'valid with exception', + document: { + paths: { + '/resource/{id}': { + delete: { + 'x-xgen-IPA-exception': { + 'xgen-IPA-108-delete-request-no-body': 'Bulk delete operation', + }, + requestBody: { + content: { + 'application/json': { + schema: { type: 'object' }, + }, + }, + }, + }, + }, + }, + }, + errors: [], + }, +]); diff --git a/tools/spectral/ipa/__tests__/deleteMethodResponseShouldNotHaveSchema.test.js b/tools/spectral/ipa/__tests__/deleteMethodResponseShouldNotHaveSchema.test.js new file mode 100644 index 0000000000..216d9848fb --- /dev/null +++ b/tools/spectral/ipa/__tests__/deleteMethodResponseShouldNotHaveSchema.test.js @@ -0,0 +1,73 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +testRule('xgen-IPA-108-delete-response-should-be-empty', [ + { + name: 'valid DELETE with void 204', + document: { + paths: { + '/resource/{id}': { + delete: { + responses: { + 204: {}, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid DELETE with non-void 204', + document: { + paths: { + '/resource/{id}': { + delete: { + responses: { + 204: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { type: 'object' }, + }, + }, + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-108-delete-response-should-be-empty', + message: + 'DELETE method should return an empty response. The response should not have a schema property and reference to models http://go/ipa/108', + path: ['paths', '/resource/{id}', 'delete'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'valid with exception', + document: { + paths: { + '/resource/{id}': { + delete: { + 'x-xgen-IPA-exception': { + 'xgen-IPA-108-delete-response-should-be-empty': 'Legacy API', + }, + responses: { + 204: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { type: 'object' }, + }, + }, + }, + }, + }, + }, + }, + }, + errors: [], + }, +]); diff --git a/tools/spectral/ipa/ipa-spectral.yaml b/tools/spectral/ipa/ipa-spectral.yaml index d9ee29a7e6..0693a5a88e 100644 --- a/tools/spectral/ipa/ipa-spectral.yaml +++ b/tools/spectral/ipa/ipa-spectral.yaml @@ -6,6 +6,7 @@ extends: - ./rulesets/IPA-113.yaml - ./rulesets/IPA-123.yaml - ./rulesets/IPA-106.yaml + - ./rulesets/IPA-108.yaml overrides: - files: diff --git a/tools/spectral/ipa/rulesets/IPA-108.yaml b/tools/spectral/ipa/rulesets/IPA-108.yaml new file mode 100644 index 0000000000..0503083527 --- /dev/null +++ b/tools/spectral/ipa/rulesets/IPA-108.yaml @@ -0,0 +1,41 @@ +# IPA-108: Delete +# http://go/ipa/108 + +rules: + xgen-IPA-108-delete-response-should-be-empty: + description: Delete method response should not have schema reference to object http://go/ipa/108 + message: '{{error}} http://go/ipa/108' + severity: warn + given: $.paths[*].delete + then: + function: deleteMethodResponseShouldNotHaveSchema + + xgen-IPA-108-delete-method-return-204-response: + description: DELETE method must return 204 No Content + message: '{{error}} http://go/ipa/108' + severity: warn + given: $.paths[*].delete + then: + function: deleteMethod204Response + + xgen-IPA-108-delete-request-no-body: + description: DELETE method must not have request body + message: '{{error}} http://go/ipa/108' + severity: warn + given: $.paths[*].delete + then: + function: deleteMethodNoRequestBody + + xgen-IPA-108-delete-include-404-response: + description: DELETE method must include 404 response and return it when resource not found + message: '{{error}} http://go/ipa/108' + severity: warn + given: $.paths[*].delete + then: + function: deleteMethod404Response + +functions: + - deleteMethodResponseShouldNotHaveSchema + - deleteMethod204Response + - deleteMethodNoRequestBody + - deleteMethod404Response diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 43f9ae370a..1528064482 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -42,6 +42,17 @@ For rule definitions, see [IPA-106.yaml](https://github.com/mongodb/openapi/blob | ------------------------------------------------------------------ | -------------------------------------------------------------------------------- | -------- | | xgen-IPA-106-create-method-request-body-is-request-suffixed-object | The Create method request should be a Request suffixed object. http://go/ipa/106 | warn | +### IPA-108 + +For rule definitions, see [IPA-108.yaml](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/IPA-108.yaml). + +| Rule Name | Description | Severity | +| ---------------------------------------------- | ------------------------------------------------------ | -------- | +| xgen-IPA-108-delete-response-should-be-empty | Delete method response must be empty http://go/ipa/108 | warn | +| xgen-IPA-108-delete-method-return-204-response | DELETE method must return 204 No Content | warn | +| xgen-IPA-108-delete-request-no-body | DELETE method must not have request body | warn | +| xgen-IPA-108-delete-include-404-response | DELETE method must include 404 response | warn | + ### IPA-109 For rule definitions, see [IPA-109.yaml](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/IPA-109.yaml). diff --git a/tools/spectral/ipa/rulesets/functions/deleteMethod204Response.js b/tools/spectral/ipa/rulesets/functions/deleteMethod204Response.js new file mode 100644 index 0000000000..ff1f054155 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/deleteMethod204Response.js @@ -0,0 +1,29 @@ +import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; +import { hasException } from './utils/exceptions.js'; + +const RULE_NAME = 'xgen-IPA-108-delete-method-return-204-response'; +const ERROR_MESSAGE = 'DELETE method should return 204 No Content status code'; + +/** + * Delete method should return 204 No Content status code + * + * @param {object} input - The delete operation object + * @param {object} _ - Unused + * @param {object} context - The context object containing the path + */ +export default (input, _, { path }) => { + const deleteOp = input; + if (!deleteOp) return; + + if (hasException(deleteOp, RULE_NAME)) { + collectException(deleteOp, RULE_NAME, path); + return; + } + + const responses = deleteOp.responses || {}; + if (!responses['204']) { + return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); + } + + collectAdoption(path, RULE_NAME); +}; diff --git a/tools/spectral/ipa/rulesets/functions/deleteMethod404Response.js b/tools/spectral/ipa/rulesets/functions/deleteMethod404Response.js new file mode 100644 index 0000000000..a81e0ff789 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/deleteMethod404Response.js @@ -0,0 +1,29 @@ +import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; +import { hasException } from './utils/exceptions.js'; + +const RULE_NAME = 'xgen-IPA-108-delete-include-404-response'; +const ERROR_MESSAGE = 'DELETE method should include 404 status code for not found resources'; + +/** + * Delete method should include 404 status code for not found resources + * + * @param {object} input - The delete operation object + * @param {object} _ - Unused + * @param {object} context - The context object containing the path + */ +export default (input, _, { path }) => { + const deleteOp = input; + if (!deleteOp) return; + + if (hasException(deleteOp, RULE_NAME)) { + collectException(deleteOp, RULE_NAME, path); + return; + } + + const responses = deleteOp.responses || {}; + if (!responses['404']) { + return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); + } + + collectAdoption(path, RULE_NAME); +}; diff --git a/tools/spectral/ipa/rulesets/functions/deleteMethodNoRequestBody.js b/tools/spectral/ipa/rulesets/functions/deleteMethodNoRequestBody.js new file mode 100644 index 0000000000..59f719e682 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/deleteMethodNoRequestBody.js @@ -0,0 +1,28 @@ +import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; +import { hasException } from './utils/exceptions.js'; + +const RULE_NAME = 'xgen-IPA-108-delete-request-no-body'; +const ERROR_MESSAGE = 'DELETE method should not have a request body'; + +/** + * Delete method should not have a request body + * + * @param {object} input - The delete operation object + * @param {object} _ - Unused + * @param {object} context - The context object containing the path + */ +export default (input, _, { path }) => { + const deleteOp = input; + if (!deleteOp) return; + + if (hasException(deleteOp, RULE_NAME)) { + collectException(deleteOp, RULE_NAME, path); + return; + } + + if (deleteOp.requestBody) { + return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); + } + + collectAdoption(path, RULE_NAME); +}; diff --git a/tools/spectral/ipa/rulesets/functions/deleteMethodResponseShouldNotHaveSchema.js b/tools/spectral/ipa/rulesets/functions/deleteMethodResponseShouldNotHaveSchema.js new file mode 100644 index 0000000000..2024006cb5 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/deleteMethodResponseShouldNotHaveSchema.js @@ -0,0 +1,35 @@ +import { hasException } from './utils/exceptions.js'; +import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; + +const RULE_NAME = 'xgen-IPA-108-delete-response-should-be-empty'; +const ERROR_MESSAGE = + 'DELETE method should return an empty response. The response should not have a schema property and reference to models'; + +/** + * Delete method should return an empty response + * @param {object} input - The delete operation object + * @param {object} _ - Unused + * @param {object} context - The context object containing the path + */ +export default (input, _, { path }) => { + const deleteOp = input; + if (!deleteOp) return; + + if (hasException(deleteOp, RULE_NAME)) { + collectException(deleteOp, RULE_NAME, path); + return; + } + + const responses = deleteOp.responses || {}; + for (const [status, response] of Object.entries(responses)) { + if (status === '204' && response.content) { + for (const contentType of Object.keys(response.content)) { + if (response.content[contentType].schema) { + return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); + } + } + } + } + + collectAdoption(path, RULE_NAME); +}; diff --git a/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js b/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js index 653c632fc6..71c81e822f 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js +++ b/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js @@ -4,15 +4,15 @@ import { EXCEPTION_EXTENSION } from './exceptions.js'; /** * Collects a violation entry and returns formatted error data. * - * @param {Array} path - The JSON path for the object where the rule violation occurred. + * @param {Array} jsonPath - The JSON path for the object where the rule violation occurred. Example: 'paths./pets.get]) * @param {string} ruleName - The name of the rule that was violated. * @param {string|Array} errorData - The error information. Can be either a string message or an array of error objects. * @returns {Array} An array of error objects. Each object has a 'message' property. * @throws {Error} Throws an error if errorData is neither a string nor an array. * */ -export function collectAndReturnViolation(path, ruleName, errorData) { - collector.add(EntryType.VIOLATION, path, ruleName); +export function collectAndReturnViolation(jsonPath, ruleName, errorData) { + collector.add(EntryType.VIOLATION, jsonPath, ruleName); if (typeof errorData === 'string') { return [{ message: errorData }]; @@ -29,8 +29,8 @@ export function collectAndReturnViolation(path, ruleName, errorData) { * @param {Array} path - The JSON path for the object where the rule adoption occurred. * @param {string} ruleName - The name of the rule that was adopted. */ -export function collectAdoption(path, ruleName) { - collector.add(EntryType.ADOPTION, path, ruleName); +export function collectAdoption(jsonPath, ruleName) { + collector.add(EntryType.ADOPTION, jsonPath, ruleName); } /** @@ -40,9 +40,9 @@ export function collectAdoption(path, ruleName) { * @param {Array} path - The JSON path for the object where the rule exception occurred. * @param {string} ruleName - The name of the rule that the exception is defined for. */ -export function collectException(object, ruleName, path) { +export function collectException(object, ruleName, jsonPath) { let exceptionReason = object[EXCEPTION_EXTENSION][ruleName]; if (exceptionReason) { - collector.add(EntryType.EXCEPTION, path, ruleName, exceptionReason); + collector.add(EntryType.EXCEPTION, jsonPath, ruleName, exceptionReason); } } From f61b6c979ed8477a1c74f8083bd47faf42bfe490 Mon Sep 17 00:00:00 2001 From: wtrocki Date: Wed, 5 Mar 2025 23:20:56 +0100 Subject: [PATCH 2/2] fix: autogenerated docs --- tools/spectral/ipa/rulesets/README.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 1528064482..9f66ad60ae 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -46,12 +46,12 @@ For rule definitions, see [IPA-106.yaml](https://github.com/mongodb/openapi/blob For rule definitions, see [IPA-108.yaml](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/IPA-108.yaml). -| Rule Name | Description | Severity | -| ---------------------------------------------- | ------------------------------------------------------ | -------- | -| xgen-IPA-108-delete-response-should-be-empty | Delete method response must be empty http://go/ipa/108 | warn | -| xgen-IPA-108-delete-method-return-204-response | DELETE method must return 204 No Content | warn | -| xgen-IPA-108-delete-request-no-body | DELETE method must not have request body | warn | -| xgen-IPA-108-delete-include-404-response | DELETE method must include 404 response | warn | +| Rule Name | Description | Severity | +| ---------------------------------------------- | ----------------------------------------------------------------------------------- | -------- | +| xgen-IPA-108-delete-response-should-be-empty | Delete method response should not have schema reference to object http://go/ipa/108 | warn | +| xgen-IPA-108-delete-method-return-204-response | DELETE method must return 204 No Content | warn | +| xgen-IPA-108-delete-request-no-body | DELETE method must not have request body | warn | +| xgen-IPA-108-delete-include-404-response | DELETE method must include 404 response and return it when resource not found | warn | ### IPA-109