From 4374c32a75ae46522cb78965b1989ac2f08edc8a Mon Sep 17 00:00:00 2001 From: Shams Asari Date: Thu, 30 Nov 2017 17:39:51 +0000 Subject: [PATCH] HttpUtils methods now throw an IOException when a request isn't successful, rather than returning a Boolean. This has been the cause of previous bugs as people forget to check for the false case. --- .../corda/bank/api/BankOfCordaClientApi.kt | 2 -- .../kotlin/net/corda/irs/IRSDemoTest.kt | 4 +-- .../corda/irs/web/demo/IrsDemoClientApi.kt | 8 +++--- .../net/corda/vega/SimmValuationTest.kt | 6 ++--- .../kotlin/net/corda/testing/http/HttpApi.kt | 14 +++++----- .../net/corda/testing/http/HttpUtils.kt | 26 +++++-------------- 6 files changed, 23 insertions(+), 37 deletions(-) diff --git a/samples/bank-of-corda-demo/src/main/kotlin/net/corda/bank/api/BankOfCordaClientApi.kt b/samples/bank-of-corda-demo/src/main/kotlin/net/corda/bank/api/BankOfCordaClientApi.kt index ac0a136f7c..ef020df6ef 100644 --- a/samples/bank-of-corda-demo/src/main/kotlin/net/corda/bank/api/BankOfCordaClientApi.kt +++ b/samples/bank-of-corda-demo/src/main/kotlin/net/corda/bank/api/BankOfCordaClientApi.kt @@ -2,7 +2,6 @@ package net.corda.bank.api import net.corda.bank.api.BankOfCordaWebApi.IssueRequestParams import net.corda.client.rpc.CordaRPCClient -import net.corda.core.contracts.Amount import net.corda.core.messaging.startFlow import net.corda.core.transactions.SignedTransaction import net.corda.core.utilities.NetworkHostAndPort @@ -10,7 +9,6 @@ import net.corda.core.utilities.OpaqueBytes import net.corda.core.utilities.getOrThrow import net.corda.finance.flows.CashIssueAndPaymentFlow import net.corda.testing.http.HttpApi -import java.util.* /** * Interface for communicating with Bank of Corda node diff --git a/samples/irs-demo/src/integration-test/kotlin/net/corda/irs/IRSDemoTest.kt b/samples/irs-demo/src/integration-test/kotlin/net/corda/irs/IRSDemoTest.kt index bf850cc841..6b53396fa4 100644 --- a/samples/irs-demo/src/integration-test/kotlin/net/corda/irs/IRSDemoTest.kt +++ b/samples/irs-demo/src/integration-test/kotlin/net/corda/irs/IRSDemoTest.kt @@ -110,7 +110,7 @@ class IRSDemoTest { private fun runDateChange(nodeApi: HttpApi) { log.info("Running date change against ${nodeApi.root}") - assertThat(nodeApi.putJson("demodate", "\"$futureDate\"")).isTrue() + nodeApi.putJson("demodate", "\"$futureDate\"") } private fun runTrade(nodeApi: HttpApi, oracle: Party) { @@ -123,7 +123,7 @@ class IRSDemoTest { private fun runUploadRates(nodeApi: HttpApi) { log.info("Running upload rates against ${nodeApi.root}") val fileContents = loadResourceFile("net/corda/irs/simulation/example.rates.txt") - assertThat(nodeApi.postPlain("fixes", fileContents)).isTrue() + nodeApi.postPlain("fixes", fileContents) } private fun loadResourceFile(filename: String): String { diff --git a/samples/irs-demo/web/src/test/kotlin/net/corda/irs/web/demo/IrsDemoClientApi.kt b/samples/irs-demo/web/src/test/kotlin/net/corda/irs/web/demo/IrsDemoClientApi.kt index 82e81de047..9bebc4481c 100644 --- a/samples/irs-demo/web/src/test/kotlin/net/corda/irs/web/demo/IrsDemoClientApi.kt +++ b/samples/irs-demo/web/src/test/kotlin/net/corda/irs/web/demo/IrsDemoClientApi.kt @@ -8,7 +8,7 @@ import org.apache.commons.io.IOUtils /** * Interface for communicating with nodes running the IRS demo. */ -class IRSDemoClientApi(private val hostAndPort: NetworkHostAndPort) { +class IRSDemoClientApi(hostAndPort: NetworkHostAndPort) { private val api = HttpApi.fromHostAndPort(hostAndPort, apiRoot) fun runTrade(tradeId: String, oracleName: CordaX500Name) { @@ -17,14 +17,14 @@ class IRSDemoClientApi(private val hostAndPort: NetworkHostAndPort) { api.postJson("deals", tradeFile) } - fun runDateChange(newDate: String): Boolean { - return api.putJson("demodate", "\"$newDate\"") + fun runDateChange(newDate: String) { + api.putJson("demodate", "\"$newDate\"") } // TODO: Add uploading of files to the HTTP API fun runUploadRates() { val fileContents = IOUtils.toString(Thread.currentThread().contextClassLoader.getResourceAsStream("net/corda/irs/simulation/example.rates.txt"), Charsets.UTF_8.name()) - check(api.postPlain("fixes", fileContents)) + api.postPlain("fixes", fileContents) println("Rates successfully uploaded!") } diff --git a/samples/simm-valuation-demo/src/integration-test/kotlin/net/corda/vega/SimmValuationTest.kt b/samples/simm-valuation-demo/src/integration-test/kotlin/net/corda/vega/SimmValuationTest.kt index 2b74779a1c..ee92826c94 100644 --- a/samples/simm-valuation-demo/src/integration-test/kotlin/net/corda/vega/SimmValuationTest.kt +++ b/samples/simm-valuation-demo/src/integration-test/kotlin/net/corda/vega/SimmValuationTest.kt @@ -38,7 +38,7 @@ class SimmValuationTest { val nodeBParty = getPartyWithName(nodeAApi, nodeBLegalName) val nodeAParty = getPartyWithName(nodeBApi, nodeALegalName) - assertThat(createTradeBetween(nodeAApi, nodeBParty, testTradeId)).isTrue() + createTradeBetween(nodeAApi, nodeBParty, testTradeId) assertTradeExists(nodeBApi, nodeAParty, testTradeId) assertTradeExists(nodeAApi, nodeBParty, testTradeId) runValuationsBetween(nodeAApi, nodeBParty) @@ -55,10 +55,10 @@ class SimmValuationTest { return partyApi.getJson("whoami") } - private fun createTradeBetween(partyApi: HttpApi, counterparty: PortfolioApi.ApiParty, tradeId: String): Boolean { + private fun createTradeBetween(partyApi: HttpApi, counterparty: PortfolioApi.ApiParty, tradeId: String) { val trade = SwapDataModel(tradeId, "desc", valuationDate, "EUR_FIXED_1Y_EURIBOR_3M", valuationDate, LocalDate.parse("2020-01-02"), BuySell.BUY, BigDecimal.valueOf(1000), BigDecimal.valueOf(0.1)) - return partyApi.putJson("${counterparty.id}/trades", trade) + partyApi.putJson("${counterparty.id}/trades", trade) } private fun assertTradeExists(partyApi: HttpApi, counterparty: PortfolioApi.ApiParty, tradeId: String) { diff --git a/testing/test-utils/src/main/kotlin/net/corda/testing/http/HttpApi.kt b/testing/test-utils/src/main/kotlin/net/corda/testing/http/HttpApi.kt index 7e948285dc..b0ffd991e8 100644 --- a/testing/test-utils/src/main/kotlin/net/corda/testing/http/HttpApi.kt +++ b/testing/test-utils/src/main/kotlin/net/corda/testing/http/HttpApi.kt @@ -1,6 +1,7 @@ package net.corda.testing.http import com.fasterxml.jackson.databind.ObjectMapper +import net.corda.client.jackson.JacksonSupport import net.corda.core.utilities.NetworkHostAndPort import java.net.URL @@ -29,16 +30,17 @@ class HttpApi(val root: URL, val mapper: ObjectMapper = defaultMapper) { /** * Send a GET request to the path on the API specified. */ - inline fun getJson(path: String, params: Map = mapOf()) = HttpUtils.getJson(URL(root, path), params, mapper) + inline fun getJson(path: String, params: Map = mapOf()): T { + return HttpUtils.getJson(URL(root, path), params, mapper) + } private fun toJson(any: Any) = any as? String ?: HttpUtils.defaultMapper.writeValueAsString(any) companion object { - fun fromHostAndPort(hostAndPort: NetworkHostAndPort, base: String, protocol: String = "http", mapper: ObjectMapper = defaultMapper): HttpApi - = HttpApi(URL("$protocol://$hostAndPort/$base/"), mapper) - - private val defaultMapper: ObjectMapper by lazy { - net.corda.client.jackson.JacksonSupport.createNonRpcMapper() + fun fromHostAndPort(hostAndPort: NetworkHostAndPort, base: String, protocol: String = "http", mapper: ObjectMapper = defaultMapper): HttpApi { + return HttpApi(URL("$protocol://$hostAndPort/$base/"), mapper) } + + private val defaultMapper: ObjectMapper by lazy { JacksonSupport.createNonRpcMapper() } } } diff --git a/testing/test-utils/src/main/kotlin/net/corda/testing/http/HttpUtils.kt b/testing/test-utils/src/main/kotlin/net/corda/testing/http/HttpUtils.kt index cf1af9b9ab..5e87f4e36b 100644 --- a/testing/test-utils/src/main/kotlin/net/corda/testing/http/HttpUtils.kt +++ b/testing/test-utils/src/main/kotlin/net/corda/testing/http/HttpUtils.kt @@ -5,7 +5,6 @@ import okhttp3.MediaType import okhttp3.OkHttpClient import okhttp3.Request import okhttp3.RequestBody -import org.slf4j.LoggerFactory import java.io.IOException import java.net.URL import java.util.concurrent.TimeUnit @@ -14,8 +13,6 @@ import java.util.concurrent.TimeUnit * A small set of utilities for making HttpCalls, aimed at demos and tests. */ object HttpUtils { - private val logger = LoggerFactory.getLogger(javaClass) - private val client by lazy { OkHttpClient.Builder() .connectTimeout(5, TimeUnit.SECONDS) @@ -26,19 +23,19 @@ object HttpUtils { net.corda.client.jackson.JacksonSupport.createNonRpcMapper() } - fun putJson(url: URL, data: String): Boolean { + fun putJson(url: URL, data: String) { val body = RequestBody.create(MediaType.parse("application/json; charset=utf-8"), data) - return makeRequest(Request.Builder().url(url).header("Content-Type", "application/json").put(body).build()) + makeRequest(Request.Builder().url(url).header("Content-Type", "application/json").put(body).build()) } fun postJson(url: URL, data: String) { val body = RequestBody.create(MediaType.parse("application/json; charset=utf-8"), data) - makeExceptionalRequest(Request.Builder().url(url).header("Content-Type", "application/json").post(body).build()) + makeRequest(Request.Builder().url(url).header("Content-Type", "application/json").post(body).build()) } - fun postPlain(url: URL, data: String): Boolean { + fun postPlain(url: URL, data: String) { val body = RequestBody.create(MediaType.parse("text/plain; charset=utf-8"), data) - return makeRequest(Request.Builder().url(url).post(body).build()) + makeRequest(Request.Builder().url(url).post(body).build()) } inline fun getJson(url: URL, params: Map = mapOf(), mapper: ObjectMapper = defaultMapper): T { @@ -47,21 +44,10 @@ object HttpUtils { return mapper.readValue(parameterisedUrl, T::class.java) } - // TODO Move everything to use this instead of makeRequest - private fun makeExceptionalRequest(request: Request) { + private fun makeRequest(request: Request) { val response = client.newCall(request).execute() if (!response.isSuccessful) { throw IOException("${request.method()} to ${request.url()} returned a ${response.code()}: ${response.body().string()}") } } - - private fun makeRequest(request: Request): Boolean { - val response = client.newCall(request).execute() - - if (!response.isSuccessful) { - logger.error("Could not fulfill HTTP request of type ${request.method()} to ${request.url()}. Status Code: ${response.code()}. Message: ${response.body().string()}") - } - - return response.isSuccessful - } }