diff --git a/src/client/geary-application.vala b/src/client/geary-application.vala index 27575d3e..3a991d26 100644 --- a/src/client/geary-application.vala +++ b/src/client/geary-application.vala @@ -93,7 +93,7 @@ along with Geary; if not, write to the Free Software Foundation, Inc., { "log-conversations", 0, 0, OptionArg.NONE, ref log_conversations, N_("Log conversation monitoring"), null }, { "log-network", 0, 0, OptionArg.NONE, ref log_network, N_("Log network activity"), null }, { "log-replay-queue", 0, 0, OptionArg.NONE, ref log_replay_queue, N_("Log IMAP replay queue"), null }, - { "log-serializer", 0, 0, OptionArg.NONE, ref log_serializer, N_("Log netowkr serialization"), null }, + { "log-serializer", 0, 0, OptionArg.NONE, ref log_serializer, N_("Log network serialization"), null }, { "log-periodic", 0, 0, OptionArg.NONE, ref log_periodic, N_("Log periodic activity"), null }, { "log-sql", 0, 0, OptionArg.NONE, ref log_sql, N_("Log database queries (generates lots of messages)"), null }, { "log-folder-normalization", 0, 0, OptionArg.NONE, ref log_folder_normalization, N_("Log folder normalization"), null }, diff --git a/src/client/geary-controller.vala b/src/client/geary-controller.vala index 8ec0764e..b1f899e1 100644 --- a/src/client/geary-controller.vala +++ b/src/client/geary-controller.vala @@ -371,6 +371,9 @@ public class GearyController { yield current_folder.close_async(); } + if (current_folder != null) + current_folder.opened.disconnect(on_current_folder_opened); + if (folder != null) debug("switching to %s", folder.to_string()); @@ -386,6 +389,8 @@ public class GearyController { update_ui(); + current_folder.opened.connect(on_current_folder_opened); + current_conversations = new Geary.ConversationMonitor(current_folder, false, MessageListStore.REQUIRED_FIELDS); @@ -405,6 +410,18 @@ public class GearyController { current_conversations.lazy_load(-1, -1, Geary.Folder.ListFlags.LOCAL_ONLY, cancellable_folder); } + public void on_current_folder_opened(Geary.Folder.OpenState state, int count) { + // when BOTH (or only REMOTE) is opened and no conversations are available, seed the + // ConversationMonitor, as its possible to call Folder.list's variants while opening and + // only receive the local mail + if ((state == Geary.Folder.OpenState.BOTH || state == Geary.Folder.OpenState.REMOTE) + && (current_conversations.get_conversation_count() == 0)) { + debug("Reseed of ConversationMonitor from opened folder %s", current_folder.to_string()); + current_conversations.lazy_load(-1, FETCH_EMAIL_CHUNK_COUNT, Geary.Folder.ListFlags.NONE, + cancellable_folder); + } + } + public void on_scan_started() { main_window.message_list_view.enable_load_more = false; set_busy(true); diff --git a/src/engine/api/geary-conversation-monitor.vala b/src/engine/api/geary-conversation-monitor.vala index 4ace788a..0e82bf81 100644 --- a/src/engine/api/geary-conversation-monitor.vala +++ b/src/engine/api/geary-conversation-monitor.vala @@ -317,6 +317,10 @@ public class Geary.ConversationMonitor : Object { email_flags_changed(conversation, email); } + public int get_conversation_count() { + return conversations.size; + } + public Gee.Collection get_conversations() { return conversations.read_only_view; } diff --git a/src/engine/api/geary-folder.vala b/src/engine/api/geary-folder.vala index c291c303..a52d3008 100644 --- a/src/engine/api/geary-folder.vala +++ b/src/engine/api/geary-folder.vala @@ -284,12 +284,21 @@ public interface Geary.Folder : Object { * emails in a folder without determining the count first. * * If the caller would prefer the Folder return emails it has immediately available rather than - * make an expensive I/O call to "properly" fetch the emails, it should pass ListFlags.LOCAL_ONLY. + * make an expensive network call to "properly" fetch the emails, it should pass ListFlags.LOCAL_ONLY. * However, this also means avoiding a full synchronization, so it's possible the fetched * emails do not correspond to what's actually available on the server. The best use of this * method is to quickly retrieve a block of email for display or processing purposes, * immediately followed by a non-fast list operation and then merging the two results. * + * Likewise, if this is called while Folder is in an OPENING or LOCAL state (that is, the remote + * server is not yet available), only local mail will be returned. This is to avoid two poor + * situations: (a) waiting to connect to the server to ensure that positional addressing is + * correctly calculated (and potentially missing the opportunity to return available local data) + * and (b) fetching locally, waiting, then fetching remotely, which means the returned emails + * could potentially mix stale and fresh data. A ListFlag may be offered in the future to allow + * the caller to force the engine to wait for a server connection before continuing. See + * get_open_state() and "opened" for more information. + * * Note that LOCAL_ONLY only returns the emails with the required fields that are available in * the Folder's local store. It may have fewer or incomplete messages, meaning that this will * return an incomplete list. @@ -417,6 +426,11 @@ public interface Geary.Folder : Object { * ListFlags as a parameter. See list_email_async() for more information. Note that one * flag (ListFlags.EXCLUDING_ID) makes no sense in this context. * + * This method also works like the list variants in that it will not wait for the server to + * connect if called in the OPENING state. A ListFlag option may be offered in the future to + * force waiting for the server to connect. Unlike the list variants, if in the OPENING state + * and the message is not found locally, EngineError.NOT_FOUND is thrown. + * * The Folder must be opened prior to attempting this operation. */ public abstract async Geary.Email fetch_email_async(Geary.EmailIdentifier email_id, diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala index 462a9c20..d82602ae 100644 --- a/src/engine/imap-db/imap-db-account.vala +++ b/src/engine/imap-db/imap-db-account.vala @@ -94,7 +94,11 @@ private class Geary.ImapDB.Account : Object { // get the parent of this folder, creating parents if necessary ... ok if this fails, // that just means the folder has no parents int64 parent_id = Db.INVALID_ROWID; - do_fetch_parent_id(cx, path, true, out parent_id, cancellable); + if (!do_fetch_parent_id(cx, path, true, out parent_id, cancellable)) { + debug("Unable to find parent ID to %s clone folder", path.to_string()); + + return Db.TransactionOutcome.ROLLBACK; + } // create the folder object Db.Statement stmt = cx.prepare( @@ -103,8 +107,10 @@ private class Geary.ImapDB.Account : Object { stmt.bind_string(0, path.basename); stmt.bind_rowid(1, parent_id); stmt.bind_int(2, properties.messages); - stmt.bind_int64(3, (properties.uid_validity != null) ? properties.uid_validity.value : 0); - stmt.bind_int64(4, (properties.uid_next != null) ? properties.uid_next.value : 0); + stmt.bind_int64(3, (properties.uid_validity != null) ? properties.uid_validity.value + : Imap.UIDValidity.INVALID); + stmt.bind_int64(4, (properties.uid_next != null) ? properties.uid_next.value + : Imap.UID.INVALID); stmt.bind_string(5, properties.attrs.serialize()); stmt.exec(cancellable); @@ -126,15 +132,20 @@ private class Geary.ImapDB.Account : Object { yield db.exec_transaction_async(Db.TransactionType.RW, (cx) => { int64 parent_id; - if (!do_fetch_parent_id(cx, path, true, out parent_id, cancellable)) + if (!do_fetch_parent_id(cx, path, true, out parent_id, cancellable)) { + debug("Unable to find parent ID of %s to update properties", path.to_string()); + return Db.TransactionOutcome.ROLLBACK; + } Db.Statement stmt = cx.prepare( "UPDATE FolderTable SET last_seen_total=?, uid_validity=?, uid_next=?, attributes=? " + "WHERE parent_id=? AND name=?"); stmt.bind_int(0, properties.messages); - stmt.bind_int64(1, properties.uid_validity.value); - stmt.bind_int64(2, properties.uid_next.value); + stmt.bind_int64(1, (properties.uid_validity != null) ? properties.uid_validity.value + : Imap.UIDValidity.INVALID); + stmt.bind_int64(2, (properties.uid_next != null) ? properties.uid_next.value + : Imap.UID.INVALID); stmt.bind_string(3, properties.attrs.serialize()); stmt.bind_rowid(4, parent_id); stmt.bind_string(4, path.basename); @@ -163,10 +174,14 @@ private class Geary.ImapDB.Account : Object { yield db.exec_transaction_async(Db.TransactionType.RO, (cx) => { int64 parent_id = Db.INVALID_ROWID; if (parent != null) { - if (!do_fetch_folder_id(cx, parent, false, out parent_id, cancellable)) + if (!do_fetch_folder_id(cx, parent, false, out parent_id, cancellable)) { + debug("Unable to find folder ID for %s to list folders", parent.to_string()); + return Db.TransactionOutcome.ROLLBACK; + } - assert(parent_id != Db.INVALID_ROWID); + if (parent_id == Db.INVALID_ROWID) + throw new EngineError.NOT_FOUND("Folder %s not found", parent.to_string()); } Db.Statement stmt; @@ -230,8 +245,9 @@ private class Geary.ImapDB.Account : Object { yield db.exec_transaction_async(Db.TransactionType.RO, (cx) => { try { int64 folder_id; - if (do_fetch_folder_id(cx, path, false, out folder_id, cancellable)) - exists = (folder_id != Db.INVALID_ROWID); + do_fetch_folder_id(cx, path, false, out folder_id, cancellable); + + exists = (folder_id != Db.INVALID_ROWID); } catch (EngineError err) { // treat NOT_FOUND as non-exceptional situation if (!(err is EngineError.NOT_FOUND)) @@ -256,10 +272,14 @@ private class Geary.ImapDB.Account : Object { int64 folder_id = Db.INVALID_ROWID; Imap.FolderProperties? properties = null; yield db.exec_transaction_async(Db.TransactionType.RO, (cx) => { - if (!do_fetch_folder_id(cx, path, false, out folder_id, cancellable)) + if (!do_fetch_folder_id(cx, path, false, out folder_id, cancellable)) { + debug("Unable to find folder ID for %s to fetch", path.to_string()); + return Db.TransactionOutcome.DONE; + } - assert(folder_id != Db.INVALID_ROWID); + if (folder_id == Db.INVALID_ROWID) + return Db.TransactionOutcome.DONE; Db.Statement stmt = cx.prepare( "SELECT last_seen_total, uid_validity, uid_next, attributes FROM FolderTable WHERE id=?"); @@ -373,6 +393,9 @@ private class Geary.ImapDB.Account : Object { // Transaction helper methods // + // If the FolderPath has no parent, returns true and folder_id will be set to Db.INVALID_ROWID. + // If cannot create path or there is a logical problem traversing it, returns false with folder_id + // set to Db.INVALID_ROWID. private bool do_fetch_folder_id(Db.Connection cx, Geary.FolderPath path, bool create, out int64 folder_id, Cancellable? cancellable) throws Error { check_open(); @@ -404,6 +427,9 @@ private class Geary.ImapDB.Account : Object { if (!result.finished) { id = result.rowid_at(0); } else if (!create) { + debug("Unable to return folder ID for %s: not creating paths in table", + path.to_string()); + return false; } else { // not found, create it @@ -430,15 +456,16 @@ private class Geary.ImapDB.Account : Object { // parent_id is now the folder being searched for folder_id = parent_id; - return (folder_id != Db.INVALID_ROWID); + return true; } + // See do_fetch_folder_id() for return semantics. private bool do_fetch_parent_id(Db.Connection cx, Geary.FolderPath path, bool create, out int64 parent_id, Cancellable? cancellable = null) throws Error { if (path.is_root()) { parent_id = Db.INVALID_ROWID; - return false; + return true; } return do_fetch_folder_id(cx, path.get_parent(), create, out parent_id, cancellable); diff --git a/src/engine/imap-db/imap-db-folder.vala b/src/engine/imap-db/imap-db-folder.vala index ac78e668..3b111bf3 100644 --- a/src/engine/imap-db/imap-db-folder.vala +++ b/src/engine/imap-db/imap-db-folder.vala @@ -162,6 +162,26 @@ private class Geary.ImapDB.Folder : Object, Geary.ReferenceSemantics { return count; } + // Updates both the FolderProperties and the value in the local store. Must be called while + // open. + public async void update_remote_message_count(int count, Cancellable? cancellable) throws Error { + check_open(); + + yield db.exec_transaction_async(Db.TransactionType.RW, (cx) => { + Db.Statement stmt = cx.prepare( + "UPDATE FolderTable SET last_seen_total=? WHERE id=?"); + stmt.bind_int(0, Numeric.int_floor(count, 0)); + stmt.bind_rowid(1, folder_id); + + stmt.exec(cancellable); + + return Db.TransactionOutcome.COMMIT; + }, cancellable); + + if (properties != null) + properties.messages = count; + } + public async int get_id_position_async(Geary.EmailIdentifier id, ListFlags flags, Cancellable? cancellable) throws Error { check_open(); @@ -522,6 +542,8 @@ private class Geary.ImapDB.Folder : Object, Geary.ReferenceSemantics { // Mark messages as removed (but not expunged) from the folder. Marked messages are skipped // on most operations unless ListFlags.INCLUDE_MARKED_REMOVED is true. Use remove_marked_email_async() // to formally remove the messages from the folder. + // + // TODO: Need to verify each EmailIdentifier before adding to marked_removed collection. public async void mark_removed_async(Gee.Collection ids, bool mark_removed, Cancellable? cancellable) throws Error { check_open(); diff --git a/src/engine/imap-engine/imap-engine-generic-folder.vala b/src/engine/imap-engine/imap-engine-generic-folder.vala index c429fe5d..09d2c596 100644 --- a/src/engine/imap-engine/imap-engine-generic-folder.vala +++ b/src/engine/imap-engine/imap-engine-generic-folder.vala @@ -13,7 +13,6 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde internal ImapDB.Folder local_folder { get; protected set; } internal Imap.Folder? remote_folder { get; protected set; default = null; } - internal int remote_count { get; private set; default = -1; } private weak GenericAccount account; private Imap.Account remote; @@ -25,6 +24,7 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde private NonblockingSemaphore remote_semaphore; private ReplayQueue? replay_queue = null; private NonblockingMutex normalize_email_positions_mutex = new NonblockingMutex(); + private int remote_count = -1; public GenericFolder(GenericAccount account, Imap.Account remote, ImapDB.Account local, ImapDB.Folder local_folder, SpecialFolderType special_folder_type) { @@ -101,6 +101,18 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde return Geary.Folder.OpenState.OPENING; } + // Returns the synchronized remote count (-1 if not opened) and the last seen remote count (stored + // locally, -1 if not available) + // + // Return value is the remote_count, unless the remote is unopened, in which case it's the + // last_seen_remote_count (which may be -1). + internal int get_remote_counts(out int remote_count, out int last_seen_remote_count) { + remote_count = this.remote_count; + last_seen_remote_count = (local_folder.get_properties() != null) ? local_folder.get_properties().messages : -1; + + return (remote_count >= 0) ? remote_count : last_seen_remote_count; + } + private async bool normalize_folders(Geary.Imap.Folder remote_folder, Cancellable? cancellable) throws Error { debug("normalize_folders %s", to_string()); @@ -594,9 +606,14 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde to_string(), err.message); } - // save new remote count + // save new remote count internally and in local store bool changed = (remote_count != new_remote_count); remote_count = new_remote_count; + try { + yield local_folder.update_remote_message_count(remote_count, null); + } catch (Error update_err) { + debug("Unable to save appended remote count for %s: %s", to_string(), update_err.message); + } if (appended.size > 0) notify_email_appended(appended); @@ -670,10 +687,16 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde debug("Error fetching new local count for %s: %s", to_string(), new_count_err.message); } - // save new remote count and notify of change + // save new remote count internally and in local store bool changed = (remote_count != new_remote_count); remote_count = new_remote_count; + try { + yield local_folder.update_remote_message_count(remote_count, null); + } catch (Error update_err) { + debug("Unable to save removed remote count for %s: %s", to_string(), update_err.message); + } + // notify of change if (!marked && owned_id != null) notify_email_removed(new Geary.Singleton(owned_id)); @@ -817,13 +840,6 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde return; } - // listing by ID requires the remote to be open and fully synchronized, as there's no - // reliable way to determine certain counts and positions without it - // - // TODO: Need to deal with this in a sane manner when offline - yield throw_if_remote_not_ready_async(cancellable); - assert(remote_count >= 0); - // Schedule list operation and wait for completion. ListEmailByID op = new ListEmailByID(this, initial_id, count, required_fields, flags, accumulator, cb, cancellable); @@ -876,8 +892,7 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde return; } - // Unlike list_email_by_id, don't need to wait for remote to open because not dealing with - // a range of emails, but specific ones by ID + // Schedule list operation and wait for completion. ListEmailBySparseID op = new ListEmailBySparseID(this, ids, required_fields, flags, accumulator, cb, cancellable); replay_queue.schedule(op); @@ -947,39 +962,44 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde check_id(method, id); } - // Converts a remote position to a local position, assuming that the remote has been completely - // opened. local_count must be supplied because that's not held by EngineFolder (unlike - // remote_count). remote_pos is 1-based. + // Converts a remote position to a local position. remote_pos is 1-based. // - // Returns a negative value if not available in local folder or remote is not open yet. - internal int remote_position_to_local_position(int remote_pos, int local_count) { + // Returns negative value if remote_count is smaller than local_count or remote_pos is out of + // range. + internal static int remote_position_to_local_position(int remote_pos, int local_count, int remote_count) { assert(remote_pos >= 1); + assert(local_count >= 0); + assert(remote_count >= 0); - if (remote_count < 0) { - debug("[%s] remote_position_to_local_position called before remote opened", to_string()); - } else if (remote_count < local_count) { - debug("[%s] remote_position_to_local_position: remote_count=%d < local_count=%d", - to_string(), remote_count, local_count); + if (remote_count < local_count) { + debug("remote_position_to_local_position: remote_count=%d < local_count=%d", + remote_count, local_count); + } else if (remote_pos > remote_count) { + debug("remote_position_to_local_position: remote_pos=%d > remote_count=%d", + remote_pos, remote_count); } - return (remote_count >= 0) ? remote_pos - (remote_count - local_count) : -1; + return (remote_pos <= remote_count) ? remote_pos - (remote_count - local_count) : -1; } - // Converts a local position to a remote position, assuming that the remote has been completely - // opened. See remote_position_to_local_position for more caveats. + // Converts a local position to a remote position. local_pos is 1-based. // - // Returns a negative value if remote is not open. - internal int local_position_to_remote_position(int local_pos, int local_count) { + // Returns negative value if remote_count is smaller than local_count or if local_pos is out + // of range. + internal static int local_position_to_remote_position(int local_pos, int local_count, int remote_count) { assert(local_pos >= 1); + assert(local_count >= 0); + assert(remote_count >= 0); - if (remote_count < 0) { - debug("[%s] local_position_to_remote_position called before remote opened", to_string()); - } else if (remote_count < local_count) { - debug("[%s] local_position_to_remote_position: remote_count=%d < local_count=%d", - to_string(), remote_count, local_count); + if (remote_count < local_count) { + debug("local_position_to_remote_position: remote_count=%d < local_count=%d", + remote_count, local_count); + } else if (local_pos > local_count) { + debug("local_position_to_remote_position: local_pos=%d > local_count=%d", + local_pos, local_count); } - return (remote_count >= 0) ? remote_count - (local_count - local_pos) : -1; + return (local_pos <= local_count) ? remote_count - (local_count - local_pos) : -1; } // In order to maintain positions for all messages without storing all of them locally, @@ -1066,8 +1086,7 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde Geary.EmailFlags? flags_to_add, Geary.EmailFlags? flags_to_remove, Cancellable? cancellable = null) throws Error { check_open("mark_email_async"); - yield throw_if_remote_not_ready_async(cancellable); - + replay_queue.schedule(new MarkEmail(this, to_mark, flags_to_add, flags_to_remove, cancellable)); } @@ -1075,19 +1094,17 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde public virtual async void copy_email_async(Gee.List to_copy, Geary.FolderPath destination, Cancellable? cancellable = null) throws Error { check_open("copy_email_async"); - yield throw_if_remote_not_ready_async(cancellable); - + replay_queue.schedule(new CopyEmail(this, to_copy, destination)); } public virtual async void move_email_async(Gee.List to_move, Geary.FolderPath destination, Cancellable? cancellable = null) throws Error { check_open("move_email_async"); - yield throw_if_remote_not_ready_async(cancellable); - + replay_queue.schedule(new MoveEmail(this, to_move, destination)); } - + private void on_email_flags_changed(Gee.Map changed) { notify_email_flags_changed(changed); } diff --git a/src/engine/imap-engine/replay-ops/imap-engine-expunge-email.vala b/src/engine/imap-engine/replay-ops/imap-engine-expunge-email.vala index aef2fe02..ca31d7f4 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-expunge-email.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-expunge-email.vala @@ -21,11 +21,21 @@ private class Geary.ImapEngine.ExpungeEmail : Geary.ImapEngine.SendReplayOperati } public override async ReplayOperation.Status replay_local_async() throws Error { + if (to_remove.size <= 0) + return ReplayOperation.Status.COMPLETED; + + int remote_count; + int last_seen_remote_count; + original_count = engine.get_remote_counts(out remote_count, out last_seen_remote_count); + + // because this value is only used for reporting count changes, offer best-possible service + if (original_count < 0) + original_count = to_remove.size; + yield engine.local_folder.mark_removed_async(to_remove, true, cancellable); engine.notify_email_removed(to_remove); - original_count = engine.remote_count; engine.notify_email_count_changed(original_count - to_remove.size, Geary.Folder.CountChangeReason.REMOVED); diff --git a/src/engine/imap-engine/replay-ops/imap-engine-fetch-email.vala b/src/engine/imap-engine/replay-ops/imap-engine-fetch-email.vala index bbd90eaa..d13f9dbd 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-fetch-email.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-fetch-email.vala @@ -44,8 +44,13 @@ private class Geary.ImapEngine.FetchEmail : Geary.ImapEngine.SendReplayOperation if (email != null && email.fields.fulfills(required_fields)) return ReplayOperation.Status.COMPLETED; - // If local only and not found fully in local store, throw NOT_FOUND; there is no fallback - if (flags.is_all_set(Folder.ListFlags.LOCAL_ONLY)) { + int remote_count; + int last_seen_remote_count; + engine.get_remote_counts(out remote_count, out last_seen_remote_count); + + // If local only (or not connected) and not found fully in local store, throw NOT_FOUND; + // there is no fallback + if (flags.is_all_set(Folder.ListFlags.LOCAL_ONLY) || remote_count < 0) { throw new EngineError.NOT_FOUND("Email %s with fields %Xh not found in %s", id.to_string(), required_fields, to_string()); } diff --git a/src/engine/imap-engine/replay-ops/imap-engine-list-email-by-id.vala b/src/engine/imap-engine/replay-ops/imap-engine-list-email-by-id.vala index df21dd8f..9f07807d 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-list-email-by-id.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-list-email-by-id.vala @@ -28,8 +28,16 @@ private class Geary.ImapEngine.ListEmailByID : Geary.ImapEngine.ListEmail { initial_id.to_string(), engine.to_string()); } + int remote_count; + int last_seen_remote_count; + int usable_remote_count = engine.get_remote_counts(out remote_count, out last_seen_remote_count); + + // use local count if both remote counts unavailable + if (usable_remote_count < 0) + usable_remote_count = local_count; + // normalize the initial position to the remote folder's addressing - initial_position = engine.local_position_to_remote_position(initial_position, local_count); + initial_position = engine.local_position_to_remote_position(initial_position, local_count, usable_remote_count); if (initial_position <= 0) { throw new EngineError.NOT_FOUND("Cannot map email ID %s in %s to remote folder", initial_id.to_string(), engine.to_string()); @@ -44,7 +52,7 @@ private class Geary.ImapEngine.ListEmailByID : Geary.ImapEngine.ListEmail { high = excluding_id ? initial_position - 1 : initial_position; } else if (count > 0) { low = excluding_id ? initial_position + 1 : initial_position; - high = (count != int.MAX) ? (initial_position + count - 1) : engine.remote_count; + high = (count != int.MAX) ? (initial_position + count - 1) : usable_remote_count; } else { // count == 0 low = initial_position; @@ -71,7 +79,13 @@ private class Geary.ImapEngine.ListEmailByID : Geary.ImapEngine.ListEmail { this.low = low; this.count = actual_count; - return yield base.replay_local_async(); + // Always return completed if the base class says so + if ((yield base.replay_local_async()) == ReplayOperation.Status.COMPLETED) + return ReplayOperation.Status.COMPLETED; + + // Only return CONTINUE if connected to the remote (otherwise possibility of mixing stale + // and fresh email data in single call) + return (remote_count >= 0) ? ReplayOperation.Status.CONTINUE : ReplayOperation.Status.COMPLETED; } public override string describe_state() { diff --git a/src/engine/imap-engine/replay-ops/imap-engine-list-email-by-sparse-id.vala b/src/engine/imap-engine/replay-ops/imap-engine-list-email-by-sparse-id.vala index 52408189..55a6b987 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-list-email-by-sparse-id.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-list-email-by-sparse-id.vala @@ -143,7 +143,11 @@ private class Geary.ImapEngine.ListEmailBySparseID : Geary.ImapEngine.SendReplay cb(fulfilled, null); } - if (local_only || unfulfilled.size == 0) { + int remote_count; + int last_seen_remote_count; + owner.get_remote_counts(out remote_count, out last_seen_remote_count); + + if (local_only || unfulfilled.size == 0 || remote_count < 0) { if (cb != null) cb(null, null); diff --git a/src/engine/imap-engine/replay-ops/imap-engine-list-email.vala b/src/engine/imap-engine/replay-ops/imap-engine-list-email.vala index 4bec425c..1a8ce60c 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-list-email.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-list-email.vala @@ -82,38 +82,32 @@ private class Geary.ImapEngine.ListEmail : Geary.ImapEngine.SendReplayOperation } public override async ReplayOperation.Status replay_local_async() throws Error { - int local_count; - if (!local_only) { - // normalize the position (ordering) of what's available locally with the situation on - // the server ... this involves fetching the PROPERTIES of the missing emails from - // the server and caching them locally - yield engine.normalize_email_positions_async(low, count, out local_count, cancellable); - } else { - // local_only means just that - local_count = yield engine.local_folder.get_email_count_async(ImapDB.Folder.ListFlags.NONE, - cancellable); + int local_count = yield engine.local_folder.get_email_count_async(ImapDB.Folder.ListFlags.NONE, + cancellable); + + int remote_count; + int last_seen_remote_count; + int usable_remote_count = engine.get_remote_counts(out remote_count, out last_seen_remote_count); + + if (usable_remote_count <= 0) { + if (cb != null) + cb(null, null); + + Logging.debug(Logging.Flag.REPLAY, + "ListEmail.replay_local_async %s: No usable remote count, completed", engine.to_string()); + + return ReplayOperation.Status.COMPLETED; } - // normalize the arguments so they reflect cardinal positions ... remote_count can be -1 - // if the folder is in the process of opening - int local_low = 0; - if (!local_only && yield engine.wait_for_remote_ready_async(cancellable)) { - engine.normalize_span_specifiers(ref low, ref count, engine.remote_count); - - // because the local store caches messages starting from the newest (at the end of the list) - // to the earliest fetched by the user, need to adjust the low value to match its offset - // and range - if (low > 0) - local_low = engine.remote_position_to_local_position(low, local_count); - } else { - engine.normalize_span_specifiers(ref low, ref count, local_count); - if (low > 0) - local_low = low.clamp(1, local_count); - } + engine.normalize_span_specifiers(ref low, ref count, usable_remote_count); + + int local_low = engine.remote_position_to_local_position(low, local_count, usable_remote_count).clamp(1, local_count); Logging.debug(Logging.Flag.REPLAY, - "ListEmail.replay_local %s: low=%d count=%d local_count=%d remote_count=%d local_low=%d", - engine.to_string(), low, count, local_count, engine.remote_count, local_low); + "ListEmail.replay_local_async %s: low=%d count=%d local_low=%d local_count=%d remote_count=%d " + + "last_seen_remote_count=%d usable_remote_count=%d local_only=%s remote_only=%s", + engine.to_string(), low, count, local_low, local_count, remote_count, last_seen_remote_count, + usable_remote_count, local_only.to_string(), remote_only.to_string()); if (!remote_only && local_low > 0) { try { @@ -129,8 +123,8 @@ private class Geary.ImapEngine.ListEmail : Geary.ImapEngine.SendReplayOperation local_list_size = (local_list != null) ? local_list.size : 0; // fixup local email positions to match server's positions - if (local_list_size > 0 && engine.remote_count > 0 && local_count < engine.remote_count) { - int adjustment = engine.remote_count - local_count; + if (local_list_size > 0 && usable_remote_count > 0 && local_count < usable_remote_count) { + int adjustment = usable_remote_count - local_count; foreach (Geary.Email email in local_list) email.update_position(email.position + adjustment); } @@ -150,6 +144,10 @@ private class Geary.ImapEngine.ListEmail : Geary.ImapEngine.SendReplayOperation } } + Logging.debug(Logging.Flag.REPLAY, + "ListEmail.replay_local_async %s: local_list_size=%d fulfilled=%d", to_string(), + local_list_size, fulfilled.size); + // report fulfilled if (fulfilled.size > 0) { if (accumulator != null) @@ -159,8 +157,15 @@ private class Geary.ImapEngine.ListEmail : Geary.ImapEngine.SendReplayOperation cb(fulfilled, null); } - // if local list matches total asked for, or if only returning local versions, exit - if (fulfilled.size == count || local_only) { + // if local list matches total asked for, if only returning local versions, or if not + // connected to the remote, operation is completed + // + // NOTE: Do NOT want to wait for remote to open in replay_remote_async() if work was done + // here using last_seen_remote_count, as there's a high possibility that positional + // addressing will be out of sync will return bogus email to caller; in other words, it + // means returning a combination of dirty local email and validated remote email, which is + // bad news + if (fulfilled.size == count || local_only || remote_count < 0) { if (!local_only) assert(unfulfilled.size == 0); @@ -176,6 +181,11 @@ private class Geary.ImapEngine.ListEmail : Geary.ImapEngine.SendReplayOperation public override async ReplayOperation.Status replay_remote_async() throws Error { yield engine.throw_if_remote_not_ready_async(cancellable); + // normalize the email positions in the local store, so the positions being requested + // from the server are available in the database + int local_count; + yield engine.normalize_email_positions_async(low, count, out local_count, cancellable); + // go through the positions from (low) to (low + count) and see if they're not already // present in local_list; whatever isn't present needs to be fetched in full // diff --git a/src/engine/imap-engine/replay-ops/imap-engine-mark-email.vala b/src/engine/imap-engine/replay-ops/imap-engine-mark-email.vala index 26cd1cfc..c9931f5c 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-mark-email.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-mark-email.vala @@ -26,7 +26,13 @@ private class Geary.ImapEngine.MarkEmail : Geary.ImapEngine.SendReplayOperation } public override async ReplayOperation.Status replay_local_async() throws Error { + if (to_mark.size == 0) + return ReplayOperation.Status.COMPLETED; + // Save original flags, then set new ones. + // TODO: Make this atomic (otherwise there stands a chance backout_local_async() will + // reapply the wrong flags): should get the original flags and the new flags in the same + // operation as the marking procedure, so original flags and reported flags are correct original_flags = yield engine.local_folder.get_email_flags_async(to_mark, cancellable); yield engine.local_folder.mark_email_async(to_mark, flags_to_add, flags_to_remove, cancellable); diff --git a/src/engine/imap-engine/replay-ops/imap-engine-move-email.vala b/src/engine/imap-engine/replay-ops/imap-engine-move-email.vala index 9160708a..403ed209 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-move-email.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-move-email.vala @@ -23,10 +23,20 @@ private class Geary.ImapEngine.MoveEmail : Geary.ImapEngine.SendReplayOperation } public override async ReplayOperation.Status replay_local_async() throws Error { + if (to_move.size <= 0) + return ReplayOperation.Status.COMPLETED; + + int remote_count; + int last_seen_remote_count; + original_count = engine.get_remote_counts(out remote_count, out last_seen_remote_count); + + // as this value is only used for reporting, offer best-possible service + if (original_count < 0) + original_count = to_move.size; + yield engine.local_folder.mark_removed_async(to_move, true, cancellable); engine.notify_email_removed(to_move); - original_count = engine.remote_count; engine.notify_email_count_changed(original_count - to_move.size, Geary.Folder.CountChangeReason.REMOVED); diff --git a/src/engine/imap/api/imap-folder-properties.vala b/src/engine/imap/api/imap-folder-properties.vala index ba56ee02..ed643da2 100644 --- a/src/engine/imap/api/imap-folder-properties.vala +++ b/src/engine/imap/api/imap-folder-properties.vala @@ -5,7 +5,8 @@ */ public class Geary.Imap.FolderProperties { - public int messages { get; private set; } + // messages can be updated a variety of ways, so it's available as a public set + public int messages { get; set; } public int recent { get; private set; } public int unseen { get; private set; } public UIDValidity? uid_validity { get; private set; } diff --git a/src/engine/imap/message/imap-message-data.vala b/src/engine/imap/message/imap-message-data.vala index 41596165..4d250fec 100644 --- a/src/engine/imap/message/imap-message-data.vala +++ b/src/engine/imap/message/imap-message-data.vala @@ -65,6 +65,11 @@ public class Geary.Imap.UID : Geary.Common.Int64MessageData, Geary.Imap.MessageD } public class Geary.Imap.UIDValidity : Geary.Common.Int64MessageData, Geary.Imap.MessageData { + // Using statics because int32.MAX is static, not const (??) + public static int64 MIN = 1; + public static int64 MAX = int32.MAX; + public static int64 INVALID = -1; + public UIDValidity(int64 value) { base (value); }