fix: dont validate schema properties called 'responses' as responses (#194)

Author: dpopp07

.travis.yml

@@ -6,6 +6,7 @@ cache:
   npm: false
 script:
 - npm run test-travis
+- npm run lint
 after_success:
 - npm run report-coverage
 deploy:

src/plugins/utils/index.js

@@ -3,4 +3,5 @@
 module.exports.checkCase = require('./caseConventionCheck');
 module.exports.walk = require('./walk');
 module.exports.isParameterObject = require('./isParameter');
+module.exports.isResponseObject = require('./is-response');
 module.exports.hasRefProperty = require('./hasRefProperty');

src/plugins/utils/is-response.js

@@ -0,0 +1,35 @@
+module.exports = (path, isOAS3 = true) => {
+  const operations = [
+    'get',
+    'put',
+    'post',
+    'delete',
+    'options',
+    'head',
+    'patch',
+    'trace'
+  ];
+
+  const pathLength = path.length;
+
+  // a necessary but not sufficient check for a response object
+  // without these, a schema property named "responses"
+  // may be validated as a response
+  const isResponsesProperty = path[pathLength - 1] === 'responses';
+
+  // three scenarios:
+  // 1) inside of an operation
+  const isOperationResponse = operations.includes(path[pathLength - 2]);
+
+  // 2) inside of components -> responses (oas 3)
+  const isResponseInComponents =
+    pathLength === 2 && path[pathLength - 2] === 'components' && isOAS3;
+
+  // 3) top level responses (swagger 2)
+  const isTopLevelResponse = pathLength === 1 && !isOAS3;
+
+  return (
+    isResponsesProperty &&
+    (isOperationResponse || isResponseInComponents || isTopLevelResponse)
+  );
+};

src/plugins/validation/2and3/semantic-validators/items-required-for-array-objects.js

@@ -28,23 +28,23 @@ const checkReqProp = function(jsSpec, obj, requiredProp) {
   } else if (Array.isArray(obj.anyOf) || Array.isArray(obj.oneOf)) {
     const childList = obj.anyOf || obj.oneOf;
     let reqPropDefined = true;
-    childList.forEach((childObj) => {
+    childList.forEach(childObj => {
       if (!checkReqProp(jsSpec, childObj, requiredProp)) {
         reqPropDefined = false;
       }
     });
     return reqPropDefined;
   } else if (Array.isArray(obj.allOf)) {
     let reqPropDefined = false;
-    obj.allOf.forEach((childObj) => {
+    obj.allOf.forEach(childObj => {
       if (checkReqProp(jsSpec, childObj, requiredProp)) {
         reqPropDefined = true;
       }
     });
     return reqPropDefined;
   }
   return false;
-}
+};
 
 module.exports.validate = function({ jsSpec }, config) {
   const messages = new MessageCarrier();
@@ -71,7 +71,8 @@ module.exports.validate = function({ jsSpec }, config) {
       }
 
       // Assertation 2
-      const undefinedRequiredProperties = config.schemas.undefined_required_properties;
+      const undefinedRequiredProperties =
+        config.schemas.undefined_required_properties;
       if (Array.isArray(obj.required)) {
         obj.required.forEach((requiredProp, i) => {
           if (!checkReqProp(jsSpec, obj, requiredProp)) {

src/plugins/validation/2and3/semantic-validators/responses.js

@@ -1,5 +1,5 @@
 const each = require('lodash/each');
-const { walk } = require('../../../utils');
+const { walk, isResponseObject } = require('../../../utils');
 const MessageCarrier = require('../../../utils/messageCarrier');
 
 const INLINE_SCHEMA_MESSAGE =
@@ -11,10 +11,9 @@ module.exports.validate = function({ jsSpec, isOAS3 }, config) {
   config = config.responses;
 
   walk(jsSpec, [], function(obj, path) {
-    const contentsOfResponsesObject = path[path.length - 1] === 'responses';
     const isRef = !!obj.$ref;
 
-    if (contentsOfResponsesObject && !isRef) {
+    if (isResponseObject(path, isOAS3) && !isRef) {
       each(obj, (response, responseKey) => {
         if (isOAS3) {
           each(response.content, (mediaType, mediaTypeKey) => {

src/plugins/validation/oas3/semantic-validators/responses.js

@@ -15,7 +15,7 @@
 // Assertation 5. Response bodies with application/json content should not use schema
 // type: string, format: binary.
 
-const { walk } = require('../../../utils');
+const { walk, isResponseObject } = require('../../../utils');
 const MessageCarrier = require('../../../utils/messageCarrier');
 const findOctetSequencePaths = require('../../../utils/findOctetSequencePaths')
   .findOctetSequencePaths;
@@ -27,10 +27,10 @@ module.exports.validate = function({ resolvedSpec }, config) {
   config = config.responses;
 
   walk(resolvedSpec, [], function(obj, path) {
-    const contentsOfResponsesObject =
-      path[0] === 'paths' && path[path.length - 1] === 'responses';
-
-    if (contentsOfResponsesObject) {
+    // because we are using the resolved spec here,
+    // and only want to validate the responses inside of operations,
+    // check that we are within the `paths` object
+    if (isResponseObject(path) && path[0] === 'paths') {
       const [statusCodes, successCodes] = getResponseCodes(obj);
 
       const binaryStringStatus = configSchemas.json_or_param_binary_string;

test/plugins/utils/is-response.test.js

@@ -0,0 +1,55 @@
+const expect = require('expect');
+const { isResponseObject } = require('../../../src/plugins/utils');
+
+describe('is response object - util', () => {
+  describe('OpenAPI 3', () => {
+    const isOas3 = true;
+
+    it('should return false for top-level responses objects', () => {
+      const path = ['responses'];
+      expect(isResponseObject(path, isOas3)).toBe(false);
+    });
+
+    it('should return true for components responses objects', () => {
+      const path = ['components', 'responses'];
+
+      // the second argument, `isOas3`, is optional. test that optionality
+      expect(isResponseObject(path)).toBe(true);
+    });
+
+    it('should return true for operation responses', () => {
+      const path = ['paths', '/resource', 'post', 'responses'];
+      expect(isResponseObject(path, isOas3)).toBe(true);
+    });
+
+    it('should return false for non responses', () => {
+      const path = ['paths', '/resource', 'post', 'parameters'];
+      expect(isResponseObject(path, isOas3)).toBe(false);
+    });
+
+    it('should return false for schemas with properties named responses', () => {
+      const path = [
+        'components',
+        'schemas',
+        'MySchema',
+        'properties',
+        'responses'
+      ];
+      expect(isResponseObject(path, isOas3)).toBe(false);
+    });
+  });
+
+  describe('Swagger 2', () => {
+    const isOas3 = false;
+
+    it('should return true for top-level responses objects', () => {
+      const path = ['responses'];
+      expect(isResponseObject(path, isOas3)).toBe(true);
+    });
+
+    it('should return false for components responses objects', () => {
+      const path = ['components', 'responses'];
+      expect(isResponseObject(path, isOas3)).toBe(false);
+    });
+  });
+});

test/plugins/validation/oas3/responses.test.js

@@ -110,7 +110,13 @@ describe('validation plugin - semantic - responses - oas3', function() {
     const res = validate({ resolvedSpec: spec }, config);
     expect(res.warnings.length).toEqual(0);
     expect(res.errors.length).toEqual(1);
-    expect(res.errors[0].path).toEqual(['paths', '/pets', 'get', 'responses', '200']);
+    expect(res.errors[0].path).toEqual([
+      'paths',
+      '/pets',
+      'get',
+      'responses',
+      '200'
+    ]);
     expect(res.errors[0].message).toEqual(
       'All responses must include a description.'
     );