Skip to content

Commit

Permalink
fix: deep array merge
Browse files Browse the repository at this point in the history 
deep-extend does not support array merge. There was special code added
to merge top-level arrays, but that was a shallow merge.

Use deepmerge instead of deep-extend to merge arrays also. Default merge
settings seem to work well - all tests pass.

Add a test case based on swagger-api/swagger-ui#7618

Fixes: 2f5bb86 ("Fix and test for swagger-ui #3328: swagger-api/swagger-ui#3328. Added manual logic to merge arrays after calling deepExtend within `mergeDeep` function")
  • Loading branch information
@lipnitsk
lipnitsk committed Nov 6, 2021
1 parent 5a7548e commit 944380d
Show file tree
Hide file tree
Showing 3 changed files with 251 additions and 39 deletions.
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@
"btoa": "^1.2.1",
"cookie": "~0.4.1",
"cross-fetch": "^3.1.4",
"deep-extend": "~0.6.0",
"deepmerge": "~4.2.2",
"fast-json-patch": "^3.0.0-1",
"form-data-encoder": "^1.4.3",
"formdata-node": "^4.0.0",
Expand Down
41 changes: 3 additions & 38 deletions src/specmap/lib/index.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as jsonPatch from 'fast-json-patch';
import deepExtend from 'deep-extend';
import cloneDeep from 'lodash/cloneDeep';
import deepmerge from 'deepmerge'

export default {
add,
Expand Down Expand Up @@ -40,42 +39,8 @@ function applyPatch(obj, patch, opts) {
jsonPatch.applyPatch(obj, [replace(patch.path, newValue)]);
} else if (patch.op === 'mergeDeep') {
const currentValue = getInByJsonPath(obj, patch.path);

// Iterate the properties of the patch
// eslint-disable-next-line no-restricted-syntax, guard-for-in
for (const prop in patch.value) {
const propVal = patch.value[prop];
const isArray = Array.isArray(propVal);
if (isArray) {
// deepExtend doesn't merge arrays, so we will do it manually
const existing = currentValue[prop] || [];
currentValue[prop] = existing.concat(propVal);
} else if (isObject(propVal) && !isArray) {
// If it's an object, iterate it's keys and merge
// if there are conflicting keys, merge deep, otherwise shallow merge
let currentObj = { ...currentValue[prop] };
// eslint-disable-next-line no-restricted-syntax
for (const key in propVal) {
if (Object.prototype.hasOwnProperty.call(currentObj, key)) {
// if there is a single conflicting key, just deepExtend the entire value
// and break from the loop (since all future keys are also merged)
// We do this because we can't deepExtend two primitives
// (currentObj[key] & propVal[key] may be primitives).
//
// we also deeply assign here, since we aren't in control of
// how deepExtend affects existing nested objects
currentObj = deepExtend(cloneDeep(currentObj), propVal);
break;
} else {
Object.assign(currentObj, { [key]: propVal[key] });
}
}
currentValue[prop] = currentObj;
} else {
// It's a primitive, just replace existing
currentValue[prop] = propVal;
}
}
const newValue = deepmerge(currentValue, patch.value);
obj = jsonPatch.applyPatch(obj, [replace(patch.path, newValue)]).newDocument;
} else if (patch.op === 'add' && patch.path === '' && isObject(patch.value)) {
// { op: 'add', path: '', value: { a: 1, b: 2 }}
// has no effect: json patch refuses to do anything.
Expand Down
247 changes: 247 additions & 0 deletions test/bugs/ui-7618.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,247 @@
// https://github.com/swagger-api/swagger-ui/issues/7618

import resolveSubtree from '../../src/subtree-resolver';

const spec = {
"openapi": "3.0.3",
"info": {
"version": "1.0.0",
"title": "test",
"description": "test",
"contact": {
"name": "test",
"email": "[email protected]"
}
},
"servers": [
{
"url": "https://localhost/api"
}
],
"tags": [
{
"name": "Test",
"description": "Test"
}
],
"paths": {
"/one": {
"get": {
"description": "test",
"operationId": "one",
"tags": [
"Test"
],
"parameters": [
{
"name": "test",
"in": "header",
"required": true,
"schema": {
"type": "string"
}
}
],
"responses": {
"default": {
"description": "Response",
"content": {
"text/plain": {
"schema": {
"type": "integer",
"enum": [
401,
404,
500
]
}
}
}
}
}
}
}
},
"components": {
"schemas": {
"parent": {
"type": "object",
"required": [
"required1"
],
"properties": {
"required1": {
"type": "string"
},
"nested1": {
"type": "object",
"required": [
"nestedrequired1"
],
"properties": {
"nestedrequired1": {
"type": "string"
}
}
}
}
},
"child": {
"allOf": [
{
"$ref": "#/components/schemas/parent"
},
{
"type": "object",
"required": [
"required2"
],
"properties": {
"required2": {
"type": "string"
},
"nested1": {
"type": "object",
"required": [
"nestedrequired2"
],
"properties": {
"nestedrequired2": {
"type": "string"
}
}
}
}
}
]
}
}
}
};

test('should resolve test case from UI-7618 correctly', async () => {
const res = await resolveSubtree(spec, []);

expect(res).toEqual({
"spec": {
"openapi": "3.0.3",
"info": {
"version": "1.0.0",
"title": "test",
"description": "test",
"contact": {
"name": "test",
"email": "[email protected]"
}
},
"servers": [
{
"url": "https://localhost/api"
}
],
"tags": [
{
"name": "Test",
"description": "Test"
}
],
"paths": {
"/one": {
"get": {
"description": "test",
"operationId": "one",
"tags": [
"Test"
],
"parameters": [
{
"name": "test",
"in": "header",
"required": true,
"schema": {
"type": "string"
}
}
],
"responses": {
"default": {
"description": "Response",
"content": {
"text/plain": {
"schema": {
"type": "integer",
"enum": [
401,
404,
500
]
}
}
}
}
},
"__originalOperationId": "one"
}
}
},
"components": {
"schemas": {
"parent": {
"type": "object",
"required": [
"required1"
],
"properties": {
"required1": {
"type": "string"
},
"nested1": {
"type": "object",
"required": [
"nestedrequired1"
],
"properties": {
"nestedrequired1": {
"type": "string"
}
}
}
}
},
"child": {
"type": "object",
"required": [
"required1",
"required2"
],
"properties": {
"required1": {
"type": "string"
},
"nested1": {
"type": "object",
"required": [
"nestedrequired1",
"nestedrequired2"
],
"properties": {
"nestedrequired1": {
"type": "string"
},
"nestedrequired2": {
"type": "string"
}
}
},
"required2": {
"type": "string"
}
}
}
}
},
"$$normalized": true
},
"errors": []
});
});

0 comments on commit 944380d

Please sign in to comment.