From 2ee94dc0ff5c495454836439b91806c1ec681158 Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Mon, 3 Mar 2025 12:25:42 +0000 Subject: [PATCH 01/10] CLOUDP-303615: IPA - Update check if resource is singleton/standard --- .../eachResourceHasGetMethod.test.js | 9 +- .../getMethodReturnsSingleResource.test.js | 12 +- .../ipa/__tests__/singletonHasNoId.test.js | 16 +- .../utils/resourceEvaluation.test.js | 228 +++++++++++++----- ...ternatesBetweenResourceNameAndPathParam.js | 3 +- .../functions/eachResourceHasGetMethod.js | 7 +- .../getMethodReturnsSingleResource.js | 4 +- .../rulesets/functions/singletonHasNoId.js | 4 +- .../functions/utils/resourceEvaluation.js | 76 ++++-- 9 files changed, 253 insertions(+), 106 deletions(-) diff --git a/tools/spectral/ipa/__tests__/eachResourceHasGetMethod.test.js b/tools/spectral/ipa/__tests__/eachResourceHasGetMethod.test.js index b7f77abc08..b3faefdca5 100644 --- a/tools/spectral/ipa/__tests__/eachResourceHasGetMethod.test.js +++ b/tools/spectral/ipa/__tests__/eachResourceHasGetMethod.test.js @@ -43,9 +43,6 @@ testRule('xgen-IPA-104-resource-has-GET', [ '/custom:method': { post: {}, }, - '/singleton': { - get: {}, - }, }, }, errors: [], @@ -87,8 +84,8 @@ testRule('xgen-IPA-104-resource-has-GET', [ '/custom:method': { post: {}, }, - '/singleton': { - patch: {}, + '/standardWithoutSubResource': { + get: {}, }, }, }, @@ -120,7 +117,7 @@ testRule('xgen-IPA-104-resource-has-GET', [ { code: 'xgen-IPA-104-resource-has-GET', message: 'APIs must provide a get method for resources. http://go/ipa/104', - path: ['paths', '/singleton'], + path: ['paths', '/standardWithoutSubResource'], severity: DiagnosticSeverity.Warning, }, ], diff --git a/tools/spectral/ipa/__tests__/getMethodReturnsSingleResource.test.js b/tools/spectral/ipa/__tests__/getMethodReturnsSingleResource.test.js index 118f3745f5..302e606ef0 100644 --- a/tools/spectral/ipa/__tests__/getMethodReturnsSingleResource.test.js +++ b/tools/spectral/ipa/__tests__/getMethodReturnsSingleResource.test.js @@ -73,7 +73,7 @@ testRule('xgen-IPA-104-get-method-returns-single-resource', [ }, }, }, - '/singleton': { + '/resource/{id}/singleton': { get: { responses: { 200: { @@ -127,7 +127,7 @@ testRule('xgen-IPA-104-get-method-returns-single-resource', [ }, }, }, - '/arraySingleton': { + '/resource/{id}/arraySingleton': { get: { responses: { 200: { @@ -142,7 +142,7 @@ testRule('xgen-IPA-104-get-method-returns-single-resource', [ }, }, }, - '/paginatedSingleton': { + '/resource/{id}/paginatedSingleton': { get: { responses: { 200: { @@ -197,7 +197,7 @@ testRule('xgen-IPA-104-get-method-returns-single-resource', [ 'Get methods should return data for a single resource. This method returns an array or a paginated response. http://go/ipa/104', path: [ 'paths', - '/arraySingleton', + '/resource/{id}/arraySingleton', 'get', 'responses', '200', @@ -212,7 +212,7 @@ testRule('xgen-IPA-104-get-method-returns-single-resource', [ 'Get methods should return data for a single resource. This method returns an array or a paginated response. http://go/ipa/104', path: [ 'paths', - '/paginatedSingleton', + '/resource/{id}/paginatedSingleton', 'get', 'responses', '200', @@ -245,7 +245,7 @@ testRule('xgen-IPA-104-get-method-returns-single-resource', [ }, }, }, - '/paginatedSingleton': { + '/resource/{id}/paginatedSingleton': { get: { responses: { 200: { diff --git a/tools/spectral/ipa/__tests__/singletonHasNoId.test.js b/tools/spectral/ipa/__tests__/singletonHasNoId.test.js index dc448a8e05..716ca78a52 100644 --- a/tools/spectral/ipa/__tests__/singletonHasNoId.test.js +++ b/tools/spectral/ipa/__tests__/singletonHasNoId.test.js @@ -47,7 +47,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ patch: {}, delete: {}, }, - '/singleton1': { + '/standard/{exampleId}/singleton1': { get: { responses: { 200: { @@ -65,7 +65,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ }, }, }, - '/singleton2': { + '/standard/{exampleId}/singleton2': { get: { responses: { 200: { @@ -92,7 +92,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ name: 'invalid resources', document: { paths: { - '/singleton1': { + '/standard/{exampleId}/singleton1': { get: { responses: { 200: { @@ -111,7 +111,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ }, }, }, - '/singleton2': { + '/standard/{exampleId}/singleton2': { get: { responses: { 200: { @@ -130,7 +130,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ }, }, }, - '/singleton3': { + '/standard/{exampleId}/singleton3': { get: { responses: { 200: { @@ -164,19 +164,19 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ { code: 'xgen-IPA-113-singleton-must-not-have-id', message: 'Singleton resources must not have a user-provided or system-generated ID. http://go/ipa/113', - path: ['paths', '/singleton1'], + path: ['paths', '/standard/{exampleId}/singleton1'], severity: DiagnosticSeverity.Warning, }, { code: 'xgen-IPA-113-singleton-must-not-have-id', message: 'Singleton resources must not have a user-provided or system-generated ID. http://go/ipa/113', - path: ['paths', '/singleton2'], + path: ['paths', '/standard/{exampleId}/singleton2'], severity: DiagnosticSeverity.Warning, }, { code: 'xgen-IPA-113-singleton-must-not-have-id', message: 'Singleton resources must not have a user-provided or system-generated ID. http://go/ipa/113', - path: ['paths', '/singleton3'], + path: ['paths', '/standard/{exampleId}/singleton3'], severity: DiagnosticSeverity.Warning, }, ], diff --git a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js index 9ca9913787..10f4841d5d 100644 --- a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js +++ b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js @@ -5,84 +5,186 @@ import { isStandardResource, } from '../../rulesets/functions/utils/resourceEvaluation'; -const standardResourcePaths = ['/standard', '/standard/{id}']; +const standardResourcePaths = ['/standard', '/standard/{exampleId}']; const nestedStandardResourcePaths = ['/standard/{exampleId}/nested', '/standard/{exampleId}/nested/{exampleId}']; -const standardResourceWithCustomPaths = ['/customStandard', '/customStandard/{id}', '/customStandard:method']; +const standardResourceWithCustomPaths = ['/customStandard', '/customStandard/{exampleId}', '/customStandard:method']; -const singletonResourcePaths = ['/singleton']; +const standardResourceMissingSubPath = ['/standardMissingSub']; -const singletonResourceWithCustomPaths = ['/customSingleton', '/customSingleton:method']; +const standardResourceMissingSubPathInvalidFormat = ['/standard/nestedStandardMissingSub']; -const nestedSingletonResourcePaths = ['/standard/{exampleId}/nestedSingleton']; +const standardResourcePathsInvalidFormat = ['/resource1/resource2', '/resource1/resource2/{exampleId}']; -describe('tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js', () => { - describe('getResourcePaths', () => { - it('returns the paths for a resource based on parent path', () => { - const allPaths = standardResourcePaths.concat( - nestedStandardResourcePaths, - standardResourceWithCustomPaths, - singletonResourcePaths, - singletonResourceWithCustomPaths, - nestedSingletonResourcePaths - ); - expect(getResourcePaths('/standard', allPaths)).toEqual(standardResourcePaths); - expect(getResourcePaths('/standard/{exampleId}/nested', allPaths)).toEqual(nestedStandardResourcePaths); - expect(getResourcePaths('/customStandard', allPaths)).toEqual(standardResourceWithCustomPaths); - expect(getResourcePaths('/singleton', allPaths)).toEqual(singletonResourcePaths); - expect(getResourcePaths('/customSingleton', allPaths)).toEqual(singletonResourceWithCustomPaths); - expect(getResourcePaths('/standard/{exampleId}/nestedSingleton', allPaths)).toEqual(nestedSingletonResourcePaths); - }); - }); - describe('isStandardResource', () => { - it('returns true for a standard resource', () => { - expect(isStandardResource(standardResourcePaths)).toBe(true); - }); +const singletonResourcePaths = ['/standard/{exampleId}/singleton']; - it('returns true for a standard resource with custom methods', () => { - expect(isStandardResource(standardResourceWithCustomPaths)).toBe(true); - }); +const singletonResourceWithCustomPaths = [ + '/standard/{exampleId}/customSingleton', + '/standard/{exampleId}/customSingleton:method', +]; - it('returns true for a nested standard resource', () => { - expect(isStandardResource(nestedStandardResourcePaths)).toBe(true); - }); - - it('returns false for a singleton resource', () => { - expect(isStandardResource(singletonResourcePaths)).toBe(false); - }); +const allPaths = standardResourcePaths.concat( + nestedStandardResourcePaths, + standardResourceWithCustomPaths, + standardResourceMissingSubPath, + standardResourceMissingSubPathInvalidFormat, + standardResourcePathsInvalidFormat, + singletonResourcePaths, + singletonResourceWithCustomPaths +); - it('returns false for a singleton resource with custom methods', () => { - expect(isStandardResource(singletonResourceWithCustomPaths)).toBe(false); - }); - - it('returns false for a nested singleton resource', () => { - expect(isStandardResource(nestedSingletonResourcePaths)).toBe(false); +describe('tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js', () => { + describe('getResourcePaths', () => { + const testCases = [ + { + description: 'standard resource', + resourceCollectionPath: '/standard', + expectedPaths: standardResourcePaths, + }, + { + description: 'nested standard resource', + resourceCollectionPath: '/standard/{exampleId}/nested', + expectedPaths: nestedStandardResourcePaths, + }, + { + description: 'standard resource with custom methods', + resourceCollectionPath: '/customStandard', + expectedPaths: standardResourceWithCustomPaths, + }, + { + description: 'standard resource with missing sub-path', + resourceCollectionPath: '/standardMissingSub', + expectedPaths: standardResourceMissingSubPath, + }, + { + description: 'nested standard resource with missing sub-path', + resourceCollectionPath: '/standard/nestedStandardMissingSub', + expectedPaths: standardResourceMissingSubPathInvalidFormat, + }, + { + description: 'standard resource with missing sub-path', + resourceCollectionPath: '/resource1/resource2', + expectedPaths: standardResourcePathsInvalidFormat, + }, + { + description: 'singleton resource', + resourceCollectionPath: '/standard/{exampleId}/singleton', + expectedPaths: singletonResourcePaths, + }, + { + description: 'singleton resource with custom methods', + resourceCollectionPath: '/standard/{exampleId}/customSingleton', + expectedPaths: singletonResourceWithCustomPaths, + }, + ]; + + testCases.forEach((testCase) => { + it(`returns the expected paths for ${testCase.description}`, () => { + expect(getResourcePaths(testCase.resourceCollectionPath, allPaths)).toEqual(testCase.expectedPaths); + }); }); }); - describe('isSingletonResource', () => { - it('returns true for a singleton resource', () => { - expect(isSingletonResource(singletonResourcePaths)).toBe(true); - }); - - it('returns true for a singleton resource with custom methods', () => { - expect(isSingletonResource(singletonResourceWithCustomPaths)).toBe(true); - }); - - it('returns true for a nested singleton resource', () => { - expect(isSingletonResource(nestedSingletonResourcePaths)).toBe(true); - }); - - it('returns false for a standard resource', () => { - expect(isSingletonResource(standardResourcePaths)).toBe(false); - }); - it('returns false for a standard resource with custom methods', () => { - expect(isSingletonResource(standardResourceWithCustomPaths)).toBe(false); + describe('isStandardResource', () => { + const testCases = [ + { + description: 'standard resource', + resourcePaths: standardResourcePaths, + isStandardResource: true, + }, + { + description: 'standard resource with custom methods', + resourcePaths: standardResourceWithCustomPaths, + isStandardResource: true, + }, + { + description: 'standard resource with missing sub-path', + resourcePaths: standardResourceMissingSubPath, + isStandardResource: true, + }, + { + description: 'nested standard resource with missing sub-path', + resourcePaths: standardResourceMissingSubPathInvalidFormat, + isStandardResource: true, + }, + { + description: 'nested standard resource', + resourcePaths: nestedStandardResourcePaths, + isStandardResource: true, + }, + { + description: 'singleton resource', + resourcePaths: singletonResourcePaths, + isStandardResource: false, + }, + { + description: 'singleton resource with custom methods', + resourcePaths: singletonResourceWithCustomPaths, + isStandardResource: false, + }, + { + description: 'invalid path format', + resourcePaths: standardResourcePathsInvalidFormat, + isStandardResource: true, + }, + ]; + + testCases.forEach((testCase) => { + it(`returns ${testCase.isStandardResource} for ${testCase.description}`, () => { + expect(isStandardResource(testCase.resourcePaths)).toEqual(testCase.isStandardResource); + }); }); + }); - it('returns false for a nested standard resource', () => { - expect(isSingletonResource(nestedStandardResourcePaths)).toBe(false); + describe('isSingletonResource', () => { + const testCases = [ + { + description: 'standard resource', + resourcePaths: standardResourcePaths, + isSingletonResource: false, + }, + { + description: 'standard resource with custom methods', + resourcePaths: standardResourceWithCustomPaths, + isSingletonResource: false, + }, + { + description: 'standard resource with missing sub-path', + resourcePaths: standardResourceMissingSubPath, + isSingletonResource: false, + }, + { + description: 'nested standard resource with missing sub-path', + resourcePaths: standardResourceMissingSubPathInvalidFormat, + isSingletonResource: false, + }, + { + description: 'invalid path format', + resourcePaths: standardResourcePathsInvalidFormat, + isSingletonResource: false, + }, + { + description: 'nested standard resource', + resourcePaths: nestedStandardResourcePaths, + isSingletonResource: false, + }, + { + description: 'singleton resource', + resourcePaths: singletonResourcePaths, + isSingletonResource: true, + }, + { + description: 'singleton resource with custom methods', + resourcePaths: singletonResourceWithCustomPaths, + isSingletonResource: true, + }, + ]; + + testCases.forEach((testCase) => { + it(`returns ${testCase.isSingletonResource} for ${testCase.description}`, () => { + expect(isSingletonResource(testCase.resourcePaths)).toEqual(testCase.isSingletonResource); + }); }); }); }); diff --git a/tools/spectral/ipa/rulesets/functions/eachPathAlternatesBetweenResourceNameAndPathParam.js b/tools/spectral/ipa/rulesets/functions/eachPathAlternatesBetweenResourceNameAndPathParam.js index d432f9686f..b1c48408b9 100644 --- a/tools/spectral/ipa/rulesets/functions/eachPathAlternatesBetweenResourceNameAndPathParam.js +++ b/tools/spectral/ipa/rulesets/functions/eachPathAlternatesBetweenResourceNameAndPathParam.js @@ -1,11 +1,10 @@ import { isPathParam } from './utils/componentUtils.js'; import { hasException } from './utils/exceptions.js'; import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; +import { AUTH_PREFIX, UNAUTH_PREFIX } from './utils/resourceEvaluation.js'; const RULE_NAME = 'xgen-IPA-102-path-alternate-resource-name-path-param'; const ERROR_MESSAGE = 'API paths must alternate between resource name and path params.'; -const AUTH_PREFIX = '/api/atlas/v2'; -const UNAUTH_PREFIX = '/api/atlas/v2/unauth'; const getPrefix = (path) => { if (path.includes(UNAUTH_PREFIX)) { diff --git a/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js b/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js index 34388b7bf6..ef3ff1661c 100644 --- a/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js +++ b/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js @@ -1,6 +1,6 @@ import { hasGetMethod, - isChild, + isSingleResource, isCustomMethod, isStandardResource, isSingletonResource, @@ -13,7 +13,7 @@ const RULE_NAME = 'xgen-IPA-104-resource-has-GET'; const ERROR_MESSAGE = 'APIs must provide a get method for resources.'; export default (input, _, { path, documentInventory }) => { - if (isChild(input) || isCustomMethod(input)) { + if (isSingleResource(input) || isCustomMethod(input)) { return; } @@ -31,6 +31,9 @@ export default (input, _, { path, documentInventory }) => { return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); } } else if (isStandardResource(resourcePaths)) { + if (resourcePaths.length === 1) { + return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); + } if (!hasGetMethod(oas.paths[resourcePaths[1]])) { return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); } diff --git a/tools/spectral/ipa/rulesets/functions/getMethodReturnsSingleResource.js b/tools/spectral/ipa/rulesets/functions/getMethodReturnsSingleResource.js index d51e2695fb..935bb82cf0 100644 --- a/tools/spectral/ipa/rulesets/functions/getMethodReturnsSingleResource.js +++ b/tools/spectral/ipa/rulesets/functions/getMethodReturnsSingleResource.js @@ -1,4 +1,4 @@ -import { isChild, isCustomMethod, isSingletonResource, getResourcePaths } from './utils/resourceEvaluation.js'; +import { isSingleResource, isCustomMethod, isSingletonResource, getResourcePaths } from './utils/resourceEvaluation.js'; import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; import { getAllSuccessfulResponseSchemas } from './utils/methodUtils.js'; import { hasException } from './utils/exceptions.js'; @@ -14,7 +14,7 @@ export default (input, _, { path, documentInventory }) => { const resourcePath = path[1]; const resourcePaths = getResourcePaths(resourcePath, Object.keys(oas.paths)); - if (isCustomMethod(resourcePath) || (!isChild(resourcePath) && !isSingletonResource(resourcePaths))) { + if (isCustomMethod(resourcePath) || (!isSingleResource(resourcePath) && !isSingletonResource(resourcePaths))) { return; } diff --git a/tools/spectral/ipa/rulesets/functions/singletonHasNoId.js b/tools/spectral/ipa/rulesets/functions/singletonHasNoId.js index 29ef51b297..fec427f5db 100644 --- a/tools/spectral/ipa/rulesets/functions/singletonHasNoId.js +++ b/tools/spectral/ipa/rulesets/functions/singletonHasNoId.js @@ -1,7 +1,7 @@ import { getResourcePaths, hasGetMethod, - isChild, + isSingleResource, isCustomMethod, isSingletonResource, } from './utils/resourceEvaluation.js'; @@ -15,7 +15,7 @@ const ERROR_MESSAGE = 'Singleton resources must not have a user-provided or syst export default (input, opts, { path, documentInventory }) => { const resourcePath = path[1]; - if (isCustomMethod(resourcePath) || isChild(resourcePath)) { + if (isCustomMethod(resourcePath) || isSingleResource(resourcePath)) { return; } diff --git a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js index e4d9147a27..26a874b88a 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js +++ b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js @@ -1,4 +1,7 @@ -export function isChild(path) { +export const AUTH_PREFIX = '/api/atlas/v2'; +export const UNAUTH_PREFIX = '/api/atlas/v2/unauth'; + +export function isSingleResource(path) { return path.endsWith('}'); } @@ -19,7 +22,7 @@ export function getCustomMethodName(path) { */ export function isSingletonResource(resourcePaths) { if (resourcePaths.length === 1) { - return true; + return resourceBelongsToSingleParent(resourcePaths[0]); } const additionalPaths = resourcePaths.slice(1); return additionalPaths.every(isCustomMethod); @@ -33,11 +36,11 @@ export function isSingletonResource(resourcePaths) { * @returns {boolean} */ export function isStandardResource(resourcePaths) { - if (resourcePaths.length === 2 && isChild(resourcePaths[1])) { - return true; + if (resourcePaths.length === 1) { + return !resourceBelongsToSingleParent(resourcePaths[0]); } - if (resourcePaths.length < 3 || !isChild(resourcePaths[1])) { - return false; + if (resourcePaths.length === 2) { + return isSingleResource(resourcePaths[1]); } const additionalPaths = resourcePaths.slice(2); return additionalPaths.every(isCustomMethod); @@ -54,17 +57,60 @@ export function hasGetMethod(pathObject) { } /** - * Get all paths for a resource based on the parent path + * Get all paths for a resource based on the path for the resource collection + * For example, resource collection path '/resource' may return ['/resource', '/resource{id}', '/resource{id}:customMethod'] * - * @param parent the parent path string - * @param allPaths all paths as an array of strings - * @returns {string[]} all paths for a resource, including the parent + * @param {string} resourceCollectionPath the path string + * @param {Array} allPaths all paths + * @returns {string[]} all paths for a resource, including the path for the resource collection */ -export function getResourcePaths(parent, allPaths) { - const childPathPattern = new RegExp(`^${parent}/{[a-zA-Z]+}$`); - const customChildMethodPattern = new RegExp(`^${parent}/{[a-zA-Z]+}:+[a-zA-Z]+$`); - const customMethodPattern = new RegExp(`^${parent}:+[a-zA-Z]+$`); +export function getResourcePaths(resourceCollectionPath, allPaths) { + const singleResourcePathPattern = new RegExp(`^${resourceCollectionPath}/{[a-zA-Z]+}$`); + const singleResourceCustomMethodPattern = new RegExp(`^${resourceCollectionPath}/{[a-zA-Z]+}:+[a-zA-Z]+$`); + const customMethodPattern = new RegExp(`^${resourceCollectionPath}:+[a-zA-Z]+$`); return allPaths.filter( - (p) => parent === p || childPathPattern.test(p) || customMethodPattern.test(p) || customChildMethodPattern.test(p) + (p) => + resourceCollectionPath === p || + singleResourcePathPattern.test(p) || + customMethodPattern.test(p) || + singleResourceCustomMethodPattern.test(p) ); } + +/** + * Checks whether a resource belongs to one parent resource. + * For example, '/resource' returns false, '/resource/{id}/child' returns true. + * + * @param {string} resourcePath a path for a resource + * @returns {boolean} + */ +function resourceBelongsToSingleParent(resourcePath) { + // Ignore /api/atlas/v2 and /api/atlas/v2/unauth + let path = resourcePath; + if (resourcePath.startsWith(AUTH_PREFIX)) { + path = resourcePath.slice(AUTH_PREFIX.length); + } + if (resourcePath.startsWith(UNAUTH_PREFIX)) { + path = resourcePath.slice(UNAUTH_PREFIX.length); + } + + if (path === '') { + return true; + } + + let resourcePathSections = path.split('/'); + if (resourcePathSections[0] === '') { + resourcePathSections.shift(); + } + if (resourcePathSections.length < 2) { + return false; + } + if (isSingleResource(resourcePathSections[resourcePathSections.length - 1])) { + resourcePathSections = resourcePathSections.slice(0, resourcePathSections.length - 2); + } + if (resourcePathSections.length === 1) { + return false; + } + const parentResourceSection = resourcePathSections[resourcePathSections.length - 2]; + return isSingleResource(parentResourceSection); +} From ec6324371b7845bdf122bd46f9dc5ab073079562 Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Mon, 3 Mar 2025 15:18:16 +0000 Subject: [PATCH 02/10] CLOUDP-303615: Address comments --- .../ipa/__tests__/singletonHasNoId.test.js | 22 ++++++++-------- .../utils/resourceEvaluation.test.js | 25 +++++++++++-------- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/tools/spectral/ipa/__tests__/singletonHasNoId.test.js b/tools/spectral/ipa/__tests__/singletonHasNoId.test.js index 716ca78a52..0864a2a2a5 100644 --- a/tools/spectral/ipa/__tests__/singletonHasNoId.test.js +++ b/tools/spectral/ipa/__tests__/singletonHasNoId.test.js @@ -6,7 +6,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ name: 'valid resources', document: { paths: { - '/standard': { + '/resource': { post: {}, get: { responses: { @@ -26,7 +26,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ }, }, }, - '/standard/{exampleId}': { + '/resource/{exampleId}': { get: { responses: { 200: { @@ -47,7 +47,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ patch: {}, delete: {}, }, - '/standard/{exampleId}/singleton1': { + '/resource/{exampleId}/singleton1': { get: { responses: { 200: { @@ -65,7 +65,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ }, }, }, - '/standard/{exampleId}/singleton2': { + '/resource/{exampleId}/singleton2': { get: { responses: { 200: { @@ -92,7 +92,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ name: 'invalid resources', document: { paths: { - '/standard/{exampleId}/singleton1': { + '/resource/{exampleId}/singleton1': { get: { responses: { 200: { @@ -111,7 +111,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ }, }, }, - '/standard/{exampleId}/singleton2': { + '/resource/{exampleId}/singleton2': { get: { responses: { 200: { @@ -130,7 +130,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ }, }, }, - '/standard/{exampleId}/singleton3': { + '/resource/{exampleId}/singleton3': { get: { responses: { 200: { @@ -164,19 +164,19 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ { code: 'xgen-IPA-113-singleton-must-not-have-id', message: 'Singleton resources must not have a user-provided or system-generated ID. http://go/ipa/113', - path: ['paths', '/standard/{exampleId}/singleton1'], + path: ['paths', '/resource/{exampleId}/singleton1'], severity: DiagnosticSeverity.Warning, }, { code: 'xgen-IPA-113-singleton-must-not-have-id', message: 'Singleton resources must not have a user-provided or system-generated ID. http://go/ipa/113', - path: ['paths', '/standard/{exampleId}/singleton2'], + path: ['paths', '/resource/{exampleId}/singleton2'], severity: DiagnosticSeverity.Warning, }, { code: 'xgen-IPA-113-singleton-must-not-have-id', message: 'Singleton resources must not have a user-provided or system-generated ID. http://go/ipa/113', - path: ['paths', '/standard/{exampleId}/singleton3'], + path: ['paths', '/resource/{exampleId}/singleton3'], severity: DiagnosticSeverity.Warning, }, ], @@ -185,7 +185,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ name: 'invalid resources with exceptions', document: { paths: { - '/singleton1': { + '/resource/{exampleId}/singleton1': { 'x-xgen-IPA-exception': { 'xgen-IPA-113-singleton-must-not-have-id': 'reason', }, diff --git a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js index 10f4841d5d..c52551de4b 100644 --- a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js +++ b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js @@ -5,11 +5,16 @@ import { isStandardResource, } from '../../rulesets/functions/utils/resourceEvaluation'; -const standardResourcePaths = ['/standard', '/standard/{exampleId}']; +const standardResourcePaths = ['/resource', '/resource/{exampleId}']; -const nestedStandardResourcePaths = ['/standard/{exampleId}/nested', '/standard/{exampleId}/nested/{exampleId}']; +const nestedStandardResourcePaths = ['/resource/{exampleId}/nested', '/resource/{exampleId}/nested/{exampleId}']; -const standardResourceWithCustomPaths = ['/customStandard', '/customStandard/{exampleId}', '/customStandard:method']; +const standardResourceWithCustomPaths = [ + '/customStandard', + '/customStandard/{exampleId}', + '/customStandard/{id}:method', + '/customStandard:method', +]; const standardResourceMissingSubPath = ['/standardMissingSub']; @@ -17,11 +22,11 @@ const standardResourceMissingSubPathInvalidFormat = ['/standard/nestedStandardMi const standardResourcePathsInvalidFormat = ['/resource1/resource2', '/resource1/resource2/{exampleId}']; -const singletonResourcePaths = ['/standard/{exampleId}/singleton']; +const singletonResourcePaths = ['/resource/{exampleId}/singleton']; const singletonResourceWithCustomPaths = [ - '/standard/{exampleId}/customSingleton', - '/standard/{exampleId}/customSingleton:method', + '/resource/{exampleId}/customSingleton', + '/resource/{exampleId}/customSingleton:method', ]; const allPaths = standardResourcePaths.concat( @@ -39,12 +44,12 @@ describe('tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js', () const testCases = [ { description: 'standard resource', - resourceCollectionPath: '/standard', + resourceCollectionPath: '/resource', expectedPaths: standardResourcePaths, }, { description: 'nested standard resource', - resourceCollectionPath: '/standard/{exampleId}/nested', + resourceCollectionPath: '/resource/{exampleId}/nested', expectedPaths: nestedStandardResourcePaths, }, { @@ -69,12 +74,12 @@ describe('tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js', () }, { description: 'singleton resource', - resourceCollectionPath: '/standard/{exampleId}/singleton', + resourceCollectionPath: '/resource/{exampleId}/singleton', expectedPaths: singletonResourcePaths, }, { description: 'singleton resource with custom methods', - resourceCollectionPath: '/standard/{exampleId}/customSingleton', + resourceCollectionPath: '/resource/{exampleId}/customSingleton', expectedPaths: singletonResourceWithCustomPaths, }, ]; From fdee054b35f209ff412c612c890174447e8b2b10 Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Tue, 4 Mar 2025 16:28:09 +0000 Subject: [PATCH 03/10] CLOUDP-303615: Test --- .../getMethodReturnsSingleResource.test.js | 30 +++++++ .../ipa/__tests__/singletonHasNoId.test.js | 14 ++-- .../utils/resourceEvaluation.test.js | 35 ++++---- .../eachCustomMethodMustBeGetOrPost.js | 6 +- .../eachCustomMethodMustUseCamelCase.js | 6 +- .../functions/eachResourceHasGetMethod.js | 8 +- .../getMethodReturnsSingleResource.js | 54 +++++++----- .../rulesets/functions/singletonHasNoId.js | 5 +- .../functions/utils/resourceEvaluation.js | 82 +++++++++++++++---- 9 files changed, 169 insertions(+), 71 deletions(-) diff --git a/tools/spectral/ipa/__tests__/getMethodReturnsSingleResource.test.js b/tools/spectral/ipa/__tests__/getMethodReturnsSingleResource.test.js index 302e606ef0..c532752e35 100644 --- a/tools/spectral/ipa/__tests__/getMethodReturnsSingleResource.test.js +++ b/tools/spectral/ipa/__tests__/getMethodReturnsSingleResource.test.js @@ -97,6 +97,21 @@ testRule('xgen-IPA-104-get-method-returns-single-resource', [ name: 'invalid resources', document: { paths: { + '/arrayResource': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/PaginatedSchema', + }, + }, + }, + }, + }, + }, + }, '/arrayResource/{id}': { get: { responses: { @@ -112,6 +127,21 @@ testRule('xgen-IPA-104-get-method-returns-single-resource', [ }, }, }, + '/paginatedResource': { + get: { + responses: { + 200: { + content: { + 'application/vnd.atlas.2024-08-05+json': { + schema: { + $ref: '#/components/schemas/PaginatedSchema', + }, + }, + }, + }, + }, + }, + }, '/paginatedResource/{id}': { get: { responses: { diff --git a/tools/spectral/ipa/__tests__/singletonHasNoId.test.js b/tools/spectral/ipa/__tests__/singletonHasNoId.test.js index 0864a2a2a5..5dd74866b5 100644 --- a/tools/spectral/ipa/__tests__/singletonHasNoId.test.js +++ b/tools/spectral/ipa/__tests__/singletonHasNoId.test.js @@ -92,7 +92,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ name: 'invalid resources', document: { paths: { - '/resource/{exampleId}/singleton1': { + '/resource/{exampleId}/singletonOne': { get: { responses: { 200: { @@ -111,7 +111,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ }, }, }, - '/resource/{exampleId}/singleton2': { + '/resource/{exampleId}/singletonTwo': { get: { responses: { 200: { @@ -130,7 +130,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ }, }, }, - '/resource/{exampleId}/singleton3': { + '/resource/{exampleId}/singletonThree': { get: { responses: { 200: { @@ -164,19 +164,19 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ { code: 'xgen-IPA-113-singleton-must-not-have-id', message: 'Singleton resources must not have a user-provided or system-generated ID. http://go/ipa/113', - path: ['paths', '/resource/{exampleId}/singleton1'], + path: ['paths', '/resource/{exampleId}/singletonOne'], severity: DiagnosticSeverity.Warning, }, { code: 'xgen-IPA-113-singleton-must-not-have-id', message: 'Singleton resources must not have a user-provided or system-generated ID. http://go/ipa/113', - path: ['paths', '/resource/{exampleId}/singleton2'], + path: ['paths', '/resource/{exampleId}/singletonTwo'], severity: DiagnosticSeverity.Warning, }, { code: 'xgen-IPA-113-singleton-must-not-have-id', message: 'Singleton resources must not have a user-provided or system-generated ID. http://go/ipa/113', - path: ['paths', '/resource/{exampleId}/singleton3'], + path: ['paths', '/resource/{exampleId}/singletonThree'], severity: DiagnosticSeverity.Warning, }, ], @@ -185,7 +185,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ name: 'invalid resources with exceptions', document: { paths: { - '/resource/{exampleId}/singleton1': { + '/resource/{exampleId}/singletonOne': { 'x-xgen-IPA-exception': { 'xgen-IPA-113-singleton-must-not-have-id': 'reason', }, diff --git a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js index c52551de4b..6f307e9b39 100644 --- a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js +++ b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js @@ -5,6 +5,7 @@ import { isStandardResource, } from '../../rulesets/functions/utils/resourceEvaluation'; +// Standard resources const standardResourcePaths = ['/resource', '/resource/{exampleId}']; const nestedStandardResourcePaths = ['/resource/{exampleId}/nested', '/resource/{exampleId}/nested/{exampleId}']; @@ -18,10 +19,7 @@ const standardResourceWithCustomPaths = [ const standardResourceMissingSubPath = ['/standardMissingSub']; -const standardResourceMissingSubPathInvalidFormat = ['/standard/nestedStandardMissingSub']; - -const standardResourcePathsInvalidFormat = ['/resource1/resource2', '/resource1/resource2/{exampleId}']; - +// Singleton resources const singletonResourcePaths = ['/resource/{exampleId}/singleton']; const singletonResourceWithCustomPaths = [ @@ -29,14 +27,19 @@ const singletonResourceWithCustomPaths = [ '/resource/{exampleId}/customSingleton:method', ]; +// Neither standard nor singleton resources +const resourceMissingSubPathInvalidFormat = ['/standard/nestedStandardMissingSub']; + +const resourcePathsInvalidFormat = ['/resourceOne/resourceTwo', '/resourceOne/resourceTwo/{exampleId}']; + const allPaths = standardResourcePaths.concat( nestedStandardResourcePaths, standardResourceWithCustomPaths, standardResourceMissingSubPath, - standardResourceMissingSubPathInvalidFormat, - standardResourcePathsInvalidFormat, singletonResourcePaths, - singletonResourceWithCustomPaths + singletonResourceWithCustomPaths, + resourceMissingSubPathInvalidFormat, + resourcePathsInvalidFormat ); describe('tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js', () => { @@ -65,12 +68,12 @@ describe('tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js', () { description: 'nested standard resource with missing sub-path', resourceCollectionPath: '/standard/nestedStandardMissingSub', - expectedPaths: standardResourceMissingSubPathInvalidFormat, + expectedPaths: resourceMissingSubPathInvalidFormat, }, { description: 'standard resource with missing sub-path', - resourceCollectionPath: '/resource1/resource2', - expectedPaths: standardResourcePathsInvalidFormat, + resourceCollectionPath: '/resourceOne/resourceTwo', + expectedPaths: resourcePathsInvalidFormat, }, { description: 'singleton resource', @@ -110,8 +113,8 @@ describe('tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js', () }, { description: 'nested standard resource with missing sub-path', - resourcePaths: standardResourceMissingSubPathInvalidFormat, - isStandardResource: true, + resourcePaths: resourceMissingSubPathInvalidFormat, + isStandardResource: false, }, { description: 'nested standard resource', @@ -130,8 +133,8 @@ describe('tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js', () }, { description: 'invalid path format', - resourcePaths: standardResourcePathsInvalidFormat, - isStandardResource: true, + resourcePaths: resourcePathsInvalidFormat, + isStandardResource: false, }, ]; @@ -161,12 +164,12 @@ describe('tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js', () }, { description: 'nested standard resource with missing sub-path', - resourcePaths: standardResourceMissingSubPathInvalidFormat, + resourcePaths: resourceMissingSubPathInvalidFormat, isSingletonResource: false, }, { description: 'invalid path format', - resourcePaths: standardResourcePathsInvalidFormat, + resourcePaths: resourcePathsInvalidFormat, isSingletonResource: false, }, { diff --git a/tools/spectral/ipa/rulesets/functions/eachCustomMethodMustBeGetOrPost.js b/tools/spectral/ipa/rulesets/functions/eachCustomMethodMustBeGetOrPost.js index 3327c2af31..976e1f0e28 100644 --- a/tools/spectral/ipa/rulesets/functions/eachCustomMethodMustBeGetOrPost.js +++ b/tools/spectral/ipa/rulesets/functions/eachCustomMethodMustBeGetOrPost.js @@ -1,4 +1,4 @@ -import { isCustomMethod } from './utils/resourceEvaluation.js'; +import { isCustomMethodIdentifier } from './utils/resourceEvaluation.js'; import { hasException } from './utils/exceptions.js'; import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; @@ -11,7 +11,9 @@ export default (input, opts, { path }) => { // Extract the path key (e.g., '/a/{exampleId}:method') from the JSONPath. let pathKey = path[1]; - if (!isCustomMethod(pathKey)) return; + if (!isCustomMethodIdentifier(pathKey)) { + return; + } if (hasException(input, RULE_NAME)) { collectException(input, RULE_NAME, path); diff --git a/tools/spectral/ipa/rulesets/functions/eachCustomMethodMustUseCamelCase.js b/tools/spectral/ipa/rulesets/functions/eachCustomMethodMustUseCamelCase.js index 2862054155..d445c4a6d1 100644 --- a/tools/spectral/ipa/rulesets/functions/eachCustomMethodMustUseCamelCase.js +++ b/tools/spectral/ipa/rulesets/functions/eachCustomMethodMustUseCamelCase.js @@ -1,4 +1,4 @@ -import { getCustomMethodName, isCustomMethod } from './utils/resourceEvaluation.js'; +import { getCustomMethodName, isCustomMethodIdentifier } from './utils/resourceEvaluation.js'; import { hasException } from './utils/exceptions.js'; import { casing } from '@stoplight/spectral-functions'; import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; @@ -9,7 +9,9 @@ export default (input, opts, { path }) => { // Extract the path key (e.g., '/a/{exampleId}:method') from the JSONPath. let pathKey = path[1]; - if (!isCustomMethod(pathKey)) return; + if (!isCustomMethodIdentifier(pathKey)) { + return; + } if (hasException(input, RULE_NAME)) { collectException(input, RULE_NAME, path); diff --git a/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js b/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js index ef3ff1661c..973e8880d1 100644 --- a/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js +++ b/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js @@ -1,10 +1,10 @@ import { hasGetMethod, - isSingleResource, - isCustomMethod, isStandardResource, isSingletonResource, getResourcePaths, + isResourceCollectionIdentifier, + isSingleResourceIdentifier, } from './utils/resourceEvaluation.js'; import { hasException } from './utils/exceptions.js'; import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; @@ -13,7 +13,7 @@ const RULE_NAME = 'xgen-IPA-104-resource-has-GET'; const ERROR_MESSAGE = 'APIs must provide a get method for resources.'; export default (input, _, { path, documentInventory }) => { - if (isSingleResource(input) || isCustomMethod(input)) { + if (!isResourceCollectionIdentifier(input)) { return; } @@ -31,7 +31,7 @@ export default (input, _, { path, documentInventory }) => { return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); } } else if (isStandardResource(resourcePaths)) { - if (resourcePaths.length === 1) { + if (resourcePaths.length === 1 || !isSingleResourceIdentifier(resourcePaths[1])) { return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); } if (!hasGetMethod(oas.paths[resourcePaths[1]])) { diff --git a/tools/spectral/ipa/rulesets/functions/getMethodReturnsSingleResource.js b/tools/spectral/ipa/rulesets/functions/getMethodReturnsSingleResource.js index 935bb82cf0..363822db68 100644 --- a/tools/spectral/ipa/rulesets/functions/getMethodReturnsSingleResource.js +++ b/tools/spectral/ipa/rulesets/functions/getMethodReturnsSingleResource.js @@ -1,4 +1,11 @@ -import { isSingleResource, isCustomMethod, isSingletonResource, getResourcePaths } from './utils/resourceEvaluation.js'; +import { + getResourceCollectionIdentifier, + getResourcePaths, + isResourceCollectionIdentifier, + isSingleResourceIdentifier, + isSingletonResource, + isStandardResource, +} from './utils/resourceEvaluation.js'; import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; import { getAllSuccessfulResponseSchemas } from './utils/methodUtils.js'; import { hasException } from './utils/exceptions.js'; @@ -12,28 +19,33 @@ const ERROR_MESSAGE_STANDARD_RESOURCE = export default (input, _, { path, documentInventory }) => { const oas = documentInventory.resolved; const resourcePath = path[1]; - const resourcePaths = getResourcePaths(resourcePath, Object.keys(oas.paths)); - if (isCustomMethod(resourcePath) || (!isSingleResource(resourcePath) && !isSingletonResource(resourcePaths))) { - return; - } - - const errors = []; + if ( + (isSingleResourceIdentifier(resourcePath) && + isStandardResource(getResourcePaths(getResourceCollectionIdentifier(resourcePath), Object.keys(oas.paths)))) || + (isResourceCollectionIdentifier(resourcePath) && + isSingletonResource(getResourcePaths(resourcePath, Object.keys(oas.paths)))) + ) { + const errors = []; - const responseSchemas = getAllSuccessfulResponseSchemas(input); - responseSchemas.forEach(({ schemaPath, schema }) => { - const fullPath = path.concat(schemaPath); - const responseObject = resolveObject(oas, fullPath); + const responseSchemas = getAllSuccessfulResponseSchemas(input); + responseSchemas.forEach(({ schemaPath, schema }) => { + const fullPath = path.concat(schemaPath); + const responseObject = resolveObject(oas, fullPath); - if (hasException(responseObject, RULE_NAME)) { - collectException(responseObject, RULE_NAME, fullPath); - } else if (schemaIsPaginated(schema) || schemaIsArray(schema)) { - collectAndReturnViolation(fullPath, RULE_NAME, ERROR_MESSAGE_STANDARD_RESOURCE); - errors.push({ path: fullPath, message: ERROR_MESSAGE_STANDARD_RESOURCE }); - } else { - collectAdoption(fullPath, RULE_NAME); - } - }); + if (hasException(responseObject, RULE_NAME)) { + collectException(responseObject, RULE_NAME, fullPath); + } else if (schemaIsPaginated(schema) || schemaIsArray(schema)) { + collectAndReturnViolation(fullPath, RULE_NAME, ERROR_MESSAGE_STANDARD_RESOURCE); + errors.push({ + path: fullPath, + message: ERROR_MESSAGE_STANDARD_RESOURCE, + }); + } else { + collectAdoption(fullPath, RULE_NAME); + } + }); - return errors; + return errors; + } }; diff --git a/tools/spectral/ipa/rulesets/functions/singletonHasNoId.js b/tools/spectral/ipa/rulesets/functions/singletonHasNoId.js index fec427f5db..68ebed5865 100644 --- a/tools/spectral/ipa/rulesets/functions/singletonHasNoId.js +++ b/tools/spectral/ipa/rulesets/functions/singletonHasNoId.js @@ -1,9 +1,8 @@ import { getResourcePaths, hasGetMethod, - isSingleResource, - isCustomMethod, isSingletonResource, + isResourceCollectionIdentifier, } from './utils/resourceEvaluation.js'; import { hasException } from './utils/exceptions.js'; import { getAllSuccessfulResponseSchemas } from './utils/methodUtils.js'; @@ -15,7 +14,7 @@ const ERROR_MESSAGE = 'Singleton resources must not have a user-provided or syst export default (input, opts, { path, documentInventory }) => { const resourcePath = path[1]; - if (isCustomMethod(resourcePath) || isSingleResource(resourcePath)) { + if (!isResourceCollectionIdentifier(resourcePath)) { return; } diff --git a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js index 26a874b88a..2c62a6401e 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js +++ b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js @@ -1,11 +1,45 @@ export const AUTH_PREFIX = '/api/atlas/v2'; export const UNAUTH_PREFIX = '/api/atlas/v2/unauth'; -export function isSingleResource(path) { - return path.endsWith('}'); +/** + * Checks if a path represents a collection of resources/singleton resource. + * + * @param {string} path the path to evaluate + * @returns {boolean} true if the path represents a collection of resources/singleton resource, false otherwise + */ +export function isResourceCollectionIdentifier(path) { + const p = removePrefix(path); + const childPattern = new RegExp(`^.*}/[a-zA-Z]+$`); + const basePattern = new RegExp(`^/[a-zA-Z]+$`); + return basePattern.test(p) || childPattern.test(p); +} + +/** + * Checks if a path represents a single resource. For example: + * '/resource/{id}' returns true + * '/resource/{id}/child' returns false + * '/resource/{id}/{id}' returns false + * + * @param {string} path the path to evaluate + * @returns {boolean} true if the path represents a single resource, false otherwise + */ +export function isSingleResourceIdentifier(path) { + const pattern = new RegExp(`^.*/[a-zA-Z]+/{[a-zA-Z]+}$`); + return pattern.test(path); } -export function isCustomMethod(path) { +/** + * Returns the resource collection identifier for a resource based on a single resource identifier. + * + * @param {string} singleResourceIdentifier a valid single resource identifier + * @returns {string} the resource collection identifier + */ +export function getResourceCollectionIdentifier(singleResourceIdentifier) { + const pathSections = singleResourceIdentifier.split('/'); + return pathSections.slice(0, pathSections.length - 1).join('/'); +} + +export function isCustomMethodIdentifier(path) { return path.includes(':'); } @@ -13,6 +47,10 @@ export function getCustomMethodName(path) { return path.split(':')[1]; } +export function isPathParam(string) { + return string.startsWith('{') && string.endsWith('}'); +} + /** * Checks if a resource is a singleton resource ({@link https://docs.devprod.prod.corp.mongodb.com/ipa/113 IPA-113}) based on the paths for the * resource. The resource may have custom methods. Use {@link getResourcePaths} to get all paths of a resource. @@ -21,11 +59,16 @@ export function getCustomMethodName(path) { * @returns {boolean} */ export function isSingletonResource(resourcePaths) { + if (!isResourceCollectionIdentifier(resourcePaths[0])) { + return false; + } + if (resourcePaths.length === 1) { return resourceBelongsToSingleParent(resourcePaths[0]); } + // TODO check for POST? const additionalPaths = resourcePaths.slice(1); - return additionalPaths.every(isCustomMethod); + return additionalPaths.every(isCustomMethodIdentifier); } /** @@ -36,14 +79,18 @@ export function isSingletonResource(resourcePaths) { * @returns {boolean} */ export function isStandardResource(resourcePaths) { + if (!isResourceCollectionIdentifier(resourcePaths[0]) || isSingletonResource(resourcePaths)) { + return false; + } if (resourcePaths.length === 1) { + // TODO check for POST? return !resourceBelongsToSingleParent(resourcePaths[0]); } if (resourcePaths.length === 2) { - return isSingleResource(resourcePaths[1]); + return isSingleResourceIdentifier(resourcePaths[1]); } const additionalPaths = resourcePaths.slice(2); - return additionalPaths.every(isCustomMethod); + return isSingleResourceIdentifier(resourcePaths[1]) && additionalPaths.every(isCustomMethodIdentifier); } /** @@ -86,14 +133,7 @@ export function getResourcePaths(resourceCollectionPath, allPaths) { */ function resourceBelongsToSingleParent(resourcePath) { // Ignore /api/atlas/v2 and /api/atlas/v2/unauth - let path = resourcePath; - if (resourcePath.startsWith(AUTH_PREFIX)) { - path = resourcePath.slice(AUTH_PREFIX.length); - } - if (resourcePath.startsWith(UNAUTH_PREFIX)) { - path = resourcePath.slice(UNAUTH_PREFIX.length); - } - + const path = removePrefix(resourcePath); if (path === '') { return true; } @@ -105,12 +145,22 @@ function resourceBelongsToSingleParent(resourcePath) { if (resourcePathSections.length < 2) { return false; } - if (isSingleResource(resourcePathSections[resourcePathSections.length - 1])) { + if (isPathParam(resourcePathSections[resourcePathSections.length - 1])) { resourcePathSections = resourcePathSections.slice(0, resourcePathSections.length - 2); } if (resourcePathSections.length === 1) { return false; } const parentResourceSection = resourcePathSections[resourcePathSections.length - 2]; - return isSingleResource(parentResourceSection); + return isPathParam(parentResourceSection); +} + +function removePrefix(path) { + if (path.startsWith(AUTH_PREFIX)) { + return path.slice(AUTH_PREFIX.length); + } + if (path.startsWith(UNAUTH_PREFIX)) { + return path.slice(UNAUTH_PREFIX.length); + } + return path; } From fa14af21f582fef539c69fde80257b99f4c71e2c Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Tue, 4 Mar 2025 17:36:24 +0000 Subject: [PATCH 04/10] CLOUDP-303615: Fix --- .../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 2c62a6401e..41ac8ab07c 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js +++ b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js @@ -66,7 +66,6 @@ export function isSingletonResource(resourcePaths) { if (resourcePaths.length === 1) { return resourceBelongsToSingleParent(resourcePaths[0]); } - // TODO check for POST? const additionalPaths = resourcePaths.slice(1); return additionalPaths.every(isCustomMethodIdentifier); } @@ -83,8 +82,7 @@ export function isStandardResource(resourcePaths) { return false; } if (resourcePaths.length === 1) { - // TODO check for POST? - return !resourceBelongsToSingleParent(resourcePaths[0]); + return true; } if (resourcePaths.length === 2) { return isSingleResourceIdentifier(resourcePaths[1]); From 8cf5fbc340c7977adcf0cd585f70d50d30deb837 Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Tue, 4 Mar 2025 17:39:57 +0000 Subject: [PATCH 05/10] CLOUDP-303615: Fix merge issue --- .../getResponseCodeShouldBe200OK.test.js | 28 +++++++++---------- .../functions/getResponseCodeShouldBe200OK.js | 4 +-- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/tools/spectral/ipa/__tests__/getResponseCodeShouldBe200OK.test.js b/tools/spectral/ipa/__tests__/getResponseCodeShouldBe200OK.test.js index 4d26b4109b..bde99cc0ab 100644 --- a/tools/spectral/ipa/__tests__/getResponseCodeShouldBe200OK.test.js +++ b/tools/spectral/ipa/__tests__/getResponseCodeShouldBe200OK.test.js @@ -31,7 +31,7 @@ testRule('xgen-IPA-104-get-method-response-code-is-200', [ }, }, }, - '/singleton': { + '/resource/{id}/singleton': { get: { responses: { 400: {}, @@ -47,8 +47,8 @@ testRule('xgen-IPA-104-get-method-response-code-is-200', [ name: 'invalid methods', document: { paths: { - '/resource1': { get: { responses: {} } }, - '/resource1/{id}': { + '/resourceOne': { get: { responses: {} } }, + '/resourceOne/{id}': { get: { responses: { 201: {}, @@ -57,8 +57,8 @@ testRule('xgen-IPA-104-get-method-response-code-is-200', [ }, }, }, - '/resource2': { get: { responses: {} } }, - '/resource2/{id}': { + '/resourceTwo': { get: { responses: {} } }, + '/resourceTwo/{id}': { get: { responses: { 400: {}, @@ -66,8 +66,8 @@ testRule('xgen-IPA-104-get-method-response-code-is-200', [ }, }, }, - '/resource3': { get: { responses: {} } }, - '/resource3/{id}': { + '/resourceThree': { get: { responses: {} } }, + '/resourceThree/{id}': { get: { responses: { 200: {}, @@ -84,21 +84,21 @@ testRule('xgen-IPA-104-get-method-response-code-is-200', [ code: 'xgen-IPA-104-get-method-response-code-is-200', message: 'The Get method must return a 200 OK response. This method either lacks a 200 OK response or defines a different 2xx status code. http://go/ipa/104', - path: ['paths', '/resource1/{id}', 'get'], + path: ['paths', '/resourceOne/{id}', 'get'], severity: DiagnosticSeverity.Warning, }, { code: 'xgen-IPA-104-get-method-response-code-is-200', message: 'The Get method must return a 200 OK response. This method either lacks a 200 OK response or defines a different 2xx status code. http://go/ipa/104', - path: ['paths', '/resource2/{id}', 'get'], + path: ['paths', '/resourceTwo/{id}', 'get'], severity: DiagnosticSeverity.Warning, }, { code: 'xgen-IPA-104-get-method-response-code-is-200', message: 'The Get method must return a 200 OK response. This method either lacks a 200 OK response or defines a different 2xx status code. http://go/ipa/104', - path: ['paths', '/resource3/{id}', 'get'], + path: ['paths', '/resourceThree/{id}', 'get'], severity: DiagnosticSeverity.Warning, }, ], @@ -107,8 +107,8 @@ testRule('xgen-IPA-104-get-method-response-code-is-200', [ name: 'invalid method with exception', document: { paths: { - '/resource1': { get: { responses: {} } }, - '/resource1/{id}': { + '/resourceOne': { get: { responses: {} } }, + '/resourceOne/{id}': { get: { responses: { 201: {}, @@ -120,8 +120,8 @@ testRule('xgen-IPA-104-get-method-response-code-is-200', [ }, }, }, - '/resource2': { get: { responses: {} } }, - '/resource2/{id}': { + '/resourceTwo': { get: { responses: {} } }, + '/resourceTwo/{id}': { get: { responses: { 400: {}, diff --git a/tools/spectral/ipa/rulesets/functions/getResponseCodeShouldBe200OK.js b/tools/spectral/ipa/rulesets/functions/getResponseCodeShouldBe200OK.js index aab6a0ff8f..96177ff921 100644 --- a/tools/spectral/ipa/rulesets/functions/getResponseCodeShouldBe200OK.js +++ b/tools/spectral/ipa/rulesets/functions/getResponseCodeShouldBe200OK.js @@ -1,6 +1,6 @@ import { hasException } from './utils/exceptions.js'; import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; -import { isChild, isCustomMethod } from './utils/resourceEvaluation.js'; +import { isSingleResourceIdentifier } from './utils/resourceEvaluation.js'; const RULE_NAME = 'xgen-IPA-104-get-method-response-code-is-200'; const ERROR_MESSAGE = @@ -9,7 +9,7 @@ const ERROR_MESSAGE = export default (input, _, { path }) => { const resourcePath = path[1]; - if (isCustomMethod(resourcePath) || !isChild(resourcePath)) { + if (!isSingleResourceIdentifier(resourcePath)) { return; } From 3d2693e229213652a0e987af4d6ddbce08164441 Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Wed, 5 Mar 2025 15:34:19 +0000 Subject: [PATCH 06/10] CLOUDP-303615: Fix --- .../getMethodReturnsSingleResource.test.js | 4 +- .../getResponseCodeShouldBe200OK.test.js | 29 ++ .../utils/resourceEvaluation.test.js | 278 +++++++++++------- tools/spectral/ipa/rulesets/IPA-104.yaml | 1 - .../functions/eachResourceHasGetMethod.js | 14 +- .../getMethodReturnsSingleResource.js | 25 +- .../functions/getResponseCodeShouldBe200OK.js | 49 +-- .../rulesets/functions/singletonHasNoId.js | 26 +- .../functions/utils/resourceEvaluation.js | 83 +++--- 9 files changed, 292 insertions(+), 217 deletions(-) diff --git a/tools/spectral/ipa/__tests__/getMethodReturnsSingleResource.test.js b/tools/spectral/ipa/__tests__/getMethodReturnsSingleResource.test.js index c532752e35..e645931746 100644 --- a/tools/spectral/ipa/__tests__/getMethodReturnsSingleResource.test.js +++ b/tools/spectral/ipa/__tests__/getMethodReturnsSingleResource.test.js @@ -224,7 +224,7 @@ testRule('xgen-IPA-104-get-method-returns-single-resource', [ { code: 'xgen-IPA-104-get-method-returns-single-resource', message: - 'Get methods should return data for a single resource. This method returns an array or a paginated response. http://go/ipa/104', + 'Get methods for singleton resource should return data for a single resource. This method returns an array or a paginated response. If this is not a singleton resource, please implement all standard methods. http://go/ipa/104', path: [ 'paths', '/resource/{id}/arraySingleton', @@ -239,7 +239,7 @@ testRule('xgen-IPA-104-get-method-returns-single-resource', [ { code: 'xgen-IPA-104-get-method-returns-single-resource', message: - 'Get methods should return data for a single resource. This method returns an array or a paginated response. http://go/ipa/104', + 'Get methods for singleton resource should return data for a single resource. This method returns an array or a paginated response. If this is not a singleton resource, please implement all standard methods. http://go/ipa/104', path: [ 'paths', '/resource/{id}/paginatedSingleton', diff --git a/tools/spectral/ipa/__tests__/getResponseCodeShouldBe200OK.test.js b/tools/spectral/ipa/__tests__/getResponseCodeShouldBe200OK.test.js index bde99cc0ab..32396024d6 100644 --- a/tools/spectral/ipa/__tests__/getResponseCodeShouldBe200OK.test.js +++ b/tools/spectral/ipa/__tests__/getResponseCodeShouldBe200OK.test.js @@ -34,6 +34,7 @@ testRule('xgen-IPA-104-get-method-response-code-is-200', [ '/resource/{id}/singleton': { get: { responses: { + 200: {}, 400: {}, 500: {}, }, @@ -77,6 +78,15 @@ testRule('xgen-IPA-104-get-method-response-code-is-200', [ }, }, }, + '/resource/{id}/singleton': { + get: { + responses: { + 202: {}, + 400: {}, + 500: {}, + }, + }, + }, }, }, errors: [ @@ -101,6 +111,13 @@ testRule('xgen-IPA-104-get-method-response-code-is-200', [ path: ['paths', '/resourceThree/{id}', 'get'], severity: DiagnosticSeverity.Warning, }, + { + code: 'xgen-IPA-104-get-method-response-code-is-200', + message: + 'The Get method must return a 200 OK response. This method either lacks a 200 OK response or defines a different 2xx status code. http://go/ipa/104', + path: ['paths', '/resource/{id}/singleton', 'get'], + severity: DiagnosticSeverity.Warning, + }, ], }, { @@ -132,6 +149,18 @@ testRule('xgen-IPA-104-get-method-response-code-is-200', [ }, }, }, + '/resource/{id}/singleton': { + get: { + responses: { + 202: {}, + 400: {}, + 500: {}, + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-104-get-method-response-code-is-200': 'reason', + }, + }, + }, }, }, errors: [], diff --git a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js index 6f307e9b39..c98c8e029a 100644 --- a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js +++ b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js @@ -1,197 +1,247 @@ import { describe, expect, it } from '@jest/globals'; import { - getResourcePaths, + getResourcePathItems, + isResourceCollectionIdentifier, + isSingleResourceIdentifier, isSingletonResource, - isStandardResource, } from '../../rulesets/functions/utils/resourceEvaluation'; -// Standard resources -const standardResourcePaths = ['/resource', '/resource/{exampleId}']; +const resource = { + '/resource': { + post: {}, + get: {}, + }, + '/resource/{id}': { + get: {}, + patch: {}, + delete: {}, + }, +}; -const nestedStandardResourcePaths = ['/resource/{exampleId}/nested', '/resource/{exampleId}/nested/{exampleId}']; +const resourceMissingMethods = { + '/resourceMissingMethods': { + get: {}, + }, +}; -const standardResourceWithCustomPaths = [ - '/customStandard', - '/customStandard/{exampleId}', - '/customStandard/{id}:method', - '/customStandard:method', -]; +const childResourceMissingSubPath = { + '/resource/{id}/childResourceMissingSubPath': { + post: {}, + get: {}, + }, +}; -const standardResourceMissingSubPath = ['/standardMissingSub']; +const childResource = { + '/resource/{id}/child': { + post: {}, + get: {}, + }, + '/resource/{id}/child/{id}': { + get: {}, + patch: {}, + delete: {}, + }, +}; -// Singleton resources -const singletonResourcePaths = ['/resource/{exampleId}/singleton']; +const singleton = { + '/resource/{id}/singleton': { + get: {}, + patch: {}, + }, +}; -const singletonResourceWithCustomPaths = [ - '/resource/{exampleId}/customSingleton', - '/resource/{exampleId}/customSingleton:method', -]; +const customMethodResource = { + '/custom': { + post: {}, + get: {}, + }, + '/custom/{id}': { + get: {}, + patch: {}, + delete: {}, + }, + '/custom/{id}:method': { + post: {}, + }, + '/custom:method': { + post: {}, + }, +}; -// Neither standard nor singleton resources -const resourceMissingSubPathInvalidFormat = ['/standard/nestedStandardMissingSub']; - -const resourcePathsInvalidFormat = ['/resourceOne/resourceTwo', '/resourceOne/resourceTwo/{exampleId}']; - -const allPaths = standardResourcePaths.concat( - nestedStandardResourcePaths, - standardResourceWithCustomPaths, - standardResourceMissingSubPath, - singletonResourcePaths, - singletonResourceWithCustomPaths, - resourceMissingSubPathInvalidFormat, - resourcePathsInvalidFormat -); +const mockOas = { + paths: { + ...resource, + ...childResource, + ...resourceMissingMethods, + ...childResourceMissingSubPath, + ...singleton, + ...customMethodResource, + }, +}; describe('tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js', () => { - describe('getResourcePaths', () => { + describe('isSingletonResource', () => { const testCases = [ { description: 'standard resource', - resourceCollectionPath: '/resource', - expectedPaths: standardResourcePaths, + resourcePathItems: resource, + isSingletonResource: false, }, { description: 'nested standard resource', - resourceCollectionPath: '/resource/{exampleId}/nested', - expectedPaths: nestedStandardResourcePaths, - }, - { - description: 'standard resource with custom methods', - resourceCollectionPath: '/customStandard', - expectedPaths: standardResourceWithCustomPaths, + resourcePathItems: childResource, + isSingletonResource: false, }, { - description: 'standard resource with missing sub-path', - resourceCollectionPath: '/standardMissingSub', - expectedPaths: standardResourceMissingSubPath, + description: 'standard resource with missing methods', + resourcePathItems: resourceMissingMethods, + isSingletonResource: false, }, { description: 'nested standard resource with missing sub-path', - resourceCollectionPath: '/standard/nestedStandardMissingSub', - expectedPaths: resourceMissingSubPathInvalidFormat, - }, - { - description: 'standard resource with missing sub-path', - resourceCollectionPath: '/resourceOne/resourceTwo', - expectedPaths: resourcePathsInvalidFormat, + resourcePathItems: childResourceMissingSubPath, + isSingletonResource: false, }, { description: 'singleton resource', - resourceCollectionPath: '/resource/{exampleId}/singleton', - expectedPaths: singletonResourcePaths, + resourcePathItems: singleton, + isSingletonResource: true, }, { - description: 'singleton resource with custom methods', - resourceCollectionPath: '/resource/{exampleId}/customSingleton', - expectedPaths: singletonResourceWithCustomPaths, + description: 'standard resource with custom methods', + resourcePathItems: customMethodResource, + isSingletonResource: false, }, ]; testCases.forEach((testCase) => { - it(`returns the expected paths for ${testCase.description}`, () => { - expect(getResourcePaths(testCase.resourceCollectionPath, allPaths)).toEqual(testCase.expectedPaths); + it(`returns ${testCase.isSingletonResource} for ${testCase.description}`, () => { + expect(isSingletonResource(testCase.resourcePathItems)).toEqual(testCase.isSingletonResource); }); }); }); - describe('isStandardResource', () => { + describe('getResourcePathItems', () => { const testCases = [ { description: 'standard resource', - resourcePaths: standardResourcePaths, - isStandardResource: true, + path: '/resource', + expectedResourcePathItems: resource, }, { - description: 'standard resource with custom methods', - resourcePaths: standardResourceWithCustomPaths, - isStandardResource: true, + description: 'standard child resource', + path: '/resource/{id}/child', + expectedResourcePathItems: childResource, }, { - description: 'standard resource with missing sub-path', - resourcePaths: standardResourceMissingSubPath, - isStandardResource: true, + description: 'singleton resource', + path: '/resource/{id}/singleton', + expectedResourcePathItems: singleton, }, { - description: 'nested standard resource with missing sub-path', - resourcePaths: resourceMissingSubPathInvalidFormat, - isStandardResource: false, + description: 'resource with custom methods', + path: '/custom', + expectedResourcePathItems: customMethodResource, + }, + ]; + + testCases.forEach((testCase) => { + it(`returns the expected path items for ${testCase.description}`, () => { + const result = getResourcePathItems(testCase.path, mockOas.paths); + const expectedResult = testCase.expectedResourcePathItems; + + expect(Object.keys(result)).toEqual(Object.keys(expectedResult)); + + Object.keys(result).forEach((key) => { + expect(Object.keys(result[key])).toEqual(Object.keys(expectedResult[key])); + }); + }); + }); + }); + + describe('isResourceCollectionIdentifier', () => { + const testCases = [ + { + description: 'collection identifier', + path: '/resource', + isResourceCollectionIdentifier: true, }, { - description: 'nested standard resource', - resourcePaths: nestedStandardResourcePaths, - isStandardResource: true, + description: 'collection identifier child', + path: '/resource/{id}/child', + isResourceCollectionIdentifier: true, }, { - description: 'singleton resource', - resourcePaths: singletonResourcePaths, - isStandardResource: false, + description: 'invalid parent collection identifier child', + path: '/resourceOne/resourceTwo/{id}/child', + isResourceCollectionIdentifier: true, }, { - description: 'singleton resource with custom methods', - resourcePaths: singletonResourceWithCustomPaths, - isStandardResource: false, + description: 'invalid parent collection identifier child', + path: '/resourceOne/{id}/{id}/child', + isResourceCollectionIdentifier: true, }, { - description: 'invalid path format', - resourcePaths: resourcePathsInvalidFormat, - isStandardResource: false, + description: 'single identifier', + path: '/resource/{id}', + isResourceCollectionIdentifier: false, + }, + { + description: 'single identifier child', + path: '/resource/{id}/child/{id}', + isResourceCollectionIdentifier: false, }, ]; testCases.forEach((testCase) => { - it(`returns ${testCase.isStandardResource} for ${testCase.description}`, () => { - expect(isStandardResource(testCase.resourcePaths)).toEqual(testCase.isStandardResource); + it(`returns ${testCase.isResourceCollectionIdentifier} for ${testCase.description}`, () => { + expect(isResourceCollectionIdentifier(testCase.path)).toEqual(testCase.isResourceCollectionIdentifier); }); }); }); - describe('isSingletonResource', () => { + describe('isSingleResourceIdentifier', () => { const testCases = [ { - description: 'standard resource', - resourcePaths: standardResourcePaths, - isSingletonResource: false, - }, - { - description: 'standard resource with custom methods', - resourcePaths: standardResourceWithCustomPaths, - isSingletonResource: false, + description: 'collection identifier', + path: '/resource', + isSingleResourceIdentifier: false, }, { - description: 'standard resource with missing sub-path', - resourcePaths: standardResourceMissingSubPath, - isSingletonResource: false, + description: 'collection identifier child', + path: '/resource/{id}/child', + isSingleResourceIdentifier: false, }, { - description: 'nested standard resource with missing sub-path', - resourcePaths: resourceMissingSubPathInvalidFormat, - isSingletonResource: false, + description: 'invalid parent collection identifier child', + path: '/resourceOne/resourceTwo/{id}/child', + isSingleResourceIdentifier: false, }, { - description: 'invalid path format', - resourcePaths: resourcePathsInvalidFormat, - isSingletonResource: false, + description: 'invalid parent collection identifier child', + path: '/resourceOne/{id}/{id}/child', + isSingleResourceIdentifier: false, }, { - description: 'nested standard resource', - resourcePaths: nestedStandardResourcePaths, - isSingletonResource: false, + description: 'invalid single identifier child', + path: '/resourceOne/{id}/{id}', + isSingleResourceIdentifier: false, }, { - description: 'singleton resource', - resourcePaths: singletonResourcePaths, - isSingletonResource: true, + description: 'single identifier', + path: '/resource/{id}', + isSingleResourceIdentifier: true, }, { - description: 'singleton resource with custom methods', - resourcePaths: singletonResourceWithCustomPaths, - isSingletonResource: true, + description: 'single identifier child', + path: '/resource/{id}/child/{id}', + isSingleResourceIdentifier: true, }, ]; testCases.forEach((testCase) => { - it(`returns ${testCase.isSingletonResource} for ${testCase.description}`, () => { - expect(isSingletonResource(testCase.resourcePaths)).toEqual(testCase.isSingletonResource); + it(`returns ${testCase.isSingleResourceIdentifier} for ${testCase.description}`, () => { + expect(isSingleResourceIdentifier(testCase.path)).toEqual(testCase.isSingleResourceIdentifier); }); }); }); diff --git a/tools/spectral/ipa/rulesets/IPA-104.yaml b/tools/spectral/ipa/rulesets/IPA-104.yaml index da80f33d11..1326d1d9a4 100644 --- a/tools/spectral/ipa/rulesets/IPA-104.yaml +++ b/tools/spectral/ipa/rulesets/IPA-104.yaml @@ -22,7 +22,6 @@ rules: given: '$.paths[*].get' then: function: 'getMethodReturnsSingleResource' - xgen-IPA-104-get-method-response-code-is-200: description: 'The Get method must return a 200 OK response. http://go/ipa/104' message: '{{error}} http://go/ipa/104' diff --git a/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js b/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js index 973e8880d1..223166e845 100644 --- a/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js +++ b/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js @@ -1,8 +1,7 @@ import { hasGetMethod, - isStandardResource, isSingletonResource, - getResourcePaths, + getResourcePathItems, isResourceCollectionIdentifier, isSingleResourceIdentifier, } from './utils/resourceEvaluation.js'; @@ -24,17 +23,18 @@ export default (input, _, { path, documentInventory }) => { return; } - const resourcePaths = getResourcePaths(input, Object.keys(oas.paths)); + const resourcePathItems = getResourcePathItems(input, oas.paths); + const resourcePaths = Object.keys(resourcePathItems); - if (isSingletonResource(resourcePaths)) { - if (!hasGetMethod(oas.paths[resourcePaths[0]])) { + if (isSingletonResource(resourcePathItems)) { + if (!hasGetMethod(resourcePathItems[resourcePaths[0]])) { return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); } - } else if (isStandardResource(resourcePaths)) { + } else { if (resourcePaths.length === 1 || !isSingleResourceIdentifier(resourcePaths[1])) { return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); } - if (!hasGetMethod(oas.paths[resourcePaths[1]])) { + if (!hasGetMethod(resourcePathItems[resourcePaths[1]])) { return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); } } diff --git a/tools/spectral/ipa/rulesets/functions/getMethodReturnsSingleResource.js b/tools/spectral/ipa/rulesets/functions/getMethodReturnsSingleResource.js index 363822db68..b7ca5adfc6 100644 --- a/tools/spectral/ipa/rulesets/functions/getMethodReturnsSingleResource.js +++ b/tools/spectral/ipa/rulesets/functions/getMethodReturnsSingleResource.js @@ -1,10 +1,8 @@ import { - getResourceCollectionIdentifier, - getResourcePaths, + getResourcePathItems, isResourceCollectionIdentifier, isSingleResourceIdentifier, isSingletonResource, - isStandardResource, } from './utils/resourceEvaluation.js'; import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; import { getAllSuccessfulResponseSchemas } from './utils/methodUtils.js'; @@ -15,17 +13,18 @@ import { resolveObject } from './utils/componentUtils.js'; const RULE_NAME = 'xgen-IPA-104-get-method-returns-single-resource'; const ERROR_MESSAGE_STANDARD_RESOURCE = 'Get methods should return data for a single resource. This method returns an array or a paginated response.'; +const ERROR_MESSAGE_SINGLETON_RESOURCE = + 'Get methods for singleton resource should return data for a single resource. This method returns an array or a paginated response. If this is not a singleton resource, please implement all standard methods.'; export default (input, _, { path, documentInventory }) => { const oas = documentInventory.resolved; const resourcePath = path[1]; - if ( - (isSingleResourceIdentifier(resourcePath) && - isStandardResource(getResourcePaths(getResourceCollectionIdentifier(resourcePath), Object.keys(oas.paths)))) || - (isResourceCollectionIdentifier(resourcePath) && - isSingletonResource(getResourcePaths(resourcePath, Object.keys(oas.paths)))) - ) { + const isSingleResource = isSingleResourceIdentifier(resourcePath); + const isSingleton = + isResourceCollectionIdentifier(resourcePath) && isSingletonResource(getResourcePathItems(resourcePath, oas.paths)); + + if (isSingleResource || isSingleton) { const errors = []; const responseSchemas = getAllSuccessfulResponseSchemas(input); @@ -36,10 +35,14 @@ export default (input, _, { path, documentInventory }) => { if (hasException(responseObject, RULE_NAME)) { collectException(responseObject, RULE_NAME, fullPath); } else if (schemaIsPaginated(schema) || schemaIsArray(schema)) { - collectAndReturnViolation(fullPath, RULE_NAME, ERROR_MESSAGE_STANDARD_RESOURCE); + collectAndReturnViolation( + fullPath, + RULE_NAME, + isSingleton ? ERROR_MESSAGE_SINGLETON_RESOURCE : ERROR_MESSAGE_STANDARD_RESOURCE + ); errors.push({ path: fullPath, - message: ERROR_MESSAGE_STANDARD_RESOURCE, + message: isSingleton ? ERROR_MESSAGE_SINGLETON_RESOURCE : ERROR_MESSAGE_STANDARD_RESOURCE, }); } else { collectAdoption(fullPath, RULE_NAME); diff --git a/tools/spectral/ipa/rulesets/functions/getResponseCodeShouldBe200OK.js b/tools/spectral/ipa/rulesets/functions/getResponseCodeShouldBe200OK.js index 96177ff921..0d32abbbca 100644 --- a/tools/spectral/ipa/rulesets/functions/getResponseCodeShouldBe200OK.js +++ b/tools/spectral/ipa/rulesets/functions/getResponseCodeShouldBe200OK.js @@ -1,36 +1,43 @@ import { hasException } from './utils/exceptions.js'; import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; -import { isSingleResourceIdentifier } from './utils/resourceEvaluation.js'; +import { + getResourcePathItems, + isResourceCollectionIdentifier, + isSingleResourceIdentifier, + isSingletonResource, +} from './utils/resourceEvaluation.js'; const RULE_NAME = 'xgen-IPA-104-get-method-response-code-is-200'; const ERROR_MESSAGE = 'The Get method must return a 200 OK response. This method either lacks a 200 OK response or defines a different 2xx status code.'; -export default (input, _, { path }) => { +export default (input, _, { path, documentInventory }) => { const resourcePath = path[1]; + const oas = documentInventory.resolved; + + if ( + isSingleResourceIdentifier(resourcePath) || + (isResourceCollectionIdentifier(resourcePath) && isSingletonResource(getResourcePathItems(resourcePath, oas.paths))) + ) { + if (hasException(input, RULE_NAME)) { + collectException(input, RULE_NAME, path); + return; + } - if (!isSingleResourceIdentifier(resourcePath)) { - return; - } - - if (hasException(input, RULE_NAME)) { - collectException(input, RULE_NAME, path); - return; - } + if (input['responses']) { + const responses = input['responses']; - if (input['responses']) { - const responses = input['responses']; + // If there is no 200 response, return a violation + if (!responses['200']) { + return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); + } - // If there is no 200 response, return a violation - if (!responses['200']) { - return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); + // If there are other 2xx responses that are not 200, return a violation + if (Object.keys(responses).some((key) => key.startsWith('2') && key !== '200')) { + return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); + } } - // If there are other 2xx responses that are not 200, return a violation - if (Object.keys(responses).some((key) => key.startsWith('2') && key !== '200')) { - return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); - } + collectAdoption(path, RULE_NAME); } - - collectAdoption(path, RULE_NAME); }; diff --git a/tools/spectral/ipa/rulesets/functions/singletonHasNoId.js b/tools/spectral/ipa/rulesets/functions/singletonHasNoId.js index 68ebed5865..718e0dbf15 100644 --- a/tools/spectral/ipa/rulesets/functions/singletonHasNoId.js +++ b/tools/spectral/ipa/rulesets/functions/singletonHasNoId.js @@ -1,5 +1,5 @@ import { - getResourcePaths, + getResourcePathItems, hasGetMethod, isSingletonResource, isResourceCollectionIdentifier, @@ -18,22 +18,22 @@ export default (input, opts, { path, documentInventory }) => { return; } - if (hasException(input, RULE_NAME)) { - collectException(input, RULE_NAME, path); - return; - } - const oas = documentInventory.resolved; - const resourcePaths = getResourcePaths(resourcePath, Object.keys(oas.paths)); + const resourcePathItems = getResourcePathItems(resourcePath, oas.paths); - if (isSingletonResource(resourcePaths) && hasGetMethod(input)) { - const resourceSchemas = getAllSuccessfulResponseSchemas(input['get']); - if (resourceSchemas.some(({ schema }) => schemaHasIdProperty(schema))) { - return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); + if (isSingletonResource(resourcePathItems)) { + if (hasException(input, RULE_NAME)) { + collectException(input, RULE_NAME, path); + return; + } + if (hasGetMethod(input)) { + const resourceSchemas = getAllSuccessfulResponseSchemas(input['get']); + if (resourceSchemas.some(({ schema }) => schemaHasIdProperty(schema))) { + return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); + } + collectAdoption(path, RULE_NAME); } } - - collectAdoption(path, RULE_NAME); }; function schemaHasIdProperty(schema) { diff --git a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js index 41ac8ab07c..e561951279 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js +++ b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js @@ -2,7 +2,10 @@ export const AUTH_PREFIX = '/api/atlas/v2'; export const UNAUTH_PREFIX = '/api/atlas/v2/unauth'; /** - * Checks if a path represents a collection of resources/singleton resource. + * Checks if a path represents a collection of resources/a singleton resource. For example: + * '/resource' returns true + * '/resource/{id}/child' returns true + * '/resource/child' returns false * * @param {string} path the path to evaluate * @returns {boolean} true if the path represents a collection of resources/singleton resource, false otherwise @@ -28,17 +31,6 @@ export function isSingleResourceIdentifier(path) { return pattern.test(path); } -/** - * Returns the resource collection identifier for a resource based on a single resource identifier. - * - * @param {string} singleResourceIdentifier a valid single resource identifier - * @returns {string} the resource collection identifier - */ -export function getResourceCollectionIdentifier(singleResourceIdentifier) { - const pathSections = singleResourceIdentifier.split('/'); - return pathSections.slice(0, pathSections.length - 1).join('/'); -} - export function isCustomMethodIdentifier(path) { return path.includes(':'); } @@ -52,74 +44,69 @@ export function isPathParam(string) { } /** - * Checks if a resource is a singleton resource ({@link https://docs.devprod.prod.corp.mongodb.com/ipa/113 IPA-113}) based on the paths for the - * resource. The resource may have custom methods. Use {@link getResourcePaths} to get all paths of a resource. + * Checks if a resource is a singleton resource ({@link https://docs.devprod.prod.corp.mongodb.com/ipa/113 IPA-113}) based on the path items for the + * resource. The resource may have custom methods. Use {@link getResourcePathItems} to get all path items of a resource. * - * @param resourcePaths all paths for the resource to be evaluated as an array of strings + * @param resourcePathItems all path items for the resource to be evaluated as an array of strings * @returns {boolean} */ -export function isSingletonResource(resourcePaths) { +export function isSingletonResource(resourcePathItems) { + const resourcePaths = Object.keys(resourcePathItems); if (!isResourceCollectionIdentifier(resourcePaths[0])) { return false; } if (resourcePaths.length === 1) { - return resourceBelongsToSingleParent(resourcePaths[0]); + return resourceBelongsToSingleParent(resourcePaths[0]) && !hasPostMethod(resourcePathItems[resourcePaths[0]]); } const additionalPaths = resourcePaths.slice(1); return additionalPaths.every(isCustomMethodIdentifier); } /** - * Checks if a resource is a standard resource ({@link https://docs.devprod.prod.corp.mongodb.com/ipa/103 IPA-103}) based on the paths for the - * resource. The resource may have custom methods. Use {@link getResourcePaths} to get all paths of a resource. + * Checks if a path object has a GET method * - * @param resourcePaths all paths for the resource to be evaluated as an array of strings + * @param pathObject the path object to evaluate * @returns {boolean} */ -export function isStandardResource(resourcePaths) { - if (!isResourceCollectionIdentifier(resourcePaths[0]) || isSingletonResource(resourcePaths)) { - return false; - } - if (resourcePaths.length === 1) { - return true; - } - if (resourcePaths.length === 2) { - return isSingleResourceIdentifier(resourcePaths[1]); - } - const additionalPaths = resourcePaths.slice(2); - return isSingleResourceIdentifier(resourcePaths[1]) && additionalPaths.every(isCustomMethodIdentifier); +export function hasGetMethod(pathObject) { + return Object.keys(pathObject).includes('get'); } /** - * Checks if a path object has a GET method + * Checks if a path object has a POST method * * @param pathObject the path object to evaluate * @returns {boolean} */ -export function hasGetMethod(pathObject) { - return Object.keys(pathObject).includes('get'); +export function hasPostMethod(pathObject) { + return Object.keys(pathObject).includes('post'); } /** - * Get all paths for a resource based on the path for the resource collection - * For example, resource collection path '/resource' may return ['/resource', '/resource{id}', '/resource{id}:customMethod'] + * Get all path items for a resource based on the path for the resource collection + * For example, resource collection path '/resource' may return path items for ['/resource', '/resource{id}', '/resource{id}:customMethod'] * - * @param {string} resourceCollectionPath the path string - * @param {Array} allPaths all paths - * @returns {string[]} all paths for a resource, including the path for the resource collection + * @param {string} resourceCollectionPath the path for the resource collection + * @param {Object} allPathItems all path items + * @returns {Object} all path items for a resource, including the path for the resource collection */ -export function getResourcePaths(resourceCollectionPath, allPaths) { +export function getResourcePathItems(resourceCollectionPath, allPathItems) { const singleResourcePathPattern = new RegExp(`^${resourceCollectionPath}/{[a-zA-Z]+}$`); const singleResourceCustomMethodPattern = new RegExp(`^${resourceCollectionPath}/{[a-zA-Z]+}:+[a-zA-Z]+$`); const customMethodPattern = new RegExp(`^${resourceCollectionPath}:+[a-zA-Z]+$`); - return allPaths.filter( - (p) => - resourceCollectionPath === p || - singleResourcePathPattern.test(p) || - customMethodPattern.test(p) || - singleResourceCustomMethodPattern.test(p) - ); + return Object.keys(allPathItems) + .filter( + (p) => + resourceCollectionPath === p || + singleResourcePathPattern.test(p) || + customMethodPattern.test(p) || + singleResourceCustomMethodPattern.test(p) + ) + .reduce((obj, key) => { + obj[key] = allPathItems[key]; + return obj; + }, {}); } /** From 0942421c5a767306d3c3f3e2ea8df9a0f0505f46 Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Wed, 5 Mar 2025 15:36:14 +0000 Subject: [PATCH 07/10] CLOUDP-303615: Fix merge issue --- .../createMethodRequestBodyIsRequestSuffixedObject.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsRequestSuffixedObject.js b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsRequestSuffixedObject.js index 4f3944a4c9..8838664a42 100644 --- a/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsRequestSuffixedObject.js +++ b/tools/spectral/ipa/rulesets/functions/createMethodRequestBodyIsRequestSuffixedObject.js @@ -1,6 +1,6 @@ import { hasException } from './utils/exceptions.js'; import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; -import { isCustomMethod } from './utils/resourceEvaluation.js'; +import { isCustomMethodIdentifier } from './utils/resourceEvaluation.js'; import { resolveObject } from './utils/componentUtils.js'; const RULE_NAME = 'xgen-IPA-106-create-method-request-body-is-request-suffixed-object'; @@ -11,7 +11,7 @@ export default (input, _, { path, documentInventory }) => { const oas = documentInventory.unresolved; const resourcePath = path[1]; - if (isCustomMethod(resourcePath)) { + if (isCustomMethodIdentifier(resourcePath)) { return; } From 4ed2f6a628f4db6fe9a98ae60e95874b32e45fc9 Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Wed, 5 Mar 2025 15:39:06 +0000 Subject: [PATCH 08/10] CLOUDP-303615: Cleanup --- .../getMethodReturnsSingleResource.test.js | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/tools/spectral/ipa/__tests__/getMethodReturnsSingleResource.test.js b/tools/spectral/ipa/__tests__/getMethodReturnsSingleResource.test.js index e645931746..84e453fe85 100644 --- a/tools/spectral/ipa/__tests__/getMethodReturnsSingleResource.test.js +++ b/tools/spectral/ipa/__tests__/getMethodReturnsSingleResource.test.js @@ -97,21 +97,6 @@ testRule('xgen-IPA-104-get-method-returns-single-resource', [ name: 'invalid resources', document: { paths: { - '/arrayResource': { - get: { - responses: { - 200: { - content: { - 'application/vnd.atlas.2024-08-05+json': { - schema: { - $ref: '#/components/schemas/PaginatedSchema', - }, - }, - }, - }, - }, - }, - }, '/arrayResource/{id}': { get: { responses: { @@ -127,21 +112,6 @@ testRule('xgen-IPA-104-get-method-returns-single-resource', [ }, }, }, - '/paginatedResource': { - get: { - responses: { - 200: { - content: { - 'application/vnd.atlas.2024-08-05+json': { - schema: { - $ref: '#/components/schemas/PaginatedSchema', - }, - }, - }, - }, - }, - }, - }, '/paginatedResource/{id}': { get: { responses: { From b8e965b31245cfc0747bac24214893992218765b Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Wed, 5 Mar 2025 15:40:52 +0000 Subject: [PATCH 09/10] CLOUDP-303615: Cleanup --- tools/spectral/ipa/__tests__/singletonHasNoId.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/spectral/ipa/__tests__/singletonHasNoId.test.js b/tools/spectral/ipa/__tests__/singletonHasNoId.test.js index 5dd74866b5..f38759a0f3 100644 --- a/tools/spectral/ipa/__tests__/singletonHasNoId.test.js +++ b/tools/spectral/ipa/__tests__/singletonHasNoId.test.js @@ -47,7 +47,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ patch: {}, delete: {}, }, - '/resource/{exampleId}/singleton1': { + '/resource/{exampleId}/singletonOne': { get: { responses: { 200: { @@ -65,7 +65,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ }, }, }, - '/resource/{exampleId}/singleton2': { + '/resource/{exampleId}/singletonTwo': { get: { responses: { 200: { From e839bde6832e96cf720d7a8d50ebb9b7d721ca14 Mon Sep 17 00:00:00 2001 From: Lovisa Berggren Date: Wed, 5 Mar 2025 16:22:20 +0000 Subject: [PATCH 10/10] CLOUDP-303615: Fix --- .../ipa/rulesets/functions/eachResourceHasGetMethod.js | 5 +++-- .../ipa/rulesets/functions/utils/resourceEvaluation.js | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js b/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js index 223166e845..40b1f36c02 100644 --- a/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js +++ b/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js @@ -31,10 +31,11 @@ export default (input, _, { path, documentInventory }) => { return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); } } else { - if (resourcePaths.length === 1 || !isSingleResourceIdentifier(resourcePaths[1])) { + if (resourcePaths.length === 1) { return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); } - if (!hasGetMethod(resourcePathItems[resourcePaths[1]])) { + const singleResourcePath = resourcePaths.find((p) => isSingleResourceIdentifier(p)); + if (!singleResourcePath || !hasGetMethod(resourcePathItems[singleResourcePath])) { 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 e561951279..e231693c65 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js +++ b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js @@ -52,14 +52,15 @@ export function isPathParam(string) { */ export function isSingletonResource(resourcePathItems) { const resourcePaths = Object.keys(resourcePathItems); - if (!isResourceCollectionIdentifier(resourcePaths[0])) { + const collectionIdentifier = resourcePaths.filter((p) => isResourceCollectionIdentifier(p)); + if (collectionIdentifier.length !== 1) { return false; } if (resourcePaths.length === 1) { return resourceBelongsToSingleParent(resourcePaths[0]) && !hasPostMethod(resourcePathItems[resourcePaths[0]]); } - const additionalPaths = resourcePaths.slice(1); + const additionalPaths = resourcePaths.splice(resourcePaths.indexOf(collectionIdentifier[0]), 1); return additionalPaths.every(isCustomMethodIdentifier); }