Support flexible matching in request path

Question

Support flexible matching in request path

mefellows opened this issue 7 years ago · 8 comments

Feature request from 2017 Developer survey:

Flexible path matching (especially for the javascript mock server) to allow dynamic resource ids e.g. /employee/{id}.

There was an issue on the pact-js repository that I can't seem to find.

Answer 1 · 2017-11-13T23:04:56.000Z

You can do this with normal terms on the consumer side, but we'll need the generators (which are like terms for the provider) for doing it on the provider side. Eg. You can set up the mock service to match a request for any employee /employee/{id}, and then run the verification against against a known id /employee/1. Generators will give us the ability to mock /employee/1, but then have the provider decided which ID to use when verifying.

Answer 2 · 2019-06-01T00:49:52.000Z

I'm trying to attempt this and failing :(

I am trying to create a pact that will accept a query param with a UUID, and return a valid response.

We are trying to load the pact, into pact-stub-service and would like it so that pact-stub-service will replicate the behaviour in our test, whereby it will use the matcher and allows us to accept any UUID.

My two tests, one using a regex matcher in query string and one in the path

jestpact.pactWith(
  { consumer: "test-consumer", provider: "param-provider", port },
  async (provider: any) => {
    describe("query param matcher test", () => {
      it("should respond 200 when a notification is send", async () => {
        const interaction: InteractionObject = {
          state: "Service is up and healthy",
          uponReceiving: "a notification",
          withRequest: {
            method: "GET",
            path: "/notifications",
            query: {
              signingId: regex({generate: '9eb2484d-405e-4ff7-bc3a-b0181e4efb30', matcher: "\\d+"})
            }
          },
          willRespondWith: {
            status: 200,
            body: "",
            headers: {
              "Content-Type": "application/json"
            }
          }
        };

        await provider.addInteraction(interaction);

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

    describe("path param matcher test", () => {
      it.only("should respond 200 when a notification is send", async () => {
        // const signingId = regex({generate: '9eb2484d-405e-4ff7-bc3a-b0181e4efb30', matcher: "\\d+"})

        const interaction: InteractionObject = {
          state: "Service is up and healthy",
          uponReceiving: "a notification",
          withRequest: {
            method: "GET",
            path: term({ generate: '/notifications?signingId=123', matcher: '/notifications\\?signingId\\=\\d+' })
            // query: {
            //   signingId: 
            // }
          },
          willRespondWith: {
            status: 200,
            body: "",
            headers: {
              "Content-Type": "application/json"
            }
          }
        };

        await provider.addInteraction(interaction);

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

The first test will generate this pact file

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

Which when loaded into the pact-stub-service, it returns this, and the pact is hardcoded to the value specified in the pact, rather than the regex

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

Using the 2nd test, it fails to match an interaction during the pact test, as when the request is received by the mock service used by pact-js, it strips the query off the path, and can't match

I, [2019-06-01T01:34:55.081516 #93758]  INFO -- : Registered expected interaction GET /notifications?signingId=123
D, [2019-06-01T01:34:55.081787 #93758] DEBUG -- : {
  "description": "a notification",
  "providerState": "Service is up and healthy",
  "request": {
    "method": "GET",
    "path": {
      "json_class": "Pact::Term",
      "data": {
        "generate": "/notifications?signingId=123",
        "matcher": {"json_class":"Regexp","o":0,"s":"/notifications\\?signingId\\=\\d+"}
      }
    }
  },
  "response": {
    "status": 200,
    "headers": {
      "Content-Type": "application/json"
    },
    "body": ""
  }
}
I, [2019-06-01T01:34:55.089368 #93758]  INFO -- : Received request GET /notifications?signingId=123
D, [2019-06-01T01:34:55.089475 #93758] DEBUG -- : {
  "path": "/notifications",
  "query": "signingId=123",
  "method": "get",
  "headers": {
    "Host": "localhost:9999",
    "Accept-Encoding": "gzip, deflate",
    "User-Agent": "node-superagent/3.8.3",
    "Connection": "close",
    "Version": "HTTP/1.1"
  }
}
E, [2019-06-01T01:34:55.089577 #93758] ERROR -- : No matching interaction found for GET /notifications?signingId=123
E, [2019-06-01T01:34:55.089598 #93758] ERROR -- : Interaction diffs for that route:
E, [2019-06-01T01:34:55.089616 #93758] ERROR -- : 
W, [2019-06-01T01:34:55.096614 #93758]  WARN -- : Verifying - actual interactions do not match expected interactions. 
Missing requests:
	GET /notifications?signingId=123

Unexpected requests:
	GET /notifications?signingId=123


W, [2019-06-01T01:34:55.096654 #93758]  WARN -- : Missing requests:
	GET /notifications?signingId=123

Unexpected requests:
	GET /notifications?signingId=123


I, [2019-06-01T01:34:55.103560 #93758]  INFO -- : Cleared interactions

I have an example repo here

You can run the tests with the following but please note that currently the 2nd test in src/pact/client/params.pacttest.ts will fail due to the reason above, so you may want to do a test.only on line 17 before you run step 4

git clone git@github.com:YOU54F/jest-pact-typescript.git
git checkout queryParamMatching
yarn install
yarn run pact-test
cd docker/pact-stub-service
make copy-pacts
cd ../docker
docker-compose up

The pact-stub server will spin up and you will see the matching warning.

if you send

curl -v http://localhost:8084/notifications?signingId=9eb2484d-405e-4ff7-bc3a-b0181e4efb30 will work

curl -v http://localhost:8084/notifications?signingId=9eb2484d-405e-4ff7-bc3a-b0181e4efb31 will return an error

    "message": "No interaction found for GET /notifications?signingId=9eb2484d-405e-4ff7-bc3a-b0181e4efb31",
    "interaction_diffs": [
        {
            "description": "a notification",
            "provider_state": "Service is up and healthy",
            "query": {
                "EXPECTED": "signingId=9eb2484d-405e-4ff7-bc3a-b0181e4efb30",
                "ACTUAL": "signingId=9eb2484d-405e-4ff7-bc3a-b0181e4efb31"
            }
        }
    ]
}

We have had to switch to wiremock as an interim measure :( which makes me most sad.

@YOU54F

Answer 3 · 2019-06-01T01:30:33.000Z

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!

Answer 4 · 2019-06-09T21:35:27.000Z

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

check if a query exists and if it does, then
parse it into a hash of params
convert it into a string
add it to hash['request']
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? Happy to be corrected if I am going completely arse about face =D

Answer 5 · 2020-11-02T21:26:04.000Z

Is there anything that can be done to help move this forward? We've started running into this issue with matchers around query params not properly being reflected when we run pact-stub-service.

Answer 6 · 2020-11-03T21:06:36.000Z

@mikahjc trying to get my head back around this issue. Can you give me the exact scenario that's causing you problems?

Answer 7 · 2020-11-03T23:04:20.000Z

Ok, I think I've fixed it. Try upgrading and see how it goes.

Answer 8 · 2020-11-04T15:45:01.000Z

Seems to be working now. For reference, the scenario we were running into was described at the end of YOU54F's first comment: we could get the stub service to work by sending the exact query param + value defined in the contract in the request, but if you changed the value it would consider it not a match and return 500 even if it should match the given matcher in the contract.