[geary/cherry-pick-dc656d7a] Merge branch 'wip/289-folder-not-fully-populated' into 'mainline'
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/cherry-pick-dc656d7a] Merge branch 'wip/289-folder-not-fully-populated' into 'mainline'
- Date: Sun, 7 Apr 2019 00:21:03 +0000 (UTC)
commit 1fa7eb81d3352c4ed16e777f77373e27a479f0e8
Author: Michael Gratton <mike vee net>
Date: Sun Apr 7 00:20:18 2019 +0000
Merge branch 'wip/289-folder-not-fully-populated' into 'mainline'
Fix ConversationMonitor sometimes not loading more from remote
See #289
See merge request GNOME/geary!195
(cherry picked from commit dc656d7aad33205336a0d2260f8d6527e93b12d8)
3a44628d Fix ConversationMonitor sometimes not loading more from remote
d6ca47ba Ensure ImapDb.Folder updates email total when marking removed
03dbe0e0 Don't unncessarily check for more conversations to load
6e5c51cd Improve how the client triggers conversation auto-loading
df80569b Remove some unused code
src/client/application/geary-controller.vala | 9 +++--
.../conversation-list/conversation-list-view.vala | 46 +++++++++-------------
src/engine/app/app-conversation-monitor.vala | 29 +++++++++++---
.../app-fill-window-operation.vala | 10 +++--
.../conversation-monitor/app-insert-operation.vala | 3 --
.../conversation-monitor/app-remove-operation.vala | 6 ---
src/engine/imap-db/imap-db-folder.vala | 29 ++++++++------
.../imap-engine/imap-engine-generic-account.vala | 4 +-
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 +
12 files changed, 100 insertions(+), 66 deletions(-)
---
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index bcc3ace7..48f3d07a 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -3069,11 +3069,14 @@ public class GearyController : Geary.BaseObject {
}
}
- private void on_scan_completed() {
+ private void on_scan_completed(Geary.App.ConversationMonitor monitor) {
// Done scanning. Check if we have enough messages to fill
// the conversation list; if not, trigger a load_more();
- if (!main_window.conversation_list_has_scrollbar()) {
- debug("Not enough messages, loading more for folder %s", current_folder.to_string());
+ if (!main_window.conversation_list_has_scrollbar() &&
+ monitor == this.current_conversations &&
+ monitor.can_load_more) {
+ debug("Not enough messages, loading more for folder %s",
+ current_folder.to_string());
on_load_more();
}
}
diff --git a/src/client/conversation-list/conversation-list-view.vala
b/src/client/conversation-list/conversation-list-view.vala
index 8cc8160d..73b74755 100644
--- a/src/client/conversation-list/conversation-list-view.vala
+++ b/src/client/conversation-list/conversation-list-view.vala
@@ -12,9 +12,6 @@ public class ConversationListView : Gtk.TreeView, Geary.BaseInterface {
private bool enable_load_more = true;
- // Used to avoid repeated calls to load_more(). Contains the last "upper" bound of the
- // scroll adjustment seen at the call to load_more().
- private double last_upper = -1.0;
private bool reset_adjustment = false;
private Geary.App.ConversationMonitor? conversation_monitor;
private Gee.Set<Geary.App.Conversation>? current_visible_conversations = null;
@@ -101,7 +98,6 @@ public class ConversationListView : Gtk.TreeView, Geary.BaseInterface {
old_store.rows_reordered.disconnect(on_rows_changed);
old_store.row_changed.disconnect(on_rows_changed);
old_store.row_deleted.disconnect(on_rows_changed);
- old_store.row_deleted.disconnect(on_row_deleted);
old_store.destroy();
}
@@ -110,7 +106,6 @@ public class ConversationListView : Gtk.TreeView, Geary.BaseInterface {
new_store.rows_reordered.connect(on_rows_changed);
new_store.row_changed.connect(on_rows_changed);
new_store.row_deleted.connect(on_rows_changed);
- new_store.row_deleted.connect(on_row_deleted);
new_store.conversations_removed.connect(on_conversations_removed);
new_store.conversations_added.connect(on_conversations_added);
}
@@ -140,6 +135,20 @@ public class ConversationListView : Gtk.TreeView, Geary.BaseInterface {
}
}
+ private void check_load_more() {
+ // Check if we're at the very bottom of the list. If we are,
+ // it's time to issue a load_more signal.
+ Gtk.Adjustment adjustment = ((Gtk.Scrollable) this).get_vadjustment();
+ double upper = adjustment.get_upper();
+ double threshold = upper - adjustment.page_size - LOAD_MORE_HEIGHT;
+ if (this.conversation_monitor.can_load_more &&
+ adjustment.get_value() >= threshold) {
+ load_more();
+ }
+
+ schedule_visible_conversations_changed();
+ }
+
private void on_conversation_monitor_changed() {
if (conversation_monitor != null) {
conversation_monitor.scan_started.disconnect(on_scan_started);
@@ -155,11 +164,12 @@ public class ConversationListView : Gtk.TreeView, Geary.BaseInterface {
}
private void on_scan_started() {
- enable_load_more = false;
+ this.enable_load_more = false;
}
private void on_scan_completed() {
- enable_load_more = true;
+ this.enable_load_more = true;
+ check_load_more();
// Select the first conversation, if autoselect is enabled,
// nothing has been selected yet and we're not composing.
@@ -323,20 +333,9 @@ public class ConversationListView : Gtk.TreeView, Geary.BaseInterface {
}
private void on_value_changed() {
- if (!enable_load_more)
- return;
-
- // Check if we're at the very bottom of the list. If we are, it's time to
- // issue a load_more signal.
- Gtk.Adjustment adjustment = ((Gtk.Scrollable) this).get_vadjustment();
- double upper = adjustment.get_upper();
- if (adjustment.get_value() >= upper - adjustment.page_size - LOAD_MORE_HEIGHT &&
- upper > last_upper) {
- load_more();
- last_upper = upper;
+ if (this.enable_load_more) {
+ check_load_more();
}
-
- schedule_visible_conversations_changed();
}
private static Gtk.TreeViewColumn create_column(ConversationListStore.Column column,
@@ -463,13 +462,6 @@ public class ConversationListView : Gtk.TreeView, Geary.BaseInterface {
}
}
- private void on_row_deleted(Gtk.TreePath path) {
- // if one or more rows are deleted in the model, reset the last upper limit so scrolling to
- // the bottom will always activate a reload (this is particularly important if the model
- // is cleared)
- last_upper = -1.0;
- }
-
private void on_rows_changed() {
schedule_visible_conversations_changed();
}
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/src/engine/app/conversation-monitor/app-insert-operation.vala
b/src/engine/app/conversation-monitor/app-insert-operation.vala
index f97f689b..14f63ebb 100644
--- a/src/engine/app/conversation-monitor/app-insert-operation.vala
+++ b/src/engine/app/conversation-monitor/app-insert-operation.vala
@@ -38,8 +38,5 @@ private class Geary.App.InsertOperation : ConversationOperation {
this.monitor.base_folder.to_string(),
this.inserted_ids.size);
yield this.monitor.load_by_sparse_id(to_insert);
-
- // Check to see if we need any more
- this.monitor.check_window_count();
}
}
diff --git a/src/engine/app/conversation-monitor/app-remove-operation.vala
b/src/engine/app/conversation-monitor/app-remove-operation.vala
index 6e31436a..33cbc6d8 100644
--- a/src/engine/app/conversation-monitor/app-remove-operation.vala
+++ b/src/engine/app/conversation-monitor/app-remove-operation.vala
@@ -40,12 +40,6 @@ private class Geary.App.RemoveOperation : ConversationOperation {
trimmed,
(this.source_folder == this.monitor.base_folder) ? this.removed_ids : null
);
-
- // Check we still have enough conversations if any were
- // removed
- if (!removed.is_empty) {
- this.monitor.check_window_count();
- }
}
}
diff --git a/src/engine/imap-db/imap-db-folder.vala b/src/engine/imap-db/imap-db-folder.vala
index 24b22827..f43519df 100644
--- a/src/engine/imap-db/imap-db-folder.vala
+++ b/src/engine/imap-db/imap-db-folder.vala
@@ -156,7 +156,6 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
* unless update_uid_info is true.
*/
public async void update_folder_status(Geary.Imap.FolderProperties remote_properties,
- bool update_uid_info,
bool respect_marked_for_remove,
Cancellable? cancellable)
throws Error {
@@ -199,9 +198,6 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
stmt.bind_rowid(2, this.folder_id);
stmt.exec(cancellable);
- if (update_uid_info)
- do_update_uid_info(cx, remote_properties, cancellable);
-
if (remote_properties.status_messages >= 0) {
do_update_last_seen_status_total(
cx, remote_properties.status_messages, cancellable
@@ -218,11 +214,6 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
this.properties.recent = remote_properties.recent;
this.properties.attrs = remote_properties.attrs;
- if (update_uid_info) {
- this.properties.uid_validity = remote_properties.uid_validity;
- this.properties.uid_next = remote_properties.uid_next;
- }
-
// only update STATUS MESSAGES count if previously set, but use this count as the
// "authoritative" value until another SELECT/EXAMINE or MESSAGES response
if (remote_properties.status_messages >= 0) {
@@ -1057,6 +1048,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
public async Gee.Set<ImapDB.EmailIdentifier>? mark_removed_async(
Gee.Collection<ImapDB.EmailIdentifier>? ids, bool mark_removed, Cancellable? cancellable)
throws Error {
+ int total_changed = 0;
int unread_count = 0;
Gee.Set<ImapDB.EmailIdentifier> removed_ids = new Gee.HashSet<ImapDB.EmailIdentifier>();
yield db.exec_transaction_async(Db.TransactionType.RW, (cx) => {
@@ -1069,6 +1061,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
if (locs == null || locs.size == 0)
return Db.TransactionOutcome.DONE;
+ total_changed = locs.size;
unread_count = do_get_unread_count_for_ids(cx, ids, cancellable);
Gee.HashSet<Imap.UID> uids = new Gee.HashSet<Imap.UID>();
@@ -1077,14 +1070,26 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
removed_ids.add(location.email_id);
}
- if (uids.size > 0)
- do_mark_unmark_removed(cx, uids, mark_removed, cancellable);
-
+ do_mark_unmark_removed(cx, uids, mark_removed, cancellable);
do_add_to_unread_count(cx, -unread_count, cancellable);
return Db.TransactionOutcome.DONE;
}, cancellable);
+
+ // Update the folder properties so client sees the changes
+ // right away
+
+ // Email total
+ if (mark_removed) {
+ total_changed = -total_changed;
+ }
+ int total = this.properties.select_examine_messages + total_changed;
+ if (total >= 0) {
+ this.properties.set_select_examine_message_count(total);
+ }
+
+ // Unread total
if (unread_count > 0)
properties.set_status_unseen(properties.email_unread - unread_count);
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala
b/src/engine/imap-engine/imap-engine-generic-account.vala
index cb7f54af..51cb23e5 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -1266,7 +1266,7 @@ internal class Geary.ImapEngine.UpdateRemoteFolders : AccountOperation {
// it's opened
try {
yield minimal_folder.local_folder.update_folder_status(
- remote_folder.properties, false, false, cancellable
+ remote_folder.properties, false, cancellable
);
} catch (Error update_error) {
debug("Unable to update local folder %s with remote properties: %s",
@@ -1402,7 +1402,7 @@ internal class Geary.ImapEngine.RefreshFolderUnseen : FolderOperation {
this.folder.to_string())) {
yield local_folder.update_folder_status(
- remote_folder.properties, false, true, cancellable
+ remote_folder.properties, true, cancellable
);
((GenericAccount) this.account).update_folder(this.folder);
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 ab3ea78c..8c4500ab 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',
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
