From 0cf5ab734b7b62e42f5c7c8fa08f26a3b0f9bbe7 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Fri, 25 Oct 2019 14:54:53 +1100 Subject: [PATCH] Merge contacts when harvesting them Now that ContactStore no longer merges contacts, do so in the harvester. --- src/engine/api/geary-contact.vala | 10 ++- .../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 contacts, - RFC822.MailboxAddresses? addresses, - Email.Field type, - int importance) { + private async void add_contacts(Gee.Map 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 contacts, - RFC822.MailboxAddress address, - Email.Field type, - int importance) { + private async void add_contact(Gee.Map 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 contacts = call.called_arg>(0); + Gee.Collection contacts = update_call.called_arg>(0); assert_int(1, contacts.size, "contacts length"); Contact? created = Collection.get_first(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 contacts = call.called_arg>(0); + Gee.Collection contacts = update_call.called_arg>(0); Contact? created = Collection.get_first(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 contacts = call.called_arg>(0); + Gee.Collection contacts = update_call.called_arg>(0); Contact? created = Collection.get_first(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 contacts = call.called_arg>(0); + Gee.Collection contacts = update_call.called_arg>(0); Contact? created = Collection.get_first(contacts) as Contact; assert_int( Contact.Importance.RECEIVED_FROM,