fix: fix a problem where the provider version trim property is ignored (

#1156) * fix: fix a problem where the provider version trim property is ignored * fix: use ProviderVersion in TestResultAccumulator * fix: linting Co-authored-by: anto <[email protected]>
pact-foundation · Jul 3, 2020 · 8807d25 · 8807d25
1 parent a5765fd
commit 8807d25
Show file tree

Hide file tree

Showing 11 changed files with 104 additions and 90 deletions.
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
@@ -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<I>(
   }
 
   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
@@ -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 <pactFile> or <pactUrl>)"
       }
       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
@@ -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
@@ -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
@@ -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
@@ -134,7 +134,7 @@ interface IProviderVerifier {
   /**
    * Callback to get the provider version
    */
-  var providerVersion: Supplier<String?>
+  var providerVersion: Supplier<String>
 
   /**
    * 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<Any, Boolean> = Function { false },
   override var executeBuildSpecificTask: BiConsumer<Any, ProviderState> = BiConsumer { _, _ -> },
   override var projectClasspath: Supplier<List<URL>> = Supplier { emptyList<URL>() },
   override var reporters: List<VerifierReporter> = listOf(AnsiConsoleReporter("console", File("/tmp/"))),
   override var providerMethodInstance: Function<Method, Any> = Function { m -> m.declaringClass.newInstance() },
-  override var providerVersion: Supplier<String?> = Supplier { System.getProperty(PACT_PROVIDER_VERSION) },
+  override var providerVersion: Supplier<String> = ProviderVersion { System.getProperty(PACT_PROVIDER_VERSION) },
   override var providerTag: Supplier<String?>? = Supplier { System.getProperty(PACT_PROVIDER_TAG) }
 ) : IProviderVerifier {
 
@@ -665,8 +666,11 @@ 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
@@ -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<String> {
+
+  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/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/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
@@ -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'
+  }
+}
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
@@ -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'() {