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

What is the idiomatic way to have default params in APIs? #766

Open
Batou99 opened this issue Jun 8, 2017 · 13 comments
Open

What is the idiomatic way to have default params in APIs? #766

Batou99 opened this issue Jun 8, 2017 · 13 comments

Comments

@Batou99
Copy link

Batou99 commented Jun 8, 2017

I am currently defining an API with:

type DeviceApi = 
           "v2/devices/catalog/search" :> Header "X-HEADER" String
                                             :> QueryParam "name" String
                                             :> QueryParam "description" String
                                             :> QueryParam "page" Int
                                             :> QueryParam "limit" Int
                                             :> QueryParam "tags" [Tag]
                                             :> QueryParam "serial" [Tag]
                                             :> QueryParam "dir" SortDir
                                             :> QueryParam "sort" SortBy
                                             :> ReqBody '[JSON] FilterReqBody
                                             :> Get '[JSON] DevicePaginatedListing

And using this to generate a function to call the endpoint

catalogSearch :: AuthHeader -> Maybe String -> Maybe String -> Maybe Int -> Maybe Int -> Maybe [Tag] -> Maybe [Tag] -> Maybe SortDir -> Maybe SortBy -> FilterReqBody -> ClientM DevicePaginatedListing

The issue is that the function signature is quite ugly, I'd like to define a record type and default params to be able to generate the function like this:

data CatalogSearchParams = CatalogSearchParams { deviceName :: Maybe String
                                               , desc :: Maybe String
                                               , page :: Maybe Int
                                               , limit :: Maybe Int
                                               , tags :: Maybe [Tag]
                                               , serial :: Maybe [Tag]
                                               , dir :: Maybe SortDir
                                               , sort :: Maybe SortBy
                                               , reqBody :: FilterReqBody}

defaultCatalogSearchParams = CatalogSearchParams { deviceName = Nothing
                                                 , desc = Nothing
                                                 , page = Just 1
                                                 , limit = Just 1000
                                                 , tags = Nothing
                                                 , serial = Nothing
                                                 , dir = Just Asc
                                                 , sort = Just Name
                                                 , reqBody = FilterReqBody [] }

catalogSearch :: AuthHeader -> CatalogSearchParams -> ClientM DevicePaginatedListing
catalogSearch = client api

Which of course does not work.
How do you normally deal with endpoints that accept a lot of query params? I am sure this is something not uncommon.

@phadej
Copy link
Contributor

phadej commented Jun 11, 2017

I has almost working prototype, but it depends on yet unreleased generics-sop-0.3.1.0. I hope I'll complete it during ZuriHac.

@phadej
Copy link
Contributor

phadej commented Jun 11, 2017

See haskell-servant/servant-contrib#4

I won't say it's idiomatic, as I just wrote it; but that's something I could imagine using myself. (Required parameters will complicate the story)

@fizruk
Copy link
Member

fizruk commented Jun 14, 2017

I think there was an issue somewhere (can't find it now) to add combinator(s) that'd use ToForm/FromForm to parse FormURLEncoded request bodies and/or QueryParams.

I think that would be more "idiomatic" than ManyQueryParams, since it would use normal Haskell records (together with Generic-based deriving).

@haskell-servant/maintainers what do you think?

@3noch
Copy link

3noch commented Jun 14, 2017

Would it be difficult to add a combinator that could masquerade as a QueryParam but provide a default?

type Api = "test" :> (QueryParam "value" Int :?> 10) :> Get '[JSON] Value

where QueryParam "value" Int :?> 10 is itself a QueryParam that wraps another one.

If there is/were a way to customize what the Nothing case means when passing data to the handler, then this could be easy.

This would be also very handy when generating docs since defaults are an important part of documentation that is not currently captured.

@dmjio
Copy link
Member

dmjio commented Jun 14, 2017 via email

@3noch
Copy link

3noch commented Jun 14, 2017

You can always add defaults in your handler. The question is: how do you capture that information in the API specification? I think we have two options, specify the default in the type itself, or use type classes to allow the default to be at the value level. But the type-class approach would be very boilerplate heavy. Perhaps you could combine them to get the best of both worlds.

@fizruk
Copy link
Member

fizruk commented Jun 14, 2017

@3noch I agree with @dmjio. Unfortunately DataKinds can't yet lift any (even constant) terms to the type level.

Besides, I don't like defaults parameter values on the handler side.
If you don't have a value for a query parameter, but you need it — throw 400 (or use RequiredQueryParam if/when it lands in Servant 😉).

AFAIK default parameters are mostly useful when you have many parameters and would like to omit few. But then I'd rather pack those parameters in a record data type and have a smart constructor (maybe more than one) for it with some defaults.
That is roughly the same idea @Batou99 originally posted.

@3noch
Copy link

3noch commented Jun 14, 2017

@fizruk I use Servant often to write specifications for other APIs. They don't tend to have the same values as Haskellers and they use defaults liberally. Not being able to easily encode them in the specification is a problem, IMO. It's even more annoying when they're not documented, and servant, currently, has no way to generate docs for defaults.

@fizruk
Copy link
Member

fizruk commented Jun 14, 2017

@3noch if you're a client of such API then I guess you can treat any QueryParam with default value as RequiredQueryParam and always pass values explicitly. No need to encode defaults.
Note that you can still copy those default values for client functions.

@3noch
Copy link

3noch commented Jun 14, 2017

@fizruk Right, yes. I should have been clearer. I actually hope to use servant specs as the engine for generating docs for such APIs.

@phadej
Copy link
Contributor

phadej commented Jun 15, 2017

@fizruk the ManyQueryParams is a show-off example. It says in the type which query arguments it uses, where FromForm would hide that inside term-value instance. I think both cases have their uses, but definitely FromForm based approach is simpler and probably good enough for most (almost all) cases.

Similar reasoning applies to default values. Sometimes you want them shown at type-level, but you can bake them into an instance as well.

The drawback with FromForm approach, is that you need to repeat yourself more. Especially with default values, it's hard to use Generics to derive proper instance.

Side-note: IIRC swagger supports default values?

@Batou99
Copy link
Author

Batou99 commented Jun 27, 2017

The issue here (for me) it's less about mandatory vs optional params and more about easy to call functions.

In the example I show above the function catalogSearch it's awful to call. It has 8 possible QueryParams and the ReqBody, that makes calling the function very cumbersome.

If I declare a record to wrap the QueryParams and declare a wrapper function to pass the record instead it's way less cumbersome, but it forces me to declare the records and the wrapper for each function defined in the API.

I think this is a common enough case to merit a standard idiomatic solution. I just don't know if there is already one (does not look like it) or if we need something for it.

@coderfromhere
Copy link

This is still highly desired for mapping onto real-world APIs out there

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

6 participants