Skip to content

Commit b365641

Browse files
rholshausenrholshausen
committed
fix: Only coerce strings to numbers when comparing headers and query parameters
1 parent 00e4b40 commit b365641

11 files changed

+233
-131
lines changed

‎core/matchers/src/main/kotlin/au/com/dius/pact/core/matchers/MatcherExecutor.kt

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,11 @@ fun <M : Mismatch> domatch(
118118
path: List<String>,
119119
expected: Any?,
120120
actual: Any?,
121-
mismatchFn: MismatchFactory<M>
121+
mismatchFn: MismatchFactory<M>,
122+
context: MatchingContext? = null
122123
): List<M> {
123124
val result = matchers.rules.map { matchingRule ->
124-
domatch(matchingRule, path, expected, actual, mismatchFn, matchers.cascaded)
125+
domatch(matchingRule, path, expected, actual, mismatchFn, matchers.cascaded, context)
125126
}
126127

127128
return if (matchers.ruleLogic == RuleLogic.AND) {
@@ -142,13 +143,14 @@ fun <M : Mismatch> domatch(
142143
expected: Any?,
143144
actual: Any?,
144145
mismatchFn: MismatchFactory<M>,
145-
cascaded: Boolean
146+
cascaded: Boolean,
147+
context: MatchingContext? = null
146148
): List<M> {
147149
logger.debug { "Matching value $actual at $path with $matcher" }
148150
return when (matcher) {
149151
is RegexMatcher -> matchRegex(matcher.regex, path, expected, actual, mismatchFn)
150152
is TypeMatcher -> matchType(path, expected, actual, mismatchFn, true)
151-
is NumberTypeMatcher -> matchNumber(matcher.numberType, path, expected, actual, mismatchFn)
153+
is NumberTypeMatcher -> matchNumber(matcher.numberType, path, expected, actual, mismatchFn, context)
152154
is DateMatcher -> matchDate(matcher.format, path, expected, actual, mismatchFn)
153155
is TimeMatcher -> matchTime(matcher.format, path, expected, actual, mismatchFn)
154156
is TimestampMatcher -> matchDateTime(matcher.format, path, expected, actual, mismatchFn)
@@ -315,7 +317,8 @@ fun <M : Mismatch> matchNumber(
315317
path: List<String>,
316318
expected: Any?,
317319
actual: Any?,
318-
mismatchFactory: MismatchFactory<M>
320+
mismatchFactory: MismatchFactory<M>,
321+
context: MatchingContext? = null
319322
): List<M> {
320323
if (expected == null && actual != null) {
321324
return listOf(mismatchFactory.create(expected, actual,
@@ -324,22 +327,21 @@ fun <M : Mismatch> matchNumber(
324327
when (numberType) {
325328
NumberTypeMatcher.NumberType.NUMBER -> {
326329
logger.debug { "comparing type of ${valueOf(actual)} (${typeOf(actual)}) to a number at $path" }
327-
if (actual is JsonValue && !actual.isNumber || actual is Attr && actual.nodeValue.matches(decimalRegex) ||
328-
actual !is JsonValue && actual !is Node && actual !is Number) {
330+
if (!matchInteger(actual, context) && !matchDecimal(actual, context)) {
329331
return listOf(mismatchFactory.create(expected, actual,
330332
"Expected ${valueOf(actual)} (${typeOf(actual)}) to be a number", path))
331333
}
332334
}
333335
NumberTypeMatcher.NumberType.INTEGER -> {
334336
logger.debug { "comparing type of ${valueOf(actual)} (${typeOf(actual)}) to an integer at $path" }
335-
if (!matchInteger(actual)) {
337+
if (!matchInteger(actual, context)) {
336338
return listOf(mismatchFactory.create(expected, actual,
337339
"Expected ${valueOf(actual)} (${typeOf(actual)}) to be an integer", path))
338340
}
339341
}
340342
NumberTypeMatcher.NumberType.DECIMAL -> {
341343
logger.debug { "comparing type of ${valueOf(actual)} (${typeOf(actual)}) to a decimal at $path" }
342-
if (!matchDecimal(actual)) {
344+
if (!matchDecimal(actual, context)) {
343345
return listOf(mismatchFactory.create(expected, actual,
344346
"Expected ${valueOf(actual)} (${typeOf(actual)}) to be a decimal number",
345347
path))
@@ -349,7 +351,7 @@ fun <M : Mismatch> matchNumber(
349351
return emptyList()
350352
}
351353

352-
fun matchDecimal(actual: Any?): Boolean {
354+
fun matchDecimal(actual: Any?, context: MatchingContext? = null): Boolean {
353355
val result = when {
354356
actual == 0 -> true
355357
actual is Float -> true
@@ -360,24 +362,24 @@ fun matchDecimal(actual: Any?): Boolean {
360362
bigDecimal == BigDecimal.ZERO || bigDecimal.scale() > 0
361363
}
362364
actual is JsonValue.Integer -> decimalRegex.matches(actual.toString())
363-
isString(actual) -> decimalRegex.matches(actual.toString())
364-
actual is Attr -> decimalRegex.matches(actual.nodeValue)
365+
isString(actual) && context?.coerceNumbers ?: false -> decimalRegex.matches(actual.toString())
366+
actual is Node -> decimalRegex.matches(actual.nodeValue)
365367
else -> false
366368
}
367369
logger.debug { "${valueOf(actual)} (${typeOf(actual)}) matches decimal number -> $result" }
368370
return result
369371
}
370372

371-
fun matchInteger(actual: Any?): Boolean {
373+
fun matchInteger(actual: Any?, context: MatchingContext? = null): Boolean {
372374
val result = when {
373375
actual is Int -> true
374376
actual is Long -> true
375377
actual is BigInteger -> true
376378
actual is JsonValue.Integer -> true
377379
actual is BigDecimal && actual.scale() == 0 -> true
378380
actual is JsonValue.Decimal -> integerRegex.matches(actual.toString())
379-
isString(actual) -> integerRegex.matches(actual.toString())
380-
actual is Attr -> integerRegex.matches(actual.nodeValue)
381+
isString(actual) && context?.coerceNumbers ?: false -> integerRegex.matches(actual.toString())
382+
actual is Node -> integerRegex.matches(actual.nodeValue)
381383
else -> false
382384
}
383385
logger.debug { "${valueOf(actual)} (${typeOf(actual)}) matches integer -> $result" }

‎core/matchers/src/main/kotlin/au/com/dius/pact/core/matchers/Matchers.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ object Matchers : KLogging() {
7070
pathComparator: Comparator<String> = Comparator.naturalOrder()
7171
): List<M> {
7272
val matcherDef = context.selectBestMatcher(path, pathComparator)
73-
return domatch(matcherDef, path, expected, actual, mismatchFn)
73+
return domatch(matcherDef, path, expected, actual, mismatchFn, context)
7474
}
7575

7676
/**

‎core/matchers/src/main/kotlin/au/com/dius/pact/core/matchers/Matching.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ import org.apache.commons.codec.binary.Hex
2525
data class MatchingContext @JvmOverloads constructor(
2626
val matchers: MatchingRuleCategory,
2727
val allowUnexpectedKeys: Boolean,
28-
val pluginConfiguration: Map<String, PluginConfiguration> = mapOf()
28+
val pluginConfiguration: Map<String, PluginConfiguration> = mapOf(),
29+
val coerceNumbers: Boolean = false
2930
) {
3031
@JvmOverloads
3132
fun matcherDefined(path: List<String>, pathComparator: Comparator<String> = Comparator.naturalOrder()): Boolean {

‎core/matchers/src/main/kotlin/au/com/dius/pact/core/matchers/RequestMatching.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,8 @@ class RequestMatching(private val expectedPact: Pact) {
126126

127127
val pathContext = MatchingContext(expected.matchingRules.rulesForCategory("path"), false, pluginConfiguration)
128128
val bodyContext = MatchingContext(expected.matchingRules.rulesForCategory("body"), false, pluginConfiguration)
129-
val queryContext = MatchingContext(expected.matchingRules.rulesForCategory("query"), false, pluginConfiguration)
130-
val headerContext = MatchingContext(expected.matchingRules.rulesForCategory("header"), false, pluginConfiguration)
129+
val queryContext = MatchingContext(expected.matchingRules.rulesForCategory("query"), false, pluginConfiguration, true)
130+
val headerContext = MatchingContext(expected.matchingRules.rulesForCategory("header"), false, pluginConfiguration, true)
131131

132132
return RequestMatchResult(Matching.matchMethod(expected.method, actual.method),
133133
Matching.matchPath(expected, actual, pathContext),

‎core/matchers/src/main/kotlin/au/com/dius/pact/core/matchers/ResponseMatching.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ object ResponseMatching : KLogging() {
3030
): List<Mismatch> {
3131
val statusContext = MatchingContext(expected.matchingRules.rulesForCategory("status"), true, pluginConfiguration)
3232
val bodyContext = MatchingContext(expected.matchingRules.rulesForCategory("body"), true, pluginConfiguration)
33-
val headerContext = MatchingContext(expected.matchingRules.rulesForCategory("header"), true, pluginConfiguration)
33+
val headerContext = MatchingContext(expected.matchingRules.rulesForCategory("header"), true, pluginConfiguration, true)
3434

3535
val bodyResults = Matching.matchBody(expected.asHttpPart(), actual.asHttpPart(), bodyContext)
3636
val typeResult = if (bodyResults.typeMismatch != null) {

‎core/matchers/src/test/groovy/au/com/dius/pact/core/matchers/MatcherExecutorKtSpec.groovy

Lines changed: 182 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,18 @@
11
package au.com.dius.pact.core.matchers
22

3+
import au.com.dius.pact.core.model.matchingrules.MatchingRuleCategory
34
import au.com.dius.pact.core.support.json.JsonValue
5+
import groovy.transform.TupleConstructor
6+
import org.w3c.dom.Document
7+
import org.w3c.dom.NamedNodeMap
8+
import org.w3c.dom.Node
9+
import org.w3c.dom.NodeList
10+
import org.w3c.dom.UserDataHandler
411
import spock.lang.Specification
512

6-
@SuppressWarnings('LineLength')
13+
import static au.com.dius.pact.core.model.matchingrules.NumberTypeMatcher.NumberType.NUMBER
14+
15+
@SuppressWarnings(['LineLength', 'SpaceAroundOperator', 'GetterMethodCouldBeProperty'])
716
class MatcherExecutorKtSpec extends Specification {
817

918
def 'match regex'() {
@@ -28,4 +37,176 @@ class MatcherExecutorKtSpec extends Specification {
2837
'04.5.7' | [new HeaderMismatch('test', '', '04.5.7', "'04.5.7' is not a valid semantic version")]
2938
new JsonValue.StringValue('4.5.7') | []
3039
}
40+
41+
def 'matching numbers'() {
42+
given:
43+
def mismatchFactory = { a, b, c, d -> new HeaderMismatch('test', '', actual.toString(), c) } as MismatchFactory
44+
45+
expect:
46+
MatcherExecutorKt.matchNumber(NUMBER, ['$'], expected, actual, mismatchFactory, null)?[0]?.mismatch == result
47+
48+
where:
49+
50+
expected | actual | result
51+
null | null | 'Expected null (Null) to be a number'
52+
null | '4.5' | "Expected '4.5' (String) to be a null value"
53+
100 | null | 'Expected null (Null) to be a number'
54+
100 | '4.5' | "Expected '4.5' (String) to be a number"
55+
100 | 4.5 | null
56+
100 | 4 | null
57+
100 | new JsonValue.StringValue('4.5.7.8') | "Expected '4.5.7.8' (String) to be a number"
58+
100 | new JsonValue.Integer(200) | null
59+
100 | new JsonValue.Decimal(200.10) | null
60+
100 | false | 'Expected false (Boolean) to be a number'
61+
100 | [100] | 'Expected [100] (Array) to be a number'
62+
100 | [a: 200.3, b: 200, c: 300] | 'Expected {a=200.3, b=200, c=300} (LinkedHashMap) to be a number'
63+
100 | 2.300g | null
64+
100 | 2.300d | null
65+
100 | new TestNode('not a number') | 'Expected TestNode(not a number) (TestNode) to be a number'
66+
100 | new TestNode('22.33.44') | 'Expected TestNode(22.33.44) (TestNode) to be a number'
67+
100 | new TestNode('22.33') | null
68+
}
69+
70+
def 'matching numbers with coercion enabled'() {
71+
given:
72+
def mismatchFactory = { a, b, c, d -> new HeaderMismatch('test', '', actual.toString(), c) } as MismatchFactory
73+
def context = new MatchingContext(new MatchingRuleCategory('test'), false, [:], true)
74+
75+
expect:
76+
MatcherExecutorKt.matchNumber(NUMBER, ['$'], expected, actual, mismatchFactory, context)?[0]?.mismatch == result
77+
78+
where:
79+
80+
expected | actual | result
81+
100 | '4.5' | null
82+
100 | 4.5 | null
83+
100 | 4 | null
84+
100 | new JsonValue.StringValue('4.5.7.8') | "Expected '4.5.7.8' (String) to be a number"
85+
100 | new JsonValue.StringValue('4.5') | null
86+
100 | new JsonValue.Integer(200) | null
87+
100 | new JsonValue.Decimal(200.10) | null
88+
}
89+
90+
@SuppressWarnings('UnnecessaryCast')
91+
def 'matching integer values'() {
92+
expect:
93+
MatcherExecutorKt.matchInteger(value, null) == result
94+
95+
where:
96+
97+
value | result
98+
'100' | false
99+
'100x' | false
100+
100 | true
101+
100.0 | false
102+
100i | true
103+
100l | true
104+
100 as BigInteger | true
105+
100g | true
106+
BigInteger.ZERO | true
107+
BigDecimal.ZERO | true
108+
}
109+
110+
@SuppressWarnings('UnnecessaryCast')
111+
def 'matching integer values with coercion enabled'() {
112+
given:
113+
def context = new MatchingContext(new MatchingRuleCategory('test'), false, [:], true)
114+
115+
expect:
116+
MatcherExecutorKt.matchInteger(value, context) == result
117+
118+
where:
119+
120+
value | result
121+
'100' | true
122+
'100x' | false
123+
'x100' | false
124+
100 | true
125+
}
126+
127+
@SuppressWarnings('UnnecessaryCast')
128+
def 'matching decimal number values'() {
129+
expect:
130+
MatcherExecutorKt.matchDecimal(value, null) == result
131+
132+
where:
133+
134+
value | result
135+
new JsonValue.Decimal('0'.chars) | true
136+
'100' | false
137+
'100.0' | false
138+
100 | false
139+
100.0 | true
140+
100.0f | true
141+
100.0d | true
142+
100i | false
143+
100l | false
144+
100 as BigInteger | false
145+
BigInteger.ZERO | false
146+
BigDecimal.ZERO | true
147+
}
148+
149+
@SuppressWarnings('UnnecessaryCast')
150+
def 'matching decimal number values with coercion enabled'() {
151+
given:
152+
def context = new MatchingContext(new MatchingRuleCategory('test'), false, [:], true)
153+
154+
expect:
155+
MatcherExecutorKt.matchDecimal(value, context) == result
156+
157+
where:
158+
159+
value | result
160+
new JsonValue.Decimal('0'.chars) | true
161+
'100' | false
162+
'100.0' | true
163+
'100.0x' | false
164+
'x100.0' | false
165+
}
166+
167+
@TupleConstructor
168+
@SuppressWarnings(['EmptyMethod', 'UnusedMethodParameter'])
169+
static class TestNode implements Node {
170+
String value
171+
172+
String toString() { "TestNode($value)" }
173+
174+
String getNodeName() { 'TestNode' }
175+
String getNodeValue() { value }
176+
void setNodeValue(String nodeValue) { }
177+
short getNodeType() { 0 }
178+
Node getParentNode() { null }
179+
NodeList getChildNodes() { null }
180+
Node getFirstChild() { null }
181+
Node getLastChild() { null }
182+
Node getPreviousSibling() { null }
183+
Node getNextSibling() { null }
184+
NamedNodeMap getAttributes() { null }
185+
Document getOwnerDocument() { null }
186+
Node insertBefore(Node newChild, Node refChild) { null }
187+
Node replaceChild(Node newChild, Node oldChild) { null }
188+
Node removeChild(Node oldChild) { null }
189+
Node appendChild(Node newChild) { null }
190+
boolean hasChildNodes() { false }
191+
Node cloneNode(boolean deep) { null }
192+
void normalize() { }
193+
boolean isSupported(String feature, String version) { false }
194+
String getNamespaceURI() { null }
195+
String getPrefix() { null }
196+
void setPrefix(String prefix) { }
197+
String getLocalName() { null }
198+
boolean hasAttributes() { false }
199+
String getBaseURI() { null }
200+
short compareDocumentPosition(Node other) { 0 }
201+
String getTextContent() { null }
202+
void setTextContent(String textContent) { }
203+
boolean isSameNode(Node other) { false }
204+
String lookupPrefix(String namespaceURI) { null }
205+
boolean isDefaultNamespace(String namespaceURI) { false }
206+
String lookupNamespaceURI(String prefix) { null }
207+
boolean isEqualNode(Node arg) { false }
208+
Object getFeature(String feature, String version) { null }
209+
Object setUserData(String key, Object data, UserDataHandler handler) { null }
210+
Object getUserData(String key) { null }
211+
}
31212
}

0 commit comments

Comments
 (0)