Tapir-redoc annotations not working as expected

tapir
1

Hello Guys,

Something about Tapir annotations with Redoc is not working as I would expect:

When annotating attributes with @Schema.annotation.deprecated it mostly gets picked up.

But when a case class is used for another attribute, it switches to using a $ref to a Schema. Which makes sense for the case class structure, but not for the deprecation.
Which will then have all attributes of the type deprecated or none of them!?

Eg in this SSCCE based on zip generatd with adopt-tapir.softwaremill.com:

  case class Book(
      title: String,
      year: Int,
      a1: CcA,
      a2: CcA,
      @Schema.annotations.deprecated b1: CcB,
      b2: CcB,
      c1: CcC,
      @Schema.annotations.deprecated c2: CcC,
      @Schema.annotations.deprecated d1: CcD,
      @Schema.annotations.deprecated d2: CcD
  )

where b2 is but shouldn’t be deprecated in the yaml, and c2 is not but should have been.

2

also for sealed traits eg in the two commits in
Sealedtraits also not working as expected by andersbohn · Pull Request #3 · andersbohn/tapir-redoc-issue · GitHub - looks like it works using the concrete case class, but when using the trait as attribute type, the trait become a referenced schema, which IS deprecated: true , but this does not show in the UI ?!

3

added a unittest checking the yaml GitHub - andersbohn/tapir-redoc-issue at sealedtraits.
so it about tapir scala → openapi (not redoc/swagger)

related to this old discussion: redoc: Description of properties with a schema reference does not get rendered | gitmotion.com

which looks like this issue `@description` behavior is counterintuitive · Issue #1203 · softwaremill/tapir · GitHub

4

@andersbohn thanks for the investigation, indeed this seems to be the same issue as the one you linked (1203). It’s due to the fact that we currently assume that there’s a single schema for each type - which, with customisations, might not be the case.

However, with OpenAPI 3.1 now fully supported by Swagger (the dominant “consumer” of the specs generated by tapir), maybe we can fix this.

To make sure we’re on the same page, here’s my simplified example. First, the data classes and the endpoint:

case class Data1(x: String)
case class Data2(@deprecated @description("aaa") a: Data1, @description("bbb") b: Data1)

val e = infallibleEndpoint.get.in(jsonBody[Data2])

When interpreted to yaml, we get (somewhat simplified):

openapi: 3.1.0
info:
  title: Fruits
  version: '1.0'
paths:
  /:
    get:
      operationId: getRoot
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Data2'
        required: true
      responses:
        '200':
          description: ''
components:
  schemas:
    Data1:
      required:
      - x
      type: object
      properties:
        x:
          type: string
      description: aaa
      deprecated: true
    Data2:
      required:
      - a
      - b
      type: object
      properties:
        a:
          $ref: '#/components/schemas/Data1'
        b:
          $ref: '#/components/schemas/Data1'

So the schema for Data1 now has the properties of the first customisation, while the second is lost. Instead, we’d want to add these customisations to the ref-s:

openapi: 3.1.0
info:
  title: Fruits
  version: '1.0'
paths:
  /:
    get:
      operationId: getRoot
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Data2'
        required: true
      responses:
        '200':
          description: ''
components:
  schemas:
    Data1:
      required:
      - x
      type: object
      properties:
        x:
          type: string
    Data2:
      required:
      - a
      - b
      type: object
      properties:
        a:
          $ref: '#/components/schemas/Data1'
          description: aaa
          deprecated: true
        b:
          $ref: '#/components/schemas/Data1'
          description: bbb

This renders properly in Swagger UI and redoc according to my tests. There’s also a number of other cases (respons bodies, top-level references etc.) that we’d have to check.

But I think my stratego to implement this would be to see if there are competing schemas for the same type, keep only the common fields, and any extras would get moved to $ref. Hopefully this won’t produce some unwanted results :wink:

5

As a work-around, you can inline the child schemas (instead of using $refs), if they have no “name” property.

In case of the example above, this can be done by changing the schema associated to the json’s body:

val e = endpoint.get.in(
  jsonBody[Data2].schema(
    _.modify(_.a)(_.name(None)).modify(_.b)(_.name(None))))

This renders:

openapi: 3.1.0
info:
  title: Fruits
  version: '1.0'
paths:
  /:
    get:
      operationId: getRoot
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Data2'
        required: true
      responses:
        '200':
          description: ''
        '400':
          description: 'Invalid value for: body'
          content:
            text/plain:
              schema:
                type: string
components:
  schemas:
    Data2:
      required:
      - a
      - b
      type: object
      properties:
        a:
          required:
          - x
          type: object
          properties:
            x:
              type: string
          description: aaa
          deprecated: true
        b:
          required:
          - x
          type: object
          properties:
            x:
              type: string
          description: bbb
6

thanks so much @adamw - this morning I finally understood the (somewhat cryptic reference ) in Generating OpenAPI documentation — tapir 1.x documentation about this and was experimenting with schema.name(None) - (no modify needed, but will try adding it now :slight_smile: )

I also started drafting a ~rewrite of the question, focusing more on this, including more precise sample on GitHub - andersbohn/tapir-redoc-issue at simplest_inlined - but maybe l8r.

seems allowing sibling description/deprecation for $refs would be very nice feature. ( I saw somewhere suggested workaround about wrapping $ref and descrtion in some AllOf, but cant find that/how-to anywhere in the doc/code)

7 
sealed trait Address - OneAddress, AnotherKindOfAddress, ...

sealed trait Events
  Bitcoin
    SomethingHappened (address: Address, ...)
    SomethingHappenedForOne (address: OneAddress, ...)
  Ethereum
    SomethingVerySpecial (@deprecated oldAddress: AnotherKindOfAddress)
  many more events

So inlining the deprecated usage spot is not gonna work, as it is also part of that big coproduct and is used a thousand other places.

IOW seems to me we will require the new feature about annotating $ref-usage

8

Yes, might be so, but that’s a deeper issue than I expected :wink: See Allow usage-site customisation of referenced schemas by adamw · Pull Request #3228 · softwaremill/tapir · GitHub for some early progress.

The additional complexity is due to the fact that there have been some Json Schema / OpenAPI changes since tapir was originally released and we need to catch up.

9

aaw, cool, tnx. Tapir and macros and zio and scala - all so nice when it just works, and then can so quickly get … well hard :slight_smile:

For some reason on our bigger setup the inlined subclasses just seems to disappear. But in the example it just duplicates the whole event-subtree on the affected subclass.

just to confirm that this is also not usable for our setup.

so clicked vote for the PR :innocent:

10

Hm I’m not sure what’s the problem with schema inlining, to debug this I’d need a self-contained, minimised, reproducing example - if you could provide one, I’ll take a look :slight_smile:

In the meantime, I’ve release tapir 1.8.0, which should properly represent schemas with independent usage-site modifications - please test and let us know if it works.

1 Like
11

amazing, so fast, thanks @adamw - I just quickly checked on my sample and it works exactly as it should Inside nested 1 8 0 by andersbohn · Pull Request #5 · andersbohn/tapir-redoc-issue · GitHub :tada:

as for our prod code base, that is a different and likely way longer story, including getting to upgrade to your new release.

oh, not sure if I should click close issue somewhere, but if not clear: as reported I would say it is fixed :+1:

2 Likes
12

working on reproducing, but running into another odd NPE for quite small addition of a an extra subclass:
wip on multi nested inline coproducts and ref by andersbohn · Pull Request #6 · andersbohn/tapir-redoc-issue · GitHub :thinking:

13

ok, this was just stupid wrong order of implicit schema vals causing the NPEs :see_no_evil:

14

Happens every now and then :slight_smile:

So in the end, is there some problem on the tapir side that needs to be still investigated or are we good on this front?

15

@adamw yeah :slight_smile: tnx - so I just managed to reproduce - of not sure if it is a mistake in my code, but when using @deprecated option[t] it is not picked up:

In EventB the @deprecated deprAName gets correctly represented in the openapi, but the optional deprOptionAName does not.

  case class EventB(
      aName: AName,
      @Schema.annotations.deprecated
      deprAName: AName,
      @Schema.annotations.deprecated
      deprOptionAName: Option[AName],
      bName: BName
  ) extends Event

sscce_undepr_option_t - Library.scala#L17

as seen in the docs.yaml # 82 :

  ...
  deprAName:
    $ref: '#/components/schemas/AName'
    deprecated: true
  deprOptionAName:
    $ref: '#/components/schemas/AName'
  ...

(produced with the included testcase EndpointsSpec)

16

Ah this seems to be a bug. Can you create another GH issue? :slight_smile:

1 Like
17

done [BUG] Annotations on referenced fields not working for optionals · Issue #3288 · softwaremill/tapir · GitHub :+1:

18

@adamw got notified the bug was fixed, tnx, is it possible for me to check it before it gets released? I can’t find any snapshot release there…

19

We don’t publish snapshots, you can build the required modules from source, though

20

yes, I did try a bit, but that is another big challenge it seems, for another channel :slight_smile: may have to wait ( I assume it will be for 1.9.x )