GNOME.org
 

[geary/wip/713150-conversations] Further improvements

commit 5944b9b3aa5573d67d302bf736603c34e72115f4
Author: Jim Nelson <jim yorba org>
Date:   Thu Feb 26 18:25:01 2015 -0800

    Further improvements

 src/CMakeLists.txt                                 |    1 +
 src/engine/app/app-conversation-monitor.vala       |   43 +++++++++++++------
 src/engine/app/app-conversation.vala               |   21 ----------
 .../conversation-monitor/app-conversation-set.vala |    2 +
 .../app-external-remove-operation.vala             |   22 ++++++++++
 .../conversation-monitor/app-remove-operation.vala |    2 +-
 src/engine/imap-db/imap-db-account.vala            |   14 +++++-
 src/engine/imap-db/imap-db-conversation.vala       |   14 ++++--
 8 files changed, 75 insertions(+), 44 deletions(-)
---
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 327923a..aabd3b4 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -55,6 +55,7 @@ engine/app/conversation-monitor/app-conversation-operation-queue.vala
 engine/app/conversation-monitor/app-conversation-operation.vala
 engine/app/conversation-monitor/app-conversation-set.vala
 engine/app/conversation-monitor/app-external-append-operation.vala
+engine/app/conversation-monitor/app-external-remove-operation.vala
 engine/app/conversation-monitor/app-fill-window-operation.vala
 engine/app/conversation-monitor/app-local-load-operation.vala
 engine/app/conversation-monitor/app-remove-operation.vala
diff --git a/src/engine/app/app-conversation-monitor.vala b/src/engine/app/app-conversation-monitor.vala
index e706a39..f44b2ae 100644
--- a/src/engine/app/app-conversation-monitor.vala
+++ b/src/engine/app/app-conversation-monitor.vala
@@ -311,6 +311,7 @@ public class Geary.App.ConversationMonitor : BaseObject {
         folder.opened.connect(on_folder_opened);
         folder.account.email_flags_changed.connect(on_account_email_flags_changed);
         folder.account.email_locally_complete.connect(on_account_email_locally_complete);
+        folder.account.email_removed.connect(on_account_email_removed);
         
         try {
             yield folder.open_async(open_flags, cancellable);
@@ -323,6 +324,7 @@ public class Geary.App.ConversationMonitor : BaseObject {
             folder.opened.disconnect(on_folder_opened);
             folder.account.email_flags_changed.disconnect(on_account_email_flags_changed);
             folder.account.email_locally_complete.disconnect(on_account_email_locally_complete);
+            folder.account.email_removed.disconnect(on_account_email_removed);
             
             throw err;
         }
@@ -364,6 +366,7 @@ public class Geary.App.ConversationMonitor : BaseObject {
         folder.opened.disconnect(on_folder_opened);
         folder.account.email_flags_changed.disconnect(on_account_email_flags_changed);
         folder.account.email_locally_complete.disconnect(on_account_email_locally_complete);
+        folder.account.email_removed.disconnect(on_account_email_removed);
         
         Error? close_err = null;
         try {
@@ -575,9 +578,14 @@ public class Geary.App.ConversationMonitor : BaseObject {
         operation_queue.add(new FillWindowOperation(this, false));
     }
     
-    private void on_account_email_locally_complete(Geary.Folder folder,
-        Gee.Collection<Geary.EmailIdentifier> complete_ids) {
-        operation_queue.add(new ExternalAppendOperation(this, folder, complete_ids));
+    private void on_account_email_locally_complete(Folder folder, Gee.Collection<EmailIdentifier> 
complete_ids) {
+        if (!folder.path.equal_to(this.folder.path))
+            operation_queue.add(new ExternalAppendOperation(this, folder, complete_ids));
+    }
+    
+    private void on_account_email_removed(Folder folder, Gee.Collection<EmailIdentifier> removed_ids) {
+        if (!folder.path.equal_to(this.folder.path))
+            operation_queue.add(new ExternalRemoveOperation(this, folder, removed_ids));
     }
     
     internal async void append_emails_async(Gee.Collection<Geary.EmailIdentifier> appended_ids) {
@@ -587,9 +595,9 @@ public class Geary.App.ConversationMonitor : BaseObject {
         yield load_by_sparse_id(appended_ids, required_fields, Geary.Folder.ListFlags.NONE, null);
     }
     
-    internal async void remove_emails_async(Gee.Collection<Geary.EmailIdentifier> removed_ids) {
-        debug("%d messages(s) removed from %s, trimming/removing conversations...", removed_ids.size,
-            folder.to_string());
+    internal async void remove_emails_async(FolderPath path, Gee.Collection<EmailIdentifier> removed_ids) {
+        debug("%d messages(s) removed from %s, trimming/removing conversations in %s...", removed_ids.size,
+            path.to_string(), folder.to_string());
         
         Gee.HashSet<Conversation> removed_conversations = new Gee.HashSet<Conversation>();
         Gee.HashMultiMap<Conversation, Email> trimmed_conversations = new Gee.HashMultiMap<
@@ -598,26 +606,33 @@ public class Geary.App.ConversationMonitor : BaseObject {
         // remove the emails from internal state, noting which conversations are trimmed or flat-out
         // removed (evaporated)
         foreach (EmailIdentifier removed_id in removed_ids) {
-            primary_email_id_to_conversation.unset(removed_id);
+            // If processing removes from the primary folder, remove immediately from this mapping
+            if (path.equal_to(folder.path))
+                primary_email_id_to_conversation.unset(removed_id);
             
-            Conversation conversation;
-            if (!all_email_id_to_conversation.unset(removed_id, out conversation))
+            Conversation? conversation = all_email_id_to_conversation[removed_id];
+            if (conversation == null)
                 continue;
             
-            Geary.Email? removed_email = conversation.get_email_by_id(removed_id);
-            if (removed_email == null)
+            Email? email = conversation.get_email_by_id(removed_id);
+            if (email == null)
                 continue;
             
-            bool removed = conversation.remove(removed_email, folder.path);
+            // Remove from conversation by *path*, which means it may not be fully removed if
+            // detected in other paths
+            bool fully_removed = conversation.remove(email, path);
             
+            // if conversation is empty or has no messages in primary folder path, remove it
+            // entirely
             if (conversation.get_count() == 0 || !conversation.any_in_folder_path(folder.path)) {
                 // could have been trimmed earlier in the loop
                 trimmed_conversations.remove_all(conversation);
                 
                 conversations.remove(conversation);
                 removed_conversations.add(conversation);
-            } else if (removed) {
-                trimmed_conversations.set(conversation, removed_email);
+            } else if (fully_removed) {
+                // since the email was fully removed from conversation, report as trimmed
+                trimmed_conversations.set(conversation, email);
             }
         }
         
diff --git a/src/engine/app/app-conversation.vala b/src/engine/app/app-conversation.vala
index b3d5aee..7dc16a9 100644
--- a/src/engine/app/app-conversation.vala
+++ b/src/engine/app/app-conversation.vala
@@ -107,27 +107,6 @@ public class Geary.App.Conversation : BaseObject {
     }
     
     /**
-     * Returns the number of emails in the conversation in a particular folder.
-     *
-     * TODO: Remove?
-     */
-    public async int get_count_in_folder_async(Geary.Account account, Geary.FolderPath path,
-        Cancellable? cancellable) throws Error {
-        Gee.MultiMap<Geary.EmailIdentifier, Geary.FolderPath>? folder_map
-            = yield account.get_containing_folders_async(emails.keys, cancellable);
-        
-        int count = 0;
-        if (folder_map != null) {
-            foreach (Geary.EmailIdentifier id in folder_map.get_keys()) {
-                if (path in folder_map.get(id))
-                    ++count;
-            }
-        }
-        
-        return count;
-    }
-    
-    /**
      * Returns the number of { link Email}s in the conversation in the specified { link FolderPath}.
      */
     public bool any_in_folder_path(Geary.FolderPath path) {
diff --git a/src/engine/app/conversation-monitor/app-conversation-set.vala 
b/src/engine/app/conversation-monitor/app-conversation-set.vala
index 87a8212..5758b0f 100644
--- a/src/engine/app/conversation-monitor/app-conversation-set.vala
+++ b/src/engine/app/conversation-monitor/app-conversation-set.vala
@@ -355,6 +355,7 @@ private class Geary.App.ConversationSet : BaseObject {
      */
     public async bool check_conversation_in_folder_async(Conversation conversation, Geary.Account account,
         Geary.FolderPath required_folder_path, Cancellable? cancellable) throws Error {
+        /*
         if ((yield conversation.get_count_in_folder_async(account, required_folder_path, cancellable)) == 0) 
{
             debug("Evaporating conversation %s because it has no emails in %s",
                 conversation.to_string(), required_folder_path.to_string());
@@ -362,6 +363,7 @@ private class Geary.App.ConversationSet : BaseObject {
             
             return false;
         }
+        */
         
         return true;
     }
diff --git a/src/engine/app/conversation-monitor/app-external-remove-operation.vala 
b/src/engine/app/conversation-monitor/app-external-remove-operation.vala
new file mode 100644
index 0000000..9002715
--- /dev/null
+++ b/src/engine/app/conversation-monitor/app-external-remove-operation.vala
@@ -0,0 +1,22 @@
+/* Copyright 2013-2015 Yorba Foundation
+ *
+ * This software is licensed under the GNU Lesser General Public License
+ * (version 2.1 or later).  See the COPYING file in this distribution.
+ */
+
+private class Geary.App.ExternalRemoveOperation : ConversationOperation {
+    private Geary.FolderPath path;
+    private Gee.Collection<Geary.EmailIdentifier> removed_ids;
+    
+    public ExternalRemoveOperation(ConversationMonitor monitor, Geary.Folder folder,
+        Gee.Collection<Geary.EmailIdentifier> removed_ids) {
+        base(monitor);
+        
+        path = folder.path;
+        this.removed_ids = removed_ids;
+    }
+    
+    public override async void execute_async() {
+        yield monitor.remove_emails_async(path, removed_ids);
+    }
+}
diff --git a/src/engine/app/conversation-monitor/app-remove-operation.vala 
b/src/engine/app/conversation-monitor/app-remove-operation.vala
index 6bb633a..217c620 100644
--- a/src/engine/app/conversation-monitor/app-remove-operation.vala
+++ b/src/engine/app/conversation-monitor/app-remove-operation.vala
@@ -13,6 +13,6 @@ private class Geary.App.RemoveOperation : ConversationOperation {
     }
     
     public override async void execute_async() {
-        yield monitor.remove_emails_async(removed_ids);
+        yield monitor.remove_emails_async(monitor.folder.path, removed_ids);
     }
 }
diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala
index 05ab9aa..e638c06 100644
--- a/src/engine/imap-db/imap-db-account.vala
+++ b/src/engine/imap-db/imap-db-account.vala
@@ -699,7 +699,7 @@ private class Geary.ImapDB.Account : BaseObject {
                     continue;
                 
                 Gee.HashSet<ImapDB.EmailIdentifier>? associated_ids =
-                    Conversation.do_fetch_associated_email_ids(cx, db_id.message_id, cancellable);
+                    Conversation.do_fetch_associated_email_ids(cx, db_id, cancellable);
                 if (associated_ids == null || associated_ids.size == 0)
                     continue;
                 
@@ -709,8 +709,16 @@ private class Geary.ImapDB.Account : BaseObject {
                 foreach (ImapDB.EmailIdentifier associated_id in associated_ids) {
                     Email? email;
                     Gee.Collection<FolderPath?>? known_paths;
-                    do_fetch_message(cx, associated_id.message_id, required_fields, search_predicate,
-                        out email, out known_paths, cancellable);
+                    try {
+                        do_fetch_message(cx, associated_id.message_id, required_fields, search_predicate,
+                            out email, out known_paths, cancellable);
+                    } catch (Error err) {
+                        if (err is EngineError.NOT_FOUND)
+                            continue;
+                        
+                        throw err;
+                    }
+                    
                     if (email != null) {
                         association.add(email.id, known_paths);
                         found_ids.add((ImapDB.EmailIdentifier) email.id);
diff --git a/src/engine/imap-db/imap-db-conversation.vala b/src/engine/imap-db/imap-db-conversation.vala
index 35319d0..fd1a802 100644
--- a/src/engine/imap-db/imap-db-conversation.vala
+++ b/src/engine/imap-db/imap-db-conversation.vala
@@ -235,17 +235,22 @@ private int64 do_merge_conversations(Db.Connection cx, Gee.Set<int64?> conversat
 }
 
 private Gee.HashSet<ImapDB.EmailIdentifier>? do_fetch_associated_email_ids(Db.Connection cx,
-    int64 message_id, Cancellable? cancellable) throws Error {
+    ImapDB.EmailIdentifier id, Cancellable? cancellable) throws Error {
+    // In case not indexed in conversation table, always mark this message as a member of the
+    // conversation, even if it's a singleton
+    Gee.HashSet<ImapDB.EmailIdentifier> associated_message_ids = new Gee.HashSet<ImapDB.EmailIdentifier>();
+    associated_message_ids.add(id);
+    
     Db.Statement stmt = cx.prepare("""
         SELECT conversation_id
         FROM MessageConversationTable
         WHERE message_id = ?
     """);
-    stmt.bind_rowid(0, message_id);
+    stmt.bind_rowid(0, id.message_id);
     
     Db.Result result = stmt.exec(cancellable);
     if (result.finished || result.is_null_at(0))
-        return null;
+        return associated_message_ids;
     
     int64 conversation_id = result.rowid_at(0);
     
@@ -256,13 +261,12 @@ private Gee.HashSet<ImapDB.EmailIdentifier>? do_fetch_associated_email_ids(Db.Co
     """);
     stmt.bind_rowid(0, conversation_id);
     
-    Gee.HashSet<ImapDB.EmailIdentifier> associated_message_ids = new Gee.HashSet<ImapDB.EmailIdentifier>();
     for (result = stmt.exec(cancellable); !result.finished; result.next(cancellable)) {
         if (!result.is_null_at(0))
             associated_message_ids.add(new ImapDB.EmailIdentifier(result.rowid_at(0), null));
     }
     
-    return associated_message_ids.size > 0 ? associated_message_ids : null;
+    return associated_message_ids;
 }
 
 }
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]