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..84e453fe85 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: { @@ -194,10 +194,10 @@ 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', - '/arraySingleton', + '/resource/{id}/arraySingleton', 'get', 'responses', '200', @@ -209,10 +209,10 @@ 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', - '/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__/getResponseCodeShouldBe200OK.test.js b/tools/spectral/ipa/__tests__/getResponseCodeShouldBe200OK.test.js index 4d26b4109b..32396024d6 100644 --- a/tools/spectral/ipa/__tests__/getResponseCodeShouldBe200OK.test.js +++ b/tools/spectral/ipa/__tests__/getResponseCodeShouldBe200OK.test.js @@ -31,9 +31,10 @@ testRule('xgen-IPA-104-get-method-response-code-is-200', [ }, }, }, - '/singleton': { + '/resource/{id}/singleton': { get: { responses: { + 200: {}, 400: {}, 500: {}, }, @@ -47,8 +48,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 +58,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 +67,8 @@ testRule('xgen-IPA-104-get-method-response-code-is-200', [ }, }, }, - '/resource3': { get: { responses: {} } }, - '/resource3/{id}': { + '/resourceThree': { get: { responses: {} } }, + '/resourceThree/{id}': { get: { responses: { 200: {}, @@ -77,6 +78,15 @@ testRule('xgen-IPA-104-get-method-response-code-is-200', [ }, }, }, + '/resource/{id}/singleton': { + get: { + responses: { + 202: {}, + 400: {}, + 500: {}, + }, + }, + }, }, }, errors: [ @@ -84,21 +94,28 @@ 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', '/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', '/resource2/{id}', 'get'], + 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', '/resource3/{id}', 'get'], + path: ['paths', '/resource/{id}/singleton', 'get'], severity: DiagnosticSeverity.Warning, }, ], @@ -107,8 +124,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,10 +137,22 @@ testRule('xgen-IPA-104-get-method-response-code-is-200', [ }, }, }, - '/resource2': { get: { responses: {} } }, - '/resource2/{id}': { + '/resourceTwo': { get: { responses: {} } }, + '/resourceTwo/{id}': { + get: { + responses: { + 400: {}, + 500: {}, + }, + 'x-xgen-IPA-exception': { + 'xgen-IPA-104-get-method-response-code-is-200': 'reason', + }, + }, + }, + '/resource/{id}/singleton': { get: { responses: { + 202: {}, 400: {}, 500: {}, }, diff --git a/tools/spectral/ipa/__tests__/singletonHasNoId.test.js b/tools/spectral/ipa/__tests__/singletonHasNoId.test.js index dc448a8e05..f38759a0f3 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: {}, }, - '/singleton1': { + '/resource/{exampleId}/singletonOne': { get: { responses: { 200: { @@ -65,7 +65,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ }, }, }, - '/singleton2': { + '/resource/{exampleId}/singletonTwo': { get: { responses: { 200: { @@ -92,7 +92,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ name: 'invalid resources', document: { paths: { - '/singleton1': { + '/resource/{exampleId}/singletonOne': { get: { responses: { 200: { @@ -111,7 +111,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ }, }, }, - '/singleton2': { + '/resource/{exampleId}/singletonTwo': { get: { responses: { 200: { @@ -130,7 +130,7 @@ testRule('xgen-IPA-113-singleton-must-not-have-id', [ }, }, }, - '/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', '/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', '/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', '/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: { - '/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 9ca9913787..c98c8e029a 100644 --- a/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js +++ b/tools/spectral/ipa/__tests__/utils/resourceEvaluation.test.js @@ -1,88 +1,248 @@ import { describe, expect, it } from '@jest/globals'; import { - getResourcePaths, + getResourcePathItems, + isResourceCollectionIdentifier, + isSingleResourceIdentifier, isSingletonResource, - isStandardResource, } from '../../rulesets/functions/utils/resourceEvaluation'; -const standardResourcePaths = ['/standard', '/standard/{id}']; - -const nestedStandardResourcePaths = ['/standard/{exampleId}/nested', '/standard/{exampleId}/nested/{exampleId}']; - -const standardResourceWithCustomPaths = ['/customStandard', '/customStandard/{id}', '/customStandard:method']; - -const singletonResourcePaths = ['/singleton']; - -const singletonResourceWithCustomPaths = ['/customSingleton', '/customSingleton:method']; - -const nestedSingletonResourcePaths = ['/standard/{exampleId}/nestedSingleton']; +const resource = { + '/resource': { + post: {}, + get: {}, + }, + '/resource/{id}': { + get: {}, + patch: {}, + delete: {}, + }, +}; + +const resourceMissingMethods = { + '/resourceMissingMethods': { + get: {}, + }, +}; + +const childResourceMissingSubPath = { + '/resource/{id}/childResourceMissingSubPath': { + post: {}, + get: {}, + }, +}; + +const childResource = { + '/resource/{id}/child': { + post: {}, + get: {}, + }, + '/resource/{id}/child/{id}': { + get: {}, + patch: {}, + delete: {}, + }, +}; + +const singleton = { + '/resource/{id}/singleton': { + get: {}, + patch: {}, + }, +}; + +const customMethodResource = { + '/custom': { + post: {}, + get: {}, + }, + '/custom/{id}': { + get: {}, + patch: {}, + delete: {}, + }, + '/custom/{id}:method': { + post: {}, + }, + '/custom:method': { + post: {}, + }, +}; + +const mockOas = { + paths: { + ...resource, + ...childResource, + ...resourceMissingMethods, + ...childResourceMissingSubPath, + ...singleton, + ...customMethodResource, + }, +}; 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('isSingletonResource', () => { + const testCases = [ + { + description: 'standard resource', + resourcePathItems: resource, + isSingletonResource: false, + }, + { + description: 'nested standard resource', + resourcePathItems: childResource, + isSingletonResource: false, + }, + { + description: 'standard resource with missing methods', + resourcePathItems: resourceMissingMethods, + isSingletonResource: false, + }, + { + description: 'nested standard resource with missing sub-path', + resourcePathItems: childResourceMissingSubPath, + isSingletonResource: false, + }, + { + description: 'singleton resource', + resourcePathItems: singleton, + isSingletonResource: true, + }, + { + description: 'standard resource with custom methods', + resourcePathItems: customMethodResource, + isSingletonResource: false, + }, + ]; + + testCases.forEach((testCase) => { + it(`returns ${testCase.isSingletonResource} for ${testCase.description}`, () => { + expect(isSingletonResource(testCase.resourcePathItems)).toEqual(testCase.isSingletonResource); + }); }); }); - describe('isStandardResource', () => { - it('returns true for a standard resource', () => { - expect(isStandardResource(standardResourcePaths)).toBe(true); - }); - - it('returns true for a standard resource with custom methods', () => { - expect(isStandardResource(standardResourceWithCustomPaths)).toBe(true); - }); - - 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); - }); - - 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('getResourcePathItems', () => { + const testCases = [ + { + description: 'standard resource', + path: '/resource', + expectedResourcePathItems: resource, + }, + { + description: 'standard child resource', + path: '/resource/{id}/child', + expectedResourcePathItems: childResource, + }, + { + description: 'singleton resource', + path: '/resource/{id}/singleton', + expectedResourcePathItems: singleton, + }, + { + 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('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('isResourceCollectionIdentifier', () => { + const testCases = [ + { + description: 'collection identifier', + path: '/resource', + isResourceCollectionIdentifier: true, + }, + { + description: 'collection identifier child', + path: '/resource/{id}/child', + isResourceCollectionIdentifier: true, + }, + { + description: 'invalid parent collection identifier child', + path: '/resourceOne/resourceTwo/{id}/child', + isResourceCollectionIdentifier: true, + }, + { + description: 'invalid parent collection identifier child', + path: '/resourceOne/{id}/{id}/child', + isResourceCollectionIdentifier: true, + }, + { + 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.isResourceCollectionIdentifier} for ${testCase.description}`, () => { + expect(isResourceCollectionIdentifier(testCase.path)).toEqual(testCase.isResourceCollectionIdentifier); + }); }); + }); - it('returns false for a nested standard resource', () => { - expect(isSingletonResource(nestedStandardResourcePaths)).toBe(false); + describe('isSingleResourceIdentifier', () => { + const testCases = [ + { + description: 'collection identifier', + path: '/resource', + isSingleResourceIdentifier: false, + }, + { + description: 'collection identifier child', + path: '/resource/{id}/child', + isSingleResourceIdentifier: false, + }, + { + description: 'invalid parent collection identifier child', + path: '/resourceOne/resourceTwo/{id}/child', + isSingleResourceIdentifier: false, + }, + { + description: 'invalid parent collection identifier child', + path: '/resourceOne/{id}/{id}/child', + isSingleResourceIdentifier: false, + }, + { + description: 'invalid single identifier child', + path: '/resourceOne/{id}/{id}', + isSingleResourceIdentifier: false, + }, + { + description: 'single identifier', + path: '/resource/{id}', + isSingleResourceIdentifier: true, + }, + { + description: 'single identifier child', + path: '/resource/{id}/child/{id}', + isSingleResourceIdentifier: true, + }, + ]; + + testCases.forEach((testCase) => { + 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/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; } 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/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..40b1f36c02 100644 --- a/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js +++ b/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js @@ -1,10 +1,9 @@ import { hasGetMethod, - isChild, - isCustomMethod, - isStandardResource, isSingletonResource, - getResourcePaths, + getResourcePathItems, + isResourceCollectionIdentifier, + isSingleResourceIdentifier, } from './utils/resourceEvaluation.js'; import { hasException } from './utils/exceptions.js'; import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; @@ -13,7 +12,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 (!isResourceCollectionIdentifier(input)) { return; } @@ -24,14 +23,19 @@ 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)) { - if (!hasGetMethod(oas.paths[resourcePaths[1]])) { + } else { + if (resourcePaths.length === 1) { + return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE); + } + 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/getMethodReturnsSingleResource.js b/tools/spectral/ipa/rulesets/functions/getMethodReturnsSingleResource.js index d51e2695fb..b7ca5adfc6 100644 --- a/tools/spectral/ipa/rulesets/functions/getMethodReturnsSingleResource.js +++ b/tools/spectral/ipa/rulesets/functions/getMethodReturnsSingleResource.js @@ -1,4 +1,9 @@ -import { isChild, isCustomMethod, isSingletonResource, getResourcePaths } from './utils/resourceEvaluation.js'; +import { + getResourcePathItems, + isResourceCollectionIdentifier, + isSingleResourceIdentifier, + isSingletonResource, +} from './utils/resourceEvaluation.js'; import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; import { getAllSuccessfulResponseSchemas } from './utils/methodUtils.js'; import { hasException } from './utils/exceptions.js'; @@ -8,32 +13,42 @@ 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]; - const resourcePaths = getResourcePaths(resourcePath, Object.keys(oas.paths)); - if (isCustomMethod(resourcePath) || (!isChild(resourcePath) && !isSingletonResource(resourcePaths))) { - return; - } + const isSingleResource = isSingleResourceIdentifier(resourcePath); + const isSingleton = + isResourceCollectionIdentifier(resourcePath) && isSingletonResource(getResourcePathItems(resourcePath, oas.paths)); - const errors = []; + if (isSingleResource || isSingleton) { + 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, + isSingleton ? ERROR_MESSAGE_SINGLETON_RESOURCE : ERROR_MESSAGE_STANDARD_RESOURCE + ); + errors.push({ + path: fullPath, + message: isSingleton ? ERROR_MESSAGE_SINGLETON_RESOURCE : ERROR_MESSAGE_STANDARD_RESOURCE, + }); + } else { + collectAdoption(fullPath, RULE_NAME); + } + }); - return errors; + return errors; + } }; diff --git a/tools/spectral/ipa/rulesets/functions/getResponseCodeShouldBe200OK.js b/tools/spectral/ipa/rulesets/functions/getResponseCodeShouldBe200OK.js index aab6a0ff8f..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 { isChild, isCustomMethod } 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 (isCustomMethod(resourcePath) || !isChild(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 29ef51b297..718e0dbf15 100644 --- a/tools/spectral/ipa/rulesets/functions/singletonHasNoId.js +++ b/tools/spectral/ipa/rulesets/functions/singletonHasNoId.js @@ -1,9 +1,8 @@ import { - getResourcePaths, + getResourcePathItems, hasGetMethod, - isChild, - isCustomMethod, isSingletonResource, + isResourceCollectionIdentifier, } from './utils/resourceEvaluation.js'; import { hasException } from './utils/exceptions.js'; import { getAllSuccessfulResponseSchemas } from './utils/methodUtils.js'; @@ -15,26 +14,26 @@ 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)) { - return; - } - - if (hasException(input, RULE_NAME)) { - collectException(input, RULE_NAME, path); + if (!isResourceCollectionIdentifier(resourcePath)) { 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 e4d9147a27..e231693c65 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js +++ b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js @@ -1,8 +1,37 @@ -export function isChild(path) { - return path.endsWith('}'); +export const AUTH_PREFIX = '/api/atlas/v2'; +export const UNAUTH_PREFIX = '/api/atlas/v2/unauth'; + +/** + * 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 + */ +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) { +export function isCustomMethodIdentifier(path) { return path.includes(':'); } @@ -10,61 +39,114 @@ 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. + * 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); + const collectionIdentifier = resourcePaths.filter((p) => isResourceCollectionIdentifier(p)); + if (collectionIdentifier.length !== 1) { + return false; + } + if (resourcePaths.length === 1) { - return true; + return resourceBelongsToSingleParent(resourcePaths[0]) && !hasPostMethod(resourcePathItems[resourcePaths[0]]); } - const additionalPaths = resourcePaths.slice(1); - return additionalPaths.every(isCustomMethod); + const additionalPaths = resourcePaths.splice(resourcePaths.indexOf(collectionIdentifier[0]), 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 (resourcePaths.length === 2 && isChild(resourcePaths[1])) { - return true; - } - if (resourcePaths.length < 3 || !isChild(resourcePaths[1])) { - return false; - } - const additionalPaths = resourcePaths.slice(2); - return additionalPaths.every(isCustomMethod); +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 parent path + * 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 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 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(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]+$`); - return allPaths.filter( - (p) => parent === p || childPathPattern.test(p) || customMethodPattern.test(p) || customChildMethodPattern.test(p) - ); +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 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; + }, {}); +} + +/** + * 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 + const path = removePrefix(resourcePath); + if (path === '') { + return true; + } + + let resourcePathSections = path.split('/'); + if (resourcePathSections[0] === '') { + resourcePathSections.shift(); + } + if (resourcePathSections.length < 2) { + return false; + } + 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 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; }