Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove-unused-components to handle circular schemas #1789

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/mean-fireants-lay.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@redocly/openapi-core": major
---

Fixed an issue where remove-unused-components was not remove all the circular schemas
88 changes: 88 additions & 0 deletions packages/core/src/decorators/oas3/__tests__/circular-ref.yaml
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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',
},
},
},
},
},
});
});
});
85 changes: 74 additions & 11 deletions packages/core/src/decorators/oas3/remove-unused-components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>();
const stack = new Set<string>();
const circularDeps = new Set<string>();

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;

Expand All @@ -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] === '/'))
)
);

Expand Down Expand Up @@ -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(),
});
}
Expand All @@ -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)) {
Expand Down