Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fast search #2179

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/Core/FlatpakBackend.vala
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,10 @@ public class AppCenterCore.FlatpakBackend : Object {
return apps.values;
}

public SearchEngine get_search_engine () {
return new SearchEngine (package_list.values.to_array (), user_appstream_pool);
}

public Gee.Collection<Package> search_applications (string query, AppStream.Category? category) {
var results = new Gee.TreeSet<AppCenterCore.Package> ();
var comps = user_appstream_pool.search (query);
Expand Down Expand Up @@ -501,10 +505,6 @@ public class AppCenterCore.FlatpakBackend : Object {
return apps.values;
}

public Gee.Collection<Package> search_applications_mime (string query) {
return new Gee.ArrayList<Package> ();
}

public Package? get_package_for_component_id (string id) {
var suffixed_id = id + ".desktop";
foreach (var package in package_list.values) {
Expand Down
25 changes: 25 additions & 0 deletions src/Core/Package.vala
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,31 @@ public class AppCenterCore.Package : Object {
}
}

public uint cached_search_score = 0;
public uint matches_search (string[] queries) {
// TODO: We don't use AppStream.Component.search_matches_all because it has some broken vapi
// (or at least I think so: the c code takes gchar** but vapi says string)

if (queries.length == 0) {
cached_search_score = 0;
return 0;
}

uint score = 0;
foreach (var query in queries) {
var query_score = component.search_matches (query);

if (query_score == 0) {
score = 0;
break;
}

score += query_score;
}
cached_search_score = score / queries.length;
return cached_search_score;
}

private string? name = null;
public string? get_name () {
if (name != null) {
Expand Down
74 changes: 74 additions & 0 deletions src/Core/SearchEngine.vala
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* SPDX-FileCopyrightText: 2024 elementary, Inc. (https://elementary.io)
* SPDX-License-Identifier: GPL-3.0-or-later
*
* Authored by: Leonhard Kargl <leo.kargl@proton.me>
*/

public class AppCenterCore.SearchEngine : Object {
public ListModel results { get; private set; }

private ListStore packages;
private AppStream.Pool pool;

private string[] query;
private AppStream.Category? category;

public SearchEngine (Package[] packages, AppStream.Pool pool) {
var unique_packages = new Gee.HashMap<string, Package> ();
foreach (var package in packages) {
var package_component_id = package.normalized_component_id;
if (unique_packages.has_key (package_component_id)) {
if (package.origin_score > unique_packages[package_component_id].origin_score) {
unique_packages[package_component_id] = package;
}
} else {
unique_packages[package_component_id] = package;
}
}

this.packages.splice (0, 0, unique_packages.values.to_array ());
this.pool = pool;
}
Comment on lines +17 to +32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

@leolost2605 leolost2605 Jun 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I didn't use it here because we don't need the packages array but only the liststore with the unique packages so it seemed kinda pointless to first put the array into a property and then in construct extracting this stuff and then setting the property to null to free the memory. Alternatively we could ofc do all of this in the flatpackbackend and hand over the liststore right away 🤷

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that might be the way to go here so we can stick to coding conventions


construct {
packages = new ListStore (typeof (Package));

var filter_model = new Gtk.FilterListModel (packages, new Gtk.CustomFilter ((obj) => {
var package = (Package) obj;

if (category != null && !package.component.is_member_of_category (category)) {
return false;
}

return ((Package) obj).matches_search (query) > 0;
})) {
incremental = true
};

var sort_model = new Gtk.SortListModel (filter_model, new Gtk.CustomSorter ((obj1, obj2) => {
var package1 = (Package) obj1;
var package2 = (Package) obj2;
return (int) (package2.cached_search_score - package1.cached_search_score);
}));

results = sort_model;
}

public void search (string query, AppStream.Category? category) {
this.query = pool.build_search_tokens (query);
this.category = category;
packages.items_changed (0, packages.n_items, packages.n_items);
}

/**
* This should be called if the engine is no longer needed.
* We need this because thanks to how vala sets delegates we get a reference cycle,
* where the filter and sorter keep a reference on us and we on them.
* Setting results to null will free them and they will in turn free us.
* https://gitlab.gnome.org/GNOME/vala/-/issues/957
*/
public void cleanup () {
results = null;
}
}
3 changes: 2 additions & 1 deletion src/Views/AppInfoView.vala
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,8 @@ public class AppCenter.Views.AppInfoView : Adw.NavigationPage {
overflow = VISIBLE
};

action_stack = new ActionStack (package);
action_stack = new ActionStack ();
action_stack.package = package;
Comment on lines +213 to +214
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do:

Suggested change
action_stack = new ActionStack ();
action_stack.package = package;
action_stack = new ActionStack () {
package = package
};


var button_box = new Gtk.Box (Gtk.Orientation.HORIZONTAL, 0) {
halign = Gtk.Align.END,
Expand Down
125 changes: 48 additions & 77 deletions src/Views/SearchView.vala
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,12 @@ public class AppCenter.SearchView : Adw.NavigationPage {
public string search_term { get; construct; }
public bool mimetype { get; set; default = false; }

private GLib.ListStore list_store;
private AppCenterCore.SearchEngine search_engine;
private Gtk.SearchEntry search_entry;
private Gtk.NoSelection selection_model;
private Gtk.ListView list_view;
private Gtk.ScrolledWindow scrolled;
private Gtk.Stack stack;
private Granite.Placeholder alert_view;

public SearchView (string search_term) {
Expand Down Expand Up @@ -61,23 +65,37 @@ public class AppCenter.SearchView : Adw.NavigationPage {
};
headerbar.pack_start (new BackButton ());

list_store = new GLib.ListStore (typeof (AppCenterCore.Package));
search_engine = AppCenterCore.FlatpakBackend.get_default ().get_search_engine ();

var list_box = new Gtk.ListBox () {
activate_on_single_click = true,
selection_model = new Gtk.NoSelection (search_engine.results);

var factory = new Gtk.SignalListItemFactory ();
factory.setup.connect ((obj) => {
var list_item = (Gtk.ListItem) obj;
list_item.child = new Widgets.ListPackageRowGrid (null);
});
factory.bind.connect ((obj) => {
var list_item = (Gtk.ListItem) obj;
((Widgets.ListPackageRowGrid) list_item.child).bind ((AppCenterCore.Package) list_item.item);
});

list_view = new Gtk.ListView (selection_model, factory) {
single_click_activate = true,
hexpand = true,
vexpand = true
};
list_box.bind_model (list_store, create_row_from_package);
list_box.set_placeholder (alert_view);

var scrolled = new Gtk.ScrolledWindow () {
child = list_box,
scrolled = new Gtk.ScrolledWindow () {
child = list_view,
hscrollbar_policy = Gtk.PolicyType.NEVER
};

stack = new Gtk.Stack ();
stack.add_child (alert_view);
stack.add_child (scrolled);

var toolbarview = new Adw.ToolbarView () {
content = scrolled
content = stack
};
toolbarview.add_top_bar (headerbar);

Expand All @@ -91,10 +109,10 @@ public class AppCenter.SearchView : Adw.NavigationPage {
search_entry.grab_focus ();
});

list_box.row_activated.connect ((row) => {
if (row is Widgets.PackageRow) {
show_app (((Widgets.PackageRow) row).get_package ());
}
selection_model.items_changed.connect (on_items_changed);

list_view.activate.connect ((index) => {
show_app ((AppCenterCore.Package) search_engine.results.get_item (index));
});

search_entry.search_changed.connect (search);
Expand All @@ -113,29 +131,34 @@ public class AppCenter.SearchView : Adw.NavigationPage {
});
}

private void search () {
list_store.remove_all ();
~SearchView () {
search_engine.cleanup ();
}

private void on_items_changed () {
list_view.scroll_to (0, NONE, null);

if (selection_model.n_items > 0) {
stack.visible_child = scrolled;
} else {
stack.visible_child = alert_view;
}
}

private void search () {
if (search_entry.text.length >= VALID_QUERY_LENGTH) {
var dyn_flathub_link = "<a href='https://flathub.org/apps/search/%s'>%s</a>".printf (search_entry.text, _("Flathub"));
alert_view.description = _("Try changing search terms. You can also sideload Flatpak apps e.g. from %s").printf (dyn_flathub_link);

unowned var flatpak_backend = AppCenterCore.FlatpakBackend.get_default ();

Gee.Collection<AppCenterCore.Package> found_apps;

if (mimetype) {
found_apps = flatpak_backend.search_applications_mime (search_entry.text);
add_packages (found_apps);
// This didn't do anything so TODO
} else {
var category = update_category ();

found_apps = flatpak_backend.search_applications (search_entry.text, category);
add_packages (found_apps);
search_engine.search (search_entry.text, update_category ());
}

} else {
alert_view.description = _("The search term must be at least 3 characters long.");
stack.visible_child = alert_view;
}

if (mimetype) {
Expand All @@ -156,56 +179,4 @@ public class AppCenter.SearchView : Adw.NavigationPage {
search_entry.placeholder_text = _("Search Apps");
return null;
}

public void add_packages (Gee.Collection<AppCenterCore.Package> packages) {
foreach (var package in packages) {
// Don't show plugins or fonts in search and category views
if (package.kind != AppStream.ComponentKind.ADDON && package.kind != AppStream.ComponentKind.FONT) {
GLib.CompareDataFunc<AppCenterCore.Package> sort_fn = (a, b) => {
return compare_packages (a, b);
};

list_store.insert_sorted (package, sort_fn);
}
}
}

private Gtk.Widget create_row_from_package (Object object) {
unowned var package = (AppCenterCore.Package) object;
return new Widgets.PackageRow.list (package);
}

private int search_priority (string name) {
if (name != null && search_entry.text != "") {
var name_lower = name.down ();
var term_lower = search_entry.text.down ();

var term_position = name_lower.index_of (term_lower);

// App name starts with our search term, highest priority
if (term_position == 0) {
return 2;
// App name contains our search term, high priority
} else if (term_position != -1) {
return 1;
}
}

// Otherwise, normal appstream search ranking order
return 0;
}

private int compare_packages (AppCenterCore.Package p1, AppCenterCore.Package p2) {
if ((p1.kind == AppStream.ComponentKind.ADDON) != (p2.kind == AppStream.ComponentKind.ADDON)) {
return p1.kind == AppStream.ComponentKind.ADDON ? 1 : -1;
}

int sp1 = search_priority (p1.get_name ());
int sp2 = search_priority (p2.get_name ());
if (sp1 != sp2) {
return sp2 - sp1;
}

return p1.get_name ().collate (p2.get_name ());
}
}
Loading
Loading