Skip to content

Commit

Permalink
Invoke issuing API requests on background thread (#3507)
Browse files Browse the repository at this point in the history 
Previously requests were being fired on the main thread
(`runBlocking`). Move requests to a background thread and dispatch
results on main thread.

Fixes #3499
  • Loading branch information
@mshafrir-stripe
mshafrir-stripe authored Mar 23, 2021
1 parent d0df834 commit 8e469a9
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 112 deletions.
64 changes: 39 additions & 25 deletions stripe/src/main/java/com/stripe/android/IssuingCardPinService.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,20 @@ import com.stripe.android.Stripe.Companion.appInfo
import com.stripe.android.exception.InvalidRequestException
import com.stripe.android.networking.StripeApiRepository
import com.stripe.android.networking.StripeRepository
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import kotlin.coroutines.CoroutineContext

/**
* Methods for retrieval / update of a Stripe Issuing card
*/
class IssuingCardPinService @VisibleForTesting internal constructor(
keyProvider: EphemeralKeyProvider,
private val stripeRepository: StripeRepository,
private val operationIdFactory: OperationIdFactory = StripeOperationIdFactory()
private val operationIdFactory: OperationIdFactory = StripeOperationIdFactory(),
private val workContext: CoroutineContext = Dispatchers.IO
) {
private val retrievalListeners = mutableMapOf<String, IssuingCardPinRetrievalListener>()
private val updateListeners = mutableMapOf<String, IssuingCardPinUpdateListener>()
Expand Down Expand Up @@ -133,8 +138,8 @@ class IssuingCardPinService @VisibleForTesting internal constructor(
operation: EphemeralOperation.Issuing.RetrievePin,
listener: IssuingCardPinRetrievalListener
) {
runCatching {
runBlocking {
CoroutineScope(workContext).launch {
runCatching {
requireNotNull(
stripeRepository.retrieveIssuingCardPin(
operation.cardId,
Expand All @@ -145,19 +150,23 @@ class IssuingCardPinService @VisibleForTesting internal constructor(
) {
"Could not retrieve issuing card PIN."
}
}
}.fold(
onSuccess = listener::onIssuingCardPinRetrieved,
onFailure = {
onRetrievePinError(it, listener)
}
)
}.fold(
onSuccess = { pin ->
withContext(Dispatchers.Main) {
listener.onIssuingCardPinRetrieved(pin)
}
},
onFailure = {
onRetrievePinError(it, listener)
}
)
}
}

private fun onRetrievePinError(
private suspend fun onRetrievePinError(
throwable: Throwable,
listener: IssuingCardPinRetrievalListener
) {
) = withContext(Dispatchers.Main) {
when (throwable) {
is InvalidRequestException -> {
when (throwable.stripeError?.code) {
Expand Down Expand Up @@ -213,27 +222,32 @@ class IssuingCardPinService @VisibleForTesting internal constructor(
operation: EphemeralOperation.Issuing.UpdatePin,
listener: IssuingCardPinUpdateListener
) {
runCatching {
runBlocking {
CoroutineScope(workContext).launch {
runCatching {
stripeRepository.updateIssuingCardPin(
operation.cardId,
operation.newPin,
operation.verificationId,
operation.userOneTimeCode,
ephemeralKey.secret
)
}
}.fold(
onSuccess = {
listener.onIssuingCardPinUpdated()
},
onFailure = {
onUpdatePinError(it, listener)
}
)
}.fold(
onSuccess = {
withContext(Dispatchers.Main) {
listener.onIssuingCardPinUpdated()
}
},
onFailure = {
onUpdatePinError(it, listener)
}
)
}
}

private fun onUpdatePinError(throwable: Throwable, listener: IssuingCardPinUpdateListener) {
private suspend fun onUpdatePinError(
throwable: Throwable,
listener: IssuingCardPinUpdateListener
) = withContext(Dispatchers.Main) {
when (throwable) {
is InvalidRequestException -> {
when (throwable.stripeError?.code) {
Expand Down
154 changes: 67 additions & 87 deletions stripe/src/test/java/com/stripe/android/IssuingCardPinServiceTest.kt
View file
Original file line number Diff line number Diff line change
@@ -1,68 +1,58 @@
package com.stripe.android

import androidx.test.core.app.ApplicationProvider
import com.nhaarman.mockitokotlin2.argThat
import com.google.common.truth.Truth.assertThat
import com.nhaarman.mockitokotlin2.mock
import com.nhaarman.mockitokotlin2.verify
import com.nhaarman.mockitokotlin2.whenever
import com.stripe.android.networking.ApiRequest
import com.stripe.android.networking.ApiRequestExecutor
import com.stripe.android.networking.ApiRequestMatcher
import com.stripe.android.networking.StripeApiRepository
import com.stripe.android.networking.StripeRequest
import com.stripe.android.networking.StripeResponse
import com.stripe.android.exception.InvalidRequestException
import com.stripe.android.networking.AbsFakeStripeRepository
import com.stripe.android.testharness.TestEphemeralKeyProvider
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.TestCoroutineDispatcher
import kotlinx.coroutines.test.resetMain
import kotlinx.coroutines.test.setMain
import org.json.JSONObject
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
import kotlin.test.AfterTest
import kotlin.test.BeforeTest
import kotlin.test.Test

/**
* Test class for [IssuingCardPinService].
*/
@ExperimentalCoroutinesApi
@RunWith(RobolectricTestRunner::class)
class IssuingCardPinServiceTest {
private val stripeApiRequestExecutor: ApiRequestExecutor = mock()
private val retrievalListener: IssuingCardPinService.IssuingCardPinRetrievalListener = mock()
private val updateListener: IssuingCardPinService.IssuingCardPinUpdateListener = mock()

private val stripeRepository = StripeApiRepository(
ApplicationProvider.getApplicationContext(),
ApiKeyFixtures.FAKE_PUBLISHABLE_KEY,
stripeApiRequestExecutor = stripeApiRequestExecutor,
analyticsRequestExecutor = {}
)
private val testDispatcher = TestCoroutineDispatcher()

private val stripeRepository = FakeStripeRepository()
private val service = IssuingCardPinService(
TestEphemeralKeyProvider().also {
it.setNextRawEphemeralKey(EPHEMERAL_KEY.toString())
},
stripeRepository,
OperationIdFactory.get()
OperationIdFactory.get(),
testDispatcher
)

@Test
fun testRetrieval() {
val response = StripeResponse(
200,
"""
{
"card": "ic_abcdef",
"pin": "1234"
}
""".trimIndent()
)
@BeforeTest
fun setup() {
Dispatchers.setMain(testDispatcher)
}

whenever(
stripeApiRequestExecutor.execute(
argThat(
ApiRequestMatcher(
StripeRequest.Method.GET,
"https://api.stripe.com/v1/issuing/cards/ic_abcdef/pin?verification%5Bone_time_code%5D=123-456&verification%5Bid%5D=iv_abcd",
ApiRequest.Options("ek_test_123")
)
)
)
).thenReturn(response)
@AfterTest
fun cleanup() {
Dispatchers.resetMain()
testDispatcher.cleanupTestCoroutines()
}

@Test
fun `retrievePin() should call onIssuingCardPinRetrieved() on listener when successful`() {
stripeRepository.retrievedPin = { PIN }

service.retrievePin(
"ic_abcdef",
Expand All @@ -72,71 +62,37 @@ class IssuingCardPinServiceTest {
)

verify(retrievalListener)
.onIssuingCardPinRetrieved("1234")
.onIssuingCardPinRetrieved(PIN)
}

@Test
fun testUpdate() {
val response = StripeResponse(
200,
"""
{
"card": "ic_abcdef",
"pin": ""
}
""".trimIndent()
)

whenever(
stripeApiRequestExecutor.execute(
argThat(
ApiRequestMatcher(
StripeRequest.Method.POST,
"https://api.stripe.com/v1/issuing/cards/ic_abcdef/pin",
ApiRequest.Options("ek_test_123")
)
)
)
).thenReturn(response)

fun `updatePin() should call onIssuingCardPinUpdated() on listener when successful`() {
service.updatePin(
"ic_abcdef",
"1234",
"5678",
"iv_abcd",
"123-456",
updateListener
)

verify(updateListener)
.onIssuingCardPinUpdated()

assertThat(stripeRepository.updatePinCalls)
.isEqualTo(1)
}

@Test
fun testRetrievalFailsWithReason() {
val response = StripeResponse(
400,
"""
{
"error": {
"code": "incorrect_code",
"message": "Verification failed",
"type": "invalid_request_error"
}
}
""".trimIndent()
)

whenever(
stripeApiRequestExecutor.execute(
argThat(
ApiRequestMatcher(
StripeRequest.Method.GET,
"https://api.stripe.com/v1/issuing/cards/ic_abcdef/pin?verification%5Bone_time_code%5D=123-456&verification%5Bid%5D=iv_abcd",
ApiRequest.Options("ek_test_123")
)
fun `retrievePin() should call onError() on listener when there is an error`() {
stripeRepository.retrievedPin = {
throw InvalidRequestException(
stripeError = StripeError(
code = "incorrect_code",
message = "Verification failed",
type = "invalid_request_error"
)
)
).thenReturn(response)
}

service.retrievePin(
"ic_abcdef",
Expand All @@ -152,7 +108,31 @@ class IssuingCardPinServiceTest {
)
}

private class FakeStripeRepository : AbsFakeStripeRepository() {
var retrievedPin: () -> String? = { null }
var updatePinCalls = 0

override suspend fun retrieveIssuingCardPin(
cardId: String,
verificationId: String,
userOneTimeCode: String,
ephemeralKeySecret: String
): String? = retrievedPin()

override suspend fun updateIssuingCardPin(
cardId: String,
newPin: String,
verificationId: String,
userOneTimeCode: String,
ephemeralKeySecret: String
) {
updatePinCalls++
}
}

private companion object {
private const val PIN = "1234"

private val EPHEMERAL_KEY = JSONObject(
"""
{
Expand Down

0 comments on commit 8e469a9

Please sign in to comment.