Support duplicate form names in multipart forms

[ixti]

Resolves: httprb/http#663

[ixti]

This change is 

[mathisto]

@ixti I added a line to your spec for multipart 'pairs'. I think that a boundary is required between each 'body part' as per RFC 1341 7.2. You should double check me though, I don't wade in these water's every day.

I altered the coerce method to take an Array or Hash. We had talked about a more generic Enumerator but I think it might be better to restrict rather than abstract in this case.

Additionally, I added a spec helper config that was really helpful for figuring out what was failing. I suspect the config is not everyone's cup of tea since the default max_formatted_output_length is 200. Let me know if you want it ganked or if it can/should stay.

Let me know what you think. Would love to talk shop.

[ixti]

@ixti I added a line to your spec for multipart 'pairs'. I think that a boundary is required between each 'body part' as per RFC 1341 7.2. You should double check me though, I don't wade in these water's every day.

Yeah, thank you. It was unintentional typo from my end.

I altered the coerce method to take an Array or Hash. We had talked about a more generic Enumerator but I think it might be better to restrict rather than abstract in this case.

After some consideration, I think I'm actually more in favour of more flexible handling. What I mean is, we allow this:

{ :foo => [1, 2, 3] }

That is supported now, and will create 3 parts with same name, but different values (1, 2, and 3), so we should support at least:

[:foo, [1, 2, 3]]

Which we do with this RP currently. But why not allow any Enumerable? For example:

some_array.lazy.select(&some_filter)

Right now we will convert the above to Hash and if it yields duplicate keys - it will be inconsistent with Array.

Additionally, I added a spec helper config that was really helpful for figuring out what was failing. I suspect the config is not everyone's cup of tea since the default max_formatted_output_length is 200. Let me know if you want it ganked or if it can/should stay.

I think I'm more against this one than agree with. :D It can be easily done local-only adding:

# file: .rspec-local
--require ~/.my-rspec-config.rb

# file: ~/.my-rspec-config.rb

RSpec.configure do |config|
  # overwrite project's config as you feel it :D
end

[ixti]

@mathisto I have rewrote tests a bit (and added similar test to urlencoded one too).

[ixti]

After giving it a second thought, Rack and Addressable support only nil, Array, and Hash. So, I guess I'm fine with proposed solution.

[ixti]

@mathisto can you, please, make sure that both urlencoded and multipart encoders behave consistently?

[ixti]

The more I think the more I believe that there's no point in limiting to Array/Hash. I mean we really only care that it's an Enumerable that yields at one or two arguments, and we don't care about the rest:

[[1], [2, 3], [4, 5, 6]].map { |k, v| "#{k}=#{v}" }.join("&")
# => "1=&2=3&4=5"

{ 1 => nil, 2 => 3 }.map { |k, v| "#{k}=#{v}" }.join("&")
# => "1=&2=3"

Enumerator
  .new { |y| y << [1] << [2, 3] << [4, 5, 6] }
  .map { |k, v| "#{k}=#{v}" }
  .join("&")
# => "1=&2=3&4=5"

So, I'm not really sure why we treat Hash and Array specially. I agree, that Enumerator or any kind of Enumerable as an input value makes not much sense, and probably will never be used, but I don't really see why would treating Hash and Array only is better than being less strict. :))

[ixti]

In fact I think I would accept any input that responds to :each, but in most cases those that implement #each, also include Enumerable, thus, I believe Enumerable is the best candidate :D

[mathisto]

Ok I like it. Especially dropping the coerce. I'll knock this out tonight after the kids are in bed.