Skip to content

Commit 911bd0c

Browse files
yelizhenden-mdbyelizhenden-mdb
committed
address the comments
1 parent dce329d commit 911bd0c

9 files changed

+231
-27
lines changed

‎tools/spectral/ipa/__tests__/IPA102CollectionIdentifierCamelCase.test.js‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,4 +270,69 @@ testRule('xgen-IPA-102-collection-identifier-camelCase', [
270270
},
271271
errors: [],
272272
},
273+
{
274+
name: 'child paths have exceptions along with parent exceptions',
275+
document: {
276+
paths: {
277+
'/resource_groups': {
278+
'x-xgen-IPA-exception': {
279+
'xgen-IPA-102-collection-identifier-camelCase': 'Legacy API path that cannot be changed',
280+
},
281+
},
282+
'/resource_groups/{id}': {
283+
'x-xgen-IPA-exception': {
284+
'xgen-IPA-102-collection-identifier-camelCase': 'Legacy API path that cannot be changed',
285+
},
286+
},
287+
'/resource_groups/{id}/User-Profiles': {
288+
'x-xgen-IPA-exception': {
289+
'xgen-IPA-102-collection-identifier-camelCase': 'Legacy API path that cannot be changed',
290+
},
291+
},
292+
'/resource_groups/{id}/User-Profiles/{profileId}': {
293+
'x-xgen-IPA-exception': {
294+
'xgen-IPA-102-collection-identifier-camelCase': 'Legacy API path that cannot be changed',
295+
},
296+
},
297+
},
298+
},
299+
errors: [
300+
{
301+
code: 'xgen-IPA-102-collection-identifier-camelCase',
302+
message:
303+
'This component adopts the rule and does not need an exception. Please remove the exception. https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-102-collection-identifier-camelCase',
304+
path: [
305+
'paths',
306+
'/resource_groups/{id}',
307+
'x-xgen-IPA-exception',
308+
'xgen-IPA-102-collection-identifier-camelCase',
309+
],
310+
severity: DiagnosticSeverity.Error,
311+
},
312+
{
313+
code: 'xgen-IPA-102-collection-identifier-camelCase',
314+
message:
315+
'This component adopts the rule and does not need an exception. Please remove the exception. https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-102-collection-identifier-camelCase',
316+
path: [
317+
'paths',
318+
'/resource_groups/{id}/User-Profiles',
319+
'x-xgen-IPA-exception',
320+
'xgen-IPA-102-collection-identifier-camelCase',
321+
],
322+
severity: DiagnosticSeverity.Error,
323+
},
324+
{
325+
code: 'xgen-IPA-102-collection-identifier-camelCase',
326+
message:
327+
'This component adopts the rule and does not need an exception. Please remove the exception. https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-102-collection-identifier-camelCase',
328+
path: [
329+
'paths',
330+
'/resource_groups/{id}/User-Profiles/{profileId}',
331+
'x-xgen-IPA-exception',
332+
'xgen-IPA-102-collection-identifier-camelCase',
333+
],
334+
severity: DiagnosticSeverity.Error,
335+
},
336+
],
337+
},
273338
]);

‎tools/spectral/ipa/__tests__/IPA102CollectionIdentifierPattern.test.js‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,4 +105,64 @@ testRule('xgen-IPA-102-collection-identifier-pattern', [
105105
},
106106
errors: [],
107107
},
108+
{
109+
name: 'child paths have exceptions along with parent exceptions',
110+
document: {
111+
paths: {
112+
'/resource-groups': {
113+
'x-xgen-IPA-exception': {
114+
'xgen-IPA-102-collection-identifier-pattern': 'Legacy API path that cannot be changed',
115+
},
116+
},
117+
'/resource-groups/{id}': {
118+
'x-xgen-IPA-exception': {
119+
'xgen-IPA-102-collection-identifier-pattern': 'Legacy API path that cannot be changed',
120+
},
121+
},
122+
'/resource-groups/{id}/sub_resources': {
123+
'x-xgen-IPA-exception': {
124+
'xgen-IPA-102-collection-identifier-pattern': 'Legacy API path that cannot be changed',
125+
},
126+
},
127+
'/resource-groups/{id}/sub_resources/{subId}': {
128+
'x-xgen-IPA-exception': {
129+
'xgen-IPA-102-collection-identifier-pattern': 'Legacy API path that cannot be changed',
130+
},
131+
},
132+
},
133+
},
134+
errors: [
135+
{
136+
code: 'xgen-IPA-102-collection-identifier-pattern',
137+
message:
138+
'This component adopts the rule and does not need an exception. Please remove the exception. https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-102-collection-identifier-pattern',
139+
path: ['paths', '/resource-groups/{id}', 'x-xgen-IPA-exception', 'xgen-IPA-102-collection-identifier-pattern'],
140+
severity: DiagnosticSeverity.Error,
141+
},
142+
{
143+
code: 'xgen-IPA-102-collection-identifier-pattern',
144+
message:
145+
'This component adopts the rule and does not need an exception. Please remove the exception. https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-102-collection-identifier-pattern',
146+
path: [
147+
'paths',
148+
'/resource-groups/{id}/sub_resources',
149+
'x-xgen-IPA-exception',
150+
'xgen-IPA-102-collection-identifier-pattern',
151+
],
152+
severity: DiagnosticSeverity.Error,
153+
},
154+
{
155+
code: 'xgen-IPA-102-collection-identifier-pattern',
156+
message:
157+
'This component adopts the rule and does not need an exception. Please remove the exception. https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-102-collection-identifier-pattern',
158+
path: [
159+
'paths',
160+
'/resource-groups/{id}/sub_resources/{subId}',
161+
'x-xgen-IPA-exception',
162+
'xgen-IPA-102-collection-identifier-pattern',
163+
],
164+
severity: DiagnosticSeverity.Error,
165+
},
166+
],
167+
},
108168
]);

‎tools/spectral/ipa/__tests__/IPA102EachPathAlternatesBetweenResourceNameAndPathParam.test.js‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,4 +167,52 @@ testRule('xgen-IPA-102-path-alternate-resource-name-path-param', [
167167
},
168168
errors: [],
169169
},
170+
{
171+
name: 'child paths have exceptions along with parent exceptions',
172+
document: {
173+
paths: {
174+
'/api/atlas/v2/resourceName1/resourceName2': {
175+
'x-xgen-IPA-exception': {
176+
'xgen-IPA-102-path-alternate-resource-name-path-param': 'parent exception reason',
177+
},
178+
},
179+
'/api/atlas/v2/resourceName1/resourceName2/child': {
180+
'x-xgen-IPA-exception': {
181+
'xgen-IPA-102-path-alternate-resource-name-path-param': 'child exception reason',
182+
},
183+
},
184+
'/api/atlas/v2/resourceName1/resourceName2/child/{id}': {
185+
'x-xgen-IPA-exception': {
186+
'xgen-IPA-102-path-alternate-resource-name-path-param': 'child exception reason',
187+
},
188+
},
189+
},
190+
},
191+
errors: [
192+
{
193+
code: 'xgen-IPA-102-path-alternate-resource-name-path-param',
194+
message:
195+
'This component adopts the rule and does not need an exception. Please remove the exception. https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-102-path-alternate-resource-name-path-param',
196+
path: [
197+
'paths',
198+
'/api/atlas/v2/resourceName1/resourceName2/child',
199+
'x-xgen-IPA-exception',
200+
'xgen-IPA-102-path-alternate-resource-name-path-param',
201+
],
202+
severity: DiagnosticSeverity.Error,
203+
},
204+
{
205+
code: 'xgen-IPA-102-path-alternate-resource-name-path-param',
206+
message:
207+
'This component adopts the rule and does not need an exception. Please remove the exception. https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-102-path-alternate-resource-name-path-param',
208+
path: [
209+
'paths',
210+
'/api/atlas/v2/resourceName1/resourceName2/child/{id}',
211+
'x-xgen-IPA-exception',
212+
'xgen-IPA-102-path-alternate-resource-name-path-param',
213+
],
214+
severity: DiagnosticSeverity.Error,
215+
},
216+
],
217+
},
170218
]);

‎tools/spectral/ipa/__tests__/utils/collectionUtils.test.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,6 @@ describe('tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js', () =>
125125
expect(result[0].message).toEqual(
126126
'This component adopts the rule and does not need an exception. Please remove the exception.'
127127
);
128-
expect(collector.add).toHaveBeenCalledTimes(1);
129-
expect(collector.add).toHaveBeenCalledWith(TEST_ENTRY_TYPE.VIOLATION, testPath, testRuleName);
130128
});
131129
});
132130

‎tools/spectral/ipa/rulesets/functions/IPA102CollectionIdentifierCamelCase.js‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ const ERROR_MESSAGE = 'Collection identifiers must be in camelCase.';
99
/**
1010
* Checks if collection identifiers in paths follow camelCase convention
1111
*
12+
* The function checks the entire path hierarchy. If any parent path has an exception, the exception will be inherited.
13+
*
1214
* @param {object} input - The path key from the OpenAPI spec
1315
* @param {object} options - Rule configuration options
1416
* @param {object} context - The context object containing the path and documentInventory
@@ -23,8 +25,11 @@ export default (input, options, { path, documentInventory }) => {
2325
const violations = checkViolations(pathKey, path, ignoredValues);
2426

2527
// Check for exceptions in path hierarchy
26-
const pathWithException = findExceptionInPathHierarchy(oas, pathKey, RULE_NAME);
27-
const objectToCheck = pathWithException ? oas.paths[pathWithException] : oas.paths[input];
28+
const result = findExceptionInPathHierarchy(oas, pathKey, RULE_NAME, path);
29+
if (result?.error) {
30+
return result.error;
31+
}
32+
const objectToCheck = result ? oas.paths[result.parentPath] : oas.paths[input];
2833

2934
return evaluateAndCollectAdoptionStatus(violations, RULE_NAME, objectToCheck, path);
3035
};

‎tools/spectral/ipa/rulesets/functions/IPA102CollectionIdentifierPattern.js‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,23 @@ const VALID_IDENTIFIER_PATTERN = /^[a-z][a-zA-Z0-9]*$/;
99
/**
1010
* Checks if collection identifiers in paths begin with a lowercase letter and contain only ASCII letters and numbers
1111
*
12+
* The function checks the entire path hierarchy. If any parent path has an exception, the exception will be inherited.
13+
*
1214
* @param {object} input - The paths object from the OpenAPI spec
1315
* @param {object} _ - Unused
14-
* @param {object} context - The context object containing the path
16+
* @param {object} context - The context object containing the path and documentInventory
1517
*/
1618
export default (input, _, { path, documentInventory }) => {
1719
const oas = documentInventory.resolved;
1820

1921
const violations = checkViolations(input, path);
2022

2123
// Check for exceptions in path hierarchy
22-
const pathWithException = findExceptionInPathHierarchy(oas, input, RULE_NAME);
23-
const objectToCheck = pathWithException ? oas.paths[pathWithException] : oas.paths[input];
24+
const result = findExceptionInPathHierarchy(oas, input, RULE_NAME, path);
25+
if (result?.error) {
26+
return result.error;
27+
}
28+
const objectToCheck = result ? oas.paths[result.parentPath] : oas.paths[input];
2429

2530
return evaluateAndCollectAdoptionStatus(violations, RULE_NAME, objectToCheck, path);
2631
};

‎tools/spectral/ipa/rulesets/functions/IPA102EachPathAlternatesBetweenResourceNameAndPathParam.js‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,15 @@ const validatePathStructure = (elements) => {
2323
});
2424
};
2525

26+
/**
27+
* Checks if the resource identifier components alternate between collection identifiers and resourceIDs
28+
*
29+
* The function checks the entire path hierarchy. If any parent path has an exception, the exception will be inherited.
30+
*
31+
* @param {object} input - The path key from the OpenAPI spec
32+
* @param {object} _ - Unused
33+
* @param {object} context - The context object containing the path and documentInventory
34+
*/
2635
export default (input, _, { path, documentInventory }) => {
2736
const oas = documentInventory.resolved;
2837

@@ -39,8 +48,12 @@ export default (input, _, { path, documentInventory }) => {
3948
const errors = checkViolationsAndReturnErrors(suffixWithLeadingSlash, path);
4049

4150
// Check for exceptions in path hierarchy
42-
const pathWithException = findExceptionInPathHierarchy(oas, input, RULE_NAME);
43-
const objectToCheck = pathWithException ? oas.paths[pathWithException] : oas.paths[input];
51+
const result = findExceptionInPathHierarchy(oas, input, RULE_NAME, path);
52+
if (result?.error) {
53+
return result.error;
54+
}
55+
56+
const objectToCheck = result ? oas.paths[result.parentPath] : oas.paths[input];
4457

4558
return evaluateAndCollectAdoptionStatus(errors, RULE_NAME, objectToCheck, path);
4659
};

‎tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import collector, { EntryType } from '../../../metrics/collector.js';
2-
import { EXCEPTION_EXTENSION, hasException } from './exceptions.js';
2+
import { EXCEPTION_EXTENSION, getUnnecessaryExceptionError, hasException } from './exceptions.js';
33

44
/**
55
* Evaluates and collects adoptions, exceptions and violations based on the rule, evaluated object and the validation errors.
@@ -21,12 +21,7 @@ export function evaluateAndCollectAdoptionStatus(validationErrors, ruleName, obj
2121
return collectAndReturnViolation(objectPath, ruleName, validationErrors);
2222
}
2323
if (hasException(object, ruleName)) {
24-
return collectAndReturnViolation(objectPath, ruleName, [
25-
{
26-
path: [...objectPath, EXCEPTION_EXTENSION, ruleName],
27-
message: 'This component adopts the rule and does not need an exception. Please remove the exception.',
28-
},
29-
]);
24+
return returnViolation(getUnnecessaryExceptionError(objectPath, ruleName));
3025
}
3126
collectAdoption(objectPath, ruleName);
3227
}
@@ -60,6 +55,10 @@ export function evaluateAndCollectAdoptionStatusWithoutExceptions(validationErro
6055
function collectAndReturnViolation(jsonPath, ruleName, errorData) {
6156
collector.add(EntryType.VIOLATION, jsonPath, ruleName);
6257

58+
return returnViolation(errorData);
59+
}
60+
61+
export function returnViolation(errorData) {
6362
if (typeof errorData === 'string') {
6463
return [{ message: errorData }];
6564
} else if (Array.isArray(errorData)) {

‎tools/spectral/ipa/rulesets/functions/utils/exceptions.js‎

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,28 @@ export function hasException(object, ruleName) {
1414
return false;
1515
}
1616

17+
export function getUnnecessaryExceptionError(objectPath, ruleName) {
18+
return [
19+
{
20+
path: [...objectPath, EXCEPTION_EXTENSION, ruleName],
21+
message: 'This component adopts the rule and does not need an exception. Please remove the exception.',
22+
},
23+
];
24+
}
25+
1726
/**
18-
* Finds an exception in the path hierarchy of an OpenAPI Specification (OAS) document
19-
* for a specific rule name, starting from the current path and traversing up to parent paths.
27+
* Finds an exception for a specified rule name in the current path or its parent paths within the given OpenAPI Specification (OAS) object.
2028
*
21-
* @param {object} oas - OpenAPI Specification document containing the paths.
22-
* @param {string} currentPath - Current path to check for exceptions.
23-
* @param {string} ruleName - Name of the rule for which the exception is being checked.
24-
* @return {string|null} Returns the path where the exception is found,
25-
* or null if no exception is found in the current path or its hierarchy.
29+
* @param {Object} oas - The OpenAPI Specification object containing path definitions.
30+
* @param {string} currentPath - The path to start searching for exceptions.
31+
* @param {string} ruleName - The name of the rule to check for exceptions.
32+
* @param {string} jsonPath - The JSON path to the current operation or entity being checked.
33+
* @return {string|null|Object} The parent path with the rule exception if found, or null if no exceptions exist.
2634
*/
27-
export function findExceptionInPathHierarchy(oas, currentPath, ruleName) {
28-
// Check current path first
35+
export function findExceptionInPathHierarchy(oas, currentPath, ruleName, jsonPath) {
36+
let currentPathHasException = false;
2937
if (hasException(oas.paths[currentPath], ruleName)) {
30-
return currentPath;
38+
currentPathHasException = true;
3139
}
3240

3341
// Check parent paths by removing segments from the end
@@ -36,7 +44,10 @@ export function findExceptionInPathHierarchy(oas, currentPath, ruleName) {
3644
for (let i = pathSegments.length - 1; i > 0; i--) {
3745
const parentPath = '/' + pathSegments.slice(0, i).join('/');
3846
if (oas.paths[parentPath] && hasException(oas.paths[parentPath], ruleName)) {
39-
return parentPath;
47+
if (currentPathHasException) {
48+
return { error: getUnnecessaryExceptionError(jsonPath, ruleName) };
49+
}
50+
return { parentPath: parentPath };
4051
}
4152
}
4253

0 commit comments

Comments
 (0)