Closed Bug 782325 Opened 8 years ago Closed 8 years ago

Ensure IM conversations are reindexed when the gloda database is reset

Categories

(Thunderbird :: Instant Messaging, defect)

15 Branch
x86
All
defect
Not set
normal

Tracking

(thunderbird15 fixed, thunderbird16 fixed)

RESOLVED FIXED
Thunderbird 17.0
Tracking Status
thunderbird15 --- fixed
thunderbird16 --- fixed

People

(Reporter: mconley, Assigned: mconley)

Details

Attachments

(1 file, 1 obsolete file)

We want to remove the JSON cache when the gloda database is nuked.
(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.
Attached patch Patch v1 (obsolete) — Splinter Review
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.
Assignee: nobody → mconley
Status: NEW → ASSIGNED
Attachment #651835 - Flags: review?(bugmail)
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.
Attachment #651835 - Flags: review?(bugmail) → review-
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.
Attached patch Patch v2Splinter Review
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.
Attachment #651835 - Attachment is obsolete: true
Attachment #651851 - Flags: review?(bugmail)
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.)
Attachment #651851 - Flags: review?(bugmail) → review+
Comment on attachment 651851 [details] [diff] [review]
Patch v2

Awesome - thanks for the reviews, asuth!
Attachment #651851 - Flags: approval-comm-beta?
Attachment #651851 - Flags: approval-comm-aurora?
comm-central: https://hg.mozilla.org/comm-central/rev/af2a5b505284
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 17.0
Attachment #651851 - Flags: approval-comm-beta?
Attachment #651851 - Flags: approval-comm-beta+
Attachment #651851 - Flags: approval-comm-aurora?
Attachment #651851 - Flags: approval-comm-aurora+
You need to log in before you can comment on or make changes to this bug.