Commit 82c1e640 authored by Michael Gratton's avatar Michael Gratton 🤞

Merge branch 'wip/search-cleanup' into 'master'

Search cleanup

See merge request !97
parents 55a6d79f beccdd49
Pipeline #57125 passed with stages
in 15 minutes and 4 seconds
......@@ -1271,7 +1271,13 @@ public class GearyController : Geary.BaseObject {
Geary.App.EmailStore? store = get_store_for_folder(
convo.base_folder
);
if (store != null) {
// It's possible for a conversation with zero email to
// be selected, when it has just evaporated after its
// last email was removed but the conversation monitor
// hasn't signalled its removal yet. In this case,
// just don't load it since it will soon disappear.
if (store != null && convo.get_count() > 0) {
viewer.load_conversation.begin(
convo,
store,
......@@ -2267,7 +2273,7 @@ public class GearyController : Geary.BaseObject {
}
private void on_find_in_conversation_action(SimpleAction action) {
this.main_window.conversation_viewer.conversation_find_bar.set_search_mode(true);
this.main_window.conversation_viewer.enable_find();
}
private void on_search_activated(SimpleAction action) {
......
......@@ -611,6 +611,9 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
// load timeout here since this will attempt to fetch
// from the remote
this.fetch_remote_body.begin();
} catch (GLib.IOError.CANCELLED err) {
this.body_loading_timeout.reset();
throw err;
} catch (GLib.Error err) {
this.body_loading_timeout.reset();
handle_load_failure(err);
......@@ -622,6 +625,7 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
try {
yield update_body();
} catch (GLib.Error err) {
this.body_loading_timeout.reset();
handle_load_failure(err);
throw err;
}
......
......@@ -48,8 +48,189 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
private const int MARK_READ_PADDING = 50;
/** Manages find/search term matching in a conversation. */
public class SearchManager : Geary.BaseObject {
// The list that owns this manager
private weak ConversationListBox list;
// Conversation being managed
private Geary.App.Conversation conversation;
// Cached search terms to apply to new messages
private Gee.Set<string>? terms = null;
// Total number of search matches found
private uint matches_found = 0;
// Cancellable used when highlighting search matches
private GLib.Cancellable highlight_cancellable = new GLib.Cancellable();
/** Fired when the number of matching emails has changed. */
public signal void matches_updated(uint matches);
internal SearchManager(ConversationListBox list,
Geary.App.Conversation conversation) {
this.list = list;
this.conversation = conversation;
}
/**
* Loads search term matches for this list's emails.
*/
public async void highlight_matching_email(Geary.SearchQuery query)
throws GLib.Error {
cancel();
// Keep a copy of the current cancellable so it can't get
// changed out from underneath the execution of this method
GLib.Cancellable cancellable = this.highlight_cancellable;
Geary.Account account = this.conversation.base_folder.account;
Gee.Collection<Geary.EmailIdentifier>? matching =
yield account.local_search_async(
query,
this.conversation.get_count(),
0,
null,
this.conversation.get_email_ids(),
cancellable
);
if (matching != null) {
Gee.Set<string>? terms =
yield account.get_search_matches_async(
query, matching, cancellable
);
if (cancellable.is_cancelled()) {
throw new GLib.IOError.CANCELLED(
"Search term highlighting cancelled"
);
}
if (terms != null && !terms.is_empty) {
this.terms = terms;
// Scroll to the first matching row first
EmailRow? first = null;
foreach (Geary.EmailIdentifier id in matching) {
EmailRow? row = this.list.get_email_row_by_id(id);
if (row != null &&
(first == null || row.get_index() < first.get_index())) {
first = row;
}
}
if (first != null) {
this.list.scroll_to(first);
}
// Now expand them all
foreach (Geary.EmailIdentifier id in matching) {
EmailRow? row = this.list.get_email_row_by_id(id);
if (row != null) {
apply_terms(row, terms, cancellable);
row.expand.begin();
}
}
}
}
}
/**
* Highlights matching terms in the given email row, if any.
*/
internal void highlight_row_if_matching(EmailRow row) {
if (this.terms != null) {
apply_terms(row, this.terms, this.highlight_cancellable);
}
}
/**
* Removes search term highlighting from all messages.
*/
public void unmark_terms() {
cancel();
this.list.foreach((child) => {
EmailRow? row = child as EmailRow;
if (row != null) {
if (row.is_search_match) {
row.is_search_match = false;
foreach (ConversationMessage msg_view in row.view) {
msg_view.unmark_search_terms();
}
}
}
});
}
public void cancel() {
this.highlight_cancellable.cancel();
this.highlight_cancellable = new Cancellable();
this.terms = null;
this.matches_found = 0;
notify_matches_updated();
}
private void apply_terms(EmailRow row,
Gee.Set<string>? terms,
GLib.Cancellable cancellable) {
if (row.view.message_body_state == COMPLETED) {
this.apply_terms_impl.begin(
row, terms, cancellable, apply_terms_impl_finished
);
} else {
row.view.notify["message-body-state"].connect(() => {
this.apply_terms_impl.begin(
row, terms, cancellable, apply_terms_impl_finished
);
});
}
}
// This should only be called from apply_terms above
private async uint apply_terms_impl(EmailRow row,
Gee.Set<string>? terms,
GLib.Cancellable cancellable)
throws GLib.IOError.CANCELLED {
uint count = 0;
foreach (ConversationMessage view in row.view) {
if (cancellable.is_cancelled()) {
throw new GLib.IOError.CANCELLED(
"Applying search terms cancelled"
);
}
count += yield view.highlight_search_terms(terms, cancellable);
}
row.is_search_match = (count > 0);
return count;
}
private void apply_terms_impl_finished(GLib.Object? obj,
GLib.AsyncResult res) {
try {
this.matches_found += this.apply_terms_impl.end(res);
notify_matches_updated();
} catch (GLib.IOError.CANCELLED err) {
// All good
}
}
private inline void notify_matches_updated() {
matches_updated(this.matches_found);
}
}
// Base class for list rows it the list box
private abstract class ConversationRow : Gtk.ListBoxRow, Geary.BaseInterface {
internal abstract class ConversationRow : Gtk.ListBoxRow, Geary.BaseInterface {
protected const string EXPANDED_CLASS = "geary-expanded";
......@@ -76,7 +257,7 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
public signal void should_scroll();
public ConversationRow(Geary.Email? email) {
protected ConversationRow(Geary.Email? email) {
base_ref();
this.email = email;
show();
......@@ -122,7 +303,7 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
// Displays a single ConversationEmail in the list box
private class EmailRow : ConversationRow {
internal class EmailRow : ConversationRow {
private const string MATCH_CLASS = "geary-matched";
......@@ -181,7 +362,7 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
// Displays a loading widget in the list box
private class LoadingRow : ConversationRow {
internal class LoadingRow : ConversationRow {
protected const string LOADING_CLASS = "geary-loading";
......@@ -203,7 +384,7 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
// Displays a single embedded composer in the list box
private class ComposerRow : ConversationRow {
internal class ComposerRow : ConversationRow {
// The embedded composer for this row
public ComposerEmbed view { get; private set; }
......@@ -281,6 +462,9 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
/** Conversation being displayed. */
public Geary.App.Conversation conversation { get; private set; }
/** Search manager for highlighting search terms in this list. */
public SearchManager search { get; private set; }
// Used to load messages in conversation.
private Geary.App.EmailStore email_store;
......@@ -303,12 +487,6 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
// The id of the draft referred to by the current composer.
private Geary.EmailIdentifier? draft_id = null;
// Cached search terms to apply to new messages
private Gee.Set<string>? search_terms = null;
// Total number of search matches found
private uint search_matches_found = 0;
private Geary.TimeoutManager mark_read_timer;
......@@ -365,9 +543,6 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
public signal void mark_emails(Gee.Collection<Geary.EmailIdentifier> emails,
Geary.EmailFlags? flags_to_add, Geary.EmailFlags? flags_to_remove);
/** Fired when an email that matches the current search terms is found. */
public signal void search_matches_updated(uint matches);
/**
* Constructs a new conversation list box instance.
......@@ -383,6 +558,8 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
this.avatar_store = avatar_store;
this.config = config;
this.search = new SearchManager(this, conversation);
this.mark_read_timer = new Geary.TimeoutManager.milliseconds(
MARK_READ_TIMEOUT_MSEC, this.check_mark_read
);
......@@ -407,6 +584,7 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
}
public override void destroy() {
this.search.cancel();
this.cancellable.cancel();
this.email_rows.clear();
this.mark_read_timer.reset();
......@@ -571,75 +749,6 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
}
}
/**
* Loads search term matches for this list's emails.
*/
public async void highlight_matching_email(Geary.SearchQuery query)
throws GLib.Error {
this.search_terms = null;
this.search_matches_found = 0;
Geary.Account account = this.conversation.base_folder.account;
Gee.Collection<Geary.EmailIdentifier>? matching =
yield account.local_search_async(
query,
this.conversation.get_count(),
0,
null,
this.conversation.get_email_ids(),
this.cancellable
);
if (matching != null) {
this.search_terms = yield account.get_search_matches_async(
query, matching, this.cancellable
);
if (this.search_terms != null) {
EmailRow? first = null;
foreach (Geary.EmailIdentifier id in matching) {
EmailRow? row = this.email_rows.get(id);
if (row != null &&
(first == null || row.get_index() < first.get_index())) {
first = row;
}
}
if (first != null) {
scroll_to(first);
}
foreach (Geary.EmailIdentifier id in matching) {
EmailRow? row = this.email_rows.get(id);
if (row != null) {
apply_search_terms(row);
row.expand.begin();
}
}
}
}
}
/**
* Removes search term highlighting from all messages.
*/
public void unmark_search_terms() {
this.search_terms = null;
this.search_matches_found = 0;
this.foreach((child) => {
EmailRow? row = child as EmailRow;
if (row != null) {
if (row.is_search_match) {
row.is_search_match = false;
foreach (ConversationMessage msg_view in row.view) {
msg_view.unmark_search_terms();
}
}
}
});
search_matches_updated(this.search_matches_found);
}
/**
* Increases the magnification level used for displaying messages.
*/
......@@ -670,6 +779,11 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
});
}
/** Returns the email row for the given id, if any. */
internal EmailRow? get_email_row_by_id(Geary.EmailIdentifier id) {
return this.email_rows.get(id);
}
private async void finish_loading(Geary.SearchQuery? query,
Gee.LinkedList<Geary.Email> to_insert,
Gee.LinkedList<Geary.Email> to_append)
......@@ -721,7 +835,7 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
// XXX this sucks for large conversations because it can take
// a long time for the load to complete and hence for
// matches to show up.
yield highlight_matching_email(query);
yield this.search.highlight_matching_email(query);
}
}
......@@ -762,6 +876,7 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
if (!this.cancellable.is_cancelled()) {
EmailRow row = add_email(full_email);
row.view.load_avatar.begin(this.avatar_store);
this.search.highlight_row_if_matching(row);
yield row.expand();
}
}
......@@ -815,11 +930,6 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
}
email_added(view);
// Apply any existing search terms to the new row
if (this.search_terms != null) {
apply_search_terms(row);
}
return row;
}
......@@ -901,33 +1011,6 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
}
}
private void apply_search_terms(EmailRow row) {
if (row.view.message_body_state == COMPLETED) {
this.apply_search_terms_impl.begin(row);
} else {
row.view.notify["message-body-state"].connect(() => {
this.apply_search_terms_impl.begin(row);
});
}
}
// This should only be called from apply_search_terms above
private async void apply_search_terms_impl(EmailRow row) {
bool found = false;
foreach (ConversationMessage view in row.view) {
if (this.search_terms == null) {
break;
}
uint count = yield view.highlight_search_terms(this.search_terms);
if (count > 0) {
found = true;
}
this.search_matches_found += count;
}
row.is_search_match = found;
search_matches_updated(this.search_matches_found);
}
/**
* Returns an new Iterable over all email views in the viewer
*/
......
......@@ -666,10 +666,9 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
* Highlighting includes both in the message headers, and the
* mesage body. returns the number of matching search terms.
*/
public async uint highlight_search_terms(Gee.Set<string> search_matches) {
// Remove existing highlights
this.web_view.get_find_controller().search_finish();
public async uint highlight_search_terms(Gee.Set<string> search_matches,
GLib.Cancellable cancellable)
throws GLib.IOError.CANCELLED {
uint headers_found = 0;
uint webkit_found = 0;
foreach(string raw_match in search_matches) {
......@@ -689,7 +688,9 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
}
}
webkit_found += yield this.web_view.highlight_search_terms(search_matches);
webkit_found += yield this.web_view.highlight_search_terms(
search_matches, cancellable
);
return headers_found + webkit_found;
}
......
......@@ -220,6 +220,12 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface {
set_visible_child(this.empty_search_page);
}
/** Shows and focuses the find entry. */
public void enable_find() {
this.conversation_find_bar.set_search_mode(true);
this.conversation_find_entry.grab_focus();
}
/**
* Shows a conversation in the viewer.
*/
......@@ -246,7 +252,7 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface {
// are expanded and highlighted as they are added.
this.conversation_find_next.set_sensitive(false);
this.conversation_find_prev.set_sensitive(false);
new_list.search_matches_updated.connect((count) => {
new_list.search.matches_updated.connect((count) => {
bool found = count > 0;
this.conversation_find_entry.set_icon_from_icon_name(
Gtk.EntryIconPosition.PRIMARY,
......@@ -383,7 +389,7 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface {
}
} else {
// Find became disabled, re-show search terms if any
this.current_list.unmark_search_terms();
this.current_list.search.unmark_terms();
Geary.SearchFolder? search_folder = (
this.current_list.conversation.base_folder
as Geary.SearchFolder
......@@ -391,7 +397,7 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface {
if (search_folder != null) {
Geary.SearchQuery? search_query = search_folder.search_query;
if (search_query != null) {
this.current_list.highlight_matching_email.begin(
this.current_list.search.highlight_matching_email.begin(
search_query
);
}
......@@ -409,7 +415,7 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface {
this.current_list.conversation.base_folder.account
);
if (query != null) {
this.current_list.highlight_matching_email.begin(query);
this.current_list.search.highlight_matching_email.begin(query);
}
}
}
......
......@@ -101,7 +101,14 @@ public class ConversationWebView : ClientWebView {
*
* Returns the number of matching search terms.
*/
public async uint highlight_search_terms(Gee.Collection<string> search_matches) {
public async uint highlight_search_terms(Gee.Collection<string> terms,
GLib.Cancellable cancellable)
throws GLib.IOError.CANCELLED {
WebKit.FindController controller = get_find_controller();
// Remove existing highlights
controller.search_finish();
// XXX WK2 doesn't deal with the multiple highlighting
// required by search folder matches, only single highlighting
// for a fine-like interface. For now, just highlight the
......@@ -109,25 +116,20 @@ public class ConversationWebView : ClientWebView {
uint found = 0;
WebKit.FindController controller = get_find_controller();
SourceFunc callback = this.highlight_search_terms.callback;
ulong found_handler = 0;
ulong not_found_handler = 0;
found_handler = controller.found_text.connect((count) => {
ulong found_handler = controller.found_text.connect((count) => {
found = count;
controller.disconnect(found_handler);
controller.disconnect(not_found_handler);
callback();
});
not_found_handler = controller.failed_to_find_text.connect(() => {
controller.disconnect(found_handler);
controller.disconnect(not_found_handler);
ulong not_found_handler = controller.failed_to_find_text.connect(() => {
callback();
});
ulong cancelled_handler = cancellable.cancelled.connect(() => {
callback();
});
controller.search(
Geary.Collection.get_first(search_matches),
Geary.Collection.get_first(terms),
WebKit.FindOptions.CASE_INSENSITIVE |
WebKit.FindOptions.WRAP_AROUND,
128
......@@ -135,6 +137,16 @@ public class ConversationWebView : ClientWebView {
yield;
controller.disconnect(found_handler);
controller.disconnect(not_found_handler);
cancellable.disconnect(cancelled_handler);
if (cancellable.is_cancelled()) {
throw new IOError.CANCELLED(
"ConversationWebView highlight search terms cancelled"
);
}
return found;
}
......
......@@ -24,7 +24,7 @@ internal abstract class Geary.App.ConversationOperation : BaseObject {
public ConversationOperation(ConversationMonitor? monitor,
bool allow_duplicates = false) {
bool allow_duplicates = true) {
this.monitor = monitor;
this.allow_duplicates = allow_duplicates;
}
......
......@@ -15,7 +15,7 @@ private class Geary.App.InsertOperation : ConversationOperation {
public InsertOperation(ConversationMonitor monitor,
Gee.Collection<EmailIdentifier> inserted_ids) {
base(monitor, false);
base(monitor);
this.inserted_ids = inserted_ids;
}
......
......@@ -1092,7 +1092,8 @@ private class Geary.ImapDB.Account : BaseObject {
Gee.Collection<Geary.EmailIdentifier>? search_ids = null, Cancellable? cancellable = null)
throws Error {
debug("Search: %s", q.to_string());
debug("Search terms, offset/limit: %s %d/%d",
q.to_string(), offset, limit);
check_open();
ImapDB.SearchQuery query = check_search_query(q);
......@@ -1121,28 +1122,11 @@ private class Geary.ImapDB.Account : BaseObject {
// Do this outside of transaction to catch invalid search ids up-front
string? search_ids_sql = get_search_ids_sql(search_ids);
// for some searches, results are stripped if they're too "greedy", but this requires
// examining the matched text, which has an expense to fetch, so avoid doing so unless
// necessary
bool strip_results = true;
// HORIZON strategy is configured in such a way to allow all stemmed variants to match,
// so don't do any stripping in that case
//
// If any of the search terms is exact-match (no prefix matching) or none have stemmed
// variants, then don't do stripping of "greedy" stemmed matching (because in both cases,
// there are none)
if (query.strategy == Geary.SearchQuery.Strategy.HORIZON)
strip_results = false;
else if (traverse<SearchTerm>(query.get_all_terms()).any(term => term.stemmed == null || term.is_exact))
strip_results = false;
debug(strip_results ? "Stripping results..." : "Not stripping results...");
bool strip_greedy = should_strip_greedy_results(query);
Gee.Set<EmailIdentifier> matching_ids = new Gee.HashSet<EmailIdentifier>();
Gee.Map<EmailIdentifier,Gee.Set<string>>? search_matches = null;
Gee.Set<ImapDB.EmailIdentifier> unstripped_ids = new Gee.HashSet<ImapDB.EmailIdentifier>();
Gee.Map<ImapDB.EmailIdentifier, Gee.Set<string>>? search_results = null;
yield db.exec_transaction_async(Db.TransactionType.RO, (cx) => {
string blacklisted_ids_sql = do_get_blacklisted_message_ids_sql(
folder_blacklist, cx, cancellable);
......@@ -1198,47 +1182,38 @@ private class Geary.ImapDB.Account : BaseObject {
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);
unstripped_ids.add(id);
matching_ids.add(id);
id_map.set(message_id, id);
result.next(cancellable);
}
if (!strip_results)
return Db.TransactionOutcome.DONE;
search_results = do_get_search_matches(cx, query, id_map, cancellable);
if (strip_greedy && !id_map.is_empty) {
search_matches = do_get_search_matches(
cx, query, id_map, cancellable
);
}
return Db.TransactionOutcome.DONE;
}, cancellable);
if (unstripped_ids == null || unstripped_ids.size == 0)
return null;
// at this point, there should be some "full" search results to strip from
if (strip_results)
assert(search_results != null && search_results.size > 0);
debug("Matching emails found: %d", matching_ids.size);