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

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

Closed
JanC opened this issue Jan 25, 2022 · 0 comments · Fixed by #298
Closed

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

JanC opened this issue Jan 25, 2022 · 0 comments · Fixed by #298

Comments

@JanC
Copy link
Contributor

JanC commented Jan 25, 2022

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

case "Cat":
self = .cat(try Cat(from: decoder))
case "Dog":
self = .dog(try Dog(from: decoder))
case "cat":
self = .cat(try Cat(from: decoder))
case "dog":
self = .dog(try Dog(from: decoder))
case "woof":
self = .dog(try Dog(from: decoder))

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
@JanC JanC changed the title Too many cases generated when discriminator has an explicit mapping Too many cases generated when oneOf discriminator explicit mappings Jan 25, 2022
@JanC JanC changed the title Too many cases generated when oneOf discriminator explicit mappings Too many cases generated when oneOf discriminator has explicit mappings Jan 25, 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 a pull request may close this issue.

1 participant