Multi-value query parameters are not sent to the provider correctly

[daniel-chambers]

In the scenario where the consumer is making a request like:

"request": {
    "method": "GET",
    "path": "/query",
    "query": {
        "such": "wow",
        "test": ["a","b"]
    }

I would expect the verifier to make a call to the provider with the URL /query?such=wow&test=a&test=b. This does not appear to be happening.

I've made a branch here where I've added to the test cases in this project to reproduce this problem. I've basically added a new endpoint to the api.rb that responds with the query string it receives, and a PACT that verifies the expected response has both values for the query string parameter in it. This pact is then run through verification in test.sh and fails where it should succeed.

  1) Verifying a pact between me and they Greeting with GET /query?such=wow&test=a&test=b returns a response which has a matching body
     Failure/Error: expect(response_body).to match_term expected_response_body, diff_options

       Actual: {"greeting":"Query: such=wow&test=b"}

       @@ -1,4 +1,4 @@
        {
       -  "greeting": "Query: such=wow&test=a&test=b"
       +  "greeting": "Query: such=wow&test=b"
        }

       Key: - means "expected, but was not found".
            + means "actual, should not be found".
            Values where the expected matches the actual are not shown.

The output seems to indicate the verifier is making the request to the correct URL (on the first line), but that appears to be a lie, since a manual curl to the webserver using that URL (http://localhost:4567/query?such=wow&test=a&test=b) returns:

{"greeting":"Query: such=wow&test=a&test=b"}

Please forgive any sins in the changes on that branch; I don't write Ruby so I'm just feeling in the darkness here.

[mefellows]

Thanks @daniel-chambers, we'll have to look into this. FYI I won't be able investigate until next week if I'm lucky or the week after (I'm away this weekend and in Sydney for work most of next week).

[bethesque]

I'll have a look when I have a moment. Thanks for making a glorious reproducible example!

[bethesque]

Ok, this is a tricky one. The problem is occurring because the rack code (rack is the underlying http request framework that is use by the majority of ruby web servers) expects the name of a parameter that has multiple values to be suffixed with a [] to indicate that it is an array. If you change your query to {"such":"wow", "test[]": ["a","b"]} and your expectation to "Query: such=wow&test[]=a&test[]=b" it passes.

If the name is not suffixed with a [] then when the query is parsed into a Hash, the existing value gets overridden (as you probably would have surmised already).

If this is not an option for you, it's going to be tricky to solve without potentially causing backwards compatibility issues for other users. I'm not saying it's impossible, but I'm not sure how we'd do it yet.

[bethesque]

According to the all authoritative stackoverflow here and here, it seems that there is no HTTP spec for what the correct behaviour should be when there are multiple parameters with the same name. Different frameworks have different behaviours. Rack Test has taken the stance that the last value is the "correct" one, as this test implies.

    it "parses query strings with repeated variable names correctly" do
      request "/foo?bar=2&bar=3"
      last_request.GET.should == { "bar" => "3" }
    end

Using [] to ensure that all values are maintained and are treated as an array seems to be the defacto standard (jQuery, Rails, Django, php at least) and is the one I will recommend to you. I have managed to do some dirty dirty Ruby monkey patching to make your example pass, but it's not code that I can put in the main codebase without breaking the existing behaviour for other users (which I'm obviously not going to do).

I'm going to leave the code here for future reference, but it is very very dirty!

require 'rack/test'

module RackTestSessionHack
  def parse_nested_query *args
    Rack::Utils.parse_query *args
  end
end

Rack::Test::Session.send(:include, RackTestSessionHack)

module Rack
  module Test

    module Utils
      def build_nested_query(value, prefix = nil)
        case value
        when Array
          value.map do |v|
            # unless unescape(prefix) =~ /\[\]$/
            #   prefix = "#{prefix}[]"
            # end
            build_nested_query(v, "#{prefix}")
          end.join("&")
        when Hash
          value.map do |k, v|
            build_nested_query(v, prefix ? "#{prefix}[#{escape(k)}]" : escape(k))
          end.join("&")
        when NilClass
          prefix.to_s
        else
          "#{prefix}=#{escape(value)}"
        end
      end
    end
  end
end

[mefellows]

Great spot @bethesque! I seemed to recall the [] being standard, but as you say, annoyingly it's quite ambiguous.

Hopefully most modern web frameworks can deal with both cases.

@uglyog this is something to consider for the shared library, ideally we can support both methods of specifying array query string values.

[daniel-chambers]

I'm personally not so certain the the library's implementation framework's opinions about query parameters should leak through here. It seems strange to me that something that is supposed to replay recorded requests (recorded in JSON, not even in Ruby) should not be able to replay them faithfully as they were recorded.

I have no problem with the api.rb consumer here not being able to accept multi-value parameters, that's fine. In my opinion the mock provider, which is simply replaying recorded requests, ought to be able to replay them as they are defined in the JSON pact spec. And that pact spec clearly allowed for multi-value params (doco example).