Fix bug in identifying local, indirect circulars

Fixes #82
whitlockjc · Jun 14, 2016 · c7a90b5 · c7a90b5
1 parent 63ac955
commit c7a90b5
Show file tree

Hide file tree

Showing 10 changed files with 215 additions and 19 deletions.
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
@@ -1,5 +1,9 @@
 ## Release Notes
 
+### v2.1.6 (2016-06-14)
+
+* Fixed a bug where identifying circular references failed for local, indirect references *(Issue #82)_
+
 ### v2.1.5 (2016-02-02)
 
 * Fixed an issue with altering the original input

diff --git a/bin/json-refs b/bin/json-refs
@@ -134,7 +134,7 @@ program
         var errors = [];
 
         if (!that.force) {
-          Object.keys(results.refs).forEach(function (refPtr) {
+          Object.keys(results.refs).sort().forEach(function (refPtr) {
             var refDetails = results.refs[refPtr];
 
             if (refDetails.type === 'invalid' || refDetails.error || (that.warningsAsErrors && refDetails.warning)) {

diff --git a/browser/json-refs-min.js b/browser/json-refs-min.js
diff --git a/browser/json-refs-standalone-min.js b/browser/json-refs-standalone-min.js
diff --git a/browser/json-refs-standalone.js b/browser/json-refs-standalone.js
diff --git a/browser/json-refs.js b/browser/json-refs.js
diff --git a/index.js b/index.js
@@ -315,6 +315,45 @@ function findRefsRecursive (obj, options, parents, parentPtrs, allRefs, indirect
   });
 
   allTasks = allTasks
+    .then(function () {
+      // Identify indirect, local circular references (Issue 82)
+      var circulars = [];
+
+      function walkRefs (parentPtrs, parentRefs, refPtr, ref) {
+        Object.keys(allRefs.refs).forEach(function (dRefPtr) {
+          var dRefDetails = allRefs.refs[dRefPtr];
+
+          // Do not process already processed references or references that are not a nested references
+          if (dRefPtr !== refPtr && dRefPtr.indexOf(ref + '/') === 0) {
+            if (parentRefs.indexOf(ref) > -1) {
+              if (circulars.indexOf(ref) === -1) {
+                circulars.push(ref);
+              }
+            } else {
+              walkRefs(parentPtrs.concat(refPtr), parentRefs.concat(ref), dRefPtr, dRefDetails.uri);
+            }
+          }
+        });
+      }
+
+      Object.keys(allRefs.refs).forEach(function (refPtr) {
+        var refDetails = allRefs.refs[refPtr];
+
+        // Only process local, non-circular references
+        if (refDetails.type === 'local' && !refDetails.circular && circulars.indexOf(refDetails.uri) === -1) {
+          walkRefs([], [], refPtr, refDetails.uri);
+        }
+      });
+
+      Object.keys(allRefs.refs).forEach(function (refPtr) {
+        var refDetails = allRefs.refs[refPtr];
+
+        if (circulars.indexOf(refDetails.uri) > -1) {
+          refDetails.circular = true;
+          refDetails.value = refDetails.def;
+        }
+      });
+    })
     .then(function () {
       return allRefs;
     });

diff --git a/test/browser/documents/test-document.yaml b/test/browser/documents/test-document.yaml
@@ -16,6 +16,32 @@ circular:
   # Reference to an ancestor
   ancestor:
     $ref: '#/circular'
+  # Reference to another object that references this one
+  User:
+    type: object
+    properties:
+      status:
+        $ref: '#/circular/Status'
+  # Reference to another object that references this one
+  Status:
+    type: object
+    properties:
+      user:
+        $ref: '#/circular/User'
+      message:
+        $ref: '#/circular/Message'
+  # Reference to another object that itself has an indirect reference to this one
+  Message:
+    type: object
+    properties:
+      author:
+        $ref: '#/circular/User'
+  # Reference to another object that itself has an indirect reference to itself
+  StatusWrapper:
+    type: object
+    properties:
+      status:
+        $ref: '#/circular/Status'
 
 definitions:
   HumanName:

diff --git a/test/test-cli.js b/test/test-cli.js
@@ -273,11 +273,11 @@ describe('json-refs CLI', function () {
             '',
             '  error: Document has invalid references:',
             '',
-            '  #/missing: JSON Pointer points to missing location: #/some/missing/path',
             '  #/invalid: HTTP URIs must have a host.',
-            '  #/remote/relative/missing: JSON Pointer points to missing location: #/some/missing/path',
-            '  #/remote/relative/child/missing: JSON Pointer points to missing location: #/some/missing/path',
+            '  #/missing: JSON Pointer points to missing location: #/some/missing/path',
             '  #/remote/relative/child/ancestor/missing: JSON Pointer points to missing location: #/some/missing/path',
+            '  #/remote/relative/child/missing: JSON Pointer points to missing location: #/some/missing/path',
+            '  #/remote/relative/missing: JSON Pointer points to missing location: #/some/missing/path',
             '',
             ''
           ].join('\n'));
@@ -366,12 +366,12 @@ describe('json-refs CLI', function () {
             '',
             '  error: Document has invalid references:',
             '',
-            '  #/missing: JSON Pointer points to missing location: #/some/missing/path',
-            '  #/warning: Extra JSON Reference properties will be ignored: ignored',
             '  #/invalid: HTTP URIs must have a host.',
-            '  #/remote/relative/missing: JSON Pointer points to missing location: #/some/missing/path',
-            '  #/remote/relative/child/missing: JSON Pointer points to missing location: #/some/missing/path',
+            '  #/missing: JSON Pointer points to missing location: #/some/missing/path',
             '  #/remote/relative/child/ancestor/missing: JSON Pointer points to missing location: #/some/missing/path',
+            '  #/remote/relative/child/missing: JSON Pointer points to missing location: #/some/missing/path',
+            '  #/remote/relative/missing: JSON Pointer points to missing location: #/some/missing/path',
+            '  #/warning: Extra JSON Reference properties will be ignored: ignored',
             '',
             ''
           ].join('\n'));
@@ -401,14 +401,14 @@ describe('json-refs CLI', function () {
           '',
           '  error: Document has invalid references:',
           '',
-          '  #/deferred: JSON Pointer points to missing location: #/project/name',
-          '  #/missing: JSON Pointer points to missing location: #/some/missing/path',
           '  #/ancestor/deferred: JSON Pointer points to missing location: #/project/name',
           '  #/ancestor/missing: JSON Pointer points to missing location: #/some/missing/path',
-          '  #/ancestor/nested/deferred: JSON Pointer points to missing location: #/project/name',
-          '  #/ancestor/nested/missing: JSON Pointer points to missing location: #/some/missing/path',
           '  #/ancestor/nested/child/deferred: JSON Pointer points to missing location: #/project/name',
           '  #/ancestor/nested/child/missing: JSON Pointer points to missing location: #/some/missing/path',
+          '  #/ancestor/nested/deferred: JSON Pointer points to missing location: #/project/name',
+          '  #/ancestor/nested/missing: JSON Pointer points to missing location: #/some/missing/path',
+          '  #/deferred: JSON Pointer points to missing location: #/project/name',
+          '  #/missing: JSON Pointer points to missing location: #/some/missing/path',
           '',
           '',
         ].join('\n');

diff --git a/test/test-json-refs.js b/test/test-json-refs.js
@@ -333,6 +333,46 @@ describe('json-refs API', function () {
             circular: true,
             value: testDocument.circular.root
           },
+          '#/circular/User/properties/status': {
+            def: testDocument.circular.User.properties.status,
+            uri: testDocument.circular.User.properties.status.$ref,
+            uriDetails: URI.parse(testDocument.circular.User.properties.status.$ref),
+            type: 'local',
+            circular: true,
+            value: testDocument.circular.User.properties.status
+          },
+          '#/circular/Status/properties/user': {
+            def: testDocument.circular.Status.properties.user,
+            uri: testDocument.circular.Status.properties.user.$ref,
+            uriDetails: URI.parse(testDocument.circular.Status.properties.user.$ref),
+            type: 'local',
+            circular: true,
+            value: testDocument.circular.Status.properties.user
+          },
+          '#/circular/Status/properties/message': {
+            def: testDocument.circular.Status.properties.message,
+            uri: testDocument.circular.Status.properties.message.$ref,
+            uriDetails: URI.parse(testDocument.circular.Status.properties.message.$ref),
+            type: 'local',
+            circular: true,
+            value: testDocument.circular.Status.properties.message
+          },
+          '#/circular/Message/properties/author': {
+            def: testDocument.circular.Message.properties.author,
+            uri: testDocument.circular.Message.properties.author.$ref,
+            uriDetails: URI.parse(testDocument.circular.Message.properties.author.$ref),
+            type: 'local',
+            circular: true,
+            value: testDocument.circular.Message.properties.author
+          },
+          '#/circular/StatusWrapper/properties/status': {
+            def: testDocument.circular.StatusWrapper.properties.status,
+            uri: testDocument.circular.StatusWrapper.properties.status.$ref,
+            uriDetails: URI.parse(testDocument.circular.StatusWrapper.properties.status.$ref),
+            type: 'local',
+            circular: true,
+            value: testDocument.circular.StatusWrapper.properties.status
+          },
           '#/remote/absolute': {
             def: testDocument.remote.absolute,
             uri: testDocument.remote.absolute.$ref,
@@ -504,7 +544,11 @@ describe('json-refs API', function () {
           ],
           circular: {
             root: testDocument.circular.root,
-            ancestor: testDocument.circular.ancestor
+            ancestor: testDocument.circular.ancestor,
+            User: testDocument.circular.User,
+            Status: testDocument.circular.Status,
+            Message: testDocument.circular.Message,
+            StatusWrapper: testDocument.circular.StatusWrapper
           },
           definitions: {
             HumanName: testDocument.definitions.HumanName,
@@ -603,6 +647,11 @@ describe('json-refs API', function () {
       '#/array/1': testDocument.array[1],
       '#/circular/root': testDocument.circular.root,
       '#/circular/ancestor': testDocument.circular.ancestor,
+      '#/circular/User/properties/status': testDocument.circular.User.properties.status,
+      '#/circular/Status/properties/user': testDocument.circular.Status.properties.user,
+      '#/circular/Status/properties/message': testDocument.circular.Status.properties.message,
+      '#/circular/Message/properties/author': testDocument.circular.Message.properties.author,
+      '#/circular/StatusWrapper/properties/status': testDocument.circular.StatusWrapper.properties.status,
       '#/definitions/Person/properties/name': testDocument.definitions.Person.properties.name,
       '#/invalid': testDocument.invalid,
       '#/local': testDocument.local,