Closed
Bug 467685
Opened 16 years ago
Closed 16 years ago
Gloda message indexer and grokNounItem concept of "new" is insufficient, results in db errors
Categories
(MailNews Core :: Database, defect, P1)
MailNews Core
Database
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: asuth, Assigned: asuth)
References
Details
Attachments
(1 file)
(deleted),
patch
|
Bienvenu
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Updated•16 years ago
|
Priority: -- → P1
Assignee | ||
Comment 1•16 years ago
|
||
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.
Attachment #351668 -
Flags: review?(bienvenu)
Assignee | ||
Updated•16 years ago
|
Whiteboard: [has patch][needs review bienvenu]
Updated•16 years ago
|
Attachment #351668 -
Flags: review?(bienvenu) → review+
Comment 2•16 years ago
|
||
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.
Assignee | ||
Comment 3•16 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Whiteboard: [has patch][needs review bienvenu]
You need to log in
before you can comment on or make changes to this bug.
Description
•