[geary/wip/135-contact-popovers: 61/62] Fix sender names from GitLab, Discourse, etc displayed with wrong names



commit 07ceb9168499579dbff393af764a8639221394c8
Author: Michael Gratton <mike vee net>
Date:   Wed Mar 27 17:04:00 2019 +1100

    Fix sender names from GitLab, Discourse, etc displayed with wrong names
    
    This caches Folks individuals instead of Contacts, so a new Contact
    instance is created for each email address not in Folks. As a result,
    contacts based on the same email address but different names are not
    used for email addresses with different names.

 .../application/application-contact-store.vala     | 58 +++++++++++-----------
 src/client/util/util-cache.vala                    |  7 +++
 2 files changed, 37 insertions(+), 28 deletions(-)
---
diff --git a/src/client/application/application-contact-store.vala 
b/src/client/application/application-contact-store.vala
index c4fc75b0..8aa2b96e 100644
--- a/src/client/application/application-contact-store.vala
+++ b/src/client/application/application-contact-store.vala
@@ -26,16 +26,14 @@ public class Application.ContactStore : Geary.BaseObject {
 
     internal Folks.IndividualAggregator individuals;
 
-    // Address for storing contacts for a specific email address.
-    // This primary cache is used to fast-path common-case lookups.
-    private Util.Cache.Lru<Contact> address_cache =
-        new Util.Cache.Lru<Contact>(LRU_CACHE_MAX);
+    // Cache for storing Folks individuals by email address. Store
+    // nulls so that negative lookups are cached as well.
+    private Util.Cache.Lru<Folks.Individual?> folks_address_cache =
+        new Util.Cache.Lru<Folks.Individual?>(LRU_CACHE_MAX);
 
-    // Folks cache used for storing contacts backed by a Folk
-    // individual. This secondary cache is used in case an individual
-    // has more than one email address.
-    private Util.Cache.Lru<Contact> folks_cache =
-        new Util.Cache.Lru<Contact>(LRU_CACHE_MAX);
+    // Cache for contacts backed by Folks individual's id.
+    private Util.Cache.Lru<Contact?> contact_id_cache =
+        new Util.Cache.Lru<Contact?>(LRU_CACHE_MAX);
 
 
     /** Constructs a new contact store for an account. */
@@ -56,8 +54,8 @@ public class Application.ContactStore : Geary.BaseObject {
 
     /** Closes the store, flushing all caches. */
     public void close() {
-        this.address_cache.clear();
-        this.folks_cache.clear();
+        this.folks_address_cache.clear();
+        this.contact_id_cache.clear();
     }
 
     /**
@@ -71,23 +69,27 @@ public class Application.ContactStore : Geary.BaseObject {
     public async Contact load(Geary.RFC822.MailboxAddress mailbox,
                               GLib.Cancellable? cancellable)
         throws GLib.Error {
-        Contact? contact = this.address_cache.get_entry(mailbox.address);
+        Folks.Individual? individual = null;
+        // Do a double lookup here in case of cache hit since null
+        // values are used to be able to cache Folks lookup failures
+        // (as well as successes).
+        if (this.folks_address_cache.has_key(mailbox.address)) {
+            individual = this.folks_address_cache.get_entry(mailbox.address);
+        } else {
+            individual = yield search_match(mailbox.address, cancellable);
+            this.folks_address_cache.set_entry(mailbox.address, individual);
+        }
+
+        Contact? contact = null;
+        if (individual != null) {
+            this.contact_id_cache.get_entry(individual.id);
+        }
         if (contact == null) {
-            Folks.Individual? individual = yield search_match(
-                mailbox.address, cancellable
-            );
+            Geary.Contact? engine =
+                this.account.get_contact_store().get_by_rfc822(mailbox);
+            contact = new Contact(this, individual, engine, mailbox);
             if (individual != null) {
-                contact = this.folks_cache.get_entry(individual.id);
-            }
-
-            if (contact == null) {
-                Geary.Contact? engine =
-                    this.account.get_contact_store().get_by_rfc822(mailbox);
-                contact = new Contact(this, individual, engine, mailbox);
-                if (individual != null) {
-                    this.folks_cache.set_entry(individual.id, contact);
-                }
-                this.address_cache.set_entry(mailbox.address, contact);
+                this.contact_id_cache.set_entry(individual.id, contact);
             }
         }
         return contact;
@@ -131,10 +133,10 @@ public class Application.ContactStore : Geary.BaseObject {
         Gee.MultiMap<Folks.Individual?,Folks.Individual?> changes) {
         foreach (Folks.Individual? individual in changes.get_keys()) {
             if (individual != null) {
-                this.folks_cache.remove_entry(individual.id);
+                this.contact_id_cache.remove_entry(individual.id);
                 foreach (Folks.EmailFieldDetails email in
                          individual.email_addresses) {
-                    this.address_cache.remove_entry(email.value);
+                    this.folks_address_cache.remove_entry(email.value);
                 }
             }
         }
diff --git a/src/client/util/util-cache.vala b/src/client/util/util-cache.vala
index 3161fd39..ffc0a41b 100644
--- a/src/client/util/util-cache.vala
+++ b/src/client/util/util-cache.vala
@@ -58,6 +58,13 @@ public class Util.Cache.Lru<T> : Geary.BaseObject {
         this.max_size = max_size;
     }
 
+    /**
+     * Determines if the given key exists in the cache.
+     */
+    public bool has_key(string key) {
+        return this.cache.has_key(key);
+    }
+
     /**
      * Sets an entry in the cache, replacing any existing entry.
      *


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]