Bug 990219 - Part 2: Add merge function in storage.
MozReview-Commit-ID: AWYsnzmVJAY
--- a/browser/extensions/formautofill/ProfileStorage.jsm
+++ b/browser/extensions/formautofill/ProfileStorage.jsm
@@ -166,18 +166,22 @@ class AutofillRecords {
/**
* Adds a new record.
*
* @param {Object} record
* The new record for saving.
*/
add(record) {
+ // Try to merge to existing record before adding new record
+ if (this._mergeToStorage(record)) {
+ return;
+ }
+
this.log.debug("add:", record);
-
let recordToSave = this._clone(record);
this._normalizeRecord(recordToSave);
let guid;
while (!guid || this._findByGUID(guid)) {
guid = gUUIDGenerator.generateUUID().toString()
.replace(/[{}-]/g, "").substring(0, 12);
}
@@ -263,16 +267,67 @@ class AutofillRecords {
this._store.data[this._collectionName] =
this._store.data[this._collectionName].filter(record => record.guid != guid);
this._store.saveSoon();
Services.obs.notifyObservers(null, "formautofill-storage-changed", "remove");
}
/**
+ * Merge new record into the specified record if mergeable.
+ *
+ * @param {string} guid
+ * Indicates which record to merge.
+ * @param {Object} record
+ * The new record used to merge into the old one.
+ * @returns {boolean}
+ * Return true if record is merged into target with specific guid or false if not.
+ */
+ mergeIfPossible(guid, record) {
+ this.log.debug("merge:", guid, record);
+
+ let recordFound = this._findByGUID(guid);
+ if (!recordFound) {
+ throw new Error("No matching record.");
+ }
+
+ let recordToMerge = this._clone(record);
+ this._normalizeRecord(recordToMerge);
+ let hasMatchingField = false;
+
+ for (let field of this.VALID_FIELDS) {
+ if (recordToMerge[field] !== undefined && recordFound[field] !== undefined) {
+ if (recordToMerge[field] != recordFound[field]) {
+ this.log.debug("Conflicts: field", field, "has different value.");
+ return false;
+ }
+ hasMatchingField = true;
+ }
+ }
+
+ // We merge the record only when at least one field has the same value.
+ if (!hasMatchingField) {
+ this.log.debug("Unable to merge because there's no field has same value");
+ return false;
+ }
+
+ for (let field in recordToMerge) {
+ if (this.VALID_FIELDS.includes(field)) {
+ recordFound[field] = recordToMerge[field];
+ }
+ }
+
+ recordFound.timeLastModified = Date.now();
+
+ this._store.saveSoon();
+ Services.obs.notifyObservers(null, "formautofill-storage-changed", "merge");
+ return true;
+ }
+
+ /**
* Returns the record with the specified GUID.
*
* @param {string} guid
* Indicates which record to retrieve.
* @returns {Object}
* A clone of the record.
*/
get(guid) {
@@ -357,16 +412,33 @@ class AutofillRecords {
}
if (typeof record[key] !== "string" &&
typeof record[key] !== "number") {
throw new Error(`"${key}" contains invalid data type.`);
}
}
}
+ /**
+ * Merge the record if storage has a mergeable record.
+ * @param {Object} targetRecord
+ * The record for merge.
+ * @returns {boolean}
+ * Return true if the target record is mergeable or false if not.
+ */
+ _mergeToStorage(targetRecord) {
+ for (let record of this._store.data[this._collectionName]) {
+ if (this.mergeIfPossible(record.guid, targetRecord)) {
+ return true;
+ }
+ }
+ this.log.debug("Unable to merge into storage");
+ return false;
+ }
+
// An interface to be inherited.
_recordReadProcessor(record, config) {}
// An interface to be inherited.
_recordWriteProcessor(record) {}
}
class Addresses extends AutofillRecords {
--- a/browser/extensions/formautofill/test/unit/test_addressRecords.js
+++ b/browser/extensions/formautofill/test/unit/test_addressRecords.js
@@ -27,16 +27,23 @@ const TEST_ADDRESS_2 = {
country: "US",
};
const TEST_ADDRESS_3 = {
"street-address": "Other Address",
"postal-code": "12345",
};
+const TEST_ADDRESS_4 = {
+ "given-name": "Timothy",
+ "additional-name": "John",
+ "family-name": "Berners-Lee",
+ organization: "World Wide Web Consortium",
+};
+
const TEST_ADDRESS_WITH_INVALID_FIELD = {
"street-address": "Another Address",
invalidField: "INVALID",
};
let prepareTestRecords = async function(path) {
let profileStorage = new ProfileStorage(path);
await profileStorage.initialize();
@@ -178,16 +185,23 @@ add_task(async function test_add() {
do_check_neq(addresses[0].guid, undefined);
do_check_neq(addresses[0].timeCreated, undefined);
do_check_eq(addresses[0].timeLastModified, addresses[0].timeCreated);
do_check_eq(addresses[0].timeLastUsed, 0);
do_check_eq(addresses[0].timesUsed, 0);
Assert.throws(() => profileStorage.addresses.add(TEST_ADDRESS_WITH_INVALID_FIELD),
/"invalidField" is not a valid field\./);
+
+ // Saving a mergeable address will not add a new address.
+ let timeLastModified = addresses[0].timeLastModified;
+ profileStorage.addresses.add(TEST_ADDRESS_4);
+ addresses = profileStorage.addresses.getAll();
+ do_check_eq(addresses.length, 2);
+ do_check_neq(addresses[0].timeLastModified, timeLastModified);
});
add_task(async function test_update() {
let path = getTempFile(TEST_STORE_FILE_NAME).path;
await prepareTestRecords(path);
let profileStorage = new ProfileStorage(path);
await profileStorage.initialize();
@@ -279,8 +293,46 @@ add_task(async function test_remove() {
await profileStorage.initialize();
addresses = profileStorage.addresses.getAll();
do_check_eq(addresses.length, 1);
Assert.throws(() => profileStorage.addresses.get(guid), /No matching record\./);
});
+
+add_task(async function test_merge() {
+ let path = getTempFile(TEST_STORE_FILE_NAME).path;
+ await prepareTestRecords(path);
+
+ let profileStorage = new ProfileStorage(path);
+ await profileStorage.initialize();
+
+ let addresses = profileStorage.addresses.getAll();
+
+ do_check_eq(addresses.length, 2);
+
+ // Merge a superset
+ let superset = profileStorage.addresses._clone(TEST_ADDRESS_2);
+ superset.tel = "123456";
+ do_check_eq(profileStorage.addresses.mergeIfPossible(addresses[1].guid, superset), true);
+ do_check_eq(profileStorage.addresses.getAll()[1].tel, "123456");
+
+ // Merge a subset
+ let subset = profileStorage.addresses._clone(TEST_ADDRESS_1);
+ delete subset.tel;
+ do_check_eq(profileStorage.addresses.mergeIfPossible(addresses[0].guid, subset), true);
+ do_check_eq(profileStorage.addresses.getAll()[0].tel, TEST_ADDRESS_1.tel);
+
+ // Merge an address with partial overlaps
+ let overlap = profileStorage.addresses._clone(TEST_ADDRESS_2);
+ delete overlap.country;
+ overlap["postal-code"] = "12345";
+ do_check_eq(profileStorage.addresses.mergeIfPossible(addresses[1].guid, overlap), true);
+ do_check_eq(profileStorage.addresses.getAll()[1].country, TEST_ADDRESS_2.country);
+ do_check_eq(profileStorage.addresses.getAll()[1]["postal-code"], "12345");
+
+ // Unable to merge because of conflict
+ do_check_eq(profileStorage.addresses.mergeIfPossible(addresses[1].guid, TEST_ADDRESS_3), false);
+
+ // Unable to merge because no overlap
+ do_check_eq(profileStorage.addresses.mergeIfPossible(addresses[1].guid, TEST_ADDRESS_4), false);
+});
--- a/browser/extensions/formautofill/test/unit/test_creditCardRecords.js
+++ b/browser/extensions/formautofill/test/unit/test_creditCardRecords.js
@@ -35,24 +35,24 @@ const TEST_CREDIT_CARD_WITH_2_DIGITS_YEA
};
const TEST_CREDIT_CARD_WITH_INVALID_FIELD = {
"cc-name": "John Doe",
invalidField: "INVALID",
};
const TEST_CREDIT_CARD_WITH_INVALID_EXPIRY_DATE = {
- "cc-name": "John Doe",
+ "cc-name": "John Doe Sr.",
"cc-number": "1111222233334444",
"cc-exp-month": 13,
"cc-exp-year": -3,
};
const TEST_CREDIT_CARD_WITH_SPACES_BETWEEN_DIGITS = {
- "cc-name": "John Doe",
+ "cc-name": "John Doe Jr.",
"cc-number": "1111 2222 3333 4444",
};
const TEST_CREDIT_CARD_WITH_INVALID_NUMBERS = {
"cc-name": "John Doe",
"cc-number": "abcdefg",
};
--- a/browser/extensions/formautofill/test/unit/test_transformFields.js
+++ b/browser/extensions/formautofill/test/unit/test_transformFields.js
@@ -106,22 +106,22 @@ const ADDRESS_NORMALIZE_TESTCASES = [
},
},
{
description: "Has both \"name\" and split names",
address: {
"name": "John Doe",
"given-name": "Timothy",
"additional-name": "John",
- "family-name": "Berners-Lee",
+ "family-name": "Berners",
},
expectedResult: {
"given-name": "Timothy",
"additional-name": "John",
- "family-name": "Berners-Lee",
+ "family-name": "Berners",
},
},
{
description: "Has \"name\", and some of split names are omitted",
address: {
"name": "John Doe",
"given-name": "Timothy",
},