From 684facf025fa01d581e1382cfeb485b0f1ccb0e6 Mon Sep 17 00:00:00 2001 From: Luke Yang Date: Thu, 29 Jun 2023 13:21:25 -0400 Subject: [PATCH] Add `kola` tests and fix `Name & Summary` search `kola` tests added to test basic search functionality (name, summary, name & summary, and glob matches). Small fix for the duplicate removal feature. The `set` used to store existing query results needed to be passed as a pointer to the `query_results_to_builder` function. This allowed the data within the `set` to persist outside the function call. A large was change made to the `Name & Summary` search feature. Previously, the function would only return results that contained ALL search terms within BOTH the name and summary. With this commit, results in which search terms match EITHER the name or summary will be returned. This better matches `dnf`/`yum` CLI functionality. --- src/app/rpmostree-pkg-builtins.cxx | 4 +- src/daemon/rpmostreed-os.cxx | 119 +++++++++++++++++++++----- tests/kolainst/nondestructive/misc.sh | 33 +++++++ 3 files changed, 134 insertions(+), 22 deletions(-) diff --git a/src/app/rpmostree-pkg-builtins.cxx b/src/app/rpmostree-pkg-builtins.cxx index ec8790360f..1ca3564c83 100644 --- a/src/app/rpmostree-pkg-builtins.cxx +++ b/src/app/rpmostree-pkg-builtins.cxx @@ -406,8 +406,8 @@ rpmostree_builtin_search (int argc, char **argv, RpmOstreeCommandInvocation *inv if (query_set.size () == 0) { - g_print ("No matches found."); + g_print ("No matches found.\n"); } return TRUE; -} \ No newline at end of file +} diff --git a/src/daemon/rpmostreed-os.cxx b/src/daemon/rpmostreed-os.cxx index 2275c8f2db..52185c663b 100644 --- a/src/daemon/rpmostreed-os.cxx +++ b/src/daemon/rpmostreed-os.cxx @@ -1155,61 +1155,140 @@ struct cstrless /* wrapper function to both query for packages and add them to the builder */ static void -query_results_to_builder (HyQuery query, GVariantBuilder *builder, const gchar *id) +query_results_to_builder (HyQuery query, GVariantBuilder *builder, const gchar *id, + std::set *result_set) { - std::set result_set; g_autoptr (GPtrArray) pkglist = hy_query_run (query); - for (guint i = 0; i < pkglist->len && result_set.size () < 50; i++) + for (guint i = 0; i < pkglist->len && (*result_set).size () < 50; i++) { auto pkg = static_cast (g_ptr_array_index (pkglist, i)); - if (!result_set.count (dnf_package_get_name (pkg))) + if (!(*result_set).count (dnf_package_get_name (pkg))) { os_add_package_info_to_builder (pkg, builder, id); - result_set.insert (dnf_package_get_name (pkg)); + (*result_set).insert (dnf_package_get_name (pkg)); } } } +/* helper function to apply Name/Summary or HY_EQ/HY_SUBSTR filters on search term */ +static void +apply_search_filter (HyQuery *query, int keyname, const gchar *const name, int cmp_type) +{ + if (!hy_is_glob_pattern (name)) + { + hy_query_filter (*query, keyname, cmp_type | HY_ICASE, name); + } + else + { + hy_query_filter (*query, keyname, HY_GLOB | HY_ICASE, name); + } +} + /* helper function to filter package query results */ static void search_packages_by_filter (HyQuery query, GVariantBuilder *builder, const gchar *const *names, std::vector keynames, const gchar *id) { - /* case insensitive exact match between search term and package */ - hy_query_clear (query); - for (guint j = 0; j < keynames.size (); j++) + std::set result_set; + HyQuery intermediate_query = hy_query_clone (query); + HyQuery final_query = hy_query_clone (query); + + int names_count = 0; + for (guint i = 0; names[i] != NULL; i++) { + names_count++; + } + + /* Name/Summary matches */ + if (keynames.size () < 2) + { + hy_query_clear (query); for (guint i = 0; names[i] != NULL; i++) { - if (!hy_is_glob_pattern (names[i])) + apply_search_filter (&query, keynames[0], names[i], HY_EQ); + } + query_results_to_builder (query, builder, id, &result_set); + + hy_query_clear (query); + for (guint i = 0; names[i] != NULL; i++) + { + apply_search_filter (&query, keynames[0], names[i], HY_SUBSTR); + } + query_results_to_builder (query, builder, id, &result_set); + } + + /* Name AND Summary matches for more than one search term */ + /* ========================================================================================= + For each search term, apply a query with the keyname filter (Name or Summary) and unions the + results. This allows multi-term searches to return matches when search terms are found in + either the Name or Summary of a package. After this, return the intersection of the results + for each search term to filter out results that do not contain all matching terms. + ========================================================================================= */ + else if (keynames.size () >= 2 && names_count >= 2) + { + + for (guint i = 0; names[i] != NULL; i++) + { + hy_query_clear (intermediate_query); + for (guint j = 0; j < keynames.size (); j++) + { + hy_query_clear (query); + apply_search_filter (&query, keynames[j], names[i], HY_EQ); + + if (j != 0) + { + hy_query_union (intermediate_query, query); + } + else + { + intermediate_query = hy_query_clone (query); + } + + hy_query_clear (query); + apply_search_filter (&query, keynames[j], names[i], HY_SUBSTR); + hy_query_union (intermediate_query, query); + } + + if (i != 0) { - hy_query_filter (query, keynames[j], HY_EQ | HY_ICASE, names[i]); + hy_query_intersection (final_query, intermediate_query); } else { - hy_query_filter (query, keynames[j], HY_GLOB | HY_ICASE, names[i]); + final_query = hy_query_clone (intermediate_query); } } + query_results_to_builder (final_query, builder, id, &result_set); } - query_results_to_builder (query, builder, id); - /* case insensitive substring match of search term within package */ - hy_query_clear (query); - for (guint j = 0; j < keynames.size (); j++) + /* Name AND Summary matches for only one search term */ + /* ========================================================================================= + For the case of a single search term, return the intersection of both Name and Summary matches + of the search term (instead of the union for multi-term searches). + ========================================================================================= */ + else if (keynames.size () >= 2 && names_count < 2) { - for (guint i = 0; names[i] != NULL; i++) + for (guint i = 0; i < keynames.size (); i++) { - if (!hy_is_glob_pattern (names[i])) + hy_query_clear (query); + apply_search_filter (&query, keynames[i], names[0], HY_EQ); + intermediate_query = hy_query_clone (query); + + hy_query_clear (query); + apply_search_filter (&query, keynames[i], names[0], HY_SUBSTR); + hy_query_union (intermediate_query, query); + + if (i != 0) { - hy_query_filter (query, keynames[j], HY_SUBSTR | HY_ICASE, names[i]); + hy_query_intersection (final_query, intermediate_query); } else { - hy_query_filter (query, keynames[j], HY_GLOB | HY_ICASE, names[i]); + final_query = hy_query_clone (intermediate_query); } } + query_results_to_builder (final_query, builder, id, &result_set); } - query_results_to_builder (query, builder, id); } static gboolean diff --git a/tests/kolainst/nondestructive/misc.sh b/tests/kolainst/nondestructive/misc.sh index 6965b2f280..223bd45d61 100755 --- a/tests/kolainst/nondestructive/misc.sh +++ b/tests/kolainst/nondestructive/misc.sh @@ -101,6 +101,39 @@ rpmostree_busctl_call_os GetPackages as 1 should-not-exist-p-equals-np > out.txt assert_file_has_content_literal out.txt 'aa{sv} 0' echo "ok dbus GetPackages" +rpmostree_busctl_call_os Search as 1 testdaemon > out.txt +assert_file_has_content_literal out.txt '"epoch" t 0' +assert_file_has_content_literal out.txt '"reponame" s "libtest"' +assert_file_has_content_literal out.txt '"nevra" s "testdaemon' +rpmostree_busctl_call_os Search as 1 should-not-exist-p-equals-np > out.txt +assert_file_has_content_literal out.txt 'aa{sv} 0' +echo "ok dbus Search" + +rpm-ostree search testdaemon > out.txt +assert_file_has_content_literal out.txt '===== Name Matched =====' +assert_file_has_content_literal out.txt 'testdaemon : awesome-daemon-for-testing' +echo "ok Search name match" + +rpm-ostree search awesome-daemon > out.txt +assert_file_has_content_literal out.txt '===== Summary Matched =====' +assert_file_has_content_literal out.txt 'testdaemon : awesome-daemon-for-testing' +echo "ok Search summary match" + +rpm-ostree search testdaemon awesome-daemon > out.txt +assert_file_has_content_literal out.txt '===== Summary & Name Matched =====' +assert_file_has_content_literal out.txt 'testdaemon : awesome-daemon-for-testing' +echo "ok Search name and summary match" + +rpm-ostree search "test*" > out.txt +assert_file_has_content_literal out.txt '===== Summary & Name Matched =====' +assert_file_has_content_literal out.txt '===== Name Matched =====' +assert_file_has_content_literal out.txt '===== Summary Matched =====' +assert_file_has_content_literal out.txt 'testdaemon : awesome-daemon-for-testing' +assert_file_has_content_literal out.txt 'testpkg-etc : testpkg-etc' +assert_file_has_content_literal out.txt 'testpkg-post-infinite-loop : testpkg-post-infinite-loop' +assert_file_has_content_literal out.txt 'testpkg-touch-run : testpkg-touch-run' +echo "ok Search glob pattern match" + # Verify operations as non-root runuser -u core rpm-ostree status echo "ok status doesn't require root"