From b3bc72b3327a46cb951f54288cff7e142fdd3370 Mon Sep 17 00:00:00 2001 From: Jim Nelson Date: Tue, 10 Apr 2012 14:33:16 -0700 Subject: [PATCH] Further work from two weeks ago simplifying the Folder interface Because the SQLite and IMAP folder classes no longer implement Geary.Folder, they can be simplified greatly, which this patch does. --- src/engine/imap/api/imap-folder.vala | 91 +--------- src/engine/imap/message/imap-message-set.vala | 19 +++ src/engine/impl/geary-email-flag-watcher.vala | 7 +- .../impl/geary-generic-imap-folder.vala | 36 ++-- .../impl/geary-send-replay-operations.vala | 156 +----------------- src/engine/sqlite/api/sqlite-folder.vala | 14 -- .../email/sqlite-message-location-table.vala | 37 ----- 7 files changed, 53 insertions(+), 307 deletions(-) diff --git a/src/engine/imap/api/imap-folder.vala b/src/engine/imap/api/imap-folder.vala index e037c97d..9bb96124 100644 --- a/src/engine/imap/api/imap-folder.vala +++ b/src/engine/imap/api/imap-folder.vala @@ -27,7 +27,7 @@ private class Geary.Imap.Folder : Object { readonly = Trillian.UNKNOWN; properties = (status != null) - ? new Imap.FolderProperties.status(status , info.attrs) + ? new Imap.FolderProperties.status(status, info.attrs) : new Imap.FolderProperties(0, 0, 0, null, null, info.attrs); } @@ -98,91 +98,26 @@ private class Geary.Imap.Folder : Object { return mailbox.exists; } - public async Gee.List? list_email_async(int low, int count, Geary.Email.Field fields, - Geary.Folder.ListFlags flags, Cancellable? cancellable = null) throws Error { + public async Gee.List? list_email_async(MessageSet msg_set, Geary.Email.Field fields, + Cancellable? cancellable = null) throws Error { if (mailbox == null) throw new EngineError.OPEN_REQUIRED("%s not opened", to_string()); - Geary.Folder.normalize_span_specifiers(ref low, ref count, mailbox.exists); - - return yield mailbox.list_set_async(new MessageSet.range(low, count), fields, cancellable); - } - - public async Gee.List? list_email_sparse_async(int[] by_position, - Geary.Email.Field fields, Geary.Folder.ListFlags flags, Cancellable? cancellable = null) - throws Error { - if (mailbox == null) - throw new EngineError.OPEN_REQUIRED("%s not opened", to_string()); - - return yield mailbox.list_set_async(new MessageSet.sparse(by_position), fields, cancellable); - } - - public async Gee.List? list_email_by_id_async(Geary.EmailIdentifier email_id, - int count, Geary.Email.Field fields, Geary.Folder.ListFlags flags, Cancellable? cancellable = null) - throws Error { - if (mailbox == null) - throw new EngineError.OPEN_REQUIRED("%s not opened", to_string()); - - UID uid = ((Imap.EmailIdentifier) email_id).uid; - if (flags.is_all_set(Geary.Folder.ListFlags.EXCLUDING_ID)) { - if (count > 1) - uid = new UID(uid.value + 1); - else if (count < 0) - uid = new UID(uid.value - 1); - } - - MessageSet msg_set; - if (count > 0) { - msg_set = (count == int.MAX) - ? new MessageSet.uid_range_to_highest(uid) - : new MessageSet.uid_range_by_count(uid, count); - } else if (count < 0) { - msg_set = (count != int.MIN) - ? new MessageSet.uid_range(new UID(1), uid) - : new MessageSet.uid_range_by_count(uid, count); - } else { - // count == 0 - msg_set = new MessageSet.uid(uid); - } - return yield mailbox.list_set_async(msg_set, fields, cancellable); } - public async Geary.Email fetch_email_async(Geary.EmailIdentifier id, - Geary.Email.Field fields, Cancellable? cancellable = null) throws Error { - if (mailbox == null) - throw new EngineError.OPEN_REQUIRED("%s not opened", to_string()); - - Gee.List? list = yield mailbox.list_set_async( - new MessageSet.uid(((Imap.EmailIdentifier) id).uid), fields, cancellable); - - if (list == null || list.size == 0) { - throw new EngineError.NOT_FOUND("Unable to fetch email %s from %s", id.to_string(), - to_string()); - } - - if (list.size != 1) { - throw new EngineError.BAD_RESPONSE("Too many responses (%d) from %s when fetching %s", - list.size, to_string(), id.to_string()); - } - - return list[0]; - } - - public async void remove_email_async(Gee.List email_ids, - Cancellable? cancellable = null) throws Error { + public async void remove_email_async(MessageSet msg_set, Cancellable? cancellable = null) throws Error { if (mailbox == null) throw new EngineError.OPEN_REQUIRED("%s not opened", to_string()); Gee.List flags = new Gee.ArrayList(); flags.add(MessageFlag.DELETED); - yield mailbox.mark_email_async(message_set_from_id_list(email_ids), flags, null, cancellable); + yield mailbox.mark_email_async(msg_set, flags, null, cancellable); yield mailbox.expunge_email_async(cancellable); } - public async void mark_email_async( - Gee.List to_mark, Geary.EmailFlags? flags_to_add, + public async void mark_email_async(MessageSet msg_set, Geary.EmailFlags? flags_to_add, Geary.EmailFlags? flags_to_remove, Cancellable? cancellable = null) throws Error { if (mailbox == null) throw new EngineError.OPEN_REQUIRED("%s not opened", to_string()); @@ -192,19 +127,7 @@ private class Geary.Imap.Folder : Object { MessageFlag.from_email_flags(flags_to_add, flags_to_remove, out msg_flags_add, out msg_flags_remove); - yield mailbox.mark_email_async(message_set_from_id_list(to_mark), msg_flags_add, - msg_flags_remove, cancellable); - } - - private MessageSet message_set_from_id_list(Gee.List list) { - Geary.Imap.UID[] sparse_set = new Geary.Imap.UID[list.size]; - int i = 0; - foreach(Geary.EmailIdentifier id in list) { - sparse_set[i] = ((Geary.Imap.EmailIdentifier) id).uid; - i++; - } - - return new MessageSet.uid_sparse(sparse_set); + yield mailbox.mark_email_async(msg_set, msg_flags_add, msg_flags_remove, cancellable); } public string to_string() { diff --git a/src/engine/imap/message/imap-message-set.vala b/src/engine/imap/message/imap-message-set.vala index 92b50f9c..910ab590 100644 --- a/src/engine/imap/message/imap-message-set.vala +++ b/src/engine/imap/message/imap-message-set.vala @@ -22,6 +22,10 @@ public class Geary.Imap.MessageSet { is_uid = true; } + public MessageSet.email_id(Geary.EmailIdentifier email_id) { + MessageSet.uid(((Geary.Imap.EmailIdentifier) email_id).uid); + } + public MessageSet.range(int low_msg_num, int count) { assert(low_msg_num > 0); assert(count > 0); @@ -92,6 +96,13 @@ public class Geary.Imap.MessageSet { is_uid = true; } + public MessageSet.email_id_collection(Gee.Collection email_ids) { + assert(email_ids.size > 0); + + value = build_sparse_range(email_id_collection_to_int64(email_ids)); + is_uid = true; + } + public MessageSet.sparse_to_highest(int[] msg_nums) { value = "%s:*".printf(build_sparse_range(msg_array_to_int64(msg_nums))); } @@ -169,6 +180,14 @@ public class Geary.Imap.MessageSet { return ret; } + private static int64[] email_id_collection_to_int64(Gee.Collection email_ids) { + int64[] ret = new int64[0]; + foreach (Geary.EmailIdentifier email_id in email_ids) + ret += ((Geary.Imap.EmailIdentifier) email_id).uid.value; + + return ret; + } + public Parameter to_parameter() { // Message sets are not quoted, even if they use an atom-special character (this *might* // be a Gmailism...) diff --git a/src/engine/impl/geary-email-flag-watcher.vala b/src/engine/impl/geary-email-flag-watcher.vala index b3dca881..e132608d 100755 --- a/src/engine/impl/geary-email-flag-watcher.vala +++ b/src/engine/impl/geary-email-flag-watcher.vala @@ -121,10 +121,11 @@ private class Geary.EmailFlagWatcher : Object { changed_map.set(e.id, e.properties.email_flags); } - debug("FlagWatcher: %d email flags changed in %s", changed_map.size, folder.to_string()); - - if (!cancellable.is_cancelled() && changed_map.size > 0) + if (!cancellable.is_cancelled() && changed_map.size > 0) { + debug("FlagWatcher: %d email flags changed in %s", changed_map.size, folder.to_string()); + email_flags_changed(changed_map); + } } } diff --git a/src/engine/impl/geary-generic-imap-folder.vala b/src/engine/impl/geary-generic-imap-folder.vala index 16823635..384a2d23 100644 --- a/src/engine/impl/geary-generic-imap-folder.vala +++ b/src/engine/impl/geary-generic-imap-folder.vala @@ -134,12 +134,9 @@ private class Geary.GenericImapFolder : Geary.AbstractFolder { // store all the new emails' UIDs and properties (primarily flags) in the local store, // to normalize the database against the remote folder if (uid_start_value < remote_properties.uid_next.value) { - Geary.Imap.EmailIdentifier uid_start = new Geary.Imap.EmailIdentifier( - new Geary.Imap.UID(uid_start_value)); - - Gee.List? newest = yield imap_remote_folder.list_email_by_id_async( - uid_start, int.MAX, Geary.Email.Field.PROPERTIES, Geary.Folder.ListFlags.NONE, - cancellable); + Gee.List? newest = yield imap_remote_folder.list_email_async( + new Imap.MessageSet.uid_range_to_highest(new Geary.Imap.UID(uid_start_value)), + Geary.Email.Field.PROPERTIES, cancellable); if (newest != null && newest.size > 0) { foreach (Geary.Email email in newest) @@ -186,8 +183,8 @@ private class Geary.GenericImapFolder : Geary.AbstractFolder { // 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 imap_remote_folder.list_email_by_id_async( - earliest_id, int.MAX, Geary.Email.Field.PROPERTIES, Geary.Folder.ListFlags.NONE, + Gee.List? old_remote = yield imap_remote_folder.list_email_async( + new Imap.MessageSet.uid_range_to_highest(earliest_uid), Geary.Email.Field.PROPERTIES, cancellable); int remote_length = (old_remote != null) ? old_remote.size : 0; @@ -461,9 +458,9 @@ private class Geary.GenericImapFolder : Geary.AbstractFolder { // normalize starting at the message *after* the highest position of the local store, // which has now changed - Gee.List? list = yield remote_folder.list_email_async(remote_count + 1, -1, - Geary.Sqlite.Folder.REQUIRED_FOR_DUPLICATE_DETECTION, Geary.Folder.ListFlags.NONE, - null); + Gee.List? list = yield remote_folder.list_email_async( + new Imap.MessageSet.range_to_highest(remote_count + 1), + Geary.Sqlite.Folder.REQUIRED_FOR_DUPLICATE_DETECTION, null); assert(list != null && list.size > 0); Gee.HashSet created = new Gee.HashSet( @@ -687,8 +684,8 @@ private class Geary.GenericImapFolder : Geary.AbstractFolder { } // pull from server - Gee.List? remote_list = yield remote_folder.list_email_sparse_async( - list, full_fields, Geary.Folder.ListFlags.NONE, cancellable); + Gee.List? remote_list = yield remote_folder.list_email_async( + new Imap.MessageSet.sparse(list), full_fields, cancellable); if (remote_list == null || remote_list.size == 0) break; @@ -774,9 +771,14 @@ private class Geary.GenericImapFolder : Geary.AbstractFolder { if (!yield wait_for_remote_to_open(cancellable)) throw new EngineError.SERVER_UNAVAILABLE("No connection to %s", remote.to_string()); - Geary.Email email = yield remote_folder.fetch_email_async(id, fields, cancellable); + Gee.List? list = yield remote_folder.list_email_async(new Imap.MessageSet.email_id(id), + fields, cancellable); + + if (list == null || list.size != 1) + throw new EngineError.NOT_FOUND("Unable to fetch %s in %s", id.to_string(), to_string()); // save to local store + Geary.Email email = list[0]; if (yield local_folder.create_email_async(email, cancellable)) notify_email_locally_appended(new Geary.Singleton(email.id)); @@ -848,9 +850,9 @@ private class Geary.GenericImapFolder : Geary.AbstractFolder { // Normalize the local folder by fetching EmailIdentifiers for all missing email as well // as fields for duplicate detection - Gee.List? list = yield remote_folder.list_email_async(high, prefetch_count, - Geary.Sqlite.Folder.REQUIRED_FOR_DUPLICATE_DETECTION, Geary.Folder.ListFlags.NONE, - cancellable); + Gee.List? list = yield remote_folder.list_email_async( + new Imap.MessageSet.range(high, prefetch_count), + Geary.Sqlite.Folder.REQUIRED_FOR_DUPLICATE_DETECTION, cancellable); if (list == null || list.size != prefetch_count) { throw new EngineError.BAD_PARAMETERS("Unable to prefetch %d email starting at %d in %s", count, low, to_string()); diff --git a/src/engine/impl/geary-send-replay-operations.vala b/src/engine/impl/geary-send-replay-operations.vala index 1be1c351..325d7404 100644 --- a/src/engine/impl/geary-send-replay-operations.vala +++ b/src/engine/impl/geary-send-replay-operations.vala @@ -39,8 +39,8 @@ private class Geary.MarkEmail : Geary.SendReplayOperation { } public override async bool replay_remote() throws Error { - yield engine.remote_folder.mark_email_async(to_mark, flags_to_add, flags_to_remove, - cancellable); + yield engine.remote_folder.mark_email_async(new Imap.MessageSet.email_id_collection(to_mark), + flags_to_add, flags_to_remove, cancellable); return true; } @@ -84,7 +84,8 @@ private class Geary.RemoveEmail : Geary.SendReplayOperation { // 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(to_remove, cancellable); + yield engine.remote_folder.remove_email_async(new Imap.MessageSet.email_id_collection(to_remove), + cancellable); return true; } @@ -327,152 +328,3 @@ private class Geary.ListEmailByID : Geary.ListEmail { } } -private class Geary.ListEmailSparse : Geary.SendReplayOperation { - private GenericImapFolder engine; - private int[] by_position; - private Geary.Email.Field required_fields; - private Gee.List? accumulator = null; - private weak EmailCallback? cb; - private Cancellable? cancellable; - private bool local_only; - - private int[] needed_by_position = new int[0]; - - public ListEmailSparse(GenericImapFolder engine, int[] by_position, Geary.Email.Field required_fields, - Gee.List? accumulator, EmailCallback? cb, Cancellable? cancellable, - bool local_only) { - base("ListEmailSparse"); - - this.engine = engine; - this.by_position = by_position; - this.required_fields = required_fields; - this.accumulator = accumulator; - this.cb = cb; - this.cancellable = cancellable; - this.local_only = local_only; - } - - public override async bool replay_local() throws Error { - int low, high; - Arrays.int_find_high_low(by_position, out low, out high); - - int local_count, local_offset; - if (!local_only) { - // normalize the position (ordering) of what's available locally with the situation on - // the server - yield engine.normalize_email_positions_async(low, high - low + 1, out local_count, - cancellable); - - local_offset = (engine.remote_count > local_count) ? (engine.remote_count - local_count - - 1) : 0; - } else { - local_count = yield engine.local_folder.get_email_count_async(cancellable); - local_offset = 0; - } - - // Fixup all the positions to match the local store's notions - if (local_offset > 0) { - int[] local_by_position = new int[by_position.length]; - for (int ctr = 0; ctr < by_position.length; ctr++) - local_by_position[ctr] = by_position[ctr] - local_offset; - - by_position = local_by_position; - } - - Gee.List? local_list = null; - try { - local_list = yield engine.local_folder.list_email_sparse_async(by_position, - required_fields, Folder.ListFlags.NONE, cancellable); - } catch (Error local_err) { - if (cb != null) - cb(null, local_err); - - throw local_err; - } - - int local_list_size = (local_list != null) ? local_list.size : 0; - - // reverse the process, fixing up all the returned messages to match the server's notions - if (local_list_size > 0 && local_offset > 0) { - foreach (Geary.Email email in local_list) - email.update_position(email.position + local_offset); - } - - if (local_list_size == by_position.length || local_only) { - if (accumulator != null) - accumulator.add_all(local_list); - - // report and signal finished - if (cb != null) { - cb(local_list, null); - cb(null, null); - } - - return true; - } - - // go through the list looking for anything not already in the sparse by_position list - // to fetch from the server; since by_position is not guaranteed to be sorted, the local - // list needs to be searched each iteration. - // - // TODO: Optimize this, especially if large lists/sparse sets are supplied - foreach (int position in by_position) { - bool found = false; - if (local_list != null) { - foreach (Geary.Email email2 in local_list) { - if (email2.position == position) { - found = true; - - break; - } - } - } - - if (!found) - needed_by_position += position; - } - - if (needed_by_position.length == 0) { - if (local_list != null && local_list.size > 0) { - if (accumulator != null) - accumulator.add_all(local_list); - - if (cb != null) - cb(local_list, null); - } - - // signal finished - if (cb != null) - cb(null, null); - - return true; - } - - return false; - } - - public override async bool replay_remote() throws Error { - Gee.List? remote_list = null; - try { - // if cb != null, it will be called by remote_list_email(), so don't call again with - // returned list - remote_list = yield engine.remote_list_email(needed_by_position, required_fields, cb, - cancellable); - } catch (Error remote_err) { - if (cb != null) - cb(null, remote_err); - - throw remote_err; - } - - if (accumulator != null && remote_list != null && remote_list.size > 0) - accumulator.add_all(remote_list); - - // signal finished - if (cb != null) - cb(null, null); - - return true; - } -} - diff --git a/src/engine/sqlite/api/sqlite-folder.vala b/src/engine/sqlite/api/sqlite-folder.vala index f5b3452b..0f8fde49 100644 --- a/src/engine/sqlite/api/sqlite-folder.vala +++ b/src/engine/sqlite/api/sqlite-folder.vala @@ -283,20 +283,6 @@ private class Geary.Sqlite.Folder : Object, Geary.ReferenceSemantics { return yield do_list_email_async(transaction, list, required_fields, true, cancellable); } - public async Gee.List? list_email_sparse_async(int[] by_position, - Geary.Email.Field required_fields, Geary.Folder.ListFlags flags, Cancellable? cancellable = null) - throws Error { - check_open(); - - Transaction transaction = yield db.begin_transaction_async("Folder.list_email_sparse_async", - cancellable); - - Gee.List? list = yield location_table.list_sparse_async(transaction, - folder_row.id, by_position, cancellable); - - return yield do_list_email_async(transaction, list, required_fields, false, cancellable); - } - public async Gee.List? list_email_by_id_async(Geary.EmailIdentifier initial_id, int count, Geary.Email.Field required_fields, Geary.Folder.ListFlags flags, Cancellable? cancellable = null) throws Error { diff --git a/src/engine/sqlite/email/sqlite-message-location-table.vala b/src/engine/sqlite/email/sqlite-message-location-table.vala index e23ca65a..940fb993 100644 --- a/src/engine/sqlite/email/sqlite-message-location-table.vala +++ b/src/engine/sqlite/email/sqlite-message-location-table.vala @@ -79,43 +79,6 @@ public class Geary.Sqlite.MessageLocationTable : Geary.Sqlite.Table { return list; } - /** - * All positions are one-based. - */ - public async Gee.List? list_sparse_async(Transaction? transaction, - int64 folder_id, int[] by_position, Cancellable? cancellable) throws Error { - Transaction locked = yield obtain_lock_async(transaction, "MessageLocationTable.list_sparse_async", - cancellable); - - // reuse the query for each iteration - SQLHeavy.Query query = locked.prepare( - "SELECT id, message_id, ordering FROM MessageLocationTable WHERE folder_id = ? " - + "AND remove_marker = 0 ORDER BY ordering LIMIT 1 OFFSET ?"); - - Gee.List list = new Gee.ArrayList(); - foreach (int position in by_position) { - check_cancel(cancellable, "list_sparse_async"); - - assert(position >= 1); - - query.bind_int64(0, folder_id); - query.bind_int(1, position - 1); - - SQLHeavy.QueryResult results = yield query.execute_async(); - check_cancel(cancellable, "list_sparse_async"); - - if (results.finished) - continue; - - list.add(new MessageLocationRow(this, results.fetch_int64(0), results.fetch_int64(1), - folder_id, results.fetch_int64(2), position)); - - query.clear(); - } - - return (list.size > 0) ? list : null; - } - public async Gee.List? list_ordering_async(Transaction? transaction, int64 folder_id, int64 low_ordering, int64 high_ordering, Cancellable? cancellable) throws Error {