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: allof client generation handling #1873

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gausnes
Copy link

@gausnes gausnes commented Mar 31, 2022

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

NestJS swagger currently uses allOf in somewhat of a weird usage based on examples within the documentation. This is fine for rendering the specification, but causes issues when attempting to generate models and clients off of the emitted specification. OpenAPI Generator is not inspect the required flag for composite schemas.

This is not the only solution to this issue, but was the simplest. I spent some time evaluating fixing this within the generator code, but it was significantly more involved. This change does not feel like an explicitly inerrant as the resulting object of compose schema use is an object.

properties:
  id:
    description: Identifier
    allOf:
      - $ref: '#/components/schemas/Identifier'
  name:
    type: string
required:
  - eventId

What is the new behavior?

Typing these properties as objects helps the generator inspect the expected fields for correct generation.

properties:
  id:
    description: Identifier
    type: object
    allOf:
      - $ref: '#/components/schemas/Identifier'
  name:
    type: string
required:
  - eventId

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@gausnes gausnes force-pushed the fix/openapi-allof-generation branch from 6fa2bd6 to 06589af Compare March 31, 2022 22:35
@@ -161,7 +161,7 @@ export class SchemaObjectFactory {

const schemaCombinators = ['oneOf', 'anyOf', 'allOf'];
if (schemaCombinators.some((key) => key in property)) {
delete (property as SchemaObjectMetadata).type;
(property as SchemaObjectMetadata).type = 'object'
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't label as a breaking change as this is adding not removing properties. Based on my testing OpenAPI validators were happy with this addition.

@gausnes gausnes force-pushed the fix/openapi-allof-generation branch from 06589af to e7d98ef Compare March 31, 2022 22:36
@TiBarification
Copy link

Is it possible to add these changes to main repo?

@fredx21
Copy link

fredx21 commented Apr 4, 2023

We are also impacted by this problem. What is required for the merge?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants