Skip to content

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 ainame, kean 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 💪💪💪

@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