softwaremill / tapir

Rapid development of self-documenting APIs
https://tapir.softwaremill.com
Apache License 2.0
1.34k stars 405 forks source link

Recursive Definitions OpenAPI generation #354

Closed jmpicnic closed 2 years ago

jmpicnic commented 4 years ago

First, thanks for putting together a strong foundation for defining API's from the Scala Type system. It has been a great help already.

As I have used the package more, I was trying to build a schema to describe flexible filters in the body and I am running into some oddities in the Yaml OAS generation. I am currently using 0.11.9, but from the release notes, I don't think it will be different in 0.12.3 (planning to upgrade soon)

I have boiled down the issues I am seeing to this sample test:

object SimpleTest extends App {
    sealed trait Term
    sealed trait Literal extends Term
    case class I(v: Long)
    case class D(v: Double)
    case class S(v: String)

    sealed trait Reference extends Term
    case object IRef extends Reference
    case object DRef extends Reference
    case object SRef extends Reference

    sealed trait Op
    case object Eq extends Op
    case object Lt extends Op
    case object Gt extends Op

    sealed trait Compose
    case object And extends Compose
    case object Or extends Compose

    sealed trait Clause
    case class Expression(op: Op, l: Reference, r: Term) extends Clause
    case class Not(not: Clause) extends Clause
    case class Composite(op: Compose, over: List[Clause]) extends Clause

    import tapir._
    import io.circe.generic.auto._
    import io.circe.{Decoder, Encoder}
    import tapir.json.circe.encoderDecoderCodec

    private lazy val body =   jsonBody[Clause]

    val endpointDef =
        endpoint.post
            .in(body)
            .out(jsonBody[String].description("OK"))
            .errorOut(jsonBody[String].description("An error occurred"))

    import tapir.docs.openapi._
    import tapir.openapi.OpenAPI
    import tapir.openapi.circe.yaml._
    val oas: OpenAPI = (endpointDef :: Nil).toOpenAPI("FILTER ENDPOINT", "1.0.0-SNAMPSHOT")
    val schema = oas.toYaml
    println(s"Schema:\n$schema")
}

The generated Yaml is:

Schema:
openapi: 3.0.1
info:
  title: FILTER ENDPOINT
  version: 1.0.0-SNAMPSHOT
paths:
  /:
    post:
      operationId: postRoot
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Clause'
        required: true
      responses:
        '200':
          description: OK
          content:
            application/json:
              schema:
                type: string
        default:
          description: An error occurred
          content:
            application/json:
              schema:
                type: string
components:
  schemas:
    Or:
      type: object
    Term:
      oneOf:
      - $ref: '#/components/schemas/IRef'
      - $ref: '#/components/schemas/DRef'
      - $ref: '#/components/schemas/SRef'
    Not:
      required:
      - not
      type: object
      properties:
        not:
          $ref: '#/components/schemas/Clause'
    Expression:
      required:
      - op
      - l
      - r
      type: object
      properties:
        op:
          $ref: '#/components/schemas/Op'
        l:
          $ref: '#/components/schemas/Reference'
        r:
          $ref: '#/components/schemas/Term'
    Clause:
      oneOf:
      - $ref: '#/components/schemas/Expression'
    Reference:
      oneOf:
      - $ref: '#/components/schemas/IRef'
      - $ref: '#/components/schemas/DRef'
      - $ref: '#/components/schemas/SRef'
    IRef:
      type: object
    DRef:
      type: object
    Op:
      oneOf:
      - $ref: '#/components/schemas/Eq'
      - $ref: '#/components/schemas/Lt'
      - $ref: '#/components/schemas/Gt'
    Eq:
      type: object
    Compose:
      oneOf:
      - $ref: '#/components/schemas/And'
      - $ref: '#/components/schemas/Or'
    Gt:
      type: object
    Composite:
      required:
      - op
      - over
      type: object
      properties:
        op:
          $ref: '#/components/schemas/Compose'
        over:
          type: array
          items:
            $ref: '#/components/schemas/Clause'
    And:
      type: object
    SRef:
      type: object
    Lt:
      type: object

The things that are stumping me are:

  1. The Clause schema only lists Expression as a one of its options, I would expect to see Expression, Composite, Not
  2. The schema for Term should be oneOf with options Reference and Literal instead of repeating the Reference Options and skipping Literal altogether.
  3. I would expect the case object types to be translated as a string schemas with an enum constraint instead of unqualified objects

I have tried to provide some hints using the custom types descriptions in the docs but with not much success. The results vary a bit, but I am not able to get them to line up, in particular #1, & #2

Could you give me some guidance on how to give enough information to be able to get the schema (and hopefully then validators, encoders and decoders) to generate the code I am expecting? Or may be I am missing something in the way I am approaching the problem.

Thanks a lot for your help.

Miguel

PS. Please feel free to use the example in any way it may be useful for unit test, docs, or anything else.

adamw commented 4 years ago
  1. is fixed in 0.12.9
  2. when automatically deriving a schema, we only get the leaf implementations from Magnolia, not the intermediary traits. That's a design decision and I think the right one. Here, the traits form a nice tree structure, but in general this can be an arbitrary graph. You can have other traits which are added to some, otherwise non-related implementations. How could Magnolia (or tapir) know which traits are the "main" ones? (btw. literals are not in the schema as your case classes don't extend Literal :) )
  3. again, that a consequence of how Magnolia works, but I also think it makes the right choice here. In this example, the case objects indeed form an enumeration. But in general, you could have a case object, plus some case classes, which would form a hierarchy. So you would need to special case the only-case-object scenario

To "fix" 2&3, I think the right answer is to write down the problematic parts of the schema by hand. Automatic derivation has its limits :)

In your case, I think you would need to define:

implicit opSchema: Schema[Op] = implicitly[Schema[String]]
implicit opValidator: Validator[Op] = Validator.enum[Op]

// similar for compose

implicit val termSchema: Schema[Term]  = Schema(SCoproduct(...))
gbastkowski commented 4 years ago

I believe I have a similar issue, although I'm not quite sure if this is completely the same thing. My data structure:

case class AnimalHolder(value: Animal)

sealed trait Animal

case class Boar(weight: Int) extends Animal

case class Cat(name: String, weight: Int) extends Animal

I'm using circe with semi-automatic codec derivation My endpoint definition:

  def post: Endpoint[AnimalHolder, Unit, Unit, Nothing] = endpoint
    .post
    .in(jsonBody[AnimalHolder])
    .out(statusCode(Created))

Generating an OpenAPI YAML leads to the following:

    Animal:
      oneOf:
      - $ref: '#/components/schemas/Boar'
      - $ref: '#/components/schemas/Cat'
    Boar:
      allOf:
      - $ref: '#/components/schemas/Animal'
      - required:
        ...
    Cat:
      allOf:
      - $ref: '#/components/schemas/Animal'
      - required:
        ...

Both, SwaggerUI and ReDoc, have trouble visualizing this schema and I wonder if it shouldn't look more like

    Cat:
      required:
      ...

I'd appreciate any hints on how I can either change my model/circe derivation or the OpenAPI schema derivation to remove this recursion.

adamw commented 4 years ago

@gbastkowski your definition isn't recursive, but the way of generating the schema that you mention was changed in https://github.com/softwaremill/tapir/issues/308. What kind of problems does this cause with SwaggerUI?

adamw commented 4 years ago

@gbastkowski ah I ran this locally and I can see this problem; let's move to #308 and discuss further there

gbastkowski commented 4 years ago

This is the SwaggerUI rendering:

Boar {
      description:    the value
      weight*         integer
      oneOf ->        {
                      }
                      Cat {
                        name*   string      Cats are pets and therefore should have a name
                        weight* integer
                      }
}
Cat {
      description:    the value
      name*       string                Cats are pets and therefore should have a name
      weight*         integer
      oneOf ->        Boar {
                        weight* integer
                      }
                      {
                      }
}
adamw commented 2 years ago

Closing as this is partly fixed, partly expected