diff --git a/tools/spectral/ipa/__tests__/getMethodReturnsSingleResource.test.js b/tools/spectral/ipa/__tests__/getMethodReturnsSingleResource.test.js new file mode 100644 index 0000000000..118f3745f5 --- /dev/null +++ b/tools/spectral/ipa/__tests__/getMethodReturnsSingleResource.test.js @@ -0,0 +1,271 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +const componentSchemas = { + schemas: { + Schema: { + properties: { + exampleProperty: { + type: 'string', + }, + }, + }, + PaginatedSchema: { + type: 'array', + }, + ArraySchema: { + properties: { + results: { + type: 'array', + }, + }, + }, + }, +}; + +testRule('xgen-IPA-104-get-method-returns-single-resource', [ + { + name: 'valid resources', + document: { + paths: { + '/resource': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/PaginatedSchema', + }, + }, + }, + }, + }, + }, + }, + '/resource/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/Schema', + }, + }, + }, + }, + }, + }, + }, + '/resource/{id}:getAllThings': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/ArraySchema', + }, + }, + }, + }, + }, + }, + }, + '/singleton': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/Schema', + }, + }, + }, + }, + }, + }, + }, + }, + components: componentSchemas, + }, + errors: [], + }, + { + name: 'invalid resources', + document: { + paths: { + '/arrayResource/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/ArraySchema', + }, + }, + }, + }, + }, + }, + }, + '/paginatedResource/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/PaginatedSchema', + }, + }, + }, + }, + }, + }, + }, + '/arraySingleton': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/ArraySchema', + }, + }, + }, + }, + }, + }, + }, + '/paginatedSingleton': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/PaginatedSchema', + }, + }, + }, + }, + }, + }, + }, + }, + components: componentSchemas, + }, + errors: [ + { + code: 'xgen-IPA-104-get-method-returns-single-resource', + message: + 'Get methods should return data for a single resource. This method returns an array or a paginated response. http://go/ipa/104', + path: [ + 'paths', + '/arrayResource/{id}', + 'get', + 'responses', + '200', + 'content', + 'application/vnd.atlas.2024-08-05+json', + ], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-104-get-method-returns-single-resource', + message: + 'Get methods should return data for a single resource. This method returns an array or a paginated response. http://go/ipa/104', + path: [ + 'paths', + '/paginatedResource/{id}', + 'get', + 'responses', + '200', + 'content', + 'application/vnd.atlas.2024-08-05+json', + ], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-104-get-method-returns-single-resource', + message: + 'Get methods should return data for a single resource. This method returns an array or a paginated response. http://go/ipa/104', + path: [ + 'paths', + '/arraySingleton', + 'get', + 'responses', + '200', + 'content', + 'application/vnd.atlas.2024-08-05+json', + ], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-104-get-method-returns-single-resource', + message: + 'Get methods should return data for a single resource. This method returns an array or a paginated response. http://go/ipa/104', + path: [ + 'paths', + '/paginatedSingleton', + 'get', + 'responses', + '200', + 'content', + 'application/vnd.atlas.2024-08-05+json', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid resources with exceptions', + document: { + paths: { + '/arrayResource/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + 'x-xgen-IPA-exception': { + 'xgen-IPA-104-get-method-returns-single-resource': 'reason', + }, + schema: { + $ref: '#/components/schemas/ArraySchema', + }, + }, + }, + }, + }, + }, + }, + '/paginatedSingleton': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + 'x-xgen-IPA-exception': { + 'xgen-IPA-104-get-method-returns-single-resource': 'reason', + }, + schema: { + $ref: '#/components/schemas/PaginatedSchema', + }, + }, + }, + }, + }, + }, + }, + }, + components: componentSchemas, + }, + errors: [], + }, +]); diff --git a/tools/spectral/ipa/rulesets/IPA-104.yaml b/tools/spectral/ipa/rulesets/IPA-104.yaml index f0ef7eed24..5dc44a952a 100644 --- a/tools/spectral/ipa/rulesets/IPA-104.yaml +++ b/tools/spectral/ipa/rulesets/IPA-104.yaml @@ -3,6 +3,7 @@ functions: - eachResourceHasGetMethod + - getMethodReturnsSingleResource rules: xgen-IPA-104-resource-has-GET: @@ -13,3 +14,10 @@ rules: then: field: '@key' function: 'eachResourceHasGetMethod' + xgen-IPA-104-get-method-returns-single-resource: + description: 'The purpose of the get method is to return data from a single resource. http://go/ipa/104' + message: '{{error}} http://go/ipa/104' + severity: warn + given: '$.paths[*].get' + then: + function: 'getMethodReturnsSingleResource' diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 1698192e74..614b2fa8ef 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -28,9 +28,10 @@ For rule definitions, see [IPA-102.yaml](https://github.com/mongodb/openapi/blob For rule definitions, see [IPA-104.yaml](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/IPA-104.yaml). -| Rule Name | Description | Severity | -| ----------------------------- | --------------------------------------------------------------- | -------- | -| xgen-IPA-104-resource-has-GET | APIs must provide a get method for resources. http://go/ipa/104 | warn | +| Rule Name | Description | Severity | +| ----------------------------------------------- | ----------------------------------------------------------------------------------------- | -------- | +| xgen-IPA-104-resource-has-GET | APIs must provide a get method for resources. http://go/ipa/104 | warn | +| xgen-IPA-104-get-method-returns-single-resource | The purpose of the get method is to return data from a single resource. http://go/ipa/104 | warn | ### IPA-109 diff --git a/tools/spectral/ipa/rulesets/functions/getMethodReturnsSingleResource.js b/tools/spectral/ipa/rulesets/functions/getMethodReturnsSingleResource.js new file mode 100644 index 0000000000..d51e2695fb --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/getMethodReturnsSingleResource.js @@ -0,0 +1,39 @@ +import { isChild, isCustomMethod, isSingletonResource, getResourcePaths } from './utils/resourceEvaluation.js'; +import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; +import { getAllSuccessfulResponseSchemas } from './utils/methodUtils.js'; +import { hasException } from './utils/exceptions.js'; +import { schemaIsArray, schemaIsPaginated } from './utils/schemaUtils.js'; +import { resolveObject } from './utils/componentUtils.js'; + +const RULE_NAME = 'xgen-IPA-104-get-method-returns-single-resource'; +const ERROR_MESSAGE_STANDARD_RESOURCE = + 'Get methods should return data for a single resource. This method returns an array or a paginated response.'; + +export default (input, _, { path, documentInventory }) => { + const oas = documentInventory.resolved; + const resourcePath = path[1]; + const resourcePaths = getResourcePaths(resourcePath, Object.keys(oas.paths)); + + if (isCustomMethod(resourcePath) || (!isChild(resourcePath) && !isSingletonResource(resourcePaths))) { + return; + } + + const errors = []; + + const responseSchemas = getAllSuccessfulResponseSchemas(input); + responseSchemas.forEach(({ schemaPath, schema }) => { + const fullPath = path.concat(schemaPath); + const responseObject = resolveObject(oas, fullPath); + + if (hasException(responseObject, RULE_NAME)) { + collectException(responseObject, RULE_NAME, fullPath); + } else if (schemaIsPaginated(schema) || schemaIsArray(schema)) { + collectAndReturnViolation(fullPath, RULE_NAME, ERROR_MESSAGE_STANDARD_RESOURCE); + errors.push({ path: fullPath, message: ERROR_MESSAGE_STANDARD_RESOURCE }); + } else { + collectAdoption(fullPath, RULE_NAME); + } + }); + + return errors; +}; diff --git a/tools/spectral/ipa/rulesets/functions/singletonHasNoId.js b/tools/spectral/ipa/rulesets/functions/singletonHasNoId.js index d208774fbd..29ef51b297 100644 --- a/tools/spectral/ipa/rulesets/functions/singletonHasNoId.js +++ b/tools/spectral/ipa/rulesets/functions/singletonHasNoId.js @@ -6,7 +6,7 @@ import { isSingletonResource, } from './utils/resourceEvaluation.js'; import { hasException } from './utils/exceptions.js'; -import { getAllSuccessfulGetResponseSchemas } from './utils/methodUtils.js'; +import { getAllSuccessfulResponseSchemas } from './utils/methodUtils.js'; import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; const RULE_NAME = 'xgen-IPA-113-singleton-must-not-have-id'; @@ -28,8 +28,8 @@ export default (input, opts, { path, documentInventory }) => { const resourcePaths = getResourcePaths(resourcePath, Object.keys(oas.paths)); if (isSingletonResource(resourcePaths) && hasGetMethod(input)) { - const resourceSchemas = getAllSuccessfulGetResponseSchemas(input); - if (resourceSchemas.some((schema) => schemaHasIdProperty(schema))) { + const resourceSchemas = getAllSuccessfulResponseSchemas(input['get']); + if (resourceSchemas.some(({ schema }) => schemaHasIdProperty(schema))) { return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); } } diff --git a/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js b/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js index 9cfd621d16..653c632fc6 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js +++ b/tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js @@ -1,9 +1,10 @@ import collector, { EntryType } from '../../../metrics/collector.js'; import { EXCEPTION_EXTENSION } from './exceptions.js'; + /** * Collects a violation entry and returns formatted error data. * - * @param {string} path - The JSON path for the object where the rule violation occurred. + * @param {Array} path - The JSON path for the object where the rule violation occurred. * @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. @@ -25,7 +26,7 @@ export function collectAndReturnViolation(path, ruleName, errorData) { /** * Collects an adoption entry. * - * @param {string} path - The JSON path for the object where the rule adoption occurred. + * @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) { @@ -36,7 +37,7 @@ export function collectAdoption(path, ruleName) { * Collects an exception entry. * * @param object the object to evaluate - * @param {string} path - The JSON path for the object where the rule exception occurred. + * @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) { diff --git a/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js b/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js index c13315e882..8781360966 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js +++ b/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js @@ -1,18 +1,30 @@ /** - * Returns a list of all successful response schemas for the 'get' method of the passed resource, i.e. for any 2xx response. + * Returns a list of all successful response schemas for the passed operation, i.e. for any 2xx response. * - * @param {object} pathObject the object for the path - * @returns {Object[]} all 2xx 'get' response schemas + * @param {object} operationObject the object for the operation + * @returns {Object[{schemaPath: Array, schema: Object}]} all 2xx response schemas and the path to each schema */ -export function getAllSuccessfulGetResponseSchemas(pathObject) { - const responses = pathObject['get']['responses']; +export function getAllSuccessfulResponseSchemas(operationObject) { + const path = []; + + const responses = operationObject['responses']; + path.push('responses'); + const successfulResponseKey = Object.keys(responses).filter((k) => k.startsWith('2'))[0]; + path.push(successfulResponseKey); + const responseContent = responses[successfulResponseKey]['content']; + path.push('content'); + const result = []; Object.keys(responseContent).forEach((k) => { const schema = responseContent[k]['schema']; + const schemaPath = path.concat([k]); if (schema) { - result.push(schema); + result.push({ + schemaPath, + schema, + }); } }); return result; diff --git a/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js b/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js new file mode 100644 index 0000000000..42035daa56 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js @@ -0,0 +1,9 @@ +export function schemaIsPaginated(schema) { + const fields = Object.keys(schema); + return fields.includes('properties') && Object.keys(schema['properties']).includes('results'); +} + +export function schemaIsArray(schema) { + const fields = Object.keys(schema); + return fields.includes('type') && schema['type'] === 'array'; +}