tapir icon indicating copy to clipboard operation
tapir copied to clipboard

[ENHANCEMENT] Setting names on Schema.oneOfWrapped schemas

Open andybryant opened this issue 2 years ago • 8 comments

Tapir version: 1.2.11 Scala version: 2.13.10

We have been using Schema.oneOfWrapped for endpoints that take a variety of inputs or generate more than one type of output. This has worked well, but the one issue has been the redocly OpenAPI docs just show 'Object' for each of the options, and there's no drop down to pick between the different types on the examples.

image

If we provide names for the generated schemas, the docs all work perfectly...

image

To generate the names in the screenshot above I modified the macro here https://github.com/softwaremill/tapir/blob/master/core/src/main/scala-2/sttp/tapir/internal/OneOfMacro.scala#L90-L92

to the following...

      val subclassesSchemas = subclasses.map(
        subclass =>
          q"${subclass.name.encodedName.toString} -> Schema.wrapWithSingleFieldProduct(implicitly[Schema[${subclass.asType}]])($conf).name(SName($conf.toEncodedName($conf.toDiscriminatorValue(implicitly[Schema[${subclass.asType}]].name.getOrElse(SName.Unit)))))"
      )

This sets the schema name to the encoded name from the provided configuration. To generate the version in the screenshot I provided an instance that splits up the words with a space

  implicit val config: Configuration = Configuration.default.copy(toEncodedName = _.replaceAll("([a-z])([A-Z]+)", "$1 $2"))

I'm not sure if I'm using the Configuration as you intended, or if adding the names by default has other issues, but this patch has been very useful for us and it would be great if a variation of it was included with Tapir.

andybryant avatar Apr 17 '23 00:04 andybryant

I realised after submitting this that the toEncodedName attribute of the Configuration is also used to format field names which is not good. You can see that on the screenshot above for "account Id".

It might need a separate configuration to override the name encoding. To have it the same as the discriminator field this works...

      val subclassesSchemas = subclasses.map(
        subclass =>
          q"${subclass.name.encodedName.toString} -> Schema.wrapWithSingleFieldProduct(implicitly[Schema[${subclass.asType}]])($conf).name(SName($conf.toDiscriminatorValue(implicitly[Schema[${subclass.asType}]].name.getOrElse(SName.Unit)))"
      )

andybryant avatar Apr 17 '23 05:04 andybryant

We are hitting the same problem with the rendering of oneOf fields in redoc. Are there already plans to include this proposal in tapir?

sebastianvoss avatar Nov 17 '23 01:11 sebastianvoss

@sebastianvoss yes this should be doable, but it would be best to have some input from which we could more easily create a test case - preferrably at the level of OpenAPI (yaml): what is the spec that is generated now, and what should be generated

adamw avatar Nov 17 '23 10:11 adamw

@adamw Thanks for your fast feedback. Here is a sample test case:

The test case fails because of the additional title fields for SubThingNumber and SubThingText. The render problem when using a spec without title fields can be reproduced be uploading the spec below without title fields to https://redocly.github.io/redoc/.

import io.circe.{Decoder, Encoder}
import io.circe.generic.extras.Configuration
import io.circe.generic.extras.semiauto.{
  deriveConfiguredDecoder,
  deriveConfiguredEncoder
}
import io.circe.generic.semiauto.{deriveDecoder, deriveEncoder}
import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.should
import sttp.tapir._
import sttp.tapir.docs.openapi.OpenAPIDocsInterpreter
import sttp.tapir.generic.auto._
import sttp.tapir.json.circe.jsonBody
import sttp.apispec.openapi.circe.yaml._

sealed trait SubThing

case class SubThingText(text: String) extends SubThing
case class SubThingNumber(number: Int) extends SubThing

case class Thing(field: SubThing)

object SubThing {
  implicit val config: Configuration =
    Configuration.default.withDiscriminator("type")

  implicit val encoder: Encoder[SubThing] = deriveConfiguredEncoder
  implicit val decoder: Decoder[SubThing] = deriveConfiguredDecoder
}

object Thing {
  implicit val encoder: Encoder[Thing] = deriveEncoder
  implicit val decoder: Decoder[Thing] = deriveDecoder
}

class SchemaSpec extends AnyFlatSpec with should.Matchers {
  "Tapir" should "generate schema with title" in {
    val e = endpoint
      .in("test")
      .out(jsonBody[Thing])

    val docs = OpenAPIDocsInterpreter().toOpenAPI(
      List(e),
      "",
      ""
    )
    val yaml = docs.toYaml

    yaml should be("""openapi: 3.1.0
                     |info:
                     |  title: ''
                     |  version: ''
                     |paths:
                     |  /test:
                     |    get:
                     |      operationId: getTest
                     |      responses:
                     |        '200':
                     |          description: ''
                     |          content:
                     |            application/json:
                     |              schema:
                     |                $ref: '#/components/schemas/Thing'
                     |components:
                     |  schemas:
                     |    SubThing:
                     |      oneOf:
                     |      - $ref: '#/components/schemas/SubThingNumber'
                     |      - $ref: '#/components/schemas/SubThingText'
                     |    SubThingNumber:
                     |      title: SubThingNumber
                     |      required:
                     |      - number
                     |      type: object
                     |      properties:
                     |        number:
                     |          type: integer
                     |          format: int32
                     |    SubThingText:
                     |      title: SubThingText
                     |      required:
                     |      - text
                     |      type: object
                     |      properties:
                     |        text:
                     |          type: string
                     |    Thing:
                     |      required:
                     |      - field
                     |      type: object
                     |      properties:
                     |        field:
                     |          $ref: '#/components/schemas/SubThing'
                     |""".stripMargin)
  }
}

sebastianvoss avatar Nov 17 '23 11:11 sebastianvoss

@sebastianvoss that's not exactly what is proposed in this issue, but you can specify the title in the schema, e.g.:

@title("abc")
case class SubThingNumber(number: Int) extends SubThing

// or using implicits

implicit val subThingTextSchema: Schema[SubThingText] = Schema.derived[SubThingText].title("xyz")

In the second case the implicit must be visible from the point where you reference the top-level schema, e.g. by invoking jsonBody[Thing].

Maybe this would be sufficient?

adamw avatar Nov 17 '23 16:11 adamw

@adamw You are right, I did not look close enough and the screenshots @andybryant added were showing the same problems we are facing.

Thanks a lot for proposing the solution, this is how we are doing it. Maybe it might be useful to automate this by using sttp.tapir.generic.Configuration. What do you think?

sebastianvoss avatar Nov 17 '23 17:11 sebastianvoss

@sebastianvoss so this would auto-generate the title property during automatic derivation? One worry that I have is that such auto-generated title properties wouldn't really carry much more information than names (as they would just have different formatting). One use-case is redoc+one-of, but that seems quite narrow.

adamw avatar Nov 17 '23 19:11 adamw

@adamw You are probably right. We will use the proposed solution to achieve the desired redoc UI rendering.

sebastianvoss avatar Nov 19 '23 23:11 sebastianvoss