From 4cea0fb0d24da3c73459e9da5a5bb59dbd43e41b Mon Sep 17 00:00:00 2001 From: Jim Nelson Date: Thu, 10 Nov 2011 13:20:48 -0800 Subject: [PATCH] Removed messages are now removed from conversations: Closes #4347 Conversations now fully monitor the Folder for both additions and removals of messages. This exposed a couple of bugs in the database code, which are fixed here as well. I also used this opportunity to clean up some of the internal Conversations code, simplifying Node management. #4333 will be a big boost here when implemented. --- src/client/ui/main-window.vala | 12 ++ src/client/ui/message-list-store.vala | 23 +- src/engine/api/geary-conversation.vala | 56 +++++ src/engine/api/geary-conversations.vala | 202 +++++++++++++++--- src/engine/api/geary-email-identifier.vala | 8 +- src/engine/api/geary-folder.vala | 13 +- .../imap/api/imap-email-identifier.vala | 12 +- src/engine/imap/api/imap-folder.vala | 6 + src/engine/impl/geary-engine-folder.vala | 35 +-- src/engine/sqlite/api/sqlite-folder.vala | 71 +++--- .../sqlite/email/sqlite-message-table.vala | 2 + src/engine/util/util-interfaces.vala | 17 ++ 12 files changed, 362 insertions(+), 95 deletions(-) diff --git a/src/client/ui/main-window.vala b/src/client/ui/main-window.vala index cec67eea..e5244d60 100644 --- a/src/client/ui/main-window.vala +++ b/src/client/ui/main-window.vala @@ -189,6 +189,8 @@ public class MainWindow : Gtk.Window { current_conversations.scan_completed.connect(on_scan_completed); current_conversations.conversations_added.connect(on_conversations_added); current_conversations.conversation_appended.connect(on_conversation_appended); + current_conversations.conversation_trimmed.connect(on_conversation_trimmed); + current_conversations.conversation_removed.connect(on_conversation_removed); current_conversations.updated_placeholders.connect(on_updated_placeholders); // Do a quick-list of the messages (which should return what's in the local store) if @@ -227,6 +229,14 @@ public class MainWindow : Gtk.Window { message_list_store.update_conversation(conversation); } + public void on_conversation_trimmed(Geary.Conversation conversation, Geary.Email email) { + message_list_store.update_conversation(conversation); + } + + public void on_conversation_removed(Geary.Conversation conversation) { + message_list_store.remove_conversation(conversation); + } + public void on_updated_placeholders(Geary.Conversation conversation, Gee.Collection email) { message_list_store.update_conversation(conversation); @@ -265,6 +275,8 @@ public class MainWindow : Gtk.Window { private void on_conversation_selected(Geary.Conversation? conversation) { if (conversation != null) do_select_message.begin(conversation, on_select_message_completed); + else + message_viewer.clear(); } private async void do_select_message(Geary.Conversation conversation) throws Error { diff --git a/src/client/ui/message-list-store.vala b/src/client/ui/message-list-store.vala index 2fa16228..1dd5d442 100644 --- a/src/client/ui/message-list-store.vala +++ b/src/client/ui/message-list-store.vala @@ -54,7 +54,7 @@ public class MessageListStore : Gtk.TreeStore { Gee.SortedSet? pool = conversation.get_pool_sorted(compare_email); - if (pool != null) + if (pool != null && pool.size > 0) set(iter, Column.MESSAGE_DATA, new FormattedMessageData.from_email(pool.first(), pool.size), Column.MESSAGE_OBJECT, conversation @@ -71,6 +71,13 @@ public class MessageListStore : Gtk.TreeStore { } Gee.SortedSet? pool = conversation.get_pool_sorted(compare_email); + if (pool == null) { + // empty conversation, remove from store + remove_conversation(conversation); + + return; + } + Geary.Email preview = pool.first(); FormattedMessageData? existing = null; @@ -81,6 +88,17 @@ public class MessageListStore : Gtk.TreeStore { set(iter, Column.MESSAGE_DATA, new FormattedMessageData.from_email(preview, pool.size)); } + public void remove_conversation(Geary.Conversation conversation) { + Gtk.TreeIter iter; + if (!find_conversation(conversation, out iter)) { + // unknown, nothing to do here + return; + } + + if (!remove(iter)) + debug("Unable to remove conversation from MessageListStore"); + } + public bool has_conversation(Geary.Conversation conversation) { int count = get_count(); for (int ctr = 0; ctr < count; ctr++) { @@ -97,6 +115,9 @@ public class MessageListStore : Gtk.TreeStore { public Geary.Email? get_newest_message_at_index(int index) { Geary.Conversation? c = get_conversation_at_index(index); + if (c == null) + return null; + Gee.SortedSet? pool = c.get_pool_sorted(compare_email); return pool != null ? pool.first() : null; diff --git a/src/engine/api/geary-conversation.vala b/src/engine/api/geary-conversation.vala index 47f63fc6..f2cb80b3 100644 --- a/src/engine/api/geary-conversation.vala +++ b/src/engine/api/geary-conversation.vala @@ -14,12 +14,68 @@ public interface Geary.ConversationNode : Object { * null (it's a placeholder). */ public abstract Geary.Email? get_email(); + + /** + * Returns the Conversation the ConversationNode is a part of. + */ + public abstract Geary.Conversation get_conversation(); } public abstract class Geary.Conversation : Object { protected Conversation() { } + /** + * Returns the total number of ConversationNodes in the Conversation, both threaded and orphaned. + */ + public virtual int get_count() { + return count_conversation_nodes(false); + } + + /** + * Returns the total number of ConversationNodes *with email* in the Conversation, both threaded + * and orphaned. + */ + public virtual int get_usable_count() { + return count_conversation_nodes(true); + } + + private int count_conversation_nodes(bool usable) { + int count = 0; + + // start with origin + ConversationNode? origin = get_origin(); + if (origin != null) + count += count_nodes(origin, usable); + + // add orphans + Gee.Collection? orphans = get_orphans(); + if (orphans != null) { + foreach (ConversationNode orphan in orphans) + count += count_nodes(orphan, usable); + } + + return count; + } + + private int count_nodes(ConversationNode current, bool usable) { + // start with current Node + int count; + if (usable) + count = current.get_email() != null ? 1 : 0; + else + count = 1; + + // add to it all its children + Gee.Collection? children = get_replies(current); + if (children != null) { + foreach (ConversationNode child in children) + count += count_nodes(child, usable); + } + + return count; + } + /** * Returns a ConversationNode that is the origin of the entire conversation. * diff --git a/src/engine/api/geary-conversations.vala b/src/engine/api/geary-conversations.vala index 749aac60..5f7085c6 100644 --- a/src/engine/api/geary-conversations.vala +++ b/src/engine/api/geary-conversations.vala @@ -13,9 +13,9 @@ public class Geary.Conversations : Object { private class Node : Object, ConversationNode { public RFC822.MessageID? node_id { get; private set; } - public Geary.Email? email { get; set; } + public Geary.Email? email; public RFC822.MessageID? parent_id { get; set; default = null; } - public ImplConversation? conversation { get; set; default = null; } + public ImplConversation? conversation = null; private Gee.Set? children_ids = null; @@ -24,6 +24,11 @@ public class Geary.Conversations : Object { this.email = email; } + public Node.single(Geary.Email email) { + node_id = null; + this.email = email; + } + public void add_child(RFC822.MessageID child_id) { if (children_ids == null) { children_ids = new Gee.HashSet(Geary.Hashable.hash_func, @@ -40,24 +45,19 @@ public class Geary.Conversations : Object { public Geary.Email? get_email() { return email; } - } - - private class SingleConversationNode : Object, ConversationNode { - public Geary.Email email; - public SingleConversationNode(Geary.Email email) { - this.email = email; - } - - public Geary.Email? get_email() { - return email; + public Geary.Conversation get_conversation() { + // this method should only be called when the Conversation has been set on the Node + assert(conversation != null); + + return conversation; } } private class ImplConversation : Conversation { public weak Geary.Conversations? owner; public RFC822.MessageID? origin; - public SingleConversationNode? single_node; + public Node? single_node; private Gee.HashSet? orphans = null; @@ -67,16 +67,12 @@ public class Geary.Conversations : Object { // Rather than keep a reference to the origin node (creating a cross-reference from it // to the conversation), keep only the Message-ID, unless it's a SingleConversationNode, // which can't be addressed by a Message-ID (it has none) - single_node = origin_node as SingleConversationNode; - if (single_node != null) { - origin = null; - } else { - origin = ((Node) origin_node).node_id; - assert(origin != null); - } + origin = ((Node) origin_node).node_id; + if (origin == null) + single_node = (Node) origin_node; } - // Cannot be used for SingleConversationNodes. + // Cannot be used for Conversations with single nodes public void set_origin(ConversationNode new_origin_node) { assert(single_node == null); @@ -186,6 +182,8 @@ public class Geary.Conversations : Object { private Geary.Email.Field required_fields; private Gee.Map id_map = new Gee.HashMap(Geary.Hashable.hash_func, Geary.Equalable.equal_func); + private Gee.Map geary_id_map = new Gee.HashMap< + Geary.EmailIdentifier, Node>(Geary.Hashable.hash_func, Geary.Equalable.equal_func); private Gee.Set conversations = new Gee.HashSet(); private bool monitor_new = false; private Cancellable? cancellable_monitor = null; @@ -195,23 +193,72 @@ public class Geary.Conversations : Object { * If id is not null, then the scan is starting at an identifier and progressing according to * count (see Geary.Folder.list_email_by_id_async()). Otherwise, the scan is using positional * addressing and low is a valid one-based position (see Geary.Folder.list_email_async()). + * + * Note that more than one load can be initiated, due to Conversations being completely + * asynchronous. "scan-started", "scan-error", and "scan-completed" will be fired (as + * appropriate) for each individual load request; that is, there is no internal counter to ensure + * only a single "scan-completed" is fired to indiciate multiple loads have finished. */ public virtual signal void scan_started(Geary.EmailIdentifier? id, int low, int count) { } + /** + * "scan-error" is fired when an Error is encounted while loading messages. It will be followed + * by a "scan-completed" signal. + */ public virtual signal void scan_error(Error err) { } + /** + * "scan-completed" is fired when the scan of the email has finished. + */ public virtual signal void scan_completed() { } + /** + * "conversations-added" indicates that one or more new Conversations have been detected while + * processing email, either due to a user-initiated load request or due to monitoring. + */ public virtual signal void conversations_added(Gee.Collection conversations) { } + /** + * "conversations-removed" is fired when all the usable email in a Conversation has been removed. + * Although the Conversation structure remains intact, there's no usable Email objects in any + * ConversationNode. Conversations will then remove the Conversation object. + * + * Note that this can only occur when monitoring is enabled. There is (currently) no + * user call to manually remove email from Conversations. + */ + public virtual signal void conversation_removed(Conversation conversation) { + } + + /** + * "conversation-appended" is fired when one or more Email objects have been added to the + * specified Conversation. This can happen due to a user-initiated load or while monitoring + * the Folder. + */ public virtual signal void conversation_appended(Conversation conversation, Gee.Collection email) { } + /** + * "conversation-trimmed" is fired when an Email has been removed from the Folder, and therefore + * from the specified Conversation. If the trimmed Email is the last usable Email in the + * Conversation, this signal will be followed by "conversation-removed". + * + * There is (currently) no user-specified call to manually remove Email from Conversations. + * This is only called when monitoring is enabled. + */ + public virtual signal void conversation_trimmed(Conversation conversation, Geary.Email email) { + } + + /** + * "updated-placeholders" is fired when a ConversationNode in a Conversation was earlier + * detected (i.e. referenced by another Email) but the actual Email was not available. This + * signal indicates the Email was discovered (either by loading additional messages or from + * monitoring) and is now available. + */ public virtual signal void updated_placeholders(Conversation conversation, Gee.Collection email) { } @@ -226,8 +273,10 @@ public class Geary.Conversations : Object { foreach (ImplConversation conversation in conversations) conversation.owner = null; - if (monitor_new) + if (monitor_new) { folder.messages_appended.disconnect(on_folder_messages_appended); + folder.message_removed.disconnect(on_folder_message_removed); + } } protected virtual void notify_scan_started(Geary.EmailIdentifier? id, int low, int count) { @@ -246,11 +295,19 @@ public class Geary.Conversations : Object { conversations_added(conversations); } + protected virtual void notify_conversation_removed(Conversation conversation) { + conversation_removed(conversation); + } + protected virtual void notify_conversation_appended(Conversation conversation, Gee.Collection email) { conversation_appended(conversation, email); } + protected virtual void notify_conversation_trimmed(Conversation conversation, Geary.Email email) { + conversation_trimmed(conversation, email); + } + protected virtual void notify_updated_placeholders(Conversation conversation, Gee.Collection email) { updated_placeholders(conversation, email); @@ -267,6 +324,7 @@ public class Geary.Conversations : Object { monitor_new = true; cancellable_monitor = cancellable; folder.messages_appended.connect(on_folder_messages_appended); + folder.message_removed.connect(on_folder_message_removed); return true; } @@ -362,10 +420,15 @@ public class Geary.Conversations : Object { if (email.message_id == null) { debug("Email %s: No Message-ID", email.to_string()); - bool added = new_conversations.add( - new ImplConversation(this, new SingleConversationNode(email))); + Node singleton = new Node.single(email); + ImplConversation conversation = new ImplConversation(this, singleton); + singleton.conversation = conversation; + + bool added = new_conversations.add(conversation); assert(added); + new_email_for_node(singleton); + continue; } @@ -377,15 +440,12 @@ public class Geary.Conversations : Object { // emails refer to this one) Node? node = get_node(email.message_id); if (node != null) { - if (node.email != null) { - message("Duplicate email found while threading: %s vs. %s", node.email.to_string(), - email.to_string()); - } - // even with duplicates, this new email is considered authoritative // (Note that if the node's conversation is null then it's been added this loop, // in which case it's not an updated placeholder but part of a new conversation) node.email = email; + new_email_for_node(node); + if (node.conversation != null) updated_placeholders.set(node.conversation, email); } else { @@ -541,9 +601,79 @@ public class Geary.Conversations : Object { id_map.set(node.node_id, node); + new_email_for_node(node); + return node; } + private void new_email_for_node(Node node) { + Geary.Email? email = node.get_email(); + if (email == null) + return; + + // Possible this method will be called multiple times when processing mail (just a fact of + // life), so be sure before issuing warning + Node? replacement = geary_id_map.get(email.id); + if (replacement != null && replacement != node) { + debug("WARNING: Replacing node in conversation model with new node of same EmailIdentifier %s", + email.id.to_string()); + } + + geary_id_map.set(email.id, node); + } + + private void remove_email(Geary.EmailIdentifier removed_id) { + // Remove EmailIdentifier from map + Node node; + if (!geary_id_map.unset(removed_id, out node)) { + debug("Removed email %s not found on conversations model", removed_id.to_string()); + + return; + } + + // Drop email from the Node and signal it's been trimmed from the conversation + Geary.Email? email = node.email; + node.email = null; + + Conversation conversation = node.get_conversation(); + + if (email != null) + notify_conversation_trimmed(conversation, email); + + if (conversation.get_usable_count() == 0) { + // prune all Nodes in the conversation tree + prune_nodes((Node) conversation.get_origin()); + + // prune all orphan Nodes + Gee.Collection? orphans = conversation.get_orphans(); + if (orphans != null) { + foreach (ConversationNode orphan in orphans) + prune_nodes((Node) orphan); + } + + // remove the Conversation from the master list + bool removed = conversations.remove((ImplConversation) conversation); + assert(removed); + + // done + notify_conversation_removed(conversation); + } + } + + private void prune_nodes(Node node) { + Gee.Set? children = node.get_children(); + if (children != null) { + foreach (RFC822.MessageID child in children) { + Node? child_node = id_map.get(child); + if (child_node != null) + prune_nodes(child_node); + } + } + + bool removed = id_map.unset(node.node_id); + assert(removed); + } + private inline Node? get_node(RFC822.MessageID node_id) { return id_map.get(node_id); } @@ -582,13 +712,15 @@ public class Geary.Conversations : Object { return; } + debug("Message(s) appended to %s, fetching email above %s", folder.to_string(), + highest.to_string()); + // Want to get the one *after* the highest position in the list - Geary.EmailIdentifier next = highest.next(); - - debug("Message(s) appended to %s, fetching email at %s and above", folder.to_string(), - next.to_string()); - - lazy_load_by_id(next, int.MAX, Folder.ListFlags.NONE, cancellable_monitor); + lazy_load_by_id(highest, int.MAX, Folder.ListFlags.EXCLUDING_ID, cancellable_monitor); + } + + private void on_folder_message_removed(Geary.EmailIdentifier removed_id) { + remove_email(removed_id); } } diff --git a/src/engine/api/geary-email-identifier.vala b/src/engine/api/geary-email-identifier.vala index dffe45a0..7c8948ec 100644 --- a/src/engine/api/geary-email-identifier.vala +++ b/src/engine/api/geary-email-identifier.vala @@ -19,15 +19,13 @@ * it may not be. */ -public abstract class Geary.EmailIdentifier : Object, Geary.Equalable, Geary.Comparable { +public abstract class Geary.EmailIdentifier : Object, Geary.Equalable, Geary.Comparable, Geary.Hashable { public abstract int64 ordering { get; protected set; } - public abstract EmailIdentifier next(); - - public abstract EmailIdentifier previous(); - public abstract bool equals(Geary.Equalable other); + public abstract uint to_hash(); + public virtual int compare(Geary.Comparable o) { Geary.EmailIdentifier? other = o as Geary.EmailIdentifier; if (other == null) diff --git a/src/engine/api/geary-folder.vala b/src/engine/api/geary-folder.vala index 9bee068c..1d071007 100644 --- a/src/engine/api/geary-folder.vala +++ b/src/engine/api/geary-folder.vala @@ -27,7 +27,8 @@ public interface Geary.Folder : Object { [Flags] public enum ListFlags { NONE = 0, - FAST; + FAST, + EXCLUDING_ID; public bool is_any_set(ListFlags flags) { return (this & flags) != 0; @@ -298,6 +299,14 @@ public interface Geary.Folder : Object { * * To fetch all available messages in one direction or another, use int.MIN or int.MAX. * + * initial_id *must* be an EmailIdentifier available to the Folder for this to work, as listing + * a range inevitably requires positional addressing under the covers. However, since it's + * some times desirable to list messages excluding the specified EmailIdentifier, callers may + * use ListFlags.EXCLUDING_ID (which is a flag only recognized by this method and + * lazy_list_email_by_id()). This ListFlag *must* be supported by all Folders and will not + * necessarily be returned by get_supported_flags(). Note that this flag doesn't make sense + * when count is zero or one and will be ignored. + * * There's no guarantee of the returned messages' order. * * There is (currently) no sparse version of list_email_by_id_async(). @@ -311,7 +320,7 @@ public interface Geary.Folder : Object { /** * Similar in contract to lazy_list_email_async(), but uses Geary.EmailIdentifier rather than * positional addressing, much like list_email_by_id_async(). See that method for more - * information on its contract and how the count parameter works. + * information on its contract and how the count and flags parameters work. * * Like the other "lazy" methods, this method will call EmailCallback while the operation is * processing. This method does not block. diff --git a/src/engine/imap/api/imap-email-identifier.vala b/src/engine/imap/api/imap-email-identifier.vala index 1733a12c..b9a7cd50 100644 --- a/src/engine/imap/api/imap-email-identifier.vala +++ b/src/engine/imap/api/imap-email-identifier.vala @@ -9,19 +9,15 @@ private class Geary.Imap.EmailIdentifier : Geary.EmailIdentifier { public Imap.UID uid { get; private set; } - public override Geary.EmailIdentifier next() { - return new Geary.Imap.EmailIdentifier(new Imap.UID((uid.value + 1).clamp(1, uint32.MAX))); - } - - public override Geary.EmailIdentifier previous() { - return new Geary.Imap.EmailIdentifier(new Imap.UID((uid.value - 1).clamp(1, uint32.MAX))); - } - public EmailIdentifier(Imap.UID uid) { this.uid = uid; ordering = uid.value; } + public override uint to_hash() { + return Geary.Hashable.int64_hash(uid.value); + } + public override bool equals(Equalable o) { Geary.Imap.EmailIdentifier? other = o as Geary.Imap.EmailIdentifier; if (other == null) diff --git a/src/engine/imap/api/imap-folder.vala b/src/engine/imap/api/imap-folder.vala index 64d69528..73651af6 100644 --- a/src/engine/imap/api/imap-folder.vala +++ b/src/engine/imap/api/imap-folder.vala @@ -139,6 +139,12 @@ private class Geary.Imap.Folder : Geary.AbstractFolder, Geary.RemoteFolder { throw new EngineError.OPEN_REQUIRED("%s not opened", to_string()); UID uid = ((Imap.EmailIdentifier) email_id).uid; + if (flags.is_all_set(Geary.Folder.ListFlags.EXCLUDING_ID)) { + if (count > 1) + uid = new UID(uid.value + 1); + else if (count < 0) + uid = new UID(uid.value - 1); + } MessageSet msg_set; if (count > 0) { diff --git a/src/engine/impl/geary-engine-folder.vala b/src/engine/impl/geary-engine-folder.vala index 660e2d59..38f3dee3 100644 --- a/src/engine/impl/geary-engine-folder.vala +++ b/src/engine/impl/geary-engine-folder.vala @@ -250,8 +250,10 @@ private class Geary.EngineFolder : Geary.AbstractFolder { Geary.Email.Field.PROPERTIES, Geary.Folder.ListFlags.NONE, null); assert(list != null && list.size > 0); - foreach (Geary.Email email in list) + foreach (Geary.Email email in list) { + debug("Creating Email ID %s", email.id.to_string()); yield local_folder.create_email_async(email, null); + } // save new remote count remote_count = new_remote_count; @@ -281,24 +283,26 @@ private class Geary.EngineFolder : Geary.AbstractFolder { else assert(new_remote_count >= 0); - if (id == null) { + Geary.EmailIdentifier? owned_id = id; + if (owned_id == null) { try { Gee.List? local = yield local_folder.list_email_async(remote_position, 1, Geary.Email.Field.NONE, Geary.Folder.ListFlags.NONE, null); if (local != null && local.size > 0) - id = local[0].id; + owned_id = local[0].id; } catch (Error err) { debug("Unable to determine ID of removed message #%d from %s: %s", remote_position, to_string(), err.message); } } - if (id != null) { + if (owned_id != null) { + debug("Removing from local store Email ID %s", owned_id.to_string()); try { // Reflect change in the local store and notify subscribers - yield local_folder.remove_email_async(id, null); + yield local_folder.remove_email_async(owned_id, null); - notify_message_removed(id); + notify_message_removed(owned_id); } catch (Error err2) { debug("Unable to remove message #%d from %s: %s", remote_position, to_string(), err2.message); @@ -634,7 +638,7 @@ private class Geary.EngineFolder : Geary.AbstractFolder { Cancellable? cancellable = null) throws Error { Gee.List list = new Gee.ArrayList(); yield do_list_email_by_id_async(initial_id, count, required_fields, list, null, cancellable, - flags.is_all_set(Folder.ListFlags.FAST)); + flags.is_all_set(Folder.ListFlags.FAST), flags.is_all_set(Folder.ListFlags.EXCLUDING_ID)); return (list.size > 0) ? list : null; } @@ -643,24 +647,23 @@ private class Geary.EngineFolder : Geary.AbstractFolder { Geary.Email.Field required_fields, Folder.ListFlags flags, EmailCallback cb, Cancellable? cancellable = null) { do_lazy_list_email_by_id_async.begin(initial_id, count, required_fields, cb, cancellable, - flags.is_all_set(Folder.ListFlags.FAST)); + flags.is_all_set(Folder.ListFlags.FAST), flags.is_all_set(Folder.ListFlags.EXCLUDING_ID)); } private async void do_lazy_list_email_by_id_async(Geary.EmailIdentifier initial_id, int count, - Geary.Email.Field required_fields, EmailCallback cb, Cancellable? cancellable, bool local_only) { + Geary.Email.Field required_fields, EmailCallback cb, Cancellable? cancellable, bool local_only, + bool excluding_id) { try { yield do_list_email_by_id_async(initial_id, count, required_fields, null, cb, cancellable, - local_only); + local_only, excluding_id); } catch (Error err) { cb(null, err); } } - // STRATEGY: Determine position number of message at initial_id then work via positions from - // that point on. private async void do_list_email_by_id_async(Geary.EmailIdentifier initial_id, int count, Geary.Email.Field required_fields, Gee.List? accumulator, EmailCallback? cb, - Cancellable? cancellable, bool local_only) throws Error { + Cancellable? cancellable, bool local_only, bool excluding_id) throws Error { if (!opened) throw new EngineError.OPEN_REQUIRED("%s is not open", to_string()); @@ -678,7 +681,7 @@ private class Geary.EngineFolder : Geary.AbstractFolder { int initial_position = yield local_folder.get_id_position_async(initial_id, cancellable); if (initial_position <= 0) { throw new EngineError.NOT_FOUND("Email ID %s in %s not known to local store", - initial_id.to_string(), to_string); + initial_id.to_string(), to_string()); } // since count can also indicate "to earliest" or "to latest", normalize @@ -687,9 +690,9 @@ private class Geary.EngineFolder : Geary.AbstractFolder { int low, high; if (count < 0) { low = (count != int.MIN) ? (initial_position + count + 1) : 1; - high = initial_position; + high = excluding_id ? initial_position - 1 : initial_position; } else if (count > 0) { - low = initial_position; + low = excluding_id ? initial_position + 1 : initial_position; high = (count != int.MAX) ? (initial_position + count - 1) : remote_count; } else { // count == 0 diff --git a/src/engine/sqlite/api/sqlite-folder.vala b/src/engine/sqlite/api/sqlite-folder.vala index dc2a4a78..36f6fdea 100644 --- a/src/engine/sqlite/api/sqlite-folder.vala +++ b/src/engine/sqlite/api/sqlite-folder.vala @@ -182,7 +182,7 @@ private class Geary.Sqlite.Folder : Geary.AbstractFolder, Geary.LocalFolder, Gea public override async Gee.List? list_email_by_id_async(Geary.EmailIdentifier initial_id, int count, Geary.Email.Field required_fields, Geary.Folder.ListFlags flags, Cancellable? cancellable = null) throws Error { - if (count == 0) { + if (count == 0 || count == 1) { Geary.Email email = yield fetch_email_async(initial_id, required_fields, cancellable); Gee.List singleton = new Gee.ArrayList(); @@ -194,17 +194,18 @@ private class Geary.Sqlite.Folder : Geary.AbstractFolder, Geary.LocalFolder, Gea check_open(); Geary.Imap.UID uid = ((Geary.Imap.EmailIdentifier) initial_id).uid; + bool excluding_id = flags.is_all_set(Geary.Folder.ListFlags.EXCLUDING_ID); Transaction transaction = yield db.begin_transaction_async("Folder.list_email_by_id_async", cancellable); int64 low, high; if (count < 0) { - high = uid.value; + high = excluding_id ? uid.value - 1 : uid.value; low = (count != int.MIN) ? (high + count).clamp(1, uint32.MAX) : -1; } else { - // count > 0 - low = uid.value; + // count > 1 + low = excluding_id ? uid.value + 1 : uid.value; high = (count != int.MAX) ? (low + count).clamp(1, uint32.MAX) : -1; } @@ -228,14 +229,17 @@ private class Geary.Sqlite.Folder : Geary.AbstractFolder, Geary.LocalFolder, Gea Gee.List emails = new Gee.ArrayList(); foreach (MessageLocationRow location_row in list) { // fetch the message itself - MessageRow? message_row = yield message_table.fetch_async(transaction, - location_row.message_id, required_fields, cancellable); - assert(message_row != null); - - // only add to the list if the email contains all the required fields (because - // properties comes out of a separate table, skip this if properties are requested) - if (!message_row.fields.fulfills(required_fields.clear(Geary.Email.Field.PROPERTIES))) - continue; + MessageRow? message_row = null; + if (required_fields != Geary.Email.Field.NONE && required_fields != Geary.Email.Field.PROPERTIES) { + message_row = yield message_table.fetch_async(transaction, location_row.message_id, + required_fields, cancellable); + assert(message_row != null); + + // only add to the list if the email contains all the required fields (because + // properties comes out of a separate table, skip this if properties are requested) + if (!message_row.fields.fulfills(required_fields.clear(Geary.Email.Field.PROPERTIES))) + continue; + } ImapMessagePropertiesRow? properties = null; if (required_fields.require(Geary.Email.Field.PROPERTIES)) { @@ -253,7 +257,12 @@ private class Geary.Sqlite.Folder : Geary.AbstractFolder, Geary.LocalFolder, Gea continue; } - Geary.Email email = message_row.to_email(position, new Geary.Imap.EmailIdentifier(uid)); + Geary.Imap.EmailIdentifier email_id = new Geary.Imap.EmailIdentifier(uid); + + Geary.Email email = (message_row != null) + ? message_row.to_email(position, email_id) + : new Geary.Email(position, email_id); + if (properties != null) email.set_email_properties(properties.get_imap_email_properties()); @@ -279,6 +288,16 @@ private class Geary.Sqlite.Folder : Geary.AbstractFolder, Geary.LocalFolder, Gea to_string()); } + int position = yield location_row.get_position_async(transaction, cancellable); + if (position == -1) { + throw new EngineError.NOT_FOUND("Unable to determine position of email %s in %s", + id.to_string(), to_string()); + } + + // loopback on perverse case + if (required_fields == Geary.Email.Field.NONE) + return new Geary.Email(position, id); + MessageRow? message_row = yield message_table.fetch_async(transaction, location_row.message_id, required_fields, cancellable); if (message_row == null) { @@ -305,12 +324,6 @@ private class Geary.Sqlite.Folder : Geary.AbstractFolder, Geary.LocalFolder, Gea } } - int position = yield location_row.get_position_async(transaction, 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(position, id); if (properties != null) email.set_email_properties(properties.get_imap_email_properties()); @@ -468,16 +481,18 @@ private class Geary.Sqlite.Folder : Geary.AbstractFolder, Geary.LocalFolder, Gea if (email.fields == Geary.Email.Field.NONE) return; - MessageRow? message_row = yield message_table.fetch_async(transaction, message_id, email.fields, - cancellable); - assert(message_row != null); - - message_row.merge_from_network(email); - - // possible nothing has changed or been added - if (message_row.fields != Geary.Email.Field.NONE) - yield message_table.merge_async(transaction, message_row, cancellable); + if (email.fields != Geary.Email.Field.PROPERTIES) { + MessageRow? message_row = yield message_table.fetch_async(transaction, message_id, email.fields, + cancellable); + assert(message_row != null); + message_row.merge_from_network(email); + + // possible nothing has changed or been added + if (message_row.fields != Geary.Email.Field.NONE) + yield message_table.merge_async(transaction, message_row, cancellable); + } + // update IMAP properties if (email.fields.fulfills(Geary.Email.Field.PROPERTIES)) { Geary.Imap.EmailProperties properties = (Geary.Imap.EmailProperties) email.properties; diff --git a/src/engine/sqlite/email/sqlite-message-table.vala b/src/engine/sqlite/email/sqlite-message-table.vala index 2c03f923..5ad938ab 100644 --- a/src/engine/sqlite/email/sqlite-message-table.vala +++ b/src/engine/sqlite/email/sqlite-message-table.vala @@ -189,6 +189,8 @@ public class Geary.Sqlite.MessageTable : Geary.Sqlite.Table { public async MessageRow? fetch_async(Transaction? transaction, int64 id, Geary.Email.Field requested_fields, Cancellable? cancellable = null) throws Error { assert(requested_fields != Geary.Email.Field.NONE); + // PROPERTIES are handled by the appropriate PropertiesTable + assert(requested_fields != Geary.Email.Field.PROPERTIES); Transaction locked = yield obtain_lock_async(transaction, "MessageTable.fetch_async", cancellable); diff --git a/src/engine/util/util-interfaces.vala b/src/engine/util/util-interfaces.vala index 17716677..a57301e4 100644 --- a/src/engine/util/util-interfaces.vala +++ b/src/engine/util/util-interfaces.vala @@ -26,5 +26,22 @@ public interface Geary.Hashable { public static uint hash_func(void *ptr) { return ((Hashable *) ptr)->to_hash(); } + + public static uint int64_hash(int64 value) { + return hash_memory(&value, sizeof(int64)); + } + + /** + * A rotating-XOR hash that can be used to hash memory buffers of any size. Use only if + * equality is determined by memory contents. + */ + public static uint hash_memory(void *ptr, size_t bytes) { + uint8 *u8 = (uint8 *) ptr; + uint32 hash = 0; + for (int ctr = 0; ctr < bytes; ctr++) + hash = (hash << 4) ^ (hash >> 28) ^ (*u8++); + + return hash; + } }