Merge pull request #1015 from asteffey/verbose-pact-sources

fix: Use concise Pact Source descriptions

Author: Ronald Holshausen

core/model/src/main/kotlin/au/com/dius/pact/core/model/PactSource.kt

@@ -22,15 +22,24 @@ data class DirectorySource<I> @JvmOverloads constructor(
   val dir: File,
   val pacts: MutableMap<File, Pact<I>> = mutableMapOf()
 ) : PactSource()
-  where I : Interaction
+  where I : Interaction {
+  override fun description() = "Directory $dir"
+}
 
 data class PactBrokerSource<I> @JvmOverloads constructor(
   val host: String,
   val port: String?,
   val scheme: String = "http",
   val pacts: MutableMap<Consumer, MutableList<Pact<I>>> = mutableMapOf()
 ) : PactSource()
-  where I : Interaction
+  where I : Interaction {
+  override fun description() =
+    if (port == null) {
+      "Pact Broker $scheme://$host"
+    } else {
+      "Pact Broker $scheme://$host:$port"
+    }
+}
 
 data class FileSource<I> @JvmOverloads constructor(val file: File, val pact: Pact<I>? = null) :
   PactSource() where I : Interaction {

core/model/src/test/groovy/au/com/dius/pact/core/model/DirectorySourceSpec.groovy

@@ -0,0 +1,17 @@
+package au.com.dius.pact.core.model
+
+import spock.lang.Specification
+import spock.lang.Unroll
+
+class DirectorySourceSpec extends Specification {
+
+  @Unroll
+  def 'description includes the directory Pacts are contained in'() {
+    when:
+    def path = new File('/target/pacts')
+    def source = new DirectorySource(path)
+
+    then:
+    source.description() == "Directory ${path}"
+  }
+}

core/model/src/test/groovy/au/com/dius/pact/core/model/PactBrokerSourceSpec.groovy

@@ -0,0 +1,18 @@
+package au.com.dius.pact.core.model
+
+import spock.lang.Specification
+import spock.lang.Unroll
+
+class PactBrokerSourceSpec extends Specification {
+
+  @Unroll
+  def 'description includes the URL for the Broker'() {
+    expect:
+    source.description() == description
+
+    where:
+    source                                                  | description
+    new PactBrokerSource('localhost', '80', 'http')         | 'Pact Broker http://localhost:80'
+    new PactBrokerSource('www.example.com', '443', 'https') | 'Pact Broker https://www.example.com:443'
+  }
+}

provider/pact-jvm-provider-junit/src/test/groovy/au/com/dius/pact/provider/junit/loader/PactBrokerLoaderSpec.groovy

@@ -1,6 +1,7 @@
 package au.com.dius.pact.provider.junit.loader
 
 import au.com.dius.pact.core.model.Pact
+import au.com.dius.pact.core.model.PactBrokerSource
 import au.com.dius.pact.core.model.UrlSource
 import au.com.dius.pact.core.pactbroker.IHalClient
 import au.com.dius.pact.core.pactbroker.InvalidHalResponse
@@ -138,6 +139,26 @@ class PactBrokerLoaderSpec extends Specification {
     1 * brokerClient.fetchConsumers('test') >> []
   }
 
+  @RestoreSystemProperties
+  void 'Uses fallback PactBroker System Properties for PactSource'() {
+    given:
+    host = 'my.pactbroker.host'
+    port = '4711'
+    System.setProperty('pactbroker.host', host)
+    System.setProperty('pactbroker.port', port)
+
+    when:
+    def pactSource = new PactBrokerLoader(MinimalPactBrokerAnnotation.getAnnotation(PactBroker)).pactSource
+
+    then:
+    assert pactSource instanceof PactBrokerSource
+
+    def pactBrokerSource = (PactBrokerSource) pactSource
+    assert pactBrokerSource.scheme == 'http'
+    assert pactBrokerSource.host == host
+    assert pactBrokerSource.port == port
+  }
+
   @RestoreSystemProperties
   void 'Fails when no fallback system properties are set'() {
     given:

provider/pact-jvm-provider/src/main/kotlin/au/com/dius/pact/provider/junit/loader/PactBrokerLoader.kt

@@ -1,11 +1,13 @@
 package au.com.dius.pact.provider.junit.loader
 
+import au.com.dius.pact.core.model.Consumer
 import au.com.dius.pact.core.model.DefaultPactReader
 import au.com.dius.pact.core.model.Interaction
 import au.com.dius.pact.core.model.Pact
 import au.com.dius.pact.core.model.PactBrokerSource
 import au.com.dius.pact.core.model.PactReader
 import au.com.dius.pact.core.model.UrlSource
+import au.com.dius.pact.core.model.PactSource
 import au.com.dius.pact.core.pactbroker.PactBrokerClient
 import au.com.dius.pact.core.support.expressions.ExpressionParser.parseExpression
 import au.com.dius.pact.core.support.expressions.ExpressionParser.parseListExpression
@@ -35,8 +37,7 @@ open class PactBrokerLoader(
   valueResolver: ValueResolver? = null
 ) : OverrideablePactLoader {
 
-  private var _pactSource: PactBrokerSource<Interaction> = PactBrokerSource(
-    pactBrokerHost, pactBrokerPort, pactBrokerScheme)
+  private var pacts: MutableMap<Consumer, MutableList<Pact<Interaction>>> = mutableMapOf()
   private var resolver: ValueResolver? = valueResolver
   private var overriddenPactUrl: String? = null
   private var overriddenConsumer: String? = null
@@ -100,7 +101,10 @@ open class PactBrokerLoader(
     return valueResolver
   }
 
-  override fun getPactSource() = _pactSource
+  override fun getPactSource(): PactSource? {
+    val resolver = setupValueResolver()
+    return getPactBrokerSource(resolver)
+  }
 
   override fun setValueResolver(valueResolver: ValueResolver) {
     this.resolver = valueResolver
@@ -140,6 +144,16 @@ open class PactBrokerLoader(
   }
 
   private fun brokerUrl(resolver: ValueResolver): URIBuilder {
+    val (host, port, scheme) = getPactBrokerSource(resolver)
+
+    val uriBuilder = URIBuilder().setScheme(scheme).setHost(host)
+    if (port.isNotEmpty()) {
+      uriBuilder.port = Integer.parseInt(port)
+    }
+    return uriBuilder
+  }
+
+  private fun getPactBrokerSource(resolver: ValueResolver): PactBrokerSource<Interaction> {
     val scheme = parseExpression(pactBrokerScheme, resolver)
     val host = parseExpression(pactBrokerHost, resolver)
     val port = parseExpression(pactBrokerPort, resolver)
@@ -154,11 +168,11 @@ open class PactBrokerLoader(
         "Please provide a valid port number or specify the system property 'pactbroker.port'.", pactBrokerPort))
     }
 
-    val uriBuilder = URIBuilder().setScheme(scheme).setHost(host)
-    if (port.isNotEmpty()) {
-      uriBuilder.port = Integer.parseInt(port)
+    return if (scheme == null) {
+      PactBrokerSource(host, port, pacts = pacts)
+    } else {
+      PactBrokerSource(host, port, scheme, pacts)
     }
-    return uriBuilder
   }
 
   private fun getUrlForProvider(providerName: String, tag: String, pactBrokerClient: PactBrokerClient): String {
@@ -172,7 +186,6 @@ open class PactBrokerLoader(
 
   open fun loadPact(consumer: ConsumerInfo, options: Map<String, Any>): Pact<Interaction> {
     val pact = pactReader.loadPact(consumer.pactSource!!, options) as Pact<Interaction>
-    val pacts = this.pactSource.pacts
     val pactConsumer = consumer.toPactConsumer()
     val pactList = pacts.getOrDefault(pactConsumer, mutableListOf())
     pactList.add(pact)