Amber: [Router] Allow multiple values for a single parameter key

[maiha]

Description

Amber::Router::ParamsParser can't handle multiple values for a parameter.

• https://github.com/amberframework/amber/blob/master/src/amber/router/params_parser.cr#L146-L147

      HTTP::Params.parse(values) do |key, value|
        params[key] = value

This always overwrites previous values where params is a Amber::Router::Params < Hash(String, String) .

Steps to Reproduce

require "amber"

request  = HTTP::Request.new("GET", "/?foo=bar&foo=baz")
response = HTTP::Server::Response.new(IO::Memory.new)
context  = HTTP::Server::Context.new(request, response)
context.params["foo"] # => "baz"

Expected behavior:

We should provide another method to fetch all values like Rails, Crystal, Kemal and any other libraries.

Crystal

require "http/params"

params = HTTP::Params.parse("foo=bar&foo=baz")
params.fetch("foo")     # => "bar"
params.fetch_all("foo") # => ["bar", "baz"]

Kemal

require "kemal"

request  = HTTP::Request.new("GET", "/?foo=bar&foo=baz")
params = Kemal::ParamParser.new(request)
params.query["foo"]           # => "bar"
params.query.fetch_all("foo") # => ["bar", "baz"]

Versions

• Amber CLI (amberframework.org) - v0.6.4
• Crystal 0.24.1 (2017-12-22)

Amber is a quite cool design, and I love it. 😄
I thinks it is time to polish it for production use! If no one has started this problem, I will try it.

Best regards,

[elorest]

In amber we've tried to unify all types of params into params rather than having to call params.query, params.body etc. This makes this a bit more difficult. I'm going to work on this for a bit.

[eliasjpr]

I believe this can be as simple as just appending to the param key @elorest

@maiha thank you for the thorough submission.

[robacarp]

I don't recall anything like this from Rails, and I don't see anything in their codebase that shows anything like params.fetch_all. My experience with it says later parameters simply overwrite so previous parameters aren't available. Unless you're thinking of hash and array parameters?

Apparently there is no standard (9th slide) for what to do when passing multiple parameters with the same name.

What's the benefit to passing multiple parameters with the same name?

[maiha]

Hi @robacarp

What's the benefit to passing multiple parameters with the same name?

It is necessary to process the select multiple form.
For example, <select name=lang multiple> would post parameters like this.

lang=ruby&lang=crystal

Thanks,

[drujensen]

In Rails you use a params array:

<select name="job[]" size="5" multiple="multiple">

The ending [] tells it you are sending an array.

[elorest]

@eliasjpr I'll submit something soon and you can let me know what you think. I've looked at kemals current implementation, and played with a way to make it work without changing our end functionality.

[robacarp]

I think I like the [] functionality like Rails and php have. At least that’s what’s familiar to me. But I assume it’d be easier to implement a known array type returning parameter method than to give the param method the possibility of returning an array for every param field.

Looking forward to a pull request!

[elorest]

It's difficult to add this functionality without breaking current functionality I care more about. I'm still trying a couple things though. @eliasjpr I don't understand your statement above, could you elaborate?

I believe this can be as simple as just appending to the param key @elorest

[elorest]

@marksiemers and I played with this last night and decided to create a fetch_all method which re-parses request and returns the entire array of params. This allows params to continue to function how they are without additional slow downs for most params which aren't expected to return arrays.

[eliasjpr]

Fixed with #637