feat(ipa): error on unneeded exceptions IPA 005-104

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

@@ -0,0 +1,167 @@
+import { afterEach, describe, expect, it, jest } from '@jest/globals';
+import {
+  evaluateAndCollectAdoptionStatus,
+  evaluateAndCollectAdoptionStatusWithoutExceptions,
+} from '../../rulesets/functions/utils/collectionUtils.js';
+import collector from '../../metrics/collector.js';
+
+const TEST_ERROR_MESSAGE = 'error message';
+const TEST_ENTRY_TYPE = {
+  EXCEPTION: 'exceptions',
+  VIOLATION: 'violations',
+  ADOPTION: 'adoptions',
+};
+
+jest.mock('../../metrics/collector.js', () => {
+  return {
+    getInstance: jest.fn(),
+    add: jest.fn(),
+    EntryType: TEST_ENTRY_TYPE,
+  };
+});
+
+describe('tools/spectral/ipa/rulesets/functions/utils/collectionUtils.js', () => {
+  describe('collectExceptionAdoptionViolations', () => {
+    afterEach(() => {
+      jest.clearAllMocks();
+    });
+
+    it('returns errors and collects violations when there are errors', () => {
+      const testRuleName = 'xgen-IPA-XXX-rule';
+      const testPath = ['paths', '/resource'];
+      const testObject = {
+        get: {},
+      };
+      const testErrors = [
+        {
+          path: testPath,
+          message: TEST_ERROR_MESSAGE,
+        },
+      ];
+
+      const result = evaluateAndCollectAdoptionStatus(testErrors, testRuleName, testObject, testPath);
+
+      expect(result).toEqual(testErrors);
+      expect(collector.add).toHaveBeenCalledTimes(1);
+      expect(collector.add).toHaveBeenCalledWith(TEST_ENTRY_TYPE.VIOLATION, testPath, testRuleName);
+    });
+
+    it('returns errors and collects violations when there are errors - ignores exception for another rule', () => {
+      const testRuleName = 'xgen-IPA-XXX-rule';
+      const testPath = ['paths', '/resource'];
+      const testObject = {
+        get: {},
+        'x-xgen-IPA-exception': {
+          'xgen-IPA-XXX-some-other-rule': 'reason',
+        },
+      };
+      const testErrors = [
+        {
+          path: testPath,
+          message: TEST_ERROR_MESSAGE,
+        },
+      ];
+
+      const result = evaluateAndCollectAdoptionStatus(testErrors, testRuleName, testObject, testPath);
+
+      expect(result).toEqual(testErrors);
+      expect(collector.add).toHaveBeenCalledTimes(1);
+      expect(collector.add).toHaveBeenCalledWith(TEST_ENTRY_TYPE.VIOLATION, testPath, testRuleName);
+    });
+
+    it('returns empty and collects adoptions when there are no errors', () => {
+      const testRuleName = 'xgen-IPA-XXX-rule';
+      const testPath = ['paths', '/resource'];
+      const testObject = {
+        get: {},
+      };
+      const testNoErrors = [];
+
+      const result = evaluateAndCollectAdoptionStatus(testNoErrors, testRuleName, testObject, testPath);
+
+      expect(result).toEqual(undefined);
+      expect(collector.add).toHaveBeenCalledTimes(1);
+      expect(collector.add).toHaveBeenCalledWith(TEST_ENTRY_TYPE.ADOPTION, testPath, testRuleName);
+    });
+
+    it('returns empty and collects exceptions when there are errors and exceptions', () => {
+      const testRuleName = 'xgen-IPA-XXX-rule';
+      const testPath = ['paths', '/resource'];
+      const testObject = {
+        get: {},
+        'x-xgen-IPA-exception': {
+          'xgen-IPA-XXX-rule': 'reason',
+        },
+      };
+      const testErrors = [
+        {
+          path: testPath,
+          message: TEST_ERROR_MESSAGE,
+        },
+      ];
+
+      const result = evaluateAndCollectAdoptionStatus(testErrors, testRuleName, testObject, testPath);
+
+      expect(result).toEqual(undefined);
+      expect(collector.add).toHaveBeenCalledTimes(1);
+      expect(collector.add).toHaveBeenCalledWith(TEST_ENTRY_TYPE.EXCEPTION, testPath, testRuleName, 'reason');
+    });
+
+    it('returns error when there are exceptions and no errors', () => {
+      const testRuleName = 'xgen-IPA-XXX-rule';
+      const testPath = ['paths', '/resource'];
+      const testObject = {
+        get: {},
+        'x-xgen-IPA-exception': {
+          'xgen-IPA-XXX-rule': 'reason',
+        },
+      };
+
+      const result = evaluateAndCollectAdoptionStatus([], testRuleName, testObject, testPath);
+
+      expect(result.length).toEqual(1);
+      expect(Object.keys(result[0])).toEqual(['path', 'message']);
+      expect(result[0].path).toEqual([...testPath, 'x-xgen-IPA-exception', testRuleName]);
+      expect(result[0].message).toEqual(
+        'This component adopts the rule and does not need an exception. Please remove the exception.'
+      );
+      expect(collector.add).toHaveBeenCalledTimes(1);
+      expect(collector.add).toHaveBeenCalledWith(TEST_ENTRY_TYPE.VIOLATION, testPath, testRuleName);
+    });
+  });
+
+  describe('evaluateAndCollectAdoptionStatusWithoutExceptions', () => {
+    afterEach(() => {
+      jest.clearAllMocks();
+    });
+
+    it('returns errors and collects violations when there are errors', () => {
+      const testRuleName = 'xgen-IPA-XXX-rule';
+      const testPath = ['paths', '/resource'];
+      const testErrors = [
+        {
+          path: testPath,
+          message: TEST_ERROR_MESSAGE,
+        },
+      ];
+
+      const result = evaluateAndCollectAdoptionStatusWithoutExceptions(testErrors, testRuleName, testPath);
+
+      expect(result).toEqual(testErrors);
+      expect(collector.add).toHaveBeenCalledTimes(1);
+      expect(collector.add).toHaveBeenCalledWith(TEST_ENTRY_TYPE.VIOLATION, testPath, testRuleName);
+    });
+
+    it('returns empty and collects adoptions when there are no errors', () => {
+      const testRuleName = 'xgen-IPA-XXX-rule';
+      const testPath = ['paths', '/resource'];
+      const testNoErrors = [];
+
+      const result = evaluateAndCollectAdoptionStatusWithoutExceptions(testNoErrors, testRuleName, testPath);
+
+      expect(result).toEqual(undefined);
+      expect(collector.add).toHaveBeenCalledTimes(1);
+      expect(collector.add).toHaveBeenCalledWith(TEST_ENTRY_TYPE.ADOPTION, testPath, testRuleName);
+    });
+  });
+});

tools/spectral/ipa/ipa-spectral.yaml

@@ -124,3 +124,8 @@ overrides:
       - '**#/components/schemas/DataLakeHTTPStore/allOf/1/properties/urls' # external field, to be covered by CLOUDP-293178
     rules:
       xgen-IPA-124-array-max-items: 'off'
+  - files: # To be removed in CLOUDP-337392
+      - '**#/paths/~1api~1atlas~1v2~1orgs~1%7BorgId%7D~1groups'
+      - '**#/paths/~1api~1atlas~1v2~1groups~1%7BgroupId%7D'
+    rules:
+      xgen-IPA-104-get-method-response-has-no-input-fields: 'off'

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

@@ -1,4 +1,4 @@
-import { collectAdoption, collectAndReturnViolation, handleInternalError } from './utils/collectionUtils.js';
+import { evaluateAndCollectAdoptionStatusWithoutExceptions, handleInternalError } from './utils/collectionUtils.js';
 
 const RULE_NAME = 'xgen-IPA-005-exception-extension-format';
 const ERROR_MESSAGE = 'IPA exceptions must have a valid rule name and a reason.';
@@ -7,10 +7,7 @@ const RULE_NAME_PREFIX = 'xgen-IPA-';
 // Note: This rule does not allow exceptions
 export default (input, _, { path }) => {
   const errors = checkViolationsAndReturnErrors(input, path);
-  if (errors.length !== 0) {
-    return collectAndReturnViolation(path, RULE_NAME, errors);
-  }
-  collectAdoption(path, RULE_NAME);
+  return evaluateAndCollectAdoptionStatusWithoutExceptions(errors, RULE_NAME, path);
 };
 
 function isValidException(ruleName, reason) {

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

@@ -1,10 +1,4 @@
-import {
-  collectAdoption,
-  collectAndReturnViolation,
-  collectException,
-  handleInternalError,
-} from './utils/collectionUtils.js';
-import { hasException } from './utils/exceptions.js';
+import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js';
 import { isPathParam } from './utils/componentUtils.js';
 import { casing } from '@stoplight/spectral-functions';
 
@@ -22,21 +16,12 @@ export default (input, options, { 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;
-  }
-
   // Extract ignored values from options
   const ignoredValues = options?.ignoredValues || [];
 
   const violations = checkViolations(pathKey, path, ignoredValues);
-  if (violations.length > 0) {
-    return collectAndReturnViolation(path, RULE_NAME, violations);
-  }
 
-  return collectAdoption(path, RULE_NAME);
+  return evaluateAndCollectAdoptionStatus(violations, RULE_NAME, oas.paths[input], path);
 };
 
 function checkViolations(pathKey, path, ignoredValues = []) {

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

@@ -1,5 +1,4 @@
-import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js';
-import { hasException } from './utils/exceptions.js';
+import { evaluateAndCollectAdoptionStatus } from './utils/collectionUtils.js';
 
 const RULE_NAME = 'xgen-IPA-102-collection-identifier-pattern';
 const ERROR_MESSAGE =
@@ -15,20 +14,10 @@ const VALID_IDENTIFIER_PATTERN = /^[a-z][a-zA-Z0-9]*$/;
  */
 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(input, path);
 
-  const violations = checkViolations(pathKey, path);
-  if (violations.length > 0) {
-    return collectAndReturnViolation(path, RULE_NAME, violations);
-  }
-
-  return collectAdoption(path, RULE_NAME);
+  return evaluateAndCollectAdoptionStatus(violations, RULE_NAME, oas.paths[input], path);
 };
 
 function checkViolations(pathKey, path) {

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

@@ -1,11 +1,5 @@
 import { isPathParam } from './utils/componentUtils.js';
-import { hasException } from './utils/exceptions.js';
-import {
-  collectAdoption,
-  collectAndReturnViolation,
-  collectException,
-  handleInternalError,
-} from './utils/collectionUtils.js';
+import { evaluateAndCollectAdoptionStatus, handleInternalError } 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';
@@ -41,16 +35,9 @@ export default (input, _, { path, documentInventory }) => {
     return;
   }
 
-  if (hasException(oas.paths[input], RULE_NAME)) {
-    collectException(oas.paths[input], RULE_NAME, path);
-    return;
-  }
-
   const errors = checkViolationsAndReturnErrors(suffixWithLeadingSlash, path);
-  if (errors.length !== 0) {
-    return collectAndReturnViolation(path, RULE_NAME, errors);
-  }
-  collectAdoption(path, RULE_NAME);
+
+  return evaluateAndCollectAdoptionStatus(errors, RULE_NAME, oas.paths[input], path);
 };
 
 function checkViolationsAndReturnErrors(suffixWithLeadingSlash, path) {

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

@@ -5,13 +5,7 @@ import {
   isResourceCollectionIdentifier,
   isSingleResourceIdentifier,
 } from './utils/resourceEvaluation.js';
-import { hasException } from './utils/exceptions.js';
-import {
-  collectAdoption,
-  collectAndReturnViolation,
-  collectException,
-  handleInternalError,
-} from './utils/collectionUtils.js';
+import { evaluateAndCollectAdoptionStatus, handleInternalError } from './utils/collectionUtils.js';
 
 const RULE_NAME = 'xgen-IPA-104-resource-has-GET';
 const ERROR_MESSAGE = 'APIs must provide a get method for resources.';
@@ -23,16 +17,9 @@ export default (input, _, { path, documentInventory }) => {
 
   const oas = documentInventory.resolved;
 
-  if (hasException(oas.paths[input], RULE_NAME)) {
-    collectException(oas.paths[input], RULE_NAME, path);
-    return;
-  }
-
   const errors = checkViolationsAndReturnErrors(oas.paths, input, path);
-  if (errors.length !== 0) {
-    return collectAndReturnViolation(path, RULE_NAME, errors);
-  }
-  collectAdoption(path, RULE_NAME);
+
+  return evaluateAndCollectAdoptionStatus(errors, RULE_NAME, oas.paths[input], path);
 };
 
 function checkViolationsAndReturnErrors(oasPaths, input, path) {

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

@@ -1,5 +1,4 @@
-import { hasException } from './utils/exceptions.js';
-import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js';
+import { evaluateAndCollectAdoptionStatus } from './utils/collectionUtils.js';
 import {
   getResourcePathItems,
   isResourceCollectionIdentifier,
@@ -23,17 +22,9 @@ export default (input, _, { path, documentInventory }) => {
     return;
   }
 
-  if (hasException(input, RULE_NAME)) {
-    collectException(input, RULE_NAME, path);
-    return;
-  }
-
   const errors = checkViolationsAndReturnErrors(input, path);
 
-  if (errors.length !== 0) {
-    return collectAndReturnViolation(path, RULE_NAME, errors);
-  }
-  collectAdoption(path, RULE_NAME);
+  return evaluateAndCollectAdoptionStatus(errors, RULE_NAME, input, path);
 };
 
 function checkViolationsAndReturnErrors(getOperationObject, path) {

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

@@ -1,5 +1,4 @@
-import { hasException } from './utils/exceptions.js';
-import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js';
+import { evaluateAndCollectAdoptionStatus } from './utils/collectionUtils.js';
 import {
   getResourcePathItems,
   isResourceCollectionIdentifier,
@@ -30,11 +29,6 @@ export default (input, _, { path, documentInventory }) => {
     return;
   }
 
-  if (hasException(contentPerMediaType, RULE_NAME)) {
-    collectException(contentPerMediaType, RULE_NAME, path);
-    return;
-  }
-
   const errors = checkForbiddenPropertyAttributesAndReturnErrors(
     contentPerMediaType.schema,
     'writeOnly',
@@ -43,8 +37,5 @@ export default (input, _, { path, documentInventory }) => {
     ERROR_MESSAGE
   );
 
-  if (errors.length !== 0) {
-    return collectAndReturnViolation(path, RULE_NAME, errors);
-  }
-  return collectAdoption(path, RULE_NAME);
+  return evaluateAndCollectAdoptionStatus(errors, RULE_NAME, contentPerMediaType, path);
 };