Bug 534449 - Gloda should index sent messages right away instead of waiting for me to open the sent folder. r=bienvenu
authorAndrew Sutherland <asutherland@asutherland.org>
Wed, 19 Jan 2011 16:49:12 -0800
changeset 6986 afa759fe196cc5123b68d82753128ced2ba089eb
parent 6985 fefbbf03688aa4c322f3630c072c27f27c9c9f1d
child 6987 be18bde0f11cb9d08b31d1c8ff5399469321d8c2
push idunknown
push userunknown
push dateunknown
reviewersbienvenu
bugs534449
Bug 534449 - Gloda should index sent messages right away instead of waiting for me to open the sent folder. r=bienvenu
mailnews/db/gloda/modules/collection.js
mailnews/db/gloda/modules/datastore.js
mailnews/db/gloda/modules/index_msg.js
mailnews/db/gloda/test/unit/base_index_messages.js
mailnews/db/gloda/test/unit/resources/glodaTestHelper.js
mailnews/test/resources/folderEventLogHelper.js
mailnews/test/resources/messageInjection.js
--- a/mailnews/db/gloda/modules/collection.js
+++ b/mailnews/db/gloda/modules/collection.js
@@ -132,17 +132,31 @@ var GlodaCollectionManager = {
       }
     }
 
     return null;
   },
 
   /**
    * Lookup multiple nouns by ID from the cache/existing collections.
-   * @return [The number that were found, the number that were not found.]
+   *
+   * @param aNounID The kind of noun identified by its ID.
+   * @param aIDMap A dictionary/map whose keys must be gloda noun ids for the
+   *     given noun type and whose values are ignored.
+   * @param aTargetMap An object to hold the noun id's (key) and noun instances
+   *     (value) for the noun instances that were found available in memory
+   *     because they were cached or in existing query collections.
+   * @param [aDoCache=true] Should we add any items to the cache that we found
+   *     in collections that were in memory but not in the cache?  You would
+   *     likely want to pass false if you are only updating in-memory
+   *     representations rather than performing a new query.
+   *
+   * @return [The number that were found, the number that were not found,
+   *          a dictionary whose keys are the ids of noun instances that
+   *          were not found.]
    */
   cacheLookupMany: function gloda_colm_cacheLookupMany(aNounID, aIDMap,
       aTargetMap, aDoCache) {
     let foundCount = 0, notFoundCount = 0, notFound = {};
 
     let cache = this._cachesByNoun[aNounID];
 
     if (cache) {
--- a/mailnews/db/gloda/modules/datastore.js
+++ b/mailnews/db/gloda/modules/datastore.js
@@ -2495,44 +2495,91 @@ var GlodaDatastore = {
    *  database locations.  Also, update the in-memory representations.
    */
   updateMessageLocations: function gloda_ds_updateMessageLocations(aMessageIds,
       aNewMessageKeys, aDestFolder, aDoNotNotify) {
     let statement = this._updateMessageLocationStatement;
     let destFolderID = (typeof(aDestFolder) == "number") ? aDestFolder :
                          this._mapFolder(aDestFolder).id;
 
-    let modifiedItems = [];
+    // map gloda id to the new message key for in-memory rep transform below
+    let cacheLookupMap = {};
 
     for (let iMsg = 0; iMsg < aMessageIds.length; iMsg++) {
-      let id = aMessageIds[iMsg];
+      let id = aMessageIds[iMsg], msgKey = aNewMessageKeys[iMsg];
       statement.bindInt64Parameter(0, destFolderID);
-      statement.bindInt64Parameter(1, aNewMessageKeys[iMsg]);
+      statement.bindInt64Parameter(1, msgKey);
       statement.bindInt64Parameter(2, id);
       statement.executeAsync(this.trackAsync());
 
-      // so, if the message is currently loaded, we also need to change it up...
-      // XXX we should be using cacheLookupMany.
-      let message = GlodaCollectionManager.cacheLookupOne(
-        GlodaMessage.prototype.NOUN_ID, id);
-      if (message) {
-        message._folderID = destFolderID;
-        message._messageKey = aNewMessageKeys[iMsg];
-        modifiedItems.push(message);
-      }
+      cacheLookupMap[id] = msgKey;
+    }
+
+    // - perform the cache lookup so we can update in-memory representations
+    // found in memory items, and converted to list form for notification
+    let inMemoryItems = {}, modifiedItems = [];
+    GlodaCollectionManager.cacheLookupMany(GlodaMessage.prototype.NOUN_ID,
+                                           cacheLookupMap,
+                                           inMemoryItems,
+                                           /* do not cache */ false);
+    for each (let [glodaId, glodaMsg] in Iterator(inMemoryItems)) {
+      glodaMsg._folderID = destFolderID;
+      glodaMsg._messageKey = cacheLookupMap[glodaId];
+      modifiedItems.push(glodaMsg);
     }
 
     // tell the collection manager about the modified messages so it can update
     //  any existing views...
     if (!aDoNotNotify && modifiedItems.length) {
       GlodaCollectionManager.itemsModified(GlodaMessage.prototype.NOUN_ID,
                                            modifiedItems);
     }
   },
 
+  get _updateMessageKeyStatement() {
+    let statement = this._createAsyncStatement(
+      "UPDATE messages SET messageKey = ?1 WHERE id = ?2");
+    this.__defineGetter__("_updateMessageKeyStatement",
+                          function() statement);
+    return this._updateMessageKeyStatement;
+  },
+
+  /**
+   * Update the message keys for the gloda messages with the given id's.  This
+   *  is to be used in response to msgKeyChanged notifications and is similar to
+   *  `updateMessageLocations` except that we do not update the folder and we
+   *  do not perform itemsModified notifications (because message keys are not
+   *  intended to be relevant to the gloda message abstraction).
+   */
+  updateMessageKeys: function(aMessageIds, aNewMessageKeys) {
+    let statement = this._updateMessageKeyStatement;
+
+    // map gloda id to the new message key for in-memory rep transform below
+    let cacheLookupMap = {};
+
+    for (let iMsg = 0; iMsg < aMessageIds.length; iMsg++) {
+      let id = aMessageIds[iMsg], msgKey = aNewMessageKeys[iMsg];
+      statement.bindInt64Parameter(0, msgKey);
+      statement.bindInt64Parameter(1, id);
+      statement.executeAsync(this.trackAsync());
+
+      cacheLookupMap[id] = msgKey;
+    }
+
+    // - perform the cache lookup so we can update in-memory representations
+    let inMemoryItems = {};
+    GlodaCollectionManager.cacheLookupMany(GlodaMessage.prototype.NOUN_ID,
+                                           cacheLookupMap,
+                                           inMemoryItems,
+                                           /* do not cache */ false);
+    for each (let [glodaId, glodaMsg] in Iterator(inMemoryItems)) {
+      glodaMsg._messageKey = cacheLookupMap[glodaId];
+    }
+  },
+
   /**
    * Asynchronously mutate message folder id/message keys for the given
    *  messages, indicating that we are moving them to the target folder, but
    *  don't yet know their target message keys.
    *
    * Updates in-memory representations too.
    */
   updateMessageFoldersByKeyPurging:
--- a/mailnews/db/gloda/modules/index_msg.js
+++ b/mailnews/db/gloda/modules/index_msg.js
@@ -122,16 +122,22 @@ function range(begin, end) {
 /**
  * We do not set properties on the messages until we perform a DB commit; this
  *  helper class tracks messages that we have indexed but are not yet marked
  *  as such on their header.
  */
 var PendingCommitTracker = {
   /**
    * Maps message URIs to their gloda ids.
+   *
+   * I am not entirely sure why I chose the URI for the key rather than
+   *  gloda folder ID + message key.  Most likely it was to simplify debugging
+   *  since the gloda folder ID is opaque while the URI is very informative.  It
+   *  is also possible I was afraid of IMAP folder renaming triggering a UID
+   *  renumbering?
    */
   _indexedMessagesPendingCommitByKey: {},
   /**
    * Map from the pending commit gloda id to a tuple of [the corresponding
    *  message header, dirtyState].
    */
   _indexedMessagesPendingCommitByGlodaId: {},
   /**
@@ -268,20 +274,23 @@ var PendingCommitTracker = {
     this._indexedMessagesPendingCommitByKey[newKey] = glodaId;
 
     // only clobber the header, not the dirty state
     this._indexedMessagesPendingCommitByGlodaId[glodaId][0] = aNewHdr;
   },
 
   /**
    * A blind move is one where we have the source header but not the destination
-   *  header.  This happens for IMAP messages.
+   *  header.  This happens for IMAP messages that do not involve offline fake
+   *  headers.
    * XXX Since IMAP moves will propagate the gloda-id/gloda-dirty bits for us,
    *  we could detect the other side of the move when it shows up as a
-   *  msgsClassified event and restore the mapping information.
+   *  msgsClassified event and restore the mapping information.  Since the
+   *  offline fake header case should now cover the bulk of IMAP move
+   *  operations, we probably do not need to pursue this.
    *
    * We just re-dispatch to noteDirtyHeader because we can't do anything more
    *  clever.
    */
   noteBlindMove: function PendingCommitTracker_noteBlindMove(aOldHdr) {
     this.noteDirtyHeader(aOldHdr);
   },
 
@@ -1883,40 +1892,106 @@ var GlodaMsgIndexer = {
     let [glodaId, glodaDirty] = PendingCommitTracker.getGlodaState(aMsgHdr);
     return glodaId >= GLODA_FIRST_VALID_MESSAGE_ID &&
            glodaDirty != GlodaMsgIndexer.kMessageFilthy &&
            glodaFolder &&
            glodaFolder.dirtyStatus != glodaFolder.kFolderFilthy;
   },
 
   /* *********** Event Processing *********** */
+
+  /**
+   * Tracks messages we have received msgKeyChanged notifications for in order
+   *  to provide batching and to suppress needless reindexing when we receive
+   *  the expected follow-up msgsClassified notification.
+   *
+   * The entries in this dictionary should be extremely short-lived as we
+   *  receive the msgKeyChanged notification as the offline fake header is
+   *  converted into a real header (which is accompanied by a msgAdded
+   *  notification we don't pay attention to).  Once the headers finish
+   *  updating, the message classifier will get its at-bat and should likely
+   *  find that the messages have already been classified and so fast-path
+   *  them.
+   *
+   * The keys in this dictionary are chosen to be consistent with those of
+   *  PendingCommitTracker: the folder.URI + "#" + the (new) message key.
+   * The values in the dictionary are either an object with "id" (the gloda
+   *  id), "key" (the new message key), and "dirty" (is it dirty and so
+   *  should still be queued for indexing) attributes, or null indicating that
+   *  no change in message key occurred and so no database changes are required.
+   */
+  _keyChangedBatchInfo: {},
+
   /**
    * Common logic for things that want to feed event-driven indexing.  This gets
    *  called by both |_msgFolderListener.msgsClassified| when we are first
    *  seeing a message as well as by |_folderListener| when things happen to
    *  existing messages.  Although we could slightly specialize for the
    *  new-to-us case, it works out to be cleaner to just treat them the same
    *  and take a very small performance hit.
+   *
+   * @param aMsgHdrs Something fixIterator will work on to return an iterator
+   *     on the set of messages that we should treat as potentially changed.
+   * @param aDirtyingEvent Is this event inherently dirtying?  Receiving a
+   *     msgsClassified notification is not inherently dirtying because it is
+   *     just telling us that a message exists.  We use this knowledge to
+   *     ignore the msgsClassified notifications for messages we have received
+   *     msgKeyChanged notifications for and fast-pathed.  Since it is possible
+   *     for user action to do something that dirties the message between the
+   *     time we get the msgKeyChanged notification and when we receive the
+   *     msgsClassified notification, we want to make sure we don't get
+   *     confused.  (Although since we remove the message from our ignore-set
+   *     after the first notification, we would likely just mistakenly treat
+   *     the msgsClassified notification as something dirtying, so it would
+   *     still work out...)
    */
   _reindexChangedMessages: function gloda_indexer_reindexChangedMessage(
-    aMsgHdrs) {
+                                      aMsgHdrs, aDirtyingEvent) {
     let glodaIdsNeedingDeletion = null;
+    let messageKeyChangedIds = null, messageKeyChangedNewKeys = null;
     for each (let msgHdr in fixIterator(aMsgHdrs, nsIMsgDBHdr)) {
       // -- Index this folder?
       let msgFolder = msgHdr.folder;
       if (!this.shouldIndexFolder(msgFolder)) {
         continue;
       }
       // -- Ignore messages in filthy folders!
       // A filthy folder can only be processed by an indexing sweep, and at
       //  that point the message will get indexed.
       let glodaFolder = GlodaDatastore._mapFolder(msgHdr.folder);
       if (glodaFolder.dirtyStatus == glodaFolder.kFolderFilthy)
         continue;
 
+      // -- msgKeyChanged event follow-up
+      if (!aDirtyingEvent) {
+        let keyChangedKey = msgHdr.folder.URI + "#" + msgHdr.messageKey;
+        if (keyChangedKey in this._keyChangedBatchInfo) {
+          var keyChangedInfo = this._keyChangedBatchInfo[keyChangedKey];
+          delete this._keyChangedBatchInfo[keyChangedKey];
+
+          // Null means to ignore this message because the key did not change
+          //  (and the message was not dirty so it is safe to ignore.)
+          if (keyChangedInfo == null)
+            continue;
+          // (the key may be null if we only generated the entry because the
+          //  message was dirty)
+          if (keyChangedInfo.key !== null) {
+            if (messageKeyChangedIds == null) {
+              messageKeyChangedIds = [];
+              messageKeyChangedNewKeys = [];
+            }
+            messageKeyChangedIds.push(keyChangedInfo.id);
+            messageKeyChangedNewKeys.push(keyChangedInfo.key);
+          }
+          // ignore the message because it was not dirty
+          if (!keyChangedInfo.isDirty)
+            continue;
+        }
+      }
+
       // -- Index this message?
       // We index local messages, IMAP messages that are offline, and IMAP
       // messages that aren't offline but whose folders aren't offline either
       let isFolderLocal = msgFolder instanceof nsIMsgLocalMailFolder;
       if (!isFolderLocal) {
         if (!(msgHdr.flags & nsMsgMessageFlags.Offline) &&
             (msgFolder.flags & nsMsgFolderFlags.Offline)) {
           continue;
@@ -1978,16 +2053,21 @@ var GlodaMsgIndexer = {
         this._pendingAddJob.items.push(
           [GlodaDatastore._mapFolder(msgFolder).id, msgHdr.messageKey]);
       }
       else {
         this.indexingSweepNeeded = true;
       }
     }
 
+    // Process any message key changes (from earlier msgKeyChanged events)
+    if (messageKeyChangedIds != null)
+      GlodaDatastore.updateMessageKeys(messageKeyChangedIds,
+                                       messageKeyChangedNewKeys);
+
     // If we accumulated any deletions in there, batch them off now.
     if (glodaIdsNeedingDeletion) {
       GlodaDatastore.markMessagesDeletedByIDs(glodaIdsNeedingDeletion);
       this.pendingDeletions = true;
     }
   },
 
 
@@ -2010,26 +2090,32 @@ var GlodaMsgIndexer = {
      *  junk/trait classification has run (or decided not to run) and all
      *  filters have run.  The msgsClassified notification provides that for us.
      */
     msgAdded: function gloda_indexer_msgAdded(aMsgHdr) {
       // we are never called! we do not enable this bit!
     },
 
     /**
-     * XXX We treat all messages we see as if they have undergone a dirtying
-     *  event.  However, we should really be leveraging the hard work of the
-     *  mailnews IMAP subsystem to fast-path the IMAP move case and just
-     *  update the location information.
+     * Process (apparently newly added) messages that have been looked at by
+     *  the message classifier.  This ensures that if the message was going
+     *  to get marked as spam, this will have already happened.
+     *
+     * Besides truly new (to us) messages, We will also receive this event for
+     *  messages that are the result of IMAP message move/copy operations,
+     *  including both moves that generated offline fake headers and those that
+     *  did not.  In the offline fake header case, however, we are able to
+     *  ignore their msgsClassified events because we will have received a
+     *  msgKeyChanged notification sometime in the recent past.
      */
     msgsClassified: function gloda_indexer_msgsClassified(
                       aMsgHdrs, aJunkClassified, aTraitClassified) {
       this.indexer._log.debug("msgsClassified notification");
       try {
-        GlodaMsgIndexer._reindexChangedMessages(aMsgHdrs.enumerate());
+        GlodaMsgIndexer._reindexChangedMessages(aMsgHdrs.enumerate(), false);
       }
       catch (ex) {
         this.indexer._log.error("Explosion in msgsClassified handling: " +
                                 ex.stack);
       }
     },
 
     /**
@@ -2061,33 +2147,33 @@ var GlodaMsgIndexer = {
         GlodaMsgIndexer._datastore.markMessagesDeletedByIDs(glodaMessageIds);
         GlodaMsgIndexer.pendingDeletions = true;
       }
     },
 
     /**
      * Process a move or copy.
      *
-     * Moves to a local folder are dealt with efficiently because we get both
-     *  the source and destination headers.  The only non-obvious thing is that
-     *  we need to make sure that we deal with the impact of filthy folders and
-     *  messages on gloda-id's (they invalidate the gloda-id).
+     * Moves to a local folder or an IMAP folder where we are generating offline
+     *  fake headers are dealt with efficiently because we get both the source
+     *  and destination headers.  The main ingredient to having offline fake
+     *  headers is that allowUndo was true when the operation was performance.
+     *  The only non-obvious thing is that we need to make sure that we deal
+     *  with the impact of filthy folders and messages on gloda-id's (they
+     *  invalidate the gloda-id).
      *
-     * Moves to an IMAP folder do not provide us with the target header, but the
-     *  IMAP code does have support for propagating properties on the message
-     *  header so when we see it in the msgsClassified (or msgAdded if we used
-     *  that anymore), it should have the properties of the source message
-     *  copied over.
+     * Moves to an IMAP folder that do not generate offline fake headers do not
+     *  provide us with the target header, but the IMAP SetPendingAttributes
+     *  logic will still attempt to propagate the properties on the message
+     *  header so when we eventually see it in the msgsClassified notification,
+     *  it should have the properties of the source message copied over.
      * We make sure that gloda-id's do not get propagated when messages are
      *  moved from IMAP folders that are marked filthy or are marked as not
      *  supposed to be indexed by clearing the pending attributes for the header
      *  being tracked by the destination IMAP folder.
-     * XXX We will receive a msgsClassified event for each message, so the
-     *  main thing we need to do is provide a hint to the indexing logic that
-     *  the gloda message in question should be reused and is not a duplicate.
      * We could fast-path the IMAP move case in msgsClassified by noticing that
      *  a message is showing up with a gloda-id header already and just
      *  performing an async location update.
      *
      * Moves that occur involving 'compacted' folders are fine and do not
      *  require special handling here.  The one tricky super-edge-case that
      *  can happen (and gets handled by the compaction pass) is the move of a
      *  message that got gloda indexed that did not already have a gloda-id and
@@ -2299,20 +2385,55 @@ var GlodaMsgIndexer = {
           this.indexer.indexingSweepNeeded = true;
         }
       } catch (ex) {
         this.indexer._log.error("Problem encountered during message move/copy" +
           ": " + ex + "\n\n" + ex.stack + "\n\n");
       }
     },
 
+    /**
+     * Queue up message key changes that are a result of offline fake headers
+     *  being made real for the actual update during the msgsClassified
+     *  notification that is expected after this.  We defer the
+     *  actual work (if there is any to be done; the fake header might have
+     *  guessed the right UID correctly) so that we can batch our work.
+     *
+     * The expectation is that there will be no meaningful time window between
+     *  this notification and the msgsClassified notification since the message
+     *  classifier should not actually need to classify the messages (they
+     *  should already have been classified) and so can fast-path them.
+     */
     msgKeyChanged: function gloda_indexer_msgKeyChangeded(aOldMsgKey,
                              aNewMsgHdr) {
-      this.indexer._log.debug("MsgKeyChanged notification. " + aOldMsgKey +
-                              " to " + aNewMsgHdr.msgKey);
+      try {
+        let val = null, newKey = aNewMsgHdr.messageKey;
+        let [glodaId, glodaDirty] =
+          PendingCommitTracker.getGlodaState(aNewMsgHdr);
+        // take no action on filthy messages,
+        // generate an entry if dirty or the keys don't match.
+        if ((glodaDirty !== GlodaMsgIndexer.kMessageFilthy) &&
+            ((glodaDirty === GlodaMsgIndexer.kMessageDirty) ||
+             (aOldMsgKey !== newKey))) {
+          val = {
+            id: glodaId,
+            key: (aOldMsgKey !== newKey) ? newKey : null,
+            isDirty: glodaDirty === GlodaMsgIndexer.kMessageDirty,
+          };
+        }
+
+        let key = aNewMsgHdr.folder.URI + "#" + aNewMsgHdr.messageKey;
+        this.indexer._keyChangedBatchInfo[key] = val;
+      }
+      // this is more for the unit test to fail rather than user error reporting
+      catch (ex) {
+        this.indexer._log.error("Problem encountered during msgKeyChanged" +
+                                " notification handling: " + ex + "\n\n" +
+                                ex.stack + " \n\n");
+      }
     },
     /**
      * Handles folder no-longer-exists-ence.  We mark all messages as deleted
      *  and remove the folder from our URI table.  Currently, if a folder that
      *  contains other folders is deleted, we may either receive one
      *  notification for the folder that is deleted, or a notification for the
      *  folder and one for each of its descendents.  This depends upon the
      *  underlying account implementation, so we explicitly handle each case.
@@ -2494,17 +2615,17 @@ var GlodaMsgIndexer = {
         //  indexing sweep queued or active, and if it's already active that
         //  this folder is in the queue to be processed.)
         if (glodaFolder.dirtyStatus == glodaFolder.kFolderDirty)
           GlodaIndexer.indexJob(new IndexingJob("folder", glodaFolder.id));
       }
       else if (aEvent == "JunkStatusChanged") {
         this.indexer._log.debug("JunkStatusChanged notification");
         aItem.QueryInterface(Ci.nsIArray);
-        GlodaMsgIndexer._reindexChangedMessages(aItem.enumerate());
+        GlodaMsgIndexer._reindexChangedMessages(aItem.enumerate(), true);
       }
     },
   },
 
   /**
    * A nsIFolderListener (listening on nsIMsgMailSession so we get all of
    *  these events) PRIMARILY to get folder loaded notifications.  Because of
    *  deficiencies in the nsIMsgFolderListener's events at this time, we also
@@ -2553,17 +2674,17 @@ var GlodaMsgIndexer = {
       if (aProperty == this._kKeywordsAtom ||
           // We could care less about the new flag changing.
           (aProperty == this._kStatusAtom &&
            (aOldValue ^ aNewValue) != nsMsgMessageFlags.New &&
            // We do care about IMAP deletion, but msgsDeleted tells us that, so
            //  ignore IMAPDeleted too...
            (aOldValue ^ aNewValue) != nsMsgMessageFlags.IMAPDeleted) ||
           aProperty == this._kFlaggedAtom) {
-        GlodaMsgIndexer._reindexChangedMessages([aMsgHdr]);
+        GlodaMsgIndexer._reindexChangedMessages([aMsgHdr], true);
       }
     },
 
     /**
      * Get folder loaded notifications for folders that had to do some
      *  (asynchronous) processing before they could be opened.
      */
     OnItemEvent: function gloda_indexer_OnItemEvent(aFolder, aEvent) {
--- a/mailnews/db/gloda/test/unit/base_index_messages.js
+++ b/mailnews/db/gloda/test/unit/base_index_messages.js
@@ -893,60 +893,70 @@ function test_imap_add_unread_to_folder(
   yield wait_for_message_injection();
   yield wait_for_gloda_indexer(msgSet);
 }
 
 /* ===== Message Moving ===== */
 
 /**
  * Moving a message between folders should result in us knowing that the message
- *  is in the target location.  In the case of local moves, this happens
- *  automatically.  In the case of IMAP moves, we need to force the target folder
- *  to be updated.
- *
- * @todo Implication of UIDPLUS on IMAP are not understood / tested.
+ *  is in the target location.
  */
 function test_message_moving() {
   // - inject and insert
   // source folder with the message we care about
   let [srcFolder, msgSet] = make_folder_with_sets([{count: 1}]);
   yield wait_for_message_injection();
   // dest folder with some messages in it to test some wacky local folder moving
   //  logic.  (Local moves try and update the correspondence immediately.)
   let [destFolder, ignoreSet] = make_folder_with_sets([{count: 2}]);
   yield wait_for_message_injection();
 
-
   // (we want the gloda message mapping...)
   yield wait_for_gloda_indexer([msgSet, ignoreSet], {augment: true});
   let gmsg = msgSet.glodaMessages[0];
+  // save off the message key so we can make sure it changes.
+  let oldMessageKey = msgSet.getMsgHdr(0).messageKey;
 
-  // - move it to a new folder
+  // - fastpath (offline) move it to a new folder
   mark_sub_test_start("initial move");
-  yield async_move_messages(msgSet, destFolder);
+  yield async_move_messages(msgSet, destFolder, true);
 
   // - make sure gloda sees it in the new folder
-  // (In the local case, tThe move generates an itemsModified notification, so
-  //  we see it as indexing traffic even if the indexer never goes active.)
-  // (In the IMAP case, the message actually gets reindexed in the target
-  //  folder.)
-  yield wait_for_gloda_indexer(msgSet);
+  // Since we are doing offline IMAP moves, the fast-path should be taken and
+  //  so we should receive an itemsModified notification without a call to
+  //  Gloda.grokNounItem.
+  yield wait_for_gloda_indexer(msgSet, {fullyIndexed: 0});
 
   do_check_eq(gmsg.folderURI,
               get_real_injection_folder(destFolder).URI);
 
   // - make sure the message key is correct!
   do_check_eq(gmsg.messageKey, msgSet.getMsgHdr(0).messageKey);
+  // (sanity check that the messageKey actually changed for the message...)
+  do_check_neq(gmsg.messageKey, oldMessageKey);
 
-  // - move it back to its origin folder
+  // - make sure the indexer's _keyChangedBatchInfo dict is empty
+  for each (let [evilKey, evilValue] in
+              Iterator(GlodaMsgIndexer._keyChangedBatchInfo)) {
+    mark_failure(["GlodaMsgIndexer._keyChangedBatchInfo should be empty but",
+                  "has key:",  evilKey, "and value:", evilValue, "."]);
+  }
+
+  // - slowpath (IMAP online) move it back to its origin folder
   mark_sub_test_start("move it back");
-  yield async_move_messages(msgSet, srcFolder);
-  yield wait_for_gloda_indexer(msgSet);
+  yield async_move_messages(msgSet, srcFolder, false);
+  // In the IMAP case we will end up reindexing the message because we will
+  //  not be able to fast-path, but the local case will still be fast-pathed.
+  yield wait_for_gloda_indexer(msgSet,
+                               {fullyIndexed: message_injection_is_local() ?
+                                                0 : 1});
   do_check_eq(gmsg.folderURI,
               get_real_injection_folder(srcFolder).URI);
+  do_check_eq(gmsg.messageKey, msgSet.getMsgHdr(0).messageKey);
 }
 
 /**
  * Moving a gloda-indexed message out of a filthy folder should result in the
  *  destination message not having a gloda-id.
  */
 
 /* ===== Message Copying ===== */
--- a/mailnews/db/gloda/test/unit/resources/glodaTestHelper.js
+++ b/mailnews/db/gloda/test/unit/resources/glodaTestHelper.js
@@ -254,16 +254,17 @@ if (logHelperHasInterestedListeners) {
     return rv;
   };
 }
 
 const _wait_for_gloda_indexer_defaults = {
   verifier: null,
   augment: false,
   deleted: null,
+  fullyIndexed: null,
 
   // Things should not be recovering or failing and cleaning up unless the test
   //  is expecting it.
   recovered: 0,
   failedToRecover: 0,
   cleanedUp: 0,
   hadNoCleanUp: 0,
 };
@@ -291,16 +292,21 @@ const _wait_for_gloda_indexer_defaults =
  *     verifier function for this given set of messages, or undefined if it is
  *     the first message.)
  * @param [aConfig.augment=false] Should we augment the synthetic message sets
  *     with references to their corresponding gloda messages?  The messages
  *     will show up in a 'glodaMessages' list on the syn set.
  * @param [aConfig.deleted] A single SyntheticMessageSet or list of them
  *     containing messages that should be recognized as deleted by the gloda
  *     indexer in this pass.
+ * @param [aConfig.fullyIndexed] A count of the number of messages we expect
+ *     to observe being fully indexed.  This is relevant because in the case
+ *     of message moves, gloda may generate an onItemsModified notification but
+ *     not reindex the message.  This attribute allows the tests to distinguish
+ *     between the two cases.
  */
 function wait_for_gloda_indexer(aSynMessageSets, aConfig) {
   let ims = _indexMessageState;
 
   if (aSynMessageSets == null)
     aSynMessageSets = [];
   else if (!Array.isArray(aSynMessageSets))
     aSynMessageSets = [aSynMessageSets];
@@ -320,16 +326,18 @@ function wait_for_gloda_indexer(aSynMess
   if (ims.deletionSynSets && !Array.isArray(ims.deletionSynSets))
     ims.deletionSynSets = [ims.deletionSynSets];
 
   ims.expectedWorkerRecoveredCount = get_val("recovered");
   ims.expectedFailedToRecoverCount = get_val("failedToRecover");
   ims.expectedCleanedUpCount = get_val("cleanedUp");
   ims.expectedHadNoCleanUpCount = get_val("hadNoCleanUp");
 
+  ims.expectedNumFullIndexed = get_val("fullyIndexed");
+
   // If we are waiting on certain events to occur first, block on those.
   if (ims.interestingEvents.length) {
     ims.waitingForEvents = true;
     mark_action("glodaTestHelper", "waiting for events", ims.interestingEvents);
     return false;
   }
 
   // if we are still indexing, there is nothing to do right now; save off
@@ -352,16 +360,21 @@ var _indexMessageState = {
   _init: function _indexMessageState_init() {
     if (this._inited)
       return;
 
     Gloda.addIndexerListener(this.onIndexNotification);
     this.catchAllCollection = Gloda._wildcardCollection(Gloda.NOUN_MESSAGE);
     this.catchAllCollection.listener = this;
 
+    // Register us with gloda as an attribute provider so that we can
+    //  distinguish between fully reindexed messages and fastpath indexed
+    //  messages.
+    Gloda._attrProviderOrderByNoun[Gloda.NOUN_MESSAGE].push(this);
+
     // waitingForEvents support
     // (we want this to happen after gloda registers its own listener, and it
     //  does.)
     let notificationService =
       Cc["@mozilla.org/messenger/msgnotificationservice;1"].
       getService(Ci.nsIMsgFolderNotificationService);
     notificationService.addListener(this,
       Ci.nsIMsgFolderNotificationService.msgsClassified);
@@ -383,16 +396,18 @@ var _indexMessageState = {
   /** Expected value of |_workerRecoveredCount| at assertion time */
   expectedWorkerRecoveredCount: null,
   /** Expected value of |_workerFailedToRecoverCount| at assertion time */
   expectedFailedToRecoverCount: null,
   /** Expected value of |_workerCleanedUpCount| at assertion time */
   expectedCleanedUpCount: null,
   /** Expected value of |_workerHadNoCleanUpCount| at assertion time */
   expectedHadNoCleanUpCount: null,
+  /** Exepected value of |_numFullIndexed| at assertion time */
+  expectedNumFullIndexed: null,
 
   /** The number of times a worker had a recover helper and it recovered. */
   _workerRecoveredCount: 0,
   /**
    * The number of times a worker had a recover helper and it did not recover.
    */
   _workerFailedToRecoverCount: 0,
   /**
@@ -553,24 +568,32 @@ var _indexMessageState = {
                     "Expected", this.expectedCleanedUpCount,
                     "actual", this._workerCleanedUpCount]);
     if (this.expectedHadNoCleanUpCount != null &&
         this.expectedHadNoCleanUpCount != this._workerHadNoCleanUpCount)
       mark_failure(["Expected worker-had-no-cleanup count did not match actual!",
                     "Expected", this.expectedHadNoCleanUpCount,
                     "actual", this._workerHadNoCleanUpCount]);
 
+    if (this.expectedNumFullIndexed != null &&
+        this.expectedNumFullIndexed != this._numFullIndexed)
+      mark_failure(["Expected number of fully indexed messages did not match.",
+                    "Expected", this.expectedNumFullIndexed,
+                    "actual", this._numFullIndexed]);
+
     this._glodaMessagesByMessageId = {};
     this._glodaDeletionsByMessageId = {};
 
     this._workerRecoveredCount = 0;
     this._workerFailedToRecoverCount = 0;
     this._workerCleanedUpCount = 0;
     this._workerHadNoCleanUpCount = 0;
 
+    this._numFullIndexed = 0;
+
     // make sure xpcshell head.js knows we tested something
     _passedChecks++;
   },
 
   /*
    * Our catch-all collection listener.  Any time a new message gets indexed,
    *  we should receive an onItemsAdded call.  Any time an existing message
    *  gets reindexed, we should receive an onItemsModified call.  Any time an
@@ -616,16 +639,33 @@ var _indexMessageState = {
         mark_failure(
           ["Gloda message", item, "already deleted once since the last" +
             "wait_for_gloda_indexer call!"]);
 
       this._glodaDeletionsByMessageId[item.headerMessageID] = item;
     }
   },
 
+  /**
+   * The number of messages that were fully (re)indexed using
+   *  Gloda.grokNounItem.
+   */
+  _numFullIndexed: 0,
+
+  providerName: "glodaTestHelper:fakeProvider",
+  /**
+   * Fake attribute provider processing function so we can distinguish
+   *  between fully reindexed messages and fast-path modified messages.
+   */
+  process: function(aItem, aRawReps, aIsConceptuallyNew, aCallbackHandle) {
+    this._numFullIndexed++;
+
+    yield Gloda.kWorkDone;
+  },
+
   _numItemsAdded : 0,
 
   /**
    * Gloda indexer listener, used to know when all active indexing jobs have
    *  completed so that we can try and process all the things that should have
    *  been processed.
    */
   onIndexNotification: function(aStatus, aPrettyName, aJobIndex,
--- a/mailnews/test/resources/folderEventLogHelper.js
+++ b/mailnews/test/resources/folderEventLogHelper.js
@@ -57,16 +57,17 @@ function registerFolderEventLogHelper() 
   let notificationService =
     Cc["@mozilla.org/messenger/msgnotificationservice;1"]
       .getService(Ci.nsIMsgFolderNotificationService);
   notificationService.addListener(_folderEventLogHelper_msgFolderListener,
         Ci.nsIMsgFolderNotificationService.msgAdded |
         Ci.nsIMsgFolderNotificationService.msgsClassified |
         Ci.nsIMsgFolderNotificationService.msgsDeleted |
         Ci.nsIMsgFolderNotificationService.msgsMoveCopyCompleted |
+        Ci.nsIMsgFolderNotificationService.msgKeyChanged |
         Ci.nsIMsgFolderNotificationService.folderDeleted |
         Ci.nsIMsgFolderNotificationService.folderMoveCopyCompleted |
         Ci.nsIMsgFolderNotificationService.folderRenamed |
         Ci.nsIMsgFolderNotificationService.itemEvent);
 }
 
 /**
  * nsIMsgFolderListener implementation to logHelper events that gloda cares
@@ -114,16 +115,21 @@ let _folderEventLogHelper_msgFolderListe
       for each (let msgHdr in fixIterator(aDestMsgs.enumerate(),
                                           Components.interfaces.nsIMsgDBHdr)) {
         args.push(msgHdr);
       }
     }
     mark_action("msgEvent", "msgsMoveCopyCompleted", args);
   },
 
+  msgKeyChanged: function felh_msgKeyChanged(aOldMsgKey, aNewMsgHdr) {
+    let args = ["old key", aOldMsgKey, "new header", aNewMsgHdr];
+    mark_action("msgEvent", "msgKeyChanged", args);
+  },
+
   folderAdded: function felh_folderAdded(aFolder) {
     mark_action("msgEvent", "folderAdded", [aFolder]);
   },
 
   folderDeleted: function felh_folderDeleted(aFolder) {
     mark_action("msgEvent", "folderDeleted", [aFolder]);
   },
 
--- a/mailnews/test/resources/messageInjection.js
+++ b/mailnews/test/resources/messageInjection.js
@@ -857,23 +857,31 @@ function wait_for_message_injection() {
     return false;
   else
     return true;
 }
 
 /**
  * Asynchronously move messages in the given set to the destination folder.
  *
- * The IMAP case is much more complex, at least in the unit testing world:
- * XXX We have to force an update of the source folder because the fake
- *  server only allows one connection and that one connection currently
- *  is focused on destFolder; we have to force an update of srcFolder to
- *  get the move to actually hit the IMAP server.
+ * For IMAP moves we force an update of the source folder and then the
+ *  destination folder.  This ensures that any (pseudo-)offline operations in
+ *  the source folder have had a chance to run and that we have seen the changes
+ *  in the target folder.
+ * We additionally cause all of the message bodies to be downloaded in the
+ *  target folder if the folder has the Offline flag set.
+ *
+ * @param aSynMessageSet The messages to move.
+ * @param aDestFolder The target folder.
+ * @param [aAllowUndo=false] Should we generate undo operations and, as a
+ *     side-effect, offline operations?  (The code uses undo operations as
+ *     a proxy-indicator for it coming from the UI and therefore performing
+ *     pseudo-offline operations instead of trying to do things online.)
  */
-function async_move_messages(aSynMessageSet, aDestFolder) {
+function async_move_messages(aSynMessageSet, aDestFolder, aAllowUndo) {
   mark_action("messageInjection", "moving messages", aSynMessageSet.msgHdrList);
   return async_run({func: function () {
       // we need to make sure all folder promises are fulfilled
       yield wait_for_async_promises();
       // and then we can make sure we have the actual folder
       let realDestFolder = get_real_injection_folder(aDestFolder);
 
       let copyService = Cc["@mozilla.org/messenger/messagecopyservice;1"]
@@ -888,17 +896,17 @@ function async_move_messages(aSynMessage
         //  destination headers.
         if (!message_injection_is_local())
           _messageInjectionSetup.notifyListeners(
             "onMovingMessagesWithoutDestHeaders", [realDestFolder]);
 
         copyService.CopyMessages(folder, xpcomHdrArray,
                                  realDestFolder, /* move */ true,
                                  asyncCopyListener, null,
-                                 /* do not allow undo, leaks */ false);
+                                 Boolean(aAllowUndo));
         // update the synthetic message set's folder entry...
         aSynMessageSet._folderSwap(folder, realDestFolder);
         yield false;
 
         // IMAP special case per function doc...
         if (!message_injection_is_local()) {
           mark_action("messageInjection",
                       "forcing update of folder so IMAP move issued",