Last Comment Bug 782325 - Ensure IM conversations are reindexed when the gloda database is reset
: Ensure IM conversations are reindexed when the gloda database is reset
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Instant Messaging (show other bugs)
: 15 Branch
: x86 All
: -- normal (vote)
: Thunderbird 17.0
Assigned To: Mike Conley (:mconley) - (Needinfo me!)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-13 10:13 PDT by Mike Conley (:mconley) - (Needinfo me!)
Modified: 2012-08-20 07:17 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed


Attachments
Patch v1 (7.84 KB, patch)
2012-08-14 11:16 PDT, Mike Conley (:mconley) - (Needinfo me!)
bugmail: review-
Details | Diff | Splinter Review
Patch v2 (10.67 KB, patch)
2012-08-14 12:20 PDT, Mike Conley (:mconley) - (Needinfo me!)
bugmail: review+
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑beta+
Details | Diff | Splinter Review

Description Mike Conley (:mconley) - (Needinfo me!) 2012-08-13 10:13:20 PDT
We want to remove the JSON cache when the gloda database is nuked.
Comment 1 Andrew Sutherland [:asuth] 2012-08-13 14:50:49 PDT
(followup to IRC discussion:) Yes, since SQLite uses schema_version internally and mozStorage/gloda uses user_version, I think the preference is the easiest way to handle associating a generated id for the gloda database so that dependent data like this JSON cache can know when to be blown away.
Comment 2 Mike Conley (:mconley) - (Needinfo me!) 2012-08-14 11:16:29 PDT
Created attachment 651835 [details] [diff] [review]
Patch v1

This patch makes GlodaDatastore stash a UUID string in mailnews.database.global.datastore.id when the database is created, or if the GlodaDatastore is initted and no such pref value exists.

The IM indexer now stores this datastoreID in the JSON cache, and checks for it when doing the initial folder sweep. If the datastoreID from the JSON cache does not match the one that is stored in preferences, this means that the cache is no longer valid, so it is ignored and eventually overwritten.
Comment 3 Andrew Sutherland [:asuth] 2012-08-14 11:33:42 PDT
Comment on attachment 651835 [details] [diff] [review]
Patch v1

The logic is good and I am super-happy about your detailed comments, but I would strongly prefer that Gloda, GlodaDatastore, or GlodaIndexer be the interface to the uuid rather than having index_im access the pref directly.  Specifically I'm concerned that since it lives in a different folder and cargo culting is rife in our codebase and extensions that it's safest to have a more explicit interface.
Comment 4 Andrew Sutherland [:asuth] 2012-08-14 11:36:37 PDT
Also, I think you could add a unit test for this logic pretty easily to test_corrupt_database.js.  Specifically, set the preference to a made-up UUID at the start of the file before gloda gets bootstrapped.  Then, near the end of the test, check that the exposed variable is now a new UUID and also directly probe that the preference has that same value.
Comment 5 Mike Conley (:mconley) - (Needinfo me!) 2012-08-14 12:20:13 PDT
Created attachment 651851 [details] [diff] [review]
Patch v2

Thanks for the review, asuth!

Ok, the datastoreID is now accessed via a getter in Gloda, and the IM indexer has been updated accordingly. I've also augmented the current test in test_corrupt_database.js to check on our UUID regeneration behaviour.
Comment 6 Andrew Sutherland [:asuth] 2012-08-14 13:48:57 PDT
Comment on attachment 651851 [details] [diff] [review]
Patch v2

Awesome, thanks!  Bonus points for checking the new uuid length!  (Redeemable in snacks at your local Mozilla Space.)
Comment 7 Mike Conley (:mconley) - (Needinfo me!) 2012-08-14 13:51:42 PDT
Comment on attachment 651851 [details] [diff] [review]
Patch v2

Awesome - thanks for the reviews, asuth!
Comment 8 Mike Conley (:mconley) - (Needinfo me!) 2012-08-14 13:55:44 PDT
comm-central: https://hg.mozilla.org/comm-central/rev/af2a5b505284
Comment 9 Mike Conley (:mconley) - (Needinfo me!) 2012-08-20 07:17:52 PDT
comm-aurora: https://hg.mozilla.org/releases/comm-aurora/rev/9ffbd5c6d544
comm-beta: https://hg.mozilla.org/releases/comm-beta/rev/24698041be04

Note You need to log in before you can comment on or make changes to this bug.