commit bcc86c1763f0800c43a3bb7a085cbf04957c7152 parent 043c079eb57cb203ee1cc9b1184c4c8a3381bfca Author: fmasalha <fmasalha@mozilla.com> Date: Tue, 16 Dec 2025 16:31:01 +0000 Bug 1979206 - Handled AutofillApiException and changed getAllAddresses to return Result type r=android-reviewers,android-l10n-reviewers,flod,matt-tighe Differential Revision: https://phabricator.services.mozilla.com/D275124 Diffstat:
13 files changed, 70 insertions(+), 26 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 @@ -88,9 +88,10 @@ interface CreditCardsAddressesStorage { /** * Retrieves a list of all the addresses. * - * @return A list of all [Address]. + * @return A [Result] containing the list of [Address]s on success, or a failure if the + * underlying autofill storage throws an [AutofillApiException]. */ - suspend fun getAllAddresses(): List<Address> + suspend fun getAllAddresses(): Result<List<Address>> /** * Updates the fields in the provided address. 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 @@ -148,8 +148,8 @@ class AutofillCreditCardsAddressesStorage( } } - override suspend fun getAllAddresses(): List<Address> = withContext(coroutineContext) { - conn.getStorage().getAllAddresses().map { it.into() } + override suspend fun getAllAddresses(): Result<List<Address>> = withContext(coroutineContext) { + Result.runCatching { conn.getStorage().getAllAddresses().map { it.into() } } } override suspend fun updateAddress(guid: String, address: UpdatableAddressFields) = 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 @@ -17,6 +17,7 @@ import mozilla.components.concept.storage.CreditCardsAddressesStorageDelegate import mozilla.components.concept.storage.ManagedKey import mozilla.components.concept.storage.NewCreditCardFields import mozilla.components.concept.storage.UpdatableCreditCardFields +import mozilla.components.support.base.log.logger.Logger import mozilla.components.support.ktx.kotlin.last4Digits /** @@ -35,6 +36,7 @@ class GeckoCreditCardsAddressesStorageDelegate( private val isCreditCardAutofillEnabled: () -> Boolean = { false }, private val isAddressAutofillEnabled: () -> Boolean = { false }, ) : CreditCardsAddressesStorageDelegate { + private val logger = Logger("GeckoCCAddressesStorageDelegate") override suspend fun getOrGenerateKey(): ManagedKey { val crypto = storage.value.getCreditCardCrypto() @@ -53,7 +55,10 @@ class GeckoCreditCardsAddressesStorageDelegate( if (!isAddressAutofillEnabled()) { emptyList() } else { - storage.value.getAllAddresses() + storage.value.getAllAddresses().getOrElse { + logger.error("Failed to load addresses for autofill", it) + emptyList() + } } } 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 @@ -317,7 +317,7 @@ class AutofillCreditCardsAddressesStorageTest { val address2 = storage.addAddress(addressFields2) val address3 = storage.addAddress(addressFields3) - val addresses = storage.getAllAddresses() + val addresses = storage.getAllAddresses().getOrThrow() val savedAddress1 = addresses.find { it == address1 } assertNotNull(savedAddress1) 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 @@ -214,7 +214,7 @@ class GeckoCreditCardsAddressesStorageDelegateTest { runTest(testDispatcher) { val storage: AutofillCreditCardsAddressesStorage = mock() val storedAddresses = listOf<Address>(mock(), mock()) - doReturn(storedAddresses).`when`(storage).getAllAddresses() + doReturn(Result.success(storedAddresses)).`when`(storage).getAllAddresses() delegate = GeckoCreditCardsAddressesStorageDelegate(lazy { storage }, testDispatcher, isAddressAutofillEnabled = { true }) 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 @@ -1002,7 +1002,8 @@ open class FenixApplication : LocaleAwareApplication(), Provider { GlobalScope.launch(IO) { try { val autoFillStorage = applicationContext.components.core.autofillStorage - Addresses.savedAll.set(autoFillStorage.getAllAddresses().size.toLong()) + val addresses = autoFillStorage.getAllAddresses().getOrElse { throw it } + Addresses.savedAll.set(addresses.size.toLong()) CreditCards.savedAll.set(autoFillStorage.getAllCreditCards().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/AddressesTools.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/debugsettings/addresses/AddressesTools.kt @@ -31,6 +31,7 @@ import kotlinx.coroutines.launch import mozilla.components.compose.base.button.FilledButton import mozilla.components.concept.storage.Address import mozilla.components.concept.storage.CreditCardsAddressesStorage +import mozilla.components.support.base.log.logger.Logger import org.mozilla.fenix.R import org.mozilla.fenix.compose.SwitchWithLabel import org.mozilla.fenix.compose.list.RadioButtonListItem @@ -55,29 +56,43 @@ fun AddressesTools( possibleDebugRegions = possibleDebugRegions.updateRegionEnabled(region, isEnabled) } + val logger = Logger + val scope = rememberCoroutineScope() var addresses by remember { mutableStateOf(listOf<Address>()) } LaunchedEffect(Unit) { - addresses = creditCardsAddressesStorage.getAllAddresses() + creditCardsAddressesStorage.getAllAddresses() + .onSuccess { addresses = it } + .onFailure { logger.error("Failed to load addresses", it) } } val onAddAddress: (String) -> Unit = { selectedLangTag -> scope.launch { creditCardsAddressesStorage.addAddress(selectedLangTag.generateFakeAddressForLangTag()) - addresses = creditCardsAddressesStorage.getAllAddresses() + creditCardsAddressesStorage.getAllAddresses() + .onSuccess { addresses = it } + .onFailure { logger.error("Failed to reload addresses after add", it) } } } val onDeleteAddress: (Address) -> Unit = { address -> scope.launch { creditCardsAddressesStorage.deleteAddress(address.guid) - addresses = creditCardsAddressesStorage.getAllAddresses() + creditCardsAddressesStorage.getAllAddresses() + .onSuccess { addresses = it } + .onFailure { logger.error("Failed to reload addresses after delete", it) } } } val onDeleteAllAddresses: () -> Unit = { scope.launch { - creditCardsAddressesStorage.getAllAddresses().forEach { address -> - creditCardsAddressesStorage.deleteAddress(address.guid) - } - addresses = creditCardsAddressesStorage.getAllAddresses() + creditCardsAddressesStorage.getAllAddresses() + .onSuccess { loaded -> + loaded.forEach { address -> + creditCardsAddressesStorage.deleteAddress(address.guid) + } + creditCardsAddressesStorage.getAllAddresses() + .onSuccess { addresses = it } + .onFailure { logger.error("Failed to reload addresses after delete all", it) } + } + .onFailure { logger.error("Failed to load addresses before delete all", it) } } } 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 @@ -123,9 +123,8 @@ internal class FakeCreditCardsAddressesStorage : CreditCardsAddressesStorage { return addresses.find { it.guid == guid } } - override suspend fun getAllAddresses(): List<Address> { - return addresses - } + override suspend fun getAllAddresses(): Result<List<Address>> = + Result.success(addresses) override suspend fun updateAddress(guid: String, address: UpdatableAddressFields) { throw UnsupportedOperationException() diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/address/AddressManagementFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/address/AddressManagementFragment.kt @@ -12,6 +12,7 @@ import androidx.fragment.app.Fragment import androidx.fragment.compose.content 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 @@ -95,10 +96,16 @@ class AddressManagementFragment : Fragment() { */ private fun loadAddresses() { lifecycleScope.launch { - val addresses = requireContext().components.core.autofillStorage.getAllAddresses() - + val result = requireContext().components.core.autofillStorage.getAllAddresses() lifecycleScope.launch(Dispatchers.Main) { - store.dispatch(AutofillAction.UpdateAddresses(addresses)) + result.onSuccess { store.dispatch(AutofillAction.UpdateAddresses(it)) } + result.onFailure { + Snackbar.make( + requireView(), + R.string.autofill_addresses_load_error, + Snackbar.LENGTH_LONG, + ).show() + } } } } 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 @@ -24,6 +24,7 @@ import androidx.preference.Preference import androidx.preference.SwitchPreference import com.google.android.material.color.MaterialColors import com.google.android.material.dialog.MaterialAlertDialogBuilder +import com.google.android.material.snackbar.Snackbar import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.delay import kotlinx.coroutines.launch @@ -402,12 +403,18 @@ class AutofillSettingFragment : BiometricPromptPreferenceFragment() { val creditCards = requireComponents.core.autofillStorage.getAllCreditCards() lifecycleScope.launch(Dispatchers.Main) { - store.dispatch(AutofillAction.UpdateAddresses(addresses)) + 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() + } } + isAutofillStateLoaded = true } - - 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 @@ -16,6 +16,7 @@ import mozilla.components.lib.state.MiddlewareContext import mozilla.components.lib.state.Store import mozilla.components.service.fxa.manager.FxaAccountManager import mozilla.components.service.sync.autofill.AutofillCreditCardsAddressesStorage +import mozilla.components.support.base.log.logger.Logger import org.mozilla.fenix.settings.autofill.AutofillScreenDestination import org.mozilla.fenix.settings.logins.ui.LoginsAction @@ -43,6 +44,8 @@ internal class AutofillSettingsMiddleware( private val scope = CoroutineScope(ioDispatcher) private lateinit var observer: AccountObserver + private val logger = Logger + override fun invoke( context: MiddlewareContext<AutofillSettingsState, AutofillSettingsAction>, next: (AutofillSettingsAction) -> Unit, @@ -108,8 +111,12 @@ internal class AutofillSettingsMiddleware( val addresses = autofillSettingsStorage.getAllAddresses() val creditCards = autofillSettingsStorage.getAllCreditCards() - dispatch(UpdateAddresses(addresses = addresses)) + addresses.onSuccess { dispatch(UpdateAddresses(addresses = it)) } dispatch(UpdateCreditCards(creditCards = creditCards)) + val failure = addresses.exceptionOrNull() + if (failure != null) { + logger.error("Failed to load autofill data", failure) + } } private fun Store<AutofillSettingsState, AutofillSettingsAction>.registerObserverForAccountChanges( diff --git a/mobile/android/fenix/app/src/main/res/values/strings.xml b/mobile/android/fenix/app/src/main/res/values/strings.xml @@ -2533,6 +2533,8 @@ <string name="preferences_addresses_add_address">Add address</string> <!-- Preference option for managing saved addresses --> <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> <!-- 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 --> diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/settings/creditcards/ui/FakeCreditCardsStorage.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/settings/creditcards/ui/FakeCreditCardsStorage.kt @@ -96,7 +96,7 @@ class FakeCreditCardsStorage( override suspend fun getAddress(guid: String): Address? = null - override suspend fun getAllAddresses(): List<Address> = emptyList() + override suspend fun getAllAddresses(): Result<List<Address>> = Result.success(emptyList()) override suspend fun updateAddress(guid: String, address: UpdatableAddressFields) = Unit