diff --git a/js/imap/folder.js b/js/imap/folder.js index de061a9e..481fc8a9 100644 --- a/js/imap/folder.js +++ b/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++) { diff --git a/test/test-files.json b/test/test-files.json index 1021f0eb..07f0cd87 100644 --- a/test/test-files.json +++ b/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"] }, diff --git a/test/unit/resources/th_main.js b/test/unit/resources/th_main.js index ed09c5f9..bda3cfb1 100644 --- a/test/unit/resources/th_main.js +++ b/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(); + } }; } }); diff --git a/test/unit/test_move_offline.js b/test/unit/test_move_offline.js new file mode 100644 index 00000000..d3edb583 --- /dev/null +++ b/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