Soostone / uri-bytestring

Haskell URI parsing as ByteStrings
BSD 3-Clause "New" or "Revised" License
35 stars 28 forks source link

query flags #12

Open mf59816 opened 9 years ago

mf59816 commented 9 years ago

What would you say to a pull request changing

newtype Query = Query { queryPairs :: [(ByteString, ByteString)] }

to

newtype Query = Query { queryParams :: [QueryParam] }

data QueryParam
  = QueryParam ByteString ByteString
  | QueryFlag ByteString

queryPairs :: Query -> [(ByteString, ByteString)]

It is a breaking change for everybody who matches against the newtype, but it's easy enough to fix, and I think it'd be worth it.

For discussion, see: https://github.com/haskell-servant/servant/issues/139#issuecomment-114752961

alpmestan commented 9 years ago

Note that it could also be accomplished with:

newtype Query = Query { queryPairs :: [(ByteString, Maybe ByteString)] }

if that's any better. (I think that's how http-types does it)

ozataman commented 9 years ago

While waiting for @MichaelXavier to respond to the idea, I'm curious: Does the RFC support the flag/matrix parameters as mentioned in the discussion above? In general, we tried to adhere to the standard pretty closely in uri-bytestring, only with a few escape valves for lax parsing, etc.

Of the two possible forms, I think I prefer @alpmestan's suggestion a bit more for clarity, flexibility and less indirection. We could certainly still have the queryPairs function for convenience.

mf59816 commented 9 years ago

Re query flags, I think http://tools.ietf.org/html/rfc3986#section-3.4 is the relevant section of the standard. Disappointingly, it defines the query as an opaque string, so there aren't even any binding rules for non-flag key-value pairs:

      query       = *( pchar / "/" / "?" )

In my opinion that means we are not moving further away from the rfc by implementing this issue.

christian-marie commented 9 years ago

As for matrix params, they're basically just a convenient convention. The characters are allowed in path segments, but they aren't actually a thing. This is the most "official" thing I've ever been able to dig up on them:

http://www.w3.org/DesignIssues/MatrixURIs.html

MichaelXavier commented 9 years ago

I like @alpmestan's solution the best for "flags". Its hard to take a really firm stance since the standard is so vague and doesn't even account for the degree of parameter parsing we have today, so the only guidance is my gut and API usability. Correct me if I'm wrong but flags seem like they're just queries without values and no =. Using a Maybe seems like a gentle way of distinguishing an empty value from no = or value provided at all. There's plenty of machinery to handle maybes. Having an ADT case saying it represents a flag presumes a bit too much and would definitely make processing URIs a bit too difficult.

This is the first time I'm hearing about Matrix params but I'll discuss that in the appropriate issue #13

mf59816 commented 9 years ago

I think distinguishing the cases &flag=& and &flag& will make things more fragile rather than more convenient. Communication parties will surprisingly interchange the two, and either the uri-bytestring application will just ignore the value anyway because it already knows it's a flag, or get confused by a Just "" where it expects a Nothing.

I would rather just turn all empty ByteStrings into Nothings during parsing.

MichaelXavier commented 9 years ago

@mf59816 I can see that argument as long as the semantics are well-documented in the code. If the user doesn't care about this distinction, its easy to add fromMaybe "".

mf59816 commented 9 years ago

After a bit more thinking I'm not so sure any more. The fact that there can only be one of Just "" and Nothing, and the two are semantically equivalent, is bothering me, and it would make for more documentation.

But I have yet another idea. What about keeping the query as an opaque string (the minimum structure required by the RFC), and then doing something aeson-ish like this:

class IsQueryPair queryPair where
    parseQuery :: [(ByteString, ByteString)] -> Maybe [queryPair]
    parseMatrixQuery :: [(ByteString, ByteString)] -> Maybe [queryPair]
    toQuery ...
    toMatrixQuery ....

decodeQuery :: ByteString -> Maybe [queryPair]
....

This, together with making the path a list of segments (as specified in the standard), would make for both very flexible and quite strongly typed code: everybody could just decide for themselves how their queries should be structured.

domenkozar commented 8 months ago

We've written the following helper for the current api:

getQueryParam :: URI.URIRef URI.Absolute -> ByteString -> Maybe ByteString
getQueryParam uri param =
  map snd $ head $ filter (\(key, _) -> key == param) $ URI.queryPairs $ URI.uriQuery uri