[pos] Improve coroutine-based merchant library access

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<ConfigResponse> = response {
-        httpClient.get("$baseUrl/config") as ConfigResponse
+    suspend fun getConfig(baseUrl: String): Response<ConfigResponse> = withContext(ioDispatcher) {
+        response {
+            httpClient.get("$baseUrl/config") as ConfigResponse
+        }
     }
 
     suspend fun postOrder(
         merchantConfig: MerchantConfig,
         orderRequest: PostOrderRequest
-    ): Response<PostOrderResponse> = response {
-        httpClient.post(merchantConfig.urlFor("private/orders")) {
-            header(Authorization, "ApiKey ${merchantConfig.apiKey}")
-            contentType(Json)
-            body = orderRequest
-        } as PostOrderResponse
+    ): Response<PostOrderResponse> = 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<CheckPaymentResponse> = response {
-        httpClient.get(merchantConfig.urlFor("private/orders/$orderId")) {
-            header(Authorization, "ApiKey ${merchantConfig.apiKey}")
-        } as CheckPaymentResponse
+    ): Response<CheckPaymentResponse> = 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<Unit> = response {
-        httpClient.delete(merchantConfig.urlFor("private/orders/$orderId")) {
-            header(Authorization, "ApiKey ${merchantConfig.apiKey}")
-        } as Unit
+    ): Response<Unit> = withContext(ioDispatcher) {
+        response {
+            httpClient.delete(merchantConfig.urlFor("private/orders/$orderId")) {
+                header(Authorization, "ApiKey ${merchantConfig.apiKey}")
+            } as Unit
+        }
     }
 
-    suspend fun getOrderHistory(merchantConfig: MerchantConfig): Response<OrderHistory> = response {
-        httpClient.get(merchantConfig.urlFor("private/orders")) {
-            header(Authorization, "ApiKey ${merchantConfig.apiKey}")
-        } as OrderHistory
-    }
+    suspend fun getOrderHistory(merchantConfig: MerchantConfig): Response<OrderHistory> =
+        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<RefundResponse> = response {
-        httpClient.post(merchantConfig.urlFor("private/orders/$orderId/refund")) {
-            header(Authorization, "ApiKey ${merchantConfig.apiKey}")
-            contentType(Json)
-            body = request
-        } as RefundResponse
+    ): Response<RefundResponse> = 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<out T> private constructor(
     private val value: Any?
 ) {
-
     companion object {
         suspend fun <T> response(request: suspend () -> T): Response<T> {
             return try {
@@ -45,7 +44,7 @@ class Response<out T> 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<out T> 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<HistoryResult> = 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<Payment>()
     val payment: LiveData<Payment> = 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'