diff --git a/src/client/application/application-controller.vala b/src/client/application/application-controller.vala index 2237aa61..6aec77ac 100644 --- a/src/client/application/application-controller.vala +++ b/src/client/application/application-controller.vala @@ -1088,7 +1088,7 @@ internal class Application.Controller : update_account_status(); // Stop any background processes - context.search.clear(); + context.search.clear_query(); context.contacts.close(); context.cancellable.cancel(); diff --git a/src/client/application/application-main-window.vala b/src/client/application/application-main-window.vala index 18016410..d0f0ae21 100644 --- a/src/client/application/application-main-window.vala +++ b/src/client/application/application-main-window.vala @@ -323,7 +323,6 @@ public class Application.MainWindow : private GLib.Cancellable action_update_cancellable = new GLib.Cancellable(); private GLib.Cancellable folder_open = new GLib.Cancellable(); - private GLib.Cancellable search_open = new GLib.Cancellable(); private Geary.TimeoutManager update_ui_timeout; private int64 update_ui_last = 0; @@ -971,10 +970,8 @@ public class Application.MainWindow : internal void start_search(string query_text, bool is_interactive) { var context = get_selected_account_context(); if (context != null) { - // Stop any search in progress - this.search_open.cancel(); - var cancellable = this.search_open = new GLib.Cancellable(); - + // If the current folder is not the search folder, save it + // so it can be re-selected later when search is closed if (this.previous_non_search_folder == null && this.selected_folder != null && this.selected_folder.used_as != SEARCH) { @@ -993,7 +990,7 @@ public class Application.MainWindow : this.folder_list.set_search( this.application.engine, context.search ); - yield context.search.search(query, cancellable); + context.search.update_query(query); } catch (GLib.Error error) { handle_error(context.account.information, error); } @@ -1001,10 +998,8 @@ public class Application.MainWindow : } internal void stop_search(bool is_interactive) { - // Stop any search in progress - this.search_open.cancel(); - this.search_open = new GLib.Cancellable(); - + // If the search folder is current selected, deselect and + // re-select any previously selected folder if (this.selected_folder == null || this.selected_folder.used_as == SEARCH) { var to_select = this.previous_non_search_folder; @@ -1026,7 +1021,7 @@ public class Application.MainWindow : this.folder_list.remove_search(); foreach (var context in this.controller.get_account_contexts()) { - context.search.clear(); + context.search.clear_query(); } } diff --git a/src/engine/api/geary-search-query.vala b/src/engine/api/geary-search-query.vala index 60c6b62f..62f7416d 100644 --- a/src/engine/api/geary-search-query.vala +++ b/src/engine/api/geary-search-query.vala @@ -26,7 +26,7 @@ * @see Account.new_search_query * @see Account.local_search_async * @see Account.get_search_matches_async - * @see App.SearchFolder.search + * @see App.SearchFolder.update_query */ public abstract class Geary.SearchQuery : BaseObject { diff --git a/src/engine/app/app-search-folder.vala b/src/engine/app/app-search-folder.vala index 981cb5a7..a8fa9404 100644 --- a/src/engine/app/app-search-folder.vala +++ b/src/engine/app/app-search-folder.vala @@ -110,10 +110,10 @@ public class Geary.App.SearchFolder : new Gee.HashSet(); // The email present in the folder, sorted - private Gee.TreeSet contents; + private Gee.SortedSet entries; // Map of engine ids to search ids - private Gee.Map id_map; + private Gee.Map ids; private Nonblocking.Mutex result_mutex = new Nonblocking.Mutex(); @@ -131,7 +131,8 @@ public class Geary.App.SearchFolder : account.email_removed.connect(on_account_email_removed); account.email_locally_removed.connect(on_account_email_removed); - new_contents(); + this.entries = new_entry_set(); + this.ids = new_id_map(); // Always exclude emails that don't live anywhere from search // results. @@ -147,53 +148,40 @@ public class Geary.App.SearchFolder : } /** - * Executes the given query over the account's local email. + * Sets the current search query for the folder. * - * Calling this will block until the search is complete. + * Calling this method will start the search folder asynchronously + * in the background. If the given query is not equal to the + * existing query, the folder's contents will be updated to + * reflect the changed query. */ - public async void search(SearchQuery query, GLib.Cancellable? cancellable) - throws GLib.Error { + public void update_query(SearchQuery query) { if (this.query == null || !this.query.equal_to(query)) { - int result_mutex_token = yield result_mutex.claim_async(); - - clear(); - - if (cancellable != null) { - GLib.Cancellable @internal = this.executing; - cancellable.cancelled.connect(() => { @internal.cancel(); }); - } + this.executing.cancel(); + this.executing = new GLib.Cancellable(); this.query = query; - GLib.Error? error = null; - try { - yield do_search_async(null, null, this.executing); - } catch(Error e) { - error = e; - } - - result_mutex.release(ref result_mutex_token); - - if (error != null) { - throw error; - } + this.update.begin(); } } /** * Cancels and clears the search query and results. * - * The {@link query} property will be cleared. + * The {@link query} property will be set to null. */ - public void clear() { + public void clear_query() { this.executing.cancel(); this.executing = new GLib.Cancellable(); - var old_ids = this.id_map; - new_contents(); + this.query = null; + var old_ids = this.ids; + + this.entries = new_entry_set(); + this.ids = new_id_map(); + notify_email_removed(old_ids.keys); notify_email_count_changed(0, REMOVED); - - this.query = null; } /** @@ -220,10 +208,16 @@ public class Geary.App.SearchFolder : Gee.Collection ids, GLib.Cancellable? cancellable = null) throws GLib.Error { + debug("Waiting for checking contains"); + int result_mutex_token = yield this.result_mutex.claim_async(cancellable); + var existing_ids = this.ids; + result_mutex.release(ref result_mutex_token); + + debug("Checking contains"); return Geary.traverse( ids ).filter( - (id) => this.id_map.has_key(id) + (id) => existing_ids.has_key(id) ).to_hash_set(); } @@ -234,17 +228,22 @@ public class Geary.App.SearchFolder : Folder.ListFlags flags, Cancellable? cancellable = null ) throws GLib.Error { - int result_mutex_token = yield result_mutex.claim_async(); + debug("Waiting to list email"); + int result_mutex_token = yield this.result_mutex.claim_async(cancellable); + var existing_entries = this.entries; + var existing_ids = this.ids; + result_mutex.release(ref result_mutex_token); + debug("Listing email"); var engine_ids = new Gee.LinkedList(); if (Folder.ListFlags.OLDEST_TO_NEWEST in flags) { EmailEntry? oldest = null; - if (!this.contents.is_empty) { + if (!existing_entries.is_empty) { if (initial_id == null) { - oldest = this.contents.last(); + oldest = existing_entries.last(); } else { - oldest = this.id_map.get(initial_id); + oldest = existing_ids.get(initial_id); if (oldest == null) { throw new EngineError.NOT_FOUND( @@ -253,13 +252,13 @@ public class Geary.App.SearchFolder : } if (!(Folder.ListFlags.INCLUDING_ID in flags)) { - oldest = contents.higher(oldest); + oldest = existing_entries.higher(oldest); } } } if (oldest != null) { var iter = ( - this.contents.iterator_at(oldest) as + existing_entries.iterator_at(oldest) as Gee.BidirIterator ); engine_ids.add(oldest.id); @@ -270,11 +269,11 @@ public class Geary.App.SearchFolder : } else { // Newest to oldest EmailEntry? newest = null; - if (!this.contents.is_empty) { + if (!existing_entries.is_empty) { if (initial_id == null) { - newest = this.contents.first(); + newest = existing_entries.first(); } else { - newest = this.id_map.get(initial_id); + newest = existing_ids.get(initial_id); if (newest == null) { throw new EngineError.NOT_FOUND( @@ -283,13 +282,13 @@ public class Geary.App.SearchFolder : } if (!(Folder.ListFlags.INCLUDING_ID in flags)) { - newest = contents.lower(newest); + newest = existing_entries.lower(newest); } } } if (newest != null) { var iter = ( - this.contents.iterator_at(newest) as + existing_entries.iterator_at(newest) as Gee.BidirIterator ); engine_ids.add(newest.id); @@ -313,8 +312,6 @@ public class Geary.App.SearchFolder : } } - result_mutex.release(ref result_mutex_token); - if (list_error != null) { throw list_error; } @@ -392,7 +389,7 @@ public class Geary.App.SearchFolder : private void require_id(EmailIdentifier id) throws EngineError.NOT_FOUND { - if (!this.id_map.has_key(id)) { + if (!this.ids.has_key(id)) { throw new EngineError.NOT_FOUND( "Id not found: %s", id.to_string() ); @@ -403,17 +400,114 @@ public class Geary.App.SearchFolder : Gee.Collection to_check ) { var available = new Gee.LinkedList(); - var id_map = this.id_map; + var ids = this.ids; var iter = to_check.iterator(); while (iter.next()) { var id = iter.get(); - if (id_map.has_key(id)) { + if (ids.has_key(id)) { available.add(id); } } return available; } + private async void append(Folder folder, + Gee.Collection ids) { + // Grab the cancellable before the lock so that if the current + // search is cancelled while waiting, this doesn't go and try + // to update the new results. + var cancellable = this.executing; + + debug("Waiting to append to search results"); + try { + int result_mutex_token = yield this.result_mutex.claim_async( + cancellable + ); + try { + if (!this.exclude_folders.contains(folder.path)) { + yield do_search_async(ids, null, cancellable); + } + } catch (GLib.Error error) { + this.account.report_problem( + new AccountProblemReport(this.account.information, error) + ); + } + + this.result_mutex.release(ref result_mutex_token); + } catch (GLib.IOError.CANCELLED mutex_err) { + // all good + } catch (GLib.Error mutex_err) { + warning("Error acquiring lock: %s", mutex_err.message); + } + } + + private async void update() { + // Grab the cancellable before the lock so that if the current + // search is cancelled while waiting, this doesn't go and try + // to update the new results. + var cancellable = this.executing; + + debug("Waiting to update search results"); + try { + int result_mutex_token = yield this.result_mutex.claim_async( + cancellable + ); + try { + yield do_search_async(null, null, cancellable); + } catch (GLib.Error error) { + this.account.report_problem( + new AccountProblemReport(this.account.information, error) + ); + } + + this.result_mutex.release(ref result_mutex_token); + } catch (GLib.IOError.CANCELLED mutex_err) { + // all good + } catch (GLib.Error mutex_err) { + warning("Error acquiring lock: %s", mutex_err.message); + } + } + + private async void remove(Folder folder, + Gee.Collection ids) { + + // Grab the cancellable before the lock so that if the current + // search is cancelled while waiting, this doesn't go and try + // to update the new results. + var cancellable = this.executing; + + debug("Waiting to remove from search results"); + try { + int result_mutex_token = yield this.result_mutex.claim_async( + cancellable + ); + + // Ensure this happens inside the lock so it is working with + // up-to-date data + var id_map = this.ids; + var relevant_ids = ( + traverse(ids) + .filter(id => id_map.has_key(id)) + .to_linked_list() + ); + if (relevant_ids.size > 0) { + try { + yield do_search_async(null, relevant_ids, cancellable); + } catch (GLib.Error error) { + this.account.report_problem( + new AccountProblemReport(this.account.information, error) + ); + } + } + + this.result_mutex.release(ref result_mutex_token); + } catch (GLib.IOError.CANCELLED mutex_err) { + // all good + } catch (GLib.Error mutex_err) { + warning("Error acquiring lock: %s", mutex_err.message); + } + } + // 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 @@ -424,11 +518,15 @@ public class Geary.App.SearchFolder : Gee.Collection? remove_ids, GLib.Cancellable? cancellable) throws GLib.Error { - var id_map = this.id_map; - var contents = this.contents; + debug("Processing search results"); + var entries = new_entry_set(); + var ids = new_id_map(); var added = new Gee.LinkedList(); var removed = new Gee.LinkedList(); + entries.add_all(this.entries); + ids.set_all(this.ids); + if (remove_ids == null) { // Adding email to the search, either searching all local // email if to_add is null, or adding only a matching @@ -465,24 +563,24 @@ public class Geary.App.SearchFolder : var hashed_results = new Gee.HashSet(); hashed_results.add_all(id_results); - var existing = id_map.map_iterator(); + var existing = ids.map_iterator(); while (existing.next()) { if (!hashed_results.contains(existing.get_key())) { var entry = existing.get_value(); existing.unset(); - contents.remove(entry); + entries.remove(entry); removed.add(entry.id); } } } foreach (var email in email_results) { - if (!id_map.has_key(email.id)) { + if (!ids.has_key(email.id)) { var entry = new EmailEntry( email.id, email.properties.date_received ); - contents.add(entry); - id_map.set(email.id, entry); + entries.add(entry); + ids.set(email.id, entry); added.add(email.id); } } @@ -491,89 +589,53 @@ public class Geary.App.SearchFolder : // Removing email, can just remove them directly foreach (var id in remove_ids) { EmailEntry entry; - if (id_map.unset(id, out entry)) { - contents.remove(entry); + if (ids.unset(id, out entry)) { + entries.remove(entry); removed.add(id); } } } - this._properties.set_total(this.contents.size); + if (!cancellable.is_cancelled()) { + this.entries = entries; + this.ids = ids; - // 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. + this._properties.set_total(entries.size); - 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 |= Folder.CountChangeReason.INSERTED; - } - if (removed.size > 0) { - notify_email_removed(removed); - reason |= Folder.CountChangeReason.REMOVED; - } - if (reason != CountChangeReason.NONE) - notify_email_count_changed(this.contents.size, reason); - } + // 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. - 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(); - - GLib.Error? error = null; - try { - if (!this.exclude_folders.contains(folder.path)) { - yield do_search_async(ids, null, cancellable); + Folder.CountChangeReason reason = CountChangeReason.NONE; + if (removed.size > 0) { + notify_email_removed(removed); + reason |= Folder.CountChangeReason.REMOVED; } - } catch (GLib.Error e) { - error = e; - } - - result_mutex.release(ref result_mutex_token); - - if (error != null) - throw error; - } - - 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(); - - GLib.Error? error = null; - try { - var id_map = this.id_map; - var relevant_ids = ( - traverse(ids) - .filter(id => id_map.has_key(id)) - .to_linked_list() - ); - - if (relevant_ids.size > 0) { - yield do_search_async(null, relevant_ids, cancellable); + 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 |= Folder.CountChangeReason.INSERTED; } - } catch (GLib.Error e) { - error = e; + if (reason != CountChangeReason.NONE) { + notify_email_count_changed(this.entries.size, reason); + } + debug("Processing done, entries/ids: %d/%d", entries.size, ids.size); + } else { + debug("Processing cancelled, dropping entries/ids: %d/%d", entries.size, ids.size); } - - result_mutex.release(ref result_mutex_token); - - if (error != null) - throw error; } - private inline void new_contents() { - this.contents = new Gee.TreeSet(EmailEntry.compare_to); - this.id_map = new Gee.HashMap(); + private inline Gee.SortedSet new_entry_set() { + return new Gee.TreeSet(EmailEntry.compare_to); + } + + private inline Gee.Map new_id_map() { + return new Gee.HashMap(); } private void include_folder(Folder folder) { @@ -613,40 +675,14 @@ public class Geary.App.SearchFolder : private void on_email_locally_complete(Folder folder, Gee.Collection ids) { if (this.query != null) { - this.do_append.begin( - folder, ids, null, - (obj, res) => { - try { - this.do_append.end(res); - } catch (GLib.Error error) { - this.account.report_problem( - new AccountProblemReport( - this.account.information, error - ) - ); - } - } - ); + this.append.begin(folder, ids); } } private void on_account_email_removed(Folder folder, Gee.Collection ids) { if (this.query != null) { - this.do_remove.begin( - folder, ids, null, - (obj, res) => { - try { - this.do_remove.end(res); - } catch (GLib.Error error) { - this.account.report_problem( - new AccountProblemReport( - this.account.information, error - ) - ); - } - } - ); + this.remove.begin(folder, ids); } }