Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Ensure IM conversations are reindexed when the gloda database is reset

RESOLVED FIXED in Thunderbird 17.0

Status

Thunderbird
Instant Messaging
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mconley, Assigned: mconley)

Tracking

15 Branch
Thunderbird 17.0
x86
All

Thunderbird Tracking Flags

(thunderbird15 fixed, thunderbird16 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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.
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.
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.
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.
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
Last Resolved: 5 years ago
tracking-thunderbird15: --- → ?
tracking-thunderbird16: --- → ?
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+
comm-aurora: https://hg.mozilla.org/releases/comm-aurora/rev/9ffbd5c6d544
comm-beta: https://hg.mozilla.org/releases/comm-beta/rev/24698041be04
status-thunderbird15: --- → fixed
status-thunderbird16: --- → fixed
tracking-thunderbird15: ? → ---
tracking-thunderbird16: ? → ---
You need to log in before you can comment on or make changes to this bug.