softwaremill / sttp-apispec

OpenAPI, AsyncAPI and JSON Schema Scala models.
Apache License 2.0
23 stars 11 forks source link

AsyncAPI: Discriminator not valid #43

Open ccarlile opened 1 year ago

ccarlile commented 1 year ago

When rendering an enum schema, AsyncAPI.toYaml returns something like this:

components:
  schemas:
    MyEvent:
      oneOf:
      - $ref: '#/components/schemas/MyEvent1'
      - $ref: '#/components/schemas/MyEvent2'
      - $ref: '#/components/schemas/MyEvent3'

      discriminator:
        propertyName: eventType
        mapping:
          MyEvent1: '#/components/schemas/MyEvent1'
          MyEvent2: '#/components/schemas/MyEvent2'
          MyEvent3: '#/components/schemas/MyEvent3'
AsyncAPI Studio says this is an error, and the spec says: field type description
discriminator string Adds support for polymorphism. The discriminator is the schema property name that is used to differentiate between other schema that inherit this schema. The property name used MUST be defined at this schema and it MUST be in the required property list. When used, the value MUST be the name of this schema or any schema that inherits it. See Composition and Inheritance for more details.

Changing the outputted yaml to the following rendered as expected:

components:
  schemas:
    MyEvent:
      oneOf:
      - $ref: '#/components/schemas/MyEvent1'
      - $ref: '#/components/schemas/MyEvent2'
      - $ref: '#/components/schemas/MyEvent3'

      discriminator: eventType
ccarlile commented 1 year ago

I ended up using a lens to remove the discriminator. Would you be open to accepting a PR with something like this?

import monocle._

type ReferenceOrSchema = ReferenceOr[Schema]

val getComponents: Optional[AsyncAPI, Components] =
  Optional.apply[AsyncAPI, Components](_.components)(comp =>
    api => api.copy(components = Some(comp)))

val getReferenceOrSchema: Optional[Components, ListMap[String, ReferenceOrSchema]] =
  GenLens[Components].apply(_.schemas).asOptional

implicit val lmeRos: Each[ListMap[String, ReferenceOrSchema], ReferenceOrSchema] = Each.listMapEach

val getDiscriminator: Traversal[sttp.apispec.Schema, Option[Discriminator]] =
  GenLens[Schema].apply(_.discriminator).asTraversal

val getSchema: Traversal[ReferenceOrSchema, Schema] =
  Optional.apply[ReferenceOrSchema, Schema](_.toOption)(schema => _ => Right(schema)).asTraversal

val allSchemas =
  getComponents
    .andThen(getReferenceOrSchema)
    .each[ReferenceOrSchema]
    .andThen(getSchema)

def removeDiscriminator(docs: AsyncAPI): AsyncAPI =
  allSchemas
    .andThen(getDiscriminator)
    .modify(_ => None)(docs)

removeDiscriminator(docs)
adamw commented 1 year ago

I think the proper fix here would be to fix the serialiser to properly represent polymorphism. It seems that the way discriminators are represented in schemas diverge from OpenAPI. Currently this is modelled as follows: https://github.com/softwaremill/sttp-apispec/blob/1d9fb42ef94280c9c9cfdbe961c7317a40173b50/apispec-model/src/main/scala/sttp/apispec/Schema.scala#L48, so we require a Discriminator object which has a propertyName. This should become an Either[String, Discriminator], but then would a "sole" property name be a proper discriminator for OpenAPI?

But once we have that, the proper place to transform the schemas would probably be here: https://github.com/softwaremill/tapir/blob/e1079258720b81420452a1a5ad6868f3d84009ac/docs/asyncapi-docs/src/main/scala/sttp/tapir/docs/asyncapi/EndpointToAsyncAPIComponents.scala#L16

For the time being, you workaround is good of course :) But I think I'd prefer to fix the "root cause" if possible rather than include it in the sources.

Btw. if it's an enum, shouldn't this be rendered as a normal property with an enum validator? Can you share some example code through which you get the above result?