SpringCloud/spring-cloud-contract
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Inconsistent regex() usage and invalid client stubs after passed server verification

Browse Source 
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
This commit is contained in:
Marcin Grzejszczak
2017-09-01 12:25:35 +02:00
parent ba6d4a5bc6
commit 27a481ce25
5 changed files with 54 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
spring-cloud-contract-spec/src/main/groovy/org/springframework/cloud/contract/spec/internal/BodyMatchers.groovy
View File

@@ -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)
}

5
spring-cloud-contract-spec/src/main/groovy/org/springframework/cloud/contract/spec/internal/Common.groovy
View File

@@ -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)
}

56
spring-cloud-contract-spec/src/main/groovy/org/springframework/cloud/contract/spec/internal/RegexPatterns.groovy
View File

@@ -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[]

26
spring-cloud-contract-spec/src/test/groovy/org/springframework/cloud/contract/spec/internal/RegexPatternsSpec.groovy
View File

@@ -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

1
spring-cloud-contract-verifier/src/main/groovy/org/springframework/cloud/contract/verifier/builder/MethodBodyBuilder.groovy
View File

@@ -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)