fix: let invalid path exceptions propogate so verification fails #957

Author: Ronald Holshausen

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

@@ -2,7 +2,6 @@ package au.com.dius.pact.core.matchers
 
 import au.com.dius.pact.core.matchers.util.corresponds
 import au.com.dius.pact.core.matchers.util.tails
-import au.com.dius.pact.core.model.InvalidPathExpression
 import au.com.dius.pact.core.model.PathToken
 import au.com.dius.pact.core.model.matchingrules.MatchingRuleGroup
 import au.com.dius.pact.core.model.matchingrules.MatchingRules
@@ -28,34 +27,24 @@ object Matchers : KLogging() {
   }
 
   fun matchesPath(pathExp: String, path: List<String>): Int {
-    return try {
-      val parseResult = parsePath(pathExp)
-      val filter = tails(path.reversed()).filter { l ->
-        corresponds(l.reversed(), parseResult) { pathElement, pathToken ->
-          matchesToken(pathElement, pathToken) != 0
-        }
-      }
-      if (filter.isNotEmpty()) {
-        filter.maxBy { seq -> seq.size }?.size ?: 0
-      } else {
-        0
+    val parseResult = parsePath(pathExp)
+    val filter = tails(path.reversed()).filter { l ->
+      corresponds(l.reversed(), parseResult) { pathElement, pathToken ->
+        matchesToken(pathElement, pathToken) != 0
       }
-    } catch (e: InvalidPathExpression) {
-      logger.warn(e) { "Path expression $pathExp is invalid, ignoring" }
+    }
+    return if (filter.isNotEmpty()) {
+      filter.maxBy { seq -> seq.size }?.size ?: 0
+    } else {
       0
     }
   }
 
   fun calculatePathWeight(pathExp: String, path: List<String>): Int {
-    return try {
-      val parseResult = parsePath(pathExp)
-      path.zip(parseResult).asSequence().map {
-          matchesToken(it.first, it.second)
-      }.reduce { acc, i -> acc * i }
-    } catch (e: InvalidPathExpression) {
-      logger.warn(e) { "Path expression $pathExp is invalid, ignoring" }
-      0
-    }
+    val parseResult = parsePath(pathExp)
+    return path.zip(parseResult).asSequence().map {
+        matchesToken(it.first, it.second)
+    }.reduce { acc, i -> acc * i }
   }
 
   fun resolveMatchers(

core/matchers/src/test/groovy/au/com/dius/pact/core/matchers/MatchersSpec.groovy

@@ -1,5 +1,6 @@
 package au.com.dius.pact.core.matchers
 
+import au.com.dius.pact.core.model.InvalidPathExpression
 import au.com.dius.pact.core.model.OptionalBody
 import au.com.dius.pact.core.model.matchingrules.Category
 import au.com.dius.pact.core.model.matchingrules.EqualsMatcher
@@ -238,6 +239,22 @@ class MatchersSpec extends Specification {
     Matchers.INSTANCE.matchesPath('$[*]', ['$', 'str']) == 0
   }
 
+  def 'path matching - throws an exception if path is invalid'() {
+    when:
+    Matchers.INSTANCE.matchesPath("\$.serviceNode.entity.status.thirdNode['@description]", ['a'])
+
+    then:
+    thrown(InvalidPathExpression)
+  }
+
+  def 'calculatePathWeight - throws an exception if path is invalid'() {
+    when:
+    Matchers.INSTANCE.calculatePathWeight("\$.serviceNode.entity.status.thirdNode['@description]", ['a'])
+
+    then:
+    thrown(InvalidPathExpression)
+  }
+
   def 'resolveMatchers returns all matchers for the general case'() {
     given:
     def matchers = new MatchingRulesImpl()

provider/pact-jvm-provider/src/main/kotlin/au/com/dius/pact/provider/ProviderVerifier.kt

@@ -419,16 +419,17 @@ open class ProviderVerifier @JvmOverloads constructor (
     reporters.forEach { it.finaliseReport() }
   }
 
+  @JvmOverloads
   fun verifyInteraction(
     provider: IProviderInfo,
     consumer: IConsumerInfo,
     failures: MutableMap<String, Any>,
-    interaction: Interaction
+    interaction: Interaction,
+    providerClient: ProviderClient = ProviderClient(provider, HttpClientFactory())
   ): TestResult {
     var interactionMessage = "Verifying a pact between ${consumer.name} and ${provider.name}" +
     " - ${interaction.description} "
 
-    val providerClient = ProviderClient(provider, HttpClientFactory())
     val stateChangeResult = stateChangeHandler.executeStateChange(this, provider, consumer, interaction, interactionMessage,
       failures, providerClient)
     if (stateChangeResult.stateChangeResult is Ok) {

provider/pact-jvm-provider/src/test/groovy/au/com/dius/pact/provider/ProviderVerifierSpec.groovy

@@ -4,6 +4,7 @@ import au.com.dius.pact.core.model.BrokerUrlSource
 import au.com.dius.pact.core.model.Consumer
 import au.com.dius.pact.core.model.FileSource
 import au.com.dius.pact.core.model.Interaction
+import au.com.dius.pact.core.model.InvalidPathExpression
 import au.com.dius.pact.core.model.OptionalBody
 import au.com.dius.pact.core.model.Pact
 import au.com.dius.pact.core.model.PactReader
@@ -16,12 +17,16 @@ import au.com.dius.pact.core.model.Response
 import au.com.dius.pact.core.model.UnknownPactSource
 import au.com.dius.pact.core.model.UrlSource
 import au.com.dius.pact.core.model.generators.Generators
+import au.com.dius.pact.core.model.matchingrules.MatchingRules
 import au.com.dius.pact.core.model.matchingrules.MatchingRulesImpl
+import au.com.dius.pact.core.model.matchingrules.RegexMatcher
 import au.com.dius.pact.core.model.messaging.Message
 import au.com.dius.pact.core.pactbroker.TestResult
 import au.com.dius.pact.core.pactbroker.PactBrokerClient
 import au.com.dius.pact.provider.reporters.VerifierReporter
 import au.com.dius.pact.com.github.michaelbull.result.Ok
+import groovy.json.JsonOutput
+import org.apache.http.entity.ContentType
 import spock.lang.Specification
 import spock.lang.Unroll
 import spock.util.environment.RestoreSystemProperties
@@ -584,6 +589,46 @@ class ProviderVerifierSpec extends Specification {
     result.results[0].interactionId == '1234'
   }
 
+  def 'verifyInteraction returns an error result if any matcher paths are invalid'() {
+    given:
+    ProviderInfo provider = new ProviderInfo('Test Provider')
+    ConsumerInfo consumer = new ConsumerInfo(name: 'Test Consumer', pactSource: UnknownPactSource.INSTANCE)
+    def failures = [:]
+    MatchingRules matchingRules = new MatchingRulesImpl()
+    matchingRules.addCategory('body')
+      .addRule("\$.serviceNode.entity.status.thirdNode['@description]", new RegexMatcher('.*'))
+    def json = JsonOutput.toJson([
+      serviceNode: [
+        entity: [
+          status: [
+            thirdNode: [
+              '@description': 'Test'
+            ]
+          ]
+        ]
+      ]
+    ])
+    Interaction interaction = new RequestResponseInteraction('Test Interaction',
+      [new ProviderState('Test State')], new Request(),
+      new Response(200, [:], OptionalBody.body(json.bytes), matchingRules), '1234')
+    def client = Mock(ProviderClient)
+    client.makeRequest(_) >> [
+      statusCode: 200,
+      headers: [:],
+      contentType: ContentType.APPLICATION_JSON,
+      data: json
+    ]
+
+    when:
+    def result = verifier.verifyInteraction(provider, consumer, failures, interaction, client)
+
+    then:
+    result instanceof TestResult.Failed
+    result.results.size() == 1
+    result.results[0].message == 'Request to provider failed with an exception'
+    result.results[0].exception instanceof InvalidPathExpression
+  }
+
   def 'verifyResponseFromProvider returns an error result if the request to the provider fails with an exception'() {
     given:
     ProviderInfo provider = new ProviderInfo('Test Provider')