Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ToSwaggerSchemaObject class #6

Closed
fizruk opened this issue Nov 6, 2015 · 11 comments
Closed

ToSwaggerSchemaObject class #6

fizruk opened this issue Nov 6, 2015 · 11 comments

Comments

@fizruk
Copy link
Member

fizruk commented Nov 6, 2015

Currently SwaggerModel is used for response/request body and this is bad:

  1. It does not correspond to Schema Object in the swagger specification and thus does not allow to use primitive types for request/response bodies.
  2. It makes Generic implementation of ToSwaggerModel hard or impossible.

I suggest we add classes and types according to swagger specification:

data SwaggerSchemaObject
  = Array [SwaggerSchemaObject]
  | Boolean Bool
  ...
  | Object SwaggerModel

class ToSwaggerSchemaObject a where
  toSwaggerSchemaObject :: Proxy a -> SwaggerSchemaObject
@dmjio
Copy link
Member

dmjio commented Nov 6, 2015

I think this is a great idea. I didn't realize model definitions could be specified as arrays.

An object to hold data types that can be consumed and produced by operations. These data types can be primitives, arrays or models.

Previously, I was specifying the type as array and in the items field putting a reference to the model. This is necessary to do when dealing with QueryParams, but not with ReqBody. According to the docs array must be specified.

Required if type is "array". Describes the type of items in the array.

Similar to this:

{
  "description": "A complex object array response",
  "schema": {
    "type": "array",
    "items": {
      "$ref": "#/definitions/VeryComplexType" /* just an object. */
    }
  }
}

This would simplify things if we didn't have to deal with the items properly and could just use $ref to models (for request bodies).

I think all Request bodies / Response bodies should have external model definitions, where the name of the model corresponds to the name of the constructor. This means everything should be wrapped in a newtype, including primitives like 'string'.

So it would look something like:

newtype Id = Id String deriving Show
data User = User { uid :: Text } deriving Show
type API = "api" :> QueryParams "params" Param :>  ReqBody '[JSON] Id :> Get '[JSON] User

Id above would generate the following model

// in definitions
{ "Id" : { 
    "type" : "string"
  }
}

User would generate the following model:

{ "User" : { 
    "type" : "object"
  , "properties" : {
       "uid" : "string" 
    }
  }
}

I'm ok with the definition you have proposed, I would just move the SwaggerSchemaObject out of a list.

data SwaggerSchemaObject
  = Array SwaggerSchemaObject -- not a list
  | Boolean Bool
  | Object SwaggerModel

There's also a lot of information that I'm not allowing a user to specify on things like paths, query and body parameters.

I think ToSwaggerPath, ToSwaggerQueryParam and ToSwaggerBody classes would make sense.

@fizruk
Copy link
Member Author

fizruk commented Nov 6, 2015

I'm ok with the definition you have proposed, I would just move the SwaggerSchemaObject out of a list.

Note that in case of array, you can

  • either specify a single schema for every item in the array, or
  • specify a separate schema for every item (i.e. model a tuple).

This is specified for JSON Schema's items.
And Swagger Schema defines items as "taken from the JSON Schema definition but [its definition was] adjusted to the Swagger Specification".

Actually, Swagger specification is not that obvious :)

I have been thinking and have come to a conclusion that it would be better to have a separate swagger2 package for Swagger 2.0 data model and to build servant-swagger using that model.
I have spent some time today reading Swagger specification and swagger2-0.1 is almost ready, I will publish it to GitHub (and Hackage) soon. Even if it won't fit for servant-swagger purpose, it is a nice exercise :)

@dmjio
Copy link
Member

dmjio commented Nov 6, 2015

Yes, the swagger spec is not obvious at all in my opinion ;)

@dmjio
Copy link
Member

dmjio commented Nov 6, 2015

I suppose a user could encode a tuple in haskell to a heterogenous list in javascript...

> λ: encode (1 :: Int, "hey" :: String)
"[1,\"hey\"]"

Why anyone would want to do this, I don't know :)

I'd be interested to see how you model the specification. I had that idea originally, to create a separate swagger package, but one already exists on hackage (albeit using the older 1.2 version of the Swagger API).

@dmjio
Copy link
Member

dmjio commented Nov 6, 2015

@fizruk, this might interest you as well, http://hackage.haskell.org/package/jsonschema-gen

@dmjio
Copy link
Member

dmjio commented Nov 6, 2015

@fizruk, I think together we can come up with something that works well 👍

@fizruk
Copy link
Member Author

fizruk commented Nov 8, 2015

@dmjio
Copy link
Member

dmjio commented Nov 9, 2015

@fizruk, Great work! Do you want servant-swagger to migrate to it? I don't see lenses, but I guess we can just derive those manually.

@fizruk
Copy link
Member Author

fizruk commented Nov 9, 2015

@dmjio Yes, the idea is to get swagger model out of servant-swagger.
I would like you to review swagger2 and tell if you need anything changed there to start migration (or maybe discussion of migration).

@dmjio
Copy link
Member

dmjio commented Nov 9, 2015

Lenses seem appropriate (servant-docs uses them as well). Otherwise, it looks fantastic 👍
EDIT: which there is a pending pull for lenses

@fizruk
Copy link
Member Author

fizruk commented Dec 31, 2015

Closed in #9.

@fizruk fizruk closed this as completed Dec 31, 2015
fisx pushed a commit to wireapp/servant-swagger that referenced this issue Jun 1, 2019
fisx added a commit to wireapp/servant-swagger that referenced this issue Jun 1, 2019
Until now, we have built against haskell-servant#6, but the release is based on
the alternative in haskell-servant#7.  tinylog does not split up `new` into a
version with and a version without effects, but reads the
`Settings` for a flag that determines whether effects should be
had or not.  haskell-servant#7 also lacks some helpers that were not general
enough, so I've copied them verbatim from haskell-servant#6 into this commit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants