From 6ce275f61d66450323b51127b69cfb72a742aae1 Mon Sep 17 00:00:00 2001 From: Jim Nelson Date: Tue, 14 May 2013 15:36:33 -0700 Subject: [PATCH 1/7] Geary no longer spikes to 100% CPU: Closes #6602 Folders can now be FAST_OPEN'd, which is used by the background account synchronizer to quickly determine what changes have occurred to a folder and get it up-to-date. I've used Geary for a few days with these changes and have seen a dramatic difference in sluggishness. There are still CPU spikes when background work is detected, but not like before and not nearly as sustained. --- src/client/geary-controller.vala | 4 +- src/dbusservice/controller.vala | 2 +- src/dbusservice/dbus-conversations.vala | 4 +- .../abstract/geary-abstract-folder.vala | 3 +- .../api/geary-conversation-monitor.vala | 8 +- src/engine/api/geary-folder.vala | 29 ++++- src/engine/imap-db/imap-db-account.vala | 118 ++++++++++++++---- src/engine/imap-db/imap-db-folder.vala | 2 +- .../imap-db/outbox/smtp-outbox-folder.vala | 6 +- .../imap-engine-account-synchronizer.vala | 2 +- .../imap-engine-generic-account.vala | 2 +- .../imap-engine-generic-folder.vala | 106 +++++++++++----- .../imap/api/imap-folder-properties.vala | 44 ++++++- src/engine/imap/api/imap-folder.vala | 14 +-- 14 files changed, 255 insertions(+), 89 deletions(-) diff --git a/src/client/geary-controller.vala b/src/client/geary-controller.vala index c93a242b..08d6dd0c 100644 --- a/src/client/geary-controller.vala +++ b/src/client/geary-controller.vala @@ -476,7 +476,7 @@ public class GearyController { update_ui(); - current_conversations = new Geary.ConversationMonitor(current_folder, false, + current_conversations = new Geary.ConversationMonitor(current_folder, Geary.Folder.OpenFlags.NONE, ConversationListStore.REQUIRED_FIELDS); if (inboxes.values.contains(current_folder)) { @@ -739,7 +739,7 @@ public class GearyController { main_window.folder_list.select_folder(select_folder); } - folder.open_async.begin(false, inbox_cancellables.get(folder.account)); + folder.open_async.begin(Geary.Folder.OpenFlags.NONE, inbox_cancellables.get(folder.account)); new_messages_monitor.add_folder(folder, inbox_cancellables.get(folder.account)); } diff --git a/src/dbusservice/controller.vala b/src/dbusservice/controller.vala index 102c1e25..2d1742bc 100644 --- a/src/dbusservice/controller.vala +++ b/src/dbusservice/controller.vala @@ -62,7 +62,7 @@ public class Geary.DBus.Controller { warning("No inbox folder found"); return; } - yield folder.open_async(false, null); + yield folder.open_async(Geary.Folder.OpenFlags.NONE, null); conversations = new Geary.DBus.Conversations(folder); diff --git a/src/dbusservice/dbus-conversations.vala b/src/dbusservice/dbus-conversations.vala index 2de5b6f7..c35935c4 100644 --- a/src/dbusservice/dbus-conversations.vala +++ b/src/dbusservice/dbus-conversations.vala @@ -27,8 +27,8 @@ public class Geary.DBus.Conversations : Object { public Conversations(Geary.Folder folder) { this.folder = folder; - conversations = new Geary.ConversationMonitor(folder, false, Geary.Email.Field.ENVELOPE | - Geary.Email.Field.FLAGS); + conversations = new Geary.ConversationMonitor(folder, Geary.Folder.OpenFlags.NONE, + Geary.Email.Field.ENVELOPE | Geary.Email.Field.FLAGS); start_monitoring_async.begin(); } diff --git a/src/engine/abstract/geary-abstract-folder.vala b/src/engine/abstract/geary-abstract-folder.vala index 57195a2d..fdc47be0 100644 --- a/src/engine/abstract/geary-abstract-folder.vala +++ b/src/engine/abstract/geary-abstract-folder.vala @@ -68,7 +68,8 @@ public abstract class Geary.AbstractFolder : BaseObject, Geary.Folder { public abstract Geary.Folder.OpenState get_open_state(); - public abstract async void open_async(bool readonly, Cancellable? cancellable = null) throws Error; + public abstract async bool open_async(Geary.Folder.OpenFlags open_flags, Cancellable? cancellable = null) + throws Error; public abstract async void wait_for_open_async(Cancellable? cancellable = null) throws Error; diff --git a/src/engine/api/geary-conversation-monitor.vala b/src/engine/api/geary-conversation-monitor.vala index 19e4407b..972dbcfb 100644 --- a/src/engine/api/geary-conversation-monitor.vala +++ b/src/engine/api/geary-conversation-monitor.vala @@ -172,7 +172,7 @@ public class Geary.ConversationMonitor : BaseObject { public bool is_monitoring { get; private set; default = false; } private Geary.Email.Field required_fields; - private bool readonly; + private Geary.Folder.OpenFlags open_flags; private Gee.Set conversations = new Gee.HashSet(); private Gee.HashMap geary_id_map = new Gee.HashMap< Geary.EmailIdentifier, ImplConversation>(); @@ -307,9 +307,9 @@ public class Geary.ConversationMonitor : BaseObject { folder.to_string()); } - public ConversationMonitor(Geary.Folder folder, bool readonly, Geary.Email.Field required_fields) { + public ConversationMonitor(Geary.Folder folder, Geary.Folder.OpenFlags open_flags, Geary.Email.Field required_fields) { this.folder = folder; - this.readonly = readonly; + this.open_flags = open_flags; this.required_fields = required_fields | REQUIRED_FIELDS; folder.account.information.notify["imap-credentials"].connect(on_imap_credentials_notified); @@ -402,7 +402,7 @@ public class Geary.ConversationMonitor : BaseObject { bool reseed_now = (folder.get_open_state() != Geary.Folder.OpenState.CLOSED); try { - yield folder.open_async(readonly, cancellable); + yield folder.open_async(open_flags, cancellable); } catch (Error err) { is_monitoring = false; diff --git a/src/engine/api/geary-folder.vala b/src/engine/api/geary-folder.vala index f3ac6c4a..34c9ac5e 100644 --- a/src/engine/api/geary-folder.vala +++ b/src/engine/api/geary-folder.vala @@ -49,6 +49,28 @@ public interface Geary.Folder : BaseObject { REMOVED } + /** + * Flags describing open modifiers. + * + * * FAST_OPEN: perform the minimal amount of activity possible to open the folder + * and be synchronized with the server. This may mean some attributes of + * the messages (such as their flags or other metadata) may not be up-to-date + * when the folder opens. Not all folders will support this flag. + */ + [Flags] + public enum OpenFlags { + NONE = 0, + FAST_OPEN; + + public bool is_any_set(OpenFlags flags) { + return (this & flags) != 0; + } + + public bool is_all_set(OpenFlags flags) { + return (this & flags) == flags; + } + } + /** * Flags used for retrieving email. * @@ -245,13 +267,16 @@ public interface Geary.Folder : BaseObject { * associated signals will be fired as well. * * If the Folder has been opened previously, an internal open count is incremented and the - * method returns. There are no other side-effects. + * method returns. There are no other side-effects. This means it's possible for the + * open_flags parameter to be ignored. See the returned result for more information. * * A Folder may be reopened after it has been closed. This allows for Folder objects to be * emitted by the Account object cheaply, but the client should only have a few open at a time, * as each may represent an expensive resource (such as a network connection). + * + * Returns false if already opened. */ - public abstract async void open_async(bool readonly, Cancellable? cancellable = null) throws Error; + public abstract async bool open_async(OpenFlags open_flags, Cancellable? cancellable = null) throws Error; /** * Wait for the Folder to become fully open or fails to open due to error. If not opened diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala index 3ddcd9d0..f8653e58 100644 --- a/src/engine/imap-db/imap-db-account.vala +++ b/src/engine/imap-db/imap-db-account.vala @@ -133,7 +133,11 @@ private class Geary.ImapDB.Account : BaseObject { }, cancellable); } - public async void update_folder_async(Geary.Imap.Folder imap_folder, Cancellable? cancellable = null) + /** + * Only updates folder's STATUS message count, attributes, recent, and unseen; UIDVALIDITY and UIDNEXT + * updated when the folder is SELECT/EXAMINED (see update_folder_select_examine_async()) + */ + public async void update_folder_status_async(Geary.Imap.Folder imap_folder, Cancellable? cancellable) throws Error { check_open(); @@ -151,34 +155,19 @@ private class Geary.ImapDB.Account : BaseObject { Db.Statement stmt; if (parent_id != Db.INVALID_ROWID) { stmt = cx.prepare( - "UPDATE FolderTable SET uid_validity=?, uid_next=?, attributes=? " - + "WHERE parent_id=? AND name=?"); - stmt.bind_int64(0, (properties.uid_validity != null) ? properties.uid_validity.value - : Imap.UIDValidity.INVALID); - stmt.bind_int64(1, (properties.uid_next != null) ? properties.uid_next.value - : Imap.UID.INVALID); - stmt.bind_string(2, properties.attrs.serialize()); - stmt.bind_rowid(3, parent_id); - stmt.bind_string(4, path.basename); + "UPDATE FolderTable SET attributes=? WHERE parent_id=? AND name=?"); + stmt.bind_string(0, properties.attrs.serialize()); + stmt.bind_rowid(1, parent_id); + stmt.bind_string(2, path.basename); } else { stmt = cx.prepare( - "UPDATE FolderTable SET uid_validity=?, uid_next=?, attributes=? " - + "WHERE parent_id IS NULL AND name=?"); - stmt.bind_int64(0, (properties.uid_validity != null) ? properties.uid_validity.value - : Imap.UIDValidity.INVALID); - stmt.bind_int64(1, (properties.uid_next != null) ? properties.uid_next.value - : Imap.UID.INVALID); - stmt.bind_string(2, properties.attrs.serialize()); - stmt.bind_string(3, path.basename); + "UPDATE FolderTable SET attributes=? WHERE parent_id IS NULL AND name=?"); + stmt.bind_string(0, properties.attrs.serialize()); + stmt.bind_string(1, path.basename); } stmt.exec(); - if (properties.select_examine_messages >= 0) { - do_update_last_seen_total(cx, parent_id, path.basename, properties.select_examine_messages, - cancellable); - } - if (properties.status_messages >= 0) { do_update_last_seen_status_total(cx, parent_id, path.basename, properties.status_messages, cancellable); @@ -187,10 +176,83 @@ private class Geary.ImapDB.Account : BaseObject { return Db.TransactionOutcome.COMMIT; }, cancellable); - // update properties in the local folder + // update appropriate properties in the local folder ImapDB.Folder? db_folder = get_local_folder(path); - if (db_folder != null) - db_folder.set_properties(properties); + if (db_folder != null) { + Imap.FolderProperties local_properties = db_folder.get_properties(); + + local_properties.unseen = properties.unseen; + local_properties.recent = properties.recent; + local_properties.attrs = properties.attrs; + + if (properties.status_messages >= 0) + local_properties.set_status_message_count(properties.status_messages, false); + } + } + + /** + * Only updates folder's SELECT/EXAMINE message count, UIDVALIDITY, UIDNEXT, unseen, and recent. + * See also update_folder_status_async(). + */ + public async void update_folder_select_examine_async(Geary.Imap.Folder imap_folder, Cancellable? cancellable) + throws Error { + check_open(); + + Geary.Imap.FolderProperties properties = imap_folder.get_properties(); + Geary.FolderPath path = imap_folder.get_path(); + + yield db.exec_transaction_async(Db.TransactionType.RW, (cx) => { + int64 parent_id; + if (!do_fetch_parent_id(cx, path, true, out parent_id, cancellable)) { + debug("Unable to find parent ID of %s to update properties", path.to_string()); + + return Db.TransactionOutcome.ROLLBACK; + } + + int64 uid_validity = (properties.uid_validity != null) ? properties.uid_validity.value + : Imap.UIDValidity.INVALID; + int64 uid_next = (properties.uid_next != null) ? properties.uid_next.value + : Imap.UID.INVALID; + + Db.Statement stmt; + if (parent_id != Db.INVALID_ROWID) { + stmt = cx.prepare( + "UPDATE FolderTable SET uid_validity=?, uid_next=? WHERE parent_id=? AND name=?"); + stmt.bind_int64(0, uid_validity); + stmt.bind_int64(1, uid_next); + stmt.bind_rowid(2, parent_id); + stmt.bind_string(3, path.basename); + } else { + stmt = cx.prepare( + "UPDATE FolderTable SET uid_validity=?, uid_next=? WHERE parent_id IS NULL AND name=?"); + stmt.bind_int64(0, uid_validity); + stmt.bind_int64(1, uid_next); + stmt.bind_string(2, path.basename); + } + + stmt.exec(); + + if (properties.select_examine_messages >= 0) { + do_update_last_seen_select_examine_total(cx, parent_id, path.basename, + properties.select_examine_messages, cancellable); + } + + return Db.TransactionOutcome.COMMIT; + }, cancellable); + + // update appropriate properties in the local folder + ImapDB.Folder? db_folder = get_local_folder(path); + if (db_folder != null) { + Imap.FolderProperties local_properties = db_folder.get_properties(); + + local_properties.unseen = properties.unseen; + local_properties.recent = properties.recent; + local_properties.uid_validity = properties.uid_validity; + local_properties.uid_next = properties.uid_next; + + if (properties.select_examine_messages >= 0) + local_properties.set_select_examine_message_count(properties.select_examine_messages); + } } private void initialize_contacts(Cancellable? cancellable = null) throws Error { @@ -664,11 +726,13 @@ private class Geary.ImapDB.Account : BaseObject { return (parent_path == null ? null : parent_path.get_child(name)); } - private void do_update_last_seen_total(Db.Connection cx, int64 parent_id, string name, int total, + // For SELECT/EXAMINE responses, not STATUS responses + private void do_update_last_seen_select_examine_total(Db.Connection cx, int64 parent_id, string name, int total, Cancellable? cancellable) throws Error { do_update_total(cx, parent_id, name, "last_seen_total", total, cancellable); } + // For STATUS responses, not SELECT/EXAMINE responses private void do_update_last_seen_status_total(Db.Connection cx, int64 parent_id, string name, int total, Cancellable? cancellable) throws Error { do_update_total(cx, parent_id, name, "last_seen_status_total", total, cancellable); diff --git a/src/engine/imap-db/imap-db-folder.vala b/src/engine/imap-db/imap-db-folder.vala index 370887a3..733dde14 100644 --- a/src/engine/imap-db/imap-db-folder.vala +++ b/src/engine/imap-db/imap-db-folder.vala @@ -93,7 +93,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics { this.properties = properties; } - public async void open_async(bool readonly, Cancellable? cancellable = null) throws Error { + public async void open_async(Cancellable? cancellable = null) throws Error { if (opened) throw new EngineError.ALREADY_OPEN("%s already open", to_string()); diff --git a/src/engine/imap-db/outbox/smtp-outbox-folder.vala b/src/engine/imap-db/outbox/smtp-outbox-folder.vala index bbff4905..0702f6a2 100644 --- a/src/engine/imap-db/outbox/smtp-outbox-folder.vala +++ b/src/engine/imap-db/outbox/smtp-outbox-folder.vala @@ -212,12 +212,14 @@ private class Geary.SmtpOutboxFolder : Geary.AbstractFolder, Geary.FolderSupport throw new EngineError.OPEN_REQUIRED("Outbox not open"); } - public override async void open_async(bool readonly, Cancellable? cancellable = null) + public override async bool open_async(Geary.Folder.OpenFlags open_flags, Cancellable? cancellable = null) throws Error { if (open_count++ > 0) - return; + return false; notify_opened(Geary.Folder.OpenState.LOCAL, properties.email_total); + + return true; } public override async void close_async(Cancellable? cancellable = null) throws Error { diff --git a/src/engine/imap-engine/imap-engine-account-synchronizer.vala b/src/engine/imap-engine/imap-engine-account-synchronizer.vala index 71beab34..604dfa2c 100644 --- a/src/engine/imap-engine/imap-engine-account-synchronizer.vala +++ b/src/engine/imap-engine/imap-engine-account-synchronizer.vala @@ -237,7 +237,7 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject { } try { - yield folder.open_async(true, bg_cancellable); + yield folder.open_async(Folder.OpenFlags.FAST_OPEN, bg_cancellable); yield folder.wait_for_open_async(bg_cancellable); } catch (Error err) { // don't need to close folder; if either calls throws an error, the folder is not open diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala b/src/engine/imap-engine/imap-engine-generic-account.vala index d369c85d..457774ac 100644 --- a/src/engine/imap-engine/imap-engine-generic-account.vala +++ b/src/engine/imap-engine/imap-engine-generic-account.vala @@ -337,7 +337,7 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.AbstractAccount { } try { - yield local.update_folder_async(remote_folder, cancellable); + yield local.update_folder_status_async(remote_folder, 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 51a6750b..a9a563ea 100644 --- a/src/engine/imap-engine/imap-engine-generic-folder.vala +++ b/src/engine/imap-engine/imap-engine-generic-folder.vala @@ -11,6 +11,8 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde private const Geary.Email.Field NORMALIZATION_FIELDS = Geary.Email.Field.PROPERTIES | Geary.Email.Field.FLAGS | ImapDB.Folder.REQUIRED_FOR_DUPLICATE_DETECTION; + private const Geary.Email.Field FAST_NORMALIZATION_FIELDS = + Geary.Email.Field.PROPERTIES | ImapDB.Folder.REQUIRED_FOR_DUPLICATE_DETECTION; public override Account account { get { return _account; } } internal ImapDB.Folder local_folder { get; protected set; } @@ -102,8 +104,13 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde return (remote_count >= 0) ? remote_count : last_seen_remote_count; } - private async bool normalize_folders(Geary.Imap.Folder remote_folder, Cancellable? cancellable) throws Error { - debug("normalize_folders %s", to_string()); + 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()); Geary.Imap.FolderProperties local_properties = local_folder.get_properties(); Geary.Imap.FolderProperties remote_properties = remote_folder.get_properties(); @@ -147,17 +154,54 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde return true; } + // 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()); + + 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(); + // 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 Geary.Imap.EmailIdentifier current_start_id = new Geary.Imap.EmailIdentifier(earliest_uid, local_folder.get_path()); + + // if fast-open and 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_id = new Geary.Imap.EmailIdentifier(latest_uid.next(), local_folder.get_path()); + + 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_id.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()); + } + } + for (;;) { // 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_id_async( - current_start_id, NORMALIZATION_CHUNK_COUNT, NORMALIZATION_FIELDS, + current_start_id, NORMALIZATION_CHUNK_COUNT, + is_fast_open ? FAST_NORMALIZATION_FIELDS : NORMALIZATION_FIELDS, ImapDB.Folder.ListFlags.PARTIAL_OK, cancellable); // verify still open @@ -213,23 +257,26 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde break; if (remote_uid != null && local_uid != null && remote_uid.value == local_uid.value) { - // 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); - all_flags_changed.set(remote_email.id, remote_email.email_flags); - - 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()); + // 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); + all_flags_changed.set(remote_email.id, remote_email.email_flags); + + 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()); + } } } @@ -408,9 +455,10 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde throw new EngineError.ALREADY_CLOSED("%s failed to open", to_string()); } - public override async void open_async(bool readonly, Cancellable? cancellable = null) throws Error { + public override async bool open_async(Geary.Folder.OpenFlags open_flags, Cancellable? cancellable = null) + throws Error { if (open_count++ > 0) - return; + return false; remote_semaphore = new Geary.Nonblocking.ReportingSemaphore(false); @@ -418,7 +466,7 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde replay_queue = new ReplayQueue(get_path().to_string(), remote_semaphore); try { - yield local_folder.open_async(readonly, cancellable); + yield local_folder.open_async(cancellable); } catch (Error err) { notify_open_failed(OpenFailed.LOCAL_FAILED, err); @@ -436,22 +484,24 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde // wait_for_remote_ready_async(), which uses a NonblockingSemaphore to indicate that the remote // is open (or has failed to open). This allows for early calls to list and fetch emails // can work out of the local cache until the remote is ready. - open_remote_async.begin(readonly, cancellable); + open_remote_async.begin(open_flags, cancellable); + + return true; } - private async void open_remote_async(bool readonly, Cancellable? cancellable) { + private async void open_remote_async(Geary.Folder.OpenFlags open_flags, Cancellable? cancellable) { try { debug("Opening remote %s", to_string()); Imap.Folder folder = (Imap.Folder) yield remote.fetch_folder_async(local_folder.get_path(), cancellable); - yield folder.open_async(readonly, cancellable); + yield folder.open_async(cancellable); // allow subclasses to examine the opened folder and resolve any vital // inconsistencies - if (yield normalize_folders(folder, cancellable)) { + if (yield normalize_folders(folder, open_flags, cancellable)) { // update flags, properties, etc. - yield local.update_folder_async(folder, cancellable); + yield local.update_folder_select_examine_async(folder, cancellable); // signals folder.messages_appended.connect(on_remote_messages_appended); diff --git a/src/engine/imap/api/imap-folder-properties.vala b/src/engine/imap/api/imap-folder-properties.vala index 7f95a141..c666ba7e 100644 --- a/src/engine/imap/api/imap-folder-properties.vala +++ b/src/engine/imap/api/imap-folder-properties.vala @@ -4,6 +4,40 @@ * (version 2.1 or later). See the COPYING file in this distribution. */ +/** + * Geary.Imap.FolderProperties represent the Geary API's notion of FolderProperties but + * also hangs additional useful information available to IMAP-specific code (in the Engine, + * that includes imap, imap-engine, and imap-db). + * + * One important concept here is that there are two IMAP commands that return this information: + * STATUS (which is used by the background folder monitor to watch for specific events) and + * SELECT/EXAMINE (which is used to "enter" or "cd" into a folder and perform operations on mail + * within). + * + * Experience has shown that these commands are *not* guaranteed to return the same information, + * even if no state has changed on the server. This would seem to be a server bug, but one that + * has to be worked around. + * + * In any event, the properties here are updated by the following logic: + * + * When a folder is first "seen" by Geary, it generates an Imap.FolderProperties object with all + * the fields filled in except for status_messages or select_examine_messages, depending on which + * command was used to discover it. (In practice, the folder will be first recognized via STATUS, + * but this isn't guaranteed.) + * + * When new STATUS information comes in, this object's status_messages, unseen, recent, and attrs + * fields are updated. + * + * When a SELECT/EXAMINE occurs on this folder, this object's select_examine_messages, unseen, + * recent, uid_validity, and uid_next are updated. + * + * Over time, this object accumulates information depending on what operation was last + * performed on it. + * + * The base class' email_total is updated when either *_messages is updated; however, SELECT/EXAMINE + * is considered more authoritative than STATUS. + */ + public class Geary.Imap.FolderProperties : Geary.FolderProperties { /** * -1 if the Folder was not opened via SELECT or EXAMINE. @@ -13,11 +47,11 @@ public class Geary.Imap.FolderProperties : Geary.FolderProperties { * -1 if the FolderProperties were not obtained via a STATUS command */ public int status_messages { get; private set; } - public int unseen { get; private set; } - public int recent { get; private set; } - public UIDValidity? uid_validity { get; private set; } - public UID? uid_next { get; private set; } - public MailboxAttributes attrs { get; private set; } + public int unseen { get; internal set; } + public int recent { get; internal set; } + public UIDValidity? uid_validity { get; internal set; } + public UID? uid_next { get; internal set; } + public MailboxAttributes attrs { get; internal set; } // Note that unseen from SELECT/EXAMINE is the *position* of the first unseen message, // not the total unseen count, so it should not be passed in here, but rather the unseen diff --git a/src/engine/imap/api/imap-folder.vala b/src/engine/imap/api/imap-folder.vala index 9a67da71..ff288de0 100644 --- a/src/engine/imap/api/imap-folder.vala +++ b/src/engine/imap/api/imap-folder.vala @@ -10,7 +10,6 @@ private class Geary.Imap.Folder : BaseObject { private ClientSessionManager session_mgr; private MailboxInformation info; private Geary.FolderPath path; - private Trillian readonly; private Imap.FolderProperties properties; private Mailbox? mailbox = null; @@ -26,8 +25,6 @@ private class Geary.Imap.Folder : BaseObject { this.info = info; this.path = path; - readonly = Trillian.UNKNOWN; - properties = new Imap.FolderProperties.status(status, info.attrs); } @@ -37,8 +34,6 @@ private class Geary.Imap.Folder : BaseObject { this.info = info; this.path = path; - readonly = Trillian.UNKNOWN; - properties = new Imap.FolderProperties(0, 0, 0, null, null, info.attrs); } @@ -50,15 +45,11 @@ private class Geary.Imap.Folder : BaseObject { return properties; } - public async void open_async(bool readonly, Cancellable? cancellable = null) throws Error { + public async void open_async(Cancellable? cancellable = null) throws Error { if (mailbox != null) throw new EngineError.ALREADY_OPEN("%s already open", to_string()); - mailbox = yield session_mgr.select_examine_mailbox(path, info.delim, !readonly, - cancellable); - - // update with new information - this.readonly = Trillian.from_boolean(readonly); + mailbox = yield session_mgr.select_mailbox(path, info.delim, cancellable); // connect to signals mailbox.exists_altered.connect(on_exists_altered); @@ -86,7 +77,6 @@ private class Geary.Imap.Folder : BaseObject { mailbox.disconnected.disconnect(on_disconnected); mailbox = null; - readonly = Trillian.UNKNOWN; } private void on_exists_altered(int old_exists, int new_exists) { From 974e9459a3e7ac1d0b5d889b5ff1881dfd206f1e Mon Sep 17 00:00:00 2001 From: Robert Schroll Date: Tue, 14 May 2013 18:07:53 -0700 Subject: [PATCH 2/7] Allow external images from whitelisted senders to be displayed: Closes #5642 This also cleans up some of the update logic for ContactTable, which could drop contacts' human-readable names when updating their importance. --- sql/CMakeLists.txt | 1 + sql/version-009.sql | 6 + src/CMakeLists.txt | 5 +- src/client/views/conversation-viewer.vala | 74 +++++++++-- src/engine/api/geary-contact-flags.vala | 50 ++++++++ src/engine/api/geary-contact-store.vala | 9 +- src/engine/api/geary-contact.vala | 9 +- src/engine/api/geary-conversation.vala | 6 +- src/engine/api/geary-email-flags.vala | 121 +++--------------- ...-email-flag.vala => geary-named-flag.vala} | 16 ++- src/engine/api/geary-named-flags.vala | 107 ++++++++++++++++ src/engine/imap-db/imap-db-account.vala | 23 +++- src/engine/imap-db/imap-db-contact.vala | 69 ++++++++-- src/engine/imap-db/imap-db-database.vala | 2 +- src/engine/imap-db/imap-db-folder.vala | 8 +- .../imap-engine-contact-store.vala | 26 ++++ src/engine/imap/api/imap-email-flags.vala | 8 +- 17 files changed, 392 insertions(+), 148 deletions(-) create mode 100644 sql/version-009.sql create mode 100644 src/engine/api/geary-contact-flags.vala rename src/engine/api/{geary-email-flag.vala => geary-named-flag.vala} (54%) create mode 100644 src/engine/api/geary-named-flags.vala create mode 100644 src/engine/imap-engine/imap-engine-contact-store.vala diff --git a/sql/CMakeLists.txt b/sql/CMakeLists.txt index 86490129..02cc18f9 100644 --- a/sql/CMakeLists.txt +++ b/sql/CMakeLists.txt @@ -8,3 +8,4 @@ install(FILES version-005.sql DESTINATION ${SQL_DEST}) install(FILES version-006.sql DESTINATION ${SQL_DEST}) install(FILES version-007.sql DESTINATION ${SQL_DEST}) install(FILES version-008.sql DESTINATION ${SQL_DEST}) +install(FILES version-009.sql DESTINATION ${SQL_DEST}) diff --git a/sql/version-009.sql b/sql/version-009.sql new file mode 100644 index 00000000..ab236902 --- /dev/null +++ b/sql/version-009.sql @@ -0,0 +1,6 @@ +-- +-- Add flags column to the ContactTable +-- + +ALTER TABLE ContactTable ADD COLUMN flags TEXT; + diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 5c8dfd4b..810fbfd4 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -18,13 +18,13 @@ engine/api/geary-attachment.vala engine/api/geary-base-object.vala engine/api/geary-composed-email.vala engine/api/geary-contact.vala +engine/api/geary-contact-flags.vala engine/api/geary-contact-importance.vala engine/api/geary-contact-store.vala engine/api/geary-conversation.vala engine/api/geary-conversation-monitor.vala engine/api/geary-credentials.vala engine/api/geary-credentials-mediator.vala -engine/api/geary-email-flag.vala engine/api/geary-email-flags.vala engine/api/geary-email-identifier.vala engine/api/geary-email-properties.vala @@ -42,6 +42,8 @@ engine/api/geary-folder-supports-mark.vala engine/api/geary-folder-supports-move.vala engine/api/geary-folder-supports-remove.vala engine/api/geary-logging.vala +engine/api/geary-named-flag.vala +engine/api/geary-named-flags.vala engine/api/geary-service-provider.vala engine/api/geary-special-folder-type.vala @@ -123,6 +125,7 @@ engine/imap-db/outbox/smtp-outbox-folder-root.vala engine/imap-engine/imap-engine.vala engine/imap-engine/imap-engine-account-synchronizer.vala engine/imap-engine/imap-engine-batch-operations.vala +engine/imap-engine/imap-engine-contact-store.vala engine/imap-engine/imap-engine-email-flag-watcher.vala engine/imap-engine/imap-engine-email-prefetcher.vala engine/imap-engine/imap-engine-generic-account.vala diff --git a/src/client/views/conversation-viewer.vala b/src/client/views/conversation-viewer.vala index 7fc58dff..9e0f2487 100644 --- a/src/client/views/conversation-viewer.vala +++ b/src/client/views/conversation-viewer.vala @@ -190,17 +190,23 @@ public class ConversationViewer : Gtk.Box { } if (remote_images) { - if (email.load_remote_images().is_certain()) { - show_images_email(div_message); + Geary.Contact contact = current_folder.account.get_contact_store().get_by_rfc822( + email.get_primary_originator()); + bool always_load = contact != null && contact.always_load_remote_images(); + + if (always_load || email.load_remote_images().is_certain()) { + show_images_email(div_message, false); } else { WebKit.DOM.HTMLElement remote_images_bar = Util.DOM.select(div_message, ".remote_images"); try { ((WebKit.DOM.Element) remote_images_bar).get_class_list().add("show"); remote_images_bar.set_inner_html("""%s %s - """.printf( + + """.printf( remote_images_bar.get_inner_html(), - _("This message contains remote images."), _("Show Images"))); + _("This message contains remote images."), _("Show images"), + _("Always show from sender"))); } catch (Error error) { warning("Error showing remote images bar: %s", error.message); } @@ -239,6 +245,7 @@ public class ConversationViewer : Gtk.Box { bind_event(web_view, ".attachment_container .attachment", "click", (Callback) on_attachment_clicked, this); bind_event(web_view, ".attachment_container .attachment", "contextmenu", (Callback) on_attachment_menu, this); bind_event(web_view, ".remote_images .show_images", "click", (Callback) on_show_images, this); + bind_event(web_view, ".remote_images .show_from", "click", (Callback) on_show_images_from, this); bind_event(web_view, ".remote_images .close_show_images", "click", (Callback) on_close_show_images, this); // Update the search results @@ -689,11 +696,50 @@ public class ConversationViewer : Gtk.Box { ConversationViewer conversation_viewer) { WebKit.DOM.HTMLElement? email_element = closest_ancestor(element, ".email"); if (email_element != null) - conversation_viewer.show_images_email(email_element); + conversation_viewer.show_images_email(email_element, true); } - private void show_images_email(WebKit.DOM.Element email_element) { - // TODO: Remember that these images have been shown. + private static void on_show_images_from(WebKit.DOM.Element element, WebKit.DOM.Event event, + ConversationViewer conversation_viewer) { + Geary.Email? email = conversation_viewer.get_email_from_element(element); + if (email == null) + return; + + Geary.ContactStore contact_store = + conversation_viewer.current_folder.account.get_contact_store(); + Geary.Contact? contact = contact_store.get_by_rfc822(email.get_primary_originator()); + if (contact == null) { + debug("Couldn't find contact for %s", email.from.to_string()); + return; + } + + Geary.ContactFlags flags = new Geary.ContactFlags(); + flags.add(Geary.ContactFlags.ALWAYS_LOAD_REMOTE_IMAGES); + Gee.ArrayList contact_list = new Gee.ArrayList(); + contact_list.add(contact); + contact_store.mark_contacts_async.begin(contact_list, flags, null); + + WebKit.DOM.Document document = conversation_viewer.web_view.get_dom_document(); + try { + WebKit.DOM.NodeList nodes = document.query_selector_all(".email"); + for (ulong i = 0; i < nodes.length; i ++) { + WebKit.DOM.Element? email_element = nodes.item(i) as WebKit.DOM.Element; + if (email_element != null) { + WebKit.DOM.Element? address = email_element.query_selector(".address_name"); + if (address != null) { + WebKit.DOM.Element? mailto_link = address.parent_node as WebKit.DOM.Element; + if (mailto_link != null && contact.normalized_email == + mailto_link.get_attribute("href").substring(7).normalize().casefold()) + conversation_viewer.show_images_email(email_element, false); + } + } + } + } catch (Error error) { + debug("Error showing images: %s", error.message); + } + } + + private void show_images_email(WebKit.DOM.Element email_element, bool remember) { try { WebKit.DOM.NodeList body_nodes = email_element.query_selector_all(".body"); for (ulong j = 0; j < body_nodes.length; j++) { @@ -720,12 +766,14 @@ public class ConversationViewer : Gtk.Box { warning("Error showing images: %s", error.message); } - // only add flag to load remote images if not already present - Geary.Email? message = get_email_from_element(email_element); - if (message != null && !message.load_remote_images().is_certain()) { - Geary.EmailFlags flags = new Geary.EmailFlags(); - flags.add(Geary.EmailFlags.LOAD_REMOTE_IMAGES); - mark_message(message, flags, null); + if (remember) { + // only add flag to load remote images if not already present + Geary.Email? message = get_email_from_element(email_element); + if (message != null && !message.load_remote_images().is_certain()) { + Geary.EmailFlags flags = new Geary.EmailFlags(); + flags.add(Geary.EmailFlags.LOAD_REMOTE_IMAGES); + mark_message(message, flags, null); + } } } diff --git a/src/engine/api/geary-contact-flags.vala b/src/engine/api/geary-contact-flags.vala new file mode 100644 index 00000000..237e04b8 --- /dev/null +++ b/src/engine/api/geary-contact-flags.vala @@ -0,0 +1,50 @@ +/* Copyright 2013 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. + */ + +/** + * A collection of NamedFlags that can be used to enable/disable various user-defined + * options for a contact. System- or Geary-defined flags are available as static + * members. + */ + +public class Geary.ContactFlags : Geary.NamedFlags { + private static NamedFlag? _always_load_remote_images = null; + public static NamedFlag ALWAYS_LOAD_REMOTE_IMAGES { get { + if (_always_load_remote_images == null) + _always_load_remote_images = new NamedFlag("ALWAYSLOADREMOTEIMAGES"); + + return _always_load_remote_images; + } } + + public ContactFlags() { + } + + public static ContactFlags deserialize(string? flags) { + if (String.is_empty(flags)) + return new ContactFlags(); + + ContactFlags result = new ContactFlags(); + + string[] tokens = flags.split(" "); + foreach (string flag in tokens) + result.add(new NamedFlag(flag)); + + return result; + } + + public inline bool always_load_remote_images() { + return contains(ALWAYS_LOAD_REMOTE_IMAGES); + } + + public string serialize() { + string ret = ""; + foreach (NamedFlag flag in list) + ret += flag.serialize() + " "; + + return ret.strip(); + } +} + diff --git a/src/engine/api/geary-contact-store.vala b/src/engine/api/geary-contact-store.vala index b1c73c82..2a308762 100644 --- a/src/engine/api/geary-contact-store.vala +++ b/src/engine/api/geary-contact-store.vala @@ -4,7 +4,7 @@ * (version 2.1 or later). See the COPYING file in this distribution. */ -public class Geary.ContactStore : BaseObject { +public abstract class Geary.ContactStore : BaseObject { public Gee.Collection contacts { owned get { return contact_map.values; } } @@ -24,6 +24,13 @@ public class Geary.ContactStore : BaseObject { update_contact(contact); } + public abstract async void mark_contacts_async(Gee.Collection contacts, ContactFlags? to_add, + ContactFlags? to_remove) throws Error; + + public Contact? get_by_rfc822(Geary.RFC822.MailboxAddress address) { + return contact_map[address.address.normalize().casefold()]; + } + private void update_contact(Contact contact) { Contact? old_contact = contact_map[contact.normalized_email]; if (old_contact == null) { diff --git a/src/engine/api/geary-contact.vala b/src/engine/api/geary-contact.vala index 180a4460..9ac575b9 100644 --- a/src/engine/api/geary-contact.vala +++ b/src/engine/api/geary-contact.vala @@ -9,12 +9,15 @@ public class Geary.Contact : BaseObject { public string email { get; private set; } public string? real_name { get; private set; } public int highest_importance { get; set; } + public ContactFlags? contact_flags { get; set; default = null; } - public Contact(string email, string? real_name, int highest_importance, string? normalized_email = null) { + public Contact(string email, string? real_name, int highest_importance, + string? normalized_email = null, ContactFlags? contact_flags = null) { this.normalized_email = normalized_email ?? email.normalize().casefold(); this.email = email; this.real_name = real_name; this.highest_importance = highest_importance; + this.contact_flags = contact_flags; } public Contact.from_rfc822_address(RFC822.MailboxAddress address, int highest_importance) { @@ -24,4 +27,8 @@ public class Geary.Contact : BaseObject { public RFC822.MailboxAddress get_rfc822_address() { return new RFC822.MailboxAddress(real_name, email); } + + public inline bool always_load_remote_images() { + return contact_flags != null && contact_flags.always_load_remote_images(); + } } diff --git a/src/engine/api/geary-conversation.vala b/src/engine/api/geary-conversation.vala index d78091f6..c31a30e2 100644 --- a/src/engine/api/geary-conversation.vala +++ b/src/engine/api/geary-conversation.vala @@ -126,7 +126,7 @@ public abstract class Geary.Conversation : BaseObject { */ public abstract Geary.EmailIdentifier? get_lowest_email_id(); - private bool check_flag(Geary.EmailFlag flag, bool contains) { + private bool check_flag(Geary.NamedFlag flag, bool contains) { foreach (Geary.Email email in get_emails(Ordering.NONE)) { if (email.email_flags != null && email.email_flags.contains(flag) == contains) return true; @@ -135,11 +135,11 @@ public abstract class Geary.Conversation : BaseObject { return false; } - private bool has_flag(Geary.EmailFlag flag) { + private bool has_flag(Geary.NamedFlag flag) { return check_flag(flag, true); } - private bool is_missing_flag(Geary.EmailFlag flag) { + private bool is_missing_flag(Geary.NamedFlag flag) { return check_flag(flag, false); } } diff --git a/src/engine/api/geary-email-flags.vala b/src/engine/api/geary-email-flags.vala index dd4c0dc3..1a505036 100644 --- a/src/engine/api/geary-email-flags.vala +++ b/src/engine/api/geary-email-flags.vala @@ -4,97 +4,44 @@ * (version 2.1 or later). See the COPYING file in this distribution. */ -public class Geary.EmailFlags : BaseObject, Gee.Hashable { - private static EmailFlag? _unread = null; - public static EmailFlag UNREAD { get { +/** + * A collection of NamedFlags that can be used to enable/disable various user-defined + * options for an email message. System- or Geary-defined flags are available as static + * members. + * + * Note that how flags are represented by a particular email storage system may differ from + * how they're presented here. In particular, the manner of serializing and deserializing + * the flags may be handled by an internal subclass. + */ + +public class Geary.EmailFlags : Geary.NamedFlags { + private static NamedFlag? _unread = null; + public static NamedFlag UNREAD { get { if (_unread == null) - _unread = new EmailFlag("UNREAD"); + _unread = new NamedFlag("UNREAD"); return _unread; } } - private static EmailFlag? _flagged = null; - public static EmailFlag FLAGGED { get { + private static NamedFlag? _flagged = null; + public static NamedFlag FLAGGED { get { if (_flagged == null) - _flagged = new EmailFlag("FLAGGED"); + _flagged = new NamedFlag("FLAGGED"); return _flagged; } } - private static EmailFlag? _load_remote_images = null; - public static EmailFlag LOAD_REMOTE_IMAGES { get { + private static NamedFlag? _load_remote_images = null; + public static NamedFlag LOAD_REMOTE_IMAGES { get { if (_load_remote_images == null) - _load_remote_images = new EmailFlag("LOADREMOTEIMAGES"); + _load_remote_images = new NamedFlag("LOADREMOTEIMAGES"); return _load_remote_images; } } - private Gee.Set list = new Gee.HashSet(); - - public virtual signal void added(Gee.Collection flags) { - } - - public virtual signal void removed(Gee.Collection flags) { - } - public EmailFlags() { } - protected virtual void notify_added(Gee.Collection flags) { - added(flags); - } - - protected virtual void notify_removed(Gee.Collection flags) { - removed(flags); - } - - public bool contains(EmailFlag flag) { - return list.contains(flag); - } - - public Gee.Set get_all() { - return list.read_only_view; - } - - public virtual void add(EmailFlag flag) { - if (!list.contains(flag)) { - list.add(flag); - notify_added(new Collection.SingleItem(flag)); - } - } - - public virtual void add_all(EmailFlags flags) { - Gee.ArrayList added = new Gee.ArrayList(); - foreach (EmailFlag flag in flags.get_all()) { - if (!list.contains(flag)) - added.add(flag); - } - - list.add_all(added); - notify_added(added); - } - - public virtual bool remove(EmailFlag flag) { - bool removed = list.remove(flag); - if (removed) - notify_removed(new Collection.SingleItem(flag)); - - return removed; - } - - public virtual bool remove_all(EmailFlags flags) { - Gee.ArrayList removed = new Gee.ArrayList(); - foreach (EmailFlag flag in flags.get_all()) { - if (list.contains(flag)) - removed.add(flag); - } - - list.remove_all(removed); - notify_removed(removed); - - return removed.size > 0; - } - // Convenience method to check if the unread flag is set. public inline bool is_unread() { return contains(UNREAD); @@ -107,33 +54,5 @@ public class Geary.EmailFlags : BaseObject, Gee.Hashable { public inline bool load_remote_images() { return contains(LOAD_REMOTE_IMAGES); } - - public bool equal_to(Geary.EmailFlags other) { - if (this == other) - return true; - - if (list.size != other.list.size) - return false; - - foreach (EmailFlag flag in list) { - if (!other.contains(flag)) - return false; - } - - return true; - } - - public uint hash() { - return Geary.String.stri_hash(to_string()); - } - - public string to_string() { - string ret = "["; - foreach (EmailFlag flag in list) { - ret += flag.to_string() + " "; - } - - return ret + "]"; - } } diff --git a/src/engine/api/geary-email-flag.vala b/src/engine/api/geary-named-flag.vala similarity index 54% rename from src/engine/api/geary-email-flag.vala rename to src/engine/api/geary-named-flag.vala index f25277b3..13534c81 100644 --- a/src/engine/api/geary-email-flag.vala +++ b/src/engine/api/geary-named-flag.vala @@ -4,14 +4,20 @@ * (version 2.1 or later). See the COPYING file in this distribution. */ -public class Geary.EmailFlag : BaseObject, Gee.Hashable { +/** + * Geary offers a couple of places where the user may mark an object (email, contact) + * with a named flag. The presence of the flag indicates if the state is enabled/on + * or disabled/off. + */ + +public class Geary.NamedFlag : BaseObject, Gee.Hashable { private string name; - public EmailFlag(string name) { + public NamedFlag(string name) { this.name = name; } - public bool equal_to(Geary.EmailFlag other) { + public bool equal_to(Geary.NamedFlag other) { if (this == other) return true; @@ -22,6 +28,10 @@ public class Geary.EmailFlag : BaseObject, Gee.Hashable { return name.down().hash(); } + public string serialize() { + return name; + } + public string to_string() { return name; } diff --git a/src/engine/api/geary-named-flags.vala b/src/engine/api/geary-named-flags.vala new file mode 100644 index 00000000..0592ecec --- /dev/null +++ b/src/engine/api/geary-named-flags.vala @@ -0,0 +1,107 @@ +/* Copyright 2011-2013 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. + */ + +/** + * A signalled collection of NamedFlags. Currently Geary uses these flags for enabling/disabling + * options with email or contacts. + */ + +public class Geary.NamedFlags : BaseObject, Gee.Hashable { + protected Gee.Set list = new Gee.HashSet(); + + public virtual signal void added(Gee.Collection flags) { + } + + public virtual signal void removed(Gee.Collection flags) { + } + + public NamedFlags() { + } + + protected virtual void notify_added(Gee.Collection flags) { + added(flags); + } + + protected virtual void notify_removed(Gee.Collection flags) { + removed(flags); + } + + public bool contains(NamedFlag flag) { + return list.contains(flag); + } + + public Gee.Set get_all() { + return list.read_only_view; + } + + public virtual void add(NamedFlag flag) { + if (!list.contains(flag)) { + list.add(flag); + notify_added(new Collection.SingleItem(flag)); + } + } + + public virtual void add_all(NamedFlags flags) { + Gee.ArrayList added = new Gee.ArrayList(); + foreach (NamedFlag flag in flags.get_all()) { + if (!list.contains(flag)) + added.add(flag); + } + + list.add_all(added); + notify_added(added); + } + + public virtual bool remove(NamedFlag flag) { + bool removed = list.remove(flag); + if (removed) + notify_removed(new Collection.SingleItem(flag)); + + return removed; + } + + public virtual bool remove_all(NamedFlags flags) { + Gee.ArrayList removed = new Gee.ArrayList(); + foreach (NamedFlag flag in flags.get_all()) { + if (list.contains(flag)) + removed.add(flag); + } + + list.remove_all(removed); + notify_removed(removed); + + return removed.size > 0; + } + + public bool equal_to(Geary.NamedFlags other) { + if (this == other) + return true; + + if (list.size != other.list.size) + return false; + + foreach (NamedFlag flag in list) { + if (!other.contains(flag)) + return false; + } + + return true; + } + + public uint hash() { + return Geary.String.stri_hash(to_string()); + } + + public string to_string() { + string ret = "["; + foreach (NamedFlag flag in list) { + ret += flag.to_string() + " "; + } + + return ret + "]"; + } +} + diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala index f8653e58..09a4de0a 100644 --- a/src/engine/imap-db/imap-db-account.vala +++ b/src/engine/imap-db/imap-db-account.vala @@ -23,11 +23,11 @@ private class Geary.ImapDB.Account : BaseObject { private ImapDB.Database? db = null; private Gee.HashMap folder_refs = new Gee.HashMap(); - public ContactStore contact_store { get; private set; } + public ImapEngine.ContactStore contact_store { get; private set; } public Account(Geary.AccountInformation account_information) { this.account_information = account_information; - contact_store = new ContactStore(); + contact_store = new ImapEngine.ContactStore(this); name = "IMAP database account for %s".printf(account_information.imap_credentials.user); } @@ -262,14 +262,14 @@ private class Geary.ImapDB.Account : BaseObject { Db.TransactionOutcome outcome = db.exec_transaction(Db.TransactionType.RO, (context) => { Db.Statement statement = context.prepare( - "SELECT email, real_name, highest_importance, normalized_email " + + "SELECT email, real_name, highest_importance, normalized_email, flags " + "FROM ContactTable"); Db.Result result = statement.exec(cancellable); while (!result.finished) { try { Contact contact = new Contact(result.string_at(0), result.string_at(1), - result.int_at(2), result.string_at(3)); + result.int_at(2), result.string_at(3), ContactFlags.deserialize(result.string_at(4))); contacts.add(contact); } catch (Geary.DatabaseError err) { // We don't want to abandon loading all contacts just because there was a @@ -552,6 +552,21 @@ private class Geary.ImapDB.Account : BaseObject { return email; } + public async void update_contact_flags_async(Geary.Contact contact, Cancellable? cancellable) + throws Error{ + check_open(); + + yield db.exec_transaction_async(Db.TransactionType.RW, (cx, cancellable) => { + Db.Statement update_stmt = + cx.prepare("UPDATE ContactTable SET flags=? WHERE email=?"); + update_stmt.bind_string(0, contact.contact_flags.serialize()); + update_stmt.bind_string(1, contact.email); + update_stmt.exec(cancellable); + + return Db.TransactionOutcome.COMMIT; + }, cancellable); + } + private void clear_duplicate_folders() { int count = 0; diff --git a/src/engine/imap-db/imap-db-contact.vala b/src/engine/imap-db/imap-db-contact.vala index 9568575c..f109328f 100644 --- a/src/engine/imap-db/imap-db-contact.vala +++ b/src/engine/imap-db/imap-db-contact.vala @@ -6,19 +6,64 @@ namespace Geary.ImapDB { -private static void do_update_contact_importance(Db.Connection connection, Contact contact, - Cancellable? cancellable = null) throws Error { - // TODO: Don't overwrite a non-null real_name with a null real_name. - Db.Statement statement = connection.prepare( - "INSERT OR REPLACE INTO ContactTable(normalized_email, email, real_name, highest_importance) " - + "VALUES(?, ?, ?, MAX(COALESCE((SELECT highest_importance FROM ContactTable " - + "WHERE email=?1), -1), ?))"); - statement.bind_string(0, contact.normalized_email); - statement.bind_string(1, contact.email); - statement.bind_string(2, contact.real_name); - statement.bind_int(3, contact.highest_importance); +private Contact? do_fetch_contact(Db.Connection cx, string email, Cancellable? cancellable) + throws Error { + Db.Statement stmt = cx.prepare( + "SELECT real_name, highest_importance, normalized_email, flags FROM ContactTable " + + "WHERE email=?"); + stmt.bind_string(0, email); - statement.exec(cancellable); + Db.Result result = stmt.exec(cancellable); + if (result.finished) + return null; + + return new Contact(email, result.string_at(0), result.int_at(1), result.string_at(2), + ContactFlags.deserialize(result.string_at(3))); +} + +// Insert or update a contact in the ContactTable. If contact already exists, flags are merged +// and the importance is updated to the highest importance seen. +private void do_update_contact(Db.Connection connection, Contact contact, + Cancellable? cancellable) throws Error { + Contact? existing_contact = do_fetch_contact(connection, contact.email, cancellable); + + // If not found, insert and done + if (existing_contact == null) { + Db.Statement stmt = connection.prepare( + "INSERT INTO ContactTable(normalized_email, email, real_name, flags, highest_importance) " + + "VALUES(?, ?, ?, ?, ?)"); + stmt.bind_string(0, contact.normalized_email); + stmt.bind_string(1, contact.email); + stmt.bind_string(2, contact.real_name); + stmt.bind_string(3, (contact.contact_flags != null) ? contact.contact_flags.serialize() : null); + stmt.bind_int(4, contact.highest_importance); + + stmt.exec(cancellable); + + return; + } + + // merge two flags sets together + ContactFlags? merged_flags = contact.contact_flags; + if (existing_contact.contact_flags != null) { + if (merged_flags != null) + merged_flags.add_all(existing_contact.contact_flags); + else + merged_flags = existing_contact.contact_flags; + } + + // update remaining fields, careful not to overwrite non-null real_name with null (but + // using latest real_name if supplied) ... email is not updated (it's how existing_contact was + // keyed), normalized_email is inserted at the same time as email, leaving only real_name, + // flags, and highest_importance + Db.Statement stmt = connection.prepare( + "UPDATE ContactTable SET real_name=?, flags=?, highest_importance=? WHERE email=?"); + stmt.bind_string(0, !String.is_empty(contact.real_name) ? contact.real_name : existing_contact.real_name); + stmt.bind_string(1, (merged_flags != null) ? merged_flags.serialize() : null); + stmt.bind_int(2, int.max(contact.highest_importance, existing_contact.highest_importance)); + stmt.bind_string(3, contact.email); + + stmt.exec(cancellable); } } diff --git a/src/engine/imap-db/imap-db-database.vala b/src/engine/imap-db/imap-db-database.vala index 72878584..8f2828c1 100644 --- a/src/engine/imap-db/imap-db-database.vala +++ b/src/engine/imap-db/imap-db-database.vala @@ -43,7 +43,7 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase { MessageAddresses message_addresses = new MessageAddresses.from_result(account_owner_email, result); foreach (Contact contact in message_addresses.contacts) - do_update_contact_importance(get_master_connection(), contact); + do_update_contact(get_master_connection(), contact, null); result.next(); } } catch (Error err) { diff --git a/src/engine/imap-db/imap-db-folder.vala b/src/engine/imap-db/imap-db-folder.vala index 733dde14..e993404b 100644 --- a/src/engine/imap-db/imap-db-folder.vala +++ b/src/engine/imap-db/imap-db-folder.vala @@ -559,12 +559,12 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics { Geary.Imap.EmailFlags flags = ((Geary.Imap.EmailFlags) map.get(id)); if (flags_to_add != null) { - foreach (Geary.EmailFlag flag in flags_to_add.get_all()) + foreach (Geary.NamedFlag flag in flags_to_add.get_all()) flags.add(flag); } if (flags_to_remove != null) { - foreach (Geary.EmailFlag flag in flags_to_remove.get_all()) + foreach (Geary.NamedFlag flag in flags_to_remove.get_all()) flags.remove(flag); } } @@ -912,7 +912,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics { MessageAddresses message_addresses = new MessageAddresses.from_email(account_owner_email, email); foreach (Contact contact in message_addresses.contacts) - do_update_contact_importance(cx, contact, cancellable); + do_update_contact(cx, contact, cancellable); updated_contacts = message_addresses.contacts; return true; @@ -1238,7 +1238,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics { MessageAddresses message_addresses = new MessageAddresses.from_row(account_owner_email, row); foreach (Geary.Contact contact in message_addresses.contacts) - do_update_contact_importance(cx, contact, cancellable); + do_update_contact(cx, contact, cancellable); updated_contacts = message_addresses.contacts; } diff --git a/src/engine/imap-engine/imap-engine-contact-store.vala b/src/engine/imap-engine/imap-engine-contact-store.vala new file mode 100644 index 00000000..0972d3c5 --- /dev/null +++ b/src/engine/imap-engine/imap-engine-contact-store.vala @@ -0,0 +1,26 @@ +/* Copyright 2013 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. + */ + +internal class Geary.ImapEngine.ContactStore : Geary.ContactStore { + private weak ImapDB.Account account; + + internal ContactStore(ImapDB.Account account) { + this.account = account; + } + + public override async void mark_contacts_async(Gee.Collection contacts, ContactFlags? to_add, + ContactFlags? to_remove) throws Error{ + foreach (Contact contact in contacts) { + if (to_add != null) + contact.contact_flags.add_all(to_add); + + if (to_remove != null) + contact.contact_flags.remove_all(to_remove); + + yield account.update_contact_flags_async(contact, null); + } + } +} diff --git a/src/engine/imap/api/imap-email-flags.vala b/src/engine/imap/api/imap-email-flags.vala index 90808b45..4e996bde 100644 --- a/src/engine/imap/api/imap-email-flags.vala +++ b/src/engine/imap/api/imap-email-flags.vala @@ -20,8 +20,8 @@ public class Geary.Imap.EmailFlags : Geary.EmailFlags { add(LOAD_REMOTE_IMAGES); } - protected override void notify_added(Gee.Collection added) { - foreach (EmailFlag flag in added) { + protected override void notify_added(Gee.Collection added) { + foreach (NamedFlag flag in added) { if (flag.equal_to(UNREAD)) message_flags.remove(MessageFlag.SEEN); @@ -35,8 +35,8 @@ public class Geary.Imap.EmailFlags : Geary.EmailFlags { base.notify_added(added); } - protected override void notify_removed(Gee.Collection removed) { - foreach (EmailFlag flag in removed) { + protected override void notify_removed(Gee.Collection removed) { + foreach (NamedFlag flag in removed) { if (flag.equal_to(UNREAD)) message_flags.add(MessageFlag.SEEN); From 4e0d2a9958c35488516432f29b2073f190abe276 Mon Sep 17 00:00:00 2001 From: Jim Nelson Date: Wed, 15 May 2013 12:39:55 -0700 Subject: [PATCH 3/7] Valadoc for email_count_changed refers to nonexistant signals: Closes #6949 Fixed, plus other signals and enums docs cleaned up. --- src/engine/api/geary-folder.vala | 129 ++++++++++++++++++------------- 1 file changed, 75 insertions(+), 54 deletions(-) diff --git a/src/engine/api/geary-folder.vala b/src/engine/api/geary-folder.vala index 34c9ac5e..c6bfc266 100644 --- a/src/engine/api/geary-folder.vala +++ b/src/engine/api/geary-folder.vala @@ -21,7 +21,10 @@ public interface Geary.Folder : BaseObject { } /** - * The "closed" signal will be fired multiple times after a Folder is opened. It is fired + * Provides the reason why the folder is closing or closed when the {@link closed} signal + * is fired. + * + * The closed signal will be fired multiple times after a Folder is opened. It is fired * after the remote and local sessions close for various reasons, and fires once and only * once when the folder is completely closed. * @@ -29,7 +32,7 @@ public interface Geary.Folder : BaseObject { * value. The same is true for REMOTE_CLOSE and REMOTE_ERROR. A REMOTE_ERROR can trigger * a LOCAL_CLOSE and vice-versa. The values may be called in any order. * - * When the local and remote stores have closed (either normally or due to errors, FOLDER_CLOSED + * When the local and remote stores have closed (either normally or due to errors), FOLDER_CLOSED * will be sent. */ public enum CloseReason { @@ -50,16 +53,17 @@ public interface Geary.Folder : BaseObject { } /** - * Flags describing open modifiers. - * - * * FAST_OPEN: perform the minimal amount of activity possible to open the folder - * and be synchronized with the server. This may mean some attributes of - * the messages (such as their flags or other metadata) may not be up-to-date - * when the folder opens. Not all folders will support this flag. + * Flags modifying the behavior of open_async(). */ [Flags] public enum OpenFlags { NONE = 0, + /** + * Perform the minimal amount of activity possible to open the folder + * and be synchronized with the server. This may mean some attributes of + * the messages (such as their flags or other metadata) may not be up-to-date + * when the folder opens. Not all folders will support this flag. + */ FAST_OPEN; public bool is_any_set(OpenFlags flags) { @@ -72,17 +76,23 @@ public interface Geary.Folder : BaseObject { } /** - * Flags used for retrieving email. - * - * * LOCAL_ONLY: fetch from the local store only - * * FORCE_UPDATE: fetch from remote only (results merged into local store) - * * EXCLUDING_ID: exclude the provided ID + * Flags modifying how email is retrieved. */ [Flags] public enum ListFlags { NONE = 0, + /** + * Fetch from the local store only. + */ LOCAL_ONLY, + /** + * Fetch from remote store only (results merged into local store). + */ FORCE_UPDATE, + /** + * Exclude the provided EmailIdentifier (only respected by {@link list_email_by_id_async} and + * {@link lazy_list_email_by_id}). + */ EXCLUDING_ID; public bool is_any_set(ListFlags flags) { @@ -97,28 +107,32 @@ public interface Geary.Folder : BaseObject { public abstract Geary.Account account { get; } /** - * This is fired when the Folder is successfully opened by a caller. It will only fire once - * until the Folder is closed, with the OpenState indicating what has been opened and the count - * indicating the number of messages in the folder (in the case of OpenState.BOTH or - * OpenState.REMOTE, it refers to the authoritative number). + * Fired when the folder is successfully opened by a caller. * - * OpenState.REMOTE will only fire if there's no local store, indicating that it's not a - * synchronized folder but rather one entirely backed by a network server. Geary currently - * has no such folder implemented like this. + * It will only fire once until the Folder is closed, with the {@link OpenState} indicating what + * has been opened and the count indicating the number of messages in the folder. In the case + * of {@link OpenState.BOTH} or {@link OpenState.REMOTE}, it refers to the authoritative number. + * For {@link OpenState.LOCAL}, it refers to the number of messages in the local store. * - * This signal will never fire with Geary.OpenState.CLOSED as a parameter. + * {@link OpenState.REMOTE} will only be passed if there's no local store, indicating that it's + * not a synchronized folder but rather one entirely backed by a network server. Geary + * currently has no such folder implemented like this. + * + * This signal will never fire with {@link OpenState.CLOSED} as a parameter. + * + * @see get_open_state */ public signal void opened(OpenState state, int count); /** - * This is fired when open_async() fails for one or more reasons. + * Fired when {@link open_async} fails for one or more reasons. * - * See open_async() and "opened" for more information on how opening a Folder works, in particular - * how open_async() may return immediately although the remote has not completely opened. - * This signal may be called in the context of, or after completion of, open_async(). It will - * *not* be called after close_async() has completed, however. + * See open_async and {@link opened} for more information on how opening a Folder works, i particular + * how open_async may return immediately although the remote has not completely opened. + * This signal may be called in the context of, or after completion of, open_async. It will + * ''not'' be called after {@link close_async} has completed, however. * - * Note that this signal may be fired *and* open_async() throw an Error. + * Note that this signal may be fired ''and'' open_async throw an Error. * * This signal may be fired more than once before the Folder is closed. It will only fire once * for each type of failure, however. @@ -126,66 +140,73 @@ public interface Geary.Folder : BaseObject { public signal void open_failed(OpenFailed failure, Error? err); /** - * This is fired when the Folder is closed, either by the caller or due to errors in the local - * or remote store(s). It will fire three times: to report how the local store closed + * Fired when the Folder is closed, either by the caller or due to errors in the local + * or remote store(s). + * + * It will fire three times: to report how the local store closed * (gracefully or due to error), how the remote closed (similarly) and finally with - * FOLDER_CLOSED. The first two may come in either order; the third is always the last. + * {@link CloseReason.FOLDER_CLOSED}. The first two may come in either order; the third is + * always the last. */ public signal void closed(CloseReason reason); /** - * "email-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). + * Fired when email has been appended to the list of messages in the folder. + * + * The {@link EmailIdentifier} for all appended messages is supplied as a signal parameter. + * Email positions remain valid, but the total count of the messages in the folder has changed. + * + * @see email_locally_appended */ public signal void email_appended(Gee.Collection ids); /** - * "email-locally-appended" is fired when previously unknown messages have been appended to the - * list of messages in the folder. This is similar to "email-appended", but that signal - * lists all messages appended to the folder. "email-locally-appended" only reports emails that + * Fired when previously unknown messages have been appended to the list of email in the folder. + * + * This is similar to {@link email_appended}, but that signal + * lists ''all'' messages appended to the folder. email_locally_appended only reports email that * have not been seen prior. Hence, an email that is removed from the folder and returned * later will not be listed here (unless it was removed from the local store in the meantime). * * Note that these messages were appended as well, hence their positional addressing may have - * changed since last seen in this folder. However, it's important to realize that this list - * does *not* represent all newly appended messages. + * changed since last seen in this folder. + * + * @see email_appended */ public signal void email_locally_appended(Gee.Collection ids); /** - * "email-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). + * Fired when email has been removed (deleted or moved) from the folder. * - * NOTE: It's possible for the remote server to report a message has been removed that is not + * This may occur due to the local user's action or reported from the server (i.e. another + * client has performed the action). Email positions greater than the removed emails are + * affected. + * + * ''Note:'' It's possible for the remote server to report a message has been removed that is not * known locally (and therefore the caller could not have record of). If this happens, this - * signal will *not* fire, although "email-count-changed" will. + * signal will ''not'' fire, although {@link email_count_changed} will. */ public signal void email_removed(Gee.Collection ids); /** - * "email-count-changed" is fired when the total count of email in a folder has changed in any way. + * Fired when the total count of email in a folder has changed in any way. * - * Note that this signal will be fired alongside "messages-appended" or "message-removed". - * That is, do not use both signals to process email count changes; one will suffice. - * This signal will fire after those (although see the note at "messages-removed"). + * Note that this signal will fire after {@link email_appended}, {@link email_locally_appended}, + * and {@link email_removed} (although see the note at email_removed). */ public signal void email_count_changed(int new_count, CountChangeReason reason); /** - * "email-flags-changed" is fired when an email's flag changed. - * - * This signal will be fired both when changes occur on the client side via the - * mark_email_async() method as well as changes occur remotely. + * Fired when the supplied email flags have changed, whether due to local action or reported by + * the server. */ public signal void email_flags_changed(Gee.Map map); /** - * "special-folder-type-changed" is fired when the special folder type has changed. + * Fired when the {@link SpecialFolderType} has changed. * - * This will usually happen when the local account object has been updated with data - * from the remote account. + * This will usually happen when the local object has been updated with data discovered from the + * remote account. */ public signal void special_folder_type_changed(Geary.SpecialFolderType old_type, Geary.SpecialFolderType new_type); From 1b946da34b94924e2abdacfeb546a5608bc2ecaf Mon Sep 17 00:00:00 2001 From: Jim Nelson Date: Thu, 16 May 2013 11:23:27 -0700 Subject: [PATCH 4/7] Clean up log output: Closes #6730, Closes #5944 Inserting the git commit number a build time is not included here; that's going to be more problematic than it sounds, especially if the number needs to travel along in a tarball or made available on the bzr repo on Launchpad. --- src/client/geary-application.vala | 11 +++++- src/engine/api/geary-engine.vala | 1 + src/engine/api/geary-logging.vala | 60 +++++++++++++++++++++++++------ 3 files changed, 61 insertions(+), 11 deletions(-) diff --git a/src/client/geary-application.vala b/src/client/geary-application.vala index bd60677c..001f4e94 100644 --- a/src/client/geary-application.vala +++ b/src/client/geary-application.vala @@ -94,6 +94,7 @@ along with Geary; if not, write to the Free Software Foundation, Inc., public override int startup() { exec_dir = (File.new_for_path(Environment.find_program_in_path(args[0]))).get_parent(); + Geary.Logging.init(); Configuration.init(is_installed(), GSETTINGS_DIR); Date.init(); WebKit.set_cache_model(WebKit.CacheModel.DOCUMENT_BROWSER); @@ -102,7 +103,15 @@ along with Geary; if not, write to the Free Software Foundation, Inc., if (ec != 0) return ec; - return Args.parse(args); + ec = Args.parse(args); + if (ec != 0) + return ec; + + // do *after* parsing args, as they dicate where logging is sent to, if anywhere + message("%s %s prefix=%s exec_dir=%s is_installed=%s", NAME, VERSION, INSTALL_PREFIX, + exec_dir.get_path(), is_installed().to_string()); + + return 0; } public override bool exiting(bool panicked) { diff --git a/src/engine/api/geary-engine.vala b/src/engine/api/geary-engine.vala index f79a4e4b..5de77e18 100644 --- a/src/engine/api/geary-engine.vala +++ b/src/engine/api/geary-engine.vala @@ -89,6 +89,7 @@ public class Geary.Engine : BaseObject { is_initialized = true; + Logging.init(); RFC822.init(); ImapEngine.init(); Imap.init(); diff --git a/src/engine/api/geary-logging.vala b/src/engine/api/geary-logging.vala index 3c96d409..8368d4a2 100644 --- a/src/engine/api/geary-logging.vala +++ b/src/engine/api/geary-logging.vala @@ -4,6 +4,13 @@ * (version 2.1 or later). See the COPYING file in this distribution. */ +/** + * Provides some control over Engine logging. + * + * This is a crude implementation and could be improved. Most importantly, when the Engine is + * initialized logging is disabled for all users of GLib's logging functions. + */ + namespace Geary.Logging { [Flags] @@ -26,9 +33,23 @@ public enum Flag { } } +private int init_count = 0; private Flag logging_flags = Flag.NONE; private unowned FileStream? stream = null; +/** + * Must be called before ''any'' call to the Logging namespace. + * + * This will be initialized by the Engine when it's opened, but applications may want to set up + * logging before that, in which case, call this directly. + */ +public void init() { + if (init_count++ != 0) + return; + + log_to(null); +} + /** * Replaces the current logging flags with flags. Use Geary.Logging.Flag.NONE to clear all * logging flags. @@ -90,19 +111,38 @@ public inline void debug(Flag flags, string fmt, ...) { logv(null, LogLevelFlags.LEVEL_DEBUG, fmt, va_list()); } -public void log_to(FileStream stream) { +/** + * Registers a FileStream to receive all log output from the Engine, be it via the specialized + * Logging calls (which use the topic-based {@link Flag} or GLib's standard issue + * debug/message/error/etc. calls ... thus, calling this will also affect the Engine user's calls + * to those functions. + * + * If stream is null, no logging occurs. This is default. + */ +public void log_to(FileStream? stream) { Logging.stream = stream; - // TODO: Should handle all LogLevels - Log.set_handler(null, LogLevelFlags.LEVEL_DEBUG, on_log_debug); + Log.set_handler(null, LogLevelFlags.LEVEL_DEBUG, + (domain, levels, msg) => { on_log(" [deb]", levels, msg); }); + Log.set_handler(null, LogLevelFlags.LEVEL_INFO, + (domain, levels, msg) => { on_log(" [inf]", levels, msg); }); + Log.set_handler(null, LogLevelFlags.LEVEL_MESSAGE, + (domain, levels, msg) => { on_log(" [msg]", levels, msg); }); + Log.set_handler(null, LogLevelFlags.LEVEL_WARNING, + (domain, levels, msg) => { on_log("*[wrn]", levels, msg); }); + Log.set_handler(null, LogLevelFlags.LEVEL_CRITICAL, + (domain, levels, msg) => { on_log("![crt]", levels, msg); }); + Log.set_handler(null, LogLevelFlags.LEVEL_ERROR, + (domain, levels, msg) => { on_log("![err]", levels, msg); }); +} + +private void on_log(string prefix, LogLevelFlags log_levels, string message) { + if (stream == null) + return; + + Time tm = Time.local(time_t()); + stream.printf("%s %02d:%02d:%02d %s\n", prefix, tm.hour, tm.minute, tm.second, message); } -private void on_log_debug(string? log_domain, LogLevelFlags log_levels, string message) { - if (stream != null) { - Time tm = Time.local(time_t()); - stream.printf(" \x001b[%dm[deb]\x001b[0m %02d:%02d:%02d %s\n", 2 + 30 + 60, tm.hour, - tm.minute, tm.second, message); - } } -} // namespace From 850be85af712b13fb548141841bf00a8d0b6c5ee Mon Sep 17 00:00:00 2001 From: Robert Schroll Date: Thu, 16 May 2013 16:45:24 -0700 Subject: [PATCH 5/7] Include full name in header mailto: links: Closes #6951 --- src/client/views/conversation-viewer.vala | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/client/views/conversation-viewer.vala b/src/client/views/conversation-viewer.vala index 9e0f2487..f4841e41 100644 --- a/src/client/views/conversation-viewer.vala +++ b/src/client/views/conversation-viewer.vala @@ -725,13 +725,17 @@ public class ConversationViewer : Gtk.Box { for (ulong i = 0; i < nodes.length; i ++) { WebKit.DOM.Element? email_element = nodes.item(i) as WebKit.DOM.Element; if (email_element != null) { - WebKit.DOM.Element? address = email_element.query_selector(".address_name"); - if (address != null) { - WebKit.DOM.Element? mailto_link = address.parent_node as WebKit.DOM.Element; - if (mailto_link != null && contact.normalized_email == - mailto_link.get_attribute("href").substring(7).normalize().casefold()) - conversation_viewer.show_images_email(email_element, false); + string? address = null; + WebKit.DOM.Element? address_el = email_element.query_selector(".address_value"); + if (address_el != null) { + address = ((WebKit.DOM.HTMLElement) address_el).get_inner_text(); + } else { + address_el = email_element.query_selector(".address_name"); + if (address_el != null) + address = ((WebKit.DOM.HTMLElement) address_el).get_inner_text(); } + if (address != null && address.normalize().casefold() == contact.normalized_email) + conversation_viewer.show_images_email(email_element, false); } } } catch (Error error) { @@ -1177,11 +1181,13 @@ public class ConversationViewer : Gtk.Box { string value = ""; Gee.List list = addresses.get_all(); foreach (Geary.RFC822.MailboxAddress a in list) { - value += "".printf(a.address); if (a.name != null) { + value += "".printf( + Uri.escape_string("%s <%s>".printf(a.name, a.address))); value += "%s ".printf(a.name); value += "%s".printf(a.address); } else { + value += "".printf(a.address); value += "%s".printf(a.address); } value += ""; @@ -1254,7 +1260,7 @@ public class ConversationViewer : Gtk.Box { private void on_hovering_over_link(string? title, string? url) { // Copy the link the user is hovering over. Note that when the user mouses-out, // this signal is called again with null for both parameters. - hover_url = url; + hover_url = url != null ? Uri.unescape_string(url) : null; message_overlay_label.label = hover_url; } From 539c3bcdd9c498e847bb1c44771d8a5bdb96ce19 Mon Sep 17 00:00:00 2001 From: Jim Nelson Date: Thu, 16 May 2013 16:50:13 -0700 Subject: [PATCH 6/7] Prevent opening log entry from printing on subsequent runs of Geary The geary process can start multiple times to send commands via the command-line to a running application. The addition of the entry log entry was being printed with each execution, not just for the running application. --- src/client/geary-application.vala | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/client/geary-application.vala b/src/client/geary-application.vala index 001f4e94..b78d0b88 100644 --- a/src/client/geary-application.vala +++ b/src/client/geary-application.vala @@ -103,15 +103,7 @@ along with Geary; if not, write to the Free Software Foundation, Inc., if (ec != 0) return ec; - ec = Args.parse(args); - if (ec != 0) - return ec; - - // do *after* parsing args, as they dicate where logging is sent to, if anywhere - message("%s %s prefix=%s exec_dir=%s is_installed=%s", NAME, VERSION, INSTALL_PREFIX, - exec_dir.get_path(), is_installed().to_string()); - - return 0; + return Args.parse(args); } public override bool exiting(bool panicked) { @@ -137,7 +129,13 @@ along with Geary; if not, write to the Free Software Foundation, Inc., handle_args(args); return; } - + + // do *after* parsing args, as they dicate where logging is sent to, if anywhere, and only + // after activate (which means this is only logged for the one user-visible instance, not + // the other instances called when sending commands to the app via the command-line) + message("%s %s prefix=%s exec_dir=%s is_installed=%s", NAME, VERSION, INSTALL_PREFIX, + exec_dir.get_path(), is_installed().to_string()); + Geary.Engine.instance.account_available.connect(on_account_available); Geary.Engine.instance.account_unavailable.connect(on_account_unavailable); From f2c4c947edcbbba074a6b213e01f38f95e1eb23b Mon Sep 17 00:00:00 2001 From: Charles Lindsay Date: Thu, 16 May 2013 18:26:02 -0700 Subject: [PATCH 7/7] Add attachments in local search; fix #6954 --- src/engine/imap-db/imap-db-account.vala | 1 + 1 file changed, 1 insertion(+) diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala index 09a4de0a..be6f1f44 100644 --- a/src/engine/imap-db/imap-db-account.vala +++ b/src/engine/imap-db/imap-db-account.vala @@ -496,6 +496,7 @@ private class Geary.ImapDB.Account : BaseObject { // Ignore any messages that don't have the required fields. if (partial_ok || row.fields.fulfills(requested_fields)) { Geary.Email email = row.to_email(-1, new Geary.ImapDB.EmailIdentifier(id)); + Geary.ImapDB.Folder.do_add_attachments(cx, email, id, cancellable); Gee.Set? folders = do_find_email_folders(cx, id, cancellable); if (folders == null) {