Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement GetBalance client call and server endpoint #217

Merged
merged 26 commits into from
Mar 31, 2024
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
5894279
adding balance, fixing offerings. wip
jiyoonie9 Mar 26, 2024
0dba7c2
adding optional success field to close message
jiyoonie9 Mar 26, 2024
f21cdf0
fixing test data message and resource constructions. more tests are p…
jiyoonie9 Mar 26, 2024
35aa08d
in the middle of refactoring - all tests using test vectors are still…
jiyoonie9 Mar 27, 2024
a1d2f91
adding a todo
jiyoonie9 Mar 27, 2024
09f4762
Merge branch 'main' of github.com:TBD54566975/tbdex-kt into 207-proto…
jiyoonie9 Mar 29, 2024
494156b
refactoring method name in vector test
jiyoonie9 Mar 29, 2024
6a57244
bumping submodule commit
jiyoonie9 Mar 29, 2024
6b74577
adding ktdocs
jiyoonie9 Mar 29, 2024
7a0ae63
more ktdocs
jiyoonie9 Mar 29, 2024
18dd13f
adding balance and resource tests for balance. adding balance test ve…
jiyoonie9 Mar 29, 2024
ceadf77
adding new client method to fetch balances
jiyoonie9 Mar 29, 2024
0ce5957
wrote client tests
jiyoonie9 Mar 29, 2024
da2aafe
adding server endpoint for balance api
jiyoonie9 Mar 29, 2024
15d18d3
Merge branch 'main' of github.com:TBD54566975/tbdex-kt into 133-get-b…
jiyoonie9 Mar 30, 2024
8a31cda
adding detekt fixes. still 1 test failing due to rfq schema error
jiyoonie9 Mar 30, 2024
61a1446
Merge branch 'main' of github.com:TBD54566975/tbdex-kt into 133-get-b…
jiyoonie9 Mar 31, 2024
393e9eb
adding requesterDid to balanceApi.getBalance param. doing an auth tok…
jiyoonie9 Mar 31, 2024
1fad2ed
refactoring existing tests. add new test for getbalance
jiyoonie9 Mar 31, 2024
01d6a12
adding ktdoc for fakeBalancesApi#addBalance
jiyoonie9 Mar 31, 2024
9f90c3c
Apply suggestions from code review
jiyoonie9 Mar 31, 2024
0e8d498
adding try catch around parsing balance and offering
jiyoonie9 Mar 31, 2024
c67f00a
Update httpserver/src/main/kotlin/tbdex/sdk/httpserver/TbdexHttpServe…
jiyoonie9 Mar 31, 2024
43bf4c8
adding balancesEnabled config
jiyoonie9 Mar 31, 2024
f500e35
aligning with tbdex-js way of making balancesapi optional
jiyoonie9 Mar 31, 2024
c71048d
removed testserver
jiyoonie9 Mar 31, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ object RequestToken {
.didDocument?.findAssertionMethodById(assertionMethodId)
?: throw RequestTokenCreateException("Assertion method not found")

// TODO: ensure that publicKeyJwk is not null
val publicKeyJwk = assertionMethod.publicKeyJwk
check(publicKeyJwk != null) { "publicKeyJwk is null" }
val keyAlias = did.keyManager.getDeterministicAlias(publicKeyJwk)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import tbdex.sdk.httpclient.models.GetExchangesFilter
import tbdex.sdk.httpclient.models.GetOfferingsFilter
import tbdex.sdk.httpclient.models.TbdexResponseException
import tbdex.sdk.protocol.Validator
import tbdex.sdk.protocol.models.Balance
import tbdex.sdk.protocol.models.Close
import tbdex.sdk.protocol.models.Message
import tbdex.sdk.protocol.models.Offering
Expand All @@ -29,6 +30,8 @@ import web5.sdk.dids.did.BearerDid
*/
object TbdexHttpClient {
private val client = OkHttpClient()
private const val CONTENT_TYPE = "Content-Type"
private const val AUTHORIZATION = "Authorization"
private val jsonMediaType = "application/json; charset=utf-8".toMediaType()
private const val JSON_HEADER = "application/json"

Expand All @@ -41,6 +44,7 @@ object TbdexHttpClient {
* @return A list of [Offering] matching the request.
* @throws TbdexResponseException for request or response errors.
*/
@Suppress("SwallowedException")
fun getOfferings(pfiDid: String, filter: GetOfferingsFilter? = null): List<Offering> {
val pfiServiceEndpoint = getPfiServiceEndpoint(pfiDid)
val baseUrl = "$pfiServiceEndpoint/offerings/"
Expand All @@ -64,7 +68,70 @@ object TbdexHttpClient {
val jsonNode = jsonMapper.readTree(responseString)
return jsonNode.get("data").elements()
.asSequence()
.map { Offering.parse(it.toString()) }
.map {
try {
Offering.parse(it.toString())
} catch (e: Exception) {
throw TbdexResponseException(
"response status: ${response.code}",
errors = listOf(
ErrorDetail(
detail = "Failed to parse offering ${e.message}"
)
)
)
}
}
.toList()
}

else -> throw buildResponseException(response)
}
}

/**
* Fetches balances from a PFI.
*
* @param pfiDid The decentralized identifier of the PFI.
* @param requesterDid The decentralized identifier of the entity requesting the balances.
* @return A list of [Balance] matching the request.
* @throws TbdexResponseException for request or response errors.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking out loud: in tbdex-js, we have both TbdexRequestError and TbdexResponseError, representing errors that happen before and after the client call respectively. It's a slight misnomer to throw a RequestException for both request and response errors, so I'm making a mental note that in my next tidy-up PR, I may explore introducing a TbdexRequestException class. In any case, it's relatively low priority and outside the scope of this PR.

*/
@Suppress("SwallowedException")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what exception gets swallowed?

Copy link
Contributor Author

@jiyoonie9 jiyoonie9 Mar 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the exception thrown if Balance.parse(it.toString()) fails in the try/catch, line 123. it gets re-thrown as TbdexResponseException as implemented per this suggestion

fun getBalances(pfiDid: String, requesterDid: BearerDid): List<Balance> {
val pfiServiceEndpoint = getPfiServiceEndpoint(pfiDid)
val baseUrl = "$pfiServiceEndpoint/balances/"
val requestToken = RequestToken.generate(requesterDid, pfiDid)

val request = Request.Builder()
.url(baseUrl)
.addHeader(CONTENT_TYPE, JSON_HEADER)
.addHeader(AUTHORIZATION, "Bearer $requestToken")
.get()
.build()

val response: Response = client.newCall(request).execute()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah does this throw an exception if the request fails? e.g. timeout, EHANGUP etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct. from the docs:

Throws:
IOException - if the request could not be executed due to cancellation, 
a connectivity problem or timeout. Because networks can fail during an exchange,
it is possible that the remote server accepted the request before the failure.
IllegalStateException - when the call has already been executed.

when {
response.isSuccessful -> {
val responseString = response.body?.string()
// response body is an object with a data field
val jsonNode = jsonMapper.readTree(responseString)
return jsonNode.get("data").elements()
.asSequence()
.map {
try {
Balance.parse(it.toString())
} catch (e: Exception) {
throw TbdexResponseException(
"response status: ${response.code}",
errors = listOf(
ErrorDetail(
detail = "Failed to parse balance ${e.message}"
)
)
)
}
}
.toList()
}

Expand Down Expand Up @@ -115,7 +182,7 @@ object TbdexHttpClient {

val request = Request.Builder()
.url(url)
.addHeader("Content-Type", JSON_HEADER)
.addHeader(CONTENT_TYPE, JSON_HEADER)
.post(requestBody)
.build()

Expand Down Expand Up @@ -170,7 +237,7 @@ object TbdexHttpClient {

val request = Request.Builder()
.url(url)
.addHeader("Content-Type", JSON_HEADER)
.addHeader(CONTENT_TYPE, JSON_HEADER)
.put(requestBody)
.build()

Expand All @@ -195,8 +262,8 @@ object TbdexHttpClient {

val request = Request.Builder()
.url(baseUrl)
.addHeader("Content-Type", JSON_HEADER)
.addHeader("Authorization", "Bearer $requestToken")
.addHeader(CONTENT_TYPE, JSON_HEADER)
.addHeader(AUTHORIZATION, "Bearer $requestToken")
.get()
.build()

Expand Down Expand Up @@ -237,8 +304,8 @@ object TbdexHttpClient {

val request = Request.Builder()
.url(httpUrlBuilder.build())
.addHeader("Content-Type", JSON_HEADER)
.addHeader("Authorization", "Bearer $requestToken")
.addHeader(CONTENT_TYPE, JSON_HEADER)
.addHeader(AUTHORIZATION, "Bearer $requestToken")
.get()
.build()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,42 @@ class TbdexHttpClientTest {
assertThrows<TbdexResponseException> { TbdexHttpClient.getOfferings(pfiDid.uri, null) }
}

@Test
fun `get balances success via mockwebserver`() {
val balance = TestData.getBalance()
balance.sign(TestData.PFI_DID)
val mockBalance = listOf(balance)
val mockResponseString = Json.jsonMapper.writeValueAsString(mapOf("data" to mockBalance))
server.enqueue(MockResponse().setBody(mockResponseString).setResponseCode(HttpURLConnection.HTTP_OK))

val requesterDid = DidDht.create(InMemoryKeyManager())
val response = TbdexHttpClient.getBalances(pfiDid.uri, requesterDid)

assertEquals(1, response.size)
}

@Test
fun `get balances fail via mockwebserver`() {
val errorDetails = mapOf(
"errors" to listOf(
ErrorDetail(
id = "1",
status = "401",
code = "Unauthorized",
title = "Unauthorized",
detail = "The request is unauthorized.",
source = null,
meta = null
)
)
)

val mockResponseString = Json.jsonMapper.writeValueAsString(errorDetails)
server.enqueue(MockResponse().setBody(mockResponseString).setResponseCode(HttpURLConnection.HTTP_BAD_REQUEST))
val requesterDid = DidDht.create(InMemoryKeyManager())
assertThrows<TbdexResponseException> { TbdexHttpClient.getBalances(pfiDid.uri, requesterDid) }
}

@Test
fun `createExchange(Rfq) submits RFQ`() {

Expand Down
12 changes: 12 additions & 0 deletions httpclient/src/test/kotlin/tbdex/sdk/httpclient/TestData.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package tbdex.sdk.httpclient

import com.danubetech.verifiablecredentials.CredentialSubject
import de.fxlae.typeid.TypeId
import tbdex.sdk.protocol.models.Balance
import tbdex.sdk.protocol.models.BalanceData
import tbdex.sdk.protocol.models.MessageKind
import tbdex.sdk.protocol.models.Offering
import tbdex.sdk.protocol.models.OfferingData
Expand Down Expand Up @@ -74,6 +76,16 @@ object TestData {
return offering
}

fun getBalance() =
Balance.create(
from = PFI_DID.uri,
data = BalanceData(
currencyCode = "BTC",
available = "0.001",
)
)


fun getRfq(
to: String = PFI_DID.uri,
offeringId: String = TypeId.generate(ResourceKind.offering.name).toString(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,16 @@ import io.ktor.server.routing.put
import io.ktor.server.routing.route
import io.ktor.server.routing.routing
import tbdex.sdk.httpserver.handlers.createExchange
import tbdex.sdk.httpserver.handlers.getBalances
import tbdex.sdk.httpserver.handlers.getExchange
import tbdex.sdk.httpserver.handlers.getExchanges
import tbdex.sdk.httpserver.handlers.getOfferings
import tbdex.sdk.httpserver.handlers.submitMessage
import tbdex.sdk.httpserver.models.BalancesApi
import tbdex.sdk.httpserver.models.Callbacks
import tbdex.sdk.httpserver.models.CreateExchangeCallback
import tbdex.sdk.httpserver.models.ExchangesApi
import tbdex.sdk.httpserver.models.FakeBalancesApi
import tbdex.sdk.httpserver.models.FakeExchangesApi
import tbdex.sdk.httpserver.models.FakeOfferingsApi
import tbdex.sdk.httpserver.models.GetExchangeCallback
Expand Down Expand Up @@ -59,7 +62,8 @@ class TbdexHttpServerConfig(
val port: Int,
val pfiDid: String? = null,
val offeringsApi: OfferingsApi? = null,
val exchangesApi: ExchangesApi? = null
val exchangesApi: ExchangesApi? = null,
val balancesApi: BalancesApi? = null
)


Expand All @@ -77,6 +81,7 @@ class TbdexHttpServer(private val config: TbdexHttpServerConfig) {
internal val pfiDid = config.pfiDid ?: "did:ex:pfi"
internal val offeringsApi = config.offeringsApi ?: FakeOfferingsApi()
internal val exchangesApi = config.exchangesApi ?: FakeExchangesApi()
internal val balancesApi = config.balancesApi ?: FakeBalancesApi()
jiyoonie9 marked this conversation as resolved.
Show resolved Hide resolved

/**
* Configures the Ktor application with necessary settings, including content negotiation.
Expand All @@ -102,6 +107,23 @@ class TbdexHttpServer(private val config: TbdexHttpServerConfig) {
)
}

get("/offerings") {
getOfferings(
call,
offeringsApi,
callbacks.getOfferings
)
}

get("/balances") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be an optional endpoint

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooh, i added the suggested change you posted earlier, but actually instead o that, i'd like to wrap this in a condition that checks for a new config value balancesEnabled

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think it will be good for testing to still have balancesApi = config.balancesApi ?: FakeBalancesApi()

Copy link
Contributor

@kirahsapong kirahsapong Mar 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i thought it might be helpful to share how the optional endpoint is implemented in js right now: https://github.com/TBD54566975/tbdex-js/blob/main/packages/http-server/src/http-server.ts#L45

and also this comment thread on the PR for it: TBD54566975/tbdex-js#212 (comment)

@KendallWeihe's proposal was simplifying it by keeping consistent with the other fields, and having the consumer explicitly pass in FakeBalancesApi() when instantiating the server to indicate they wanted to opt in / enable it

but either approach is cool, prob just good to make sure its aligned. i can open an issue in JS to add a balancesEnabled config value if we want to go with it, lmk!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okey, i've aligned on essentially using balancesApi null check essentially the config on whether to enable balances. i like balancesEnabled config as it's slightly more explicit? but can see how things can go awry there too. let's revisit it if there's feedback from sdk consumers to change this

getBalances(
call,
balancesApi,
callbacks.getBalances,
pfiDid
)
}

route("/exchanges") {
post {
createExchange(
Expand Down Expand Up @@ -139,13 +161,6 @@ class TbdexHttpServer(private val config: TbdexHttpServerConfig) {
}
}

get("/offerings") {
getOfferings(
call,
offeringsApi,
callbacks.getOfferings
)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package tbdex.sdk.httpserver.handlers

import io.ktor.http.HttpHeaders
import io.ktor.http.HttpStatusCode
import io.ktor.server.application.ApplicationCall
import io.ktor.server.response.respond
import tbdex.sdk.httpclient.RequestToken
import tbdex.sdk.httpclient.models.ErrorDetail
import tbdex.sdk.httpserver.models.BalancesApi
import tbdex.sdk.httpserver.models.CallbackError
import tbdex.sdk.httpserver.models.ErrorResponse
import tbdex.sdk.httpserver.models.GetBalancesCallback
import tbdex.sdk.protocol.models.Balance

/**
* Get balances response
*
* @property data list of Balances
*/
class GetBalancesResponse(
val data: List<Balance>?
)
/**
* Get balances request handler
*
* @param call Ktor server application call
* @param balancesApi Balances API interface
* @param callback Callback function to be invoked
*/
@Suppress("SwallowedException")
suspend fun getBalances(
call: ApplicationCall,
balancesApi: BalancesApi,
callback: GetBalancesCallback?,
pfiDid: String
) {

val authzHeader = call.request.headers[HttpHeaders.Authorization]
if (authzHeader == null) {
call.respond(
HttpStatusCode.Unauthorized,
ErrorResponse(
errors = listOf(
ErrorDetail(
detail = "Authorization header required"
)
)
)
)
return
}

val arr = authzHeader.split("Bearer ")
if (arr.size != 2) {
call.respond(
HttpStatusCode.Unauthorized,
ErrorResponse(
errors = listOf(
ErrorDetail(
detail = "Malformed Authorization header. Expected: Bearer TOKEN_HERE"
)
)
)
)
return
}

val token = arr[1]
val requesterDid: String
try {
requesterDid = RequestToken.verify(token, pfiDid)
} catch (e: Exception) {
call.respond(
HttpStatusCode.Unauthorized,
ErrorResponse(
errors = listOf(
ErrorDetail(
detail = "Could not verify Authorization header."
)
)
)
)
return
Comment on lines +38 to +83
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't used ktor before but if it has the concept of middleware, maybe it'd be helpful to abstract this logic out into a verifyAuthzHeader middleware? and if ktor doesn't have the middleware concept, could just tuck this into a method and use it for all protected endpoints. (note for future PR)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. there is a ton of repeat here for all protected endpoints' request handler. Issue #227

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice. had a similar issue in swift TBD54566975/tbdex-swift#85

js could use some drying up also. will create an issue

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

val balances = balancesApi.getBalances(requesterDid)

if (callback == null) {
call.respond(HttpStatusCode.OK, GetBalancesResponse(data = balances))
return
}

try {
callback.invoke(call)
} catch (e: CallbackError) {
call.respond(e.statusCode, ErrorResponse(e.details))
return
} catch (e: Exception) {
val errorDetail = ErrorDetail(detail = e.message ?: "Unknown error while getting Balances")
call.respond(HttpStatusCode.InternalServerError, ErrorResponse(listOf(errorDetail)))
return
}

call.respond(HttpStatusCode.OK, GetBalancesResponse(data = balances))
}
Loading
Loading