From 7442caf88e7e013c0703edc82f8dcffcc8a1d10a Mon Sep 17 00:00:00 2001 From: Jim Nelson Date: Fri, 29 Jul 2011 20:10:43 -0700 Subject: [PATCH] Detect deleted (or moved) messages in open folder: #3793 This commit finishes the second half of #3793 by detecting when messages have been deleted (or moved out of) an open folder and notifying the system of the change. The nonblocking synchronization primitives have been beefed up and may be broken out to a separate library some day. This commit also introduces the ReplayQueue, which replays events that occur on the server locally. This class will probably become much more important as time goes on, perhaps used to store user events that are replayed on the server as well. --- src/client/ui/main-window.vala | 9 +- src/client/ui/message-list-store.vala | 28 ++++ src/engine/api/geary-abstract-folder.vala | 10 +- src/engine/api/geary-email-location.vala | 38 +++++- src/engine/api/geary-engine-error.vala | 3 +- src/engine/api/geary-engine-folder.vala | 128 ++++++++++++++---- src/engine/api/geary-folder.vala | 30 ++-- src/engine/api/geary-generic-imap-folder.vala | 6 +- src/engine/api/geary-replay-queue.vala | 83 ++++++++++++ src/engine/imap/api/imap-email-location.vala | 4 +- src/engine/imap/api/imap-folder.vala | 28 ++-- src/engine/imap/transport/imap-mailbox.vala | 74 ++++++---- ...la => nonblocking-abstract-semaphore.vala} | 22 ++- .../nonblocking/nonblocking-mailbox.vala | 2 +- src/engine/nonblocking/nonblocking-mutex.vala | 2 +- .../nonblocking/nonblocking-variants.vala | 24 ++++ src/engine/sqlite/abstract/sqlite-row.vala | 2 +- src/engine/sqlite/api/sqlite-folder.vala | 33 +++-- .../email/sqlite-message-location-row.vala | 28 +++- .../email/sqlite-message-location-table.vala | 45 +++++- src/wscript | 4 +- 21 files changed, 485 insertions(+), 118 deletions(-) create mode 100644 src/engine/api/geary-replay-queue.vala rename src/engine/nonblocking/{nonblocking-semaphore.vala => nonblocking-abstract-semaphore.vala} (82%) create mode 100644 src/engine/nonblocking/nonblocking-variants.vala diff --git a/src/client/ui/main-window.vala b/src/client/ui/main-window.vala index a0ba7134..9fd19c17 100644 --- a/src/client/ui/main-window.vala +++ b/src/client/ui/main-window.vala @@ -205,12 +205,12 @@ public class MainWindow : Gtk.Window { message_list_store.clear(); if (current_folder != null) { - current_folder.list_appended.disconnect(on_folder_list_appended); + current_folder.messages_appended.disconnect(on_folder_messages_appended); yield current_folder.close_async(); } current_folder = folder; - current_folder.list_appended.connect(on_folder_list_appended); + current_folder.messages_appended.connect(on_folder_messages_appended); yield current_folder.open_async(true); @@ -287,7 +287,7 @@ public class MainWindow : Gtk.Window { } } - private void on_folder_list_appended() { + private void on_folder_messages_appended() { int high = message_list_store.get_highest_folder_position(); if (high < 0) { debug("Unable to find highest message position in %s", current_folder.to_string()); @@ -295,6 +295,9 @@ public class MainWindow : Gtk.Window { return; } + debug("Message(s) appended to %s, fetching email at %d and above", current_folder.to_string(), + high + 1); + // Want to get the one *after* the highest position in the message list current_folder.lazy_list_email(high + 1, -1, MessageListStore.REQUIRED_FIELDS, on_list_email_ready); diff --git a/src/client/ui/message-list-store.vala b/src/client/ui/message-list-store.vala index bf892056..d70ca22f 100644 --- a/src/client/ui/message-list-store.vala +++ b/src/client/ui/message-list-store.vala @@ -83,6 +83,8 @@ public class MessageListStore : Gtk.TreeStore { Column.SUBJECT, subject, Column.MESSAGE_OBJECT, envelope ); + + envelope.location.position_deleted.connect(on_email_position_deleted); } public Geary.Email? get_message_at(Gtk.TreePath path) { @@ -117,6 +119,27 @@ public class MessageListStore : Gtk.TreeStore { return high; } + private bool remove_at_position(int position) { + Gtk.TreeIter iter; + if (!get_iter_first(out iter)) + return false; + + do { + Geary.Email email; + get(iter, Column.MESSAGE_OBJECT, out email); + + if (email.location.position == position) { + remove(iter); + + email.location.position_deleted.disconnect(on_email_position_deleted); + + return true; + } + } while (iter_next(ref iter)); + + return false; + } + private int sort_by_date(Gtk.TreeModel model, Gtk.TreeIter aiter, Gtk.TreeIter biter) { Geary.Email aenvelope; get(aiter, Column.MESSAGE_OBJECT, out aenvelope); @@ -132,6 +155,11 @@ public class MessageListStore : Gtk.TreeStore { return aenvelope.location.position - benvelope.location.position; } + private void on_email_position_deleted(int position) { + if (!remove_at_position(position)) + debug("on_email_position_deleted: unable to find email at position %d", position); + } + private static string to_markup(string str, string? pre = null, string? post = null) { return "%s%s%s".printf( (pre != null) ? pre : "", diff --git a/src/engine/api/geary-abstract-folder.vala b/src/engine/api/geary-abstract-folder.vala index 65b3794d..2e5c9128 100644 --- a/src/engine/api/geary-abstract-folder.vala +++ b/src/engine/api/geary-abstract-folder.vala @@ -13,8 +13,12 @@ public abstract class Geary.AbstractFolder : Object, Geary.Folder { closed(reason); } - protected virtual void notify_list_appended(int total) { - list_appended(total); + protected virtual void notify_messages_appended(int total) { + messages_appended(total); + } + + protected virtual void notify_message_removed(int position, int total) { + message_removed(position, total); } public abstract Geary.FolderPath get_path(); @@ -79,7 +83,7 @@ public abstract class Geary.AbstractFolder : Object, Geary.Folder { public abstract async Geary.Email fetch_email_async(Geary.EmailIdentifier id, Geary.Email.Field required_fields, Cancellable? cancellable = null) throws Error; - public abstract async void remove_email_async(Geary.Email email, Cancellable? cancellable = null) + public abstract async void remove_email_async(int position, Cancellable? cancellable = null) throws Error; public virtual string to_string() { diff --git a/src/engine/api/geary-email-location.vala b/src/engine/api/geary-email-location.vala index 643762d1..9366045c 100644 --- a/src/engine/api/geary-email-location.vala +++ b/src/engine/api/geary-email-location.vala @@ -8,9 +8,45 @@ public class Geary.EmailLocation : Object { public int position { get; private set; } public int64 ordering { get; private set; } - public EmailLocation(int position, int64 ordering) { + private weak Geary.Folder folder; + + public signal void position_altered(int old_position, int new_position); + + public signal void position_deleted(int position); + + public EmailLocation(Geary.Folder folder, int position, int64 ordering) { + assert(position >= 1); + + this.folder = folder; this.position = position; this.ordering = ordering; + + folder.message_removed.connect(on_message_removed); + } + + ~EmailLocation() { + folder.message_removed.disconnect(on_message_removed); + } + + private void on_message_removed(int position, int total) { + // if the removed position is greater than this one's, no change in this position + if (this.position < position) + return; + + // if the same, can't adjust (adjust it to what?), but notify that this EmailLocation has + // been removed + if (this.position == position) { + position_deleted(position); + + return; + } + + // adjust this position downward + int old_position = this.position; + this.position--; + assert(this.position >= 1); + + position_altered(old_position, this.position); } } diff --git a/src/engine/api/geary-engine-error.vala b/src/engine/api/geary-engine-error.vala index cb658a8f..fd5ea968 100644 --- a/src/engine/api/geary-engine-error.vala +++ b/src/engine/api/geary-engine-error.vala @@ -12,6 +12,7 @@ public errordomain Geary.EngineError { READONLY, BAD_PARAMETERS, INCOMPLETE_MESSAGE, - SERVER_UNAVAILABLE + SERVER_UNAVAILABLE, + CLOSED } diff --git a/src/engine/api/geary-engine-folder.vala b/src/engine/api/geary-engine-folder.vala index 2882f9b0..7dbe863c 100644 --- a/src/engine/api/geary-engine-folder.vala +++ b/src/engine/api/geary-engine-folder.vala @@ -7,13 +7,48 @@ private class Geary.EngineFolder : Geary.AbstractFolder { private const int REMOTE_FETCH_CHUNK_COUNT = 10; + private class ReplayAppend : ReplayOperation { + public EngineFolder owner; + public int new_remote_count; + + public ReplayAppend(EngineFolder owner, int new_remote_count) { + base ("Append"); + + this.owner = owner; + this.new_remote_count = new_remote_count; + } + + public override async void replay() { + yield owner.do_replay_appended_messages(new_remote_count); + } + } + + private class ReplayRemoval : ReplayOperation { + public EngineFolder owner; + public int position; + public int new_remote_count; + + public ReplayRemoval(EngineFolder owner, int position, int new_remote_count) { + base ("Removal"); + + this.owner = owner; + this.position = position; + this.new_remote_count = new_remote_count; + } + + public override async void replay() { + yield owner.do_replay_remove_message(position, new_remote_count); + } + } + private RemoteAccount remote; private LocalAccount local; private LocalFolder local_folder; private RemoteFolder? remote_folder = null; private int remote_count = -1; private bool opened = false; - private Geary.NonblockingSemaphore remote_semaphore = new Geary.NonblockingSemaphore(true); + private NonblockingSemaphore remote_semaphore = new NonblockingSemaphore(); + private ReplayQueue? replay_queue = null; public EngineFolder(RemoteAccount remote, LocalAccount local, LocalFolder local_folder) { this.remote = remote; @@ -87,13 +122,17 @@ private class Geary.EngineFolder : Geary.AbstractFolder { yield local.update_folder_async(folder, cancellable); // signals - folder.list_appended.connect(on_remote_list_appended); + folder.messages_appended.connect(on_remote_messages_appended); + folder.message_removed.connect(on_remote_message_removed); // state remote_count = yield folder.get_email_count_async(cancellable); // all set; bless the remote folder as opened remote_folder = folder; + + // start the replay queue + replay_queue = new ReplayQueue(); } else { debug("Unable to prepare remote folder %s: prepare_opened_file() failed", to_string()); } @@ -129,31 +168,40 @@ private class Geary.EngineFolder : Geary.AbstractFolder { // this method to complete (much like open_async()) if (remote_folder != null) { yield remote_semaphore.wait_async(); - remote_semaphore = new Geary.NonblockingSemaphore(true); + remote_semaphore = new Geary.NonblockingSemaphore(); RemoteFolder? folder = remote_folder; remote_folder = null; // signals - folder.list_appended.disconnect(on_remote_list_appended); + folder.messages_appended.disconnect(on_remote_messages_appended); + folder.message_removed.disconnect(on_remote_message_removed); folder.close_async.begin(cancellable); + // close the replay queue *after* the folder has been closed (in case any final upcalls + // come and can be handled) + yield replay_queue.close_async(); + replay_queue = null; + notify_closed(CloseReason.FOLDER_CLOSED); } opened = false; } - private void on_remote_list_appended(int total) { - // need to prefetch PROPERTIES (or, in the future NONE or LOCATION) fields to create a - // normalized placeholder in the local database of the message, so all positions are - // properly relative to the end of the message list; once this is done, notify user of new - // messages - do_normalize_appended_messages.begin(total); + private void on_remote_messages_appended(int total) { + debug("on_remote_messages_appended: total=%d", total); + replay_queue.schedule(new ReplayAppend(this, total)); } - private async void do_normalize_appended_messages(int new_remote_count) { + // Need to prefetch PROPERTIES (or, in the future NONE or LOCATION) fields to create a + // normalized placeholder in the local database of the message, so all positions are + // properly relative to the end of the message list; once this is done, notify user of new + // messages. + // + // This MUST only be called from ReplayAppend. + private async void do_replay_appended_messages(int new_remote_count) { // this only works when the list is grown assert(new_remote_count > remote_count); @@ -161,13 +209,13 @@ private class Geary.EngineFolder : Geary.AbstractFolder { // if no mail in local store, nothing needs to be done here; the store is "normalized" int local_count = yield local_folder.get_email_count_async(); if (local_count == 0) { - notify_list_appended(new_remote_count); + notify_messages_appended(new_remote_count); return; } if (!yield wait_for_remote_to_open()) { - notify_list_appended(new_remote_count); + notify_messages_appended(new_remote_count); return; } @@ -184,13 +232,47 @@ private class Geary.EngineFolder : Geary.AbstractFolder { // save new remote count remote_count = new_remote_count; - notify_list_appended(new_remote_count); + notify_messages_appended(new_remote_count); } catch (Error err) { debug("Unable to normalize local store of newly appended messages to %s: %s", to_string(), err.message); } } + private void on_remote_message_removed(int position, int total) { + debug("on_remote_message_removed: position=%d total=%d", position, total); + replay_queue.schedule(new ReplayRemoval(this, position, total)); + } + + // This MUST only be called from ReplayRemoval. + private async void do_replay_remove_message(int remote_position, int new_remote_count) { + try { + // calculate the local position of the message in the local store + int local_count = yield local_folder.get_email_count_async(); + int local_low = ((remote_count - local_count) + 1).clamp(1, remote_count); + + if (remote_position < local_low) { + debug("do_replay_remove_message: Not removing message at %d from local store, not present", + remote_position); + + remote_count = new_remote_count; + + return; + } + + // Adjust remote position to local position + yield local_folder.remove_email_async((remote_position - local_low) + 1); + + // save new remote count + remote_count = new_remote_count; + + notify_message_removed(remote_position, new_remote_count); + } catch (Error err) { + debug("Unable to remove message #%d from %s: %s", remote_position, to_string(), + err.message); + } + } + public override async int get_email_count_async(Cancellable? cancellable = null) throws Error { // TODO: Use monitoring to avoid round-trip to the server if (!opened) @@ -291,8 +373,8 @@ private class Geary.EngineFolder : Geary.AbstractFolder { if (local_list_size > 0 && local_count < remote_count) { int adjustment = remote_count - local_count; foreach (Geary.Email email in local_list) { - email.update_location(new Geary.EmailLocation(email.location.position + adjustment, - email.location.ordering)); + email.update_location(new Geary.EmailLocation(this, + email.location.position + adjustment, email.location.ordering)); } } @@ -412,7 +494,8 @@ private class Geary.EngineFolder : Geary.AbstractFolder { if (local_list_size > 0) { foreach (Geary.Email email in local_list) { int new_position = email.location.position + local_offset; - email.update_location(new Geary.EmailLocation(new_position, email.location.ordering)); + email.update_location(new Geary.EmailLocation(this, new_position, + email.location.ordering)); } } @@ -599,18 +682,13 @@ private class Geary.EngineFolder : Geary.AbstractFolder { return email; } - public override async void remove_email_async(Geary.Email email, Cancellable? cancellable = null) + public override async void remove_email_async(int position, Cancellable? cancellable = null) throws Error { if (!opened) throw new EngineError.OPEN_REQUIRED("Folder %s not opened", to_string()); - if (remote_folder == null) { - throw new EngineError.READONLY("Unable to delete from %s: remote unavailable", - to_string()); - } - - yield remote_folder.remove_email_async(email, cancellable); - yield local_folder.remove_email_async(email, cancellable); + // TODO: + throw new EngineError.READONLY("EngineFolder currently cannot remove email"); } // In order to maintain positions for all messages without storing all of them locally, diff --git a/src/engine/api/geary-folder.vala b/src/engine/api/geary-folder.vala index ac34d0e2..5c285fa1 100644 --- a/src/engine/api/geary-folder.vala +++ b/src/engine/api/geary-folder.vala @@ -41,11 +41,18 @@ public interface Geary.Folder : Object { public signal void closed(CloseReason reason); /** - * "list-appended" is fired when new messages have been appended to the list of messages in the - * folder (and therefore old message position numbers remain valid, but the total count of the - * messages in the folder has changed). + * "messages-appended" is fired when new messages have been appended to the list of messages in + * the folder (and therefore old message position numbers remain valid, but the total count of + * the messages in the folder has changed). */ - public signal void list_appended(int total); + public signal void messages_appended(int total); + + /** + * "message-removed" is fired when a message has been removed (deleted or moved) from the + * folder (and therefore old message position numbers may no longer be valid, i.e. those after + * the removed message). + */ + public signal void message_removed(int position, int total); /** * This helper method should be called by implementors of Folder rather than firing the signal @@ -66,7 +73,14 @@ public interface Geary.Folder : Object { * directly. This allows subclasses and superclasses the opportunity to inspect the email * and update state before and/or after the signal has been fired. */ - protected abstract void notify_list_appended(int total); + protected abstract void notify_messages_appended(int total); + + /** + * This helper method should be called by implementors of Folder rather than firing the signal + * directly. This allows subclasses and superclasses the opportunity to inspect the email + * and update state before and/or after the signal has been fired. + */ + protected abstract void notify_message_removed(int position, int total); public abstract Geary.FolderPath get_path(); @@ -212,12 +226,12 @@ public interface Geary.Folder : Object { Geary.Email.Field required_fields, Cancellable? cancellable = null) throws Error; /** - * Removes the email from the folder, determined by its EmailLocation. If the email location - * is invalid for any reason, EngineError.NOT_FOUND is thrown. + * Removes the email at the supplied position from the folder. If the email position is + * invalid for any reason, EngineError.NOT_FOUND is thrown. * * The Folder must be opened prior to attempting this operation. */ - public abstract async void remove_email_async(Geary.Email email, Cancellable? cancellable = null) + public abstract async void remove_email_async(int position, Cancellable? cancellable = null) throws Error; /** diff --git a/src/engine/api/geary-generic-imap-folder.vala b/src/engine/api/geary-generic-imap-folder.vala index fc3fd0fb..c92b2992 100644 --- a/src/engine/api/geary-generic-imap-folder.vala +++ b/src/engine/api/geary-generic-imap-folder.vala @@ -163,7 +163,8 @@ private class Geary.GenericImapFolder : Geary.EngineFolder { // local's email on the server has been removed, remove locally try { - yield local_folder.remove_email_async(old_local[local_ctr], cancellable); + yield local_folder.remove_email_async(old_local[local_ctr].location.position, + cancellable); } catch (Error remove_err) { debug("Unable to remove discarded email from %s: %s", to_string(), remove_err.message); @@ -185,7 +186,8 @@ private class Geary.GenericImapFolder : Geary.EngineFolder { // remove anything left over for (; local_ctr < local_length; local_ctr++) { try { - yield local_folder.remove_email_async(old_local[local_ctr], cancellable); + yield local_folder.remove_email_async(old_local[local_ctr].location.position, + cancellable); } catch (Error discard_err) { debug("Unable to discard email from %s: %s", to_string(), discard_err.message); } diff --git a/src/engine/api/geary-replay-queue.vala b/src/engine/api/geary-replay-queue.vala new file mode 100644 index 00000000..fcc3e0be --- /dev/null +++ b/src/engine/api/geary-replay-queue.vala @@ -0,0 +1,83 @@ +/* Copyright 2011 Yorba Foundation + * + * This software is licensed under the GNU Lesser General Public License + * (version 2.1 or later). See the COPYING file in this distribution. + */ + +private abstract class Geary.ReplayOperation { + private string name; + + public ReplayOperation(string name) { + this.name = name; + } + + public abstract async void replay(); +} + +private class Geary.ReplayQueue { + private class ReplayClose : ReplayOperation { + public NonblockingSemaphore semaphore = new NonblockingSemaphore(); + + public ReplayClose() { + base ("Close"); + } + + public override async void replay() { + try { + semaphore.notify(); + } catch (Error err) { + error("Unable to notify that replay queue is closed: %s", err.message); + } + } + } + + private NonblockingMailbox queue = new NonblockingMailbox(); + private bool closed = false; + + public ReplayQueue() { + do_process_queue.begin(); + } + + public void schedule(ReplayOperation op) { + try { + queue.send(op); + } catch (Error err) { + error("Unable to schedule operation on replay queue: %s", err.message); + } + } + + public async void close_async() throws EngineError { + if (closed) + throw new EngineError.CLOSED("Closed"); + + closed = true; + + // flush a ReplayClose operation down the pipe so all enqueued operations complete + ReplayClose replay_close = new ReplayClose(); + schedule(replay_close); + try { + yield replay_close.semaphore.wait_async(); + } catch (Error err) { + error("Error waiting for replay queue to close: %s", err.message); + } + } + + private async void do_process_queue() { + for (;;) { + // if queue is empty and the ReplayQueue is closed, bail out; ReplayQueue cannot be + // restarted + if (queue.size == 0 && closed) + break; + + ReplayOperation op; + try { + op = yield queue.recv_async(); + } catch (Error err) { + error("Unable to receive next replay operation on queue: %s", err.message); + } + + yield op.replay(); + } + } +} + diff --git a/src/engine/imap/api/imap-email-location.vala b/src/engine/imap/api/imap-email-location.vala index bc6f4526..097bc719 100644 --- a/src/engine/imap/api/imap-email-location.vala +++ b/src/engine/imap/api/imap-email-location.vala @@ -9,8 +9,8 @@ */ private class Geary.Imap.EmailLocation : Geary.EmailLocation { - public EmailLocation(int position, Geary.Imap.UID uid) { - base (position, uid.value); + public EmailLocation(Geary.Folder folder, int position, Geary.Imap.UID uid) { + base (folder, position, uid.value); } } diff --git a/src/engine/imap/api/imap-folder.vala b/src/engine/imap/api/imap-folder.vala index 2f815e59..6a583ecd 100644 --- a/src/engine/imap/api/imap-folder.vala +++ b/src/engine/imap/api/imap-folder.vala @@ -74,9 +74,14 @@ public class Geary.Imap.Folder : Geary.AbstractFolder, Geary.RemoteFolder, Geary notify_closed(CloseReason.FOLDER_CLOSED); } - private void on_exists_altered(int exists) { + private void on_exists_altered(int old_exists, int new_exists) { assert(mailbox != null); - notify_list_appended(exists); + assert(old_exists != new_exists); + + // only use this signal to notify of additions; removals are handled with the expunged + // signal + if (new_exists > old_exists) + notify_messages_appended(new_exists); } private void on_flags_altered(FetchResults flags) { @@ -84,9 +89,10 @@ public class Geary.Imap.Folder : Geary.AbstractFolder, Geary.RemoteFolder, Geary // TODO: Notify of changes } - private void on_expunged(MessageNumber expunged) { + private void on_expunged(MessageNumber expunged, int total) { assert(mailbox != null); - // TODO: Notify of changes + + notify_message_removed(expunged.value, total); } public override async int get_email_count_async(Cancellable? cancellable = null) throws Error { @@ -110,7 +116,7 @@ public class Geary.Imap.Folder : Geary.AbstractFolder, Geary.RemoteFolder, Geary normalize_span_specifiers(ref low, ref count, mailbox.exists); - return yield mailbox.list_set_async(new MessageSet.range(low, count), fields, cancellable); + return yield mailbox.list_set_async(this, new MessageSet.range(low, count), fields, cancellable); } public override async Gee.List? list_email_sparse_async(int[] by_position, @@ -118,7 +124,7 @@ public class Geary.Imap.Folder : Geary.AbstractFolder, Geary.RemoteFolder, Geary 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); + return yield mailbox.list_set_async(this, new MessageSet.sparse(by_position), fields, cancellable); } public async Gee.List? list_email_uid_async(Geary.Imap.UID? low, @@ -130,7 +136,7 @@ public class Geary.Imap.Folder : Geary.AbstractFolder, Geary.RemoteFolder, Geary ? new MessageSet.uid_range((low != null) ? low : new Geary.Imap.UID(1), high) : new MessageSet.uid_range_to_highest(low); - return yield mailbox.list_set_async(msg_set, fields, cancellable); + return yield mailbox.list_set_async(this, msg_set, fields, cancellable); } public override async Geary.Email fetch_email_async(Geary.EmailIdentifier id, @@ -140,18 +146,14 @@ public class Geary.Imap.Folder : Geary.AbstractFolder, Geary.RemoteFolder, Geary // TODO: If position out of range, throw EngineError.NOT_FOUND - return yield mailbox.fetch_async(((Imap.EmailIdentifier) id).uid, fields, cancellable); + return yield mailbox.fetch_async(this, ((Imap.EmailIdentifier) id).uid, fields, cancellable); } - public override async void remove_email_async(Geary.Email email, Cancellable? cancellable = null) + public override async void remove_email_async(int position, Cancellable? cancellable = null) throws Error { if (mailbox == null) throw new EngineError.OPEN_REQUIRED("%s not opened", to_string()); - Geary.Imap.UID? uid = ((Geary.Imap.EmailIdentifier) email.id).uid; - if (uid == null) - throw new EngineError.NOT_FOUND("Removing email requires UID"); - throw new EngineError.READONLY("IMAP currently read-only"); } } diff --git a/src/engine/imap/transport/imap-mailbox.vala b/src/engine/imap/transport/imap-mailbox.vala index 653c1897..b081bcd4 100644 --- a/src/engine/imap/transport/imap-mailbox.vala +++ b/src/engine/imap/transport/imap-mailbox.vala @@ -15,13 +15,13 @@ public class Geary.Imap.Mailbox : Geary.SmartReference { private SelectedContext context; - public signal void exists_altered(int exists); + public signal void exists_altered(int old_exists, int new_exists); public signal void recent_altered(int recent); public signal void flags_altered(FetchResults flags); - public signal void expunged(MessageNumber msg_num); + public signal void expunged(MessageNumber msg_num, int total); public signal void closed(); @@ -34,23 +34,23 @@ public class Geary.Imap.Mailbox : Geary.SmartReference { context.closed.connect(on_closed); context.disconnected.connect(on_disconnected); - context.unsolicited_exists.connect(on_unsolicited_exists); - context.unsolicited_expunged.connect(on_unsolicited_expunged); - context.unsolicited_flags.connect(on_unsolicited_flags); - context.unsolicited_recent.connect(on_unsolicited_recent); + context.exists_altered.connect(on_exists_altered); + context.expunged.connect(on_expunged); + context.flags_altered.connect(on_flags_altered); + context.recent_altered.connect(on_recent_altered); } ~Mailbox() { context.closed.disconnect(on_closed); context.disconnected.disconnect(on_disconnected); - context.session.unsolicited_exists.disconnect(on_unsolicited_exists); - context.session.unsolicited_expunged.disconnect(on_unsolicited_expunged); - context.session.unsolicited_flags.disconnect(on_unsolicited_flags); - context.session.unsolicited_recent.disconnect(on_unsolicited_recent); + context.exists_altered.disconnect(on_exists_altered); + context.expunged.disconnect(on_expunged); + context.flags_altered.disconnect(on_flags_altered); + context.recent_altered.disconnect(on_recent_altered); } - public async Gee.List? list_set_async(MessageSet msg_set, Geary.Email.Field fields, - Cancellable? cancellable = null) throws Error { + public async Gee.List? list_set_async(Geary.Folder folder, MessageSet msg_set, + Geary.Email.Field fields, Cancellable? cancellable = null) throws Error { if (context.is_closed()) throw new ImapError.NOT_SELECTED("Mailbox %s closed", name); @@ -78,7 +78,7 @@ public class Geary.Imap.Mailbox : Geary.SmartReference { // see fields_to_fetch_data_types() for why this is guaranteed assert(uid != null); - Geary.Email email = new Geary.Email(new Geary.Imap.EmailLocation(res.msg_num, uid), + Geary.Email email = new Geary.Email(new Geary.Imap.EmailLocation(folder, res.msg_num, uid), new Geary.Imap.EmailIdentifier(uid)); fetch_results_to_email(res, fields, email); @@ -88,7 +88,7 @@ public class Geary.Imap.Mailbox : Geary.SmartReference { return (msgs != null && msgs.size > 0) ? msgs : null; } - public async Geary.Email fetch_async(Geary.Imap.UID uid, Geary.Email.Field fields, + public async Geary.Email fetch_async(Geary.Folder folder, Geary.Imap.UID uid, Geary.Email.Field fields, Cancellable? cancellable = null) throws Error { if (context.is_closed()) throw new ImapError.NOT_SELECTED("Mailbox %s closed", name); @@ -113,7 +113,7 @@ public class Geary.Imap.Mailbox : Geary.SmartReference { if (results.length != 1) throw new ImapError.SERVER_ERROR("Too many responses from server: %d", results.length); - Geary.Email email = new Geary.Email(new Geary.Imap.EmailLocation(results[0].msg_num, uid), + Geary.Email email = new Geary.Email(new Geary.Imap.EmailLocation(folder, results[0].msg_num, uid), new Geary.Imap.EmailIdentifier(uid)); fetch_results_to_email(results[0], fields, email); @@ -128,19 +128,19 @@ public class Geary.Imap.Mailbox : Geary.SmartReference { disconnected(local); } - private void on_unsolicited_exists(int exists) { - exists_altered(exists); + private void on_exists_altered(int old_exists, int new_exists) { + exists_altered(old_exists, new_exists); } - private void on_unsolicited_recent(int recent) { + private void on_recent_altered(int recent) { recent_altered(recent); } - private void on_unsolicited_expunged(MessageNumber msg_num) { - expunged(msg_num); + private void on_expunged(MessageNumber msg_num, int total) { + expunged(msg_num, total); } - private void on_unsolicited_flags(FetchResults flags) { + private void on_flags_altered(FetchResults flags) { flags_altered(flags); } @@ -276,13 +276,13 @@ private class Geary.Imap.SelectedContext : Object, Geary.ReferenceSemantics { protected int manual_ref_count { get; protected set; } - public signal void unsolicited_exists(int exists); + public signal void exists_altered(int old_exists, int new_exists); - public signal void unsolicited_recent(int recent); + public signal void recent_altered(int recent); - public signal void unsolicited_expunged(MessageNumber expunged); + public signal void expunged(MessageNumber msg_num, int total); - public signal void unsolicited_flags(FetchResults flags); + public signal void flags_altered(FetchResults flags); public signal void closed(); @@ -325,21 +325,35 @@ private class Geary.Imap.SelectedContext : Object, Geary.ReferenceSemantics { } private void on_unsolicited_exists(int exists) { + // only report if changed; note that on_solicited_expunged also fires this signal + if (this.exists == exists) + return; + + int old_exists = this.exists; this.exists = exists; - unsolicited_exists(exists); + + exists_altered(old_exists, this.exists); } private void on_unsolicited_recent(int recent) { this.recent = recent; - unsolicited_recent(recent); + + recent_altered(recent); } - private void on_unsolicited_expunged(MessageNumber expunged) { - unsolicited_expunged(expunged); + private void on_unsolicited_expunged(MessageNumber msg_num) { + assert(exists > 0); + + // update exists count along with reporting the deletion + int old_exists = exists; + exists--; + + exists_altered(old_exists, exists); + expunged(msg_num, exists); } private void on_unsolicited_flags(FetchResults results) { - unsolicited_flags(results); + flags_altered(results); } private void on_session_mailbox_changed(string? old_mailbox, string? new_mailbox, bool readonly) { diff --git a/src/engine/nonblocking/nonblocking-semaphore.vala b/src/engine/nonblocking/nonblocking-abstract-semaphore.vala similarity index 82% rename from src/engine/nonblocking/nonblocking-semaphore.vala rename to src/engine/nonblocking/nonblocking-abstract-semaphore.vala index 10cf7ba4..bc2c983a 100644 --- a/src/engine/nonblocking/nonblocking-semaphore.vala +++ b/src/engine/nonblocking/nonblocking-abstract-semaphore.vala @@ -4,10 +4,11 @@ * (version 2.1 or later). See the COPYING file in this distribution. */ -public class Geary.NonblockingSemaphore { +public abstract class Geary.NonblockingAbstractSemaphore { private class Pending { public SourceFunc cb; public Cancellable? cancellable; + public bool passed = false; public signal void cancelled(); @@ -30,19 +31,21 @@ public class Geary.NonblockingSemaphore { } private bool broadcast; + private bool autoreset; private Cancellable? cancellable; private bool passed = false; private Gee.List pending_queue = new Gee.LinkedList(); - public NonblockingSemaphore(bool broadcast, Cancellable? cancellable = null) { + protected NonblockingAbstractSemaphore(bool broadcast, bool autoreset, Cancellable? cancellable = null) { this.broadcast = broadcast; + this.autoreset = autoreset; this.cancellable = cancellable; if (cancellable != null) cancellable.cancelled.connect(on_cancelled); } - ~NonblockingSemaphore() { + ~NonblockingAbstractSemaphore() { if (pending_queue.size > 0) warning("Nonblocking semaphore destroyed with %d pending callers", pending_queue.size); } @@ -51,13 +54,18 @@ public class Geary.NonblockingSemaphore { if (pending_queue.size == 0) return; + // in both cases, mark the Pending object(s) as passed in case this is an auto-reset + // semaphore if (all) { - foreach (Pending pending in pending_queue) + foreach (Pending pending in pending_queue) { + pending.passed = passed; Idle.add(pending.cb); + } pending_queue.clear(); } else { Pending pending = pending_queue.remove_at(0); + pending.passed = passed; Idle.add(pending.cb); } } @@ -68,6 +76,9 @@ public class Geary.NonblockingSemaphore { passed = true; trigger(broadcast); + + if (autoreset) + reset(); } // TODO: Allow the caller to pass their own cancellable in if they want to be able to cancel @@ -87,6 +98,9 @@ public class Geary.NonblockingSemaphore { yield; pending.cancelled.disconnect(on_pending_cancelled); + + if (pending.passed) + return; } } diff --git a/src/engine/nonblocking/nonblocking-mailbox.vala b/src/engine/nonblocking/nonblocking-mailbox.vala index 72a12b35..4206168e 100644 --- a/src/engine/nonblocking/nonblocking-mailbox.vala +++ b/src/engine/nonblocking/nonblocking-mailbox.vala @@ -8,7 +8,7 @@ public class Geary.NonblockingMailbox : Object { public int size { get { return queue.size; } } private Gee.List queue; - private NonblockingSemaphore spinlock = new NonblockingSemaphore(false); + private NonblockingSpinlock spinlock = new NonblockingSpinlock(); public NonblockingMailbox() { queue = new Gee.LinkedList(); diff --git a/src/engine/nonblocking/nonblocking-mutex.vala b/src/engine/nonblocking/nonblocking-mutex.vala index 9cf9dc93..7286cd8a 100644 --- a/src/engine/nonblocking/nonblocking-mutex.vala +++ b/src/engine/nonblocking/nonblocking-mutex.vala @@ -5,7 +5,7 @@ */ public class Geary.NonblockingMutex { - private NonblockingSemaphore spinlock = new NonblockingSemaphore(false); + private NonblockingSpinlock spinlock = new NonblockingSpinlock(); private bool locked = false; private int next_token = 0; private int locked_token = -1; diff --git a/src/engine/nonblocking/nonblocking-variants.vala b/src/engine/nonblocking/nonblocking-variants.vala new file mode 100644 index 00000000..f84ab902 --- /dev/null +++ b/src/engine/nonblocking/nonblocking-variants.vala @@ -0,0 +1,24 @@ +/* Copyright 2011 Yorba Foundation + * + * This software is licensed under the GNU Lesser General Public License + * (version 2.1 or later). See the COPYING file in this distribution. + */ + +public class Geary.NonblockingSemaphore : Geary.NonblockingAbstractSemaphore { + public NonblockingSemaphore(Cancellable? cancellable = null) { + base (true, false, cancellable); + } +} + +public class Geary.NonblockingEvent : Geary.NonblockingAbstractSemaphore { + public NonblockingEvent(Cancellable? cancellable = null) { + base (true, true, cancellable); + } +} + +public class Geary.NonblockingSpinlock : Geary.NonblockingAbstractSemaphore { + public NonblockingSpinlock(Cancellable? cancellable = null) { + base (false, true, cancellable); + } +} + diff --git a/src/engine/sqlite/abstract/sqlite-row.vala b/src/engine/sqlite/abstract/sqlite-row.vala index 0deac4f8..38c0ca85 100644 --- a/src/engine/sqlite/abstract/sqlite-row.vala +++ b/src/engine/sqlite/abstract/sqlite-row.vala @@ -7,7 +7,7 @@ public abstract class Geary.Sqlite.Row { public const int64 INVALID_ID = -1; - private Table table; + protected Table table; public Row(Table table) { this.table = table; diff --git a/src/engine/sqlite/api/sqlite-folder.vala b/src/engine/sqlite/api/sqlite-folder.vala index 4df3da2e..30eb60e6 100644 --- a/src/engine/sqlite/api/sqlite-folder.vala +++ b/src/engine/sqlite/api/sqlite-folder.vala @@ -110,7 +110,7 @@ public class Geary.Sqlite.Folder : Geary.AbstractFolder, Geary.LocalFolder, Gear yield imap_message_properties_table.create_async(properties_row, cancellable); } - notify_list_appended(yield get_email_count_async(cancellable)); + notify_messages_appended(yield get_email_count_async(cancellable)); } public override async Gee.List? list_email_async(int low, int count, @@ -179,9 +179,15 @@ public class Geary.Sqlite.Folder : Geary.AbstractFolder, Geary.LocalFolder, Gear } Geary.Imap.UID uid = new Geary.Imap.UID(location_row.ordering); + int position = yield location_row.get_position_async(cancellable); + if (position == -1) { + debug("Unable to locate position of email during list of %s, dropping", to_string()); + + continue; + } Geary.Email email = message_row.to_email( - new Geary.Imap.EmailLocation(location_row.position, uid), + new Geary.Imap.EmailLocation(this, position, uid), new Geary.Imap.EmailIdentifier(uid)); if (properties != null) email.set_email_properties(properties.get_imap_email_properties()); @@ -231,9 +237,13 @@ public class Geary.Sqlite.Folder : Geary.AbstractFolder, Geary.LocalFolder, Gear } } - // TODO: Would be helpful if proper position was known - Geary.Email email = message_row.to_email( - new Geary.Imap.EmailLocation(location_row.position, uid), id); + int position = yield location_row.get_position_async(cancellable); + if (position == -1) { + throw new EngineError.NOT_FOUND("Unable to determine position of email %s in %s", + id.to_string(), to_string()); + } + + Geary.Email email = message_row.to_email(new Geary.Imap.EmailLocation(this, position, uid), id); if (properties != null) email.set_email_properties(properties.get_imap_email_properties()); @@ -248,7 +258,7 @@ public class Geary.Sqlite.Folder : Geary.AbstractFolder, Geary.LocalFolder, Gear return (ordering >= 1) ? new Geary.Imap.UID(ordering) : null; } - public override async void remove_email_async(Geary.Email email, Cancellable? cancellable = null) + public override async void remove_email_async(int position, Cancellable? cancellable = null) throws Error { check_open(); @@ -256,13 +266,12 @@ public class Geary.Sqlite.Folder : Geary.AbstractFolder, Geary.LocalFolder, Gear // (since it may be located in multiple folders). This means at some point in the future // a vacuum will be required to remove emails that are completely unassociated with the // account - Geary.Imap.UID? uid = ((Geary.Imap.EmailIdentifier) email.id).uid; - if (uid == null) - throw new EngineError.NOT_FOUND("UID required to delete local email"); + if (!yield location_table.remove_by_position_async(folder_row.id, position, cancellable)) { + throw new EngineError.NOT_FOUND("Message #%d in local store of %s not found", position, + to_string()); + } - yield location_table.remove_by_ordering_async(folder_row.id, uid.value, cancellable); - - // TODO: Notify of changes + notify_message_removed(position, yield get_email_count_async(cancellable)); } public async bool is_email_present(Geary.EmailIdentifier id, out Geary.Email.Field available_fields, diff --git a/src/engine/sqlite/email/sqlite-message-location-row.vala b/src/engine/sqlite/email/sqlite-message-location-row.vala index e4d7e2fe..07aca2e8 100644 --- a/src/engine/sqlite/email/sqlite-message-location-row.vala +++ b/src/engine/sqlite/email/sqlite-message-location-row.vala @@ -9,12 +9,8 @@ public class Geary.Sqlite.MessageLocationRow : Geary.Sqlite.Row { public int64 message_id { get; private set; } public int64 folder_id { get; private set; } public int64 ordering { get; private set; } - /** - * Note that position is not stored in the database, but rather determined by its location - * determined by the sorted ordering. If the database call is unable to easily determine the - * position of the message in the folder, this will be set to -1. - */ - public int position { get; private set; } + + private int position; public MessageLocationRow(MessageLocationTable table, int64 id, int64 message_id, int64 folder_id, int64 ordering, int position) { @@ -37,5 +33,25 @@ public class Geary.Sqlite.MessageLocationRow : Geary.Sqlite.Row { ordering = fetch_int64_for(result, MessageLocationTable.Column.ORDERING); this.position = position; } + + /** + * Note that position is not stored in the database, but rather determined by its location + * determined by the sorted ordering column. In some cases the database can determine the + * position easily and will supply it to this object at construction time. In other cases it's + * not so straightforward and another database query will be required. This method handles + * both cases. + * + * If the call ever returns a position of -1, that indicates the message does not exist in the + * database. + */ + public async int get_position_async(Cancellable? cancellable = null) throws Error { + if (position >= 1) + return position; + + position = yield ((MessageLocationTable) table).fetch_position_async(id, folder_id, + cancellable); + + return (position >= 1) ? position : -1; + } } diff --git a/src/engine/sqlite/email/sqlite-message-location-table.vala b/src/engine/sqlite/email/sqlite-message-location-table.vala index 4e71d394..14c28813 100644 --- a/src/engine/sqlite/email/sqlite-message-location-table.vala +++ b/src/engine/sqlite/email/sqlite-message-location-table.vala @@ -178,6 +178,27 @@ public class Geary.Sqlite.MessageLocationTable : Geary.Sqlite.Table { folder_id, ordering, -1); } + public async int fetch_position_async(int64 id, int64 folder_id, Cancellable? cancellable = null) + throws Error { + SQLHeavy.Query query = db.prepare( + "SELECT id FROM MessageLocationTable WHERE folder_id = ? ORDER BY ordering"); + query.bind_int64(0, folder_id); + + SQLHeavy.QueryResult results = yield query.execute_async(cancellable); + + int position = 1; + while (!results.finished) { + if (results.fetch_int64(0) == id) + return position; + + yield results.next_async(cancellable); + position++; + } + + // not found + return -1; + } + public async int fetch_count_for_folder_async(int64 folder_id, Cancellable? cancellable = null) throws Error { SQLHeavy.Query query = db.prepare( @@ -219,14 +240,30 @@ public class Geary.Sqlite.MessageLocationTable : Geary.Sqlite.Table { return (!result.finished) ? result.fetch_int64(0) : -1; } - public async void remove_by_ordering_async(int64 folder_id, int64 ordering, + public async bool remove_by_position_async(int64 folder_id, int position, Cancellable? cancellable = null) throws Error { - SQLHeavy.Query query = db.prepare( - "DELETE FROM MessageLocationTable WHERE folder_id = ? AND ordering = ?"); + assert(position >= 1); + + SQLHeavy.Transaction transaction = db.begin_transaction(); + + SQLHeavy.Query query = transaction.prepare( + "SELECT id FROM MessageLocationTable WHERE folder_id = ? ORDER BY ordering LIMIT 1 OFFSET ?"); query.bind_int64(0, folder_id); - query.bind_int64(1, ordering); + query.bind_int(1, position - 1); + + SQLHeavy.QueryResult results = yield query.execute_async(cancellable); + if (results.finished) + return false; + + query = transaction.prepare( + "DELETE FROM MessageLocationTable WHERE id = ?"); + query.bind_int64(0, results.fetch_int(0)); yield query.execute_async(cancellable); + + yield transaction.commit_async(); + + return true; } } diff --git a/src/wscript b/src/wscript index d79e458c..299749a8 100644 --- a/src/wscript +++ b/src/wscript @@ -36,6 +36,7 @@ def build(bld): '../engine/api/geary-local-interfaces.vala', '../engine/api/geary-personality.vala', '../engine/api/geary-remote-interfaces.vala', + '../engine/api/geary-replay-queue.vala', '../engine/api/geary-special-folder.vala', '../engine/common/common-message-data.vala', @@ -83,7 +84,8 @@ def build(bld): '../engine/nonblocking/nonblocking-mailbox.vala', '../engine/nonblocking/nonblocking-mutex.vala', - '../engine/nonblocking/nonblocking-semaphore.vala', + '../engine/nonblocking/nonblocking-abstract-semaphore.vala', + '../engine/nonblocking/nonblocking-variants.vala', '../engine/rfc822/rfc822-error.vala', '../engine/rfc822/rfc822-mailbox-addresses.vala',