From d56079c0d27dbd91b42b2fd336ca2b6fce802591 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 +++++++++++++++ .../junitsupport/loader/PactBrokerLoader.kt | 42 +++++++++++++------ .../loader/PactBrokerLoaderSpec.groovy | 14 ++++++- 4 files changed, 77 insertions(+), 15 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 51cc1aa770..15a15cf967 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 @@ -145,7 +145,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/junitsupport/loader/PactBrokerLoader.kt b/provider/src/main/kotlin/au/com/dius/pact/provider/junitsupport/loader/PactBrokerLoader.kt index c1a82fbb3a..bf1249ef95 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 @@ -23,15 +23,18 @@ import com.github.michaelbull.result.Ok import mu.KLogging import org.apache.hc.core5.net.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 import kotlin.reflect.full.isSubtypeOf import kotlin.reflect.full.starProjectedType +import kotlin.reflect.jvm.kotlinFunction /** * Out-of-the-box implementation of {@link PactLoader} that downloads pacts from Pact broker @@ -372,19 +375,34 @@ open class PactBrokerLoader( } @JvmStatic - fun testClassHasSelectorsMethod(testClass: Class<*>?): KCallable<*>? { - val projectedType = SelectorBuilder::class.starProjectedType - return 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}.") + } } + + 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 50e2fc1b3b..d83197639d 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 @@ -1444,8 +1444,6 @@ class PactBrokerLoaderSpec extends Specification { null | false PactBrokerLoaderSpec | false FullPactBrokerAnnotation | false - IncorrectTypesOnSelectorMethod | false - IncorrectTypesOnSelectorMethod2 | false IncorrectScopeOnSelectorMethod | false CorrectSelectorMethod | true CorrectSelectorMethod2 | true @@ -1455,6 +1453,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: