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

Allow anyOf with polymorphism #191

Open
atollk opened this issue Apr 11, 2023 · 2 comments
Open

Allow anyOf with polymorphism #191

atollk opened this issue Apr 11, 2023 · 2 comments

Comments

@atollk
Copy link
Contributor

atollk commented Apr 11, 2023

Currently, the way to use discriminator-based polymorphism with Fabrikt is:

    polymorphicEnumDiscriminator:
      type: object
      discriminator:
        propertyName: some_enum
        mapping:
          obj_one_only: '#/components/schemas/ConcreteImplOneLegacy'
          obj_two_first: '#/components/schemas/ConcreteImplTwoLegacy'
          obj_two_second: '#/components/schemas/ConcreteImplTwoLegacy'
          obj_three: '#/components/schemas/ConcreteImplThreeLegacy'
      properties:
        some_enum:
          $ref: '#/components/schemas/EnumDiscriminator'

However, from all sources I could find, this isn't actually correct.
https://swagger.io/docs/specification/data-models/inheritance-and-polymorphism/

You actually need to specify the possible "subschemas" with anyOf or oneOf. That is:

    polymorphicEnumDiscriminator:
      type: object
      discriminator:
        propertyName: some_enum
        mapping:
          obj_one_only: '#/components/schemas/ConcreteImplOne'
          obj_two_first: '#/components/schemas/ConcreteImplTwo'
          obj_two_second: '#/components/schemas/ConcreteImplTwo'
          obj_three: '#/components/schemas/ConcreteImplThree'
      anyOf:
        - $ref: '#/components/schemas/ConcreteImplOne'
        - $ref: '#/components/schemas/ConcreteImplTwo'
        - $ref: '#/components/schemas/ConcreteImplTwo'
        - $ref: '#/components/schemas/ConcreteImplThree'
      properties:
        some_enum:
          $ref: '#/components/schemas/EnumDiscriminator'

This is also highlighted by the fact that many other OAS generators do not properly process the first variant. For example, https://editor.swagger.io/ renders it as an object with only a single property:

image

At the moment, the second variant causes a StackOverflowError in Fabrikt.

@cjbooms
Copy link
Owner

cjbooms commented Jun 14, 2023

Hi @atollk I am still unsure what to do with this issue and your related pull request. I personally have always struggled with the intention of anyOf. Where does it sit between oneOf and allOf?

I have recently improved the documentation around polymorphism in fabrikt. I would appreciate if you give it a read and let me know if this is still an issue we need to pursue.
https://github.com/cjbooms/fabrikt/pull/218/files

@atollk
Copy link
Contributor Author

atollk commented Jun 15, 2023

I'd say anyOf and oneOf are very similar, as both represent "Union" types, but different from allOf, which represents an "Intersection" type.

For all practical purposes that I have seen, anyOf and oneOf are the same. When using the discriminator feature, they are by definition.

In my experience, OpenAPI generators tend to support anyOf more often than oneOf. I suspect the reason for that is that to properly support anyOf, you just need some form of type union, whereas for oneOf, you'd also need to implement a form of not which I have not seen for any generator ever.

Anyway, the point of this PR isn't anyOf specific. Afair it was just that oneOf wasn't supported by Fabrikt when I opened it. The point is that at the moment, neither anyOf nor oneOf are used correctly with discriminators, i.e. both the OAS definition and other code generators disagree with the implementation of Fabrikt.

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 a pull request may close this issue.

2 participants