jhthorsen / mojolicious-plugin-openapi

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

V3: OPTIONS and HEAD Routes Automatically Vivified #193

Closed atancasis closed 3 years ago

atancasis commented 3 years ago

Let me first preface this issue with a statement that the current behavior of how these methods behave might be a result of design convictions taken for this Plugin (in the case of OPTIONS) and/or a limitation or design conviction of Mojolicious (in the case of HEAD), so please don't hesitate to let me know if this behavior are either intended and this issue is invalid OR if it's valid but non-trivial to fix.

Without explicit operation objects defined as path item objects for options and head, it appears that these routes are automatically vivified.

This behavior, however, appears to only apply for the OPTIONS and HEAD methods, whereas HTTP 404 are correctly being returned for every other method without specifications.

Outlined below is a test suite illustrating this behavior:

use Test::More;
use Test::Mojo;

use Mojolicious::Lite;

get '/items' => sub {
    my $c = shift->openapi->valid_input or return;
    $c->render(openapi => {id=>1}, status => 200);
}, 'get_items';

plugin OpenAPI => {url => 'data:///schema.json', schema => 'v3'};

my $t = Test::Mojo->new();
$t->get_ok('/items')->status_is(200);
$t->put_ok('/items')->status_is(404);
$t->post_ok('/items')->status_is(404);
$t->delete_ok('/items')->status_is(404);
$t->options_ok('/items')->status_is(404);
$t->head_ok('/items')->status_is(404);
$t->patch_ok('/items')->status_is(404);

# no trace_ok() in Test::Mojo
$t->request_ok($t->ua->build_tx(TRACE => '/items'))->status_is(404);

done_testing;

__DATA__
@@ schema.json
{
  "openapi": "3.0.3",
  "info": { "title": "Test", "version": "0.0.0" },
  "paths": {
    "/items": {
      "get": {
        "operationId": "get_items",
        "responses": {
          "200": {
            "description": "ok",
            "content": {
              "application/json": { "schema": { "$ref": "#/components/schemas/Item" } }
            }
          }
        }
      }
    }
  },
  "components": {
    "schemas": {
      "Item": {
        "required": ["id"],
        "properties": {
          "id": { "type": "integer" }
        }
      }
    }
  }
}

Whereby:

ok 1 - GET /items
ok 2 - 200 OK
ok 3 - PUT /items
ok 4 - 404 Not Found
ok 5 - POST /items
ok 6 - 404 Not Found
ok 7 - DELETE /items
ok 8 - 404 Not Found
ok 9 - OPTIONS /items
not ok 10 - 404 Not Found
#   Failed test '404 Not Found'
#   at v3-head.t line 22.
#          got: '200'
#     expected: '404'
ok 11 - HEAD /items
not ok 12 - 404 Not Found
#   Failed test '404 Not Found'
#   at v3-head.t line 23.
#          got: '200'
#     expected: '404'
ok 13 - PATCH /items
ok 14 - 404 Not Found
ok 15 - TRACE /items
ok 16 - 404 Not Found

Specifically:

  1. When OPTIONS is invoked, the application returns the OAS —which I understand might be a design conviction or feature of the plugin itself, but took me by surprise.

  2. When HEAD is invoked, I understand in this section of the Mojolicious routing guide that there's a caveat on this method:

There is one small exception, HEAD requests are considered equal to GET, but content will not be sent with the response even if it is present.

But thought I should check and ask if both of these behaviors are to be expected or if there's a way to coerce the plugin to NOT autovivify these operation paths unless explicitly specified and defined in the OAS.

In the spirit of completeness and fairness however, it should be noted that both these endpoints, despite being autovivified or installed with default handlers, still operate in a reasonable manner, wherein:

Once again, thanks for the excellent excellent plugin and work that has been done on this plugin. Just wanted to express that this plugin is heavily used in our core platforms for more than a year now and is by far, from my estimate, the most advanced or feature-rich, even across language and evaluating the ecosystem as a whole, in terms of bootstrapping applications from OAS.

Please don't hesitate to let me know if there's any additional details and/or clarifications that I can provide. Thanks!

jhthorsen commented 3 years ago

Thank you for the kind words.

  1. The https://metacpan.org/pod/Mojolicious::Plugin::OpenAPI::SpecRenderer plugin is enabled by default, which results in OPTIONS being added. It also enables the spec to be rendered on the "basePath". Example here: https://convos.chat/api.html and https://convos.chat/api.json. You can disable the plugin completely by passing in https://metacpan.org/pod/Mojolicious::Plugin::OpenAPI#plugins or you can also configure the SpecRenderer to your desire.
  2. The old M::P::Swagger2 version used to have a lot of custom functionality, while M::P::OpenAPI tries to work together as much as possible with the ground rules set by Mojolicious. That is why "HEAD" works like it does. But to be honest, I'm not entirely sure if it's easy to prevent this behavior either.

You might want to join #perl-openapi on irc.freenode.net if you have further questions. A two way dialog for these kinds of questions could be better.

atancasis commented 3 years ago

Understood, thanks again @jhthorsen, much appreciated! I'll close this issue in this case.