467685 - Gloda message indexer and grokNounItem concept of "new" is insufficient, results in db errors

Status: RESOLVED FIXED

(MailNews Core :: Database, defect, P1)

Description

[Andrew Sutherland [:asuth] (he/him)]

Bug 466731's addition of logging of failed database statements (which in turn produces a unit test error) helpfully reveals that we were screwing up the message updating when we indexed a message that previously had existed as a "ghost".

The net effect of this is that the message would not have its folder ID, message key, or javascript representations of extracted attributes updated.  The message attributes and full-text body would be properly set, so you would be able to search for the message correctly, but displaying it would turn out poorly.  (You would be unable to locate the body to stream it, gloda attributes would not be populated, etc.)  Because of the caching/collection manager layer, these deficiencies may not be entirely visible until the message is forced to be loaded from disk.

Although the unit tests were deficient about seeing the database error, this also suggests the unit tests need to at least have a few cases that explicitly verify that what we load from disk is equivalent to what we possess in memory.  Forcing the cache to be cleared and all active collections forgotten about should provide such a test without having to run all our tests in both configurations.

I have a fix for the raw problem but need to update some documentation and the unit tests.

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

Attachment: v1 _log fix, plus split concept of new for indexer/grokNounItem, add test from disk, documentation

I've rounded out the patch with a test case that nukes the cache/collections and checks that the load from disk does what we expect.

This patch also includes the entire 1-line patch from bug 466731 attachment 350245 [details] [diff] [review], because we need that patch to make sure we are doing the right thing (and it needs us to not break unit tests when committed).  As such, this bug will close bug 466731 when this patch (or its successor) is committed.

Comment 2

[David :Bienvenu]

Comment on attachment 351668 [details] [diff] [review]
v1 _log fix, plus split concept of new for indexer/grokNounItem, add test from disk, documentation

this doesn't quite apply cleanly because of the log4moz move, but once I fixed that, the tests worked.

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

Checked in: http://hg.mozilla.org/comm-central/rev/45c5c6bbbac8