diff --git a/package-lock.json b/package-lock.json index 35f6382a80..ef76f7833e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15,9 +15,7 @@ "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" }, @@ -9894,18 +9892,6 @@ "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 1871593c7d..d947ef1d08 100644 --- a/package.json +++ b/package.json @@ -30,9 +30,7 @@ "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 10927b9265..b320736887 100644 --- a/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js +++ b/tools/spectral/ipa/__tests__/createMethodRequestBodyIsGetResponse.test.js @@ -99,7 +99,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/SchemaOne', + type: 'string', }, }, 'application/vnd.atlas.2024-01-01+json': { diff --git a/tools/spectral/ipa/__tests__/utils/compareUtils.test.js b/tools/spectral/ipa/__tests__/utils/compareUtils.test.js new file mode 100644 index 0000000000..3b722616a1 --- /dev/null +++ b/tools/spectral/ipa/__tests__/utils/compareUtils.test.js @@ -0,0 +1,208 @@ +import { describe, expect, it } from '@jest/globals'; +import { isDeepEqual, omitDeep } from '../../rulesets/functions/utils/compareUtils'; + +describe('isDeepEqual', () => { + it('handles primitive values', () => { + expect(isDeepEqual(1, 1)).toBe(true); + expect(isDeepEqual('hello', 'hello')).toBe(true); + expect(isDeepEqual(true, true)).toBe(true); + expect(isDeepEqual(null, null)).toBe(true); + expect(isDeepEqual(undefined, undefined)).toBe(true); + + expect(isDeepEqual(1, 2)).toBe(false); + expect(isDeepEqual('hello', 'world')).toBe(false); + expect(isDeepEqual(true, false)).toBe(false); + expect(isDeepEqual(null, undefined)).toBe(false); + expect(isDeepEqual(1, '1')).toBe(false); + }); + + it('handles simple objects', () => { + expect(isDeepEqual({}, {})).toBe(true); + expect(isDeepEqual({ a: 1 }, { a: 1 })).toBe(true); + expect(isDeepEqual({ a: 1, b: 2 }, { a: 1, b: 2 })).toBe(true); + + expect(isDeepEqual({ a: 1 }, { a: 2 })).toBe(false); + expect(isDeepEqual({ a: 1 }, { b: 1 })).toBe(false); + expect(isDeepEqual({ a: 1 }, { a: 1, b: 2 })).toBe(false); + }); + + it('handles arrays', () => { + expect(isDeepEqual([], [])).toBe(true); + expect(isDeepEqual([1, 2], [1, 2])).toBe(true); + + expect(isDeepEqual([1, 2], [2, 1])).toBe(false); + expect(isDeepEqual([1, 2], [1, 2, 3])).toBe(false); + }); + + it('handles nested objects', () => { + expect(isDeepEqual({ a: 1, b: { c: 2 } }, { a: 1, b: { c: 2 } })).toBe(true); + + expect(isDeepEqual({ a: 1, b: { c: 2 } }, { a: 1, b: { c: 3 } })).toBe(false); + + expect(isDeepEqual({ a: 1, b: { c: 2, d: 3 } }, { a: 1, b: { c: 2 } })).toBe(false); + }); + + it('handles nested arrays', () => { + expect(isDeepEqual({ a: [1, 2, { b: 3 }] }, { a: [1, 2, { b: 3 }] })).toBe(true); + + expect(isDeepEqual({ a: [1, 2, { b: 3 }] }, { a: [1, 2, { b: 4 }] })).toBe(false); + }); + + it('handles mixed types', () => { + expect(isDeepEqual({ a: 1 }, [1])).toBe(false); + expect(isDeepEqual({ a: 1 }, null)).toBe(false); + expect(isDeepEqual(null, { a: 1 })).toBe(false); + }); +}); + +describe('omitDeep', () => { + it('handles primitives', () => { + expect(omitDeep(1, 'any')).toBe(1); + expect(omitDeep('hello', 'any')).toBe('hello'); + expect(omitDeep(null, 'any')).toBe(null); + expect(omitDeep(undefined, 'any')).toBe(undefined); + }); + + it('handles shallow objects', () => { + expect(omitDeep({ a: 1, b: 2 }, 'a')).toEqual({ b: 2 }); + expect(omitDeep({ a: 1, b: 2 }, 'c')).toEqual({ a: 1, b: 2 }); + expect(omitDeep({ a: 1, b: 2 }, 'a', 'b')).toEqual({}); + }); + + it('handles arrays', () => { + expect( + omitDeep( + [ + { a: 1, b: 2 }, + { a: 3, b: 4 }, + ], + 'a' + ) + ).toEqual([{ b: 2 }, { b: 4 }]); + }); + + it('handles nested objects', () => { + const input = { + a: 1, + b: { + c: 2, + d: 3, + e: { + f: 4, + g: 5, + }, + }, + h: 6, + }; + + const expected = { + a: 1, + b: { + d: 3, + e: { + g: 5, + }, + }, + h: 6, + }; + + expect(omitDeep(input, 'c', 'f')).toEqual(expected); + }); + + it('handles deeply nested arrays', () => { + const input = { + items: [ + { id: 1, name: 'item1', metadata: { created: '2023', readOnly: true } }, + { id: 2, name: 'item2', metadata: { created: '2023', readOnly: true } }, + ], + }; + + const expected = { + items: [ + { id: 1, name: 'item1', metadata: { created: '2023' } }, + { id: 2, name: 'item2', metadata: { created: '2023' } }, + ], + }; + + expect(omitDeep(input, 'readOnly')).toEqual(expected); + }); + + it('handles complex schemas', () => { + const schema = { + type: 'object', + properties: { + name: { type: 'string' }, + id: { type: 'string', readOnly: true }, + details: { + type: 'object', + properties: { + createdAt: { type: 'string', readOnly: true }, + description: { type: 'string' }, + }, + }, + items: { + type: 'array', + items: { + type: 'object', + properties: { + itemId: { type: 'string', readOnly: true }, + itemName: { type: 'string' }, + }, + }, + }, + }, + }; + + const expected = { + type: 'object', + properties: { + name: { type: 'string' }, + id: { type: 'string' }, + details: { + type: 'object', + properties: { + createdAt: { type: 'string' }, + description: { type: 'string' }, + }, + }, + items: { + type: 'array', + items: { + type: 'object', + properties: { + itemId: { type: 'string' }, + itemName: { type: 'string' }, + }, + }, + }, + }, + }; + + expect(omitDeep(schema, 'readOnly')).toEqual(expected); + }); + + it('handles multiple keys to omit', () => { + const input = { + a: 1, + b: 2, + c: { + d: 3, + e: 4, + f: { + g: 5, + h: 6, + }, + }, + }; + + expect(omitDeep(input, 'a', 'e', 'g')).toEqual({ + b: 2, + c: { + d: 3, + f: { + h: 6, + }, + }, + }); + }); +}); diff --git a/tools/spectral/ipa/rulesets/IPA-106.yaml b/tools/spectral/ipa/rulesets/IPA-106.yaml index 238be3dbe9..97c8b0beef 100644 --- a/tools/spectral/ipa/rulesets/IPA-106.yaml +++ b/tools/spectral/ipa/rulesets/IPA-106.yaml @@ -16,17 +16,21 @@ rules: field: '@key' function: 'createMethodRequestBodyIsRequestSuffixedObject' xgen-IPA-106-create-method-should-not-have-query-parameters: - description: 'Create operations should not use query parameters. http://go/ipa/xxx' + description: 'Create operations should not use query parameters. http://go/ipa/106' message: '{{error}} http://go/ipa/106' severity: warn 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' + description: | + Request body content of the Create method and response content of the Get method should refer to the same resource. + readOnly/writeOnly properties will be ignored. http://go/ipa/106 message: '{{error}} http://go/ipa/106' severity: warn given: '$.paths[*].post.requestBody.content' then: field: '@key' function: 'createMethodRequestBodyIsGetResponse' + functionOptions: + ignoredValues: ['readOnly', 'writeOnly'] diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index ab7314dc90..d52041443f 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -52,11 +52,13 @@ For rule definitions, see [IPA-105.yaml](https://github.com/mongodb/openapi/blob For rule definitions, see [IPA-106.yaml](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/IPA-106.yaml). -| Rule Name | Description | Severity | -| ------------------------------------------------------------------ | -------------------------------------------------------------------------------- | -------- | -| 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 | +| Rule Name | Description | Severity | +| ------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | +| 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/106 | warn | +| xgen-IPA-106-create-method-request-body-is-get-method-response | Request body content of the Create method and response content of the Get method should refer to the same resource. +readOnly/writeOnly properties will be ignored. http://go/ipa/106 + | warn | ### IPA-108 diff --git a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js index 3ade0cfdb5..7fae75826f 100644 --- a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js +++ b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsGetResponse.js @@ -1,7 +1,6 @@ import { getResponseOfGetMethodByMediaType, isCustomMethodIdentifier } from './utils/resourceEvaluation.js'; import { resolveObject } from './utils/componentUtils.js'; -import { isEqual } from 'lodash'; -import omitDeep from 'omit-deep-lodash'; +import { isDeepEqual, omitDeep } from './utils/compareUtils.js'; import { hasException } from './utils/exceptions.js'; import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; @@ -9,7 +8,7 @@ const RULE_NAME = 'xgen-IPA-106-create-method-request-body-is-get-method-respons const ERROR_MESSAGE = 'The request body schema properties must match the response body schema properties of the Get method.'; -export default (input, _, { path, documentInventory }) => { +export default (input, opts, { path, documentInventory }) => { const oas = documentInventory.resolved; const resourcePath = path[1]; let mediaType = input; @@ -34,7 +33,8 @@ export default (input, _, { path, documentInventory }) => { const errors = checkViolationsAndReturnErrors( path, postMethodRequestContentPerMediaType, - getMethodResponseContentPerMediaType + getMethodResponseContentPerMediaType, + opts ); if (errors.length !== 0) { @@ -47,14 +47,16 @@ export default (input, _, { path, documentInventory }) => { function checkViolationsAndReturnErrors( path, postMethodRequestContentPerMediaType, - getMethodResponseContentPerMediaType + getMethodResponseContentPerMediaType, + opts ) { const errors = []; + const ignoredValues = opts?.ignoredValues || []; if ( - !isEqual( - omitDeep(postMethodRequestContentPerMediaType.schema, 'readOnly', 'writeOnly'), - omitDeep(getMethodResponseContentPerMediaType.schema, 'readOnly', 'writeOnly') + !isDeepEqual( + omitDeep(postMethodRequestContentPerMediaType.schema, ...ignoredValues), + omitDeep(getMethodResponseContentPerMediaType.schema, ...ignoredValues) ) ) { errors.push({ diff --git a/tools/spectral/ipa/rulesets/functions/utils/compareUtils.js b/tools/spectral/ipa/rulesets/functions/utils/compareUtils.js new file mode 100644 index 0000000000..57d20ee327 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/utils/compareUtils.js @@ -0,0 +1,64 @@ +// utils/compareUtils.js + +/** + * Deep equality check between two values + * Does not handle circular references + * @param {*} value1 First value to compare + * @param {*} value2 Second value to compare + * @returns {boolean} Whether the values are deeply equal + */ +export function isDeepEqual(value1, value2) { + // If the values are strictly equal (including handling null/undefined) + if (value1 === value2) return true; + + // If either value is null or not an object, they're not equal (we already checked strict equality) + if (value1 == null || value2 == null || typeof value1 !== 'object' || typeof value2 !== 'object') { + return false; + } + + const keys1 = Object.keys(value1); + const keys2 = Object.keys(value2); + + // Different number of properties + if (keys1.length !== keys2.length) return false; + + // Check that all properties in value1 exist in value2 and are equal + for (const key of keys1) { + if (!keys2.includes(key)) return false; + + // Recursive equality check for nested objects + if (!isDeepEqual(value1[key], value2[key])) return false; + } + + return true; +} + +/** + * Deep clone an object while omitting specific properties + * @param {object} obj Object to clone and omit properties from + * @param {...string} keys Properties to omit + * @returns {object} New object without the specified properties + */ +export function omitDeep(obj, ...keys) { + if (!obj || typeof obj !== 'object') return obj; + + // Handle arrays + if (Array.isArray(obj)) { + return obj.map((item) => omitDeep(item, ...keys)); + } + + // Handle regular objects + return Object.entries(obj).reduce((result, [key, value]) => { + // Skip properties that should be omitted + if (keys.includes(key)) return result; + + // Handle nested objects/arrays recursively + if (value && typeof value === 'object') { + result[key] = omitDeep(value, ...keys); + } else { + result[key] = value; + } + + return result; + }, {}); +}