Skip to content

Commit

Permalink
feat: add retry when there are unknown results for canIDeploy #1241
Browse files Browse the repository at this point in the history
  • Loading branch information
Ronald Holshausen committed Nov 14, 2020
1 parent 6720091 commit 8ec6d2a
Show file tree
Hide file tree
Showing 24 changed files with 344 additions and 183 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ fun loadPactFromUrl(
): Pair<JsonValue.Object, PactSource> {
return when (source) {
is BrokerUrlSource -> {
val brokerClient = PactBrokerClient(source.pactBrokerUrl, options)
val brokerClient = PactBrokerClient(source.pactBrokerUrl, options.toMutableMap())
val pactResponse = brokerClient.fetchPact(source.url, source.encodePath)
pactResponse.pactFile to source.copy(attributes = pactResponse.links, options = options, tag = source.tag)
}
Expand Down
2 changes: 2 additions & 0 deletions core/pactbroker/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ dependencies {
api 'com.michael-bull.kotlin-result:kotlin-result:1.1.6'

testRuntime "org.junit.vintage:junit-vintage-engine:${project.junit5Version}"
testRuntime "org.junit.jupiter:junit-jupiter-engine:${project.junit5Version}"
testImplementation "org.junit.jupiter:junit-jupiter-api:${project.junit5Version}"
testCompile "ch.qos.logback:logback-classic:${project.logbackVersion}"
testCompile "org.codehaus.groovy:groovy:${project.groovyVersion}"
testCompile "org.codehaus.groovy:groovy-json:${project.groovyVersion}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ sealed class Latest {
data class UseLatestTag(val latestTag: String) : Latest()
}

data class CanIDeployResult(val ok: Boolean, val message: String, val reason: String)
data class CanIDeployResult(val ok: Boolean, val message: String, val reason: String, val unknown: Int? = null)

/**
* Consumer version selector. See https://docs.pact.io/pact_broker/advanced_topics/selectors
Expand Down Expand Up @@ -101,12 +101,22 @@ interface IPactBrokerClient {
val options: Map<String, Any>
}

data class PactBrokerClientConfig(
val retryCountWhileUnknown: Int = 0,
val retryWhileUnknownInterval: Int = 10
)

/**
* Client for the pact broker service
*/
open class PactBrokerClient(val pactBrokerUrl: String, override val options: Map<String, Any>) : IPactBrokerClient {
open class PactBrokerClient(
val pactBrokerUrl: String,
@Deprecated("Move use of options to PactBrokerClientConfig")
override val options: MutableMap<String, Any>,
val config: PactBrokerClientConfig = PactBrokerClientConfig()
) : IPactBrokerClient {

constructor(pactBrokerUrl: String) : this(pactBrokerUrl, mapOf())
constructor(pactBrokerUrl: String) : this(pactBrokerUrl, mutableMapOf())

/**
* Fetches all consumers for the given provider
Expand Down Expand Up @@ -412,16 +422,23 @@ open class PactBrokerClient(val pactBrokerUrl: String, override val options: Map

open fun canIDeploy(pacticipant: String, pacticipantVersion: String, latest: Latest, to: String?): CanIDeployResult {
val halClient = newHalClient()
val result = halClient.getJson("/matrix" + buildMatrixQuery(pacticipant, pacticipantVersion, latest, to),
false)
return when (result) {
is Ok<JsonValue> -> {
val summary = result.value["summary"].asObject()
CanIDeployResult(Json.toBoolean(summary["deployable"]), "", Json.toString(summary["reason"]))
}
is Err<Exception> -> {
logger.error(result.error) { "Pact broker matrix query failed: ${result.error.message}" }
CanIDeployResult(false, result.error.message.toString(), "")
val path = "/matrix" + buildMatrixQuery(pacticipant, pacticipantVersion, latest, to)
return retryWith(
"canIDeploy: Retrying request as there are unknown results",
config.retryCountWhileUnknown,
config.retryWhileUnknownInterval,
{ result -> result.ok && result.unknown != null && result.unknown > 0 }
) {
when (val result = halClient.getJson(path, false)) {
is Ok<JsonValue> -> {
val summary = result.value["summary"].asObject()
CanIDeployResult(Json.toBoolean(summary["deployable"]), "", Json.toString(summary["reason"]),
Json.toInteger(summary["unknown"]))
}
is Err<Exception> -> {
logger.error(result.error) { "Pact broker matrix query failed: ${result.error.message}" }
CanIDeployResult(false, result.error.message.toString(), "")
}
}
}
}
Expand Down Expand Up @@ -469,5 +486,23 @@ open class PactBrokerClient(val pactBrokerUrl: String, override val options: Map
}
}
}

fun <T> retryWith(
message: String,
count: Int,
interval: Int,
predicate: (T) -> Boolean,
function: () -> T
): T {
var counter = 1
var result = function()
while (counter < count && predicate(result)) {
counter += 1
logger.info { "$message [$counter/$count]" }
Thread.sleep((interval * 1000).toLong())
result = function()
}
return result
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ class PactBrokerClientSpec extends Specification {
halClient.forAll(_, _) >> { args -> args[1].accept([name: 'bob', href: 'http://bob.com/']) }

PactBrokerClient client = Spy(PactBrokerClient, constructorArgs: [
'http://pactBrokerUrl', MapsKt.mapOf(new Pair('authentication', ['Basic', '1', '2']))]) {
'http://pactBrokerUrl', MapsKt.mapOf(new Pair('authentication', ['Basic', '1', '2'])),
new PactBrokerClientConfig()]) {
newHalClient() >> halClient
}

Expand Down Expand Up @@ -154,7 +155,8 @@ class PactBrokerClientSpec extends Specification {
halClient.forAll(_, _) >> { args -> args[1].accept([name: 'bob', href: 'http://bob.com/']) }

PactBrokerClient client = Spy(PactBrokerClient, constructorArgs: [
'http://pactBrokerUrl', MapsKt.mapOf(new Pair('authentication', ['Basic', '1', '2']))]) {
'http://pactBrokerUrl', MapsKt.mapOf(new Pair('authentication', ['Basic', '1', '2'])),
new PactBrokerClientConfig()]) {
newHalClient() >> halClient
}

Expand Down Expand Up @@ -528,4 +530,37 @@ class PactBrokerClientSpec extends Specification {
1 * client.fetchConsumers('provider') >> []
result instanceof Ok
}

@Issue('#1241')
def 'can i deploy - should retry when there are unknown results'() {
given:
def halClient = Mock(IHalClient)
def config = new PactBrokerClientConfig(10, 0)
PactBrokerClient client = Spy(PactBrokerClient, constructorArgs: ['baseUrl', [:], config]) {
newHalClient() >> halClient
}
def json1 = JsonParser.parseString('''
|{
| "summary": {
| "deployable": true,
| "reason": "some text",
| "unknown": 1
| }
|}'''.stripMargin())
def json2 = JsonParser.parseString('''
|{
| "summary": {
| "deployable": true,
| "reason": "some text",
| "unknown": 0
| }
|}'''.stripMargin())

when:
def result = client.canIDeploy('test', '1.2.3', new Latest.UseLatest(true), '')

then:
3 * halClient.getJson(_, _) >> new Ok(json1) >> new Ok(json1) >> new Ok(json2)
result.ok
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package au.com.dius.pact.core.pactbroker

import org.hamcrest.MatcherAssert.assertThat
import org.hamcrest.Matchers.equalTo
import org.junit.jupiter.api.Test

class PactBrokerClientTest {
@Test
fun retryWithWhenCountIsZeroRunsOnce() {
var counter = 0
val result = PactBrokerClient.retryWith("Test", 0, 0, { false }) {
counter += 1
counter
}
assertThat(result, equalTo(1))
}

@Test
fun retryWithWhenCountIsOneRunsOnce() {
var counter = 0
val result = PactBrokerClient.retryWith("Test", 1, 0, { false }) {
counter += 1
counter
}
assertThat(result, equalTo(1))
}

@Test
fun retryWithWhenCountIsGreaterThanOneButPredicateIsFalseRunsOnce() {
var counter = 0
val result = PactBrokerClient.retryWith("Test", 10, 0, { false }) {
counter += 1
counter
}
assertThat(result, equalTo(1))
}

@Test
fun retryWithWhenCountIsGreaterThanOneAndPredicateIsTrueRunsTheNumberOfTimeByTheCount() {
var counter = 0
val result = PactBrokerClient.retryWith("Test", 10, 0, { true }) {
counter += 1
counter
}
assertThat(result, equalTo(10))
}

@Test
fun retryWithWhenCountIsGreaterThanOneRunsUntilThePredicateIsFalse() {
var counter = 0
val result = PactBrokerClient.retryWith("Test", 10, 0, { v -> v < 5 }) {
counter += 1
counter
}
assertThat(result, equalTo(5))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ object Json {
else -> false
}

fun toInteger(value: JsonValue?) = when {
value == null -> null
value.isNumber -> value.asNumber().toInt()
else -> null
}

fun escape(s: String): String = ESCAPE_JSON.translate(s)

private val ESCAPE_JSON = AggregateTranslator(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import com.typesafe.scalalogging.StrictLogging
import scala.collection.JavaConverters._
import java.io.{File, IOException}

import au.com.dius.pact.core.pactbroker.{PactBrokerClient, RequestFailedException}
import au.com.dius.pact.core.pactbroker.{PactBrokerClient, PactBrokerClientConfig, RequestFailedException}

object Publish extends StrictLogging {

Expand Down Expand Up @@ -49,10 +49,10 @@ object Publish extends StrictLogging {

try {
val options = getOptions(authToken)
val brokerClient: PactBrokerClient = new PactBrokerClient(broker, options.asJava)
val brokerClient: PactBrokerClient = new PactBrokerClient(broker, options.asJava, new PactBrokerClientConfig())
val res = brokerClient.uploadPactFile(pact, consumerVersion, tags.getOrElse(List()).asJava)
if( res.component2() == null) {
logger.debug("Pact succesfully shared. deleting file..")
logger.debug("Pact successfully shared. deleting file..")
removePact(pact)
new Response(200, ResponseUtils.CrossSiteHeaders.asJava, OptionalBody.body(res.component1().getBytes()))
} else {
Expand All @@ -66,16 +66,14 @@ object Publish extends StrictLogging {
}
}

private def getOptions(authToken: Option[String]): Map[String, _] = {
var options: Map[String, _]= Map()
private def getOptions(authToken: Option[String]): Map[String, Object] = {
var options: Map[String, Object]= Map()
if(authToken.isDefined) {
options = Map("authentication" -> List("bearer",authToken.get).asJava)
}
options
}



private def removePact(file: File): Unit = {
if (file.exists()) {
file.delete()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import au.com.dius.pact.consumer.groovy.PactBuilder
import au.com.dius.pact.core.pactbroker.ConsumerVersionSelector
import au.com.dius.pact.core.pactbroker.Latest
import au.com.dius.pact.core.pactbroker.PactBrokerClient
import au.com.dius.pact.core.pactbroker.PactBrokerClientConfig
import au.com.dius.pact.core.pactbroker.TestResult
import com.github.michaelbull.result.Err
import com.github.michaelbull.result.Ok
Expand All @@ -19,7 +20,8 @@ class PactBrokerClientPactSpec extends Specification {
private PactBuilder pactBroker, imaginaryBroker

def setup() {
pactBrokerClient = new PactBrokerClient('http://localhost:8080', [halClient: [maxPublishRetries: 0]])
pactBrokerClient = new PactBrokerClient('http://localhost:8080', [halClient: [maxPublishRetries: 0]],
new PactBrokerClientConfig())
pactFile = File.createTempFile('pact', '.json')
pactContents = '''
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,15 @@ package au.com.dius.pact.provider.gradle

import au.com.dius.pact.core.pactbroker.Latest
import au.com.dius.pact.core.pactbroker.PactBrokerClient
import org.apache.commons.lang3.StringUtils
import org.gradle.api.DefaultTask
import com.github.ajalt.mordant.TermColors
import org.gradle.api.GradleScriptException
import org.gradle.api.tasks.TaskAction
import com.github.ajalt.mordant.TermColors

/**
* Task to push pact files to a pact broker
*/
@SuppressWarnings(['Println', 'DuplicateStringLiteral'])
class PactCanIDeployTask extends DefaultTask {
class PactCanIDeployTask extends PactCanIDeployBaseTask {

private static final String PACTICIPANT = 'pacticipant'
private static final String PACTICIPANT_VERSION = 'pacticipantVersion'
Expand Down Expand Up @@ -74,16 +72,4 @@ class PactCanIDeployTask extends DefaultTask {
}
latest
}

private static PactBrokerClient setupBrokerClient(Broker config) {
def options = [:]
if (StringUtils.isNotEmpty(config.pactBrokerToken)) {
options.authentication = [config.pactBrokerAuthenticationScheme ?: 'bearer',
config.pactBrokerToken]
} else if (StringUtils.isNotEmpty(config.pactBrokerUsername)) {
options.authentication = [config.pactBrokerAuthenticationScheme ?: 'basic',
config.pactBrokerUsername, config.pactBrokerPassword]
}
new PactBrokerClient(config.pactBrokerUrl, options)
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package au.com.dius.pact.provider.gradle

import au.com.dius.pact.core.pactbroker.PactBrokerClient
import au.com.dius.pact.core.pactbroker.PactBrokerClientConfig
import au.com.dius.pact.core.pactbroker.RequestFailedException
import com.github.michaelbull.result.Ok
import groovy.io.FileType
Expand Down Expand Up @@ -44,7 +45,7 @@ class PactPublishTask extends DefaultTask {
options.authentication = [brokerConfig.pactBrokerAuthenticationScheme ?: 'basic',
brokerConfig.pactBrokerUsername, brokerConfig.pactBrokerPassword]
}
def brokerClient = new PactBrokerClient(brokerConfig.pactBrokerUrl, options)
def brokerClient = new PactBrokerClient(brokerConfig.pactBrokerUrl, options, new PactBrokerClientConfig())
File pactDirectory = pactPublish.pactDirectory as File
boolean anyFailed = false
pactDirectory.eachFileMatch(FileType.FILES, ~/.*\.json/) { pactFile ->
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,22 @@
package au.com.dius.pact.provider.gradle

import groovy.transform.ToString

/**
* Config for pact broker
*/
@ToString
class Broker {
String pactBrokerUrl
String pactBrokerToken
String pactBrokerUsername
String pactBrokerPassword
String pactBrokerAuthenticationScheme
data class Broker(
var pactBrokerUrl: String? = null,
var pactBrokerToken: String? = null,
var pactBrokerUsername: String? = null,
var pactBrokerPassword: String? = null,
var pactBrokerAuthenticationScheme: String? = null,
var retryCountWhileUnknown: Int? = null,
var retryWhileUnknownInterval: Int? = null
) {
override fun toString(): String {
val password = if (pactBrokerPassword != null) "".padEnd(pactBrokerPassword!!.length, '*') else null
return "Broker(pactBrokerUrl=$pactBrokerUrl, pactBrokerToken=$pactBrokerToken, " +
"pactBrokerUsername=$pactBrokerUsername, pactBrokerPassword=$password, " +
"pactBrokerAuthenticationScheme=$pactBrokerAuthenticationScheme, " +
"retryCountWhileUnknown=$retryCountWhileUnknown, retryWhileUnknownInterval=$retryWhileUnknownInterval)"
}
}
Loading

0 comments on commit 8ec6d2a

Please sign in to comment.