jhthorsen / mojolicious-plugin-openapi

OpenAPI / Swagger plugin for Mojolicious
53 stars 41 forks source link

Schema for a default response #235

Closed polettix closed 2 years ago

polettix commented 2 years ago

Hi! I'd like to define this (simplified):

...
components:
  schemas:
    response_base:
      type: object
      required: [status]
      properties:
        status:
          type: integer
    response_error:
      allOf:
        - $ref: '#/components/schemas/response_base'
        - properties:
            reason:
              type: string
...

and set response_error as the default error schema, e.g.:

...
my $spec = YAML::LoadFile('api-spec.yaml');
plugin OpenAPI => {
   default_response => {
      schema => $spec->{components}{schemas}{response_error},
...

This is currently not possible, because the reference to response_base confuses the validation process:

[...] You have to call resolve() before validate() to lookup "#/components/schemas/response_base" [...]

I think it would be useful because we might keep the whole schema inside the specification file and leverage inheritance to ease changing stuff.

(Related to #198 but not the same in my understanding). #wishlist

jhthorsen commented 2 years ago

I don't see how this is possible. You have to either define "/components/schemas/response_base" in your own schema or just avoid any ref in the default response. Another solution would be to completely disable the default response and just define all the responses in your own schema.

jhthorsen commented 2 years ago

Another solution, if "/components/schemas/response_error" already exists in your schema, is to simply load the plugin with "name" instead of "schema":

plugin OpenAPI => {default_response => {name => 'response_error'}, ...};
polettix commented 2 years ago

Both /components/schemas/response_base and /components/schemas/response_error would end up inside my schema in the YAML definition - that's the wish in the original request :grin:

I'll give name a try, it seems very promising thanks!

jhthorsen commented 2 years ago

Note that you also have to override https://metacpan.org/pod/Mojolicious::Plugin::OpenAPI#RENDERER, since you have to rewrite the default error documents to suit your new structure.

https://metacpan.org/release/JHTHORSEN/Mojolicious-Plugin-OpenAPI-5.05/source/t/basic-custom-renderer.t has a test for OpenAPIv2, but it's the same for OpenAPIv3.

polettix commented 2 years ago

Thanks for the follow-up, I was already including a custom renderer.

By the way, I see that we need to set the content_type explicitly in the response, which is fair but might get a better default than text/html because most of the time people are sending back JSON-encoded data :wink:

polettix commented 2 years ago

I gave it a try and it seems to work (it might have unveiled a bug in some other part, I'll open another Issue in case).

In particular, this does what I was expecting (I tested removing reason in the 500 case and it raises an exception), I'm putting it here in case others hit my same problem:

#!/usr/bin/env perl
use v5.24;
use warnings;
use Mojolicious::Lite -signatures;
use Mojo::JSON 'encode_json';

get "/item/:id" => sub ($c) {
   $c->openapi->valid_input or return;
   my $id = $c->param('id');
   if ($id == 404) {
      $c->render(
         status  => 404,
         openapi => {status => 404, reason => 'Not Found'},
      );
   } ## end if ($c->param('id') > ...)
   elsif ($id == 500) {
      $c->render(
         status  => 500,
         openapi => {status => 500, reason => 'Internal Server Error'},
      );
   } ## end if ($c->param('id') > ...)
   else {
      $c->render(
         status  => 200,
         openapi => {status => 200},
      );
   } ## end else [ if ($c->param('id') > ...)]
  },
  'item';

plugin OpenAPI => {
   url => "data:///api.yaml",
   default_response => { name => 'exception', },
   renderer => sub ($c, $data) {
      $c->res->headers->content_type('application/json');
      return encode_json($data);
   },
};
app->start;

__DATA__
@@ api.yaml
openapi: 3.0.0
info:
  title: Example Web API
  version: 0.0.1
  description: PoC definition
components:
  schemas:
    base:
      type: object
      properties:
        status:
          type: integer
    exception:
      type: object
      allOf:
        - $ref: '#/components/schemas/base'
        - type: object
          required: [ reason ]
          properties:
            reason:
              type: string
paths:
  /item/{id}:
    get:
      x-mojo-name: item
      responses:
        '200':
          description: OK
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/base'
jhthorsen commented 2 years ago

By the way, I see that we need to set the content_type explicitly…

That’s what the default renderer does. If you want a custom behavior you’re handed full power. This won’t change.

Your renderer doesn’t work though, since it ignores (does not rewrite) the default response structure on invalid input and output. (As shown in the test earlier)