Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for nested hashes #84

Closed
wants to merge 3 commits into from

Conversation

mariochavez
Copy link

Without this Weary can't be upgraded to latest addressable gem version

So now we can pass to query_values a hash like:

{:public=>true, :files=>{"file1.txt"=>{:content=>"String file contents"}}}

And get it converted to:

{"public"=>true, "files[file1.txt][content]"=>"String file contents"}

Which query_params will finally convert to:

["flag1=value1", "flag2%5Bflag3%5D%5Bflag4%5D=value2"] or ["flag1=value1", "flag2[flag3][flag4]=value2"]

But this change broke the following tests where the hashes have become valid:

  1. Addressable::URI when assigning query values should raise an error attempting to assign {'a' => {'b' => ['c']}}
    Failure/Error: (lambda do
    expected TypeError but nothing was raised

    ./spec/addressable/uri_spec.rb:4925:in `block (2 levels) in <top (required)>'

  2. Addressable::URI when assigning query values should raise an error attempting to assign {:b => '2', :a => {:c => '1'}}
    Failure/Error: (lambda do
    expected TypeError but nothing was raised

    ./spec/addressable/uri_spec.rb:4932:in `block (2 levels) in <top (required)>'

  3. Addressable::URI when assigning query values should raise an error attempting to assign {:a => 'a', :b => {:c => true, :d => 'd'}}
    Failure/Error: (lambda do
    expected TypeError but nothing was raised

    ./spec/addressable/uri_spec.rb:4959:in `block (2 levels) in <top (required)>'

  4. Addressable::URI when assigning query values should raise an error attempting to assign {:a => 'a', :b => {:c => true, :d => 'd'}}
    Failure/Error: (lambda do
    expected TypeError but nothing was raised

    ./spec/addressable/uri_spec.rb:4968:in `block (2 levels) in <top (required)>'

{"a"=>{"b"=>["c"]}} => {"a[b]"=>["c"]}
{:b => '2', :a => {:c => '1'}} => {"b"=>"2", "a[c]"=>"1"}
{:a=>"a", :b=>{:c=>true, :d=>"d"}} => {"a"=>"a", "b[c]"=>true, "b[d]"=>"d"}
{:a => 'a', :b => {:c => true, :d => 'd'}} => {"a"=>"a", "b[c]"=>true, "b[d]"=>"d"}

Please advice if this patch is ok and if I should just delete failed tests since their test cases are not valid anymore with this patch.

@travisbot
Copy link

This pull request fails (merged 27c7f56 into ee198d4).

@sporkmonger
Copy link
Owner

Provisionally closing this as I won't accept pull requests with failing tests under any circumstances. This issue has already been covered in #77.

I'm somewhat open to solutions that support nested hashes as input with the understanding that these would be normalized to the currently supported flattened form and completely unsupported on the parsing side of things. However, I remain reluctant even there because, as I've said elsewhere, I think nested structures being shoved into query parameters are a bad idea generally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants