Commit dc656d7a authored by Michael Gratton's avatar Michael Gratton 🤞

Merge branch 'wip/289-folder-not-fully-populated' into 'mainline'

Fix ConversationMonitor sometimes not loading more from remote

See #289

See merge request !195
parents bb8123eb df80569b
Pipeline #73617 passed with stages
in 35 minutes and 2 seconds
......@@ -3025,11 +3025,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();
}
}
......
......@@ -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();
}
......
......@@ -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));
}
}
......
......@@ -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();
}
}
......
......@@ -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();
}
}
......@@ -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();
}
}
}
......@@ -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);
......
......@@ -1255,7 +1255,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",
......@@ -1391,7 +1391,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);
......
......@@ -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;
......
/*
* 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
);
}
}
......@@ -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");
......
......@@ -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',
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment