From 4feba6c970f1a5ea119a92b8ada4497092e38e43 Mon Sep 17 00:00:00 2001 From: Jim Nelson Date: Fri, 23 Aug 2013 16:58:24 -0700 Subject: [PATCH] Quicker and simpler folder normalization: Closes #7364 Folder normalization is now much simpler, faster, and requires less resources than prior implementation. Normalization affects the ReplayOperations, so an interface was changed here. --- src/engine/imap-db/imap-db-account.vala | 6 +- .../imap-db/imap-db-email-identifier.vala | 4 + src/engine/imap-db/imap-db-folder.vala | 139 ++++-- .../imap-engine-email-flag-watcher.vala | 25 +- .../imap-engine-generic-account.vala | 24 +- .../imap-engine-generic-folder.vala | 466 ++++++------------ .../imap-engine-receive-replay-operation.vala | 7 +- .../imap-engine-replay-operation.vala | 28 +- .../imap-engine/imap-engine-replay-queue.vala | 24 +- .../imap-engine-abstract-list-email.vala | 57 +-- .../replay-ops/imap-engine-copy-email.vala | 18 +- .../replay-ops/imap-engine-expunge-email.vala | 33 +- .../replay-ops/imap-engine-fetch-email.vala | 25 +- .../imap-engine-list-email-by-sparse-id.vala | 7 + .../replay-ops/imap-engine-mark-email.vala | 35 +- .../replay-ops/imap-engine-move-email.vala | 36 +- .../imap/api/imap-folder-properties.vala | 12 +- src/engine/imap/api/imap-folder.vala | 25 +- src/engine/imap/message/imap-uid.vala | 7 +- src/engine/util/util-collection.vala | 8 + 20 files changed, 425 insertions(+), 561 deletions(-) diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala index b1adc59f..e218c142 100644 --- a/src/engine/imap-db/imap-db-account.vala +++ b/src/engine/imap-db/imap-db-account.vala @@ -404,8 +404,8 @@ private class Geary.ImapDB.Account : BaseObject { return exists; } - public async Geary.ImapDB.Folder fetch_folder_async(Geary.FolderPath path, - Cancellable? cancellable = null) throws Error { + public async Geary.ImapDB.Folder fetch_folder_async(Geary.FolderPath path, Cancellable? cancellable) + throws Error { check_open(); // check references table first @@ -908,8 +908,6 @@ private class Geary.ImapDB.Account : BaseObject { // 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(); - int length = path.get_path_length(); if (length < 0) throw new EngineError.BAD_PARAMETERS("Invalid path %s", path.to_string()); diff --git a/src/engine/imap-db/imap-db-email-identifier.vala b/src/engine/imap-db/imap-db-email-identifier.vala index 72840c60..8f54a7eb 100644 --- a/src/engine/imap-db/imap-db-email-identifier.vala +++ b/src/engine/imap-db/imap-db-email-identifier.vala @@ -37,6 +37,10 @@ private class Geary.ImapDB.EmailIdentifier : Geary.EmailIdentifier { this.message_id = message_id; } + public bool has_uid() { + return (uid != null) && uid.is_valid(); + } + public override int natural_sort_comparator(Geary.EmailIdentifier o) { ImapDB.EmailIdentifier? other = o as ImapDB.EmailIdentifier; if (other == null) diff --git a/src/engine/imap-db/imap-db-folder.vala b/src/engine/imap-db/imap-db-folder.vala index 27de8893..f4617023 100644 --- a/src/engine/imap-db/imap-db-folder.vala +++ b/src/engine/imap-db/imap-db-folder.vala @@ -548,6 +548,41 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics { return email; } + public async Gee.SortedSet? list_uids_by_range_async(Imap.UID first_uid, Imap.UID last_uid, + Cancellable? cancellable) throws Error { + // order correctly + Imap.UID start, end; + if (first_uid.compare_to(last_uid) < 0) { + start = first_uid; + end = last_uid; + } else { + start = last_uid; + end = first_uid; + } + + Gee.SortedSet uids = new Gee.TreeSet(); + yield db.exec_transaction_async(Db.TransactionType.RO, (cx) => { + Db.Statement stmt = cx.prepare(""" + SELECT ordering + FROM MessageLocationTable + WHERE folder_id = ? AND ordering >= ? AND ordering <= ? + """); + stmt.bind_rowid(0, folder_id); + stmt.bind_int64(1, start.value); + stmt.bind_int64(2, end.value); + + Db.Result result = stmt.exec(cancellable); + while (!result.finished) { + uids.add(new Imap.UID(result.int64_at(0))); + result.next(cancellable); + } + + return Db.TransactionOutcome.DONE; + }, cancellable); + + return (uids.size > 0) ? uids : null; + } + // pos is 1-based. This method does not respect messages marked for removal. public async ImapDB.EmailIdentifier? get_id_at_async(int pos, Cancellable? cancellable) throws Error { assert(pos >= 1); @@ -625,6 +660,23 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics { return id; } + public async Gee.Set get_ids_async(Gee.Collection uids, + Cancellable? cancellable) throws Error { + Gee.Set ids = new Gee.HashSet(); + yield db.exec_transaction_async(Db.TransactionType.RO, (cx) => { + foreach (Imap.UID uid in uids) { + LocationIdentifier? location = do_get_location_for_uid(cx, uid, ListFlags.NONE, + cancellable); + if (location != null) + ids.add(location.email_id); + } + + return Db.TransactionOutcome.DONE; + }, cancellable); + + return (ids.size > 0) ? ids : null; + } + // This does not respect messages marked for removal. public async ImapDB.EmailIdentifier? get_earliest_id_async(Cancellable? cancellable) throws Error { return yield get_id_extremes_async(true, cancellable); @@ -1055,8 +1107,23 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics { // if found, merge, and associate if necessary if (location != null) { - do_merge_email(cx, location, email, out pre_fields, out post_fields, - out updated_contacts, ref unread_count_change, !associated, cancellable); + if (!associated) + do_associate_with_folder(cx, location.message_id, location.uid, cancellable); + + // If the email came from the Imap layer, we need to fill in the id. + ImapDB.EmailIdentifier email_id = (ImapDB.EmailIdentifier) email.id; + if (email_id.message_id == Db.INVALID_ROWID) + email_id.promote_with_message_id(location.message_id); + + // special-case updating flags, which happens often and should only write to the DB + // if necessary + if (email.fields != Geary.Email.Field.FLAGS) { + do_merge_email(cx, location, email, out pre_fields, out post_fields, + out updated_contacts, ref unread_count_change, cancellable); + } else { + do_merge_email_flags(cx, location, email, out pre_fields, out post_fields, + out updated_contacts, ref unread_count_change, cancellable); + } // return false to indicate a merge return false; @@ -1481,23 +1548,15 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics { } if (new_fields.is_any_set(Geary.Email.Field.FLAGS)) { - // Fetch existing flags. + // Fetch existing flags to update unread count Geary.EmailFlags? old_flags = do_get_email_flags_single(cx, row.id, cancellable); Geary.EmailFlags new_flags = new Geary.Imap.EmailFlags( Geary.Imap.MessageFlags.deserialize(row.email_flags)); - if (old_flags != null) { - // Update unread count if needed. - if (old_flags.contains(Geary.EmailFlags.UNREAD) && !new_flags.contains( - Geary.EmailFlags.UNREAD)) - unread_count_change--; - else if (!old_flags.contains(Geary.EmailFlags.UNREAD) && new_flags.contains( - Geary.EmailFlags.UNREAD)) - unread_count_change++; - } else if (new_flags.contains(Geary.EmailFlags.UNREAD)) { - // No previous flags. + if (old_flags != null && (old_flags.is_unread() != new_flags.is_unread())) + unread_count_change += new_flags.is_unread() ? 1 : -1; + else if (new_flags.is_unread()) unread_count_change++; - } Db.Statement stmt = cx.prepare( "UPDATE MessageTable SET flags=? WHERE id=?"); @@ -1590,24 +1649,45 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics { } } + // This *replaces* the stored flags, it does not OR them ... this is simply a fast-path over + // do_merge_email(), as updating FLAGS happens often and doesn't require a lot of extra work + private void do_merge_email_flags(Db.Connection cx, LocationIdentifier location, Geary.Email email, + out Geary.Email.Field pre_fields, out Geary.Email.Field post_fields, + out Gee.Collection updated_contacts, ref int unread_count_change, + Cancellable? cancellable) throws Error { + assert(email.fields == Geary.Email.Field.FLAGS); + + // no contacts were harmed in the production of this email + updated_contacts = new Gee.ArrayList(); + + // fetch MessageRow and its fields, note that the fields now include FLAGS if they didn't + // already + MessageRow row = do_fetch_message_row(cx, location.message_id, Geary.Email.Field.FLAGS, + out pre_fields, cancellable); + post_fields = pre_fields; + + // compare flags for (a) any change at all and (b) unread changes + Geary.Email row_email = row.to_email(location.email_id); + + if (row_email.email_flags != null && row_email.email_flags.equal_to(email.email_flags)) + return; + + if (row_email.email_flags.is_unread() != email.email_flags.is_unread()) + unread_count_change += email.email_flags.is_unread() ? 1 : -1; + + // write them out to the message row + Gee.Map map = new Gee.HashMap(); + map.set((ImapDB.EmailIdentifier) email.id, email.email_flags); + + do_set_email_flags(cx, map, cancellable); + post_fields |= Geary.Email.Field.FLAGS; + } + private void do_merge_email(Db.Connection cx, LocationIdentifier location, Geary.Email email, out Geary.Email.Field pre_fields, out Geary.Email.Field post_fields, out Gee.Collection updated_contacts, ref int unread_count_change, - bool associate_with_folder, Cancellable? cancellable) throws Error { - // If the email came from the Imap layer, we need to fill in the id. - ImapDB.EmailIdentifier email_id = (ImapDB.EmailIdentifier) email.id; - if (email_id.message_id == Db.INVALID_ROWID) - email_id.promote_with_message_id(location.message_id); - - int new_unread_count = 0; - - if (associate_with_folder) { - // Note: no check is performed here to prevent double-adds. The caller of this method - // is responsible for only setting associate_with_folder if required. - do_associate_with_folder(cx, location.message_id, location.uid, cancellable); - unread_count_change++; - } - + Cancellable? cancellable) throws Error { // Default to an empty list, in case we never call do_merge_message_row. updated_contacts = new Gee.LinkedList(); @@ -1627,6 +1707,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics { do_add_attachments(cx, combined_email, location.message_id, cancellable); // Merge in any fields in the submitted email that aren't already in the database or are mutable + int new_unread_count = 0; if (((fetched_fields & email.fields) != email.fields) || email.fields.is_any_set(Geary.Email.MUTABLE_FIELDS)) { Geary.Email.Field new_fields; diff --git a/src/engine/imap-engine/imap-engine-email-flag-watcher.vala b/src/engine/imap-engine/imap-engine-email-flag-watcher.vala index 7e6be4d2..4d414ab6 100644 --- a/src/engine/imap-engine/imap-engine-email-flag-watcher.vala +++ b/src/engine/imap-engine/imap-engine-email-flag-watcher.vala @@ -5,17 +5,19 @@ */ /** - * Because IMAP doesn't offer a standard mechanism for notifications of email flags changing, - * have to poll for changes, annoyingly. This class performs this task by monitoring the supplied + * Monitor an open {@link ImapEngine.GenericFolder} for changes to {@link EmailFlags}. + * + * Because IMAP doesn't offer a standard mechanism for server notifications of email flags changing, + * have to poll for changes. This class performs this task by monitoring the supplied * folder for its "opened" and "closed" signals and periodically polling for changes. * * Note that EmailFlagWatcher doesn't maintain a reference to the Geary.Folder it's watching. */ + private class Geary.ImapEngine.EmailFlagWatcher : BaseObject { public const int DEFAULT_FLAG_WATCH_SEC = 3 * 60; private const int PULL_CHUNK_COUNT = 100; - private const int MAX_EMAIL_WATCHED = 1000; public bool enabled { get; set; default = true; } @@ -51,7 +53,7 @@ private class Geary.ImapEngine.EmailFlagWatcher : BaseObject { cancellable = new Cancellable(); if (watch_id == 0) - watch_id = Timeout.add_seconds(seconds, on_flag_watch); + watch_id = Idle.add(on_opened_update_flags); } private void on_closed(Geary.Folder.CloseReason close_reason) { @@ -66,6 +68,17 @@ private class Geary.ImapEngine.EmailFlagWatcher : BaseObject { watch_id = 0; } + private bool on_opened_update_flags() { + if (enabled) + flag_watch_async.begin(); + + // this callback was immediately called due to open, schedule next ones for here on out + // on a timer + watch_id = Timeout.add_seconds(seconds, on_flag_watch); + + return false; + } + private bool on_flag_watch() { if (!enabled) { // try again later @@ -100,7 +113,7 @@ private class Geary.ImapEngine.EmailFlagWatcher : BaseObject { Geary.EmailIdentifier? lowest = null; int total = 0; - do { + for (;;) { Gee.List? list_local = yield folder.list_email_by_id_async(lowest, PULL_CHUNK_COUNT, Geary.Email.Field.FLAGS, Geary.Folder.ListFlags.LOCAL_ONLY, cancellable); if (list_local == null || list_local.is_empty) @@ -139,7 +152,7 @@ private class Geary.ImapEngine.EmailFlagWatcher : BaseObject { if (!cancellable.is_cancelled() && changed_map.size > 0) email_flags_changed(changed_map); - } while (total < MAX_EMAIL_WATCHED); + } Logging.debug(Logging.Flag.PERIODIC, "do_flag_watch_async: completed %s, %d messages updates", folder.to_string(), total); diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala b/src/engine/imap-engine/imap-engine-generic-account.vala index 70d84376..6c2a5353 100644 --- a/src/engine/imap-engine/imap-engine-generic-account.vala +++ b/src/engine/imap-engine/imap-engine-generic-account.vala @@ -433,12 +433,28 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.AbstractAccount { } } - // always update, openable or not; update UIDs if already open, otherwise will keep + // always update, openable or not; update UIDs if remote opened, otherwise will keep // signalling that it's changed (because the only time UIDNEXT/UIDValidity is updated - // is when the folder is first opened) + // is when the remote folder is first opened) try { - yield local.update_folder_status_async(remote_folder, - generic_folder.get_open_state() != Geary.Folder.OpenState.CLOSED, cancellable); + bool update_uid_info; + switch (generic_folder.get_open_state()) { + case Folder.OpenState.REMOTE: + case Folder.OpenState.BOTH: + update_uid_info = true; + break; + + case Folder.OpenState.LOCAL: + case Folder.OpenState.OPENING: + case Folder.OpenState.CLOSED: + update_uid_info = false; + break; + + default: + assert_not_reached(); + } + + yield local.update_folder_status_async(remote_folder, update_uid_info, cancellable); } catch (Error update_error) { debug("Unable to update local folder %s with remote properties: %s", remote_folder.to_string(), update_error.message); diff --git a/src/engine/imap-engine/imap-engine-generic-folder.vala b/src/engine/imap-engine/imap-engine-generic-folder.vala index 35c5e07f..4758811a 100644 --- a/src/engine/imap-engine/imap-engine-generic-folder.vala +++ b/src/engine/imap-engine/imap-engine-generic-folder.vala @@ -9,11 +9,6 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde internal const int REMOTE_FETCH_CHUNK_COUNT = 50; private const int NORMALIZATION_CHUNK_COUNT = 5000; - private const Geary.Email.Field NORMALIZATION_FIELDS = - Geary.Email.Field.PROPERTIES | Geary.Email.Field.FLAGS | ImapDB.Folder.REQUIRED_FIELDS; - private const Geary.Email.Field FAST_NORMALIZATION_FIELDS = - Geary.Email.Field.PROPERTIES | ImapDB.Folder.REQUIRED_FIELDS; - public override Account account { get { return _account; } } public override FolderProperties properties { get { return _properties; } } @@ -104,11 +99,7 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde private async bool normalize_folders(Geary.Imap.Folder remote_folder, Geary.Folder.OpenFlags open_flags, Cancellable? cancellable) throws Error { - bool is_fast_open = open_flags.is_any_set(Geary.Folder.OpenFlags.FAST_OPEN); - if (is_fast_open) - debug("fast-open normalize_folders %s", to_string()); - else - debug("normalize_folders %s", to_string()); + debug("%s: Begin normalizing remote and local folders", to_string()); Geary.Imap.FolderProperties local_properties = local_folder.get_properties(); Geary.Imap.FolderProperties remote_properties = remote_folder.properties; @@ -116,16 +107,16 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde // and both must have their next UID's (it's possible they don't if it's a non-selectable // folder) if (local_properties.uid_next == null || local_properties.uid_validity == null) { - debug("Unable to verify UID next for %s: missing local UID next (%s) and/or validity (%s)", - path.to_string(), (local_properties.uid_next == null).to_string(), + debug("%s: Unable to verify UIDs: missing local UIDNEXT (%s) and/or UIDVALIDITY (%s)", + to_string(), (local_properties.uid_next == null).to_string(), (local_properties.uid_validity == null).to_string()); return false; } if (remote_properties.uid_next == null || remote_properties.uid_validity == null) { - debug("Unable to verify UID next for %s: missing remote UID next (%s) and/or validity (%s)", - path.to_string(), (remote_properties.uid_next == null).to_string(), + debug("%s: Unable to verify UIDs: missing remote UIDNEXT (%s) and/or UIDVALIDITY (%s)", + to_string(), (remote_properties.uid_next == null).to_string(), (remote_properties.uid_validity == null).to_string()); return false; @@ -133,12 +124,12 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde // If UIDVALIDITY changes, all email in the folder must be removed as the UIDs are now // invalid ... we merely detach the emails (leaving their contents behind) so duplicate - // detection can fix them up. But once all UIDs are removed, it's must like the next + // detection can fix them up. But once all UIDs are removed, it's much like the next // if case where no earliest UID available, so simply exit. // // see http://tools.ietf.org/html/rfc3501#section-2.3.1.1 if (local_properties.uid_validity.value != remote_properties.uid_validity.value) { - debug("%s UID validity changed, detaching all email: %s -> %s", path.to_string(), + debug("%s: UID validity changed, detaching all email: %s -> %s", to_string(), local_properties.uid_validity.value.to_string(), remote_properties.uid_validity.value.to_string()); @@ -149,330 +140,187 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde // fetch email from earliest email to last to (a) remove any deletions and (b) update // any flags that may have changed - ImapDB.EmailIdentifier? earliest_id = yield local_folder.get_earliest_id_async(cancellable); - ImapDB.EmailIdentifier? latest_id = yield local_folder.get_latest_id_async(cancellable); + ImapDB.EmailIdentifier? local_earliest_id = yield local_folder.get_earliest_id_async(cancellable); + ImapDB.EmailIdentifier? local_latest_id = yield local_folder.get_latest_id_async(cancellable); - // verify still open - check_open("normalize_folders (local earliest UID)"); + // verify still open; this is required throughout after each yield, as a close_async() can + // come in ay any time since this does not run in the context of open_async() + check_open("normalize_folders (local earliest/latest UID)"); // if no earliest UID, that means no messages in local store, so nothing to update - if (earliest_id == null || !earliest_id.uid.is_valid() || latest_id == null || !latest_id.uid.is_valid()) { - debug("No earliest and/or latest UID in local %s, nothing to normalize", to_string()); + if (local_earliest_id == null || local_latest_id == null) { + debug("%s: local store empty, nothing to normalize", to_string()); return true; } + assert(local_earliest_id.has_uid()); + assert(local_latest_id.has_uid()); + // if UIDNEXT has changed, that indicates messages have been appended (and possibly removed) int64 uidnext_diff = remote_properties.uid_next.value - local_properties.uid_next.value; - // fast-open means no updating of flags, so if UIDNEXT is the same as last time AND the total count - // of email is the same, then nothing has been added or removed (but flags may have changed) - if (is_fast_open && uidnext_diff == 0 && local_properties.email_total == remote_properties.email_total) { - debug("No messages added/removed in fast-open of %s, normalization completed", to_string()); + // if UIDNEXT is the same as last time AND the total count of email is the same, then + // nothing has been added or removed + if (uidnext_diff == 0 && local_properties.email_total == remote_properties.email_total) { + debug("%s: No messages added/removed since last opened, normalization completed", to_string()); return true; } - Gee.Collection all_appended_ids = new Gee.ArrayList(); - Gee.Collection all_locally_appended_ids = new Gee.ArrayList(); - Gee.Collection all_removed_ids = new Gee.ArrayList(); - Gee.Map all_flags_changed = new Gee.HashMap(); - Gee.Map uid_to_email_id_map - = new Gee.HashMap(); + // a full normalize works from the highest possible UID on the remote and work down to the lowest UID on + // the local; this covers all messages appended since last seen as well as any removed + Imap.UID last_uid = remote_properties.uid_next.previous(); - // to match all flags and find all removed interior to the local store's vector of messages, start from - // local earliest message and work upwards - Imap.UID current_start_uid = earliest_id.uid; - - // if fast-open and the difference in UIDNEXT values equals the difference in message count, then only + // if the difference in UIDNEXT values equals the difference in message count, then only // an append could have happened, so only pull in the new messages ... note that this is not foolproof, // as UIDs are not guaranteed to increase by 1; however, this is a standard implementation practice, // so it's worth looking for // // (Also, this cannot fail; if this situation exists, then it cannot by definition indicate another // situation, esp. messages being removed.) - if (is_fast_open) { - if (uidnext_diff == (remote_properties.select_examine_messages - local_properties.select_examine_messages)) { - current_start_uid = latest_id.uid.next(); - - debug("fast-open %s: Messages only appended (local/remote UIDNEXT=%s/%s total=%d/%d diff=%s), only gathering new mail at %s", - to_string(), local_properties.uid_next.to_string(), remote_properties.uid_next.to_string(), - local_properties.select_examine_messages, remote_properties.select_examine_messages, uidnext_diff.to_string(), - current_start_uid.to_string()); - } else { - debug("fast-open %s: Messages appended/removed (local/remote UIDNEXT=%s/%s total=%d/%d diff=%s)", to_string(), - local_properties.uid_next.to_string(), remote_properties.uid_next.to_string(), - local_properties.select_examine_messages, remote_properties.select_examine_messages, uidnext_diff.to_string()); - } + Imap.UID first_uid; + if (uidnext_diff == (remote_properties.select_examine_messages - local_properties.select_examine_messages)) { + first_uid = local_latest_id.uid.next(); + + debug("%s: Messages only appended (local/remote UIDNEXT=%s/%s total=%d/%d diff=%s), gathering mail UIDs %s:%s", + to_string(), local_properties.uid_next.to_string(), remote_properties.uid_next.to_string(), + local_properties.select_examine_messages, remote_properties.select_examine_messages, uidnext_diff.to_string(), + first_uid.to_string(), last_uid.to_string()); + } else { + first_uid = local_earliest_id.uid; + + debug("%s: Messages appended/removed (local/remote UIDNEXT=%s/%s total=%d/%d diff=%s), gathering mail UIDs %s:%s", + to_string(), local_properties.uid_next.to_string(), remote_properties.uid_next.to_string(), + local_properties.select_examine_messages, remote_properties.select_examine_messages, uidnext_diff.to_string(), + first_uid.to_string(), last_uid.to_string()); } - Geary.Email.Field normalization_fields = is_fast_open ? FAST_NORMALIZATION_FIELDS : NORMALIZATION_FIELDS; + // get all the UIDs in said range from the local store, sorted; convert to non-null + // for ease of use later + Gee.SortedSet? local_uids = yield local_folder.list_uids_by_range_async( + first_uid, last_uid, cancellable); + if (local_uids == null) + local_uids = new Gee.TreeSet(); - for (;;) { - Imap.UID current_end_uid = new Imap.UID(current_start_uid.value + NORMALIZATION_CHUNK_COUNT); - - // Get the local emails in the range ... use PARTIAL_OK to ensure all emails are normalized - Gee.List? old_local = yield local_folder.list_email_by_uid_range_async( - current_start_uid, current_end_uid, normalization_fields, - ImapDB.Folder.ListFlags.PARTIAL_OK | ImapDB.Folder.ListFlags.INCLUDING_ID, + check_open("normalize_folders (list local)"); + + // Do the same on the remote ... make non-null for ease of use later + Gee.SortedSet? remote_uids = yield remote_folder.list_uids_async( + new Imap.MessageSet.uid_range(first_uid, last_uid), cancellable); + if (remote_uids == null) + remote_uids = new Gee.TreeSet(); + + check_open("normalize_folders (list remote)"); + + // walk local UIDs looking for UIDs no longer on remote, removing those that are available + // make the next pass that much shorter + Gee.HashSet removed_uids = new Gee.HashSet(); + foreach (Imap.UID local_uid in local_uids) { + // if in local but not remote, consider removed from remote + if (!remote_uids.remove(local_uid)) + removed_uids.add(local_uid); + } + + // everything remaining in remote has been added since folder last seen ... whether they're + // discovered (inserted) or appended depends on the highest local UID + Gee.HashSet appended_uids = new Gee.HashSet(); + Gee.HashSet discovered_uids = new Gee.HashSet(); + foreach (Imap.UID remote_uid in remote_uids) { + if (remote_uid.compare_to(local_latest_id.uid) > 0) + appended_uids.add(remote_uid); + else + discovered_uids.add(remote_uid); + } + + // fetch from the server the local store's required flags for all appended/inserted messages + // (which is simply equal to all remaining remote UIDs) + Gee.List? to_create = null; + if (remote_uids.size > 0) { + // for new messages, get the local store's required fields (which provide duplicate + // detection) + to_create = yield remote_folder.list_email_async( + new Imap.MessageSet.uid_sparse(remote_uids.to_array()), ImapDB.Folder.REQUIRED_FIELDS, cancellable); - - // verify still open - check_open("normalize_folders (list local)"); - - // be sure they're sorted from earliest to latest - if (old_local != null) - old_local.sort(ImapDB.EmailIdentifier.compare_email_uid_ascending); - - int local_length = (old_local != null) ? old_local.size : 0; - - // if nothing, keep going because there could be remote messages to pull down - Imap.MessageSet msg_set = new Imap.MessageSet.uid_range(current_start_uid, current_end_uid); - - // Get the remote emails in the range to either add any not known, remove deleted messages, - // and update the flags of the remainder - Gee.List? old_remote = yield remote_folder.list_email_async(msg_set, - normalization_fields, cancellable); - - // verify still open after I/O - check_open("normalize_folders (list remote)"); - - // sort earliest to latest - if (old_remote != null) - old_remote.sort(ImapDB.EmailIdentifier.compare_email_uid_ascending); - - int remote_length = (old_remote != null) ? old_remote.size : 0; - - int remote_ctr = 0; - int local_ctr = 0; - Gee.ArrayList to_create_or_merge = new Gee.ArrayList(); - Gee.ArrayList appended_ids = new Gee.ArrayList(); - Gee.ArrayList removed_ids = new Gee.ArrayList(); - for (;;) { - // this loop can be long, so manually check for cancellation - if (cancellable != null && cancellable.is_cancelled()) - throw new IOError.CANCELLED("Folder %s normalization cancelled", to_string()); - - Geary.Email? remote_email = null; - Geary.Imap.UID? remote_uid = null; - if (old_remote != null && remote_ctr < remote_length) { - remote_email = old_remote[remote_ctr]; - remote_uid = ((ImapDB.EmailIdentifier) remote_email.id).uid; - assert(remote_uid != null); - } - - Geary.Email? local_email = null; - Geary.Imap.UID? local_uid = null; - if (old_local != null && local_ctr < local_length) { - local_email = old_local[local_ctr]; - local_uid = ((ImapDB.EmailIdentifier) local_email.id).uid; - assert(local_uid != null); - } - - if (local_email == null && remote_email == null) - break; - - if (remote_uid != null && local_uid != null && remote_uid.value == local_uid.value) { - // only update flags if not doing a fast-open - if (!is_fast_open) { - // same, update flags (if changed) and move on - // Because local is PARTIAL_OK, EmailFlags may not be present - Geary.Imap.EmailFlags? local_email_flags = (Geary.Imap.EmailFlags) local_email.email_flags; - Geary.Imap.EmailFlags remote_email_flags = (Geary.Imap.EmailFlags) remote_email.email_flags; - - if ((local_email_flags == null) || !local_email_flags.equal_to(remote_email_flags)) { - // check before writebehind - if (replay_queue.query_local_writebehind_operation(ReplayOperation.WritebehindOperation.UPDATE_FLAGS, - remote_email.id, (Imap.EmailFlags) remote_email.email_flags)) { - to_create_or_merge.add(remote_email); - - // We can't keep a map of EmailId => flags, because sometimes the EmailId - // changes its hash value (see ImapDB.EmailIdentifier.promote_with_message_id). - // Instead, we index by UID (which doesn't change during this function), and - // rebuild the map of EmailId => flags later. - all_flags_changed.set(((ImapDB.EmailIdentifier) remote_email.id).uid, - remote_email.email_flags); - uid_to_email_id_map.set(((ImapDB.EmailIdentifier) remote_email.id).uid, - remote_email.id); - - Logging.debug(Logging.Flag.FOLDER_NORMALIZATION, "%s: merging remote ID %s", - to_string(), remote_email.id.to_string()); - } else { - Logging.debug(Logging.Flag.FOLDER_NORMALIZATION, "%s: writebehind cancelled for merge of %s", - to_string(), remote_email.id.to_string()); - } - } - } - - remote_ctr++; - local_ctr++; - } else if (remote_uid != null && (local_uid == null || remote_uid.value < local_uid.value)) { - // remote we'd not seen before is present, add and move to next remote - // check for writebehind before doing - if (replay_queue.query_local_writebehind_operation(ReplayOperation.WritebehindOperation.CREATE, - remote_email.id, null)) { - to_create_or_merge.add(remote_email); - - Logging.debug(Logging.Flag.FOLDER_NORMALIZATION, "%s: appending inside remote ID %s", - to_string(), remote_email.id.to_string()); - } else { - Logging.debug(Logging.Flag.FOLDER_NORMALIZATION, "%s: writebehind cancelled for inside append of %s", - to_string(), remote_email.id.to_string()); - } - - remote_ctr++; - } else { - if (remote_uid != null && local_uid != null) - assert(remote_uid.value > local_uid.value); - else - assert(local_uid != null); - - // local's email on the server has been removed, remove locally - // check writebehind first - if (replay_queue.query_local_writebehind_operation(ReplayOperation.WritebehindOperation.REMOVE, - local_email.id, null)) { - removed_ids.add(local_email.id); - - Logging.debug(Logging.Flag.FOLDER_NORMALIZATION, "%s: removing inside local ID %s", - to_string(), local_email.id.to_string()); - } else { - Logging.debug(Logging.Flag.FOLDER_NORMALIZATION, "%s: writebehind cancelled for remove of %s", - to_string(), local_email.id.to_string()); - } - - local_ctr++; - } - } - - // add newly-discovered emails to local store ... only report these as appended; earlier - // CreateEmailOperations were updates of emails existing previously or additions of emails - // that were on the server earlier but not stored locally (i.e. this value represents emails - // added to the top of the stack) - for (; remote_ctr < remote_length; remote_ctr++) { - Geary.Email remote_email = old_remote[remote_ctr]; - - // again, have to check for writebehind - if (replay_queue.query_local_writebehind_operation(ReplayOperation.WritebehindOperation.CREATE, - remote_email.id, null)) { - to_create_or_merge.add(remote_email); - appended_ids.add(remote_email.id); - - Logging.debug(Logging.Flag.FOLDER_NORMALIZATION, "%s: appending outside remote %s", - to_string(), remote_email.id.to_string()); - } else { - Logging.debug(Logging.Flag.FOLDER_NORMALIZATION, "%s: writebehind cancelled for outside append of %s", - to_string(), remote_email.id.to_string()); - } - } - - // remove anything left over ... use local count rather than remote as we're still in a stage - // where only the local messages are available - for (; local_ctr < local_length; local_ctr++) { - Geary.Email local_email = old_local[local_ctr]; - - // again, check for writebehind - if (replay_queue.query_local_writebehind_operation(ReplayOperation.WritebehindOperation.REMOVE, - local_email.id, null)) { - removed_ids.add(local_email.id); - - Logging.debug(Logging.Flag.FOLDER_NORMALIZATION, "%s: removing outside remote %s", - to_string(), local_email.id.to_string()); - } else { - Logging.debug(Logging.Flag.FOLDER_NORMALIZATION, "%s: writebehind cancelled for outside remove %s", - to_string(), local_email.id.to_string()); - } - } - - // from here on the only write operations being performed on the folder are creating or updating - // existing emails or removing them, both operations being performed using EmailIdentifiers - // rather than positional addressing ... this means the order of operation is not important - // and can be batched up rather than performed serially - Nonblocking.Batch batch = new Nonblocking.Batch(); - - CreateLocalEmailOperation? create_op = null; - if (to_create_or_merge.size > 0) { - create_op = new CreateLocalEmailOperation(local_folder, to_create_or_merge, - normalization_fields); - batch.add(create_op); - } - - if (removed_ids.size > 0) - batch.add(new RemoveLocalEmailOperation(local_folder, removed_ids)); - - // execute them all at once - if (batch.size > 0) { - yield batch.execute_all_async(cancellable); - - // check still open a third time - check_open("normalize_folders (batch execute)"); - - if (batch.get_first_exception_message() != null) { - debug("Error while preparing opened folder %s: %s", to_string(), - batch.get_first_exception_message()); - } - - // throw the first exception, if one occurred - batch.throw_first_exception(); - } - - // add changes to master lists - all_removed_ids.add_all(removed_ids); - all_appended_ids.add_all(appended_ids); - - // look for local additions (email not known to the local store) to signal - if (create_op != null) { - foreach (Geary.Email email in create_op.created.keys) { - // true means created, not merged - if (create_op.created.get(email)) - all_locally_appended_ids.add(email.id); - } - } - - // increment the next start id after the current end, as it now points to the last id examined - current_start_uid = current_end_uid.next(); - - // if gone past both local and remote extremes, time to exit (note that UIDNEXT is the - // *next* UID on the remote side, not the last UID) - // TODO: If UIDNEXT isn't available on server, will need to fetch the highest UID and - // add one - if (current_start_uid.compare_to(latest_id.uid) > 0 - && current_start_uid.compare_to(remote_properties.uid_next) >= 0) - break; } - // notify emails that have been removed (see note above about why not all Creates are - // signalled) - if (all_removed_ids.size > 0) { - debug("Notifying of %d removed emails since %s last seen", all_removed_ids.size, to_string()); - notify_email_removed(all_removed_ids); - } + check_open("normalize_folders (list remote appended/inserted required fields)"); - // notify local additions - if (all_locally_appended_ids.size > 0) { - debug("Notifying of %d locally appended emails since %s last seen", all_locally_appended_ids.size, - to_string()); - notify_email_locally_appended(all_locally_appended_ids); - notify_email_discovered(all_locally_appended_ids); - } - - // notify additions - if (all_appended_ids.size > 0) { - debug("Notifying of %d appended emails since %s last seen", all_appended_ids.size, to_string()); - notify_email_appended(all_appended_ids); - } - - // notify flag changes - if (all_flags_changed.size > 0) { - Gee.Map changed_flags - = new Gee.HashMap(); - foreach (Imap.UID uid in all_flags_changed.keys) - changed_flags.set(uid_to_email_id_map.get(uid), all_flags_changed.get(uid)); + // store new messages and add IDs to the appended/discovered EmailIdentifier buckets + Gee.Set appended_ids = new Gee.HashSet(); + Gee.Set locally_appended_ids = new Gee.HashSet(); + Gee.Set discovered_ids = new Gee.HashSet(); + if (to_create != null && to_create.size > 0) { + Gee.Map? created_or_merged = yield local_folder.create_or_merge_email_async( + to_create, cancellable); + assert(created_or_merged != null); - debug("Notifying of %d changed flags since %s last seen", changed_flags.size, to_string()); - notify_email_flags_changed(changed_flags); + foreach (Email email in created_or_merged.keys) { + ImapDB.EmailIdentifier id = (ImapDB.EmailIdentifier) email.id; + bool created = created_or_merged.get(email); + + // report all appended email, but separate out email never seen before (created) + // as locally-appended + if (appended_uids.contains(id.uid)) { + appended_ids.add(id); + + if (created) + locally_appended_ids.add(id); + } else if (discovered_uids.contains(id.uid) && created) { + discovered_ids.add(id); + } + } } - debug("Completed normalize_folder %s", to_string()); + check_open("normalize_folders (created/merged appended/discovered emails)"); + + // Convert removed UIDs into EmailIdentifiers and detach immediately + Gee.Set? removed_ids = null; + if (removed_uids.size > 0) { + removed_ids = yield local_folder.get_ids_async(removed_uids, cancellable); + yield local_folder.detach_multiple_emails_async(removed_ids, cancellable); + } + + check_open("normalize_folders (removed emails)"); + + // + // now normalized + // notify subscribers of changes + // + + if (removed_ids != null && removed_ids.size > 0) { + // there may be operations pending on the remote queue for these removed emails; notify + // operations that the email has shuffled off this mortal coil + replay_queue.notify_remote_removed_during_normalization(removed_ids); + + // notify subscribers about emails that have been removed + debug("%s: Notifying of %d removed emails since last opened", to_string(), removed_ids.size); + notify_email_removed(removed_ids); + } + + // notify local discovered (i.e. emails that are in the interior of the vector not seen + // before -- this can happen during vector expansion when the app crashes or closes before + // writing out everything) + if (discovered_ids.size > 0) { + debug("%s: Notifying of %d discovered emails since last opened", to_string(), discovered_ids.size); + notify_email_discovered(discovered_ids); + } + + // notify appended (new email added since the folder was last opened) + if (appended_ids.size > 0) { + debug("%s: Notifying of %d appended emails since last opened", to_string(), appended_ids.size); + notify_email_appended(appended_ids); + } + + // notify locally appended (new email never seen before added since the folder was last + // opened) + if (locally_appended_ids.size > 0) { + debug("%s: Notifying of %d locally appended emails since last opened", to_string(), + locally_appended_ids.size); + notify_email_locally_appended(locally_appended_ids); + } + + debug("%s: Completed normalize_folder", to_string()); return true; } diff --git a/src/engine/imap-engine/imap-engine-receive-replay-operation.vala b/src/engine/imap-engine/imap-engine-receive-replay-operation.vala index 92f7dee2..5640b403 100644 --- a/src/engine/imap-engine/imap-engine-receive-replay-operation.vala +++ b/src/engine/imap-engine/imap-engine-receive-replay-operation.vala @@ -9,11 +9,8 @@ private abstract class Geary.ImapEngine.ReceiveReplayOperation : Geary.ImapEngin base (name, ReplayOperation.Scope.LOCAL_ONLY); } - public override bool query_local_writebehind_operation(ReplayOperation.WritebehindOperation op, - EmailIdentifier id, Imap.EmailFlags? flags) { - debug("Warning: ReceiveReplayOperation.query_local_writebehind_operation() called"); - - return true; + public override void notify_remote_removed_during_normalization(Gee.Collection ids) { + debug("Warning: ReceiveReplayOperation.notify_remote_removed_during_normalization() called"); } public override async ReplayOperation.Status replay_remote_async() throws Error { diff --git a/src/engine/imap-engine/imap-engine-replay-operation.vala b/src/engine/imap-engine/imap-engine-replay-operation.vala index 429c01f2..f6c429eb 100644 --- a/src/engine/imap-engine/imap-engine-replay-operation.vala +++ b/src/engine/imap-engine/imap-engine-replay-operation.vala @@ -32,12 +32,6 @@ private abstract class Geary.ImapEngine.ReplayOperation : Geary.BaseObject { CONTINUE } - public enum WritebehindOperation { - CREATE, - REMOVE, - UPDATE_FLAGS - } - private static int next_opnum = 0; public string name { get; set; } @@ -71,24 +65,16 @@ private abstract class Geary.ImapEngine.ReplayOperation : Geary.BaseObject { * See Scope for conditions where this method will be called. * * This method is called only when the ReplayOperation is blocked waiting to execute a remote - * command and an exterior operation is going to occur that may alter the state on the local - * database (i.e. altering state behind the execution of this operation's replay_local_async()). - * This primarily happens during folder normalization (initial synchronization with the server + * command and its discovered that the supplied email(s) are no longer on the server. + * This happens during folder normalization (initial synchronization with the server * when a folder is opened) where ReplayOperations are allowed to execute locally and enqueue - * for remote operation in preparation for the folder to open. (There may be other - * circumstances in the future where this method may be called.) + * for remote operation in preparation for the folder to fully open. * - * The method should examine the supplied operation and return true if it's okay to proceed - * (and modifying its own operation to reflect the change that will occur before it's allowed to - * proceed, or merely not performing any operation in replay_remote_async()) or false if the - * supplied operation should *not* execute so that this ReplayOperation's command may execute - * shortly. - * - * flags will only be non-null when op is UPDATE_FLAGS. In that case, if this method returns - * true, it may also modify the EmailFlags. Those flags will be written to the local store. + * The ReplayOperation should remove any reference to the emails so not to attempt operation + * on the server. If it's discovered in replay_remote_async() that there are no more operations + * to perform, it should simply exit without contacting the server. */ - public abstract bool query_local_writebehind_operation(WritebehindOperation op, EmailIdentifier id, - Imap.EmailFlags? flags); + public abstract void notify_remote_removed_during_normalization(Gee.Collection ids); /** * See Scope for conditions where this method will be called. diff --git a/src/engine/imap-engine/imap-engine-replay-queue.vala b/src/engine/imap-engine/imap-engine-replay-queue.vala index 18b0bcfc..f9c3a161 100644 --- a/src/engine/imap-engine/imap-engine-replay-queue.vala +++ b/src/engine/imap-engine/imap-engine-replay-queue.vala @@ -15,10 +15,7 @@ private class Geary.ImapEngine.ReplayQueue : Geary.BaseObject { return Status.CONTINUE; } - public override bool query_local_writebehind_operation(ReplayOperation.WritebehindOperation op, - EmailIdentifier id, Imap.EmailFlags? flags) { - // whatever, no problem, do what you will - return true; + public override void notify_remote_removed_during_normalization(Gee.Collection ids) { } public override async ReplayOperation.Status replay_remote_async() throws Error { @@ -150,21 +147,12 @@ private class Geary.ImapEngine.ReplayQueue : Geary.BaseObject { * changes that need to be synchronized on the client. If this change is written before the * enqueued replay operations execute, the potential exists to be unsynchronized. * - * This call gives all enqueued remote replay operations a chance to cancel or update their - * own state due to a writebehind operation. See - * ReplayOperation.query_local_writebehind_operation() for more information. + * This call gives all enqueued remote replay operations a chance to update their own state. + * See ReplayOperation.notify_remote_removed_during_normalization() for more information. */ - public bool query_local_writebehind_operation(ReplayOperation.WritebehindOperation op, - Geary.EmailIdentifier id, Imap.EmailFlags? flags) { - // Although any replay operation can cancel the writebehind operation, give all a chance to - // see it as it may affect their internal state - bool proceed = true; - foreach (ReplayOperation replay_op in remote_queue.get_all()) { - if (!replay_op.query_local_writebehind_operation(op, id, flags)) - proceed = false; - } - - return proceed; + public void notify_remote_removed_during_normalization(Gee.Collection ids) { + foreach (ReplayOperation replay_op in remote_queue.get_all()) + replay_op.notify_remote_removed_during_normalization(ids); } public async void close_async(Cancellable? cancellable = null) throws Error { diff --git a/src/engine/imap-engine/replay-ops/imap-engine-abstract-list-email.vala b/src/engine/imap-engine/replay-ops/imap-engine-abstract-list-email.vala index ef1d2a28..e77f59ba 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-abstract-list-email.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-abstract-list-email.vala @@ -75,44 +75,29 @@ private abstract class Geary.ImapEngine.AbstractListEmail : Geary.ImapEngine.Sen this.flags = flags; } - public override bool query_local_writebehind_operation(ReplayOperation.WritebehindOperation op, - EmailIdentifier id, Imap.EmailFlags? flags) { - // don't need to check if id is present here, all paths deal with this possibility - // correctly + public override void notify_remote_removed_during_normalization(Gee.Collection ids) { + // remove email already picked up from local store ... for email reported via the + // callback, too late + if (accumulator != null) { + Collection.remove_if(accumulator, (email) => { + return ids.contains((ImapDB.EmailIdentifier) email.id); + }); + } - switch (op) { - case ReplayOperation.WritebehindOperation.REMOVE: - // remove email already picked up from local store ... for email reported via the - // callback, too late - if (accumulator != null) { - Gee.HashSet wb_removed = new Gee.HashSet(); - foreach (Geary.Email email in accumulator) { - if (email.id.equal_to(id)) - wb_removed.add(email); - } - - accumulator.remove_all(wb_removed); + // remove from unfulfilled list, as there's nothing to fetch from the server + // this funky little loop ensures that all mentions of the EmailIdentifier in + // the unfulfilled MultiMap are removed, but must restart loop because removing + // within a foreach invalidates the Iterator + foreach (Geary.EmailIdentifier id in ids) { + bool removed = false; + do { + removed = false; + foreach (Geary.Email.Field field in unfulfilled.get_keys()) { + removed = unfulfilled.remove(field, (ImapDB.EmailIdentifier) id); + if (removed) + break; } - - // remove from unfulfilled list, as there's nothing to fetch from the server - // this funky little loop ensures that all mentions of the EmailIdentifier in - // the unfulfilled MultiMap are removed, but must restart loop because removing - // within a foreach invalidates the Iterator - bool removed = false; - do { - removed = false; - foreach (Geary.Email.Field field in unfulfilled.get_keys()) { - removed = unfulfilled.remove(field, (ImapDB.EmailIdentifier) id); - if (removed) - break; - } - } while (removed); - - return true; - - default: - // ignored - return true; + } while (removed); } } diff --git a/src/engine/imap-engine/replay-ops/imap-engine-copy-email.vala b/src/engine/imap-engine/replay-ops/imap-engine-copy-email.vala index a14161e4..f0a3aa38 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-copy-email.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-copy-email.vala @@ -30,22 +30,14 @@ private class Geary.ImapEngine.CopyEmail : Geary.ImapEngine.SendReplayOperation return ReplayOperation.Status.CONTINUE; } - public override bool query_local_writebehind_operation(ReplayOperation.WritebehindOperation op, - EmailIdentifier id, Imap.EmailFlags? flags) { - ImapDB.EmailIdentifier? imapdb_id = id as ImapDB.EmailIdentifier; - if (imapdb_id == null) - return true; - - // only interested in messages going away (i.e. can't be copied) ... - // note that this method operates exactly the same way whether the EmailIdentifer is in - // the to_copy list or not. - if (op == ReplayOperation.WritebehindOperation.REMOVE) - to_copy.remove(imapdb_id); - - return true; + public override void notify_remote_removed_during_normalization(Gee.Collection ids) { + to_copy.remove_all(ids); } public override async ReplayOperation.Status replay_remote_async() throws Error { + if (to_copy.size == 0) + return ReplayOperation.Status.COMPLETED; + Gee.Set? uids = yield engine.local_folder.get_uids_async(to_copy, ImapDB.Folder.ListFlags.NONE, cancellable); 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 3817138a..7d4e12d9 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 @@ -45,38 +45,19 @@ private class Geary.ImapEngine.ExpungeEmail : Geary.ImapEngine.SendReplayOperati return ReplayOperation.Status.CONTINUE; } - public override bool query_local_writebehind_operation(ReplayOperation.WritebehindOperation op, - EmailIdentifier id, Imap.EmailFlags? flags) { - ImapDB.EmailIdentifier? imapdb_id = id as ImapDB.EmailIdentifier; - if (imapdb_id == null) - return true; - - if (!removed_ids.contains(imapdb_id)) - return true; - - switch (op) { - case ReplayOperation.WritebehindOperation.CREATE: - // don't allow for the message to be created, it will be removed on the server by - // this operation - return false; - - case ReplayOperation.WritebehindOperation.REMOVE: - // removed locally, to be removed remotely, don't bother writing locally - return false; - - default: - // ignored - return true; - } + public override void notify_remote_removed_during_normalization(Gee.Collection ids) { + removed_ids.remove_all(ids); } public override async ReplayOperation.Status replay_remote_async() throws Error { // Remove from server. Note that this causes the receive replay queue to kick into // action, removing the e-mail but *NOT* firing a signal; the "remove marker" indicates // that the signal has already been fired. - yield engine.remote_folder.remove_email_async( - new Imap.MessageSet.uid_sparse(ImapDB.EmailIdentifier.to_uids(removed_ids).to_array()), - cancellable); + if (removed_ids.size > 0) { + yield engine.remote_folder.remove_email_async( + new Imap.MessageSet.uid_sparse(ImapDB.EmailIdentifier.to_uids(removed_ids).to_array()), + cancellable); + } return ReplayOperation.Status.COMPLETED; } 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 aa9547bc..83158aff 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 @@ -14,7 +14,7 @@ private class Geary.ImapEngine.FetchEmail : Geary.ImapEngine.SendReplayOperation private Folder.ListFlags flags; private Cancellable? cancellable; private Imap.UID? uid = null; - private bool writebehind_removed = false; + private bool remote_removed = false; public FetchEmail(GenericFolder engine, ImapDB.EmailIdentifier id, Email.Field required_fields, Folder.ListFlags flags, Cancellable? cancellable) { @@ -77,28 +77,13 @@ private class Geary.ImapEngine.FetchEmail : Geary.ImapEngine.SendReplayOperation return ReplayOperation.Status.CONTINUE; } - public override bool query_local_writebehind_operation(ReplayOperation.WritebehindOperation op, - EmailIdentifier id, Imap.EmailFlags? flags) { - if (!this.id.equal_to(id)) - return true; - - switch (op) { - case ReplayOperation.WritebehindOperation.REMOVE: - writebehind_removed = true; - - return true; - - case ReplayOperation.WritebehindOperation.CREATE: - default: - // still need to do the full fetch for CREATE, since it's unknown (currently) what - // fields are available locally; otherwise, ignored - return true; - } + public override void notify_remote_removed_during_normalization(Gee.Collection ids) { + remote_removed = ids.contains(id); } public override async ReplayOperation.Status replay_remote_async() throws Error { - if (writebehind_removed) { - throw new EngineError.NOT_FOUND("Unable to fetch %s in %s (removed with writebehind)", + if (remote_removed) { + throw new EngineError.NOT_FOUND("Unable to fetch %s in %s (removed from remote)", id.to_string(), engine.to_string()); } 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 f296ab99..a743fec1 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 @@ -67,6 +67,13 @@ private class Geary.ImapEngine.ListEmailBySparseID : Geary.ImapEngine.AbstractLi return ReplayOperation.Status.CONTINUE; } + public override void notify_remote_removed_during_normalization( + Gee.Collection removed_ids) { + ids.remove_all(removed_ids); + + base.notify_remote_removed_during_normalization(removed_ids); + } + public override async void backout_local_async() throws Error { // R/O, nothing to backout } 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 288f04bf..0e9d5468 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 @@ -49,38 +49,9 @@ private class Geary.ImapEngine.MarkEmail : Geary.ImapEngine.SendReplayOperation return ReplayOperation.Status.CONTINUE; } - public override bool query_local_writebehind_operation(ReplayOperation.WritebehindOperation op, - EmailIdentifier id, Imap.EmailFlags? flags) { - ImapDB.EmailIdentifier? imapdb_id = id as ImapDB.EmailIdentifier; - if (imapdb_id == null) - return true; - - if (!original_flags.has_key(imapdb_id)) - return true; - - switch (op) { - case ReplayOperation.WritebehindOperation.REMOVE: - // don't bother updating on server - original_flags.unset(imapdb_id); - - return true; - - case ReplayOperation.WritebehindOperation.UPDATE_FLAGS: - // user's mark operation takes precedence over server's, update supplied flags - // and continue - if (flags_to_add != null && flags != null) - flags.add_all(flags_to_add); - - if (flags_to_remove != null && flags != null) - flags.remove_all(flags_to_remove); - - return true; - - case ReplayOperation.WritebehindOperation.CREATE: - default: - // not interested in other operations - return true; - } + public override void notify_remote_removed_during_normalization(Gee.Collection ids) { + // don't bother updating on server or backing out locally + Collection.map_unset_all_keys(original_flags, ids); } public override async ReplayOperation.Status replay_remote_async() throws Error { 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 f95b985d..9730add7 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 @@ -46,36 +46,18 @@ private class Geary.ImapEngine.MoveEmail : Geary.ImapEngine.SendReplayOperation return ReplayOperation.Status.CONTINUE; } - - public override bool query_local_writebehind_operation(ReplayOperation.WritebehindOperation op, - EmailIdentifier id, Imap.EmailFlags? flags) { - ImapDB.EmailIdentifier? imapdb_id = id as ImapDB.EmailIdentifier; - if (imapdb_id == null) - return true; - - if (!moved_ids.contains(imapdb_id)) - return true; - - switch (op) { - case ReplayOperation.WritebehindOperation.CREATE: - // don't allow for it to be created, it's already been marked for removal - return false; - - case ReplayOperation.WritebehindOperation.REMOVE: - case ReplayOperation.WritebehindOperation.UPDATE_FLAGS: - // don't bother, already removed - return false; - - default: - // ignored - return true; - } + + public override void notify_remote_removed_during_normalization(Gee.Collection ids) { + // don't bother updating on server or backing out locally + moved_ids.remove_all(ids); } public override async ReplayOperation.Status replay_remote_async() throws Error { - yield engine.remote_folder.move_email_async( - new Imap.MessageSet.uid_sparse(ImapDB.EmailIdentifier.to_uids(moved_ids).to_array()), - destination, cancellable); + if (moved_ids.size > 0) { + yield engine.remote_folder.move_email_async( + new Imap.MessageSet.uid_sparse(ImapDB.EmailIdentifier.to_uids(moved_ids).to_array()), + destination, cancellable); + } return ReplayOperation.Status.COMPLETED; } diff --git a/src/engine/imap/api/imap-folder-properties.vala b/src/engine/imap/api/imap-folder-properties.vala index 99f55670..0673017d 100644 --- a/src/engine/imap/api/imap-folder-properties.vala +++ b/src/engine/imap/api/imap-folder-properties.vala @@ -109,12 +109,18 @@ public class Geary.Imap.FolderProperties : Geary.FolderProperties { return true; } + // UIDVALIDITY changes indicate the entire folder's contents have potentially altered and + // the client needs to reset its local vector + if (uid_validity != null && other.uid_validity != null && !uid_validity.equal_to(other.uid_validity)) { + debug("%s FolderProperties changed: UIDVALIDITY=%s other.UIDVALIDITY=%s", name, + uid_validity.to_string(), other.uid_validity.to_string()); + + return true; + } + // Gmail includes Chat messages in STATUS results but not in SELECT/EXAMINE // results, so message count comparison has to be from the same origin ... use SELECT/EXAMINE // first, as it's more authoritative in many ways - // - // TODO: If this continues to work, it might be worthwhile to change the result of this - // method to boolean if (select_examine_messages >= 0 && other.select_examine_messages >= 0) { int diff = select_examine_messages - other.select_examine_messages; if (diff != 0) { diff --git a/src/engine/imap/api/imap-folder.vala b/src/engine/imap/api/imap-folder.vala index 0dcc3e60..44521b0c 100644 --- a/src/engine/imap/api/imap-folder.vala +++ b/src/engine/imap/api/imap-folder.vala @@ -324,6 +324,25 @@ private class Geary.Imap.Folder : BaseObject { } } + // Utility method for listing a UID range + public async Gee.SortedSet? list_uids_async(MessageSet msg_set, Cancellable? cancellable) + throws Error { + Gee.List? list = yield list_email_async(msg_set, Geary.Email.Field.NONE, + cancellable); + if (list == null || list.size == 0) + return null; + + Gee.SortedSet uids = new Gee.TreeSet(); + foreach (Geary.Email email in list) { + Imap.UID? uid = ((ImapDB.EmailIdentifier) email.id).uid; + assert(uid != null); + + uids.add(uid); + } + + return uids; + } + // Returns a no-message-id ImapDB.EmailIdentifier with the UID stored in it. public async Gee.List? list_email_async(MessageSet msg_set, Geary.Email.Field fields, Cancellable? cancellable) throws Error { @@ -335,8 +354,10 @@ private class Geary.Imap.Folder : BaseObject { // if not a UID FETCH, request UIDs for all messages so their EmailIdentifier can be // created without going back to the database (assuming the messages have already been - // pulled down, not a guarantee) - if (!msg_set.is_uid) + // pulled down, not a guarantee); if request is for NONE, that guarantees that the + // EmailIdentifier will be set, and so fetch UIDs (which looks funny but works when + // listing a range for contents: UID FETCH x:y UID) + if (!msg_set.is_uid || fields == Geary.Email.Field.NONE) cmds.add(new FetchCommand.data_type(msg_set, FetchDataType.UID)); // convert bulk of the "basic" fields into a one or two FETCH commands (some servers have diff --git a/src/engine/imap/message/imap-uid.vala b/src/engine/imap/message/imap-uid.vala index f949ef47..f04fa475 100644 --- a/src/engine/imap/message/imap-uid.vala +++ b/src/engine/imap/message/imap-uid.vala @@ -58,12 +58,7 @@ public class Geary.Imap.UID : Geary.MessageData.Int64MessageData, Geary.Imap.Mes } public virtual int compare_to(Geary.Imap.UID other) { - if (value < other.value) - return -1; - else if (value > other.value) - return 1; - else - return 0; + return (int) (value - other.value).clamp(-1, 1); } public string serialize() { diff --git a/src/engine/util/util-collection.vala b/src/engine/util/util-collection.vala index 688b60cd..64e09580 100644 --- a/src/engine/util/util-collection.vala +++ b/src/engine/util/util-collection.vala @@ -82,6 +82,14 @@ public void multi_map_set_all(Gee.MultiMap dest, K key, Gee.Collecti dest.set(key, value); } +/** + * Removes all keys from the Map. + */ +public void map_unset_all_keys(Gee.Map map, Gee.Collection keys) { + foreach (K key in keys) + map.unset(key); +} + /** * Return a MultiMap of value => key of the input map's key => values. */