jhthorsen / mojolicious-plugin-openapi

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

Compatible with J::V 5.00 #223

Closed jhthorsen closed 3 years ago

jhthorsen commented 3 years ago

Updated the codebase to be in sync with https://github.com/jhthorsen/json-validator/pull/251

Can be tested with:

cpanm https://github.com/jhthorsen/json-validator/archive/v5_oh.tar.gz
cpanm https://github.com/jhthorsen/mojolicious-plugin-openapi/archive/v5_oh.tar.gz

Note that the version number will be wrong, but it will have the new code.

HEM42 commented 3 years ago

First issue (which also existed in 4.06)

From the documentation

   version_from_class
    Can be used to overridden "/info/version" in the API specification, from
    the return value from the "VERSION()" method in "version_from_class".

    This will only have an effect if "version" is "0".

    Defaults to the current $app.

As of 4.06 and current development version, only setting version_from_class will give the desired effect of setting the version in the rendered spec. We originally upgraded from 3.35 -> 4.06.

It no longer defaults to the current $app

HEM42 commented 3 years ago

When rendering the spec, the securitySchemes component is removed from the original spec, and no longer part of the rendered spec.

This is not a problem in released 4.06

Here is a simplified test that fails under v5_oh

https://gist.github.com/HEM42/d60a65e1ca1627db89eb0f137abbf15c

HEM42 commented 3 years ago

When rendering spec, additional type is added parameters definition

Example from defined spec:

  /installation/{id}:
    get:
      description: <b>get_installation_info</b><br/>Retrieves information for installation
      x-mojo-to: installation#info
      operationId: get_installation_info
      security:
        - apiKey: []
      parameters:
        - in: path
          name: id
          description: Installation id
          required: true
          schema:
            type: string

Rendered spec looks like, notice the extra type: string

  /installation/{id}:
    get:
      description: <b>get_installation_info</b><br/>Retrieves information for installation
      x-mojo-to: installation#info
      operationId: get_installation_info
      security:
        - apiKey: []
      parameters:
        - in: path
          name: id
          description: Installation id
          required: true
          schema:
            type: string
          type: string

This was also a problem in released version 4.06

jhthorsen commented 3 years ago

Thanks for your feedback!

HEM42 commented 3 years ago

Thanks for your feedback!

This works now :)

  • [x] additional type is added parameters definition -- Can you check again? Maybe it was fixed when fixing the "securitySchemes" issue.

Still an issue, I have updated the gist; https://gist.github.com/HEM42/d60a65e1ca1627db89eb0f137abbf15c with an additional test for this

jhthorsen commented 3 years ago

I'm not sure why my initial test did not pick up the bug. It looks very similar... Either way: Thank you for the gist ❤️

jhthorsen commented 3 years ago

5.00 is now released 👍