From fdb9f999fd6b1cb19ef72ac846a4d4059724f5de Mon Sep 17 00:00:00 2001 From: Emanuele Date: Thu, 2 Jul 2020 16:33:44 +0100 Subject: [PATCH 1/3] fix: fix a problem where the provider version trim property is ignored --- .../pact/provider/junit/InteractionRunner.kt | 10 +--- .../pact/provider/maven/PactProviderMojo.kt | 3 +- .../pact/provider/maven/PactPublishMojo.kt | 4 +- .../provider/maven/PactPublishMojoSpec.groovy | 12 +++-- .../com/dius/pact/provider/ProviderUtils.kt | 20 -------- .../dius/pact/provider/ProviderVerifier.kt | 8 +-- .../com/dius/pact/provider/ProviderVersion.kt | 37 ++++++++++++++ .../pact/provider/ProviderUtilsSpec.groovy | 51 ------------------- .../pact/provider/ProviderVersionSpec.groovy | 33 ++++++++++++ 9 files changed, 89 insertions(+), 89 deletions(-) create mode 100644 provider/src/main/kotlin/au/com/dius/pact/provider/ProviderVersion.kt create mode 100644 provider/src/test/groovy/au/com/dius/pact/provider/ProviderVersionSpec.groovy diff --git a/provider/junit/src/main/kotlin/au/com/dius/pact/provider/junit/InteractionRunner.kt b/provider/junit/src/main/kotlin/au/com/dius/pact/provider/junit/InteractionRunner.kt index 3ee205c055..de795e8de1 100644 --- a/provider/junit/src/main/kotlin/au/com/dius/pact/provider/junit/InteractionRunner.kt +++ b/provider/junit/src/main/kotlin/au/com/dius/pact/provider/junit/InteractionRunner.kt @@ -8,7 +8,7 @@ import au.com.dius.pact.core.model.PactSource import au.com.dius.pact.core.model.ProviderState import au.com.dius.pact.provider.DefaultTestResultAccumulator import au.com.dius.pact.provider.IProviderVerifier -import au.com.dius.pact.provider.ProviderUtils +import au.com.dius.pact.provider.ProviderVersion import au.com.dius.pact.provider.TestResultAccumulator import au.com.dius.pact.provider.VerificationFailureType import au.com.dius.pact.provider.VerificationResult @@ -187,13 +187,7 @@ open class InteractionRunner( } private fun providerVersion(): String { - val version = System.getProperty("pact.provider.version") - return if (version != null) { - ProviderUtils.getProviderVersion(version) - } else { - logger.warn { "Set the provider version using the 'pact.provider.version' property. Defaulting to '0.0.0'" } - "0.0.0" - } + return ProviderVersion { System.getProperty("pact.provider.version") }.get() } protected open fun createTest(): Any { diff --git a/provider/maven/src/main/kotlin/au/com/dius/pact/provider/maven/PactProviderMojo.kt b/provider/maven/src/main/kotlin/au/com/dius/pact/provider/maven/PactProviderMojo.kt index 2e000cd7c4..2870a11e9d 100644 --- a/provider/maven/src/main/kotlin/au/com/dius/pact/provider/maven/PactProviderMojo.kt +++ b/provider/maven/src/main/kotlin/au/com/dius/pact/provider/maven/PactProviderMojo.kt @@ -9,6 +9,7 @@ import au.com.dius.pact.provider.IProviderVerifier import au.com.dius.pact.provider.PactVerifierException import au.com.dius.pact.provider.ProviderUtils import au.com.dius.pact.provider.ProviderVerifier +import au.com.dius.pact.provider.ProviderVersion import au.com.dius.pact.provider.VerificationResult import au.com.dius.pact.provider.reporters.ReporterManager import org.apache.maven.plugin.MojoFailureException @@ -62,7 +63,7 @@ open class PactProviderMojo : PactBaseMojo() { "You must specify the pact file to execute for consumer '${consumer.name}' (use or )" } verifier.checkBuildSpecificTask = Function { false } - verifier.providerVersion = Supplier { ProviderUtils.getProviderVersion(projectVersion) } + verifier.providerVersion = ProviderVersion { projectVersion } verifier.projectClasspath = Supplier { classpathElements.map { File(it).toURI().toURL() } } diff --git a/provider/maven/src/main/kotlin/au/com/dius/pact/provider/maven/PactPublishMojo.kt b/provider/maven/src/main/kotlin/au/com/dius/pact/provider/maven/PactPublishMojo.kt index 1bbc446e32..99da74b129 100644 --- a/provider/maven/src/main/kotlin/au/com/dius/pact/provider/maven/PactPublishMojo.kt +++ b/provider/maven/src/main/kotlin/au/com/dius/pact/provider/maven/PactPublishMojo.kt @@ -46,9 +46,9 @@ open class PactPublishMojo : PactBaseMojo() { } val snapShotDefinitionString = "-SNAPSHOT" - val emptyString = "" if (trimSnapshot && projectVersion.contains(snapShotDefinitionString)) { - projectVersion = projectVersion.replaceFirst(snapShotDefinitionString, emptyString) + val snapshotRegex = Regex(".*($snapShotDefinitionString)") + projectVersion = projectVersion.removeRange(snapshotRegex.find(projectVersion)!!.groups[1]!!.range) } if (brokerClient == null) { diff --git a/provider/maven/src/test/groovy/au/com/dius/pact/provider/maven/PactPublishMojoSpec.groovy b/provider/maven/src/test/groovy/au/com/dius/pact/provider/maven/PactPublishMojoSpec.groovy index d14f802dfa..718f7eb1d8 100644 --- a/provider/maven/src/test/groovy/au/com/dius/pact/provider/maven/PactPublishMojoSpec.groovy +++ b/provider/maven/src/test/groovy/au/com/dius/pact/provider/maven/PactPublishMojoSpec.groovy @@ -106,16 +106,22 @@ class PactPublishMojoSpec extends Specification { assert mojo.projectVersion == '1.0.0' } - def 'trimSnapshot=true removes the "-SNAPSHOT" in the middle'() { + def 'trimSnapshot=true removes the last occurrence of "-SNAPSHOT"'() { given: - mojo.projectVersion = '1.0.0-SNAPSHOT-3ffe453efr' + mojo.projectVersion = projectVersion mojo.trimSnapshot = true when: mojo.execute() then: - assert mojo.projectVersion == '1.0.0-3ffe453efr' + assert mojo.projectVersion == result + + where: + projectVersion | result + '1.0.0-NOT-A-SNAPSHOT-abc-SNAPSHOT' | '1.0.0-NOT-A-SNAPSHOT-abc' + '1.0.0-NOT-A-SNAPSHOT-abc-SNAPSHOT-re234hj' | '1.0.0-NOT-A-SNAPSHOT-abc-re234hj' + '1.0.0-SNAPSHOT-re234hj' | '1.0.0-re234hj' } def 'trimSnapshot=false leaves version unchanged'() { 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 3d0fde0a61..269fd77eb0 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 @@ -4,7 +4,6 @@ import au.com.dius.pact.core.model.DefaultPactReader import au.com.dius.pact.core.model.FileSource import au.com.dius.pact.core.model.Interaction import org.apache.commons.io.FilenameUtils -import org.apache.commons.lang3.BooleanUtils import java.io.File /** @@ -70,23 +69,4 @@ object ProviderUtils { fun isS3Url(pactFile: Any?): Boolean { return pactFile is String && pactFile.toLowerCase().startsWith("s3://") } - - @JvmStatic - fun getProviderVersion(projectVersion: String): String { - val trimSnapshotProperty = System.getProperty(ProviderVerifier.PACT_PROVIDER_VERSION_TRIM_SNAPSHOT) - val isTrimSnapshot: Boolean = if (trimSnapshotProperty == null || trimSnapshotProperty.isBlank()) { - false - } else { - BooleanUtils.toBoolean(trimSnapshotProperty) - } - return if (isTrimSnapshot) trimSnapshot(projectVersion) else projectVersion - } - - private fun trimSnapshot(providerVersion: String): String { - val SNAPSHOT_STRING = "-SNAPSHOT" - if (providerVersion.contains(SNAPSHOT_STRING)) { - return providerVersion.replaceFirst(SNAPSHOT_STRING, "") - } - return providerVersion - } } diff --git a/provider/src/main/kotlin/au/com/dius/pact/provider/ProviderVerifier.kt b/provider/src/main/kotlin/au/com/dius/pact/provider/ProviderVerifier.kt index 4365ddbda9..3583f7482e 100644 --- a/provider/src/main/kotlin/au/com/dius/pact/provider/ProviderVerifier.kt +++ b/provider/src/main/kotlin/au/com/dius/pact/provider/ProviderVerifier.kt @@ -134,7 +134,7 @@ interface IProviderVerifier { /** * Callback to get the provider version */ - var providerVersion: Supplier + var providerVersion: Supplier /** * Callback to get the provider tag @@ -228,13 +228,14 @@ interface IProviderVerifier { */ @Suppress("TooManyFunctions") open class ProviderVerifier @JvmOverloads constructor ( + override var pactLoadFailureMessage: Any? = null, override var checkBuildSpecificTask: Function = Function { false }, override var executeBuildSpecificTask: BiConsumer = BiConsumer { _, _ -> }, override var projectClasspath: Supplier> = Supplier { emptyList() }, override var reporters: List = listOf(AnsiConsoleReporter("console", File("/tmp/"))), override var providerMethodInstance: Function = Function { m -> m.declaringClass.newInstance() }, - override var providerVersion: Supplier = Supplier { System.getProperty(PACT_PROVIDER_VERSION) }, + override var providerVersion: Supplier = ProviderVersion { System.getProperty(PACT_PROVIDER_VERSION) }, override var providerTag: Supplier? = Supplier { System.getProperty(PACT_PROVIDER_TAG) } ) : IProviderVerifier { @@ -665,8 +666,7 @@ open class ProviderVerifier @JvmOverloads constructor ( publishingResultsDisabled() -> reporters.forEach { it.warnPublishResultsSkippedBecauseDisabled(PACT_VERIFIER_PUBLISH_RESULTS) } - else -> verificationReporter.reportResults(pact, result.toTestResult(), providerVersion.get() ?: "0.0.0", - client, providerTag?.get()) + else -> verificationReporter.reportResults(pact, result.toTestResult(), providerVersion.get(), client, providerTag?.get()) } result } diff --git a/provider/src/main/kotlin/au/com/dius/pact/provider/ProviderVersion.kt b/provider/src/main/kotlin/au/com/dius/pact/provider/ProviderVersion.kt new file mode 100644 index 0000000000..a573dff842 --- /dev/null +++ b/provider/src/main/kotlin/au/com/dius/pact/provider/ProviderVersion.kt @@ -0,0 +1,37 @@ +package au.com.dius.pact.provider + +import org.slf4j.Logger +import org.slf4j.LoggerFactory +import java.util.function.Supplier + +/** + * Should always wrap a provider version string with this Supplier in order to avoid repeating any logic. + */ +class ProviderVersion(val source: () -> String?) : Supplier { + + companion object { + const val FALLBACK_VALUE = "0.0.0" + const val SNAPSHOT_DEFINITION_STRING = "-SNAPSHOT" + val snapshotRegex = Regex(".*($SNAPSHOT_DEFINITION_STRING)") + val logger: Logger = LoggerFactory.getLogger(ProviderVersion::class.java) + } + + override fun get(): String { + val version = source() ?: FALLBACK_VALUE + + if (version == FALLBACK_VALUE) { + logger.warn("Provider version not set, defaulting to '$FALLBACK_VALUE'") + } + + val trimSnapshotProperty = System.getProperty(ProviderVerifier.PACT_PROVIDER_VERSION_TRIM_SNAPSHOT) + val isTrimSnapshot = if (trimSnapshotProperty.isNullOrBlank()) false else trimSnapshotProperty.toBoolean() + return if (isTrimSnapshot) trimSnapshot(version) else version + } + + private fun trimSnapshot(providerVersion: String): String { + if (providerVersion.contains(SNAPSHOT_DEFINITION_STRING)) { + return providerVersion.removeRange(snapshotRegex.find(providerVersion)!!.groups[1]!!.range) + } + return providerVersion + } +} diff --git a/provider/src/test/groovy/au/com/dius/pact/provider/ProviderUtilsSpec.groovy b/provider/src/test/groovy/au/com/dius/pact/provider/ProviderUtilsSpec.groovy index 814bba8467..a3e6c0855d 100644 --- a/provider/src/test/groovy/au/com/dius/pact/provider/ProviderUtilsSpec.groovy +++ b/provider/src/test/groovy/au/com/dius/pact/provider/ProviderUtilsSpec.groovy @@ -2,7 +2,6 @@ package au.com.dius.pact.provider import spock.lang.IgnoreIf import spock.lang.Specification -import spock.util.environment.RestoreSystemProperties @SuppressWarnings('UnnecessaryBooleanExpression') class ProviderUtilsSpec extends Specification { @@ -75,54 +74,4 @@ class ProviderUtilsSpec extends Specification { new ProviderInfo(packagesToScan: ['d.e.f']) | new ConsumerInfo() || ['d.e.f'] } - @RestoreSystemProperties - def 'provider versions with trim snapshot property true' () { - - expect: - ProviderUtils.getProviderVersion(projectVersion) == result - - where: - systemProperty | projectVersion || result - System.setProperty(ProviderVerifier.PACT_PROVIDER_VERSION_TRIM_SNAPSHOT, 'true') | - '1.0.0-SNAPSHOT-re234hj' | - '1.0.0-re234hj' - } - - @RestoreSystemProperties - def 'provider versions with trim snapshot property false' () { - - expect: - ProviderUtils.getProviderVersion(projectVersion) == result - - where: - systemProperty | projectVersion || result - System.setProperty(ProviderVerifier.PACT_PROVIDER_VERSION_TRIM_SNAPSHOT, 'false') | - '1.0.0-SNAPSHOT-re234hj' | - '1.0.0-SNAPSHOT-re234hj' - } - - @RestoreSystemProperties - def 'provider versions with trim snapshot property wrong value' () { - - expect: - ProviderUtils.getProviderVersion(projectVersion) == result - - where: - systemProperty | projectVersion || result - System.setProperty(ProviderVerifier.PACT_PROVIDER_VERSION_TRIM_SNAPSHOT, 'aweirdstring') | - '1.0.0-SNAPSHOT-re234hj' | - '1.0.0-SNAPSHOT-re234hj' - } - - @RestoreSystemProperties - def 'provider versions with trim snapshot property not set' () { - - expect: - ProviderUtils.getProviderVersion(projectVersion) == result - - where: - systemProperty | projectVersion || result - _ | '1.0.0-SNAPSHOT-re234hj' | '1.0.0-SNAPSHOT-re234hj' - } - } diff --git a/provider/src/test/groovy/au/com/dius/pact/provider/ProviderVersionSpec.groovy b/provider/src/test/groovy/au/com/dius/pact/provider/ProviderVersionSpec.groovy new file mode 100644 index 0000000000..feef582758 --- /dev/null +++ b/provider/src/test/groovy/au/com/dius/pact/provider/ProviderVersionSpec.groovy @@ -0,0 +1,33 @@ +package au.com.dius.pact.provider + +import org.spockframework.lang.Wildcard +import spock.lang.Specification + +class ProviderVersionSpec extends Specification { + + def cleanup() { + System.clearProperty(ProviderVerifier.PACT_PROVIDER_VERSION_TRIM_SNAPSHOT) + } + + def 'provider version respects the property pact.provider.version.trimSnapshot'() { + + given: + if (propertyValue != Wildcard.INSTANCE) { + System.setProperty(ProviderVerifier.PACT_PROVIDER_VERSION_TRIM_SNAPSHOT, propertyValue as String) + } + + expect: + new ProviderVersion({ projectVersion }).get() == result + + where: + propertyValue | projectVersion | result + 'true' | '1.0.0-NOT-A-SNAPSHOT-abc-SNAPSHOT' | '1.0.0-NOT-A-SNAPSHOT-abc' + 'true' | '1.0.0-NOT-A-SNAPSHOT-abc-SNAPSHOT-re234hj' | '1.0.0-NOT-A-SNAPSHOT-abc-re234hj' + 'true' | '1.0.0-SNAPSHOT-re234hj' | '1.0.0-re234hj' + 'false' | '1.0.0-SNAPSHOT-re234hj' | '1.0.0-SNAPSHOT-re234hj' + 'aweirdstring' | '1.0.0-SNAPSHOT-re234hj' | '1.0.0-SNAPSHOT-re234hj' + 'true' | null | '0.0.0' + 'false' | null | '0.0.0' + _ | '1.0.0-SNAPSHOT-re234hj' | '1.0.0-SNAPSHOT-re234hj' + } +} From 29cb4cf718ee7456ed157ed884b3e72a05478444 Mon Sep 17 00:00:00 2001 From: anto Date: Thu, 2 Jul 2020 22:58:44 +0100 Subject: [PATCH 2/3] fix: use ProviderVersion in TestResultAccumulator --- .../au/com/dius/pact/provider/TestResultAccumulator.kt | 2 +- .../pact/provider/TestResultAccumulatorSpec.groovy | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/provider/src/main/kotlin/au/com/dius/pact/provider/TestResultAccumulator.kt b/provider/src/main/kotlin/au/com/dius/pact/provider/TestResultAccumulator.kt index f7686bd1b9..dffe6ffecb 100644 --- a/provider/src/main/kotlin/au/com/dius/pact/provider/TestResultAccumulator.kt +++ b/provider/src/main/kotlin/au/com/dius/pact/provider/TestResultAccumulator.kt @@ -96,7 +96,7 @@ object DefaultTestResultAccumulator : TestResultAccumulator, KLogging() { } fun lookupProviderVersion(): String { - val version = System.getProperty("pact.provider.version") + val version = ProviderVersion { System.getProperty("pact.provider.version") }.get() return if (version.isNullOrEmpty()) { logger.warn { "Set the provider version using the 'pact.provider.version' property. Defaulting to '0.0.0'" } "0.0.0" diff --git a/provider/src/test/groovy/au/com/dius/pact/provider/TestResultAccumulatorSpec.groovy b/provider/src/test/groovy/au/com/dius/pact/provider/TestResultAccumulatorSpec.groovy index 095c435bc9..7f710cc635 100644 --- a/provider/src/test/groovy/au/com/dius/pact/provider/TestResultAccumulatorSpec.groovy +++ b/provider/src/test/groovy/au/com/dius/pact/provider/TestResultAccumulatorSpec.groovy @@ -41,6 +41,16 @@ class TestResultAccumulatorSpec extends Specification { testResultAccumulator.lookupProviderVersion() == '0.0.0' } + @RestoreSystemProperties + def 'lookupProviderVersion - trims snapshot if system property is set'() { + given: + System.setProperty('pact.provider.version', '1.2.3-SNAPSHOT') + System.setProperty('pact.provider.version.trimSnapshot', 'true') + + expect: + testResultAccumulator.lookupProviderVersion() == '1.2.3' + } + @Unroll @SuppressWarnings('LineLength') def 'allInteractionsVerified returns #result when #condition'() { From 0385801b2b2b8e905d872e4f8ad2065053257a37 Mon Sep 17 00:00:00 2001 From: anto Date: Thu, 2 Jul 2020 22:58:55 +0100 Subject: [PATCH 3/3] fix: linting --- .../kotlin/au/com/dius/pact/provider/ProviderVerifier.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/provider/src/main/kotlin/au/com/dius/pact/provider/ProviderVerifier.kt b/provider/src/main/kotlin/au/com/dius/pact/provider/ProviderVerifier.kt index 3583f7482e..598079a096 100644 --- a/provider/src/main/kotlin/au/com/dius/pact/provider/ProviderVerifier.kt +++ b/provider/src/main/kotlin/au/com/dius/pact/provider/ProviderVerifier.kt @@ -666,7 +666,11 @@ open class ProviderVerifier @JvmOverloads constructor ( publishingResultsDisabled() -> reporters.forEach { it.warnPublishResultsSkippedBecauseDisabled(PACT_VERIFIER_PUBLISH_RESULTS) } - else -> verificationReporter.reportResults(pact, result.toTestResult(), providerVersion.get(), client, providerTag?.get()) + else -> verificationReporter.reportResults(pact, + result.toTestResult(), + providerVersion.get(), + client, + providerTag?.get()) } result }