From 5f28a9267d9165498fad69e4573ab05e996d79f1 Mon Sep 17 00:00:00 2001 From: Yeliz Henden Date: Thu, 6 Mar 2025 16:42:23 +0000 Subject: [PATCH 1/8] CLOUDP-304053: IPA-106:Create - The resource must be the request body --- ...eateMethodRequestBodyIsGetResponse.test.js | 235 ++++++++++++++++++ tools/spectral/ipa/rulesets/IPA-106.yaml | 9 + .../createMethodRequestBodyIsGetResponse.js | 41 +++ .../functions/utils/resourceEvaluation.js | 40 +++ .../rulesets/functions/utils/schemaUtils.js | 62 +++++ 5 files changed, 387 insertions(+) create mode 100644 tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js create mode 100644 tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js diff --git a/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js b/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js new file mode 100644 index 0000000000..94bb4de471 --- /dev/null +++ b/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js @@ -0,0 +1,235 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +const componentSchemas = { + schemas: { + SchemaRequest: { + type: 'object', + }, + Schema: { + type: 'object', + }, + }, +}; +testRule('xgen-IPA-106-create-method-request-body-is-get-method-response', [ + { + name: 'valid methods', + document: { + components: componentSchemas, + paths: { + '/resource': { + post: { + requestBody: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaRequest', + }, + }, + 'application/vnd.atlas.2024-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaRequest', + }, + }, + }, + }, + }, + }, + '/resource/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/Schema', + }, + }, + }, + }, + }, + }, + }, + '/resource/{id}:customMethod': { + post: { + requestBody: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/Schema', + }, + }, + 'application/vnd.atlas.2024-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaRequest', + }, + }, + }, + }, + }, + }, + } + }, + errors: [], + }, + { + name: 'invalid methods', + document: { + components: componentSchemas, + paths: { + '/resource': { + post: { + requestBody: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/Schema', + }, + }, + }, + }, + }, + }, + '/resource2': { + post: { + requestBody: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/Schema', + }, + }, + 'application/vnd.atlas.2024-01-01+json': { + schema: { + $ref: '#/components/schemas/Schema', + }, + }, + }, + }, + }, + }, + '/resource3': { + post: { + requestBody: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + type: 'object', + }, + }, + }, + }, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-106-create-method-request-body-is-get-method-response', + message: 'The response body schema must reference a schema with a Request suffix. http://go/ipa/106', + 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 response body schema must reference a schema with a Request suffix. 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: 'The response body schema must reference a schema with a Request suffix. http://go/ipa/106', + path: ['paths', '/resource2', '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 response body schema must reference a schema with a Request suffix. http://go/ipa/106', + path: ['paths', '/resource2', '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: 'The response body schema is defined inline and must reference a predefined schema. http://go/ipa/106', + path: ['paths', '/resource3', 'post', 'requestBody', 'content', 'application/vnd.atlas.2023-01-01+json'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid method with exception', + document: { + components: componentSchemas, + paths: { + '/resource': { + post: { + requestBody: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/Schema', + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-106-create-method-request-body-is-get-method-response': 'reason', + }, + }, + 'application/vnd.atlas.2024-01-01+json': { + schema: { + type: 'array', + items: { + $ref: '#/components/schemas/Schema', + }, + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-106-create-method-request-body-is-get-method-response': 'reason', + }, + }, + }, + }, + }, + }, + '/resource2': { + post: { + requestBody: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/Schema', + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-106-create-method-request-body-is-get-method-response': 'reason', + }, + }, + 'application/vnd.atlas.2024-01-01+json': { + schema: { + $ref: '#/components/schemas/Schema', + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-106-create-method-request-body-is-get-method-response': 'reason', + }, + }, + }, + }, + }, + }, + '/resource3': { + post: { + requestBody: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + type: 'object', + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-106-create-method-request-body-is-get-method-response': 'reason', + }, + }, + }, + }, + }, + }, + }, + }, + errors: [], + }, +]); diff --git a/tools/spectral/ipa/rulesets/IPA-106.yaml b/tools/spectral/ipa/rulesets/IPA-106.yaml index e9f17fdb5c..aa7c18049c 100644 --- a/tools/spectral/ipa/rulesets/IPA-106.yaml +++ b/tools/spectral/ipa/rulesets/IPA-106.yaml @@ -4,6 +4,7 @@ functions: - createMethodRequestBodyIsRequestSuffixedObject - createMethodShouldNotHaveQueryParameters + - createMethodRequestBodyIsGetResponse rules: xgen-IPA-106-create-method-request-body-is-request-suffixed-object: @@ -21,3 +22,11 @@ rules: given: '$.paths[*].post' then: function: 'createMethodShouldNotHaveQueryParameters' + xgen-IPA-106-create-method-request-body-is-get-method-response: + description: 'The Create method request should be a Get method response. http://go/ipa/106' + message: '{{error}} http://go/ipa/106' + severity: warn + given: '$.paths[*].post.requestBody.content' + then: + field: '@key' + function: 'createMethodRequestBodyIsGetResponse' diff --git a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js new file mode 100644 index 0000000000..19ef1ce4f1 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js @@ -0,0 +1,41 @@ +import { getResponseOfGetMethodByMediaType, isCustomMethodIdentifier } from './utils/resourceEvaluation.js'; +import { resolveObject } from './utils/componentUtils.js'; +import { compareSchemas } from './utils/schemaUtils.js'; +import { hasException } from './utils/exceptions.js'; +import { + collectAdoption, + collectAndReturnViolation, + collectException, +} from './utils/collectionUtils.js'; + +const RULE_NAME = 'xgen-IPA-106-create-method-request-body-is-get-method-response'; +const ERROR_MESSAGE = 'The request body schema properties must match the response body schema properties of the Get method.'; + + +export default (input, _, { path, documentInventory }) => { + const oas = documentInventory.resolved; + const resourcePath = path[1]; + + if (isCustomMethodIdentifier(resourcePath)) { + return; + } + + const contentPerMediaType = resolveObject(oas, path); + + if (hasException(contentPerMediaType, RULE_NAME)) { + collectException(contentPerMediaType, RULE_NAME, path); + return; + } + + let mediaType = input; + const getMethodResponseContentPerMediaType = getResponseOfGetMethodByMediaType(mediaType, resourcePath, oas); + if (!getMethodResponseContentPerMediaType) { + return; + } + + const postMethodRequestContentPerMediaType = resolveObject(oas, path); + if (compareSchemas(postMethodRequestContentPerMediaType.schema, getMethodResponseContentPerMediaType.schema)) { + collectAdoption(path, RULE_NAME); + } + return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); +}; diff --git a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js index e231693c65..f35e09b6e5 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js +++ b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js @@ -150,3 +150,43 @@ function removePrefix(path) { } return path; } + +/** + * Checks if resource has Get method + * @param {string} mediaType the media type to check for + * @param {string} pathForResourceCollection the path for the collection of resources + * @param {Object} oas the OpenAPI document + */ +export function getResponseOfGetMethodByMediaType(mediaType, pathForResourceCollection, oas) { + const resourcePathItems = getResourcePathItems(pathForResourceCollection, oas.paths); + const resourcePaths = Object.keys(resourcePathItems); + if (resourcePaths.length === 1) { + return null; + } + let singleResourcePath = resourcePaths.find( + (path) => !isCustomMethodIdentifier(path) && path !== pathForResourceCollection + ); + if (singleResourcePath.length === 0) { + 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; +} diff --git a/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js b/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js index 42035daa56..3eae11c7dd 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js +++ b/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js @@ -7,3 +7,65 @@ export function schemaIsArray(schema) { const fields = Object.keys(schema); return fields.includes('type') && schema['type'] === 'array'; } + +/** + * Extract properties from schema, handling allOf and schema references + * @param {object} schema - Schema object + * @returns {object} - Combined properties object + */ +function getSchemaProperties(schema) { + let props = {}; + + if (schema.properties) { + props = { ...schema.properties }; + } + + // Handle allOf + if (schema.allOf) { + schema.allOf.forEach((subSchema) => { + const subProps = getSchemaProperties(subSchema); + props = { ...props, ...subProps }; + }); + } + + return props; +} + +/** + * Compares two schemas and returns inconsistent fields + * @param {object} postSchema - POST request schema + * @param {object} getSchema - GET response schema + * @returns {Array} - Array of inconsistent field names + */ +export function compareSchemas(postSchema, getSchema) { + const inconsistentFields = []; + + // Collect all properties from post schema + const postProps = getSchemaProperties(postSchema); + const getProps = getSchemaProperties(getSchema); + + // Compare properties from post schema that should be in get schema + for (const prop in postProps) { + // Skip writeOnly properties + if (postProps[prop].writeOnly) { + continue; + } + + // If property doesn't exist in GET schema + if (!getProps[prop]) { + inconsistentFields.push(prop); + continue; + } + + // For objects, compare nested properties + if (postProps[prop].type === 'object' && getProps[prop].type === 'object') { + const nestedInconsistencies = compareSchemas(postProps[prop], getProps[prop]); + if (nestedInconsistencies.length > 0) { + // Prefix nested fields with their parent name + inconsistentFields.push(...nestedInconsistencies.map((field) => `${prop}.${field}`)); + } + } + } + + return inconsistentFields; +} From 29c75ff44e167d7a9840693f6b600cce48ddcbe9 Mon Sep 17 00:00:00 2001 From: Yeliz Henden Date: Thu, 6 Mar 2025 17:29:52 +0000 Subject: [PATCH 2/8] rule implementation fix --- .../createMethodRequestBodyIsGetResponse.js | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js index 19ef1ce4f1..7c50e2b1e2 100644 --- a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js +++ b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js @@ -2,15 +2,11 @@ import { getResponseOfGetMethodByMediaType, isCustomMethodIdentifier } from './u import { resolveObject } from './utils/componentUtils.js'; import { compareSchemas } from './utils/schemaUtils.js'; import { hasException } from './utils/exceptions.js'; -import { - collectAdoption, - collectAndReturnViolation, - collectException, -} from './utils/collectionUtils.js'; +import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; const RULE_NAME = 'xgen-IPA-106-create-method-request-body-is-get-method-response'; -const ERROR_MESSAGE = 'The request body schema properties must match the response body schema properties of the Get method.'; - +const ERROR_MESSAGE = + 'The request body schema properties must match the response body schema properties of the Get method.'; export default (input, _, { path, documentInventory }) => { const oas = documentInventory.resolved; @@ -20,10 +16,10 @@ export default (input, _, { path, documentInventory }) => { return; } - const contentPerMediaType = resolveObject(oas, path); + const postMethodRequestContentPerMediaType = resolveObject(oas, path); - if (hasException(contentPerMediaType, RULE_NAME)) { - collectException(contentPerMediaType, RULE_NAME, path); + if (hasException(postMethodRequestContentPerMediaType, RULE_NAME)) { + collectException(postMethodRequestContentPerMediaType, RULE_NAME, path); return; } @@ -33,9 +29,14 @@ export default (input, _, { path, documentInventory }) => { return; } - const postMethodRequestContentPerMediaType = resolveObject(oas, path); - if (compareSchemas(postMethodRequestContentPerMediaType.schema, getMethodResponseContentPerMediaType.schema)) { + const inconsistentFields = compareSchemas( + postMethodRequestContentPerMediaType.schema, + getMethodResponseContentPerMediaType.schema + ); + if (inconsistentFields.length === 0) { collectAdoption(path, RULE_NAME); + } else { + return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); } - return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); + }; From 5e4caa96595b646df0fe9aae3ec2c1270782a531 Mon Sep 17 00:00:00 2001 From: Yeliz Henden Date: Fri, 7 Mar 2025 09:20:32 +0000 Subject: [PATCH 3/8] changes in compareSchemas --- .../ipa/rulesets/functions/utils/schemaUtils.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js b/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js index 3eae11c7dd..b3462922c9 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js +++ b/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js @@ -57,14 +57,27 @@ export function compareSchemas(postSchema, getSchema) { continue; } + if(postProps[prop].type !== getProps[prop].type) { + inconsistentFields.push(prop); + continue; + } + // For objects, compare nested properties - if (postProps[prop].type === 'object' && getProps[prop].type === 'object') { + if (postProps[prop].type === 'object') { const nestedInconsistencies = compareSchemas(postProps[prop], getProps[prop]); if (nestedInconsistencies.length > 0) { // Prefix nested fields with their parent name inconsistentFields.push(...nestedInconsistencies.map((field) => `${prop}.${field}`)); } } + + if(postProps[prop].type === 'array') { + const nestedInconsistencies = compareSchemas(postProps[prop].items, getProps[prop].items); + if (nestedInconsistencies.length > 0) { + // Prefix nested fields with their parent name + inconsistentFields.push(...nestedInconsistencies.map((field) => `${prop}.${field}`)); + } + } } return inconsistentFields; From e422df8f22f07d100ee6578012a61d7b7402a9fa Mon Sep 17 00:00:00 2001 From: Yeliz Henden Date: Mon, 10 Mar 2025 10:00:08 +0000 Subject: [PATCH 4/8] use lodash library for deep obj comparison --- package-lock.json | 14 + package.json | 2 + ...eateMethodRequestBodyIsGetResponse.test.js | 273 +++++++++++++++--- .../createMethodRequestBodyIsGetResponse.js | 55 +++- .../functions/utils/resourceEvaluation.js | 22 +- .../rulesets/functions/utils/schemaUtils.js | 75 ----- 6 files changed, 300 insertions(+), 141 deletions(-) diff --git a/package-lock.json b/package-lock.json index 28af3bf60c..192c3034ef 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15,7 +15,9 @@ "apache-arrow": "^19.0.1", "dotenv": "^16.4.7", "eslint-plugin-jest": "^28.10.0", + "lodash": "^4.17.21", "markdown-table": "^3.0.4", + "omit-deep-lodash": "^1.1.7", "openapi-to-postmanv2": "4.25.0", "parquet-wasm": "^0.6.1" }, @@ -9262,6 +9264,18 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/omit-deep-lodash": { + "version": "1.1.7", + "resolved": "https://registry.npmjs.org/omit-deep-lodash/-/omit-deep-lodash-1.1.7.tgz", + "integrity": "sha512-9m9gleSMoxq3YO8aCq5pGUrqG9rKF0w/P70JHQ1ymjUQA/3+fVa2Stju9XORJKLmyLYEO3zzX40MJYaYl5Og4w==", + "license": "MIT", + "dependencies": { + "lodash": "~4.17.21" + }, + "engines": { + "node": ">=0.10.0" + } + }, "node_modules/once": { "version": "1.4.0", "resolved": "https://registry.npmjs.org/once/-/once-1.4.0.tgz", diff --git a/package.json b/package.json index d3a0cabfc2..24ce9f5fe4 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,9 @@ "apache-arrow": "^19.0.1", "dotenv": "^16.4.7", "eslint-plugin-jest": "^28.10.0", + "lodash": "^4.17.21", "markdown-table": "^3.0.4", + "omit-deep-lodash": "^1.1.7", "openapi-to-postmanv2": "4.25.0", "parquet-wasm": "^0.6.1" }, diff --git a/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js b/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js index 94bb4de471..0df758efb7 100644 --- a/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js +++ b/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js @@ -3,14 +3,44 @@ import { DiagnosticSeverity } from '@stoplight/types'; const componentSchemas = { schemas: { - SchemaRequest: { + SchemaOne: { + type: 'string', + }, + SchemaTwo: { type: 'object', + properties: { + name: { + type: 'string', + readOnly: true, + }, + }, }, - Schema: { + SchemaThree: { type: 'object', + properties: { + name: { + type: 'string', + }, + someArray: { + type: 'array', + items: { + type: 'string', + }, + }, + }, + }, + SchemaFour: { + type: 'object', + properties: { + name: { + type: 'string', + writeOnly: true, + }, + }, }, }, }; + testRule('xgen-IPA-106-create-method-request-body-is-get-method-response', [ { name: 'valid methods', @@ -23,12 +53,12 @@ testRule('xgen-IPA-106-create-method-request-body-is-get-method-response', [ content: { 'application/vnd.atlas.2023-01-01+json': { schema: { - $ref: '#/components/schemas/SchemaRequest', + $ref: '#/components/schemas/SchemaOne', }, }, 'application/vnd.atlas.2024-01-01+json': { schema: { - $ref: '#/components/schemas/SchemaRequest', + $ref: '#/components/schemas/SchemaOne', }, }, }, @@ -42,7 +72,73 @@ testRule('xgen-IPA-106-create-method-request-body-is-get-method-response', [ content: { 'application/vnd.atlas.2023-01-01+json': { schema: { - $ref: '#/components/schemas/Schema', + $ref: '#/components/schemas/SchemaOne', + }, + }, + }, + }, + }, + }, + }, + '/resourceTwo': { + post: { + requestBody: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaTwo', + }, + }, + 'application/vnd.atlas.2024-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaTwo', + }, + }, + }, + }, + }, + }, + '/resourceTwo/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaFour', + }, + }, + }, + }, + }, + }, + }, + '/resourceThree': { + post: { + requestBody: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaThree', + }, + }, + 'application/vnd.atlas.2024-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaThree', + }, + }, + }, + }, + }, + }, + '/resourceThree/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaThree', }, }, }, @@ -56,19 +152,19 @@ testRule('xgen-IPA-106-create-method-request-body-is-get-method-response', [ content: { 'application/vnd.atlas.2023-01-01+json': { schema: { - $ref: '#/components/schemas/Schema', + $ref: '#/components/schemas/SchemaOne', }, }, 'application/vnd.atlas.2024-01-01+json': { schema: { - $ref: '#/components/schemas/SchemaRequest', + $ref: '#/components/schemas/SchemaOne', }, }, }, }, }, }, - } + }, }, errors: [], }, @@ -83,38 +179,93 @@ testRule('xgen-IPA-106-create-method-request-body-is-get-method-response', [ content: { 'application/vnd.atlas.2023-01-01+json': { schema: { - $ref: '#/components/schemas/Schema', + $ref: '#/components/schemas/SchemaOne', + }, + }, + 'application/vnd.atlas.2024-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaOne', + }, + }, + }, + }, + }, + }, + '/resource/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaTwo', + }, }, }, }, }, }, }, - '/resource2': { + '/resourceTwo': { post: { requestBody: { content: { 'application/vnd.atlas.2023-01-01+json': { schema: { - $ref: '#/components/schemas/Schema', + $ref: '#/components/schemas/SchemaTwo', }, }, 'application/vnd.atlas.2024-01-01+json': { schema: { - $ref: '#/components/schemas/Schema', + $ref: '#/components/schemas/SchemaTwo', + }, + }, + }, + }, + }, + }, + '/resourceTwo/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaThree', + }, }, }, }, }, }, }, - '/resource3': { + '/resourceThree': { post: { requestBody: { content: { 'application/vnd.atlas.2023-01-01+json': { schema: { - type: 'object', + $ref: '#/components/schemas/SchemaOne', + }, + }, + 'application/vnd.atlas.2024-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaThree', + }, + }, + }, + }, + }, + }, + '/resourceThree/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaThree', + }, }, }, }, @@ -126,32 +277,23 @@ testRule('xgen-IPA-106-create-method-request-body-is-get-method-response', [ errors: [ { code: 'xgen-IPA-106-create-method-request-body-is-get-method-response', - message: 'The response body schema must reference a schema with a Request suffix. http://go/ipa/106', + 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.2023-01-01+json'], severity: DiagnosticSeverity.Warning, }, { code: 'xgen-IPA-106-create-method-request-body-is-get-method-response', - message: 'The response body schema must reference a schema with a Request suffix. http://go/ipa/106', - path: ['paths', '/resource', 'post', 'requestBody', 'content', 'application/vnd.atlas.2024-01-01+json'], + 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.2023-01-01+json'], severity: DiagnosticSeverity.Warning, }, { code: 'xgen-IPA-106-create-method-request-body-is-get-method-response', - message: 'The response body schema must reference a schema with a Request suffix. http://go/ipa/106', - path: ['paths', '/resource2', '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 response body schema must reference a schema with a Request suffix. http://go/ipa/106', - path: ['paths', '/resource2', '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: 'The response body schema is defined inline and must reference a predefined schema. http://go/ipa/106', - path: ['paths', '/resource3', 'post', 'requestBody', 'content', 'application/vnd.atlas.2023-01-01+json'], + message: + 'The request body schema properties must match the response body schema properties of the Get method. http://go/ipa/106', + path: ['paths', '/resourceThree', 'post', 'requestBody', 'content', 'application/vnd.atlas.2023-01-01+json'], severity: DiagnosticSeverity.Warning, }, ], @@ -167,7 +309,7 @@ testRule('xgen-IPA-106-create-method-request-body-is-get-method-response', [ content: { 'application/vnd.atlas.2023-01-01+json': { schema: { - $ref: '#/components/schemas/Schema', + $ref: '#/components/schemas/SchemaOne', }, 'x-xgen-IPA-exception': { 'xgen-IPA-106-create-method-request-body-is-get-method-response': 'reason', @@ -175,26 +317,35 @@ testRule('xgen-IPA-106-create-method-request-body-is-get-method-response', [ }, 'application/vnd.atlas.2024-01-01+json': { schema: { - type: 'array', - items: { - $ref: '#/components/schemas/Schema', - }, + $ref: '#/components/schemas/SchemaOne', }, - 'x-xgen-IPA-exception': { - 'xgen-IPA-106-create-method-request-body-is-get-method-response': 'reason', + }, + }, + }, + }, + }, + '/resource/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaTwo', + }, }, }, }, }, }, }, - '/resource2': { + '/resourceTwo': { post: { requestBody: { content: { 'application/vnd.atlas.2023-01-01+json': { schema: { - $ref: '#/components/schemas/Schema', + $ref: '#/components/schemas/SchemaTwo', }, 'x-xgen-IPA-exception': { 'xgen-IPA-106-create-method-request-body-is-get-method-response': 'reason', @@ -202,28 +353,60 @@ testRule('xgen-IPA-106-create-method-request-body-is-get-method-response', [ }, 'application/vnd.atlas.2024-01-01+json': { schema: { - $ref: '#/components/schemas/Schema', + $ref: '#/components/schemas/SchemaTwo', }, - 'x-xgen-IPA-exception': { - 'xgen-IPA-106-create-method-request-body-is-get-method-response': 'reason', + }, + }, + }, + }, + }, + '/resourceTwo/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaThree', + }, }, }, }, }, }, }, - '/resource3': { + '/resourceThree': { post: { requestBody: { content: { 'application/vnd.atlas.2023-01-01+json': { schema: { - type: 'object', + $ref: '#/components/schemas/SchemaOne', }, 'x-xgen-IPA-exception': { 'xgen-IPA-106-create-method-request-body-is-get-method-response': 'reason', }, }, + 'application/vnd.atlas.2024-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaThree', + }, + }, + }, + }, + }, + }, + '/resourceThree/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaThree', + }, + }, + }, }, }, }, diff --git a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js index 7c50e2b1e2..0b3ee12549 100644 --- a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js +++ b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js @@ -1,6 +1,7 @@ import { getResponseOfGetMethodByMediaType, isCustomMethodIdentifier } from './utils/resourceEvaluation.js'; import { resolveObject } from './utils/componentUtils.js'; -import { compareSchemas } from './utils/schemaUtils.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'; @@ -11,32 +12,56 @@ const ERROR_MESSAGE = export default (input, _, { path, documentInventory }) => { const oas = documentInventory.resolved; const resourcePath = path[1]; + let mediaType = input; + if (isCustomMethodIdentifier(resourcePath) || !mediaType.endsWith('json')) { + return; + } - if (isCustomMethodIdentifier(resourcePath)) { + const getMethodResponseContentPerMediaType = getResponseOfGetMethodByMediaType(mediaType, resourcePath, oas); + if (!getMethodResponseContentPerMediaType || !getMethodResponseContentPerMediaType.schema) { return; } const postMethodRequestContentPerMediaType = resolveObject(oas, path); - - if (hasException(postMethodRequestContentPerMediaType, RULE_NAME)) { - collectException(postMethodRequestContentPerMediaType, RULE_NAME, path); + if (!postMethodRequestContentPerMediaType.schema) { return; } - - let mediaType = input; - const getMethodResponseContentPerMediaType = getResponseOfGetMethodByMediaType(mediaType, resourcePath, oas); - if (!getMethodResponseContentPerMediaType) { + if (hasException(postMethodRequestContentPerMediaType, RULE_NAME)) { + collectException(postMethodRequestContentPerMediaType, RULE_NAME, path); return; } - const inconsistentFields = compareSchemas( - postMethodRequestContentPerMediaType.schema, - getMethodResponseContentPerMediaType.schema + const errors = checkViolationsAndReturnErrors( + path, + postMethodRequestContentPerMediaType, + getMethodResponseContentPerMediaType ); - if (inconsistentFields.length === 0) { - collectAdoption(path, RULE_NAME); - } else { + + if (errors.length !== 0) { return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); } + collectAdoption(path, RULE_NAME); }; + +function checkViolationsAndReturnErrors( + path, + postMethodRequestContentPerMediaType, + getMethodResponseContentPerMediaType +) { + const errors = []; + + if ( + !isEqual( + omitDeep(postMethodRequestContentPerMediaType.schema, 'readOnly', 'writeOnly'), + omitDeep(getMethodResponseContentPerMediaType.schema, 'readOnly', 'writeOnly') + ) + ) { + errors.push({ + path: path, + message: ERROR_MESSAGE, + }); + } + + return errors; +} \ No newline at end of file diff --git a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js index f35e09b6e5..390cc8e9e3 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js +++ b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js @@ -152,10 +152,18 @@ function removePrefix(path) { } /** - * Checks if resource has Get method - * @param {string} mediaType the media type to check for - * @param {string} pathForResourceCollection the path for the collection of resources - * @param {Object} oas the OpenAPI document + * 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); @@ -163,12 +171,14 @@ export function getResponseOfGetMethodByMediaType(mediaType, pathForResourceColl if (resourcePaths.length === 1) { return null; } - let singleResourcePath = resourcePaths.find( + + const singleResourcePath = resourcePaths.find( (path) => !isCustomMethodIdentifier(path) && path !== pathForResourceCollection ); - if (singleResourcePath.length === 0) { + if (!singleResourcePath) { return null; } + const singleResourcePathObject = resourcePathItems[singleResourcePath]; if (!hasGetMethod(singleResourcePathObject)) { return null; diff --git a/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js b/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js index b3462922c9..42035daa56 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js +++ b/tools/spectral/ipa/rulesets/functions/utils/schemaUtils.js @@ -7,78 +7,3 @@ export function schemaIsArray(schema) { const fields = Object.keys(schema); return fields.includes('type') && schema['type'] === 'array'; } - -/** - * Extract properties from schema, handling allOf and schema references - * @param {object} schema - Schema object - * @returns {object} - Combined properties object - */ -function getSchemaProperties(schema) { - let props = {}; - - if (schema.properties) { - props = { ...schema.properties }; - } - - // Handle allOf - if (schema.allOf) { - schema.allOf.forEach((subSchema) => { - const subProps = getSchemaProperties(subSchema); - props = { ...props, ...subProps }; - }); - } - - return props; -} - -/** - * Compares two schemas and returns inconsistent fields - * @param {object} postSchema - POST request schema - * @param {object} getSchema - GET response schema - * @returns {Array} - Array of inconsistent field names - */ -export function compareSchemas(postSchema, getSchema) { - const inconsistentFields = []; - - // Collect all properties from post schema - const postProps = getSchemaProperties(postSchema); - const getProps = getSchemaProperties(getSchema); - - // Compare properties from post schema that should be in get schema - for (const prop in postProps) { - // Skip writeOnly properties - if (postProps[prop].writeOnly) { - continue; - } - - // If property doesn't exist in GET schema - if (!getProps[prop]) { - inconsistentFields.push(prop); - continue; - } - - if(postProps[prop].type !== getProps[prop].type) { - inconsistentFields.push(prop); - continue; - } - - // For objects, compare nested properties - if (postProps[prop].type === 'object') { - const nestedInconsistencies = compareSchemas(postProps[prop], getProps[prop]); - if (nestedInconsistencies.length > 0) { - // Prefix nested fields with their parent name - inconsistentFields.push(...nestedInconsistencies.map((field) => `${prop}.${field}`)); - } - } - - if(postProps[prop].type === 'array') { - const nestedInconsistencies = compareSchemas(postProps[prop].items, getProps[prop].items); - if (nestedInconsistencies.length > 0) { - // Prefix nested fields with their parent name - inconsistentFields.push(...nestedInconsistencies.map((field) => `${prop}.${field}`)); - } - } - } - - return inconsistentFields; -} From b0cc88929a1363e6032cfa1ecf8effc95349cb37 Mon Sep 17 00:00:00 2001 From: Yeliz Henden Date: Mon, 10 Mar 2025 10:07:08 +0000 Subject: [PATCH 5/8] prettier fix --- .../rulesets/functions/createMethodRequestBodyIsGetResponse.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js index 0b3ee12549..3ade0cfdb5 100644 --- a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js +++ b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js @@ -64,4 +64,4 @@ function checkViolationsAndReturnErrors( } return errors; -} \ No newline at end of file +} From 394e721fef1607e037c41c4e586ed76d2a41b33d Mon Sep 17 00:00:00 2001 From: Yeliz Henden Date: Mon, 10 Mar 2025 10:24:19 +0000 Subject: [PATCH 6/8] fixes --- .github/workflows/spectral-lint.yml | 7 +++++++ tools/spectral/ipa/rulesets/IPA-106.yaml | 2 +- tools/spectral/ipa/rulesets/README.md | 1 + 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/spectral-lint.yml b/.github/workflows/spectral-lint.yml index e7b5b3faeb..8065fa61de 100644 --- a/.github/workflows/spectral-lint.yml +++ b/.github/workflows/spectral-lint.yml @@ -29,6 +29,13 @@ jobs: sparse-checkout: | openapi/ tools/spectral + - name: Setup Node + uses: actions/setup-node@v4 + with: + node-version: '20.x' + cache: 'npm' + - name: Install npm dependencies + run: npm install - name: Spectral action uses: stoplightio/spectral-action@2ad0b9302e32a77c1caccf474a9b2191a8060d83 with: diff --git a/tools/spectral/ipa/rulesets/IPA-106.yaml b/tools/spectral/ipa/rulesets/IPA-106.yaml index aa7c18049c..238be3dbe9 100644 --- a/tools/spectral/ipa/rulesets/IPA-106.yaml +++ b/tools/spectral/ipa/rulesets/IPA-106.yaml @@ -1,4 +1,4 @@ -# IPA-104: Create +# IPA-106: Create # http://go/ipa/106 functions: diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index 619ef6572b..0cc069658b 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -43,6 +43,7 @@ For rule definitions, see [IPA-106.yaml](https://github.com/mongodb/openapi/blob | ------------------------------------------------------------------ | -------------------------------------------------------------------------------- | -------- | | xgen-IPA-106-create-method-request-body-is-request-suffixed-object | The Create method request should be a Request suffixed object. http://go/ipa/106 | warn | | xgen-IPA-106-create-method-should-not-have-query-parameters | Create operations should not use query parameters. http://go/ipa/xxx | warn | +| xgen-IPA-106-create-method-request-body-is-get-method-response | The Create method request should be a Get method response. http://go/ipa/106 | warn | ### IPA-109 From 15b9eb31182d7c09d318896c23f0d1b9f2a728f4 Mon Sep 17 00:00:00 2001 From: Yeliz Henden Date: Mon, 10 Mar 2025 13:26:20 +0000 Subject: [PATCH 7/8] address the comments --- .../ipa/rulesets/functions/utils/resourceEvaluation.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js index 390cc8e9e3..c2abacf73e 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js +++ b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js @@ -172,9 +172,7 @@ export function getResponseOfGetMethodByMediaType(mediaType, pathForResourceColl return null; } - const singleResourcePath = resourcePaths.find( - (path) => !isCustomMethodIdentifier(path) && path !== pathForResourceCollection - ); + const singleResourcePath = resourcePaths.find((path) => isSingleResourceIdentifier(path)); if (!singleResourcePath) { return null; } From debc2ec68eb76a03939e5b94d4f18b8f316a74c9 Mon Sep 17 00:00:00 2001 From: Yeliz Henden Date: Mon, 10 Mar 2025 15:35:46 +0000 Subject: [PATCH 8/8] address the comments --- ...eateMethodRequestBodyIsGetResponse.test.js | 136 ++++++++++++++++++ 1 file changed, 136 insertions(+) diff --git a/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js b/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js index 0df758efb7..10927b9265 100644 --- a/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js +++ b/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js @@ -38,6 +38,52 @@ const componentSchemas = { }, }, }, + SchemaCircularOne: { + type: 'object', + properties: { + thing: { + $ref: '#/components/schemas/SchemaCircularTwo', + }, + }, + }, + SchemaCircularTwo: { + type: 'object', + properties: { + otherThing: { + $ref: '#/components/schemas/SchemaCircularOne', + }, + }, + }, + }, +}; + +const animals = { + schemas: { + Animal: { + type: 'object', + oneOf: [ + { + $ref: '#/components/schemas/Dog', + }, + { + $ref: '#/components/schemas/Cat', + }, + ], + }, + Dog: { + allOf: [ + { + $ref: '#/components/schemas/Animal', + }, + ], + }, + Cat: { + allOf: [ + { + $ref: '#/components/schemas/Animal', + }, + ], + }, }, }; @@ -272,6 +318,39 @@ testRule('xgen-IPA-106-create-method-request-body-is-get-method-response', [ }, }, }, + '/resourceCircular': { + post: { + requestBody: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaCircularOne', + }, + }, + 'application/vnd.atlas.2024-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaCircularOne', + }, + }, + }, + }, + }, + }, + '/resourceCircular/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/SchemaCircularTwo', + }, + }, + }, + }, + }, + }, + }, }, }, errors: [ @@ -296,6 +375,63 @@ 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: + '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.2023-01-01+json'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid oneOf case', + document: { + components: animals, + paths: { + '/animalResource': { + post: { + requestBody: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/Animal', + }, + }, + 'application/vnd.atlas.2024-01-01+json': { + schema: { + $ref: '#/components/schemas/Animal', + }, + }, + }, + }, + }, + }, + '/animalResource/{id}': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2023-01-01+json': { + schema: { + $ref: '#/components/schemas/Dog', + }, + }, + }, + }, + }, + }, + }, + }, + }, + errors: [ + { + 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.2023-01-01+json'], + severity: DiagnosticSeverity.Warning, + }, ], }, {