[geary] Improved sorting and Gtk.ListStore manipulations & lookup: Bug #713190



commit e52652854354db379f70afb425a5261c6746d937
Author: Jim Nelson <jim yorba org>
Date:   Mon Feb 2 17:40:31 2015 -0800

    Improved sorting and Gtk.ListStore manipulations & lookup: Bug #713190
    
    Although this doesn't solve the problems described in the bug, there
    was some perceived improvement in reducing the occurrence.  These
    changes also clean up the code, being verbose about distinguishing
    between sorting by sent date (i.e. the Date: header field) and
    received date (i.e. in EmailProperties, i.e. IMAP's INTERNALDATE).
    There was one comparator that did not have a proper stabilizer; that's
    fixed here as well.

 src/client/application/geary-controller.vala       |   28 ++--
 .../conversation-list/conversation-list-store.vala |  148 +++++++++++++-------
 .../formatted-conversation-data.vala               |    4 +-
 .../conversation-viewer/conversation-viewer.vala   |   11 ++-
 src/client/util/util-email.vala                    |    6 +-
 src/engine/api/geary-email.vala                    |   85 +++++++-----
 src/engine/app/app-conversation.vala               |  119 ++++++++++------
 .../imap-engine/imap-engine-email-prefetcher.vala  |    4 +-
 8 files changed, 248 insertions(+), 157 deletions(-)
---
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index 2a64731..7a055f4 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -1117,8 +1117,6 @@ public class GearyController : Geary.BaseObject {
         account.sending_monitor.start.disconnect(on_sending_started);
         account.sending_monitor.finish.disconnect(on_sending_finished);
         
-        if (main_window.conversation_list_store.account_owner_email == account.information.email)
-            main_window.conversation_list_store.account_owner_email = null;
         main_window.folder_list.remove_account(account);
         
         if (inboxes.has_key(account)) {
@@ -1307,9 +1305,6 @@ public class GearyController : Geary.BaseObject {
         if (!(current_folder is Geary.SearchFolder))
             previous_non_search_folder = current_folder;
         
-        main_window.conversation_list_store.set_current_folder(current_folder, conversation_cancellable);
-        main_window.conversation_list_store.account_owner_email = current_account.information.email;
-        
         main_window.main_toolbar.copy_folder_menu.clear();
         main_window.main_toolbar.move_folder_menu.clear();
         foreach(Geary.Folder f in current_folder.account.list_folders()) {
@@ -1418,7 +1413,7 @@ public class GearyController : Geary.BaseObject {
             return;
         
         // TODO: Determine how to map between conversations and drafts correctly.
-        on_edit_draft(activated.get_latest_email(Geary.App.Conversation.Location.IN_FOLDER));
+        on_edit_draft(activated.get_latest_recv_email(Geary.App.Conversation.Location.IN_FOLDER));
     }
     
     private void on_edit_draft(Geary.Email draft) {
@@ -1649,11 +1644,13 @@ public class GearyController : Geary.BaseObject {
         dialog.run();
     }
     
+    // latest_sent_only uses Email's Date: field, which corresponds to how they're sorted in the
+    // ConversationViewer
     private Gee.ArrayList<Geary.EmailIdentifier> get_conversation_email_ids(
-        Geary.App.Conversation conversation, bool latest_only,
+        Geary.App.Conversation conversation, bool latest_sent_only,
         Gee.ArrayList<Geary.EmailIdentifier> add_to) {
-        if (latest_only) {
-            Geary.Email? latest = conversation.get_latest_email(
+        if (latest_sent_only) {
+            Geary.Email? latest = conversation.get_latest_sent_email(
                 Geary.App.Conversation.Location.IN_FOLDER_OUT_OF_FOLDER);
             if (latest != null)
                 add_to.add(latest.id);
@@ -1665,20 +1662,19 @@ public class GearyController : Geary.BaseObject {
     }
     
     private Gee.Collection<Geary.EmailIdentifier> get_conversation_collection_email_ids(
-        Gee.Collection<Geary.App.Conversation> conversations, bool latest_only = false) {
+        Gee.Collection<Geary.App.Conversation> conversations, bool latest_sent_only) {
         Gee.ArrayList<Geary.EmailIdentifier> ret = new Gee.ArrayList<Geary.EmailIdentifier>();
         
         foreach(Geary.App.Conversation c in conversations)
-            get_conversation_email_ids(c, latest_only, ret);
+            get_conversation_email_ids(c, latest_sent_only, ret);
         
         return ret;
     }
     
-    private Gee.ArrayList<Geary.EmailIdentifier> get_selected_email_ids(
-        bool latest_only) {
+    private Gee.ArrayList<Geary.EmailIdentifier> get_selected_email_ids(bool latest_sent_only) {
         Gee.ArrayList<Geary.EmailIdentifier> ids = new Gee.ArrayList<Geary.EmailIdentifier>();
         foreach (Geary.App.Conversation conversation in selected_conversations)
-            get_conversation_email_ids(conversation, latest_only, ids);
+            get_conversation_email_ids(conversation, latest_sent_only, ids);
         return ids;
     }
     
@@ -1701,7 +1697,9 @@ public class GearyController : Geary.BaseObject {
             
             // Only check the messages that "Mark as Unread" would mark, so we
             // don't add the menu option and have it not do anything.
-            Geary.Email? latest = conversation.get_latest_email(
+            //
+            // Sort by Date: field to correspond with ConversationViewer ordering
+            Geary.Email? latest = conversation.get_latest_sent_email(
                 Geary.App.Conversation.Location.IN_FOLDER_OUT_OF_FOLDER);
             if (latest != null && latest.email_flags != null
                 && !latest.email_flags.contains(Geary.EmailFlags.UNREAD))
diff --git a/src/client/conversation-list/conversation-list-store.vala 
b/src/client/conversation-list/conversation-list-store.vala
index 179937b..12859cd 100644
--- a/src/client/conversation-list/conversation-list-store.vala
+++ b/src/client/conversation-list/conversation-list-store.vala
@@ -3,7 +3,16 @@
  * This software is licensed under the GNU Lesser General Public License
  * (version 2.1 or later).  See the COPYING file in this distribution.
  */
- 
+
+/**
+ * A Gtk.ListStore of sorted { link Geary.App.Conversation}s.
+ *
+ * Conversations are sorted by { link Geary.EmailProperties.date_received} (IMAP's INTERNALDATE)
+ * rather than the Date: header, as that ensures newly received email sort to the top where the
+ * user expects to see them.  The ConversationViewer sorts by the Date: header, as that presents
+ * better to the user.
+ */
+
 public class ConversationListStore : Gtk.ListStore {
     public const Geary.Email.Field REQUIRED_FIELDS =
         Geary.Email.Field.ENVELOPE | Geary.Email.Field.FLAGS | Geary.Email.Field.PROPERTIES;
@@ -13,12 +22,14 @@ public class ConversationListStore : Gtk.ListStore {
     
     public enum Column {
         CONVERSATION_DATA,
-        CONVERSATION_OBJECT;
+        CONVERSATION_OBJECT,
+        ROW_WRAPPER;
         
         public static Type[] get_types() {
             return {
                 typeof (FormattedConversationData), // CONVERSATION_DATA
-                typeof (Geary.App.Conversation)     // CONVERSATION_OBJECT
+                typeof (Geary.App.Conversation),    // CONVERSATION_OBJECT
+                typeof (RowWrapper)                 // ROW_WRAPPER
             };
         }
         
@@ -26,24 +37,50 @@ public class ConversationListStore : Gtk.ListStore {
             switch (this) {
                 case CONVERSATION_DATA:
                     return "data";
+                
                 case CONVERSATION_OBJECT:
                     return "envelope";
                 
+                case ROW_WRAPPER:
+                    return "wrapper";
+                
                 default:
                     assert_not_reached();
             }
         }
     }
     
-    public string? account_owner_email { get; set; default = null; }
+    private class RowWrapper : Geary.BaseObject {
+        public Geary.App.Conversation conversation;
+        public Gtk.TreeRowReference row;
+        
+        public RowWrapper(Gtk.TreeModel model, Geary.App.Conversation conversation, Gtk.TreePath path) {
+            this.conversation = conversation;
+            this.row = new Gtk.TreeRowReference(model, path);
+        }
+        
+        public Gtk.TreePath get_path() {
+            return row.get_path();
+        }
+        
+        public Gtk.TreeIter get_iter() {
+            Gtk.TreeIter iter;
+            bool valid = row.get_model().get_iter(out iter, get_path());
+            assert(valid);
+            
+            return iter;
+        }
+    }
+    
     public Geary.ProgressMonitor preview_monitor { get; private set; default = 
         new Geary.SimpleProgressMonitor(Geary.ProgressType.ACTIVITY); }
     public bool is_clearing { get; private set; default = false; }
     
     private Geary.App.ConversationMonitor conversation_monitor;
-    private Geary.Folder? current_folder = null;
+    private Gee.HashMap<Geary.App.Conversation, RowWrapper> row_map = new Gee.HashMap<
+        Geary.App.Conversation, RowWrapper>();
     private Geary.App.EmailStore? email_store = null;
-    private Cancellable? cancellable_folder = null;
+    private Cancellable cancellable = new Cancellable();
     private bool loading_local_only = true;
     private Geary.Nonblocking.Mutex refresh_mutex = new Geary.Nonblocking.Mutex();
     private uint update_id = 0;
@@ -68,6 +105,9 @@ public class ConversationListStore : Gtk.ListStore {
     ~ConversationListStore() {
         if (update_id != 0)
             Source.remove(update_id);
+        
+        GearyApplication.instance.controller.notify[GearyController.PROP_CURRENT_CONVERSATION].
+            disconnect(on_conversation_monitor_changed);
     }
     
     private void on_conversation_monitor_changed() {
@@ -82,10 +122,17 @@ public class ConversationListStore : Gtk.ListStore {
             conversation_monitor.email_flags_changed.disconnect(on_email_flags_changed);
         }
         
+        cancellable.cancel();
+        cancellable = new Cancellable();
+        
         clear();
+        row_map.clear();
         conversation_monitor = GearyApplication.instance.controller.current_conversations;
+        email_store = null;
         
         if (conversation_monitor != null) {
+            email_store = new Geary.App.EmailStore(conversation_monitor.folder.account);
+            
             // add all existing conversations
             on_conversations_added(conversation_monitor.get_conversations());
             
@@ -100,12 +147,6 @@ public class ConversationListStore : Gtk.ListStore {
         is_clearing = false;
     }
     
-    public void set_current_folder(Geary.Folder? current_folder, Cancellable? cancellable_folder) {
-        this.current_folder = current_folder;
-        this.cancellable_folder = cancellable_folder;
-        email_store = (current_folder == null ? null : new Geary.App.EmailStore(current_folder.account));
-    }
-    
     public Geary.App.Conversation? get_conversation_at_path(Gtk.TreePath path) {
         Gtk.TreeIter iter;
         if (!get_iter(out iter, path))
@@ -114,14 +155,6 @@ public class ConversationListStore : Gtk.ListStore {
         return get_conversation_at_iter(iter);
     }
     
-    public Gtk.TreePath? get_path_for_conversation(Geary.App.Conversation conversation) {
-        Gtk.TreeIter iter;
-        if (!get_iter_for_conversation(conversation, out iter))
-            return null;
-        
-        return get_path(iter);
-    }
-    
     private async void refresh_previews_async(Geary.App.ConversationMonitor conversation_monitor) {
         // Use a mutex because it's possible for the conversation monitor to fire multiple
         // "scan-started" signals as messages come in fast and furious, but only want to process
@@ -151,7 +184,7 @@ public class ConversationListStore : Gtk.ListStore {
     
     // should only be called by refresh_previews_async()
     private async void do_refresh_previews_async(Geary.App.ConversationMonitor conversation_monitor) {
-        if (current_folder == null || !GearyApplication.instance.config.display_preview)
+        if (conversation_monitor == null || !GearyApplication.instance.config.display_preview)
             return;
         
         Gee.Set<Geary.EmailIdentifier> needing_previews = get_emails_needing_previews();
@@ -162,7 +195,7 @@ public class ConversationListStore : Gtk.ListStore {
         if (emails.size < 1)
             return;
         
-        debug("Displaying %d previews for %s...", emails.size, current_folder.to_string());
+        debug("Displaying %d previews for %s...", emails.size, conversation_monitor.folder.to_string());
         foreach (Geary.Email email in emails) {
             Geary.App.Conversation? conversation = conversation_monitor.get_conversation_for_email(email.id);
             if (conversation != null)
@@ -170,7 +203,7 @@ public class ConversationListStore : Gtk.ListStore {
             else
                 debug("Couldn't find conversation for %s", email.id.to_string());
         }
-        debug("Displayed %d previews for %s", emails.size, current_folder.to_string());
+        debug("Displayed %d previews for %s", emails.size, conversation_monitor.folder.to_string());
     }
     
     private async Gee.Collection<Geary.Email> do_get_previews_async(
@@ -181,7 +214,7 @@ public class ConversationListStore : Gtk.ListStore {
         try {
             debug("Loading %d previews...", emails_needing_previews.size);
             emails = yield email_store.list_email_by_sparse_id_async(emails_needing_previews,
-                ConversationListStore.WITH_PREVIEW_FIELDS, flags, cancellable_folder);
+                ConversationListStore.WITH_PREVIEW_FIELDS, flags, cancellable);
             debug("Loaded %d previews...", emails_needing_previews.size);
         } catch (Error err) {
             // Ignore NOT_FOUND, as that's entirely possible when waiting for the remote to open
@@ -203,7 +236,7 @@ public class ConversationListStore : Gtk.ListStore {
         foreach (Geary.App.Conversation conversation in sorted_conversations) {
             // find oldest unread message for the preview
             Geary.Email? need_preview = null;
-            foreach (Geary.Email email in 
conversation.get_emails(Geary.App.Conversation.Ordering.DATE_ASCENDING)) {
+            foreach (Geary.Email email in 
conversation.get_emails(Geary.App.Conversation.Ordering.RECV_DATE_ASCENDING)) {
                 if (email.email_flags.is_unread()) {
                     need_preview = email;
                     
@@ -214,7 +247,7 @@ public class ConversationListStore : Gtk.ListStore {
             // if all are read, use newest in-folder message, then newest out-of-folder if not
             // present
             if (need_preview == null) {
-                need_preview = 
conversation.get_latest_email(Geary.App.Conversation.Location.IN_FOLDER_OUT_OF_FOLDER);
+                need_preview = 
conversation.get_latest_recv_email(Geary.App.Conversation.Location.IN_FOLDER_OUT_OF_FOLDER);
                 if (need_preview == null)
                     continue;
             }
@@ -256,10 +289,19 @@ public class ConversationListStore : Gtk.ListStore {
     
     private void set_row(Gtk.TreeIter iter, Geary.App.Conversation conversation, Geary.Email preview) {
         FormattedConversationData conversation_data = new FormattedConversationData(conversation,
-            preview, current_folder, account_owner_email);
+            preview, conversation_monitor.folder, conversation_monitor.folder.account.information.email);
+        
+        Gtk.TreePath? path = get_path(iter);
+        assert(path != null);
+        RowWrapper wrapper = new RowWrapper(this, conversation, path);
+        
         set(iter,
             Column.CONVERSATION_DATA, conversation_data,
-            Column.CONVERSATION_OBJECT, conversation);
+            Column.CONVERSATION_OBJECT, conversation,
+            Column.ROW_WRAPPER, wrapper
+        );
+        
+        row_map.set(conversation, wrapper);
     }
     
     private void refresh_conversation(Geary.App.Conversation conversation) {
@@ -270,7 +312,7 @@ public class ConversationListStore : Gtk.ListStore {
             return;
         }
         
-        Geary.Email? last_email = conversation.get_latest_email(Geary.App.Conversation.Location.ANYWHERE);
+        Geary.Email? last_email = 
conversation.get_latest_recv_email(Geary.App.Conversation.Location.ANYWHERE);
         if (last_email == null) {
             debug("Cannot refresh conversation: last email is null");
             
@@ -278,21 +320,13 @@ public class ConversationListStore : Gtk.ListStore {
             return;
         }
         
-        FormattedConversationData? existing_message_data = get_message_data_at_iter(iter);
+        set_row(iter, conversation, last_email);
         
-        if (existing_message_data == null || !existing_message_data.preview.id.equal_to(last_email.id)) {
-            set_row(iter, conversation, last_email);
-        } else if (existing_message_data != null &&
-            existing_message_data.num_emails != conversation.get_count()) {
-            existing_message_data.num_emails = conversation.get_count();
-            
-            Gtk.TreePath? path = get_path(iter);
-            if (path != null) {
-                row_changed(path, iter);
-            } else {
-                debug("Cannot refresh conversation: no path for iterator");
-            }
-        }
+        Gtk.TreePath? path = get_path(iter);
+        if (path != null)
+            row_changed(path, iter);
+        else
+            debug("Cannot refresh conversation: no path for iterator");
     }
     
     private void refresh_flags(Geary.App.Conversation conversation) {
@@ -315,20 +349,26 @@ public class ConversationListStore : Gtk.ListStore {
             row_changed(path, iter);
     }
     
-    private bool get_iter_for_conversation(Geary.App.Conversation conversation, out Gtk.TreeIter iter) {
-        if (!get_iter_first(out iter))
-            return false;
+    public Gtk.TreePath? get_path_for_conversation(Geary.App.Conversation conversation) {
+        RowWrapper? wrapper = row_map.get(conversation);
         
-        do {
-            if (get_conversation_at_iter(iter) == conversation)
-                return true;
-        } while (iter_next(ref iter));
+        return (wrapper != null) ? wrapper.get_path() : null;
+    }
+    
+    private bool get_iter_for_conversation(Geary.App.Conversation conversation, out Gtk.TreeIter iter) {
+        // use get_iter_first() because boxing Gtk.TreeIter with a nullable is problematic with
+        // current bindings
+        RowWrapper? wrapper = row_map.get(conversation);
+        if (wrapper != null)
+            iter = wrapper.get_iter();
+        else
+            get_iter_first(out iter);
         
-        return false;
+        return wrapper != null;
     }
     
     private bool has_conversation(Geary.App.Conversation conversation) {
-        return get_iter_for_conversation(conversation, null);
+        return row_map.has_key(conversation);
     }
     
     private Geary.App.Conversation? get_conversation_at_iter(Gtk.TreeIter iter) {
@@ -349,10 +389,12 @@ public class ConversationListStore : Gtk.ListStore {
         Gtk.TreeIter iter;
         if (get_iter_for_conversation(conversation, out iter))
             remove(iter);
+        
+        row_map.remove(conversation);
     }
     
     private bool add_conversation(Geary.App.Conversation conversation) {
-        Geary.Email? last_email = conversation.get_latest_email(Geary.App.Conversation.Location.ANYWHERE);
+        Geary.Email? last_email = 
conversation.get_latest_recv_email(Geary.App.Conversation.Location.ANYWHERE);
         if (last_email == null) {
             debug("Cannot add conversation: last email is null");
             
diff --git a/src/client/conversation-list/formatted-conversation-data.vala 
b/src/client/conversation-list/formatted-conversation-data.vala
index 65e02ae..211361a 100644
--- a/src/client/conversation-list/formatted-conversation-data.vala
+++ b/src/client/conversation-list/formatted-conversation-data.vala
@@ -116,7 +116,7 @@ public class FormattedConversationData : Geary.BaseObject {
     
     public bool update_date_string() {
         // get latest email *in folder* for the conversation's date, fall back on out-of-folder
-        Geary.Email? latest = 
conversation.get_latest_email(Geary.App.Conversation.Location.IN_FOLDER_OUT_OF_FOLDER);
+        Geary.Email? latest = 
conversation.get_latest_recv_email(Geary.App.Conversation.Location.IN_FOLDER_OUT_OF_FOLDER);
         if (latest == null || latest.properties == null)
             return false;
         
@@ -181,7 +181,7 @@ public class FormattedConversationData : Geary.BaseObject {
         // Build chronological list of AuthorDisplay records, setting to unread if any message by
         // that author is unread
         Gee.ArrayList<ParticipantDisplay> list = new Gee.ArrayList<ParticipantDisplay>();
-        foreach (Geary.Email message in 
conversation.get_emails(Geary.App.Conversation.Ordering.DATE_ASCENDING)) {
+        foreach (Geary.Email message in 
conversation.get_emails(Geary.App.Conversation.Ordering.RECV_DATE_ASCENDING)) {
             // only display if something to display
             Geary.RFC822.MailboxAddresses? addresses = use_to ? message.to : message.from;
             if (addresses == null || addresses.size < 1)
diff --git a/src/client/conversation-viewer/conversation-viewer.vala 
b/src/client/conversation-viewer/conversation-viewer.vala
index d07c3df..01b5f35 100644
--- a/src/client/conversation-viewer/conversation-viewer.vala
+++ b/src/client/conversation-viewer/conversation-viewer.vala
@@ -4,6 +4,13 @@
  * (version 2.1 or later).  See the COPYING file in this distribution.
  */
 
+/**
+ * A WebKit view displaying all the emails in a { link Geary.App.Conversation}.
+ *
+ * Unlike ConversationListStore (which sorts by date received), ConversationViewer sorts by the
+ * { link Geary.Email.date} field (the Date: header), as that's the date displayed to the user.
+ */
+
 public class ConversationViewer : Gtk.Box {
     public const Geary.Email.Field REQUIRED_FIELDS =
         Geary.Email.Field.HEADER
@@ -129,7 +136,7 @@ public class ConversationViewer : Gtk.Box {
     
     // List of emails in this view.
     public Gee.TreeSet<Geary.Email> messages { get; private set; default = 
-        new Gee.TreeSet<Geary.Email>(Geary.Email.compare_date_ascending); }
+        new Gee.TreeSet<Geary.Email>(Geary.Email.compare_sent_date_ascending); }
     
     // The HTML viewer to view the emails.
     public ConversationWebView web_view { get; private set; }
@@ -499,7 +506,7 @@ public class ConversationViewer : Gtk.Box {
         // Fetch full messages.
         Gee.Collection<Geary.Email>? messages_to_add
             = yield list_full_messages_async(conversation.get_emails(
-            Geary.App.Conversation.Ordering.DATE_ASCENDING), cancellable);
+            Geary.App.Conversation.Ordering.SENT_DATE_ASCENDING), cancellable);
         
         // Add messages.
         if (messages_to_add != null) {
diff --git a/src/client/util/util-email.vala b/src/client/util/util-email.vala
index 1f0c66f..464888f 100644
--- a/src/client/util/util-email.vala
+++ b/src/client/util/util-email.vala
@@ -5,8 +5,8 @@
  */
 
 public int compare_conversation_ascending(Geary.App.Conversation a, Geary.App.Conversation b) {
-    Geary.Email? a_latest = a.get_latest_email(Geary.App.Conversation.Location.IN_FOLDER_OUT_OF_FOLDER);
-    Geary.Email? b_latest = b.get_latest_email(Geary.App.Conversation.Location.IN_FOLDER_OUT_OF_FOLDER);
+    Geary.Email? a_latest = a.get_latest_recv_email(Geary.App.Conversation.Location.IN_FOLDER_OUT_OF_FOLDER);
+    Geary.Email? b_latest = b.get_latest_recv_email(Geary.App.Conversation.Location.IN_FOLDER_OUT_OF_FOLDER);
     
     if (a_latest == null)
         return (b_latest == null) ? 0 : -1;
@@ -15,7 +15,7 @@ public int compare_conversation_ascending(Geary.App.Conversation a, Geary.App.Co
     
     // use date-received so newly-arrived messages float to the top, even if they're send date
     // was earlier (think of mailing lists that batch up forwarded mail)
-    return a_latest.properties.date_received.compare(b_latest.properties.date_received);
+    return Geary.Email.compare_recv_date_ascending(a_latest, b_latest);
 }
 
 public int compare_conversation_descending(Geary.App.Conversation a, Geary.App.Conversation b) {
diff --git a/src/engine/api/geary-email.vala b/src/engine/api/geary-email.vala
index 6100252..26ab8ae 100644
--- a/src/engine/api/geary-email.vala
+++ b/src/engine/api/geary-email.vala
@@ -361,24 +361,61 @@ public class Geary.Email : BaseObject {
     }
     
     /**
-     * CompareFunc to sort Email by date.  If the date field is not available on both Emails, their
-     * identifiers are compared.
+     * CompareFunc to sort { link Email} by { link date} ascending.
+     *
+     * If the date field is unavailable on either Email, their identifiers are compared to
+     * stabilize the sort.
      */
-    public static int compare_date_ascending(Geary.Email aemail, Geary.Email bemail) {
-        int diff = 0;
-        if (aemail.date != null && bemail.date != null)
-            diff = aemail.date.value.compare(bemail.date.value);
+    public static int compare_sent_date_ascending(Geary.Email aemail, Geary.Email bemail) {
+        if (aemail.date == null || bemail.date == null) {
+            GLib.message("Warning: comparing email for sent date but no Date: field loaded");
+            
+            return compare_id_ascending(aemail, bemail);
+        }
+        
+        int compare = aemail.date.value.compare(bemail.date.value);
         
         // stabilize sort by using the mail identifier's stable sort ordering
-        return (diff != 0) ? diff : compare_id_ascending(aemail, bemail);
+        return (compare != 0) ? compare : compare_id_ascending(aemail, bemail);
     }
     
     /**
-     * CompareFunc to sort Email by date.  If the date field is not available on both Emails, their
-     * identifiers are compared.
+     * CompareFunc to sort { link Email} by { link date} descending.
+     *
+     * If the date field is unavailable on either Email, their identifiers are compared to
+     * stabilize the sort.
      */
-    public static int compare_date_descending(Geary.Email aemail, Geary.Email bemail) {
-        return compare_date_ascending(bemail, aemail);
+    public static int compare_sent_date_descending(Geary.Email aemail, Geary.Email bemail) {
+        return compare_sent_date_ascending(bemail, aemail);
+    }
+    
+    /**
+     * CompareFunc to sort { link Email} by { link EmailProperties.date_received} ascending.
+     *
+     * If { link properties} is unavailable on either Email, their identifiers are compared to
+     * stabilize the sort.
+     */
+    public static int compare_recv_date_ascending(Geary.Email aemail, Geary.Email bemail) {
+        if (aemail.properties == null || bemail.properties == null) {
+            GLib.message("Warning: comparing email for received date but email properties not loaded");
+            
+            return compare_id_ascending(aemail, bemail);
+        }
+        
+        int compare = aemail.properties.date_received.compare(bemail.properties.date_received);
+        
+        // stabilize sort with identifiers
+        return (compare != 0) ? compare : compare_id_ascending(aemail, bemail);
+    }
+    
+    /**
+     * CompareFunc to sort { link Email} by { link EmailProperties.date_received} descending.
+     *
+     * If { link properties} is unavailable on either Email, their identifiers are compared to
+     * stabilize the sort.
+     */
+    public static int compare_recv_date_descending(Geary.Email aemail, Geary.Email bemail) {
+        return compare_recv_date_ascending(bemail, aemail);
     }
     
     // only used to stabilize a sort
@@ -394,8 +431,11 @@ public class Geary.Email : BaseObject {
         Geary.EmailProperties? aprop = (Geary.EmailProperties) aemail.properties;
         Geary.EmailProperties? bprop = (Geary.EmailProperties) bemail.properties;
         
-        if (aprop == null || bprop == null)
+        if (aprop == null || bprop == null) {
+            GLib.message("Warning: comparing email by size but email properties not loaded");
+            
             return compare_id_ascending(aemail, bemail);
+        }
         
         int cmp = (int) (aprop.total_bytes - bprop.total_bytes).clamp(-1, 1);
         
@@ -409,26 +449,5 @@ public class Geary.Email : BaseObject {
     public static int compare_size_descending(Geary.Email aemail, Geary.Email bemail) {
         return compare_size_ascending(bemail, aemail);
     }
-    
-    /**
-     * CompareFunc to sort Email by EmailProperties.date_received.  If not available, emails are
-     * compared by EmailIdentifier.
-     */
-    public static int compare_date_received_ascending(Geary.Email aemail, Geary.Email bemail) {
-        if (aemail.properties == null || bemail.properties == null)
-            return compare_id_ascending(aemail, bemail);
-        
-        int cmp = aemail.properties.date_received.compare(bemail.properties.date_received);
-        
-        return (cmp != 0) ? cmp : compare_id_ascending(aemail, bemail);
-    }
-    
-    /**
-     * CompareFunc to sort Email by EmailProperties.date_received.  If not available, emails are
-     * compared by EmailIdentifier.
-     */
-    public static int compare_date_received_descending(Geary.Email aemail, Geary.Email bemail) {
-        return compare_date_received_ascending(bemail, aemail);
-    }
 }
 
diff --git a/src/engine/app/app-conversation.vala b/src/engine/app/app-conversation.vala
index 121a707..92db693 100644
--- a/src/engine/app/app-conversation.vala
+++ b/src/engine/app/app-conversation.vala
@@ -13,8 +13,10 @@ public class Geary.App.Conversation : BaseObject {
      */
     public enum Ordering {
         NONE,
-        DATE_ASCENDING,
-        DATE_DESCENDING,
+        SENT_DATE_ASCENDING,
+        SENT_DATE_DESCENDING,
+        RECV_DATE_ASCENDING,
+        RECV_DATE_DESCENDING
     }
     
     /**
@@ -46,10 +48,14 @@ public class Geary.App.Conversation : BaseObject {
     
     // this isn't ideal but the cost of adding an email to multiple sorted sets once versus
     // the number of times they're accessed makes it worth it
-    private Gee.SortedSet<Email> date_ascending = new Gee.TreeSet<Email>(
-        Geary.Email.compare_date_ascending);
-    private Gee.SortedSet<Email> date_descending = new Gee.TreeSet<Email>(
-        Geary.Email.compare_date_descending);
+    private Gee.SortedSet<Email> sent_date_ascending = new Gee.TreeSet<Email>(
+        Geary.Email.compare_sent_date_ascending);
+    private Gee.SortedSet<Email> sent_date_descending = new Gee.TreeSet<Email>(
+        Geary.Email.compare_sent_date_descending);
+    private Gee.SortedSet<Email> recv_date_ascending = new Gee.TreeSet<Email>(
+        Geary.Email.compare_recv_date_ascending);
+    private Gee.SortedSet<Email> recv_date_descending = new Gee.TreeSet<Email>(
+        Geary.Email.compare_recv_date_descending);
     
     // by storing all paths for each EmailIdentifier, can lookup without blocking
     private Gee.HashMultiMap<Geary.EmailIdentifier, Geary.FolderPath> path_map = new Gee.HashMultiMap<
@@ -120,26 +126,34 @@ public class Geary.App.Conversation : BaseObject {
     }
     
     /**
-     * Returns all the email in the conversation sorted according to the specifier.
+     * Returns all the email in the conversation sorted and filtered according to the specifiers.
      *
      * { link Location.IN_FOLDER} and { link Location.OUT_OF_FOLDER} are the
      * only preferences honored; the others ({ link Location.IN_FOLDER_OUT_OF_FOLDER},
      * { link Location.IN_FOLDER_OUT_OF_FOLDER}, and { link Location.ANYWHERE}
      * are all treated as ANYWHERE.
      */
-    public Gee.List<Geary.Email> get_emails(Ordering ordering, Location location = Location.ANYWHERE) {
-        Gee.List<Geary.Email> list;
+    public Gee.Collection<Geary.Email> get_emails(Ordering ordering, Location location = Location.ANYWHERE) {
+        Gee.Collection<Geary.Email> email;
         switch (ordering) {
-            case Ordering.DATE_ASCENDING:
-                list = Collection.to_array_list<Email>(date_ascending);
+            case Ordering.SENT_DATE_ASCENDING:
+                email = sent_date_ascending;
             break;
             
-            case Ordering.DATE_DESCENDING:
-                list = Collection.to_array_list<Email>(date_descending);
+            case Ordering.SENT_DATE_DESCENDING:
+                email = sent_date_descending;
+            break;
+            
+            case Ordering.RECV_DATE_ASCENDING:
+                email = recv_date_ascending;
+            break;
+            
+            case Ordering.RECV_DATE_DESCENDING:
+                email = recv_date_descending;
             break;
             
             case Ordering.NONE:
-                list = Collection.to_array_list<Email>(emails.values);
+                email = emails.values;
             break;
             
             default:
@@ -148,28 +162,29 @@ public class Geary.App.Conversation : BaseObject {
         
         switch (location) {
             case Location.IN_FOLDER:
-                Collection.remove_if<Email>(list, (email) => {
-                    return !is_in_current_folder(email.id);
-                });
+                email = traverse<Email>(email)
+                    .filter((e) => !is_in_current_folder(e.id))
+                    .to_array_list();
             break;
             
             case Location.OUT_OF_FOLDER:
-                Collection.remove_if<Email>(list, (email) => {
-                    return is_in_current_folder(email.id);
-                });
+                email = traverse<Email>(email)
+                    .filter((e) => is_in_current_folder(e.id))
+                    .to_array_list();
             break;
             
             case Location.IN_FOLDER_OUT_OF_FOLDER:
             case Location.OUT_OF_FOLDER_IN_FOLDER:
             case Location.ANYWHERE:
-                // let the list pass untouched
+                // make a modifiable copy
+                email = traverse<Email>(email).to_array_list();
             break;
             
             default:
                 assert_not_reached();
         }
         
-        return list;
+        return email;
     }
     
     public bool is_in_current_folder(Geary.EmailIdentifier id) {
@@ -214,8 +229,10 @@ public class Geary.App.Conversation : BaseObject {
             return false;
         
         emails.set(email.id, email);
-        date_ascending.add(email);
-        date_descending.add(email);
+        sent_date_ascending.add(email);
+        sent_date_descending.add(email);
+        recv_date_ascending.add(email);
+        recv_date_descending.add(email);
         
         Gee.Set<RFC822.MessageID>? ancestors = email.get_ancestors();
         if (ancestors != null)
@@ -232,8 +249,10 @@ public class Geary.App.Conversation : BaseObject {
     // Returns the removed Message-IDs
     internal Gee.Set<RFC822.MessageID>? remove(Email email) {
         emails.unset(email.id);
-        date_ascending.remove(email);
-        date_descending.remove(email);
+        sent_date_ascending.remove(email);
+        sent_date_descending.remove(email);
+        recv_date_ascending.remove(email);
+        recv_date_descending.remove(email);
         path_map.remove_all(email.id);
         
         Gee.Set<RFC822.MessageID> removed_message_ids = new Gee.HashSet<RFC822.MessageID>();
@@ -276,28 +295,36 @@ public class Geary.App.Conversation : BaseObject {
     
     /**
      * Returns the earliest (first sent) email in the Conversation.
-     *
-     * Note that sorting in { link Conversation} is done by the RFC822 Date: header (i.e.
-     * { link Email.date}) and not the date received (i.e. { link EmailProperties.date_received}).
      */
-    public Geary.Email? get_earliest_email(Location location) {
-        return get_single_email(Ordering.DATE_ASCENDING, location);
+    public Geary.Email? get_earliest_sent_email(Location location) {
+        return get_single_email(Ordering.SENT_DATE_ASCENDING, location);
     }
     
     /**
      * Returns the latest (most recently sent) email in the Conversation.
-     *
-     * Note that sorting in { link Conversation} is done by the RFC822 Date: header (i.e.
-     * { link Email.date}) and not the date received (i.e. { link EmailProperties.date_received}).
      */
-    public Geary.Email? get_latest_email(Location location) {
-        return get_single_email(Ordering.DATE_DESCENDING, location);
+    public Geary.Email? get_latest_sent_email(Location location) {
+        return get_single_email(Ordering.SENT_DATE_DESCENDING, location);
+    }
+    
+    /**
+     * Returns the earliest (first received) email in the Conversation.
+     */
+    public Geary.Email? get_earliest_recv_email(Location location) {
+        return get_single_email(Ordering.RECV_DATE_ASCENDING, location);
+    }
+    
+    /**
+     * Returns the latest (most recently received) email in the Conversation.
+     */
+    public Geary.Email? get_latest_recv_email(Location location) {
+        return get_single_email(Ordering.RECV_DATE_DESCENDING, location);
     }
     
     private Geary.Email? get_single_email(Ordering ordering, Location location) {
         // note that the location-ordering preferences are treated as ANYWHERE by get_emails()
-        Gee.List<Geary.Email> list = get_emails(ordering, location);
-        if (list.size == 0)
+        Gee.Collection<Geary.Email> all = get_emails(ordering, location);
+        if (all.size == 0)
             return null;
         
         // Because IN_FOLDER_OUT_OF_FOLDER and OUT_OF_FOLDER_IN_FOLDER are treated as ANYWHERE,
@@ -306,21 +333,19 @@ public class Geary.App.Conversation : BaseObject {
             case Location.IN_FOLDER:
             case Location.OUT_OF_FOLDER:
             case Location.ANYWHERE:
-                return Collection.get_first<Email>(list);
+                return traverse<Email>(all).first();
             
             case Location.IN_FOLDER_OUT_OF_FOLDER:
-                Geary.Email? found = Collection.find_first<Email>(list, (email) => {
-                    return is_in_current_folder(email.id);
-                });
+                Geary.Email? found = traverse<Email>(all)
+                    .first_matching((email) => is_in_current_folder(email.id));
                 
-                return (found != null) ? found : list.first();
+                return found ?? traverse<Email>(all).first();
             
             case Location.OUT_OF_FOLDER_IN_FOLDER:
-                Geary.Email? found = Collection.find_first<Email>(list, (email) => {
-                    return !is_in_current_folder(email.id);
-                });
+                Geary.Email? found = traverse<Email>(all)
+                    .first_matching((email) => !is_in_current_folder(email.id));
                 
-                return (found != null) ? found : list.first();
+                return found ?? traverse<Email>(all).first();
             
             default:
                 assert_not_reached();
diff --git a/src/engine/imap-engine/imap-engine-email-prefetcher.vala 
b/src/engine/imap-engine/imap-engine-email-prefetcher.vala
index 871963f..ff7dc71 100644
--- a/src/engine/imap-engine/imap-engine-email-prefetcher.vala
+++ b/src/engine/imap-engine/imap-engine-email-prefetcher.vala
@@ -24,7 +24,7 @@ private class Geary.ImapEngine.EmailPrefetcher : Object {
     private int start_delay_sec;
     private Nonblocking.Mutex mutex = new Nonblocking.Mutex();
     private Gee.TreeSet<Geary.Email> prefetch_emails = new Gee.TreeSet<Geary.Email>(
-        Email.compare_date_received_descending);
+        Email.compare_recv_date_descending);
     private uint schedule_id = 0;
     private Cancellable cancellable = new Cancellable();
     
@@ -167,7 +167,7 @@ private class Geary.ImapEngine.EmailPrefetcher : Object {
     private async void do_prefetch_batch_async() throws Error {
         // snarf up all requested Emails for this round
         Gee.TreeSet<Geary.Email> emails = prefetch_emails;
-        prefetch_emails = new Gee.TreeSet<Geary.Email>(Email.compare_date_received_descending);
+        prefetch_emails = new Gee.TreeSet<Geary.Email>(Email.compare_recv_date_descending);
         
         if (emails.size == 0)
             return;



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