[geary/wip/289-folder-not-fully-populated] Fix ConversationMonitor sometimes not loading more from remote

[Michael Gratton <mjog src gnome org>]

commit 3a44628da92d432c36f4a9dbeb392dfbf820d6b5
Author: Michael Gratton <mike vee net>
Date:   Thu Apr 4 16:57:53 2019 +1100

    Fix ConversationMonitor sometimes not loading more from remote
    
    If a FillWindowOperation didn't load a full amount of messages, it just
    assumed that there were no more to load. This is not true however when
    loading locally, the folder's vector isn't fully expanded, and it gets
    to the end of the vector.
    
    This patch fixes the operation to also queue another fill if the
    monitor's folder message window is smaller than the folder's total
    message count.
    
    Fixes #289

 src/engine/app/app-conversation-monitor.vala       | 29 ++++++++++++++++++----
 .../app-fill-window-operation.vala                 | 10 +++++---
 test/engine/api/geary-folder-mock.vala             |  2 +-
 test/engine/api/geary-folder-properties-mock.vala  | 24 ++++++++++++++++++
 test/engine/app/app-conversation-monitor-test.vala |  3 ---
 test/meson.build                                   |  1 +
 6 files changed, 56 insertions(+), 13 deletions(-)
---
diff --git a/src/engine/app/app-conversation-monitor.vala b/src/engine/app/app-conversation-monitor.vala
index 83a1a337..14d5ff55 100644
--- a/src/engine/app/app-conversation-monitor.vala
+++ b/src/engine/app/app-conversation-monitor.vala
@@ -85,6 +85,16 @@ public class Geary.App.ConversationMonitor : BaseObject {
     /** Determines if this monitor is monitoring the base folder. */
     public bool is_monitoring { get; private set; default = false; }
 
+    /** Determines if more conversations can be loaded. */
+    public bool can_load_more {
+        get {
+            return (
+                this.base_folder.properties.email_total >
+                this.folder_window_size
+            );
+        }
+    }
+
     /** Minimum number of emails large conversations should contain. */
     public int min_window_count {
         get { return _min_window_count; }
@@ -103,6 +113,13 @@ public class Geary.App.ConversationMonitor : BaseObject {
     /** The set of all conversations loaded by the monitor. */
     internal ConversationSet conversations { get; private set; }
 
+    /** The number of messages currently loaded from the base folder. */
+    internal uint folder_window_size {
+        get {
+            return (this.window.is_empty) ? 0 : this.window.size;
+        }
+    }
+
     /** The oldest message from the base folder in the loaded window. */
     internal EmailIdentifier? window_lowest {
         owned get {
@@ -116,10 +133,10 @@ public class Geary.App.ConversationMonitor : BaseObject {
     private Cancellable? operation_cancellable = null;
 
     // Set of known, in-folder emails, explicitly loaded for the
-    // monitor's window. This exists purely to support the
-    // window_lowest property above, but we need to maintain a sorted
-    // set of all known messages since if the last known email is
-    // removed, we won't know what the next lowest is. Only email
+    // monitor's window. This exists purely to support the window_size
+    // and window_lowest properties above, but we need to maintain a
+    // sorted set of all known messages since if the last known email
+    // is removed, we won't know what the next lowest is. Only email
     // listed by one of the load_by_*_id methods are added here. Other
     // in-folder messages pulled in for a conversation aren't added,
     // since they may not be within the load window.
@@ -345,7 +362,9 @@ public class Geary.App.ConversationMonitor : BaseObject {
 
     /** Ensures enough conversations are present, otherwise loads more. */
     internal void check_window_count() {
-        if (this.is_monitoring && this.conversations.size < this.min_window_count) {
+        if (this.is_monitoring &&
+            this.can_load_more &&
+            this.conversations.size < this.min_window_count) {
             this.queue.add(new FillWindowOperation(this));
         }
     }
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 5348984e..40231bc9 100644
--- a/src/engine/app/conversation-monitor/app-fill-window-operation.vala
+++ b/src/engine/app/conversation-monitor/app-fill-window-operation.vala
@@ -46,10 +46,12 @@ private class Geary.App.FillWindowOperation : ConversationOperation {
             this.monitor.window_lowest, num_to_load
         );
 
-        // Check to see if we need any more, but only if we actually
-        // loaded some, so we don't keep loop loading when we have
-        // already loaded all in the folder.
-        if (loaded == num_to_load) {
+        // Check to see if we need any more, but only if there might
+        // be some more to load either locally or from the remote. If
+        // we loaded the full amount, there might be some more
+        // locally, so try that. If not, but the monitor thinks there
+        // are more to load, then we have go check the remote.
+        if (loaded == num_to_load || this.monitor.can_load_more) {
             this.monitor.check_window_count();
         }
     }
diff --git a/test/engine/api/geary-folder-mock.vala b/test/engine/api/geary-folder-mock.vala
index 35f56020..e663341a 100644
--- a/test/engine/api/geary-folder-mock.vala
+++ b/test/engine/api/geary-folder-mock.vala
@@ -46,7 +46,7 @@ public class Geary.MockFolder : Folder, MockObject {
                       SpecialFolderType type,
                       ProgressMonitor? monitor) {
         this._account = account;
-        this._properties = properties;
+        this._properties = properties ?? new MockFolderPoperties();
         this._path = path;
         this._type = type;
         this._opening_monitor = monitor;
diff --git a/test/engine/api/geary-folder-properties-mock.vala 
b/test/engine/api/geary-folder-properties-mock.vala
new file mode 100644
index 00000000..5cefcada
--- /dev/null
+++ b/test/engine/api/geary-folder-properties-mock.vala
@@ -0,0 +1,24 @@
+/*
+ * Copyright 2019 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.
+ */
+
+public class Geary.MockFolderPoperties : FolderProperties {
+
+
+    public MockFolderPoperties() {
+        base(
+            0,
+            0,
+            Trillian.UNKNOWN,
+            Trillian.UNKNOWN,
+            Trillian.UNKNOWN,
+            false,
+            false,
+            false
+        );
+    }
+
+}
diff --git a/test/engine/app/app-conversation-monitor-test.vala 
b/test/engine/app/app-conversation-monitor-test.vala
index dfdcfe89..582aa0b7 100644
--- a/test/engine/app/app-conversation-monitor-test.vala
+++ b/test/engine/app/app-conversation-monitor-test.vala
@@ -235,9 +235,6 @@ class Geary.App.ConversationMonitorTest : TestCase {
         assert_int(2, monitor.size, "Initial conversation count");
         assert_equal(e2.id, monitor.window_lowest, "Lowest window id");
 
-        // Removing a message will trigger another async load
-        this.base_folder.expect_call("list_email_by_id_async");
-
         this.base_folder.email_removed(new Gee.ArrayList<EmailIdentifier>.wrap({e2.id}));
         wait_for_signal(monitor, "conversations-removed");
         assert_int(1, monitor.size, "Conversation count");
diff --git a/test/meson.build b/test/meson.build
index 4f7f7554..3414a95f 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -18,6 +18,7 @@ geary_test_engine_sources = [
   'engine/api/geary-email-identifier-mock.vala',
   'engine/api/geary-email-properties-mock.vala',
   'engine/api/geary-folder-mock.vala',
+  'engine/api/geary-folder-properties-mock.vala',
 
   'engine/api/geary-account-information-test.vala',
   'engine/api/geary-attachment-test.vala',