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.
This commit is contained in:
Michael Gratton 2019-12-09 12:35:02 +11:00 committed by Michael James Gratton
parent 4d0ed05a2b
commit c93cfc38b1
9 changed files with 215 additions and 161 deletions

View file

@ -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<Geary.App.Conversation> visible) {

View file

@ -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
);
}

View file

@ -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;
/**

View file

@ -1,12 +1,13 @@
/* Copyright 2016 Software Freedom Conservancy Inc.
/*
* Copyright 2016 Software Freedom Conservancy Inc.
* Copyright 2019 Michael Gratton <mike@vee.net>
*
* 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<string>? get_search_matches_async(
Gee.Collection<Geary.EmailIdentifier> ids, Cancellable? cancellable = null) throws Error;
}
Gee.Collection<EmailIdentifier> ids,
GLib.Cancellable? cancellable = null
) throws GLib.Error;
}

View file

@ -1,20 +1,27 @@
/* Copyright 2016 Software Freedom Conservancy Inc.
/*
* Copyright 2016 Software Freedom Conservancy Inc.
* Copyright 2019 Michael Gratton <mike@vee.met>
*
* 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;
}

View file

@ -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<Geary.EmailIdentifier> ids, Cancellable? cancellable) throws Error {
private async void append_new_email_async(Geary.Folder folder,
Gee.Collection<Geary.EmailIdentifier> 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<Geary.EmailIdentifier> ids, Cancellable? cancellable) throws Error {
private async void handle_removed_email_async(Geary.Folder folder,
Gee.Collection<Geary.EmailIdentifier> 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<ImapDB.SearchEmailIdentifier> relevant_ids
= Geary.traverse<Geary.EmailIdentifier>(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<ImapDB.SearchEmailIdentifier> 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<Geary.EmailIdentifier>? add_ids,
Gee.Collection<ImapDB.SearchEmailIdentifier>? remove_ids, Cancellable? cancellable) throws Error {
private async void do_search_async(Gee.Collection<Geary.EmailIdentifier>? add_ids,
Gee.Collection<SearchEmailIdentifier>? 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<ImapDB.SearchEmailIdentifier> 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<SearchEmailIdentifier> 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<ImapDB.SearchEmailIdentifier> added
= Gee.List.empty<ImapDB.SearchEmailIdentifier>();
@ -338,11 +335,16 @@ private class Geary.ImapDB.SearchFolder : Geary.SearchFolder, Geary.FolderSuppor
* search query.
*/
public override async Gee.Set<string>? get_search_matches_async(
Gee.Collection<Geary.EmailIdentifier> 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<Geary.EmailIdentifier> ids,
GLib.Cancellable? cancellable = null
) throws GLib.Error {
Gee.Set<string>? 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>(
ImapDB.SearchEmailIdentifier.compare_descending);
this.search_results = new Gee.TreeSet<ImapDB.SearchEmailIdentifier>(
SearchEmailIdentifier.compare_descending
);
}
private void on_folders_available_unavailable(Gee.Collection<Geary.Folder>? 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<Geary.EmailIdentifier> 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<Geary.EmailIdentifier> 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);
}
}
}

View file

@ -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<SearchTerm> all = new Gee.ArrayList<SearchTerm>();
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:

View file

@ -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<Geary.EmailIdentifier>? local_search_async(Geary.SearchQuery query,

View file

@ -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<EmailIdentifier>?