diff --git a/src/engine/app/app-search-folder.vala b/src/engine/app/app-search-folder.vala index 3e6716a9..2e11f5a2 100644 --- a/src/engine/app/app-search-folder.vala +++ b/src/engine/app/app-search-folder.vala @@ -14,7 +14,7 @@ * folder interface. */ public class Geary.App.SearchFolder : - Geary.AbstractLocalFolder, Geary.FolderSupport.Remove { + AbstractLocalFolder, FolderSupport.Remove { /** Number of messages to include in the initial search. */ @@ -23,136 +23,18 @@ public class Geary.App.SearchFolder : /** The canonical name of the search folder. */ public const string MAGIC_BASENAME = "$GearyAccountSearchFolder$"; - private const Geary.SpecialFolderType[] EXCLUDE_TYPES = { - Geary.SpecialFolderType.SPAM, - Geary.SpecialFolderType.TRASH, - Geary.SpecialFolderType.DRAFTS, + private const SpecialFolderType[] EXCLUDE_TYPES = { + SpecialFolderType.SPAM, + SpecialFolderType.TRASH, + SpecialFolderType.DRAFTS, // Orphan emails (without a folder) are also excluded; see ct or. }; - /** Internal identifier used by the search folder */ - internal class EmailIdentifier : - Geary.EmailIdentifier, Gee.Comparable { + private class FolderPropertiesImpl : FolderProperties { - private const string VARIANT_TYPE = "(y(vx))"; - - - public static int compare_descending(EmailIdentifier a, EmailIdentifier b) { - return b.compare_to(a); - } - - public static Gee.Collection to_source_ids( - Gee.Collection ids - ) { - var engine_ids = new Gee.LinkedList(); - foreach (var id in ids) { - var search_id = id as EmailIdentifier; - engine_ids.add(search_id.source_id ?? id); - } - return engine_ids; - } - - public static Geary.EmailIdentifier to_source_id( - Geary.EmailIdentifier id - ) { - var search_id = id as EmailIdentifier; - return search_id.source_id ?? id; - } - - - public Geary.EmailIdentifier source_id { get; private set; } - - public GLib.DateTime? date_received { get; private set; } - - - public EmailIdentifier(Geary.EmailIdentifier source_id, - GLib.DateTime? date_received) { - this.source_id = source_id; - this.date_received = date_received; - } - - public EmailIdentifier.from_variant(GLib.Variant serialised, - Account account) - throws EngineError.BAD_PARAMETERS { - if (serialised.get_type_string() != VARIANT_TYPE) { - throw new EngineError.BAD_PARAMETERS( - "Invalid serialised id type: %s", serialised.get_type_string() - ); - } - GLib.Variant inner = serialised.get_child_value(1); - this( - account.to_email_identifier( - inner.get_child_value(0).get_variant() - ), - new GLib.DateTime.from_unix_utc( - inner.get_child_value(1).get_int64() - ) - ); - } - - public override uint hash() { - return this.source_id.hash(); - } - - public override bool equal_to(Geary.EmailIdentifier other) { - return ( - this.get_type() == other.get_type() && - this.source_id.equal_to(((EmailIdentifier) other).source_id) - ); - } - - public override GLib.Variant to_variant() { - // Return a tuple to satisfy the API contract, add an 's' to - // inform GenericAccount that it's an IMAP id. - return new GLib.Variant.tuple(new Variant[] { - new GLib.Variant.byte('s'), - new GLib.Variant.tuple(new Variant[] { - new GLib.Variant.variant(this.source_id.to_variant()), - new GLib.Variant.int64(this.date_received.to_unix()) - }) - }); - } - - public override string to_string() { - return "%s(%s,%lld)".printf( - this.get_type().name(), - this.source_id.to_string(), - this.date_received.to_unix() - ); - } - - public override int natural_sort_comparator(Geary.EmailIdentifier o) { - EmailIdentifier? other = o as EmailIdentifier; - if (other == null) - return 1; - - return compare_to(other); - } - - public virtual int compare_to(EmailIdentifier other) { - // if both have date received, compare on that, using stable sort if the same - if (date_received != null && other.date_received != null) { - int compare = date_received.compare(other.date_received); - return (compare != 0) ? compare : stable_sort_comparator(other); - } - - // if neither have date received, fall back on stable sort - if (date_received == null && other.date_received == null) - return stable_sort_comparator(other); - - // put identifiers with no date ahead of those with - return (date_received == null ? -1 : 1); - } - - } - - - private class FolderProperties : Geary.FolderProperties { - - - public FolderProperties(int total, int unread) { + public FolderPropertiesImpl(int total, int unread) { base(total, unread, Trillian.FALSE, Trillian.FALSE, Trillian.TRUE, true, true, false); } @@ -163,6 +45,36 @@ public class Geary.App.SearchFolder : } + // Represents an entry in the folder. Does not implement + // Gee.Comparable since that would require extending GLib.Object + // and hence make them very heavyweight. + private class EmailEntry { + + + public static int compare_to(EmailEntry a, EmailEntry b) { + int cmp = 0; + if (a != b && a.id != b.id && !a.id.equal_to(b.id)) { + cmp = a.received.compare(b.received); + if (cmp == 0) { + cmp = a.id.stable_sort_comparator(b.id); + } + } + return cmp; + } + + + public EmailIdentifier id; + public GLib.DateTime received; + + + public EmailEntry(EmailIdentifier id, GLib.DateTime received) { + this.id = id; + this.received = received; + } + + } + + /** {@inheritDoc} */ public override Account account { get { return _account; } @@ -170,10 +82,10 @@ public class Geary.App.SearchFolder : private weak Account _account; /** {@inheritDoc} */ - public override Geary.FolderProperties properties { + public override FolderProperties properties { get { return _properties; } } - private Geary.FolderProperties _properties; + private FolderPropertiesImpl _properties; /** {@inheritDoc} */ public override FolderPath path { @@ -187,32 +99,30 @@ public class Geary.App.SearchFolder : * Always returns {@link SpecialFolderType.SEARCH}. */ public override SpecialFolderType special_folder_type { - get { - return Geary.SpecialFolderType.SEARCH; - } + get { return SpecialFolderType.SEARCH; } } /** The query being evaluated by this folder, if any. */ - public Geary.SearchQuery? query { get; protected set; default = null; } + public SearchQuery? query { get; protected set; default = null; } // Folders that should be excluded from search - private Gee.HashSet exclude_folders = - new Gee.HashSet(); + private Gee.HashSet exclude_folders = + new Gee.HashSet(); // The email present in the folder, sorted - private Gee.TreeSet contents; + private Gee.TreeSet contents; // Map of engine ids to search ids - private Gee.Map id_map; + private Gee.Map id_map; - private Geary.Nonblocking.Mutex result_mutex = new Geary.Nonblocking.Mutex(); + private Nonblocking.Mutex result_mutex = new Nonblocking.Mutex(); private GLib.Cancellable executing = new GLib.Cancellable(); - public SearchFolder(Geary.Account account, FolderRoot root) { + public SearchFolder(Account account, FolderRoot root) { this._account = account; - this._properties = new FolderProperties(0, 0); + this._properties = new FolderPropertiesImpl(0, 0); this._path = root.get_child(MAGIC_BASENAME, Trillian.TRUE); account.folders_available_unavailable.connect(on_folders_available_unavailable); @@ -220,7 +130,7 @@ public class Geary.App.SearchFolder : account.email_locally_complete.connect(on_email_locally_complete); account.email_removed.connect(on_account_email_removed); - clear_contents(); + new_contents(); // Always exclude emails that don't live anywhere from search // results. @@ -274,10 +184,10 @@ public class Geary.App.SearchFolder : this.executing.cancel(); this.executing = new GLib.Cancellable(); - var old_contents = this.contents; - clear_contents(); - notify_email_removed(old_contents); - notify_email_count_changed(0, Geary.Folder.CountChangeReason.REMOVED); + var old_ids = this.id_map; + new_contents(); + notify_email_removed(old_ids.keys); + notify_email_count_changed(0, REMOVED); this.query = null; } @@ -289,33 +199,31 @@ public class Geary.App.SearchFolder : * match any of the non-negated text operators in {@link query}. */ public async Gee.Set? get_search_matches_async( - Gee.Collection ids, + Gee.Collection targets, GLib.Cancellable? cancellable = null ) throws GLib.Error { Gee.Set? results = null; if (this.query != null) { results = yield account.get_search_matches_async( - this.query, - EmailIdentifier.to_source_ids(ids), - cancellable + this.query, check_ids(targets), cancellable ); } return results; } public override async Gee.List? list_email_by_id_async( - Geary.EmailIdentifier? initial_id, + EmailIdentifier? initial_id, int count, Email.Field required_fields, - Geary.Folder.ListFlags flags, + Folder.ListFlags flags, Cancellable? cancellable = null ) throws GLib.Error { int result_mutex_token = yield result_mutex.claim_async(); - var engine_ids = new Gee.LinkedList(); + var engine_ids = new Gee.LinkedList(); - if (Geary.Folder.ListFlags.OLDEST_TO_NEWEST in flags) { - EmailIdentifier? oldest = null; + if (Folder.ListFlags.OLDEST_TO_NEWEST in flags) { + EmailEntry? oldest = null; if (!this.contents.is_empty) { if (initial_id == null) { oldest = this.contents.last(); @@ -324,11 +232,11 @@ public class Geary.App.SearchFolder : if (oldest == null) { throw new EngineError.NOT_FOUND( - "Initial id not found %s", initial_id.to_string() + "Initial id not found: %s", initial_id.to_string() ); } - if (!(Geary.Folder.ListFlags.INCLUDING_ID in flags)) { + if (!(Folder.ListFlags.INCLUDING_ID in flags)) { oldest = contents.higher(oldest); } } @@ -336,16 +244,16 @@ public class Geary.App.SearchFolder : if (oldest != null) { var iter = ( this.contents.iterator_at(oldest) as - Gee.BidirIterator + Gee.BidirIterator ); - engine_ids.add(oldest.source_id); + engine_ids.add(oldest.id); while (engine_ids.size < count && iter.previous()) { - engine_ids.add(iter.get().source_id); + engine_ids.add(iter.get().id); } } } else { // Newest to oldest - EmailIdentifier? newest = null; + EmailEntry? newest = null; if (!this.contents.is_empty) { if (initial_id == null) { newest = this.contents.first(); @@ -354,11 +262,11 @@ public class Geary.App.SearchFolder : if (newest == null) { throw new EngineError.NOT_FOUND( - "Initial id not found %s", initial_id.to_string() + "Initial id not found: %s", initial_id.to_string() ); } - if (!(Geary.Folder.ListFlags.INCLUDING_ID in flags)) { + if (!(Folder.ListFlags.INCLUDING_ID in flags)) { newest = contents.lower(newest); } } @@ -366,11 +274,11 @@ public class Geary.App.SearchFolder : if (newest != null) { var iter = ( this.contents.iterator_at(newest) as - Gee.BidirIterator + Gee.BidirIterator ); - engine_ids.add(newest.source_id); + engine_ids.add(newest.id); while (engine_ids.size < count && iter.next()) { - engine_ids.add(iter.get().source_id); + engine_ids.add(iter.get().id); } } } @@ -398,62 +306,67 @@ public class Geary.App.SearchFolder : return results; } - public override async Gee.List? list_email_by_sparse_id_async( - Gee.Collection ids, - Geary.Email.Field required_fields, - Geary.Folder.ListFlags flags, + public override async Gee.List? list_email_by_sparse_id_async( + Gee.Collection list, + Email.Field required_fields, + Folder.ListFlags flags, Cancellable? cancellable = null ) throws GLib.Error { return yield this.account.list_local_email_async( - EmailIdentifier.to_source_ids(ids), - required_fields, - cancellable + check_ids(list), required_fields, cancellable ); } - public override async Gee.Map? list_local_email_fields_async( - Gee.Collection ids, Cancellable? cancellable = null) throws Error { - // TODO: This method is not currently called, but is required by the interface. Before completing - // this feature, it should either be implemented either here or in AbstractLocalFolder. + public override async Gee.Map? list_local_email_fields_async( + Gee.Collection list, + GLib.Cancellable? cancellable = null) + throws GLib.Error { + check_ids(list); + // TODO: This method is not currently called, but is required + // by the interface. Before completing this feature, it + // should either be implemented either here or in + // AbstractLocalFolder. error("Search folder does not implement list_local_email_fields_async"); } - public override async Geary.Email fetch_email_async(Geary.EmailIdentifier id, - Geary.Email.Field required_fields, - Geary.Folder.ListFlags flags, - Cancellable? cancellable = null) + public override async Email fetch_email_async(EmailIdentifier fetch, + Email.Field required_fields, + Folder.ListFlags flags, + GLib.Cancellable? cancellable = null) throws GLib.Error { + require_id(fetch); return yield this.account.local_fetch_email_async( - EmailIdentifier.to_source_id(id), required_fields, cancellable + fetch, required_fields, cancellable ); } public virtual async void remove_email_async( - Gee.Collection email_ids, + Gee.Collection remove, GLib.Cancellable? cancellable = null ) throws GLib.Error { - Gee.MultiMap? ids_to_folders = + Gee.MultiMap? ids_to_folders = yield account.get_containing_folders_async( - EmailIdentifier.to_source_ids(email_ids), - cancellable + check_ids(remove), cancellable ); if (ids_to_folders != null) { - Gee.MultiMap folders_to_ids = - Geary.Collection.reverse_multi_map(ids_to_folders); + Gee.MultiMap folders_to_ids = + Collection.reverse_multi_map( + ids_to_folders + ); - foreach (Geary.FolderPath path in folders_to_ids.get_keys()) { - Geary.Folder folder = account.get_folder(path); - Geary.FolderSupport.Remove? remove = folder as Geary.FolderSupport.Remove; - if (remove != null) { - Gee.Collection ids = folders_to_ids.get(path); + foreach (FolderPath path in folders_to_ids.get_keys()) { + Folder folder = account.get_folder(path); + FolderSupport.Remove? removable = folder as FolderSupport.Remove; + if (removable != null) { + Gee.Collection ids = folders_to_ids.get(path); debug("Search folder removing %d emails from %s", ids.size, folder.to_string()); bool open = false; try { - yield folder.open_async(Geary.Folder.OpenFlags.NONE, cancellable); + yield folder.open_async(NONE, cancellable); open = true; - yield remove.remove_email_async(ids, cancellable); + yield removable.remove_email_async(ids, cancellable); } finally { if (open) { try { @@ -468,14 +381,38 @@ public class Geary.App.SearchFolder : } } + private void require_id(EmailIdentifier id) + throws EngineError.NOT_FOUND { + if (!this.id_map.has_key(id)) { + throw new EngineError.NOT_FOUND( + "Id not found: %s", id.to_string() + ); + } + } + + private Gee.List check_ids( + Gee.Collection to_check + ) { + var available = new Gee.LinkedList(); + var id_map = this.id_map; + var iter = to_check.iterator(); + while (iter.next()) { + var id = iter.get(); + if (id_map.has_key(id)) { + available.add(id); + } + } + return available; + } + // NOTE: you must call this ONLY after locking result_mutex_token. // If both *_ids parameters are null, the results of this search are // considered to be the full new set. If non-null, the results are // considered to be a delta and are added or subtracted from the full set. // add_ids are new ids to search for, remove_ids are ids in our result set // and will be removed. - private async void do_search_async(Gee.Collection? add_ids, - Gee.Collection? remove_ids, + private async void do_search_async(Gee.Collection? add_ids, + Gee.Collection? remove_ids, GLib.Cancellable? cancellable) throws GLib.Error { var id_map = this.id_map; @@ -492,7 +429,7 @@ public class Geary.App.SearchFolder : // we could be smarter about only fetching the search // results in list_email_async() etc., but this leads to // some more complications when redoing the search. - Gee.Collection? id_results = + Gee.Collection? id_results = yield this.account.local_search_async( this.query, MAX_RESULT_EMAILS, @@ -516,68 +453,68 @@ public class Geary.App.SearchFolder : // Not appending new email, so remove any not // found in the results. Add to a set first to // avoid O(N^2) lookup complexity. - var hashed_results = new Gee.HashSet(); + var hashed_results = new Gee.HashSet(); hashed_results.add_all(id_results); var existing = id_map.map_iterator(); while (existing.next()) { if (!hashed_results.contains(existing.get_key())) { - var search_id = existing.get_value(); + var entry = existing.get_value(); existing.unset(); - contents.remove(search_id); - removed.add(search_id); + contents.remove(entry); + removed.add(entry.id); } } } foreach (var email in email_results) { if (!id_map.has_key(email.id)) { - var search_id = new EmailIdentifier( + var entry = new EmailEntry( email.id, email.properties.date_received ); - id_map.set(email.id, search_id); - contents.add(search_id); - added.add(search_id); + contents.add(entry); + id_map.set(email.id, entry); + added.add(email.id); } } } } else { // Removing email, can just remove them directly foreach (var id in remove_ids) { - EmailIdentifier search_id; - if (id_map.unset(id, out search_id)) { - contents.remove(search_id); - removed.add(search_id); + EmailEntry entry; + if (id_map.unset(id, out entry)) { + contents.remove(entry); + removed.add(id); } } } - ((FolderProperties) this.properties).set_total(this.contents.size); + this._properties.set_total(this.contents.size); // Note that we probably shouldn't be firing these signals from inside // our mutex lock. Keep an eye on it, and if there's ever a case where // it might cause problems, it shouldn't be too hard to move the // firings outside. - Geary.Folder.CountChangeReason reason = CountChangeReason.NONE; + Folder.CountChangeReason reason = CountChangeReason.NONE; if (added.size > 0) { // TODO: we'd like to be able to use APPENDED here when applicable, // but because of the potential to append a thousand results at // once and the ConversationMonitor's inability to handle that // gracefully (#7464), we always use INSERTED for now. notify_email_inserted(added); - reason |= Geary.Folder.CountChangeReason.INSERTED; + reason |= Folder.CountChangeReason.INSERTED; } if (removed.size > 0) { notify_email_removed(removed); - reason |= Geary.Folder.CountChangeReason.REMOVED; + reason |= Folder.CountChangeReason.REMOVED; } if (reason != CountChangeReason.NONE) notify_email_count_changed(this.contents.size, reason); } - private async void do_append(Geary.Folder folder, - Gee.Collection ids, + private async void do_append(Folder folder, + Gee.Collection ids, GLib.Cancellable? cancellable) throws GLib.Error { int result_mutex_token = yield result_mutex.claim_async(); @@ -597,8 +534,8 @@ public class Geary.App.SearchFolder : throw error; } - private async void do_remove(Geary.Folder folder, - Gee.Collection ids, + private async void do_remove(Folder folder, + Gee.Collection ids, GLib.Cancellable? cancellable) throws GLib.Error { int result_mutex_token = yield result_mutex.claim_async(); @@ -625,18 +562,16 @@ public class Geary.App.SearchFolder : throw error; } - private void clear_contents() { - this.contents = new Gee.TreeSet( - EmailIdentifier.compare_descending - ); - this.id_map = new Gee.HashMap(); + private inline void new_contents() { + this.contents = new Gee.TreeSet(EmailEntry.compare_to); + this.id_map = new Gee.HashMap(); } - private void include_folder(Geary.Folder folder) { + private void include_folder(Folder folder) { this.exclude_folders.remove(folder.path); } - private void exclude_folder(Geary.Folder folder) { + private void exclude_folder(Folder folder) { this.exclude_folders.add(folder.path); } @@ -644,18 +579,20 @@ public class Geary.App.SearchFolder : this.exclude_folders.add(null); } - private void on_folders_available_unavailable(Gee.Collection? available, - Gee.Collection? unavailable) { + private void on_folders_available_unavailable( + Gee.Collection? available, + Gee.Collection? unavailable + ) { if (available != null) { // Exclude it from searching if it's got the right special type. - foreach(Geary.Folder folder in Geary.traverse(available) + foreach(Folder folder in traverse(available) .filter(f => f.special_folder_type in EXCLUDE_TYPES)) exclude_folder(folder); } } - private void on_folders_special_type(Gee.Collection folders) { - foreach (Geary.Folder folder in folders) { + private void on_folders_special_type(Gee.Collection folders) { + foreach (Folder folder in folders) { if (folder.special_folder_type in EXCLUDE_TYPES) { exclude_folder(folder); } else { @@ -664,8 +601,8 @@ public class Geary.App.SearchFolder : } } - private void on_email_locally_complete(Geary.Folder folder, - Gee.Collection ids) { + private void on_email_locally_complete(Folder folder, + Gee.Collection ids) { if (this.query != null) { this.do_append.begin( folder, ids, null, @@ -674,7 +611,7 @@ public class Geary.App.SearchFolder : this.do_append.end(res); } catch (GLib.Error error) { this.account.report_problem( - new Geary.AccountProblemReport( + new AccountProblemReport( this.account.information, error ) ); @@ -684,8 +621,8 @@ public class Geary.App.SearchFolder : } } - private void on_account_email_removed(Geary.Folder folder, - Gee.Collection ids) { + private void on_account_email_removed(Folder folder, + Gee.Collection ids) { if (this.query != null) { this.do_remove.begin( folder, ids, null, @@ -694,7 +631,7 @@ public class Geary.App.SearchFolder : this.do_remove.end(res); } catch (GLib.Error error) { this.account.report_problem( - new Geary.AccountProblemReport( + new AccountProblemReport( this.account.information, error ) ); diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala b/src/engine/imap-engine/imap-engine-generic-account.vala index c94ea9f6..0605c5de 100644 --- a/src/engine/imap-engine/imap-engine-generic-account.vala +++ b/src/engine/imap-engine/imap-engine-generic-account.vala @@ -375,10 +375,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account { char type = (char) serialised.get_child_value(0).get_byte(); if (type == 'i') return new ImapDB.EmailIdentifier.from_variant(serialised); - if (type == 's') - return new App.SearchFolder.EmailIdentifier.from_variant( - serialised, this - ); if (type == 'o') return new Outbox.EmailIdentifier.from_variant(serialised); diff --git a/test/engine/imap-engine/imap-engine-generic-account-test.vala b/test/engine/imap-engine/imap-engine-generic-account-test.vala index 8698f12f..b22af954 100644 --- a/test/engine/imap-engine/imap-engine-generic-account-test.vala +++ b/test/engine/imap-engine/imap-engine-generic-account-test.vala @@ -99,21 +99,6 @@ public class Geary.ImapEngine.GenericAccountTest : TestCase { ) ) ); - assert_non_null( - test_article.to_email_identifier( - new GLib.Variant( - "(yr)", - 's', - new GLib.Variant( - "(vx)", - new GLib.Variant( - "(yr)", 'o', new GLib.Variant("(xx)", 1, 2) - ), - 3 - ) - ) - ) - ); } }