Add headers and query params support

[fizruk]

Two issues have been created for servant (haskell-servant/servant#137 and haskell-servant/servant#213) where path-pieces package was considered to be used instead of a locally defined ToText/FromText classes.

However, @jkarni has some objections, one beeing that servant uses those classes not only for path pieces, but also for headers and query parameters.

@gregwebs has suggested to use this class (again, locally):

class ToApiData where
  toUrlPiece :: a -> Tex
  default toUrlPiece :: ToPathPiece a => a -> Text
  toUrlPiece = toPathPiece
  toHeader :: a -> ByteString
  toHeader = encodeUtf8 . toUrlPiece
  toQueryParam :: a -> Text
  toQueryParam = toUrlPiece

I personally think that there is no need in code duplication and @gregwebs's suggestion can be applied directly to path-pieces (perhaps there should be a name change though). That is it should be possible to replace PathPiece with ToApiData/FromApiData.

There are not that many reverse dependencies for path-pieces, so updating them should not be much trouble.

What do you think?

[gregwebs]

@fizruk of course I am in favor of this, thanks for getting things going again.

bikeshed questions:

• will it help users out to have a new package name
• is anything helped by moving the repo be moved to a new organization?
• is To/FromApiData the best name? How about just To/FromApi ?

technical questions:

• what does FromApi look like? In particular, I think path-pieces made a mistake of returning a Maybe, Either is always better.

[fizruk]

I think adding headers/query param support to path-pieces without changing its name will likely confuse new users. Although I don't have a good name in mind.

Do you want to move this to @haskell-servant? I don't think this changes anything from my perspective.

If we stick with currently suggested class implementation, I like ToApiData/FromApiData more.

I think FromApiData might have both Maybe and Either String methods:

class FromApiData a where
  fromUrlPiece :: Text -> Maybe a
  parseUrlPiece :: Text -> Either String a
  ...
  fromUrlPiece = either (const Nothing) Just . parseUrlPiece
  parseUrlPiece piece = case fromUrlPiece piece of
    Nothing -> Left ("parse error on `" ++ piece ++ "'")
    Just x  -> Right x
  ...

The reasoning is that user most likely is okay with default error message. OTOH having both Maybe and Either String makes classes fat (6 methods each, 1 for minimal definition).

One more question:

• I suspect that "API data" is not limited to the UrlPiece-Header-QueryParam trio. How can we add a new type of API data later? Should we try to make ToApi/FromApi open w.r.t. API data types? E.g.:

class ToApi apiType a where
  toApi :: proxy apiType -> a -> Text

class FromApi apiType a where
  fromApi :: proxy apiType -> Text -> Maybe a

data UrlPiece

instance ToApi   UrlPiece Text where toApi   _ = id
instance FromApi UrlPiece Text where fromApi _ = Just

toUrlPiece   = toApi   (Proxy :: Proxy UrlPiece)
fromUrlPiece = fromApi (Proxy :: Proxy UrlPiece)

I am not sure this is a good idea though.

[gregwebs]

I think we can extend the typeclass by using default definitions. We can do that now with PathPiece if we want to, but the name won't make sense.

I don't see the reason to have methods that return a Maybe. One can always hush an Either into a Maybe. We can add helper function to define the Either in terms of a Maybe

fromUrlPieceMay :: FromApiData fad => (Text -> Maybe fad) -> Text -> Either Text fad
fromUrlPieceMay parser input =
  let res = parser input
  in  case res of
        Just ok -> Right ok
        Nothing -> Left $ "fromUrlPiece: could not convert: " <> input

[gregwebs]

To be clear, using the above, one can now define the either version that would be required from the typeclass

fromUrlPiece = fromUrlPieceMay maybeParser

[gregwebs]

One more bikeshed: this is specifically an http api. So a better name would be http-api-data. IT seems like there should be some better names to combine with http though.

So we can create the new package under haskell-servant with the typeclass we are discussing. We can update all the reverse dependencies to define HttpApiData. It is still problematic forOne more bikeshed: this is specifically an http a Yesod to make the breaking change to HttpApiData though. What we can do instead is define PathPiece using HTtpApiData to supply a default definition and wait some time while users transition to HttpApiData. (actually Yesod probably won't make the breaking change until it enhances its routing).

[fizruk]

Sorry, I've failed to parse this sentence:

It is still problematic forOne more bikeshed: this is specifically an http a Yesod to make the breaking change to HttpApiData though.

Specifically the "an http a Yesod" part. If I understood correctly, you are saying that this change is breaking for Yesod and we could continue to provide PathPieces class to allow slow transition. Is that what you suggest?

Otherwise I agree to have Either methods and prepend http to package/class name.

I suggest the following workflow:

• clone and rename path-pieces to http-api-data (I think it is worth saving repository history);
• add ToHttpApiData/FromHttpApiData classes;
• add default implementation to PathPiece class;
• send pull requests to all path-pieces reverse dependencies, switching them to use http-api-data.

Does this make sense?

P. S. We have not discussed PathMultiPiece class. I don't think it can or should be replaced by ToHttpApiData/FromHttpApiData classes. I also am not sure if we need a similar pair of classes for multi pieces.

[gregwebs]

sorry, I screwed that text up :) Your interpretation is correct. Even if we change the reverse deps, Yesod has users that have written PathPiece instances, so yesod may want to keep PathPiece around a while longer and implement default definitions in terms of http-api-data.

The plan makes sense, including ignoring PathMultiPiece for now.

[gregwebs]

actually, in terms of fixing the reverse deps, to avoid breakage during the transition, each reverse dep should still continue to define To/FromPathPiece (but not it can do it in terms of ApiData).

[gregwebs]

@fizruk are you interested in creating this library?

[fizruk]

@gregwebs I think so, yes. Thank you for the ping! I should have enough time this weekend if it's okay to wait just a little bit more :)

[gregwebs]

Sure, let me know if you need any help!

[fizruk]

I have made some progress, please see fizruk/http-api-data. Some questions arose in the process, see issues.

[fizruk]

The first version of http-api-data has just been released on Hackage. Now I am going to make PR to path-pieces to use http-api-data.