commit 6224e0d18940ad45d96715b28b63850086411b7f parent bcc86c1763f0800c43a3bb7a085cbf04957c7152 Author: fmasalha <fmasalha@mozilla.com> Date: Tue, 16 Dec 2025 16:31:01 +0000 Bug 1979206 - Handled AutofillApiException and changed getAllCreditCards to return Result type r=android-reviewers,android-l10n-reviewers,flod,matt-tighe Differential Revision: https://phabricator.services.mozilla.com/D275125 Diffstat:
14 files changed, 70 insertions(+), 30 deletions(-)
diff --git a/mobile/android/android-components/components/concept/storage/src/main/java/mozilla/components/concept/storage/CreditCardsAddressesStorage.kt b/mobile/android/android-components/components/concept/storage/src/main/java/mozilla/components/concept/storage/CreditCardsAddressesStorage.kt @@ -49,9 +49,10 @@ interface CreditCardsAddressesStorage { /** * Retrieves a list of all the credit cards. * - * @return A list of all [CreditCard]. + * @return A [Result] containing the list of [CreditCard]s on success, or a failure if the + * underlying autofill storage throws an [AutofillApiException]. */ - suspend fun getAllCreditCards(): List<CreditCard> + suspend fun getAllCreditCards(): Result<List<CreditCard>> /** * Deletes the credit card with the given [guid]. @@ -442,6 +443,11 @@ interface CreditCardValidationDelegate { * can be used to update its information. */ data class CanBeUpdated(val foundCreditCard: CreditCard) : Result() + + /** + * Indicates that an error occurred in the storage layer. + */ + data object StorageFailure : Result() } /** diff --git a/mobile/android/android-components/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/creditcard/CreditCardSaveDialogFragment.kt b/mobile/android/android-components/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/creditcard/CreditCardSaveDialogFragment.kt @@ -32,6 +32,7 @@ import mozilla.components.feature.prompts.dialog.KEY_SHOULD_DISMISS_ON_LOAD import mozilla.components.feature.prompts.dialog.PromptDialogFragment import mozilla.components.feature.prompts.facts.emitCreditCardAutofillCreatedFact import mozilla.components.feature.prompts.facts.emitCreditCardAutofillUpdatedFact +import mozilla.components.support.base.log.logger.Logger import mozilla.components.support.ktx.android.content.appName import mozilla.components.support.ktx.android.view.toScope import mozilla.components.support.utils.creditCardIssuerNetwork @@ -54,6 +55,8 @@ internal class CreditCardSaveDialogFragment : PromptDialogFragment() { @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) internal var confirmResult: Result = Result.CanBeCreated + private val logger = Logger("CreditCardSaveDialogFragment") + override fun onCreateDialog(savedInstanceState: Bundle?): Dialog { return BottomSheetDialog(requireContext(), R.style.MozDialogStyle).apply { setCancelable(true) @@ -125,6 +128,7 @@ internal class CreditCardSaveDialogFragment : PromptDialogFragment() { is Result.CanBeUpdated -> { emitCreditCardAutofillUpdatedFact() } + else -> {} } } @@ -141,7 +145,8 @@ internal class CreditCardSaveDialogFragment : PromptDialogFragment() { */ private fun updateUI(view: View) = view.toScope().launch(IO) { val validationDelegate = feature?.creditCardValidationDelegate ?: return@launch - confirmResult = validationDelegate.shouldCreateOrUpdate(creditCard) + val result = validationDelegate.shouldCreateOrUpdate(creditCard) + confirmResult = result withContext(Main) { when (confirmResult) { @@ -158,6 +163,14 @@ internal class CreditCardSaveDialogFragment : PromptDialogFragment() { confirmButtonText = requireContext().getString(R.string.mozac_feature_prompt_update_confirmation), showMessageBody = false, ) + is Result.StorageFailure -> { + logger.error("Failed to validate credit card before showing prompt") + feature?.onCancel( + sessionId = sessionId, + promptRequestUID = promptRequestUID, + ) + dismiss() + } } } } diff --git a/mobile/android/android-components/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/AutofillCreditCardsAddressesStorage.kt b/mobile/android/android-components/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/AutofillCreditCardsAddressesStorage.kt @@ -123,8 +123,8 @@ class AutofillCreditCardsAddressesStorage( } } - override suspend fun getAllCreditCards(): List<CreditCard> = withContext(coroutineContext) { - conn.getStorage().getAllCreditCards().map { it.into() } + override suspend fun getAllCreditCards(): Result<List<CreditCard>> = withContext(coroutineContext) { + Result.runCatching { conn.getStorage().getAllCreditCards().map { it.into() } } } override suspend fun deleteCreditCard(guid: String): Boolean = withContext(coroutineContext) { diff --git a/mobile/android/android-components/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/DefaultCreditCardValidationDelegate.kt b/mobile/android/android-components/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/DefaultCreditCardValidationDelegate.kt @@ -26,7 +26,9 @@ class DefaultCreditCardValidationDelegate( override suspend fun shouldCreateOrUpdate(creditCard: CreditCardEntry): Result = withContext(coroutineContext) { - val creditCards = storage.value.getAllCreditCards() + val creditCards = storage.value.getAllCreditCards().getOrElse { + return@withContext Result.StorageFailure + } val foundCreditCard = if (creditCards.isEmpty()) { // No credit cards exist in the storage -> create a new credit card diff --git a/mobile/android/android-components/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegate.kt b/mobile/android/android-components/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegate.kt @@ -71,7 +71,10 @@ class GeckoCreditCardsAddressesStorageDelegate( if (!isCreditCardAutofillEnabled()) { emptyList() } else { - storage.value.getAllCreditCards() + storage.value.getAllCreditCards().getOrElse { + logger.error("Failed to load credit cards for autofill", it) + emptyList() + } } } @@ -105,6 +108,7 @@ class GeckoCreditCardsAddressesStorageDelegate( ), ) } + is CreditCardValidationDelegate.Result.StorageFailure -> {} } } } diff --git a/mobile/android/android-components/components/service/sync-autofill/src/test/java/mozilla/components/service/sync/autofill/AutofillCreditCardsAddressesStorageTest.kt b/mobile/android/android-components/components/service/sync-autofill/src/test/java/mozilla/components/service/sync/autofill/AutofillCreditCardsAddressesStorageTest.kt @@ -128,7 +128,7 @@ class AutofillCreditCardsAddressesStorageTest { val creditCard2 = storage.addCreditCard(creditCardFields2) val creditCard3 = storage.addCreditCard(creditCardFields3) - val creditCards = storage.getAllCreditCards() + val creditCards = storage.getAllCreditCards().getOrThrow() val key = storage.crypto.getOrGenerateKey() val savedCreditCard1 = creditCards.find { it == creditCard1 } diff --git a/mobile/android/android-components/components/service/sync-autofill/src/test/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegateTest.kt b/mobile/android/android-components/components/service/sync-autofill/src/test/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegateTest.kt @@ -79,7 +79,7 @@ class GeckoCreditCardsAddressesStorageDelegateTest { runTest(testDispatcher) { val storage: AutofillCreditCardsAddressesStorage = mock() val storedCards = listOf<CreditCard>(mock()) - doReturn(storedCards).`when`(storage).getAllCreditCards() + doReturn(Result.success(storedCards)).`when`(storage).getAllCreditCards() delegate = GeckoCreditCardsAddressesStorageDelegate(lazy { storage }, testDispatcher, isCreditCardAutofillEnabled = { true }) val result = delegate.onCreditCardsFetch() @@ -93,7 +93,7 @@ class GeckoCreditCardsAddressesStorageDelegateTest { runTest(testDispatcher) { val storage: AutofillCreditCardsAddressesStorage = mock() val storedCards = listOf<CreditCard>(mock()) - doReturn(storedCards).`when`(storage).getAllCreditCards() + doReturn(Result.success(storedCards)).`when`(storage).getAllCreditCards() delegate = GeckoCreditCardsAddressesStorageDelegate(lazy { storage }, testDispatcher, isCreditCardAutofillEnabled = { false }) val result = delegate.onCreditCardsFetch() @@ -228,7 +228,7 @@ class GeckoCreditCardsAddressesStorageDelegateTest { runTest(testDispatcher) { val storage: AutofillCreditCardsAddressesStorage = mock() val storedCards = listOf<CreditCard>(mock()) - doReturn(storedCards).`when`(storage).getAllCreditCards() + doReturn(Result.success(storedCards)).`when`(storage).getAllCreditCards() delegate = GeckoCreditCardsAddressesStorageDelegate(lazy { storage }, testDispatcher, isAddressAutofillEnabled = { false }) val result = delegate.onAddressesFetch() diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/FenixApplication.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/FenixApplication.kt @@ -1004,7 +1004,8 @@ open class FenixApplication : LocaleAwareApplication(), Provider { val autoFillStorage = applicationContext.components.core.autofillStorage val addresses = autoFillStorage.getAllAddresses().getOrElse { throw it } Addresses.savedAll.set(addresses.size.toLong()) - CreditCards.savedAll.set(autoFillStorage.getAllCreditCards().size.toLong()) + val ccs = autoFillStorage.getAllCreditCards().getOrElse { throw it } + CreditCards.savedAll.set(ccs.size.toLong()) } catch (e: AutofillApiException) { logger.error("Failed to fetch autofill data", e) } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/debugsettings/addresses/FakeCreditCardsAddressesStorage.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/debugsettings/addresses/FakeCreditCardsAddressesStorage.kt @@ -104,7 +104,9 @@ internal class FakeCreditCardsAddressesStorage : CreditCardsAddressesStorage { throw UnsupportedOperationException() } - override suspend fun getAllCreditCards(): List<CreditCard> = creditCards + override suspend fun getAllCreditCards(): Result<List<CreditCard>> { + return Result.success(creditCards) + } override suspend fun deleteCreditCard(guid: String): Boolean { return creditCards.remove(creditCards.find { it.guid == guid }) diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/debugsettings/creditcards/CreditCardsTools.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/debugsettings/creditcards/CreditCardsTools.kt @@ -45,26 +45,27 @@ fun CreditCardsTools( val scope = rememberCoroutineScope() var creditCards by remember { mutableStateOf(listOf<CreditCard>()) } LaunchedEffect(Unit) { - creditCards = creditCardsAddressesStorage.getAllCreditCards() + creditCards = creditCardsAddressesStorage.getAllCreditCards().getOrDefault(emptyList()) } val onAddCreditCard: () -> Unit = { scope.launch { creditCardsAddressesStorage.addCreditCard(FakeCreditCardsAddressesStorage.generateCreditCard()) - creditCards = creditCardsAddressesStorage.getAllCreditCards() + creditCards = creditCardsAddressesStorage.getAllCreditCards().getOrDefault(emptyList()) } } val onDeleteCreditCard: (CreditCard) -> Unit = { creditCard -> scope.launch { creditCardsAddressesStorage.deleteCreditCard(creditCard.guid) - creditCards = creditCardsAddressesStorage.getAllCreditCards() + creditCards = creditCardsAddressesStorage.getAllCreditCards().getOrDefault(emptyList()) } } val onDeleteAllCreditCards: () -> Unit = { scope.launch { - creditCardsAddressesStorage.getAllCreditCards().forEach { creditCard -> - creditCardsAddressesStorage.deleteCreditCard(creditCard.guid) - } - creditCards = creditCardsAddressesStorage.getAllCreditCards() + creditCardsAddressesStorage.getAllCreditCards().getOrDefault(emptyList()) + .forEach { creditCard -> + creditCardsAddressesStorage.deleteCreditCard(creditCard.guid) + } + creditCards = creditCardsAddressesStorage.getAllCreditCards().getOrDefault(emptyList()) } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/autofill/AutofillSettingFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/autofill/AutofillSettingFragment.kt @@ -404,13 +404,16 @@ class AutofillSettingFragment : BiometricPromptPreferenceFragment() { lifecycleScope.launch(Dispatchers.Main) { addresses.onSuccess { store.dispatch(AutofillAction.UpdateAddresses(it)) } - store.dispatch(AutofillAction.UpdateCreditCards(creditCards)) - if (addresses.isFailure) { - Snackbar.make( - requireView(), - R.string.autofill_addresses_load_error, - Snackbar.LENGTH_LONG, - ).show() + creditCards.onSuccess { store.dispatch(AutofillAction.UpdateCreditCards(it)) } + val errorMessageId = when { + addresses.isFailure && creditCards.isFailure -> R.string.autofill_load_error + addresses.isFailure -> R.string.autofill_addresses_load_error + creditCards.isFailure -> R.string.autofill_credit_card_load_error + else -> null + } + + errorMessageId?.let { + Snackbar.make(requireView(), it, Snackbar.LENGTH_LONG).show() } } isAutofillStateLoaded = true diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/autofill/ui/AutofillSettingsMiddleware.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/autofill/ui/AutofillSettingsMiddleware.kt @@ -112,8 +112,8 @@ internal class AutofillSettingsMiddleware( val creditCards = autofillSettingsStorage.getAllCreditCards() addresses.onSuccess { dispatch(UpdateAddresses(addresses = it)) } - dispatch(UpdateCreditCards(creditCards = creditCards)) - val failure = addresses.exceptionOrNull() + creditCards.onSuccess { dispatch(UpdateCreditCards(creditCards = it)) } + val failure = addresses.exceptionOrNull() ?: creditCards.exceptionOrNull() if (failure != null) { logger.error("Failed to load autofill data", failure) } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/creditcards/CreditCardsManagementFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/creditcards/CreditCardsManagementFragment.kt @@ -10,6 +10,7 @@ import android.view.View import android.view.ViewGroup import androidx.lifecycle.lifecycleScope import androidx.navigation.fragment.findNavController +import com.google.android.material.snackbar.Snackbar import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import mozilla.components.lib.state.ext.consumeFrom @@ -101,7 +102,10 @@ class CreditCardsManagementFragment : SecureFragment() { val creditCards = requireContext().components.core.autofillStorage.getAllCreditCards() lifecycleScope.launch(Dispatchers.Main) { - store.dispatch(AutofillAction.UpdateCreditCards(creditCards)) + creditCards.onSuccess { store.dispatch(AutofillAction.UpdateCreditCards(it)) } + creditCards.onFailure { + Snackbar.make(requireView(), R.string.autofill_load_error, Snackbar.LENGTH_LONG).show() + } } } } diff --git a/mobile/android/fenix/app/src/main/res/values/strings.xml b/mobile/android/fenix/app/src/main/res/values/strings.xml @@ -2535,6 +2535,10 @@ <string name="preferences_addresses_manage_addresses">Manage addresses</string> <!-- Snackbar message shown when autofill addresses data cannot be loaded --> <string name="autofill_addresses_load_error">Could not load addresses autofill data</string> + <!-- Snackbar message shown when autofill data cannot be loaded --> + <string name="autofill_load_error">Could not load autofill data</string> + <!-- Snackbar message shown when autofill credit card data cannot be loaded --> + <string name="autofill_credit_card_load_error">Could not load credit card data</string> <!-- Preference for saving and filling addresses --> <string name="preferences_addresses_save_and_autofill_addresses_2">Save and fill addresses</string> <!-- Preference summary for saving and filling address data -->