diff --git a/.changeset/quiet-snakes-buy.md b/.changeset/quiet-snakes-buy.md new file mode 100644 index 0000000000..9e26110fd3 --- /dev/null +++ b/.changeset/quiet-snakes-buy.md @@ -0,0 +1,8 @@ +--- +"@redocly/openapi-core": patch +"@redocly/cli": patch +--- + +Fixed an issue where `--component-renaming-conflicts-severity` ignored conflicts when different files had components with the same name but different content. + +**Warning:** Autogenrated component names and `$ref` paths in bundled documents may differ from older releases. diff --git a/packages/core/src/__tests__/__snapshots__/bundle-oas.test.ts.snap b/packages/core/src/__tests__/__snapshots__/bundle-oas.test.ts.snap index 56cad3ec3d..a57b3da45e 100644 --- a/packages/core/src/__tests__/__snapshots__/bundle-oas.test.ts.snap +++ b/packages/core/src/__tests__/__snapshots__/bundle-oas.test.ts.snap @@ -26,7 +26,7 @@ components: $ref: '#/components/schemas/schema-a' examples: first: - $ref: '#/components/examples/param-a-first' + $ref: '#/components/examples/first-2' second: $ref: '#/components/examples/second' path-param: @@ -41,7 +41,7 @@ components: examples: first: value: b1 - param-a-first: + first-2: value: a1 second: value: a2 diff --git a/packages/core/src/__tests__/__snapshots__/bundle.test.ts.snap b/packages/core/src/__tests__/__snapshots__/bundle.test.ts.snap index 92d77e2686..a861b60211 100644 --- a/packages/core/src/__tests__/__snapshots__/bundle.test.ts.snap +++ b/packages/core/src/__tests__/__snapshots__/bundle.test.ts.snap @@ -1,5 +1,28 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html +exports[`bundle > should bundle external pointer refs and warn for conflicting names 1`] = ` +openapi: 3.1.0 +paths: + /foo: + put: + parameters: + - $ref: '#/components/parameters/User-2' + get: + parameters: + - $ref: '#/components/parameters/User' +components: + parameters: + User: + name: test + in: query + description: test-B + User-2: + name: test + in: path + description: test-A + +`; + exports[`bundle > should bundle external refs 1`] = ` openapi: 3.0.0 paths: @@ -26,7 +49,7 @@ components: $ref: '#/components/schemas/schema-a' examples: first: - $ref: '#/components/examples/param-a-first' + $ref: '#/components/examples/first-2' second: $ref: '#/components/examples/second' path-param: @@ -41,7 +64,7 @@ components: examples: first: value: b1 - param-a-first: + first-2: value: a1 second: value: a2 @@ -124,6 +147,51 @@ components: `; +exports[`bundle > should keep dotted JSON pointer schema keys and rename conflicting User schemas 1`] = ` +openapi: 3.0.0 +info: + title: Test ref with dots in component names + version: '1.0' + description: Demo + license: + name: DEMO + url: https://demo.com +servers: + - url: http://demo.com/api +paths: + /org-user: + get: + responses: + '200': + description: ok + content: + application/json: + schema: + $ref: '#/components/schemas/my.org.User' + post: + responses: + '200': + description: ok + content: + application/json: + schema: + additionalProperties: + $ref: '#/components/schemas/my.User' +components: + schemas: + my.org.User: + type: object + properties: + orgId: + type: string + my.User: + type: object + properties: + displayName: + type: string + +`; + exports[`bundle > should normalize self-file explicit $ref in nested referenced file 1`] = ` openapi: 3.0.0 info: diff --git a/packages/core/src/__tests__/bundle-oas.test.ts b/packages/core/src/__tests__/bundle-oas.test.ts index 0beba237b7..70d6ba7c41 100644 --- a/packages/core/src/__tests__/bundle-oas.test.ts +++ b/packages/core/src/__tests__/bundle-oas.test.ts @@ -47,7 +47,11 @@ describe('bundle-oas', () => { config: await createEmptyRedoclyConfig({}), ref: path.join(__dirname, 'fixtures/refs/openapi-with-external-refs.yaml'), }); - expect(problems).toHaveLength(0); + expect(problems).toHaveLength(1); + expect(problems[0].severity).toBe('warn'); + expect(problems[0].message).toEqual( + `Two schemas are referenced with the same name but different content. Renamed first to first-2.` + ); expect(res.parsed).toMatchSnapshot(); }); diff --git a/packages/core/src/__tests__/bundle.test.ts b/packages/core/src/__tests__/bundle.test.ts index c9a88868a9..7da48ba2da 100644 --- a/packages/core/src/__tests__/bundle.test.ts +++ b/packages/core/src/__tests__/bundle.test.ts @@ -64,7 +64,11 @@ describe('bundle', () => { config: await createConfig({}), ref: path.join(__dirname, 'fixtures/refs/openapi-with-external-refs.yaml'), }); - expect(problems).toHaveLength(0); + expect(problems).toHaveLength(1); + expect(problems[0].severity).toBe('warn'); + expect(problems[0].message).toEqual( + `Two schemas are referenced with the same name but different content. Renamed first to first-2.` + ); expect(res.parsed).toMatchSnapshot(); }); @@ -119,6 +123,67 @@ describe('bundle', () => { ); }); + it('should bundle external pointer refs and warn for conflicting names', async () => { + const { bundle: res, problems } = await bundle({ + config: await createConfig({}), + ref: path.join( + __dirname, + 'fixtures/refs/openapi-with-external-refs-pointer-conflicting-names.yaml' + ), + }); + expect(problems).toHaveLength(1); + expect(problems[0].severity).toBe('warn'); + expect(problems[0].message).toEqual( + `Two schemas are referenced with the same name but different content. Renamed User to User-2.` + ); + expect(res.parsed).toMatchSnapshot(); + }); + + it('should bundle external pointer refs and do not show warnings for conflicting names', async () => { + const { problems } = await bundle({ + config: await createConfig({}), + ref: path.join( + __dirname, + 'fixtures/refs/openapi-with-external-refs-pointer-conflicting-names.yaml' + ), + componentRenamingConflicts: 'off', + }); + expect(problems).toHaveLength(0); + }); + + it('should report error-severity problems for conflicting pointer ref names', async () => { + const { problems } = await bundle({ + config: await createConfig({}), + ref: path.join( + __dirname, + 'fixtures/refs/openapi-with-external-refs-pointer-conflicting-names.yaml' + ), + componentRenamingConflicts: 'error', + }); + expect(problems).toHaveLength(1); + expect(problems[0].severity).toBe('error'); + expect(problems[0].message).toEqual( + `Two schemas are referenced with the same name but different content. Renamed User to User-2.` + ); + }); + + it('should keep dotted JSON pointer schema keys and rename conflicting User schemas', async () => { + const { bundle: res, problems } = await bundle({ + config: await createConfig({}), + ref: path.join( + __dirname, + 'fixtures/refs/openapi-bundle-external-schema-names-and-user-conflict.yaml' + ), + }); + + expect(problems).toHaveLength(0); + const schemas = (res.parsed as { components: { schemas: Record } }).components + .schemas; + expect(schemas['my.org.User']).toBeDefined(); + expect(schemas['my.User']).toBeDefined(); + expect(res.parsed).toMatchSnapshot(); + }); + it('should dereferenced correctly when used with dereference', async () => { const { bundle: res, problems } = await bundleDocument({ externalRefResolver: new BaseResolver(), diff --git a/packages/core/src/__tests__/fixtures/refs/component-a.yaml b/packages/core/src/__tests__/fixtures/refs/component-a.yaml new file mode 100644 index 0000000000..7a151d3d71 --- /dev/null +++ b/packages/core/src/__tests__/fixtures/refs/component-a.yaml @@ -0,0 +1,6 @@ +components: + parameters: + User: + name: test + in: path + description: test-A diff --git a/packages/core/src/__tests__/fixtures/refs/component-b.yaml b/packages/core/src/__tests__/fixtures/refs/component-b.yaml new file mode 100644 index 0000000000..4c1323f61a --- /dev/null +++ b/packages/core/src/__tests__/fixtures/refs/component-b.yaml @@ -0,0 +1,6 @@ +components: + parameters: + User: + name: test + in: query + description: test-B diff --git a/packages/core/src/__tests__/fixtures/refs/external-ref-with-dots.yaml b/packages/core/src/__tests__/fixtures/refs/external-ref-with-dots.yaml new file mode 100644 index 0000000000..838d9fa666 --- /dev/null +++ b/packages/core/src/__tests__/fixtures/refs/external-ref-with-dots.yaml @@ -0,0 +1,12 @@ +components: + schemas: + my.org.User: + type: object + properties: + orgId: + type: string + my.User: + type: object + properties: + displayName: + type: string diff --git a/packages/core/src/__tests__/fixtures/refs/openapi-bundle-external-schema-names-and-user-conflict.yaml b/packages/core/src/__tests__/fixtures/refs/openapi-bundle-external-schema-names-and-user-conflict.yaml new file mode 100644 index 0000000000..7a68a00946 --- /dev/null +++ b/packages/core/src/__tests__/fixtures/refs/openapi-bundle-external-schema-names-and-user-conflict.yaml @@ -0,0 +1,29 @@ +openapi: 3.0.0 +info: + title: Test ref with dots in component names + version: '1.0' + description: Demo + license: + name: DEMO + url: https://demo.com +servers: + - url: http://demo.com/api +paths: + /org-user: + get: + responses: + '200': + description: ok + content: + application/json: + schema: + $ref: external-ref-with-dots.yaml#/components/schemas/my.org.User + post: + responses: + '200': + description: ok + content: + application/json: + schema: + additionalProperties: + $ref: external-ref-with-dots.yaml#/components/schemas/my.User diff --git a/packages/core/src/__tests__/fixtures/refs/openapi-with-external-refs-pointer-conflicting-names.yaml b/packages/core/src/__tests__/fixtures/refs/openapi-with-external-refs-pointer-conflicting-names.yaml new file mode 100644 index 0000000000..0f189eebc2 --- /dev/null +++ b/packages/core/src/__tests__/fixtures/refs/openapi-with-external-refs-pointer-conflicting-names.yaml @@ -0,0 +1,9 @@ +openapi: 3.1.0 +paths: + /foo: + put: + parameters: + - $ref: 'component-a.yaml#/components/parameters/User' + get: + parameters: + - $ref: 'component-b.yaml#/components/parameters/User' diff --git a/packages/core/src/bundle/bundle-visitor.ts b/packages/core/src/bundle/bundle-visitor.ts index cdb7b06446..480d87cec7 100644 --- a/packages/core/src/bundle/bundle-visitor.ts +++ b/packages/core/src/bundle/bundle-visitor.ts @@ -5,6 +5,7 @@ import { replaceRef, isExternalValue, isRef, + pointerBaseName, refBaseName, type Location, } from '../ref-utils.js'; @@ -12,7 +13,6 @@ import { type ResolvedRefMap, type Document } from '../resolve.js'; import { reportUnresolvedRef } from '../rules/common/no-unresolved-refs.js'; import { type OasRef } from '../typings/openapi.js'; import { dequal } from '../utils/dequal.js'; -import { isTruthy } from '../utils/is-truthy.js'; import { makeRefId } from '../utils/make-ref-id.js'; import { type Oas3Visitor, type Oas2Visitor } from '../visitors.js'; import { type UserContext, type ResolveResult } from '../walk.js'; @@ -274,27 +274,9 @@ export function makeBundleVisitor({ componentType: string, ctx: UserContext ) { - const [fileRef, pointer] = [target.location.source.absoluteRef, target.location.pointer]; const componentsGroup = components[componentType]; - - let name = ''; - - const refParts = pointer.slice(2).split('/').filter(isTruthy); // slice(2) removes "#/" - while (refParts.length > 0) { - name = refParts.pop() + (name ? `-${name}` : ''); - if ( - !componentsGroup || - !componentsGroup[name] || - isEqualOrEqualRef(componentsGroup[name], target, ctx) - ) { - return name; - } - } - - name = refBaseName(fileRef) + (name ? `_${name}` : ''); - if (!componentsGroup[name] || isEqualOrEqualRef(componentsGroup[name], target, ctx)) { - return name; - } + const [fileRef, pointer] = [target.location.source.absoluteRef, target.location.pointer]; + let name = pointerBaseName(pointer) || refBaseName(fileRef); const prevName = name; let serialId = 2; @@ -303,7 +285,7 @@ export function makeBundleVisitor({ serialId++; } - if (!componentsGroup[name]) { + if (!componentsGroup[name] && prevName !== name) { ctx.report({ message: `Two schemas are referenced with the same name but different content. Renamed ${prevName} to ${name}.`, location: ctx.location,