refactor: introduces MessageCarrier to simplify adding warnings and e…

…rrors (#141) Adds a message carrier class to handle collecting errors and warnings
IBM · Feb 19, 2020 · 628d6c0 · 628d6c0
1 parent a29043a
commit 628d6c0
Show file tree

Hide file tree

Showing 36 changed files with 1,064 additions and 955 deletions.
diff --git a/src/plugins/utils/messageCarrier.js b/src/plugins/utils/messageCarrier.js
@@ -0,0 +1,42 @@
+'use strict';
+
+module.exports = class MessageCarrier {
+  constructor() {
+    this._messages = {
+      error: [],
+      warning: []
+    };
+  }
+
+  get messages() {
+    return this._messages;
+  }
+
+  get errors() {
+    return this._messages.error;
+  }
+
+  get warnings() {
+    return this._messages.warning;
+  }
+
+  // status should be 'off', 'error', or 'warning'
+  addMessage(path, message, status) {
+    if (this._messages[status]) {
+      this._messages[status].push({
+        path,
+        message
+      });
+    }
+  }
+
+  addMessageWithAuthId(path, message, authId, status) {
+    if (this._messages[status]) {
+      this._messages[status].push({
+        path,
+        message,
+        authId
+      });
+    }
+  }
+};
diff --git a/src/plugins/validation/2and3/semantic-validators/info.js b/src/plugins/validation/2and3/semantic-validators/info.js
@@ -3,17 +3,20 @@
 
 // Assertation 2:
 // making sure that the required version and title are defined properly
+
+const MessageCarrier = require('../../../utils/messageCarrier');
+
 module.exports.validate = function({ jsSpec }) {
-  const errors = [];
-  const warnings = [];
+  const messages = new MessageCarrier();
 
   const info = jsSpec.info;
   const hasInfo = info && typeof info === 'object';
   if (!hasInfo) {
-    errors.push({
-      path: ['info'],
-      message: 'API definition must have an `info` object'
-    });
+    messages.addMessage(
+      ['info'],
+      'API definition must have an `info` object',
+      'error'
+    );
   } else {
     const title = jsSpec.info.title;
     const hasTitle =
@@ -23,16 +26,18 @@ module.exports.validate = function({ jsSpec }) {
       typeof version === 'string' && version.toString().trim().length > 0;
 
     if (!hasTitle) {
-      errors.push({
-        path: ['info', 'title'],
-        message: '`info` object must have a string-type `title` field'
-      });
+      messages.addMessage(
+        ['info', 'title'],
+        '`info` object must have a string-type `title` field',
+        'error'
+      );
     } else if (!hasVersion) {
-      errors.push({
-        path: ['info', 'version'],
-        message: '`info` object must have a string-type `version` field'
-      });
+      messages.addMessage(
+        ['info', 'version'],
+        '`info` object must have a string-type `version` field',
+        'error'
+      );
     }
   }
-  return { errors, warnings };
+  return messages;
 };
diff --git a/src/plugins/validation/2and3/semantic-validators/items-required-for-array-objects.js b/src/plugins/validation/2and3/semantic-validators/items-required-for-array-objects.js
@@ -9,10 +9,10 @@
 // Headers with 'array' type require an 'items' property
 
 const { walk } = require('../../../utils');
+const MessageCarrier = require('../../../utils/messageCarrier');
 
 module.exports.validate = function({ jsSpec }) {
-  const errors = [];
-  const warnings = [];
+  const messages = new MessageCarrier();
 
   walk(jsSpec, [], function(obj, path) {
     // `definitions` for Swagger 2, `schemas` for OAS 3
@@ -28,23 +28,22 @@ module.exports.validate = function({ jsSpec }) {
 
       // Assertation 1
       if (obj.type === 'array' && typeof obj.items !== 'object') {
-        errors.push({
-          path: path.join('.'),
-          message:
-            "Schema objects with 'array' type require an 'items' property"
-        });
+        messages.addMessage(
+          path.join('.'),
+          "Schema objects with 'array' type require an 'items' property",
+          'error'
+        );
       }
 
       // Assertation 2
       if (Array.isArray(obj.required)) {
         obj.required.forEach((requiredProp, i) => {
           if (!obj.properties || !obj.properties[requiredProp]) {
-            const pathStr = path.concat([`required[${i}]`]).join('.');
-            errors.push({
-              path: pathStr,
-              message:
-                "Schema properties specified as 'required' must be defined"
-            });
+            messages.addMessage(
+              path.concat([`required[${i}]`]).join('.'),
+              "Schema properties specified as 'required' must be defined",
+              'error'
+            );
           }
         });
       }
@@ -53,13 +52,14 @@ module.exports.validate = function({ jsSpec }) {
     // this only applies to Swagger 2
     if (path[path.length - 2] === 'headers') {
       if (obj.type === 'array' && typeof obj.items !== 'object') {
-        errors.push({
+        messages.addMessage(
           path,
-          message: "Headers with 'array' type require an 'items' property"
-        });
+          "Headers with 'array' type require an 'items' property",
+          'error'
+        );
       }
     }
   });
 
-  return { errors, warnings };
+  return messages;
 };
diff --git a/src/plugins/validation/2and3/semantic-validators/operation-ids.js b/src/plugins/validation/2and3/semantic-validators/operation-ids.js
@@ -6,10 +6,10 @@ const pickBy = require('lodash/pickBy');
 const reduce = require('lodash/reduce');
 const merge = require('lodash/merge');
 const each = require('lodash/each');
+const MessageCarrier = require('../../../utils/messageCarrier');
 
 module.exports.validate = function({ resolvedSpec }) {
-  const errors = [];
-  const warnings = [];
+  const messages = new MessageCarrier();
 
   const validOperationKeys = [
     'get',
@@ -137,10 +137,11 @@ module.exports.validate = function({ resolvedSpec }) {
       const hasBeenSeen = tallyOperationId(op.operationId);
       if (hasBeenSeen) {
         // Assertation 1: Operations must have a unique operationId.
-        errors.push({
-          path: op.path + '.operationId',
-          message: 'operationIds must be unique'
-        });
+        messages.addMessage(
+          op.path + '.operationId',
+          'operationIds must be unique',
+          'error'
+        );
       } else {
         // Assertation 2: OperationId must conform to naming conventions
         const regex = RegExp(/{[a-zA-Z0-9_-]+\}$/m);
@@ -153,17 +154,18 @@ module.exports.validate = function({ resolvedSpec }) {
         );
 
         if (checkPassed === false) {
-          warnings.push({
-            path: op.path + '.operationId',
-            message: `operationIds should follow consistent naming convention. operationId verb should be ${verbs}`.replace(
+          messages.addMessage(
+            op.path + '.operationId',
+            `operationIds should follow consistent naming convention. operationId verb should be ${verbs}`.replace(
               ',',
               ' or '
-            )
-          });
+            ),
+            'warning'
+          );
         }
       }
     }
   });
 
-  return { errors, warnings };
+  return messages;
 };
diff --git a/src/plugins/validation/2and3/semantic-validators/operations-shared.js b/src/plugins/validation/2and3/semantic-validators/operations-shared.js
@@ -19,11 +19,10 @@ const map = require('lodash/map');
 const each = require('lodash/each');
 const findIndex = require('lodash/findIndex');
 const { checkCase, hasRefProperty } = require('../../../utils');
+const MessageCarrier = require('../../../utils/messageCarrier');
 
 module.exports.validate = function({ jsSpec, resolvedSpec, isOAS3 }, config) {
-  const result = {};
-  result.error = [];
-  result.warning = [];
+  const messages = new MessageCarrier();
 
   config = config.operations;
 
@@ -50,10 +49,11 @@ module.exports.validate = function({ jsSpec, resolvedSpec, isOAS3 }, config) {
       // check for operations that have a $ref property
       // these are illegal in the spec
       if (hasRefProperty(jsSpec, ['paths', pathKey, opKey])) {
-        result.error.push({
-          path: `paths.${pathKey}.${opKey}.$ref`,
-          message: '$ref found in illegal location'
-        });
+        messages.addMessage(
+          `paths.${pathKey}.${opKey}.$ref`,
+          '$ref found in illegal location',
+          'error'
+        );
       }
 
       // check for unique name/in properties in params
@@ -68,11 +68,11 @@ module.exports.validate = function({ jsSpec, resolvedSpec, isOAS3 }, config) {
         // it also will favor complaining about parameters later in the spec, which
         // makes more sense to the user.
         if (paramIndex !== nameAndInComboIndex) {
-          result.error.push({
-            path: `paths.${pathKey}.${opKey}.parameters[${paramIndex}]`,
-            message:
-              "Operation parameters must have unique 'name' + 'in' properties"
-          });
+          messages.addMessage(
+            `paths.${pathKey}.${opKey}.parameters[${paramIndex}]`,
+            "Operation parameters must have unique 'name' + 'in' properties",
+            'error'
+          );
         }
       });
 
@@ -87,11 +87,11 @@ module.exports.validate = function({ jsSpec, resolvedSpec, isOAS3 }, config) {
                 (content.schema.type === 'array' || content.schema.items);
 
               if (isArray) {
-                result[checkStatusArrRes].push({
-                  path: `paths.${pathKey}.${opKey}.responses.${name}.content.${contentType}.schema`,
-                  message:
-                    'Arrays MUST NOT be returned as the top-level structure in a response body.'
-                });
+                messages.addMessage(
+                  `paths.${pathKey}.${opKey}.responses.${name}.content.${contentType}.schema`,
+                  'Arrays MUST NOT be returned as the top-level structure in a response body.',
+                  checkStatusArrRes
+                );
               }
             });
           } else {
@@ -100,11 +100,11 @@ module.exports.validate = function({ jsSpec, resolvedSpec, isOAS3 }, config) {
               (response.schema.type === 'array' || response.schema.items);
 
             if (isArray) {
-              result[checkStatusArrRes].push({
-                path: `paths.${pathKey}.${opKey}.responses.${name}.schema`,
-                message:
-                  'Arrays MUST NOT be returned as the top-level structure in a response body.'
-              });
+              messages.addMessage(
+                `paths.${pathKey}.${opKey}.responses.${name}.schema`,
+                'Arrays MUST NOT be returned as the top-level structure in a response body.',
+                checkStatusArrRes
+              );
             }
           }
         });
@@ -115,23 +115,22 @@ module.exports.validate = function({ jsSpec, resolvedSpec, isOAS3 }, config) {
         op.operationId.length > 0 &&
         !!op.operationId.toString().trim();
       if (!hasOperationId) {
-        const checkStatus = config.no_operation_id;
-        if (checkStatus !== 'off') {
-          result[checkStatus].push({
-            path: `paths.${pathKey}.${opKey}.operationId`,
-            message: 'Operations must have a non-empty `operationId`.'
-          });
-        }
+        messages.addMessage(
+          `paths.${pathKey}.${opKey}.operationId`,
+          'Operations must have a non-empty `operationId`.',
+          config.no_operation_id
+        );
       } else {
         // check operationId for case convention
         const checkStatus = config.operation_id_case_convention[0];
         const caseConvention = config.operation_id_case_convention[1];
         const isCorrectCase = checkCase(op.operationId, caseConvention);
-        if (!isCorrectCase && checkStatus != 'off') {
-          result[checkStatus].push({
-            path: `paths.${pathKey}.${opKey}.operationId`,
-            message: `operationIds must follow case convention: ${caseConvention}`
-          });
+        if (!isCorrectCase) {
+          messages.addMessage(
+            `paths.${pathKey}.${opKey}.operationId`,
+            `operationIds must follow case convention: ${caseConvention}`,
+            checkStatus
+          );
         }
       }
       const hasOperationTags = op.tags && op.tags.length > 0;
@@ -143,27 +142,23 @@ module.exports.validate = function({ jsSpec, resolvedSpec, isOAS3 }, config) {
         }
         for (let i = 0, len = op.tags.length; i < len; i++) {
           if (!resolvedTags.includes(op.tags[i])) {
-            const checkStatus = config.unused_tag;
-            if (checkStatus !== 'off') {
-              result[checkStatus].push({
-                path: `paths.${pathKey}.${opKey}.tags`,
-                message: 'tag is not defined at the global level: ' + op.tags[i]
-              });
-            }
+            messages.addMessage(
+              `paths.${pathKey}.${opKey}.tags`,
+              'tag is not defined at the global level: ' + op.tags[i],
+              config.unused_tag
+            );
           }
         }
       }
 
       const hasSummary =
         op.summary && op.summary.length > 0 && !!op.summary.toString().trim();
       if (!hasSummary) {
-        const checkStatus = config.no_summary;
-        if (checkStatus !== 'off') {
-          result[checkStatus].push({
-            path: `paths.${pathKey}.${opKey}.summary`,
-            message: 'Operations must have a non-empty `summary` field.'
-          });
-        }
+        messages.addMessage(
+          `paths.${pathKey}.${opKey}.summary`,
+          'Operations must have a non-empty `summary` field.',
+          config.no_summary
+        );
       }
 
       // this should be good with resolved spec, but double check
@@ -180,11 +175,11 @@ module.exports.validate = function({ jsSpec, resolvedSpec, isOAS3 }, config) {
               }
             } else {
               if (param.required) {
-                result[checkStatusParamOrder].push({
-                  path: `paths.${pathKey}.${opKey}.parameters[${indx}]`,
-                  message:
-                    'Required parameters should appear before optional parameters.'
-                });
+                messages.addMessage(
+                  `paths.${pathKey}.${opKey}.parameters[${indx}]`,
+                  'Required parameters should appear before optional parameters.',
+                  checkStatusParamOrder
+                );
               }
             }
           }
@@ -193,5 +188,5 @@ module.exports.validate = function({ jsSpec, resolvedSpec, isOAS3 }, config) {
     });
   });
 
-  return { errors: result.error, warnings: result.warning };
+  return messages;
 };