From a8e254c7abda3b602332dca9046595ff369f96e6 Mon Sep 17 00:00:00 2001 From: wtrocki Date: Mon, 10 Mar 2025 17:44:13 +0100 Subject: [PATCH 1/2] CLOUDP-304930: IPA 102: collection pattern --- .../collectionIdentifierPattern.test.js | 92 +++++++++++++++++++ tools/spectral/ipa/rulesets/IPA-102.yaml | 15 ++- tools/spectral/ipa/rulesets/README.md | 7 +- .../functions/collectionIdentifierPattern.js | 55 +++++++++++ 4 files changed, 163 insertions(+), 6 deletions(-) create mode 100644 tools/spectral/ipa/__tests__/collectionIdentifierPattern.test.js create mode 100644 tools/spectral/ipa/rulesets/functions/collectionIdentifierPattern.js diff --git a/tools/spectral/ipa/__tests__/collectionIdentifierPattern.test.js b/tools/spectral/ipa/__tests__/collectionIdentifierPattern.test.js new file mode 100644 index 0000000000..987a0a1792 --- /dev/null +++ b/tools/spectral/ipa/__tests__/collectionIdentifierPattern.test.js @@ -0,0 +1,92 @@ +import testRule from './__helpers__/testRule'; +import { DiagnosticSeverity } from '@stoplight/types'; + +testRule('xgen-IPA-102-collection-identifier-pattern', [ + { + name: 'valid collection identifiers', + document: { + paths: { + '/resources': {}, + '/users': {}, + '/resourceGroups': {}, + '/api/v2/customers/payments': {}, + }, + }, + errors: [], + }, + { + name: 'valid with path parameters', + document: { + paths: { + '/resources/{id}': {}, + '/users/{userId}/profiles': {}, + }, + }, + errors: [], + }, + { + name: 'valid with custom methods', + document: { + paths: { + '/resources:create': {}, + '/users/{userId}:activate': {}, + }, + }, + errors: [], + }, + { + name: 'invalid starts with uppercase', + document: { + paths: { + '/Resources': {}, + }, + }, + errors: [ + { + code: 'xgen-IPA-102-collection-identifier-pattern', + message: + "Collection identifiers must begin with a lowercase letter and contain only ASCII letters and numbers (/[a-z][a-zA-Z0-9]*/). Path segment 'Resources' in path '/Resources' doesn't match the required pattern. http://go/ipa/102", + path: ['paths', '/Resources'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'invalid with special characters', + document: { + paths: { + '/resource-groups': {}, + '/user_profiles': {}, + }, + }, + errors: [ + { + code: 'xgen-IPA-102-collection-identifier-pattern', + message: + "Collection identifiers must begin with a lowercase letter and contain only ASCII letters and numbers (/[a-z][a-zA-Z0-9]*/). Path segment 'resource-groups' in path '/resource-groups' doesn't match the required pattern. http://go/ipa/102", + path: ['paths', '/resource-groups'], + severity: DiagnosticSeverity.Warning, + }, + { + code: 'xgen-IPA-102-collection-identifier-pattern', + message: + "Collection identifiers must begin with a lowercase letter and contain only ASCII letters and numbers (/[a-z][a-zA-Z0-9]*/). Path segment 'user_profiles' in path '/user_profiles' doesn't match the required pattern. http://go/ipa/102", + path: ['paths', '/user_profiles'], + severity: DiagnosticSeverity.Warning, + }, + ], + }, + { + name: 'valid with path-level exception', + document: { + paths: { + '/resource-groups': { + 'x-xgen-IPA-exception': { + 'xgen-IPA-102-collection-identifier-pattern': 'Legacy API path that cannot be changed', + }, + }, + }, + }, + errors: [], + }, +]); diff --git a/tools/spectral/ipa/rulesets/IPA-102.yaml b/tools/spectral/ipa/rulesets/IPA-102.yaml index d20a80c0ef..a8d9dc0175 100644 --- a/tools/spectral/ipa/rulesets/IPA-102.yaml +++ b/tools/spectral/ipa/rulesets/IPA-102.yaml @@ -1,9 +1,6 @@ # IPA-102: Resource Identifiers # http://go/ipa/102 -functions: - - eachPathAlternatesBetweenResourceNameAndPathParam - rules: xgen-IPA-102-path-alternate-resource-name-path-param: description: 'Paths should alternate between resource names and path params. http://go/ipa/102' @@ -13,3 +10,15 @@ rules: then: field: '@key' function: 'eachPathAlternatesBetweenResourceNameAndPathParam' + + xgen-IPA-102-collection-identifier-pattern: + description: Collection identifiers must begin with a lowercase letter and contain only ASCII letters and numbers. http://go/ipa/102 + message: '{{error}} http://go/ipa/102' + severity: warn + given: $.paths + then: + function: collectionIdentifierPattern + +functions: + - collectionIdentifierPattern + - eachPathAlternatesBetweenResourceNameAndPathParam diff --git a/tools/spectral/ipa/rulesets/README.md b/tools/spectral/ipa/rulesets/README.md index e0139de793..ac5f456d30 100644 --- a/tools/spectral/ipa/rulesets/README.md +++ b/tools/spectral/ipa/rulesets/README.md @@ -20,9 +20,10 @@ For rule definitions, see [IPA-005.yaml](https://github.com/mongodb/openapi/blob For rule definitions, see [IPA-102.yaml](https://github.com/mongodb/openapi/blob/main/tools/spectral/ipa/rulesets/IPA-102.yaml). -| Rule Name | Description | Severity | -| ---------------------------------------------------- | -------------------------------------------------------------------------------- | -------- | -| xgen-IPA-102-path-alternate-resource-name-path-param | Paths should alternate between resource names and path params. http://go/ipa/102 | error | +| Rule Name | Description | Severity | +| ---------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------- | -------- | +| xgen-IPA-102-path-alternate-resource-name-path-param | Paths should alternate between resource names and path params. http://go/ipa/102 | error | +| xgen-IPA-102-collection-identifier-pattern | Collection identifiers must begin with a lowercase letter and contain only ASCII letters and numbers. http://go/ipa/102 | warn | ### IPA-104 diff --git a/tools/spectral/ipa/rulesets/functions/collectionIdentifierPattern.js b/tools/spectral/ipa/rulesets/functions/collectionIdentifierPattern.js new file mode 100644 index 0000000000..e22301c9d9 --- /dev/null +++ b/tools/spectral/ipa/rulesets/functions/collectionIdentifierPattern.js @@ -0,0 +1,55 @@ +import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js'; +import { hasException } from './utils/exceptions.js'; + +const RULE_NAME = 'xgen-IPA-102-collection-identifier-pattern'; +const ERROR_MESSAGE = + 'Collection identifiers must begin with a lowercase letter and contain only ASCII letters and numbers (/[a-z][a-zA-Z0-9]*/).'; +const VALID_IDENTIFIER_PATTERN = /^[a-z][a-zA-Z0-9]*$/; + +/** + * Checks if collection identifiers in paths begin with a lowercase letter and contain only ASCII letters and numbers + * + * @param {object} input - The paths object from the OpenAPI spec + * @param {object} _ - Unused + * @param {object} context - The context object containing the path + */ +export default (input, _, { path }) => { + const violations = []; + + Object.keys(input).forEach((pathKey) => { + // Check for exception at the path level + if (input[pathKey] && hasException(input[pathKey], RULE_NAME)) { + collectException(input[pathKey], RULE_NAME, [...path, pathKey]); + return; + } + + // Skip path parameters and custom methods + const pathSegments = pathKey.split('/').filter((segment) => segment.length > 0); + + pathSegments.forEach((segment) => { + // Skip path parameters (those inside curly braces) + if (segment.startsWith('{') && segment.endsWith('}')) { + return; + } + + // Skip segments with custom methods (containing :) + if (segment.includes(':')) { + return; + } + + // Check the pattern + if (!VALID_IDENTIFIER_PATTERN.test(segment)) { + violations.push({ + message: `${ERROR_MESSAGE} Path segment '${segment}' in path '${pathKey}' doesn't match the required pattern.`, + path: [...path, pathKey], + }); + } + }); + }); + + if (violations.length > 0) { + return collectAndReturnViolation(path, RULE_NAME, violations); + } + + return collectAdoption(path, RULE_NAME); +}; From de4968f81e1c0b8b781d26429ee8b46227a7ad11 Mon Sep 17 00:00:00 2001 From: wtrocki Date: Tue, 11 Mar 2025 09:42:10 +0100 Subject: [PATCH 2/2] fix: remove for loop --- tools/spectral/ipa/rulesets/IPA-102.yaml | 1 + .../functions/collectionIdentifierPattern.js | 70 ++++++++++--------- 2 files changed, 38 insertions(+), 33 deletions(-) diff --git a/tools/spectral/ipa/rulesets/IPA-102.yaml b/tools/spectral/ipa/rulesets/IPA-102.yaml index a8d9dc0175..766f67c1c4 100644 --- a/tools/spectral/ipa/rulesets/IPA-102.yaml +++ b/tools/spectral/ipa/rulesets/IPA-102.yaml @@ -17,6 +17,7 @@ rules: severity: warn given: $.paths then: + field: '@key' function: collectionIdentifierPattern functions: diff --git a/tools/spectral/ipa/rulesets/functions/collectionIdentifierPattern.js b/tools/spectral/ipa/rulesets/functions/collectionIdentifierPattern.js index e22301c9d9..2fff421760 100644 --- a/tools/spectral/ipa/rulesets/functions/collectionIdentifierPattern.js +++ b/tools/spectral/ipa/rulesets/functions/collectionIdentifierPattern.js @@ -13,43 +13,47 @@ const VALID_IDENTIFIER_PATTERN = /^[a-z][a-zA-Z0-9]*$/; * @param {object} _ - Unused * @param {object} context - The context object containing the path */ -export default (input, _, { path }) => { - const violations = []; - - Object.keys(input).forEach((pathKey) => { - // Check for exception at the path level - if (input[pathKey] && hasException(input[pathKey], RULE_NAME)) { - collectException(input[pathKey], RULE_NAME, [...path, pathKey]); - return; - } - - // Skip path parameters and custom methods - const pathSegments = pathKey.split('/').filter((segment) => segment.length > 0); - - pathSegments.forEach((segment) => { - // Skip path parameters (those inside curly braces) - if (segment.startsWith('{') && segment.endsWith('}')) { - return; - } - - // Skip segments with custom methods (containing :) - if (segment.includes(':')) { - return; - } - - // Check the pattern - if (!VALID_IDENTIFIER_PATTERN.test(segment)) { - violations.push({ - message: `${ERROR_MESSAGE} Path segment '${segment}' in path '${pathKey}' doesn't match the required pattern.`, - path: [...path, pathKey], - }); - } - }); - }); +export default (input, _, { path, documentInventory }) => { + const oas = documentInventory.resolved; + const pathKey = input; + + // Check for exception at the path level + if (hasException(oas.paths[input], RULE_NAME)) { + collectException(oas.paths[input], RULE_NAME, path); + return; + } + const violations = checkViolations(pathKey, path); if (violations.length > 0) { return collectAndReturnViolation(path, RULE_NAME, violations); } return collectAdoption(path, RULE_NAME); }; + +function checkViolations(pathKey, path) { + const violations = []; + // Skip path parameters and custom methods + const pathSegments = pathKey.split('/').filter((segment) => segment.length > 0); + + pathSegments.forEach((segment) => { + // Skip path parameters (those inside curly braces) + if (segment.startsWith('{') && segment.endsWith('}')) { + return; + } + + // Skip segments with custom methods (containing :) + if (segment.includes(':')) { + return; + } + + // Check the pattern + if (!VALID_IDENTIFIER_PATTERN.test(segment)) { + violations.push({ + message: `${ERROR_MESSAGE} Path segment '${segment}' in path '${pathKey}' doesn't match the required pattern.`, + path: [...path, pathKey], + }); + } + }); + return violations; +}