Multiple ResponseSchemas

[godwinpang]

@ResponseSchema({200: SomeObject, 400: SomeErrorObject})
handler() { ... }

Originally posted by @epiphone in #9 (comment)

Hi! Thanks for making this awesome library.
I have a use case for multiple ResponseSchemas, what do you think of the mapping implementation as referenced in the above comment? If it sounds good I can work on implementing it.

[epiphone]

Hey, thanks for taking the time!

One question with the above design is how to handle additional properties like contentType and isArray?

One option would be to add a similar nested options object as 2nd parameter:

@ResponseSchema({200: SomeObject, 400: SomeErrorObject}, { 200: { isArray: true } })
handler() { ... }

...or we could add a further level of nesting in the 1st parameter:

@ResponseSchema({200: { body: SomeObject, isArray: true } }, { 400: { body: SomeErrorObject } })
handler() { ... }

Personally I find both approaches a bit messy and would prefer supporting multiple separate ResponseSchema decorators, i.e. the following:

@ResponseSchema(SomeObject,, { isArray: true, statusCode: 200 }) // no need to specify the default status code, strictly speaking
@ResponseSchema(SomeErrorObject, { statusCode: 400 })
handler() { ... }

What do you think?

[godwinpang]

Yep - I agree with you. It fits better into the library's design as well since the current OpenAPI decorator is also implemented with multiple decorators. @thaigillespie you interested in taking this on?

[godwinpang]

Just based on local testing, it looks like chaining multiple separate ResponseSchema decorators is already supported. I can add documentation + a test or two if that sounds right.

[epiphone]

Oh right, looks like ResponseSchema uses _.merge inside so multiple decorators just might work.

Tests to verify this and docs would be awesome!

[thai-truong]

Hello @epiphone . I just wanted to update this thread a little bit, sorry for the inactivity : ) I think that using multiple ResponseSchema decorators for an endpoint does not work for the same status code (so for a 200 status code, only one @ResponseSchema is applied even if there are multiple present). This is the behavior I am seeing so far for the project I'm working on. Here is a relevant snippet of code:

@Get('/')
  @ResponseSchema(MultipleUserNameResponse)
  @ResponseSchema(MultipleAppUserResponse)
  @OpenAPI({ security: [{ TokenAuth: [] }] })
  async getMultipleUsers(
    @QueryParams() multipleUserQuery: MultipleUserQuery
  ): Promise<MultipleAppUserResponse | MultipleUserNameResponse> {
    const multipleUsers = await this.appUserService.getAllAppUsers(multipleUserQuery);

    return { users: multipleUsers };
  }

For this one, there is a query parameter "names" where if it is true, then I get the names of all users and store that in an array. Otherwise (if there is no query parameter present), I get every info we store of a user, including their names. Regardless of whether I make a GET request to this endpoint with the query params or not, openapi only shows one response schema for successful GET query (which is indicated by 200 status code). Particularly, openapi shows the one on the bottom as the response schema for 200 status code (MultipleAppUserResponse for this code snippet), which makes sense based on how decorators work in typescript. Consequently, if I do make a GET request to this endpoint, say with "names" query parameter set to true, then I get

{
  property_1: undefined
  property_2: undefined
  name: {name as a string}
  property_3: undefined
  property_4: undefined
  etc.
} -> Follows the schema of MultipleAppUserResponse

as opposed to

{
  name: {name as a string}
} -> Follows the schema of MultipleUserNameResponse

The response will always follow the schema of MultipleAppUserResponse, since openapi only takes one response schema instead of both.

There is a temporary solution for this as of now, but I think it would be nice to have a more permanent solution to use.

Furthermore, I saw a stackoverflow thread that talks about oneOf, which is something that openapi v3 offers that allows for multiple request/response bodies defined for the same status code of an operation.

Perhaps an implementation of oneOf would suffice here? Or maybe what you suggested above would work? What do you think?

[epiphone]

So if I got it right (tests to verify this would be great) multiple ResponseSchemas work right now as long as each decorator uses a different statusCode or contentType. This is thanks to _.merge's deep merging logic.

What doesn't work is the case @thaigillespie described above, i.e., multiple ResponseSchemas for the same statusCode and contentType. oneOf seems like a good solution here indeed, probably implemented as part of the merging logic here:

routing-controllers-openapi/src/decorators.ts

Line 58 in 81bfab5

: _.merge({}, acc, oaParam)

.

One downside of oneOf is that swagger-ui still can't render it. I think other doc viewers like https://github.com/Redocly/redoc can though.

[godwinpang]

hey also wanted to chime in here for a little bit in terms of how we could implement something as discussed above

We see here that the schema registered for any path is a single schema that's returned from getParamSchema. Does this also come into play?

[godwinpang]

I guess another question for @epiphone is if you'd want to maintain backward compatibility with OpenAPI v2 and/or swagger-ui; it looks like we already use the allOf operator here, so perhaps we're clear to onboard oneOf?

[godwinpang]

Looks like there's already a test for multiple response schemas @epiphone

[epiphone]

We see here that the schema registered for any path is a single schema that's returned from getParamSchema. Does this also come into play?

Hmm not quite sure what you mean. The linked code handles request bodies, not response bodies, right?

...maintain backward compatibility with OpenAPI v2 and/or swagger-ui; it looks like we already use the allOf operator here, so perhaps we're clear to onboard oneOf?

Using oneOf is fine; this library generates OpenAPI v3 schemas or at least that's the intention.

Looks like there's already a test for multiple response schemas

Great 😄 Forgot about that one.

[godwinpang]

ah I misread the function name - sorry about that! looks like this is just a simple modification here then :)

[godwinpang]

opened up a PR @epiphone :)