fix: Target request filter validation for MockMvcTarget #983

Author: Ronald Holshausen

core/pact-broker/src/main/kotlin/au/com/dius/pact/core/pactbroker/HalClient.kt

@@ -271,7 +271,7 @@ open class HalClient @JvmOverloads constructor(
       if (response.statusLine.statusCode < 300) {
         val contentType = ContentType.getOrDefault(response.entity)
         if (isJsonResponse(contentType)) {
-          return@of JsonParser().parse(EntityUtils.toString(response.entity))
+          return@of JsonParser.parseString(EntityUtils.toString(response.entity))
         } else {
           throw InvalidHalResponse("Expected a HAL+JSON response from the pact broker, but got '$contentType'")
         }
@@ -408,16 +408,16 @@ open class HalClient @JvmOverloads constructor(
       if (isJsonResponse(contentType)) {
         var error = ""
         if (body.isNotEmpty()) {
-          val jsonBody = JsonParser().parse(body)
+          val jsonBody = JsonParser.parseString(body)
           if (jsonBody != null && jsonBody.obj.has("errors")) {
             if (jsonBody["errors"].isJsonArray) {
               error = " - " + jsonBody["errors"].asJsonArray.joinToString(", ") { it.asString }
             } else if (jsonBody["errors"].isJsonObject) {
-              error = " - " + jsonBody["errors"].asJsonObject.entrySet().joinToString(", ") {
-                if (it.value.isJsonArray) {
-                  "${it.key}: ${it.value.array.joinToString(", ") { it.asString }}"
+              error = " - " + jsonBody["errors"].asJsonObject.entrySet().joinToString(", ") { entry ->
+                if (entry.value.isJsonArray) {
+                  "${entry.key}: ${entry.value.array.joinToString(", ") { it.asString }}"
                 } else {
-                  "${it.key}: ${it.value.asString}"
+                  "${entry.key}: ${entry.value.asString}"
                 }
               }
             }

core/support/src/main/kotlin/au/com/dius/pact/core/support/Json.kt

@@ -107,7 +107,7 @@ object Json {
     }
   }
 
-  fun prettyPrint(json: String) = gsonPretty.toJson(JsonParser().parse(json))
+  fun prettyPrint(json: String): String = gsonPretty.toJson(JsonParser.parseString(json))
 
   fun exceptionToJson(exp: Exception) = jsonObject("message" to exp.message,
     "exceptionClass" to exp.javaClass.name)

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

@@ -84,24 +84,18 @@ open class InteractionRunner<I>(
     validatePublicVoidNoArgMethods(After::class.java, false, errors)
     validateStateChangeMethods(testClass, errors)
     validateConstructor(errors)
-    val targetField = validateTestTarget(errors)
+    validateTestTarget(errors)
     validateRules(errors)
-    validateTargetRequestFilters(errors, targetField)
+    validateTargetRequestFilters(errors)
 
     if (errors.isNotEmpty()) {
       throw InitializationError(errors)
     }
   }
 
-  private fun validateTargetRequestFilters(errors: MutableList<Throwable>, targetField: FrameworkField) {
+  private fun validateTargetRequestFilters(errors: MutableList<Throwable>) {
     testClass.getAnnotatedMethods(TargetRequestFilter::class.java).forEach { method ->
       method.validatePublicVoid(false, errors)
-      val requestClass = (targetField.type.newInstance() as Target).getRequestClass()
-      if (method.method.parameterTypes.size != 1) {
-        errors.add(Exception("Method ${method.name} should take only a single ${requestClass.simpleName} parameter"))
-      } else if (!requestClass.isAssignableFrom(method.method.parameterTypes[0])) {
-        errors.add(Exception("Method ${method.name} should take only a single HttpRequest parameter"))
-      }
     }
   }
 

provider/pact-jvm-provider-junit/src/main/kotlin/au/com/dius/pact/provider/junit/target/AmqpTarget.kt

@@ -100,7 +100,9 @@ open class AmqpTarget @JvmOverloads constructor(
 
     if (source is PactBrokerSource<*>) {
       val (_, _, _, pacts) = source
-      providerInfo.consumers = pacts.entries.flatMap { e -> e.value.map { p -> ConsumerInfo(e.key.name, p) } }.toMutableList()
+      providerInfo.consumers = pacts.entries.flatMap { e ->
+        e.value.map { p -> ConsumerInfo(e.key.name, p) }
+      }.toMutableList()
     } else if (source is DirectorySource<*>) {
       val (_, pacts) = source
       providerInfo.consumers = pacts.entries.map { e -> ConsumerInfo(e.value.consumer.name, e.value) }.toMutableList()

provider/pact-jvm-provider-junit/src/main/kotlin/au/com/dius/pact/provider/junit/target/BaseTarget.kt

@@ -15,6 +15,7 @@ import java.io.File
 import java.util.function.BiConsumer
 import java.util.function.Supplier
 import org.apache.commons.lang3.tuple.Pair
+import org.junit.runners.model.FrameworkMethod
 
 /**
  * Out-of-the-box implementation of [Target],
@@ -100,4 +101,15 @@ abstract class BaseTarget : TestClassAwareTarget {
     this.stateHandlers.add(stateHandler)
     return this
   }
+
+  protected fun validateTargetRequestFilters(methods: MutableList<FrameworkMethod>) {
+    methods.forEach { method ->
+      val requestClass = getRequestClass()
+      if (method.method.parameterTypes.size != 1) {
+        throw Exception("Method ${method.name} should take only a single ${requestClass.simpleName} parameter")
+      } else if (!requestClass.isAssignableFrom(method.method.parameterTypes[0])) {
+        throw Exception("Method ${method.name} should take only a single ${requestClass.simpleName} parameter")
+      }
+    }
+  }
 }

provider/pact-jvm-provider-junit/src/main/kotlin/au/com/dius/pact/provider/junit/target/HttpTarget.kt

@@ -118,6 +118,8 @@ open class HttpTarget
 
     val methods = testClass.getAnnotatedMethods(TargetRequestFilter::class.java)
     if (methods.isNotEmpty()) {
+      validateTargetRequestFilters(methods)
+
       providerInfo.requestFilter = Consumer { httpRequest: HttpRequest ->
         methods.forEach { method ->
           try {

provider/pact-jvm-provider-spring/src/main/kotlin/au/com/dius/pact/provider/spring/target/MockMvcTarget.kt

@@ -131,6 +131,7 @@ class MockMvcTarget @JvmOverloads constructor(
 
     val methods = testClass.getAnnotatedMethods(TargetRequestFilter::class.java)
     if (methods.isNotEmpty()) {
+      validateTargetRequestFilters(methods)
       providerInfo.requestFilter = Consumer<MockHttpServletRequestBuilder> { httpRequest ->
         methods.forEach { method ->
           try {