Fix odd behaviors in Stripe.possibleCardBrands

stripe · Jul 7, 2023 · ee863ed · ee863ed
1 parent a9cf650
commit ee863ed
Show file tree

Hide file tree

Showing 7 changed files with 38 additions and 52 deletions.
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
@@ -10,28 +10,31 @@ internal class DefaultCardAccountRangeRepository(
     private val staticSource: CardAccountRangeSource,
     private val store: CardAccountRangeStore
 ) : CardAccountRangeRepository {
+
     override suspend fun getAccountRange(
         cardNumber: CardNumber.Unvalidated
     ): AccountRange? {
-        return cardNumber.bin?.let { bin ->
-            if (store.contains(bin)) {
-                inMemorySource.getAccountRange(cardNumber)
-            } else {
-                remoteSource.getAccountRange(cardNumber)
-            } ?: staticSource.getAccountRange(cardNumber)
+        return getAccountRanges(cardNumber).orEmpty().firstOrNull {
+            it.binRange.matches(cardNumber)
         }
     }
 
     override suspend fun getAccountRanges(
         cardNumber: CardNumber.Unvalidated
     ): List<AccountRange>? {
-        return cardNumber.bin?.let { bin ->
-            if (store.contains(bin)) {
-                inMemorySource.getAccountRanges(cardNumber)
-            } else {
-                remoteSource.getAccountRanges(cardNumber)
-            } ?: staticSource.getAccountRanges(cardNumber)
+        val bin = cardNumber.bin ?: return null
+
+        val ranges = if (store.contains(bin)) {
+            inMemorySource.getAccountRanges(cardNumber)
+        } else {
+            remoteSource.getAccountRanges(cardNumber)
         }
+
+        if (!ranges.isNullOrEmpty()) {
+            store.save(bin, ranges)
+        }
+
+        return ranges?.takeIf { it.isNotEmpty() } ?: staticSource.getAccountRanges(cardNumber)
     }
 
     override val loading: Flow<Boolean> = combine(

diff --git a/...s-core/src/main/java/com/stripe/android/cards/DefaultCardAccountRangeRepositoryFactory.kt b/...s-core/src/main/java/com/stripe/android/cards/DefaultCardAccountRangeRepositoryFactory.kt
@@ -54,7 +54,6 @@ class DefaultCardAccountRangeRepositoryFactory(
                 ApiRequest.Options(
                     publishableKey
                 ),
-                DefaultCardAccountRangeStore(appContext),
                 DefaultAnalyticsRequestExecutor(),
                 PaymentAnalyticsRequestFactory(appContext, publishableKey)
             ),
@@ -88,7 +87,6 @@ class DefaultCardAccountRangeRepositoryFactory(
                     ApiRequest.Options(
                         publishableKey
                     ),
-                    DefaultCardAccountRangeStore(appContext),
                     DefaultAnalyticsRequestExecutor(),
                     PaymentAnalyticsRequestFactory(appContext, publishableKey)
                 )

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
@@ -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
@@ -9,10 +9,9 @@ import com.stripe.android.networking.StripeRepository
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
 
-internal class RemoteCardAccountRangeSource(
+internal class RemoteCardAccountRangeSource constructor(
     private val stripeRepository: StripeRepository,
     private val requestOptions: ApiRequest.Options,
-    private val cardAccountRangeStore: CardAccountRangeStore,
     private val analyticsRequestExecutor: AnalyticsRequestExecutor,
     private val paymentAnalyticsRequestFactory: PaymentAnalyticsRequestFactory
 ) : CardAccountRangeSource {
@@ -26,23 +25,20 @@ internal class RemoteCardAccountRangeSource(
         cardNumber: CardNumber.Unvalidated
     ): List<AccountRange>? {
         return cardNumber.bin?.let { bin ->
-            _loading.value = true
+            val accountRanges = withLoading {
+                stripeRepository.getCardMetadata(bin, requestOptions)?.accountRanges
+            }
 
-            val accountRanges =
-                stripeRepository.getCardMetadata(bin, requestOptions)?.accountRanges.orEmpty()
-            cardAccountRangeStore.save(bin, accountRanges)
 
-            _loading.value = false
 
-            when {
-                accountRanges.isNotEmpty() -> {
-                    if (cardNumber.isValidLuhn) {
-                        onCardMetadataMissingRange()
-                    }
-                    accountRanges
+            if (!accountRanges.isNullOrEmpty()) {
+                val hasMatch = accountRanges.any { it.binRange.matches(cardNumber) }
+                if (!hasMatch && cardNumber.isValidLuhn) {
+                    onCardMetadataMissingRange()
                 }
-                else -> null
             }
+
+            accountRanges
         }
     }
 
@@ -51,4 +47,11 @@ internal class RemoteCardAccountRangeSource(
             paymentAnalyticsRequestFactory.createRequest(PaymentAnalyticsEvent.CardMetadataMissingRange)
         )
     }
+
+    private inline fun <T> withLoading(block: () -> T): T {
+        _loading.value = true
+        val result = block()
+        _loading.value = false
+        return result
+    }
 }
diff --git a/payments-core/src/test/java/com/stripe/android/cards/CardAccountRangeServiceTest.kt b/payments-core/src/test/java/com/stripe/android/cards/CardAccountRangeServiceTest.kt
@@ -40,9 +40,7 @@ class CardAccountRangeServiceTest {
 
     @Test
     fun `test the card metadata service is called only for UnionPay cards`() = runTest {
-        val setRemoteCheckedCards =
-            UNIONPAY16_ACCOUNTS
-                .plus(UNIONPAY19_ACCOUNTS)
+        val setRemoteCheckedCards = UNIONPAY16_ACCOUNTS + UNIONPAY19_ACCOUNTS
         setRemoteCheckedCards.forEach {
             testIfRemoteCalled(it.binRange.low, true)
             testIfRemoteCalled(it.binRange.high, true)
@@ -77,7 +75,7 @@ class CardAccountRangeServiceTest {
             } else {
                 never()
             }
-        ).getAccountRange(cardNumber)
+        ).getAccountRanges(cardNumber)
     }
 
     private fun createMockRemoteDefaultCardAccountRangeRepository(
@@ -134,7 +132,6 @@ class CardAccountRangeServiceTest {
         return RemoteCardAccountRangeSource(
             StripeApiRepository(applicationContext, { publishableKey }),
             ApiRequest.Options(publishableKey),
-            DefaultCardAccountRangeStore(applicationContext),
             DefaultAnalyticsRequestExecutor(),
             PaymentAnalyticsRequestFactory(applicationContext, publishableKey)
         )

diff --git a/...ents-core/src/test/java/com/stripe/android/cards/DefaultCardAccountRangeRepositoryTest.kt b/...ents-core/src/test/java/com/stripe/android/cards/DefaultCardAccountRangeRepositoryTest.kt
@@ -263,14 +263,13 @@ internal class DefaultCardAccountRangeRepositoryTest {
     ): CardAccountRangeRepository {
         return DefaultCardAccountRangeRepository(
             inMemorySource = InMemoryCardAccountRangeSource(store),
-            remoteSource = createRemoteCardAccountRangeSource(store),
+            remoteSource = createRemoteCardAccountRangeSource(),
             staticSource = StaticCardAccountRangeSource(),
-            store = realStore
+            store = realStore,
         )
     }
 
     private fun createRemoteCardAccountRangeSource(
-        store: CardAccountRangeStore,
         publishableKey: String = ApiKeyFixtures.DEFAULT_PUBLISHABLE_KEY
     ): CardAccountRangeSource {
         val stripeRepository = StripeApiRepository(
@@ -280,7 +279,6 @@ internal class DefaultCardAccountRangeRepositoryTest {
         return RemoteCardAccountRangeSource(
             stripeRepository,
             ApiRequest.Options(publishableKey),
-            store,
             { },
             PaymentAnalyticsRequestFactory(application, publishableKey)
         )

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
@@ -24,14 +24,12 @@ import kotlin.test.Test
 
 @RunWith(RobolectricTestRunner::class)
 internal class RemoteCardAccountRangeSourceTest {
-    private val cardAccountRangeStore = mock<CardAccountRangeStore>()
 
     @Test
     fun `getAccountRange() should return expected AccountRange`() = runTest {
         val remoteCardAccountRangeSource = RemoteCardAccountRangeSource(
             FakeStripeRepository(VISA_METADATA),
             REQUEST_OPTIONS,
-            cardAccountRangeStore,
             { },
             PaymentAnalyticsRequestFactory(
                 ApplicationProvider.getApplicationContext(),
@@ -54,10 +52,6 @@ internal class RemoteCardAccountRangeSourceTest {
                 country = "GB"
             )
         )
-        verify(cardAccountRangeStore).save(
-            BinFixtures.VISA,
-            AccountRangeFixtures.DEFAULT
-        )
     }
 
     @Test
@@ -66,7 +60,6 @@ internal class RemoteCardAccountRangeSourceTest {
             val remoteCardAccountRangeSource = RemoteCardAccountRangeSource(
                 FakeStripeRepository(EMPTY_METADATA),
                 REQUEST_OPTIONS,
-                cardAccountRangeStore,
                 { },
                 PaymentAnalyticsRequestFactory(
                     ApplicationProvider.getApplicationContext(),
@@ -79,10 +72,6 @@ internal class RemoteCardAccountRangeSourceTest {
                     CardNumberFixtures.VISA
                 )
             ).isNull()
-            verify(cardAccountRangeStore).save(
-                BinFixtures.VISA,
-                emptyList()
-            )
         }
 
     @Test
@@ -93,7 +82,6 @@ internal class RemoteCardAccountRangeSourceTest {
             val remoteCardAccountRangeSource = RemoteCardAccountRangeSource(
                 repository,
                 REQUEST_OPTIONS,
-                cardAccountRangeStore,
                 { },
                 PaymentAnalyticsRequestFactory(
                     ApplicationProvider.getApplicationContext(),
@@ -108,7 +96,6 @@ internal class RemoteCardAccountRangeSourceTest {
             ).isNull()
 
             verify(repository, never()).getCardMetadata(any(), any())
-            verify(cardAccountRangeStore, never()).save(any(), any())
         }
 
     @Test
@@ -134,7 +121,6 @@ internal class RemoteCardAccountRangeSourceTest {
                     )
                 ),
                 REQUEST_OPTIONS,
-                cardAccountRangeStore,
                 {
                     analyticsRequests.add(it)
                 },