Merge pull request #357 from mozilla-b2g/offline-moves

Bug 839273 - [email] Offline moves can break sync of target folder. r=asuth

js/imap/folder.js

@@ -432,6 +432,16 @@ console.log('BISECT CASE', serverUIDs.length, 'curDaysDelta', curDaysDelta);
         if (progressCallback)
           progressCallback(0.25);
 
+        // Ignore not-yet-synced local messages (messages without a
+        // srvid), such as messages from a partially-completed local
+        // move. Because they have no server ID, we can't compare them
+        // to anything currently on the server anyway.
+        for (var iMsg = 0; iMsg < headers.length; iMsg++) {
+          if (headers[iMsg].srvid === null) {
+            headers.splice(iMsg--, 1);
+          }
+        }
+
         // -- infer deletion, flag to distinguish known messages
         // rather than splicing lists and causing shifts, we null out values.
         for (var iMsg = 0; iMsg < headers.length; iMsg++) {

test/test-files.json

@@ -32,6 +32,10 @@
   },
   "tests": {
 
+    "test_move_offline.js": {
+      "variants": ["imap:fake"]
+    },
+
     "test_outbox_stuck_retry.js": {
       "variants": ["imap:fake", "pop3:fake", "activesync:fake"]
     },

test/unit/resources/th_main.js

@@ -1479,13 +1479,14 @@ var TestCommonAccountMixins = {
               slice.userCanGrowUpwards,
               slice.userCanGrowDownwards, slice.status,
               newEmailCount === undefined ? null : newEmailCount);
-            if (!_saveToThing) {
-              slice.die();
-            }
           }
           else {
             self._logger.viewWithoutExpectationsCompleted();
           }
+          // The slice must die even if we don't expect values.
+          if (!_saveToThing) {
+            slice.die();
+          }
         };
       }
     });

test/unit/test_move_offline.js

@@ -0,0 +1,122 @@
+define(['rdcommon/testcontext', './resources/th_main',
+        'slog', 'mailapi', 'exports'],
+       function($tc, $th_main, slog, $mailapi, exports) {
+
+var TD = exports.TD = $tc.defineTestsFor(
+  { id: 'test_move_offline' }, null,
+  [$th_main.TESTHELPER], ['app']);
+
+/**
+ * Ensure that partially-completed offline moves don't break the sync
+ * of the target folder if we attempt to refresh the target folder
+ * before the move completes (bug 839273).
+ *
+ * Test the following scenario:
+ *
+ * 1. Go offline and move a message from one folder to another.
+ *    Because we're offline, only the local side will complete.
+ *
+ * 2. Force the job system to temporarily stop running jobs, to
+ *    simulate a backlog of queued jobs, causing the server side of
+ *    the move to be delayed until AFTER we try to sync the target
+ *    folder.
+ *
+ * 3. Star a message in the target folder *on the server*.
+ *
+ * 4. Go online and view the target folder's contents. We should see
+ *    the newly-starred message, indicating that the sync completed
+ *    successfully, even though we left a not-yet-completed "move"
+ *    message in the target folder.
+ */
+TD.commonCase('partially-completed move does not break sync', function(T, RT) {
+  T.group('setup');
+  var testUniverse = T.actor('testUniverse', 'U'),
+      testAccount = T.actor('testAccount', 'A',
+                            { universe: testUniverse }),
+      eSync = T.lazyLogger('check');
+
+  var sourceFolder = testAccount.do_createTestFolder(
+    'test_move_source',
+    { count: 5, age: { days: 1 }, age_incr: { days: 1 } });
+
+  var targetFolder = testAccount.do_createTestFolder(
+    'test_move_target',
+    { count: 2, age: { days: 1 }, age_incr: { days: 1 } });
+
+  var sourceView = testAccount.do_openFolderView(
+    'sourceView', sourceFolder,
+    { count: 5, full: 5, flags: 0, changed: 0, deleted: 0 },
+    { top: true, bottom: true, grow: false },
+    { syncedToDawnOfTime: true });
+
+  var targetView = testAccount.do_openFolderView(
+    'targetView', targetFolder,
+    { count: 2, full: 2, flags: 0, changed: 0, deleted: 0 },
+    { top: true, bottom: true, grow: false },
+    { syncedToDawnOfTime: true });
+
+  testUniverse.do_pretendToBeOffline(true);
+
+  var FAKE_SERVER_OP = { serverStatus: 'doing' };
+
+  T.action('try the move job op', testAccount, function() {
+    var messageToMove = sourceView.slice.items[1];
+
+    // The local op will complete; the server won't since we're offline.
+    testAccount.expect_runOp(
+      'move', { local: true, server: false, save: true });
+
+    var log = new slog.LogChecker(T, RT);
+    // The local job will succeed and it will release its mutexes without having
+    // experienced any errors.
+    log.mustLog('mailslice:mutex-released',
+                { folderId: sourceFolder.id, err: null });
+    log.mustLog('mailslice:mutex-released',
+                { folderId: targetFolder.id, err: null });
+
+    // Force the job system to appear backlogged, by inserting a fake
+    // operation at the beginning. This op will block the server queue
+    // from completing the move.
+    testUniverse.universe._queueAccountOp(
+      testAccount.account, FAKE_SERVER_OP);
+
+    // Move the message locally.
+    testUniverse.MailAPI.moveMessages(
+      [messageToMove], targetFolder.mailFolder);
+
+    // Now, flag the first message in the target folder.
+    testAccount.modifyMessageFlagsOnServerButNotLocally(
+      targetView, [0], ['\\Flagged'], null);
+  });
+
+  testAccount.do_closeFolderView(sourceView);
+  testAccount.do_closeFolderView(targetView);
+
+  // Go back online.
+  testUniverse.do_pretendToBeOffline(false);
+
+  // Open the folder again and sync; if all is well, we should see the
+  // starred message, indicating that the sync succesfully fetched
+  // updates.
+
+  // (I would have liked to just set proper 'count'/'flags'
+  // expectations with do_viewFolder, but the locally-moved message
+  // breaks th_main's localMessages bookkeeping. Otherwise, we could
+  // have just run a sync expecting the proper changes.)
+  var targetView2 = testAccount.do_openFolderView(
+    'targetView2', targetFolder,
+    null,
+    { top: true, bottom: true, grow: false },
+    { syncedToDawnOfTime: true });
+
+  T.action("verify folder contents", eSync, function() {
+    var starredMsg = targetView2.slice.items[0];
+    eSync.expect_namedValue("first message is starred", true);
+    eSync.namedValue("first message is starred", starredMsg.isStarred);
+  });
+
+  testAccount.do_closeFolderView(targetView2);
+});
+
+
+}); // end define