Skip to content

Commit

Permalink
fix: remove unused components was skipping schemas with a top-level a…
Browse files Browse the repository at this point in the history
…llOf. fixes #1209
  • Loading branch information
jedelson-pagerduty committed Aug 8, 2023
1 parent fddb9a0 commit 718aa7b
Show file tree
Hide file tree
Showing 5 changed files with 235 additions and 10 deletions.
6 changes: 6 additions & 0 deletions .changeset/light-games-clap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@redocly/openapi-core': patch
'@redocly/cli': patch
---

Fixed an issue where remove-unused-components would not remove schemas which had a top-level allOf
Original file line number Diff line number Diff line change
Expand Up @@ -152,4 +152,77 @@ describe('oas2 remove-unused-components', () => {
},
});
});

it('should remove unused components even if they have a root allOf', async () => {
const document = parseYamlToDocument(
outdent`
swagger: '2.0'
paths:
/pets:
get:
produces:
- application/json
parameters: []
responses:
'200':
schema:
$ref: '#/definitions/Used'
operationId: listPets
summary: List all pets
definitions:
Unused:
allOf:
- type: object
properties:
one:
type: string
- type: object
properties:
two:
type: string
Used:
properties:
link:
$ref: '#/definitions/InnerUsed/properties/link'
type: object
`,
'foobar.yaml'
);

const results = await bundleDocument({
externalRefResolver: new BaseResolver(),
document,
config: await makeConfig({}),
removeUnusedComponents: true,
});

expect(results.bundle.parsed).toEqual({
swagger: '2.0',
definitions: {
Used: {
properties: {
link: { $ref: '#/definitions/InnerUsed/properties/link' },
},
type: 'object',
},
},
paths: {
'/pets': {
get: {
produces: ['application/json'],
parameters: [],
summary: 'List all pets',
operationId: 'listPets',
responses: {
'200': {
schema: {
$ref: '#/definitions/Used',
},
},
},
},
},
},
});
});
});
6 changes: 2 additions & 4 deletions packages/core/src/rules/oas2/remove-unused-components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,8 @@ export const RemoveUnusedComponents: Oas2Rule = () => {
},
},
NamedSchemas: {
Schema(schema, { location, key }) {
if (!schema.allOf) {
registerComponent(location, 'definitions', key.toString());
}
Schema(_schema, { location, key }) {
registerComponent(location, 'definitions', key.toString());
},
},
NamedParameters: {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { outdent } from 'outdent';
import { cloneDeep } from 'lodash';
import { parseYamlToDocument, makeConfig } from '../../../../__tests__/utils';
import { bundleDocument } from '../../../bundle';
import { BaseResolver } from '../../../resolve';
Expand Down Expand Up @@ -168,4 +169,147 @@ describe('oas3 remove-unused-components', () => {
},
});
});

it('should remove unused components even if they have a root allOf', async () => {
const document = parseYamlToDocument(
outdent`
openapi: "3.0.0"
paths:
/pets:
get:
summary: List all pets
operationId: listPets
responses:
'200':
content:
application/json:
schema:
$ref: '#/components/schemas/Used'
components:
schemas:
Unused:
allOf:
- type: object
properties:
one:
type: string
- type: object
properties:
two:
type: string
Used:
type: object
properties:
link:
type: string
`,
'foobar.yaml'
);

const results = await bundleDocument({
externalRefResolver: new BaseResolver(),
document,
config: await makeConfig({}),
removeUnusedComponents: true,
});

expect(results.bundle.parsed).toEqual({
openapi: '3.0.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: { type: 'string' },
},
},
},
},
});
});

it('should remove unused components even if they have a root allOf and a discriminator', async () => {
const document = parseYamlToDocument(
outdent`
openapi: "3.0.0"
paths:
/pets:
get:
summary: List all pets
operationId: listPets
responses:
'200':
content:
application/json:
schema:
type: array
items:
$ref: '#/components/schemas/Pet'
components:
schemas:
Pet:
type: object
properties:
petType:
type: string
discriminator:
propertyName: petType
required:
- petType
Cat:
allOf:
- $ref: '#/components/schemas/Pet'
- type: object
properties:
name:
type: string
Dog:
allOf:
- $ref: '#/components/schemas/Pet'
- type: object
properties:
bark:
type: string
Lizard:
allOf:
- $ref: '#/components/schemas/Pet'
- type: object
properties:
lovesRocks:
type: boolean
`,
'foobar.yaml'
);

const orig = cloneDeep(document.parsed);

const results = await bundleDocument({
externalRefResolver: new BaseResolver(),
document,
config: await makeConfig({}),
removeUnusedComponents: true,
});

// unchanged document
expect(results.bundle.parsed).toEqual(orig);
});
});
16 changes: 10 additions & 6 deletions packages/core/src/rules/oas3/remove-unused-components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@ export const RemoveUnusedComponents: Oas3Rule = () => {
function registerComponent(
location: Location,
componentType: keyof Oas3Components,
name: string
name: string,
used = false
): void {
components.set(location.absolutePointer, {
used: components.get(location.absolutePointer)?.used || false,
used: components.get(location.absolutePointer)?.used || used,
componentType,
name,
});
Expand Down Expand Up @@ -65,10 +66,13 @@ export const RemoveUnusedComponents: Oas3Rule = () => {
},
},
NamedSchemas: {
Schema(schema, { location, key }) {
if (!schema.allOf) {
registerComponent(location, 'schemas', key.toString());
}
Schema(schema, { location, key, resolve }) {
registerComponent(
location,
'schemas',
key.toString(),
schema.allOf && schema.allOf.some((v) => v.$ref && resolve(v).node?.discriminator)
);
},
},
NamedParameters: {
Expand Down

0 comments on commit 718aa7b

Please sign in to comment.