[geary/wip/3.32-avatars: 50/50] Attempt to de-mangle From names from Mailman, GitLab, etc

[Michael Gratton <mjog src gnome org>]

commit 6c8f192148ef76054a267651718ec32f21eeb208
Author: Michael Gratton <mike vee net>
Date:   Sat Mar 9 20:06:45 2019 +1100

    Attempt to de-mangle From names from Mailman, GitLab, etc
    
    Some software like the above will mangle From mailbox names by appending
    "via Some Service" to the From mailbox name. This messes up generating
    default avatars for the actual people sending these messages, so
    attempt to de-mangle the names.
    
    This involves moving primary originator determination from the engine
    to the client, since it's now a policy thing. Add unit tests.

 .../conversation-viewer/conversation-email.vala    |  16 ++-
 .../conversation-viewer/conversation-message.vala  |   4 +-
 src/client/notification/libnotify.vala             |   3 +-
 src/client/util/util-email.vala                    |  59 +++++++++++
 src/engine/api/geary-email.vala                    |  19 ----
 src/engine/rfc822/rfc822-message.vala              |  19 ----
 test/client/util/util-email-test.vala              | 115 +++++++++++++++++++++
 test/meson.build                                   |   2 +
 test/test-client.vala                              |   1 +
 9 files changed, 192 insertions(+), 46 deletions(-)
---
diff --git a/src/client/conversation-viewer/conversation-email.vala 
b/src/client/conversation-viewer/conversation-email.vala
index 3a94b2f1..ea57e20e 100644
--- a/src/client/conversation-viewer/conversation-email.vala
+++ b/src/client/conversation-viewer/conversation-email.vala
@@ -289,6 +289,9 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
     /** Determines if the email is a draft message. */
     public bool is_draft { get; private set; }
 
+    /** The email's primary originator, if any. */
+    public Geary.RFC822.MailboxAddress? primary_originator { get; private set; }
+
     /** The view displaying the email's primary message headers and body. */
     public ConversationMessage primary_message { get; private set; }
 
@@ -444,6 +447,7 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
         base_ref();
         this.email = email;
         this.is_draft = is_draft;
+        this.primary_originator = Util.Email.get_primary_originator(email);
         this.email_store = email_store;
         this.contact_store = email_store.account.get_contact_store();
         this.avatar_store = avatar_store;
@@ -507,9 +511,11 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
         // Construct the view for the primary message, hook into it
 
         bool load_images = email.load_remote_images().is_certain();
-        Geary.Contact contact = this.contact_store.get_by_rfc822(
-            email.get_primary_originator()
-        );
+
+        Geary.Contact? contact = null;
+        if (this.primary_originator != null) {
+            contact = this.contact_store.get_by_rfc822(this.primary_originator);
+        }
         if (contact != null)  {
             load_images |= contact.always_load_remote_images();
         }
@@ -577,7 +583,7 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
         } catch (IOError.CANCELLED err) {
             // okay
         } catch (Error err) {
-            Geary.RFC822.MailboxAddress? from = this.email.get_primary_originator();
+            Geary.RFC822.MailboxAddress? from = this.primary_originator;
             debug("Avatar load failed for \"%s\": %s",
                   from != null ? from.to_string() : "<unknown>", err.message);
         }
@@ -1038,7 +1044,7 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
 
 
     private void on_remember_remote_images(ConversationMessage view) {
-        Geary.RFC822.MailboxAddress? sender = this.email.get_primary_originator();
+        Geary.RFC822.MailboxAddress? sender = this.primary_originator;
         if (sender != null) {
             Geary.Contact? contact = this.contact_store.get_by_rfc822(sender);
             if (contact != null) {
diff --git a/src/client/conversation-viewer/conversation-message.vala 
b/src/client/conversation-viewer/conversation-message.vala
index 17614426..e8cd4f27 100644
--- a/src/client/conversation-viewer/conversation-message.vala
+++ b/src/client/conversation-viewer/conversation-message.vala
@@ -289,7 +289,7 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
                                           bool load_remote_images,
                                           Configuration config) {
         this(
-            email.get_primary_originator(),
+            Util.Email.get_primary_originator(email),
             email.from,
             email.reply_to,
             email.sender,
@@ -315,7 +315,7 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
                                             bool load_remote_images,
                                             Configuration config) {
         this(
-            message.get_primary_originator(),
+            Util.Email.get_primary_originator(message),
             message.from,
             message.reply_to,
             message.sender,
diff --git a/src/client/notification/libnotify.vala b/src/client/notification/libnotify.vala
index d82a5a9c..11bf793f 100644
--- a/src/client/notification/libnotify.vala
+++ b/src/client/notification/libnotify.vala
@@ -95,7 +95,8 @@ public class Libnotify : Geary.BaseObject {
             return;
 
         // possible to receive email with no originator
-        Geary.RFC822.MailboxAddress? primary = email.get_primary_originator();
+        Geary.RFC822.MailboxAddress? primary =
+            Util.Email.get_primary_originator(email);
         if (primary == null) {
             notify_new_mail(folder, 1);
 
diff --git a/src/client/util/util-email.vala b/src/client/util/util-email.vala
index 05a82828..55bda5a1 100644
--- a/src/client/util/util-email.vala
+++ b/src/client/util/util-email.vala
@@ -38,6 +38,65 @@ namespace Util.Email {
         return !Geary.String.is_empty(cleaned) ? cleaned : _("(no subject)");
     }
 
+    /**
+     * Returns a mailbox for the primary originator of an email.
+     *
+     * RFC 822 allows multiple and absent From header values, and
+     * software such as Mailman and GitLab will mangle the names in
+     * From mailboxes. This provides a canonical means to obtain a
+     * mailbox (that is, name and email address) for the first
+     * originator, and with the mailbox's name having been fixed up
+     * where possible.
+     *
+     * The first From mailbox is used and de-mangled if found, if not
+     * the Sender mailbox is used if present, else the first Reply-To
+     * mailbox is used.
+     */
+    public Geary.RFC822.MailboxAddress?
+        get_primary_originator(Geary.EmailHeaderSet email) {
+        Geary.RFC822.MailboxAddress? primary = null;
+        if (email.from != null && email.from.size > 0) {
+            // We have a From address, so attempt to de-mangle it
+            Geary.RFC822.MailboxAddresses? from = email.from;
+
+            string from_name = "";
+            if (from != null && from.size > 0) {
+                primary = from[0];
+                from_name = primary.name ?? "";
+            }
+
+            Geary.RFC822.MailboxAddresses? reply_to = email.reply_to;
+            Geary.RFC822.MailboxAddress? primary_reply_to = null;
+            string reply_to_name = "";
+            if (reply_to != null && reply_to.size > 0) {
+                primary_reply_to = reply_to[0];
+                reply_to_name = primary_reply_to.name ?? "";
+            }
+
+            // Spaces are important
+            const string VIA = " via ";
+
+            if (reply_to_name != "" && from_name.has_prefix(reply_to_name)) {
+                // Mailman sometimes sends the true originator as the
+                // Reply-To for the email
+                primary = primary_reply_to;
+            } else if (VIA in from_name) {
+                // Mailman, GitLib, Discourse and others send the
+                // originator's name prefixing something starting with
+                // "via".
+                primary = new Geary.RFC822.MailboxAddress(
+                    from_name.split(VIA, 2)[0], primary.address
+                );
+            }
+        } else if (email.sender != null) {
+            primary = email.sender;
+        } else if (email.reply_to != null && email.reply_to.size > 0) {
+            primary = email.reply_to[0];
+        }
+
+        return primary;
+    }
+
     /**
      * Returns a shortened recipient list suitable for display.
      *
diff --git a/src/engine/api/geary-email.vala b/src/engine/api/geary-email.vala
index e22d5762..2b4d7ec4 100644
--- a/src/engine/api/geary-email.vala
+++ b/src/engine/api/geary-email.vala
@@ -530,25 +530,6 @@ public class Geary.Email : BaseObject, EmailHeaderSet {
         return (preview != null) ? preview.buffer.to_string() : "";
     }
 
-    /**
-     * Returns the primary originator of an email, which is defined as the first mailbox address
-     * in From:, Sender:, or Reply-To:, in that order, depending on availability.
-     *
-     * Returns null if no originators are present.
-     */
-    public RFC822.MailboxAddress? get_primary_originator() {
-        if (from != null && from.size > 0)
-            return from[0];
-
-        if (sender != null)
-            return sender;
-
-        if (reply_to != null && reply_to.size > 0)
-            return reply_to[0];
-
-        return null;
-    }
-
     public string to_string() {
         return "[%s] ".printf(id.to_string());
     }
diff --git a/src/engine/rfc822/rfc822-message.vala b/src/engine/rfc822/rfc822-message.vala
index 9bbfb5a0..5f70b854 100644
--- a/src/engine/rfc822/rfc822-message.vala
+++ b/src/engine/rfc822/rfc822-message.vala
@@ -441,25 +441,6 @@ public class Geary.RFC822.Message : BaseObject, EmailHeaderSet {
             : "";
     }
 
-    /**
-     * Returns the primary originator of an email, which is defined as the first mailbox address
-     * in From:, Sender:, or Reply-To:, in that order, depending on availability.
-     *
-     * Returns null if no originators are present.
-     */
-    public RFC822.MailboxAddress? get_primary_originator() {
-        if (from != null && from.size > 0)
-            return from[0];
-
-        if (sender != null)
-            return sender;
-
-        if (reply_to != null && reply_to.size > 0)
-            return reply_to[0];
-
-        return null;
-    }
-
     public Gee.List<RFC822.MailboxAddress>? get_recipients() {
         Gee.List<RFC822.MailboxAddress> addrs = new Gee.ArrayList<RFC822.MailboxAddress>();
 
diff --git a/test/client/util/util-email-test.vala b/test/client/util/util-email-test.vala
new file mode 100644
index 00000000..3630276c
--- /dev/null
+++ b/test/client/util/util-email-test.vala
@@ -0,0 +1,115 @@
+/*
+ * Copyright 2019 Michael Gratton <mike vee net>
+ *
+ * This software is licensed under the GNU Lesser General Public License
+ * (version 2.1 or later). See the COPYING file in this distribution.
+ */
+
+public class Util.Email.Test : TestCase {
+
+    public Test() {
+        base("UtilEmailTest");
+        add_test("null_originator", null_originator);
+        add_test("from_originator", from_originator);
+        add_test("sender_originator", sender_originator);
+        add_test("reply_to_originator", reply_to_originator);
+        add_test("reply_to_via_originator", reply_to_via_originator);
+        add_test("plain_via_originator", plain_via_originator);
+    }
+
+    public void null_originator() throws GLib.Error {
+        Geary.RFC822.MailboxAddress? originator = get_primary_originator(
+            new_email(null, null, null)
+        );
+
+        assert_null(originator);
+    }
+
+    public void from_originator() throws GLib.Error {
+        Geary.RFC822.MailboxAddress? originator = get_primary_originator(
+            new_email(
+                new Geary.RFC822.MailboxAddress("from", "from example com"),
+                new Geary.RFC822.MailboxAddress("sender", "sender example com"),
+                new Geary.RFC822.MailboxAddress("reply-to", "reply-to example com")
+            )
+        );
+
+        assert_non_null(originator);
+        assert_string("from", originator.name);
+        assert_string("from example com", originator.address);
+    }
+
+    public void sender_originator() throws GLib.Error {
+        Geary.RFC822.MailboxAddress? originator = get_primary_originator(
+            new_email(
+                null,
+                new Geary.RFC822.MailboxAddress("sender", "sender example com"),
+                new Geary.RFC822.MailboxAddress("reply-to", "reply-to example com")
+            )
+        );
+
+        assert_non_null(originator);
+        assert_string("sender", originator.name);
+        assert_string("sender example com", originator.address);
+    }
+
+    public void reply_to_originator() throws GLib.Error {
+        Geary.RFC822.MailboxAddress? originator = get_primary_originator(
+            new_email(
+                null,
+                null,
+                new Geary.RFC822.MailboxAddress("reply-to", "reply-to example com")
+            )
+        );
+
+        assert_non_null(originator);
+        assert_string("reply-to", originator.name);
+        assert_string("reply-to example com", originator.address);
+    }
+
+    public void reply_to_via_originator() throws GLib.Error {
+        Geary.RFC822.MailboxAddress? originator = get_primary_originator(
+            new_email(
+                new Geary.RFC822.MailboxAddress("test via bot", "bot example com"),
+                null,
+                new Geary.RFC822.MailboxAddress("test", "test example com")
+            )
+        );
+
+        assert_non_null(originator);
+        assert_string("test", originator.name);
+        assert_string("test example com", originator.address);
+    }
+
+    public void plain_via_originator() throws GLib.Error {
+        Geary.RFC822.MailboxAddress? originator = get_primary_originator(
+            new_email(
+                new Geary.RFC822.MailboxAddress("test via bot", "bot example com"),
+                null,
+                null
+            )
+        );
+
+        assert_non_null(originator);
+        assert_string("test", originator.name);
+        assert_string("bot example com", originator.address);
+    }
+
+    private Geary.Email new_email(Geary.RFC822.MailboxAddress? from,
+                                  Geary.RFC822.MailboxAddress? sender,
+                                  Geary.RFC822.MailboxAddress? reply_to)
+        throws GLib.Error {
+        Geary.Email email = new Geary.Email(new Geary.MockEmailIdentifer(1));
+        email.set_originators(
+            from != null
+            ? new Geary.RFC822.MailboxAddresses(Geary.Collection.single(from))
+            : null,
+            sender,
+            reply_to != null
+            ? new Geary.RFC822.MailboxAddresses(Geary.Collection.single(reply_to))
+            : null
+        );
+        return email;
+    }
+
+}
diff --git a/test/meson.build b/test/meson.build
index 6054796a..ab3ea78c 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -68,6 +68,7 @@ geary_test_client_sources = [
   # and the engine test sute needs to depend
   # geary-engine_internal.vapi, which leads to duplicate symbols when
   # linking
+  'engine/api/geary-email-identifier-mock.vala',
   'engine/api/geary-credentials-mediator-mock.vala',
 
   'client/accounts/accounts-manager-test.vala',
@@ -76,6 +77,7 @@ geary_test_client_sources = [
   'client/components/client-web-view-test-case.vala',
   'client/composer/composer-web-view-test.vala',
   'client/util/util-avatar-test.vala',
+  'client/util/util-email-test.vala',
 
   'js/client-page-state-test.vala',
   'js/composer-page-state-test.vala',
diff --git a/test/test-client.vala b/test/test-client.vala
index 3386d183..1852ba6e 100644
--- a/test/test-client.vala
+++ b/test/test-client.vala
@@ -44,6 +44,7 @@ int main(string[] args) {
     client.add_suite(new ComposerWebViewTest().get_suite());
     client.add_suite(new ConfigurationTest().get_suite());
     client.add_suite(new Util.Avatar.Test().get_suite());
+    client.add_suite(new Util.Email.Test().get_suite());
 
     TestSuite js = new TestSuite("js");