fix: raise an exception when the consumerVersionSelectors method has …

…the wrong signature #1594

provider/junit5/src/main/kotlin/au/com/dius/pact/provider/junit5/PactJUnit5VerificationProvider.kt

@@ -144,7 +144,7 @@ open class PactVerificationInvocationContextProvider : TestTemplateInvocationCon
 
     logger.debug { "Pact sources on test class:\n ${pactSources.joinToString("\n") { it.first.toString() }}" }
     return pactSources.map { (pactSource, annotation) ->
-      instantiatePactLoader(pactSource, context.requiredTestClass, context.testInstance, annotation)
+      instantiatePactLoader(pactSource, context.requiredTestClass, context.testInstance.orElse(null), annotation)
     }.map {
       checkForOverriddenPactUrl(it,
         context.requiredTestClass.getAnnotation(AllowOverridePactUrl::class.java),

provider/junit5spring/src/test/kotlin/au/com/dius/pact/provider/spring/junit5/ConsumerVersionSelectorTest.kt

@@ -0,0 +1,34 @@
+package au.com.dius.pact.provider.spring.junit5
+
+import au.com.dius.pact.provider.junitsupport.IgnoreNoPactsToVerify
+import au.com.dius.pact.provider.junitsupport.Provider
+import au.com.dius.pact.provider.junitsupport.loader.PactBroker
+import au.com.dius.pact.provider.junit5.PactVerificationContext
+import au.com.dius.pact.provider.junitsupport.loader.PactBrokerConsumerVersionSelectors
+import au.com.dius.pact.provider.junitsupport.loader.SelectorBuilder
+import org.junit.jupiter.api.Disabled
+import org.junit.jupiter.api.TestTemplate
+import org.junit.jupiter.api.extension.ExtendWith
+import org.springframework.boot.autoconfigure.SpringBootApplication
+import org.springframework.boot.test.context.SpringBootTest
+import org.springframework.test.context.junit.jupiter.SpringExtension
+
+@ExtendWith(SpringExtension::class)
+@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
+@Provider("Animal Profile Service")
+@PactBroker
+@IgnoreNoPactsToVerify(ignoreIoErrors = "true")
+@Disabled
+open class ConsumerVersionSelectorTest {
+  companion object {
+    @PactBrokerConsumerVersionSelectors
+    @JvmStatic
+    fun consumerVersionSelectors() = SelectorBuilder().branch("current")
+  }
+
+  @TestTemplate
+  @ExtendWith(PactVerificationSpringProvider::class)
+  fun pactVerificationTestTemplate(context: PactVerificationContext?) {
+    context?.verifyInteraction()
+  }
+}

provider/src/main/kotlin/au/com/dius/pact/provider/ProviderUtils.kt

@@ -134,7 +134,12 @@ object ProviderUtils : KLogging() {
     return result
   }
 
-  fun instantiatePactLoader(pactSource: PactSource, testClass: Class<*>, testInstance: Any?, annotation: Annotation?): PactLoader {
+  fun instantiatePactLoader(
+    pactSource: PactSource,
+    testClass: Class<*>,
+    testInstance: Any?,
+    annotation: Annotation?
+  ): PactLoader {
     val pactLoaderClass = pactSource.value
     val pactLoader = try {
       // Checks if there is a constructor with one argument of type Class.

provider/src/main/kotlin/au/com/dius/pact/provider/junitsupport/loader/PactBrokerLoader.kt

@@ -24,10 +24,12 @@ import com.github.michaelbull.result.Ok
 import mu.KLogging
 import org.apache.http.client.utils.URIBuilder
 import java.io.IOException
+import java.lang.reflect.Modifier
 import java.net.URI
 import java.net.URISyntaxException
 import kotlin.reflect.KCallable
 import kotlin.reflect.KClass
+import kotlin.reflect.KFunction
 import kotlin.reflect.KParameter
 import kotlin.reflect.KVisibility
 import kotlin.reflect.full.findAnnotation
@@ -371,25 +373,34 @@ open class PactBrokerLoader(
     }
 
     @JvmStatic
-    fun testClassHasSelectorsMethod(testClass: Class<*>?): KCallable<*>? {
-      val projectedType = SelectorBuilder::class.starProjectedType
-      return try {
-        testClass?.kotlin?.members?.firstOrNull { method ->
-          method.findAnnotation<PactBrokerConsumerVersionSelectors>() != null && (
-            // static method
-            method.parameters.isEmpty()
-              // instance method
-              || (method.parameters.size == 1 && method.parameters[0].kind == KParameter.Kind.INSTANCE)
-            && method.visibility == KVisibility.PUBLIC
-            && (
-              method.returnType.isSubtypeOf(projectedType)
-              || method.returnType.isSubtypeOf(List::class.starProjectedType)
-            )
-          )
+    fun testClassHasSelectorsMethod(testClass: Class<*>?): KFunction<*>? {
+      val method = testClass?.methods?.firstOrNull { method ->
+        method.getAnnotation(PactBrokerConsumerVersionSelectors::class.java) != null
+      }
+
+      if (method != null) {
+        if (method.parameterCount > 0) {
+          throw IllegalAccessException("Consumer version selector methods must not have any parameters. " +
+            "Method ${method.name} has ${method.parameterCount}.")
+        }
+        val modifiers = method.modifiers
+        if (!Modifier.isPublic(modifiers)) {
+          throw IllegalAccessException("Consumer version selector methods must be public and static. " +
+            "Method ${method.name} is not accessible.")
+        }
+        if (!method.trySetAccessible() && !method.canAccess(null)) {
+          throw IllegalAccessException("Consumer version selector methods must be public and static. " +
+            "Method ${method.name} is not accessible (canAccess returned false).")
+        }
+
+        if (!SelectorBuilder::class.java.isAssignableFrom(method.returnType)
+          && !List::class.java.isAssignableFrom(method.returnType)) {
+          throw IllegalAccessException("Consumer version selector methods must be return either a SelectorBuilder or" +
+            " a list of ConsumerVersionSelectors. ${method.name} returns a ${method.returnType.simpleName}.")
         }
-      } catch (e: NullPointerException) {
-        null
       }
+
+      return method?.kotlinFunction
     }
   }
 }

provider/src/test/groovy/au/com/dius/pact/provider/junitsupport/loader/PactBrokerLoaderSpec.groovy

@@ -1442,8 +1442,6 @@ class PactBrokerLoaderSpec extends Specification {
     null                                  | false
     PactBrokerLoaderSpec                  | false
     FullPactBrokerAnnotation              | false
-    IncorrectTypesOnSelectorMethod        | false
-    IncorrectTypesOnSelectorMethod2       | false
     IncorrectScopeOnSelectorMethod        | false
     CorrectSelectorMethod                 | true
     CorrectSelectorMethod2                | true
@@ -1453,6 +1451,18 @@ class PactBrokerLoaderSpec extends Specification {
     KotlinAbstractClassWithSelectorMethod | true
   }
 
+  @Unroll
+  def 'test Class Has Selectors Method - invalid methods'() {
+    when:
+    PactBrokerLoader.testClassHasSelectorsMethod(clazz)
+
+    then:
+    thrown(IllegalAccessException)
+
+    where:
+    clazz << [IncorrectTypesOnSelectorMethod, IncorrectTypesOnSelectorMethod2 ]
+  }
+
   @Unroll
   def 'Invoke Selectors Method'() {
     expect:

settings.gradle

@@ -35,4 +35,4 @@ project(':provider:scalasupport_2.13').projectDir = file('provider/scalasupport'
 
 include 'pact-jvm-server'
 include   'pact-specification-test'
-include 'pact-publish'
+//include 'pact-publish'