Convert folder unseen update into an account op, only schedule if closed.

If the folder is open, we should be getting notifications of flag changes
from the server, so double-checking is kind of pointless, and since it's
implemented using an IMAP STATUS, also expensive for the server.

* src/engine/imap-engine/imap-engine-generic-account.vala
  (GenericAccount): Convert refresh_unseen_async method into a new
  RefreshFolderUnseen account operation, add a guard to ensure the folder
  is closed when executing. Remove hash map of timeouts and replace with
  a single timeout manager on MinimalFolder itself.

* src/engine/imap-engine/imap-engine-minimal-folder.vala (MinimalFolder):
  Add timeout manager for unseen update, only start it running if closed,
  when triggered, schedule a RefreshFolderUnseen op on the account.
This commit is contained in:
Michael James Gratton 2017-11-24 15:39:14 +11:00
parent d4abaaafbc
commit d9e23d553e
2 changed files with 142 additions and 101 deletions

View file

@ -9,7 +9,6 @@
private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
private const int REFRESH_FOLDER_LIST_SEC = 2 * 60;
private const int REFRESH_UNSEEN_SEC = 1;
private const Geary.SpecialFolderType[] SUPPORTED_SPECIAL_FOLDERS = {
Geary.SpecialFolderType.DRAFTS,
Geary.SpecialFolderType.SENT,
@ -28,9 +27,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
private Gee.HashMap<FolderPath, MinimalFolder> folder_map = new Gee.HashMap<
FolderPath, MinimalFolder>();
private Gee.HashMap<FolderPath, Folder> local_only = new Gee.HashMap<FolderPath, Folder>();
private Gee.HashMap<FolderPath, uint> refresh_unseen_timeout_ids
= new Gee.HashMap<FolderPath, uint>();
private Gee.HashSet<Geary.Folder> in_refresh_unseen = new Gee.HashSet<Geary.Folder>();
private AccountProcessor? processor;
private AccountSynchronizer sync;
private TimeoutManager refresh_folder_timer;
@ -111,28 +107,28 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
}
}
}
protected override void notify_email_appended(Geary.Folder folder, Gee.Collection<Geary.EmailIdentifier> ids) {
base.notify_email_appended(folder, ids);
reschedule_unseen_update(folder);
schedule_unseen_update(folder);
}
protected override void notify_email_inserted(Geary.Folder folder, Gee.Collection<Geary.EmailIdentifier> ids) {
base.notify_email_inserted(folder, ids);
reschedule_unseen_update(folder);
schedule_unseen_update(folder);
}
protected override void notify_email_removed(Geary.Folder folder, Gee.Collection<Geary.EmailIdentifier> ids) {
base.notify_email_removed(folder, ids);
reschedule_unseen_update(folder);
schedule_unseen_update(folder);
}
protected override void notify_email_flags_changed(Geary.Folder folder,
Gee.Map<Geary.EmailIdentifier, Geary.EmailFlags> flag_map) {
base.notify_email_flags_changed(folder, flag_map);
reschedule_unseen_update(folder);
schedule_unseen_update(folder);
}
private void check_open() throws EngineError {
if (!open)
throw new EngineError.OPEN_REQUIRED("Account %s not opened", to_string());
@ -348,93 +344,10 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
return all_folders;
}
// Subclasses should implement this to return their flavor of a MinimalFolder with the
// appropriate interfaces attached. The returned folder should have its SpecialFolderType
// set using either the properties from the local folder or its path.
//
// This won't be called to build the Outbox or search folder, but for all others (including Inbox) it will.
protected abstract MinimalFolder new_folder(ImapDB.Folder local_folder);
/**
* Hooks up and queues an {@link UpdateRemoteFolders} operation.
*/
private void update_remote_folders() {
UpdateRemoteFolders op = new UpdateRemoteFolders(
this,
this.remote,
this.local,
this.local_only.keys,
get_supported_special_folders()
);
op.completed.connect(() => {
this.refresh_folder_timer.start();
});
try {
queue_operation(op);
} catch (Error err) {
// oh well
}
}
private void reschedule_unseen_update(Geary.Folder folder) {
if (!folder_map.has_key(folder.path))
return;
if (refresh_unseen_timeout_ids.get(folder.path) != 0)
Source.remove(refresh_unseen_timeout_ids.get(folder.path));
refresh_unseen_timeout_ids.set(folder.path,
Timeout.add_seconds(REFRESH_UNSEEN_SEC, () => on_refresh_unseen(folder)));
}
private bool on_refresh_unseen(Geary.Folder folder) {
// If we're in the process already, reschedule for later.
if (in_refresh_unseen.contains(folder))
return true;
// add here, remove in completed callback
in_refresh_unseen.add(folder);
refresh_unseen_async.begin(folder, null, on_refresh_unseen_completed);
refresh_unseen_timeout_ids.unset(folder.path);
return false;
}
private void on_refresh_unseen_completed(Object? source, AsyncResult result) {
try {
refresh_unseen_async.end(result);
} catch (Error e) {
debug("Error refreshing unseen counts: %s", e.message);
}
}
private async void refresh_unseen_async(Geary.Folder folder, Cancellable? cancellable)
throws Error {
debug("Refreshing unseen counts for %s", folder.to_string());
try {
Imap.Folder remote_folder = yield remote.fetch_folder_cached_async(
folder.path,
true,
cancellable
);
yield local.update_folder_status_async(remote_folder, false, true, cancellable);
} finally {
// added when call scheduled (above)
in_refresh_unseen.remove(folder);
}
}
public override Geary.ContactStore get_contact_store() {
return local.contact_store;
}
protected virtual Geary.SpecialFolderType[] get_supported_special_folders() {
return SUPPORTED_SPECIAL_FOLDERS;
}
public override async bool folder_exists_async(Geary.FolderPath path,
Cancellable? cancellable = null) throws Error {
check_open();
@ -703,6 +616,48 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
return minimal_folder;
}
// Subclasses should implement this to return their flavor of a MinimalFolder with the
// appropriate interfaces attached. The returned folder should have its SpecialFolderType
// set using either the properties from the local folder or its path.
//
// This won't be called to build the Outbox or search folder, but for all others (including Inbox) it will.
protected abstract MinimalFolder new_folder(ImapDB.Folder local_folder);
/**
* Hooks up and queues an {@link UpdateRemoteFolders} operation.
*/
private void update_remote_folders() {
UpdateRemoteFolders op = new UpdateRemoteFolders(
this,
this.remote,
this.local,
this.local_only.keys,
get_supported_special_folders()
);
op.completed.connect(() => {
this.refresh_folder_timer.start();
});
try {
queue_operation(op);
} catch (Error err) {
// oh well
}
}
/**
* Hooks up and queues an {@link RefreshFolderUnseen} operation.
*/
private void schedule_unseen_update(Geary.Folder folder) {
MinimalFolder? impl = folder as MinimalFolder;
if (impl != null) {
impl.refresh_unseen();
}
}
protected virtual Geary.SpecialFolderType[] get_supported_special_folders() {
return SUPPORTED_SPECIAL_FOLDERS;
}
private void compile_special_search_names() {
/*
* Compiles the list of names used to search for special
@ -1078,3 +1033,52 @@ internal class Geary.ImapEngine.UpdateRemoteFolders : AccountOperation {
}
}
/**
* Account operation that updates a folder's unseen message count.
*
* This performs a IMAP STATUS on the folder, but only if it is not
* open - if it is open it is already maintaining its unseen count.
*/
internal class Geary.ImapEngine.RefreshFolderUnseen : AccountOperation {
private weak Geary.Folder folder;
private weak Imap.Account remote;
private weak ImapDB.Account local;
internal RefreshFolderUnseen(Geary.Folder folder,
Imap.Account remote,
ImapDB.Account local) {
this.folder = folder;
this.remote = remote;
this.local = local;
}
public override bool equal_to(AccountOperation op) {
return (
base.equal_to(op) &&
this.folder.path.equal_to(((RefreshFolderUnseen) op).folder.path)
);
}
public override string to_string() {
return "%s(%s)".printf(base.to_string(), folder.path.to_string());
}
public override async void execute(Cancellable cancellable) throws Error {
if (this.folder.get_open_state() == Geary.Folder.OpenState.CLOSED) {
Imap.Folder remote_folder = yield remote.fetch_folder_cached_async(
folder.path,
true,
cancellable
);
yield local.update_folder_status_async(
remote_folder, false, true, cancellable
);
}
}
}

View file

@ -26,7 +26,10 @@
private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport.Copy,
Geary.FolderSupport.Mark, Geary.FolderSupport.Move {
private const int FORCE_OPEN_REMOTE_TIMEOUT_SEC = 10;
private const int REFRESH_UNSEEN_TIMEOUT_SEC = 1;
public override Account account { get { return _account; } }
@ -69,6 +72,7 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
private int remote_count = -1;
private Nonblocking.Mutex open_mutex = new Nonblocking.Mutex();
private Nonblocking.Mutex close_mutex = new Nonblocking.Mutex();
private TimeoutManager refresh_unseen_timer;
private Cancellable? open_cancellable = null;
@ -117,6 +121,10 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
this.replay_queue = new ReplayQueue(this);
this.email_prefetcher = new EmailPrefetcher(this);
this.refresh_unseen_timer = new TimeoutManager.seconds(
REFRESH_UNSEEN_TIMEOUT_SEC, on_refresh_unseen
);
// Notify now to ensure that wait_for_close_async does not
// block if never opened.
this.closed_semaphore.blind_notify();
@ -573,15 +581,19 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
// first open gets to name the flags, but see note above
this.open_flags = open_flags;
// reset to force waiting in wait_for_open_async()
remote_semaphore.reset();
this.remote_semaphore.reset();
// reset to force waiting in wait_for_close_async()
closed_semaphore.reset();
this.closed_semaphore.reset();
// reset unseen count refresh since it will be updated when
// the remote opens
this.refresh_unseen_timer.reset();
this.open_cancellable = new Cancellable();
// Unless NO_DELAY is set, do NOT open the remote side here; wait for the ReplayQueue to
// require a remote connection or wait_for_open_async() to be called ... this allows for
// fast local-only operations to occur, local-only either because (a) the folder has all
@ -1576,6 +1588,31 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
remote_opened.to_string());
}
/**
* Schedules a refresh of the unseen count for the folder.
*
* This will only refresh folders that are not open, since if they
* are open or opening, they will already be updated. Hence it is safe to be called on closed folders.
*/
internal void refresh_unseen() {
if (this.open_count == 0) {
this.refresh_unseen_timer.start();
}
}
private void on_refresh_unseen() {
// We queue an account operation since the folder itself is
// closed and hence does not have a connection to use for it.
RefreshFolderUnseen op = new RefreshFolderUnseen(
this, this.remote, this.local
);
try {
this._account.queue_operation(op);
} catch (Error err) {
// oh well
}
}
private void on_remote_ready() {
start_open_remote();
}