⭐ Football Update: spring cloud/spring cloud contract/pull/

TYsewyn · 2018-03-07T18:54:30Z

PR to track the Pact v3 support.

❗️This is currently a work in progress❗️

Tasks

Notes on implementation:

Only supports the AND rule logic to combine multiple matchers for the same json path
Only the body and header matchers are currently supported, path and query matchers will be added in a future release.

Introduces breaking changes:

Returns a collection of Pact contracts instead of one (see d942bfd#diff-3c9a6ccf897ca28d932eb2bef60dc2e9R27)
~~The testMatchers and stubMatchers have been changed to be able to add path, query and header matchers~~ This has been reverted in favor of a non-invasive change using deprecation

Closes #583
Fixes #595

codecov-io · 2018-03-07T19:23:27Z

Codecov Report

Merging #569 into master will increase coverage by 0.41%.
The diff coverage is 63.45%.

@@             Coverage Diff              @@
##             master     #569      +/-   ##
============================================
+ Coverage     61.06%   61.48%   +0.41%     
- Complexity     1834     1935     +101     
============================================
  Files           210      217       +7     
  Lines          7534     7918     +384     
  Branches       1125     1234     +109     
============================================
+ Hits           4601     4868     +267     
- Misses         2291     2373      +82     
- Partials        642      677      +35

Impacted Files	Coverage Δ	Complexity Δ
...ud/contract/verifier/converter/YamlContract.groovy	`100% <ø> (ø)`	`0 <0> (ø)`	⬇️
...k/cloud/contract/spec/internal/BodyMatchers.groovy	`18.51% <ø> (+18.51%)`	`5 <0> (+5)`	⬆️
.../cloud/contract/spec/internal/OutputMessage.groovy	`29.16% <0%> (-2.66%)`	`3 <0> (ø)`
...contract/spec/internal/ResponseBodyMatchers.groovy	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...tract/spec/internal/PatternValueDslProperty.groovy	`26.31% <0%> (-3.99%)`	`6 <0> (ø)`
...k/cloud/contract/spec/internal/MatchingType.groovy	`66.66% <0%> (-33.34%)`	`7 <7> (+1)`
...ramework/cloud/contract/spec/internal/Input.groovy	`33.33% <0%> (-2.67%)`	`3 <0> (ø)`
...ework/cloud/contract/spec/internal/Response.groovy	`50% <100%> (+20%)`	`9 <2> (+3)`	⬆️
...saging/stream/StubRunnerStreamMessageSelector.java	`91.11% <100%> (ø)`	`11 <0> (ø)`	⬇️
...verifier/builder/MessagingMethodBodyBuilder.groovy	`81.81% <100%> (ø)`	`10 <0> (ø)`	⬇️
... and 32 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff8fd0a...009aa9e. Read the comment docs.

marcingrzejszczak · 2018-03-16T16:32:44Z

Apparently in Pact 2.0 support, I haven't noticed that in the Pact matchers section you could do $.body and pass a regular expression, or $.headers and match a header. I haven't noticed the headers part :/

…sing the v3 spec. Introduces breaking change returning a list of `Pact`s instead of one.

… Cloud contracts.

marcingrzejszczak · 2018-03-19T10:03:20Z

Related - #583

…racts

marcingrzejszczak

WOW, this is absolutely fantastic work, so, first of all, congratulations! I gave my feedback which is, generally speaking, mostly about some minor tweaks and changes. As far as the very conversion is concerned, I guess you know more about Pact then I do, so I can provide little feedback. In general, it looks great and I would be more than happy to merge it once you feel that it's pretty stable :)

marcingrzejszczak · 2018-03-20T10:40:19Z

 			<groupId>au.com.dius</groupId>
-			<artifactId>pact-jvm-model</artifactId>
-			<optional>true</optional>
+			<artifactId>pact-jvm-consumer-java8_2.12</artifactId>


Why not optional? Do we want to force that particular version?

My bad, that wasn't supposed to happen. Then again, I won't know if it will work with future versions.
(except when somebody files an issue)

marcingrzejszczak · 2018-03-20T10:42:58Z

+	private static DslPart traverse(Object value, DslPart parent, Closure dslPropertyValueExtractor) {
+		boolean isRoot = parent == null
+		Object v = value
+


It's just a matter of preference, but could you get rid of all of those separations between blocks? If not, then I'll do it later

Consider it done

marcingrzejszczak · 2018-03-20T10:43:44Z

+			}
+		}
+
+		p


Can you add return ? I prefer to always explicitly define return, otherwise you can accidentally return sth you didn't want to return, and also it's more comprehensible for others. Also if you refactor from a method that returns sth to void, your code will still compile even though it shouldn't :)

Consider it done

marcingrzejszczak · 2018-03-20T10:45:08Z

+		})
+	}
+
+	private static void processMap(Map<String, Object> values, PactDslJsonBody jsonObject, Closure dslPropertyValueExtractor) {


Isn't iterating JSON a fantastic experience? :D

Oh yes, really fantastic! I'm glad I can mention this on my resume now :D

marcingrzejszczak · 2018-03-20T10:45:24Z

+class MatchingRulesConverter {
+
+	static Category matchingRulesForBody(BodyMatchers bodyMatchers) {
+		matchingRulesFor("body", bodyMatchers)


return please

marcingrzejszczak · 2018-03-20T10:49:37Z

+										}
+									} else {
+										ruleGroup.rules.each { MatchingRule rule ->
+											if (rule instanceof RegexMatcher) {


Theoretically, you could rewrite this to call jsonPath only once and just pass different value basing on the instance of rule.

Good idea! But then you would need to explain me how I could pass the current closure context to another method.
Closures, they're possibly great when you understand it completely. :D

Ah ok, so just ignore it.

marcingrzejszczak · 2018-03-20T10:51:24Z

-			return dsl.request.url.queryParameters
-		}
-		throw new IllegalStateException("No url provided")
+		pactContracts


missing return (I wasn't adding this comment everywhere since it doesn't make much sense to repeat myself)

marcingrzejszczak · 2018-03-20T10:52:35Z

+@PackageScope
+class RequestResponsePactCreator {
+
+	static final Closure requestDslPropertyValueExtractor = { DslProperty property -> property.serverValue }


ha! you have to add a modifier e.g. protected if you want this to be a constant field. Otherwise if you check out the MOP, you'll see that a private static final field was created with a public static getter method :)

Good catch! Both should be private indeed!

marcingrzejszczak · 2018-03-20T10:53:42Z

-          },
-          "$.body.something": {
-            "match": "equality"
+          "body": {


maybe we should check for the headers matchers too?

Yes, that's for some time later. They also introduced path & query matchers.
See https://github.com/pact-foundation/pact-specification/tree/version-3#drop-the-jsonpath-notation-for-matchers

marcingrzejszczak · 2018-03-20T10:54:34Z

+							})
+							jsonPath('$.id', byType())
+
+//							TODO: CURRENTLY NOT SUPPORTED


Why isn't it supported? That should work I guess OOB ?

No, it wasn't. They added new matchers to v3, check https://github.com/pact-foundation/pact-specification/tree/version-3#add-an-equality-matcher until the generators

Currently only the AND rule logic is supported.

…s of "matcher groups", eg. header, path & query

… supported by WireMock

Upgraded pact-jvm-model to 3.5.13

533f1d3

TYsewyn mentioned this pull request Mar 7, 2018

Support for Pact v3 #406

Closed

marcingrzejszczak added this to the 2.0.0.M8 milestone Mar 7, 2018

marcingrzejszczak added the enhancement label Mar 7, 2018

TYsewyn added 3 commits March 17, 2018 22:37

Merge branch 'master' into feature/gh406-upgrade-pact

6bcb70c

Enhanced the conversion of Spring Cloud contracts to Pact contracts u…

d942bfd

…sing the v3 spec. Introduces breaking change returning a list of `Pact`s instead of one.

Enhanced the conversion of Pact contracts using the v3 spec tp Spring…

9480bee

… Cloud contracts.

Implemented conversion of Pact v3 messaging to/from Spring Cloud cont…

bb44f55

…racts

marcingrzejszczak reviewed Mar 20, 2018

View reviewed changes

Updated code based on comments in the PR

9057bc8

marcingrzejszczak mentioned this pull request Mar 21, 2018

Usage of MatchingRules for response header #583

Closed

marcingrzejszczak modified the milestones: 2.0.0.M8, 2.0.0.M9 Mar 23, 2018

TYsewyn added 14 commits March 23, 2018 19:43

Added null matcher

32474f9

Merge branch 'master' into feature/gh406-upgrade-pact

8be9bbf

Added missing null matcher conversion from SCC to Pact

a3554dc

Added number, integer and decimal matchers

b2a7918

Added support for multiple matchers for the same json path.

0ce3801

Currently only the AND rule logic is supported.

Added value generator functionality

b7f90b4

Merge branch 'master' into feature/gh406-upgrade-pact

84a7eda

Refactored stubMatchers and testMatchers to support multiple type…

e37a8c5

…s of "matcher groups", eg. header, path & query

Updated test for issue spring-attic#595

af42d9e

Merge branch 'master' into feature/gh406-upgrade-pact

321cc1e

Refactored e37a8c5 in favor of a less invasive change using deprecation

165c9df

Merge branch 'master' into feature/gh406-upgrade-pact

1a7f75b

Merge branch 'master' into feature/gh406-upgrade-pact

41d0a0f

Added header matcher conversion

7ef003f

TYsewyn added 5 commits April 1, 2018 16:20

Merge branch 'master' into feature/gh406-upgrade-pact

9015506

Add/updated the ASF license header

a422132

Fixed an issue with the header matchers while updating the documentation

a484b3f

Updated the documentation

ee36e00

Updated the documentation

f62ad3a

spencergibb modified the milestones: 2.0.0.M9, 2.0.0.RC1 Apr 2, 2018

Made some last minute changes to the bodyMatchers: byNull() isn't…

009aa9e

… supported by WireMock

TYsewyn mentioned this pull request Apr 2, 2018

Add support for query and path matchers for Pact 3 #602

Closed

marcingrzejszczak merged commit de529e2 into spring-attic:master Apr 2, 2018

TYsewyn deleted the feature/gh406-upgrade-pact branch May 6, 2018 09:47

+              			}
+              		}
+              		p

Conversation

TYsewyn commented Mar 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Mar 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

marcingrzejszczak commented Mar 16, 2018

Uh oh!

marcingrzejszczak commented Mar 19, 2018

Uh oh!

marcingrzejszczak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

TYsewyn commented Mar 7, 2018 •

edited

Loading

codecov-io commented Mar 7, 2018 •

edited

Loading