GNOME Bugzilla – Bug 312178
KMail backend
Last modified: 2005-09-26 18:00:11 UTC
First try at KMail maildir backend. To make it extensible and add support for any maildir mail client later, wrote it in two files. KMaildirCrawler.cs - contains the generic MaildirCrawler and KMaildirCrawler (extending MaildirCrawler) which implements the specific details of KMail maildir format. MaildirQueryable.cs - which asks MaildirCrawlers for the folders to watch and index. It is primarily a wrapper around FilterMail. This is only a first try. There are lots of comments in the file and lots of debug messages, pls ignore them. Add these files to beagled/MaildirQueryable.
Created attachment 50037 [details] [review] beagled/MaildirQueryable/MaildirQueryable.cs This file is made similar to the backends in beagle-0.12. There might be some changes necessary in lines of recent developments.
Created attachment 50038 [details] [review] beagled/MaildirQueryable/KMaildirCrawler.cs Made against beagle-0.12.
Sorry for the typo. Add the files in beagled/MaildirQueryable/
*** Bug 311755 has been marked as a duplicate of this bug. ***
Created attachment 50299 [details] [review] new version This is based on the discussion in the thread http://mail.gnome.org/archives/dashboard-hackers/2005-August/msg00023.html
Created attachment 50330 [details] [review] Inotify problem fixed and added deletion support Fixed a bug when Inotify is not present and added support for deletion of mails . Pls try and report.
Dont review or try this. I recently came to know and started using the disconnected imap feature of kmail - so now we need to index two trees - one local and another dimap. Additionally I noticed that the mail folders can be stored in maildir/mbox format and they can be used interchangably. Adding support for these required me to rewrite the backend completely (based on the structure of Evo mail backend). It is unavoidable now to have separate crawler class and separate indexable generator classes. Work in progress. Keep watching the space - I will need review and testing once done :)
Created attachment 51406 [details] [review] + beagled/KmailQueryable/KMailDriver.cs
Created attachment 51407 [details] [review] + beagled/KmailQueryable/KMailIndexableGenerator.cs
Created attachment 51408 [details] [review] + beagled/KmailQueryable/MailCrawler.cs
Redid the backend. The queryable needs to watch multiple locations, each of which can contain an arbitrary mix of mbox and maildir mail folders. The design of Evo mail backend suits this case so the new files follow the design of EvolutionMailDriver, with some changes as needed. As a result, now we have 3 files. * KMailDriver - main indexable - which is kind of dummy - just creates seperate crawlers for different mail locations * KMailIndexableGenerator - Maildir and Mbox indexable generator * MailCrawler - does the bulk of the actual indexing work TODO: parse kmailrc ... kinda boring :( TODO: parsing the .index files to find out which mails are deleted (done by Camel in evo) - pointers ?
Created attachment 51562 [details] [review] KmailQueryable\KMailDriver.cs Some cleanup. Waiting for review on this version.
Created attachment 51563 [details] [review] KmailQueryable\KMailIndexer.cs Some cleanup. Pls review and test this version.
Created attachment 51564 [details] [review] KmailQueryable\KMailIndexableGenerator.cs Some cleanup and better use of mboxlastoffset. Pls review.
Ok, finally got a chance to look this over. Overall, very good. I only have a few nitpicks: * Rename KMailDriver.cs to KMailQueryable.cs and change the namespace as well. I suspect you copied this from the Evo backend, which calls itself a Driver as a historial artifact (and the fact that you can't rename files in CVS, argh.) * Let's move the changelogs out of the source files and into the toplevel ChangeLog file. Right now we're not requiring ChangeLog entries, but we will start at some point. * Could the directory that GuessLocalFolder() calls DirectoryInfo.GetFiles() be very big (regardless of the wildcard)? If so, use our specialided DirectoryWalker class. Ditto for Crawl() in KMailIndexer.cs. * In OnInotifyEvent() in KMailIndexer.cs, I would recommend moving the code which actually creates an indexable task out into a separate function (like AddIndexable()) and then just short circuit all of the if clauses, so make the function easier to read. (Some return and others fall through.) * In Watch(), definitely use our DirectoryWalker class or else you'll use a ton of CPU and memory if there are many files. * "Array _mbox_files = mbox_files.ToArray ();" with the comment "copy the contents as mail_directories, mbox_files might change due to async events". This is pretty gross. Are you worried about the elements being removed? If so, make a copy along the lines of "ArrayList mbox_files_copy = new ArrayList (mbox_files);" rather than using Array. (Or strongly type the array) * I see "if (folder_name == Queryable.sentmail_foldername)". By convention public fields are StudlyCapped. So this should probably beb "if (folder_name == Queryable.SentMailFolderName)". * I am pretty sure the ParentUri thing will work for mboxes, but if it causes problems I recently added a special remove task which will remove anything that matches a property, so you could remove any item which has a parent folder property of a certain name. But I think this will work too * In GetMboxFile, "filename.Remove (pos, 6).Remove (0,1);" That's pretty ugly, and not immediately obvious on inspection. Can you change it to be filename.Substring (1, pos - 2)? * Update_directories() should be UpdateDirectories(). * Try to avoid passing around Arrays, and instead use ICollection. * I think that the indexable generator's AddFiles stuff will need to be refactored a bit. I have some *huge* maildir directories, and as I mentioned before DirectoryInfo.GetFiles() sucks bad. You'll need to use DirectoryWalker.GetFiles(), which returns an IEnumerable (which actually probably makes things a little easier). * A lot of this code is derived from the Evo backend, so you should add "Copyright (C) 2005 Novell, Inc." to the comments.
Phew! In CVS, at last. Joe, thanks a ton.
I can confirm that the CVS version works nicely. Sweet :-) Thanks for the great work guys!