From 49653eff14d6aaf103a54e152eecfa345704a0cb Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Thu, 13 Mar 2025 12:09:07 +0000 Subject: [PATCH 01/12] CLOUDP-304938: Adds rule xgen-IPA-105-list-method-response-is-get-method-response --- ...tMethodResponseIsGetMethodResponse.test.js | 497 ++++++++++++++++++ tools/spectral/ipa/rulesets/IPA-105.yaml | 16 + tools/spectral/ipa/rulesets/README.md | 11 +- .../createMethodRequestBodyIsGetResponse.js | 3 +- .../listMethodResponseIsGetMethodResponse.js | 89 ++++ .../rulesets/functions/utils/methodUtils.js | 109 +++- .../functions/utils/resourceEvaluation.js | 48 -- 7 files changed, 718 insertions(+), 55 deletions(-) create mode 100644 tools/spectral/ipa/__tests__/listMethodResponseIsGetMethodResponse.test.js create mode 100644 tools/spectral/ipa/rulesets/functions/listMethodResponseIsGetMethodResponse.js diff --git a/tools/spectral/ipa/__tests__/listMethodResponseIsGetMethodResponse.test.js b/tools/spectral/ipa/__tests__/listMethodResponseIsGetMethodResponse.test.js new file mode 100644 index 0000000000..b124c6a571 --- /dev/null +++ b/tools/spectral/ipa/__tests__/listMethodResponseIsGetMethodResponse.test.js @@ -0,0 +1,497 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +const componentSchemas = { + schemas: { + PaginatedResourceSchema: { + properties: { + results: { + type: 'array', + items: { + $ref: '#/components/schemas/ResourceSchema', + }, + }, + }, + }, + ResourceSchema: { + type: 'object', + }, + PaginatedArraySchema: { + properties: { + results: { + type: 'array', + items: { + $ref: '#/components/schemas/ArraySchema', + }, + }, + }, + }, + ArraySchema: { + type: 'array', + items: { + type: 'string', + }, + }, + }, +}; + +testRule('xgen-IPA-105-list-method-response-is-get-method-response', [ + { + name: 'valid list', + document: { + paths: { + // Using ref + '/resource': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/PaginatedResourceSchema', + }, + }, + }, + }, + }, + }, + }, + '/resource/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/ResourceSchema', + }, + }, + }, + }, + }, + }, + }, + // Inline schema + '/arrayResource': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + properties: { + results: { + type: 'array', + items: { + $ref: '#/components/schemas/ArraySchema', + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + '/arrayResource/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/ArraySchema', + }, + }, + }, + }, + }, + }, + }, + // Multiple versions + '/versionedResource': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/PaginatedResourceSchema', + }, + }, + 'application/vnd.atlas.2024-01-05+json': { + schema: { + $ref: '#/components/schemas/PaginatedArraySchema', + }, + }, + }, + }, + }, + }, + }, + '/versionedResource/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/ResourceSchema', + }, + }, + 'application/vnd.atlas.2024-01-05+json': { + schema: { + $ref: '#/components/schemas/ArraySchema', + }, + }, + }, + }, + }, + }, + }, + }, + components: componentSchemas, + }, + errors: [], + }, + { + name: 'rule ignores inapplicable cases', + document: { + paths: { + // No Get method + '/resource': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/ArraySchema', + }, + }, + }, + }, + }, + }, + }, + // Singleton + '/resource/{id}/singleton': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/ArraySchema', + }, + }, + }, + }, + }, + }, + }, + // Not paginated (covered by separate rule, IPA 110) + '/resource/{id}/unPaginated': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/ArraySchema', + }, + }, + }, + }, + }, + }, + }, + '/resource/{id}/unPaginated/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/ResourceSchema', + }, + }, + }, + }, + }, + }, + }, + // Version mismatch + '/versionedResource': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-01-05+json': { + schema: { + $ref: '#/components/schemas/PaginatedResourceSchema', + }, + }, + }, + }, + }, + }, + }, + '/versionedResource/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/ArraySchema', + }, + }, + }, + }, + }, + }, + }, + }, + components: componentSchemas, + }, + errors: [], + }, + { + name: 'invalid list', + document: { + paths: { + // Using ref + '/resource': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/PaginatedArraySchema', + }, + }, + }, + }, + }, + }, + }, + '/resource/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/ResourceSchema', + }, + }, + }, + }, + }, + }, + }, + // Inline schema + '/arrayResource': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + properties: { + results: { + type: 'array', + items: { + schema: { + $ref: '#/components/schemas/ResourceSchema', + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + '/arrayResource/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/ArraySchema', + }, + }, + }, + }, + }, + }, + }, + }, + components: componentSchemas, + }, + errors: [ + { + code: 'xgen-IPA-105-list-method-response-is-get-method-response', + message: + 'The schema of each result in the List method response must be the same schema as the response of the Get method. http://go/ipa/105', + path: ['paths', '/resource', 'get', 'responses', '200', 'content', 'application/vnd.atlas.2024-08-05+json'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-105-list-method-response-is-get-method-response', + message: + 'The schema of each result in the List method response must be the same schema as the response of the Get method. http://go/ipa/105', + path: [ + 'paths', + '/arrayResource', + 'get', + 'responses', + '200', + 'content', + 'application/vnd.atlas.2024-08-05+json', + ], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid list with version mismatch', + document: { + paths: { + '/resource': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/PaginatedResourceSchema', + }, + }, + }, + }, + }, + }, + }, + '/resource/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-01-05+json': { + schema: { + $ref: '#/components/schemas/ArraySchema', + }, + }, + }, + }, + }, + }, + }, + }, + components: componentSchemas, + }, + errors: [ + { + code: 'xgen-IPA-105-list-method-response-is-get-method-response', + message: + 'The schema of each result in the List method response must be the same schema as the response of the Get method. http://go/ipa/105', + path: ['paths', '/resource', 'get', 'responses', '200', 'content', 'application/vnd.atlas.2024-08-05+json'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid with exception', + document: { + paths: { + // Using ref + '/resource': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + 'x-xgen-IPA-exception': { + 'xgen-IPA-105-list-method-response-is-get-method-response': 'reason', + }, + schema: { + $ref: '#/components/schemas/PaginatedArraySchema', + }, + }, + }, + }, + }, + }, + }, + '/resource/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/ResourceSchema', + }, + }, + }, + }, + }, + }, + }, + // Inline schema + '/arrayResource': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + 'x-xgen-IPA-exception': { + 'xgen-IPA-105-list-method-response-is-get-method-response': 'reason', + }, + schema: { + properties: { + results: { + type: 'array', + items: { + schema: { + $ref: '#/components/schemas/ResourceSchema', + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + '/arrayResource/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/ArraySchema', + }, + }, + }, + }, + }, + }, + }, + }, + components: componentSchemas, + }, + errors: [], + }, +]); diff --git a/tools/spectral/ipa/rulesets/IPA-105.yaml b/tools/spectral/ipa/rulesets/IPA-105.yaml index aed20f79ab..92e4cb09fe 100644 --- a/tools/spectral/ipa/rulesets/IPA-105.yaml +++ b/tools/spectral/ipa/rulesets/IPA-105.yaml @@ -5,6 +5,7 @@ functions: - listResponseCodeShouldBe200OK - listMethodHasNoRequestBody - eachResourceHasListMethod + - listMethodResponseIsGetMethodResponse rules: xgen-IPA-105-list-method-response-code-is-200: @@ -29,3 +30,18 @@ rules: then: field: '@key' function: 'eachResourceHasListMethod' + xgen-IPA-105-list-method-response-is-get-method-response: + description: >- + The response body of the List method should consist of the same resource object returned by the Get method. Validation checks that the List method results items reference the same schema as the Get method response.
+ - Validation applies to List methods for resource collections only
+ - Validation applies to json response content only
+ - Validation ignores responses without schema and non-paginated responses
+ - Paths with `x-xgen-IPA-exception` for this rule are excluded from validation
+ http://go/ipa/105 + message: '{{error}} http://go/ipa/105' + severity: warn + given: '$.paths[*].get.responses[*].content' + then: + field: '@key' + function: 'listMethodResponseIsGetMethodResponse' + diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index ab7314dc90..b30d6c8dd5 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -42,11 +42,12 @@ For rule definitions, see [IPA-104.yaml](https://github.com/mongodb/openapi/blob For rule definitions, see [IPA-105.yaml](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/IPA-105.yaml). -| Rule Name | Description | Severity | -| --------------------------------------------- | ------------------------------------------------------------------ | -------- | -| xgen-IPA-105-list-method-response-code-is-200 | The List method must return a 200 OK response. http://go/ipa/105 | warn | -| xgen-IPA-105-list-method-no-request-body | The List method request must not include a body. http://go/ipa/105 | warn | -| xgen-IPA-105-resource-has-list | APIs must provide a List method for resources. http://go/ipa/105 | warn | +| Rule Name | Description | Severity | +| -------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------- | +| xgen-IPA-105-list-method-response-code-is-200 | The List method must return a 200 OK response. http://go/ipa/105 | warn | +| xgen-IPA-105-list-method-no-request-body | The List method request must not include a body. http://go/ipa/105 | warn | +| xgen-IPA-105-resource-has-list | APIs must provide a List method for resources. http://go/ipa/105 | warn | +| xgen-IPA-105-list-method-response-is-get-method-response | The response body of the List method should consist of the same resource object returned by the Get method. Validation checks that the List method results items reference the same schema as the Get method response.
- Validation applies to List methods for resource collections only
- Validation applies to json response content only
- Validation ignores responses without schema and non-paginated responses
- Paths with `x-xgen-IPA-exception` for this rule are excluded from validation
http://go/ipa/105 | warn | ### IPA-106 diff --git a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js index 3ade0cfdb5..ecdefd64ab 100644 --- a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js +++ b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js @@ -1,9 +1,10 @@ -import { getResponseOfGetMethodByMediaType, isCustomMethodIdentifier } from './utils/resourceEvaluation.js'; +import { isCustomMethodIdentifier } from './utils/resourceEvaluation.js'; import { resolveObject } from './utils/componentUtils.js'; import { isEqual } from 'lodash'; import omitDeep from 'omit-deep-lodash'; import { hasException } from './utils/exceptions.js'; import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; +import { getResponseOfGetMethodByMediaType } from './utils/methodUtils.js'; const RULE_NAME = 'xgen-IPA-106-create-method-request-body-is-get-method-response'; const ERROR_MESSAGE = diff --git a/tools/spectral/ipa/rulesets/functions/listMethodResponseIsGetMethodResponse.js b/tools/spectral/ipa/rulesets/functions/listMethodResponseIsGetMethodResponse.js new file mode 100644 index 0000000000..f0b77afa40 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/listMethodResponseIsGetMethodResponse.js @@ -0,0 +1,89 @@ +import { + getResourcePathItems, + isResourceCollectionIdentifier, + isSingletonResource, +} from './utils/resourceEvaluation.js'; +import { resolveObject } from './utils/componentUtils.js'; +import { hasException } from './utils/exceptions.js'; +import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; +import { + getResponseOfListMethodByMediaType, + getResponseOfGetMethodByMediaType, + getSchemaRef, + getSchemaNameFromRef, +} from './utils/methodUtils.js'; +import { schemaIsPaginated } from './utils/schemaUtils.js'; + +const RULE_NAME = 'xgen-IPA-105-list-method-response-is-get-method-response'; +const ERROR_MESSAGE = + 'The schema of each result in the List method response must be the same schema as the response of the Get method.'; + +export default (input, _, { path, documentInventory }) => { + const oas = documentInventory.unresolved; + const resourcePath = path[1]; + const responseCode = path[4]; + const mediaType = input; + + if ( + responseCode !== '200' || + !mediaType.endsWith('json') || + !isResourceCollectionIdentifier(resourcePath) || + (isResourceCollectionIdentifier(resourcePath) && isSingletonResource(getResourcePathItems(resourcePath, oas.paths))) + ) { + return; + } + + // Ignore if the List method does not have a response schema + const listMethodResponse = getResponseOfListMethodByMediaType(mediaType, resourcePath, oas); + if (!listMethodResponse || !listMethodResponse.schema) { + return; + } + + // Get list response schema from ref or inline schema + let resolvedListSchema; + const listSchemaRef = getSchemaRef(listMethodResponse.schema); + if (!listSchemaRef) { + resolvedListSchema = listMethodResponse.schema; + } else { + const listSchemaName = getSchemaNameFromRef(listSchemaRef); + resolvedListSchema = resolveObject(oas, ['components', 'schemas', listSchemaName]); + } + + // Ignore if the List method response is not found or not paginated + if (!resolvedListSchema || !schemaIsPaginated(resolvedListSchema)) { + return; + } + + // Ignore if there is no matching Get method response, or if it does not have a schema + const getMethodRequestContentPerMediaType = getResponseOfGetMethodByMediaType(mediaType, resourcePath, oas); + if (!getMethodRequestContentPerMediaType || !getMethodRequestContentPerMediaType.schema) { + return; + } + + if (hasException(listMethodResponse, RULE_NAME)) { + collectException(listMethodResponse, RULE_NAME, path); + return; + } + + const errors = checkViolationsAndReturnErrors( + path, + resolvedListSchema.properties.results.items, + getMethodRequestContentPerMediaType.schema + ); + + if (errors.length !== 0) { + return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); + } + + collectAdoption(path, RULE_NAME); +}; + +function checkViolationsAndReturnErrors(path, listMethodResultItemsSchema, getMethodSchema) { + const listMethodSchemaRef = getSchemaRef(listMethodResultItemsSchema); + const getMethodSchemaRef = getSchemaRef(getMethodSchema); + + if (getMethodSchemaRef !== listMethodSchemaRef) { + return [{ path, message: ERROR_MESSAGE }]; + } + return []; +} diff --git a/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js b/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js index 0f97df3f52..ff3af91ff8 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js +++ b/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js @@ -1,3 +1,6 @@ +import { getResourcePathItems, hasGetMethod, isSingleResourceIdentifier } from './resourceEvaluation.js'; +import { schemaIsArray } from './schemaUtils.js'; + /** * Returns a list of all successful response schemas for the passed operation, i.e. for any 2xx response. * @@ -37,8 +40,112 @@ export function getAllSuccessfulResponseSchemas(operationObject) { * @returns {string} the schema ref */ export function getSchemaRef(schema) { - if (schema.type === 'array' && schema.items) { + if (schemaIsArray(schema) && schema.items) { return schema.items.$ref; } return schema.$ref; } + +/** + * Gets the schema name from a schema reference, for example '#/components/schemas/ResourceSchema' + * returns 'ResourceSchema'. + * + * @param {string} schemaRef the schema reference + * @returns {string} the schema name + */ +export function getSchemaNameFromRef(schemaRef) { + const sections = schemaRef.split('/'); + return sections[sections.length - 1]; +} + +/** + * Retrieves the response schema of the Get method for a resource by media type. + * If the OAS is unresolved, the returning schema may contain a reference to a schema definition. + * + * This function: + * 1. Finds all path items related to the resource collection + * 2. Identifies the single resource path (e.g., '/resource/{id}') + * 3. Checks if the single resource has a GET method + * 4. Returns the response schema for the specified media type if available + * + * @param {string} mediaType - The media type to retrieve the schema for (e.g., 'application/vnd.atlas.2023-01-01+json') + * @param {string} pathForResourceCollection - The path for the collection of resources (e.g., '/resource') + * @param {Object} oas - The resolved or unresolved OpenAPI document + * @returns {Object|null} - The response schema for the specified media type, or null if not found + */ +export function getResponseOfGetMethodByMediaType(mediaType, pathForResourceCollection, oas) { + const resourcePathItems = getResourcePathItems(pathForResourceCollection, oas.paths); + const resourcePaths = Object.keys(resourcePathItems); + if (resourcePaths.length === 1) { + return null; + } + + const singleResourcePath = resourcePaths.find((path) => isSingleResourceIdentifier(path)); + if (!singleResourcePath) { + return null; + } + + const singleResourcePathObject = resourcePathItems[singleResourcePath]; + + return getGETMethodResponseSchemaFromPathItem(singleResourcePathObject, mediaType); +} + +/** + * Retrieves the 200 response schema of the List method for a resource by media type. + * If the OAS is unresolved, the returning schema may contain a reference to a schema definition. + * + * @param {string} mediaType - The media type to retrieve the schema for (e.g., 'application/vnd.atlas.2023-01-01+json') + * @param {string} pathForResourceCollection - The path for the collection of resources (e.g., '/resource') + * @param {Object} oas - The resolved or unresolved OpenAPI document + * @returns {Object|null} - The response schema for the specified media type, or null if not found + */ +export function getResponseOfListMethodByMediaType(mediaType, pathForResourceCollection, oas) { + const pathObject = oas.paths[pathForResourceCollection]; + if (!pathObject) { + return null; + } + + return getGETMethodResponseSchemaFromPathItem(pathObject, mediaType); +} + +/** + * Returns the schema for the 200 response of the GET method for a path item by media type. + * If there is no response with the exact media type version, the latest version before the passed one is returned. + * + * @param {Object} pathItem The path item to extract the GET response from + * @param {string} mediaType The media type + * @returns {Object|null} The schema object, or null if not found + */ +function getGETMethodResponseSchemaFromPathItem(pathItem, mediaType) { + if (!hasGetMethod(pathItem)) { + return null; + } + + const getMethodObject = pathItem.get; + if (!getMethodObject.responses) { + return null; + } + + const response = getMethodObject.responses['200']; + if (!response || !response.content) { + return null; + } + + const versions = Object.keys(response.content); + if (versions.includes(mediaType)) { + const schema = response.content[mediaType]; + if (!schema) { + return null; + } + return schema; + } + const latestVersion = versions.sort().reverse()[0]; + if (latestVersion < mediaType) { + const schema = response.content[latestVersion]; + if (!schema) { + return null; + } + return schema; + } + return null; +} diff --git a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js index c2abacf73e..e231693c65 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js +++ b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js @@ -150,51 +150,3 @@ function removePrefix(path) { } return path; } - -/** - * Retrieves the response schema of the GET method for a resource by media type - * - * This function: - * 1. Finds all path items related to the resource collection - * 2. Identifies the single resource path (e.g., '/resource/{id}') - * 3. Checks if the single resource has a GET method - * 4. Returns the response schema for the specified media type if available - * - * @param {string} mediaType - The media type to retrieve the schema for (e.g., 'application/vnd.atlas.2023-01-01+json') - * @param {string} pathForResourceCollection - The path for the collection of resources (e.g., '/resource') - * @param {Object} oas - The resolved OpenAPI document - * @returns {Object|null} - The response schema for the specified media type, or null if not found - */ -export function getResponseOfGetMethodByMediaType(mediaType, pathForResourceCollection, oas) { - const resourcePathItems = getResourcePathItems(pathForResourceCollection, oas.paths); - const resourcePaths = Object.keys(resourcePathItems); - if (resourcePaths.length === 1) { - return null; - } - - const singleResourcePath = resourcePaths.find((path) => isSingleResourceIdentifier(path)); - if (!singleResourcePath) { - return null; - } - - const singleResourcePathObject = resourcePathItems[singleResourcePath]; - if (!hasGetMethod(singleResourcePathObject)) { - return null; - } - - const getMethodObject = singleResourcePathObject.get; - if (!getMethodObject.responses) { - return null; - } - - const response = getMethodObject.responses['200']; - if (!response) { - return null; - } - - const schema = response.content[mediaType]; - if (!schema) { - return null; - } - return schema; -} From 33da4b172befaf643b49dfd79cc1f682b505123b Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Thu, 13 Mar 2025 16:38:11 +0000 Subject: [PATCH 02/12] CLOUDP-304938: Fixes --- .../ipa/__tests__/__helpers__/testRule.js | 3 + ...eateMethodRequestBodyIsGetResponse.test.js | 34 ++++ ...tMethodResponseIsGetMethodResponse.test.js | 34 ++++ .../ipa/__tests__/utils/methodUtils.test.js | 185 ++++++++++++++++++ tools/spectral/ipa/rulesets/IPA-105.yaml | 1 + tools/spectral/ipa/rulesets/README.md | 12 +- .../createMethodRequestBodyIsGetResponse.js | 2 +- .../listMethodResponseIsGetMethodResponse.js | 56 ++++-- .../rulesets/functions/utils/methodUtils.js | 14 +- 9 files changed, 311 insertions(+), 30 deletions(-) create mode 100644 tools/spectral/ipa/__tests__/utils/methodUtils.test.js diff --git a/tools/spectral/ipa/__tests__/__helpers__/testRule.js b/tools/spectral/ipa/__tests__/__helpers__/testRule.js index 622a15b9db..18b1978f4b 100644 --- a/tools/spectral/ipa/__tests__/__helpers__/testRule.js +++ b/tools/spectral/ipa/__tests__/__helpers__/testRule.js @@ -13,6 +13,9 @@ export default (ruleName, tests) => { const doc = testCase.document instanceof Document ? testCase.document : JSON.stringify(testCase.document); const errors = await s.run(doc); + if (testCase.name === 'invalid methods' && errors.length !== testCase.errors.length) { + console.log('Errors:', errors); + } expect(errors.length).toEqual(testCase.errors.length); errors.forEach((error, index) => { diff --git a/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js b/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js index 10927b9265..257eec5ade 100644 --- a/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js +++ b/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js @@ -361,6 +361,13 @@ testRule('xgen-IPA-106-create-method-request-body-is-get-method-response', [ path: ['paths', '/resource', 'post', 'requestBody', 'content', 'application/vnd.atlas.2023-01-01+json'], severity: DiagnosticSeverity.Warning, }, + { + code: 'xgen-IPA-106-create-method-request-body-is-get-method-response', + message: + 'The request body schema properties must match the response body schema properties of the Get method. http://go/ipa/106', + path: ['paths', '/resource', 'post', 'requestBody', 'content', 'application/vnd.atlas.2024-01-01+json'], + severity: DiagnosticSeverity.Warning, + }, { code: 'xgen-IPA-106-create-method-request-body-is-get-method-response', message: @@ -368,6 +375,13 @@ testRule('xgen-IPA-106-create-method-request-body-is-get-method-response', [ path: ['paths', '/resourceTwo', 'post', 'requestBody', 'content', 'application/vnd.atlas.2023-01-01+json'], severity: DiagnosticSeverity.Warning, }, + { + code: 'xgen-IPA-106-create-method-request-body-is-get-method-response', + message: + 'The request body schema properties must match the response body schema properties of the Get method. http://go/ipa/106', + path: ['paths', '/resourceTwo', 'post', 'requestBody', 'content', 'application/vnd.atlas.2024-01-01+json'], + severity: DiagnosticSeverity.Warning, + }, { code: 'xgen-IPA-106-create-method-request-body-is-get-method-response', message: @@ -382,6 +396,13 @@ testRule('xgen-IPA-106-create-method-request-body-is-get-method-response', [ path: ['paths', '/resourceCircular', 'post', 'requestBody', 'content', 'application/vnd.atlas.2023-01-01+json'], severity: DiagnosticSeverity.Warning, }, + { + code: 'xgen-IPA-106-create-method-request-body-is-get-method-response', + message: + 'The request body schema properties must match the response body schema properties of the Get method. http://go/ipa/106', + path: ['paths', '/resourceCircular', 'post', 'requestBody', 'content', 'application/vnd.atlas.2024-01-01+json'], + severity: DiagnosticSeverity.Warning, + }, ], }, { @@ -432,6 +453,13 @@ testRule('xgen-IPA-106-create-method-request-body-is-get-method-response', [ path: ['paths', '/animalResource', 'post', 'requestBody', 'content', 'application/vnd.atlas.2023-01-01+json'], severity: DiagnosticSeverity.Warning, }, + { + code: 'xgen-IPA-106-create-method-request-body-is-get-method-response', + message: + 'The request body schema properties must match the response body schema properties of the Get method. http://go/ipa/106', + path: ['paths', '/animalResource', 'post', 'requestBody', 'content', 'application/vnd.atlas.2024-01-01+json'], + severity: DiagnosticSeverity.Warning, + }, ], }, { @@ -455,6 +483,9 @@ testRule('xgen-IPA-106-create-method-request-body-is-get-method-response', [ schema: { $ref: '#/components/schemas/SchemaOne', }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-106-create-method-request-body-is-get-method-response': 'reason', + }, }, }, }, @@ -491,6 +522,9 @@ testRule('xgen-IPA-106-create-method-request-body-is-get-method-response', [ schema: { $ref: '#/components/schemas/SchemaTwo', }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-106-create-method-request-body-is-get-method-response': 'reason', + }, }, }, }, diff --git a/tools/spectral/ipa/__tests__/listMethodResponseIsGetMethodResponse.test.js b/tools/spectral/ipa/__tests__/listMethodResponseIsGetMethodResponse.test.js index b124c6a571..a8adda9f96 100644 --- a/tools/spectral/ipa/__tests__/listMethodResponseIsGetMethodResponse.test.js +++ b/tools/spectral/ipa/__tests__/listMethodResponseIsGetMethodResponse.test.js @@ -333,6 +333,33 @@ testRule('xgen-IPA-105-list-method-response-is-get-method-response', [ }, }, }, + // Get without schema + '/resourceTwo': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-01-05+json': { + schema: { + $ref: '#/components/schemas/PaginatedResourceSchema', + }, + }, + }, + }, + }, + }, + }, + '/resourceTwo/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-01-05+json': {}, + }, + }, + }, + }, + }, }, components: componentSchemas, }, @@ -359,6 +386,13 @@ testRule('xgen-IPA-105-list-method-response-is-get-method-response', [ ], severity: DiagnosticSeverity.Warning, }, + { + code: 'xgen-IPA-105-list-method-response-is-get-method-response', + message: + 'Could not validate that the List method returns the same resource object as the Get method. The Get method does not have a schema. http://go/ipa/105', + path: ['paths', '/resourceTwo', 'get', 'responses', '200', 'content', 'application/vnd.atlas.2024-01-05+json'], + severity: DiagnosticSeverity.Warning, + }, ], }, { diff --git a/tools/spectral/ipa/__tests__/utils/methodUtils.test.js b/tools/spectral/ipa/__tests__/utils/methodUtils.test.js new file mode 100644 index 0000000000..13da0b1577 --- /dev/null +++ b/tools/spectral/ipa/__tests__/utils/methodUtils.test.js @@ -0,0 +1,185 @@ +import { describe, expect, it } from '@jest/globals'; +import { + getAllSuccessfulResponseSchemas, + getResponseOfGetMethodByMediaType, + getResponseOfListMethodByMediaType, +} from '../../rulesets/functions/utils/methodUtils.js'; + +describe('tools/spectral/ipa/rulesets/functions/utils/methodUtils.js', () => { + describe('getAllSuccessfulResponseSchemas', () => { + const operationObject = { + responses: { + 200: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + type: 'object', + }, + }, + 'application/vnd.atlas.2024-01-01+json': { + schema: { + type: 'array', + }, + }, + }, + description: 'OK', + }, + 401: { + content: { + 'application/json': { + schema: { + type: 'string', + }, + }, + }, + }, + }, + }; + + it('returns all 2xx response schemas', () => { + const result = getAllSuccessfulResponseSchemas(operationObject); + expect(result).toHaveLength(2); + + expect(result[0].schemaPath).toEqual(['responses', '200', 'content', 'application/vnd.atlas.2023-01-01+json']); + expect(result[0].schema.type).toEqual('object'); + + expect(result[1].schemaPath).toEqual(['responses', '200', 'content', 'application/vnd.atlas.2024-01-01+json']); + expect(result[1].schema.type).toEqual('array'); + }); + }); + + describe('getResponseOfGetMethodByMediaType', () => { + const oas = { + paths: { + '/resource': {}, + '/resource/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + type: 'object', + }, + }, + 'application/vnd.atlas.2024-01-01+json': { + schema: { + type: 'array', + }, + }, + }, + }, + }, + }, + }, + }, + }; + + const testCases = [ + { + description: 'Exact version', + requestedVersion: 'application/vnd.atlas.2023-01-01+json', + expectedMatch: 'application/vnd.atlas.2023-01-01+json', + }, + { + description: 'Exact version', + requestedVersion: 'application/vnd.atlas.2024-01-01+json', + expectedMatch: 'application/vnd.atlas.2024-01-01+json', + }, + { + description: 'Latest past version', + requestedVersion: 'application/vnd.atlas.2024-10-01+json', + expectedMatch: 'application/vnd.atlas.2024-01-01+json', + }, + { + description: 'Latest past version', + requestedVersion: 'application/vnd.atlas.2023-10-01+json', + expectedMatch: 'application/vnd.atlas.2023-01-01+json', + }, + { + description: 'No match', + requestedVersion: 'application/vnd.atlas.2020-10-01+json', + expectedMatch: '', + }, + ]; + + testCases.forEach((testCase) => { + it(`returns the expected match for ${testCase.description}`, () => { + const result = getResponseOfGetMethodByMediaType(testCase.requestedVersion, '/resource', oas); + + if (!testCase.expectedMatch) { + expect(result).toBeNull(); + } else { + expect(result).toEqual(oas.paths['/resource/{id}'].get.responses['200'].content[testCase.expectedMatch]); + } + }); + }); + }); + + describe('getResponseOfListMethodByMediaType', () => { + const oas = { + paths: { + '/resource': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + type: 'object', + }, + }, + 'application/vnd.atlas.2024-01-01+json': { + schema: { + type: 'array', + }, + }, + }, + }, + }, + }, + }, + }, + }; + + const testCases = [ + { + description: 'Exact version', + requestedVersion: 'application/vnd.atlas.2023-01-01+json', + expectedMatch: 'application/vnd.atlas.2023-01-01+json', + }, + { + description: 'Exact version', + requestedVersion: 'application/vnd.atlas.2024-01-01+json', + expectedMatch: 'application/vnd.atlas.2024-01-01+json', + }, + { + description: 'Latest past version', + requestedVersion: 'application/vnd.atlas.2024-10-01+json', + expectedMatch: 'application/vnd.atlas.2024-01-01+json', + }, + { + description: 'Latest past version', + requestedVersion: 'application/vnd.atlas.2023-10-01+json', + expectedMatch: 'application/vnd.atlas.2023-01-01+json', + }, + { + description: 'No match', + requestedVersion: 'application/vnd.atlas.2020-10-01+json', + expectedMatch: '', + }, + ]; + + testCases.forEach((testCase) => { + it(`returns the expected match for ${testCase.description}`, () => { + const result = getResponseOfListMethodByMediaType(testCase.requestedVersion, '/resource', oas); + + if (!testCase.expectedMatch) { + expect(result).toBeNull(); + } else { + expect(result).toEqual(oas.paths['/resource'].get.responses['200'].content[testCase.expectedMatch]); + } + }); + }); + }); +}); diff --git a/tools/spectral/ipa/rulesets/IPA-105.yaml b/tools/spectral/ipa/rulesets/IPA-105.yaml index 92e4cb09fe..dc6da45796 100644 --- a/tools/spectral/ipa/rulesets/IPA-105.yaml +++ b/tools/spectral/ipa/rulesets/IPA-105.yaml @@ -36,6 +36,7 @@ rules: - Validation applies to List methods for resource collections only
- Validation applies to json response content only
- Validation ignores responses without schema and non-paginated responses
+ - Validation ignores resources without a Get method
- Paths with `x-xgen-IPA-exception` for this rule are excluded from validation
http://go/ipa/105 message: '{{error}} http://go/ipa/105' diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index b30d6c8dd5..57c250f34a 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -42,12 +42,12 @@ For rule definitions, see [IPA-104.yaml](https://github.com/mongodb/openapi/blob For rule definitions, see [IPA-105.yaml](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/IPA-105.yaml). -| Rule Name | Description | Severity | -| -------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------- | -| xgen-IPA-105-list-method-response-code-is-200 | The List method must return a 200 OK response. http://go/ipa/105 | warn | -| xgen-IPA-105-list-method-no-request-body | The List method request must not include a body. http://go/ipa/105 | warn | -| xgen-IPA-105-resource-has-list | APIs must provide a List method for resources. http://go/ipa/105 | warn | -| xgen-IPA-105-list-method-response-is-get-method-response | The response body of the List method should consist of the same resource object returned by the Get method. Validation checks that the List method results items reference the same schema as the Get method response.
- Validation applies to List methods for resource collections only
- Validation applies to json response content only
- Validation ignores responses without schema and non-paginated responses
- Paths with `x-xgen-IPA-exception` for this rule are excluded from validation
http://go/ipa/105 | warn | +| Rule Name | Description | Severity | +| -------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | +| xgen-IPA-105-list-method-response-code-is-200 | The List method must return a 200 OK response. http://go/ipa/105 | warn | +| xgen-IPA-105-list-method-no-request-body | The List method request must not include a body. http://go/ipa/105 | warn | +| xgen-IPA-105-resource-has-list | APIs must provide a List method for resources. http://go/ipa/105 | warn | +| xgen-IPA-105-list-method-response-is-get-method-response | The response body of the List method should consist of the same resource object returned by the Get method. Validation checks that the List method results items reference the same schema as the Get method response.
- Validation applies to List methods for resource collections only
- Validation applies to json response content only
- Validation ignores responses without schema and non-paginated responses
- Validation ignores resources without a Get method
- Paths with `x-xgen-IPA-exception` for this rule are excluded from validation
http://go/ipa/105 | warn | ### IPA-106 diff --git a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js index ecdefd64ab..23e551c083 100644 --- a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js +++ b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js @@ -39,7 +39,7 @@ export default (input, _, { path, documentInventory }) => { ); if (errors.length !== 0) { - return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); + return collectAndReturnViolation(path, RULE_NAME, errors); } collectAdoption(path, RULE_NAME); diff --git a/tools/spectral/ipa/rulesets/functions/listMethodResponseIsGetMethodResponse.js b/tools/spectral/ipa/rulesets/functions/listMethodResponseIsGetMethodResponse.js index f0b77afa40..2407bd178d 100644 --- a/tools/spectral/ipa/rulesets/functions/listMethodResponseIsGetMethodResponse.js +++ b/tools/spectral/ipa/rulesets/functions/listMethodResponseIsGetMethodResponse.js @@ -5,12 +5,17 @@ import { } from './utils/resourceEvaluation.js'; import { resolveObject } from './utils/componentUtils.js'; import { hasException } from './utils/exceptions.js'; -import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; +import { + collectAdoption, + collectAndReturnViolation, + collectException, + handleInternalError, +} from './utils/collectionUtils.js'; import { getResponseOfListMethodByMediaType, - getResponseOfGetMethodByMediaType, getSchemaRef, getSchemaNameFromRef, + getResponseOfGetMethodByMediaType, } from './utils/methodUtils.js'; import { schemaIsPaginated } from './utils/schemaUtils.js'; @@ -39,6 +44,11 @@ export default (input, _, { path, documentInventory }) => { return; } + if (hasException(listMethodResponse, RULE_NAME)) { + collectException(listMethodResponse, RULE_NAME, path); + return; + } + // Get list response schema from ref or inline schema let resolvedListSchema; const listSchemaRef = getSchemaRef(listMethodResponse.schema); @@ -54,36 +64,46 @@ export default (input, _, { path, documentInventory }) => { return; } - // Ignore if there is no matching Get method response, or if it does not have a schema - const getMethodRequestContentPerMediaType = getResponseOfGetMethodByMediaType(mediaType, resourcePath, oas); - if (!getMethodRequestContentPerMediaType || !getMethodRequestContentPerMediaType.schema) { - return; - } - - if (hasException(listMethodResponse, RULE_NAME)) { - collectException(listMethodResponse, RULE_NAME, path); + // Ignore if there is no matching Get method + const getMethodResponseContentPerMediaType = getResponseOfGetMethodByMediaType(mediaType, resourcePath, oas); + if (!getMethodResponseContentPerMediaType) { return; } const errors = checkViolationsAndReturnErrors( path, resolvedListSchema.properties.results.items, - getMethodRequestContentPerMediaType.schema + getMethodResponseContentPerMediaType ); if (errors.length !== 0) { - return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); + return collectAndReturnViolation(path, RULE_NAME, errors); } collectAdoption(path, RULE_NAME); }; -function checkViolationsAndReturnErrors(path, listMethodResultItemsSchema, getMethodSchema) { - const listMethodSchemaRef = getSchemaRef(listMethodResultItemsSchema); - const getMethodSchemaRef = getSchemaRef(getMethodSchema); +function checkViolationsAndReturnErrors(path, listMethodResultItems, getMethodResponseContent) { + try { + // Error if the Get method does not have a schema + if (!getMethodResponseContent.schema) { + return [ + { + path, + message: `Could not validate that the List method returns the same resource object as the Get method. The Get method does not have a schema.`, + }, + ]; + } + + const listMethodSchemaRef = getSchemaRef(listMethodResultItems); + const getMethodSchemaRef = getSchemaRef(getMethodResponseContent.schema); - if (getMethodSchemaRef !== listMethodSchemaRef) { - return [{ path, message: ERROR_MESSAGE }]; + // Error if the get method resource is not the same as the list method resource + if (getMethodSchemaRef !== listMethodSchemaRef) { + return [{ path, message: ERROR_MESSAGE }]; + } + return []; + } catch (e) { + handleInternalError(RULE_NAME, path, e); } - return []; } diff --git a/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js b/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js index ff3af91ff8..2acb191e51 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js +++ b/tools/spectral/ipa/rulesets/functions/utils/methodUtils.js @@ -61,12 +61,13 @@ export function getSchemaNameFromRef(schemaRef) { /** * Retrieves the response schema of the Get method for a resource by media type. * If the OAS is unresolved, the returning schema may contain a reference to a schema definition. + * If there is no response with the exact media type version, the latest version before the passed one is returned, otherwise null * * This function: * 1. Finds all path items related to the resource collection * 2. Identifies the single resource path (e.g., '/resource/{id}') * 3. Checks if the single resource has a GET method - * 4. Returns the response schema for the specified media type if available + * 4. Returns the response schema for the specified media type if available, or the latest version * * @param {string} mediaType - The media type to retrieve the schema for (e.g., 'application/vnd.atlas.2023-01-01+json') * @param {string} pathForResourceCollection - The path for the collection of resources (e.g., '/resource') @@ -93,6 +94,7 @@ export function getResponseOfGetMethodByMediaType(mediaType, pathForResourceColl /** * Retrieves the 200 response schema of the List method for a resource by media type. * If the OAS is unresolved, the returning schema may contain a reference to a schema definition. + * If there is no response with the exact media type version, the latest version before the passed one is returned, otherwise null. * * @param {string} mediaType - The media type to retrieve the schema for (e.g., 'application/vnd.atlas.2023-01-01+json') * @param {string} pathForResourceCollection - The path for the collection of resources (e.g., '/resource') @@ -110,7 +112,7 @@ export function getResponseOfListMethodByMediaType(mediaType, pathForResourceCol /** * Returns the schema for the 200 response of the GET method for a path item by media type. - * If there is no response with the exact media type version, the latest version before the passed one is returned. + * If there is no response with the exact media type version, the latest version before the passed one is returned, otherwise null. * * @param {Object} pathItem The path item to extract the GET response from * @param {string} mediaType The media type @@ -139,9 +141,11 @@ function getGETMethodResponseSchemaFromPathItem(pathItem, mediaType) { } return schema; } - const latestVersion = versions.sort().reverse()[0]; - if (latestVersion < mediaType) { - const schema = response.content[latestVersion]; + + const orderedVersions = versions.sort().reverse(); + const latestVersionMatch = orderedVersions.find((version) => version < mediaType); + if (latestVersionMatch) { + const schema = response.content[latestVersionMatch]; if (!schema) { return null; } From e3c2cc96d29f08a4a5e2a850a22dc8358bc93af1 Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Thu, 13 Mar 2025 16:43:16 +0000 Subject: [PATCH 03/12] CLOUDP-304938: Prettier --- tools/spectral/ipa/rulesets/IPA-105.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/spectral/ipa/rulesets/IPA-105.yaml b/tools/spectral/ipa/rulesets/IPA-105.yaml index dc6da45796..d7a5110655 100644 --- a/tools/spectral/ipa/rulesets/IPA-105.yaml +++ b/tools/spectral/ipa/rulesets/IPA-105.yaml @@ -45,4 +45,3 @@ rules: then: field: '@key' function: 'listMethodResponseIsGetMethodResponse' - From be555c108e2f154bcd92bf4d514f7d271f438e3a Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Fri, 14 Mar 2025 11:10:20 +0000 Subject: [PATCH 04/12] CLOUDP-304938: Fix --- tools/spectral/ipa/__tests__/__helpers__/testRule.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/tools/spectral/ipa/__tests__/__helpers__/testRule.js b/tools/spectral/ipa/__tests__/__helpers__/testRule.js index 18b1978f4b..622a15b9db 100644 --- a/tools/spectral/ipa/__tests__/__helpers__/testRule.js +++ b/tools/spectral/ipa/__tests__/__helpers__/testRule.js @@ -13,9 +13,6 @@ export default (ruleName, tests) => { const doc = testCase.document instanceof Document ? testCase.document : JSON.stringify(testCase.document); const errors = await s.run(doc); - if (testCase.name === 'invalid methods' && errors.length !== testCase.errors.length) { - console.log('Errors:', errors); - } expect(errors.length).toEqual(testCase.errors.length); errors.forEach((error, index) => { From 0a65b64102f176da2a4a8dcce057cf41f587e5ac Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Fri, 14 Mar 2025 12:16:22 +0000 Subject: [PATCH 05/12] CLOUDP-304938: Add error for inline get schema --- ...tMethodResponseIsGetMethodResponse.test.js | 46 +++++++++++++++++++ .../listMethodResponseIsGetMethodResponse.js | 20 +++++--- 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/tools/spectral/ipa/__tests__/listMethodResponseIsGetMethodResponse.test.js b/tools/spectral/ipa/__tests__/listMethodResponseIsGetMethodResponse.test.js index a8adda9f96..9c3fbf5032 100644 --- a/tools/spectral/ipa/__tests__/listMethodResponseIsGetMethodResponse.test.js +++ b/tools/spectral/ipa/__tests__/listMethodResponseIsGetMethodResponse.test.js @@ -360,6 +360,37 @@ testRule('xgen-IPA-105-list-method-response-is-get-method-response', [ }, }, }, + // Get without schema ref + '/resourceThree': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-01-05+json': { + schema: { + $ref: '#/components/schemas/PaginatedResourceSchema', + }, + }, + }, + }, + }, + }, + }, + '/resourceThree/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-01-05+json': { + schema: { + type: 'object', + }, + }, + }, + }, + }, + }, + }, }, components: componentSchemas, }, @@ -393,6 +424,21 @@ testRule('xgen-IPA-105-list-method-response-is-get-method-response', [ path: ['paths', '/resourceTwo', 'get', 'responses', '200', 'content', 'application/vnd.atlas.2024-01-05+json'], severity: DiagnosticSeverity.Warning, }, + { + code: 'xgen-IPA-105-list-method-response-is-get-method-response', + message: + 'Could not validate that the List method returns the same resource object as the Get method. The Get method does not have a schema reference. http://go/ipa/105', + path: [ + 'paths', + '/resourceThree', + 'get', + 'responses', + '200', + 'content', + 'application/vnd.atlas.2024-01-05+json', + ], + severity: DiagnosticSeverity.Warning, + }, ], }, { diff --git a/tools/spectral/ipa/rulesets/functions/listMethodResponseIsGetMethodResponse.js b/tools/spectral/ipa/rulesets/functions/listMethodResponseIsGetMethodResponse.js index 2407bd178d..8897771c0a 100644 --- a/tools/spectral/ipa/rulesets/functions/listMethodResponseIsGetMethodResponse.js +++ b/tools/spectral/ipa/rulesets/functions/listMethodResponseIsGetMethodResponse.js @@ -11,12 +11,7 @@ import { collectException, handleInternalError, } from './utils/collectionUtils.js'; -import { - getResponseOfListMethodByMediaType, - getSchemaRef, - getSchemaNameFromRef, - getResponseOfGetMethodByMediaType, -} from './utils/methodUtils.js'; +import { getSchemaRef, getSchemaNameFromRef, getResponseOfGetMethodByMediaType } from './utils/methodUtils.js'; import { schemaIsPaginated } from './utils/schemaUtils.js'; const RULE_NAME = 'xgen-IPA-105-list-method-response-is-get-method-response'; @@ -39,7 +34,8 @@ export default (input, _, { path, documentInventory }) => { } // Ignore if the List method does not have a response schema - const listMethodResponse = getResponseOfListMethodByMediaType(mediaType, resourcePath, oas); + const listMethodResponse = oas.paths[resourcePath].get.responses['200'].content[mediaType]; + if (!listMethodResponse || !listMethodResponse.schema) { return; } @@ -98,6 +94,16 @@ function checkViolationsAndReturnErrors(path, listMethodResultItems, getMethodRe const listMethodSchemaRef = getSchemaRef(listMethodResultItems); const getMethodSchemaRef = getSchemaRef(getMethodResponseContent.schema); + // Error if the Get method does not have a schema ref + if (!getMethodSchemaRef) { + return [ + { + path, + message: `Could not validate that the List method returns the same resource object as the Get method. The Get method does not have a schema reference.`, + }, + ]; + } + // Error if the get method resource is not the same as the list method resource if (getMethodSchemaRef !== listMethodSchemaRef) { return [{ path, message: ERROR_MESSAGE }]; From 827b98fde2eedf71e6d321642dd5a5c8307ed683 Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Fri, 14 Mar 2025 12:27:01 +0000 Subject: [PATCH 06/12] CLOUDP-304938: Fix docs --- tools/spectral/ipa/rulesets/IPA-105.yaml | 18 +++++++++++------- tools/spectral/ipa/rulesets/README.md | 5 ++++- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/tools/spectral/ipa/rulesets/IPA-105.yaml b/tools/spectral/ipa/rulesets/IPA-105.yaml index d7a5110655..247810c970 100644 --- a/tools/spectral/ipa/rulesets/IPA-105.yaml +++ b/tools/spectral/ipa/rulesets/IPA-105.yaml @@ -32,13 +32,17 @@ rules: function: 'eachResourceHasListMethod' xgen-IPA-105-list-method-response-is-get-method-response: description: >- - The response body of the List method should consist of the same resource object returned by the Get method. Validation checks that the List method results items reference the same schema as the Get method response.
- - Validation applies to List methods for resource collections only
- - Validation applies to json response content only
- - Validation ignores responses without schema and non-paginated responses
- - Validation ignores resources without a Get method
- - Paths with `x-xgen-IPA-exception` for this rule are excluded from validation
- http://go/ipa/105 + The response body of the List method should consist of the same resource object returned by the Get method. http://go/ipa/105 + + ##### Implementation details + + Validation checks that the List method response contains items property with reference to the same schema as the Get method response. + + - Validation applies to List methods for resource collections only + - Validation applies to json response content only + - Validation ignores responses without schema and non-paginated responses + - Validation ignores resources without a Get method + - Paths with `x-xgen-IPA-exception` for this rule are excluded from validation message: '{{error}} http://go/ipa/105' severity: warn given: '$.paths[*].get.responses[*].content' diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 62ebaede2b..649b6135ea 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -110,7 +110,10 @@ APIs must provide a List method for resources. http://go/ipa/105 #### xgen-IPA-105-list-method-response-is-get-method-response ![warn](https://img.shields.io/badge/warning-yellow) -The response body of the List method should consist of the same resource object returned by the Get method. Validation checks that the List method results items reference the same schema as the Get method response.
- Validation applies to List methods for resource collections only
- Validation applies to json response content only
- Validation ignores responses without schema and non-paginated responses
- Validation ignores resources without a Get method
- Paths with `x-xgen-IPA-exception` for this rule are excluded from validation
http://go/ipa/105 +The response body of the List method should consist of the same resource object returned by the Get method. http://go/ipa/105 +##### Implementation details +Validation checks that the List method response contains items property with reference to the same schema as the Get method response. +- Validation applies to List methods for resource collections only - Validation applies to json response content only - Validation ignores responses without schema and non-paginated responses - Validation ignores resources without a Get method - Paths with `x-xgen-IPA-exception` for this rule are excluded from validation From 226c7a139f7413985854d9d87cd65443ccd14402 Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Fri, 14 Mar 2025 12:30:22 +0000 Subject: [PATCH 07/12] CLOUDP-304938: Fix docs --- tools/spectral/ipa/rulesets/IPA-105.yaml | 10 +++++----- tools/spectral/ipa/rulesets/README.md | 7 ++++++- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/tools/spectral/ipa/rulesets/IPA-105.yaml b/tools/spectral/ipa/rulesets/IPA-105.yaml index 247810c970..0de678f876 100644 --- a/tools/spectral/ipa/rulesets/IPA-105.yaml +++ b/tools/spectral/ipa/rulesets/IPA-105.yaml @@ -38,11 +38,11 @@ rules: Validation checks that the List method response contains items property with reference to the same schema as the Get method response. - - Validation applies to List methods for resource collections only - - Validation applies to json response content only - - Validation ignores responses without schema and non-paginated responses - - Validation ignores resources without a Get method - - Paths with `x-xgen-IPA-exception` for this rule are excluded from validation + - Validation applies to List methods for resource collections only + - Validation applies to json response content only + - Validation ignores responses without schema and non-paginated responses + - Validation ignores resources without a Get method + - Paths with `x-xgen-IPA-exception` for this rule are excluded from validation message: '{{error}} http://go/ipa/105' severity: warn given: '$.paths[*].get.responses[*].content' diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 649b6135ea..e8a0aa6412 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -113,7 +113,12 @@ APIs must provide a List method for resources. http://go/ipa/105 The response body of the List method should consist of the same resource object returned by the Get method. http://go/ipa/105 ##### Implementation details Validation checks that the List method response contains items property with reference to the same schema as the Get method response. -- Validation applies to List methods for resource collections only - Validation applies to json response content only - Validation ignores responses without schema and non-paginated responses - Validation ignores resources without a Get method - Paths with `x-xgen-IPA-exception` for this rule are excluded from validation + + - Validation applies to List methods for resource collections only + - Validation applies to json response content only + - Validation ignores responses without schema and non-paginated responses + - Validation ignores resources without a Get method + - Paths with `x-xgen-IPA-exception` for this rule are excluded from validation From 7f2edeebe478a2fdb641948a797b2de1fae58977 Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Fri, 14 Mar 2025 12:31:56 +0000 Subject: [PATCH 08/12] CLOUDP-304938: Fix docs --- .../listMethodResponseIsGetMethodResponse.test.js | 10 +++++----- tools/spectral/ipa/rulesets/IPA-105.yaml | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/spectral/ipa/__tests__/listMethodResponseIsGetMethodResponse.test.js b/tools/spectral/ipa/__tests__/listMethodResponseIsGetMethodResponse.test.js index 9c3fbf5032..c1e94ef5f0 100644 --- a/tools/spectral/ipa/__tests__/listMethodResponseIsGetMethodResponse.test.js +++ b/tools/spectral/ipa/__tests__/listMethodResponseIsGetMethodResponse.test.js @@ -398,14 +398,14 @@ testRule('xgen-IPA-105-list-method-response-is-get-method-response', [ { code: 'xgen-IPA-105-list-method-response-is-get-method-response', message: - 'The schema of each result in the List method response must be the same schema as the response of the Get method. http://go/ipa/105', + 'The schema of each result in the List method response must be the same schema as the response of the Get method. http://go/ipa-spectral#IPA-105', path: ['paths', '/resource', 'get', 'responses', '200', 'content', 'application/vnd.atlas.2024-08-05+json'], severity: DiagnosticSeverity.Warning, }, { code: 'xgen-IPA-105-list-method-response-is-get-method-response', message: - 'The schema of each result in the List method response must be the same schema as the response of the Get method. http://go/ipa/105', + 'The schema of each result in the List method response must be the same schema as the response of the Get method. http://go/ipa-spectral#IPA-105', path: [ 'paths', '/arrayResource', @@ -420,14 +420,14 @@ testRule('xgen-IPA-105-list-method-response-is-get-method-response', [ { code: 'xgen-IPA-105-list-method-response-is-get-method-response', message: - 'Could not validate that the List method returns the same resource object as the Get method. The Get method does not have a schema. http://go/ipa/105', + 'Could not validate that the List method returns the same resource object as the Get method. The Get method does not have a schema. http://go/ipa-spectral#IPA-105', path: ['paths', '/resourceTwo', 'get', 'responses', '200', 'content', 'application/vnd.atlas.2024-01-05+json'], severity: DiagnosticSeverity.Warning, }, { code: 'xgen-IPA-105-list-method-response-is-get-method-response', message: - 'Could not validate that the List method returns the same resource object as the Get method. The Get method does not have a schema reference. http://go/ipa/105', + 'Could not validate that the List method returns the same resource object as the Get method. The Get method does not have a schema reference. http://go/ipa-spectral#IPA-105', path: [ 'paths', '/resourceThree', @@ -482,7 +482,7 @@ testRule('xgen-IPA-105-list-method-response-is-get-method-response', [ { code: 'xgen-IPA-105-list-method-response-is-get-method-response', message: - 'The schema of each result in the List method response must be the same schema as the response of the Get method. http://go/ipa/105', + 'The schema of each result in the List method response must be the same schema as the response of the Get method. http://go/ipa-spectral#IPA-105', path: ['paths', '/resource', 'get', 'responses', '200', 'content', 'application/vnd.atlas.2024-08-05+json'], severity: DiagnosticSeverity.Warning, }, diff --git a/tools/spectral/ipa/rulesets/IPA-105.yaml b/tools/spectral/ipa/rulesets/IPA-105.yaml index 0de678f876..4b0b28d336 100644 --- a/tools/spectral/ipa/rulesets/IPA-105.yaml +++ b/tools/spectral/ipa/rulesets/IPA-105.yaml @@ -43,7 +43,7 @@ rules: - Validation ignores responses without schema and non-paginated responses - Validation ignores resources without a Get method - Paths with `x-xgen-IPA-exception` for this rule are excluded from validation - message: '{{error}} http://go/ipa/105' + message: '{{error}} http://go/ipa-spectral#IPA-105' severity: warn given: '$.paths[*].get.responses[*].content' then: From 6a3912f248224f861ea9eabc99600965954166ac Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Fri, 14 Mar 2025 12:33:41 +0000 Subject: [PATCH 09/12] CLOUDP-304938: Fix --- tools/spectral/ipa/rulesets/IPA-105.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/spectral/ipa/rulesets/IPA-105.yaml b/tools/spectral/ipa/rulesets/IPA-105.yaml index 4b0b28d336..fee55fd2f3 100644 --- a/tools/spectral/ipa/rulesets/IPA-105.yaml +++ b/tools/spectral/ipa/rulesets/IPA-105.yaml @@ -33,11 +33,11 @@ rules: xgen-IPA-105-list-method-response-is-get-method-response: description: >- The response body of the List method should consist of the same resource object returned by the Get method. http://go/ipa/105 - + ##### Implementation details - + Validation checks that the List method response contains items property with reference to the same schema as the Get method response. - + - Validation applies to List methods for resource collections only - Validation applies to json response content only - Validation ignores responses without schema and non-paginated responses From 54519bc223da71eeb1790b40abdb4f1e645eed8d Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Fri, 14 Mar 2025 12:39:02 +0000 Subject: [PATCH 10/12] CLOUDP-304938: Clarify pagination --- tools/spectral/ipa/rulesets/IPA-105.yaml | 1 + tools/spectral/ipa/rulesets/README.md | 1 + tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js | 6 +++++- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/spectral/ipa/rulesets/IPA-105.yaml b/tools/spectral/ipa/rulesets/IPA-105.yaml index fee55fd2f3..8e46f25bcb 100644 --- a/tools/spectral/ipa/rulesets/IPA-105.yaml +++ b/tools/spectral/ipa/rulesets/IPA-105.yaml @@ -41,6 +41,7 @@ rules: - Validation applies to List methods for resource collections only - Validation applies to json response content only - Validation ignores responses without schema and non-paginated responses + - A response is considered paginated if it contains an array property named `results` - Validation ignores resources without a Get method - Paths with `x-xgen-IPA-exception` for this rule are excluded from validation message: '{{error}} http://go/ipa-spectral#IPA-105' diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index e8a0aa6412..6836d7a540 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -117,6 +117,7 @@ Validation checks that the List method response contains items property with ref - Validation applies to List methods for resource collections only - Validation applies to json response content only - Validation ignores responses without schema and non-paginated responses + - A response is considered paginated if it contains an array property named `results` - Validation ignores resources without a Get method - Paths with `x-xgen-IPA-exception` for this rule are excluded from validation diff --git a/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js b/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js index 11648b38f1..0bec1d4a4e 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js +++ b/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js @@ -5,7 +5,11 @@ */ export function schemaIsPaginated(schema) { const fields = Object.keys(schema); - return fields.includes('properties') && Object.keys(schema['properties']).includes('results'); + return ( + fields.includes('properties') && + Object.keys(schema['properties']).includes('results') && + schema.properties.results.type === 'array' + ); } /** From 77cbbf75b94a0ae8771a710a998a021ddbc7b529 Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Fri, 14 Mar 2025 14:09:08 +0000 Subject: [PATCH 11/12] CLOUDP-304938: Fix --- .../rulesets/functions/listMethodResponseIsGetMethodResponse.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/spectral/ipa/rulesets/functions/listMethodResponseIsGetMethodResponse.js b/tools/spectral/ipa/rulesets/functions/listMethodResponseIsGetMethodResponse.js index 8897771c0a..7f5554918c 100644 --- a/tools/spectral/ipa/rulesets/functions/listMethodResponseIsGetMethodResponse.js +++ b/tools/spectral/ipa/rulesets/functions/listMethodResponseIsGetMethodResponse.js @@ -34,7 +34,7 @@ export default (input, _, { path, documentInventory }) => { } // Ignore if the List method does not have a response schema - const listMethodResponse = oas.paths[resourcePath].get.responses['200'].content[mediaType]; + const listMethodResponse = resolveObject(oas, path); if (!listMethodResponse || !listMethodResponse.schema) { return; From 18f9ffdc684644ed884ca9d942c65db4f02b15a8 Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Fri, 14 Mar 2025 14:22:29 +0000 Subject: [PATCH 12/12] CLOUDP-304938: Error for create is get rule missing get schema --- ...eateMethodRequestBodyIsGetResponse.test.js | 32 +++++++++++++++++++ .../createMethodRequestBodyIsGetResponse.js | 12 +++++-- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js b/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js index f94a3e7a51..520493d295 100644 --- a/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js +++ b/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js @@ -318,6 +318,31 @@ testRule('xgen-IPA-106-create-method-request-body-is-get-method-response', [ }, }, }, + // Missing schema for Get method + '/resourceFour': { + post: { + requestBody: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaOne', + }, + }, + }, + }, + }, + }, + '/resourceFour/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2023-01-01+json': {}, + }, + }, + }, + }, + }, '/resourceCircular': { post: { requestBody: { @@ -389,6 +414,13 @@ testRule('xgen-IPA-106-create-method-request-body-is-get-method-response', [ path: ['paths', '/resourceThree', 'post', 'requestBody', 'content', 'application/vnd.atlas.2023-01-01+json'], severity: DiagnosticSeverity.Warning, }, + { + code: 'xgen-IPA-106-create-method-request-body-is-get-method-response', + message: + 'Could not validate that the Create request body schema matches the response schema of the Get method. The Get method does not have a schema. http://go/ipa/106', + path: ['paths', '/resourceFour', 'post', 'requestBody', 'content', 'application/vnd.atlas.2023-01-01+json'], + severity: DiagnosticSeverity.Warning, + }, { code: 'xgen-IPA-106-create-method-request-body-is-get-method-response', message: diff --git a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js index a78b67b7fc..8feda5ff13 100644 --- a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js +++ b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js @@ -26,7 +26,7 @@ export default (input, opts, { path, documentInventory }) => { } const getMethodResponseContentPerMediaType = getResponseOfGetMethodByMediaType(mediaType, resourcePath, oas); - if (!getMethodResponseContentPerMediaType || !getMethodResponseContentPerMediaType.schema) { + if (!getMethodResponseContentPerMediaType) { return; } @@ -62,6 +62,14 @@ function checkViolationsAndReturnErrors( const errors = []; const ignoredValues = opts?.ignoredValues || []; + if (!getMethodResponseContentPerMediaType.schema) { + return [ + { + path, + message: `Could not validate that the Create request body schema matches the response schema of the Get method. The Get method does not have a schema.`, + }, + ]; + } if ( !isDeepEqual( omitDeep(postMethodRequestContentPerMediaType.schema, ...ignoredValues), @@ -69,7 +77,7 @@ function checkViolationsAndReturnErrors( ) ) { errors.push({ - path: path, + path, message: ERROR_MESSAGE, }); }