From 27a481ce2545585b5893aa71d2d7e322387c5027 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Fri, 1 Sep 2017 12:25:35 +0200 Subject: [PATCH] Inconsistent regex() usage and invalid client stubs after passed server verification After a short discussion, we came to the conclusion that it's confusing for RegexPatterns to return String. When defining the consumer/producer pair, you can easily create by mistake 2 string values if you forget the regex(...) and then you will only notice the issues on the consumer side due to invalid regular expression. So the solution would be to change the current return types to Pattern. The regex(String) method will remain but also we need to add regex(Pattern) to maintain the compatibility of current DSLs. regex(Pattern) will return just the Pattern. After such a change failing fast will be much easier. fixes #389 --- .../spec/internal/BodyMatchers.groovy | 8 +++ .../contract/spec/internal/Common.groovy | 5 ++ .../spec/internal/RegexPatterns.groovy | 56 +++++++++---------- .../spec/internal/RegexPatternsSpec.groovy | 26 ++++----- .../verifier/builder/MethodBodyBuilder.groovy | 1 + 5 files changed, 54 insertions(+), 42 deletions(-) diff --git a/spring-cloud-contract-spec/src/main/groovy/org/springframework/cloud/contract/spec/internal/BodyMatchers.groovy b/spring-cloud-contract-spec/src/main/groovy/org/springframework/cloud/contract/spec/internal/BodyMatchers.groovy index b865c32d62..7fe351c450 100644 --- a/spring-cloud-contract-spec/src/main/groovy/org/springframework/cloud/contract/spec/internal/BodyMatchers.groovy +++ b/spring-cloud-contract-spec/src/main/groovy/org/springframework/cloud/contract/spec/internal/BodyMatchers.groovy @@ -1,5 +1,7 @@ package org.springframework.cloud.contract.spec.internal +import java.util.regex.Pattern + import groovy.transform.Canonical import groovy.transform.CompileStatic import groovy.transform.EqualsAndHashCode @@ -46,6 +48,12 @@ class BodyMatchers { return new MatchingTypeValue(MatchingType.REGEX, regex) } + // Backward compatibility with RegexPatterns + MatchingTypeValue byRegex(Pattern regex) { + assert regex + return new MatchingTypeValue(MatchingType.REGEX, regex) + } + MatchingTypeValue byEquality() { return new MatchingTypeValue(MatchingType.EQUALITY, null) } diff --git a/spring-cloud-contract-spec/src/main/groovy/org/springframework/cloud/contract/spec/internal/Common.groovy b/spring-cloud-contract-spec/src/main/groovy/org/springframework/cloud/contract/spec/internal/Common.groovy index 122d042ef0..416fbfa367 100644 --- a/spring-cloud-contract-spec/src/main/groovy/org/springframework/cloud/contract/spec/internal/Common.groovy +++ b/spring-cloud-contract-spec/src/main/groovy/org/springframework/cloud/contract/spec/internal/Common.groovy @@ -113,6 +113,11 @@ class Common { return Pattern.compile(regex) } + // Backward compatibility with RegexPatterns + Pattern regex(Pattern regex) { + return regex + } + OptionalProperty optional(Object object) { return new OptionalProperty(object) } diff --git a/spring-cloud-contract-spec/src/main/groovy/org/springframework/cloud/contract/spec/internal/RegexPatterns.groovy b/spring-cloud-contract-spec/src/main/groovy/org/springframework/cloud/contract/spec/internal/RegexPatterns.groovy index 92bf982aa9..03b6f82afc 100644 --- a/spring-cloud-contract-spec/src/main/groovy/org/springframework/cloud/contract/spec/internal/RegexPatterns.groovy +++ b/spring-cloud-contract-spec/src/main/groovy/org/springframework/cloud/contract/spec/internal/RegexPatterns.groovy @@ -52,60 +52,60 @@ class RegexPatterns { return Pattern.compile(values.collect({"^$it\$"}).join("|")) } - String onlyAlphaUnicode() { - return ONLY_ALPHA_UNICODE.pattern() + Pattern onlyAlphaUnicode() { + return ONLY_ALPHA_UNICODE } - String number() { - return NUMBER.pattern() + Pattern number() { + return NUMBER } - String anyBoolean() { - return TRUE_OR_FALSE.pattern() + Pattern anyBoolean() { + return TRUE_OR_FALSE } - String ipAddress() { - return IP_ADDRESS.pattern() + Pattern ipAddress() { + return IP_ADDRESS } - String hostname() { - return HOSTNAME_PATTERN.pattern() + Pattern hostname() { + return HOSTNAME_PATTERN } - String email() { - return EMAIL.pattern() + Pattern email() { + return EMAIL } - String url() { - return URL.pattern() + Pattern url() { + return URL } - String uuid(){ - return UUID.pattern() + Pattern uuid(){ + return UUID } - String isoDate() { - return ANY_DATE.pattern() + Pattern isoDate() { + return ANY_DATE } - String isoDateTime() { - return ANY_DATE_TIME.pattern() + Pattern isoDateTime() { + return ANY_DATE_TIME } - String isoTime() { - return ANY_TIME.pattern() + Pattern isoTime() { + return ANY_TIME } - String iso8601WithOffset() { - return ISO8601_WITH_OFFSET.pattern() + Pattern iso8601WithOffset() { + return ISO8601_WITH_OFFSET } - String nonEmpty() { - return NON_EMPTY.pattern() + Pattern nonEmpty() { + return NON_EMPTY } - String nonBlank() { - return NON_BLANK.pattern() + Pattern nonBlank() { + return NON_BLANK } // end::regexps[] diff --git a/spring-cloud-contract-spec/src/test/groovy/org/springframework/cloud/contract/spec/internal/RegexPatternsSpec.groovy b/spring-cloud-contract-spec/src/test/groovy/org/springframework/cloud/contract/spec/internal/RegexPatternsSpec.groovy index 3071f631b3..6b34dfecfd 100644 --- a/spring-cloud-contract-spec/src/test/groovy/org/springframework/cloud/contract/spec/internal/RegexPatternsSpec.groovy +++ b/spring-cloud-contract-spec/src/test/groovy/org/springframework/cloud/contract/spec/internal/RegexPatternsSpec.groovy @@ -18,15 +18,13 @@ package org.springframework.cloud.contract.spec.internal import spock.lang.Specification -import java.util.regex.Pattern - class RegexPatternsSpec extends Specification { RegexPatterns regexPatterns = new RegexPatterns() def "should generate a regex for ip address [#textToMatch] that is a match [#shouldMatch]"() { expect: - shouldMatch == Pattern.compile(regexPatterns.ipAddress()).matcher(textToMatch).matches() + shouldMatch == regexPatterns.ipAddress().matcher(textToMatch).matches() where: textToMatch || shouldMatch '123.123.123.123' || true @@ -35,7 +33,7 @@ class RegexPatternsSpec extends Specification { def "should generate a regex for hostname [#textToMatch] that is a match [#shouldMatch]"() { expect: - shouldMatch == Pattern.compile(regexPatterns.hostname()).matcher(textToMatch).matches() + shouldMatch == regexPatterns.hostname().matcher(textToMatch).matches() where: textToMatch || shouldMatch 'https://asd.com' || true @@ -48,7 +46,7 @@ class RegexPatternsSpec extends Specification { def "should generate a regex for email [#textToMatch] that is a match [#shouldMatch]"() { expect: - shouldMatch == Pattern.compile(regexPatterns.email()).matcher(textToMatch).matches() + shouldMatch == regexPatterns.email().matcher(textToMatch).matches() where: textToMatch || shouldMatch 'asd@asd.com' || true @@ -59,7 +57,7 @@ class RegexPatternsSpec extends Specification { // @see http://formvalidation.io/validators/uri/ def "should generate a regex for url [#textToMatch] that is a match [#shouldMatch]"() { expect: - shouldMatch == Pattern.compile(regexPatterns.url()).matcher(textToMatch).matches() + shouldMatch == regexPatterns.url().matcher(textToMatch).matches() where: textToMatch || shouldMatch 'ftp://asd.com:9090/asd/a?a=b' || true @@ -130,7 +128,7 @@ class RegexPatternsSpec extends Specification { def "should generate a regex for a number [#textToMatch] that is a match [#shouldMatch]"() { expect: - shouldMatch == Pattern.compile(regexPatterns.number()).matcher(textToMatch).matches() + shouldMatch == regexPatterns.number().matcher(textToMatch).matches() where: textToMatch || shouldMatch '1' || true @@ -142,7 +140,7 @@ class RegexPatternsSpec extends Specification { def "should generate a regex for a uuid [#textToMatch] that is a match [#shouldMatch]"() { expect: - shouldMatch == Pattern.compile(regexPatterns.uuid()).matcher(textToMatch).matches() + shouldMatch == regexPatterns.uuid().matcher(textToMatch).matches() where: textToMatch || shouldMatch UUID.randomUUID().toString() || true @@ -155,7 +153,7 @@ class RegexPatternsSpec extends Specification { def "should generate a regex with date [#textToMatch] in YYYY-MM-DD format that should match [#shouldMatch]"() { expect: - shouldMatch == Pattern.compile(regexPatterns.isoDate()).matcher(textToMatch).matches() + shouldMatch == regexPatterns.isoDate().matcher(textToMatch).matches() where: textToMatch || shouldMatch "2014-03-01" || true @@ -173,7 +171,7 @@ class RegexPatternsSpec extends Specification { def "should generate a regex with datetime [#textToMatch] in YYYY-MM-DDTHH:mm:ss format that should match [#shouldMatch]"() { expect: - shouldMatch == Pattern.compile(regexPatterns.isoDateTime()).matcher(textToMatch).matches() + shouldMatch == regexPatterns.isoDateTime().matcher(textToMatch).matches() where: textToMatch || shouldMatch "2014-03-01T12:23:45" || true @@ -198,7 +196,7 @@ class RegexPatternsSpec extends Specification { def "should generate a regex with time [#textToMatch] in HH:mm:ss format that should match [#shouldMatch]"() { expect: - shouldMatch == Pattern.compile(regexPatterns.isoTime()).matcher(textToMatch).matches() + shouldMatch == regexPatterns.isoTime().matcher(textToMatch).matches() where: textToMatch || shouldMatch "12:23:45" || true @@ -214,7 +212,7 @@ class RegexPatternsSpec extends Specification { def "should generate a regex with iso8601DateTimeWithTimezone [#textToMatch] in YYYY-MM-DDTHH:mm:ss.SSSZZ format that should match [#shouldMatch]"(){ expect: - shouldMatch == Pattern.compile(regexPatterns.iso8601WithOffset()).matcher(textToMatch).matches() + shouldMatch == regexPatterns.iso8601WithOffset().matcher(textToMatch).matches() where: textToMatch || shouldMatch '2014-03-01T12:23:45Z' || true @@ -227,7 +225,7 @@ class RegexPatternsSpec extends Specification { def "should generate a regex for a non blank string [#textToMatch] that should match [#shouldMatch]"(){ expect: - shouldMatch == Pattern.compile(regexPatterns.nonBlank()).matcher(textToMatch).matches() + shouldMatch == regexPatterns.nonBlank().matcher(textToMatch).matches() where: textToMatch || shouldMatch 'Not Empty' || true @@ -237,7 +235,7 @@ class RegexPatternsSpec extends Specification { def "should generate a regex for a non empty string [#textToMatch] that should match [#shouldMatch]"() { expect: - shouldMatch == Pattern.compile(regexPatterns.nonEmpty()).matcher(textToMatch).matches() + shouldMatch == regexPatterns.nonEmpty().matcher(textToMatch).matches() where: textToMatch || shouldMatch 'Not Empty' || true diff --git a/spring-cloud-contract-verifier/src/main/groovy/org/springframework/cloud/contract/verifier/builder/MethodBodyBuilder.groovy b/spring-cloud-contract-verifier/src/main/groovy/org/springframework/cloud/contract/verifier/builder/MethodBodyBuilder.groovy index 22bf426855..74606234f5 100644 --- a/spring-cloud-contract-verifier/src/main/groovy/org/springframework/cloud/contract/verifier/builder/MethodBodyBuilder.groovy +++ b/spring-cloud-contract-verifier/src/main/groovy/org/springframework/cloud/contract/verifier/builder/MethodBodyBuilder.groovy @@ -384,6 +384,7 @@ abstract class MethodBodyBuilder { protected void methodForEqualityCheck(BodyMatcher bodyMatcher, BlockBuilder bb, Object copiedBody) { String path = quotedAndEscaped(bodyMatcher.path()) Object retrievedValue = value(copiedBody, bodyMatcher) + retrievedValue = retrievedValue instanceof Pattern ? ((Pattern) retrievedValue).pattern() : retrievedValue String valueAsParam = retrievedValue instanceof String ? quotedAndEscaped(retrievedValue.toString()) : retrievedValue.toString() if (arrayRelated(path) && MatchingType.regexRelated(bodyMatcher.matchingType())) { buildCustomMatchingConditionForEachElement(bb, path, valueAsParam)