cjbooms / fabrikt

Generates Kotlin Code from OpenApi3 Specifications
Apache License 2.0
154 stars 41 forks source link

Allow `anyOf` with polymorphism #191

Open atollk opened 1 year ago

atollk commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.