From dcfdb6a8b98debdb099991028cc8504f1aaade14 Mon Sep 17 00:00:00 2001 From: Ronald Holshausen Date: Thu, 11 Aug 2022 11:53:19 +1000 Subject: [PATCH] fix: raise an exception when the consumerVersionSelectors method has the wrong signature #1594 --- .../junit5/PactJUnit5VerificationProvider.kt | 2 +- .../junit5/ConsumerVersionSelectorTest.kt | 34 ++++++++++++++ .../com/dius/pact/provider/ProviderUtils.kt | 7 ++- .../junitsupport/loader/PactBrokerLoader.kt | 45 ++++++++++++------- .../loader/PactBrokerLoaderSpec.groovy | 14 +++++- settings.gradle | 2 +- 6 files changed, 82 insertions(+), 22 deletions(-) create mode 100644 provider/junit5spring/src/test/kotlin/au/com/dius/pact/provider/spring/junit5/ConsumerVersionSelectorTest.kt diff --git a/provider/junit5/src/main/kotlin/au/com/dius/pact/provider/junit5/PactJUnit5VerificationProvider.kt b/provider/junit5/src/main/kotlin/au/com/dius/pact/provider/junit5/PactJUnit5VerificationProvider.kt index 32fe327de7..698715c8fc 100644 --- a/provider/junit5/src/main/kotlin/au/com/dius/pact/provider/junit5/PactJUnit5VerificationProvider.kt +++ b/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), diff --git a/provider/junit5spring/src/test/kotlin/au/com/dius/pact/provider/spring/junit5/ConsumerVersionSelectorTest.kt b/provider/junit5spring/src/test/kotlin/au/com/dius/pact/provider/spring/junit5/ConsumerVersionSelectorTest.kt new file mode 100644 index 0000000000..d290802f3c --- /dev/null +++ b/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() + } +} diff --git a/provider/src/main/kotlin/au/com/dius/pact/provider/ProviderUtils.kt b/provider/src/main/kotlin/au/com/dius/pact/provider/ProviderUtils.kt index 700e585885..9f86d0c8b4 100644 --- a/provider/src/main/kotlin/au/com/dius/pact/provider/ProviderUtils.kt +++ b/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. diff --git a/provider/src/main/kotlin/au/com/dius/pact/provider/junitsupport/loader/PactBrokerLoader.kt b/provider/src/main/kotlin/au/com/dius/pact/provider/junitsupport/loader/PactBrokerLoader.kt index e88be8734b..453522211f 100644 --- a/provider/src/main/kotlin/au/com/dius/pact/provider/junitsupport/loader/PactBrokerLoader.kt +++ b/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() != 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 } } } diff --git a/provider/src/test/groovy/au/com/dius/pact/provider/junitsupport/loader/PactBrokerLoaderSpec.groovy b/provider/src/test/groovy/au/com/dius/pact/provider/junitsupport/loader/PactBrokerLoaderSpec.groovy index 01f30ee8a9..a629c36f5f 100644 --- a/provider/src/test/groovy/au/com/dius/pact/provider/junitsupport/loader/PactBrokerLoaderSpec.groovy +++ b/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: diff --git a/settings.gradle b/settings.gradle index 0b360abf93..836e2fbc1d 100644 --- a/settings.gradle +++ b/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'