diff --git a/.changeset/mean-fireants-lay.md b/.changeset/mean-fireants-lay.md new file mode 100644 index 0000000000..fa818d5e19 --- /dev/null +++ b/.changeset/mean-fireants-lay.md @@ -0,0 +1,5 @@ +--- +"@redocly/openapi-core": major +--- + +Fixed an issue where remove-unused-components was not remove all the circular schemas diff --git a/packages/core/src/decorators/oas3/__tests__/circular-ref.yaml b/packages/core/src/decorators/oas3/__tests__/circular-ref.yaml new file mode 100644 index 0000000000..6614e90930 --- /dev/null +++ b/packages/core/src/decorators/oas3/__tests__/circular-ref.yaml @@ -0,0 +1,88 @@ +openapi: 3.1.0 +info: + title: 'Test' + version: '1.0.0' +paths: + /pet: + post: + parameters: + - in: query + name: test + schema: + $ref: '#/components/schemas/wefewf' +components: + schemas: + abc: + properties: + property: + type: string + type: object + jkl: + properties: + filters: + items: + $ref: '#/components/schemas/abc' + type: array + type: object + def: + type: object + ghi: + properties: + property: + allOf: + - $ref: '#/components/schemas/jkl' + property1: + items: + $ref: '#/components/schemas/def' + type: array + property2: + items: + anyOf: + - $ref: '#/components/schemas/ghi' + - $ref: '#/components/schemas/mnor' + type: array + property3: + items: + $ref: '#/components/schemas/pqr' + type: array + property4: + allOf: + - $ref: '#/components/schemas/stu' + type: object + pqr: + properties: + property: + items: + $ref: '#/components/schemas/def' + type: array + type: object + mnor: + properties: + property: + items: + $ref: '#/components/schemas/def' + type: array + property1: + items: + anyOf: + - $ref: '#/components/schemas/ghi' + - $ref: '#/components/schemas/mnor' + type: array + property2: + items: + $ref: '#/components/schemas/pqr' + type: array + type: object + stu: + properties: + type: + allOf: + - $ref: '#/components/schemas/vwx' + default: absolute + type: object + vwx: + description: An enumeration. + wefewf: + properties: + property: + type: string diff --git a/packages/core/src/decorators/oas3/__tests__/remove-unused-components.test.ts b/packages/core/src/decorators/oas3/__tests__/remove-unused-components.test.ts index 7d0d223e88..0e529fd013 100644 --- a/packages/core/src/decorators/oas3/__tests__/remove-unused-components.test.ts +++ b/packages/core/src/decorators/oas3/__tests__/remove-unused-components.test.ts @@ -2,6 +2,8 @@ import { outdent } from 'outdent'; import { parseYamlToDocument, makeConfig } from '../../../../__tests__/utils'; import { bundleDocument } from '../../../bundle'; import { BaseResolver } from '../../../resolve'; +const fs = require('fs'); +const path = require('path'); describe('oas3 remove-unused-components', () => { it('should remove unused components', async () => { @@ -310,4 +312,148 @@ describe('oas3 remove-unused-components', () => { }, }); }); + + it('should remove simple unused recusive components', async () => { + const document = parseYamlToDocument( + outdent` + openapi: 3.1.0 + paths: + /pets: + get: + summary: List all pets + operationId: listPets + responses: + '200': + content: + application/json: + schema: + $ref: '#/components/schemas/Used' + components: + schemas: + Used: + type: object + properties: + link: + $ref: '#/components/schemas/InnerUsed' + InnerUsed: + type: object + properties: + link: + type: string + iwillbedrop: + properties: + name: + title: Name + type: string + type: object + iwontbdedrop: + properties: + Myprop: + items: + anyOf: + - $ref: '#/components/schemas/iwontbdedrop' + type: array + type: object + x-fgff: dd + `, + 'foobar.yaml' + ); + + const results = await bundleDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await makeConfig({ rules: {} }), + removeUnusedComponents: true, + }); + + expect(results.bundle.parsed).toEqual({ + openapi: '3.1.0', + paths: { + '/pets': { + get: { + summary: 'List all pets', + operationId: 'listPets', + responses: { + '200': { + content: { + 'application/json': { + schema: { + $ref: '#/components/schemas/Used', + }, + }, + }, + }, + }, + }, + }, + }, + components: { + schemas: { + InnerUsed: { + type: 'object', + properties: { + link: { + type: 'string', + }, + }, + }, + Used: { + type: 'object', + properties: { + link: { + $ref: '#/components/schemas/InnerUsed', + }, + }, + }, + }, + }, + }); + }); + + it('should remove complex unused recusive components', async () => { + const filePath = path.resolve(__dirname, 'circular-ref.yaml'); + const fileContents = fs.readFileSync(filePath, 'utf8'); + const document = parseYamlToDocument(fileContents); + + const results = await bundleDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await makeConfig({ rules: {} }), + removeUnusedComponents: true, + }); + + expect(results.bundle.parsed).toEqual({ + openapi: '3.1.0', + info: { + title: 'Test', + version: '1.0.0', + }, + paths: { + '/pet': { + post: { + parameters: [ + { + in: 'query', + name: 'test', + schema: { + $ref: '#/components/schemas/wefewf', + }, + }, + ], + }, + }, + }, + components: { + schemas: { + wefewf: { + properties: { + property: { + type: 'string', + }, + }, + }, + }, + }, + }); + }); }); diff --git a/packages/core/src/decorators/oas3/remove-unused-components.ts b/packages/core/src/decorators/oas3/remove-unused-components.ts index 4780961b9c..bdb8f284f0 100644 --- a/packages/core/src/decorators/oas3/remove-unused-components.ts +++ b/packages/core/src/decorators/oas3/remove-unused-components.ts @@ -22,6 +22,61 @@ export const RemoveUnusedComponents: Oas3Decorator = () => { }); } + function getBasePath(path: string): string { + const [fileLocation, localPointer] = path.split('#', 2); + return `${fileLocation}#${localPointer.split('/').slice(0, 4).join('/')}`; + } + + function removeCircularDependencies(): void { + const visited = new Set(); + const stack = new Set(); + const circularDeps = new Set(); + + function detectCircularDependencies(path: string): boolean { + if (stack.has(path)) { + circularDeps.add(path); + return true; + } + + if (visited.has(path)) { + return false; + } + + visited.add(path); + stack.add(path); + + const component = components.get(path); + if (component) { + for (const location of component.usedIn) { + const neighbor = getBasePath(location.absolutePointer); + if (detectCircularDependencies(neighbor)) { + return true; + } + } + } + + stack.delete(path); + return false; + } + + for (const path of components.keys()) { + if (!visited.has(path)) { + stack.clear(); + visited.clear(); + detectCircularDependencies(path); + } + } + + for (const path of circularDeps) { + const component = components.get(path); + if (component) { + component.usedIn = component.usedIn.filter( + (location) => !circularDeps.has(getBasePath(location.absolutePointer)) + ); + } + } + } + function removeUnusedComponents(root: Oas3Definition, removedPaths: string[]): number { const removedLengthStart = removedPaths.length; @@ -30,9 +85,10 @@ export const RemoveUnusedComponents: Oas3Decorator = () => { (location) => !removedPaths.some( (removed) => - location.absolutePointer.startsWith(removed) && - (location.absolutePointer.length === removed.length || - location.absolutePointer[removed.length] === '/') + location.absolutePointer.startsWith(path) || + (location.absolutePointer.startsWith(removed) && + (location.absolutePointer.length === removed.length || + location.absolutePointer[removed.length] === '/')) ) ); @@ -63,17 +119,22 @@ export const RemoveUnusedComponents: Oas3Decorator = () => { const resolvedRef = resolve(ref); if (!resolvedRef.location) return; - const [fileLocation, localPointer] = resolvedRef.location.absolutePointer.split('#', 2); - const componentLevelLocalPointer = localPointer.split('/').slice(0, 4).join('/'); - const pointer = `${fileLocation}#${componentLevelLocalPointer}`; - + const pointer = getBasePath(resolvedRef.location.absolutePointer); const registered = components.get(pointer); + const basePath = getBasePath(location.absolutePointer); - if (registered) { - registered.usedIn.push(location); - } else { + if (pointer !== basePath) { + if (registered) { + registered.usedIn.push(location); + } else { + components.set(pointer, { + usedIn: [location], + name: key.toString(), + }); + } + } else if (!registered) { components.set(pointer, { - usedIn: [location], + usedIn: [], name: key.toString(), }); } @@ -82,7 +143,9 @@ export const RemoveUnusedComponents: Oas3Decorator = () => { }, Root: { leave(root, ctx) { + removeCircularDependencies(); const data = ctx.getVisitorData() as { removedCount: number }; + data.removedCount = removeUnusedComponents(root, []); if (isEmptyObject(root.components)) {