Bug 467685 - Gloda message indexer and grokNounItem concept of "new" is insufficient, results in db errors. v1 _log fix, plus split concept of new for indexer/grokNounItem, add test from disk, documentation. r=bienvenu
authorDavid Ascher <david.ascher@gmail.com>
Sun, 07 Dec 2008 01:35:51 -0800
changeset 1356 45c5c6bbbac8360e591b58d3bdbc1c4b9b51ebb3
parent 1355 2c50e630c8a5043d2f7ac5046cb066cdda44219a
child 1357 cba1fda2f84d63185dd5ca964ec0762833b20f2e
push idunknown
push userunknown
push dateunknown
reviewersbienvenu
bugs467685
Bug 467685 - Gloda message indexer and grokNounItem concept of "new" is insufficient, results in db errors. v1 _log fix, plus split concept of new for indexer/grokNounItem, add test from disk, documentation. r=bienvenu
mailnews/db/gloda/modules/datastore.js
mailnews/db/gloda/modules/gloda.js
mailnews/db/gloda/modules/index_ab.js
mailnews/db/gloda/modules/indexer.js
mailnews/db/gloda/test/resources/glodaTestHelper.js
mailnews/db/gloda/test/unit/test_index_messages.js
--- a/mailnews/db/gloda/modules/datastore.js
+++ b/mailnews/db/gloda/modules/datastore.js
@@ -1223,17 +1223,17 @@ var GlodaDatastore = {
         this._pendingAsyncCompletedListener();
         this._pendingAsyncCompletedListener = null;
       }
     }
   },
   _asyncTrackerListener: {
     handleResult: function () {},
     handleError: function(aError) {
-        this._log.error("got error in _asyncTrackerListener.handleError(): " +
+        GlodaDatastore._log.error("got error in _asyncTrackerListener.handleError(): " +
                         aError.result + ": " + aError.message);
     },
     handleCompletion: function () {
       try {
         // the helper method exists because the other classes need to call it too
         GlodaDatastore._asyncCompleted();
       }
       catch (e) {
--- a/mailnews/db/gloda/modules/gloda.js
+++ b/mailnews/db/gloda/modules/gloda.js
@@ -396,17 +396,17 @@ var Gloda = {
       let identity = GlodaDatastore.createIdentity(contact.id, contact,
         "email", address, /* description */ "", /* relay? */ false);
       contact._identities = [identity];
       
       // give the address book indexer a chance if we have a card.
       // (it will fix-up the name based on the card as appropriate)
       if (card)
         yield aCallbackHandle.pushAndGo(
-          Gloda.grokNounItem(contact, card, true, aCallbackHandle));
+          Gloda.grokNounItem(contact, card, true, true, aCallbackHandle));
       else // grokNounItem will issue the insert for us...
         GlodaDatastore.insertContact(contact);
 
       for (let iResList = 1; iResList < nameAndResultLists.length; iResList++) {
         nameAndResultLists[iResList].push(identity);
       }
     }
 
@@ -1440,59 +1440,80 @@ var Gloda = {
     collection.query = query;
     GlodaCollectionManager.registerCollection(collection);
     return collection;
   },
 
   /**
    * Populate a gloda representation of an item given the thus-far built
    *  representation, the previous representation, and one or more raw
-   *  representations.
+   *  representations.  The attribute providers/optimizers for the given noun
+   *  type are invoked, allowing them to contribute/alter things.  Following
+   *  that, we build and persist our attribute representations.
    *
    * The result of the processing ends up with attributes in 3 different forms:
    * - Database attribute rows (to be added and removed).
    * - In-memory representation.
    * - JSON-able representation.
+   * 
+   * @param aItem The noun instance you want processed.
+   * @param aRawReps An opaque dictionary that we pass to the attribute
+   *     providers.  There is a(n implied) contract between the caller of
+   *     grokNounItem for a given noun type and the attribute providers for
+   *     that noun type, and we have nothing to do with it.
+   * @param aIsConceptuallyNew Is the item "new" in the sense that it would
+   *     never have been visible from within user code?  This translates into
+   *     whether this should trigger an itemAdded notification or an
+   *     itemModified notification.
+   * @param aIsRecordNew Is the item "new" in the sense that we should INSERT
+   *     a record rather than UPDATE-ing a record.  For example, when dealing
+   *     with messages where we may have a ghost, the ghost message is not a
+   *     new record, but is conceptually new.
+   * @param aCallbackHandle The GlodaIndexer-style callback handle that is being
+   *     used to drive this processing in an async fashion.  (See
+   *     GlodaIndexer._callbackHandle).
+   * @param aDoCache Should we allow this item to be contributed to its noun
+   *     cache?
    */
-  grokNounItem: function gloda_ns_grokNounItem(aItem, aRawReps, aIsNew,
-      aCallbackHandle, aDoCache) {
+  grokNounItem: function gloda_ns_grokNounItem(aItem, aRawReps,
+      aIsConceptuallyNew, aIsRecordNew, aCallbackHandle, aDoCache) {
     let itemNounDef = this._nounIDToDef[aItem.NOUN_ID];
     let attribsByBoundName = itemNounDef.attribsByBoundName;
     
     this._log.info(" ** grokNounItem: " + itemNounDef.name);
     
     let addDBAttribs = [];
     let removeDBAttribs = [];
     
     let jsonDict = {};
     
     let aOldItem;
-    if (aIsNew) // there is no old item if we are new.
+    if (aIsConceptuallyNew) // there is no old item if we are new.
       aOldItem = {};
     else {
       aOldItem = aItem;
       // we want to create a clone of the existing item so that we can know the
       //  deltas that happened for indexing purposes
       aItem = aItem._clone();
     }
   
     // Have the attribute providers directly set properties on the aItem
     let attrProviders = this._attrProviderOrderByNoun[aItem.NOUN_ID];
     for (let iProvider = 0; iProvider < attrProviders.length; iProvider++) {
       this._log.info("  * provider: " + attrProviders[iProvider].providerName);
       yield aCallbackHandle.pushAndGo(
-        attrProviders[iProvider].process(aItem, aRawReps, aIsNew,
+        attrProviders[iProvider].process(aItem, aRawReps, aIsConceptuallyNew,
                                          aCallbackHandle));
     }
     
     let attrOptimizers = this._attrOptimizerOrderByNoun[aItem.NOUN_ID];
     for (let iProvider = 0; iProvider < attrOptimizers.length; iProvider++) {
       this._log.info("  * optimizer: " + attrOptimizers[iProvider].providerName);
       yield aCallbackHandle.pushAndGo(
-        attrOptimizers[iProvider].optimize(aItem, aRawReps, aIsNew,
+        attrOptimizers[iProvider].optimize(aItem, aRawReps, aIsConceptuallyNew,
                                            aCallbackHandle));
     }
     
     this._log.info(" ** done with providers.");
   
     // Iterate over the attributes on the item
     for each (let [key, value] in Iterator(aItem)) {
       // ignore keys that start with underscores, they are private and not
@@ -1585,17 +1606,17 @@ var Gloda = {
       
         // replace the old value with the new values... (the 'old' item is
         //  canonical)
         aOldItem[key] = value; 
       }
       // no old value, all values are new
       else {
         // the 'old' item is still the canonical one; update it
-        if (!aIsNew)
+        if (!aIsConceptuallyNew)
           aOldItem[key] = value;
         // add the db reps on the new values
         if (attrib.singular)
           value = [value];
         addDBAttribs.push.apply(addDBAttribs,
                                 attribDB.convertValuesToDBAttributes(value));
       }
     }
@@ -1623,33 +1644,33 @@ var Gloda = {
       // delete these from the old item, as the old item is canonical, and
       //  should no longer have these values
       delete aOldItem[key];
     }
     
     aItem._jsonText = this._json.encode(jsonDict);
     this._log.debug("  json text: " + aItem._jsonText);
     
-    if (aIsNew) {
+    if (aIsRecordNew) {
       this._log.debug(" inserting item");
       itemNounDef.objInsert.call(itemNounDef.datastore, aItem);
     }
     else {
       this._log.debug(" updating item");
       itemNounDef.objUpdate.call(itemNounDef.datastore, aItem);
     }
     
     this._log.debug(" adjusting attributes, add: " + addDBAttribs + " rem: " +
         removeDBAttribs);
     itemNounDef.dbAttribAdjuster.call(itemNounDef.datastore, aItem,
       addDBAttribs, removeDBAttribs);
     
     // Cache ramifications...
     if (aDoCache === undefined || aDoCache) {
-      if (aIsNew)
+      if (aIsConceptuallyNew)
         GlodaCollectionManager.itemsAdded(aItem.NOUN_ID, [aItem]);
       else
         GlodaCollectionManager.itemsModified(aOldItem.NOUN_ID, [aOldItem]);
     }
     
     this._log.debug(" done grokking.");
     
     yield this.kWorkDone;
--- a/mailnews/db/gloda/modules/index_ab.js
+++ b/mailnews/db/gloda/modules/index_ab.js
@@ -86,17 +86,18 @@ var GlodaABIndexer = {
       let identityCollection = query.getCollection(aCallbackHandle);
       yield Gloda.kWorkAsync;
       
       if (identityCollection.items.length) {
         let identity = identityCollection.items[0];
   
         this._log.debug("Found identity, processing card.");
         yield aCallbackHandle.pushAndGo(
-            Gloda.grokNounItem(identity.contact, card, false, aCallbackHandle));
+            Gloda.grokNounItem(identity.contact, card, false, false,
+                               aCallbackHandle));
         this._log.debug("Done processing card.");
       }
     }
     
     yield GlodaIndexer.kWorkDone;
   },
   
   initialSweep: function() {
--- a/mailnews/db/gloda/modules/indexer.js
+++ b/mailnews/db/gloda/modules/indexer.js
@@ -2246,28 +2246,29 @@ var GlodaIndexer = {
       let allAttachmentNames = [att.name for each
                                 ([i, att] in Iterator(aMimeMsg.allAttachments))
                                 if (att.isRealAttachment)];
       // we need some kind of delimeter for the names.  we use a newline.
       if (allAttachmentNames)
         attachmentNames = allAttachmentNames.join("\n");
     } 
     
-    let isNew;
+    let isConceptuallyNew, isRecordNew;
     if (curMsg === null) {
       curMsg = this._datastore.createMessage(aMsgHdr.folder,
                                              aMsgHdr.messageKey,                
                                              conversationID,
                                              aMsgHdr.date,
                                              aMsgHdr.messageId);
       curMsg._conversation = conversation;
-      isNew = true;
+      isConceptuallyNew = isRecordNew = true;
     }
     else {
-      isNew = (curMsg._folderID === null); // aka was-a-ghost
+      isRecordNew = false;
+      isConceptuallyNew = (curMsg._folderID === null); // aka was-a-ghost
       // (messageKey can be null if it's not new in the move-case)
       curMsg._folderID = this._datastore._mapFolder(aMsgHdr.folder).id;
       curMsg._messageKey = aMsgHdr.messageKey;
       curMsg.date = new Date(aMsgHdr.date / 1000); 
       // note: we are assuming that our matching logic is flawless in that
       //  if this message was not a ghost, we are assuming the 'body'
       //  associated with the id is still exactly the same.  It is conceivable
       //  that there are cases where this is not true.
@@ -2282,27 +2283,27 @@ var GlodaIndexer = {
       else {
         this._log.warn("Have aMimeMsg but not bodyPlain?");
       }
     }
     else {
       this._log.warn("aMimeMsg went away?");
     }
     
-    if (isNew) {
+    if (isConceptuallyNew) {
       curMsg._isNew = true;
       curMsg._subject = aMsgHdr.mime2DecodedSubject;
       curMsg._attachmentNames = attachmentNames;
     }
     
     yield aCallbackHandle.pushAndGo(
         Gloda.grokNounItem(curMsg,
             {header: aMsgHdr, mime: aMimeMsg,
              bodyLines: curMsg._bodyLines, content: curMsg._content},
-            isNew,
+            isConceptuallyNew, isRecordNew,
             aCallbackHandle));
     
     delete curMsg._bodyLines;
     delete curMsg._content;
     delete curMsg._isNew;
     delete curMsg._subject;
     delete curMsg._attachmentNames;
     
--- a/mailnews/db/gloda/test/resources/glodaTestHelper.js
+++ b/mailnews/db/gloda/test/resources/glodaTestHelper.js
@@ -47,16 +47,18 @@ gPrefs.setBoolPref("mailnews.database.gl
 gPrefs.setBoolPref("mailnews.database.global.indexer.perform_initial_sweep",
     false);
 // yes to debug output
 gPrefs.setBoolPref("mailnews.database.global.logging.dump", true);
 
 // -- Import our modules
 Components.utils.import("resource://app/modules/gloda/public.js");
 Components.utils.import("resource://app/modules/gloda/indexer.js");
+Components.utils.import("resource://app/modules/gloda/datastore.js");
+Components.utils.import("resource://app/modules/gloda/collection.js");
 
 // -- Add a logger listener that throws when we give it a warning/error.
 Components.utils.import("resource://app/modules/gloda/log4moz.js");
 let throwingAppender = new Log4Moz.ThrowingAppender(do_throw);
 throwingAppender.level = Log4Moz.Level.Warn;
 Log4Moz.repository.rootLogger.addAppender(throwingAppender);
 
 /**
@@ -916,8 +918,58 @@ function glodaHelperRunTests(aTests, aLo
   do_timeout(aLongestTestRunTimeConceivableInSecs * 1000,
       "do_throw('Timeout running test, and we want you to have the log.');");
   
   imsInit();
   glodaHelperTests = aTests;
   glodaHelperIterator = _gh_test_iterator();
   next_test();
 }
+
+/**
+ * Wipe out almost everything from the clutches of the GlodaCollectionManager.
+ * By default, it is caching things and knows about all the non-GC'ed
+ *  collections.  Tests may want to ensure that their data is loaded from disk
+ *  rather than relying on the cache, and so, we exist.
+ * The exception to everything is that Gloda's concept of myContact and
+ *  myIdentities needs to have its collections still be reachable or invariants
+ *  are in danger of being "de-invarianted".
+ * The other exception to everything are any catch-all-collections used by our
+ *  testing/indexing process.  We don't scan for them, we just hard-code their
+ *  addition if they exist.
+ */
+function nukeGlodaCachesAndCollections() {
+  // explode if the GlodaCollectionManager somehow doesn't work like we think it
+  //  should.  (I am reluctant to put this logic in there, especially because
+  //  knowledge of the Gloda contact/identity collections simply can't be known
+  //  by the colleciton manager.)
+  if ((GlodaCollectionManager._collectionsByNoun === undefined) ||
+      (GlodaCollectionManager._cachesByNoun === undefined))
+    // we don't check the Gloda contact/identities things because they might not
+    //  get initialized if there are no identities, which is the case for our
+    //  unit tests right now...
+    do_throw("Try and remember to update the testing infrastructure when you " +
+             "change things!");
+  
+  // we can just blow away the known collections
+  GlodaCollectionManager._collectionsByNoun = {};
+  // but then we have to put the myContact / myIdentities junk back
+  if (Gloda._myContactCollection) {
+    GlodaCollectionManager.registerCollection(Gloda._myContactCollection);
+    GlodaCollectionManager.registerCollection(Gloda._myIdentitiesCollection);
+  }
+  // don't forget our testing catch-all collection!
+  if (indexMessageState.catchAllCollection) {
+    // empty it out in case it has anything in it
+    indexMessageState.catchAllCollection.clear();
+    // and now we can register it
+    GlodaCollectionManager.registerCollection(
+        indexMessageState.catchAllCollection);
+  }
+  
+  // caches aren't intended to be cleared, but we also don't want to lose our
+  //  caches, so we need to create new ones from the ashes of the old ones.
+  let oldCaches = GlodaCollectionManager._cachesByNoun;
+  GlodaCollectionManager._cachesByNoun = {};
+  for each (let cache in oldCaches) {
+    GlodaCollectionManager.defineCache(cache._nounDef, cache._maxCacheSize);
+  }
+}
--- a/mailnews/db/gloda/test/unit/test_index_messages.js
+++ b/mailnews/db/gloda/test/unit/test_index_messages.js
@@ -42,24 +42,32 @@ function test_threading() {
   indexAndPermuteMessages(scenarios.siblingsMissingParent,
                           allMessageInSameConversation,
                           next_test);
 }
 
 /* ===== Fundamental Attributes (per fundattr.js) ===== */
 
 /**
+ * Save the synthetic message created in test_attributes_fundamental for the
+ *  benefit of test_attributes_fundamental_from_disk.
+ */
+var fundamentalSyntheticMessage;
+
+/**
  * Test that we extract the 'fundamental attributes' of a message properly
  *  'Fundamental' in this case is talking about the attributes defined/extracted
  *  by gloda's fundattr.js and perhaps the core message indexing logic itself
  *  (which show up as kSpecial* attributes in fundattr.js anyways.)
  */
 function test_attributes_fundamental() {
   // create a synthetic message
   let smsg = msgGen.makeMessage();
+  // save it off for fundamentalSyntheticMessage
+  fundamentalSyntheticMessage = smsg;
   
   indexMessages([smsg], verify_attributes_fundamental, next_test);
 }
 
 function verify_attributes_fundamental(smsg, gmsg) {
   do_check_eq(gmsg.folderURI, gLocalInboxFolder.URI);
   
   // -- subject
@@ -76,16 +84,44 @@ function verify_attributes_fundamental(s
   // - to
   do_check_eq(smsg.toAddress, gmsg.to[0].value);
   do_check_eq(smsg.toName, gmsg.to[0].contact.name);
   
   // date
   do_check_eq(smsg.date.valueOf(), gmsg.date.valueOf());
 }
 
+/**
+ * We want to make sure that all of the fundamental properties also are there
+ *  when we load them from disk.  Nuke our cache, query the message back up.
+ */
+function test_attributes_fundamental_from_disk() {
+  nukeGlodaCachesAndCollections();
+  
+  GlodaDatastore.getMessagesByMessageID(
+      [fundamentalSyntheticMessage.messageId],
+      verify_attributes_fundamental_from_disk, /* callback this */ null);
+}
+
+/**
+ * We are just a wrapper around verify_attributes_fundamental, adapting the
+ *  return callback from getMessagesByMessageID.
+ * 
+ * @param aGlodaMessageLists This should be [[theGlodaMessage]].
+ */
+function verify_attributes_fundamental_from_disk(aGlodaMessageLists) {
+  do_check_eq(aGlodaMessageLists.length, 1);
+  let glodaMessageList = aGlodaMessageLists[0];
+  do_check_eq(glodaMessageList.length, 1);
+  
+  verify_attributes_fundamental(fundamentalSyntheticMessage,
+                                glodaMessageList[0]);
+  next_test();
+}
+
 /* ===== Explicit Attributes (per explattr.js) ===== */
 
 function expl_attr_twiddle_star(aMsgHdr, aDesiredState) {
   aMsgHdr.markFlagged(aDesiredState);
 }
 
 function expl_attr_verify_star(smsg, gmsg, aExpectedState) {
   do_check_eq(gmsg.starred, aExpectedState);
@@ -165,14 +201,15 @@ function test_message_deletion() {
 }
 
 /* ===== Folder Move/Rename/Copy (Single and Nested) ===== */
 
 
 var tests = [
   test_threading,
   test_attributes_fundamental,
+  test_attributes_fundamental_from_disk,
   test_attributes_explicit,
 ];
 
 function run_test() {
   glodaHelperRunTests(tests);
 }