Bug 827048 - Fix pop3 move filter and test_pop3MoveFilter.js for maildir, r=rkent
authorSuyash Agarwal <syshagarwal@gmail.com>
Fri, 04 Jul 2014 14:08:55 -0700
changeset 16446 4dc251e085621efc6944f08231c320361ba7abba
parent 16445 ab9ae24bbdee7298395789a44896a7aed57f5a38
child 16447 e2f581a3f07d2320c57bed3f737b089d8eeac9fd
push id10242
push userkent@caspia.com
push dateFri, 04 Jul 2014 21:21:42 +0000
treeherdercomm-central@bdf3156f6c1c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrkent
bugs827048
Bug 827048 - Fix pop3 move filter and test_pop3MoveFilter.js for maildir, r=rkent
mailnews/base/search/src/nsMsgFilterService.cpp
mailnews/local/src/nsMsgMaildirStore.cpp
mailnews/local/src/nsParseMailbox.cpp
mailnews/local/test/unit/test_pop3MoveFilter.js
mailnews/test/resources/POP3pump.js
mailnews/test/resources/localAccountUtils.js
--- a/mailnews/base/search/src/nsMsgFilterService.cpp
+++ b/mailnews/base/search/src/nsMsgFilterService.cpp
@@ -26,16 +26,17 @@
 #include "nsMsgBaseCID.h"
 #include "nsIMsgCopyService.h"
 #include "nsIOutputStream.h"
 #include "nsIMsgComposeService.h"
 #include "nsMsgCompCID.h"
 #include "nsNetUtil.h"
 #include "nsMsgUtils.h"
 #include "nsIMutableArray.h"
+#include "nsIMsgMailSession.h"
 #include "nsArrayUtils.h"
 #include "nsCOMArray.h"
 #include "nsIMsgFilterCustomAction.h"
 #include "nsArrayEnumerator.h"
 #include "nsMsgMessageFlags.h"
 #include "nsIMsgWindow.h"
 #include "nsIMsgSearchCustomTerm.h"
 #include "nsIMsgSearchTerm.h"
@@ -204,20 +205,27 @@ nsMsgFilterService::GetFilterStringBundl
   return NS_OK;
 }
 
 nsresult
 nsMsgFilterService::ThrowAlertMsg(const char*aMsgName, nsIMsgWindow *aMsgWindow)
 {
   nsString alertString;
   nsresult rv = GetStringFromBundle(aMsgName, getter_Copies(alertString));
-  if (NS_SUCCEEDED(rv) && !alertString.IsEmpty() && aMsgWindow)
+  nsCOMPtr<nsIMsgWindow> msgWindow(do_QueryInterface(aMsgWindow));
+  if (!msgWindow) {
+    nsCOMPtr<nsIMsgMailSession> mailSession ( do_GetService(NS_MSGMAILSESSION_CONTRACTID, &rv));
+    if (NS_SUCCEEDED(rv))
+      rv = mailSession->GetTopmostMsgWindow(getter_AddRefs(msgWindow));
+  }
+
+  if (NS_SUCCEEDED(rv) && !alertString.IsEmpty() && msgWindow)
   {
     nsCOMPtr <nsIDocShell> docShell;
-    aMsgWindow->GetRootDocShell(getter_AddRefs(docShell));
+    msgWindow->GetRootDocShell(getter_AddRefs(docShell));
     if (docShell)
     {
       nsCOMPtr<nsIPrompt> dialog(do_GetInterface(docShell));
       if (dialog && !alertString.IsEmpty())
         dialog->Alert(nullptr, alertString.get());
     }
   }
   return rv;
--- a/mailnews/local/src/nsMsgMaildirStore.cpp
+++ b/mailnews/local/src/nsMsgMaildirStore.cpp
@@ -28,16 +28,17 @@
 #include "nsMailHeaders.h"
 #include "nsParseMailbox.h"
 #include "nsIMailboxService.h"
 #include "nsMsgLocalCID.h"
 #include "nsIMsgLocalMailFolder.h"
 #include "nsITimer.h"
 #include "nsIMailboxUrl.h"
 #include "nsIMsgMailNewsUrl.h"
+#include "nsIMsgFilterPlugin.h"
 #include "nsLocalUndoTxn.h"
 #include "nsIMessenger.h"
 
 static PRLogModuleInfo* MailDirLog;
 
 nsMsgMaildirStore::nsMsgMaildirStore()
 {
   MailDirLog = PR_NewLogModule("MailDirStore");
@@ -698,66 +699,72 @@ nsMsgMaildirStore::FinishNewMessage(nsIO
   nsAutoCString fileName;
   aNewHdr->GetStringProperty("storeToken", getter_Copies(fileName));
   if (fileName.IsEmpty())
   {
     NS_ERROR("FinishNewMessage - no storeToken in msg hdr!!\n");
     return NS_ERROR_FAILURE;
   }
 
+  // path to the new destination
+  nsCOMPtr<nsIFile> toPath;
+  folderPath->Clone(getter_AddRefs(toPath));
+  toPath->Append(NS_LITERAL_STRING("cur"));
+
+  // let's check if the folder exists
+  bool exists;
+  toPath->Exists(&exists);
+  if (!exists)
+  {
+    rv = toPath->Create(nsIFile::DIRECTORY_TYPE, 0755);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
   // path to the downloaded message
   nsCOMPtr<nsIFile> fromPath;
   folderPath->Clone(getter_AddRefs(fromPath));
   fromPath->Append(NS_LITERAL_STRING("tmp"));
   fromPath->AppendNative(fileName);
 
   // let's check if the tmp file exists
-  bool exists;
   fromPath->Exists(&exists);
   if (!exists)
   {
-    NS_ERROR("FinishNewMessage - oops! file does not exist!");
-    return NS_ERROR_FAILURE;
-  }
+    // Perhaps the message has already moved. See bug 1028372 to fix this.
+    toPath->AppendNative(fileName);
+    toPath->Exists(&exists);
+    if (exists) // then there is nothing to do
+      return NS_OK;
 
-  // move to the "cur" subfolder
-  nsCOMPtr<nsIFile> toPath;
-  folderPath->Clone(getter_AddRefs(toPath));
-  toPath->Append(NS_LITERAL_STRING("cur"));
-
-  // let's check if the folder exists
-  toPath->Exists(&exists);
-  if (!exists)
-  {
-    rv = toPath->Create(nsIFile::DIRECTORY_TYPE, 0755);
-    NS_ENSURE_SUCCESS(rv, rv);
+    NS_ERROR("FinishNewMessage - oops! file does not exist!");
+    return NS_ERROR_FILE_TARGET_DOES_NOT_EXIST;
   }
 
   return fromPath->MoveToNative(toPath, fileName);
 }
 
 NS_IMETHODIMP
-nsMsgMaildirStore::MoveNewlyDownloadedMessage(nsIMsgDBHdr *aNewHdr,
+nsMsgMaildirStore::MoveNewlyDownloadedMessage(nsIMsgDBHdr *aHdr,
                                               nsIMsgFolder *aDestFolder,
                                               bool *aResult)
 {
-  NS_ENSURE_ARG_POINTER(aNewHdr);
+  NS_ENSURE_ARG_POINTER(aHdr);
   NS_ENSURE_ARG_POINTER(aDestFolder);
   NS_ENSURE_ARG_POINTER(aResult);
 
   nsCOMPtr<nsIFile> folderPath;
   nsCOMPtr<nsIMsgFolder> folder;
-  nsresult rv = aNewHdr->GetFolder(getter_AddRefs(folder));
+  nsresult rv = aHdr->GetFolder(getter_AddRefs(folder));
   NS_ENSURE_SUCCESS(rv, rv);
   rv = folder->GetFilePath(getter_AddRefs(folderPath));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // file path is stored in message header property
   nsAutoCString fileName;
-  aNewHdr->GetStringProperty("storeToken", getter_Copies(fileName));
+  aHdr->GetStringProperty("storeToken", getter_Copies(fileName));
   if (fileName.IsEmpty())
   {
     NS_ERROR("FinishNewMessage - no storeToken in msg hdr!!\n");
     return NS_ERROR_FAILURE;
   }
 
   // path to the downloaded message
   nsCOMPtr<nsIFile> fromPath;
@@ -783,18 +790,79 @@ nsMsgMaildirStore::MoveNewlyDownloadedMe
   // let's check if the folder exists
   toPath->Exists(&exists);
   if (!exists)
   {
     rv = toPath->Create(nsIFile::DIRECTORY_TYPE, 0755);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
+  nsCOMPtr<nsIMsgDatabase> destMailDB;
+  rv = aDestFolder->GetMsgDatabase(getter_AddRefs(destMailDB));
+  NS_WARN_IF_FALSE(destMailDB && NS_SUCCEEDED(rv),
+                   "failed to open mail db moving message");
+
+  nsCOMPtr<nsIMsgDBHdr> newHdr;
+  if (destMailDB)
+    rv = destMailDB->CopyHdrFromExistingHdr(nsMsgKey_None, aHdr, true,
+                                            getter_AddRefs(newHdr));
+  if (NS_SUCCEEDED(rv) && !newHdr)
+    rv = NS_ERROR_UNEXPECTED;
+
+  if (NS_FAILED(rv))
+    aDestFolder->ThrowAlertMsg("filterFolderHdrAddFailed", nullptr);
+
   rv = fromPath->MoveToNative(toPath, fileName);
   *aResult = NS_SUCCEEDED(rv);
+  if (NS_FAILED(rv))
+    aDestFolder->ThrowAlertMsg("filterFolderWriteFailed", nullptr);
+
+  if (NS_FAILED(rv)) {
+    if (destMailDB)
+      destMailDB->Close(true);
+
+    return NS_MSG_ERROR_WRITING_MAIL_FOLDER;
+  }
+
+  bool movedMsgIsNew = false;
+  // if we have made it this far then the message has successfully been
+  // written to the new folder now add the header to the destMailDB.
+
+  uint32_t newFlags;
+  newHdr->GetFlags(&newFlags);
+  nsMsgKey msgKey;
+  newHdr->GetMessageKey(&msgKey);
+  if (!(newFlags & nsMsgMessageFlags::Read))
+  {
+    nsCString junkScoreStr;
+    (void) newHdr->GetStringProperty("junkscore", getter_Copies(junkScoreStr));
+    if (atoi(junkScoreStr.get()) != nsIJunkMailPlugin::IS_SPAM_SCORE) {
+      newHdr->OrFlags(nsMsgMessageFlags::New, &newFlags);
+      destMailDB->AddToNewList(msgKey);
+      movedMsgIsNew = true;
+    }
+  }
+
+  nsCOMPtr<nsIMsgFolderNotificationService> notifier(
+    do_GetService(NS_MSGNOTIFICATIONSERVICE_CONTRACTID));
+  if (notifier)
+    notifier->NotifyMsgAdded(newHdr);
+
+  if (movedMsgIsNew)
+    aDestFolder->SetHasNewMessages(true);
+
+  nsCOMPtr<nsIMsgDatabase> sourceDB;
+  rv = folder->GetMsgDatabase(getter_AddRefs(sourceDB));
+
+  if (NS_SUCCEEDED(rv) && sourceDB)
+    sourceDB->RemoveHeaderMdbRow(aHdr);
+
+  destMailDB->SetSummaryValid(true);
+  aDestFolder->UpdateSummaryTotals(true);
+  destMailDB->Commit(nsMsgDBCommitType::kLargeCommit);
   return rv;
 }
 
 NS_IMETHODIMP
 nsMsgMaildirStore::GetMsgInputStream(nsIMsgFolder *aMsgFolder,
                                      const nsACString &aMsgToken,
                                      int64_t *aOffset,
                                      nsIMsgDBHdr *aMsgHdr,
--- a/mailnews/local/src/nsParseMailbox.cpp
+++ b/mailnews/local/src/nsParseMailbox.cpp
@@ -1827,31 +1827,29 @@ int32_t nsParseNewMailState::PublishMsgH
               }
                 m_mailDB->RemoveHeaderMdbRow(m_newMsgHdr);
               }
               break;
             case nsIMsgIncomingServer::moveDupsToTrash:
               {
                 nsCOMPtr <nsIMsgFolder> trash;
                 GetTrashFolder(getter_AddRefs(trash));
-                if (trash)
-                {
+                if (trash) {
                   uint32_t newFlags;
-                bool msgMoved;
+                  bool msgMoved;
                   m_newMsgHdr->AndFlags(~nsMsgMessageFlags::New, &newFlags);
-                nsCOMPtr<nsIMsgPluggableStore> msgStore;
-                rv = m_downloadFolder->GetMsgStore(getter_AddRefs(msgStore));
-                if (NS_SUCCEEDED(rv))
-                  msgStore->MoveNewlyDownloadedMessage(m_newMsgHdr, trash, &msgMoved);
-                if (!msgMoved)
-                {
-                  MoveIncorporatedMessage(m_newMsgHdr, m_mailDB, trash,
-                                                          nullptr, msgWindow);
-                  m_mailDB->RemoveHeaderMdbRow(m_newMsgHdr);
-                }
+                  nsCOMPtr<nsIMsgPluggableStore> msgStore;
+                  rv = m_downloadFolder->GetMsgStore(getter_AddRefs(msgStore));
+                  if (NS_SUCCEEDED(rv))
+                    rv = msgStore->MoveNewlyDownloadedMessage(m_newMsgHdr, trash, &msgMoved);
+                  if (NS_SUCCEEDED(rv) && !msgMoved) {
+                    MoveIncorporatedMessage(m_newMsgHdr, m_mailDB, trash,
+                                            nullptr, msgWindow);
+                    m_mailDB->RemoveHeaderMdbRow(m_newMsgHdr);
+                  }
                 }
               }
               break;
             case nsIMsgIncomingServer::markDupsRead:
               MarkFilteredMessageRead(m_newMsgHdr);
               break;
           }
           int32_t numNewMessages;
@@ -2051,18 +2049,18 @@ NS_IMETHODIMP nsParseNewMailState::Apply
             err = NS_OK;
             msgIsNew = false;
           }
           else
           {
             nsCOMPtr<nsIMsgPluggableStore> msgStore;
             err = m_downloadFolder->GetMsgStore(getter_AddRefs(msgStore));
             if (NS_SUCCEEDED(err))
-              msgStore->MoveNewlyDownloadedMessage(msgHdr, destIFolder, &msgMoved);
-            if (!msgMoved)
+              err = msgStore->MoveNewlyDownloadedMessage(msgHdr, destIFolder, &msgMoved);
+            if (NS_SUCCEEDED(err) && !msgMoved)
               err = MoveIncorporatedMessage(msgHdr, m_mailDB, destIFolder,
                                             filter, msgWindow);
             m_msgMovedByFilter = NS_SUCCEEDED(err);
             if (m_msgMovedByFilter)
             {
               if (loggingEnabled)
                 (void)filter->LogRuleHit(filterAction, msgHdr);
             }
--- a/mailnews/local/test/unit/test_pop3MoveFilter.js
+++ b/mailnews/local/test/unit/test_pop3MoveFilter.js
@@ -13,16 +13,22 @@ Components.utils.import("resource://gre/
 Components.utils.import("resource://testing-common/mailnews/PromiseTestUtils.jsm");
 
 const gFiles = ["../../../data/bugmail10", "../../../data/bugmail11"];
 
 // make sure limiting download size doesn't causes issues with move filters.
 Services.prefs.setBoolPref("mail.server.default.limit_offline_message_size", true);
 Services.prefs.setBoolPref("mail.server.default.leave_on_server", true);
 
+// Currently we have two mailbox storage formats.
+var gPluggableStores = [
+  "@mozilla.org/msgstore/berkeleystore;1",
+  "@mozilla.org/msgstore/maildirstore;1"
+];
+
 const bugmail10_preview = 'Do not reply to this email. You can add comments to this bug at https://bugzilla.mozilla.org/show_bug.cgi?id=436880 -- Configure bugmail: https://bugzilla.mozilla.org/userprefs.cgi?tab=email ------- You are receiving this mail because: -----';
 const bugmail11_preview = 'Bugzilla has received a request to create a user account using your email address (example@example.org). To confirm that you want to create an account using that email address, visit the following link: https://bugzilla.mozilla.org/token.cgi?t=xxx';
 
 var gMoveFolder;
 var gFilter; // the test filter
 var gFilterList;
 const gTestArray =
 [
@@ -77,37 +83,57 @@ const gTestArray =
       keys.push(hdr.messageKey);
       hdrs.push(hdr);
     }
     do_check_false(gMoveFolder.fetchMsgPreviewText(keys, keys.length, false,
                                                    null));
     do_check_eq(hdrs[0].getStringProperty('preview'), bugmail10_preview);
     do_check_eq(hdrs[1].getStringProperty('preview'), bugmail11_preview);
   },
-  function endTest() {
-    gPOP3Pump = null;
-  },
 ];
 
 function folderCount(folder)
 {
   let enumerator = folder.msgDatabase.EnumerateMessages();
   let count = 0;
   while (enumerator.hasMoreElements())
   {
     count++;
     let hdr = enumerator.getNext();
   }
   return count;
 }
 
+function setup_store(storeID)
+{
+  return function _setup_store() {
+    // Reset pop3Pump with correct mailbox format.
+    gPOP3Pump.resetPluggableStore(storeID);
+
+    // Make sure we're not quarantining messages
+    Services.prefs.setBoolPref("mailnews.downloadToTempFile", false);
+
+    if (!localAccountUtils.inboxFolder)
+      localAccountUtils.loadLocalMailAccount();
+
+    gMoveFolder = localAccountUtils.rootFolder
+                                   .createLocalSubfolder("MoveFolder");
+  }
+}
+
 function run_test()
 {
-  // Make sure we're not quarantining messages
-  Services.prefs.setBoolPref("mailnews.downloadToTempFile", false);
-  if (!localAccountUtils.inboxFolder)
-    localAccountUtils.loadLocalMailAccount();
+  for (let store of gPluggableStores) {
+    add_task(setup_store(store));
+    gTestArray.forEach(add_task);
+  }
 
-  gMoveFolder = localAccountUtils.rootFolder.createLocalSubfolder("MoveFolder");
-
-  gTestArray.forEach(add_task);
+  add_task(exitTest);
   run_next_test();
 }
+
+function exitTest()
+{
+  // Cleanup and exit the test.
+  do_print("Exiting mail tests\n");
+  gPOP3Pump = null;
+}
+
--- a/mailnews/test/resources/POP3pump.js
+++ b/mailnews/test/resources/POP3pump.js
@@ -5,16 +5,19 @@
  *
  *  gPOP3Pump:        the main access to the routine
  *  gPOP3Pump.run()   function to run to load the messages. Returns promise that
  *                    resolves when done.
  *  gPOP3Pump.files:  (in) an array of message files to load
  *  gPOP3Pump.onDone: function to execute after completion
                       (optional and deprecated)
  *  gPOP3Pump.fakeServer:  (out) the POP3 incoming server
+ *  gPOP3Pump.resetPluggableStore(): function to change the pluggable store for the
+ *                                   server to the input parameter's store.
+ *                                   (in) pluggable store contract ID
  *
  * adapted from test_pop3GetNewMail.js
  *
  * Original Author: Kent James <kent@caspia.com>
  *
  */
 
 Components.utils.import("resource:///modules/mailServices.js");
@@ -40,16 +43,18 @@ function POP3Pump()
   this._daemon = null;
   this._incomingServer = null;
   this._pop3Service = null;
   this._firstFile = true;
   this._tests = [];
   this._finalCleanup = false;
   this._expectedResult = Components.results.NS_OK;
   this._actualResult = Components.results.NS_ERROR_UNEXPECTED;
+  this._mailboxStoreContractID =
+    Services.prefs.getCharPref("mail.serverDefaultStoreContractID");
 }
 
 // nsIUrlListener implementation
 POP3Pump.prototype.OnStartRunningUrl = function OnStartRunningUrl(url) {};
 
 POP3Pump.prototype.OnStopRunningUrl = function OnStopRunningUrl(aUrl, aResult)
 {
   this._actualResult = aResult;
@@ -90,16 +95,39 @@ POP3Pump.prototype._createPop3ServerAndL
 
   if (!this.fakeServer)
     this.fakeServer = localAccountUtils.create_incoming_server("pop3", this.kPOP3_PORT,
 							       "fred", "wilma");
 
   return this.fakeServer;
 };
 
+POP3Pump.prototype.resetPluggableStore = function(aStoreContractID)
+{
+  if (aStoreContractID == this._mailboxStoreContractID)
+    return;
+
+  Services.prefs.setCharPref("mail.serverDefaultStoreContractID", aStoreContractID);
+
+  // Cleanup existing files, server and account instances, if any.
+  if (this._server)
+    this._server.stop();
+
+  if (this.fakeServer && this.fakeServer.valid) {
+    this.fakeServer.closeCachedConnections();
+    MailServices.accounts.removeIncomingServer(this.fakeServer, false);
+  }
+
+  this.fakeServer = null;
+  localAccountUtils.clearAll();
+
+  this._incomingServer = this._createPop3ServerAndLocalFolders();
+  this._mailboxStoreContractID = aStoreContractID;
+};
+
 POP3Pump.prototype._checkBusy = function _checkBusy()
 {
   if (this._tests.length == 0 && !this._finalCleanup)
   {
     this._incomingServer.closeCachedConnections();
 
     // No more tests, let everything finish
     this._server.stop();
--- a/mailnews/test/resources/localAccountUtils.js
+++ b/mailnews/test/resources/localAccountUtils.js
@@ -17,16 +17,26 @@ var CC = Components.Constructor;
 var localAccountUtils = {
   inboxFolder: undefined,
   incomingServer: undefined,
   rootFolder: undefined,
   msgAccount: undefined,
 
   _localAccountInitialized: false,
 
+  clearAll: function() {
+    this._localAccountInitialized = false;
+    if (this.msgAccount)
+      MailServices.accounts.removeAccount(this.msgAccount);
+    this.incomingServer = undefined;
+    this.msgAccount = undefined;
+    this.inboxFolder = undefined;
+    this.rootFolder = undefined;
+  },
+
   loadLocalMailAccount: function() {
     // This function is idempotent
     if (this._localAccountInitialized)
       return;
 
     MailServices.accounts.createLocalMailAccount();
 
     this.incomingServer = MailServices.accounts.localFoldersServer;