[WIP] feat: add query params to request path

[YOU54F]

Initial stab at https://github.com/pact-foundation/pact-mock_service/issues/80

Background: the reason the query is a string in the pact is that initially, we threw away all the matching information and only stored the "actual" reified request in the pact (because the matching that takes place in the mock service was already done, and the stub service didn't exist at that stage). Later, we realised the matching info was useful info, so we added it in, as it was just an addition to the json, and would be ignored by the pact verification step. Changing the query string from a string to a hash would be a breaking change for the v2 specification, however. The code on the other side expects a string and not a hash. In v3, it has been changed to a hash, but only the v3 pact reading code has been done in the ruby, (and only the format change, not new features are supported) not the v3 writing code.

Here is the code that parses the v2 request, and merges the matching rules into the request structure to create a tree of nested matchers. https://github.com/pact-foundation/pact-support/blob/master/lib/pact/consumer_contract/interaction_v2_parser.rb

We're going to have to parse the query string, if it exists, and overwrite it in the request hash before it gets parsed into the Pact::MatchingRules.merge (incidentally, this will fix another issue we get about the matching rules not being applied correctly to the string query, when the rules are expecting a hash query!). The code for parsing the query is Rack::Utils.parse_nested_query(query_string). It'll make the code a bit ugly, but sometimes you've got to do what you've got to do!

Hey @bethesque

started looking at this now, based on your comments, I am assuming your after something like

in lib/pact/consumer_contract/interaction_v2_parser.rb

we need to

1. check if a query exists and if it does, then
2. parse it into a hash of params
3. convert it into a string
4. add it to hash['request']
5. call request = parse_request(hash['request'], options)

Been playing around and this is where I've got to

def self.call hash, options
      # Parse the query string assuming
      #         'request' => {
      #           'path' => '/path',
      #           'method' => 'get',
      #           'query' => 'param=thing&param2=blah'
      #         },
      if hash['request'].has_key? 'query'
      params = Rack::Utils.parse_nested_query(hash['request']['query'])
      # {"param"=>"thing", "param2"=>"blah"}
      query = URI.encode_www_form(params)
      # param=thing&param2=blah
      hash['request']['path'] = hash['request']['path'] << '?' << query
      # /path?param=thing&param2=blah
      end
      request = parse_request(hash['request'], options)
      response = parse_response(hash['response'], options)
      provider_states = parse_provider_states(hash['providerState'] || hash['provider_state'])
      Interaction.new(symbolize_keys(hash).merge(request: request, response: response, provider_states: provider_states))
    end

Hopefully I am on the right track?

[bethesque]

Life will be easier for you if you write a test first, I promise!

[YOU54F]

Life will be easier for you if you write a test first, I promise!

Yeah, you are fully correct, apologies, I've been a naughty boy, not writing any tests in pact-support, instead I published it as a gem under my own account pact-support-saf and pulled it into the pact-mock_service (I should know better, I am an automation tester =D)

Using your changes, I have got it working, in the way that I intended, also I had to change the matching rule slightly

Here is the original pact

{
  "consumer": {
    "name": "test-consumer"
  },
  "provider": {
    "name": "param-provider"
  },
  "interactions": [
    {
      "description": "a notification",
      "providerState": "Service is up and healthy",
      "request": {
        "method": "GET",
        "path": "/queryTest",
        "query": "param=123",
        "matchingRules": {
          "$.query.param[0]": {
            "match": "regex",
            "regex": "\\d+"
          }
        }
      },
      "response": {
        "status": 200,
        "headers": {
          "Content-Type": "application/json"
        },
        "body": ""
      }
    }
  ],
  "metadata": {
    "pactSpecification": {
      "version": "2.0.0"
    }
  }
}

with a matching rule being ignored

WARN: Ignoring unsupported matching rules {"match"=>"regex", "regex"=>"\\d+"} for path $['query']['param'][0]

{
    "message": "No interaction found for GET /queryTest?param=a",
    "interaction_diffs": [
        {
            "description": "a notification",
            "provider_state": "Service is up and healthy",
            "query": {
                "param": [
                    {
                        "EXPECTED": "123",
                        "ACTUAL": "a"
                    }
                ]
            }
        }
    ]
}

{
    "message": "No interaction found for GET /queryTest?param=123456",
    "interaction_diffs": [
        {
            "description": "a notification",
            "provider_state": "Service is up and healthy",
            "query": {
                "param": [
                    {
                        "EXPECTED": "123",
                        "ACTUAL": "123456"
                    }
                ]
            }
        }
    ]
}

Changing the matching rule to the following (dropping the [0])

        "matchingRules": {
          "$.query.param": {
            "match": "regex",
            "regex": "\\d+"
          }
        }

Gives me the expected behaviour,

Error with empty param or non-digits

{
    "message": "No interaction found for GET /queryTest?param=",
    "interaction_diffs": [
        {
            "description": "a notification",
            "provider_state": "Service is up and healthy",
            "query": {
                "param": [
                    {
                        "EXPECTED_TO_MATCH": "/\\d+/",
                        "ACTUAL": ""
                    }
                ]
            }
        }
    ]
}

and a 200 response with any digits

ie

http://localhost:8080/queryTest?param=123231121

I'll push up my change now, and write a test :)

PS, here is an example test, that would generate the pact in question, I'm not sure why the matcher gets encoded as "$.query.param[0]" and where to deal with that (in pact-support, or somewhere else)

    describe("query param matcher test", () => {
      it("should respond 200 when a notification is sent", async () => {
        const interaction: InteractionObject = {
          state: "Service is up and healthy",
          uponReceiving: "a notification",
          withRequest: {
            method: "GET",
            path: "/queryTest",
            query: {
              param: regex({generate: '123', matcher: "\\d+"})
            }
          },
          willRespondWith: {
            status: 200,
            body: "",
            headers: {
              "Content-Type": "application/json"
            }
          }
        };

        await provider.addInteraction(interaction);

        await getClient()
          .get("/notifications?param=123")
          .expect(200);
      });
    });

Thanks for taking the time to review!

[bethesque]

Looking much better!

The reason for the [0] is that the way query params are encoded in the pact (in v3 at least) is that every value is actually an array - if there's only one value, it's an array of 1. This is to support foo=1&foo=2 type queries.

[bethesque]

I can't see why dropping the [0] makes it work. The 0 is correct. There must be something else we're missing if it's still giving us that warning. See if you can work it out, but if you can't, then I'll have a look at it.

[YOU54F]

I can't see why dropping the [0] makes it work. The 0 is correct. There must be something else we're missing if it's still giving us that warning. See if you can work it out, but if you can't, then I'll have a look at it.

Weird isn't it. I'll do some digging 👍 and see what I can work out!

[YOU54F]

I can't seem to create a pact, whereby we see

          "$.query.param[0]": {
            "match": "regex",
            "regex": "\\d+"
          },
          "$.query.param[1]": {
            "match": "regex",
            "regex": "\\d+"
          }

With the following test

    describe("query param matcher test2", () => {
      it("should respond 200 when 2 param1's are sent", async () => {
        const interaction: InteractionObject = {
          state: "Service is up and healthy",
          uponReceiving: "a notification2",
          withRequest: {
            method: "GET",
            path: "/paramTests",
            query: {
              'param1': eachLike('123'),
            },
          },
          willRespondWith: {
            status: 200,
            body: "",
            headers: {
              "Content-Type": "application/json"
            }
          }
        };

        await provider.addInteraction(interaction);

        await getClient()
          .get("/paramTests?param1=12345&param1=54321")
          .expect(200);
      });
    });

my pact becomes

{
  "consumer": {
    "name": "test-consumer"
  },
  "provider": {
    "name": "param-provider"
  },
  "interactions": [
    {
      "description": "a notification2",
      "providerState": "Service is up and healthy",
      "request": {
        "method": "GET",
        "path": "/paramTests",
        "query": "param1=123",
        "matchingRules": {
          "$.query.param1": {
            "min": 1
          },
          "$.query.param1[*].*": {
            "match": "type"
          }
        }
      },
      "response": {
        "status": 200,
        "headers": {
          "Content-Type": "application/json"
        },
        "body": ""
      }
    }
  ],
  "metadata": {
    "pactSpecification": {
      "version": "2.0.0"
    }
  }
}

I've created a pact with a few different query param variations which I will look to test to check the emergent behaviour with this change.

• single param
• 2 of the same param, with diff values
• 2 different params

{
  "consumer": {
    "name": "test-consumer"
  },
  "provider": {
    "name": "param-provider"
  },
  "interactions": [
    {
      "description": "a notification",
      "providerState": "Service is up and healthy",
      "request": {
        "method": "GET",
        "path": "/paramTests",
        "query": "param1=12345",
        "matchingRules": {
          "$.query.param1[0]": {
            "match": "regex",
            "regex": "\\d+"
          }
        }
      },
      "response": {
        "status": 200,
        "headers": {
          "Content-Type": "application/json"
        },
        "body": ""
      }
    },
    {
      "description": "a notification2",
      "providerState": "Service is up and healthy",
      "request": {
        "method": "GET",
        "path": "/paramTests",
        "query": "param1=123",
        "matchingRules": {
          "$.query.param1": {
            "min": 1
          },
          "$.query.param1[*].*": {
            "match": "type"
          }
        }
      },
      "response": {
        "status": 200,
        "headers": {
          "Content-Type": "application/json"
        },
        "body": ""
      }
    },
    {
      "description": "a notification3",
      "providerState": "Service is up and healthy",
      "request": {
        "method": "GET",
        "path": "/paramTests",
        "query": "param1=12345&param2=12345",
        "matchingRules": {
          "$.query.param1[0]": {
            "match": "regex",
            "regex": "\\d+"
          },
          "$.query.param2[0]": {
            "match": "regex",
            "regex": "\\d+"
          }
        }
      },
      "response": {
        "status": 200,
        "headers": {
          "Content-Type": "application/json"
        },
        "body": ""
      }
    }
  ],
  "metadata": {
    "pactSpecification": {
      "version": "2.0.0"
    }
  }
}

Updated example tests are here :- https://github.com/YOU54F/jest-pact-typescript/blob/master/src/pact/client/requestPathMatching.pacttest.ts

[YOU54F]

I've added the three pact files,

Archive.zip

pact.1.json - single param pact.2.json - 2 of the same param, with diff values pact.3.json - 2 different params

pact.1.json and pact.3.json will work in the way I intended with dropping the [0] from the pact so there is deffo some additional work required in pact support, pact.2.json, I can't get it to not give me the matcher warnings, no matter what I do

[YOU54F]

just added a test where we have two params with the same name, but diff values and it fails

        {
          "description" => "description",
          "request" => { "method" => "GET", "path" => "/" ,
          "query" => 'param1=thing1&param2=thing2'},
          "response" => { "status" => 200 },
          "providerState" => "foo"
        }

  1) Pact::InteractionV2Parser.call with 2 of the same query params but with diff values query params encodes them as a hash
     Failure/Error: expect(subject.request.query).to include :param1 => ["thing1"]

       expected {:param1 => ["thing2"]} to include {:param1 => ["thing1"]}
       Diff:
       @@ -1,2 +1,2 @@
       -:param1 => ["thing1"],
       +:param1 => ["thing2"],

[YOU54F]

I think is now complete as per https://github.com/pact-foundation/pact-support/commit/faff17c331da6aaf3f48c7d14fff9e3bba4b60cd so closing!