[geary/mjog/550-always-show-images-not-saved: 2/4] Merge contacts when harvesting them

[Michael Gratton <mjog src gnome org>]

commit 0cf5ab734b7b62e42f5c7c8fa08f26a3b0f9bbe7
Author: Michael Gratton <mike vee net>
Date:   Fri Oct 25 14:54:53 2019 +1100

    Merge contacts when harvesting them
    
    Now that ContactStore no longer merges contacts, do so in the harvester.

 src/engine/api/geary-contact.vala                  | 10 ++-
 src/engine/common/common-contact-harvester.vala    | 76 ++++++++++++++++------
 .../common/common-contact-harvester-test.vala      | 20 +++---
 3 files changed, 75 insertions(+), 31 deletions(-)
---
diff --git a/src/engine/api/geary-contact.vala b/src/engine/api/geary-contact.vala
index dfa1c71c..7da8e6ca 100644
--- a/src/engine/api/geary-contact.vala
+++ b/src/engine/api/geary-contact.vala
@@ -60,9 +60,15 @@ public class Geary.Contact : BaseObject {
     }
 
 
+    /** Normalises an email address as for {@link normalized_email}. */
+    public static string normalise_email(string address) {
+        return address.normalize().casefold();
+    }
+
+
     public string normalized_email { get; private set; }
     public string email { get; private set; }
-    public string? real_name { get; private set; }
+    public string? real_name { get; set; }
     public int highest_importance { get; set; }
     public Flags flags { get; set; default = new Flags(); }
 
@@ -70,7 +76,7 @@ public class Geary.Contact : BaseObject {
                    string? real_name,
                    int highest_importance,
                    string? normalized_email = null) {
-        this.normalized_email = normalized_email ?? email.normalize().casefold();
+        this.normalized_email = normalized_email ?? normalise_email(email);
         this.email = email;
         this.real_name = (
             (real_name != email && real_name != normalized_email)
diff --git a/src/engine/common/common-contact-harvester.vala b/src/engine/common/common-contact-harvester.vala
index ec48917d..5fb20646 100644
--- a/src/engine/common/common-contact-harvester.vala
+++ b/src/engine/common/common-contact-harvester.vala
@@ -55,16 +55,28 @@ internal class Geary.ContactHarvesterImpl : BaseObject, ContactHarvester {
             foreach (Email message in messages) {
                 if (message.fields.fulfills(REQUIRED_FIELDS)) {
                     type = Email.Field.ORIGINATORS;
-                    add_contacts(contacts, message.from, type, importance);
+                    yield add_contacts(
+                        contacts, message.from, type, importance, cancellable
+                    );
                     if (message.sender != null) {
-                        add_contact(contacts, message.sender, type, importance);
+                        yield add_contact(
+                            contacts, message.sender, type, importance, cancellable
+                        );
                     }
-                    add_contacts(contacts, message.bcc, type, importance);
+                    yield add_contacts(
+                        contacts, message.bcc, type, importance, cancellable
+                    );
 
                     type = Email.Field.RECEIVERS;
-                    add_contacts(contacts, message.to, type, importance);
-                    add_contacts(contacts, message.cc, type, importance);
-                    add_contacts(contacts, message.bcc, type, importance);
+                    yield add_contacts(
+                        contacts, message.to, type, importance, cancellable
+                    );
+                    yield add_contacts(
+                        contacts, message.cc, type, importance, cancellable
+                    );
+                    yield add_contacts(
+                        contacts, message.bcc, type, importance, cancellable
+                    );
                 }
             }
 
@@ -72,34 +84,56 @@ internal class Geary.ContactHarvesterImpl : BaseObject, ContactHarvester {
         }
     }
 
-    private void add_contacts(Gee.Map<string, Contact> contacts,
-                              RFC822.MailboxAddresses? addresses,
-                              Email.Field type,
-                              int importance) {
+    private async void add_contacts(Gee.Map<string, Contact> contacts,
+                                    RFC822.MailboxAddresses? addresses,
+                                    Email.Field type,
+                                    int importance,
+                                    GLib.Cancellable? cancellable)
+        throws GLib.Error {
         if (addresses != null) {
             foreach (RFC822.MailboxAddress address in addresses) {
-                add_contact(contacts, address, type, importance);
+                yield add_contact(
+                    contacts, address, type, importance, cancellable
+                );
             }
         }
     }
 
-    private inline void add_contact(Gee.Map<string, Contact> contacts,
-                                    RFC822.MailboxAddress address,
-                                    Email.Field type,
-                                    int importance) {
+    private async void add_contact(Gee.Map<string, Contact> contacts,
+                                   RFC822.MailboxAddress address,
+                                   Email.Field type,
+                                   int importance,
+                                   GLib.Cancellable? cancellable)
+        throws GLib.Error {
         if (address.is_valid() && !address.is_spoofed()) {
             if (type == RECEIVERS && address in this.owner_mailboxes) {
                 importance = Contact.Importance.RECEIVED_FROM;
             }
 
-            Contact contact = new Contact.from_rfc822_address(
-                address, importance
-            );
-            Contact? existing = contacts[contact.normalized_email];
-            if (existing == null ||
-                existing.highest_importance < contact.highest_importance) {
+            Contact? contact = contacts[
+                Contact.normalise_email(address.address)
+            ];
+            if (contact == null) {
+                contact = yield this.store.get_by_rfc822(address, cancellable);
+                if (contact == null) {
+                    contact = new Geary.Contact.from_rfc822_address(
+                        address, importance
+                    );
+                }
                 contacts[contact.normalized_email] = contact;
             }
+
+            // Update the contact's name if the current address is at
+            // least equal importance to the existing one so that the
+            // sender's labels are preferred.
+            if (contact.highest_importance <= importance &&
+                !String.is_empty_or_whitespace(address.name)) {
+                contact.real_name = address.name;
+            }
+
+            if (contact.highest_importance < importance) {
+                contact.highest_importance = importance;
+            }
         }
     }
 
diff --git a/test/engine/common/common-contact-harvester-test.vala 
b/test/engine/common/common-contact-harvester-test.vala
index 9db3f947..02bc73d0 100644
--- a/test/engine/common/common-contact-harvester-test.vala
+++ b/test/engine/common/common-contact-harvester-test.vala
@@ -57,7 +57,8 @@ class Geary.ContactHarvesterImplTest : TestCase {
             SpecialFolderType.INBOX,
             this.senders
         );
-        ExpectedCall call = this.store.expect_call("update_contacts");
+        this.store.expect_call("get_by_rfc822");
+        ExpectedCall update_call = this.store.expect_call("update_contacts");
         this.email.set_receivers(
             new RFC822.MailboxAddresses.single(this.test_address), null, null
         );
@@ -70,7 +71,7 @@ class Geary.ContactHarvesterImplTest : TestCase {
 
         this.store.assert_expectations();
 
-        Gee.Collection<Contact> contacts = call.called_arg<Gee.Collection<Contact>>(0);
+        Gee.Collection<Contact> contacts = update_call.called_arg<Gee.Collection<Contact>>(0);
         assert_int(1, contacts.size, "contacts length");
         Contact? created = Collection.get_first<Contact>(contacts) as Contact;
         assert_non_null(created, "contacts contents");
@@ -105,7 +106,8 @@ class Geary.ContactHarvesterImplTest : TestCase {
             SpecialFolderType.INBOX,
             this.senders
         );
-        ExpectedCall call = this.store.expect_call("update_contacts");
+        this.store.expect_call("get_by_rfc822");
+        ExpectedCall update_call = this.store.expect_call("update_contacts");
         this.email.set_receivers(
             new RFC822.MailboxAddresses.single(this.test_address), null, null
         );
@@ -118,7 +120,7 @@ class Geary.ContactHarvesterImplTest : TestCase {
 
         this.store.assert_expectations();
 
-        Gee.Collection<Contact> contacts = call.called_arg<Gee.Collection<Contact>>(0);
+        Gee.Collection<Contact> contacts = update_call.called_arg<Gee.Collection<Contact>>(0);
         Contact? created = Collection.get_first<Contact>(contacts) as Contact;
         assert_int(
             Contact.Importance.SEEN,
@@ -133,7 +135,8 @@ class Geary.ContactHarvesterImplTest : TestCase {
             SpecialFolderType.SENT,
             this.senders
         );
-        ExpectedCall call = this.store.expect_call("update_contacts");
+        this.store.expect_call("get_by_rfc822");
+        ExpectedCall update_call = this.store.expect_call("update_contacts");
         this.email.set_receivers(
             new RFC822.MailboxAddresses.single(this.test_address), null, null
         );
@@ -146,7 +149,7 @@ class Geary.ContactHarvesterImplTest : TestCase {
 
         this.store.assert_expectations();
 
-        Gee.Collection<Contact> contacts = call.called_arg<Gee.Collection<Contact>>(0);
+        Gee.Collection<Contact> contacts = update_call.called_arg<Gee.Collection<Contact>>(0);
         Contact? created = Collection.get_first<Contact>(contacts) as Contact;
         assert_int(
             Contact.Importance.SENT_TO,
@@ -161,7 +164,8 @@ class Geary.ContactHarvesterImplTest : TestCase {
             SpecialFolderType.SENT,
             this.senders
         );
-        ExpectedCall call = this.store.expect_call("update_contacts");
+        this.store.expect_call("get_by_rfc822");
+        ExpectedCall update_call = this.store.expect_call("update_contacts");
         this.email.set_receivers(
             new RFC822.MailboxAddresses.single(this.sender_address), null, null
         );
@@ -174,7 +178,7 @@ class Geary.ContactHarvesterImplTest : TestCase {
 
         this.store.assert_expectations();
 
-        Gee.Collection<Contact> contacts = call.called_arg<Gee.Collection<Contact>>(0);
+        Gee.Collection<Contact> contacts = update_call.called_arg<Gee.Collection<Contact>>(0);
         Contact? created = Collection.get_first<Contact>(contacts) as Contact;
         assert_int(
             Contact.Importance.RECEIVED_FROM,