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

Support multiple discriminator mappings to share one type #36

Merged
merged 5 commits into from
Jun 9, 2022

Conversation

imjn
Copy link
Member

@imjn imjn commented May 13, 2022

Background

Currently, CreateAPI only supports a unique type for each mapping case.
For example, if we have

Container:
      type: object
      required:
        - content
      properties:
        content:
          oneOf:
            - $ref: "#/components/schemas/A"
            - $ref: "#/components/schemas/B"
            - $ref: "#/components/schemas/C"
          discriminator:
            propertyName: kind
            mapping:
              a: "#/components/schemas/A"
              b: "#/components/schemas/B"
              c: "#/components/schemas/C"
              d: "#/components/schemas/A"

it only generates .a but .d is not generated.
This is because we only use the first case with the property type.

if let correspondingMapping = discriminator.mapping.first(where: { $1 == property.type}) {

How

In this PR, I have fixed the issue above by checking the mapping keys and updated the discriminator test case for the case when two enum cases share one type.

Hope the changes make sense. Let me know if you have better options 🙇🏽

@imjn imjn self-assigned this May 13, 2022
@imjn imjn force-pushed the fix-wrong-generation-of-oneOf branch 3 times, most recently from 358c5c5 to 1e1caa4 Compare May 16, 2022 15:55
@imjn imjn force-pushed the fix-wrong-generation-of-oneOf branch from 1e1caa4 to 9889dab Compare May 16, 2022 15:56
@imjn imjn marked this pull request as ready for review May 16, 2022 16:08
@imjn imjn requested review from kean, ainame and liamnichols May 16, 2022 16:09
@imjn imjn changed the title Fixed wrong generation of oneOf Support multiple discriminator mappings to share one type May 16, 2022
Copy link
Member

@liamnichols liamnichols left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up! It's looking good 💪💪💪

Sources/CreateAPI/Generator/Templates.swift Outdated Show resolved Hide resolved
Sources/CreateAPI/Generator/Templates.swift Outdated Show resolved Hide resolved
Sources/CreateAPI/Generator/Generator+Render.swift Outdated Show resolved Hide resolved
imjn and others added 3 commits May 17, 2022 15:56
Co-authored-by: Liam Nichols <liam.nichols.ln@gmail.com>
Co-authored-by: Liam Nichols <liam.nichols.ln@gmail.com>
@imjn imjn requested a review from liamnichols May 17, 2022 15:15
@imjn imjn force-pushed the fix-wrong-generation-of-oneOf branch from 576b9f6 to 9701306 Compare June 7, 2022 15:55
Copy link
Member

@liamnichols liamnichols left a comment

Choose a reason for hiding this comment

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

Nice one @imjn, great work! 🚀

@liamnichols liamnichols merged commit 3982881 into CreateAPI:main Jun 9, 2022
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.

2 participants