Skip to content

Commit 44e9cfa

Browse files
lovisaberggrenlovisaberggren
authored
feat(ipa): error on unneeded exceptions IPA 005-104 (#877)
1 parent 269b92c commit 44e9cfa

14 files changed

+245
-146
lines changed

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

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
import { afterEach, describe, expect, it, jest } from '@jest/globals';
2+
import {
3+
evaluateAndCollectAdoptionStatus,
4+
evaluateAndCollectAdoptionStatusWithoutExceptions,
5+
} from '../../rulesets/functions/utils/collectionUtils.js';
6+
import collector from '../../metrics/collector.js';
7+
8+
const TEST_ERROR_MESSAGE = 'error message';
9+
const TEST_ENTRY_TYPE = {
10+
EXCEPTION: 'exceptions',
11+
VIOLATION: 'violations',
12+
ADOPTION: 'adoptions',
13+
};
14+
15+
jest.mock('../../metrics/collector.js', () => {
16+
return {
17+
getInstance: jest.fn(),
18+
add: jest.fn(),
19+
EntryType: TEST_ENTRY_TYPE,
20+
};
21+
});
22+
23+
describe('tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js', () => {
24+
describe('collectExceptionAdoptionViolations', () => {
25+
afterEach(() => {
26+
jest.clearAllMocks();
27+
});
28+
29+
it('returns errors and collects violations when there are errors', () => {
30+
const testRuleName = 'xgen-IPA-XXX-rule';
31+
const testPath = ['paths', '/resource'];
32+
const testObject = {
33+
get: {},
34+
};
35+
const testErrors = [
36+
{
37+
path: testPath,
38+
message: TEST_ERROR_MESSAGE,
39+
},
40+
];
41+
42+
const result = evaluateAndCollectAdoptionStatus(testErrors, testRuleName, testObject, testPath);
43+
44+
expect(result).toEqual(testErrors);
45+
expect(collector.add).toHaveBeenCalledTimes(1);
46+
expect(collector.add).toHaveBeenCalledWith(TEST_ENTRY_TYPE.VIOLATION, testPath, testRuleName);
47+
});
48+
49+
it('returns errors and collects violations when there are errors - ignores exception for another rule', () => {
50+
const testRuleName = 'xgen-IPA-XXX-rule';
51+
const testPath = ['paths', '/resource'];
52+
const testObject = {
53+
get: {},
54+
'x-xgen-IPA-exception': {
55+
'xgen-IPA-XXX-some-other-rule': 'reason',
56+
},
57+
};
58+
const testErrors = [
59+
{
60+
path: testPath,
61+
message: TEST_ERROR_MESSAGE,
62+
},
63+
];
64+
65+
const result = evaluateAndCollectAdoptionStatus(testErrors, testRuleName, testObject, testPath);
66+
67+
expect(result).toEqual(testErrors);
68+
expect(collector.add).toHaveBeenCalledTimes(1);
69+
expect(collector.add).toHaveBeenCalledWith(TEST_ENTRY_TYPE.VIOLATION, testPath, testRuleName);
70+
});
71+
72+
it('returns empty and collects adoptions when there are no errors', () => {
73+
const testRuleName = 'xgen-IPA-XXX-rule';
74+
const testPath = ['paths', '/resource'];
75+
const testObject = {
76+
get: {},
77+
};
78+
const testNoErrors = [];
79+
80+
const result = evaluateAndCollectAdoptionStatus(testNoErrors, testRuleName, testObject, testPath);
81+
82+
expect(result).toEqual(undefined);
83+
expect(collector.add).toHaveBeenCalledTimes(1);
84+
expect(collector.add).toHaveBeenCalledWith(TEST_ENTRY_TYPE.ADOPTION, testPath, testRuleName);
85+
});
86+
87+
it('returns empty and collects exceptions when there are errors and exceptions', () => {
88+
const testRuleName = 'xgen-IPA-XXX-rule';
89+
const testPath = ['paths', '/resource'];
90+
const testObject = {
91+
get: {},
92+
'x-xgen-IPA-exception': {
93+
'xgen-IPA-XXX-rule': 'reason',
94+
},
95+
};
96+
const testErrors = [
97+
{
98+
path: testPath,
99+
message: TEST_ERROR_MESSAGE,
100+
},
101+
];
102+
103+
const result = evaluateAndCollectAdoptionStatus(testErrors, testRuleName, testObject, testPath);
104+
105+
expect(result).toEqual(undefined);
106+
expect(collector.add).toHaveBeenCalledTimes(1);
107+
expect(collector.add).toHaveBeenCalledWith(TEST_ENTRY_TYPE.EXCEPTION, testPath, testRuleName, 'reason');
108+
});
109+
110+
it('returns error when there are exceptions and no errors', () => {
111+
const testRuleName = 'xgen-IPA-XXX-rule';
112+
const testPath = ['paths', '/resource'];
113+
const testObject = {
114+
get: {},
115+
'x-xgen-IPA-exception': {
116+
'xgen-IPA-XXX-rule': 'reason',
117+
},
118+
};
119+
120+
const result = evaluateAndCollectAdoptionStatus([], testRuleName, testObject, testPath);
121+
122+
expect(result.length).toEqual(1);
123+
expect(Object.keys(result[0])).toEqual(['path', 'message']);
124+
expect(result[0].path).toEqual([...testPath, 'x-xgen-IPA-exception', testRuleName]);
125+
expect(result[0].message).toEqual(
126+
'This component adopts the rule and does not need an exception. Please remove the exception.'
127+
);
128+
expect(collector.add).toHaveBeenCalledTimes(1);
129+
expect(collector.add).toHaveBeenCalledWith(TEST_ENTRY_TYPE.VIOLATION, testPath, testRuleName);
130+
});
131+
});
132+
133+
describe('evaluateAndCollectAdoptionStatusWithoutExceptions', () => {
134+
afterEach(() => {
135+
jest.clearAllMocks();
136+
});
137+
138+
it('returns errors and collects violations when there are errors', () => {
139+
const testRuleName = 'xgen-IPA-XXX-rule';
140+
const testPath = ['paths', '/resource'];
141+
const testErrors = [
142+
{
143+
path: testPath,
144+
message: TEST_ERROR_MESSAGE,
145+
},
146+
];
147+
148+
const result = evaluateAndCollectAdoptionStatusWithoutExceptions(testErrors, testRuleName, testPath);
149+
150+
expect(result).toEqual(testErrors);
151+
expect(collector.add).toHaveBeenCalledTimes(1);
152+
expect(collector.add).toHaveBeenCalledWith(TEST_ENTRY_TYPE.VIOLATION, testPath, testRuleName);
153+
});
154+
155+
it('returns empty and collects adoptions when there are no errors', () => {
156+
const testRuleName = 'xgen-IPA-XXX-rule';
157+
const testPath = ['paths', '/resource'];
158+
const testNoErrors = [];
159+
160+
const result = evaluateAndCollectAdoptionStatusWithoutExceptions(testNoErrors, testRuleName, testPath);
161+
162+
expect(result).toEqual(undefined);
163+
expect(collector.add).toHaveBeenCalledTimes(1);
164+
expect(collector.add).toHaveBeenCalledWith(TEST_ENTRY_TYPE.ADOPTION, testPath, testRuleName);
165+
});
166+
});
167+
});

‎tools/spectral/ipa/ipa-spectral.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,3 +124,8 @@ overrides:
124124
- '**#/components/schemas/DataLakeHTTPStore/allOf/1/properties/urls' # external field, to be covered by CLOUDP-293178
125125
rules:
126126
xgen-IPA-124-array-max-items: 'off'
127+
- files: # To be removed in CLOUDP-337392
128+
- '**#/paths/~1api~1atlas~1v2~1orgs~1%7BorgId%7D~1groups'
129+
- '**#/paths/~1api~1atlas~1v2~1groups~1%7BgroupId%7D'
130+
rules:
131+
xgen-IPA-104-get-method-response-has-no-input-fields: 'off'

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { collectAdoption, collectAndReturnViolation, handleInternalError } from './utils/collectionUtils.js';
1+
import { evaluateAndCollectAdoptionStatusWithoutExceptions, handleInternalError } from './utils/collectionUtils.js';
22

33
const RULE_NAME = 'xgen-IPA-005-exception-extension-format';
44
const ERROR_MESSAGE = 'IPA exceptions must have a valid rule name and a reason.';
@@ -7,10 +7,7 @@ const RULE_NAME_PREFIX = 'xgen-IPA-';
77
// Note: This rule does not allow exceptions
88
export default (input, _, { path }) => {
99
const errors = checkViolationsAndReturnErrors(input, path);
10-
if (errors.length !== 0) {
11-
return collectAndReturnViolation(path, RULE_NAME, errors);
12-
}
13-
collectAdoption(path, RULE_NAME);
10+
return evaluateAndCollectAdoptionStatusWithoutExceptions(errors, RULE_NAME, path);
1411
};
1512

1613
function isValidException(ruleName, reason) {

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

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,4 @@
1-
import {
2-
collectAdoption,
3-
collectAndReturnViolation,
4-
collectException,
5-
handleInternalError,
6-
} from './utils/collectionUtils.js';
7-
import { hasException } from './utils/exceptions.js';
1+
import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js';
82
import { isPathParam } from './utils/componentUtils.js';
93
import { casing } from '@stoplight/spectral-functions';
104

@@ -22,21 +16,12 @@ export default (input, options, { path, documentInventory }) => {
2216
const oas = documentInventory.resolved;
2317
const pathKey = input;
2418

25-
// Check for exception at the path level
26-
if (hasException(oas.paths[input], RULE_NAME)) {
27-
collectException(oas.paths[input], RULE_NAME, path);
28-
return;
29-
}
30-
3119
// Extract ignored values from options
3220
const ignoredValues = options?.ignoredValues || [];
3321

3422
const violations = checkViolations(pathKey, path, ignoredValues);
35-
if (violations.length > 0) {
36-
return collectAndReturnViolation(path, RULE_NAME, violations);
37-
}
3823

39-
return collectAdoption(path, RULE_NAME);
24+
return evaluateAndCollectAdoptionStatus(violations, RULE_NAME, oas.paths[input], path);
4025
};
4126

4227
function checkViolations(pathKey, path, ignoredValues = []) {

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

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js';
2-
import { hasException } from './utils/exceptions.js';
1+
import { evaluateAndCollectAdoptionStatus } from './utils/collectionUtils.js';
32

43
const RULE_NAME = 'xgen-IPA-102-collection-identifier-pattern';
54
const ERROR_MESSAGE =
@@ -15,20 +14,10 @@ const VALID_IDENTIFIER_PATTERN = /^[a-z][a-zA-Z0-9]*$/;
1514
*/
1615
export default (input, _, { path, documentInventory }) => {
1716
const oas = documentInventory.resolved;
18-
const pathKey = input;
1917

20-
// Check for exception at the path level
21-
if (hasException(oas.paths[input], RULE_NAME)) {
22-
collectException(oas.paths[input], RULE_NAME, path);
23-
return;
24-
}
18+
const violations = checkViolations(input, path);
2519

26-
const violations = checkViolations(pathKey, path);
27-
if (violations.length > 0) {
28-
return collectAndReturnViolation(path, RULE_NAME, violations);
29-
}
30-
31-
return collectAdoption(path, RULE_NAME);
20+
return evaluateAndCollectAdoptionStatus(violations, RULE_NAME, oas.paths[input], path);
3221
};
3322

3423
function checkViolations(pathKey, path) {

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

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,5 @@
11
import { isPathParam } from './utils/componentUtils.js';
2-
import { hasException } from './utils/exceptions.js';
3-
import {
4-
collectAdoption,
5-
collectAndReturnViolation,
6-
collectException,
7-
handleInternalError,
8-
} from './utils/collectionUtils.js';
2+
import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js';
93
import { AUTH_PREFIX, UNAUTH_PREFIX } from './utils/resourceEvaluation.js';
104

115
const RULE_NAME = 'xgen-IPA-102-path-alternate-resource-name-path-param';
@@ -41,16 +35,9 @@ export default (input, _, { path, documentInventory }) => {
4135
return;
4236
}
4337

44-
if (hasException(oas.paths[input], RULE_NAME)) {
45-
collectException(oas.paths[input], RULE_NAME, path);
46-
return;
47-
}
48-
4938
const errors = checkViolationsAndReturnErrors(suffixWithLeadingSlash, path);
50-
if (errors.length !== 0) {
51-
return collectAndReturnViolation(path, RULE_NAME, errors);
52-
}
53-
collectAdoption(path, RULE_NAME);
39+
40+
return evaluateAndCollectAdoptionStatus(errors, RULE_NAME, oas.paths[input], path);
5441
};
5542

5643
function checkViolationsAndReturnErrors(suffixWithLeadingSlash, path) {

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

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,7 @@ import {
55
isResourceCollectionIdentifier,
66
isSingleResourceIdentifier,
77
} from './utils/resourceEvaluation.js';
8-
import { hasException } from './utils/exceptions.js';
9-
import {
10-
collectAdoption,
11-
collectAndReturnViolation,
12-
collectException,
13-
handleInternalError,
14-
} from './utils/collectionUtils.js';
8+
import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js';
159

1610
const RULE_NAME = 'xgen-IPA-104-resource-has-GET';
1711
const ERROR_MESSAGE = 'APIs must provide a get method for resources.';
@@ -23,16 +17,9 @@ export default (input, _, { path, documentInventory }) => {
2317

2418
const oas = documentInventory.resolved;
2519

26-
if (hasException(oas.paths[input], RULE_NAME)) {
27-
collectException(oas.paths[input], RULE_NAME, path);
28-
return;
29-
}
30-
3120
const errors = checkViolationsAndReturnErrors(oas.paths, input, path);
32-
if (errors.length !== 0) {
33-
return collectAndReturnViolation(path, RULE_NAME, errors);
34-
}
35-
collectAdoption(path, RULE_NAME);
21+
22+
return evaluateAndCollectAdoptionStatus(errors, RULE_NAME, oas.paths[input], path);
3623
};
3724

3825
function checkViolationsAndReturnErrors(oasPaths, input, path) {

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

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
import { hasException } from './utils/exceptions.js';
2-
import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js';
1+
import { evaluateAndCollectAdoptionStatus } from './utils/collectionUtils.js';
32
import {
43
getResourcePathItems,
54
isResourceCollectionIdentifier,
@@ -23,17 +22,9 @@ export default (input, _, { path, documentInventory }) => {
2322
return;
2423
}
2524

26-
if (hasException(input, RULE_NAME)) {
27-
collectException(input, RULE_NAME, path);
28-
return;
29-
}
30-
3125
const errors = checkViolationsAndReturnErrors(input, path);
3226

33-
if (errors.length !== 0) {
34-
return collectAndReturnViolation(path, RULE_NAME, errors);
35-
}
36-
collectAdoption(path, RULE_NAME);
27+
return evaluateAndCollectAdoptionStatus(errors, RULE_NAME, input, path);
3728
};
3829

3930
function checkViolationsAndReturnErrors(getOperationObject, path) {

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

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
import { hasException } from './utils/exceptions.js';
2-
import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js';
1+
import { evaluateAndCollectAdoptionStatus } from './utils/collectionUtils.js';
32
import {
43
getResourcePathItems,
54
isResourceCollectionIdentifier,
@@ -30,11 +29,6 @@ export default (input, _, { path, documentInventory }) => {
3029
return;
3130
}
3231

33-
if (hasException(contentPerMediaType, RULE_NAME)) {
34-
collectException(contentPerMediaType, RULE_NAME, path);
35-
return;
36-
}
37-
3832
const errors = checkForbiddenPropertyAttributesAndReturnErrors(
3933
contentPerMediaType.schema,
4034
'writeOnly',
@@ -43,8 +37,5 @@ export default (input, _, { path, documentInventory }) => {
4337
ERROR_MESSAGE
4438
);
4539

46-
if (errors.length !== 0) {
47-
return collectAndReturnViolation(path, RULE_NAME, errors);
48-
}
49-
return collectAdoption(path, RULE_NAME);
40+
return evaluateAndCollectAdoptionStatus(errors, RULE_NAME, contentPerMediaType, path);
5041
};

0 commit comments

Comments
 (0)