apple / swift-openapi-generator

Generate Swift client and server code from an OpenAPI document.
https://swiftpackageindex.com/apple/swift-openapi-generator/documentation
Apache License 2.0
1.23k stars 89 forks source link

question: working with discriminators and inheritance on nested data #515

Closed schweizerm closed 4 months ago

schweizerm commented 4 months ago

Question

Hey guys :) Currently I'm not sure if I'm not understanding things correctly or if there's indeed a missing/buggy functionality - kinda new to both openapi and swift.

In general I do have an Endpoint that returns some nested data.

Class structure look like this (pseudo): Action(details: ActionDetails) ActionDetails(type: String) ActionDetailsFoo(foo: String): ActionDetails ActionDetailsBar(bar: int): ActionDetails

GET /nextAction returns Action

{
    "details": {
        "type": "FOO",
        "foo": "whatever"
    }
}

See (manually adjusted) file with notes:

openapi: 3.0.1
info:
  title: OpenAPI definition
  version: v0
servers:
  - url: http://localhost:8080
    description: Generated server url
paths:
  /nextAction:
    get:
      tags:
        - open-api-controller
      operationId: nextAction
      responses:
        default:
          description: OK
          content:
            'application/json':
              schema:
                $ref: '#/components/schemas/Action'
components:
  schemas:
    Action:
      type: object
      properties:
        details:
          $ref: '#/components/schemas/ActionDetails'
    ActionDetails:
      required:
        - type
      type: object
      properties:
        type:
          type: string
      discriminator:
        propertyName: type
        mapping:
          FOO: '#/components/schemas/ActionDetailsFoo'
          BAR: '#/components/schemas/ActionDetailsBar'
      anyOf: # manually added; removing this will always parse an object from ActionDetails without further info
        - $ref: '#/components/schemas/ActionDetailsFoo'
        - $ref: '#/components/schemas/ActionDetailsBar'
    ActionDetailsBar:
      type: object
      allOf:
#        - $ref: '#/components/schemas/ActionDetails' # keeping this ref to parent will end up in a loop
        - type: object
          properties:
            bar:
              type: integer
              format: int32
    ActionDetailsFoo:
      type: object
      allOf:
#        - $ref: '#/components/schemas/ActionDetails'
        - type: object
          properties:
            foo:
              type: string

I've been playing around a lot with the openapi spec but no matter how I put it, I can't really get it running as expected

A) removing the anyOf on the ActionDetails level will ignore any subtype when parsing

note: the anyOf is currently manually added after generation

        public struct ActionDetails: Codable, Hashable, Sendable {
            public var _type: Swift.String
            public init(_type: Swift.String) {
                self._type = _type
            }
            public enum CodingKeys: String, CodingKey {
                case _type = "type"
            }
        }

B) anyOf within the ActionDetails and allOf within the subclasses -> endless loop during runtime

ActionDetails.init() -> ActionDetailsFoo.init() -> ActionDetails.init() -> ActionDetailsFoo.init() -> ...

C) anyOf within the ActionDetails and removing allOf within subclasses:

kinda works, but I would have to manipulate the file manually and add all attributes that they should have from inheritance manually

From my understanding a) should be the correct way, as having a discriminator should take all schemas that have the parent schema with allOf included into consideration for alternate schemas.

Therefore I'm wondering if I'm doing it wrong or if this is indeed a bug / missing feature, can someone please enlighten me? :)

Also I'm generally new to swift and wonder if there's a more convenient way of getting the actual object then a switch on the cases? Maybe casting or anything? I usually have handlers and would like to just put the ActionDetails and they already know what kind of type they'd expect.

let result = client..

// is it possible to do something like
result as? ActionDetailsFoo 

// instead of
switch(result) {
    .case ActionDetailsFoo(let foo): 
    .case ActionDetailsBar(let bar): 
}

I actually do have a similar issue when sending responses of subtypes. I wonder if there's any elegant way in which I could send any subclass of ActionDetails instead of ActionDetails that lacks all the extra information to an endpoint that expects an object of type ActionDetails.

czechboy0 commented 4 months ago

I think you should be using oneOf instead of anyOf.

oneOf uses a discriminator and allows you to dynamically deserialize values.

Try that first and see if that helps.

schweizerm commented 4 months ago

Hey @czechboy0, my bad - you're right, I mixed up things while testing.

Nevertheless this only makes option # 3 valid:

image

Sadly in that case I have to comment out the "allOf" and "copy" over all properties from the parent like this:

    ActionDetailsBar:
      type: object
      allOf:
#        - $ref: '#/components/schemas/ActionDetails' # has to be commented out or I'll end up in an overflow
        - type: object
          properties:
            type: # type added from parent (my actual case contains many more values)
              type: string
            bar:
              type: integer
              format: int32

I kinda feel like this feels hacky as I have to manually adjust the generated openapi file..

schweizerm commented 4 months ago

Sorry that's wrong. I double checked and it actually works as intended without having to copy over the attributes. Thank you!

Edit: Anyway, the generated openapi.yaml looks like this:

    ActionDetails:
      required:
        - type
      type: object
      properties:
        type:
          type: string
      discriminator:
        propertyName: type
        mapping:
          FOO: '#/components/schemas/ActionDetailsFoo'
          BAR: '#/components/schemas/ActionDetailsBar'
    ActionDetailsBar:
      type: object
      allOf:
        - $ref: '#/components/schemas/ActionDetails'
        - type: object
          properties:
            bar:
              type: integer
              format: int32

while I now need it like this

    ActionDetails:
      required:
        - type
      type: object
      properties:
        type:
          type: string
      discriminator:
        propertyName: type
        mapping:
          FOO: '#/components/schemas/ActionDetailsFoo'
          BAR: '#/components/schemas/ActionDetailsBar'
      oneOf: # gotta add this
        - $ref: '#/components/schemas/ActionDetailsFoo'
        - $ref: '#/components/schemas/ActionDetailsBar'
    ActionDetailsBar:
      type: object
      allOf:
#        - $ref: '#/components/schemas/ActionDetails' # gotta remove this 
        - type: object
          properties:
            bar:
              type: integer
              format: int32

Can this somehow be achieved during the export instead of with hacky string replacements afterwards?

czechboy0 commented 4 months ago

I think what you want is this:

components:
  schemas:
    Action:
      type: object
      properties:
        details:
          $ref: '#/components/schemas/ActionDetails'
    ActionCommon:
      type: object
      properties:
        type:
          type: string
      required:
        - type
    ActionDetails:
      oneOf:
        - $ref: '#/components/schemas/ActionDetailsFoo'
        - $ref: '#/components/schemas/ActionDetailsBar'
      discriminator:
        propertyName: type
        mapping:
          FOO: '#/components/schemas/ActionDetailsFoo'
          BAR: '#/components/schemas/ActionDetailsBar'
    ActionDetailsBar:
      type: object
      allOf:
        - $ref: '#/components/schemas/ActionCommon'
        - type: object
           properties:
             bar:
               type: integer
               format: int32
           required:
             - bar
    ActionDetailsFoo:
      type: object
      allOf:
        - $ref: '#/components/schemas/ActionCommon'
        - type: object
           properties:
             foo:
               type: string
           required:
             - foo

Note that this also means that if you add a 3rd type in the future (e.g. Baz), that'd be considered API-breaking, as providing an unknown discriminator value (BAZ) will fail deserialization.

If you'd like to instead allow unknown values, and have the application logic deal with them, and gracefully fall back, you'd change ActionDetails to:

    ActionDetails:
      anyOf:
        - oneOf:
            - $ref: '#/components/schemas/ActionDetailsFoo'
            - $ref: '#/components/schemas/ActionDetailsBar'
          discriminator:
            propertyName: type
            mapping:
              FOO: '#/components/schemas/ActionDetailsFoo'
              BAR: '#/components/schemas/ActionDetailsBar'
        - type: object

More details on open vs closed oneOf/enums: https://swiftpackageindex.com/apple/swift-openapi-generator/1.2.0/documentation/swift-openapi-generator/useful-openapi-patterns#Open-enums-and-oneOfs

TL;DR: you wrap it in an extra anyOf and add a type: object, as that'll always succeed to deserialize. And you'd first check if the type-safe oneOf is non-nil, in which case you received a known type, but if it's nil, you know you received an unknown type, and it's present in the second value, in the freeform type: object.

Now, note that while it's more flexible this way and allows for easier API evolution, it is slightly less performant as the known events will always be deserialized twice, once in the known oneOf, and once in the freeform object. It's probably still desirable for its benefits, just something to be aware of.

schweizerm commented 4 months ago

Thank you very much for your input, that totally makes sense. Also thanks for the hint about the graceful fallback, much appreciated.

I'm just a little bit worried as to my current understanding this means that I have to manipulate the autogenerated openapi file - or find a way in which it gets generated like that. But I'll do my research on this, as it's kinda unrelated to the swift openapi generator project. Again, thank you very much for your input :)

czechboy0 commented 4 months ago

You're welcome.

Out of curiosity, what are you generating the OpenAPI file from? I'd instead encourage you write the OpenAPI document by hand, and use that as the source of truth.

More details on that approach here: https://swiftpackageindex.com/apple/swift-openapi-generator/1.2.0/documentation/swift-openapi-generator/practicing-spec-driven-api-development

schweizerm commented 4 months ago

For this minimal running example I was using the springdoc openapi gradle plugin. The issue I'm facing is on a project from work - I think we use the swagger gradle plugin there. Currently the generated yaml contains more than 20k lines; so I think I'd rather find a script solution rather than doing it by hand, but I'll discuss further steps with my team :D

czechboy0 commented 4 months ago

You can start with the doc generated today, and flip the process and generate the code from the doc, instead of the other way around.

That way you can start making hand edits right away, without rewriting the doc from scratch.