User-visible problem: Ignoring threads in KMail doesn't work properly, some messages get spuriously un-ignored on next sync. Findings: - Even though the messages get the $IGNORED flag set correctly in DB, they don't have it on the IMAP server. - IMAP communication trace shows that for the affected messages, two commands are submitted: STORE +FLAGS ($IGNORED) STORE -FLAGS ($IGNORED) This is the result of ChangeItemsFlagsTask getting run with addedFlags and removedFlags both containing $IGNORED. Root cause (found using GDB on the imap resource and server, following the notification flow): DataStore::setItemsFlags doesn't properly handle setting a flag that is already set. Calling it with flags = $IGNORED makes it delete and re-add the $IGNORED flag in the DB (which is harmless) and emit itemsFlagsChanged with both addedFlags and removedFlags containing $IGNORED. Quick hacky fix: Before emitting the notification, remove flags that are both in addedFlags and removedFlags from removedFlags. However, I see a problem here in need of discussion: The way DataStore::setItemsFlags handles the DB isn't optimal either - why delete and re-insert the flag that's already there? It would possibly be better to just delete the flags that are getting removed and insert the new ones, thus doing nothing if nothing needs to be done. However, this was basically the state before commit http://quickgit.kde.org/?p=akonadi.git&a=commit&h=ee8b3d209553351c338cf5b069a2c0bda0e25298 , so I'm wondering if that's a good idea. Reproducible: Always
Patch posted for review at http://git.reviewboard.kde.org/r/114241/
Git commit daa2e624c64cafb6ec9ac6f2a619a896588eb254 by Tomáš Trnka. Committed on 01/12/2013 at 17:39. Pushed by ttrnka into branch 'master'. Don't do redundant flag changes in setItemsFlags Instead of removing all flags and then adding the requested ones back, figure out which flags are actually getting added or removed and only apply those changes. Everything is still carried out in up to two DB queries. Additionally, don't send out the change notification if nothing has actually changed. FIXED-IN:1.11.1 REVIEW:114241 M +29 -29 server/src/storage/datastore.cpp http://commits.kde.org/akonadi/daa2e624c64cafb6ec9ac6f2a619a896588eb254
Git commit faabb920d8efe5c35bba070396b8df608c5a910a by Tomáš Trnka. Committed on 01/12/2013 at 17:39. Pushed by ttrnka into branch '1.11'. Don't do redundant flag changes in setItemsFlags Instead of removing all flags and then adding the requested ones back, figure out which flags are actually getting added or removed and only apply those changes. Everything is still carried out in up to two DB queries. Additionally, don't send out the change notification if nothing has actually changed. FIXED-IN:1.11.1 REVIEW:114241 M +29 -29 server/src/storage/datastore.cpp http://commits.kde.org/akonadi/faabb920d8efe5c35bba070396b8df608c5a910a