Bug 1152651 part 2, maildir-mbox multi copy fails, r=neil, a=rkent
☠☠ backed out by 53f8ab1d339d ☠ ☠
authorR Kent James <rkent@caspia.com>
Sun, 13 Sep 2015 20:41:34 -0700
changeset 26354 8f8ed2761bf70e9ca1708f49d2d4ce4651065ecb
parent 26353 4b40a0f14dea36aa9f52d4e2e96d363d1da708ac
child 26355 addd9fa53b40010d10cf47615fda7c99f5d5a4d5
push id1850
push userclokep@gmail.com
push dateWed, 08 Mar 2017 19:29:12 +0000
treeherdercomm-esr52@028df196b2d9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersneil, rkent
bugs1152651
Bug 1152651 part 2, maildir-mbox multi copy fails, r=neil, a=rkent
mailnews/local/src/nsMailboxProtocol.cpp
mailnews/local/src/nsMailboxProtocol.h
mailnews/local/test/unit/test_pop3MultiCopy2.js
mailnews/local/test/unit/xpcshell.ini
--- a/mailnews/local/src/nsMailboxProtocol.cpp
+++ b/mailnews/local/src/nsMailboxProtocol.cpp
@@ -74,42 +74,16 @@ nsresult nsMailboxProtocol::OpenMultiple
   // XXX 64-bit
   rv = serv->CreateInputTransport(m_multipleMsgMoveCopyStream, int64_t(offset),
                                   int64_t(size), false,
                                   getter_AddRefs(m_transport));
 
   return rv;
 }
 
-nsresult nsMailboxProtocol::OpenFileSocketForReuse(nsIURI * aURL, uint64_t aStartPosition, int32_t aReadCount)
-{
-  NS_ENSURE_ARG_POINTER(aURL);
-
-  nsresult rv = NS_OK;
-  m_readCount = aReadCount;
-
-  nsCOMPtr <nsIFile> file;
-
-  rv = GetFileFromURL(aURL, getter_AddRefs(file));
-  NS_ENSURE_SUCCESS(rv, rv);
-    
-  nsCOMPtr<nsIFileInputStream> fileStream = do_CreateInstance(NS_LOCALFILEINPUTSTREAM_CONTRACTID, &rv);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  m_multipleMsgMoveCopyStream = do_QueryInterface(fileStream, &rv);
-  NS_ENSURE_SUCCESS(rv, rv);
-  fileStream->Init(file,  PR_RDONLY, 0664, false);  //just have to read the messages
-
-  rv = OpenMultipleMsgTransport(aStartPosition, aReadCount);
-
-  m_socketIsOpen = false;
-  return rv;
-}
-
-
 nsresult nsMailboxProtocol::Initialize(nsIURI * aURL)
 {
   NS_PRECONDITION(aURL, "invalid URL passed into MAILBOX Protocol");
   nsresult rv = NS_OK;
   if (aURL)
   {
     rv = aURL->QueryInterface(NS_GET_IID(nsIMailboxUrl), (void **) getter_AddRefs(m_runningUrl));
     if (NS_SUCCEEDED(rv) && m_runningUrl)
@@ -147,63 +121,56 @@ nsresult nsMailboxProtocol::Initialize(n
         uint32_t aMsgSize = 0;
         rv = m_runningUrl->GetMessageSize(&aMsgSize);
         NS_ASSERTION(NS_SUCCEEDED(rv), "oops....i messed something up");
         SetContentLength(aMsgSize);
         mailnewsUrl->SetMaxProgress(aMsgSize);
 
         if (RunningMultipleMsgUrl())
         {
-          rv = OpenFileSocketForReuse(aURL, m_msgOffset, aMsgSize);
           // if we're running multiple msg url, we clear the event sink because the multiple
           // msg urls will handle setting the progress.
           mProgressEventSink = nullptr;
         }
-        else
+
+        nsCOMPtr<nsIMsgMessageUrl> msgUrl = do_QueryInterface(m_runningUrl, &rv);
+        if (NS_SUCCEEDED(rv))
         {
-          nsCOMPtr<nsIMsgIncomingServer> server;
           nsCOMPtr<nsIMsgDBHdr> msgHdr;
-          nsCOMPtr<nsIMsgFolder> folder;
-          nsCOMPtr<nsIMsgMessageUrl> msgUrl = do_QueryInterface(m_runningUrl, &rv);
-          NS_ENSURE_SUCCESS(rv,rv);
           rv = msgUrl->GetMessageHeader(getter_AddRefs(msgHdr));
-          if (msgHdr)
+          if (NS_SUCCEEDED(rv) && msgHdr)
           {
-            msgHdr->GetFolder(getter_AddRefs(folder));
-            if (folder)
-              folder->GetServer(getter_AddRefs(server));
-          }
-          if (server)
-          {
-            nsCOMPtr<nsIMsgPluggableStore> msgStore;
-            rv = server->GetMsgStore(getter_AddRefs(msgStore));
-            NS_ENSURE_SUCCESS(rv, rv);
-
-            if (NS_SUCCEEDED(rv) && msgHdr)
+            nsCOMPtr<nsIMsgFolder> folder;
+            rv = msgHdr->GetFolder(getter_AddRefs(folder));
+            if (NS_SUCCEEDED(rv) && folder)
             {
               nsCOMPtr<nsIInputStream> stream;
               int64_t offset = 0;
               bool reusable = false;
 
               rv = folder->GetMsgInputStream(msgHdr, &reusable, getter_AddRefs(stream));
               NS_ENSURE_SUCCESS(rv, rv);
               nsCOMPtr<nsISeekableStream> seekableStream(do_QueryInterface(stream, &rv));
               NS_ENSURE_SUCCESS(rv, rv);
               seekableStream->Tell(&offset);
               // create input stream transport
               nsCOMPtr<nsIStreamTransportService> sts =
                   do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID, &rv);
               if (NS_FAILED(rv)) return rv;
               m_readCount = aMsgSize;
+              // Save the stream for reuse, but only for multiple URLs.
+              if (reusable && RunningMultipleMsgUrl())
+                m_multipleMsgMoveCopyStream = stream;
+              else
+                reusable = false;
               rv = sts->CreateInputTransport(stream, offset,
-                                             int64_t(aMsgSize), true,
+                                             int64_t(aMsgSize), !reusable,
                                              getter_AddRefs(m_transport));
 
               m_socketIsOpen = false;
-             
             }
           }
           else // must be a .eml file
             rv = OpenFileSocket(aURL, 0, aMsgSize);
         }
         NS_ASSERTION(NS_SUCCEEDED(rv), "oops....i messed something up");
       }
     }
@@ -307,50 +274,77 @@ NS_IMETHODIMP nsMailboxProtocol::OnStopR
             {
               nsCString uri;
               msgFolder->GetUriForMsg(nextMsg, uri);
               nsCOMPtr<nsIMsgMessageUrl> msgUrl = do_QueryInterface(m_runningUrl);
               if (msgUrl)
               {
                 msgUrl->SetOriginalSpec(uri.get());
                 msgUrl->SetUri(uri.get());
-                
+
                 uint64_t msgOffset;
                 nextMsg->GetMessageOffset(&msgOffset);
                 nextMsg->GetMessageSize(&msgSize);
                 // now we have to seek to the right position in the file and
                 // basically re-initialize the transport with the correct message size.
                 // then, we have to make sure the url keeps running somehow.
                 nsCOMPtr<nsISupports> urlSupports = do_QueryInterface(m_runningUrl);
                 //
                 // put us in a state where we are always notified of incoming data
                 //
-                
+
                 m_transport = 0; // open new stream transport
                 m_inputStream = 0;
                 m_outputStream = 0;
-                
-                rv = OpenMultipleMsgTransport(msgOffset, msgSize);
+
+                if (m_multipleMsgMoveCopyStream)
+                {
+                  rv = OpenMultipleMsgTransport(msgOffset, msgSize);
+                }
+                else
+                {
+                  nsCOMPtr<nsIInputStream> stream;
+                  bool reusable = false;
+                  rv = msgFolder->GetMsgInputStream(nextMsg, &reusable,
+                                                    getter_AddRefs(stream));
+                  NS_ASSERTION(!reusable, "We thought streams were not reusable!");
+
+                  if (NS_SUCCEEDED(rv))
+                  {
+                    // create input stream transport
+                    nsCOMPtr<nsIStreamTransportService> sts =
+                        do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID, &rv);
+
+                    if (NS_SUCCEEDED(rv))
+                    {
+                      m_readCount = msgSize;
+                      rv = sts->CreateInputTransport(stream, msgOffset,
+                                                     int64_t(msgSize), true,
+                                                     getter_AddRefs(m_transport));
+                    }
+                  }
+                }
+
                 if (NS_SUCCEEDED(rv))
                 {
                   if (!m_inputStream)
                     rv = m_transport->OpenInputStream(0, 0, 0, getter_AddRefs(m_inputStream));
-                  
+
                   if (NS_SUCCEEDED(rv))
                   {
                     nsCOMPtr<nsIInputStreamPump> pump;
                     rv = NS_NewInputStreamPump(getter_AddRefs(pump), m_inputStream);
                     if (NS_SUCCEEDED(rv)) {
                       rv = pump->AsyncRead(this, urlSupports);
                       if (NS_SUCCEEDED(rv))
                         m_request = pump;
                     }
                   }
                 }
-                
+
                 NS_ASSERTION(NS_SUCCEEDED(rv), "AsyncRead failed");
                 if (m_loadGroup)
                   m_loadGroup->RemoveRequest(static_cast<nsIRequest *>(this), nullptr, aStatus);
                 m_socketIsOpen = true; // mark the channel as open
                 return aStatus;
               }
             }
           }
--- a/mailnews/local/src/nsMailboxProtocol.h
+++ b/mailnews/local/src/nsMailboxProtocol.h
@@ -92,17 +92,16 @@ private:
   nsCOMPtr<nsIInputStream> m_multipleMsgMoveCopyStream;
 
   virtual nsresult ProcessProtocolState(nsIURI * url, nsIInputStream * inputStream,
                                         uint64_t sourceOffset, uint32_t length) override;
   virtual nsresult CloseSocket() override;
 
   nsresult SetupMessageExtraction();
   nsresult OpenMultipleMsgTransport(uint64_t offset, int32_t size);
-  nsresult OpenFileSocketForReuse(nsIURI * aURL, uint64_t aStartPosition, int32_t aReadCount);
   bool RunningMultipleMsgUrl();
 
   ////////////////////////////////////////////////////////////////////////////////////////
   // Protocol Methods --> This protocol is state driven so each protocol method is
   //            designed to re-act to the current "state". I've attempted to
   //            group them together based on functionality.
   ////////////////////////////////////////////////////////////////////////////////////////
 
new file mode 100644
--- /dev/null
+++ b/mailnews/local/test/unit/test_pop3MultiCopy2.js
@@ -0,0 +1,169 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * This tests that moved multiple messages from maildir->mbox and
+   mbox->maildir are correct.
+ */
+
+load("../../../resources/POP3pump.js");
+Components.utils.import("resource://gre/modules/Promise.jsm");
+Components.utils.import("resource://testing-common/mailnews/PromiseTestUtils.jsm");
+
+Services.prefs.setCharPref("mail.serverDefaultStoreContractID",
+                           "@mozilla.org/msgstore/maildirstore;1");
+
+let gInboxFolder, gTestFolder;
+
+gPOP3Pump.files = ["../../../data/bugmail1",
+                   "../../../data/draft1"];
+let gTestSubjects = ["[Bug 397009] A filter will let me tag, but not untag",
+                     "Hello, did you receive my bugmail?"];
+
+add_task(function* setupFolders() {
+  let storeID = "@mozilla.org/msgstore/maildirstore;1";
+  resetPluggableStoreLocal(storeID);
+
+  // We want to test cross-server copy, so don't defer.
+  gPOP3Pump.fakeServer.deferredToAccount = "";
+
+  // Create a test folder on the Local Folders account.
+  gTestFolder = localAccountUtils.rootFolder
+                                 .QueryInterface(Ci.nsIMsgLocalMailFolder)
+                                 .createLocalSubfolder("test");
+  dump("testFolder is at " + gTestFolder.filePath.path + "\n");
+  yield gPOP3Pump.run();
+});
+  
+add_task(function* maildirToMbox() {
+  // Test for multiple message copy for maildir->mbox.
+
+  // get message headers for the inbox folder
+  gInboxFolder = gPOP3Pump.fakeServer
+                          .rootMsgFolder
+                          .getFolderWithFlags(Ci.nsMsgFolderFlags.Inbox);
+  dump("inbox is at " + gInboxFolder.filePath.path + "\n");
+
+  // Accumulate messages to copy.
+  let messages = Cc["@mozilla.org/array;1"].createInstance(Ci.nsIMutableArray);
+  let enumerator = gInboxFolder.msgDatabase.EnumerateMessages();
+  while (enumerator.hasMoreElements()) {
+    let hdr = enumerator.getNext().QueryInterface(Ci.nsIMsgDBHdr);
+    messages.appendElement(hdr, false);
+  }
+  do_check_eq(messages.length, 2);
+
+  // Move messages to mbox test folder.
+  let promiseCopyListener = new PromiseTestUtils.PromiseCopyListener();
+  MailServices.copy.CopyMessages(gInboxFolder, messages, gTestFolder, true,
+                                 promiseCopyListener, null, false);
+  yield promiseCopyListener.promise;
+
+  // Check the destination headers.
+  messages.clear();
+  enumerator = gTestFolder.msgDatabase.EnumerateMessages();
+  let subjects = [];
+  while (enumerator.hasMoreElements()) {
+    let hdr = enumerator.getNext().QueryInterface(Ci.nsIMsgDBHdr);
+    messages.appendElement(hdr, false);
+    dump("Subject: " + hdr.subject + "\n");
+    subjects.push(hdr.subject);
+  }
+  do_check_eq(messages.length, 2);
+
+  // messages should be missing from source
+  do_check_eq(gInboxFolder.getTotalMessages(false), 0);
+
+  // Check for subjects. maildir order for messages may not match
+  // order for creation, hence the array.includes.
+  for (let subject of gTestSubjects) {
+    do_check_true(subjects.includes(subject));
+  }
+
+  // Make sure the body matches the message.
+  enumerator = gTestFolder.msgDatabase.EnumerateMessages();
+  while (enumerator.hasMoreElements()) {
+    let hdr = enumerator.getNext().QueryInterface(Ci.nsIMsgDBHdr);
+    let body = mailTestUtils.loadMessageToString(gTestFolder, hdr);
+    do_check_true(body.indexOf(hdr.subject) >= 0);
+  }
+});
+
+add_task(function* mboxToMaildir() {
+  // Test for multiple message copy for mbox->maildir.
+
+  // Accumulate messages to copy.
+  let messages = Cc["@mozilla.org/array;1"].createInstance(Ci.nsIMutableArray);
+  let enumerator = gTestFolder.msgDatabase.EnumerateMessages();
+  while (enumerator.hasMoreElements()) {
+    let hdr = enumerator.getNext().QueryInterface(Ci.nsIMsgDBHdr);
+    messages.appendElement(hdr, false);
+  }
+  do_check_eq(messages.length, 2);
+
+  // Move messages to inbox folder.
+  let promiseCopyListener = new PromiseTestUtils.PromiseCopyListener();
+  MailServices.copy.CopyMessages(gTestFolder, messages, gInboxFolder, true,
+                                 promiseCopyListener, null, false);
+  yield promiseCopyListener.promise;
+
+  // Check the destination headers.
+  messages.clear();
+  enumerator = gInboxFolder.msgDatabase.EnumerateMessages();
+  let subjects = [];
+  while (enumerator.hasMoreElements()) {
+    let hdr = enumerator.getNext().QueryInterface(Ci.nsIMsgDBHdr);
+    messages.appendElement(hdr, false);
+    dump("Subject: " + hdr.subject + "\n");
+    subjects.push(hdr.subject);
+  }
+  do_check_eq(messages.length, 2);
+
+  // messages should be missing from source
+  do_check_eq(gTestFolder.getTotalMessages(false), 0);
+
+  // Check for subjects. maildir order for messages may not match
+  // order for creation, hence the array.includes.
+  for (let subject of gTestSubjects) {
+    do_check_true(subjects.includes(subject));
+  }
+
+  // Make sure the body matches the message.
+  enumerator = gInboxFolder.msgDatabase.EnumerateMessages();
+  while (enumerator.hasMoreElements()) {
+    let hdr = enumerator.getNext().QueryInterface(Ci.nsIMsgDBHdr);
+    let body = mailTestUtils.loadMessageToString(gInboxFolder, hdr);
+    do_check_true(body.indexOf(hdr.subject) >= 0);
+  }
+});
+
+add_task(function testCleanup() {
+  gPOP3Pump = null;
+});
+
+function run_test() {
+  run_next_test();
+}
+
+// Clone of POP3pump resetPluggableStore that does not reset local folders.
+function resetPluggableStoreLocal(aStoreContractID)
+{
+  Services.prefs.setCharPref("mail.serverDefaultStoreContractID", aStoreContractID);
+
+  // Cleanup existing files, server and account instances, if any.
+  if (gPOP3Pump._server)
+    gPOP3Pump._server.stop();
+
+  if (gPOP3Pump.fakeServer && gPOP3Pump.fakeServer.valid) {
+    gPOP3Pump.fakeServer.closeCachedConnections();
+    MailServices.accounts.removeIncomingServer(gPOP3Pump.fakeServer, false);
+  }
+
+  gPOP3Pump.fakeServer = localAccountUtils.create_incoming_server("pop3",
+      gPOP3Pump.kPOP3_PORT, "fred", "wilma");
+
+  //localAccountUtils.clearAll();
+
+  gPOP3Pump._incomingServer = gPOP3Pump.fakeServer;
+  gPOP3Pump._mailboxStoreContractID = aStoreContractID;
+};
--- a/mailnews/local/test/unit/xpcshell.ini
+++ b/mailnews/local/test/unit/xpcshell.ini
@@ -25,16 +25,17 @@ requesttimeoutfactor = 2
 [test_pop3DownloadTempFileHandling.js]
 [test_pop3Duplicates.js]
 [test_pop3FilterActions.js]
 [test_pop3GSSAPIFail.js]
 [test_pop3GetNewMail.js]
 [test_pop3MoveFilter.js]
 [test_pop3MoveFilter2.js]
 [test_pop3MultiCopy.js]
+[test_pop3MultiCopy2.js]
 [test_pop3Password.js]
 [test_pop3Password2.js]
 [test_pop3Password3.js]
 [test_pop3PasswordFailure.js]
 [test_pop3PasswordFailure2.js]
 [test_pop3PasswordFailure3.js]
 [test_pop3Pump.js]
 [test_pop3ServerBrokenCRAMDisconnect.js]