From 4f7070ff43955f9b1a23fe62d07795f43dcd1494 Mon Sep 17 00:00:00 2001 From: Jon Ursenbach Date: Wed, 6 Oct 2021 11:02:27 -0700 Subject: [PATCH] fix: issues with circular references being dereferenced and unable to be stringified --- .../__tests__/__fixtures__/circular.oas.json | 62 +++++++++++++++++++ packages/api/__tests__/cache.test.js | 15 +++++ .../api/__tests__/lib/prepareParams.test.js | 9 ++- packages/api/package-lock.json | 17 +++-- packages/api/package.json | 3 +- packages/api/src/cache.js | 14 +++-- .../httpsnippet-client-api/package-lock.json | 2 +- 7 files changed, 100 insertions(+), 22 deletions(-) create mode 100644 packages/api/__tests__/__fixtures__/circular.oas.json diff --git a/packages/api/__tests__/__fixtures__/circular.oas.json b/packages/api/__tests__/__fixtures__/circular.oas.json new file mode 100644 index 00000000..c63231f6 --- /dev/null +++ b/packages/api/__tests__/__fixtures__/circular.oas.json @@ -0,0 +1,62 @@ +{ + "openapi": "3.0.1", + "info": { + "title": "API definition with a circular $ref", + "version": "1.0.0" + }, + "servers": [ + { + "url": "https://httpbin.org" + } + ], + "paths": { + "/anything": { + "get": { + "responses": { + "200": { + "description": "OK" + }, + "404": { + "description": "Not found", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ErrorMessage" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "ErrorMessage": { + "type": "object", + "additionalProperties": false, + "properties": { + "statusCode": { + "type": "integer", + "format": "int32" + }, + "error": { + "type": "string", + "nullable": true + }, + "inner": { + "$ref": "#/components/schemas/ErrorMessage" + }, + "canBeRetried": { + "type": "string", + "enum": ["Unknown", "Yes", "No"] + }, + "detailedErrorCode": { + "type": "integer", + "format": "int32" + } + } + } + } + } +} diff --git a/packages/api/__tests__/cache.test.js b/packages/api/__tests__/cache.test.js index 6134857d..a18d9662 100644 --- a/packages/api/__tests__/cache.test.js +++ b/packages/api/__tests__/cache.test.js @@ -37,6 +37,10 @@ beforeEach(async () => { readmeExampleYaml = await realFs.readFile(require.resolve('@readme/oas-examples/3.0/yaml/readme.yaml'), 'utf8'); vol.fromJSON({ + [`${[examplesDir]}/circular.json`]: await realFs.readFile( + require.resolve('./__fixtures__/circular.oas.json'), + 'utf8' + ), [`${[examplesDir]}/invalid-openapi.json`]: JSON.stringify(pkg), [`${[examplesDir]}/readme.json`]: readmeExampleJson, [`${[examplesDir]}/readme.yaml`]: readmeExampleYaml, @@ -201,6 +205,17 @@ describe('#save()', () => { expect(cacheStore.isCached()).toBe(true); }); + + it('should be able to cache a definition that contains a circular reference', async () => { + const file = path.join(examplesDir, 'circular.json'); + const cacheStore = new Cache(file); + + expect(cacheStore.isCached()).toBe(false); + + await cacheStore.saveFile(); + + expect(cacheStore.isCached()).toBe(true); + }); }); describe('#get', () => { diff --git a/packages/api/__tests__/lib/prepareParams.test.js b/packages/api/__tests__/lib/prepareParams.test.js index 4969893a..ea5ed64c 100644 --- a/packages/api/__tests__/lib/prepareParams.test.js +++ b/packages/api/__tests__/lib/prepareParams.test.js @@ -1,6 +1,5 @@ const fs = require('fs'); const Oas = require('oas'); -const $RefParser = require('@apidevtools/json-schema-ref-parser'); const readmeExample = require('@readme/oas-examples/3.0/json/readme.json'); const usptoExample = require('@readme/oas-examples/3.0/json/uspto.json'); const payloadExamples = require('../__fixtures__/payloads.oas.json'); @@ -12,11 +11,11 @@ describe('#prepareParams', () => { let usptoSpec; beforeAll(async () => { - let schema = await $RefParser.dereference(readmeExample); - readmeSpec = new Oas(schema); + readmeSpec = new Oas(readmeExample); + await readmeSpec.dereference(); - schema = await $RefParser.dereference(usptoExample); - usptoSpec = new Oas(schema); + usptoSpec = new Oas(usptoExample); + await usptoSpec.dereference(); }); it('should prepare nothing if nothing was supplied', async () => { diff --git a/packages/api/package-lock.json b/packages/api/package-lock.json index b1af0de7..6eabb041 100644 --- a/packages/api/package-lock.json +++ b/packages/api/package-lock.json @@ -6,10 +6,9 @@ "packages": { "": { "name": "api", - "version": "3.4.0", + "version": "3.4.1", "license": "MIT", "dependencies": { - "@apidevtools/json-schema-ref-parser": "^9.0.1", "@apidevtools/swagger-parser": "^10.0.1", "@readme/oas-to-har": "^13.7.2", "datauri": "^4.1.0", @@ -21,7 +20,7 @@ "make-dir": "^3.1.0", "mimer": "^2.0.2", "node-fetch": "^2.6.0", - "oas": "^14.5.1" + "oas": "^14.7.0" }, "devDependencies": { "@readme/eslint-config": "^7.2.2", @@ -9980,9 +9979,9 @@ "dev": true }, "node_modules/oas": { - "version": "14.6.1", - "resolved": "https://registry.npmjs.org/oas/-/oas-14.6.1.tgz", - "integrity": "sha512-PDO6+i1GOaTyi0SqqvO/OsVMExRE0ftNIr7t00H0drgOvHG0D+nItwC74cttRLZT980Dlg7KktKA/70RfSivMg==", + "version": "14.7.0", + "resolved": "https://registry.npmjs.org/oas/-/oas-14.7.0.tgz", + "integrity": "sha512-2ZmTMPZX36z+tjBYPt6vBcb4QCwaN4TAYoYBShKGGNlvu6ZKYKAJe/yi/lTgZk1a3nB02i/uE1IngM0NBIJQWA==", "dependencies": { "@apidevtools/json-schema-ref-parser": "^9.0.6", "cardinal": "^2.1.1", @@ -19802,9 +19801,9 @@ "dev": true }, "oas": { - "version": "14.6.1", - "resolved": "https://registry.npmjs.org/oas/-/oas-14.6.1.tgz", - "integrity": "sha512-PDO6+i1GOaTyi0SqqvO/OsVMExRE0ftNIr7t00H0drgOvHG0D+nItwC74cttRLZT980Dlg7KktKA/70RfSivMg==", + "version": "14.7.0", + "resolved": "https://registry.npmjs.org/oas/-/oas-14.7.0.tgz", + "integrity": "sha512-2ZmTMPZX36z+tjBYPt6vBcb4QCwaN4TAYoYBShKGGNlvu6ZKYKAJe/yi/lTgZk1a3nB02i/uE1IngM0NBIJQWA==", "requires": { "@apidevtools/json-schema-ref-parser": "^9.0.6", "cardinal": "^2.1.1", diff --git a/packages/api/package.json b/packages/api/package.json index da820aef..ee4c4396 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -23,7 +23,6 @@ "node": "^12 || ^14 || ^16" }, "dependencies": { - "@apidevtools/json-schema-ref-parser": "^9.0.1", "@apidevtools/swagger-parser": "^10.0.1", "@readme/oas-to-har": "^13.7.2", "datauri": "^4.1.0", @@ -35,7 +34,7 @@ "make-dir": "^3.1.0", "mimer": "^2.0.2", "node-fetch": "^2.6.0", - "oas": "^14.5.1" + "oas": "^14.7.0" }, "devDependencies": { "@readme/eslint-config": "^7.2.2", diff --git a/packages/api/src/cache.js b/packages/api/src/cache.js index 854a766a..4f4a195a 100644 --- a/packages/api/src/cache.js +++ b/packages/api/src/cache.js @@ -1,6 +1,5 @@ const fetch = require('node-fetch'); const SwaggerParser = require('@apidevtools/swagger-parser'); -const $RefParser = require('@apidevtools/json-schema-ref-parser'); const yaml = require('js-yaml'); const crypto = require('crypto'); const findCacheDir = require('find-cache-dir'); @@ -127,7 +126,15 @@ class SdkCache { return resolve(json); }) .then(res => { - return SwaggerParser.validate(res).catch(err => { + // The `validate` method handles dereferencing for us. + return SwaggerParser.validate(res, { + dereference: { + // If circular `$refs` are ignored they'll remain in the API definition as `$ref: String`. This allows us to + // not only do easy circular reference detection but also stringify and save dereferenced API definitions + // back into the cache directory. + circular: 'ignore', + }, + }).catch(err => { if (/is not a valid openapi api definition/i.test(err.message)) { throw new Error("Sorry, that doesn't look like a valid OpenAPI definition."); } @@ -135,9 +142,6 @@ class SdkCache { throw err; }); }) - .then(res => { - return $RefParser.dereference(res); - }) .then(async spec => { if (!fs.existsSync(self.dir)) { fs.mkdirSync(self.dir, { recursive: true }); diff --git a/packages/httpsnippet-client-api/package-lock.json b/packages/httpsnippet-client-api/package-lock.json index 3fda13c0..39e394de 100644 --- a/packages/httpsnippet-client-api/package-lock.json +++ b/packages/httpsnippet-client-api/package-lock.json @@ -6,7 +6,7 @@ "packages": { "": { "name": "httpsnippet-client-api", - "version": "3.4.0", + "version": "3.4.1", "license": "MIT", "dependencies": { "content-type": "^1.0.4",