diff --git a/tools/spectral/ipa/__tests__/IPA132GetMethodMustNotBeLro.test.js b/tools/spectral/ipa/__tests__/IPA132GetMethodMustNotBeLro.test.js new file mode 100644 index 0000000000..b93f04c60a --- /dev/null +++ b/tools/spectral/ipa/__tests__/IPA132GetMethodMustNotBeLro.test.js @@ -0,0 +1,162 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +const ERROR_MESSAGE = + 'The Get method must not be a long-running operation. A Get returns the current state of a resource that already exists on the server.'; + +const syncGet = { + responses: { + 200: {}, + }, +}; + +const lroGet = { + responses: { + 202: { + description: 'Accepted', + headers: { + Location: { + schema: { type: 'string', format: 'uri' }, + }, + }, + }, + }, +}; + +testRule('xgen-IPA-132-get-method-must-not-be-lro', [ + { + name: 'valid synchronous Get methods', + document: { + paths: { + '/api/atlas/v2/resourceName/{pathParam}': { + get: syncGet, + }, + // Singleton resource + '/api/atlas/v2/resourceName/{pathParam}/settings': { + get: syncGet, + }, + }, + }, + errors: [], + }, + { + name: 'methods that are not Get methods are ignored', + document: { + paths: { + // List method, covered by xgen-IPA-132-list-method-must-not-be-lro + '/api/atlas/v2/resourceName': { + get: lroGet, + }, + // Custom method + '/api/atlas/v2/resourceName/{pathParam}:customMethod': { + get: lroGet, + }, + }, + }, + errors: [], + }, + { + name: 'invalid Get method declaring a 202 response', + document: { + paths: { + '/api/atlas/v2/resourceName/{pathParam}': { + get: lroGet, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-get-method-must-not-be-lro', + message: ERROR_MESSAGE, + path: ['paths', '/api/atlas/v2/resourceName/{pathParam}', 'get', 'responses', '202'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid singleton Get method declaring a 202 response', + document: { + paths: { + '/api/atlas/v2/resourceName/{pathParam}/settings': { + get: lroGet, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-get-method-must-not-be-lro', + message: ERROR_MESSAGE, + path: ['paths', '/api/atlas/v2/resourceName/{pathParam}/settings', 'get', 'responses', '202'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid Get method declaring a 202 response alongside a 200 response', + document: { + paths: { + '/api/atlas/v2/resourceName/{pathParam}': { + get: { + responses: { + 200: {}, + 202: {}, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-get-method-must-not-be-lro', + message: ERROR_MESSAGE, + path: ['paths', '/api/atlas/v2/resourceName/{pathParam}', 'get', 'responses', '202'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid Get methods with exceptions', + document: { + paths: { + '/api/atlas/v2/resourceName/{pathParam}': { + get: { + ...lroGet, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-get-method-must-not-be-lro': 'reason', + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'compliant Get method does not need an exception', + document: { + paths: { + '/api/atlas/v2/resourceName/{pathParam}': { + get: { + ...syncGet, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-get-method-must-not-be-lro': 'reason', + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-get-method-must-not-be-lro', + message: 'This component adopts the rule and does not need an exception. Please remove the exception.', + path: [ + 'paths', + '/api/atlas/v2/resourceName/{pathParam}', + 'get', + 'x-xgen-IPA-exception', + 'xgen-IPA-132-get-method-must-not-be-lro', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, +]); diff --git a/tools/spectral/ipa/__tests__/IPA132ListMethodMustNotBeLro.test.js b/tools/spectral/ipa/__tests__/IPA132ListMethodMustNotBeLro.test.js new file mode 100644 index 0000000000..d4b37a6b9a --- /dev/null +++ b/tools/spectral/ipa/__tests__/IPA132ListMethodMustNotBeLro.test.js @@ -0,0 +1,144 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +const ERROR_MESSAGE = + 'The List method must not be a long-running operation. A List returns data from a collection that already exists on the server.'; + +const syncList = { + responses: { + 200: {}, + }, +}; + +const lroList = { + responses: { + 202: { + description: 'Accepted', + headers: { + Location: { + schema: { type: 'string', format: 'uri' }, + }, + }, + }, + }, +}; + +testRule('xgen-IPA-132-list-method-must-not-be-lro', [ + { + name: 'valid synchronous List methods', + document: { + paths: { + '/api/atlas/v2/resourceName': { + get: syncList, + }, + }, + }, + errors: [], + }, + { + name: 'methods that are not List methods are ignored', + document: { + paths: { + // Get method, covered by xgen-IPA-132-get-method-must-not-be-lro + '/api/atlas/v2/resourceName/{pathParam}': { + get: lroList, + }, + // Singleton resource Get method + '/api/atlas/v2/resourceName/{pathParam}/settings': { + get: lroList, + }, + // Custom method + '/api/atlas/v2/resourceName:customMethod': { + get: lroList, + }, + }, + }, + errors: [], + }, + { + name: 'invalid List method declaring a 202 response', + document: { + paths: { + '/api/atlas/v2/resourceName': { + get: lroList, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-list-method-must-not-be-lro', + message: ERROR_MESSAGE, + path: ['paths', '/api/atlas/v2/resourceName', 'get', 'responses', '202'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid List method declaring a 202 response alongside a 200 response', + document: { + paths: { + '/api/atlas/v2/resourceName': { + get: { + responses: { + 200: {}, + 202: {}, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-list-method-must-not-be-lro', + message: ERROR_MESSAGE, + path: ['paths', '/api/atlas/v2/resourceName', 'get', 'responses', '202'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid List methods with exceptions', + document: { + paths: { + '/api/atlas/v2/resourceName': { + get: { + ...lroList, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-list-method-must-not-be-lro': 'reason', + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'compliant List method does not need an exception', + document: { + paths: { + '/api/atlas/v2/resourceName': { + get: { + ...syncList, + 'x-xgen-IPA-exception': { + 'xgen-IPA-132-list-method-must-not-be-lro': 'reason', + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-132-list-method-must-not-be-lro', + message: 'This component adopts the rule and does not need an exception. Please remove the exception.', + path: [ + 'paths', + '/api/atlas/v2/resourceName', + 'get', + 'x-xgen-IPA-exception', + 'xgen-IPA-132-list-method-must-not-be-lro', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, +]); diff --git a/tools/spectral/ipa/rulesets/IPA-132.yaml b/tools/spectral/ipa/rulesets/IPA-132.yaml index a2b4c13503..fa9bcc505e 100644 --- a/tools/spectral/ipa/rulesets/IPA-132.yaml +++ b/tools/spectral/ipa/rulesets/IPA-132.yaml @@ -12,10 +12,14 @@ functions: - IPA132OperationResponseMustUseStandardEnumValues - IPA132OperationResponseMustDefineNestedObjectStructure - IPA132OperationsShouldExposeStatusFields + - IPA132GetMethodMustNotBeLro + - IPA132ListMethodMustNotBeLro aliases: OperationResponseSchema: - '$.components.schemas.OperationResponse' + GetOperationObject: + - '$.paths[*].get' rules: xgen-IPA-132-operations-endpoint-must-not-be-global: @@ -283,3 +287,41 @@ rules: objects: - field: progress properties: [completed, total, unit] + + xgen-IPA-132-get-method-must-not-be-lro: + description: | + A Get method must not be a long-running operation. A Get returns the current state of a + single resource that already resides on the server. + + ##### Implementation details + Rule checks for the following conditions: + + - Applies to GET methods on single resources and singleton resources + - The Get method must not declare a 202 response; a Get reads state that already exists on + the server, so there is no background work to track + - Operations with `x-xgen-IPA-exception` for this rule are excluded from validation + + message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-get-method-must-not-be-lro' + severity: warn + given: '#GetOperationObject' + then: + function: IPA132GetMethodMustNotBeLro + + xgen-IPA-132-list-method-must-not-be-lro: + description: | + A List method must not be a long-running operation. A List returns data from a collection + that already exists on the server. + + ##### Implementation details + Rule checks for the following conditions: + + - Applies to GET methods on resource collections + - The List method must not declare a 202 response; a List returns existing collection + data, so the response is returned within the request cycle + - Operations with `x-xgen-IPA-exception` for this rule are excluded from validation + + message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-132-list-method-must-not-be-lro' + severity: warn + given: '#GetOperationObject' + then: + function: IPA132ListMethodMustNotBeLro diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 7657503932..030a1c9be8 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -1586,6 +1586,34 @@ Rule checks for the following conditions: rule - Schemas with `x-xgen-IPA-exception` for this rule are excluded from validation +#### xgen-IPA-132-get-method-must-not-be-lro + + ![warn](https://img.shields.io/badge/warning-yellow) +A Get method must not be a long-running operation. A Get returns the current state of a +single resource that already resides on the server. + +##### Implementation details +Rule checks for the following conditions: + + - Applies to GET methods on single resources and singleton resources + - The Get method must not declare a 202 response; a Get reads state that already exists on + the server, so there is no background work to track + - Operations with `x-xgen-IPA-exception` for this rule are excluded from validation + +#### xgen-IPA-132-list-method-must-not-be-lro + + ![warn](https://img.shields.io/badge/warning-yellow) +A List method must not be a long-running operation. A List returns data from a collection +that already exists on the server. + +##### Implementation details +Rule checks for the following conditions: + + - Applies to GET methods on resource collections + - The List method must not declare a 202 response; a List returns existing collection + data, so the response is returned within the request cycle + - Operations with `x-xgen-IPA-exception` for this rule are excluded from validation + diff --git a/tools/spectral/ipa/rulesets/functions/IPA132GetMethodMustNotBeLro.js b/tools/spectral/ipa/rulesets/functions/IPA132GetMethodMustNotBeLro.js new file mode 100644 index 0000000000..8418cdeb86 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/IPA132GetMethodMustNotBeLro.js @@ -0,0 +1,47 @@ +import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; +import { + getResourcePathItems, + isResourceCollectionIdentifier, + isSingleResourceIdentifier, + isSingletonResource, +} from './utils/resourceEvaluation.js'; + +const ERROR_MESSAGE = + 'The Get method must not be a long-running operation. A Get returns the current state of a resource that already exists on the server.'; + +/** + * Checks that a Get method is not modeled as a long-running operation defined by IPA-132, i.e. it + * does not declare a 202 response. + * + * @param {object} input - The GET operation object + * @param {object} _ - Unused + * @param {object} context - The context object containing the path, documentInventory and rule + */ +export default (input, _, { path, documentInventory, rule }) => { + const ruleName = rule.name; + const resourcePath = path[1]; + const oas = documentInventory.resolved; + const resourcePaths = getResourcePathItems(resourcePath, oas.paths); + + if ( + !isSingleResourceIdentifier(resourcePath) && + !(isResourceCollectionIdentifier(resourcePath) && isSingletonResource(resourcePaths)) + ) { + return; + } + + const errors = checkViolationsAndReturnErrors(input, path, ruleName); + return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); +}; + +function checkViolationsAndReturnErrors(operation, path, ruleName) { + try { + const responses = operation.responses; + if (responses && Object.keys(responses).includes('202')) { + return [{ path: [...path, 'responses', '202'], message: ERROR_MESSAGE }]; + } + return []; + } catch (e) { + return handleInternalError(ruleName, path, e); + } +} diff --git a/tools/spectral/ipa/rulesets/functions/IPA132ListMethodMustNotBeLro.js b/tools/spectral/ipa/rulesets/functions/IPA132ListMethodMustNotBeLro.js new file mode 100644 index 0000000000..24cbcdcc73 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/IPA132ListMethodMustNotBeLro.js @@ -0,0 +1,45 @@ +import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js'; +import { + getResourcePathItems, + isResourceCollectionIdentifier, + isSingletonResource, +} from './utils/resourceEvaluation.js'; + +const ERROR_MESSAGE = + 'The List method must not be a long-running operation. A List returns data from a collection that already exists on the server.'; + +/** + * Checks that a List method is not modeled as a long-running operation defined by IPA-132, i.e. it + * does not declare a 202 response. + * + * @param {object} input - The GET operation object + * @param {object} _ - Unused + * @param {object} context - The context object containing the path, documentInventory and rule + */ +export default (input, _, { path, documentInventory, rule }) => { + const ruleName = rule.name; + const resourcePath = path[1]; + const oas = documentInventory.resolved; + + if ( + !isResourceCollectionIdentifier(resourcePath) || + isSingletonResource(getResourcePathItems(resourcePath, oas.paths)) + ) { + return; + } + + const errors = checkViolationsAndReturnErrors(input, path, ruleName); + return evaluateAndCollectAdoptionStatus(errors, ruleName, input, path); +}; + +function checkViolationsAndReturnErrors(operation, path, ruleName) { + try { + const responses = operation.responses; + if (responses && Object.keys(responses).includes('202')) { + return [{ path: [...path, 'responses', '202'], message: ERROR_MESSAGE }]; + } + return []; + } catch (e) { + return handleInternalError(ruleName, path, e); + } +}