[Bug] oneOf without encapsulated schema generates noncompilable code

[alexdremov]

Consider such structure:

  /path:
    get:
      summary: Some summary
      security:
        - APIAccess: []
      operationId: pathRequest
      parameters: []
      responses:
        '200':
          description: Some response
          content:
              application/json:
                  schema:
                      oneOf:
                          - $ref: "#/components/schemas/Message"
                          - $ref: "#/components/schemas/Status"

So, response with 200 code can be either Message or Status schema. Such structure results in this code:

extension API {
    /** Some summary */
    public enum PathRequest {
    ...
        public enum Response: APIResponseValue, CustomStringConvertible, CustomDebugStringConvertible {
            public typealias SuccessType = Status200

            /** Some response */
            case status200(Status200)

            public var success: Status200? {
                switch self {
                case .status200(let response): return response
                }
            }
...
        }
    }
}

Which results in compile error Cannot find type 'Status200' in scope – that's true it was not defined.

I solved this problem by encapsulating oneOf inside custom schema:

schemas:
    Entity:
        oneOf:
            - $ref: "#/components/schemas/Message"
            - $ref: "#/components/schemas/Status"

However, I spent some time figuring out how to overcome this bug. I expected framework to automatically generating such schema.

[yonaskolb]

I think this is due to the regression that https://github.com/yonaskolb/SwagGen/pull/269 fixed. I'll look at getting a release out

[alexdremov]

Hope so. Thanks!

[yonaskolb]

That version is now released. Could you test 4.5.0 and report back?

[alexdremov]

Nope, it did not solve the issue. I checked the version, it's 4.5.0, but the problem still persists.

Minimal complete example that reproduces this error:

openapi: 3.0.0
info:
  version: 1.0.0
  title: Example
paths:
  /path:
    get:
      summary: Some summary
      operationId: pathRequest
      parameters: []
      responses:
        '200':
          description: Some response
          content:
              application/json:
                  schema:
                      oneOf:
                          - $ref: "#/components/schemas/APIMessage"
                          - $ref: "#/components/schemas/APIStatus"
components:
  schemas:
    APIStatus:
      type: object
      required: [status]
      properties:
        status:
          type: string
    APIMessage:
      type: object
      required: [message]
      properties:
        message:
          type: string

[yonaskolb]

@liamnichols you’ve delved into the context here recently. Any thoughts?

[liamnichols]

Hmmm this is an interesting one... I don't think its related to the other recent changes since they were focused around when only a single item exists in the group of anyOf whereas in this case two items exist.

@AlexRoar: When you define the Entity schema and it works, does the generated Entity code look/behave correctly? I'm not completely familiar with the output that we are expecting.

In the minimal example above, does it generate any models beyond APIMessage and APIStatus? I know that in the original example it was trying to reference a non-existent Status200 struct but i'm wondering if the problem is that its generating a model that is not called Status200 of if its not generating a model at all?

[alexdremov]

@liamnichols it generates APIMessage and APIStatus models only. There's no file that somehow represents oneOf logic

[alexdremov]

Tested code generated with Entity schema. It's even worser. It compiles but does not work correctly.

openapi: 3.0.0
info:
  version: 1.0.0
  title: Example
paths:
  /path:
    get:
      summary: Some summary
      operationId: pathRequest
      parameters: []
      responses:
        '200':
          description: Some response
          content:
              application/json:
                  schema:
                    $ref : "#/components/schemas/APIOneOf"

components:
  schemas:
    APIOneOf:
         oneOf:
            - $ref: "#/components/schemas/APIMessage"
            - $ref: "#/components/schemas/APIStatus"
    APIStatus:
      type: object
      required: [status]
      properties:
        status:
          type: string
    APIMessage:
      type: object
      required: [message]
      properties:
        message:
          type: string

It throws an error in Entity model file – I named it APIOneOf:

public enum APIOneOf: Codable, Equatable {
    case aPIMessage(APIMessage)
    case aPIStatus(APIStatus)

    public init(from decoder: Decoder) throws {
        let container = try decoder.container(keyedBy: StringCodingKey.self)
        let discriminator: String = try container.decode("")     <<<<<<<<<<<<<<**HERE**
        switch discriminator {
        case "APIMessage":
            self = .aPIMessage(try APIMessage(from: decoder))
        case "APIStatus":
            self = .aPIStatus(try APIStatus(from: decoder))
        default:
            throw DecodingError.dataCorrupted(DecodingError.Context.init(codingPath: decoder.codingPath, debugDescription: "Couldn't find type to decode with discriminator \(discriminator)"))
        }
    }

    public func encode(to encoder: Encoder) throws {
        var container = encoder.singleValueContainer()
        switch self {
        case .aPIMessage(let content):
            try container.encode(content)
        case .aPIStatus(let content):
            try container.encode(content)
        }
    }
}

container.decode("") does not look right at all

[alexdremov]

Adding discriminator property to the specification helped, but this is an undocumented requirement of this tool. Looking forward to see this in README at least.

Still, the problem that I described in the very first message is relevant.

Possible unexpected behaviour place: generated code makes additional assumptions about discriminator value:

        switch discriminator {
        case "APIMessage":
            self = .aPIMessage(try APIMessage(from: decoder))
        case "APIStatus":
            self = .aPIStatus(try APIStatus(from: decoder))
        case "message":
            self = . aPIMessage(try APIMessage(from: decoder))
        case "status":
            self = . aPIStatus(try APIStatus(from: decoder))
        }

I did not mapped APIMessage type value to the APIMessage model. Yet, this mapping is here.

Feature request:

Also, as discriminator value is not mandatory in the OpenAPI standard, It's good to add some other discrimination method. For example, checking if object has all required fields for one of oneOf models. Even more optimised – identifying model by Set(model properties) /exclude \union (Set(all other models)).

[liamnichols]

as discriminator value is not mandatory in the OpenAPI standard

I think it's not mandatory because you don't need it if you're just performing validation or doc generation. The discriminator is only helpful/useful for API consumers (like SwagGen 😄)

For example, checking if object has all required fields for one of oneOf models. Even more optimised – identifying model by Set(model properties) /exclude \union (Set(all other models)).

In this specific use case this would be handy but it very easily falls apart. For example, what if there are nullable properties in the equation? You wouldn't be able to discriminate reliably based on properties alone if one or more of them aren't guaranteed to be returned. This could also get pretty tricky in terms of how the code would actually be generated since SwagGen produces the Model.swift files using a Stencil template making it hard to account for a lot of conditional and dynamic logic if multiple properties were involved.

---

In your particular example, do you have control over the schema or the API itself? If you have control over both, I would suggest introducing a discriminator property on both responses but if you only have control over the schema then it might be better to describe this as a single schema with two nullable properties.

I'd also be interested to see how other tools like openapi-generator generate code for this kind of schema. I'll admit that I haven't used OpenAPI for long

[alexdremov]

Thanks for your suggestions! I stopped using openapi-generator as it uses old Alamofire :)

I have control over both, so it's really not a problem.

[b00tsy]

discriminator works as advertised for me: https://swagger.io/docs/specification/data-models/inheritance-and-polymorphism/

[alexdremov]

discriminator works as advertised for me: https://swagger.io/docs/specification/data-models/inheritance-and-polymorphism/

I believe that it can be closed now. Just good to know that such issue can appear and there is a way to fix it