fix: handle multi-file spec circular reference edge case (#333)

The validator chokes on a specific scenario - when a multi-file spec
has schemas that reference each other in a ciruclar manner across files.
The bundler we use to put together mutli-file specs doesn't store the
reference in a helpful way, so there's not really a way to report the
location of the circular reference the way we do for other scenarios.
All we can really do is avoid crashing completely and send the user the
fully resolved path to the problematic schema that they will need to
trace manually.

A test is added to capture the behavior. This test case would exhibit the
crash without the fix.

src/cli-validator/utils/circular-references-ibm.js

@@ -64,6 +64,7 @@ const convertPaths = function(jsSpec, resolvedPaths) {
   resolvedPaths.forEach(path => {
     let realPath = [];
     let previous = jsSpec;
+
     // path is an array of keys leading to the circular reference
     for (let i = 0; i < path.length; i++) {
       const key = path[i];
@@ -79,12 +80,14 @@ const convertPaths = function(jsSpec, resolvedPaths) {
       const lastKey = i === path.length - 1;
       if (!lastKey) {
         const nextKey = path[i + 1];
+
         // Only follow $ref if the next key is not present
         if (!current[nextKey] && current.$ref) {
           // locationArray holds the keys to the references object in the spec
           const locationArray = current.$ref
             .split('/')
             .filter(refKey => refKey !== '#');
+
           // since we are following a ref to the first object level,
           // realPath needs to be reset
           realPath = [...locationArray];
@@ -95,7 +98,25 @@ const convertPaths = function(jsSpec, resolvedPaths) {
           for (const refKey of locationArray) {
             const refCurrent = refPrevious[refKey];
             refPrevious = refCurrent;
+
+            // this should only happen in a specific multi-file spec scenario - schemas across
+            // files that reference one another (creating a circular reference)
+            // the bundler doesn't handle this situation well, so there isn't a lot we can do other than
+            // break out of the loop to avoid any runtime errors
+            if (!refPrevious) {
+              break;
+            }
           }
+
+          // as mentioned above, this should only happen in rare, multi-file spec scenarios.
+          // break from the loop, then go ahead and return the resolved path to the user,
+          // who will have to manually trace it to the source
+          // there is not really a better way to handle it without major changes
+          if (!refPrevious) {
+            realPath = path;
+            break;
+          }
+
           // set the parent current object to the object found at the
           // referenced path to continue using the keys in the parent loop
           current = refPrevious;

test/cli-validator/mockFiles/multi-file-spec/city.yaml

@@ -0,0 +1,7 @@
+schema:
+  properties:
+    name:
+      type: string
+    primary_team:
+      type: object
+      $ref: ./sports-team.yaml#/schema

test/cli-validator/mockFiles/multi-file-spec/main.yaml

@@ -10,3 +10,13 @@ paths:
             application/json:
               schema:
                 $ref: "./schema.yaml#/components/schemas/SchemaDef"
+  /circular_example:
+    get:
+      summary: Summary
+      responses:
+        "200":
+          description: OK
+          content:
+            application/json:
+              schema:
+                $ref: "./schema.yaml#/components/schemas/CircularSchema"

test/cli-validator/mockFiles/multi-file-spec/schema.yaml

@@ -4,3 +4,10 @@ components:
   schemas:
     SchemaDef:
       type: object
+    CircularSchema:
+      type: object
+      properties:
+        id:
+          type: string
+        city:
+          $ref: ./city.yaml#/schema

test/cli-validator/mockFiles/multi-file-spec/sports-team.yaml

@@ -0,0 +1,7 @@
+schema:
+  properties:
+    name:
+      type: string
+    location:
+      type: object
+      $ref: ./city.yaml#/schema

test/cli-validator/tests/circular-refs-validator.test.js

@@ -1,6 +1,9 @@
 const commandLineValidator = require('../../../src/cli-validator/runValidator');
 const circularRefsValidator = require('../../../src/cli-validator/utils/circular-references-ibm');
-const { getCapturedText } = require('../../test-utils');
+const {
+  getCapturedText,
+  getMessageAndPathFromCapturedText
+} = require('../../test-utils');
 
 describe('cli tool - test circular reference module', function() {
   it('should correctly validate a file with circular references', async function() {
@@ -87,4 +90,35 @@ describe('cli tool - test circular reference module', function() {
       )
     ).toEqual(true);
   });
+
+  it('should not fail when a multi-file spec contains circular references across files', async function() {
+    const consoleSpy = jest.spyOn(console, 'log').mockImplementation(() => {});
+
+    const program = {};
+    program.args = ['./test/cli-validator/mockFiles/multi-file-spec/main.yaml'];
+    program.default_mode = true;
+    program.ruleset = 'test/spectral/mockFiles/mockConfig/extends-default.yaml';
+
+    const exitCode = await commandLineValidator(program);
+
+    const capturedText = getCapturedText(consoleSpy.mock.calls);
+    consoleSpy.mockRestore();
+
+    expect(exitCode).toEqual(0);
+
+    const messages = getMessageAndPathFromCapturedText(
+      'Swagger object should not contain circular references.',
+      capturedText
+    );
+
+    expect(messages.length).toBe(1);
+
+    expect(messages[0][0].get('Message')).toEqual(
+      'Swagger object should not contain circular references.'
+    );
+
+    expect(messages[0][1].get('Path')).toEqual(
+      'paths./circular_example.get.responses.200.content.application/json.schema.properties.city.properties.primary_team.properties.location.properties'
+    );
+  });
 });