HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Getting messages asynchronously using MailKit

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
mailkitgettingmessagesasynchronouslyusing

Problem

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> EmailQueryAsync(string query, Action callBack)
{
    IList result = new List();

    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 uniqueIds = await client.Inbox.SearchAsync(SearchQuery.FromContains(query));

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

        while (messageTasks.Count > 0)
        {
            Task 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...

Solution

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> EmailQueryAsync(string query, Action callBack)
{
    return Task.Factory.StartNew (() => {
        var result = new List();

        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.

Code Snippets

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;
        }
    }
}

Context

StackExchange Code Review Q#92307, answer score: 7

Revisions (0)

No revisions yet.