Bug 1365150 - profile.get returns null instead of throwing, .add returns the new GUID. r?lchang draft
authorMark Hammond <mhammond@skippinet.com.au>
Tue, 09 May 2017 15:38:49 +1000
changeset 578504 75f7f17ade163e9ce815b557de755eb7e5a2a5cf
parent 578175 3e166b6838931b3933ca274331f9e0e115af5cc0
child 578506 e80ca77b2c96271524d62a4377a9c2ba1f1ed367
child 579570 c5c6c2debbfe15cfb9271af3921256718985b960
push id58940
push userbmo:markh@mozilla.com
push dateTue, 16 May 2017 04:21:04 +0000
reviewerslchang
bugs1365150
milestone55.0a1
Bug 1365150 - profile.get returns null instead of throwing, .add returns the new GUID. r?lchang MozReview-Commit-ID: AHe231q6HpR
browser/extensions/formautofill/ProfileStorage.jsm
browser/extensions/formautofill/test/unit/test_addressRecords.js
browser/extensions/formautofill/test/unit/test_creditCardRecords.js
--- a/browser/extensions/formautofill/ProfileStorage.jsm
+++ b/browser/extensions/formautofill/ProfileStorage.jsm
@@ -164,16 +164,18 @@ class AutofillRecords {
     return SCHEMA_VERSION;
   }
 
   /**
    * Adds a new record.
    *
    * @param {Object} record
    *        The new record for saving.
+   * @returns {string}
+   *          The GUID of the newly added item..
    */
   add(record) {
     this.log.debug("add:", record);
 
     let recordToSave = this._clone(record);
     this._normalizeRecord(recordToSave);
 
     let guid;
@@ -189,16 +191,17 @@ class AutofillRecords {
     recordToSave.timeLastModified = now;
     recordToSave.timeLastUsed = 0;
     recordToSave.timesUsed = 0;
 
     this._store.data[this._collectionName].push(recordToSave);
     this._store.saveSoon();
 
     Services.obs.notifyObservers(null, "formautofill-storage-changed", "add");
+    return recordToSave.guid;
   }
 
   /**
    * Update the specified record.
    *
    * @param  {string} guid
    *         Indicates which record to update.
    * @param  {Object} record
@@ -275,17 +278,17 @@ class AutofillRecords {
    * @returns {Object}
    *          A clone of the record.
    */
   get(guid) {
     this.log.debug("get:", guid);
 
     let recordFound = this._findByGUID(guid);
     if (!recordFound) {
-      throw new Error("No matching record.");
+      return null;
     }
 
     // The record is cloned to avoid accidental modifications from outside.
     let clonedRecord = this._clone(recordFound);
     this._recordReadProcessor(clonedRecord);
     return clonedRecord;
   }
 
--- a/browser/extensions/formautofill/test/unit/test_addressRecords.js
+++ b/browser/extensions/formautofill/test/unit/test_addressRecords.js
@@ -38,19 +38,19 @@ const TEST_ADDRESS_WITH_INVALID_FIELD = 
 };
 
 let prepareTestRecords = async function(path) {
   let profileStorage = new ProfileStorage(path);
   await profileStorage.initialize();
 
   let onChanged = TestUtils.topicObserved("formautofill-storage-changed",
                                           (subject, data) => data == "add");
-  profileStorage.addresses.add(TEST_ADDRESS_1);
+  do_check_true(profileStorage.addresses.add(TEST_ADDRESS_1));
   await onChanged;
-  profileStorage.addresses.add(TEST_ADDRESS_2);
+  do_check_true(profileStorage.addresses.add(TEST_ADDRESS_2));
   await profileStorage._saveImmediately();
 };
 
 let do_check_record_matches = (recordWithMeta, record) => {
   for (let key in record) {
     do_check_eq(recordWithMeta[key], record[key]);
   }
 };
@@ -115,18 +115,17 @@ add_task(async function test_get() {
 
   let address = profileStorage.addresses.get(guid);
   do_check_record_matches(address, TEST_ADDRESS_1);
 
   // Modifying output shouldn't affect the storage.
   address.organization = "test";
   do_check_record_matches(profileStorage.addresses.get(guid), TEST_ADDRESS_1);
 
-  Assert.throws(() => profileStorage.addresses.get("INVALID_GUID"),
-    /No matching record\./);
+  do_check_eq(profileStorage.addresses.get("INVALID_GUID"), null);
 });
 
 add_task(async function test_getByFilter() {
   let path = getTempFile(TEST_STORE_FILE_NAME).path;
   await prepareTestRecords(path);
 
   let profileStorage = new ProfileStorage(path);
   await profileStorage.initialize();
@@ -277,10 +276,10 @@ add_task(async function test_remove() {
 
   profileStorage = new ProfileStorage(path);
   await profileStorage.initialize();
 
   addresses = profileStorage.addresses.getAll();
 
   do_check_eq(addresses.length, 1);
 
-  Assert.throws(() => profileStorage.addresses.get(guid), /No matching record\./);
+  do_check_eq(profileStorage.addresses.get(guid), null);
 });
--- a/browser/extensions/formautofill/test/unit/test_creditCardRecords.js
+++ b/browser/extensions/formautofill/test/unit/test_creditCardRecords.js
@@ -57,19 +57,19 @@ const TEST_CREDIT_CARD_WITH_INVALID_NUMB
 };
 
 let prepareTestCreditCards = async function(path) {
   let profileStorage = new ProfileStorage(path);
   await profileStorage.initialize();
 
   let onChanged = TestUtils.topicObserved("formautofill-storage-changed",
                                           (subject, data) => data == "add");
-  profileStorage.creditCards.add(TEST_CREDIT_CARD_1);
+  do_check_true(profileStorage.creditCards.add(TEST_CREDIT_CARD_1));
   await onChanged;
-  profileStorage.creditCards.add(TEST_CREDIT_CARD_2);
+  do_check_true(profileStorage.creditCards.add(TEST_CREDIT_CARD_2));
   await profileStorage._saveImmediately();
 };
 
 let reCCNumber = /^(\*+)(.{4})$/;
 
 let do_check_credit_card_matches = (creditCardWithMeta, creditCard) => {
   for (let key in creditCard) {
     if (key == "cc-number") {
@@ -145,18 +145,17 @@ add_task(async function test_get() {
 
   let creditCard = profileStorage.creditCards.get(guid);
   do_check_credit_card_matches(creditCard, TEST_CREDIT_CARD_1);
 
   // Modifying output shouldn't affect the storage.
   creditCards[0]["cc-name"] = "test";
   do_check_credit_card_matches(profileStorage.creditCards.get(guid), TEST_CREDIT_CARD_1);
 
-  Assert.throws(() => profileStorage.creditCards.get("INVALID_GUID"),
-    /No matching record\./);
+  do_check_eq(profileStorage.creditCards.get("INVALID_GUID"), null);
 });
 
 add_task(async function test_getByFilter() {
   let path = getTempFile(TEST_STORE_FILE_NAME).path;
   await prepareTestCreditCards(path);
 
   let profileStorage = new ProfileStorage(path);
   await profileStorage.initialize();
@@ -316,10 +315,10 @@ add_task(async function test_remove() {
 
   profileStorage = new ProfileStorage(path);
   await profileStorage.initialize();
 
   creditCards = profileStorage.creditCards.getAll();
 
   do_check_eq(creditCards.length, 1);
 
-  Assert.throws(() => profileStorage.creditCards.get(guid), /No matching record\./);
+  do_check_eq(profileStorage.creditCards.get(guid), null);
 });