Bug 1188456 - Helper to deduplicate password manager logins. r=MattN
authorBernardo P. Rittmeyer <bernardo@rittme.com>
Thu, 30 Jul 2015 00:46:06 -0700
changeset 287266 d6b5e49c24b6cdd9f82f025a6050ec0dbec0af08
parent 287265 d73454e9c889faf3e9af400f9ee3694f2ad6425e
child 287267 bc77654068518194309441fd8560c6fb04ca6181
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1188456
milestone42.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1188456 - Helper to deduplicate password manager logins. r=MattN
toolkit/components/passwordmgr/LoginHelper.jsm
toolkit/components/passwordmgr/test/unit/head.js
toolkit/components/passwordmgr/test/unit/test_logins_change.js
--- a/toolkit/components/passwordmgr/LoginHelper.jsm
+++ b/toolkit/components/passwordmgr/LoginHelper.jsm
@@ -255,16 +255,53 @@ this.LoginHelper = {
 
     // Throws if there are bogus values.
     this.checkLoginValues(newLogin);
 
     return newLogin;
   },
 
   /**
+   * Removes duplicates from a list of logins.
+   *
+   * @param {nsILoginInfo[]} logins
+   *        A list of logins we want to deduplicate.
+   *
+   * @param {string[] = ["username", "password"]} uniqueKeys
+   *        A list of login attributes to use as unique keys for the deduplication.
+   *
+   * @returns {nsILoginInfo[]} list of unique logins.
+   */
+  dedupeLogins(logins, uniqueKeys = ["username", "password"]) {
+    const KEY_DELIMITER = ":";
+
+    // Generate a unique key string from a login.
+    function getKey(login, uniqueKeys) {
+      return uniqueKeys.reduce((prev, key) => prev + KEY_DELIMITER + login[key], "");
+    }
+
+    // We use a Map to easily lookup logins by their unique keys.
+    let loginsByKeys = new Map();
+    for (let login of logins) {
+      let key = getKey(login, uniqueKeys);
+      // If we find a more recently used login for the same key, replace the existing one.
+      if (loginsByKeys.has(key)) {
+        let loginDate = login.QueryInterface(Ci.nsILoginMetaInfo).timeLastUsed;
+        let storedLoginDate = loginsByKeys.get(key).QueryInterface(Ci.nsILoginMetaInfo).timeLastUsed;
+        if (loginDate < storedLoginDate) {
+          continue;
+        }
+      }
+      loginsByKeys.set(key, login);
+    }
+    // Return the map values in the form of an array.
+    return [...loginsByKeys.values()];
+  },
+
+  /**
    * Open the password manager window.
    *
    * @param {Window} window
    *                 the window from where we want to open the dialog
    *
    * @param {string} [filterString=""]
    *                 the filterString parameter to pass to the login manager dialog
    */
--- a/toolkit/components/passwordmgr/test/unit/head.js
+++ b/toolkit/components/passwordmgr/test/unit/head.js
@@ -12,16 +12,17 @@
 ////////////////////////////////////////////////////////////////////////////////
 //// Globals
 
 const { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/LoginRecipes.jsm");
+Cu.import("resource://gre/modules/LoginHelper.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "DownloadPaths",
                                   "resource://gre/modules/DownloadPaths.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "FileUtils",
                                   "resource://gre/modules/FileUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "OS",
                                   "resource://gre/modules/osfile.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Promise",
--- a/toolkit/components/passwordmgr/test/unit/test_logins_change.js
+++ b/toolkit/components/passwordmgr/test/unit/test_logins_change.js
@@ -47,16 +47,39 @@ function checkLoginInvalid(aLoginInfo, a
     passwordField: aLoginInfo.passwordField,
   })), aExpectedError);
 
   // Verify that no data was stored by the previous calls.
   LoginTestUtils.checkLogins([testLogin]);
   Services.logins.removeLogin(testLogin);
 }
 
+/**
+ * Verifies that two objects are not the same instance
+ * but have equal attributes.
+ *
+ * @param {Object} objectA
+ *        An object to compare.
+ *
+ * @param {Object} objectB
+ *        Another object to compare.
+ *
+ * @param {string[]} attributes
+ *        Attributes to compare.
+ *
+ * @return true if all passed attributes are equal for both objects, false otherwise.
+ */
+function compareAttributes(objectA, objectB, attributes) {
+  // If it's the same object, we want to return false.
+  if (objectA == objectB) {
+    return false;
+  }
+  return attributes.every(attr => objectA[attr] == objectB[attr]);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 //// Tests
 
 /**
  * Tests that adding logins to the database works.
  */
 add_task(function test_addLogin_removeLogin()
 {
@@ -290,8 +313,74 @@ add_task(function test_modifyLogin_nsIPr
   // Modifying a login to match an existing one should not be possible.
   Assert.throws(
          () => Services.logins.modifyLogin(loginInfo, differentLoginProperties),
          /already exists/);
   LoginTestUtils.checkLogins([loginInfo, differentLoginInfo]);
 
   LoginTestUtils.clearData();
 });
+
+/**
+ * Tests the login deduplication function.
+ */
+add_task(function test_deduplicate_logins() {
+  // Different key attributes combinations and the amount of unique
+  // results expected for the TestData login list.
+  let keyCombinations = [
+    {
+      keyset: ["username", "password"],
+      results: 13,
+    },
+    {
+      keyset: ["hostname", "username"],
+      results: 17,
+    },
+    {
+      keyset: ["hostname", "username", "password"],
+      results: 18,
+    },
+    {
+      keyset: ["hostname", "username", "password", "formSubmitURL"],
+      results: 22,
+    },
+  ];
+
+  let logins = TestData.loginList();
+
+  for (let testCase of keyCombinations) {
+    // Deduplicate the logins using the current testcase keyset.
+    let deduped = LoginHelper.dedupeLogins(logins, testCase.keyset);
+    Assert.equal(deduped.length, testCase.results, "Correct amount of results.");
+
+    // Checks that every login after deduping is unique.
+    Assert.ok(deduped.every(loginA =>
+      deduped.every(loginB => !compareAttributes(loginA, loginB, testCase.keyset))
+    ), "Every login is unique.");
+  }
+});
+
+/**
+ * Ensure that the login deduplication function keeps the most recent login.
+ */
+add_task(function test_deduplicate_keeps_most_recent() {
+  // Logins to deduplicate.
+  let logins = [
+    TestData.formLogin({timeLastUsed: Date.UTC(2004, 11, 4, 0, 0, 0)}),
+    TestData.formLogin({formSubmitURL: "http://example.com", timeLastUsed: Date.UTC(2015, 11, 4, 0, 0, 0)}),
+  ];
+
+  // Deduplicate the logins.
+  let deduped = LoginHelper.dedupeLogins(logins);
+  Assert.equal(deduped.length, 1, "Deduplicated the logins array.");
+
+  // Verify that the remaining login have the most recent date.
+  let loginTimeLastUsed = deduped[0].QueryInterface(Ci.nsILoginMetaInfo).timeLastUsed;
+  Assert.equal(loginTimeLastUsed, Date.UTC(2015, 11, 4, 0, 0, 0), "Most recent login was kept.");
+
+  // Deduplicate the reverse logins array.
+  deduped = LoginHelper.dedupeLogins(logins.reverse());
+  Assert.equal(deduped.length, 1, "Deduplicated the reversed logins array.");
+
+  // Verify that the remaining login have the most recent date.
+  loginTimeLastUsed = deduped[0].QueryInterface(Ci.nsILoginMetaInfo).timeLastUsed;
+  Assert.equal(loginTimeLastUsed, Date.UTC(2015, 11, 4, 0, 0, 0), "Most recent login was kept.");
+});