GetShopTV / swagger2

Swagger 2.0 data model.
http://hackage.haskell.org/package/swagger2
BSD 3-Clause "New" or "Revised" License
74 stars 59 forks source link

What part of Swagger spec describes SwaggerItemsArray? #107

Open qrilka opened 7 years ago

qrilka commented 7 years ago

In our code I've tried using SwaggerItemsArray with the following ToSchema instance:

instance ToSchema QuartileCounts where
  declareNamedSchema _  = do
    iSchema <- declareSchemaRef (Proxy @ (Sum Integer))
    return . named "QuartileCounts" $ mempty
      & type_ .~ SwaggerArray
      & items ?~ SwaggerItemsArray [iSchema, iSchema, iSchema, iSchema]

which should cover as I understand values like e.g. [21, 6, 4, 1]. But with a swagger.json for that Python client (which uses bravado to access Swagger endpoints) on unmarshaling line:

obj_type = schema_object_spec.get('type')

And it looks like according to the spec (as shown at http://swagger.io/specification/#itemsObject or https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#itemsObject ) items property should be a "Items Object" and not an array. For our case I'll go with SwaggerItemsObject as this "tuple" is homogeneous but what about other tuples - at the moment it looks to me that swagger file generated with swagger2 will be incorrect.

qrilka commented 7 years ago

It looks like jso-schema spec allows arrays though - https://tools.ietf.org/html/draft-fge-json-schema-validation-00#page-9

qrilka commented 7 years ago

And in the next spec https://github.com/OAI/OpenAPI-Specification/blob/OpenAPI.next/versions/3.0.md#properties it is even said explicitly

items - Value MUST be an object and not an array. Inline or referenced schema MUST be of a Schema Object and not a standard JSON Schema. items MUST be present if the type is array.

fizruk commented 7 years ago

@qrilka array stands for both homogeneous lists (like haskell lists) and tuples (heterogeneous lists of fixed size).

For homogeneous lists items property is just one Schema that describes type for all items. For tuples items property is a list of Schemas, one Schema per value in a tuple.

Note though that you can limit the size for homogeneous lists using maxItems and minItems. So in your example it's best to use a homogeneous list:

instance ToSchema QuartileCounts where
  declareNamedSchema _  = declareNamedSchema (Proxy @ [Sum Integer])
    & name ?~ "QuartileCounts"
    & schema.minItems ?~ 4
    & schema.maxItems ?~ 4

It is possible that I misread the specification (which was and is incredibly hard to read, TBH).

It is also possible that at the time when I introduced SwaggerItemsArray this option was not excluded from the spec (it certainly looked differently a year ago). I would not be surprised if it was a bug in the spec that I brought into swagger2 and that they've patched it later silently without any version bump.

Note that without SwaggerItemsArray it becomes impossible to create Schema for non-record product types. If we remove it, we have to add a TypeError for GToSchema and GToParamSchema explaining this to users.

qrilka commented 7 years ago

@fizruk actually I've written almost the same code as you've pasted above :)

fizruk commented 7 years ago

To be clear, I'm happy with the way things are right now. I don't think any of my APIs rely on SwaggerItemsArray, but it makes a nice representation for tuples.

Nonetheless, someone should read Swagger specification thoroughly and decide whether SwaggerItemsArray has to go. If yes, then someone (else) should send a PR fixing it for everyone.

paulyoung commented 4 years ago

I pasted my swagger JSON into https://editor.swagger.io/ and saw this:

Screen Shot 2020-04-14 at 2 35 57 PM Screen Shot 2020-04-14 at 2 35 44 PM

All of my instances are derived.