From f0670e2f3936f0223c02e9ec0d0de52f31a3539f Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Tue, 11 Aug 2020 09:41:40 -0300 Subject: [pos] Improve coroutine-based merchant library access --- merchant-lib/build.gradle | 1 + .../main/java/net/taler/merchantlib/MerchantApi.kt | 76 +++++++++++++--------- .../main/java/net/taler/merchantlib/Response.kt | 4 +- .../java/net/taler/merchantlib/MerchantApiTest.kt | 20 +++--- .../net/taler/merchantpos/config/ConfigManager.kt | 6 +- .../taler/merchantpos/history/HistoryManager.kt | 19 +++--- .../taler/merchantpos/payment/PaymentManager.kt | 72 ++++++++++---------- .../net/taler/merchantpos/refund/RefundManager.kt | 24 ++++--- taler-kotlin-android/build.gradle | 7 +- 9 files changed, 122 insertions(+), 107 deletions(-) diff --git a/merchant-lib/build.gradle b/merchant-lib/build.gradle index 33e8379..5082253 100644 --- a/merchant-lib/build.gradle +++ b/merchant-lib/build.gradle @@ -56,4 +56,5 @@ dependencies { testImplementation 'junit:junit:4.13' testImplementation "io.ktor:ktor-client-mock-jvm:$ktor_version" testImplementation "io.ktor:ktor-client-logging-jvm:$ktor_version" + testImplementation 'org.jetbrains.kotlinx:kotlinx-coroutines-test:1.3.8' } diff --git a/merchant-lib/src/main/java/net/taler/merchantlib/MerchantApi.kt b/merchant-lib/src/main/java/net/taler/merchantlib/MerchantApi.kt index c92d4d2..a4ca397 100644 --- a/merchant-lib/src/main/java/net/taler/merchantlib/MerchantApi.kt +++ b/merchant-lib/src/main/java/net/taler/merchantlib/MerchantApi.kt @@ -27,63 +27,81 @@ import io.ktor.client.request.post import io.ktor.http.ContentType.Application.Json import io.ktor.http.HttpHeaders.Authorization import io.ktor.http.contentType +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext import kotlinx.serialization.json.Json import kotlinx.serialization.json.JsonConfiguration import net.taler.merchantlib.Response.Companion.response -class MerchantApi(private val httpClient: HttpClient) { +class MerchantApi( + private val httpClient: HttpClient = getDefaultHttpClient(), + private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO +) { - suspend fun getConfig(baseUrl: String): Response = response { - httpClient.get("$baseUrl/config") as ConfigResponse + suspend fun getConfig(baseUrl: String): Response = withContext(ioDispatcher) { + response { + httpClient.get("$baseUrl/config") as ConfigResponse + } } suspend fun postOrder( merchantConfig: MerchantConfig, orderRequest: PostOrderRequest - ): Response = response { - httpClient.post(merchantConfig.urlFor("private/orders")) { - header(Authorization, "ApiKey ${merchantConfig.apiKey}") - contentType(Json) - body = orderRequest - } as PostOrderResponse + ): Response = withContext(ioDispatcher) { + response { + httpClient.post(merchantConfig.urlFor("private/orders")) { + header(Authorization, "ApiKey ${merchantConfig.apiKey}") + contentType(Json) + body = orderRequest + } as PostOrderResponse + } } suspend fun checkOrder( merchantConfig: MerchantConfig, orderId: String - ): Response = response { - httpClient.get(merchantConfig.urlFor("private/orders/$orderId")) { - header(Authorization, "ApiKey ${merchantConfig.apiKey}") - } as CheckPaymentResponse + ): Response = withContext(ioDispatcher) { + response { + httpClient.get(merchantConfig.urlFor("private/orders/$orderId")) { + header(Authorization, "ApiKey ${merchantConfig.apiKey}") + } as CheckPaymentResponse + } } suspend fun deleteOrder( merchantConfig: MerchantConfig, orderId: String - ): Response = response { - httpClient.delete(merchantConfig.urlFor("private/orders/$orderId")) { - header(Authorization, "ApiKey ${merchantConfig.apiKey}") - } as Unit + ): Response = withContext(ioDispatcher) { + response { + httpClient.delete(merchantConfig.urlFor("private/orders/$orderId")) { + header(Authorization, "ApiKey ${merchantConfig.apiKey}") + } as Unit + } } - suspend fun getOrderHistory(merchantConfig: MerchantConfig): Response = response { - httpClient.get(merchantConfig.urlFor("private/orders")) { - header(Authorization, "ApiKey ${merchantConfig.apiKey}") - } as OrderHistory - } + suspend fun getOrderHistory(merchantConfig: MerchantConfig): Response = + withContext(ioDispatcher) { + response { + httpClient.get(merchantConfig.urlFor("private/orders")) { + header(Authorization, "ApiKey ${merchantConfig.apiKey}") + } as OrderHistory + } + } suspend fun giveRefund( merchantConfig: MerchantConfig, orderId: String, request: RefundRequest - ): Response = response { - httpClient.post(merchantConfig.urlFor("private/orders/$orderId/refund")) { - header(Authorization, "ApiKey ${merchantConfig.apiKey}") - contentType(Json) - body = request - } as RefundResponse + ): Response = withContext(ioDispatcher) { + response { + httpClient.post(merchantConfig.urlFor("private/orders/$orderId/refund")) { + header(Authorization, "ApiKey ${merchantConfig.apiKey}") + contentType(Json) + body = request + } as RefundResponse + } } - } fun getDefaultHttpClient(): HttpClient = HttpClient(OkHttp) { diff --git a/merchant-lib/src/main/java/net/taler/merchantlib/Response.kt b/merchant-lib/src/main/java/net/taler/merchantlib/Response.kt index 65a12a9..fb48b46 100644 --- a/merchant-lib/src/main/java/net/taler/merchantlib/Response.kt +++ b/merchant-lib/src/main/java/net/taler/merchantlib/Response.kt @@ -25,7 +25,6 @@ import kotlinx.serialization.Serializable class Response private constructor( private val value: Any? ) { - companion object { suspend fun response(request: suspend () -> T): Response { return try { @@ -45,7 +44,7 @@ class Response private constructor( val isFailure: Boolean get() = value is Failure - suspend fun handle(onFailure: ((String) -> Any)? = null, onSuccess: ((T) -> Any)? = null) { + suspend fun handle(onFailure: ((String) -> Unit)? = null, onSuccess: ((T) -> Unit)? = null) { if (value is Failure) onFailure?.let { it(getFailureString(value)) } else onSuccess?.let { @Suppress("UNCHECKED_CAST") @@ -86,5 +85,4 @@ class Response private constructor( val code: Int?, val hint: String? ) - } diff --git a/merchant-lib/src/test/java/net/taler/merchantlib/MerchantApiTest.kt b/merchant-lib/src/test/java/net/taler/merchantlib/MerchantApiTest.kt index f9f5e87..992af6f 100644 --- a/merchant-lib/src/test/java/net/taler/merchantlib/MerchantApiTest.kt +++ b/merchant-lib/src/test/java/net/taler/merchantlib/MerchantApiTest.kt @@ -17,7 +17,9 @@ package net.taler.merchantlib import io.ktor.http.HttpStatusCode.Companion.NotFound -import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.TestCoroutineDispatcher +import kotlinx.coroutines.test.runBlockingTest import net.taler.common.Amount import net.taler.common.ContractProduct import net.taler.common.ContractTerms @@ -28,9 +30,10 @@ import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Test +@ExperimentalCoroutinesApi class MerchantApiTest { - private val api = MerchantApi(httpClient) + private val api = MerchantApi(httpClient, TestCoroutineDispatcher()) private val merchantConfig = MerchantConfig( baseUrl = "http://example.net/", instance = "testInstance", @@ -39,7 +42,7 @@ class MerchantApiTest { private val orderId = "orderIdFoo" @Test - fun testGetConfig() = runBlocking { + fun testGetConfig() = runBlockingTest { httpClient.giveJsonResponse("https://backend.int.taler.net/config") { """ { @@ -54,7 +57,7 @@ class MerchantApiTest { } @Test - fun testPostOrder() = runBlocking { + fun testPostOrder() = runBlockingTest { val product = ContractProduct( productId = "foo", description = "bar", @@ -111,7 +114,7 @@ class MerchantApiTest { } @Test - fun testCheckOrder() = runBlocking { + fun testCheckOrder() = runBlockingTest { val unpaidResponse = CheckPaymentResponse.Unpaid(false, "http://taler.net/foo") httpClient.giveJsonResponse("http://example.net/instances/testInstance/private/orders/$orderId") { """{ @@ -140,7 +143,7 @@ class MerchantApiTest { } @Test - fun testDeleteOrder() = runBlocking { + fun testDeleteOrder() = runBlockingTest { httpClient.giveJsonResponse("http://example.net/instances/testInstance/private/orders/$orderId") { "{}" } @@ -163,7 +166,7 @@ class MerchantApiTest { } @Test - fun testGetOrderHistory() = runBlocking { + fun testGetOrderHistory() = runBlockingTest { httpClient.giveJsonResponse("http://example.net/instances/testInstance/private/orders") { """{ "orders": [ { @@ -213,7 +216,7 @@ class MerchantApiTest { } @Test - fun testGiveRefund() = runBlocking { + fun testGiveRefund() = runBlockingTest { httpClient.giveJsonResponse("http://example.net/instances/testInstance/private/orders/$orderId/refund") { """{ "taler_refund_uri": "taler://refund/foo/bar" @@ -227,5 +230,4 @@ class MerchantApiTest { assertEquals("taler://refund/foo/bar", it.talerRefundUri) } } - } diff --git a/merchant-terminal/src/main/java/net/taler/merchantpos/config/ConfigManager.kt b/merchant-terminal/src/main/java/net/taler/merchantpos/config/ConfigManager.kt index c0b01a2..67e3685 100644 --- a/merchant-terminal/src/main/java/net/taler/merchantpos/config/ConfigManager.kt +++ b/merchant-terminal/src/main/java/net/taler/merchantpos/config/ConfigManager.kt @@ -30,6 +30,7 @@ import io.ktor.client.features.ClientRequestException import io.ktor.client.request.get import io.ktor.client.request.header import io.ktor.http.HttpHeaders.Authorization +import io.ktor.http.HttpStatusCode.Companion.Unauthorized import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch @@ -114,7 +115,7 @@ class ConfigManager( Log.e(TAG, "Error retrieving merchant config", e) val msg = if (e is ClientRequestException) { context.getString( - if (e.response.status.value == 401) R.string.config_auth_error + if (e.response.status == Unauthorized) R.string.config_auth_error else R.string.config_error_network ) } else { @@ -145,7 +146,7 @@ class ConfigManager( Log.e(TAG, "Error handling configuration by ${receiver::class.java.simpleName}", e) context.getString(R.string.config_error_unknown) } - if (result != null) { // error + if (result != null) { // error mConfigUpdateResult.postValue(ConfigUpdateResult.Error(result)) return } @@ -178,7 +179,6 @@ class ConfigManager( private fun onNetworkError(msg: String) = scope.launch(Dispatchers.Main) { mConfigUpdateResult.value = ConfigUpdateResult.Error(msg) } - } sealed class ConfigUpdateResult { diff --git a/merchant-terminal/src/main/java/net/taler/merchantpos/history/HistoryManager.kt b/merchant-terminal/src/main/java/net/taler/merchantpos/history/HistoryManager.kt index aabe4cc..d880eaa 100644 --- a/merchant-terminal/src/main/java/net/taler/merchantpos/history/HistoryManager.kt +++ b/merchant-terminal/src/main/java/net/taler/merchantpos/history/HistoryManager.kt @@ -20,8 +20,8 @@ import androidx.annotation.UiThread import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch +import net.taler.common.assertUiThread import net.taler.merchantlib.MerchantApi import net.taler.merchantlib.OrderHistoryEntry import net.taler.merchantpos.config.ConfigManager @@ -44,20 +44,19 @@ class HistoryManager( val items: LiveData = mItems @UiThread - internal fun fetchHistory() { + internal fun fetchHistory() = scope.launch { mIsLoading.value = true val merchantConfig = configManager.merchantConfig!! - scope.launch(Dispatchers.IO) { - api.getOrderHistory(merchantConfig).handle(::onHistoryError) { - mIsLoading.postValue(false) - mItems.postValue(HistoryResult.Success(it.orders)) - } + api.getOrderHistory(merchantConfig).handle(::onHistoryError) { + assertUiThread() + mIsLoading.value = false + mItems.value = HistoryResult.Success(it.orders) } } private fun onHistoryError(msg: String) { - mIsLoading.postValue(false) - mItems.postValue(HistoryResult.Error(msg)) + assertUiThread() + mIsLoading.value = false + mItems.value = HistoryResult.Error(msg) } - } diff --git a/merchant-terminal/src/main/java/net/taler/merchantpos/payment/PaymentManager.kt b/merchant-terminal/src/main/java/net/taler/merchantpos/payment/PaymentManager.kt index 6bab0e6..b39355a 100644 --- a/merchant-terminal/src/main/java/net/taler/merchantpos/payment/PaymentManager.kt +++ b/merchant-terminal/src/main/java/net/taler/merchantpos/payment/PaymentManager.kt @@ -23,13 +23,14 @@ import androidx.annotation.UiThread import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import net.taler.common.Duration +import net.taler.common.assertUiThread import net.taler.merchantlib.CheckPaymentResponse import net.taler.merchantlib.MerchantApi import net.taler.merchantlib.PostOrderRequest -import net.taler.merchantlib.PostOrderResponse import net.taler.merchantpos.MainActivity.Companion.TAG import net.taler.merchantpos.R import net.taler.merchantpos.config.ConfigManager @@ -50,12 +51,16 @@ class PaymentManager( private val mPayment = MutableLiveData() val payment: LiveData = mPayment + private var checkJob: Job? = null - private val checkTimer = object : CountDownTimer(TIMEOUT, CHECK_INTERVAL) { + private val checkTimer: CountDownTimer = object : CountDownTimer(TIMEOUT, CHECK_INTERVAL) { override fun onTick(millisUntilFinished: Long) { val orderId = payment.value?.orderId if (orderId == null) cancel() - else checkPayment(orderId) + // only start new job if old one doesn't exist or is complete + else if (checkJob == null || checkJob?.isCompleted == true) { + checkJob = checkPayment(orderId) + } } override fun onFinish() { @@ -64,44 +69,39 @@ class PaymentManager( } @UiThread - fun createPayment(order: Order) { + fun createPayment(order: Order) = scope.launch { val merchantConfig = configManager.merchantConfig!! mPayment.value = Payment(order, order.summary, configManager.currency!!) - scope.launch(Dispatchers.IO) { - val request = PostOrderRequest( - contractTerms = order.toContractTerms(), - refundDelay = Duration(HOURS.toMillis(1)) - ) - val response = api.postOrder(merchantConfig, request) - response.handle(::onNetworkError, ::onOrderCreated) + val request = PostOrderRequest( + contractTerms = order.toContractTerms(), + refundDelay = Duration(HOURS.toMillis(1)) + ) + api.postOrder(merchantConfig, request).handle(::onNetworkError) { orderResponse -> + assertUiThread() + mPayment.value = mPayment.value!!.copy(orderId = orderResponse.orderId) + checkTimer.start() } } - private fun onOrderCreated(orderResponse: PostOrderResponse) = scope.launch(Dispatchers.Main) { - mPayment.value = mPayment.value!!.copy(orderId = orderResponse.orderId) - checkTimer.start() - } - - private fun checkPayment(orderId: String) { + private fun checkPayment(orderId: String) = scope.launch { val merchantConfig = configManager.merchantConfig!! - scope.launch(Dispatchers.IO) { - val response = api.checkOrder(merchantConfig, orderId) - response.handle(::onNetworkError, ::onPaymentChecked) - } - } - - private fun onPaymentChecked(response: CheckPaymentResponse) = scope.launch(Dispatchers.Main) { - val currentValue = requireNotNull(mPayment.value) - if (response.paid) { - mPayment.value = currentValue.copy(paid = true) - checkTimer.cancel() - } else if (currentValue.talerPayUri == null) { - response as CheckPaymentResponse.Unpaid - mPayment.value = currentValue.copy(talerPayUri = response.talerPayUri) + api.checkOrder(merchantConfig, orderId).handle(::onNetworkError) { response -> + assertUiThread() + if (!isActive) return@handle // don't continue if job was cancelled + val currentValue = requireNotNull(mPayment.value) + if (response.paid) { + mPayment.value = currentValue.copy(paid = true) + checkTimer.cancel() + } else if (currentValue.talerPayUri == null) { + response as CheckPaymentResponse.Unpaid + mPayment.value = currentValue.copy(talerPayUri = response.talerPayUri) + } } } - private fun onNetworkError(error: String) = scope.launch(Dispatchers.Main) { + private fun onNetworkError(error: String) { + assertUiThread() + Log.d(TAG, "Network error: $error") cancelPayment(error) } @@ -112,14 +112,14 @@ class PaymentManager( mPayment.value?.let { payment -> if (!payment.paid && payment.error != null) payment.orderId?.let { orderId -> Log.d(TAG, "Deleting cancelled and unpaid order $orderId") - scope.launch(Dispatchers.IO) { + scope.launch { api.deleteOrder(merchantConfig, orderId) } } } - mPayment.value = mPayment.value!!.copy(error = error) checkTimer.cancel() + checkJob?.isCancelled + checkJob = null } - } diff --git a/merchant-terminal/src/main/java/net/taler/merchantpos/refund/RefundManager.kt b/merchant-terminal/src/main/java/net/taler/merchantpos/refund/RefundManager.kt index ea2d398..25c7c5e 100644 --- a/merchant-terminal/src/main/java/net/taler/merchantpos/refund/RefundManager.kt +++ b/merchant-terminal/src/main/java/net/taler/merchantpos/refund/RefundManager.kt @@ -20,9 +20,9 @@ import androidx.annotation.UiThread import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import net.taler.common.Amount +import net.taler.common.assertUiThread import net.taler.merchantlib.MerchantApi import net.taler.merchantlib.OrderHistoryEntry import net.taler.merchantlib.RefundRequest @@ -65,27 +65,25 @@ class RefundManager( } @UiThread - internal fun refund(item: OrderHistoryEntry, amount: Amount, reason: String) { + internal fun refund(item: OrderHistoryEntry, amount: Amount, reason: String) = scope.launch { val merchantConfig = configManager.merchantConfig!! val request = RefundRequest(amount, reason) - scope.launch(Dispatchers.IO) { - api.giveRefund(merchantConfig, item.orderId, request).handle(::onRefundError) { - val result = RefundResult.Success( - refundUri = it.talerRefundUri, - item = item, - amount = amount, - reason = reason - ) - mRefundResult.postValue(result) - } + api.giveRefund(merchantConfig, item.orderId, request).handle(::onRefundError) { + assertUiThread() + mRefundResult.value = RefundResult.Success( + refundUri = it.talerRefundUri, + item = item, + amount = amount, + reason = reason + ) } } @UiThread private fun onRefundError(msg: String) { + assertUiThread() if (msg.contains("2602")) { mRefundResult.postValue(RefundResult.AlreadyRefunded) } else mRefundResult.postValue(RefundResult.Error(msg)) } - } diff --git a/taler-kotlin-android/build.gradle b/taler-kotlin-android/build.gradle index d6d6003..20590e0 100644 --- a/taler-kotlin-android/build.gradle +++ b/taler-kotlin-android/build.gradle @@ -53,15 +53,14 @@ dependencies { api project(":taler-kotlin-common") implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk8:$kotlin_version" - implementation 'androidx.appcompat:appcompat:1.1.0' - implementation 'androidx.core:core-ktx:1.3.0' + implementation 'androidx.appcompat:appcompat:1.2.0' + implementation 'androidx.core:core-ktx:1.3.1' // Navigation implementation "androidx.navigation:navigation-ui-ktx:$nav_version" implementation "androidx.navigation:navigation-fragment-ktx:$nav_version" // ViewModel and LiveData - def lifecycle_version = "2.2.0" implementation "androidx.lifecycle:lifecycle-livedata-ktx:$lifecycle_version" // QR codes @@ -71,7 +70,7 @@ dependencies { api "org.jetbrains.kotlinx:kotlinx-serialization-runtime:0.20.0" implementation "com.fasterxml.jackson.module:jackson-module-kotlin:2.10.2" - lintChecks 'com.github.thirdegg:lint-rules:0.0.4-alpha' + lintPublish 'com.github.thirdegg:lint-rules:0.0.4-alpha' testImplementation 'junit:junit:4.13' testImplementation 'org.json:json:20190722' -- cgit v1.2.3