spring-cloud / spring-cloud-contract

Support for Consumer Driven Contracts in Spring
https://cloud.spring.io/spring-cloud-contract
Apache License 2.0
719 stars 438 forks source link

Pact <-> SC-Contract DSL conversion #96

Closed marcingrzejszczak closed 7 years ago

fitzoh commented 7 years ago

I've been starting to look into this btw. Notes from @marcingrzejszczak in gitter:

https://github.com/spring-cloud/spring-cloud-contract/blob/master/spring-cloud-contract-verifier/src/main/groovy/org/springframework/cloud/contract/verifier/file/ContractFileScanner.groovy#L122
this is a class that scans the provided directory for contracts
as you can see we check for groovy files only which will have to change

we'd need an optional dep to pact-jvm in SCC
and then before we do the conversion we'd need to do an analysis if on the classpath we do have pact-jvm
cause remember that part of this will be executed in plugins
https://github.com/spring-cloud/spring-cloud-contract/blob/master/spring-cloud-contract-verifier/src/main/groovy/org/springframework/cloud/contract/verifier/util/ContractVerifierDslConverter.groovy
this thing is responsible for conversion from files or string dsls into Contract
so we'd have to change that to do a potential conversion here

AFAIR there's no dynamic matching in PACT
so we'd have to treat these situations as static equality check
also I'd remove from the DslConverter the (String dsl) version and keep the file one only
that way we at least know what's the file ending and will try to parse accordingly
also there's an open issue for RAML so maybe we could think about sth more intelligent
https://github.com/spring-cloud/spring-cloud-contract/blob/master/spring-cloud-contract-verifier/src/main/groovy/org/springframework/cloud/contract/verifier/file/ContractFileScanner.groovy#L122
this could take some map (extension -> implementation that knows what to do with this)
groovy -> DSLConverter
pact -> PactConverter
raml -> RamlConverter
or sth like this
so that's the Pact -> DSL converter
we'd need also DSL to Pact converter.

regarding matching arrays:

Andrew Fitzgerald @Fitzoh 06:31
does SCC have an easy way of saying “This field of the json object should be an array of Strings”?
...
what do we do with the array fields?
just drop them completely?
or “.*”?

Marcin Grzejszczak @marcingrzejszczak 06:36
no, let's create a single/two element array or sth

Andrew Fitzgerald @Fitzoh 06:36
ok, that makes sense

Marcin Grzejszczak @marcingrzejszczak 06:36
[
id: value(consumer(regex(uuid()))),
secretRequired: value(consumer(regex("true|false"))),
array: [
   [id: value(consumer(regex(uuid()))),
   secretRequired: value(consumer(regex("true|false")))].
   [id: value(consumer(regex(uuid()))),
   secretRequired: value(consumer(regex("true|false")))]
]
...
]
fitzoh commented 7 years ago

I did some digging through the pact-jvm code base, should check with @uglyog and see if he has any initial recommendations, specifically where that code is that parses the pact file.

uglyog commented 7 years ago

The pact file parsing is done in https://github.com/DiUS/pact-jvm/blob/master/pact-jvm-model/src/main/groovy/au/com/dius/pact/model/PactReader.groovy

There is also a class there to write a pact file out. The pact-jvm-model library can be used without any of the other pact-jvm libraries.

marcingrzejszczak commented 7 years ago

@Fitzoh @uglyog I've just merged the pluggability thing to master. Now you can easily create the org.springframework.cloud.contract.spec.ContractConverter to convert from PACT to DSL and back. Check https://github.com/spring-cloud/spring-cloud-contract/pull/161 for more info

marcingrzejszczak commented 7 years ago

The question remains which versions to support (https://github.com/DiUS/pact-jvm#supported-jdk-and-specification-versions). Currently we're SDK7 based so I guess I'll pick the V2 cause it's stable (2.4.14 is the latest I found in Maven Central - http://search.maven.org/#artifactdetails%7Cau.com.dius%7Cpact-jvm-model_2.11%7C2.4.14%7Cjar )

marcingrzejszczak commented 7 years ago

If anyone's interested this is my spike - https://github.com/spring-cloud/spring-cloud-contract/compare/issues_%2396_pact_contract

fitzoh commented 7 years ago

Looks like a good starting point! 👍 Matching stuff is definitely gonna get a little bit tricky. I'd love to dig in a little more, but I'm probably not gonna have time for the next week and a half or so.

uglyog commented 7 years ago

@marcingrzejszczak 2.4.18 is the latest version of the v2.x branch: http://search.maven.org/#artifactdetails%7Cau.com.dius%7Cpact-jvm-model%7C2.4.18%7Cjar

marcingrzejszczak commented 7 years ago

Ah I picked the wrong artifactid that's why I couldn't find the 2.4.18. Thanks for the hint @uglyog !

uglyog commented 7 years ago

That enables type-based matching (elements match if they have the same type) for the 'test' attribute in the body. They cascade, so 'test' and everything below it will match if they have the same type, while everything else will only match if they have the same value.

There is not that much docs on it, but here are some links:

Pact specification project: https://github.com/pact-foundation/pact-specification/tree/version-2#supported-matchers Ruby Pact Matching https://github.com/realestate-com-au/pact/wiki/Regular-expressions-and-type-matching-with-Pact JVM Pact Matching https://github.com/DiUS/pact-jvm/wiki/Matching

On 4 January 2017 at 01:46, Marcin Grzejszczak notifications@github.com wrote:

@uglyog https://github.com/uglyog is there a list of all the possible matching rules? https://github.com/realestate-com-au/pact/wiki/Regular- expressions-and-type-matching-with-Pact I can't figure this out.

"matchingRules" : { "$.body.test" : { "match" : "type" } }

What does it exactly mean? I can't find anywhere any concrete answer but I see that most likely it means that the JSON path $.body.test has to exist and that's it? What other matching strategies are there?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/spring-cloud/spring-cloud-contract/issues/96#issuecomment-270129466, or mute the thread https://github.com/notifications/unsubscribe-auth/AA0tshWTY5DtCD9OFE5J6Hh1QRmorCv4ks5rOl8ugaJpZM4KPzeW .

marcingrzejszczak commented 7 years ago

Thanks @uglyog !

marcingrzejszczak commented 7 years ago

Note to myself on the spike

[] - extend the dsl to support stubMatchers and testMatchers sections in request / response

example:

                          request {
                    method(GET())
                    url("/mallory") {
                        queryParameters {
                            parameter("name", "ron")
                            parameter("status", "good")
                        }
                    }
                    body(id: "123", method: "create")
                    stubMatchers {
                    ...
                    }
                }
                response {
                    status(200)
                    headers {
                        header("Content-Type", applicationJson())
                    }
                    body([[
                            [email: "rddtGwwWMEhnkAPEmsyE",
                            id: "eb0f8c17-c06a-479e-9204-14f7c95b63a6",
                            userName: "AJQrokEGPAVdOHprQpKP"]
                        ]])
                    testMatchers {
                    ...
                    }
                }

[] - before creating stubs / tests we'll need to remove from the analyzed body the elements that match the patterns. Example

Having such a body

{ 
"a" : "a", 
"b": "b" 
}

will result in creation of 2 equality checks in the autogenerated tests. If we create a separate section that looks like in Pact to do a stub / test matching then in order for the whole process of test / stub generation to look the same we can simply remove those entries from the body that we're checking. Let's say that we have such strategies

stubStrategy {
jsonPath('$.a', regex("[0-9]"))
}

testStrategy {
jsonPath('$.b', type())
}

then for the stub we would remove the entry $.a and only $.b remains. Now, we do the typical conversion to stubs as we have previously. Now we iterate over the whole stubStrategy section and add additional json paths.

We can do similar stuff for tests.

NOTE: in case of stub matching type doesn't make really sense. WireMock doesn't care about the type - it cares about the values. For tests type matching does make sense.

marcingrzejszczak commented 7 years ago

THe latter is done. Now a couple of things to fix