search

jhthorsen / mojolicious-plugin-openapi

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

Swagger2: collectionFormat is optional for multi-value parameters #150

Closed augensalat closed 5 years ago

augensalat commented 5 years ago

https://github.com/jhthorsen/mojolicious-plugin-openapi/blob/master/lib/JSON/Validator/OpenAPI/Mojolicious.pm#L88-90 requires collectionFormat to be present. This is wrong, https://swagger.io/docs/specification/2-0/describing-parameters/ states

Path parameters can be multi-valued, such as GET /users/12,34,56. This is achieved by specifying the parameter type as array.

Also see the section "Array and Multi-Value Parameters" on that page.

Here is an extended test case: https://gist.github.com/augensalat/a67937dde3efbcf4df6f3430aea89a55

jhthorsen commented 5 years ago

It also states:

A multi-value parameter must be defined with type: array and the appropriate collectionFormat.

augensalat commented 5 years ago

The description is indeed a little contradictory. Still a few lines below it's said that "csv" is the default collectionFormat. This makes it quite clear, that collectionFormat is meant as optional. Hence the implementation should check for type: "array" and not for collectionFormat. This change would probably also help to solve #149 more elegantly, because collectionFormat is 2.0 only.

The widely used Javascript swagger-tools implementation handles it exactly like that: https://github.com/apigee-127/swagger-tools/blob/master/middleware/helpers.js#L190-L219

And also this Javascript library, that has OpenAPI3 support, looks for type === 'array' (l.189ff) and treats collectionFormat as optional: https://github.com/kogosoftwarellc/open-api/blob/master/packages/openapi-request-coercer/index.ts