[geary] De-duplicate email removal code in ConversationSet.

[Michael Gratton <mjog src gnome org>]

commit 013a2f97228fd8d3ad154659c2ebe26027bd9772
Author: Michael James Gratton <mike vee net>
Date:   Thu Dec 7 15:02:58 2017 +1100

    De-duplicate email removal code in ConversationSet.
    
    * src/engine/app/conversation-monitor/app-conversation-set.vala
      (ConversationSet.remove_emails_and_check_in_folder_async): Removed in
      favour of remove_all_emails_by_identifier. Move
      check_conversation(s)_in_folder_async to ConversationMonitor so
      all ConversationSet operations are synchronised and side-effect free.
    
    * src/engine/app/app-conversation-monitor.vala (ConversationMonitor):
      Merge check_conversation(s)_in_folder_async calls together and
      simplify. Call ConversationSet.remove_all_emails_by_identifier and
      manually check for and integrated evaporated conversations.

 src/engine/app/app-conversation-monitor.vala       |   64 +++++++++--
 .../conversation-monitor/app-conversation-set.vala |  114 ++------------------
 2 files changed, 64 insertions(+), 114 deletions(-)
---
diff --git a/src/engine/app/app-conversation-monitor.vala b/src/engine/app/app-conversation-monitor.vala
index 87ae4c7..47f525d 100644
--- a/src/engine/app/app-conversation-monitor.vala
+++ b/src/engine/app/app-conversation-monitor.vala
@@ -617,23 +617,39 @@ public class Geary.App.ConversationMonitor : BaseObject {
         yield load_by_sparse_id(appended_ids, Geary.Folder.ListFlags.NONE, null);
     }
 
-    internal async void remove_emails_async(Geary.Folder source_folder,
-                                            Gee.Collection<Geary.EmailIdentifier> removed_ids) {
-        debug("%d messages(s) removed from %s, trimming/removing conversations...", removed_ids.size,
-              source_folder.to_string());
-
-        Gee.Collection<Geary.App.Conversation> removed;
-        Gee.MultiMap<Geary.App.Conversation, Geary.Email> trimmed;
-        yield conversations.remove_emails_and_check_in_folder_async(
+    internal async void remove_emails_async(Folder source_folder,
+                                            Gee.Collection<EmailIdentifier> removed_ids) {
+        debug("%d messages(s) removed from %s, trimming/removing conversations...",
+            removed_ids.size, source_folder.to_string()
+        );
+
+        Gee.Collection<Conversation> removed;
+        Gee.MultiMap<Conversation, Email> trimmed;
+        conversations.remove_all_emails_by_identifier(
             source_folder.path,
             removed_ids,
-            this.folder.account,
-            this.folder.path,
             out removed,
-            out trimmed,
-            null
+            out trimmed
         );
 
+        // Check for conversations that have been evaporated as a
+        // result, update removed and trimmed collections to reflect
+        // any that evaporated
+        try {
+            Gee.Collection<Conversation> evaporated = yield check_conversations_in_base_folder(
+                trimmed.get_keys(), null
+            );
+            removed.add_all(evaporated);
+            foreach (Conversation target in evaporated) {
+                trimmed.remove_all(target);
+            }
+        } catch (Error err) {
+            debug("Error checking conversation for messages in %s: %s",
+                  this.folder.path.to_string(), err.message);
+        }
+
+        // Fire signals, clean up
+
         foreach (Conversation conversation in trimmed.get_keys())
             notify_conversation_trimmed(conversation, trimmed.get(conversation));
 
@@ -769,6 +785,32 @@ public class Geary.App.ConversationMonitor : BaseObject {
             operation_queue.add(new FillWindowOperation(this, is_insert));
     }
 
+    /**
+     * Check conversations to see if they still exist in the base folder.
+     *
+     * Returns the set of emails that were removed due to not being in
+     * the base folder.
+     */
+    private async Gee.Collection<Conversation>
+        check_conversations_in_base_folder(Gee.Collection<Conversation> conversations,
+                                           Cancellable? cancellable)
+        throws Error {
+        Gee.ArrayList<Conversation> evaporated = new Gee.ArrayList<Conversation>();
+        foreach (Geary.App.Conversation conversation in conversations) {
+            int count = yield conversation.get_count_in_folder_async(
+                this.folder.account, this.folder.path, cancellable
+            );
+            if (count == 0) {
+                debug("Evaporating conversation %s because it has no emails in %s",
+                      conversation.to_string(), this.folder.to_string());
+                this.conversations.remove_conversation(conversation);
+                evaporated.add(conversation);
+            }
+        }
+
+        return evaporated;
+    }
+
     private void on_folder_email_appended(Gee.Collection<EmailIdentifier> appended_ids) {
         operation_queue.add(new AppendOperation(this, appended_ids));
     }
diff --git a/src/engine/app/conversation-monitor/app-conversation-set.vala 
b/src/engine/app/conversation-monitor/app-conversation-set.vala
index ef06eb6..10a0269 100644
--- a/src/engine/app/conversation-monitor/app-conversation-set.vala
+++ b/src/engine/app/conversation-monitor/app-conversation-set.vala
@@ -55,6 +55,17 @@ private class Geary.App.ConversationSet : BaseObject {
         return email_id_map.get(id);
     }
 
+    /**
+     * Removes a conversation from the set.
+     */
+    public void remove_conversation(Conversation conversation) {
+        foreach (Geary.Email conversation_email in conversation.get_emails(Conversation.Ordering.NONE))
+            remove_email_from_conversation(conversation, conversation_email);
+
+        if (!_conversations.remove(conversation))
+            error("Conversation %s already removed from set", conversation.to_string());
+    }
+
     // Returns a Collection of zero or more Conversations that have Message-IDs associated with
     // the ancestors of the supplied Email ... if more than one, then add_email() should not be
     // called
@@ -274,17 +285,6 @@ private class Geary.App.ConversationSet : BaseObject {
     }
 
     /**
-     * Removes a conversation from the set.
-     */
-    private void remove_conversation(Conversation conversation) {
-        foreach (Geary.Email conversation_email in conversation.get_emails(Conversation.Ordering.NONE))
-            remove_email_from_conversation(conversation, conversation_email);
-
-        if (!_conversations.remove(conversation))
-            error("Conversation %s already removed from set", conversation.to_string());
-    }
-
-    /**
      * Unconditionally removes an email from a conversation.
      */
     private void remove_email_from_conversation(Conversation conversation, Geary.Email email) {
@@ -391,98 +391,6 @@ private class Geary.App.ConversationSet : BaseObject {
                 _trimmed.set(conversation, email);
             }
         }
-        
-        removed = _removed;
-        trimmed = _trimmed;
-    }
-    
-    /**
-     * Make sure that the conversation has some emails in the given folder, and
-     * remove the conversation if not.  Return true if there were emails in the
-     * folder, or false if the conversation was removed.
-     */
-    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());
-            remove_conversation(conversation);
-            
-            return false;
-        }
-        
-        return true;
-    }
-    
-    /**
-     * Check a set of emails using check_conversation_in_folder_async(), return
-     * the set of emails that were removed due to not being in the folder.
-     */
-    public async Gee.Collection<Conversation> check_conversations_in_folder_async(
-        Gee.Collection<Conversation> conversations, Geary.Account account,
-        Geary.FolderPath required_folder_path, Cancellable? cancellable) {
-        Gee.ArrayList<Conversation> evaporated = new Gee.ArrayList<Conversation>();
-        foreach (Geary.App.Conversation conversation in conversations) {
-            try {
-                if (!(yield check_conversation_in_folder_async(
-                    conversation, account, required_folder_path, cancellable))) {
-                    evaporated.add(conversation);
-                }
-            } catch (Error e) {
-                debug("Unable to check conversation %s for messages in %s: %s",
-                    conversation.to_string(), required_folder_path.to_string(), e.message);
-            }
-        }
-        
-        return evaporated;
-    }
-    
-    public async void remove_emails_and_check_in_folder_async(FolderPath source_path,
-                                                              Gee.Collection<Geary.EmailIdentifier> ids,
-                                                              Account account,
-                                                              FolderPath required_folder_path,
-                                                              out Gee.Collection<Conversation> removed,
-                                                              out Gee.MultiMap<Conversation, Geary.Email> 
trimmed,
-                                                              Cancellable? cancellable) {
-        Gee.Collection<Conversation> initial_removed =
-            new Gee.HashSet<Conversation>();
-        Gee.MultiMap<Conversation, Geary.Email> initial_trimmed =
-            new Gee.HashMultiMap<Conversation, Geary.Email>();
-
-        foreach (Geary.EmailIdentifier id in ids) {
-            Geary.Email email;
-            bool removed_conversation;
-            Conversation? conversation = remove_email_by_identifier(
-                source_path, id, out email, out removed_conversation);
-
-            if (conversation == null)
-                continue;
-
-            if (removed_conversation) {
-                if (initial_trimmed.contains(conversation))
-                    initial_trimmed.remove_all(conversation);
-                initial_removed.add(conversation);
-            } else {
-                initial_trimmed.set(conversation, email);
-            }
-        }
-
-        Gee.Collection<Conversation> evaporated = yield check_conversations_in_folder_async(
-            initial_trimmed.get_keys(), account, required_folder_path, cancellable);
-
-        Gee.HashSet<Conversation> _removed = new Gee.HashSet<Conversation>();
-        _removed.add_all(initial_removed);
-        _removed.add_all(evaporated);
-
-        Gee.HashMultiMap<Conversation, Geary.Email> _trimmed =
-            new Gee.HashMultiMap<Conversation, Geary.Email>();
-
-        foreach (Conversation conversation in initial_trimmed.get_keys()) {
-            if (!(conversation in _removed)) {
-                Geary.Collection.multi_map_set_all<Conversation, Geary.Email>(
-                    _trimmed, conversation, initial_trimmed.get(conversation));
-            }
-        }
 
         removed = _removed;
         trimmed = _trimmed;