swift-openapi-generator icon indicating copy to clipboard operation
swift-openapi-generator copied to clipboard

# Less verbose initialization of discriminated union types with associated values

Open Mrteller opened this issue 9 months ago • 12 comments

Motivation

While working with generated code for OpenAPI schemas that use discriminators (representing enums with associated values in Swift), I noticed that the current implementation requires redundant specification of the discriminator value when instantiating such objects.

Using the example from issue #515:


openapi: "3.1.0"
info:
  title: Action API
  version: "1.0.0"
  description: API for handling actions
servers:
  - url: https://api.example.com/v1
    description: Production server
paths:
  /actions:
    post:
      summary: Create a new action
      operationId: createAction
      requestBody:
        required: true
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Action'
      responses:
        '201':
          description: Action created successfully
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Action'
        '400':
          description: Invalid request payload
        '500':
          description: Internal server error
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

The generated initializers require specifying the discriminator value twice:

        let response = try await client.createAction(body: .json(.init(details: .bar(.init(value1: .init(_type: "bar"), value2: .init(bar: 123))))))

What is even worse is that second time you are able to pass something unrelated. While it is possible to limit that at OpenAPI spec with the help of enum and const that will require additional work.

Expected Behavior

Since the discriminator value is already known at the call site (we're using the .foo or .bar static function), it would be more ergonomic if we didn't need to specify it again in the internal initialization. Something like:

        let response = try await client.createAction(body: .json(.init(details: .bar(.init(bar: 123)))))

Steps to Reproduce

  1. Create a new Swift package
  2. Add the OpenAPI spec above as openapi.yaml
  3. Add swift-openapi-generator plugin to Package.swift
  4. Generate code using swift package generate-code-from-openapi
  5. Observe the generated initializers require redundant specification of the discriminator value

Swift Version

Swift version 5.9.2

Operating System

macOS 14.3

Proposed solution

It hard for me to figure if invasive changes are require to implement simplified `init() for such types, but that is how I see the solution at its best.

Alternatives considered

If for some reasons existing excessive init is needed, the generated convenience laconic init or init with default value for type param is probably OK too.

Additional information

This issue is related to how the generator handles discriminated unions that represent what would naturally be Swift enums with associated values. While the current implementation works, it introduces unnecessary verbosity that could be improved. It seems the issue is related to boxing and wrapping approaches that are used in the project.

Mrteller avatar Mar 21 '25 12:03 Mrteller

First off, thanks for filing this issue and providing a really clear reproducer and what you expected to see 🙏

I think this has happened because the discriminator is not in the schema itself but in an additional allOf with a common type that only holds the discriminator.

IIUC it's not necessary to specify the discriminator this way, it being declared as a discriminator in the oneOf is enough:

components:
  schemas:
    Action:
      type: object
      properties:
        details:
          $ref: '#/components/schemas/ActionDetails'
    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
      properties:
        bar:
          type: integer
          format: int32
      required:
        - bar
    ActionDetailsFoo:
      type: object
      properties:
        foo:
          type: string
      required:
        - foo

This results in code that does not require specifying the discriminator again:

        Components.Schemas.Action(details: .foo(.init(foo: "hello")))
        Components.Schemas.Action(details: .bar(.init(bar: 123)))

Which I believe matches your expected code?

simonjbeaumont avatar Mar 21 '25 18:03 simonjbeaumont

Does this work? If you encode the value, does it include the discriminator ("type") in the JSON payload?

czechboy0 avatar Mar 21 '25 18:03 czechboy0

@czechboy0 that's a good point. I was reading the example in Conditions for Using the Discriminator Object in the OAS which don't expressly show the discriminator property in the child schemas. I note it also says "To avoid redundancy, the discriminator MAY be added to a parent schema definition,". I guess, if we took the example in this issue, it would look like:

    Action:
      type: object
      required:
        - type
      properties:
        type:
          type: string

        details:
          $ref: '#/components/schemas/ActionDetails'
    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
      properties:
        bar:
          type: integer
          format: int32
      required:
        - bar
    ActionDetailsFoo:
      type: object
      properties:
        foo:
          type: string
      required:
        - foo

without the need for the common object too.

However, we're back to the issue posed, that the initializer requires you to manually specify the matching discriminator value. So my suggestion is likely incomplete (apologies @Mrteller!)

simonjbeaumont avatar Mar 21 '25 19:03 simonjbeaumont

Thanks a lot for the idea, @czechboy0. It looked very promising.

Does this work? If you encode the value, does it include the discriminator ("type") in the JSON payload?

Unfortunately it does not. While it might work for decoding (still not tested) it skips `discriminator's propertyName when encoding.

While the proposed way of structuring OpenAPI specification indeed results in laconic and logical type generation, I wonder how canonical it is. I would have never figured out that myself. All references that I've seen imply the presence of discriminator as a property of a payload scheme in one way or another. The references I mainly used are: Polymorphism section on swagger.io and Cat, Dog, Lizard samples from OpenAPI-Specification/versions /3.1.1.md). The latter says:

propertyName string REQUIRED. The name of the property in the payload that will hold the discriminating value. This property SHOULD be required in the payload schema, as the behavior when the property is absent is undefined.

Mrteller avatar Mar 22 '25 18:03 Mrteller

@simonjbeaumont , your suggestion could help if I was in control to the API. But I'm just trying to describe it in terms of OpenAPI spec.

Mrteller avatar Mar 22 '25 18:03 Mrteller

...I wonder how canonical it is. I would have never figured out that myself. All references that I've seen imply the presence of discriminator as a property of a payload scheme in one way or another.

I agree. Most of the examples in the spec are incomplete in that they are not clear where the discriminator field is defined. Either they are incomplete schemas for illustration purposes and the discriminator should be implied to be a field within the oneOf child schemas, or it's implied that the discriminator is in the parent schema of the oneOf.

Your citation above is useful: it states that the discriminator should be listed as a required field. There's another section that has some useful bits too:

4.8.25.2 Conditions for Using the Discriminator Object

The Discriminator Object is legal only when using one of the composite keywords oneOf, anyOf, allOf.

In both the oneOf and anyOf use cases, where those keywords are adjacent to discriminator, all possible schemas MUST be listed explicitly.

To avoid redundancy, the discriminator MAY be added to a parent schema definition, and all schemas building on the parent schema via an allOf construct may be used as an alternate schema.

The allOf form of discriminator is only useful for non-validation use cases; validation with the parent schema with this form of discriminator does not perform a search for child schemas or use them in validation in any way. This is because discriminator cannot change the validation outcome, and no standard JSON Schema keyword connects the parent schema to the child schemas.

The behavior of any configuration of oneOf, anyOf, allOf and discriminator that is not described above is undefined.

My read here is that there are two valid places for the discriminator to be. Within the child schemas of the oneOf or in a parent schema.

I'd imagine that generating more ergonomic initialisers for the first would be easier. But if we'd want to be able to generate overloads for any type that contains the discriminator field in some ancestor, I think that'd be much harder.

Imagine the following:

    Outer:
      type: object
      required: [outer, inner, whichInner]
      properties:
        outer:
          type: string
        inner:
          $ref: '#/components/schemas/Inner'
        whichInner:
          type: string
    Inner:
      oneOf:
        - $ref: '#/components/schemas/Inner1'
        - $ref: '#/components/schemas/Inner2'
      discriminator:
        propertyName: whichInner # this it provided in Outer
        mapping:
          Inner1: '#/components/schemas/Inner1'
          Inner2: '#/components/schemas/Inner2'
    Inner1:
      type: object
      properties:
        inner1:
          type: string
    Inner2:
      type: object
      properties:
        inner2:
          type: integer

In this example, the discriminator is defined in Inner but provided in the parent, in Outer.

Here we could imagine that instead of generating Outer.init(outer: String, inner: Inner, whichInner: String) we could generate only Outer.init(outer: String, inner: Inner) and use the discriminator mapping to set the Outer.whichInner property.

This will require us to check every property in a schema to see if it is used as a discriminator for another property.

However, I'm not sure how we can extend this to support it being in any parent schema. For example:

    A:
      type: object
      required: [a, b, whichC]
      properties:
        a:
          type: string
        b:
          $ref: '#/components/schemas/B'
        whichC:
          type: string
    B:
      type: object
      required: [b, c]
      properties:
        b:
          type: string
        c:
          $ref: '#/components/schemas/C'
    C:
      oneOf:
        - $ref: '#/components/schemas/C1'
        - $ref: '#/components/schemas/C2'
      discriminator:
        propertyName: whichC  # this is way back in A
        mapping:
          C1: '#/components/schemas/C1'
          C2: '#/components/schemas/C2'
    C1:
      type: string
    C2:
      type: integer

Note that A.whichC determines the value of B.c. I think from the reading of the MAY in the spec, that this is legal.

The ask here is that A.init doesn't take a whichC parameter that users have to get right.

The challenge is that, at the definition of A, we don't know that whichC is used as a discriminator. So we'd need to descend into all children (breaking cycles) and work out if any of its properties is used as a discriminator at any depth.

Even then, I'm not sure what we should generate in that case since we have B in the middle.

  • Should C still be represented as an enum that can be constructed on its own, since the parent holding the discriminator is elsewhere?
  • What should the initializers for B look like, since they hold a C but do not hold the discriminator?
  • What should the initializers for A look like, since we don't have the enum cases at the next depth? Should we promote all of B's initializer parameters into parameters for A.init? If we, did, how deep would we support?

Maybe we could add some value by supporting just one layer of depth?

simonjbeaumont avatar Mar 24 '25 10:03 simonjbeaumont

I think the key to getting ahead is to narrow the number of cases covered to as few as possible unambiguous cases. Discriminator checking is at least in some form possible in the oneOf' case. The current version of the generator also only supports the oneOf` option for the discriminator so far. So I support the attention being paid to this form of syntax. If there is one way to express the API, the lack of support for all discriminator varieties can easily be overcome.

My understanding of OpenAPI concept

It is our responsibility to include the discriminator field in the encoding somehow by explicitly describing it in the OpenAPI schema. Looks like the most consistent place for the discriminator is in some wrapper, some outer schema that should be used mainly for requests. Let me switch back to Action sample.

The proposition on handling frequent pattern with structure of the object depends on the discriminator filed in it

The way to describe cases in question

openapi: "3.1.1"
info:
  title: Sample API that shows discriminator usage 
  version: "1.0.0"
  description: Create objects which types depend on discriminator propertyName consistently and decode them as enums with associated values.
servers:
  - url: https://api.example.com/v1
    description: Example server
paths:
  /laconic-actions:
    post:
      summary: Create a new action with laconic response
      operationId: createActionWithLaconicResponse
      requestBody:
        required: true
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Action'
      responses:
        '201':
          description: Action created successfully
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/ActionDetails'
components:
  schemas:
    Action:
      allOf:
        - type: object
          required: [whichAction]
          properties:
            whichAction:
              type: string
              enum:
                - fooType
                - barType
        - $ref: '#/components/schemas/ActionDetails'
    ActionDetails:
      oneOf:
        - $ref: '#/components/schemas/ActionDetailsFoo'
        - $ref: '#/components/schemas/ActionDetailsBar'
      discriminator:
        propertyName: whichAction
        mapping:
          fooType: '#/components/schemas/ActionDetailsFoo'
          barType: '#/components/schemas/ActionDetailsBar'
    ActionDetailsBar:
      type: object
      properties:
        bar:
          type: integer
          format: int32
      required:
        - bar
    ActionDetailsFoo:
      type: object
      properties:
        foo:
          type: string
      required:
        - foo

In this case Action ensures that the discriminator is present in objects we encode. For decoding (for responses) it is more convenient to use the ActionDetails schema since the discriminator info can be inferred from the type. There is no duplication, since ActionDetails should exist anyway. But according to OpenAPI concept, the discriminator schema is not responsible neither for holding neither for validation of discriminator property. Therefore if you try to encode with ActionDetails, you'll get exactly what it describes: one of schemas that may or may not contain a property with a name that matches discriminator. It is better to think of discriminator as an encoding-only related thing.

The way it reflected n generated code

        // This request uses `ActionDetails` schema for response which should exist in OpenAPI spec anyway in order to create
        // `Action` types for requests that are guaranteed to encode discriminator.
        // But we can give that `Action` schema convenience `init(_ value2:)`.
        let laconicResponse = try await client.createActionWithLaconicResponse(body: .json(.init(.fooType(.init(foo: "abc")))))
        let laconicAction = try laconicResponse.created.body.json
        switch laconicAction {
        case .barType(let barValue):
            print(barValue)
        case .fooType(let fooValue):
            print(fooValue)
        // Is server gives us such misformatted response, decoding exception will be raised.
        }

Syntax sugar added without interfering current implementation:

extension Components.Schemas.Action {
    init(_ value2: Components.Schemas.ActionDetails) {
        self.value2 = value2
        // If auxiliary types were generated it could be easier
        // self.value1 = .init(whichAction: .init(rawValue: value2.discriminator.rawValue))
        // OR:
        // self.value1 = .init(whichAction: value2.discriminator)
        switch value2 {
        case .barType:
            self.value1 = .init(whichAction: .barType)
        case .fooType:
            self.value1 = .init(whichAction: .fooType)
        }
    }
}

But such convenience could be added better at generation phase since generator has access to discriminator values.

The sample for the proposed solutions is here.

This approach is based on the following explanation from https://swagger.io/specification/#discriminator-object BTW, this is the most thorough info on discriminator I've come across.

A discriminator MAY be used as a "hint" to improve the efficiency of selection of the matching schema. The discriminator field cannot change the validation result of the oneOf, it can only help make the deserialization more efficient and provide better error messaging. We can specify the exact field that tells us which schema is expected to match the instance

Mrteller avatar Mar 27 '25 14:03 Mrteller

A bit unrelated note, @simonjbeaumont. I like the laconic syntax of A, B, C sample from your answer and it passes all validations and even gets Swift Client and Types code generated. But generated code doesn't compile on my side.

Mrteller avatar Mar 27 '25 14:03 Mrteller

To clarity a couple of things with regard to A, B, C sample.

Should C still be represented as an enum that can be constructed on its own, since the parent holding the discriminator is elsewhere?

I think yes, definitely. All code generators and other tools expect such entities to exist explicitly. Moreover: types generated from these schemas are convenient for decoding responses right into enums in the current implementation.

What should the initializers for B look like, since they hold a C but do not hold the discriminator? What should the initializers for A look like, since we don't have the enum cases at the next depth? Should we promote all of B's initializer parameters into parameters for A.init? If we, did, how deep would we support?

Not sure any changes should be introduced for such a case for now. While it could be validated (and it would be reasonable) even recursively, it would be ahead of what's OpenAPI 3.1.1 states for now and more strict than any existing schema validators. But some kind of clear warnings during code generation could be very welcome. If, over time, the OpenAPI standard enforces validation for this, these warnings could be turned into errors.

What should the initializers for A look like, since we don't have the enum cases at the next depth?

I think any object that contains objects with discriminator if they as well contain discriminator property at the same level (by means of allOf or explicitly) should receive (convenience?) init which I tried to elaborate. There could be different ways of implementing coupling propertyName string enum with discriminator driven enum with associated value. Maybe in this case we should hide init which allows to construct invalid object by providing inconsistent discriminator. But the fact that discriminator is actually a String enum that should exactly match oneOfs cases could be checked and utilized.

Mrteller avatar Mar 30 '25 16:03 Mrteller

One more thought on this point:

The challenge is that at the definition of A, we don't know whichC is used as a discriminator. So we'd have to descend into all children (breaking cycles) and find out if any of its properties is used as a discriminator at any depth.

Yes this is the case with oneOf use of the discriminator.

It seems that with the `allOf' use of the discriminator, all we have to do is check that the parent schema contains the necessary property. Again, there is usually no immediate need to try to handle cases where child properties could be mixed with other schema properties, thus satisfying the discriminator requirement. If we discard such rare and unusual cases, then I don't see any search loops to emerge.

Mrteller avatar Apr 01 '25 08:04 Mrteller

I came across this problem just now. I am trying to create one of the discriminated unions that are declared in the OpenAPI schema, and now I have to specify the string value of the discriminator in my code.

Image

I think my problem can be considered separately for discriminated unions only and doesn't need to solve the entire problem space (which is harder for sure). The discriminator should be omitted, and if the discriminator is the only property, then the enum should have no associated value at all.

tkrajacic avatar Oct 20 '25 10:10 tkrajacic

Thanks @tkrajacic - also adding more details from our conversation, about how even the following simple example also isn't easy to solve for:

MyType:
  oneOf:
    - $ref: '#/components/schemas/A'
    - $ref: '#/components/schemas/B'
  discriminator:
    propertyName: kind
A:
  type: object
  properties:
    kind:
      type: string
  required:
    - kind
B:
  type: object
  properties:
    kind:
      type: string
  required:
    - kind

The question is: what do the initializers of A and B look like?

One way to make the ergonomics of at least MyType better here would be to essentially add helper static functions on it that allow you to create A and B, where all the properties would be brought over except the discriminator.

This should be doable, and a contribution would be welcome here - we can provide guidance.

czechboy0 avatar Oct 21 '25 09:10 czechboy0