Bug 1590021 - Fix conversion of empty address book and don't overwrite existing backup files. r=mkmelin
authorGeoff Lankow <geoff@darktrojan.net>
Mon, 21 Oct 2019 21:31:08 +1300
changeset 37199 f5a320f3ebce383190418b463937d6b0e4f57a00
parent 37198 6a279db8f7370513cd9853ece3d8cb7ce00b62e0
child 37200 caa4039d835ac06a1e65a1b23f6951c92a7e3c73
push id395
push userclokep@gmail.com
push dateMon, 02 Dec 2019 19:38:57 +0000
reviewersmkmelin
bugs1590021
Bug 1590021 - Fix conversion of empty address book and don't overwrite existing backup files. r=mkmelin
mail/base/modules/MailMigrator.jsm
mailnews/addrbook/test/unit/data/empty.mab
mailnews/addrbook/test/unit/test_migration3.js
mailnews/addrbook/test/unit/test_migration4.js
mailnews/addrbook/test/unit/test_migration5.js
mailnews/addrbook/test/unit/xpcshell-jsaddrbook.ini
--- a/mail/base/modules/MailMigrator.jsm
+++ b/mail/base/modules/MailMigrator.jsm
@@ -713,43 +713,49 @@ var MailMigrator = {
       ].createInstance(Ci.nsIAbMDBDirectory);
 
       let cardMap = new Map();
       for (let card of database.enumerateCards(directory)) {
         if (!card.isMailList) {
           cardMap.set(card.localId, card);
         }
       }
-      await newBook._bulkAddCards(cardMap.values());
+      if (cardMap.size > 0) {
+        await newBook._bulkAddCards(cardMap.values());
 
-      for (let card of database.enumerateCards(directory)) {
-        if (card.isMailList) {
-          let mailList = Cc[
-            "@mozilla.org/addressbook/directoryproperty;1"
-          ].createInstance(Ci.nsIAbDirectory);
-          mailList.isMailList = true;
-          mailList.dirName = card.displayName;
-          mailList.listNickName = card.getProperty("NickName", "");
-          mailList.description = card.getProperty("Notes", "");
-          mailList = newBook.addMailList(mailList);
+        for (let card of database.enumerateCards(directory)) {
+          if (card.isMailList) {
+            let mailList = Cc[
+              "@mozilla.org/addressbook/directoryproperty;1"
+            ].createInstance(Ci.nsIAbDirectory);
+            mailList.isMailList = true;
+            mailList.dirName = card.displayName;
+            mailList.listNickName = card.getProperty("NickName", "");
+            mailList.description = card.getProperty("Notes", "");
+            mailList = newBook.addMailList(mailList);
 
-          directory.dbRowID = card.localId;
-          for (let listCard of database.enumerateListAddresses(directory)) {
-            mailList.addCard(
-              cardMap.get(listCard.QueryInterface(Ci.nsIAbCard).localId)
-            );
+            directory.dbRowID = card.localId;
+            for (let listCard of database.enumerateListAddresses(directory)) {
+              listCard.QueryInterface(Ci.nsIAbCard);
+              if (cardMap.has(listCard.localId)) {
+                mailList.addCard(cardMap.get(listCard.localId));
+              }
+            }
           }
         }
       }
 
       database.closeMDB(false);
       database.forceClosed();
 
-      console.log(`Renaming ${fileName}.mab to ${fileName}.mab.bak`);
-      oldFile.renameTo(profileDir, `${fileName}.mab.bak`);
+      let backupFile = profileDir.clone();
+      backupFile.append(`${fileName}.mab.bak`);
+      backupFile.createUnique(Ci.nsIFile.NORMAL_FILE_TYPE, 0o644);
+      console.log(`Renaming ${fileName}.mab to ${backupFile.leafName}`);
+      oldFile.renameTo(profileDir, backupFile.leafName);
     }
 
     let profileDir = Services.dirsvc.get("ProfD", Ci.nsIFile);
     for (let name of Services.prefs.getChildList("ldap_2.servers.")) {
       try {
         if (
           !name.endsWith(".dirType") ||
           Services.prefs.getIntPref(name) != 2
new file mode 100644
--- /dev/null
+++ b/mailnews/addrbook/test/unit/data/empty.mab
@@ -0,0 +1,27 @@
+// <!-- <mdb:mork:z v="1.4"/> -->
+< <(a=c)> // (f=iso-8859-1)
+  (B8=Notes)(B9=LastModifiedDate)(BA=RecordKey)(BB=AddrCharSet)
+  (BC=LastRecordKey)(BD=ns:addrbk:db:table:kind:pab)(BE=ListName)
+  (BF=ListNickName)(C0=ListDescription)(C1=ListTotalAddresses)
+  (C2=LowercaseListName)(C3=ns:addrbk:db:table:kind:deleted)
+  (80=ns:addrbk:db:row:scope:card:all)
+  (81=ns:addrbk:db:row:scope:list:all)
+  (82=ns:addrbk:db:row:scope:data:all)(83=UID)(84=FirstName)(85=LastName)
+  (86=PhoneticFirstName)(87=PhoneticLastName)(88=DisplayName)
+  (89=NickName)(8A=PrimaryEmail)(8B=LowercasePrimaryEmail)
+  (8C=SecondEmail)(8D=LowercaseSecondEmail)(8E=PreferMailFormat)
+  (8F=PopularityIndex)(90=WorkPhone)(91=HomePhone)(92=FaxNumber)
+  (93=PagerNumber)(94=CellularNumber)(95=WorkPhoneType)(96=HomePhoneType)
+  (97=FaxNumberType)(98=PagerNumberType)(99=CellularNumberType)
+  (9A=HomeAddress)(9B=HomeAddress2)(9C=HomeCity)(9D=HomeState)
+  (9E=HomeZipCode)(9F=HomeCountry)(A0=WorkAddress)(A1=WorkAddress2)
+  (A2=WorkCity)(A3=WorkState)(A4=WorkZipCode)(A5=WorkCountry)
+  (A6=JobTitle)(A7=Department)(A8=Company)(A9=_AimScreenName)
+  (AA=AnniversaryYear)(AB=AnniversaryMonth)(AC=AnniversaryDay)
+  (AD=SpouseName)(AE=FamilyName)(AF=WebPage1)(B0=WebPage2)(B1=BirthYear)
+  (B2=BirthMonth)(B3=BirthDay)(B4=Custom1)(B5=Custom2)(B6=Custom3)
+  (B7=Custom4)>
+
+<(80=0)>
+{1:^80 {(k^BD:c)(s=9)} 
+  [1:^82(^BC=0)]}
--- a/mailnews/addrbook/test/unit/test_migration3.js
+++ b/mailnews/addrbook/test/unit/test_migration3.js
@@ -4,20 +4,16 @@
 
 /*
  * Test auto-migration from Mork address books to JS/SQLite address books.
  *
  * This test profile has a non-default Mork address book, and no default
  * address books.
  */
 
-const { fixIterator } = ChromeUtils.import(
-  "resource:///modules/iteratorUtils.jsm"
-);
-
 add_task(async function() {
   // Copy address book to be migrated into the profile.
 
   copyABFile("data/existing.mab", "test.mab");
 
   Services.prefs.setStringPref(
     "ldap_2.servers.Test.description",
     "This is a test!"
new file mode 100644
--- /dev/null
+++ b/mailnews/addrbook/test/unit/test_migration4.js
@@ -0,0 +1,56 @@
+/* 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/. */
+
+/*
+ * Test auto-migration from Mork address books to JS/SQLite address books.
+ *
+ * This test profile has an empty (no entries, not zero-length) history book.
+ * Migration should not fail as seen in bug 1590021.
+ */
+
+add_task(async function() {
+  // Copy address book to be migrated into the profile.
+
+  copyABFile("data/empty.mab", "history.mab");
+
+  // Do the migration.
+
+  await MailMigrator._migrateAddressBooks();
+
+  // Check new files have been created, and old ones renamed.
+
+  checkFileExists("abook.sqlite", false);
+  checkFileExists("abook.mab", false);
+  checkFileExists("abook.mab.bak", false);
+  checkFileExists("history.sqlite", true);
+  checkFileExists("history.mab", false);
+  checkFileExists("history.mab.bak", true);
+
+  // Check that the default preferences are untouched.
+
+  equal(Services.prefs.getIntPref("ldap_2.servers.pab.dirType"), 101);
+  equal(
+    Services.prefs.getStringPref("ldap_2.servers.pab.filename"),
+    "abook.sqlite"
+  );
+  equal(Services.prefs.getIntPref("ldap_2.servers.history.dirType"), 101);
+  equal(
+    Services.prefs.getStringPref("ldap_2.servers.history.filename"),
+    "history.sqlite"
+  );
+
+  // Check the new address books.
+
+  let directories = [...MailServices.ab.directories];
+  equal(directories.length, 2);
+  equal(directories[0].dirType, 101);
+  equal(directories[1].dirType, 101);
+
+  let [, historyBook] = directories;
+
+  // Check we have all the right cards.
+
+  let testCards = [...historyBook.childCards];
+  equal(testCards.length, 0);
+});
new file mode 100644
--- /dev/null
+++ b/mailnews/addrbook/test/unit/test_migration5.js
@@ -0,0 +1,37 @@
+/* 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/. */
+
+/*
+ * Test auto-migration from Mork address books to JS/SQLite address books.
+ *
+ * This test profile has an existing default address book, and a file that
+ * already exists named abook.mab.bak. After migration the Mork file should
+ * be named something other than abook.mab or abook.mab.bak.
+ */
+add_task(async function() {
+  // Copy address books to be migrated into the profile.
+
+  copyABFile("data/cardForEmail.mab", "abook.mab");
+  let existingBackupFile = profileDir.clone();
+  existingBackupFile.append("abook.mab.bak");
+  existingBackupFile.create(Ci.nsIFile.NORMAL_FILE_TYPE, 0o644);
+
+  // Do the migration.
+
+  await MailMigrator._migrateAddressBooks();
+
+  // Check new files have been created, and old ones renamed.
+
+  checkFileExists("abook.sqlite", true);
+  checkFileExists("abook.mab", false);
+  checkFileExists("abook.mab.bak", true);
+
+  equal(existingBackupFile.fileSize, 0);
+
+  let profileFiles = [...profileDir.directoryEntries].map(
+    file => file.leafName
+  );
+  info("Files in profile: " + profileFiles.join(", "));
+  ok(profileFiles.includes("abook.mab-1.bak"));
+});
--- a/mailnews/addrbook/test/unit/xpcshell-jsaddrbook.ini
+++ b/mailnews/addrbook/test/unit/xpcshell-jsaddrbook.ini
@@ -7,8 +7,12 @@ tags = jsaddrbook
 
 [include:xpcshell.ini]
 [test_migration1.js]
 head = head_migration.js
 [test_migration2.js]
 head = head_migration.js
 [test_migration3.js]
 head = head_migration.js
+[test_migration4.js]
+head = head_migration.js
+[test_migration5.js]
+head = head_migration.js