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

Pact <-> SC-Contract DSL conversion #96

Closed
marcingrzejszczak opened this issue Oct 6, 2016 · 13 comments · Fixed by #188
Closed

Pact <-> SC-Contract DSL conversion #96

marcingrzejszczak opened this issue Oct 6, 2016 · 13 comments · Fixed by #188
Assignees

Comments

@marcingrzejszczak
Copy link
Contributor

No description provided.

@fitzoh
Copy link
Contributor

fitzoh commented Nov 16, 2016

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
Copy link
Contributor

fitzoh commented Nov 16, 2016

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
Copy link

uglyog commented Nov 17, 2016

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
Copy link
Contributor Author

@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 #161 for more info

@marcingrzejszczak
Copy link
Contributor Author

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 marcingrzejszczak self-assigned this Jan 2, 2017
@marcingrzejszczak
Copy link
Contributor Author

@fitzoh
Copy link
Contributor

fitzoh commented Jan 2, 2017

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
Copy link

uglyog commented Jan 3, 2017

@marcingrzejszczak
Copy link
Contributor Author

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

@uglyog
Copy link

uglyog commented Jan 4, 2017 via email

@marcingrzejszczak
Copy link
Contributor Author

Thanks @uglyog !

@marcingrzejszczak
Copy link
Contributor Author

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.

This was referenced Jan 9, 2017
marcingrzejszczak added a commit that referenced this issue Jan 10, 2017
Without this change we're forcing users to embed their dynamic properties inside the body. For some this is natural and acceptable, but especially for the users coming from the Pact world this sounds bizarre. Also some other people have a problem with remembering who the consumer / producer is etc.

With this change we're introducing the stubMatchers and testMatchers section. Thanks to this one can separate the body from defining the dynamic properties. Especially for Pact users this is more natural. Speaking of which this is a prerequisite for #96

fixes #185
marcingrzejszczak added a commit that referenced this issue Jan 10, 2017
Stub / Test Matchers (#186)
Without this change we're forcing users to embed their dynamic properties inside the body. For some this is natural and acceptable, but especially for the users coming from the Pact world this sounds bizarre. Also some other people have a problem with remembering who the consumer / producer is etc.

With this change we're introducing the stubMatchers and testMatchers section. Thanks to this one can separate the body from defining the dynamic properties. Especially for Pact users this is more natural. Speaking of which this is a prerequisite for #96

fixes #185
@marcingrzejszczak
Copy link
Contributor Author

marcingrzejszczak commented Jan 10, 2017

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

  • description - Let's remove the index and leave it as interaction name followed by an optional server state
  • Contract -> Pact conversion
  • Create a test project to test the dependency passing
  • Update the docs with Pact example and example of how to setup your project to use custom Contract converter
  • Check why the tests are not indented properly
  • Modify the pact test to contain matchers

marcingrzejszczak added a commit that referenced this issue Jan 13, 2017
with this change we're providing support for Pact based contracts. No longer do you have to set up your contracts using the Groovy DSL. In the same way as with the DSL you can use the Pact contracts to generate tests and the stubs on the producer side.

fixes #96
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants