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

OpenAPI schema for union types is missing the discriminator level. #3600

Open
kamilkloch opened this issue Mar 13, 2024 · 3 comments
Open

OpenAPI schema for union types is missing the discriminator level. #3600

kamilkloch opened this issue Mar 13, 2024 · 3 comments

Comments

@kamilkloch
Copy link
Contributor

kamilkloch commented Mar 13, 2024

Repo with code: https://github.com/kamilkloch/tapir-union-types-schema

sealed trait Fruit

object Fruit {
  case class Apple(color: String) extends Fruit

  case class Potato(weight: Double) extends Fruit

  private implicit val config: Configuration = Configuration.default
  implicit val fruitCodec: io.circe.Codec[Fruit] = deriveConfiguredCodec
}

Generated OpenApi YAML is incorrect - it is missing the discriminant level. An Apple (from Fruit trait perspective) is of shape

{ 
  "Apple" : { 
    "color": "string"
  }
}

, and not

{ 
  "color": "string"
}

Generated OpenApi YAML:

openapi: 3.1.0
info:
  title: linguistic-nightingale
  version: 1.0.0
paths:
  /fruit:
    post:
      operationId: postFruit
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Fruit'
        required: true
      responses:
        '200':
          description: ''
        '400':
          description: 'Invalid value for: body'
          content:
            text/plain:
              schema:
                type: string
components:
  schemas:
    Apple:
      required:
      - color
      type: object
      properties:
        color:
          type: string
    Fruit:
      oneOf:
      - $ref: '#/components/schemas/Apple'
      - $ref: '#/components/schemas/Potato'
    Potato:
      required:
      - weight
      type: object
      properties:
        weight:
          type: number
          format: double
@adamw
Copy link
Member

adamw commented Mar 13, 2024

As I understand the problem is that there's a mismatch between the generated schema, and the way an object is serialised to JSON. As tapir doesn't know what's your JSON library of configuration, you have to configure it separately (unless you're using pickler).

So in this case, you'll have to provide the schemas by hand, see: https://tapir.softwaremill.com/en/latest/endpoint/schemas.html#wrapper-object-discriminators

@kamilkloch
Copy link
Contributor Author

kamilkloch commented Mar 14, 2024

Thank you for pointing to https://tapir.softwaremill.com/en/latest/endpoint/schemas.html#wrapper-object-discriminators.

Schema.oneOfWrapped takes an implicit sttp.tapir.generic.Configuration, but (understandably) ignores its .withDiscriminator setting. On the other hand, Schema.derivedSchema does not ignore the .withDiscriminator setting. This means, that the schema configuration logic is split in 2 places: sttp.tapir.generic.Configuration and invocation of either Schema.oneOfWrapped or Schema.derivedSchema.

It looks like the most 2 common semi-automatic schema types (and Json codecs) for coproducts are

  1. wrapper object discriminators, or
  2. field dicriminators.

Arguably, the default tapir schema (no wrapper object discriminators, no field discriminators) does not reflect the default for most common codecs (circe and jsoniter-scala for sure).

Proposition: Schema.oneOfWrapped behavior could be the default behavior for union types (like in circe), changeable to discriminator fields via .withDiscriminator setting in the Configuration. Schema.oneOfWrapped method could be removed altogether, Schema.derivedSchema could be used instead, tapping into the single source of truth via implicit Configuration.

@adamw
Copy link
Member

adamw commented Mar 14, 2024

I'm afraid we can't really change the default strategy for schemas generation for coproducts - even though binary compatible, it would be a breaking change in the core module. Although, I agree that the current situation is not perfect - but I think we'll have to resort to having this properly documented, at least in the 1.x versions.

The situation is different with pickler, where we are free to define the configuration and serialization. That's still very much in the works (cc @kciesielski), so maybe there we can improve basing on your input.

(btw. the defaults on how coproducts are serialised are different for each json library, so it's hard to pick a good candidate ;) but that also would be fixed by pickler, which derives both json codecs & schemas in one go)

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

No branches or pull requests

2 participants