Matching rules against field name with a '.' character are not working

Question

Matching rules against field name with a '.' character are not working

Closed this issue 7 years ago · 41 comments

Hi,

I'm currently trying to test a contract with an Odata v4 endpoint.

As part of the Odata json format specifications the server usually includes information other than the raw data in the response called annotations.

Annotations are name/value pairs that have a dot (.) as part of the name, an example could be

"@odata.context": "http://localhost/$metadata#MyEntity/$entity"

Obviously I'm not interested in the exact value of the annotations field so I'm using type matchers and my pact looks something like:

{ "consumer": { "name": "OdataConsumer" }, "provider": { "name": "OdataProvider" }, "interactions": [ { "description": "A GET request to retrieve an Odata entity", "request": { "method": "get", "path": "/odata/MyEntity('1234')" }, "response": { "status": 200, "headers": { "Content-Type": "application/json; odata.metadata=minimal", "OData-Version": "4.0" }, "body": { "@odata.context": "http://localhost/$metadata#MyEntity/$entity", "anotherField": "someDataHere" }, "matchingRules": { "$.body.@odata.context": { "match": "type" }, "$.body.anotherField": { "match": "type" } } } } ], "metadata": { "pactSpecification": { "version": "2.0.0" } } }

As you can see the matching rule is not correctly escaping the annotation name and, as a result, it won't be executed. My understanding is that the (.) is interpreted as a path separator.

Is there any way to fix this behavior having matching rules working against fields with a (.) as part of their names?

Answer 1 · 2017-07-26T11:05:25.000Z

@bethesque This is an interesting one, would to hear your thoughts.

Answer 2 · 2017-07-27T10:12:22.000Z

As its json we should follow the same rules. So we should be able to quote property names e.g.

$.body."@odata.context".level[2].id

this would allow existing json functionality to be leveraged. For compatibility you may choose to just allow double quotes as it probably has the widest support for json libraries, but technically both should be allowed.

Answer 3 · 2017-07-27T14:31:56.000Z

This won't work, we don't have any control on how Pact writes matching rules so the ruby library should take care of escaping json property names with a '.'

Answer 4 · 2017-07-27T22:30:25.000Z

The correct form is $.body["@odata.context"] (squire brackets around the quoted string). If the Ruby version is not writing the matchers in this form, could you raise an issue against that project.

Answer 5 · 2017-07-27T22:44:02.000Z

For a reference, here are two tests in Pact-JVM:
https://github.com/DiUS/pact-jvm/blob/1aa4833dd3303880e4624d419ab9c437c6dc2402/pact-jvm-consumer-groovy/src/test/groovy/au/com/dius/pact/consumer/groovy/PactBodyBuilderSpec.groovy#L420
https://github.com/DiUS/pact-jvm/blob/1aa4833dd3303880e4624d419ab9c437c6dc2402/pact-jvm-consumer/src/test/java/au/com/dius/pact/consumer/PactDslJsonBodyTest.java#L71

Answer 6 · 2017-07-27T23:21:43.000Z

I'll add some logic to put square brackets and quotes around names with dots in them.

Answer 7 · 2017-07-27T23:22:48.000Z

I've raised the issue here pact-foundation/pact-support#39

Answer 8 · 2017-07-27T23:51:35.000Z

I've updated the code and released a new version of the ruby-standalone package (1.1.1).

Answer 9 · 2017-07-27T23:53:08.000Z

Will get that updated on the weekend

Answer 10 · 2017-07-28T07:07:49.000Z

This is great thanks for the quick feedback and support!

Answer 11 · 2017-07-30T00:20:08.000Z

Merged into master now and is available in 2.0.6-beta

Answer 12 · 2017-08-23T09:03:54.000Z

Hi all,

I managed to test this fix only today and unfortunately is still not working.
The pact file looks fine and the match rule now is escaped correctly:

"$.body[\"@odata.context\"]": { "match": "type" }

Yet the test is failing on the producer with the following error:

Diff
--------------------------------------
Key: - is expected
+ is actual
Matching keys and values are not shown

    {
   -  "@odata.context": "http://someUrl/$metadata#myEntity/$entity"
   +  "@odata.context": "http://localhost:13259/$metadata#myEntity/$entity"
    }

   Description of differences
   --------------------------------------
   * Expected "http://someUrl/$metadata#myEntity/$entity" but got "http://localhost:13259/$metadata#myEntity/$entity" at $.@odata.context

Can you please have a look? I'm using PactNet v2.0.11-beta and PactNet-Windows v2.0.11-beta on both consumer and provider.
Sorry again for the delay.

Thanks

Answer 13 · 2017-08-23T10:08:46.000Z

@GiampaLab That's interesting. Any ideas @bethesque?

Answer 14 · 2017-08-24T23:07:05.000Z

I've replicated the problem here

It's actually the @ symbol causing the problem, as well as the dot. I'll look into it.

Answer 15 · 2017-08-24T23:51:32.000Z

I've pushed a fix. The keys with dots needed to be quoted with single quotes, not double quotes. Can you update to 1.4.3 of the ruby standalone when you have time @neilcampbell

Answer 16 · 2017-08-24T23:55:29.000Z

Awesome stuff @bethesque! Yep will do.

Answer 17 · 2017-08-26T01:56:34.000Z

@bethesque After updating from 1.1.2 to 1.4.3, I started getting a "400 Bad Request - Invalid Hostname" coming back from my sample api when performing provider verification.

After doing some investigation it appears the windows (maybe others?) standalone is sending Host: example.org instead of Host: localhost:{port}.

I verified this by passing a custom header via --custom-provider-header "Host: localhost" to the pact-provider-verifier, and it printed INFO: Replacing header 'Host: example.org' with 'Host: localhost' to stdout when writing the verification summary.

Any ideas on this one?

Answer 18 · 2017-08-26T06:35:38.000Z

@bethesque Also the new version no longer supports the following scenarios:

When the body is a json string, see https://github.com/SEEK-Jobs/pact-net/blob/master/Samples/EventApi/Consumer.Tests/pacts/event_api_consumer-event_api.json#L131.
The error returned is JSON::ParserError: 757: unexpected token at '"1.0.22"'
When the body is explicitly null, see https://github.com/SEEK-Jobs/pact-net/blob/master/Samples/EventApi/Consumer.Tests/pacts/event_api_consumer-event_api.json#L144
The error returned is JSON::GeneratorError: only generation of JSON objects or arrays allowed

Not sure at what version these things changed, but it all worked in 1.1.2

Answer 19 · 2017-08-26T21:45:06.000Z

I've not deliberately changed anything to do with those issues, they must be side effects of something else. Perhaps the JSON version upgrade. I'll investigate.

Answer 20 · 2017-08-26T21:54:10.000Z

"1.0.22" is not a valid json object. It needs to be either a hash or an array at the top level. There's some debate here but the new Ruby JSON library obviously takes the opinion that just a string doesn't count as a standalone serialisation.

Answer 21 · 2017-08-26T22:01:17.000Z

Can you give me the stacktrace of the second one, as I can't reproduce it in my e2e project.

Answer 22 · 2017-08-27T00:28:14.000Z

I've made an e2e example using the standalone packages here and have recreated the issue. Will need to give it some thought on how to fix the second example, though (and I'm willing to be convinced otherwise) I feel that the first one is a #willnotfix.

Answer 23 · 2017-08-27T00:39:21.000Z

Actually, no, sorry, I can't reproduce the second issue.
Commit here: pact-foundation/pact-ruby-standalone-e2e-example@823abfe
Build passing here: https://travis-ci.org/pact-foundation/pact-ruby-standalone-e2e-example/builds/268791886

Can you have a look here and see if you can work out what's different between the example and the .net code?

Answer 24 · 2017-08-27T03:17:12.000Z

I've fixed the host problem in 1.4.4.

Answer 25 · 2017-08-27T03:42:38.000Z

@bethesque Awesome stuff! I was on the fence about the first one as well, but yeah technically it is supported in the json spec. Happy to mark that as a #willnotfix.

For the second item, I will look into it a bit deeper and see if I can reproduce/fix.

Answer 26 · 2017-08-27T07:20:24.000Z

@bethesque I recreated the problem using pact-ruby-standalone-e2e-example on OSX. In your example in pact-foundation/pact-ruby-standalone-e2e-example@823abfe you set the response body to null, it's when the request body is null where the issues arises.

Answer 27 · 2017-08-27T23:12:40.000Z

Ah! Thanks. How strange. I'll have a look into it.

Answer 28 · 2017-08-27T23:43:11.000Z

Now I'm playing with it, I'm unsure what a null body means. Is it the JSON string "null". Is it an empty string? Does it mean 0 Content-Length? What is our use case? I'm confused now...

Answer 29 · 2017-08-27T23:48:59.000Z

A null body has to be a body that is the JSON null value (i.e. represented string value of 'null').

Answer 30 · 2017-08-27T23:52:50.000Z

To me it's similar to the null use case for a response. We are explicitly saying that the value should be set to null (no body), and not be just any body (which is denoted when you omit the body key).

Answer 31 · 2017-08-28T00:05:01.000Z

No body would be an empty string (Content-Length: 0). The only way this makes sense as a separate use case is if it's the string "null" (which is again not a valid JSON "object"). Ron, does JVM pact support a 'property' (ie. top level string, boolean, number, null)?

Answer 32 · 2017-08-28T00:08:25.000Z

I am assuming if top level strings aren't supported at the moment, then empty string is not supported either?

Answer 33 · 2017-08-28T00:15:46.000Z

Sorry, I used ambiguous terminology. Thinking of the actual HTTP request, an empty body is where Content-Length: 0. The way this would be represented to the HTTP server would probably differ by language and implementation. Some frameworks might say the body was null, and some might return an empty string.

A JSON String or empty JSON String would haveContent-Length > 0 in that it would be "" or "foo", and both would equally not be technically JSON "objects".

Answer 34 · 2017-08-28T00:18:58.000Z

Yep, got you. So how should we be representing no body in a language agnostic way? My understanding was setting body: null, but maybe it's better to not set body and add a Content-Length: 0 header?

Answer 35 · 2017-08-28T00:35:28.000Z

In Pact JVM there are the following values for JSON bodies:

Empty body (the Content-Length header will have a value of 0)
Empty string body (represented by the empty JSON string "", Content-Length will probably = 2)
Null body (represented by the JSON null value, Content-Length will probably = 4)
JSON body, which could be a number, true, false, a string, an array or an object.

In the Pact file, we represent no body and empty body differently:

No body means there is no body attribute in the pact file, which means we do not care about the body (there could be one, or not). See https://github.com/DiUS/pact-jvm/blob/master/pact-jvm-model/src/main/kotlin/au/com/dius/pact/model/OptionalBody.kt#L8
Empty body (where Content-Length = 0) means there must not be a body. We add "body": "" to the pact file.

Empty string bodies are a difficult one. The only way to represent it correctly is with escaped strings, e.g. "\"\"".

Answer 36 · 2017-08-28T12:03:05.000Z

I have removed the null body from the consumer sample now. Will get a new release out tonight with the latest pact standalone.

Answer 37 · 2017-08-28T23:16:36.000Z

Given we're going to be moving to the Rust impl in the future, I'm reluctant to spend a heap of time on this null body thing until someone actually has a use case for it. Can we raise an issue to document the problem, and then wait until somebody jumps up and down about it?

Answer 38 · 2017-08-28T23:23:10.000Z

Sounds good to me. On which project should I document the problem?

Answer 39 · 2017-08-28T23:34:35.000Z

https://github.com/pact-foundation/pact-mock_service, that's where the pact is written in the ruby impl.

Answer 40 · 2017-08-28T23:46:30.000Z

@GiampaLab This issue has been fixed and released in 2.0.12-beta

Answer 41 · 2017-08-29T10:51:07.000Z

Thank @neilcampbell, @bethesque , @uglyog I just tested the new beta release and it's working correctly, well done!

Also we are working on a library which makes producer tests trivial to implement and supports branching you can find it here

Let me know what you think!

Thanks