From 50349b5211c466e82ffed2fc97e8abd43adcd377 Mon Sep 17 00:00:00 2001 From: Till Hellmund Date: Tue, 11 Jul 2023 11:44:10 -0400 Subject: [PATCH 1/9] Fix odd behavior in Stripe.possibleCardBrands --- .../cards/DefaultCardAccountRangeStore.kt | 3 ++- .../cards/RemoteCardAccountRangeSource.kt | 24 +++++++++++-------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/payments-core/src/main/java/com/stripe/android/cards/DefaultCardAccountRangeStore.kt b/payments-core/src/main/java/com/stripe/android/cards/DefaultCardAccountRangeStore.kt index c881b569cbb..f123b7a7176 100644 --- a/payments-core/src/main/java/com/stripe/android/cards/DefaultCardAccountRangeStore.kt +++ b/payments-core/src/main/java/com/stripe/android/cards/DefaultCardAccountRangeStore.kt @@ -44,7 +44,8 @@ internal class DefaultCardAccountRangeStore( internal fun createPrefKey(bin: Bin): String = "$PREF_KEY_ACCOUNT_RANGES:$bin" private companion object { - private const val PREF_FILE = "InMemoryCardAccountRangeSource.Store" + private const val VERSION = 2 + private const val PREF_FILE = "InMemoryCardAccountRangeSource.Store.$VERSION" private const val PREF_KEY_ACCOUNT_RANGES = "key_account_ranges" } } diff --git a/payments-core/src/main/java/com/stripe/android/cards/RemoteCardAccountRangeSource.kt b/payments-core/src/main/java/com/stripe/android/cards/RemoteCardAccountRangeSource.kt index c31bf75e7ad..0a67a86e819 100644 --- a/payments-core/src/main/java/com/stripe/android/cards/RemoteCardAccountRangeSource.kt +++ b/payments-core/src/main/java/com/stripe/android/cards/RemoteCardAccountRangeSource.kt @@ -28,21 +28,25 @@ internal class RemoteCardAccountRangeSource( return cardNumber.bin?.let { bin -> _loading.value = true - val accountRanges = - stripeRepository.getCardMetadata(bin, requestOptions)?.accountRanges.orEmpty() - cardAccountRangeStore.save(bin, accountRanges) + val accountRanges = stripeRepository.getCardMetadata( + bin = bin, + options = requestOptions, + )?.accountRanges.orEmpty() + + if (accountRanges.isNotEmpty()) { + cardAccountRangeStore.save(bin, accountRanges) + } _loading.value = false - when { - accountRanges.isNotEmpty() -> { - if (cardNumber.isValidLuhn) { - onCardMetadataMissingRange() - } - accountRanges + if (accountRanges.isNotEmpty()) { + val hasMatch = accountRanges.any { it.binRange.matches(cardNumber) } + if (!hasMatch && cardNumber.isValidLuhn) { + onCardMetadataMissingRange() } - else -> null } + + accountRanges.takeIf { it.isNotEmpty() } } } From 548a1365c33a8a6b2f402d061ee0b8086618a0dc Mon Sep 17 00:00:00 2001 From: Till Hellmund Date: Tue, 11 Jul 2023 12:21:33 -0400 Subject: [PATCH 2/9] Update tests --- .../cards/RemoteCardAccountRangeSourceTest.kt | 43 +++++++++++++++---- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/payments-core/src/test/java/com/stripe/android/cards/RemoteCardAccountRangeSourceTest.kt b/payments-core/src/test/java/com/stripe/android/cards/RemoteCardAccountRangeSourceTest.kt index d94a7b3678b..bcf214709f6 100644 --- a/payments-core/src/test/java/com/stripe/android/cards/RemoteCardAccountRangeSourceTest.kt +++ b/payments-core/src/test/java/com/stripe/android/cards/RemoteCardAccountRangeSourceTest.kt @@ -54,10 +54,6 @@ internal class RemoteCardAccountRangeSourceTest { country = "GB" ) ) - verify(cardAccountRangeStore).save( - BinFixtures.VISA, - AccountRangeFixtures.DEFAULT - ) } @Test @@ -79,12 +75,43 @@ internal class RemoteCardAccountRangeSourceTest { CardNumberFixtures.VISA ) ).isNull() - verify(cardAccountRangeStore).save( - BinFixtures.VISA, - emptyList() - ) } + @Test + fun `getAccountRange() stores server response if not empty`() = runTest { + val expectedRanges = VISA_METADATA.accountRanges + + val remoteCardAccountRangeSource = RemoteCardAccountRangeSource( + stripeRepository = FakeStripeRepository(VISA_METADATA), + requestOptions = REQUEST_OPTIONS, + cardAccountRangeStore = cardAccountRangeStore, + analyticsRequestExecutor = {}, + paymentAnalyticsRequestFactory = PaymentAnalyticsRequestFactory( + ApplicationProvider.getApplicationContext(), + ApiKeyFixtures.FAKE_PUBLISHABLE_KEY + ) + ) + + remoteCardAccountRangeSource.getAccountRange(CardNumberFixtures.VISA) + verify(cardAccountRangeStore).save(BinFixtures.VISA, expectedRanges) + } + + @Test + fun `getAccountRange() does not store server response if empty`() = runTest { + val remoteCardAccountRangeSource = RemoteCardAccountRangeSource( + stripeRepository = FakeStripeRepository(EMPTY_METADATA), + requestOptions = REQUEST_OPTIONS, + cardAccountRangeStore = cardAccountRangeStore, + analyticsRequestExecutor = {}, + paymentAnalyticsRequestFactory = PaymentAnalyticsRequestFactory( + ApplicationProvider.getApplicationContext(), + ApiKeyFixtures.FAKE_PUBLISHABLE_KEY + ) + ) + + verify(cardAccountRangeStore, never()).save(any(), any()) + } + @Test fun `getAccountRange() when card number is less than required BIN length should return null`() = runTest { From 947bfdacdc0240c109635f060ecba3fba933d895 Mon Sep 17 00:00:00 2001 From: Till Hellmund Date: Thu, 13 Jul 2023 15:30:56 -0400 Subject: [PATCH 3/9] Reduce queries for card account ranges Store an empty response to avoid queries, but fall back to static ranges if the stored/fetched response is empty. --- .../testing/AbsFakeStripeRepository.kt | 2 +- .../DefaultCardAccountRangeRepository.kt | 12 ++++-- .../cards/RemoteCardAccountRangeSource.kt | 39 ++++++++++++------- .../android/networking/StripeApiRepository.kt | 24 +++++------- .../android/networking/StripeRepository.kt | 2 +- .../cards/RemoteCardAccountRangeSourceTest.kt | 3 +- 6 files changed, 46 insertions(+), 36 deletions(-) diff --git a/payments-core-testing/src/main/java/com/stripe/android/testing/AbsFakeStripeRepository.kt b/payments-core-testing/src/main/java/com/stripe/android/testing/AbsFakeStripeRepository.kt index 1e701bd6a23..a3ff20242f2 100644 --- a/payments-core-testing/src/main/java/com/stripe/android/testing/AbsFakeStripeRepository.kt +++ b/payments-core-testing/src/main/java/com/stripe/android/testing/AbsFakeStripeRepository.kt @@ -238,7 +238,7 @@ abstract class AbsFakeStripeRepository : StripeRepository { return Result.failure(NotImplementedError()) } - override suspend fun getCardMetadata(bin: Bin, options: ApiRequest.Options): CardMetadata { + override suspend fun getCardMetadata(bin: Bin, options: ApiRequest.Options): Result { TODO("Not yet implemented") } diff --git a/payments-core/src/main/java/com/stripe/android/cards/DefaultCardAccountRangeRepository.kt b/payments-core/src/main/java/com/stripe/android/cards/DefaultCardAccountRangeRepository.kt index 0ac9a8c7e69..3d5af097693 100644 --- a/payments-core/src/main/java/com/stripe/android/cards/DefaultCardAccountRangeRepository.kt +++ b/payments-core/src/main/java/com/stripe/android/cards/DefaultCardAccountRangeRepository.kt @@ -14,11 +14,13 @@ internal class DefaultCardAccountRangeRepository( cardNumber: CardNumber.Unvalidated ): AccountRange? { return cardNumber.bin?.let { bin -> - if (store.contains(bin)) { + val range = if (store.contains(bin)) { inMemorySource.getAccountRange(cardNumber) } else { remoteSource.getAccountRange(cardNumber) - } ?: staticSource.getAccountRange(cardNumber) + } + + range ?: staticSource.getAccountRange(cardNumber) } } @@ -26,11 +28,13 @@ internal class DefaultCardAccountRangeRepository( cardNumber: CardNumber.Unvalidated ): List? { return cardNumber.bin?.let { bin -> - if (store.contains(bin)) { + val ranges = if (store.contains(bin)) { inMemorySource.getAccountRanges(cardNumber) } else { remoteSource.getAccountRanges(cardNumber) - } ?: staticSource.getAccountRanges(cardNumber) + } + + ranges?.takeIf { it.isNotEmpty() } ?: staticSource.getAccountRanges(cardNumber) } } diff --git a/payments-core/src/main/java/com/stripe/android/cards/RemoteCardAccountRangeSource.kt b/payments-core/src/main/java/com/stripe/android/cards/RemoteCardAccountRangeSource.kt index 0a67a86e819..1b9365498f1 100644 --- a/payments-core/src/main/java/com/stripe/android/cards/RemoteCardAccountRangeSource.kt +++ b/payments-core/src/main/java/com/stripe/android/cards/RemoteCardAccountRangeSource.kt @@ -26,27 +26,27 @@ internal class RemoteCardAccountRangeSource( cardNumber: CardNumber.Unvalidated ): List? { return cardNumber.bin?.let { bin -> - _loading.value = true - - val accountRanges = stripeRepository.getCardMetadata( - bin = bin, - options = requestOptions, - )?.accountRanges.orEmpty() - - if (accountRanges.isNotEmpty()) { - cardAccountRangeStore.save(bin, accountRanges) + val result = withLoading { + stripeRepository.getCardMetadata( + bin = bin, + options = requestOptions, + ).map { metadata -> + metadata.accountRanges + } } - _loading.value = false + result.onSuccess { accountRanges -> + cardAccountRangeStore.save(bin, accountRanges) - if (accountRanges.isNotEmpty()) { - val hasMatch = accountRanges.any { it.binRange.matches(cardNumber) } - if (!hasMatch && cardNumber.isValidLuhn) { - onCardMetadataMissingRange() + if (accountRanges.isNotEmpty()) { + val hasMatch = accountRanges.any { it.binRange.matches(cardNumber) } + if (!hasMatch && cardNumber.isValidLuhn) { + onCardMetadataMissingRange() + } } } - accountRanges.takeIf { it.isNotEmpty() } + result.getOrNull() } } @@ -55,4 +55,13 @@ internal class RemoteCardAccountRangeSource( paymentAnalyticsRequestFactory.createRequest(PaymentAnalyticsEvent.CardMetadataMissingRange) ) } + + private inline fun withLoading( + block: () -> Result>, + ): Result> { + _loading.value = true + val accountRanges = block() + _loading.value = false + return accountRanges + } } diff --git a/payments-core/src/main/java/com/stripe/android/networking/StripeApiRepository.kt b/payments-core/src/main/java/com/stripe/android/networking/StripeApiRepository.kt index d9b5b62b6a3..e018e19667b 100644 --- a/payments-core/src/main/java/com/stripe/android/networking/StripeApiRepository.kt +++ b/payments-core/src/main/java/com/stripe/android/networking/StripeApiRepository.kt @@ -860,21 +860,17 @@ class StripeApiRepository @JvmOverloads internal constructor( override suspend fun getCardMetadata( bin: Bin, options: ApiRequest.Options - ): CardMetadata? { - return runCatching { - fetchStripeModel( - apiRequestFactory.createGet( - getEdgeUrl("card-metadata"), - options.copy(stripeAccount = null), - mapOf("key" to options.apiKey, "bin_prefix" to bin.value) - ), - CardMetadataJsonParser(bin) - ) { - // no-op - } - }.onFailure { + ): Result { + return fetchStripeModelResult( + apiRequestFactory.createGet( + getEdgeUrl("card-metadata"), + options.copy(stripeAccount = null), + mapOf("key" to options.apiKey, "bin_prefix" to bin.value) + ), + CardMetadataJsonParser(bin) + ).onFailure { fireAnalyticsRequest(PaymentAnalyticsEvent.CardMetadataLoadFailure) - }.getOrNull() + } } /** diff --git a/payments-core/src/main/java/com/stripe/android/networking/StripeRepository.kt b/payments-core/src/main/java/com/stripe/android/networking/StripeRepository.kt index 80b194db9e5..9dda2bb0ef9 100644 --- a/payments-core/src/main/java/com/stripe/android/networking/StripeRepository.kt +++ b/payments-core/src/main/java/com/stripe/android/networking/StripeRepository.kt @@ -217,7 +217,7 @@ interface StripeRepository { suspend fun getCardMetadata( bin: Bin, options: ApiRequest.Options - ): CardMetadata? + ): Result @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) suspend fun start3ds2Auth( diff --git a/payments-core/src/test/java/com/stripe/android/cards/RemoteCardAccountRangeSourceTest.kt b/payments-core/src/test/java/com/stripe/android/cards/RemoteCardAccountRangeSourceTest.kt index bcf214709f6..4377fa70b4f 100644 --- a/payments-core/src/test/java/com/stripe/android/cards/RemoteCardAccountRangeSourceTest.kt +++ b/payments-core/src/test/java/com/stripe/android/cards/RemoteCardAccountRangeSourceTest.kt @@ -109,6 +109,7 @@ internal class RemoteCardAccountRangeSourceTest { ) ) + remoteCardAccountRangeSource.getAccountRange(CardNumberFixtures.VISA) verify(cardAccountRangeStore, never()).save(any(), any()) } @@ -187,7 +188,7 @@ internal class RemoteCardAccountRangeSourceTest { override suspend fun getCardMetadata( bin: Bin, options: ApiRequest.Options - ) = cardMetadata + ): Result = Result.success(cardMetadata) } private companion object { From 3c4bfb29e0c5585ecf5fc45487aaecd1dc93bbe0 Mon Sep 17 00:00:00 2001 From: Till Hellmund Date: Fri, 14 Jul 2023 13:21:46 -0400 Subject: [PATCH 4/9] Fix test --- .../com/stripe/android/networking/StripeApiRepositoryTest.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/payments-core/src/test/java/com/stripe/android/networking/StripeApiRepositoryTest.kt b/payments-core/src/test/java/com/stripe/android/networking/StripeApiRepositoryTest.kt index cb187d685bf..c3a7489b3eb 100644 --- a/payments-core/src/test/java/com/stripe/android/networking/StripeApiRepositoryTest.kt +++ b/payments-core/src/test/java/com/stripe/android/networking/StripeApiRepositoryTest.kt @@ -1127,8 +1127,7 @@ internal class StripeApiRepositoryTest { stripeApiRepository.getCardMetadata( BinFixtures.VISA, ApiRequest.Options(ApiKeyFixtures.DEFAULT_PUBLISHABLE_KEY) - ) - requireNotNull(cardMetadata) + ).getOrThrow() assertThat(cardMetadata.bin) .isEqualTo(BinFixtures.VISA) assertThat(cardMetadata.accountRanges) From 79eb92d31560576d10397e9cba2244988061f04c Mon Sep 17 00:00:00 2001 From: Till Hellmund Date: Fri, 14 Jul 2023 18:15:29 -0400 Subject: [PATCH 5/9] Update remote source test for new behavior --- .../cards/RemoteCardAccountRangeSourceTest.kt | 68 +++++++++++++------ 1 file changed, 47 insertions(+), 21 deletions(-) diff --git a/payments-core/src/test/java/com/stripe/android/cards/RemoteCardAccountRangeSourceTest.kt b/payments-core/src/test/java/com/stripe/android/cards/RemoteCardAccountRangeSourceTest.kt index 4377fa70b4f..1874b207f4a 100644 --- a/payments-core/src/test/java/com/stripe/android/cards/RemoteCardAccountRangeSourceTest.kt +++ b/payments-core/src/test/java/com/stripe/android/cards/RemoteCardAccountRangeSourceTest.kt @@ -20,6 +20,7 @@ import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify import org.robolectric.RobolectricTestRunner +import java.io.IOException import kotlin.test.Test @RunWith(RobolectricTestRunner::class) @@ -79,7 +80,7 @@ internal class RemoteCardAccountRangeSourceTest { @Test fun `getAccountRange() stores server response if not empty`() = runTest { - val expectedRanges = VISA_METADATA.accountRanges + val expectedRanges = AccountRangeFixtures.DEFAULT val remoteCardAccountRangeSource = RemoteCardAccountRangeSource( stripeRepository = FakeStripeRepository(VISA_METADATA), @@ -97,7 +98,7 @@ internal class RemoteCardAccountRangeSourceTest { } @Test - fun `getAccountRange() does not store server response if empty`() = runTest { + fun `getAccountRange() does store server response even if empty`() = runTest { val remoteCardAccountRangeSource = RemoteCardAccountRangeSource( stripeRepository = FakeStripeRepository(EMPTY_METADATA), requestOptions = REQUEST_OPTIONS, @@ -109,6 +110,23 @@ internal class RemoteCardAccountRangeSourceTest { ) ) + remoteCardAccountRangeSource.getAccountRange(CardNumberFixtures.VISA) + verify(cardAccountRangeStore).save(BinFixtures.VISA, emptyList()) + } + + @Test + fun `getAccountRange() does not store server response if the request failed`() = runTest { + val remoteCardAccountRangeSource = RemoteCardAccountRangeSource( + stripeRepository = FakeStripeRepository(FAILED_REQUEST), + requestOptions = REQUEST_OPTIONS, + cardAccountRangeStore = cardAccountRangeStore, + analyticsRequestExecutor = {}, + paymentAnalyticsRequestFactory = PaymentAnalyticsRequestFactory( + ApplicationProvider.getApplicationContext(), + ApiKeyFixtures.FAKE_PUBLISHABLE_KEY + ) + ) + remoteCardAccountRangeSource.getAccountRange(CardNumberFixtures.VISA) verify(cardAccountRangeStore, never()).save(any(), any()) } @@ -146,17 +164,19 @@ internal class RemoteCardAccountRangeSourceTest { val remoteCardAccountRangeSource = RemoteCardAccountRangeSource( FakeStripeRepository( - CardMetadata( - bin = BinFixtures.VISA, - accountRanges = listOf( - AccountRange( - binRange = BinRange( - low = "4242420000000000", - high = "4242424200000000" - ), - panLength = 16, - brandInfo = AccountRange.BrandInfo.Visa, - country = "GB" + Result.success( + CardMetadata( + bin = BinFixtures.VISA, + accountRanges = listOf( + AccountRange( + binRange = BinRange( + low = "4242420000000000", + high = "4242424200000000" + ), + panLength = 16, + brandInfo = AccountRange.BrandInfo.Visa, + country = "GB" + ) ) ) ) @@ -183,12 +203,12 @@ internal class RemoteCardAccountRangeSourceTest { } private class FakeStripeRepository( - private val cardMetadata: CardMetadata + private val result: Result ) : AbsFakeStripeRepository() { override suspend fun getCardMetadata( bin: Bin, options: ApiRequest.Options - ): Result = Result.success(cardMetadata) + ): Result = result } private companion object { @@ -196,14 +216,20 @@ internal class RemoteCardAccountRangeSourceTest { apiKey = ApiKeyFixtures.FAKE_PUBLISHABLE_KEY ) - private val EMPTY_METADATA = CardMetadata( - bin = BinFixtures.FAKE, - accountRanges = emptyList() + private val FAILED_REQUEST = Result.failure(IOException()) + + private val EMPTY_METADATA = Result.success( + CardMetadata( + bin = BinFixtures.FAKE, + accountRanges = emptyList() + ) ) - private val VISA_METADATA = CardMetadata( - bin = BinFixtures.VISA, - accountRanges = AccountRangeFixtures.DEFAULT + private val VISA_METADATA = Result.success( + CardMetadata( + bin = BinFixtures.VISA, + accountRanges = AccountRangeFixtures.DEFAULT + ) ) } } From 5b12cf2c3c096ac3109f278aaffb6f11b18d46d0 Mon Sep 17 00:00:00 2001 From: Till Hellmund Date: Tue, 18 Jul 2023 16:36:44 -0400 Subject: [PATCH 6/9] Make minor tweaks --- .../stripe/android/networking/StripeApiRepository.kt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/payments-core/src/main/java/com/stripe/android/networking/StripeApiRepository.kt b/payments-core/src/main/java/com/stripe/android/networking/StripeApiRepository.kt index e018e19667b..304c7f23f10 100644 --- a/payments-core/src/main/java/com/stripe/android/networking/StripeApiRepository.kt +++ b/payments-core/src/main/java/com/stripe/android/networking/StripeApiRepository.kt @@ -862,12 +862,12 @@ class StripeApiRepository @JvmOverloads internal constructor( options: ApiRequest.Options ): Result { return fetchStripeModelResult( - apiRequestFactory.createGet( - getEdgeUrl("card-metadata"), - options.copy(stripeAccount = null), - mapOf("key" to options.apiKey, "bin_prefix" to bin.value) + apiRequest = apiRequestFactory.createGet( + url = getEdgeUrl("card-metadata"), + options = options.copy(stripeAccount = null), + params = mapOf("key" to options.apiKey, "bin_prefix" to bin.value), ), - CardMetadataJsonParser(bin) + jsonParser = CardMetadataJsonParser(bin), ).onFailure { fireAnalyticsRequest(PaymentAnalyticsEvent.CardMetadataLoadFailure) } From 133af8b35ca7d0487291f92df7d7076b718be2db Mon Sep 17 00:00:00 2001 From: Till Hellmund Date: Tue, 18 Jul 2023 16:42:02 -0400 Subject: [PATCH 7/9] Add changelog entry --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8be99f307bf..e4d248e5899 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## XX.XX.XX - 2023-XX-XX +### Payments +*[FIXED][6977](https://github.com/stripe/stripe-android/pull/6977) Fixed an issue where `Stripe.retrievePossibleBrands()` returned incorrect results. + ## 20.27.2 - 2023-07-18 ### PaymentSheet From 892888f31dedf195bc5bf853441df43cc6c519ce Mon Sep 17 00:00:00 2001 From: Till Hellmund Date: Wed, 19 Jul 2023 14:44:48 -0400 Subject: [PATCH 8/9] Add more tests --- .../DefaultCardAccountRangeRepositoryTest.kt | 92 ++++++++++++++++--- 1 file changed, 79 insertions(+), 13 deletions(-) diff --git a/payments-core/src/test/java/com/stripe/android/cards/DefaultCardAccountRangeRepositoryTest.kt b/payments-core/src/test/java/com/stripe/android/cards/DefaultCardAccountRangeRepositoryTest.kt index 01944a8d4b1..566a8737f01 100644 --- a/payments-core/src/test/java/com/stripe/android/cards/DefaultCardAccountRangeRepositoryTest.kt +++ b/payments-core/src/test/java/com/stripe/android/cards/DefaultCardAccountRangeRepositoryTest.kt @@ -26,8 +26,6 @@ import org.junit.Ignore import org.junit.Rule import org.junit.runner.RunWith import org.mockito.kotlin.mock -import org.mockito.kotlin.never -import org.mockito.kotlin.verify import org.robolectric.RobolectricTestRunner import kotlin.test.Test @@ -198,21 +196,50 @@ internal class DefaultCardAccountRangeRepositoryTest { } @Test - fun `getAccountRange should not access remote source if BIN is in store`() = runTest { - val remoteSource = mock() + fun `Should return local account ranges if BIN is in store`() = runTest { + val accountRanges = FAKE_ACCOUNT_RANGES.shuffled() + + val repository = DefaultCardAccountRangeRepository( + inMemorySource = InMemoryCardAccountRangeSource(realStore), + remoteSource = FakeCardAccountRangeSource(), + staticSource = StaticCardAccountRangeSource(), + store = realStore, + ) + + val bin = requireNotNull(CardNumberFixtures.VISA.bin) + realStore.save(bin, accountRanges) + + val ranges = repository.getAccountRanges(CardNumberFixtures.VISA) + assertThat(ranges).containsExactlyElementsIn(accountRanges) + } + + @Test + fun `Should return static account ranges if remote source fails and BIN is not in store`() = runTest { + val repository = DefaultCardAccountRangeRepository( + inMemorySource = FakeCardAccountRangeSource(accountRanges = null), + remoteSource = mock(), + staticSource = StaticCardAccountRangeSource(), + store = realStore, + ) + + val ranges = repository.getAccountRanges(CardNumberFixtures.VISA) + assertThat(ranges).containsExactlyElementsIn(VISA_ACCOUNT_RANGES) + } + + @Test + fun `Should load from remote source if BIN is not in store`() = runTest { + val accountRanges = FAKE_ACCOUNT_RANGES.shuffled() + val remoteSource = FakeCardAccountRangeSource(accountRanges = accountRanges) + val repository = DefaultCardAccountRangeRepository( inMemorySource = FakeCardAccountRangeSource(), remoteSource = remoteSource, staticSource = FakeCardAccountRangeSource(), - store = realStore + store = realStore, ) - val bin = requireNotNull(CardNumberFixtures.VISA.bin) - realStore.save(bin, emptyList()) - - // should not access remote source - repository.getAccountRange(CardNumberFixtures.VISA) - verify(remoteSource, never()).getAccountRanges(CardNumberFixtures.VISA) + val ranges = repository.getAccountRanges(CardNumberFixtures.VISA) + assertThat(ranges).containsExactlyElementsIn(accountRanges) } @Test @@ -287,12 +314,13 @@ internal class DefaultCardAccountRangeRepositoryTest { } private class FakeCardAccountRangeSource( - isLoading: Boolean = false + isLoading: Boolean = false, + private val accountRanges: List? = null, ) : CardAccountRangeSource { override suspend fun getAccountRanges( cardNumber: CardNumber.Unvalidated ): List? { - return null + return accountRanges } override val loading: Flow = flowOf(isLoading) @@ -300,5 +328,43 @@ internal class DefaultCardAccountRangeRepositoryTest { private companion object { private val DEFAULT_OPTIONS = ApiRequest.Options("pk_test_vOo1umqsYxSrP5UXfOeL3ecm") + + private val VISA_ACCOUNT_RANGES = listOf( + AccountRange( + binRange = BinRange( + low = "4000000000000000", + high = "4999999999999999" + ), + panLength = 16, + brandInfo = AccountRange.BrandInfo.Visa, + ) + ) + + private val FAKE_ACCOUNT_RANGES = listOf( + AccountRange( + binRange = BinRange( + low = "4000000000000000", + high = "4999999999999999" + ), + panLength = 16, + brandInfo = AccountRange.BrandInfo.Visa, + ), + AccountRange( + binRange = BinRange( + low = "2221000000000000", + high = "2720999999999999" + ), + panLength = 16, + brandInfo = AccountRange.BrandInfo.Mastercard, + ), + AccountRange( + BinRange( + low = "5100000000000000", + high = "5599999999999999" + ), + panLength = 16, + brandInfo = AccountRange.BrandInfo.Mastercard, + ), + ) } } From d9722cbde4d75b8540f894a4ea62112668116476 Mon Sep 17 00:00:00 2001 From: Till Hellmund Date: Wed, 19 Jul 2023 15:05:39 -0400 Subject: [PATCH 9/9] =?UTF-8?q?Remove=20method=20that=E2=80=99s=20now=20un?= =?UTF-8?q?used?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../com/stripe/android/networking/StripeApiRepository.kt | 8 -------- 1 file changed, 8 deletions(-) diff --git a/payments-core/src/main/java/com/stripe/android/networking/StripeApiRepository.kt b/payments-core/src/main/java/com/stripe/android/networking/StripeApiRepository.kt index 304c7f23f10..909e4eedbdc 100644 --- a/payments-core/src/main/java/com/stripe/android/networking/StripeApiRepository.kt +++ b/payments-core/src/main/java/com/stripe/android/networking/StripeApiRepository.kt @@ -1434,14 +1434,6 @@ class StripeApiRepository @JvmOverloads internal constructor( } } - private suspend fun fetchStripeModel( - apiRequest: ApiRequest, - jsonParser: ModelJsonParser, - onResponse: () -> Unit - ): ModelType? { - return jsonParser.parse(makeApiRequest(apiRequest, onResponse).responseJson()) - } - private suspend fun fetchStripeModelResult( apiRequest: ApiRequest, jsonParser: ModelJsonParser,