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

Requiring the exact body for an expectation is not scalable #33

Closed
uglyog opened this issue May 1, 2014 · 18 comments
Closed

Requiring the exact body for an expectation is not scalable #33

uglyog opened this issue May 1, 2014 · 18 comments

Comments

@uglyog
Copy link
Member

uglyog commented May 1, 2014

Having to provide the exact body for a request is not scalable. Have a look at the resulting ruby pact issue #1

My current project has json bodies that include UUIDs and timestamps, which will never be known beforehand.

By adding a path to a matcher, it would mean I could only match on the things that are important in the body.

@uglyog
Copy link
Member Author

uglyog commented May 2, 2014

As an example, I currently need to match a body of the following form:

    {
        "event_id": "26c7343d-a19c-4bb7-88a2-107079037b17",
        "timestamp": "2014-04-14T07:02:59.154Z",
        "host": "rholshausen.engr.acx",
        "ip_address": "0:0:0:0:0:0:0:1%0",
        "event_type": "DOCUMENT:EVENT_VIEW_VIEWED",
        "user": {
            "id": 1879049107,
            "username": "poleary",
            "organisation": 1879048492
        }, "document": {
            "id": 1879099218
        }
    }

But I'm only interested in the values for event_type, user.id and document.id. And I don't care about the structure, which can change based on some context.

I would like to say something like (in ruby like pseudocode):

.uponReceiving("a PUT request with an Event in JSON format", "PUT",
    "/events/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}", headers, 
    aBodyThatMatches(
        '/eventType' => "DOCUMENT:EVENT_VIEW_VIEWED",
        '/user/id' => 1879049107,
        '/document/id' => 1879099218
    )
)

This code can match both a JSON and XML response.

@bethesque
Copy link
Member

This assumes that you're using path matchers to pull the values out of the request/response, in the actual code, yes?

@bethesque
Copy link
Member

If this was a get request, how would you specify which event would actually be queried for?

@bethesque
Copy link
Member

Ah, I think I see what you mean about not specifying the full body, you're talking about the request, not the response. We allow extra values in the response, but not in the request, because we don't want data to leak out. There is a way to turn off this request validation in the ruby pact library, but because I think it's a bad thing (and Martin Fowler agrees with me!), I've not advertised it :P

Do you really think it's a good feature to have?

@uglyog
Copy link
Member Author

uglyog commented May 2, 2014

yeah, sorry. This is in the consumer test specifying the request matching.

@kenbot
Copy link

kenbot commented May 2, 2014

We absolutely need to keep the Ruby and JVM pacts in sync, regarding
matching capabilities. It looks like, from the discussion linked above,
that the Ruby issue hasn't been fixed either, right? If so, we have a
blank slate to come up with a scheme that can do whatever matching we need
maybe with JSON path-navigation syntax or regexes or whatever.

The implementation complexity could easily get out of hand here, so we
would need to choose wisely.

On 2 May 2014 12:17, Ronald Holshausen notifications@github.com wrote:

As an example, I currently need to match a body of the following form:

{
    "event_id": "26c7343d-a19c-4bb7-88a2-107079037b17",
    "timestamp": "2014-04-14T07:02:59.154Z",
    "host": "rholshausen.engr.acx",
    "ip_address": "0:0:0:0:0:0:0:1%0",
    "event_type": "DOCUMENT:EVENT_VIEW_VIEWED",
    "user": {
        "id": 1879049107,
        "username": "poleary",
        "organisation": 1879048492
    }, "document": {
        "id": 1879099218
    }
}

But I'm only interested in the values for event_type, user.id and
document.id. And I don't care about the structure, which can change based
on some context.

I would like to say something like (in ruby like pseudocode):

.uponReceiving("a PUT request with an Event in JSON format", "PUT",
"/events/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}", headers,
aBodyThatMatches(
'/eventType' => "DOCUMENT:EVENT_VIEW_VIEWED",
'/user/id' => 1879049107,
'/document/id' => 1879099218
)
)

This code can match both a JSON and XML response.


Reply to this email directly or view it on GitHubhttps://github.com//issues/33#issuecomment-41977594
.

@bethesque
Copy link
Member

I believe there are actually three features we need to discuss here

  1. How to add regular expression matching in a way that is language independent (the ruby version has the capability, however, the serialisation of the matcher is ruby specific, so can't be used for a java verification)
  2. How to match both request and response bodies in a flexible way that allows /flexible/style/paths to be used.
  3. Whether or not there should be the ability to allow extra fields in a request body (ie. allow data to "leak")

@kenbot
Copy link

kenbot commented May 2, 2014

Incidentally, is there any thought to support any other kind of body than
JSON? It would seem inappropriately restrictive to only allow
application/json bodies. This impacts the problem at hand I think.

On 2 May 2014 12:48, bethesque notifications@github.com wrote:

I believe there are actually three features we need to discuss here

  1. How to add regular expression matching in a way that is language
    independent (the ruby version has the capability, however, the
    serialisation of the matcher is ruby specific, so can't be used for a java
    verification)
  2. How to match in a flexible way that allows /flexible/style/paths to be
    matched.
  3. Whether or not there should be the ability to allow extra fields in a
    request body (ie. allow data to "leak")


Reply to this email directly or view it on GitHubhttps://github.com//issues/33#issuecomment-41979709
.

@bethesque
Copy link
Member

The ruby implementation doesn't just support JSON, it's just that the best matching/diffing functionality is for JSON. You can specify any content type you like, but it will just do a plain text match on it.

There has always been the idea that we'd support XML as well, but nobody involved has been on a project that used XML since we started using it, and nobody has had enough motivation to do it in their free time!

@bethesque
Copy link
Member

@uglyog , if you do this, your request body will match if there are extra values in your request body, however, as you probably remember, what gets written to the pact file is the expected request, no the actual request, so you'd be missing the values you didn't specify, which may upset the provider.

  {
    :method => :post,
    :path => '/contract-pricing',
    :headers => {
      'Content-Type' => 'application/json;charset=utf-8',
      'Accept' => 'application/json, text/plain, */*'
    },
    :options => {:allow_unexpected_keys_in_body => true}
  }

As you can see, all the dodgy hacks in pact were made to support Condor :P

@uglyog
Copy link
Member Author

uglyog commented May 2, 2014

Hmm, it looks like the pact-jvm does not allow unexpected keys in the body (it is set in a constant to false).

I've done some hackery to get it to work for me by changing the constant to a var, but it should have options as per the ruby one.

I'm wondering if the actual request should be recorded for the pact verification. For my case there are values that I will not know before hand, but they will be required for the validation with the provider to succeed.

@thetrav
Copy link
Contributor

thetrav commented May 2, 2014

The request doesn't allow extra keys, the response does, I thought that was in line with the ruby version.

There's a PactServerConfig case class which could / should be used for settings like this

@bethesque
Copy link
Member

You were right in your assumption Trav, it's not supposed to be a feature, it was a hack I made to support Condor.

Ron, we'd need to move the pact serialization to the mock server if we were to use the actual request. I'm not convinced this is a good thing though. Wouldn't type based matching be a better option for the values that are unknown beforehand?

@uglyog
Copy link
Member Author

uglyog commented May 2, 2014

Type based matching would work, but then we need a regex format that works across both implementations ;-)

@bethesque
Copy link
Member

The gist that I sent you the other day was a plan for both type based matching and regexps. I think the majority of the time, we want type based matching, and then occasionally, we want exact value or generate/regexp.

@bethesque
Copy link
Member

So other people can see what I'm talking about, this https://gist.github.com/bethesque/5a35a3c1cb9fdab6dce7 is an idea I've been working on to allow type based and regexp based matching. The thoughts I went through to get to that I've documented here: https://gist.github.com/bethesque/39c42c8feff308481449

@uglyog
Copy link
Member Author

uglyog commented Oct 8, 2014

Closing this as regexp matching based on the 2.0 spec has been implemented

@uglyog uglyog closed this as completed Oct 8, 2014
@kenbot
Copy link

kenbot commented Oct 8, 2014

You're smashing it Ron. Keep up the good work!

On 9 October 2014 09:17, Ronald Holshausen notifications@github.com wrote:

Closed #33 #33.


Reply to this email directly or view it on GitHub
#33 (comment).

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

No branches or pull requests

4 participants