yonaskolb / SwagGen

OpenAPI/Swagger 3.0 Parser and Swift code generator
MIT License
627 stars 148 forks source link

Too many cases generated when oneOf discriminator has explicit mappings #297

Closed JanC closed 2 years ago

JanC commented 2 years ago

Hi,

I noticed that when I set an explicit mappings to a discriminator, the default mapping remain in the generated code when decoding.

Given this spec

    SingleAnimal:
      oneOf:
        - $ref: '#/components/schemas/Cat'
        - $ref: '#/components/schemas/Dog'
      discriminator:
        propertyName: type
        mapping:
          woof: '#/components/schemas/Dog'
          dog: '#/components/schemas/Dog'
          cat: '#/components/schemas/Cat'

The generated SingeAnimal.swift contains

https://github.com/yonaskolb/SwagGen/blob/2b3140b821129190aab28a5cf9b1e5e2151dbc9b/Specs/TestSpec/generated/Swift/Sources/Models/SingleAnimal.swift#L16-L25

As you can see, the switch on the discriminator value contains both the explicit mappings cat dog, woof values but also what seems to be the implicit mappings by the name of the reference Cat and Dog.

Since I supply explicit mappings, I would expect only the explicit ones to be present:

    let discriminator: String = try container.decode("type")
        switch discriminator {
        case "cat":
            self = .cat(try Cat(from: decoder))
        case "dog":
            self = .dog(try Dog(from: decoder))
        case "woof":
            self = .dog(try Dog(from: decoder))
        default:
            throw DecodingError.dataCorrupted(DecodingError.Context.init(codingPath: decoder.codingPath, debugDescription: "Couldn't find type to decode with discriminator \(discriminator)"))
        }

I think that if we detect an explicit mapping for a type, we should remove the implicit one in CodeFormatter.swift#L216

So just adding mapping.removeValue(forKey: reference.name) here would work:

  if let discriminatorMapping = groupSchema.discriminator?.mapping {
      for (key, value) in discriminatorMapping {
          let reference = Reference<Schema>(value)
          // remove the implicit mapping for that reference
          mapping.removeValue(forKey: reference.name)
          // add the explicit one
          mapping[key] = getReferenceContext(reference)
      }
  }

@yonaskolb any thoughts? If you agree with the approach, I can open a PR.

I used the specs.yaml from the repo but for reference I put the relevant part here:

openapi: 3.1.0
info:
  title: MyAPI
  version: '1.0'
servers:
  - url: 'http://localhost:3000'
paths:
  /pets/:
    parameters: []
    get:
      tags: []
      responses:
        '200':
          content:
            application/json:
              schema:
                type: array
                items:
                  $ref: '#/components/schemas/SingleAnimal'
      operationId: get-objects
components:
  schemas:
    Animal:
      type: object
      properties:
        animal:
          type: string
    SingleAnimal:
      oneOf:
        - $ref: '#/components/schemas/Cat'
        - $ref: '#/components/schemas/Dog'
      discriminator:
        propertyName: type
        mapping:
          woof: '#/components/schemas/Dog'
          dog: '#/components/schemas/Dog'
          cat: '#/components/schemas/Cat'
    Cat:
      allOf:
        - $ref: '#/components/schemas/Animal'
        - type: object
          properties:
            meows:
              type: boolean
    Dog:
      allOf:
        - $ref: '#/components/schemas/Animal'
        - type: object
          properties:
            barks:
              type: boolean