From b2b4e7f6461b62413178152d1a661710eb757e96 Mon Sep 17 00:00:00 2001 From: Owen Conti Date: Sat, 9 Sep 2017 16:48:51 -0600 Subject: [PATCH 01/12] Fixes #1129 Adds a patch to the end of the `allOf` function which attaches the $$ref value, found from the `patch` object --- src/specmap/lib/all-of.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/specmap/lib/all-of.js b/src/specmap/lib/all-of.js index 1a72f354d..b510e938d 100644 --- a/src/specmap/lib/all-of.js +++ b/src/specmap/lib/all-of.js @@ -17,7 +17,7 @@ export default { const parent = fullPath.slice(0, -1) let alreadyAddError = false - return [specmap.replace(parent, {})].concat(val.map((toMerge, index) => { + const allOfPatches = [specmap.replace(parent, {})].concat(val.map((toMerge, index) => { if (!specmap.isObject(toMerge)) { if (alreadyAddError) { return null @@ -31,5 +31,17 @@ export default { return specmap.mergeDeep(parent, toMerge) })) + + // Find the $$ref value from the patch's copy of the spec + // and add it as the last patch for the `allOf` resolution + let specObj = patch.value + parent.forEach((part) => { + specObj = specObj[part] + }) + allOfPatches.push(specmap.mergeDeep(parent, { + $$ref: specObj.$$ref + })) + + return allOfPatches } } From a38a1871170cb64c8b96e5263169424d1fbcb41b Mon Sep 17 00:00:00 2001 From: Owen Conti Date: Sun, 10 Sep 2017 09:43:31 -0600 Subject: [PATCH 02/12] Wrap $$ref patch in a `if` check to make we have a $$ref value to use. Added test for $$ref values, but left it as skipped because $$ref values aren't being attached during any test. --- src/specmap/lib/all-of.js | 8 ++-- test/specmap/all-of.js | 94 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 3 deletions(-) diff --git a/src/specmap/lib/all-of.js b/src/specmap/lib/all-of.js index b510e938d..3c7949ede 100644 --- a/src/specmap/lib/all-of.js +++ b/src/specmap/lib/all-of.js @@ -38,9 +38,11 @@ export default { parent.forEach((part) => { specObj = specObj[part] }) - allOfPatches.push(specmap.mergeDeep(parent, { - $$ref: specObj.$$ref - })) + if (specObj.$$ref) { + allOfPatches.push(specmap.mergeDeep(parent, { + $$ref: specObj.$$ref + })) + } return allOfPatches } diff --git a/test/specmap/all-of.js b/test/specmap/all-of.js index ae96afacd..d647209cb 100644 --- a/test/specmap/all-of.js +++ b/test/specmap/all-of.js @@ -61,6 +61,100 @@ describe('allOf', function () { }) }) + it.skip('should set $$ref values', function () { + return mapSpec({ + spec: { + Pet: { + type: 'object', + properties: { + name: { + type: 'string' + } + } + }, + Cat: { + allOf: [ + {$ref: '#/Pet'}, + { + type: 'object', + properties: { + meow: { + type: 'string' + } + } + } + ] + }, + Animal: { + type: 'object', + properties: { + pet: { + $ref: '#/Pet' + }, + cat: { + $ref: '#/Cat' + } + } + } + }, + plugins: [plugins.refs, plugins.allOf] + }).then((res) => { + console.log('res', res.spec.Animal.properties) + expect(res).toEqual({ + errors: [], + spec: { + Pet: { + $$ref: '#/Pet', + type: 'object', + properties: { + name: { + type: 'string' + } + } + }, + Cat: { + $$ref: '#/Cat', + properties: { + meow: { + type: 'string' + }, + name: { + type: 'string' + } + }, + type: 'object' + }, + Animal: { + type: 'object', + properties: { + pet: { + $$ref: '#/Pet', + properties: { + name: { + type: 'string' + } + }, + type: 'object' + }, + cat: { + $$ref: '#/Cat', + properties: { + meow: { + type: 'string' + }, + name: { + type: 'string' + } + }, + type: 'object' + } + } + } + } + }) + }) + }) + it('should return error if allOf is not an array', function () { return mapSpec({ spec: { From 74959b469eb2c808c2b0ac4770149611730c3758 Mon Sep 17 00:00:00 2001 From: Owen Conti Date: Sun, 10 Sep 2017 11:27:30 -0600 Subject: [PATCH 03/12] Update `allOf` logic to make sure objects that did not have an original $$ref value, do not have one after the `allOf` plugin runs --- src/specmap/lib/all-of.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/specmap/lib/all-of.js b/src/specmap/lib/all-of.js index 3c7949ede..3c6786180 100644 --- a/src/specmap/lib/all-of.js +++ b/src/specmap/lib/all-of.js @@ -39,10 +39,16 @@ export default { specObj = specObj[part] }) if (specObj.$$ref) { + // If there was an existing $$ref value, make sure it is applied last so it is preserved allOfPatches.push(specmap.mergeDeep(parent, { $$ref: specObj.$$ref })) } + else { + // If there was not an existing $$ref value, make sure to remove + // any $$ref value that may existing from the result of `allOf` merges + allOfPatches.push(specmap.remove([].concat(parent, '$$ref'))) + } return allOfPatches } From 121998ad6d73c25a8f76815e71daa48947ad8e64 Mon Sep 17 00:00:00 2001 From: Owen Conti Date: Sun, 10 Sep 2017 11:58:12 -0600 Subject: [PATCH 04/12] Update `allOf` logic to keep original properties from definitions. --- src/specmap/lib/all-of.js | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/specmap/lib/all-of.js b/src/specmap/lib/all-of.js index 3c6786180..e5c20bbc9 100644 --- a/src/specmap/lib/all-of.js +++ b/src/specmap/lib/all-of.js @@ -17,6 +17,15 @@ export default { const parent = fullPath.slice(0, -1) let alreadyAddError = false + // Find the original definition from the `patch.value` object + // Remove the `allOf` property so it doesn't get added to the result of the `allOf` plugin + let originalDefinitionObj = patch.value + parent.forEach((part) => { + originalDefinitionObj = originalDefinitionObj[part] + }) + originalDefinitionObj = Object.assign({}, originalDefinitionObj) + delete originalDefinitionObj.allOf + const allOfPatches = [specmap.replace(parent, {})].concat(val.map((toMerge, index) => { if (!specmap.isObject(toMerge)) { if (alreadyAddError) { @@ -32,21 +41,12 @@ export default { return specmap.mergeDeep(parent, toMerge) })) - // Find the $$ref value from the patch's copy of the spec - // and add it as the last patch for the `allOf` resolution - let specObj = patch.value - parent.forEach((part) => { - specObj = specObj[part] - }) - if (specObj.$$ref) { - // If there was an existing $$ref value, make sure it is applied last so it is preserved - allOfPatches.push(specmap.mergeDeep(parent, { - $$ref: specObj.$$ref - })) - } - else { - // If there was not an existing $$ref value, make sure to remove - // any $$ref value that may existing from the result of `allOf` merges + // Merge back the values from the original definition + allOfPatches.push(specmap.mergeDeep(parent, originalDefinitionObj)) + + // If there was not an original $$ref value, make sure to remove + // any $$ref value that may exist from the result of `allOf` merges + if (!originalDefinitionObj.$$ref) { allOfPatches.push(specmap.remove([].concat(parent, '$$ref'))) } From 157be35a592efafc42c8e87b1dcf8941f01f01e0 Mon Sep 17 00:00:00 2001 From: Owen Conti Date: Sun, 10 Sep 2017 12:02:36 -0600 Subject: [PATCH 05/12] Update `should resolve local $refs in allOf` test to account for root properties being in the result of `allOf` --- test/specmap/all-of.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/specmap/all-of.js b/test/specmap/all-of.js index d647209cb..a5a6ed63e 100644 --- a/test/specmap/all-of.js +++ b/test/specmap/all-of.js @@ -54,6 +54,7 @@ describe('allOf', function () { expect(res).toEqual({ errors: [], spec: { + bar: {baz: 4}, one: {baz: 4}, two: 2, } From 5fb4f048817e484cbd2e133c7e95140af70863f5 Mon Sep 17 00:00:00 2001 From: Kyle Shockey Date: Mon, 11 Sep 2017 16:33:50 -0700 Subject: [PATCH 06/12] Fix failing test --- test/specmap/all-of.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/specmap/all-of.js b/test/specmap/all-of.js index d647209cb..146c7e4a4 100644 --- a/test/specmap/all-of.js +++ b/test/specmap/all-of.js @@ -56,6 +56,7 @@ describe('allOf', function () { spec: { one: {baz: 4}, two: 2, + bar: {baz: 4} } }) }) From e137ca481659a812c5f2ce918eb4b483b4d5197c Mon Sep 17 00:00:00 2001 From: Kyle Shockey Date: Mon, 11 Sep 2017 16:34:08 -0700 Subject: [PATCH 07/12] Add additional test for functionality --- test/specmap/all-of.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/specmap/all-of.js b/test/specmap/all-of.js index 146c7e4a4..d1bdb9f68 100644 --- a/test/specmap/all-of.js +++ b/test/specmap/all-of.js @@ -62,6 +62,29 @@ describe('allOf', function () { }) }) + it('should not overwrite properties that are already present', function () { + return mapSpec({ + spec: { + original: 'yes', + allOf: [ + { + original: 'no', + notOriginal: 'yes' + } + ] + }, + plugins: [plugins.refs, plugins.allOf] + }).then((res) => { + expect(res).toEqual({ + errors: [], + spec: { + original: 'yes', + notOriginal: 'yes' + } + }) + }) + }) + it.skip('should set $$ref values', function () { return mapSpec({ spec: { From 1e5950a3c2d21162aa2624029e2246480a678567 Mon Sep 17 00:00:00 2001 From: Kyle Date: Mon, 11 Sep 2017 16:40:52 -0700 Subject: [PATCH 08/12] Update all-of.js --- test/specmap/all-of.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/specmap/all-of.js b/test/specmap/all-of.js index 8052e3c58..d1bdb9f68 100644 --- a/test/specmap/all-of.js +++ b/test/specmap/all-of.js @@ -54,7 +54,6 @@ describe('allOf', function () { expect(res).toEqual({ errors: [], spec: { - bar: {baz: 4}, one: {baz: 4}, two: 2, bar: {baz: 4} From 30f2457839b1398de262aa5fe90c9550b5762f6f Mon Sep 17 00:00:00 2001 From: Kyle Shockey Date: Mon, 11 Sep 2017 16:47:07 -0700 Subject: [PATCH 09/12] Unskip allOf->allOf test --- test/specmap/all-of.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/specmap/all-of.js b/test/specmap/all-of.js index d1bdb9f68..651ec299e 100644 --- a/test/specmap/all-of.js +++ b/test/specmap/all-of.js @@ -337,7 +337,7 @@ describe('allOf', function () { }) // TODO: this needs to get fixed - it.skip('should handle case, with an `allOf` referencing an `allOf` ', function () { + it('should handle case, with an `allOf` referencing an `allOf` ', function () { return mapSpec({ plugins: [plugins.refs, plugins.allOf], showDebug: true, From 1c5cf446fd42b39fefcaf12a32e14510ddf2d97b Mon Sep 17 00:00:00 2001 From: Kyle Shockey Date: Mon, 11 Sep 2017 16:51:47 -0700 Subject: [PATCH 10/12] Remove TODO --- test/specmap/all-of.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/specmap/all-of.js b/test/specmap/all-of.js index 651ec299e..83d73e54f 100644 --- a/test/specmap/all-of.js +++ b/test/specmap/all-of.js @@ -336,7 +336,6 @@ describe('allOf', function () { }) }) - // TODO: this needs to get fixed it('should handle case, with an `allOf` referencing an `allOf` ', function () { return mapSpec({ plugins: [plugins.refs, plugins.allOf], From 71751f99583b3f21fec67f559a17a96da1a4206b Mon Sep 17 00:00:00 2001 From: Owen Conti Date: Mon, 11 Sep 2017 18:04:32 -0600 Subject: [PATCH 11/12] Remove console.log line --- test/specmap/all-of.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/specmap/all-of.js b/test/specmap/all-of.js index 83d73e54f..a17514f99 100644 --- a/test/specmap/all-of.js +++ b/test/specmap/all-of.js @@ -123,7 +123,6 @@ describe('allOf', function () { }, plugins: [plugins.refs, plugins.allOf] }).then((res) => { - console.log('res', res.spec.Animal.properties) expect(res).toEqual({ errors: [], spec: { From 5b6771fc9b707b6ca17c5cce5fb5452a88f80b25 Mon Sep 17 00:00:00 2001 From: Kyle Shockey Date: Mon, 11 Sep 2017 17:59:57 -0700 Subject: [PATCH 12/12] set `allowMetaPatches` option so $$refs are present --- test/specmap/all-of.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/specmap/all-of.js b/test/specmap/all-of.js index a17514f99..14b5711a8 100644 --- a/test/specmap/all-of.js +++ b/test/specmap/all-of.js @@ -85,8 +85,9 @@ describe('allOf', function () { }) }) - it.skip('should set $$ref values', function () { + it('should set $$ref values', function () { return mapSpec({ + allowMetaPatches: true, spec: { Pet: { type: 'object',