Skip to content

Commit a36bcc3

Browse files
lipnitsklipnitsk
committed
fix: deep array merge
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")
1 parent e5788a8 commit a36bcc3

File tree

3 files changed

+221
-39
lines changed

3 files changed

+221
-39
lines changed

‎package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@
112112
"btoa": "^1.2.1",
113113
"cookie": "~0.4.1",
114114
"cross-fetch": "^3.1.4",
115-
"deep-extend": "~0.6.0",
115+
"deepmerge": "~4.2.2",
116116
"fast-json-patch": "^3.0.0-1",
117117
"form-data-encoder": "^1.4.3",
118118
"formdata-node": "^4.0.0",

‎src/specmap/lib/index.js

Lines changed: 3 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import * as jsonPatch from 'fast-json-patch';
2-
import deepExtend from 'deep-extend';
3-
import cloneDeep from 'lodash/cloneDeep';
2+
import deepmerge from 'deepmerge';
43

54
export default {
65
add,
@@ -40,42 +39,8 @@ function applyPatch(obj, patch, opts) {
4039
jsonPatch.applyPatch(obj, [replace(patch.path, newValue)]);
4140
} else if (patch.op === 'mergeDeep') {
4241
const currentValue = getInByJsonPath(obj, patch.path);
43-
44-
// Iterate the properties of the patch
45-
// eslint-disable-next-line no-restricted-syntax, guard-for-in
46-
for (const prop in patch.value) {
47-
const propVal = patch.value[prop];
48-
const isArray = Array.isArray(propVal);
49-
if (isArray) {
50-
// deepExtend doesn't merge arrays, so we will do it manually
51-
const existing = currentValue[prop] || [];
52-
currentValue[prop] = existing.concat(propVal);
53-
} else if (isObject(propVal) && !isArray) {
54-
// If it's an object, iterate it's keys and merge
55-
// if there are conflicting keys, merge deep, otherwise shallow merge
56-
let currentObj = { ...currentValue[prop] };
57-
// eslint-disable-next-line no-restricted-syntax
58-
for (const key in propVal) {
59-
if (Object.prototype.hasOwnProperty.call(currentObj, key)) {
60-
// if there is a single conflicting key, just deepExtend the entire value
61-
// and break from the loop (since all future keys are also merged)
62-
// We do this because we can't deepExtend two primitives
63-
// (currentObj[key] & propVal[key] may be primitives).
64-
//
65-
// we also deeply assign here, since we aren't in control of
66-
// how deepExtend affects existing nested objects
67-
currentObj = deepExtend(cloneDeep(currentObj), propVal);
68-
break;
69-
} else {
70-
Object.assign(currentObj, { [key]: propVal[key] });
71-
}
72-
}
73-
currentValue[prop] = currentObj;
74-
} else {
75-
// It's a primitive, just replace existing
76-
currentValue[prop] = propVal;
77-
}
78-
}
42+
const newValue = deepmerge(currentValue, patch.value);
43+
obj = jsonPatch.applyPatch(obj, [replace(patch.path, newValue)]).newDocument;
7944
} else if (patch.op === 'add' && patch.path === '' && isObject(patch.value)) {
8045
// { op: 'add', path: '', value: { a: 1, b: 2 }}
8146
// has no effect: json patch refuses to do anything.

‎test/bugs/ui-7618.js

Lines changed: 217 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,217 @@
1+
// https://github.com/swagger-api/swagger-ui/issues/7618
2+
3+
import resolveSubtree from '../../src/subtree-resolver/index.js';
4+
5+
const spec = {
6+
openapi: '3.0.3',
7+
info: {
8+
version: '1.0.0',
9+
title: 'test',
10+
description: 'test',
11+
contact: {
12+
name: 'test',
13+
email: '[email protected]',
14+
},
15+
},
16+
servers: [
17+
{
18+
url: 'https://localhost/api',
19+
},
20+
],
21+
tags: [
22+
{
23+
name: 'Test',
24+
description: 'Test',
25+
},
26+
],
27+
paths: {
28+
'/one': {
29+
get: {
30+
description: 'test',
31+
operationId: 'one',
32+
tags: ['Test'],
33+
parameters: [
34+
{
35+
name: 'test',
36+
in: 'header',
37+
required: true,
38+
schema: {
39+
type: 'string',
40+
},
41+
},
42+
],
43+
responses: {
44+
default: {
45+
description: 'Response',
46+
content: {
47+
'text/plain': {
48+
schema: {
49+
type: 'integer',
50+
enum: [401, 404, 500],
51+
},
52+
},
53+
},
54+
},
55+
},
56+
},
57+
},
58+
},
59+
components: {
60+
schemas: {
61+
parent: {
62+
type: 'object',
63+
required: ['required1'],
64+
properties: {
65+
required1: {
66+
type: 'string',
67+
},
68+
nested1: {
69+
type: 'object',
70+
required: ['nestedrequired1'],
71+
properties: {
72+
nestedrequired1: {
73+
type: 'string',
74+
},
75+
},
76+
},
77+
},
78+
},
79+
child: {
80+
allOf: [
81+
{
82+
$ref: '#/components/schemas/parent',
83+
},
84+
{
85+
type: 'object',
86+
required: ['required2'],
87+
properties: {
88+
required2: {
89+
type: 'string',
90+
},
91+
nested1: {
92+
type: 'object',
93+
required: ['nestedrequired2'],
94+
properties: {
95+
nestedrequired2: {
96+
type: 'string',
97+
},
98+
},
99+
},
100+
},
101+
},
102+
],
103+
},
104+
},
105+
},
106+
};
107+
108+
test('should resolve test case from UI-7618 correctly', async () => {
109+
const res = await resolveSubtree(spec, []);
110+
111+
expect(res).toEqual({
112+
spec: {
113+
openapi: '3.0.3',
114+
info: {
115+
version: '1.0.0',
116+
title: 'test',
117+
description: 'test',
118+
contact: {
119+
name: 'test',
120+
email: '[email protected]',
121+
},
122+
},
123+
servers: [
124+
{
125+
url: 'https://localhost/api',
126+
},
127+
],
128+
tags: [
129+
{
130+
name: 'Test',
131+
description: 'Test',
132+
},
133+
],
134+
paths: {
135+
'/one': {
136+
get: {
137+
description: 'test',
138+
operationId: 'one',
139+
tags: ['Test'],
140+
parameters: [
141+
{
142+
name: 'test',
143+
in: 'header',
144+
required: true,
145+
schema: {
146+
type: 'string',
147+
},
148+
},
149+
],
150+
responses: {
151+
default: {
152+
description: 'Response',
153+
content: {
154+
'text/plain': {
155+
schema: {
156+
type: 'integer',
157+
enum: [401, 404, 500],
158+
},
159+
},
160+
},
161+
},
162+
},
163+
__originalOperationId: 'one',
164+
},
165+
},
166+
},
167+
components: {
168+
schemas: {
169+
parent: {
170+
type: 'object',
171+
required: ['required1'],
172+
properties: {
173+
required1: {
174+
type: 'string',
175+
},
176+
nested1: {
177+
type: 'object',
178+
required: ['nestedrequired1'],
179+
properties: {
180+
nestedrequired1: {
181+
type: 'string',
182+
},
183+
},
184+
},
185+
},
186+
},
187+
child: {
188+
type: 'object',
189+
required: ['required1', 'required2'],
190+
properties: {
191+
required1: {
192+
type: 'string',
193+
},
194+
nested1: {
195+
type: 'object',
196+
required: ['nestedrequired1', 'nestedrequired2'],
197+
properties: {
198+
nestedrequired1: {
199+
type: 'string',
200+
},
201+
nestedrequired2: {
202+
type: 'string',
203+
},
204+
},
205+
},
206+
required2: {
207+
type: 'string',
208+
},
209+
},
210+
},
211+
},
212+
},
213+
$$normalized: true,
214+
},
215+
errors: [],
216+
});
217+
});

0 commit comments

Comments
 (0)