[geary/wip/713150-conversations] Fix archive bug

[Jim Nelson <jnelson src gnome org>]

commit 14dd09595ef9e13c527b819e3dc8bb0f5dc5f5ec
Author: Jim Nelson <jim yorba org>
Date:   Fri Mar 6 15:35:29 2015 -0800

    Fix archive bug
    
    This also cleans up how paths are added/removed from conversations
    as well as how conversations were being (not) dropped internally

 src/engine/app/app-conversation-monitor.vala |   67 ++++++++++++++++----------
 src/engine/app/app-conversation.vala         |   58 ++++++----------------
 src/engine/imap-db/imap-db-account.vala      |   11 ++--
 3 files changed, 64 insertions(+), 72 deletions(-)
---
diff --git a/src/engine/app/app-conversation-monitor.vala b/src/engine/app/app-conversation-monitor.vala
index 463ad5f..8308e9d 100644
--- a/src/engine/app/app-conversation-monitor.vala
+++ b/src/engine/app/app-conversation-monitor.vala
@@ -339,26 +339,14 @@ public class Geary.App.ConversationMonitor : BaseObject {
             operation_queue.add(new ReseedOperation(this, "already opened"));
         operation_queue.add(new FillWindowOperation(this, false));
         
-        folder.email_appended.connect(on_folder_email_appended);
-        folder.email_inserted.connect(on_folder_email_inserted);
-        folder.email_removed.connect(on_folder_email_removed);
-        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);
+        connect_to_folder();
         
         try {
             yield folder.open_async(open_flags, cancellable);
         } catch (Error err) {
             is_monitoring = false;
             
-            folder.email_appended.disconnect(on_folder_email_appended);
-            folder.email_inserted.disconnect(on_folder_email_inserted);
-            folder.email_removed.disconnect(on_folder_email_removed);
-            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);
+            disconnect_from_folder();
             
             throw err;
         }
@@ -397,13 +385,7 @@ public class Geary.App.ConversationMonitor : BaseObject {
         // set now to prevent reentrancy during yield or signal
         is_monitoring = false;
         
-        folder.email_appended.disconnect(on_folder_email_appended);
-        folder.email_inserted.disconnect(on_folder_email_inserted);
-        folder.email_removed.disconnect(on_folder_email_removed);
-        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);
+        disconnect_from_folder();
         
         bool closing = false;
         Error? close_err = null;
@@ -437,6 +419,32 @@ public class Geary.App.ConversationMonitor : BaseObject {
         primary_email_id_to_conversation.clear();
     }
     
+    private void connect_to_folder() {
+        folder.email_appended.connect(on_folder_email_appended);
+        folder.email_inserted.connect(on_folder_email_inserted);
+        folder.email_removed.connect(on_folder_email_removed);
+        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_added);
+        folder.account.email_appended.connect(on_account_email_added);
+        folder.account.email_discovered.connect(on_account_email_added);
+        folder.account.email_inserted.connect(on_account_email_added);
+        folder.account.email_removed.connect(on_account_email_removed);
+    }
+    
+    private void disconnect_from_folder() {
+        folder.email_appended.disconnect(on_folder_email_appended);
+        folder.email_inserted.disconnect(on_folder_email_inserted);
+        folder.email_removed.disconnect(on_folder_email_removed);
+        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_added);
+        folder.account.email_appended.disconnect(on_account_email_added);
+        folder.account.email_discovered.disconnect(on_account_email_added);
+        folder.account.email_inserted.disconnect(on_account_email_added);
+        folder.account.email_removed.disconnect(on_account_email_removed);
+    }
+    
     // By passing required_fields, this forces the email to be downloaded (if not already) at the
     // potential expense of loading it twice; use Email.Field.NONE to only load the email identifiers
     // and potentially not be able to load the email due to unavailability (but will be loaded
@@ -650,14 +658,17 @@ public class Geary.App.ConversationMonitor : BaseObject {
         operation_queue.add(new FillWindowOperation(this, false));
     }
     
-    private void on_account_email_locally_complete(Folder folder, Gee.Collection<EmailIdentifier> 
complete_ids) {
+    private void on_account_email_added(Folder folder, Gee.Collection<EmailIdentifier> added_ids) {
         if (!folder.path.equal_to(this.folder.path))
-            operation_queue.add(new ExternalAppendOperation(this, folder, complete_ids));
+            operation_queue.add(new ExternalAppendOperation(this, folder, added_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));
+        if (folder.path.equal_to(this.folder.path))
+            return;
+        
+        operation_queue.add(new ExternalRemoveOperation(this, folder, removed_ids));
+        operation_queue.add(new FillWindowOperation(this, false));
     }
     
     internal async void append_emails_async(Gee.Collection<Geary.EmailIdentifier> appended_ids) {
@@ -699,7 +710,13 @@ public class Geary.App.ConversationMonitor : BaseObject {
                 // could have been trimmed earlier in the loop
                 trimmed_conversations.remove_all(conversation);
                 
+                // strip Conversation from local storage and lookup tables
                 conversations.remove(conversation);
+                foreach (EmailIdentifier id in conversation.get_email_ids()) {
+                    primary_email_id_to_conversation.unset(id);
+                    all_email_id_to_conversation.unset(id);
+                }
+                
                 removed_conversations.add(conversation);
             } else if (fully_removed && email != null) {
                 // since the email was fully removed from conversation, report as trimmed
diff --git a/src/engine/app/app-conversation.vala b/src/engine/app/app-conversation.vala
index 5b7bf20..661a6e6 100644
--- a/src/engine/app/app-conversation.vala
+++ b/src/engine/app/app-conversation.vala
@@ -40,8 +40,6 @@ public class Geary.App.Conversation : BaseObject {
     
     private static int next_convnum = 0;
     
-    private Gee.HashMultiSet<RFC822.MessageID> message_ids = new Gee.HashMultiSet<RFC822.MessageID>();
-    
     private int convnum;
     private weak Geary.App.ConversationMonitor? owner;
     private Gee.HashMap<EmailIdentifier, Email> emails = new Gee.HashMap<EmailIdentifier, Email>();
@@ -81,8 +79,6 @@ public class Geary.App.Conversation : BaseObject {
         this.owner = owner;
         
         owner.email_flags_changed.connect(on_email_flags_changed);
-        owner.folder.account.email_discovered.connect(on_email_discovered);
-        owner.folder.account.email_removed.connect(on_email_removed);
     }
     
     ~Conversation() {
@@ -90,11 +86,10 @@ public class Geary.App.Conversation : BaseObject {
     }
     
     internal void clear_owner() {
-        if (owner != null) {
-            owner.email_flags_changed.disconnect(on_email_flags_changed);
-            owner.folder.account.email_discovered.disconnect(on_email_discovered);
-            owner.folder.account.email_removed.disconnect(on_email_removed);
-        }
+        if (owner == null)
+            return;
+        
+        owner.email_flags_changed.disconnect(on_email_flags_changed);
         
         owner = null;
     }
@@ -107,7 +102,8 @@ public class Geary.App.Conversation : BaseObject {
     }
     
     /**
-     * Returns the number of { link Email}s in the conversation in the specified { link FolderPath}.
+     * Returns true if any { link Email}s in the { link Conversation} are known to be in the
+     * specified { link FolderPath}.
      */
     public bool any_in_folder_path(Geary.FolderPath path) {
         foreach (EmailIdentifier email_id in path_map.get_keys()) {
@@ -187,16 +183,6 @@ public class Geary.App.Conversation : BaseObject {
     }
     
     /**
-     * Return all Message IDs associated with the conversation.
-     */
-    public Gee.Collection<RFC822.MessageID> get_message_ids() {
-        // Turn into a HashSet first, so we don't return duplicates.
-        Gee.HashSet<RFC822.MessageID> ids = new Gee.HashSet<RFC822.MessageID>();
-        ids.add_all(message_ids);
-        return ids;
-    }
-    
-    /**
      * Returns the email associated with the EmailIdentifier, if present in this conversation.
      */
     public Geary.Email? get_email_by_id(EmailIdentifier id) {
@@ -223,8 +209,13 @@ public class Geary.App.Conversation : BaseObject {
      *
      * known_paths should contain all the known FolderPaths this email is contained in.
      * Conversation will monitor Account for additions and removals as they occur.
+     *
+     * Paths are always added, whether or not the email was already present.
      */
     internal bool add(Email email, Gee.Collection<Geary.FolderPath> known_paths) {
+        foreach (Geary.FolderPath path in known_paths)
+            path_map.set(email.id, path);
+        
         if (emails.has_key(email.id))
             return false;
         
@@ -234,19 +225,16 @@ public class Geary.App.Conversation : BaseObject {
         recv_date_ascending.add(email);
         recv_date_descending.add(email);
         
-        Gee.Set<RFC822.MessageID>? ancestors = email.get_ancestors();
-        if (ancestors != null)
-            message_ids.add_all(ancestors);
-        
-        foreach (Geary.FolderPath path in known_paths)
-            path_map.set(email.id, path);
-        
         appended(email);
         
         return true;
     }
     
-    // Returns whether the email was completely removed from the conversation
+    /**
+     * Removes the paths from the email's known paths in the conversation.
+     *
+     * Returns true if the email is fully removed from the conversation.
+     */
     internal bool remove(Email email, Geary.FolderPath path) {
         path_map.remove(email.id, path);
         if (path_map.get(email.id).size != 0)
@@ -365,20 +353,6 @@ public class Geary.App.Conversation : BaseObject {
             email_flags_changed(email);
     }
     
-    private void on_email_discovered(Geary.Folder folder, Gee.Collection<Geary.EmailIdentifier> ids) {
-        // only add to the internal map if a part of this Conversation
-        foreach (Geary.EmailIdentifier id in ids) {
-            if (emails.has_key(id))
-                path_map.set(id, folder.path);
-        }
-    }
-    
-    private void on_email_removed(Geary.Folder folder, Gee.Collection<Geary.EmailIdentifier> ids) {
-        // To be forgiving, simply remove id without checking if it's a part of this Conversation
-        foreach (Geary.EmailIdentifier id in ids)
-            path_map.remove(id, folder.path);
-    }
-    
     public string to_string() {
         return "[#%d] (%d emails)".printf(convnum, emails.size);
     }
diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala
index ce61ad8..6fa7a4c 100644
--- a/src/engine/imap-db/imap-db-account.vala
+++ b/src/engine/imap-db/imap-db-account.vala
@@ -661,7 +661,7 @@ private class Geary.ImapDB.Account : BaseObject {
             while (!result.finished) {
                 Email? email;
                 Gee.Collection<FolderPath?>? known_paths;
-                do_fetch_message(cx, result.int64_at(0), requested_fields, search_predicate,
+                do_fetch_message(cx, result.int64_at(0), requested_fields, false, search_predicate,
                     out email, out known_paths, cancellable);
                 if (email != null) {
                     assert(known_paths != null);
@@ -710,8 +710,8 @@ private class Geary.ImapDB.Account : BaseObject {
                     Email? email;
                     Gee.Collection<FolderPath?>? known_paths;
                     try {
-                        do_fetch_message(cx, associated_id.message_id, required_fields, search_predicate,
-                            out email, out known_paths, cancellable);
+                        do_fetch_message(cx, associated_id.message_id, required_fields, false,
+                            search_predicate, out email, out known_paths, cancellable);
                     } catch (Error err) {
                         if (err is EngineError.NOT_FOUND)
                             continue;
@@ -736,7 +736,7 @@ private class Geary.ImapDB.Account : BaseObject {
     }
     
     private void do_fetch_message(Db.Connection cx, int64 message_id, Email.Field required_fields,
-        Geary.Account.EmailSearchPredicate? search_predicate, out Email? email,
+        bool include_removed, Geary.Account.EmailSearchPredicate? search_predicate, out Email? email,
         out Gee.Collection<FolderPath?>? known_paths, Cancellable? cancellable) throws Error {
         Email.Field actual_fields;
         MessageRow row = ImapDB.Folder.do_fetch_message_row(cx, message_id, required_fields,
@@ -745,7 +745,8 @@ private class Geary.ImapDB.Account : BaseObject {
         email = row.to_email(new Geary.ImapDB.EmailIdentifier(message_id, null));
         ImapDB.Folder.do_add_attachments(cx, email, message_id, cancellable);
         
-        Gee.Set<Geary.FolderPath>? folders = do_find_email_folders(cx, message_id, true, cancellable);
+        Gee.Set<Geary.FolderPath>? folders = do_find_email_folders(cx, message_id, include_removed,
+            cancellable);
         
         known_paths = new Gee.HashSet<FolderPath?>();
         if (folders == null)