diff --git a/tools/spectral/ipa/__tests__/eachPathAlternatesBetweenResourceNameAndPathParam.test.js b/tools/spectral/ipa/__tests__/eachPathAlternatesBetweenResourceNameAndPathParam.test.js index e78401cbf8..15df7bc354 100644 --- a/tools/spectral/ipa/__tests__/eachPathAlternatesBetweenResourceNameAndPathParam.test.js +++ b/tools/spectral/ipa/__tests__/eachPathAlternatesBetweenResourceNameAndPathParam.test.js @@ -134,4 +134,26 @@ testRule('xgen-IPA-102-path-alternate-resource-name-path-param', [ }, ], }, + { + name: 'invalid paths with exceptions', + document: { + paths: { + '/api/atlas/v2/unauth/resourceName1/resourceName2': { + 'x-xgen-IPA-exception': { + 'xgen-IPA-102-path-alternate-resource-name-path-param': { + reason: 'test', + }, + }, + }, + '/api/atlas/v2/resourceName/{pathParam1}/{pathParam2}': { + 'x-xgen-IPA-exception': { + 'xgen-IPA-102-path-alternate-resource-name-path-param': { + reason: 'test', + }, + }, + }, + }, + }, + errors: [], + }, ]); diff --git a/tools/spectral/ipa/__tests__/eachResourceHasGetMethod.test.js b/tools/spectral/ipa/__tests__/eachResourceHasGetMethod.test.js index 64df503f92..1faa81a4e7 100644 --- a/tools/spectral/ipa/__tests__/eachResourceHasGetMethod.test.js +++ b/tools/spectral/ipa/__tests__/eachResourceHasGetMethod.test.js @@ -125,4 +125,25 @@ testRule('xgen-IPA-104-resource-has-GET', [ }, ], }, + { + name: 'invalid method with exception', + document: { + paths: { + '/standard': { + post: {}, + get: {}, + 'x-xgen-IPA-exception': { + 'xgen-IPA-104-resource-has-GET': { + reason: 'test', + }, + }, + }, + '/standard/{exampleId}': { + patch: {}, + delete: {}, + }, + }, + }, + errors: [], + }, ]); diff --git a/tools/spectral/ipa/__tests__/exceptionExtensionFormat.test.js b/tools/spectral/ipa/__tests__/exceptionExtensionFormat.test.js new file mode 100644 index 0000000000..a73126027a --- /dev/null +++ b/tools/spectral/ipa/__tests__/exceptionExtensionFormat.test.js @@ -0,0 +1,122 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +testRule('xgen-IPA-005-exception-extension-format', [ + { + name: 'valid exceptions', + document: { + paths: { + '/path': { + 'x-xgen-IPA-exception': { + 'xgen-IPA-100-rule-name': { + reason: 'Exception', + }, + }, + }, + '/nested': { + post: { + 'x-xgen-IPA-exception': { + 'xgen-IPA-100-rule-name': { + reason: 'Exception', + }, + }, + }, + }, + }, + }, + errors: [], + }, + { + name: 'invalid exceptions', + document: { + paths: { + '/path1': { + 'x-xgen-IPA-exception': 'Exception', + }, + '/path2': { + 'x-xgen-IPA-exception': { + 'xgen-IPA-100-rule-name': 'Exception', + }, + }, + '/path3': { + 'x-xgen-IPA-exception': { + 'xgen-IPA-100-rule-name': { + reason: '', + }, + }, + }, + '/path4': { + 'x-xgen-IPA-exception': { + 'invalid-rule-name': { + reason: 'Exception', + }, + }, + }, + '/path5': { + 'x-xgen-IPA-exception': { + 'xgen-IPA-100-rule-name': { + wrongKey: 'Exception', + }, + }, + }, + '/path6': { + 'x-xgen-IPA-exception': { + 'xgen-IPA-100-rule-name': { + reason: 'Exception', + excessKey: 'Exception', + }, + }, + }, + '/path7': { + 'x-xgen-IPA-exception': { + 'xgen-IPA-100-rule-name': {}, + }, + }, + }, + }, + errors: [ + { + code: 'xgen-IPA-005-exception-extension-format', + message: 'IPA exceptions must have a valid rule name and a reason. http://go/ipa/5', + path: ['paths', '/path1', 'x-xgen-IPA-exception'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-005-exception-extension-format', + message: 'IPA exceptions must have a valid rule name and a reason. http://go/ipa/5', + path: ['paths', '/path2', 'x-xgen-IPA-exception', 'xgen-IPA-100-rule-name'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-005-exception-extension-format', + message: 'IPA exceptions must have a valid rule name and a reason. http://go/ipa/5', + path: ['paths', '/path3', 'x-xgen-IPA-exception', 'xgen-IPA-100-rule-name'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-005-exception-extension-format', + message: 'IPA exceptions must have a valid rule name and a reason. http://go/ipa/5', + path: ['paths', '/path4', 'x-xgen-IPA-exception', 'invalid-rule-name'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-005-exception-extension-format', + message: 'IPA exceptions must have a valid rule name and a reason. http://go/ipa/5', + path: ['paths', '/path5', 'x-xgen-IPA-exception', 'xgen-IPA-100-rule-name'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-005-exception-extension-format', + message: 'IPA exceptions must have a valid rule name and a reason. http://go/ipa/5', + path: ['paths', '/path6', 'x-xgen-IPA-exception', 'xgen-IPA-100-rule-name'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-005-exception-extension-format', + message: 'IPA exceptions must have a valid rule name and a reason. http://go/ipa/5', + path: ['paths', '/path7', 'x-xgen-IPA-exception', 'xgen-IPA-100-rule-name'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, +]); diff --git a/tools/spectral/ipa/__tests__/utils/exceptions.test.js b/tools/spectral/ipa/__tests__/utils/exceptions.test.js new file mode 100644 index 0000000000..92e957d910 --- /dev/null +++ b/tools/spectral/ipa/__tests__/utils/exceptions.test.js @@ -0,0 +1,67 @@ +import { describe, expect, it } from '@jest/globals'; +import { hasException } from '../../rulesets/functions/utils/exceptions'; + +const TEST_RULE_NAME_100 = 'xgen-IPA-100'; + +const objectWithIpa100Exception = { + 'x-xgen-IPA-exception': { + 'xgen-IPA-100': { + reason: 'test', + }, + }, +}; + +const objectWithNestedIpa100Exception = { + get: { + 'x-xgen-IPA-exception': { + 'xgen-IPA-100': { + reason: 'test', + }, + }, + }, +}; + +const objectWithIpa100ExceptionAndOwnerExtension = { + 'x-xgen-IPA-exception': { + 'xgen-IPA-100': { + reason: 'test', + }, + }, + 'x-xgen-owner-team': 'apix', +}; + +const objectWithIpa101Exception = { + 'x-xgen-IPA-exception': { + 'xgen-IPA-101': { + reason: 'test', + }, + }, +}; + +const objectWithIpa100And101Exception = { + 'x-xgen-IPA-exception': { + 'xgen-IPA-101': { + reason: 'test', + }, + 'xgen-IPA-100': { + reason: 'test', + }, + }, +}; + +describe('tools/spectral/ipa/rulesets/functions/utils/exceptions.js', () => { + describe('hasException', () => { + it('returns true if object has exception matching the rule name', () => { + expect(hasException(objectWithIpa100Exception, TEST_RULE_NAME_100)).toBe(true); + expect(hasException(objectWithIpa100ExceptionAndOwnerExtension, TEST_RULE_NAME_100)).toBe(true); + expect(hasException(objectWithIpa100And101Exception, TEST_RULE_NAME_100)).toBe(true); + }); + it('returns false if object does not have exception matching the rule name', () => { + expect(hasException({}, TEST_RULE_NAME_100)).toBe(false); + expect(hasException(objectWithIpa101Exception, TEST_RULE_NAME_100)).toBe(false); + }); + it('returns false if object has nested exception matching the rule name', () => { + expect(hasException(objectWithNestedIpa100Exception, TEST_RULE_NAME_100)).toBe(false); + }); + }); +}); diff --git a/tools/spectral/ipa/ipa-spectral.yaml b/tools/spectral/ipa/ipa-spectral.yaml index 9e5e2b3ab7..a948e1c544 100644 --- a/tools/spectral/ipa/ipa-spectral.yaml +++ b/tools/spectral/ipa/ipa-spectral.yaml @@ -1,3 +1,4 @@ extends: - ./rulesets/IPA-102.yaml - ./rulesets/IPA-104.yaml + - ./rulesets/IPA-005.yaml diff --git a/tools/spectral/ipa/rulesets/IPA-005.yaml b/tools/spectral/ipa/rulesets/IPA-005.yaml new file mode 100644 index 0000000000..e1bb7907be --- /dev/null +++ b/tools/spectral/ipa/rulesets/IPA-005.yaml @@ -0,0 +1,14 @@ +# IPA-5: Documenting Exceptions to IPAs +# http://go/ipa/5 + +functions: + - exceptionExtensionFormat + +rules: + xgen-IPA-005-exception-extension-format: + description: 'IPA exception extensions must follow the correct format. http://go/ipa/5' + message: '{{error}} http://go/ipa/5' + severity: warn + given: '$..x-xgen-IPA-exception' + then: + function: 'exceptionExtensionFormat' diff --git a/tools/spectral/ipa/rulesets/functions/eachPathAlternatesBetweenResourceNameAndPathParam.js b/tools/spectral/ipa/rulesets/functions/eachPathAlternatesBetweenResourceNameAndPathParam.js index fef49d146f..d3787a2cec 100644 --- a/tools/spectral/ipa/rulesets/functions/eachPathAlternatesBetweenResourceNameAndPathParam.js +++ b/tools/spectral/ipa/rulesets/functions/eachPathAlternatesBetweenResourceNameAndPathParam.js @@ -1,13 +1,19 @@ import { isPathParam } from './utils/pathUtils.js'; +import { hasException } from './utils/exceptions'; +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 ERROR_RESULT = [{ message: ERROR_MESSAGE }]; const AUTH_PREFIX = '/api/atlas/v2'; const UNAUTH_PREFIX = '/api/atlas/v2/unauth'; const getPrefix = (path) => { - if (path.includes(UNAUTH_PREFIX)) return UNAUTH_PREFIX; - if (path.includes(AUTH_PREFIX)) return AUTH_PREFIX; + if (path.includes(UNAUTH_PREFIX)) { + return UNAUTH_PREFIX; + } + if (path.includes(AUTH_PREFIX)) { + return AUTH_PREFIX; + } return null; }; @@ -18,9 +24,16 @@ const validatePathStructure = (elements) => { }); }; -export default (input) => { +export default (input, _, { documentInventory }) => { + const oas = documentInventory.resolved; + if (hasException(oas.paths[input], RULE_NAME)) { + return; + } + const prefix = getPrefix(input); - if (!prefix) return; + if (!prefix) { + return; + } let suffixWithLeadingSlash = input.slice(prefix.length); if (suffixWithLeadingSlash.length === 0) { diff --git a/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js b/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js index 3943e2e34f..62ed1ce66f 100644 --- a/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js +++ b/tools/spectral/ipa/rulesets/functions/eachResourceHasGetMethod.js @@ -6,7 +6,9 @@ import { isSingletonResource, getResourcePaths, } from './utils/resourceEvaluation.js'; +import { hasException } from './utils/exceptions'; +const RULE_NAME = 'xgen-IPA-104-resource-has-GET'; const ERROR_MESSAGE = 'APIs must provide a get method for resources.'; export default (input, _, { documentInventory }) => { @@ -15,6 +17,11 @@ export default (input, _, { documentInventory }) => { } const oas = documentInventory.resolved; + + if (hasException(oas.paths[input], RULE_NAME)) { + return; + } + const resourcePaths = getResourcePaths(input, Object.keys(oas.paths)); if (isSingletonResource(resourcePaths)) { diff --git a/tools/spectral/ipa/rulesets/functions/exceptionExtensionFormat.js b/tools/spectral/ipa/rulesets/functions/exceptionExtensionFormat.js new file mode 100644 index 0000000000..71dfefebdc --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/exceptionExtensionFormat.js @@ -0,0 +1,31 @@ +const ERROR_MESSAGE = 'IPA exceptions must have a valid rule name and a reason.'; +const RULE_NAME_PREFIX = 'xgen-IPA-'; +const REASON_KEY = 'reason'; + +// Note: This rule does not allow exceptions +export default (input, _, { path }) => { + const exemptedRules = Object.keys(input); + const errors = []; + + exemptedRules.forEach((ruleName) => { + const exception = input[ruleName]; + if (!isValidException(ruleName, exception)) { + errors.push({ + path: path.concat([ruleName]), + message: ERROR_MESSAGE, + }); + } + }); + + return errors; +}; + +function isValidException(ruleName, exception) { + const exceptionObjectKeys = Object.keys(exception); + return ( + ruleName.startsWith(RULE_NAME_PREFIX) && + exceptionObjectKeys.length === 1 && + exceptionObjectKeys.includes(REASON_KEY) && + exception[REASON_KEY] !== '' + ); +} diff --git a/tools/spectral/ipa/rulesets/functions/utils/exceptions.js b/tools/spectral/ipa/rulesets/functions/utils/exceptions.js new file mode 100644 index 0000000000..91b6f84f68 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/utils/exceptions.js @@ -0,0 +1,15 @@ +const EXCEPTION_EXTENSION = 'x-xgen-IPA-exception'; + +/** + * Checks if the object has an exception extension "x-xgen-IPA-exception" + * + * @param object the object to evaluate + * @param ruleName the name of the exempted rule + * @returns {boolean} true if the object has an exception named ruleName, otherwise false + */ +export function hasException(object, ruleName) { + if (object[EXCEPTION_EXTENSION]) { + return Object.keys(object[EXCEPTION_EXTENSION]).includes(ruleName); + } + return false; +} diff --git a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js index 04d7fb9177..0c7d61e0e1 100644 --- a/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js +++ b/tools/spectral/ipa/rulesets/functions/utils/resourceEvaluation.js @@ -54,7 +54,7 @@ export function hasGetMethod(pathObject) { * * @param parent the parent path string * @param allPaths all paths as an array of strings - * @returns {*} a string array of all paths for a resource, including the parent + * @returns {string[]} all paths for a resource, including the parent */ export function getResourcePaths(parent, allPaths) { const childPathPattern = new RegExp(`^${parent}/{[a-zA-Z]+}$`);