CLOUDP-305172: Refactor the rule implementations (#500)

Author: yelizhenden-mdb

tools/spectral/ipa/__tests__/createMethodShouldNotHaveQueryParameters.test.js

@@ -15,6 +15,13 @@ const componentSchemas = {
         type: 'string',
       },
     },
+    QueryParam2: {
+      name: 'query-param-2',
+      in: 'query',
+      schema: {
+        type: 'string',
+      },
+    },
     PathParam: {
       name: 'resource-id',
       in: 'path',
@@ -102,6 +109,9 @@ testRule('xgen-IPA-106-create-method-should-not-have-query-parameters', [
               {
                 $ref: '#/components/parameters/QueryParam',
               },
+              {
+                $ref: '#/components/parameters/QueryParam2',
+              },
             ],
           },
         },
@@ -110,13 +120,20 @@ testRule('xgen-IPA-106-create-method-should-not-have-query-parameters', [
     errors: [
       {
         code: 'xgen-IPA-106-create-method-should-not-have-query-parameters',
-        message: 'Create operations should not have query parameters. http://go/ipa/106',
+        message: 'Input parameter [filter]: Create operations should not have query parameters. http://go/ipa/106',
         path: ['paths', '/resource', 'post'],
         severity: DiagnosticSeverity.Warning,
       },
       {
         code: 'xgen-IPA-106-create-method-should-not-have-query-parameters',
-        message: 'Create operations should not have query parameters. http://go/ipa/106',
+        message: 'Input parameter [query-param]: Create operations should not have query parameters. http://go/ipa/106',
+        path: ['paths', '/resource2', 'post'],
+        severity: DiagnosticSeverity.Warning,
+      },
+      {
+        code: 'xgen-IPA-106-create-method-should-not-have-query-parameters',
+        message:
+          'Input parameter [query-param-2]: Create operations should not have query parameters. http://go/ipa/106',
         path: ['paths', '/resource2', 'post'],
         severity: DiagnosticSeverity.Warning,
       },

tools/spectral/ipa/rulesets/IPA-104.yaml

@@ -22,8 +22,9 @@ rules:
     description: 'The purpose of the Get method is to return data from a single resource. http://go/ipa/104'
     message: '{{error}} http://go/ipa/104'
     severity: warn
-    given: '$.paths[*].get'
+    given: '$.paths[*].get.responses[*].content'
     then:
+      field: '@key'
       function: 'getMethodReturnsSingleResource'
   xgen-IPA-104-get-method-response-code-is-200:
     description: 'The Get method must return a 200 OK response. http://go/ipa/104'

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

@@ -1,5 +1,10 @@
 import { hasException } from './utils/exceptions.js';
-import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js';
+import {
+  collectAdoption,
+  collectAndReturnViolation,
+  collectException,
+  handleInternalError,
+} from './utils/collectionUtils.js';
 import { isCustomMethodIdentifier } from './utils/resourceEvaluation.js';
 import { resolveObject } from './utils/componentUtils.js';
 import { getSchemaRef } from './utils/methodUtils.js';
@@ -11,28 +16,37 @@ const ERROR_MESSAGE_SCHEMA_REF = 'The response body schema is defined inline and
 export default (input, _, { path, documentInventory }) => {
   const oas = documentInventory.unresolved;
   const resourcePath = path[1];
-  const contentMediaType = path[path.length - 1];
+  const contentPerMediaType = resolveObject(oas, path);
 
-  if (isCustomMethodIdentifier(resourcePath) || !contentMediaType.endsWith('json')) {
+  if (isCustomMethodIdentifier(resourcePath) || !input.endsWith('json') || !contentPerMediaType.schema) {
     return;
   }
 
-  const contentPerMediaType = resolveObject(oas, path);
-
   if (hasException(contentPerMediaType, RULE_NAME)) {
     collectException(contentPerMediaType, RULE_NAME, path);
     return;
   }
 
-  if (contentPerMediaType.schema) {
+  const errors = checkViolationsAndReturnErrors(contentPerMediaType, path);
+  if (errors.length !== 0) {
+    return collectAndReturnViolation(path, RULE_NAME, errors);
+  }
+  collectAdoption(path, RULE_NAME);
+};
+
+function checkViolationsAndReturnErrors(contentPerMediaType, path) {
+  try {
     const schema = contentPerMediaType.schema;
     const schemaRef = getSchemaRef(schema);
+
     if (!schemaRef) {
-      return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE_SCHEMA_REF);
+      return [{ path, message: ERROR_MESSAGE_SCHEMA_REF }];
     }
     if (!schemaRef.endsWith('Request')) {
-      return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE_SCHEMA_NAME);
+      return [{ path, message: ERROR_MESSAGE_SCHEMA_NAME }];
     }
-    collectAdoption(path, RULE_NAME);
+    return [];
+  } catch (e) {
+    handleInternalError(RULE_NAME, path, e);
   }
-};
+}

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

@@ -1,5 +1,10 @@
 import { hasException } from './utils/exceptions.js';
-import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js';
+import {
+  collectAdoption,
+  collectAndReturnViolation,
+  collectException,
+  handleInternalError,
+} from './utils/collectionUtils.js';
 import { isCustomMethodIdentifier } from './utils/resourceEvaluation.js';
 
 const RULE_NAME = 'xgen-IPA-106-create-method-should-not-have-query-parameters';
@@ -24,11 +29,26 @@ export default (input, _, { path }) => {
     return;
   }
 
-  for (const parameter of postMethod.parameters) {
-    if (parameter.in === 'query' && !ignoredParameters.includes(parameter.name)) {
-      return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE);
-    }
+  const errors = checkViolationsAndReturnErrors(postMethod.parameters, path);
+  if (errors.length !== 0) {
+    return collectAndReturnViolation(path, RULE_NAME, errors);
   }
-
   collectAdoption(path, RULE_NAME);
 };
+
+function checkViolationsAndReturnErrors(postMethodParameters, path) {
+  const errors = [];
+  try {
+    for (const parameter of postMethodParameters) {
+      if (parameter.in === 'query' && !ignoredParameters.includes(parameter.name)) {
+        errors.push({
+          path: path,
+          message: `Input parameter [${parameter.name}]: ${ERROR_MESSAGE}`,
+        });
+      }
+    }
+    return errors;
+  } catch (e) {
+    handleInternalError(RULE_NAME, path, e);
+  }
+}

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

@@ -1,4 +1,9 @@
-import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js';
+import {
+  collectAdoption,
+  collectAndReturnViolation,
+  collectException,
+  handleInternalError,
+} from './utils/collectionUtils.js';
 import { hasException } from './utils/exceptions.js';
 
 const RULE_NAME = 'xgen-IPA-108-delete-method-return-204-response';
@@ -17,9 +22,29 @@ export default (input, _, { path }) => {
     return;
   }
 
-  const responses = input.responses;
-  if (!responses || !responses['204']) {
-    return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE);
+  const errors = checkViolationsAndReturnErrors(input, path);
+  if (errors.length !== 0) {
+    return collectAndReturnViolation(path, RULE_NAME, errors);
   }
-  return collectAdoption(path, RULE_NAME);
+  collectAdoption(path, RULE_NAME);
 };
+
+function checkViolationsAndReturnErrors(input, path) {
+  try {
+    console.log(input);
+    const responses = input.responses;
+    console.log(responses);
+    // If there is no 204 response, return a violation
+    if (!responses || !responses['204']) {
+      return [{ path, message: ERROR_MESSAGE }];
+    }
+
+    // If there are other 2xx responses that are not 204, return a violation
+    if (Object.keys(responses).some((key) => key.startsWith('2') && key !== '204')) {
+      return [{ path, message: ERROR_MESSAGE }];
+    }
+    return [];
+  } catch (e) {
+    handleInternalError(RULE_NAME, path, e);
+  }
+}

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

@@ -23,7 +23,7 @@ export default (input, _, { path }) => {
   }
 
   // 2. Validation
-  const errors = checkViolations(input, path);
+  const errors = checkViolationsAndReturnErrors(input, path);
   if (errors.length > 0) {
     return collectAndReturnViolation(path, RULE_NAME, errors);
   }
@@ -37,7 +37,7 @@ export default (input, _, { path }) => {
  * @param {object} jsonPathArray - The jsonPathArray covering location in the OpenAPI schema
  * @return {Array<string>} - errors array ()
  */
-function checkViolations(input, jsonPathArray) {
+function checkViolationsAndReturnErrors(input, jsonPathArray) {
   const errors = [];
   try {
     const successResponse = input;

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

@@ -1,13 +1,18 @@
 import { isCustomMethodIdentifier } from './utils/resourceEvaluation.js';
 import { hasException } from './utils/exceptions.js';
-import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js';
+import {
+  collectAdoption,
+  collectAndReturnViolation,
+  collectException,
+  handleInternalError,
+} from './utils/collectionUtils.js';
 
 const RULE_NAME = 'xgen-IPA-109-custom-method-must-be-GET-or-POST';
 const ERROR_MESSAGE = 'The HTTP method for custom methods must be GET or POST.';
 const VALID_METHODS = ['get', 'post'];
 const HTTP_METHODS = ['get', 'put', 'post', 'delete', 'options', 'head', 'patch', 'trace'];
 
-export default (input, opts, { path }) => {
+export default (input, _, { path }) => {
   // Extract the path key (e.g., '/a/{exampleId}:method') from the JSONPath.
   let pathKey = path[1];
 
@@ -20,21 +25,32 @@ export default (input, opts, { path }) => {
     return;
   }
 
-  //Extract the keys which are equivalent of the http methods
-  let keys = Object.keys(input);
-  const httpMethods = keys.filter((key) => HTTP_METHODS.includes(key));
-
-  // Check for invalid methods
-  if (httpMethods.some((method) => !VALID_METHODS.includes(method))) {
-    return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE);
-  }
-
-  // Check for multiple valid methods
-  const validMethodCount = httpMethods.filter((method) => VALID_METHODS.includes(method)).length;
-
-  if (validMethodCount > 1) {
-    return collectAndReturnViolation(path, RULE_NAME, ERROR_MESSAGE);
+  const errors = checkViolationsAndReturnErrors(input, path);
+  if (errors.length !== 0) {
+    return collectAndReturnViolation(path, RULE_NAME, errors);
   }
-
   collectAdoption(path, RULE_NAME);
 };
+
+function checkViolationsAndReturnErrors(input, path) {
+  try {
+    //Extract the keys which are equivalent of the http methods
+    let keys = Object.keys(input);
+    const httpMethods = keys.filter((key) => HTTP_METHODS.includes(key));
+
+    // Check for invalid methods
+    if (httpMethods.some((method) => !VALID_METHODS.includes(method))) {
+      return [{ path, message: ERROR_MESSAGE }];
+    }
+
+    // Check for multiple valid methods
+    const validMethodCount = httpMethods.filter((method) => VALID_METHODS.includes(method)).length;
+
+    if (validMethodCount > 1) {
+      return [{ path, message: ERROR_MESSAGE }];
+    }
+    return [];
+  } catch (e) {
+    handleInternalError(RULE_NAME, path, e);
+  }
+}

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

@@ -1,7 +1,12 @@
 import { getCustomMethodName, isCustomMethodIdentifier } from './utils/resourceEvaluation.js';
 import { hasException } from './utils/exceptions.js';
 import { casing } from '@stoplight/spectral-functions';
-import { collectAdoption, collectAndReturnViolation, collectException } from './utils/collectionUtils.js';
+import {
+  collectAdoption,
+  collectAndReturnViolation,
+  collectException,
+  handleInternalError,
+} from './utils/collectionUtils.js';
 
 const RULE_NAME = 'xgen-IPA-109-custom-method-must-use-camel-case';
 
@@ -18,16 +23,27 @@ export default (input, opts, { path }) => {
     return;
   }
 
-  let methodName = getCustomMethodName(pathKey);
-  if (methodName.length === 0 || methodName.trim().length === 0) {
-    const errorMessage = 'Custom method name cannot be empty or blank.';
-    return collectAndReturnViolation(path, RULE_NAME, errorMessage);
+  const errors = checkViolationsAndReturnErrors(pathKey, path);
+  if (errors.length !== 0) {
+    return collectAndReturnViolation(path, RULE_NAME, errors);
   }
-
-  if (casing(methodName, { type: 'camel', disallowDigits: true })) {
-    const errorMessage = `${methodName} must use camelCase format.`;
-    return collectAndReturnViolation(path, RULE_NAME, errorMessage);
-  }
-
   collectAdoption(path, RULE_NAME);
 };
+
+function checkViolationsAndReturnErrors(pathKey, path) {
+  try {
+    let methodName = getCustomMethodName(pathKey);
+    if (methodName.length === 0 || methodName.trim().length === 0) {
+      const errorMessage = 'Custom method name cannot be empty or blank.';
+      return [{ path, message: errorMessage }];
+    }
+
+    if (casing(methodName, { type: 'camel', disallowDigits: true })) {
+      const errorMessage = `${methodName} must use camelCase format.`;
+      return [{ path, message: errorMessage }];
+    }
+    return [];
+  } catch (e) {
+    handleInternalError(RULE_NAME, path, e);
+  }
+}