From 3932be05f27b320489ae5d3d5a9a4d3f264d3f54 Mon Sep 17 00:00:00 2001 From: Gabriel Luong Date: Thu, 11 Mar 2021 11:46:16 -0500 Subject: [PATCH] Issue #9838: Introduce CreditCardValidationDelegate and implement onCreditCardSave in GeckoCreditCardsAddressesStorageDelegate - Introduces `CreditCardValidationDelegate` and a default implementation in `DefaultCreditCardValidationDelegate` - Adds a `wipeLocal()` to `CreditCardsAddressesStorage` to clear out local state in between tests - Implements `onCreditCardSave` in `GeckoCreditCardsAddressesStorageDelegate` --- .../storage/CreditCardsAddressesStorage.kt | 61 +++++++- .../AutofillCreditCardsAddressesStorage.kt | 12 ++ .../DefaultCreditCardValidationDelegate.kt | 44 ++++++ ...eckoCreditCardsAddressesStorageDelegate.kt | 20 ++- .../components/service/sync/autofill/Types.kt | 27 ++++ ...AutofillCreditCardsAddressesStorageTest.kt | 95 ++++++++++++ ...DefaultCreditCardValidationDelegateTest.kt | 145 ++++++++++++++++++ docs/changelog.md | 1 + 8 files changed, 400 insertions(+), 5 deletions(-) create mode 100644 components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/DefaultCreditCardValidationDelegate.kt create mode 100644 components/service/sync-autofill/src/test/java/mozilla/components/service/sync/autofill/DefaultCreditCardValidationDelegateTest.kt diff --git a/components/concept/storage/src/main/java/mozilla/components/concept/storage/CreditCardsAddressesStorage.kt b/components/concept/storage/src/main/java/mozilla/components/concept/storage/CreditCardsAddressesStorage.kt index cc94b48d6f1..8f5131765af 100644 --- a/components/concept/storage/src/main/java/mozilla/components/concept/storage/CreditCardsAddressesStorage.kt +++ b/components/concept/storage/src/main/java/mozilla/components/concept/storage/CreditCardsAddressesStorage.kt @@ -103,6 +103,11 @@ interface CreditCardsAddressesStorage { * @param guid Unique identifier for the desired address. */ suspend fun touchAddress(guid: String) + + /** + * Clears out all local state, bringing us back to the state before the first write (or sync). + */ + suspend fun wipeLocal() } /** @@ -126,10 +131,10 @@ data class CreditCard( val expiryMonth: Long, val expiryYear: Long, val cardType: String, - val timeCreated: Long, - val timeLastUsed: Long?, - val timeLastModified: Long, - val timesUsed: Long + val timeCreated: Long = 0L, + val timeLastUsed: Long? = 0L, + val timeLastModified: Long = 0L, + val timesUsed: Long = 0L ) /** @@ -221,6 +226,46 @@ data class UpdatableAddressFields( val email: String ) +/** + * Provides a method for checking whether or not a given credit card can be stored. + */ +interface CreditCardValidationDelegate { + + /** + * The result from validating a given [CreditCard] against the credit card storage. This will + * include whether or not it can be created, updated, or neither, along with an explanation + * of any errors. + */ + sealed class Result { + /** + * Indicates that the [CreditCard] does not currently exist in the storage, and a new + * credit card entry can be created. + */ + object CanBeCreated : Result() + + /** + * Indicates that a matching [CreditCard] was found in the storage, and the [CreditCard] + * can be used to update its information. + */ + data class CanBeUpdated(val foundCreditCard: CreditCard) : Result() + + /** + * The [CreditCard] cannot be saved. + */ + object Error : Result() + } + + /** + * Determines whether a [CreditCard] can be added or updated in the credit card storage. + * + * Note that this method is not thread safe. + * + * @param newCreditCard [CreditCard] to be added or updated in the credit card storage. + * @return [Result] that indicates whether or not the [CreditCard] should be saved or updated. + */ + fun validate(newCreditCard: CreditCard): Deferred +} + /** * Used to handle [Address] and [CreditCard] storage so that the underlying engine doesn't have to. * An instance of this should be attached to the Gecko runtime in order to be used. @@ -230,22 +275,30 @@ interface CreditCardsAddressesStorageDelegate { /** * Returns all stored addresses. This is called when the engine believes an address field * should be autofilled. + * + * @return A [Deferred] that will resolve to the list of all stored addresses. */ fun onAddressesFetch(): Deferred> /** * Saves the given address to storage. + * + * @param address [Address] to be saved or updated in the address storage. */ fun onAddressSave(address: Address) /** * Returns all stored credit cards. This is called when the engine believes a credit card * field should be autofilled. + * + * @return A [Deferred] that will resolve to the list of all stored credit cards. */ fun onCreditCardsFetch(): Deferred> /** * Saves the given credit card to storage. + * + * @param creditCard [CreditCard] to be saved or updated in the credit card storage. */ fun onCreditCardSave(creditCard: CreditCard) } diff --git a/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/AutofillCreditCardsAddressesStorage.kt b/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/AutofillCreditCardsAddressesStorage.kt index 64250a9d5b8..1116af517b8 100644 --- a/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/AutofillCreditCardsAddressesStorage.kt +++ b/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/AutofillCreditCardsAddressesStorage.kt @@ -93,6 +93,18 @@ class AutofillCreditCardsAddressesStorage( coroutineContext.cancel() conn.close() } + + override suspend fun wipeLocal() = withContext(coroutineContext) { + val storage = conn.getStorage() + + storage.getAllCreditCards().forEach { creditCard -> + storage.deleteCreditCard(creditCard.guid) + } + + storage.getAllAddresses().forEach { address -> + storage.deleteAddress(address.guid) + } + } } /** diff --git a/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/DefaultCreditCardValidationDelegate.kt b/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/DefaultCreditCardValidationDelegate.kt new file mode 100644 index 00000000000..1bbf10c2941 --- /dev/null +++ b/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/DefaultCreditCardValidationDelegate.kt @@ -0,0 +1,44 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package mozilla.components.service.sync.autofill + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Deferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.async +import mozilla.components.concept.storage.CreditCard +import mozilla.components.concept.storage.CreditCardValidationDelegate +import mozilla.components.concept.storage.CreditCardValidationDelegate.Result +import mozilla.components.concept.storage.CreditCardsAddressesStorage + +/** + * A delegate that will check against the [CreditCardsAddressesStorage] to determine if a given + * [CreditCard] can be persisted and returns information about why it can or cannot. + */ +class DefaultCreditCardValidationDelegate( + private val storage: Lazy, + private val scope: CoroutineScope = CoroutineScope(Dispatchers.IO) +) : CreditCardValidationDelegate { + + override fun validate(newCreditCard: CreditCard): Deferred { + return scope.async { + val creditCards = storage.value.getAllCreditCards() + + val foundCreditCard = if (creditCards.isEmpty()) { + // No credit cards exist in the storage, create a new credit card to the storage. + null + } else { + // Attempt to find a credit card with a matching guid or card number with the + // new credit card. If an existing credit card is found, update it with the new + // credit card entry. Otherwise, create a new credit card entry in the storage. + creditCards.find { it.guid == newCreditCard.guid || it.cardNumber == newCreditCard.cardNumber } + } + + if (foundCreditCard == null) Result.CanBeCreated else Result.CanBeUpdated( + foundCreditCard + ) + } + } +} diff --git a/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegate.kt b/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegate.kt index ec4db0da93b..976a181a04e 100644 --- a/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegate.kt +++ b/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/GeckoCreditCardsAddressesStorageDelegate.kt @@ -8,8 +8,10 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Deferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.async +import kotlinx.coroutines.launch import mozilla.components.concept.storage.Address import mozilla.components.concept.storage.CreditCard +import mozilla.components.concept.storage.CreditCardValidationDelegate import mozilla.components.concept.storage.CreditCardsAddressesStorage import mozilla.components.concept.storage.CreditCardsAddressesStorageDelegate @@ -38,6 +40,22 @@ class GeckoCreditCardsAddressesStorageDelegate( } override fun onCreditCardSave(creditCard: CreditCard) { - TODO("Not yet implemented") + val validationDelegate = DefaultCreditCardValidationDelegate(storage) + scope.launch { + when (val result = validationDelegate.validate(creditCard).await()) { + is CreditCardValidationDelegate.Result.CanBeCreated -> { + storage.value.addCreditCard(creditCard.intoUpdatableCreditCardFields()) + } + is CreditCardValidationDelegate.Result.CanBeUpdated -> { + storage.value.updateCreditCard( + guid = result.foundCreditCard.guid, + creditCardFields = creditCard.intoUpdatableCreditCardFields() + ) + } + is CreditCardValidationDelegate.Result.Error -> { + // Do nothing since an error occurred and the credit card cannot be saved. + } + } + } } } diff --git a/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/Types.kt b/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/Types.kt index 475382e4bac..4a0ed5515a0 100644 --- a/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/Types.kt +++ b/components/service/sync-autofill/src/main/java/mozilla/components/service/sync/autofill/Types.kt @@ -87,3 +87,30 @@ internal fun mozilla.appservices.autofill.CreditCard.into(): CreditCard { timesUsed = this.timesUsed ) } + +internal fun Address.intoUpdatableAddressFields(): mozilla.components.concept.storage.UpdatableAddressFields { + return mozilla.components.concept.storage.UpdatableAddressFields( + givenName = this.givenName, + additionalName = this.additionalName, + familyName = this.familyName, + organization = this.organization, + streetAddress = this.streetAddress, + addressLevel3 = this.addressLevel3, + addressLevel2 = this.addressLevel2, + addressLevel1 = this.addressLevel1, + postalCode = this.postalCode, + country = this.country, + tel = this.tel, + email = this.email + ) +} + +internal fun CreditCard.intoUpdatableCreditCardFields(): mozilla.components.concept.storage.UpdatableCreditCardFields { + return mozilla.components.concept.storage.UpdatableCreditCardFields( + billingName = this.billingName, + cardNumber = this.cardNumber, + expiryMonth = this.expiryMonth, + expiryYear = this.expiryYear, + cardType = this.cardType + ) +} diff --git a/components/service/sync-autofill/src/test/java/mozilla/components/service/sync/autofill/AutofillCreditCardsAddressesStorageTest.kt b/components/service/sync-autofill/src/test/java/mozilla/components/service/sync/autofill/AutofillCreditCardsAddressesStorageTest.kt index ad094baa1ad..9e408b73891 100644 --- a/components/service/sync-autofill/src/test/java/mozilla/components/service/sync/autofill/AutofillCreditCardsAddressesStorageTest.kt +++ b/components/service/sync-autofill/src/test/java/mozilla/components/service/sync/autofill/AutofillCreditCardsAddressesStorageTest.kt @@ -28,6 +28,7 @@ class AutofillCreditCardsAddressesStorageTest { @After fun cleanup() = runBlocking { + storage.wipeLocal() storage.close() } @@ -350,4 +351,98 @@ class AutofillCreditCardsAddressesStorageTest { assertTrue(isDeleteSuccessful) assertEquals(0, storage.getAllAddresses().size) } + + @Test + fun `wipeLocal`() = runBlocking { + var addresses = storage.getAllAddresses() + var creditCards = storage.getAllCreditCards() + + assertEquals(0, addresses.size) + assertEquals(0, creditCards.size) + + val addressFields1 = UpdatableAddressFields( + givenName = "John", + additionalName = "", + familyName = "Smith", + organization = "Mozilla", + streetAddress = "123 Sesame Street", + addressLevel3 = "", + addressLevel2 = "", + addressLevel1 = "", + postalCode = "90210", + country = "US", + tel = "+1 519 555-5555", + email = "foo@bar.com" + ) + val addressFields2 = UpdatableAddressFields( + givenName = "Mary", + additionalName = "", + familyName = "Sue", + organization = "", + streetAddress = "1 New St", + addressLevel3 = "", + addressLevel2 = "York", + addressLevel1 = "SC", + postalCode = "29745", + country = "US", + tel = "+19871234567", + email = "mary@example.com" + ) + val addressFields3 = UpdatableAddressFields( + givenName = "Timothy", + additionalName = "João", + familyName = "Berners-Lee", + organization = "World Wide Web Consortium", + streetAddress = "Rua Adalberto Pajuaba, 404", + addressLevel3 = "Campos Elísios", + addressLevel2 = "Ribeirão Preto", + addressLevel1 = "SP", + postalCode = "14055-220", + country = "BR", + tel = "+0318522222222", + email = "timbr@example.org" + ) + storage.addAddress(addressFields1) + storage.addAddress(addressFields2) + storage.addAddress(addressFields3) + + val creditCardFields1 = UpdatableCreditCardFields( + billingName = "Jane Fields", + cardNumber = "4111111111111111", + expiryMonth = 12, + expiryYear = 2028, + cardType = "amex" + ) + val creditCardFields2 = UpdatableCreditCardFields( + billingName = "Banana Apple", + cardNumber = "4111111111111112", + expiryMonth = 1, + expiryYear = 2030, + cardType = "discover" + ) + val creditCardFields3 = UpdatableCreditCardFields( + billingName = "Pineapple Orange", + cardNumber = "4111111111111113", + expiryMonth = 2, + expiryYear = 2028, + cardType = "visa" + ) + storage.addCreditCard(creditCardFields1) + storage.addCreditCard(creditCardFields2) + storage.addCreditCard(creditCardFields3) + + addresses = storage.getAllAddresses() + creditCards = storage.getAllCreditCards() + + assertEquals(3, addresses.size) + assertEquals(3, creditCards.size) + + storage.wipeLocal() + + addresses = storage.getAllAddresses() + creditCards = storage.getAllCreditCards() + + assertEquals(0, addresses.size) + assertEquals(0, creditCards.size) + } } diff --git a/components/service/sync-autofill/src/test/java/mozilla/components/service/sync/autofill/DefaultCreditCardValidationDelegateTest.kt b/components/service/sync-autofill/src/test/java/mozilla/components/service/sync/autofill/DefaultCreditCardValidationDelegateTest.kt new file mode 100644 index 00000000000..2fc7fe59abc --- /dev/null +++ b/components/service/sync-autofill/src/test/java/mozilla/components/service/sync/autofill/DefaultCreditCardValidationDelegateTest.kt @@ -0,0 +1,145 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package mozilla.components.service.sync.autofill + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.TestCoroutineScope +import mozilla.components.concept.storage.CreditCard +import mozilla.components.concept.storage.CreditCardValidationDelegate.Result +import mozilla.components.concept.storage.UpdatableCreditCardFields +import mozilla.components.support.test.robolectric.testContext +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith + +@ExperimentalCoroutinesApi +@RunWith(AndroidJUnit4::class) +class DefaultCreditCardValidationDelegateTest { + + private lateinit var validationDelegate: DefaultCreditCardValidationDelegate + private lateinit var scope: TestCoroutineScope + + private val storage = AutofillCreditCardsAddressesStorage(testContext) + + init { + testContext.getDatabasePath(AUTOFILL_DB_NAME)!!.parentFile!!.mkdirs() + } + + @Before + fun before() = runBlocking { + scope = TestCoroutineScope() + validationDelegate = + DefaultCreditCardValidationDelegate(storage = lazy { storage }, scope = scope) + } + + @After + fun cleanup() = runBlocking { + storage.wipeLocal() + } + + @Test + fun `WHEN no credit cards exist in the storage, THEN add the new credit card to storage`() = + runBlocking { + val newCreditCard = createCreditCard(guid = "1") + val result = validationDelegate.validate(newCreditCard).await() + + assertEquals(Result.CanBeCreated, result) + } + + @Test + fun `WHEN existing credit card matches by guid and card number, THEN update the credit card in storage`() = + runBlocking { + val creditCardFields = UpdatableCreditCardFields( + billingName = "Pineapple Orange", + cardNumber = "4111111111111111", + expiryMonth = 2, + expiryYear = 2028, + cardType = "visa" + ) + val creditCard = storage.addCreditCard(creditCardFields) + val result = validationDelegate.validate(creditCard).await() + + assertEquals(Result.CanBeUpdated(creditCard), result) + } + + @Test + fun `WHEN existing credit card matches by guid only, THEN update the credit card in storage`() = + runBlocking { + val creditCardFields = UpdatableCreditCardFields( + billingName = "Pineapple Orange", + cardNumber = "4111111111111113", + expiryMonth = 2, + expiryYear = 2028, + cardType = "visa" + ) + val creditCard = storage.addCreditCard(creditCardFields) + val newCreditCard = createCreditCard(guid = creditCard.guid) + val result = validationDelegate.validate(newCreditCard).await() + + assertEquals(Result.CanBeUpdated(creditCard), result) + } + + @Test + fun `WHEN existing credit card matches by card number only, THEN update the credit card in storage`() = + runBlocking { + val creditCardFields = UpdatableCreditCardFields( + billingName = "Pineapple Orange", + cardNumber = "4111111111111114", + expiryMonth = 2, + expiryYear = 2028, + cardType = "visa" + ) + val creditCard = storage.addCreditCard(creditCardFields) + val newCreditCard = createCreditCard( + guid = creditCard.guid + 1, + billingName = creditCard.billingName, + cardNumber = creditCard.cardNumber, + expiryMonth = creditCard.expiryMonth, + expiryYear = creditCard.expiryYear, + cardType = creditCard.cardType + ) + val result = validationDelegate.validate(newCreditCard).await() + + assertEquals(Result.CanBeUpdated(creditCard), result) + } + + @Test + fun `WHEN existing credit card does not match by guid and card number, THEN add the new credit card to storage`() = + runBlocking { + val newCreditCard = createCreditCard(guid = "2") + val creditCardFields = UpdatableCreditCardFields( + billingName = "Pineapple Orange", + cardNumber = "4111111111111116", + expiryMonth = 2, + expiryYear = 2028, + cardType = "visa" + ) + storage.addCreditCard(creditCardFields) + + val result = validationDelegate.validate(newCreditCard).await() + + assertEquals(Result.CanBeCreated, result) + } +} + +fun createCreditCard( + guid: String = "id", + billingName: String = "Banana Apple", + cardNumber: String = "4111111111111110", + expiryMonth: Long = 1, + expiryYear: Long = 2030, + cardType: String = "amex" +) = CreditCard( + guid = guid, + billingName = billingName, + cardNumber = cardNumber, + expiryMonth = expiryMonth, + expiryYear = expiryYear, + cardType = cardType +) diff --git a/docs/changelog.md b/docs/changelog.md index fa000e1daa5..34ea797b76b 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -37,6 +37,7 @@ permalink: /changelog/ * **service-sync-autofill** * Refactors `AutofillCreditCardsAddressesStorage` from **browser-storage-sync** into its own component. [#9801](https://github.com/mozilla-mobile/android-components/issues/9801) + * Added `CreditCardValidationDelegate` which is a delegate that will check against the `CreditCardsAddressesStorage` to determine if a `CreditCard` can be persisted in storage. [#9838](https://github.com/mozilla-mobile/android-components/issues/9838) * **service-firefox-accounts**,**browser-storage-sync**,**service-nimbus**,**service-sync-logins** * Due to a temporary build issue in the Application Services project, it is not currently