Crystal: Request: HTTP::Params.encode Array value support

[Daniel-Worrall]

Coming across #5172 and interacting with APIs that take multiple of the same param, it would be very handy to be able to pass in an array of values to the parameter to be parsed out. I don't think there's a different expectation that could be made to passing in an array of values

data = {"key" => ["a", "b"], "foo" => "bar"}

puts HTTP::Params.encode(data) # => "key=a&key=b&foo=bar"

https://carc.in/#/r/702o

[Sija]

You can do it by passing Hash(String, Array(String)) to the HTTP::Params constructor and invoking #to_s.

data = {"key" => ["a", "b"], "foo" => ["bar"]}

HTTP::Params.new(data).to_s # => "key=a&key=b&foo=bar"

https://carc.in/#/r/702v

---

As a side note, it might be worth extending .encode to handle such case(s).

[vladfaust]

If we speak about keeping stdlib slim, there is https://github.com/vladfaust/http-params-serializable which acts like JSON::Serializable, but for HTTP params :slightly_smiling_face: It supports arrays and nested stuff.

[straight-shoota]

Sounds like a good idea to add an overload to HTTP::Params.encode delegating to .new and #to_s, as in the example by @Sija.

[bcardiff]

Or to extend the HTTP::Params::Builder to handle array like values in https://github.com/crystal-lang/crystal/blob/master/src/http/params.cr#L347

[rodrigopinto]

I think overloading the HTTP::Params.encode is a way to go but also extending the HTTP::Params::Builder would allow to do HTTP::Params::Builder#add("key", ["a", "b"]).

As no one started it yet, I can take care of this implementation. Is it ok?

[rodrigopinto]

Working on the PR I realized that HTTP::Params.encode(hash : Hash(String, String)) signature is restrictive in case someone wishes to passes other values types like Int32, 'Float64`, etc.

As we are implementing a method signature with hash : Hash(String, String | Array(String) for this issue, I was wondering if would make sense to have a Union type to allow other types of params for the value part of the hash.

About @Sija suggestion, we could extend the new + to_s, but would match in the same situation as it expects a Hash(String, Array(String)). WDYT?

[oprypin]

I honestly don't understand why you'd need a method that accepts string-or-array-of-strings when you already have one that accepts array. Just use that one and don't add code smells. And honestly, who ever needs to write out the params as a literal? Are we optimizing just for this far-fetched example?

[oprypin]

signature is restrictive in case someone wishes to passes other values types like Int32

Well indeed, it can't actually store any other types, only string. What's your point? Prefer hiding potential errors over adding one .to_s call to your code, as it should be done?

[oprypin]

Hmm forget my first comment. It is actually quite normal to write out such literals, e.g. when making a client for an API. And indeed some params can be single-value and some "multi-value".
I support this, at least as currently implemented in that pull request.

[rodrigopinto]

Well indeed, it can't actually store any other types, only string. What's your point? Prefer hiding potential errors over adding one .to_s call to your code, as it should be done?

My point is to reduce the duplication from the clients/frameworks that would require to _stringify _
the parameters before passing it to be encoded.