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..9f66ad60ae 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 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 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); } }