From c93cfc38b18af6cf9849423d7d2495e6c3eea80b Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Mon, 9 Dec 2019 12:35:02 +1100 Subject: [PATCH 01/13] Clean up Engine search API Clean up search API so that query string parsing and error handling can be pushed out to the client. Rename Geary.Account::open_search factory method to ::new_search_query to better reflect what it actually does. Update Geary.SearchFolder::search to be async so it can throw errors and the caller can catch them, require SearchQuery objects instead of plain text query strings. Add Geary.SearchQuery::owner property so that the query's account can be sanity checked. Update API documentation to reflect this. --- .../application/application-main-window.vala | 68 +++++--- .../conversation-viewer.vala | 18 ++- src/engine/api/geary-account.vala | 11 +- src/engine/api/geary-search-folder.vala | 59 +++---- src/engine/api/geary-search-query.vala | 35 ++-- .../imap-db/search/imap-db-search-folder.vala | 153 +++++++++--------- .../imap-db/search/imap-db-search-query.vala | 7 +- .../imap-engine-generic-account.vala | 12 +- test/engine/api/geary-account-mock.vala | 13 +- 9 files changed, 215 insertions(+), 161 deletions(-) diff --git a/src/client/application/application-main-window.vala b/src/client/application/application-main-window.vala index 24b54256..9f00d46a 100644 --- a/src/client/application/application-main-window.vala +++ b/src/client/application/application-main-window.vala @@ -902,37 +902,53 @@ public class Application.MainWindow : return closed; } - public void show_search(string text, bool is_interactive) { - Geary.SearchFolder? search_folder = null; - if (this.selected_account != null) { - search_folder = this.selected_account.get_special_folder( + internal async void start_search(string query_text, bool is_interactive) { + var account = this.selected_account; + if (account != null) { + var folder = account.get_special_folder( SEARCH ) as Geary.SearchFolder; - } + // Stop any search in progress + this.search_open.cancel(); + var cancellable = this.search_open = new GLib.Cancellable(); + + var strategy = this.application.config.get_search_strategy(); + try { + var query = yield account.new_search_query( + query_text, + strategy, + cancellable + ); + this.folder_list.set_search(this.application.engine, folder); + yield folder.search(query, cancellable); + } catch (GLib.Error error) { + handle_error(account.information, error); + } + } + } + + internal void stop_search(bool is_interactive) { // Stop any search in progress this.search_open.cancel(); - var cancellable = this.search_open = new GLib.Cancellable(); + this.search_open = new GLib.Cancellable(); - if (Geary.String.is_empty_or_whitespace(text)) { - if (this.previous_non_search_folder != null && - this.selected_folder is Geary.SearchFolder) { - this.select_folder.begin( - this.previous_non_search_folder, is_interactive - ); - } - this.folder_list.remove_search(); - if (search_folder != null) { - search_folder.clear(); - } - } else if (search_folder != null) { - search_folder.search( - text, this.application.config.get_search_strategy(), cancellable - ); - this.folder_list.set_search( - this.application.engine, search_folder + if (this.previous_non_search_folder != null && + this.selected_folder is Geary.SearchFolder) { + this.select_folder.begin( + this.previous_non_search_folder, is_interactive ); } + this.folder_list.remove_search(); + + if (this.selected_account != null) { + var folder = this.selected_account.get_special_folder( + SEARCH + ) as Geary.SearchFolder; + if (folder != null) { + folder.clear(); + } + } } internal bool select_first_inbox(bool is_interactive) { @@ -2061,7 +2077,11 @@ public class Application.MainWindow : } private void on_search(string text) { - show_search(text, true); + if (Geary.String.is_empty_or_whitespace(text)) { + stop_search(true); + } else { + this.start_search.begin(text, true); + } } private void on_visible_conversations_changed(Gee.Set visible) { diff --git a/src/client/conversation-viewer/conversation-viewer.vala b/src/client/conversation-viewer/conversation-viewer.vala index de5e185e..6ffeba11 100644 --- a/src/client/conversation-viewer/conversation-viewer.vala +++ b/src/client/conversation-viewer/conversation-viewer.vala @@ -296,10 +296,9 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface { conversation.base_folder.account, null ); if (query == null) { - Geary.SearchFolder? search_folder = - conversation.base_folder as Geary.SearchFolder; + var search_folder = conversation.base_folder as Geary.SearchFolder; if (search_folder != null) { - query = search_folder.search_query; + query = search_folder.query; } } @@ -425,8 +424,11 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface { // opening every message in the conversation as soon as // the user presses a key if (text.length >= 2) { - query = yield account.open_search( - text, this.config.get_search_strategy(), cancellable + var strategy = this.config.get_search_strategy(); + query = yield account.new_search_query( + text, + strategy, + cancellable ); } } @@ -458,10 +460,10 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface { ); this.conversation_find_undo.reset(); if (search_folder != null) { - Geary.SearchQuery? search_query = search_folder.search_query; - if (search_query != null) { + Geary.SearchQuery? query = search_folder.query; + if (query != null) { this.current_list.search.highlight_matching_email.begin( - search_query, + query, true ); } diff --git a/src/engine/api/geary-account.vala b/src/engine/api/geary-account.vala index aad174bd..0bb90aba 100644 --- a/src/engine/api/geary-account.vala +++ b/src/engine/api/geary-account.vala @@ -436,13 +436,12 @@ public abstract class Geary.Account : BaseObject, Logging.Source { * baked into the caller's code is up to the caller. CONSERVATIVE is designed to be a good * default. * - * The SearchQuery object can only be used with calls into this Account. - * - * Dropping the last reference to the SearchQuery will close it. + * The resulting object can only be used with calls into this + * account instance. */ - public abstract async Geary.SearchQuery open_search(string query, - SearchQuery.Strategy strategy, - GLib.Cancellable? cancellable) + public abstract async SearchQuery new_search_query(string query, + SearchQuery.Strategy strategy, + GLib.Cancellable? cancellable) throws GLib.Error; /** diff --git a/src/engine/api/geary-search-folder.vala b/src/engine/api/geary-search-folder.vala index 9a2d0cb8..da034c69 100644 --- a/src/engine/api/geary-search-folder.vala +++ b/src/engine/api/geary-search-folder.vala @@ -1,12 +1,13 @@ -/* Copyright 2016 Software Freedom Conservancy Inc. +/* + * Copyright 2016 Software Freedom Conservancy Inc. + * Copyright 2019 Michael Gratton * * This software is licensed under the GNU Lesser General Public License - * (version 2.1 or later). See the COPYING file in this distribution. + * (version 2.1 or later). See the COPYING file in this distribution. */ /** - * Special local {@link Folder} used to query and display search results of {@link Email} from - * across the {@link Account}'s local storage. + * A local folder to execute and collect results of search queries. * * SearchFolder is merely specified to be a Folder, but implementations may add various * {@link FolderSupport} interfaces. In particular {@link FolderSupport.Remove} should be supported, @@ -17,8 +18,9 @@ * translate those EmailIdentifiers to their own type for ordering reasons, but in general the * expectation is that the results of SearchFolder can then be applied to operations on Email in * other remote-backed folders. + * + * @see SearchQuery */ - public abstract class Geary.SearchFolder : Geary.AbstractLocalFolder { private weak Account _account; public override Account account { get { return _account; } } @@ -35,13 +37,13 @@ public abstract class Geary.SearchFolder : Geary.AbstractLocalFolder { } } - public Geary.SearchQuery? search_query { get; protected set; default = null; } - /** - * Fired when the search query has changed. This signal is fired *after* the search - * has completed. - */ - public signal void search_query_changed(Geary.SearchQuery? query); + /** The query being evaluated by this folder, if any. */ + public Geary.SearchQuery? query { get; protected set; default = null; } + + /** Emitted when the current query has been fully evaluated. */ + public signal void query_evaluation_complete(); + protected SearchFolder(Account account, FolderProperties properties, FolderPath path) { _account = account; @@ -49,33 +51,36 @@ public abstract class Geary.SearchFolder : Geary.AbstractLocalFolder { _path = path; } - protected virtual void notify_search_query_changed(SearchQuery? query) { - search_query_changed(query); - } - /** - * Sets the keyword string for this search. + * Sets the query to be evaluated for this folder. * - * This is a nonblocking call that initiates a background search which can be stopped with the - * supplied Cancellable. - * - * When the search is completed, {@link search_query_changed} will be fired. It's possible for - * the {@link search_query} property to change before completion. + * Executes an asynchronous search, which can be stopped via the + * supplied cancellable. When the search is complete, {@link + * query_evaluation_complete} will be emitted. if an error occurs, + * the signal will not be invoked. It is possible for the {@link + * query} property to change before completion. */ - public abstract void search(string query, SearchQuery.Strategy strategy, Cancellable? cancellable = null); + public abstract async void search(SearchQuery query, + GLib.Cancellable? cancellable = null) + throws GLib.Error; /** * Clears the search query and results. * - * {@link search_query_changed} will be fired and {@link search_query} will be set to null. + * The {@link query_evaluation_complete} signal will be emitted + * and {@link query} will be set to null. */ public abstract void clear(); /** - * Given a list of mail IDs, returns a set of casefolded words that match for the current - * search query. + * Returns a set of case-folded words matched by the current query. + * + * The set contains words from the given collection of email that + * match any of the non-negated text operators in {@link query}. */ public abstract async Gee.Set? get_search_matches_async( - Gee.Collection ids, Cancellable? cancellable = null) throws Error; -} + Gee.Collection ids, + GLib.Cancellable? cancellable = null + ) throws GLib.Error; +} diff --git a/src/engine/api/geary-search-query.vala b/src/engine/api/geary-search-query.vala index 17df026a..4c4ec8a3 100644 --- a/src/engine/api/geary-search-query.vala +++ b/src/engine/api/geary-search-query.vala @@ -1,20 +1,27 @@ -/* Copyright 2016 Software Freedom Conservancy Inc. +/* + * Copyright 2016 Software Freedom Conservancy Inc. + * Copyright 2019 Michael Gratton * * This software is licensed under the GNU Lesser General Public License - * (version 2.1 or later). See the COPYING file in this distribution. + * (version 2.1 or later). See the COPYING file in this distribution. */ /** - * An object to hold state for various search subsystems that might need to - * parse the same text string different ways. + * Specifies an expression for searching email in a search folder. * - * The only interaction the API user should have with this is creating new ones and then passing - * them to the search methods in the Engine. + * New instances can be constructed via {@link + * Account.new_search_query} and then passed to search methods on + * {@link Account} or {@link SearchFolder}. * - * @see Geary.Account.open_search + * @see Account.new_search_query + * @see Account.local_search_async + * @see Account.get_search_matches_async + * @see SearchFolder.search */ public abstract class Geary.SearchQuery : BaseObject { + + /** * An advisory parameter regarding search quality, scope, and breadth. * @@ -50,8 +57,14 @@ public abstract class Geary.SearchQuery : BaseObject { HORIZON } + + /** The account that owns this query. */ + public Account owner { get; private set; } + /** - * The original user search text. + * The original search text. + * + * This is used mostly for debugging. */ public string raw { get; private set; } @@ -60,7 +73,11 @@ public abstract class Geary.SearchQuery : BaseObject { */ public Strategy strategy { get; private set; } - protected SearchQuery(string raw, Strategy strategy) { + + protected SearchQuery(Account owner, + string raw, + Strategy strategy) { + this.owner = owner; this.raw = raw; this.strategy = strategy; } diff --git a/src/engine/imap-db/search/imap-db-search-folder.vala b/src/engine/imap-db/search/imap-db-search-folder.vala index 34e3ee71..a3eb8ac6 100644 --- a/src/engine/imap-db/search/imap-db-search-folder.vala +++ b/src/engine/imap-db/search/imap-db-search-folder.vala @@ -52,13 +52,15 @@ private class Geary.ImapDB.SearchFolder : Geary.SearchFolder, Geary.FolderSuppor account.email_removed.disconnect(on_account_email_removed); } - private async void append_new_email_async(Geary.SearchQuery query, Geary.Folder folder, - Gee.Collection ids, Cancellable? cancellable) throws Error { + private async void append_new_email_async(Geary.Folder folder, + Gee.Collection ids, + GLib.Cancellable? cancellable) + throws GLib.Error { int result_mutex_token = yield result_mutex.claim_async(); Error? error = null; try { - yield do_search_async(query, ids, null, cancellable); + yield do_search_async(ids, null, cancellable); } catch(Error e) { error = e; } @@ -69,11 +71,13 @@ private class Geary.ImapDB.SearchFolder : Geary.SearchFolder, Geary.FolderSuppor throw error; } - private async void handle_removed_email_async(Geary.SearchQuery query, Geary.Folder folder, - Gee.Collection ids, Cancellable? cancellable) throws Error { + private async void handle_removed_email_async(Geary.Folder folder, + Gee.Collection ids, + GLib.Cancellable? cancellable) + throws GLib.Error { int result_mutex_token = yield result_mutex.claim_async(); - Error? error = null; + GLib.Error? error = null; try { Gee.ArrayList relevant_ids = Geary.traverse(ids) @@ -81,9 +85,10 @@ private class Geary.ImapDB.SearchFolder : Geary.SearchFolder, Geary.FolderSuppor id => ImapDB.SearchEmailIdentifier.collection_get_email_identifier(search_results, id)) .to_array_list(); - if (relevant_ids.size > 0) - yield do_search_async(query, null, relevant_ids, cancellable); - } catch(Error e) { + if (relevant_ids.size > 0) { + yield do_search_async(null, relevant_ids, cancellable); + } + } catch (GLib.Error e) { error = e; } @@ -97,56 +102,40 @@ private class Geary.ImapDB.SearchFolder : Geary.SearchFolder, Geary.FolderSuppor * Clears the search query and results. */ public override void clear() { - Gee.Collection local_results = search_results; + var local_results = this.search_results; clear_search_results(); notify_email_removed(local_results); notify_email_count_changed(0, Geary.Folder.CountChangeReason.REMOVED); - if (search_query != null) { - search_query = null; - notify_search_query_changed(null); + if (this.query != null) { + this.query = null; + this.query_evaluation_complete(); } } /** * Sets the keyword string for this search. */ - public override void search(string query, Geary.SearchQuery.Strategy strategy, Cancellable? cancellable = null) { - set_search_query_async.begin(query, strategy, cancellable, on_set_search_query_complete); - } - - private void on_set_search_query_complete(Object? source, AsyncResult result) { - try { - set_search_query_async.end(result); - } catch(Error e) { - debug("Search error: %s", e.message); - } - } - - private async void set_search_query_async(string query, - Geary.SearchQuery.Strategy strategy, - Cancellable? cancellable) + public override async void search(Geary.SearchQuery query, + GLib.Cancellable? cancellable = null) throws GLib.Error { - Geary.SearchQuery search_query = yield account.open_search( - query, strategy, cancellable - ); - int result_mutex_token = yield result_mutex.claim_async(); - Error? error = null; + this.query = query; + GLib.Error? error = null; try { - yield do_search_async(search_query, null, null, cancellable); + yield do_search_async(null, null, cancellable); } catch(Error e) { error = e; } result_mutex.release(ref result_mutex_token); - this.search_query = search_query; - notify_search_query_changed(search_query); - - if (error != null) + if (error != null) { throw error; + } + + this.query_evaluation_complete(); } // NOTE: you must call this ONLY after locking result_mutex_token. @@ -155,8 +144,8 @@ private class Geary.ImapDB.SearchFolder : Geary.SearchFolder, Geary.FolderSuppor // 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 // that will be removed if this search doesn't turn them up. - private async void do_search_async(Geary.SearchQuery query, Gee.Collection? add_ids, - Gee.Collection? remove_ids, Cancellable? cancellable) throws Error { + private async void do_search_async(Gee.Collection? add_ids, + Gee.Collection? remove_ids, Cancellable? cancellable) throws Error { // There are three cases here: 1) replace full result set, where the // *_ids parameters are both null, 2) add to result set, where just // remove_ids is null, and 3) remove from result set, where just @@ -167,9 +156,17 @@ private class Geary.ImapDB.SearchFolder : Geary.SearchFolder, Geary.FolderSuppor // smarter about only fetching the search results in list_email_async() // etc., but this leads to some more complications when redoing the // search. - Gee.ArrayList results - = ImapDB.SearchEmailIdentifier.array_list_from_results(yield account.local_search_async( - query, MAX_RESULT_EMAILS, 0, exclude_folders, add_ids ?? remove_ids, cancellable)); + Gee.ArrayList results = + SearchEmailIdentifier.array_list_from_results( + yield account.local_search_async( + this.query, + MAX_RESULT_EMAILS, + 0, + exclude_folders, + add_ids ?? remove_ids, + cancellable + ) + ); Gee.List added = Gee.List.empty(); @@ -338,11 +335,16 @@ private class Geary.ImapDB.SearchFolder : Geary.SearchFolder, Geary.FolderSuppor * search query. */ public override async Gee.Set? get_search_matches_async( - Gee.Collection ids, Cancellable? cancellable = null) throws Error { - if (search_query == null) - return null; - - return yield account.get_search_matches_async(search_query, ids, cancellable); + Gee.Collection ids, + GLib.Cancellable? cancellable = null + ) throws GLib.Error { + Gee.Set? results = null; + if (this.query != null) { + results = yield account.get_search_matches_async( + this.query, ids, cancellable + ); + } + return results; } private void include_folder(Geary.Folder folder) { @@ -358,8 +360,9 @@ private class Geary.ImapDB.SearchFolder : Geary.SearchFolder, Geary.FolderSuppor } private void clear_search_results() { - search_results = new Gee.TreeSet( - ImapDB.SearchEmailIdentifier.compare_descending); + this.search_results = new Gee.TreeSet( + SearchEmailIdentifier.compare_descending + ); } private void on_folders_available_unavailable(Gee.Collection? available, @@ -384,40 +387,42 @@ private class Geary.ImapDB.SearchFolder : Geary.SearchFolder, Geary.FolderSuppor private void on_email_locally_complete(Geary.Folder folder, Gee.Collection ids) { - if (search_query != null) { + if (this.query != null) { this.append_new_email_async.begin( - search_query, folder, ids, null, on_append_new_email_complete + folder, ids, null, + (obj, res) => { + try { + this.append_new_email_async.end(res); + } catch (GLib.Error error) { + this.account.report_problem( + new Geary.AccountProblemReport( + this.account.information, error + ) + ); + } + } ); } } - private void on_append_new_email_complete(GLib.Object? source, - GLib.AsyncResult result) { - try { - this.append_new_email_async.end(result); - } catch (GLib.Error e) { - debug("Error appending new email to search results: %s", e.message); - } - } - private void on_account_email_removed(Geary.Folder folder, Gee.Collection ids) { - if (search_query != null) { + if (this.query != null) { this.handle_removed_email_async.begin( - search_query, folder, ids, null, - on_handle_removed_email_complete + folder, ids, null, + (obj, res) => { + try { + this.handle_removed_email_async.end(res); + } catch (GLib.Error error) { + this.account.report_problem( + new Geary.AccountProblemReport( + this.account.information, error + ) + ); + } + } ); } } - private void on_handle_removed_email_complete(GLib.Object? source, - GLib.AsyncResult result) { - try { - this.handle_removed_email_async.end(result); - } catch (GLib.Error e) { - debug("Error removing removed email from search results: %s", - e.message); - } - } - } diff --git a/src/engine/imap-db/search/imap-db-search-query.vala b/src/engine/imap-db/search/imap-db-search-query.vala index 35c71d7c..ea00787a 100644 --- a/src/engine/imap-db/search/imap-db-search-query.vala +++ b/src/engine/imap-db/search/imap-db-search-query.vala @@ -261,12 +261,13 @@ private class Geary.ImapDB.SearchQuery : Geary.SearchQuery { // A list of all search terms, regardless of search op field name private Gee.ArrayList all = new Gee.ArrayList(); - public async SearchQuery(ImapDB.Account account, + public async SearchQuery(Geary.Account owner, + ImapDB.Account local, string query, Geary.SearchQuery.Strategy strategy, GLib.Cancellable? cancellable) { - base(query, strategy); - this.account = account; + base(owner, query, strategy); + this.account = local; switch (strategy) { case Strategy.EXACT: diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala b/src/engine/imap-engine/imap-engine-generic-account.vala index c0394112..4edcc80e 100644 --- a/src/engine/imap-engine/imap-engine-generic-account.vala +++ b/src/engine/imap-engine/imap-engine-generic-account.vala @@ -200,6 +200,7 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account { // Halt internal tasks early so they stop using local and // remote connections. + this.search_folder.clear(); this.refresh_folder_timer.reset(); this.open_cancellable.cancel(); this.processor.stop(); @@ -524,11 +525,14 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account { return yield local.fetch_email_async(check_id(email_id), required_fields, cancellable); } - public override async Geary.SearchQuery open_search(string query, - SearchQuery.Strategy strategy, - GLib.Cancellable? cancellable) + /** {@inheritDoc} */ + public override async SearchQuery new_search_query(string query, + SearchQuery.Strategy strategy, + GLib.Cancellable? cancellable) throws GLib.Error { - return yield new ImapDB.SearchQuery(local, query, strategy, cancellable); + return yield new ImapDB.SearchQuery( + this, local, query, strategy, cancellable + ); } public override async Gee.Collection? local_search_async(Geary.SearchQuery query, diff --git a/test/engine/api/geary-account-mock.vala b/test/engine/api/geary-account-mock.vala index 1a1aeeb1..07551eb1 100644 --- a/test/engine/api/geary-account-mock.vala +++ b/test/engine/api/geary-account-mock.vala @@ -10,8 +10,9 @@ public class Geary.MockAccount : Account, MockObject { public class MockSearchQuery : SearchQuery { - internal MockSearchQuery() { - base("", SearchQuery.Strategy.EXACT); + internal MockSearchQuery(Account owner, + string raw) { + base(owner, raw, SearchQuery.Strategy.EXACT); } } @@ -214,11 +215,11 @@ public class Geary.MockAccount : Account, MockObject { ); } - public override async SearchQuery open_search(string query, - SearchQuery.Strategy strategy, - GLib.Cancellable? cancellable) + public override async SearchQuery new_search_query(string raw, + SearchQuery.Strategy strategy, + GLib.Cancellable? cancellable) throws GLib.Error { - return new MockSearchQuery(); + return new MockSearchQuery(this, raw); } public override async Gee.Collection? From 981ea845f4ec1ac2f69aadbce4056f65368ec0e6 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Tue, 10 Dec 2019 13:15:32 +1100 Subject: [PATCH 02/13] Make Geary.EmailIdentifier serialisation a bit less ad-hoc Require EmailIdentifier implementations to use an outer GVariant of the form `(yr)` (that is, a byte and an arbitrary length tuple), so that inner representations are independent of the outer format. --- src/engine/api/geary-email-identifier.vala | 2 +- src/engine/imap-db/imap-db-email-identifier.vala | 13 ++++++++----- src/engine/outbox/outbox-email-identifier.vala | 15 +++++++++------ .../imap-engine-generic-account-test.vala | 12 ++++++++++-- 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/engine/api/geary-email-identifier.vala b/src/engine/api/geary-email-identifier.vala index e1df28ed..cdea6a7b 100644 --- a/src/engine/api/geary-email-identifier.vala +++ b/src/engine/api/geary-email-identifier.vala @@ -21,7 +21,7 @@ public abstract class Geary.EmailIdentifier : BaseObject, Gee.Hashable { /** Base variant type returned by {@link to_variant}. */ - public const string BASE_VARIANT_TYPE = "(y??)"; + public const string BASE_VARIANT_TYPE = "(yr)"; // Warning: only change this if you know what you are doing. protected string unique; diff --git a/src/engine/imap-db/imap-db-email-identifier.vala b/src/engine/imap-db/imap-db-email-identifier.vala index 9829d52d..cb2c5dc1 100644 --- a/src/engine/imap-db/imap-db-email-identifier.vala +++ b/src/engine/imap-db/imap-db-email-identifier.vala @@ -9,7 +9,7 @@ private class Geary.ImapDB.EmailIdentifier : Geary.EmailIdentifier { - private const string VARIANT_TYPE = "(yxx)"; + private const string VARIANT_TYPE = "(y(xx))"; public int64 message_id { get; private set; } @@ -41,12 +41,13 @@ private class Geary.ImapDB.EmailIdentifier : Geary.EmailIdentifier { "Invalid serialised id type: %s", serialised.get_type_string() ); } + GLib.Variant inner = serialised.get_child_value(1); Imap.UID? uid = null; - int64 uid_value = serialised.get_child_value(2).get_int64(); + int64 uid_value = inner.get_child_value(1).get_int64(); if (uid_value >= 0) { uid = new Imap.UID(uid_value); } - this(serialised.get_child_value(1).get_int64(), uid); + this(inner.get_child_value(0).get_int64(), uid); } // Used to promote an id created with no_message_id to one that has a @@ -84,8 +85,10 @@ private class Geary.ImapDB.EmailIdentifier : Geary.EmailIdentifier { int64 uid_value = this.uid != null ? this.uid.value : -1; return new GLib.Variant.tuple(new Variant[] { new GLib.Variant.byte('i'), - new GLib.Variant.int64(this.message_id), - new GLib.Variant.int64(uid_value) + new GLib.Variant.tuple(new Variant[] { + new GLib.Variant.int64(this.message_id), + new GLib.Variant.int64(uid_value) + }) }); } diff --git a/src/engine/outbox/outbox-email-identifier.vala b/src/engine/outbox/outbox-email-identifier.vala index cf12b751..63b6f67c 100644 --- a/src/engine/outbox/outbox-email-identifier.vala +++ b/src/engine/outbox/outbox-email-identifier.vala @@ -9,7 +9,7 @@ private class Geary.Outbox.EmailIdentifier : Geary.EmailIdentifier { - private const string VARIANT_TYPE = "(yxx)"; + private const string VARIANT_TYPE = "(y(xx))"; public int64 message_id { get; private set; } public int64 ordering { get; private set; } @@ -28,9 +28,10 @@ private class Geary.Outbox.EmailIdentifier : Geary.EmailIdentifier { "Invalid serialised id type: %s", serialised.get_type_string() ); } - GLib.Variant mid = serialised.get_child_value(1); - GLib.Variant uid = serialised.get_child_value(2); - this(mid.get_int64(), uid.get_int64()); + GLib.Variant inner = serialised.get_child_value(1); + GLib.Variant mid = inner.get_child_value(0); + GLib.Variant ord = inner.get_child_value(1); + this(mid.get_int64(), ord.get_int64()); } public override int natural_sort_comparator(Geary.EmailIdentifier o) { @@ -46,8 +47,10 @@ private class Geary.Outbox.EmailIdentifier : Geary.EmailIdentifier { // inform GenericAccount that it's an SMTP id. return new GLib.Variant.tuple(new Variant[] { new GLib.Variant.byte('o'), - new GLib.Variant.int64(this.message_id), - new GLib.Variant.int64(this.ordering) + new GLib.Variant.tuple(new Variant[] { + new GLib.Variant.int64(this.message_id), + new GLib.Variant.int64(this.ordering) + }) }); } 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 54829e15..b22af954 100644 --- a/test/engine/imap-engine/imap-engine-generic-account-test.vala +++ b/test/engine/imap-engine/imap-engine-generic-account-test.vala @@ -86,10 +86,18 @@ public class Geary.ImapEngine.GenericAccountTest : TestCase { ); assert_non_null( - test_article.to_email_identifier(new GLib.Variant("(yxx)", 'i', 1, 2)) + test_article.to_email_identifier( + new GLib.Variant( + "(yr)", 'i', new GLib.Variant("(xx)", 1, 2) + ) + ) ); assert_non_null( - test_article.to_email_identifier(new GLib.Variant("(yxx)", 'o', 1, 2)) + test_article.to_email_identifier( + new GLib.Variant( + "(yr)", 'o', new GLib.Variant("(xx)", 1, 2) + ) + ) ); } From cb16bdc59d2abfad02f944970603594220e5bdfd Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Tue, 10 Dec 2019 13:18:45 +1100 Subject: [PATCH 03/13] Remove default Geary.EmailIdentifier::hash and ::equal_to impls Make subclasses implement these themselves and remove the unique string property, to be (hopefully) more efficient and easier for subclasses to specialise. --- src/engine/api/geary-email-identifier.vala | 28 ++++++------------- .../imap-db/imap-db-email-identifier.vala | 25 ++++++++++++----- .../outbox/outbox-email-identifier.vala | 21 +++++++++++++- .../api/geary-email-identifier-mock.vala | 28 +++++++++++++++---- 4 files changed, 69 insertions(+), 33 deletions(-) diff --git a/src/engine/api/geary-email-identifier.vala b/src/engine/api/geary-email-identifier.vala index cdea6a7b..3dda2f83 100644 --- a/src/engine/api/geary-email-identifier.vala +++ b/src/engine/api/geary-email-identifier.vala @@ -18,21 +18,18 @@ * they will be unique throughout the Geary engine. */ -public abstract class Geary.EmailIdentifier : BaseObject, Gee.Hashable { +public abstract class Geary.EmailIdentifier : + BaseObject, Gee.Hashable { /** Base variant type returned by {@link to_variant}. */ public const string BASE_VARIANT_TYPE = "(yr)"; - // Warning: only change this if you know what you are doing. - protected string unique; - protected EmailIdentifier(string unique) { - this.unique = unique; - } + /** {@inheritDoc} */ + public abstract uint hash(); - public virtual uint hash() { - return unique.hash(); - } + /** {@inheritDoc} */ + public abstract bool equal_to(EmailIdentifier other); /** * Returns a representation useful for serialisation. @@ -50,16 +47,7 @@ public abstract class Geary.EmailIdentifier : BaseObject, Gee.Hashable to_uids(Gee.Collection ids) { diff --git a/src/engine/outbox/outbox-email-identifier.vala b/src/engine/outbox/outbox-email-identifier.vala index 63b6f67c..76aa34f3 100644 --- a/src/engine/outbox/outbox-email-identifier.vala +++ b/src/engine/outbox/outbox-email-identifier.vala @@ -16,7 +16,6 @@ private class Geary.Outbox.EmailIdentifier : Geary.EmailIdentifier { public EmailIdentifier(int64 message_id, int64 ordering) { - base("Outbox.EmailIdentifier:%s".printf(message_id.to_string())); this.message_id = message_id; this.ordering = ordering; } @@ -34,6 +33,26 @@ private class Geary.Outbox.EmailIdentifier : Geary.EmailIdentifier { this(mid.get_int64(), ord.get_int64()); } + /** {@inheritDoc} */ + public override uint hash() { + return GLib.int64_hash(this.message_id); + } + + /** {@inheritDoc} */ + public override bool equal_to(Geary.EmailIdentifier other) { + return ( + this.get_type() == other.get_type() && + this.message_id == ((EmailIdentifier) other).message_id + ); + } + + /** {@inheritDoc} */ + public override string to_string() { + return "%s(%lld,%lld)".printf( + this.get_type().name(), this.message_id, this.ordering + ); + } + public override int natural_sort_comparator(Geary.EmailIdentifier o) { EmailIdentifier? other = o as EmailIdentifier; if (other == null) { diff --git a/test/engine/api/geary-email-identifier-mock.vala b/test/engine/api/geary-email-identifier-mock.vala index 2928ca16..61b56bdc 100644 --- a/test/engine/api/geary-email-identifier-mock.vala +++ b/test/engine/api/geary-email-identifier-mock.vala @@ -12,17 +12,35 @@ public class Geary.MockEmailIdentifer : EmailIdentifier { public MockEmailIdentifer(int id) { - base(id.to_string()); this.id = id; } + public override uint hash() { + return GLib.int_hash(this.id); + } + + public override bool equal_to(Geary.EmailIdentifier other) { + return ( + this.get_type() == other.get_type() && + this.id == ((MockEmailIdentifer) other).id + ); + } + + + public override string to_string() { + return "%s(%d)".printf( + this.get_type().name(), + this.id + ); + } + + public override GLib.Variant to_variant() { + return new GLib.Variant.int32(id); + } + public override int natural_sort_comparator(Geary.EmailIdentifier other) { MockEmailIdentifer? other_mock = other as MockEmailIdentifer; return (other_mock == null) ? 1 : this.id - other_mock.id; } - public override GLib.Variant to_variant() { - return new GLib.Variant.int32(id); - } - } From b3488f6cf93b22e65e46303e1a2a921cd213b33c Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Tue, 10 Dec 2019 13:36:36 +1100 Subject: [PATCH 04/13] Add Geary.Account::list_local_email_async Add new method and implementation to support breaking search folder impl out of the ImapDb package. --- src/engine/api/geary-account.vala | 16 ++++++ src/engine/imap-db/imap-db-account.vala | 42 ++++++++++++++ .../imap-engine-generic-account.vala | 23 ++++++-- test/engine/api/geary-account-mock.vala | 12 ++++ test/engine/imap-db/imap-db-account-test.vala | 57 +++++++++++++++++++ 5 files changed, 144 insertions(+), 6 deletions(-) diff --git a/src/engine/api/geary-account.vala b/src/engine/api/geary-account.vala index 0bb90aba..54b1d9db 100644 --- a/src/engine/api/geary-account.vala +++ b/src/engine/api/geary-account.vala @@ -425,6 +425,22 @@ public abstract class Geary.Account : BaseObject, Logging.Source { public abstract async Geary.Email local_fetch_email_async(Geary.EmailIdentifier email_id, Geary.Email.Field required_fields, Cancellable? cancellable = null) throws Error; + /** + * Return a collection of email with the given identifiers. + * + * The returned collection will be in the same order as the + * natural ordering of the given identifiers. + * + * Throws {@link EngineError.NOT_FOUND} if any email is not found + * and {@link EngineError.INCOMPLETE_MESSAGE} if the fields aren't + * available. + */ + public abstract async Gee.List list_local_email_async( + Gee.Collection ids, + Email.Field required_fields, + GLib.Cancellable? cancellable = null + ) throws GLib.Error; + /** * Create a new {@link SearchQuery} for this {@link Account}. * diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala index c50a7b5a..59c60125 100644 --- a/src/engine/imap-db/imap-db-account.vala +++ b/src/engine/imap-db/imap-db-account.vala @@ -804,6 +804,48 @@ private class Geary.ImapDB.Account : BaseObject { return search_matches; } + public async Gee.List? list_email(Gee.Collection ids, + Email.Field required_fields, + GLib.Cancellable? cancellable = null) + throws GLib.Error { + check_open(); + + var results = new Gee.ArrayList(); + yield db.exec_transaction_async(Db.TransactionType.RO, (cx) => { + foreach (var id in ids) { + // TODO: once we have a way of deleting messages, we won't be able + // to assume that a row id will point to the same email outside of + // transactions, because SQLite will reuse row ids. + Geary.Email.Field db_fields; + MessageRow row = Geary.ImapDB.Folder.do_fetch_message_row( + cx, id.message_id, required_fields, out db_fields, cancellable + ); + if (!row.fields.fulfills(required_fields)) { + throw new EngineError.INCOMPLETE_MESSAGE( + "Message %s only fulfills %Xh fields (required: %Xh)", + id.to_string(), row.fields, required_fields + ); + } + + Email email = row.to_email(id); + Attachment.add_attachments( + cx, + this.db.attachments_path, + email, + id.message_id, + cancellable + ); + + results.add(email); + } + return Db.TransactionOutcome.DONE; + }, + cancellable + ); + + return results; + } + public async Geary.Email fetch_email_async(ImapDB.EmailIdentifier email_id, Geary.Email.Field required_fields, Cancellable? cancellable = null) throws Error { check_open(); diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala b/src/engine/imap-engine/imap-engine-generic-account.vala index 4edcc80e..7599ee06 100644 --- a/src/engine/imap-engine/imap-engine-generic-account.vala +++ b/src/engine/imap-engine/imap-engine-generic-account.vala @@ -512,6 +512,16 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account { return (Gee.Collection) ids; } + /** {@inheritDoc} */ + public override async SearchQuery new_search_query(string query, + SearchQuery.Strategy strategy, + GLib.Cancellable? cancellable) + throws GLib.Error { + return yield new ImapDB.SearchQuery( + this, local, query, strategy, cancellable + ); + } + public override async Gee.MultiMap? local_search_message_id_async( Geary.RFC822.MessageID message_id, Geary.Email.Field requested_fields, bool partial_ok, Gee.Collection? folder_blacklist, Geary.EmailFlags? flag_blacklist, @@ -526,12 +536,13 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account { } /** {@inheritDoc} */ - public override async SearchQuery new_search_query(string query, - SearchQuery.Strategy strategy, - GLib.Cancellable? cancellable) - throws GLib.Error { - return yield new ImapDB.SearchQuery( - this, local, query, strategy, cancellable + public override async Gee.List list_local_email_async( + Gee.Collection ids, + Email.Field required_fields, + GLib.Cancellable? cancellable = null + ) throws GLib.Error { + return yield local.list_email( + check_ids(ids), required_fields, cancellable ); } diff --git a/test/engine/api/geary-account-mock.vala b/test/engine/api/geary-account-mock.vala index 07551eb1..9f074d62 100644 --- a/test/engine/api/geary-account-mock.vala +++ b/test/engine/api/geary-account-mock.vala @@ -204,6 +204,18 @@ public class Geary.MockAccount : Account, MockObject { ); } + public override async Gee.List list_local_email_async( + Gee.Collection ids, + Email.Field required_fields, + GLib.Cancellable? cancellable = null + ) throws GLib.Error { + return object_or_throw_call>( + "list_local_email_async", + {ids, box_arg(required_fields), cancellable}, + new EngineError.NOT_FOUND("Mock call") + ); + } + public override async Email local_fetch_email_async(EmailIdentifier email_id, Email.Field required_fields, Cancellable? cancellable = null) diff --git a/test/engine/imap-db/imap-db-account-test.vala b/test/engine/imap-db/imap-db-account-test.vala index a82fb1ce..3d1e90ab 100644 --- a/test/engine/imap-db/imap-db-account-test.vala +++ b/test/engine/imap-db/imap-db-account-test.vala @@ -26,6 +26,7 @@ class Geary.ImapDB.AccountTest : TestCase { add_test("fetch_base_folder", fetch_base_folder); add_test("fetch_child_folder", fetch_child_folder); add_test("fetch_nonexistent_folder", fetch_nonexistent_folder); + add_test("list_local_email", list_local_email); } public override void set_up() throws GLib.Error { @@ -310,4 +311,60 @@ class Geary.ImapDB.AccountTest : TestCase { } } + public void list_local_email() throws GLib.Error { + Email.Field fixture_fields = Email.Field.RECEIVERS; + string fixture_to = "test1@example.com"; + this.account.db.exec( + "INSERT INTO MessageTable (id, fields, to_field) " + + "VALUES (1, %d, '%s');".printf(fixture_fields, fixture_to) + ); + this.account.db.exec( + "INSERT INTO MessageTable (id, fields, to_field) " + + "VALUES (2, %d, '%s');".printf(fixture_fields, fixture_to) + ); + + this.account.list_email.begin( + iterate_array({ + new EmailIdentifier(1, null), + new EmailIdentifier(2, null) + }).to_linked_list(), + Email.Field.RECEIVERS, + null, + (obj, ret) => { async_complete(ret); } + ); + Gee.List result = this.account.list_email.end( + async_result() + ); + + assert_int(2, result.size, "Not enough email listed"); + assert_true(new EmailIdentifier(1, null).equal_to(result[0].id)); + assert_true(new EmailIdentifier(2, null).equal_to(result[1].id)); + + this.account.list_email.begin( + Collection.single(new EmailIdentifier(3, null)), + Email.Field.RECEIVERS, + null, + (obj, ret) => { async_complete(ret); } + ); + try { + this.account.list_email.end(async_result()); + assert_not_reached(); + } catch (EngineError.NOT_FOUND error) { + // All good + } + + this.account.list_email.begin( + Collection.single(new EmailIdentifier(1, null)), + Email.Field.BODY, + null, + (obj, ret) => { async_complete(ret); } + ); + try { + this.account.list_email.end(async_result()); + assert_not_reached(); + } catch (EngineError.INCOMPLETE_MESSAGE error) { + // All good + } + } + } From 924104c282af80161ff2b9dd683481ed2a722eb2 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Tue, 10 Dec 2019 20:51:20 +1100 Subject: [PATCH 05/13] Clean up search folder implementation Move SearchFolder and search EmailIdentifier implementation out of ImapDb and into its own package. Decouple both from ImapDB, and improve the implementation, fixing a few inefficiencies. Merge search FolderProperties into the SearchFoldern implementation as an inner class. Merge SearchTerm into ImapDB.SearchQuery as an inner class and move the outer class's source down a level, since it was the only file left in the imap-db/search dir. --- po/POTFILES.in | 8 +- src/engine/imap-db/imap-db-account.vala | 12 +- .../{search => }/imap-db-search-query.vala | 89 ++- .../imap-db-search-email-identifier.vala | 75 --- .../imap-db-search-folder-properties.vala | 16 - .../imap-db/search/imap-db-search-folder.vala | 428 -------------- .../imap-db/search/imap-db-search-term.vala | 62 -- .../imap-engine-gmail-search-folder.vala | 2 +- .../imap-engine-generic-account.vala | 4 +- src/engine/meson.build | 9 +- .../search/search-email-identifier.vala | 129 +++++ src/engine/search/search-folder-impl.vala | 546 ++++++++++++++++++ .../imap-engine-generic-account-test.vala | 15 + 13 files changed, 778 insertions(+), 617 deletions(-) rename src/engine/imap-db/{search => }/imap-db-search-query.vala (89%) delete mode 100644 src/engine/imap-db/search/imap-db-search-email-identifier.vala delete mode 100644 src/engine/imap-db/search/imap-db-search-folder-properties.vala delete mode 100644 src/engine/imap-db/search/imap-db-search-folder.vala delete mode 100644 src/engine/imap-db/search/imap-db-search-term.vala create mode 100644 src/engine/search/search-email-identifier.vala create mode 100644 src/engine/search/search-folder-impl.vala diff --git a/po/POTFILES.in b/po/POTFILES.in index 5ec9b69e..71229fb7 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -229,11 +229,7 @@ src/engine/imap-db/imap-db-email-identifier.vala src/engine/imap-db/imap-db-folder.vala src/engine/imap-db/imap-db-gc.vala src/engine/imap-db/imap-db-message-row.vala -src/engine/imap-db/search/imap-db-search-email-identifier.vala -src/engine/imap-db/search/imap-db-search-folder-properties.vala -src/engine/imap-db/search/imap-db-search-folder.vala -src/engine/imap-db/search/imap-db-search-query.vala -src/engine/imap-db/search/imap-db-search-term.vala +src/engine/imap-db/imap-db-search-query.vala src/engine/imap-engine/gmail/imap-engine-gmail-account.vala src/engine/imap-engine/gmail/imap-engine-gmail-all-mail-folder.vala src/engine/imap-engine/gmail/imap-engine-gmail-drafts-folder.vala @@ -365,6 +361,8 @@ src/engine/rfc822/rfc822-message.vala src/engine/rfc822/rfc822-part.vala src/engine/rfc822/rfc822-utils.vala src/engine/rfc822/rfc822.vala +src/engine/search/search-email-identifier.vala +src/engine/search/search-folder-impl.vala src/engine/smtp/smtp-authenticator.vala src/engine/smtp/smtp-capabilities.vala src/engine/smtp/smtp-client-connection.vala diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala index 59c60125..6540494a 100644 --- a/src/engine/imap-db/imap-db-account.vala +++ b/src/engine/imap-db/imap-db-account.vala @@ -575,7 +575,7 @@ private class Geary.ImapDB.Account : BaseObject { foreach (string? field in query.get_fields()) { debug(" - Field \"%s\" terms:", field); - foreach (SearchTerm? term in query.get_search_terms(field)) { + foreach (SearchQuery.Term? term in query.get_search_terms(field)) { if (term != null) { debug(" - \"%s\": %s, %s", term.original, @@ -613,7 +613,7 @@ private class Geary.ImapDB.Account : BaseObject { // . StringBuilder sql = new StringBuilder(); sql.append(""" - SELECT id, internaldate_time_t + SELECT id FROM MessageTable INDEXED BY MessageTableInternalDateTimeTIndex """); @@ -650,11 +650,7 @@ private class Geary.ImapDB.Account : BaseObject { Db.Result result = stmt.exec(cancellable); while (!result.finished) { int64 message_id = result.int64_at(0); - int64 internaldate_time_t = result.int64_at(1); - DateTime? internaldate = (internaldate_time_t == -1 - ? null : new DateTime.from_unix_local(internaldate_time_t)); - - ImapDB.EmailIdentifier id = new ImapDB.SearchEmailIdentifier(message_id, internaldate); + var id = new ImapDB.EmailIdentifier(message_id, null); matching_ids.add(id); id_map.set(message_id, id); @@ -739,7 +735,7 @@ private class Geary.ImapDB.Account : BaseObject { Gee.Set? result = results.get(id); if (result != null) { foreach (string match in result) { - foreach (SearchTerm term in query.get_all_terms()) { + foreach (SearchQuery.Term term in query.get_all_terms()) { // if prefix-matches parsed term, then don't strip if (match.has_prefix(term.parsed)) { good_match_found = true; diff --git a/src/engine/imap-db/search/imap-db-search-query.vala b/src/engine/imap-db/imap-db-search-query.vala similarity index 89% rename from src/engine/imap-db/search/imap-db-search-query.vala rename to src/engine/imap-db/imap-db-search-query.vala index ea00787a..85f556fa 100644 --- a/src/engine/imap-db/search/imap-db-search-query.vala +++ b/src/engine/imap-db/imap-db-search-query.vala @@ -43,6 +43,63 @@ private class Geary.ImapDB.SearchQuery : Geary.SearchQuery { private const string SEARCH_OP_VALUE_UNREAD = "unread"; + /** + * Various associated state with a single term in a search query. + */ + internal class Term : GLib.Object { + + /** + * The original tokenized search term with minimal other processing performed. + * + * For example, punctuation might be removed, but no casefolding has occurred. + */ + public string original { get; private set; } + + /** + * The parsed tokenized search term. + * + * Casefolding and other normalizing text operations have been performed. + */ + public string parsed { get; private set; } + + /** + * The stemmed search term. + * + * Only used if stemming is being done ''and'' the stem is different than the {@link parsed} + * term. + */ + public string? stemmed { get; private set; } + + /** + * A list of terms ready for binding to an SQLite statement. + * + * This should include prefix operators and quotes (i.e. ["party"] or [party*]). These texts + * are guaranteed not to be null or empty strings. + */ + public Gee.List sql { get; private set; default = new Gee.ArrayList(); } + + /** + * Returns true if the {@link parsed} term is exact-match only (i.e. starts with quotes) and + * there is no {@link stemmed} variant. + */ + public bool is_exact { get { return parsed.has_prefix("\"") && stemmed == null; } } + + public Term(string original, string parsed, string? stemmed, string? sql_parsed, string? sql_stemmed) { + this.original = original; + this.parsed = parsed; + this.stemmed = stemmed; + + // for now, only two variations: the parsed string and the stemmed; since stem is usually + // shorter (and will be first in the OR statement), include it first + if (!String.is_empty(sql_stemmed)) + sql.add(sql_stemmed); + + if (!String.is_empty(sql_parsed)) + sql.add(sql_parsed); + } + } + + // Maps of localised search operator names and values to their // internal forms private static Gee.HashMap search_op_names = @@ -255,11 +312,11 @@ private class Geary.ImapDB.SearchQuery : Geary.SearchQuery { // their search term values. Note that terms without an operator // are stored with null as the key. Not using a MultiMap because // we (might) need a guarantee of order. - private Gee.HashMap> field_map - = new Gee.HashMap>(); + private Gee.HashMap> field_map + = new Gee.HashMap>(); // A list of all search terms, regardless of search op field name - private Gee.ArrayList all = new Gee.ArrayList(); + private Gee.ArrayList all = new Gee.ArrayList(); public async SearchQuery(Geary.Account owner, ImapDB.Account local, @@ -306,11 +363,11 @@ private class Geary.ImapDB.SearchQuery : Geary.SearchQuery { return field_map.keys; } - public Gee.List? get_search_terms(string? field) { + public Gee.List? get_search_terms(string? field) { return field_map.has_key(field) ? field_map.get(field) : null; } - public Gee.List? get_all_terms() { + public Gee.List? get_all_terms() { return all; } @@ -330,7 +387,7 @@ private class Geary.ImapDB.SearchQuery : Geary.SearchQuery { bool strip_results = true; if (this.strategy == Geary.SearchQuery.Strategy.HORIZON) strip_results = false; - else if (traverse(this.all).any( + else if (traverse(this.all).any( term => term.stemmed == null || term.is_exact)) { strip_results = false; } @@ -342,8 +399,8 @@ private class Geary.ImapDB.SearchQuery : Geary.SearchQuery { new Gee.HashMap(); foreach (string? field in this.field_map.keys) { if (field == SEARCH_OP_IS) { - Gee.List? terms = get_search_terms(field); - foreach (SearchTerm term in terms) + Gee.List? terms = get_search_terms(field); + foreach (Term term in terms) if (term.parsed == SEARCH_OP_VALUE_READ) conditions.set(new NamedFlag("UNREAD"), true); else if (term.parsed == SEARCH_OP_VALUE_UNREAD) @@ -359,11 +416,11 @@ private class Geary.ImapDB.SearchQuery : Geary.SearchQuery { internal Gee.HashMap get_query_phrases() { Gee.HashMap phrases = new Gee.HashMap(); foreach (string? field in field_map.keys) { - Gee.List? terms = get_search_terms(field); + Gee.List? terms = get_search_terms(field); if (terms == null || terms.size == 0 || field == "is") continue; - // Each SearchTerm is an AND but the SQL text within in are OR ... this allows for + // Each Term is an AND but the SQL text within in are OR ... this allows for // each user term to be AND but the variants of each term are or. So, if terms are // [party] and [eventful] and stems are [parti] and [event], the search would be: // @@ -380,7 +437,7 @@ private class Geary.ImapDB.SearchQuery : Geary.SearchQuery { // // party* OR parti* eventful* OR event* StringBuilder builder = new StringBuilder(); - foreach (SearchTerm term in terms) { + foreach (Term term in terms) { if (term.sql.size == 0) continue; @@ -439,12 +496,12 @@ private class Geary.ImapDB.SearchQuery : Geary.SearchQuery { --quotes; } - SearchTerm? term; + Term? term; if (in_quote) { // HACK: this helps prevent a syntax error when the user types // something like from:"somebody". If we ever properly support // quotes after : we can get rid of this. - term = new SearchTerm(s, s, null, s.replace(":", " "), null); + term = new Term(s, s, null, s.replace(":", " "), null); } else { string original = s; @@ -480,7 +537,7 @@ private class Geary.ImapDB.SearchQuery : Geary.SearchQuery { if (field == SEARCH_OP_IS) { // s will have been de-translated - term = new SearchTerm(original, s, null, null, null); + term = new Term(original, s, null, null, null); } else { // SQL MATCH syntax for parsed term string? sql_s = "%s*".printf(s); @@ -506,7 +563,7 @@ private class Geary.ImapDB.SearchQuery : Geary.SearchQuery { if (String.contains_any_char(s, SEARCH_TERM_CONTINUATION_CHARS)) s = "\"%s\"".printf(s); - term = new SearchTerm(original, s, stemmed, sql_s, sql_stemmed); + term = new Term(original, s, stemmed, sql_s, sql_stemmed); } } @@ -515,7 +572,7 @@ private class Geary.ImapDB.SearchQuery : Geary.SearchQuery { // Finally, add the term if (!this.field_map.has_key(field)) { - this.field_map.set(field, new Gee.ArrayList()); + this.field_map.set(field, new Gee.ArrayList()); } this.field_map.get(field).add(term); this.all.add(term); diff --git a/src/engine/imap-db/search/imap-db-search-email-identifier.vala b/src/engine/imap-db/search/imap-db-search-email-identifier.vala deleted file mode 100644 index 0a014851..00000000 --- a/src/engine/imap-db/search/imap-db-search-email-identifier.vala +++ /dev/null @@ -1,75 +0,0 @@ -/* Copyright 2016 Software Freedom Conservancy Inc. - * - * This software is licensed under the GNU Lesser General Public License - * (version 2.1 or later). See the COPYING file in this distribution. - */ - -private class Geary.ImapDB.SearchEmailIdentifier : ImapDB.EmailIdentifier, - Gee.Comparable { - public DateTime? date_received { get; private set; } - - public SearchEmailIdentifier(int64 message_id, DateTime? date_received) { - base(message_id, null); - - this.date_received = date_received; - } - - public static int compare_descending(SearchEmailIdentifier a, SearchEmailIdentifier b) { - return b.compare_to(a); - } - - public static Gee.ArrayList array_list_from_results( - Gee.Collection? results) { - Gee.ArrayList r = new Gee.ArrayList(); - - if (results != null) { - foreach (Geary.EmailIdentifier id in results) { - SearchEmailIdentifier? search_id = id as SearchEmailIdentifier; - - assert(search_id != null); - r.add(search_id); - } - } - - return r; - } - - // Searches for a generic EmailIdentifier in a collection of SearchEmailIdentifiers. - public static SearchEmailIdentifier? collection_get_email_identifier( - Gee.Collection collection, Geary.EmailIdentifier id) { - foreach (SearchEmailIdentifier search_id in collection) { - if (id.equal_to(search_id)) - return search_id; - } - return null; - } - - public override int natural_sort_comparator(Geary.EmailIdentifier o) { - ImapDB.SearchEmailIdentifier? other = o as ImapDB.SearchEmailIdentifier; - if (other == null) - return 1; - - return compare_to(other); - } - - public virtual int compare_to(SearchEmailIdentifier 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); - } - - public override string to_string() { - return "[%s/null/%s]".printf(message_id.to_string(), - (date_received == null ? "null" : date_received.to_string())); - } -} diff --git a/src/engine/imap-db/search/imap-db-search-folder-properties.vala b/src/engine/imap-db/search/imap-db-search-folder-properties.vala deleted file mode 100644 index 870a55ef..00000000 --- a/src/engine/imap-db/search/imap-db-search-folder-properties.vala +++ /dev/null @@ -1,16 +0,0 @@ -/* Copyright 2016 Software Freedom Conservancy Inc. - * - * This software is licensed under the GNU Lesser General Public License - * (version 2.1 or later). See the COPYING file in this distribution. - */ - -private class Geary.ImapDB.SearchFolderProperties : Geary.FolderProperties { - public SearchFolderProperties(int total, int unread) { - base(total, unread, Trillian.FALSE, Trillian.FALSE, Trillian.TRUE, true, true, false); - } - - public void set_total(int total) { - this.email_total = total; - } -} - diff --git a/src/engine/imap-db/search/imap-db-search-folder.vala b/src/engine/imap-db/search/imap-db-search-folder.vala deleted file mode 100644 index a3eb8ac6..00000000 --- a/src/engine/imap-db/search/imap-db-search-folder.vala +++ /dev/null @@ -1,428 +0,0 @@ -/* Copyright 2016 Software Freedom Conservancy Inc. - * - * This software is licensed under the GNU Lesser General Public License - * (version 2.1 or later). See the COPYING file in this distribution. - */ - -private class Geary.ImapDB.SearchFolder : Geary.SearchFolder, Geary.FolderSupport.Remove { - - - /** Max number of emails that can ever be in the folder. */ - public const int MAX_RESULT_EMAILS = 1000; - - /** The canonical name of the search folder. */ - public const string MAGIC_BASENAME = "$GearySearchFolder$"; - - private const Geary.SpecialFolderType[] EXCLUDE_TYPES = { - Geary.SpecialFolderType.SPAM, - Geary.SpecialFolderType.TRASH, - Geary.SpecialFolderType.DRAFTS, - // Orphan emails (without a folder) are also excluded; see ctor. - }; - - - private Gee.HashSet exclude_folders = new Gee.HashSet(); - private Gee.TreeSet search_results; - private Geary.Nonblocking.Mutex result_mutex = new Geary.Nonblocking.Mutex(); - - - public SearchFolder(Geary.Account account, FolderRoot root) { - base( - account, - new SearchFolderProperties(0, 0), - root.get_child(MAGIC_BASENAME, Trillian.TRUE) - ); - - account.folders_available_unavailable.connect(on_folders_available_unavailable); - account.folders_special_type.connect(on_folders_special_type); - account.email_locally_complete.connect(on_email_locally_complete); - account.email_removed.connect(on_account_email_removed); - - clear_search_results(); - - // We always want to exclude emails that don't live anywhere from - // search results. - exclude_orphan_emails(); - } - - ~SearchFolder() { - account.folders_available_unavailable.disconnect(on_folders_available_unavailable); - account.folders_special_type.disconnect(on_folders_special_type); - account.email_locally_complete.disconnect(on_email_locally_complete); - account.email_removed.disconnect(on_account_email_removed); - } - - private async void append_new_email_async(Geary.Folder folder, - Gee.Collection ids, - GLib.Cancellable? cancellable) - throws GLib.Error { - int result_mutex_token = yield result_mutex.claim_async(); - - Error? error = null; - try { - yield do_search_async(ids, null, cancellable); - } catch(Error e) { - error = e; - } - - result_mutex.release(ref result_mutex_token); - - if (error != null) - throw error; - } - - private async void handle_removed_email_async(Geary.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 { - Gee.ArrayList relevant_ids - = Geary.traverse(ids) - .map_nonnull( - id => ImapDB.SearchEmailIdentifier.collection_get_email_identifier(search_results, id)) - .to_array_list(); - - if (relevant_ids.size > 0) { - yield do_search_async(null, relevant_ids, cancellable); - } - } catch (GLib.Error e) { - error = e; - } - - result_mutex.release(ref result_mutex_token); - - if (error != null) - throw error; - } - - /** - * Clears the search query and results. - */ - public override void clear() { - var local_results = this.search_results; - clear_search_results(); - notify_email_removed(local_results); - notify_email_count_changed(0, Geary.Folder.CountChangeReason.REMOVED); - - if (this.query != null) { - this.query = null; - this.query_evaluation_complete(); - } - } - - /** - * Sets the keyword string for this search. - */ - public override async void search(Geary.SearchQuery query, - GLib.Cancellable? cancellable = null) - throws GLib.Error { - int result_mutex_token = yield result_mutex.claim_async(); - - this.query = query; - GLib.Error? error = null; - try { - yield do_search_async(null, null, cancellable); - } catch(Error e) { - error = e; - } - - result_mutex.release(ref result_mutex_token); - - if (error != null) { - throw error; - } - - this.query_evaluation_complete(); - } - - // 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 - // that will be removed if this search doesn't turn them up. - private async void do_search_async(Gee.Collection? add_ids, - Gee.Collection? remove_ids, Cancellable? cancellable) throws Error { - // There are three cases here: 1) replace full result set, where the - // *_ids parameters are both null, 2) add to result set, where just - // remove_ids is null, and 3) remove from result set, where just - // add_ids is null. We can't add and remove at the same time. - assert(add_ids == null || remove_ids == null); - - // TODO: don't limit this to MAX_RESULT_EMAILS. Instead, 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.ArrayList results = - SearchEmailIdentifier.array_list_from_results( - yield account.local_search_async( - this.query, - MAX_RESULT_EMAILS, - 0, - exclude_folders, - add_ids ?? remove_ids, - cancellable - ) - ); - - Gee.List added - = Gee.List.empty(); - Gee.List removed - = Gee.List.empty(); - - if (remove_ids == null) { - added = Geary.traverse(results) - .filter(id => !(id in search_results)) - .to_array_list(); - } - if (add_ids == null) { - removed = Geary.traverse(remove_ids ?? search_results) - .filter(id => !(id in results)) - .to_array_list(); - } - - search_results.remove_all(removed); - search_results.add_all(added); - - ((ImapDB.SearchFolderProperties) properties).set_total(search_results.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; - 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; - } - if (removed.size > 0) { - notify_email_removed(removed); - reason |= Geary.Folder.CountChangeReason.REMOVED; - } - if (reason != CountChangeReason.NONE) - notify_email_count_changed(search_results.size, reason); - } - - 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) - return null; - - // TODO: as above, this is incomplete and inefficient. - int result_mutex_token = yield result_mutex.claim_async(); - - Geary.EmailIdentifier[] ids = new Geary.EmailIdentifier[search_results.size]; - int initial_index = 0; - int i = 0; - foreach (ImapDB.SearchEmailIdentifier id in search_results) { - if (initial_id != null && id.equal_to(initial_id)) - initial_index = i; - ids[i++] = id; - } - - if (initial_id == null && flags.is_all_set(Geary.Folder.ListFlags.OLDEST_TO_NEWEST)) - initial_index = ids.length - 1; - - Gee.List results = new Gee.ArrayList(); - Error? fetch_err = null; - if (initial_index >= 0) { - int increment = flags.is_oldest_to_newest() ? -1 : 1; - i = initial_index; - if (!flags.is_including_id() && initial_id != null) - i += increment; - int end = i + (count * increment); - - for (; i >= 0 && i < search_results.size && i != end; i += increment) { - try { - results.add(yield fetch_email_async(ids[i], required_fields, flags, cancellable)); - } catch (Error err) { - // Don't let missing or incomplete messages stop the list operation, which has - // different semantics from fetch - if (!(err is EngineError.NOT_FOUND) && !(err is EngineError.INCOMPLETE_MESSAGE)) { - fetch_err = err; - - break; - } - } - } - } - - result_mutex.release(ref result_mutex_token); - - if (fetch_err != null) - throw fetch_err; - - return (results.size == 0 ? null : results); - } - - public override async Gee.List? list_email_by_sparse_id_async( - Gee.Collection ids, Geary.Email.Field required_fields, - Geary.Folder.ListFlags flags, Cancellable? cancellable = null) throws Error { - // TODO: Fetch emails in a batch. - Gee.List result = new Gee.ArrayList(); - foreach(Geary.EmailIdentifier id in ids) - result.add(yield fetch_email_async(id, required_fields, flags, cancellable)); - - return (result.size == 0 ? null : result); - } - - 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. - 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) throws Error { - return yield account.local_fetch_email_async(id, required_fields, cancellable); - } - - public virtual async void - remove_email_async(Gee.Collection email_ids, - GLib.Cancellable? cancellable = null) - throws GLib.Error { - Gee.MultiMap? ids_to_folders - = yield account.get_containing_folders_async(email_ids, cancellable); - if (ids_to_folders == null) - return; - - Gee.MultiMap folders_to_ids - = Geary.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) - continue; - - Gee.Collection ids = folders_to_ids.get(path); - assert(ids.size > 0); - - 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); - open = true; - yield remove.remove_email_async( - Collection.copy(ids), cancellable - ); - } finally { - if (open) { - try { - yield folder.close_async(); - } catch (Error e) { - debug("Error closing folder %s: %s", folder.to_string(), e.message); - } - } - } - } - } - - /** - * Given a list of mail IDs, returns a set of casefolded words that match for the current - * search query. - */ - public override async Gee.Set? get_search_matches_async( - Gee.Collection ids, - GLib.Cancellable? cancellable = null - ) throws GLib.Error { - Gee.Set? results = null; - if (this.query != null) { - results = yield account.get_search_matches_async( - this.query, ids, cancellable - ); - } - return results; - } - - private void include_folder(Geary.Folder folder) { - this.exclude_folders.remove(folder.path); - } - - private void exclude_folder(Geary.Folder folder) { - this.exclude_folders.add(folder.path); - } - - private void exclude_orphan_emails() { - this.exclude_folders.add(null); - } - - private void clear_search_results() { - this.search_results = new Gee.TreeSet( - SearchEmailIdentifier.compare_descending - ); - } - - 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) - .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) { - if (folder.special_folder_type in EXCLUDE_TYPES) { - exclude_folder(folder); - } else { - include_folder(folder); - } - } - } - - private void on_email_locally_complete(Geary.Folder folder, - Gee.Collection ids) { - if (this.query != null) { - this.append_new_email_async.begin( - folder, ids, null, - (obj, res) => { - try { - this.append_new_email_async.end(res); - } catch (GLib.Error error) { - this.account.report_problem( - new Geary.AccountProblemReport( - this.account.information, error - ) - ); - } - } - ); - } - } - - private void on_account_email_removed(Geary.Folder folder, - Gee.Collection ids) { - if (this.query != null) { - this.handle_removed_email_async.begin( - folder, ids, null, - (obj, res) => { - try { - this.handle_removed_email_async.end(res); - } catch (GLib.Error error) { - this.account.report_problem( - new Geary.AccountProblemReport( - this.account.information, error - ) - ); - } - } - ); - } - } - -} diff --git a/src/engine/imap-db/search/imap-db-search-term.vala b/src/engine/imap-db/search/imap-db-search-term.vala deleted file mode 100644 index 8427b25f..00000000 --- a/src/engine/imap-db/search/imap-db-search-term.vala +++ /dev/null @@ -1,62 +0,0 @@ -/* Copyright 2016 Software Freedom Conservancy Inc. - * - * This software is licensed under the GNU Lesser General Public License - * (version 2.1 or later). See the COPYING file in this distribution. - */ - -/** - * Various associated state with a single term in a {@link ImapDB.SearchQuery}. - */ - -private class Geary.ImapDB.SearchTerm : BaseObject { - /** - * The original tokenized search term with minimal other processing performed. - * - * For example, punctuation might be removed, but no casefolding has occurred. - */ - public string original { get; private set; } - - /** - * The parsed tokenized search term. - * - * Casefolding and other normalizing text operations have been performed. - */ - public string parsed { get; private set; } - - /** - * The stemmed search term. - * - * Only used if stemming is being done ''and'' the stem is different than the {@link parsed} - * term. - */ - public string? stemmed { get; private set; } - - /** - * A list of terms ready for binding to an SQLite statement. - * - * This should include prefix operators and quotes (i.e. ["party"] or [party*]). These texts - * are guaranteed not to be null or empty strings. - */ - public Gee.List sql { get; private set; default = new Gee.ArrayList(); } - - /** - * Returns true if the {@link parsed} term is exact-match only (i.e. starts with quotes) and - * there is no {@link stemmed} variant. - */ - public bool is_exact { get { return parsed.has_prefix("\"") && stemmed == null; } } - - public SearchTerm(string original, string parsed, string? stemmed, string? sql_parsed, string? sql_stemmed) { - this.original = original; - this.parsed = parsed; - this.stemmed = stemmed; - - // for now, only two variations: the parsed string and the stemmed; since stem is usually - // shorter (and will be first in the OR statement), include it first - if (!String.is_empty(sql_stemmed)) - sql.add(sql_stemmed); - - if (!String.is_empty(sql_parsed)) - sql.add(sql_parsed); - } -} - diff --git a/src/engine/imap-engine/gmail/imap-engine-gmail-search-folder.vala b/src/engine/imap-engine/gmail/imap-engine-gmail-search-folder.vala index ef47256d..aacc0340 100644 --- a/src/engine/imap-engine/gmail/imap-engine-gmail-search-folder.vala +++ b/src/engine/imap-engine/gmail/imap-engine-gmail-search-folder.vala @@ -8,7 +8,7 @@ /** * Gmail-specific SearchFolder implementation. */ -private class Geary.ImapEngine.GmailSearchFolder : ImapDB.SearchFolder { +private class Geary.ImapEngine.GmailSearchFolder : Search.FolderImpl { private Geary.App.EmailStore email_store; diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala b/src/engine/imap-engine/imap-engine-generic-account.vala index 7599ee06..86f0c877 100644 --- a/src/engine/imap-engine/imap-engine-generic-account.vala +++ b/src/engine/imap-engine/imap-engine-generic-account.vala @@ -417,6 +417,8 @@ 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 Search.EmailIdentifier.from_variant(serialised, this); if (type == 'o') return new Outbox.EmailIdentifier.from_variant(serialised); @@ -808,7 +810,7 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account { * override this to return the correct subclass. */ protected virtual SearchFolder new_search_folder() { - return new ImapDB.SearchFolder(this, this.local_folder_root); + return new Search.FolderImpl(this, this.local_folder_root); } /** {@inheritDoc} */ diff --git a/src/engine/meson.build b/src/engine/meson.build index 3d83d81e..d98f9050 100644 --- a/src/engine/meson.build +++ b/src/engine/meson.build @@ -178,11 +178,7 @@ geary_engine_vala_sources = files( 'imap-db/imap-db-folder.vala', 'imap-db/imap-db-gc.vala', 'imap-db/imap-db-message-row.vala', - 'imap-db/search/imap-db-search-email-identifier.vala', - 'imap-db/search/imap-db-search-folder.vala', - 'imap-db/search/imap-db-search-folder-properties.vala', - 'imap-db/search/imap-db-search-query.vala', - 'imap-db/search/imap-db-search-term.vala', + 'imap-db/imap-db-search-query.vala', 'imap-engine/imap-engine.vala', 'imap-engine/imap-engine-account-operation.vala', @@ -274,6 +270,9 @@ geary_engine_vala_sources = files( 'rfc822/rfc822-part.vala', 'rfc822/rfc822-utils.vala', + 'search/search-email-identifier.vala', + 'search/search-folder-impl.vala', + 'smtp/smtp-authenticator.vala', 'smtp/smtp-capabilities.vala', 'smtp/smtp-client-connection.vala', diff --git a/src/engine/search/search-email-identifier.vala b/src/engine/search/search-email-identifier.vala new file mode 100644 index 00000000..a6c9b4ea --- /dev/null +++ b/src/engine/search/search-email-identifier.vala @@ -0,0 +1,129 @@ +/* + * Copyright 2016 Software Freedom Conservancy Inc. + * Copyright 2019 Michael Gratton + * + * This software is licensed under the GNU Lesser General Public License + * (version 2.1 or later). See the COPYING file in this distribution. + */ + +private class Geary.Search.EmailIdentifier : + Geary.EmailIdentifier, Gee.Comparable { + + + 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; + } + + /** Reconstructs an identifier from its variant representation. */ + 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() + ) + ); + } + + /** {@inheritDoc} */ + public override uint hash() { + return this.source_id.hash(); + } + + /** {@inheritDoc} */ + public override bool equal_to(Geary.EmailIdentifier other) { + return ( + this.get_type() == other.get_type() && + this.source_id.equal_to(((EmailIdentifier) other).source_id) + ); + } + + /** {@inheritDoc} */ + 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()) + }) + }); + } + + /** {@inheritDoc} */ + 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); + } + +} diff --git a/src/engine/search/search-folder-impl.vala b/src/engine/search/search-folder-impl.vala new file mode 100644 index 00000000..25009437 --- /dev/null +++ b/src/engine/search/search-folder-impl.vala @@ -0,0 +1,546 @@ +/* + * Copyright 2016 Software Freedom Conservancy Inc. + * Copyright 2019 Michael Gratton + * + * This software is licensed under the GNU Lesser General Public License + * (version 2.1 or later). See the COPYING file in this distribution. + */ + +/** + * A default implementation of a search folder. + * + * This implementation of {@link Geary.SearchFolder} uses the search + * methods on {@link Account} to implement account-wide email search. + */ +internal class Geary.Search.FolderImpl : + Geary.SearchFolder, Geary.FolderSupport.Remove { + + + /** Max number of emails that can ever be in the folder. */ + public const int MAX_RESULT_EMAILS = 1000; + + /** The canonical name of the search folder. */ + public const string MAGIC_BASENAME = "$GearySearchFolder$"; + + private const Geary.SpecialFolderType[] EXCLUDE_TYPES = { + Geary.SpecialFolderType.SPAM, + Geary.SpecialFolderType.TRASH, + Geary.SpecialFolderType.DRAFTS, + // Orphan emails (without a folder) are also excluded; see ct or. + }; + + + private class FolderProperties : Geary.FolderProperties { + + + public FolderProperties(int total, int unread) { + base(total, unread, Trillian.FALSE, Trillian.FALSE, Trillian.TRUE, true, true, false); + } + + public void set_total(int total) { + this.email_total = total; + } + + } + + + // Folders that should be excluded from search + private Gee.HashSet exclude_folders = + new Gee.HashSet(); + + // The email present in the folder, sorted + private Gee.TreeSet contents; + + // Map of engine ids to search ids + private Gee.Map id_map; + + private Geary.Nonblocking.Mutex result_mutex = new Geary.Nonblocking.Mutex(); + + + public FolderImpl(Geary.Account account, FolderRoot root) { + base( + account, + new FolderProperties(0, 0), + root.get_child(MAGIC_BASENAME, Trillian.TRUE) + ); + + account.folders_available_unavailable.connect(on_folders_available_unavailable); + account.folders_special_type.connect(on_folders_special_type); + account.email_locally_complete.connect(on_email_locally_complete); + account.email_removed.connect(on_account_email_removed); + + clear_contents(); + + // We always want to exclude emails that don't live anywhere + // from search results. + exclude_orphan_emails(); + } + + ~FolderImpl() { + account.folders_available_unavailable.disconnect(on_folders_available_unavailable); + account.folders_special_type.disconnect(on_folders_special_type); + account.email_locally_complete.disconnect(on_email_locally_complete); + account.email_removed.disconnect(on_account_email_removed); + } + + /** + * Sets the keyword string for this search. + */ + public override async void search(Geary.SearchQuery query, + GLib.Cancellable? cancellable = null) + throws GLib.Error { + int result_mutex_token = yield result_mutex.claim_async(); + + this.query = query; + GLib.Error? error = null; + try { + yield do_search_async(null, null, cancellable); + } catch(Error e) { + error = e; + } + + result_mutex.release(ref result_mutex_token); + + if (error != null) { + throw error; + } + + this.query_evaluation_complete(); + } + + /** + * Clears the search query and results. + */ + public override void clear() { + var old_contents = this.contents; + clear_contents(); + notify_email_removed(old_contents); + notify_email_count_changed(0, Geary.Folder.CountChangeReason.REMOVED); + + if (this.query != null) { + this.query = null; + this.query_evaluation_complete(); + } + } + + /** + * Given a list of mail IDs, returns a set of casefolded words that match for the current + * search query. + */ + public override async Gee.Set? get_search_matches_async( + Gee.Collection ids, + 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 + ); + } + return results; + } + + public override async Gee.List? list_email_by_id_async( + Geary.EmailIdentifier? initial_id, + int count, + Email.Field required_fields, + Geary.Folder.ListFlags flags, + Cancellable? cancellable = null + ) throws GLib.Error { + int result_mutex_token = yield result_mutex.claim_async(); + + var engine_ids = new Gee.LinkedList(); + + if (Geary.Folder.ListFlags.OLDEST_TO_NEWEST in flags) { + EmailIdentifier? oldest = null; + if (!this.contents.is_empty) { + if (initial_id == null) { + oldest = this.contents.last(); + } else { + oldest = this.id_map.get(initial_id); + + if (oldest == null) { + throw new EngineError.NOT_FOUND( + "Initial id not found %s", initial_id.to_string() + ); + } + + if (!(Geary.Folder.ListFlags.INCLUDING_ID in flags)) { + oldest = contents.higher(oldest); + } + } + } + if (oldest != null) { + var iter = ( + this.contents.iterator_at(oldest) as + Gee.BidirIterator + ); + engine_ids.add(oldest.source_id); + while (engine_ids.size < count && iter.previous()) { + engine_ids.add(iter.get().source_id); + } + } + } else { + // Newest to oldest + EmailIdentifier? newest = null; + if (!this.contents.is_empty) { + if (initial_id == null) { + newest = this.contents.first(); + } else { + newest = this.id_map.get(initial_id); + + if (newest == null) { + throw new EngineError.NOT_FOUND( + "Initial id not found %s", initial_id.to_string() + ); + } + + if (!(Geary.Folder.ListFlags.INCLUDING_ID in flags)) { + newest = contents.lower(newest); + } + } + } + if (newest != null) { + var iter = ( + this.contents.iterator_at(newest) as + Gee.BidirIterator + ); + engine_ids.add(newest.source_id); + while (engine_ids.size < count && iter.next()) { + engine_ids.add(iter.get().source_id); + } + } + } + + Gee.List? results = null; + GLib.Error? list_error = null; + if (!engine_ids.is_empty) { + try { + results = yield this.account.list_local_email_async( + engine_ids, + required_fields, + cancellable + ); + } catch (GLib.Error error) { + list_error = error; + } + } + + result_mutex.release(ref result_mutex_token); + + if (list_error != null) { + throw list_error; + } + + 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, + Cancellable? cancellable = null + ) throws GLib.Error { + return yield this.account.list_local_email_async( + EmailIdentifier.to_source_ids(ids), + 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. + 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) + throws GLib.Error { + return yield this.account.local_fetch_email_async( + EmailIdentifier.to_source_id(id), required_fields, cancellable + ); + } + + public virtual async void remove_email_async( + Gee.Collection email_ids, + GLib.Cancellable? cancellable = null + ) throws GLib.Error { + Gee.MultiMap? ids_to_folders = + yield account.get_containing_folders_async( + EmailIdentifier.to_source_ids(email_ids), + cancellable + ); + if (ids_to_folders != null) { + Gee.MultiMap folders_to_ids = + Geary.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); + + 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); + open = true; + yield remove.remove_email_async(ids, cancellable); + } finally { + if (open) { + try { + yield folder.close_async(); + } catch (Error e) { + debug("Error closing folder %s: %s", folder.to_string(), e.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 + // 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, + GLib.Cancellable? cancellable) + throws GLib.Error { + var id_map = this.id_map; + var contents = this.contents; + var added = new Gee.LinkedList(); + var removed = new Gee.LinkedList(); + + if (remove_ids == null) { + // Adding email to the search, either searching all local + // email if to_add is null, or adding only a matching + // subset of the given in to_add + // + // TODO: don't limit this to MAX_RESULT_EMAILS. Instead, + // 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 = + yield this.account.local_search_async( + this.query, + MAX_RESULT_EMAILS, + 0, + this.exclude_folders, + add_ids, // If null, will search all local email + cancellable + ); + + if (id_results != null) { + // Fetch email to get the received date for + // correct ordering in the search folder + Gee.Collection email_results = + yield this.account.list_local_email_async( + id_results, + PROPERTIES, + cancellable + ); + + if (add_ids == null) { + // 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(); + 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(); + existing.unset(); + contents.remove(search_id); + removed.add(search_id); + } + } + } + + foreach (var email in email_results) { + if (!id_map.has_key(email.id)) { + var search_id = new EmailIdentifier( + email.id, email.properties.date_received + ); + id_map.set(email.id, search_id); + contents.add(search_id); + added.add(search_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); + } + } + } + + ((FolderProperties) 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; + 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; + } + if (removed.size > 0) { + notify_email_removed(removed); + reason |= Geary.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, + 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); + } + } catch (GLib.Error e) { + error = e; + } + + result_mutex.release(ref result_mutex_token); + + if (error != null) + throw error; + } + + private async void do_remove(Geary.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); + } + } catch (GLib.Error e) { + error = e; + } + + result_mutex.release(ref result_mutex_token); + + if (error != null) + throw error; + } + + private void clear_contents() { + this.contents = new Gee.TreeSet( + EmailIdentifier.compare_descending + ); + this.id_map = new Gee.HashMap(); + } + + private void include_folder(Geary.Folder folder) { + this.exclude_folders.remove(folder.path); + } + + private void exclude_folder(Geary.Folder folder) { + this.exclude_folders.add(folder.path); + } + + private void exclude_orphan_emails() { + this.exclude_folders.add(null); + } + + 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) + .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) { + if (folder.special_folder_type in EXCLUDE_TYPES) { + exclude_folder(folder); + } else { + include_folder(folder); + } + } + } + + private void on_email_locally_complete(Geary.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 Geary.AccountProblemReport( + this.account.information, error + ) + ); + } + } + ); + } + } + + private void on_account_email_removed(Geary.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 Geary.AccountProblemReport( + this.account.information, error + ) + ); + } + } + ); + } + } + +} 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 b22af954..8698f12f 100644 --- a/test/engine/imap-engine/imap-engine-generic-account-test.vala +++ b/test/engine/imap-engine/imap-engine-generic-account-test.vala @@ -99,6 +99,21 @@ 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 + ) + ) + ) + ); } } From 130fd95b8633cc98173f0203ca29f9ecd1ec9bd7 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Thu, 12 Dec 2019 11:19:51 +1100 Subject: [PATCH 06/13] Remove Geary.Account::search_upgrade_monitor Implement the upgrade call as an account operation so the background monitor takes care of reporting its progress. --- src/client/components/search-bar.vala | 49 ++----------------- src/engine/api/geary-account.vala | 1 - src/engine/imap-db/imap-db-account.vala | 23 +++------ .../imap-engine-generic-account.vala | 40 +++++++++------ 4 files changed, 35 insertions(+), 78 deletions(-) diff --git a/src/client/components/search-bar.vala b/src/client/components/search-bar.vala index 2966b652..53e49833 100644 --- a/src/client/components/search-bar.vala +++ b/src/client/components/search-bar.vala @@ -12,8 +12,6 @@ public class SearchBar : Gtk.SearchBar { private Gtk.SearchEntry search_entry = new Gtk.SearchEntry(); private Components.EntryUndo search_undo; - private Geary.ProgressMonitor? search_upgrade_progress_monitor = null; - private MonitoredProgressBar search_upgrade_progress_bar = new MonitoredProgressBar(); private Geary.Account? current_account = null; public signal void search_text_changed(string search_text); @@ -34,12 +32,6 @@ public class SearchBar : Gtk.SearchBar { this.notify["search-mode-enabled"].connect(on_search_mode_changed); - // Search upgrade progress bar. - search_upgrade_progress_bar.show_text = true; - search_upgrade_progress_bar.visible = false; - search_upgrade_progress_bar.no_show_all = true; - - add(search_upgrade_progress_bar); add(search_entry); set_search_placeholder_text(DEFAULT_SEARCH_TEXT); @@ -59,33 +51,16 @@ public class SearchBar : Gtk.SearchBar { } public void set_account(Geary.Account? account) { - on_search_upgrade_finished(); // Reset search box. - - if (search_upgrade_progress_monitor != null) { - search_upgrade_progress_monitor.start.disconnect(on_search_upgrade_start); - search_upgrade_progress_monitor.finish.disconnect(on_search_upgrade_finished); - search_upgrade_progress_monitor = null; - } - if (current_account != null) { current_account.information.changed.disconnect( - on_information_changed); + on_information_changed + ); } if (account != null) { - search_upgrade_progress_monitor = account.search_upgrade_monitor; - search_upgrade_progress_bar.set_progress_monitor(search_upgrade_progress_monitor); - - search_upgrade_progress_monitor.start.connect(on_search_upgrade_start); - search_upgrade_progress_monitor.finish.connect(on_search_upgrade_finished); - if (search_upgrade_progress_monitor.is_in_progress) - on_search_upgrade_start(); // Remove search box, we're already in progress. - account.information.changed.connect( - on_information_changed); - - search_upgrade_progress_bar.text = - _("Indexing %s account").printf(account.information.display_name); + on_information_changed + ); } current_account = account; @@ -93,22 +68,6 @@ public class SearchBar : Gtk.SearchBar { on_information_changed(); // Set new account name. } - private void on_search_upgrade_start() { - // Set the progress bar's width to match the search entry's width. - int minimum_width = 0; - int natural_width = 0; - search_entry.get_preferred_width(out minimum_width, out natural_width); - search_upgrade_progress_bar.width_request = minimum_width; - - search_entry.hide(); - search_upgrade_progress_bar.show(); - } - - private void on_search_upgrade_finished() { - search_entry.show(); - search_upgrade_progress_bar.hide(); - } - private void on_information_changed() { var main = get_toplevel() as Application.MainWindow; if (main != null) { diff --git a/src/engine/api/geary-account.vala b/src/engine/api/geary-account.vala index 54b1d9db..c98a888a 100644 --- a/src/engine/api/geary-account.vala +++ b/src/engine/api/geary-account.vala @@ -132,7 +132,6 @@ public abstract class Geary.Account : BaseObject, Logging.Source { public Geary.ContactStore contact_store { get; protected set; } public ProgressMonitor background_progress { get; protected set; } - public ProgressMonitor search_upgrade_monitor { get; protected set; } public ProgressMonitor db_upgrade_monitor { get; protected set; } public ProgressMonitor db_vacuum_monitor { get; protected set; } diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala index 6540494a..2439df06 100644 --- a/src/engine/imap-db/imap-db-account.vala +++ b/src/engine/imap-db/imap-db-account.vala @@ -37,13 +37,12 @@ private class Geary.ImapDB.Account : BaseObject { get; private set; default = new Imap.FolderRoot("$geary-imap"); } - // Only available when the Account is opened - public IntervalProgressMonitor search_index_monitor { get; private set; - default = new IntervalProgressMonitor(ProgressType.SEARCH_INDEX, 0, 0); } - public SimpleProgressMonitor upgrade_monitor { get; private set; default = new SimpleProgressMonitor( - ProgressType.DB_UPGRADE); } - public SimpleProgressMonitor vacuum_monitor { get; private set; default = new SimpleProgressMonitor( - ProgressType.DB_VACUUM); } + public SimpleProgressMonitor upgrade_monitor { + get; private set; default = new SimpleProgressMonitor(DB_UPGRADE); + } + public SimpleProgressMonitor vacuum_monitor { + get; private set; default = new SimpleProgressMonitor(DB_VACUUM); + } /** The backing database for the account. */ public ImapDB.Database db { get; private set; } @@ -918,9 +917,6 @@ private class Geary.ImapDB.Account : BaseObject { debug("Error populating %s search table: %s", account_information.id, e.message); } - if (search_index_monitor.is_in_progress) - search_index_monitor.notify_finish(); - debug("%s: Done populating search table", account_information.id); } @@ -1014,13 +1010,6 @@ private class Geary.ImapDB.Account : BaseObject { if (count > 0) { debug("%s: Found %d/%d missing indexed messages, %d remaining...", account_information.id, count, limit, total_unindexed); - - if (!search_index_monitor.is_in_progress) { - search_index_monitor.set_interval(0, total_unindexed); - search_index_monitor.notify_start(); - } - - search_index_monitor.increment(count); } return (count < limit); diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala b/src/engine/imap-engine/imap-engine-generic-account.vala index 86f0c877..9068da79 100644 --- a/src/engine/imap-engine/imap-engine-generic-account.vala +++ b/src/engine/imap-engine/imap-engine-generic-account.vala @@ -107,7 +107,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account { ); this.background_progress = new ReentrantProgressMonitor(ACTIVITY); - this.search_upgrade_monitor = local.search_index_monitor; this.db_upgrade_monitor = local.upgrade_monitor; this.db_vacuum_monitor = local.vacuum_monitor; @@ -170,21 +169,12 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account { yield this.imap.start(cancellable); this.queue_operation(new StartPostie(this)); - // Kick off a background update of the search table, but since - // the database is getting hammered at startup, wait a bit - // before starting the update ... use the ordinal to stagger - // these being fired off (important for users with many - // accounts registered). + // Kick off a background update of the search table. // - // This is an example of an operation for which we need an - // engine-wide operation queue, not just an account-wide - // queue. - const int POPULATE_DELAY_SEC = 5; - int account_sec = this.information.ordinal.clamp(0, 10); - Timeout.add_seconds(POPULATE_DELAY_SEC + account_sec, () => { - this.local.populate_search_table.begin(cancellable); - return false; - }); + // XXX since this hammers the database, this is an example of + // an operation for which we need an engine-wide operation + // queue, not just an account-wide queue. + this.queue_operation(new PopulateSearchTable(this)); } public override async void close_async(Cancellable? cancellable = null) throws Error { @@ -1138,6 +1128,26 @@ internal class Geary.ImapEngine.StartPostie : AccountOperation { } +/** + * Account operation for populating the full-text-search table. + */ +internal class Geary.ImapEngine.PopulateSearchTable : AccountOperation { + + + internal PopulateSearchTable(GenericAccount account) { + base(account); + } + + public override async void execute(GLib.Cancellable cancellable) + throws GLib.Error { + yield ((GenericAccount) this.account).local.populate_search_table( + cancellable + ); + } + +} + + /** * Account operation that updates folders from the remote. */ From 3640bef9c9bb27abf64fd301fa0058a06631cf28 Mon Sep 17 00:00:00 2001 From: Jordi Mas Date: Thu, 12 Dec 2019 22:26:51 +0100 Subject: [PATCH 07/13] Update Catalan translation --- po/ca.po | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/po/ca.po b/po/ca.po index 2903da7c..6e32e547 100644 --- a/po/ca.po +++ b/po/ca.po @@ -148,7 +148,7 @@ msgstr "La darrera amplada registrada de la finestra de l'aplicació." #: desktop/org.gnome.Geary.gschema.xml:20 msgid "Height of window" -msgstr "Alçada de l'aplicació" +msgstr "Alçada de la finestra" #: desktop/org.gnome.Geary.gschema.xml:21 msgid "The last recorded height of the application window." @@ -614,7 +614,7 @@ msgstr "Yahoo" #. loaded but disabled by the user. #: src/client/accounts/accounts-editor-list-pane.vala:371 msgid "This account has been disabled" -msgstr "Aquest compte ha estat deshabilitat" +msgstr "Aquest compte ha estat inhabilitat" #. Translators: Tooltip for accounts that have been #. loaded but because of some error are not able to be @@ -759,7 +759,7 @@ msgstr "Copyright 2016 Software Freedom Conservancy Inc." #: src/client/application/geary-application.vala:25 msgid "Copyright 2016-2019 Geary Development Team." -msgstr "Copyright 2016-2019 Geary Development Team." +msgstr "Copyright 2016-2019 equip de desenvolupament del Geary." #: src/client/application/geary-application.vala:27 msgid "Visit the Geary web site" @@ -1434,7 +1434,7 @@ msgstr "" #. Keep, Discard or Cancel. #: src/client/composer/composer-widget.vala:1130 msgid "Do you want to keep or discard this draft message?" -msgstr "Vols mantindre o descartar aquest esborrany de missatge?" +msgstr "Vols mantenir o descartar aquest esborrany de missatge?" #. Translators: This dialog text is displayed to the #. user when closing a composer where the options are From f025f6904d5126238966c50eec938c4c857404cf Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Fri, 13 Dec 2019 12:12:07 +1100 Subject: [PATCH 08/13] Move SearchFolder management into the client Rename the search folder again to App.SearchFolder, move its id class into it as an inner class. Remove search folder from the engine so the application can manage it and it's policy in the future. Also remove the outbox from the accout's list of local folders, so that code can be removed altogether. --- po/POTFILES.in | 5 +- .../application/application-controller.vala | 10 +- .../application/application-main-window.vala | 49 ++-- .../conversation-viewer.vala | 6 +- .../folder-list-search-branch.vala | 8 +- src/client/folder-list/folder-list-tree.vala | 2 +- src/engine/api/geary-account.vala | 10 + src/engine/api/geary-search-folder.vala | 86 ------- src/engine/api/geary-search-query.vala | 4 +- .../app-search-folder.vala} | 223 +++++++++++++++--- .../gmail/imap-engine-gmail-account.vala | 4 - .../imap-engine-gmail-search-folder.vala | 46 ---- .../imap-engine-generic-account.vala | 77 ++---- src/engine/meson.build | 6 +- .../search/search-email-identifier.vala | 129 ---------- 15 files changed, 260 insertions(+), 405 deletions(-) delete mode 100644 src/engine/api/geary-search-folder.vala rename src/engine/{search/search-folder-impl.vala => app/app-search-folder.vala} (73%) delete mode 100644 src/engine/imap-engine/gmail/imap-engine-gmail-search-folder.vala delete mode 100644 src/engine/search/search-email-identifier.vala diff --git a/po/POTFILES.in b/po/POTFILES.in index 71229fb7..91798075 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -144,7 +144,6 @@ src/engine/api/geary-named-flags.vala src/engine/api/geary-problem-report.vala src/engine/api/geary-progress-monitor.vala src/engine/api/geary-revokable.vala -src/engine/api/geary-search-folder.vala src/engine/api/geary-search-query.vala src/engine/api/geary-service-information.vala src/engine/api/geary-service-provider.vala @@ -153,6 +152,7 @@ src/engine/app/app-conversation-monitor.vala src/engine/app/app-conversation.vala src/engine/app/app-draft-manager.vala src/engine/app/app-email-store.vala +src/engine/app/app-search-folder.vala src/engine/app/conversation-monitor/app-append-operation.vala src/engine/app/conversation-monitor/app-conversation-operation-queue.vala src/engine/app/conversation-monitor/app-conversation-operation.vala @@ -234,7 +234,6 @@ src/engine/imap-engine/gmail/imap-engine-gmail-account.vala src/engine/imap-engine/gmail/imap-engine-gmail-all-mail-folder.vala src/engine/imap-engine/gmail/imap-engine-gmail-drafts-folder.vala src/engine/imap-engine/gmail/imap-engine-gmail-folder.vala -src/engine/imap-engine/gmail/imap-engine-gmail-search-folder.vala src/engine/imap-engine/gmail/imap-engine-gmail-spam-trash-folder.vala src/engine/imap-engine/imap-engine-account-operation.vala src/engine/imap-engine/imap-engine-account-processor.vala @@ -361,8 +360,6 @@ src/engine/rfc822/rfc822-message.vala src/engine/rfc822/rfc822-part.vala src/engine/rfc822/rfc822-utils.vala src/engine/rfc822/rfc822.vala -src/engine/search/search-email-identifier.vala -src/engine/search/search-folder-impl.vala src/engine/smtp/smtp-authenticator.vala src/engine/smtp/smtp-capabilities.vala src/engine/smtp/smtp-client-connection.vala diff --git a/src/client/application/application-controller.vala b/src/client/application/application-controller.vala index f9e4582c..87f70a44 100644 --- a/src/client/application/application-controller.vala +++ b/src/client/application/application-controller.vala @@ -878,6 +878,7 @@ internal class Application.Controller : Geary.BaseObject { private async void open_account(Geary.Account account) { AccountContext context = new AccountContext( account, + new Geary.App.SearchFolder(account, account.local_folder_root), new Geary.App.EmailStore(account), new Application.ContactStore(account, this.folks) ); @@ -976,8 +977,10 @@ internal class Application.Controller : Geary.BaseObject { account_unavailable(context, is_shutdown); - context.cancellable.cancel(); + // Stop any background processes + context.search.clear(); context.contacts.close(); + context.cancellable.cancel(); // Explicitly close the inbox since we explicitly open it Geary.Folder? inbox = context.inbox; @@ -1770,6 +1773,9 @@ internal class Application.AccountContext : Geary.BaseObject { /** The account's Inbox folder */ public Geary.Folder? inbox = null; + /** The account's search folder */ + public Geary.App.SearchFolder search = null; + /** The account's email store */ public Geary.App.EmailStore emails { get; private set; } @@ -1818,9 +1824,11 @@ internal class Application.AccountContext : Geary.BaseObject { public AccountContext(Geary.Account account, + Geary.App.SearchFolder search, Geary.App.EmailStore emails, Application.ContactStore contacts) { this.account = account; + this.search = search; this.emails = emails; this.contacts = contacts; } diff --git a/src/client/application/application-main-window.vala b/src/client/application/application-main-window.vala index 9f00d46a..b1c8cdee 100644 --- a/src/client/application/application-main-window.vala +++ b/src/client/application/application-main-window.vala @@ -676,13 +676,14 @@ public class Application.MainWindow : !this.folder_list.select_inbox(to_select.account))) { this.folder_list.select_folder(to_select); } + + if (to_select.special_folder_type == SEARCH) { + this.previous_non_search_folder = to_select; + } } else { this.folder_list.deselect_folder(); } - if (!(to_select is Geary.SearchFolder)) { - this.previous_non_search_folder = to_select; - } update_conversation_actions(NONE); update_title(); this.main_toolbar.update_trash_button( @@ -903,27 +904,25 @@ public class Application.MainWindow : } internal async void start_search(string query_text, bool is_interactive) { - var account = this.selected_account; - if (account != null) { - var folder = account.get_special_folder( - SEARCH - ) as Geary.SearchFolder; - + 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(); var strategy = this.application.config.get_search_strategy(); try { - var query = yield account.new_search_query( + var query = yield context.account.new_search_query( query_text, strategy, cancellable ); - this.folder_list.set_search(this.application.engine, folder); - yield folder.search(query, cancellable); + this.folder_list.set_search( + this.application.engine, context.search + ); + yield context.search.search(query, cancellable); } catch (GLib.Error error) { - handle_error(account.information, error); + handle_error(context.account.information, error); } } } @@ -934,20 +933,15 @@ public class Application.MainWindow : this.search_open = new GLib.Cancellable(); if (this.previous_non_search_folder != null && - this.selected_folder is Geary.SearchFolder) { + this.selected_folder.special_folder_type == SEARCH) { this.select_folder.begin( this.previous_non_search_folder, is_interactive ); } this.folder_list.remove_search(); - if (this.selected_account != null) { - var folder = this.selected_account.get_special_folder( - SEARCH - ) as Geary.SearchFolder; - if (folder != null) { - folder.clear(); - } + foreach (var context in this.application.controller.get_account_contexts()) { + context.search.clear(); } } @@ -988,6 +982,10 @@ public class Application.MainWindow : Geary.Account.sort_by_path(to_add.account.list_folders()) ); + add_folder( + ((Geary.Smtp.ClientService) to_add.account.outgoing).outbox + ); + this.accounts.add(to_add); } } @@ -1007,14 +1005,13 @@ public class Application.MainWindow : // that when the account is gone. if (this.selected_folder != null && this.selected_folder.account == to_remove.account) { - Geary.SearchFolder? current_search = ( - this.selected_folder as Geary.SearchFolder + bool is_account_search_active = ( + this.selected_folder.special_folder_type == SEARCH ); yield select_folder(to_select, false); - // Clear the account's search folder if it existed - if (current_search != null) { + if (is_account_search_active) { this.search_bar.set_search_text(""); this.search_bar.search_mode_enabled = false; } @@ -1583,7 +1580,7 @@ public class Application.MainWindow : if (!this.has_composer) { if (this.conversations.size == 0) { // Let the user know if there's no available conversations - if (this.selected_folder is Geary.SearchFolder) { + if (this.selected_folder.special_folder_type == SEARCH) { this.conversation_viewer.show_empty_search(); } else { this.conversation_viewer.show_empty_folder(); diff --git a/src/client/conversation-viewer/conversation-viewer.vala b/src/client/conversation-viewer/conversation-viewer.vala index 6ffeba11..3de7d72a 100644 --- a/src/client/conversation-viewer/conversation-viewer.vala +++ b/src/client/conversation-viewer/conversation-viewer.vala @@ -296,7 +296,7 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface { conversation.base_folder.account, null ); if (query == null) { - var search_folder = conversation.base_folder as Geary.SearchFolder; + var search_folder = conversation.base_folder as Geary.App.SearchFolder; if (search_folder != null) { query = search_folder.query; } @@ -454,9 +454,9 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface { } else { // Find became disabled, re-show search terms if any this.current_list.search.unmark_terms(); - Geary.SearchFolder? search_folder = ( + Geary.App.SearchFolder? search_folder = ( this.current_list.conversation.base_folder - as Geary.SearchFolder + as Geary.App.SearchFolder ); this.conversation_find_undo.reset(); if (search_folder != null) { diff --git a/src/client/folder-list/folder-list-search-branch.vala b/src/client/folder-list/folder-list-search-branch.vala index d4a0013f..038e6f6c 100644 --- a/src/client/folder-list/folder-list-search-branch.vala +++ b/src/client/folder-list/folder-list-search-branch.vala @@ -8,12 +8,12 @@ * This branch is a top-level container for a search entry. */ public class FolderList.SearchBranch : Sidebar.RootOnlyBranch { - public SearchBranch(Geary.SearchFolder folder, Geary.Engine engine) { + public SearchBranch(Geary.App.SearchFolder folder, Geary.Engine engine) { base(new SearchEntry(folder, engine)); } - public Geary.SearchFolder get_search_folder() { - return (Geary.SearchFolder) ((SearchEntry) get_root()).folder; + public Geary.App.SearchFolder get_search_folder() { + return (Geary.App.SearchFolder) ((SearchEntry) get_root()).folder; } } @@ -22,7 +22,7 @@ public class FolderList.SearchEntry : FolderList.AbstractFolderEntry { Geary.Engine engine; private int account_count = 0; - public SearchEntry(Geary.SearchFolder folder, + public SearchEntry(Geary.App.SearchFolder folder, Geary.Engine engine) { base(folder); this.engine = engine; diff --git a/src/client/folder-list/folder-list-tree.vala b/src/client/folder-list/folder-list-tree.vala index 269ee831..f0a831d1 100644 --- a/src/client/folder-list/folder-list-tree.vala +++ b/src/client/folder-list/folder-list-tree.vala @@ -244,7 +244,7 @@ public class FolderList.Tree : Sidebar.Tree, Geary.BaseInterface { } public void set_search(Geary.Engine engine, - Geary.SearchFolder search_folder) { + Geary.App.SearchFolder search_folder) { if (search_branch != null && has_branch(search_branch)) { // We already have a search folder. If it's the same one, just // select it. If it's a new search folder, remove the old one and diff --git a/src/engine/api/geary-account.vala b/src/engine/api/geary-account.vala index c98a888a..37ade9e3 100644 --- a/src/engine/api/geary-account.vala +++ b/src/engine/api/geary-account.vala @@ -131,6 +131,16 @@ public abstract class Geary.Account : BaseObject, Logging.Source { */ public Geary.ContactStore contact_store { get; protected set; } + /** + * The root path for all local folders. + * + * Any local folders create by the engine or clients must use this + * as the root for local folders. + */ + public FolderRoot local_folder_root { + get; private set; default = new Geary.FolderRoot("$geary-local", true); + } + public ProgressMonitor background_progress { get; protected set; } public ProgressMonitor db_upgrade_monitor { get; protected set; } public ProgressMonitor db_vacuum_monitor { get; protected set; } diff --git a/src/engine/api/geary-search-folder.vala b/src/engine/api/geary-search-folder.vala deleted file mode 100644 index da034c69..00000000 --- a/src/engine/api/geary-search-folder.vala +++ /dev/null @@ -1,86 +0,0 @@ -/* - * Copyright 2016 Software Freedom Conservancy Inc. - * Copyright 2019 Michael Gratton - * - * This software is licensed under the GNU Lesser General Public License - * (version 2.1 or later). See the COPYING file in this distribution. - */ - -/** - * A local folder to execute and collect results of search queries. - * - * SearchFolder is merely specified to be a Folder, but implementations may add various - * {@link FolderSupport} interfaces. In particular {@link FolderSupport.Remove} should be supported, - * but again, is not required. - * - * SearchFolder is expected to produce {@link EmailIdentifier}s which can be accepted by other - * Folders within the Account (with the exception of the Outbox). Those Folders may need to - * translate those EmailIdentifiers to their own type for ordering reasons, but in general the - * expectation is that the results of SearchFolder can then be applied to operations on Email in - * other remote-backed folders. - * - * @see SearchQuery - */ -public abstract class Geary.SearchFolder : Geary.AbstractLocalFolder { - private weak Account _account; - public override Account account { get { return _account; } } - - private FolderProperties _properties; - public override FolderProperties properties { get { return _properties; } } - - private FolderPath? _path = null; - public override FolderPath path { get { return _path; } } - - public override SpecialFolderType special_folder_type { - get { - return Geary.SpecialFolderType.SEARCH; - } - } - - - /** The query being evaluated by this folder, if any. */ - public Geary.SearchQuery? query { get; protected set; default = null; } - - /** Emitted when the current query has been fully evaluated. */ - public signal void query_evaluation_complete(); - - - protected SearchFolder(Account account, FolderProperties properties, FolderPath path) { - _account = account; - _properties = properties; - _path = path; - } - - /** - * Sets the query to be evaluated for this folder. - * - * Executes an asynchronous search, which can be stopped via the - * supplied cancellable. When the search is complete, {@link - * query_evaluation_complete} will be emitted. if an error occurs, - * the signal will not be invoked. It is possible for the {@link - * query} property to change before completion. - */ - public abstract async void search(SearchQuery query, - GLib.Cancellable? cancellable = null) - throws GLib.Error; - - /** - * Clears the search query and results. - * - * The {@link query_evaluation_complete} signal will be emitted - * and {@link query} will be set to null. - */ - public abstract void clear(); - - /** - * Returns a set of case-folded words matched by the current query. - * - * The set contains words from the given collection of email that - * match any of the non-negated text operators in {@link query}. - */ - public abstract async Gee.Set? get_search_matches_async( - Gee.Collection ids, - GLib.Cancellable? cancellable = null - ) throws GLib.Error; - -} diff --git a/src/engine/api/geary-search-query.vala b/src/engine/api/geary-search-query.vala index 4c4ec8a3..0a15ff2e 100644 --- a/src/engine/api/geary-search-query.vala +++ b/src/engine/api/geary-search-query.vala @@ -11,12 +11,12 @@ * * New instances can be constructed via {@link * Account.new_search_query} and then passed to search methods on - * {@link Account} or {@link SearchFolder}. + * {@link Account} or {@link App.SearchFolder}. * * @see Account.new_search_query * @see Account.local_search_async * @see Account.get_search_matches_async - * @see SearchFolder.search + * @see App.SearchFolder.search */ public abstract class Geary.SearchQuery : BaseObject { diff --git a/src/engine/search/search-folder-impl.vala b/src/engine/app/app-search-folder.vala similarity index 73% rename from src/engine/search/search-folder-impl.vala rename to src/engine/app/app-search-folder.vala index 25009437..3e6716a9 100644 --- a/src/engine/search/search-folder-impl.vala +++ b/src/engine/app/app-search-folder.vala @@ -7,20 +7,21 @@ */ /** - * A default implementation of a search folder. + * A folder for executing and listing an account-wide email search. * - * This implementation of {@link Geary.SearchFolder} uses the search - * methods on {@link Account} to implement account-wide email search. + * This uses the search methods on {@link Account} to implement the + * search, then collects search results and presents them via the + * folder interface. */ -internal class Geary.Search.FolderImpl : - Geary.SearchFolder, Geary.FolderSupport.Remove { +public class Geary.App.SearchFolder : + Geary.AbstractLocalFolder, Geary.FolderSupport.Remove { - /** Max number of emails that can ever be in the folder. */ + /** Number of messages to include in the initial search. */ public const int MAX_RESULT_EMAILS = 1000; /** The canonical name of the search folder. */ - public const string MAGIC_BASENAME = "$GearySearchFolder$"; + public const string MAGIC_BASENAME = "$GearyAccountSearchFolder$"; private const Geary.SpecialFolderType[] EXCLUDE_TYPES = { Geary.SpecialFolderType.SPAM, @@ -30,6 +31,124 @@ internal class Geary.Search.FolderImpl : }; + /** Internal identifier used by the search folder */ + internal class EmailIdentifier : + Geary.EmailIdentifier, Gee.Comparable { + + + 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 { @@ -44,6 +163,38 @@ internal class Geary.Search.FolderImpl : } + /** {@inheritDoc} */ + public override Account account { + get { return _account; } + } + private weak Account _account; + + /** {@inheritDoc} */ + public override Geary.FolderProperties properties { + get { return _properties; } + } + private Geary.FolderProperties _properties; + + /** {@inheritDoc} */ + public override FolderPath path { + get { return _path; } + } + private FolderPath? _path = null; + + /** + * {@inheritDoc} + * + * Always returns {@link SpecialFolderType.SEARCH}. + */ + public override SpecialFolderType special_folder_type { + get { + return Geary.SpecialFolderType.SEARCH; + } + } + + /** The query being evaluated by this folder, if any. */ + public Geary.SearchQuery? query { get; protected set; default = null; } + // Folders that should be excluded from search private Gee.HashSet exclude_folders = new Gee.HashSet(); @@ -56,13 +207,13 @@ internal class Geary.Search.FolderImpl : private Geary.Nonblocking.Mutex result_mutex = new Geary.Nonblocking.Mutex(); + private GLib.Cancellable executing = new GLib.Cancellable(); - public FolderImpl(Geary.Account account, FolderRoot root) { - base( - account, - new FolderProperties(0, 0), - root.get_child(MAGIC_BASENAME, Trillian.TRUE) - ); + + public SearchFolder(Geary.Account account, FolderRoot root) { + this._account = account; + this._properties = new FolderProperties(0, 0); + this._path = root.get_child(MAGIC_BASENAME, Trillian.TRUE); account.folders_available_unavailable.connect(on_folders_available_unavailable); account.folders_special_type.connect(on_folders_special_type); @@ -71,12 +222,12 @@ internal class Geary.Search.FolderImpl : clear_contents(); - // We always want to exclude emails that don't live anywhere - // from search results. + // Always exclude emails that don't live anywhere from search + // results. exclude_orphan_emails(); } - ~FolderImpl() { + ~SearchFolder() { account.folders_available_unavailable.disconnect(on_folders_available_unavailable); account.folders_special_type.disconnect(on_folders_special_type); account.email_locally_complete.disconnect(on_email_locally_complete); @@ -84,17 +235,25 @@ internal class Geary.Search.FolderImpl : } /** - * Sets the keyword string for this search. + * Executes the given query over the account's local email. + * + * Calling this will block until the search is complete. */ - public override async void search(Geary.SearchQuery query, - GLib.Cancellable? cancellable = null) + public async void search(SearchQuery query, GLib.Cancellable? cancellable) throws GLib.Error { int result_mutex_token = yield result_mutex.claim_async(); + clear(); + + if (cancellable != null) { + GLib.Cancellable @internal = this.executing; + cancellable.cancelled.connect(() => { @internal.cancel(); }); + } + this.query = query; GLib.Error? error = null; try { - yield do_search_async(null, null, cancellable); + yield do_search_async(null, null, this.executing); } catch(Error e) { error = e; } @@ -104,30 +263,32 @@ internal class Geary.Search.FolderImpl : if (error != null) { throw error; } - - this.query_evaluation_complete(); } /** - * Clears the search query and results. + * Cancels and clears the search query and results. + * + * The {@link query} property will be cleared. */ - public override void clear() { + public void clear() { + 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); - if (this.query != null) { - this.query = null; - this.query_evaluation_complete(); - } + this.query = null; } /** - * Given a list of mail IDs, returns a set of casefolded words that match for the current - * search query. + * Returns a set of case-folded words matched by the current query. + * + * The set contains words from the given collection of email that + * match any of the non-negated text operators in {@link query}. */ - public override async Gee.Set? get_search_matches_async( + public async Gee.Set? get_search_matches_async( Gee.Collection ids, GLib.Cancellable? cancellable = null ) throws GLib.Error { diff --git a/src/engine/imap-engine/gmail/imap-engine-gmail-account.vala b/src/engine/imap-engine/gmail/imap-engine-gmail-account.vala index 8e7b3acc..0f1296db 100644 --- a/src/engine/imap-engine/gmail/imap-engine-gmail-account.vala +++ b/src/engine/imap-engine/gmail/imap-engine-gmail-account.vala @@ -78,8 +78,4 @@ private class Geary.ImapEngine.GmailAccount : Geary.ImapEngine.GenericAccount { } } - protected override SearchFolder new_search_folder() { - return new GmailSearchFolder(this, this.local_folder_root); - } - } diff --git a/src/engine/imap-engine/gmail/imap-engine-gmail-search-folder.vala b/src/engine/imap-engine/gmail/imap-engine-gmail-search-folder.vala deleted file mode 100644 index aacc0340..00000000 --- a/src/engine/imap-engine/gmail/imap-engine-gmail-search-folder.vala +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Copyright 2016 Software Freedom Conservancy Inc. - * - * This software is licensed under the GNU Lesser General Public License - * (version 2.1 or later). See the COPYING file in this distribution. - */ - -/** - * Gmail-specific SearchFolder implementation. - */ -private class Geary.ImapEngine.GmailSearchFolder : Search.FolderImpl { - - private Geary.App.EmailStore email_store; - - public GmailSearchFolder(Geary.Account account, FolderRoot root) { - base (account, root); - - this.email_store = new Geary.App.EmailStore(account); - } - - public override async void - remove_email_async(Gee.Collection email_ids, - GLib.Cancellable? cancellable = null) - throws GLib.Error { - Geary.Folder? trash_folder = null; - try { - trash_folder = yield account.get_required_special_folder_async( - Geary.SpecialFolderType.TRASH, cancellable - ); - } catch (Error e) { - debug("Error looking up trash folder in %s: %s", - account.to_string(), e.message); - } - - if (trash_folder == null) { - debug("Can't remove email from search folder because no trash folder was found in %s", - account.to_string()); - } else { - // Copying to trash from one folder is all that's required in Gmail - // to fully trash the message. - yield this.email_store.copy_email_async( - email_ids, trash_folder.path, cancellable - ); - } - } -} diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala b/src/engine/imap-engine/imap-engine-generic-account.vala index 9068da79..c94ea9f6 100644 --- a/src/engine/imap-engine/imap-engine-generic-account.vala +++ b/src/engine/imap-engine/imap-engine-generic-account.vala @@ -39,25 +39,12 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account { /** Local database for the account. */ public ImapDB.Account local { get; private set; } - /** - * The root path for all local folders. - * - * No folder exists for this path, it merely exists to provide a - * common root for the paths of all local folders. - */ - protected FolderRoot local_folder_root = new Geary.FolderRoot( - "$geary-local", true - ); - private bool open = false; private Cancellable? open_cancellable = null; private Nonblocking.Semaphore? remote_ready_lock = null; - private Geary.SearchFolder? search_folder { get; private set; default = null; } - - private Gee.HashMap folder_map = new Gee.HashMap< - FolderPath, MinimalFolder>(); - private Gee.HashMap local_only = new Gee.HashMap(); + private Gee.Map folder_map = + new Gee.HashMap(); private AccountProcessor? processor; private AccountSynchronizer sync; @@ -148,16 +135,8 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account { throw err; } - // Create/load local folders - - local_only.set(this.smtp.outbox.path, this.smtp.outbox); - - this.search_folder = new_search_folder(); - local_only.set(this.search_folder.path, this.search_folder); - this.open = true; notify_opened(); - notify_folders_available_unavailable(sort_by_path(local_only.values), null); this.queue_operation( new LoadFolders(this, this.local, get_supported_special_folders()) @@ -190,7 +169,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account { // Halt internal tasks early so they stop using local and // remote connections. - this.search_folder.clear(); this.refresh_folder_timer.reset(); this.open_cancellable.cancel(); this.processor.stop(); @@ -203,25 +181,16 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account { // Close folders and ensure they do in fact close - Gee.BidirSortedSet locals = sort_by_path(this.local_only.values); Gee.BidirSortedSet remotes = sort_by_path(this.folder_map.values); - - this.local_only.clear(); this.folder_map.clear(); - - notify_folders_available_unavailable(null, locals); notify_folders_available_unavailable(null, remotes); - foreach (Geary.Folder folder in locals) { - debug("Waiting for local to close: %s", folder.to_string()); - yield folder.wait_for_close_async(); - } foreach (Geary.Folder folder in remotes) { debug("Waiting for remote to close: %s", folder.to_string()); yield folder.wait_for_close_async(); } - // Close IMAP service manager + // Close IMAP service manager now that folders are closed try { yield this.imap.stop(); @@ -232,7 +201,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account { // Close local infrastructure - this.search_folder = null; try { yield local.close_async(cancellable); } finally { @@ -408,7 +376,9 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account { if (type == 'i') return new ImapDB.EmailIdentifier.from_variant(serialised); if (type == 's') - return new Search.EmailIdentifier.from_variant(serialised, this); + return new App.SearchFolder.EmailIdentifier.from_variant( + serialised, this + ); if (type == 'o') return new Outbox.EmailIdentifier.from_variant(serialised); @@ -432,23 +402,18 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account { throws EngineError.NOT_FOUND { Folder? folder = this.folder_map.get(path); if (folder == null) { - folder = this.local_only.get(path); - if (folder == null) { - throw new EngineError.NOT_FOUND( - "Folder not found: %s", path.to_string() - ); - } + throw new EngineError.NOT_FOUND( + "Folder not found: %s", path.to_string() + ); } return folder; } /** {@inheritDoc} */ public override Gee.Collection list_folders() { - Gee.HashSet all_folders = new Gee.HashSet(); - all_folders.add_all(this.folder_map.values); - all_folders.add_all(this.local_only.values); - - return all_folders; + var all = new Gee.HashSet(); + all.add_all(this.folder_map.values); + return all; } /** {@inheritDoc} */ @@ -793,16 +758,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account { */ protected abstract MinimalFolder new_folder(ImapDB.Folder local_folder); - /** - * Constructs a concrete search folder implementation. - * - * Subclasses with specific SearchFolder implementations should - * override this to return the correct subclass. - */ - protected virtual SearchFolder new_search_folder() { - return new Search.FolderImpl(this, this.local_folder_root); - } - /** {@inheritDoc} */ protected override void notify_folders_available_unavailable(Gee.BidirSortedSet? available, @@ -861,7 +816,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account { UpdateRemoteFolders op = new UpdateRemoteFolders( this, - this.local_only.keys, get_supported_special_folders() ); op.completed.connect(() => { @@ -1155,16 +1109,13 @@ internal class Geary.ImapEngine.UpdateRemoteFolders : AccountOperation { private weak GenericAccount generic_account; - private Gee.Collection local_folders; private Geary.SpecialFolderType[] specials; internal UpdateRemoteFolders(GenericAccount account, - Gee.Collection local_folders, Geary.SpecialFolderType[] specials) { base(account); this.generic_account = account; - this.local_folders = local_folders; this.specials = specials; } @@ -1289,10 +1240,10 @@ internal class Geary.ImapEngine.UpdateRemoteFolders : AccountOperation { .filter(f => !existing_folders.has_key(f.path)) .to_array_list(); - // If path in local but not remote (and isn't local-only, i.e. the Outbox), need to remove it + // Remove if path in local but not remote Gee.ArrayList to_remove = Geary.traverse>(existing_folders) - .filter(e => !remote_folders.has_key(e.key) && !this.local_folders.contains(e.key)) + .filter(e => !remote_folders.has_key(e.key)) .map(e => (Geary.Folder) e.value) .to_array_list(); diff --git a/src/engine/meson.build b/src/engine/meson.build index d98f9050..cb61c2aa 100644 --- a/src/engine/meson.build +++ b/src/engine/meson.build @@ -37,7 +37,6 @@ geary_engine_vala_sources = files( 'api/geary-problem-report.vala', 'api/geary-progress-monitor.vala', 'api/geary-revokable.vala', - 'api/geary-search-folder.vala', 'api/geary-search-query.vala', 'api/geary-service-information.vala', 'api/geary-service-provider.vala', @@ -47,6 +46,7 @@ geary_engine_vala_sources = files( 'app/app-conversation-monitor.vala', 'app/app-draft-manager.vala', 'app/app-email-store.vala', + 'app/app-search-folder.vala', 'app/conversation-monitor/app-append-operation.vala', 'app/conversation-monitor/app-conversation-operation-queue.vala', @@ -197,7 +197,6 @@ geary_engine_vala_sources = files( 'imap-engine/gmail/imap-engine-gmail-all-mail-folder.vala', 'imap-engine/gmail/imap-engine-gmail-drafts-folder.vala', 'imap-engine/gmail/imap-engine-gmail-folder.vala', - 'imap-engine/gmail/imap-engine-gmail-search-folder.vala', 'imap-engine/gmail/imap-engine-gmail-spam-trash-folder.vala', 'imap-engine/other/imap-engine-other-account.vala', 'imap-engine/other/imap-engine-other-folder.vala', @@ -270,9 +269,6 @@ geary_engine_vala_sources = files( 'rfc822/rfc822-part.vala', 'rfc822/rfc822-utils.vala', - 'search/search-email-identifier.vala', - 'search/search-folder-impl.vala', - 'smtp/smtp-authenticator.vala', 'smtp/smtp-capabilities.vala', 'smtp/smtp-client-connection.vala', diff --git a/src/engine/search/search-email-identifier.vala b/src/engine/search/search-email-identifier.vala deleted file mode 100644 index a6c9b4ea..00000000 --- a/src/engine/search/search-email-identifier.vala +++ /dev/null @@ -1,129 +0,0 @@ -/* - * Copyright 2016 Software Freedom Conservancy Inc. - * Copyright 2019 Michael Gratton - * - * This software is licensed under the GNU Lesser General Public License - * (version 2.1 or later). See the COPYING file in this distribution. - */ - -private class Geary.Search.EmailIdentifier : - Geary.EmailIdentifier, Gee.Comparable { - - - 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; - } - - /** Reconstructs an identifier from its variant representation. */ - 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() - ) - ); - } - - /** {@inheritDoc} */ - public override uint hash() { - return this.source_id.hash(); - } - - /** {@inheritDoc} */ - public override bool equal_to(Geary.EmailIdentifier other) { - return ( - this.get_type() == other.get_type() && - this.source_id.equal_to(((EmailIdentifier) other).source_id) - ); - } - - /** {@inheritDoc} */ - 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()) - }) - }); - } - - /** {@inheritDoc} */ - 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); - } - -} From 5eb6bb2a6d6b951070b11ddd20bc9b84bf59da7a Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Fri, 13 Dec 2019 12:21:27 +1100 Subject: [PATCH 09/13] Update and simplify SearchBar component Rename source file name and contents to match code convention, add transation comments, remove extra API in favour of simply exposing the search entry publically. Extend Hdy.Searchbar so that the width of the entry grows as needed. --- po/POTFILES.in | 2 +- .../application/application-main-window.vala | 10 +- .../components/components-search-bar.vala | 92 +++++++++++++++++++ src/client/components/search-bar.vala | 90 ------------------ src/client/meson.build | 2 +- 5 files changed, 99 insertions(+), 97 deletions(-) create mode 100644 src/client/components/components-search-bar.vala delete mode 100644 src/client/components/search-bar.vala diff --git a/po/POTFILES.in b/po/POTFILES.in index 91798075..add1424e 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -37,6 +37,7 @@ src/client/components/components-in-app-notification.vala src/client/components/components-inspector.vala src/client/components/components-placeholder-pane.vala src/client/components/components-preferences-window.vala +src/client/components/components-search-bar.vala src/client/components/components-validator.vala src/client/components/components-web-view.vala src/client/components/count-badge.vala @@ -46,7 +47,6 @@ src/client/components/main-toolbar.vala src/client/components/main-window-info-bar.vala src/client/components/monitored-progress-bar.vala src/client/components/monitored-spinner.vala -src/client/components/search-bar.vala src/client/components/status-bar.vala src/client/components/stock.vala src/client/composer/composer-box.vala diff --git a/src/client/application/application-main-window.vala b/src/client/application/application-main-window.vala index b1c8cdee..d605e027 100644 --- a/src/client/application/application-main-window.vala +++ b/src/client/application/application-main-window.vala @@ -282,7 +282,7 @@ public class Application.MainWindow : // Widget descendants public FolderList.Tree folder_list { get; private set; default = new FolderList.Tree(); } public MainToolbar main_toolbar { get; private set; } - public SearchBar search_bar { get; private set; default = new SearchBar(); } + public SearchBar search_bar { get; private set; } public ConversationListView conversation_list_view { get; private set; } public ConversationViewer conversation_viewer { get; private set; } public StatusBar status_bar { get; private set; default = new StatusBar(); } @@ -822,9 +822,9 @@ public class Application.MainWindow : /** Displays and focuses the search bar for the window. */ public void show_search_bar(string? text = null) { - this.search_bar.give_search_focus(); + this.search_bar.grab_focus(); if (text != null) { - this.search_bar.set_search_text(text); + this.search_bar.entry.text = text; } } @@ -1012,7 +1012,7 @@ public class Application.MainWindow : yield select_folder(to_select, false); if (is_account_search_active) { - this.search_bar.set_search_text(""); + this.search_bar.entry.text = ""; this.search_bar.search_mode_enabled = false; } } @@ -1178,8 +1178,8 @@ public class Application.MainWindow : this.notify["has-toplevel-focus"].connect(on_has_toplevel_focus); // Search bar + this.search_bar = new SearchBar(this.application.engine); this.search_bar.search_text_changed.connect(on_search); - this.search_bar.show(); this.search_bar_box.pack_start(this.search_bar, false, false, 0); // Folder list diff --git a/src/client/components/components-search-bar.vala b/src/client/components/components-search-bar.vala new file mode 100644 index 00000000..3988a2aa --- /dev/null +++ b/src/client/components/components-search-bar.vala @@ -0,0 +1,92 @@ +/* + * Copyright 2016 Software Freedom Conservancy Inc. + * Copyright 2019 Michael Gratton + * + * This software is licensed under the GNU Lesser General Public License + * (version 2.1 or later). See the COPYING file in this distribution. + */ + +public class SearchBar : Hdy.SearchBar { + + /// Translators: Search entry placeholder text + private const string DEFAULT_SEARCH_TEXT = _("Search"); + + public Gtk.SearchEntry entry { + get; private set; default = new Gtk.SearchEntry(); + } + + private Components.EntryUndo search_undo; + private Geary.Account? current_account = null; + private Geary.Engine engine; + + public signal void search_text_changed(string search_text); + + + public SearchBar(Geary.Engine engine) { + this.engine = engine; + this.search_undo = new Components.EntryUndo(this.entry); + + this.notify["search-mode-enabled"].connect(on_search_mode_changed); + + /// Translators: Search entry tooltip + this.entry.tooltip_text = _("Search all mail in account for keywords"); + this.entry.search_changed.connect(() => { + search_text_changed(this.entry.text); + }); + this.entry.activate.connect(() => { + search_text_changed(this.entry.text); + }); + this.entry.placeholder_text = DEFAULT_SEARCH_TEXT; + this.entry.has_focus = true; + + var column = new Hdy.Column(); + column.maximum_width = 450; + column.add(this.entry); + + connect_entry(this.entry); + add(column); + + show_all(); + } + + public override void grab_focus() { + set_search_mode(true); + this.entry.grab_focus(); + } + + public void set_account(Geary.Account? account) { + if (current_account != null) { + current_account.information.changed.disconnect( + on_information_changed + ); + } + + if (account != null) { + account.information.changed.connect( + on_information_changed + ); + } + + current_account = account; + + on_information_changed(); // Set new account name. + } + + private void on_information_changed() { + this.entry.placeholder_text = ( + this.current_account == null || this.engine.accounts_count == 1 + ? DEFAULT_SEARCH_TEXT + /// Translators: Search entry placeholder, string + /// replacement is the name of an account + : _("Search %s account").printf( + this.current_account.information.display_name + ) + ); + } + + private void on_search_mode_changed() { + if (!this.search_mode_enabled) { + this.search_undo.reset(); + } + } +} diff --git a/src/client/components/search-bar.vala b/src/client/components/search-bar.vala deleted file mode 100644 index 53e49833..00000000 --- a/src/client/components/search-bar.vala +++ /dev/null @@ -1,90 +0,0 @@ -/* Copyright 2016 Software Freedom Conservancy Inc. - * - * This software is licensed under the GNU Lesser General Public License - * (version 2.1 or later). See the COPYING file in this distribution. - */ - -public class SearchBar : Gtk.SearchBar { - private const string DEFAULT_SEARCH_TEXT = _("Search"); - - public string search_text { get { return search_entry.text; } } - public bool search_entry_has_focus { get { return search_entry.has_focus; } } - - private Gtk.SearchEntry search_entry = new Gtk.SearchEntry(); - private Components.EntryUndo search_undo; - private Geary.Account? current_account = null; - - public signal void search_text_changed(string search_text); - - public SearchBar() { - // Search entry. - search_entry.width_chars = 28; - search_entry.tooltip_text = _("Search all mail in account for keywords (Ctrl+S)"); - search_entry.search_changed.connect(() => { - search_text_changed(search_entry.text); - }); - search_entry.activate.connect(() => { - search_text_changed(search_entry.text); - }); - search_entry.has_focus = true; - - this.search_undo = new Components.EntryUndo(this.search_entry); - - this.notify["search-mode-enabled"].connect(on_search_mode_changed); - - add(search_entry); - - set_search_placeholder_text(DEFAULT_SEARCH_TEXT); - } - - public void set_search_text(string text) { - this.search_entry.text = text; - } - - public void give_search_focus() { - set_search_mode(true); - search_entry.grab_focus(); - } - - public void set_search_placeholder_text(string placeholder) { - search_entry.placeholder_text = placeholder; - } - - public void set_account(Geary.Account? account) { - if (current_account != null) { - current_account.information.changed.disconnect( - on_information_changed - ); - } - - if (account != null) { - account.information.changed.connect( - on_information_changed - ); - } - - current_account = account; - - on_information_changed(); // Set new account name. - } - - private void on_information_changed() { - var main = get_toplevel() as Application.MainWindow; - if (main != null) { - set_search_placeholder_text( - current_account == null || - main.application.engine.accounts_count == 1 - ? DEFAULT_SEARCH_TEXT : - _("Search %s account").printf( - current_account.information.display_name - ) - ); - } - } - - private void on_search_mode_changed() { - if (!this.search_mode_enabled) { - this.search_undo.reset(); - } - } -} diff --git a/src/client/meson.build b/src/client/meson.build index 92784c99..ed99bacf 100644 --- a/src/client/meson.build +++ b/src/client/meson.build @@ -37,6 +37,7 @@ geary_client_vala_sources = files( 'components/components-inspector-system-view.vala', 'components/components-placeholder-pane.vala', 'components/components-preferences-window.vala', + 'components/components-search-bar.vala', 'components/components-validator.vala', 'components/components-web-view.vala', 'components/count-badge.vala', @@ -46,7 +47,6 @@ geary_client_vala_sources = files( 'components/main-window-info-bar.vala', 'components/monitored-progress-bar.vala', 'components/monitored-spinner.vala', - 'components/search-bar.vala', 'components/status-bar.vala', 'components/stock.vala', From dda8d9baf1f1cb6d156babbdb1f4a9a2547924c7 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sat, 14 Dec 2019 16:34:48 +1100 Subject: [PATCH 10/13] Remove distinct search folder email identifiers The only reason SearchFolder.EmailIdentifier exists was to store the date for ordering the folder, but that can be done with any old class, meaning we can simply pass though existing ImapDb ids to clients, fixing a lot of bugs and corner cases along the way. --- src/engine/app/app-search-folder.vala | 403 ++++++++---------- .../imap-engine-generic-account.vala | 4 - .../imap-engine-generic-account-test.vala | 15 - 3 files changed, 170 insertions(+), 252 deletions(-) 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 - ) - ) - ) - ); } } From cc0fb9eef23bf1cab508e983edd5b2702357b73d Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sat, 14 Dec 2019 16:42:33 +1100 Subject: [PATCH 11/13] Remove unused Geary.Folder::list_local_email_fields_async method --- src/engine/api/geary-folder.vala | 13 -------- src/engine/app/app-search-folder.vala | 12 ------- .../imap-engine-minimal-folder.vala | 9 ------ src/engine/outbox/outbox-folder.vala | 31 ------------------- test/engine/api/geary-folder-mock.vala | 7 ----- 5 files changed, 72 deletions(-) diff --git a/src/engine/api/geary-folder.vala b/src/engine/api/geary-folder.vala index f2762f09..2602f87e 100644 --- a/src/engine/api/geary-folder.vala +++ b/src/engine/api/geary-folder.vala @@ -662,19 +662,6 @@ public abstract class Geary.Folder : BaseObject, Logging.Source { Gee.Collection ids, Geary.Email.Field required_fields, ListFlags flags, Cancellable? cancellable = null) throws Error; - /** - * Returns the locally available Geary.Email.Field fields for the specified emails. If a - * list or fetch operation occurs on the emails that specifies a field not returned here, - * the Engine will either have to go out to the remote server to get it, or (if - * ListFlags.LOCAL_ONLY is specified) not return it to the caller. - * - * If the EmailIdentifier is unknown locally, it will not be present in the returned Map. - * - * The Folder must be opened prior to attempting this operation. - */ - public abstract async Gee.Map? list_local_email_fields_async( - Gee.Collection ids, Cancellable? cancellable = null) throws Error; - /** * Returns a single email that fulfills the required_fields flag at the ordered position in * the folder. If the email_id is invalid for the folder's contents, an EngineError.NOT_FOUND diff --git a/src/engine/app/app-search-folder.vala b/src/engine/app/app-search-folder.vala index 2e11f5a2..5867dd0e 100644 --- a/src/engine/app/app-search-folder.vala +++ b/src/engine/app/app-search-folder.vala @@ -317,18 +317,6 @@ public class Geary.App.SearchFolder : ); } - 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 Email fetch_email_async(EmailIdentifier fetch, Email.Field required_fields, Folder.ListFlags flags, diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala b/src/engine/imap-engine/imap-engine-minimal-folder.vala index 37694c47..09f9b2ba 100644 --- a/src/engine/imap-engine/imap-engine-minimal-folder.vala +++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala @@ -1226,15 +1226,6 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport return !op.accumulator.is_empty ? op.accumulator : null; } - public override async Gee.Map? list_local_email_fields_async( - Gee.Collection ids, Cancellable? cancellable = null) throws Error { - check_open("list_local_email_fields_async"); - check_ids("list_local_email_fields_async", ids); - - return yield local_folder.list_email_fields_by_id_async( - (Gee.Collection) ids, ImapDB.Folder.ListFlags.NONE, cancellable); - } - public override async Geary.Email fetch_email_async(Geary.EmailIdentifier id, Geary.Email.Field required_fields, Geary.Folder.ListFlags flags, Cancellable? cancellable = null) throws Error { diff --git a/src/engine/outbox/outbox-folder.vala b/src/engine/outbox/outbox-folder.vala index 6501a262..7d574d17 100644 --- a/src/engine/outbox/outbox-folder.vala +++ b/src/engine/outbox/outbox-folder.vala @@ -325,37 +325,6 @@ public class Geary.Outbox.Folder : return (list.size > 0) ? list : null; } - public override async Gee.Map? - list_local_email_fields_async(Gee.Collection ids, - GLib.Cancellable? cancellable = null) - throws GLib.Error { - check_open(); - - Gee.Map map = new Gee.HashMap< - Geary.EmailIdentifier, Geary.Email.Field>(); - yield db.exec_transaction_async(Db.TransactionType.RO, (cx) => { - Db.Statement stmt = cx.prepare( - "SELECT id FROM SmtpOutboxTable WHERE ordering=?"); - foreach (Geary.EmailIdentifier id in ids) { - EmailIdentifier? outbox_id = id as EmailIdentifier; - if (outbox_id == null) - throw new EngineError.BAD_PARAMETERS("%s is not outbox EmailIdentifier", id.to_string()); - - stmt.reset(Db.ResetScope.CLEAR_BINDINGS); - stmt.bind_int64(0, outbox_id.ordering); - - // merely checking for presence, all emails in outbox have same fields - Db.Result results = stmt.exec(cancellable); - if (!results.finished) - map.set(outbox_id, Geary.Email.Field.ALL); - } - - return Db.TransactionOutcome.DONE; - }, cancellable); - - return (map.size > 0) ? map : null; - } - public override async Email fetch_email_async(Geary.EmailIdentifier id, Geary.Email.Field required_fields, diff --git a/test/engine/api/geary-folder-mock.vala b/test/engine/api/geary-folder-mock.vala index 29856044..1812e55f 100644 --- a/test/engine/api/geary-folder-mock.vala +++ b/test/engine/api/geary-folder-mock.vala @@ -110,13 +110,6 @@ public class Geary.MockFolder : Folder, MockObject { ); } - public override async Gee.Map? - list_local_email_fields_async(Gee.Collection ids, - Cancellable? cancellable = null) - throws Error { - throw new EngineError.UNSUPPORTED("Mock method"); - } - public override async Geary.Email fetch_email_async(Geary.EmailIdentifier email_id, Geary.Email.Field required_fields, From 77125fc17de69472a0de46e729c39de5944073e0 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sat, 14 Dec 2019 18:09:48 +1100 Subject: [PATCH 12/13] Remove some redundant collection copies --- .../imap-engine/imap-engine-minimal-folder.vala | 16 +++++++--------- .../replay-ops/imap-engine-mark-email.vala | 8 +++++--- .../imap-engine-move-email-prepare.vala | 5 +++-- .../replay-ops/imap-engine-remove-email.vala | 5 +++-- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala b/src/engine/imap-engine/imap-engine-minimal-folder.vala index 09f9b2ba..923b8e71 100644 --- a/src/engine/imap-engine/imap-engine-minimal-folder.vala +++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala @@ -1258,9 +1258,9 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport RemoveEmail remove = new RemoveEmail( this, - (Gee.List) - traverse(to_expunge).to_array_list(), - cancellable); + (Gee.Collection) to_expunge, + cancellable + ); replay_queue.schedule(remove); yield remove.wait_for_ready_async(cancellable); @@ -1312,8 +1312,8 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport MarkEmail mark = new MarkEmail( this, - (Gee.List) - traverse(to_mark).to_array_list(), + (Gee.Collection) + to_mark, flags_to_add, flags_to_remove, cancellable @@ -1374,10 +1374,8 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport return null; MoveEmailPrepare prepare = new MoveEmailPrepare( - this, - (Gee.List) - traverse(to_move).to_array_list(), - cancellable); + this, (Gee.Collection) to_move, cancellable + ); replay_queue.schedule(prepare); yield prepare.wait_for_ready_async(cancellable); diff --git a/src/engine/imap-engine/replay-ops/imap-engine-mark-email.vala b/src/engine/imap-engine/replay-ops/imap-engine-mark-email.vala index ddb0ed91..b48e8edb 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-mark-email.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-mark-email.vala @@ -12,9 +12,11 @@ private class Geary.ImapEngine.MarkEmail : Geary.ImapEngine.SendReplayOperation private Gee.Map? original_flags = null; private Cancellable? cancellable; - public MarkEmail(MinimalFolder engine, Gee.List to_mark, - Geary.EmailFlags? flags_to_add, Geary.EmailFlags? flags_to_remove, - Cancellable? cancellable = null) { + public MarkEmail(MinimalFolder engine, + Gee.Collection to_mark, + EmailFlags? flags_to_add, + EmailFlags? flags_to_remove, + GLib.Cancellable? cancellable = null) { base("MarkEmail", OnError.RETRY); this.engine = engine; diff --git a/src/engine/imap-engine/replay-ops/imap-engine-move-email-prepare.vala b/src/engine/imap-engine/replay-ops/imap-engine-move-email-prepare.vala index 498de990..087dc202 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-move-email-prepare.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-move-email-prepare.vala @@ -19,8 +19,9 @@ private class Geary.ImapEngine.MoveEmailPrepare : Geary.ImapEngine.SendReplayOpe private Cancellable? cancellable; private Gee.List to_move = new Gee.ArrayList(); - public MoveEmailPrepare(MinimalFolder engine, Gee.Collection to_move, - Cancellable? cancellable) { + public MoveEmailPrepare(MinimalFolder engine, + Gee.Collection to_move, + GLib.Cancellable? cancellable) { base.only_local("MoveEmailPrepare", OnError.RETRY); this.engine = engine; diff --git a/src/engine/imap-engine/replay-ops/imap-engine-remove-email.vala b/src/engine/imap-engine/replay-ops/imap-engine-remove-email.vala index 429fb259..de86ab71 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-remove-email.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-remove-email.vala @@ -11,8 +11,9 @@ private class Geary.ImapEngine.RemoveEmail : Geary.ImapEngine.SendReplayOperatio private Gee.Set? removed_ids = null; private int original_count = 0; - public RemoveEmail(MinimalFolder engine, Gee.List to_remove, - Cancellable? cancellable = null) { + public RemoveEmail(MinimalFolder engine, + Gee.Collection to_remove, + GLib.Cancellable? cancellable = null) { base("RemoveEmail", OnError.RETRY); this.engine = engine; From e7f85710e982cb110fc3eb09a376586fcf18a4ff Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 15 Dec 2019 12:20:44 +1100 Subject: [PATCH 13/13] Move display-related Util.Email.shorten_url function to Util.Gtk Add a short doc comment, tweak slightly. --- src/client/conversation-viewer/conversation-message.vala | 4 +++- src/client/util/util-email.vala | 9 --------- src/client/util/util-gtk.vala | 9 +++++++++ 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/client/conversation-viewer/conversation-message.vala b/src/client/conversation-viewer/conversation-message.vala index 3d98d991..f232968e 100644 --- a/src/client/conversation-viewer/conversation-message.vala +++ b/src/client/conversation-viewer/conversation-message.vala @@ -1279,7 +1279,9 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { WebKit.HitTestResult hit_test, uint modifiers) { this.body_container.set_tooltip_text( - hit_test.context_is_link() ? Util.Email.shorten_url(hit_test.get_link_uri()) : null + hit_test.context_is_link() + ? Util.Gtk.shorten_url(hit_test.get_link_uri()) + : null ); this.body_container.trigger_tooltip_query(); } diff --git a/src/client/util/util-email.vala b/src/client/util/util-email.vala index 784cbdca..db321b8c 100644 --- a/src/client/util/util-email.vala +++ b/src/client/util/util-email.vala @@ -281,13 +281,4 @@ namespace Util.Email { return body_text; } - private string shorten_url(string url) { - string new_url = ""; - if (url.length < 90) { - new_url = url; - } else { - new_url = url.substring(0,40) + "..." + url.substring(-40); - } - return new_url; - } } diff --git a/src/client/util/util-gtk.vala b/src/client/util/util-gtk.vala index 798ad0ed..18429bad 100644 --- a/src/client/util/util-gtk.vala +++ b/src/client/util/util-gtk.vala @@ -202,4 +202,13 @@ namespace Util.Gtk { return copy; } + /** Returns a truncated form of a URL if it is too long for display. */ + public string shorten_url(string url) { + string new_url = url; + if (url.length >= 90) { + new_url = url.substring(0,40) + "…" + url.substring(-40); + } + return new_url; + } + }