From feaa31cbbbce7f6ce61e0b1d9a4b2c0794b65ca1 Mon Sep 17 00:00:00 2001
From: Grisha Kruglov <gkruglov@mozilla.com>
Date: Thu, 5 Mar 2020 14:16:40 -0800
Subject: [PATCH] [components] Closes
 https://github.com/mozilla-mobile/android-components/issues/6184: Pass 'guid'
 correctly into update/add logins methods

---
 .../service/sync/logins/DefaultLoginValidationDelegate.kt      |  6 +++++-
 .../service/sync/logins/GeckoLoginStorageDelegate.kt           | 10 +++++-----
 .../components/service/sync/logins/SyncableLoginsStorage.kt    |  1 +
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/mobile/android/android-components/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/DefaultLoginValidationDelegate.kt b/mobile/android/android-components/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/DefaultLoginValidationDelegate.kt
index 4902c59604a6..d8c11153a9c8 100644
--- a/mobile/android/android-components/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/DefaultLoginValidationDelegate.kt
+++ b/mobile/android/android-components/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/DefaultLoginValidationDelegate.kt
@@ -28,7 +28,11 @@ class DefaultLoginValidationDelegate(
     override fun validateCanPersist(login: Login): Deferred<Result> {
         return scope.async {
             try {
-                storage.ensureValid(login)
+                // We're setting guid=null here in order to trigger de-duping logic properly.
+                // Internally, the library ensures to not dupe records against themselves
+                // (via a `guid <> :guid` check), which means we need to "pretend" this is a new record,
+                // in order for `ensureValid` to actually throw `DUPLICATE_LOGIN`.
+                storage.ensureValid(login.copy(guid = null))
                 Result.CanBeCreated
             } catch (e: InvalidRecordException) {
                 when (e.reason) {
diff --git a/mobile/android/android-components/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/GeckoLoginStorageDelegate.kt b/mobile/android/android-components/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/GeckoLoginStorageDelegate.kt
index 91f764c3fd6e..6b3e8bf0bc10 100644
--- a/mobile/android/android-components/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/GeckoLoginStorageDelegate.kt
+++ b/mobile/android/android-components/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/GeckoLoginStorageDelegate.kt
@@ -88,15 +88,15 @@ class GeckoLoginStorageDelegate(
                 Operation.CREATE -> {
                     // If an existing Login was autofilled, we want to clear its guid to
                     // avoid updating its record
-                    loginStorage.add(login.copy(guid = ""))
+                    loginStorage.add(login.copy(guid = null))
                 }
             }
         }
     }
 
     /**
-     * Returns whether an existing login record should be [UPDATE]d or a new one [CREATE]d, based
-     * on the saved [ServerPassword] and new [Login].
+     * Returns whether an existing login record should be UPDATED or a new one [CREATE]d, based
+     * on the saved [Login] and new [Login].
      */
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
     fun getPersistenceOperation(newLogin: Login, savedLogin: Login?): Operation = when {
@@ -146,8 +146,8 @@ fun Login.mergeWithLogin(login: Login): Login {
  * Converts an Android Components [Login] to an Application Services [ServerPassword]
  */
 fun Login.toServerPassword() = ServerPassword(
-    // Underlying Rust code will generate a new GUID
-    id = "",
+    // Underlying Rust code will generate a new GUID if `guid` is missing.
+    id = guid ?: "",
     username = username,
     password = password,
     hostname = origin,
diff --git a/mobile/android/android-components/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/SyncableLoginsStorage.kt b/mobile/android/android-components/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/SyncableLoginsStorage.kt
index 3686236da03e..2c2923575f5f 100644
--- a/mobile/android/android-components/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/SyncableLoginsStorage.kt
+++ b/mobile/android/android-components/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/SyncableLoginsStorage.kt
@@ -183,6 +183,7 @@ class SyncableLoginsStorage(
      */
     @Throws(IdCollisionException::class, InvalidRecordException::class, LoginsStorageException::class)
     override suspend fun add(login: Login): String = withContext(coroutineContext) {
+        check(login.guid == null) { "'guid' for a new login must be `null`" }
         conn.getStorage().add(login.toServerPassword())
     }
 
-- 
2.11.4.GIT