5
\$\begingroup\$

I'm writing a prototype for an application that uses MailKit and imap to connect to gmail. It's the first time I've used MailKit and I'm a bit stumped around the best way to do this.

Essentially, I want to write an async method hosted in a class library that downloads emails based on a simple header text search and invokes a callback each time so I can update the GUI. My attempt to do this is given in the following method.

public async Task<IList<IEmail>> EmailQueryAsync(string query, Action<IEmail> callBack)
{
    IList<IEmail> result = new List<IEmail>();

    using (ImapClient client = new ImapClient())
    {
        await client.ConnectAsync("imap.gmail.com", 993, true);

        client.AuthenticationMechanisms.Remove("XOAUTH2");

        await client.AuthenticateAsync(this.userName, this.password);

        IMailFolder folder = client.Inbox;

        await folder.OpenAsync(FolderAccess.ReadOnly);

        IList<UniqueId> uniqueIds = await client.Inbox.SearchAsync(SearchQuery.FromContains(query));

        IList<Task<MimeMessage>> messageTasks = uniqueIds.Select(m => client.Inbox.GetMessageAsync(m)).ToList();

        while (messageTasks.Count > 0)
        {
            Task<MimeMessage> messageTask = await Task.WhenAny(messageTasks);

            messageTasks.Remove(messageTask);

            MimeMessage message = await messageTask;

            IEmail email = new Email(date: message.Date, id: message.MessageId, messageBody: message.TextBody, references: message.References);

            callBack(email);

            result.Add(email);
        }

        return result;
    }
}

However, I feel there is a lot of code for what I want to do - do I really need to call so many async methods to get the messages, is it essential to have everything inside the using statement, or are there any obvious objects that I can cache. Is there a way that I can rewrite this method so I do not have to include the callBack action? Also, I'm a bit unsure about the types of exceptions I can sensibly expect to handle, timeouts etc...

\$\endgroup\$
6
\$\begingroup\$

It's hard to say how to write the ideal method for your use-case because you don't specify some of the details that would be necessary to know, such as:

How often will your code be making this query?

Is the connection always to the same IMAP server, username and password?

(if it'll be called fairly frequently with the same server/username/password combo, you could try to re-use the same ImapClient connection)

That said, I'll start with trying to at least clean up the code a bit.

public async Task<IList<IEmail>> EmailQueryAsync(string query, Action<IEmail> callBack)
{
    return Task.Factory.StartNew (() => {
        var result = new List<IEmail>();

        using (var client = new ImapClient ()) {
            client.Connect ("imap.gmail.com", 993, true);

            client.AuthenticationMechanisms.Remove ("XOAUTH2");

            client.Authenticate (this.userName, this.password);

            var folder = client.Inbox;

            folder.Open (FolderAccess.ReadOnly);

            var uids = client.Inbox.Search (SearchQuery.FromContains (query));

            foreach (var uid in uids) {
                var message = folder.GetMessage (uid);

                var email = new Email (date: message.Date, id: message.MessageId, messageBody: message.TextBody, references: message.References);

                callBack (email);

                result.Add (email);
            }

            return result;
        }
    }
}

I changed your code to put the entire block of logic into a single task which will all run on the same thread rather than your previous block which would effectively just run synchronously anyway since it didn't actually parallelize anything (it can't, really, since you can't search a folder until you've opened the folder - which you can't do until you've authenticated - which you can't do until you've connected, etc).

As far replacing your GetMessageAsync() logic goes, since the ImapClient can only process a single command at a time anyway, it doesn't make sense to spawn potentially thousands of command tasks asynchronously so that they can all just hurry up and wait for the previously queued tasks. Better to just write a synchronous loop in this scenario.

Update 2017-12-14: This will no longer be the case in MailKit 2.0. 2.0 will no longer wrap the synchronous calls using Task.Run() which means that they will no longer spawn threads.

\$\endgroup\$
  • \$\begingroup\$ Thanks for your response. Your comments make sense, and I like the approach of wrapping it up in a single task, especially since the original was not utilising the asynchronous methods. I'm expecting quite a few calls using the same credentials, so depending on performance issues, I might rewrite it to reuse the same ImapClient connection. \$\endgroup\$ – Leigh Shepperson Jun 1 '15 at 23:10
  • \$\begingroup\$ Cool, glad that my answer has helped. When you do modify it to reuse the same connection, you'll want to lock the ImapClient.SyncRoot property if multiple threads may access the client at a time. The ImapFolder.SyncRoot property can also be used - in fact, the SyncRoot properties of both objects is the same object. \$\endgroup\$ – jstedfast Jun 2 '15 at 10:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.