fix: authority parsing (#51)

* chore: bump deps

* fix: run parse authority before resolving pointer

Author: marbemac

README.md

@@ -4,7 +4,6 @@
 
 Recursively resolves JSON pointers and remote authorities.
 
-- Explore the interfaces: [TSDoc](https://stoplightio.github.io/json-ref-resolver/)
 - View the changelog: [Releases](https://github.com/stoplightio/json-ref-resolver/releases)
 
 ### Features
@@ -27,7 +26,7 @@ yarn add @stoplight/json-ref-resolver
 
 ### Usage
 
-All relevant types and options can be found in [src/types.ts](src/types.ts) or in the TSDoc.
+All relevant types and options can be found in [src/types.ts](src/types.ts).
 
 ```ts
 // Import the Resolver class.

package.json

@@ -33,37 +33,35 @@
   },
   "scripts": {
     "build": "sl-scripts build",
-    "build.docs": "sl-scripts build:typedoc",
     "commit": "git-cz",
-    "lint": "tslint -p tsconfig.json 'src/**/*.ts'",
+    "lint": "tslint -p .",
     "lint.fix": "yarn lint --fix",
     "release": "sl-scripts release",
-    "release.docs": "sl-scripts release:docs",
     "release.dryRun": "sl-scripts release --dry-run --debug",
     "test": "jest",
     "test.prod": "yarn lint && yarn test --coverage --no-cache",
     "test.update": "yarn test --updateSnapshot",
     "test.watch": "yarn test --watch"
   },
   "dependencies": {
-    "@stoplight/json": "^2.1.0",
+    "@stoplight/json": "2.1.x",
+    "@types/urijs": "1.19.1",
     "dependency-graph": "0.8.x",
     "fast-memoize": "2.x.x",
-    "immer": "^3.1.2",
+    "immer": "3.x.x",
     "lodash": "4.x.x",
     "urijs": "1.x.x"
   },
   "devDependencies": {
-    "@stoplight/scripts": "5.1.0",
+    "@stoplight/scripts": "7.0.0",
     "@types/jest": "^24.0.13",
-    "@types/lodash": "4.14.132",
-    "@types/urijs": "1.19.1",
+    "@types/lodash": "4.14.133",
     "benchmark": "2.x.x",
     "jest": "^24.8.0",
     "ts-jest": "^24.0.2",
-    "tslint": "^5.16.0",
+    "tslint": "^5.17.0",
     "tslint-config-stoplight": "^1.3.0",
-    "typescript": "3.4.5"
+    "typescript": "3.5.1"
   },
   "lint-staged": {
     "*.{ts,tsx}$": [

src/__tests__/resolver.spec.ts

@@ -1390,9 +1390,9 @@ describe('resolver', () => {
     test('should support `parseAuthorityResult` hook', async () => {
       const data = {
         markdown: '# hello',
-        bar: {
-          hello: 'world',
-        },
+        bar: `{
+          "hello": "world"
+        }`,
       };
 
       const source = {
@@ -1401,7 +1401,8 @@ describe('resolver', () => {
             $ref: 'http://foo.com/foo.md',
           },
           bar: {
-            $ref: 'http://foo.com/bar.json',
+            // IMPORTANT: including a pointer to test that the target is parsed before looking up #/hello
+            $ref: 'http://foo.com/bar.json#/hello',
           },
         },
       };
@@ -1426,7 +1427,7 @@ describe('resolver', () => {
               heading1: 'hello',
             };
           } else {
-            opts.result.added = true;
+            opts.result = JSON.parse(opts.result);
           }
 
           return opts;
@@ -1440,10 +1441,7 @@ describe('resolver', () => {
           foo: {
             heading1: 'hello',
           },
-          bar: {
-            hello: 'world',
-            added: true,
-          },
+          bar: 'world',
         },
       });
     });
@@ -1546,8 +1544,8 @@ describe('resolver', () => {
       });
 
       expect({ ...result.errors[0], authority: undefined }).toEqual({
-        code: 'PARSE_AUTHORITY',
-        message: "Error parsing lookup result for 'http://foo/': Error: some parse error!",
+        code: 'RESOLVE_AUTHORITY',
+        message: "Error: Could not parse remote reference response for 'http://foo/' - Error: some parse error!",
         pointerStack: [],
         authorityStack: [],
         path: ['definitions', 'foo'],

src/crawler.ts

@@ -1,6 +1,6 @@
 import { pointerToPath } from '@stoplight/json';
 import { DepGraph } from 'dependency-graph';
-import _get = require('lodash/get');
+import { get } from 'lodash';
 
 import * as Types from './types';
 import * as Utils from './utils';
@@ -154,7 +154,7 @@ export class ResolveCrawler implements Types.ICrawler {
           pointerStack.push(targetPointer);
 
           // if we are partially resolving
-          this.computeGraph(_get(this._runner.source, targetPath), targetPath as string[], targetPointer, pointerStack);
+          this.computeGraph(get(this._runner.source, targetPath), targetPath as string[], targetPointer, pointerStack);
 
           pointerStack.pop();
         }

src/runner.ts

@@ -1,7 +1,6 @@
 import { pathToPointer, pointerToPath, startsWith, trimStart } from '@stoplight/json';
 import produce from 'immer';
-import _get = require('lodash/get');
-import _set = require('lodash/set');
+import { get, set } from 'lodash';
 import * as URI from 'urijs';
 
 import { Cache } from './cache';
@@ -95,7 +94,7 @@ export class ResolveRunner implements Types.IResolveRunner {
     jsonPointer = jsonPointer && jsonPointer.trim();
     if (jsonPointer && jsonPointer !== '#' && jsonPointer !== '#/') {
       targetPath = pointerToPath(jsonPointer);
-      resolved.result = _get(resolved.result, targetPath);
+      resolved.result = get(resolved.result, targetPath);
     }
 
     if (!resolved.result) {
@@ -153,7 +152,7 @@ export class ResolveRunner implements Types.IResolveRunner {
             if (!resolvedTargetPath.length) {
               return r.resolved.result;
             } else {
-              _set(draft, resolvedTargetPath, r.resolved.result);
+              set(draft, resolvedTargetPath, r.resolved.result);
             }
           }
         });
@@ -181,7 +180,7 @@ export class ResolveRunner implements Types.IResolveRunner {
             if (!dependants.length) continue;
 
             const pointerPath = pointerToPath(pointer);
-            const val = _get(draft, pointerPath);
+            const val = get(draft, pointerPath);
             for (const dependant of dependants) {
               // check to prevent circular references in the resulting JS object
               // this implementation is MUCH more performant than decycling the final object to remove circulars
@@ -201,7 +200,7 @@ export class ResolveRunner implements Types.IResolveRunner {
               resolved.refMap[pathToPointer(dependantPath)] = pathToPointer(pointerPath);
 
               if (val) {
-                _set(draft, dependantPath, val);
+                set(draft, dependantPath, val);
               } else {
                 resolved.errors.push({
                   code: 'POINTER_MISSING',
@@ -221,7 +220,7 @@ export class ResolveRunner implements Types.IResolveRunner {
     }
 
     if (targetPath) {
-      resolved.result = _get(this._source, targetPath);
+      resolved.result = get(this._source, targetPath);
     } else {
       resolved.result = this._source;
     }
@@ -288,7 +287,24 @@ export class ResolveRunner implements Types.IResolveRunner {
       throw new Error(`No reader defined for scheme '${ref.scheme()}' in ref ${ref.toString()}`);
     }
 
-    const result = await reader.read(ref, this.ctx);
+    let result = await reader.read(ref, this.ctx);
+
+    // support custom parsers
+    if (this.parseAuthorityResult) {
+      try {
+        const parsed = await this.parseAuthorityResult({
+          authorityResult: result,
+          result,
+          targetAuthority: ref,
+          parentAuthority: this.authority,
+          parentPath: [],
+        });
+
+        result = parsed.result;
+      } catch (e) {
+        throw new Error(`Could not parse remote reference response for '${ref.toString()}' - ${String(e)}`);
+      }
+    }
 
     return new ResolveRunner(result, {
       depth: this.depth + 1,
@@ -380,46 +396,13 @@ export class ResolveRunner implements Types.IResolveRunner {
                 : error.path;
 
               if (errorPathInResult && errorPathInResult.length) {
-                _set(lookupResult.resolved.result, errorPathInResult, val);
+                set(lookupResult.resolved.result, errorPathInResult, val);
               } else if (lookupResult.resolved.result) {
                 lookupResult.resolved.result = val;
               }
             }
           }
         }
-
-        // support custom parsers
-        if (this.parseAuthorityResult) {
-          try {
-            // TODO: rework this to pass in an addValidation function to allow custom parsers to add their own validations
-            // then generally re-work the error system here to be based around more flexible validations
-            const parsed = await this.parseAuthorityResult({
-              authorityResult: lookupResult,
-              result: lookupResult.resolved.result,
-              targetAuthority: ref,
-              parentAuthority: this.authority,
-              parentPath,
-            });
-
-            // if (parsed.errors) {
-            // TODO: as mentioned above, allow caller to add errors/validations
-            // }
-
-            lookupResult.resolved.result = parsed.result;
-          } catch (e) {
-            // could not parse... roll back to original value
-            lookupResult.resolved.result = val;
-
-            lookupResult.error = {
-              code: 'PARSE_AUTHORITY',
-              message: `Error parsing lookup result for '${ref.toString()}': ${String(e)}`,
-              authority: ref,
-              authorityStack: this.authorityStack,
-              pointerStack,
-              path: parentPath,
-            };
-          }
-        }
       }
     }
 

tslint.json

@@ -1,3 +1,6 @@
 {
-  "extends": ["tslint-config-stoplight"]
+  "extends": ["tslint-config-stoplight"],
+  "linterOptions": {
+    "include": ["src/**/*"]
+  }
 }