[geary/mjog/invert-folder-class-hierarchy: 357/362] Geary.App.ConversationMonitor: Add support for partial email loading




commit a878fbea1293903c85af9e92d8919d5f5053355f
Author: Michael Gratton <mike vee net>
Date:   Sun Feb 21 17:01:03 2021 +1100

    Geary.App.ConversationMonitor: Add support for partial email loading
    
    Support loading partial email by explicitly allowing partial loads,
    checking for partially loaded email and avoid loading them immediately,
    then loading when the email has been completely loaded.

 src/engine/app/app-conversation-monitor.vala       | 158 +++++++++++++++------
 .../app-external-append-operation.vala             |   4 +-
 .../app-fill-window-operation.vala                 |   2 +-
 test/engine/app/app-conversation-monitor-test.vala | 155 ++++++++++++++++++--
 4 files changed, 265 insertions(+), 54 deletions(-)
---
diff --git a/src/engine/app/app-conversation-monitor.vala b/src/engine/app/app-conversation-monitor.vala
index 15bdd0e1b..8c1751ecc 100644
--- a/src/engine/app/app-conversation-monitor.vala
+++ b/src/engine/app/app-conversation-monitor.vala
@@ -1,6 +1,6 @@
 /*
  * Copyright © 2016 Software Freedom Conservancy Inc.
- * Copyright © 2018, 2020 Michael Gratton <mike vee net>
+ * Copyright © 2018-2021 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.
@@ -156,6 +156,14 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
     // Determines if the base folder was actually opened or not
     private bool base_was_opened = false;
 
+    // Set of in-folder email that doesn't meet required_fields (yet)
+    private Gee.Set<EmailIdentifier> incomplete =
+        new Gee.HashSet<EmailIdentifier>();
+
+    // Set of out-of-folder email that doesn't meet required_fields (yet)
+    private Gee.Set<EmailIdentifier> incomplete_external =
+        new Gee.HashSet<EmailIdentifier>();
+
     private Geary.Email.Field required_fields;
     private ConversationOperationQueue queue;
     private GLib.Cancellable operation_cancellable = new GLib.Cancellable();
@@ -318,6 +326,7 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
         account.email_inserted_into_folder.connect(on_email_inserted);
         account.email_removed_from_folder.connect(on_email_removed);
         account.email_flags_changed_in_folder.connect(on_email_flags_changed);
+        account.email_complete.connect(on_email_complete);
 
         this.queue.operation_error.connect(on_operation_error);
         this.queue.add(new FillWindowOperation(this));
@@ -368,6 +377,7 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
             account.email_inserted_into_folder.disconnect(on_email_inserted);
             account.email_removed_from_folder.disconnect(on_email_removed);
             account.email_flags_changed_in_folder.disconnect(on_email_flags_changed);
+            account.email_complete.disconnect(on_email_complete);
 
             yield this.queue.stop_processing_async(cancellable);
 
@@ -471,26 +481,35 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
     internal async int load_by_id_async(EmailIdentifier? initial_id,
                                         int count,
                                         Folder.ListFlags flags = Folder.ListFlags.NONE)
-        throws Error {
+        throws GLib.Error {
         notify_scan_started();
 
         int load_count = 0;
         GLib.Error? scan_error = null;
         try {
-            Gee.Collection<Geary.Email> messages =
+            Gee.Collection<Geary.Email> emails =
                 yield this.base_folder.list_email_range_by_id(
-                    initial_id, count, required_fields, flags,
+                    initial_id,
+                    count,
+                    this.required_fields,
+                    flags | INCLUDING_PARTIAL,
                     this.operation_cancellable
                 );
 
-            if (!messages.is_empty) {
-                load_count = messages.size;
-
-                foreach (Email email in messages) {
+            var i = emails.iterator();
+            while (i.next()) {
+                var email = i.get();
+                if (this.required_fields in email.fields) {
                     this.window.add(email.id);
+                } else {
+                    this.incomplete.add(email.id);
+                    i.remove();
                 }
+            }
 
-                yield process_email_async(messages, ProcessJobContext());
+            if (!emails.is_empty) {
+                load_count = emails.size;
+                yield process_email_async(emails, ProcessJobContext());
             }
         } catch (GLib.Error err) {
             scan_error = err;
@@ -506,22 +525,34 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
     }
 
     /** Loads messages from the base folder into the window. */
-    internal async void load_by_sparse_id(Gee.Collection<EmailIdentifier> ids)
-        throws Error {
+        internal async void load_by_sparse_id(
+            Gee.Collection<EmailIdentifier> ids
+        ) throws GLib.Error {
         notify_scan_started();
 
         GLib.Error? scan_error = null;
         try {
-            Gee.Collection<Geary.Email> messages =
+            Gee.Collection<Geary.Email> emails =
                 yield this.base_folder.get_multiple_email_by_id(
-                    ids, required_fields, NONE, this.operation_cancellable
+                    ids,
+                    required_fields,
+                    INCLUDING_PARTIAL,
+                    this.operation_cancellable
                 );
 
-            if (!messages.is_empty) {
-                foreach (Email email in messages) {
+            var i = emails.iterator();
+            while (i.next()) {
+                var email = i.get();
+                if (this.required_fields in email.fields) {
                     this.window.add(email.id);
+                } else {
+                    this.incomplete.add(email.id);
+                    i.remove();
                 }
-                yield process_email_async(messages, ProcessJobContext());
+            }
+
+            if (!emails.is_empty) {
+                yield process_email_async(emails, ProcessJobContext());
             }
         } catch (GLib.Error err) {
             scan_error = err;
@@ -535,42 +566,56 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
     }
 
     /**
-     * Loads messages from outside the monitor's base folder.
+     * Loads email from outside the monitor's base folder.
      *
-     * Since this requires opening and closing the other folder, it is
-     * handled separately.
+     * These messages will only be added if their references included
+     * email already in the conversation set.
      */
-    internal async void external_load_by_sparse_id(Folder folder,
-                                                   Gee.Collection<EmailIdentifier> ids)
+    internal async void external_load_by_sparse_id(Gee.Collection<EmailIdentifier> ids)
         throws GLib.Error {
         // First just get the bare minimum we need to determine if we even
         // care about the messages.
-        Gee.Set<Geary.Email> emails = yield folder.get_multiple_email_by_id(
-            ids, REFERENCES, NONE, this.operation_cancellable
-        );
-        if (!emails.is_empty) {
-            var relevant_ids = new Gee.HashSet<Geary.EmailIdentifier>();
-            foreach (var email in emails) {
+
+        Gee.Set<Geary.Email> emails =
+            yield this.base_folder.account.get_multiple_email_by_id(
+                ids, REFERENCES, INCLUDING_PARTIAL, this.operation_cancellable
+            );
+
+        var relevant_ids = new Gee.HashSet<Geary.EmailIdentifier>();
+        foreach (var email in emails) {
+            if (Email.Field.REFERENCES in email.fields) {
                 Gee.Set<RFC822.MessageID>? ancestors = email.get_ancestors();
                 if (ancestors != null &&
-                    Geary.traverse<RFC822.MessageID>(ancestors).any(id => conversations.has_message_id(id))) 
{
+                    Geary.traverse<RFC822.MessageID>(ancestors).any(
+                        id => conversations.has_message_id(id))) {
                     relevant_ids.add(email.id);
                 }
+            } else {
+                this.incomplete_external.add(email.id);
             }
+        }
 
-            // List the relevant messages again with the full set of
-            // fields, to make sure when we load them from the
-            // database we have all the data we need.
-            if (!relevant_ids.is_empty) {
-                emails = yield folder.get_multiple_email_by_id(
-                    relevant_ids,
-                    required_fields,
-                    NONE,
-                    this.operation_cancellable
-                );
-            } else {
-                emails.clear();
+        // List the relevant messages again with the full set of
+        // fields, to make sure when we load them from the
+        // database we have all the data we need.
+        if (!relevant_ids.is_empty) {
+            emails = yield this.base_folder.account.get_multiple_email_by_id(
+                relevant_ids,
+                this.required_fields,
+                INCLUDING_PARTIAL,
+                this.operation_cancellable
+            );
+
+            var i = emails.iterator();
+            while (i.next()) {
+                var email = i.get();
+                if (!(this.required_fields in email.fields)) {
+                    this.incomplete_external.add(email.id);
+                    i.remove();
+                }
             }
+        } else {
+            emails.clear();
         }
 
         if (!emails.is_empty) {
@@ -768,6 +813,8 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
 
     private void on_email_removed(Gee.Collection<EmailIdentifier> removed,
                                   Folder folder) {
+        this.incomplete.remove_all(removed);
+        this.incomplete_external.remove_all(removed);
         this.queue.add(new RemoveOperation(this, this.base_folder, removed));
     }
 
@@ -838,6 +885,37 @@ public class Geary.App.ConversationMonitor : BaseObject, Logging.Source {
         );
     }
 
+    private void on_email_complete(Gee.Collection<EmailIdentifier> completed) {
+        var was_incomplete = new Gee.HashSet<EmailIdentifier>();
+        var was_incomplete_external = new Gee.HashSet<EmailIdentifier>();
+        foreach (var email in completed) {
+            if (this.incomplete.remove(email)) {
+                was_incomplete.add(email);
+            } else if (this.incomplete_external.remove(email)) {
+                was_incomplete_external.add(email);
+            }
+        }
+        if (!was_incomplete.is_empty) {
+            this.queue.add(
+                new AppendOperation(this, was_incomplete)
+            );
+        }
+        if (!was_incomplete_external.is_empty) {
+            this.queue.add(
+                new ExternalAppendOperation(
+                    this,
+                    // Using the base folder here is technically
+                    // incorrect, but if we got to this point the
+                    // external email has already been filtered by
+                    // folder, by ExternalAppendOperation, so we can
+                    // get away with it
+                    this.base_folder,
+                    was_incomplete_external
+                )
+            );
+        }
+    }
+
     private void on_operation_error(ConversationOperation op, Error err) {
         if (!(err is GLib.IOError.CANCELLED)) {
             warning("Error executing %s: %s", op.get_type().name(), err.message);
diff --git a/src/engine/app/conversation-monitor/app-external-append-operation.vala 
b/src/engine/app/conversation-monitor/app-external-append-operation.vala
index df679d163..380a2b3d9 100644
--- a/src/engine/app/conversation-monitor/app-external-append-operation.vala
+++ b/src/engine/app/conversation-monitor/app-external-append-operation.vala
@@ -23,11 +23,11 @@ private class Geary.App.ExternalAppendOperation : BatchOperation<EmailIdentifier
         throws GLib.Error {
         if (!this.monitor.get_search_folder_blacklist().contains(folder.path) &&
             !this.monitor.conversations.is_empty) {
-            debug("Appending %d out of folder message(s) to %s",
+            debug("Adding %d out-of-folder email to conversations in %s",
                   batch.size,
                   this.folder.to_string());
 
-            yield this.monitor.external_load_by_sparse_id(this.folder, batch);
+            yield this.monitor.external_load_by_sparse_id(batch);
         }
     }
 
diff --git a/src/engine/app/conversation-monitor/app-fill-window-operation.vala 
b/src/engine/app/conversation-monitor/app-fill-window-operation.vala
index f8fe08794..187aa77da 100644
--- a/src/engine/app/conversation-monitor/app-fill-window-operation.vala
+++ b/src/engine/app/conversation-monitor/app-fill-window-operation.vala
@@ -41,7 +41,7 @@ private class Geary.App.FillWindowOperation : ConversationOperation {
 
         try {
             loaded = yield this.monitor.load_by_id_async(
-                this.monitor.window_lowest, num_to_load, NONE
+                this.monitor.window_lowest, num_to_load
             );
         } catch (EngineError.NOT_FOUND err) {
             debug("Stale FillWindowOperation: %s", err.message);
diff --git a/test/engine/app/app-conversation-monitor-test.vala 
b/test/engine/app/app-conversation-monitor-test.vala
index b75f88ba1..5f58077e3 100644
--- a/test/engine/app/app-conversation-monitor-test.vala
+++ b/test/engine/app/app-conversation-monitor-test.vala
@@ -37,6 +37,8 @@ class Geary.App.ConversationMonitorTest : TestCase {
         add_test("base_folder_message_removed", base_folder_message_removed);
         add_test("external_folder_message_appended", external_folder_message_appended);
         add_test("conversation_marked_as_deleted", conversation_marked_as_deleted);
+        add_test("incomplete_base_folder", incomplete_base_folder);
+        add_test("incomplete_external_folder", incomplete_external_folder);
     }
 
     public override void set_up() {
@@ -361,23 +363,23 @@ class Geary.App.ConversationMonitorTest : TestCase {
         ConversationMonitor monitor = setup_monitor({e1}, paths);
         assert_equal<int?>(monitor.size, 1, "Initial conversation count");
 
-        this.other_folder.expect_call(
+        // ExternalAppendOperation's blacklist check
+        this.account.expect_call("get_special_folder");
+        this.account.expect_call("get_special_folder");
+        this.account.expect_call("get_special_folder");
+
+        this.account.expect_call(
             "get_multiple_email_by_id"
         ).returns_object(
             Collection.single_set(e3)
         );
 
-        this.other_folder.expect_call(
+        this.account.expect_call(
             "get_multiple_email_by_id"
         ).returns_object(
             Collection.single_set(e3)
         );
 
-        // ExternalAppendOperation's blacklist check
-        this.account.expect_call("get_special_folder");
-        this.account.expect_call("get_special_folder");
-        this.account.expect_call("get_special_folder");
-
         /////////////////////////////////////////////////////////
         // First call to expand_conversations_async for e3's refs
 
@@ -414,7 +416,6 @@ class Geary.App.ConversationMonitorTest : TestCase {
 
         wait_for_signal(monitor, "conversations-added");
         this.base_folder.assert_expectations();
-        this.other_folder.assert_expectations();
         this.account.assert_expectations();
 
         assert_equal<int?>(monitor.size, 1, "Conversation count");
@@ -456,22 +457,154 @@ class Geary.App.ConversationMonitorTest : TestCase {
         );
     }
 
+    public void incomplete_base_folder() throws Error {
+        var incomplete = new Email(new Mock.EmailIdentifer(1));
+        var complete = setup_email(1);
+
+        var paths = new Gee.HashMultiMap<EmailIdentifier,Folder.Path>();
+        paths.set(incomplete.id, this.base_folder.path);
+
+        var monitor = new ConversationMonitor(this.base_folder, NONE, 10);
+
+        this.base_folder.expect_call("start_monitoring");
+        ValaUnit.ExpectedCall incomplete_list_call = this.base_folder
+            .expect_call("list_email_range_by_id")
+            .returns_object(Collection.single(incomplete));
+
+        monitor.start_monitoring.begin(null, this.async_completion);
+        monitor.start_monitoring.end(async_result());
+        wait_for_call(incomplete_list_call);
+        assert_equal<int?>(monitor.size, 0, "incomplete count");
+
+        // Process all of the async tasks arising from the open
+        while (this.main_loop.pending()) {
+            this.main_loop.iteration(true);
+        }
+
+        this.base_folder
+            .expect_call("get_multiple_email_by_id")
+            .returns_object(Collection.single(complete));
+        this.account.expect_call("get_special_folder");
+        this.account.expect_call("get_special_folder");
+        this.account.expect_call("get_special_folder");
+        this.account.expect_call("local_search_message_id_async");
+        this.account.expect_call("get_containing_folders_async")
+            .returns_object(paths);
+
+        this.account.email_complete(Collection.single(complete.id));
+
+        wait_for_signal(monitor, "conversations-added");
+        assert_equal<int?>(monitor.size, 1, "complete count");
+
+        this.base_folder.assert_expectations();
+        this.account.assert_expectations();
+    }
+
+    public void incomplete_external_folder() throws Error {
+        var in_folder = setup_email(1);
+        var incomplete_external = new Email(new Mock.EmailIdentifer(2));
+        var complete_external = setup_email(2, in_folder);
+
+        var in_folder_paths = new Gee.HashMultiMap<EmailIdentifier,Folder.Path>();
+        in_folder_paths.set(in_folder.id, this.base_folder.path);
+
+        var external_paths = new Gee.HashMultiMap<EmailIdentifier,Folder.Path>();
+        external_paths.set(complete_external.id, this.other_folder.path);
+
+        var related_paths = new Gee.HashMultiMap<Email,Folder.Path>();
+        related_paths.set(in_folder, this.base_folder.path);
+        related_paths.set(complete_external, this.other_folder.path);
+
+        var monitor = setup_monitor({in_folder}, in_folder_paths);
+
+        // initial call with incomplete email
+
+        this.account.expect_call("get_special_folder");
+        this.account.expect_call("get_special_folder");
+        this.account.expect_call("get_special_folder");
+
+        var initial_incomplete_load = this.account
+            .expect_call("get_multiple_email_by_id")
+            .returns_object(Collection.single(incomplete_external));
+
+        // Should not get added, since it's incomplete
+        this.account.email_appended_to_folder(
+            Collection.single(incomplete_external.id),
+            this.other_folder
+        );
+
+        wait_for_call(initial_incomplete_load);
+        while (this.main_loop.pending()) {
+            this.main_loop.iteration(true);
+        }
+
+        assert_equal<int?>(monitor.size, 1, "incomplete count");
+        var c1 = Collection.first(monitor.read_only_view);
+        assert_equal<int?>(c1.get_count(), 1, "incomplete conversation count");
+
+        // email completed
+
+        this.account.expect_call("get_special_folder");
+        this.account.expect_call("get_special_folder");
+        this.account.expect_call("get_special_folder");
+
+        this.account
+            .expect_call("get_multiple_email_by_id")
+            .returns_object(Collection.single(complete_external));
+
+        this.account
+            .expect_call("get_multiple_email_by_id")
+            .returns_object(Collection.single(complete_external));
+
+        this.account.expect_call("get_special_folder");
+        this.account.expect_call("get_special_folder");
+        this.account.expect_call("get_special_folder");
+
+        this.account.expect_call("local_search_message_id_async")
+            .returns_object(related_paths);
+        this.account.expect_call("local_search_message_id_async");
+
+        this.account.expect_call("get_special_folder");
+        this.account.expect_call("get_special_folder");
+        this.account.expect_call("get_special_folder");
+
+        this.account.expect_call("local_search_message_id_async");
+        this.account.expect_call("get_containing_folders_async")
+            .returns_object(external_paths);
+
+        this.account.email_complete(Collection.single(complete_external.id));
+
+        wait_for_signal(monitor, "conversation-appended");
+        while (this.main_loop.pending()) {
+            this.main_loop.iteration(true);
+        }
+
+        assert_equal<int?>(monitor.size, 1, "incomplete count");
+        var c2 = Collection.first(monitor.read_only_view);
+        assert_equal<int?>(c2.get_count(), 2, "incomplete conversation count");
+        assert_equal(c2.get_email_by_id(complete_external.id), complete_external,
+                     "completed email not present in conversation");
+    }
+
     private Email setup_email(int id, Email? references = null) {
         Email email = new Email(new Mock.EmailIdentifer(id));
+
         DateTime now = new DateTime.now_local();
+        email.set_send_date(new RFC822.Date(now));
+        email.set_email_properties(new Mock.EmailProperties(now));
+
         Geary.RFC822.MessageID mid = new Geary.RFC822.MessageID(
             "test%d@localhost".printf(id)
         );
-
         Geary.RFC822.MessageIDList refs_list = null;
         if (references != null) {
             refs_list = new Geary.RFC822.MessageIDList.single(
                 references.message_id
             );
         }
-        email.set_send_date(new RFC822.Date(now));
-        email.set_email_properties(new Mock.EmailProperties(now));
         email.set_full_references(mid, null, refs_list);
+
+        email.set_flags(new EmailFlags());
         return email;
     }
 


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