jhthorsen / mojolicious-plugin-openapi

OpenAPI / Swagger plugin for Mojolicious
54 stars 42 forks source link

SpecRenderer rendering incorrect spec #189

Closed christopherraa closed 4 years ago

christopherraa commented 4 years ago

Given this example application:

use Mojolicious::Lite -signatures;

post "/foo" => sub($c) { $c->render(openapi => { foo => 'yes' }) }, "foo";
post "/bar" => sub($c) { $c->render(openapi => { bar => 'no'  }) }, "bar";

plugin OpenAPI => { url => "data:///foo-spec.json", schema  => "v2", };
plugin OpenAPI => { url => "data:///bar-spec.json", schema  => "v2", };

get '/api_spec'     => \&_spec_cb;
get '/bar_api_spec' => \&_spec_cb;

app->start;

sub _spec_cb($c) {
  $c->stash(openapi_spec => JSON::Validator->new->schema("data:///bar-spec.json")->bundle);
  $c->openapi->render_spec;
}

__DATA__
@@ foo-spec.json
{
  "swagger" : "2.0",
  "info" : { "version": "0.0.1", "title" : "Foo Service" },
  "schemes" : [ "http" ],
  "basePath" : "/api",
  "paths" : {
    "/foo" : {
      "post" : {
        "x-mojo-name" : "foo",
        "parameters" : [],
        "responses" : {
          "200": {
            "description": "Foo response",
            "schema": { "type": "object" }
          }
        }
      }
    }
  }
}

@@ bar-spec.json
{
  "swagger" : "2.0",
  "info" : { "version": "0.0.1", "title" : "Bar Service" },
  "schemes" : [ "http" ],
  "basePath" : "/api",
  "paths" : {
    "/bar" : {
      "post" : {
        "x-mojo-name" : "bar",
        "parameters" : [],
        "responses" : {
          "200": {
            "description": "Bar response",
            "schema": { "type": "object" }
          }
        }
      }
    }
  }
}

For /api_spec this is rendered:

{                                    
  "basePath": "/api",                
  "definitions": {                   
    "DefaultResponse": {             
      "properties": {                
        "errors": {                  
          "items": {                 
            "properties": {          
              "message": {           
                "type": "string"     
              },                     
              "path": {              
                "type": "string"     
              }                      
            },                       
            "required": [            
              "message"              
            ],                       
            "type": "object"         
          },                         
          "type": "array"            
        }                            
      },                             
      "required": [                  
        "errors"                     
      ],                             
      "type": "object"               
    },                               
    "_definitions_DefaultResponse": {
      "properties": {                
        "errors": {                  
          "items": {                 
            "properties": {          
              "message": {           
                "type": "string"     
              },                     
              "path": {              
                "type": "string"     
              }                      
            },                       
            "required": [            
              "message"              
            ],                       
            "type": "object"         
          },                         
          "type": "array"            
        }
      },
      "required": [
        "errors"
      ],
      "type": "object"
    }
  },
  "host": "127.0.0.1:45265",
  "info": {
    "title": "Foo Service",
    "version": "0.0.1"
  },
  "paths": {
    "/foo": {
      "post": {
        "parameters": [],
        "responses": {
          "200": {
            "description": "Foo response",
            "schema": {
              "type": "object"
            }
          },
          "400": {
            "description": "Default response.",
            "schema": {
              "$ref": "#/definitions/DefaultResponse"
            }
          },
          "401": {
            "description": "Default response.",
            "schema": {
              "$ref": "#/definitions/DefaultResponse"
            }
          },
          "404": {
            "description": "Default response.",
            "schema": {
              "$ref": "#/definitions/DefaultResponse"
            }
          },
          "500": {
            "description": "Default response.",
            "schema": {
              "$ref": "#/definitions/DefaultResponse"
            }
          },
          "501": {
            "description": "Default response.",
            "schema": {
              "$ref": "#/definitions/_definitions_DefaultResponse"
            }
          }
        },
        "x-mojo-name": "foo"
      }
    }
  },
  "schemes": [
    "http"
  ],
  "swagger": "2.0"
}

For /bar_api_spec this is rendered:

{
  "basePath": "/api",
  "info": {
    "title": "Bar Service",
    "version": "0.0.1"
  },
  "paths": {
    "/bar": {
      "post": {
        "parameters": [],
        "responses": {
          "200": {
            "description": "Bar response",
            "schema": {
              "type": "object"
            }
          }
        },
        "x-mojo-name": "bar"
      }
    }
  },
  "schemes": [
    "http"
  ],
  "swagger": "2.0"
}

I would have expected to see both of the endpoints render the specification for bar, but that isn't happening.

christopherraa commented 4 years ago

Observing the exact same behaviour even though I disable the spec-renderer for the first invocation to OpenAPI:

plugin OpenAPI => { url => "data:///foo-spec.json", schema  => "v2", plugins => [qw/+Cors +Security/] };
christopherraa commented 4 years ago

Ok, progress. This is a route matching issue. If basePath in the two specs are set to /fapi the both of the endpoint render the spec for bar like expected.

jhthorsen commented 4 years ago

I've changed the API in 8dcf06cc28720f44f4d5cfdb97350b8df1b8fd14, which should resolve this issue. Will be available on CPAN before you wake up 😄

christopherraa commented 4 years ago

Dude, you're awesome! Works exactly as expected! I ended up passing undef as the first argument to render_spec() since we're not supporting partial renders of it.

sub _api_spec($c) {
  state $path = $c->app->home->rel_file($c->app->config->{openapi_spec})->to_string;
  state $spec = JSON::Validator->new->schema($path)->bundle;
  $c->openapi->render_spec(undef, $spec);
};

Output is just like I had hoped. Thanks!

jhthorsen commented 4 years ago

I think I would suggest passing in empty string instead of undef, in case I start using the path in all cases, but right now I guess it's irrelevant.

Glad it worked out 👍