From bf53c86d87a697562fe89ac59506b5bc4eb4a4ac Mon Sep 17 00:00:00 2001 From: "lu.zhongqiang" <4904644+htgylzhq@users.noreply.github.com> Date: Fri, 18 Sep 2026 13:40:08 +0900 Subject: [PATCH] fix(resolver): generate allOf $ref patches before merging members The allOf plugin generated the patches that absolutify $ref/$$ref values after pushing the mergeDeep patch, using paths relative to the allOf member. deepmerge concatenates arrays, so a member's `oneOf/0` ends up at `oneOf/2` once an earlier member has already contributed to `oneOf`, and the patch then targets a branch that has no such node: Resolver error Cannot read properties of undefined (reading 'items') Generate these patches up front instead, while every member is still in place at `allOf/`, so their paths cannot go stale. The refs plugin strips `allOf//` from the paths it records, so cycle detection keeps working exactly as before. Refs swagger-api/swagger-ui#11018 Co-Authored-By: Claude Opus 5 --- src/resolver/specmap/lib/all-of.js | 31 ++++---- test/resolver/specmap/all-of.js | 114 +++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 14 deletions(-) diff --git a/src/resolver/specmap/lib/all-of.js b/src/resolver/specmap/lib/all-of.js index 4748ba462..26f9ca5ae 100644 --- a/src/resolver/specmap/lib/all-of.js +++ b/src/resolver/specmap/lib/all-of.js @@ -41,10 +41,26 @@ export default { const patches = []; + // Generate patches that migrate $ref values based on ContextTree information, + // while each member is still in place at `allOf/`: the members are about to + // be merged onto the parent, and deepmerge concatenates arrays, so a member's + // `oneOf/0` may end up at `oneOf/2` (swagger-api/swagger-ui#11018). + // + // These have to stay patches. Applying them also re-runs the refs plugin over + // each $ref, which is what keeps the cycle detection in refs.js fed; resolving + // the values inline instead lets recursive schemas expand without bound. The + // test that catches that is test/resolver/specmap/complex.js, not this plugin's. + val.forEach((toMerge, i) => { + if (specmap.isObject(toMerge)) { + // String(i), because a numeric token breaks escapeJsonPointerToken() + patches.push(...generateAbsoluteRefPatches(toMerge, [...fullPath, String(i)], { specmap })); + } + }); + // remove existing content patches.push(specmap.replace(parent, {})); - val.forEach((toMerge, i) => { + val.forEach((toMerge) => { if (!specmap.isObject(toMerge)) { if (alreadyAddError) { return null; @@ -59,19 +75,6 @@ export default { // Deeply merge the member's contents onto the parent location patches.push(specmap.mergeDeep(parent, toMerge)); - // Generate patches that migrate $ref values based on ContextTree information - - // remove ["allOf"], which will not be present when these patches are applied - const collapsedFullPath = fullPath.slice(0, -1); - - const absoluteRefPatches = generateAbsoluteRefPatches(toMerge, collapsedFullPath, { - getBaseUrlForNodePath: (nodePath) => - specmap.getContext([...fullPath, i, ...nodePath]).baseDoc, - specmap, - }); - - patches.push(...absoluteRefPatches); - return undefined; }); diff --git a/test/resolver/specmap/all-of.js b/test/resolver/specmap/all-of.js index 421b75475..a8bec767b 100644 --- a/test/resolver/specmap/all-of.js +++ b/test/resolver/specmap/all-of.js @@ -658,4 +658,118 @@ describe('allOf', () => { }, }); }); + + describe('given allOf members with array keywords and $refs', () => { + // https://github.com/swagger-api/swagger-ui/issues/11018 + const chartYaml = ` +ChartConfiguration: + type: object + properties: + chartTypes: + type: array + items: + $ref: '#/ChartType' + allOf: + - oneOf: + - properties: + view: + enum: [summary] + - properties: + view: + enum: [detail] + - oneOf: + - properties: + chartTypes: + type: array + items: + $ref: '#/ChartType' + - properties: + view: + enum: [detail] +ChartType: + type: string + enum: [line, bar, pie] +`; + const chartsYaml = ` +charts: + post: + requestBody: + content: + application/json: + schema: + $ref: '../schemas/chart.yml#/ChartConfiguration' +`; + + afterEach(() => { + plugins.refs.clearCache(); + }); + + test('should absolutify $refs merged into concatenated arrays', async () => { + const mockPool = mockAgent.get('http://example.com'); + mockPool.intercept({ path: '/paths/charts.yml' }).reply(200, chartsYaml); + mockPool.intercept({ path: '/schemas/chart.yml' }).reply(200, chartYaml); + + const res = await mapSpec({ + plugins: [plugins.refs, plugins.allOf], + allowMetaPatches: true, + context: { baseDoc: 'http://example.com/openapi.yml' }, + spec: { + openapi: '3.0.3', + paths: { '/charts': { $ref: './paths/charts.yml#/charts' } }, + }, + }); + + expect(res.errors).toEqual([]); + const { schema } = res.spec.paths['/charts'].post.requestBody.content['application/json']; + expect(schema.oneOf).toHaveLength(4); + expect(schema.oneOf[2].properties.chartTypes.items).toEqual({ + type: 'string', + enum: ['line', 'bar', 'pie'], + $$ref: 'http://example.com/schemas/chart.yml#/ChartType', + }); + }); + + test('should not misplace $$ref patches when merging arrays', async () => { + const res = await mapSpec({ + plugins: [plugins.refs, plugins.allOf], + allowMetaPatches: true, + spec: { + definitions: { + ChartType: { type: 'string', enum: ['line', 'bar', 'pie'] }, + ChartConfiguration: { + allOf: [ + { oneOf: [{ properties: { view: { enum: ['summary'] } } }] }, + { + oneOf: [ + { + properties: { + chartTypes: { type: 'array', items: { $ref: '#/definitions/ChartType' } }, + }, + }, + ], + }, + ], + }, + }, + }, + }); + + expect(res.errors).toEqual([]); + expect(res.spec.definitions.ChartConfiguration.oneOf).toEqual([ + { properties: { view: { enum: ['summary'] } } }, + { + properties: { + chartTypes: { + type: 'array', + items: { + type: 'string', + enum: ['line', 'bar', 'pie'], + $$ref: '#/definitions/ChartType', + }, + }, + }, + }, + ]); + }); + }); });