[geary/wip/135-contact-popovers: 51/56] Update AvatarStore to load from contacts rather than mailboxes
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/wip/135-contact-popovers: 51/56] Update AvatarStore to load from contacts rather than mailboxes
- Date: Sun, 7 Apr 2019 01:46:19 +0000 (UTC)
commit e167485407ed25675dd55ca21bd5a0424d9da6e0
Author: Michael Gratton <mike vee net>
Date: Sun Mar 17 13:51:21 2019 +1100
Update AvatarStore to load from contacts rather than mailboxes
Require a Contact instance when loading avatars and fix call sites,
remove Folks lookup and just get individuals from contacts instead,
convert to use external LRU cache implementation.
Provide a means for notication libs to access AvatarStore and
ContactStore instances via NewMessagesMonitor, so they can lookup and
display contacts and avatars.
.../application/application-avatar-store.vala | 204 ++++++++-------------
src/client/application/application-contact.vala | 4 +-
src/client/application/geary-controller.vala | 22 ++-
.../conversation-contact-popover.vala | 1 +
.../conversation-viewer/conversation-message.vala | 5 +-
src/client/notification/libnotify.vala | 80 ++++----
src/client/notification/new-messages-monitor.vala | 38 +++-
7 files changed, 178 insertions(+), 176 deletions(-)
---
diff --git a/src/client/application/application-avatar-store.vala
b/src/client/application/application-avatar-store.vala
index e57bcf34..03dfdd09 100644
--- a/src/client/application/application-avatar-store.vala
+++ b/src/client/application/application-avatar-store.vala
@@ -1,5 +1,5 @@
/*
- * Copyright 2016-2018 Michael Gratton <mike vee net>
+ * Copyright 2016-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.
@@ -8,17 +8,25 @@
/**
* Email address avatar loader and cache.
+ *
+ * Avatars are loaded from a {@link Contact} object's Folks individual
+ * if present, else one will be generated using initials and
+ * background colour based on the of the source mailbox's name if
+ * present, or address. Avatars are cached at each requested logical
+ * pixel size, per Folks individual and then per source mailbox
+ * name. This strategy allows avatar bitmaps to reflect the desktop
+ * address-book's user picture if present, else provide individualised
+ * avatars, even for mail sent by software like Mailman and Discourse.
+ *
+ * Unlike {@link ContactStore}, once store instance is useful for
+ * loading and caching avatars across accounts.
*/
public class Application.AvatarStore : Geary.BaseObject {
- /** Default size of avatar images, in toolkit pixels */
+ /** Default size of avatar images, in virtual pixels */
public const int PIXEL_SIZE = 48;
- // Max age is low since we really only want to cache between
- // conversation loads.
- private const int64 MAX_CACHE_AGE_US = 5 * 1000 * 1000;
-
// Max size is low since most conversations don't get above the
// low hundreds of messages, and those that do will likely get
// many repeated participants
@@ -28,39 +36,31 @@ public class Application.AvatarStore : Geary.BaseObject {
private class CacheEntry {
- public static string to_key(Geary.RFC822.MailboxAddress mailbox) {
+ public static string to_name_key(Geary.RFC822.MailboxAddress source) {
// Use short name as the key, since it will use the name
// first, then the email address, which is especially
// important for things like GitLab email where the
// address is always the same, but the name changes. This
// ensures that each such user gets different initials.
- return mailbox.to_short_display().normalize().casefold();
- }
-
- public static int lru_compare(CacheEntry a, CacheEntry b) {
- return (a.key == b.key)
- ? 0 : (int) (a.last_used - b.last_used);
+ return source.to_short_display().normalize().casefold();
}
+ public Contact contact;
+ public Geary.RFC822.MailboxAddress source;
- public string key;
-
- public Geary.RFC822.MailboxAddress mailbox;
+ private Gee.List<Gdk.Pixbuf> pixbufs = new Gee.LinkedList<Gdk.Pixbuf>();
- // Store nulls so we can also cache avatars not found
- public Folks.Individual? individual;
- public int64 last_used;
+ public CacheEntry(Contact contact,
+ Geary.RFC822.MailboxAddress source) {
+ this.contact = contact;
+ this.source = source;
- private Gee.List<Gdk.Pixbuf> pixbufs = new Gee.LinkedList<Gdk.Pixbuf>();
+ contact.changed.connect(on_contact_changed);
+ }
- public CacheEntry(Geary.RFC822.MailboxAddress mailbox,
- Folks.Individual? individual,
- int64 last_used) {
- this.key = to_key(mailbox);
- this.mailbox = mailbox;
- this.individual = individual;
- this.last_used = last_used;
+ ~CacheEntry() {
+ this.contact.changed.disconnect(on_contact_changed);
}
public async Gdk.Pixbuf? load(int pixel_size,
@@ -76,11 +76,13 @@ public class Application.AvatarStore : Geary.BaseObject {
}
if (pixbuf == null) {
- Folks.Individual? individual = this.individual;
- if (individual != null && individual.avatar != null) {
- GLib.InputStream data = yield individual.avatar.load_async(
- pixel_size, cancellable
- );
+ Folks.Individual? individual = contact.individual;
+ if (individual != null &&
+ individual.avatar != null) {
+ GLib.InputStream data =
+ yield individual.avatar.load_async(
+ pixel_size, cancellable
+ );
pixbuf = yield new Gdk.Pixbuf.from_stream_at_scale_async(
data, pixel_size, pixel_size, true, cancellable
);
@@ -91,23 +93,14 @@ public class Application.AvatarStore : Geary.BaseObject {
if (pixbuf == null) {
string? name = null;
- // XXX should really be using the folks display name
- // here as below, but since we should the name from
- // the email address if present in
- // ConversationMessage, and since that might not match
- // the folks display name, it is confusing when the
- // initials are one thing and the name is
- // another. Re-enable below when we start using the
- // folks display name in ConversationEmail
- name = this.mailbox.to_short_display();
- // if (this.individual != null) {
- // name = this.individual.display_name;
- // } else {
- // // Use short display because it will clean up the
- // // string, use the name if present and fall back
- // // on the address if not.
- // name = this.mailbox.to_short_display();
- // }
+ if (this.contact.is_trusted) {
+ name = this.contact.display_name;
+ } else {
+ // Use short display because it will clean up the
+ // string, use the name if present and fall back
+ // on the address if not.
+ name = this.source.to_short_display();
+ }
pixbuf = Util.Avatar.generate_user_picture(name, pixel_size);
pixbuf = Util.Avatar.round_image(pixbuf);
this.pixbufs.add(pixbuf);
@@ -116,100 +109,61 @@ public class Application.AvatarStore : Geary.BaseObject {
return pixbuf;
}
+ private void on_contact_changed() {
+ this.pixbufs.clear();
+ }
+
}
- private Folks.IndividualAggregator individuals;
- private Gee.Map<string,CacheEntry> lru_cache =
- new Gee.HashMap<string,CacheEntry>();
- private Gee.SortedSet<CacheEntry> lru_ordering =
- new Gee.TreeSet<CacheEntry>(CacheEntry.lru_compare);
+ // Folks cache used for storing contacts backed by a Folk
+ // individual. This is the primary cache since we want to use the
+ // details for avatar and display name lookup from the desktop
+ // address-book if available.
+ private Util.Cache.Lru<CacheEntry> folks_cache =
+ new Util.Cache.Lru<CacheEntry>(MAX_CACHE_SIZE);
+ // Name cache uses the source mailbox's short name as the key,
+ // since this will make avatar initials match well. It is used to
+ // cache avatars for contacts not saved in the desktop
+ // address-book.
+ private Util.Cache.Lru<CacheEntry> name_cache =
+ new Util.Cache.Lru<CacheEntry>(MAX_CACHE_SIZE);
- public AvatarStore(Folks.IndividualAggregator individuals) {
- this.individuals = individuals;
- }
+ /** Closes the store, flushing all caches. */
public void close() {
- this.lru_cache.clear();
- this.lru_ordering.clear();
+ this.folks_cache.clear();
+ this.name_cache.clear();
}
- public async Gdk.Pixbuf? load(Geary.RFC822.MailboxAddress mailbox,
+ public async Gdk.Pixbuf? load(Contact contact,
+ Geary.RFC822.MailboxAddress source,
int pixel_size,
GLib.Cancellable cancellable)
throws GLib.Error {
- // Normalise the address to improve caching
- CacheEntry match = yield get_match(mailbox);
- return yield match.load(pixel_size, cancellable);
- }
-
-
- private async CacheEntry get_match(Geary.RFC822.MailboxAddress mailbox)
- throws GLib.Error {
- string key = CacheEntry.to_key(mailbox);
- int64 now = GLib.get_monotonic_time();
- CacheEntry? entry = this.lru_cache.get(key);
- if (entry != null) {
- if (entry.last_used + MAX_CACHE_AGE_US >= now) {
- // Need to remove the entry from the ordering before
- // updating the last used time since doing so changes
- // the ordering
- this.lru_ordering.remove(entry);
- entry.last_used = now;
- this.lru_ordering.add(entry);
- } else {
- this.lru_cache.unset(key);
- this.lru_ordering.remove(entry);
- entry = null;
+ CacheEntry hit = null;
+ if (contact.is_desktop_contact && contact.is_trusted) {
+ string key = contact.individual.id;
+ hit = this.folks_cache.get_entry(key);
+
+ if (hit == null) {
+ hit = new CacheEntry(contact, source);
+ this.folks_cache.set_entry(key, hit);
}
}
- if (entry == null) {
- Folks.Individual? match = yield search_match(mailbox.address);
- entry = new CacheEntry(mailbox, match, now);
- this.lru_cache.set(key, entry);
- this.lru_ordering.add(entry);
-
- // Prune the cache if needed
- if (this.lru_cache.size > MAX_CACHE_SIZE) {
- CacheEntry oldest = this.lru_ordering.first();
- this.lru_cache.unset(oldest.key);
- this.lru_ordering.remove(oldest);
- }
- }
-
- return entry;
- }
-
- private async Folks.Individual? search_match(string address)
- throws GLib.Error {
- Folks.SearchView view = new Folks.SearchView(
- this.individuals,
- new Folks.SimpleQuery(
- address,
- new string[] {
- Folks.PersonaStore.detail_key(
- Folks.PersonaDetail.EMAIL_ADDRESSES
- )
- }
- )
- );
-
- yield view.prepare();
+ if (hit == null) {
+ string key = CacheEntry.to_name_key(source);
+ hit = this.name_cache.get_entry(key);
- Folks.Individual? match = null;
- if (!view.individuals.is_empty) {
- match = view.individuals.first();
- }
-
- try {
- yield view.unprepare();
- } catch (GLib.Error err) {
- warning("Error unpreparing Folks search: %s", err.message);
+ if (hit == null) {
+ hit = new CacheEntry(contact, source);
+ this.name_cache.set_entry(key, hit);
+ }
}
- return match;
+ return yield hit.load(pixel_size, cancellable);
}
}
diff --git a/src/client/application/application-contact.vala b/src/client/application/application-contact.vala
index 67c8d661..124bb01e 100644
--- a/src/client/application/application-contact.vala
+++ b/src/client/application/application-contact.vala
@@ -59,8 +59,10 @@ public class Application.Contact : Geary.BaseObject {
public signal void changed();
+ /** The Folks individual for the contact, if any. */
+ internal Folks.Individual? individual { get; private set; }
+
private weak ContactStore store;
- private Folks.Individual? individual;
private Geary.Contact? contact;
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index 90b55aa2..8e456c23 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -125,7 +125,7 @@ public class GearyController : Geary.BaseObject {
public AutostartManager? autostart_manager { get; private set; default = null; }
public Application.AvatarStore? avatars {
- get; private set; default = null;
+ get; private set; default = new Application.AvatarStore();
}
public ContactListStoreCache contact_list_store_cache {
@@ -281,7 +281,6 @@ public class GearyController : Geary.BaseObject {
});
}
- this.avatars = new Application.AvatarStore(this.folks);
// Create the main window (must be done after creating actions.)
main_window = new MainWindow(this.application);
@@ -310,7 +309,11 @@ public class GearyController : Geary.BaseObject {
main_window.conversation_viewer.conversation_added.connect(
on_conversation_view_added
);
- new_messages_monitor = new NewMessagesMonitor(should_notify_new_messages);
+ this.new_messages_monitor = new NewMessagesMonitor(
+ this.avatars,
+ this.get_contact_store_for_account,
+ this.should_notify_new_messages
+ );
main_window.folder_list.set_new_messages_monitor(new_messages_monitor);
// New messages indicator (Ubuntuism)
@@ -321,9 +324,7 @@ public class GearyController : Geary.BaseObject {
unity_launcher = new UnityLauncher(new_messages_monitor);
- this.libnotify = new Libnotify(
- this.new_messages_monitor, this.avatars
- );
+ this.libnotify = new Libnotify(this.new_messages_monitor);
this.libnotify.invoked.connect(on_libnotify_invoked);
this.main_window.conversation_list_view.grab_focus();
@@ -1017,6 +1018,8 @@ public class GearyController : Geary.BaseObject {
main_window.folder_list.remove_account(account);
context.cancellable.cancel();
+ context.contacts.close();
+
Geary.Folder? inbox = context.inbox;
if (inbox != null) {
try {
@@ -2826,7 +2829,12 @@ public class GearyController : Geary.BaseObject {
private inline Geary.App.EmailStore? get_email_store_for_folder(Geary.Folder target) {
AccountContext? context = this.accounts.get(target.account.information);
- return context != null ? context.emails : null;
+ return (context != null) ? context.emails : null;
+ }
+
+ private Application.ContactStore? get_contact_store_for_account(Geary.Account target) {
+ AccountContext? context = this.accounts.get(target.information);
+ return (context != null) ? context.contacts : null;
}
private bool should_add_folder(Gee.Collection<Geary.Folder>? all,
diff --git a/src/client/conversation-viewer/conversation-contact-popover.vala
b/src/client/conversation-viewer/conversation-contact-popover.vala
index 38c5615b..b22dc697 100644
--- a/src/client/conversation-viewer/conversation-contact-popover.vala
+++ b/src/client/conversation-viewer/conversation-contact-popover.vala
@@ -96,6 +96,7 @@ public class Conversation.ContactPopover : Gtk.Popover {
int pixel_size = Application.AvatarStore.PIXEL_SIZE * window_scale;
try {
Gdk.Pixbuf? avatar_buf = yield loader.load(
+ this.contact,
this.mailbox,
pixel_size,
this.load_cancellable
diff --git a/src/client/conversation-viewer/conversation-message.vala
b/src/client/conversation-viewer/conversation-message.vala
index e7daec28..a61db46d 100644
--- a/src/client/conversation-viewer/conversation-message.vala
+++ b/src/client/conversation-viewer/conversation-message.vala
@@ -644,7 +644,10 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
int window_scale = get_scale_factor();
int pixel_size = Application.AvatarStore.PIXEL_SIZE * window_scale;
Gdk.Pixbuf? avatar_buf = yield loader.load(
- this.primary_originator, pixel_size, cancellable
+ this.primary_contact,
+ this.primary_originator,
+ pixel_size,
+ cancellable
);
if (avatar_buf != null) {
this.avatar.set_from_surface(
diff --git a/src/client/notification/libnotify.vala b/src/client/notification/libnotify.vala
index 11bf793f..dbce30bf 100644
--- a/src/client/notification/libnotify.vala
+++ b/src/client/notification/libnotify.vala
@@ -9,12 +9,9 @@ public class Libnotify : Geary.BaseObject {
public const Geary.Email.Field REQUIRED_FIELDS =
Geary.Email.Field.ORIGINATORS | Geary.Email.Field.SUBJECT;
- private const int AVATAR_SIZE = 32;
-
private static Canberra.Context? sound_context = null;
private weak NewMessagesMonitor monitor;
- private weak Application.AvatarStore avatars;
private Notify.Notification? current_notification = null;
private Notify.Notification? error_notification = null;
private Geary.Folder? folder = null;
@@ -23,10 +20,8 @@ public class Libnotify : Geary.BaseObject {
public signal void invoked(Geary.Folder? folder, Geary.Email? email);
- public Libnotify(NewMessagesMonitor monitor,
- Application.AvatarStore avatars) {
+ public Libnotify(NewMessagesMonitor monitor) {
this.monitor = monitor;
- this.avatars = avatars;
monitor.add_required_fields(REQUIRED_FIELDS);
@@ -51,8 +46,11 @@ public class Libnotify : Geary.BaseObject {
private void on_new_messages_arrived(Geary.Folder folder, int total, int added) {
if (added == 1 && monitor.last_new_message_folder != null &&
monitor.last_new_message != null) {
- notify_one_message_async.begin(monitor.last_new_message_folder,
- monitor.last_new_message, null);
+ notify_one_message_async.begin(
+ monitor.last_new_message_folder,
+ monitor.last_new_message,
+ null
+ );
} else if (added > 0) {
notify_new_mail(folder, added);
}
@@ -82,10 +80,9 @@ public class Libnotify : Geary.BaseObject {
issue_current_notification(folder.account.information.display_name, body, null);
}
- private async void notify_one_message_async(Geary.Folder folder, Geary.Email email,
- GLib.Cancellable? cancellable) throws GLib.Error {
- assert(email.fields.fulfills(REQUIRED_FIELDS));
-
+ private async void notify_one_message_async(Geary.Folder folder,
+ Geary.Email email,
+ GLib.Cancellable? cancellable) throws GLib.Error {
// used if notification is invoked
this.folder = folder;
this.email = email;
@@ -94,34 +91,45 @@ public class Libnotify : Geary.BaseObject {
!monitor.should_notify_new_messages(folder))
return;
- // possible to receive email with no originator
- Geary.RFC822.MailboxAddress? primary =
+ Geary.RFC822.MailboxAddress? originator =
Util.Email.get_primary_originator(email);
- if (primary == null) {
- notify_new_mail(folder, 1);
-
- return;
- }
+ if (originator != null) {
+ Application.ContactStore contacts =
+ this.monitor.get_contact_store(folder.account);
+ Application.Contact contact = yield contacts.load(
+ originator, cancellable
+ );
+
+ string body;
+ int count = monitor.get_new_message_count(folder);
+ if (count <= 1) {
+ body = Util.Email.strip_subject_prefixes(email);
+ } else {
+ body = ngettext(
+ "%s\n(%d other new message for %s)",
+ "%s\n(%d other new messages for %s)", count - 1).printf(
+ Util.Email.strip_subject_prefixes(email),
+ count - 1,
+ folder.account.information.display_name
+ );
+ }
- string body;
- int count = monitor.get_new_message_count(folder);
- if (count <= 1) {
- body = Util.Email.strip_subject_prefixes(email);
+ Gdk.Pixbuf? avatar = yield this.monitor.avatars.load(
+ contact,
+ originator,
+ Application.AvatarStore.PIXEL_SIZE,
+ cancellable
+ );
+
+ issue_current_notification(
+ contact.is_trusted
+ ? contact.display_name : originator.to_short_display(),
+ body,
+ avatar
+ );
} else {
- body = ngettext(
- "%s\n(%d other new message for %s)",
- "%s\n(%d other new messages for %s)", count - 1).printf(
- Util.Email.strip_subject_prefixes(email),
- count - 1,
- folder.account.information.display_name
- );
+ notify_new_mail(folder, 1);
}
-
- Gdk.Pixbuf? avatar = yield this.avatars.load(
- primary, AVATAR_SIZE, cancellable
- );
-
- issue_current_notification(primary.to_short_display(), body, avatar);
}
private void issue_current_notification(string summary, string body, Gdk.Pixbuf? icon) {
diff --git a/src/client/notification/new-messages-monitor.vala
b/src/client/notification/new-messages-monitor.vala
index c18c2edc..71c2e807 100644
--- a/src/client/notification/new-messages-monitor.vala
+++ b/src/client/notification/new-messages-monitor.vala
@@ -12,8 +12,17 @@
// flags to the object with add_required_fields().
public class NewMessagesMonitor : Geary.BaseObject {
+
+
+ /** Monitor hook for obtaining a contact store for an account. */
+ public delegate Application.ContactStore? GetContactStore(
+ Geary.Account account
+ );
+
+ /** Monitor hook to determine if a folder should be notified about. */
public delegate bool ShouldNotifyNewMessages(Geary.Folder folder);
+
private class MonitorInformation : Geary.BaseObject {
public Geary.Folder folder;
public Cancellable? cancellable = null;
@@ -32,9 +41,16 @@ public class NewMessagesMonitor : Geary.BaseObject {
public Geary.Folder? last_new_message_folder { get; private set; default = null; }
public Geary.Email? last_new_message { get; private set; default = null; }
- private Gee.HashMap<Geary.Folder, MonitorInformation> folder_information
- = new Gee.HashMap<Geary.Folder, MonitorInformation>();
- private unowned ShouldNotifyNewMessages? _should_notify_new_messages;
+ /** Returns an avatar store to lookup avatars for notifications. */
+ public Application.AvatarStore avatars { get; private set; }
+
+
+ private Gee.Map<Geary.Folder, MonitorInformation> folder_information =
+ new Gee.HashMap<Geary.Folder, MonitorInformation>();
+
+ private unowned GetContactStore contact_store_delegate;
+ private unowned ShouldNotifyNewMessages notify_delegate;
+
public signal void folder_added(Geary.Folder folder);
@@ -51,12 +67,22 @@ public class NewMessagesMonitor : Geary.BaseObject {
*/
public signal void new_messages_retired(Geary.Folder folder, int total);
- public NewMessagesMonitor(ShouldNotifyNewMessages? should_notify_new_messages) {
- _should_notify_new_messages = should_notify_new_messages;
+ public NewMessagesMonitor(Application.AvatarStore avatars,
+ GetContactStore contact_store_delegate,
+ ShouldNotifyNewMessages notify_delegate) {
+ this.avatars = avatars;
+ this.contact_store_delegate = contact_store_delegate;
+ this.notify_delegate = notify_delegate;
}
+ /** Determines if notifications should be made for a specific folder. */
public bool should_notify_new_messages(Geary.Folder folder) {
- return (_should_notify_new_messages == null ? true : _should_notify_new_messages(folder));
+ return this.notify_delegate(folder);
+ }
+
+ /** Returns a contact store to lookup contacts for notifications. */
+ public Application.ContactStore? get_contact_store(Geary.Account account) {
+ return this.contact_store_delegate(account);
}
public void add_folder(Geary.Folder folder, Cancellable? cancellable = null) {
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
