Support duplicate keys in `Query`
There are multiple issuess with current Query representation:
-
fullPath- doesn't return original value of uri. We are not able to call it "equivalent" from my perspecitve, -
We are trying to start using HTTPure not only for API based endpoints and we want to be able to receive data from standard
HTMLforms.HTMLallows you to send multiple values for single querykey(like with<select multiple name="key">) and it is quite common to build group of checkboxes with the same key too as multichoice widget.
I have two propositions assuming that we don't want to support empty values (like key1&key2):
-
type Query = Array (Tuple String String)with additional helper which converts this toMap -
type Query = Map String (Array String)
We are ready to work on this and provide a PR.
Additional options:
-
add original uri value to
Request -
leave query as a
String
I like the proposal to use a Map String (Array String). I had wanted to convert it to a Map String String anyways, and I think that's a clean way to handle multiple values while enabling things like the Lookup typeclass to be used naturally.
I'd suggest before getting too deep into implementation, take a look at this example and open the PR with the changes to that example that would be necessary for your proposal (including extending it to use multiple values for the same parameter) so we can discuss on sample code.
Regarding fullPath, the original purpose there was mostly for logging. It probably should be equivalent to the original request URL, but not identical. Can you speak to specific ways in which it is not equivalent, and if we fixed those, would that be sufficient or would you still need the original URL?
I'd suggest before getting too deep into implementation, take a look at this example (...)
Thanks for suggestions!
Regarding fullPath (..) Can you speak to specific ways in which it is not equivalent
I think that it is not equivalent because Query representation looses information in the current implementation. Even if we have Map it won't be equivalent because possible key order could be different etc. Even if we have Array there still be possiblity about + encoding etc. etc.
would that be sufficient or would you still need the original URL?
I think that we should provide original uri because uri RFC doesn't say anything specific about the shape of for example query value and parsing is somewhat an arbitrary choice from our side (for example purescript-uri in Extras provides different representation).
I think that we should leave users an option to interpret query in different manner (for example parsing it with purescript-uri etc.).
Ok, those are good points, I've opened https://github.com/cprussin/purescript-httpure/issues/130 to track exposing the original request URL.
I have additional design question.
Maybe we can extend low level API of serve' and serveSecure' so they accept additional configuration which can be used in our final fromHTTPRequest for parsing HTTP.Request parts. Then we can keep our nice default high level API but also allow users to provide their own parsers so they can avoid additional per request parsing from our side which they are not interested in.
What do you think?
Yeah, I like that proposal. Let's split out a separate issue to track determining the right API to expose and implementing it
As a first step I've added additional tests for query parsing which are failing in the current implementation:
https://github.com/paluh/purescript-httpure/commit/09311a6a9f4bf3daf3514e524a71a8796c59c3b5
Do you agree that we don't want to parse single equal sign as correct key definition or should we treat it as a "=" key with empty value?
I mean for example (pseudo notation and key=value added for "readability") query ?key=value&= do we want to have:
-
{ "key": ["value"], "": [""]}or -
{ "key": ["value"] }or -
{ "key": ["value"], "=": [""]}?
I am leaning towards following what Express does here, which is to ignore the empty =:
const express = require('express');
const app = express();
app.get('/', (req, res) => res.send(req.query));
app.listen(3000, () => console.log('App up'));
$ curl "localhost:3000?foo=bar&="
{"foo":"bar"}
This isn't something I feel particularly strongly about and I can be convinced otherwise, but I tend to think that httpure should follow express conventions for this kind of thing.