[gnome-software] trivial: Respect priority when deduplicating apps

[Richard Hughes <rhughes src gnome org>]

commit 2ad174ced7bf36f5c478b7c0834cbbd61de9c424
Author: Richard Hughes <richard hughsie com>
Date:   Mon Jun 27 10:38:52 2016 +0100

    trivial: Respect priority when deduplicating apps

 src/gs-app-list-private.h |   17 ++++++++++++-
 src/gs-app-list.c         |   60 ++++++++++++++++++++++++++++++++++----------
 src/gs-plugin-loader.c    |   55 ++++++++++++++++++++++++++++------------
 src/gs-self-test.c        |   22 +++++++++++++++-
 4 files changed, 121 insertions(+), 33 deletions(-)
---
diff --git a/src/gs-app-list-private.h b/src/gs-app-list-private.h
index 5ee70e8..96e0769 100644
--- a/src/gs-app-list-private.h
+++ b/src/gs-app-list-private.h
@@ -26,6 +26,20 @@
 
 G_BEGIN_DECLS
 
+/**
+ * GsAppListFilterFlags:
+ * @GS_APP_LIST_FILTER_FLAG_NONE:      No flags set
+ * @GS_APP_LIST_FILTER_FLAG_PRIORITY:  Filter by application priority
+ *
+ * Flags to use when filtering.
+ **/
+typedef enum {
+       GS_APP_LIST_FILTER_FLAG_NONE            = 0,
+       GS_APP_LIST_FILTER_FLAG_PRIORITY        = 1 << 0,
+       /*< private >*/
+       GS_APP_LIST_FILTER_FLAG_LAST
+} GsAppListFilterFlags;
+
 typedef gboolean (*GsAppListFilterFunc)                (GsApp          *app,
                                                 gpointer        user_data);
 typedef gboolean (*GsAppListSortFunc)          (GsApp          *app1,
@@ -39,7 +53,8 @@ void           gs_app_list_filter             (GsAppList      *list,
 void            gs_app_list_sort               (GsAppList      *list,
                                                 GsAppListSortFunc func,
                                                 gpointer        user_data);
-void            gs_app_list_filter_duplicates  (GsAppList      *list);
+void            gs_app_list_filter_duplicates  (GsAppList      *list,
+                                                GsAppListFilterFlags flags);
 void            gs_app_list_randomize          (GsAppList      *list);
 void            gs_app_list_remove_all         (GsAppList      *list);
 
diff --git a/src/gs-app-list.c b/src/gs-app-list.c
index f0858c1..238ff90 100644
--- a/src/gs-app-list.c
+++ b/src/gs-app-list.c
@@ -33,6 +33,7 @@
 
 #include <glib.h>
 
+#include "gs-app-private.h"
 #include "gs-app-list-private.h"
 
 struct _GsAppList
@@ -254,38 +255,69 @@ gs_app_list_randomize (GsAppList *list)
  * Filter any duplicate applications from the list.
  **/
 void
-gs_app_list_filter_duplicates (GsAppList *list)
+gs_app_list_filter_duplicates (GsAppList *list, GsAppListFilterFlags flags)
 {
        guint i;
        GsApp *app;
        GsApp *found;
        const gchar *id;
        g_autoptr(GHashTable) hash = NULL;
-       g_autoptr(GsAppList) old = NULL;
+       g_autoptr(GList) values = NULL;
+       GList *l;
 
        g_return_if_fail (GS_IS_APP_LIST (list));
 
-       /* deep copy to a temp list and clear the current one */
-       old = gs_app_list_copy (list);
-       gs_app_list_remove_all (list);
-
        /* create a new list with just the unique items */
-       hash = g_hash_table_new (g_str_hash, g_str_equal);
-       for (i = 0; i < old->array->len; i++) {
-               app = gs_app_list_index (old, i);
+       hash = g_hash_table_new_full (g_str_hash, g_str_equal,
+                                     g_free, (GDestroyNotify) g_object_unref);
+       for (i = 0; i < list->array->len; i++) {
+               app = gs_app_list_index (list, i);
                id = gs_app_get_id (app);
+               if (flags & GS_APP_LIST_FILTER_FLAG_PRIORITY)
+                       id = gs_app_get_id_no_prefix (app);
                if (id == NULL) {
-                       gs_app_list_add (list, app);
+                       g_autofree gchar *str = gs_app_to_string (app);
+                       g_debug ("ignoring as no application id for: %s", str);
                        continue;
                }
                found = g_hash_table_lookup (hash, id);
                if (found == NULL) {
-                       gs_app_list_add (list, app);
-                       g_hash_table_insert (hash, (gpointer) id,
-                                            GUINT_TO_POINTER (1));
+                       g_debug ("found new %s", gs_app_get_id (app));
+                       g_hash_table_insert (hash,
+                                            g_strdup (id),
+                                            g_object_ref (app));
+                       continue;
+               }
+
+               /* better? */
+               if (flags & GS_APP_LIST_FILTER_FLAG_PRIORITY) {
+                       if (gs_app_get_priority (app) >
+                           gs_app_get_priority (found)) {
+                               g_debug ("using better %s (priority %u > %u)",
+                                        gs_app_get_id (app),
+                                        gs_app_get_priority (app),
+                                        gs_app_get_priority (found));
+                               g_hash_table_insert (hash,
+                                                    g_strdup (id),
+                                                    g_object_ref (app));
+                               continue;
+                       }
+                       g_debug ("ignoring worse duplicate %s (priority %u > %u)",
+                                gs_app_get_id (app),
+                                gs_app_get_priority (app),
+                                gs_app_get_priority (found));
                        continue;
                }
-               g_debug ("ignoring duplicate %s", id);
+               g_debug ("ignoring duplicate %s", gs_app_get_id (app));
+               continue;
+       }
+
+       /* add back the best results to the existing list */
+       gs_app_list_remove_all (list);
+       values = g_hash_table_get_values (hash);
+       for (l = values; l != NULL; l = l->next) {
+               app = GS_APP (l->data);
+               gs_app_list_add (list, app);
        }
 }
 
diff --git a/src/gs-plugin-loader.c b/src/gs-plugin-loader.c
index b4f56a9..e233f54 100644
--- a/src/gs-plugin-loader.c
+++ b/src/gs-plugin-loader.c
@@ -639,9 +639,6 @@ gs_plugin_loader_run_results (GsPluginLoader *plugin_loader,
                        return NULL;
        }
 
-       /* filter package list */
-       gs_app_list_filter_duplicates (list);
-
        return g_steal_pointer (&list);
 }
 
@@ -1091,13 +1088,13 @@ gs_plugin_loader_get_updates_thread_cb (GTask *task,
                return;
        }
 
-       /* filter package list */
-       gs_app_list_filter_duplicates (state->list);
-
        /* remove any packages that are not proper applications or
         * OS updates */
        gs_app_list_filter (state->list, gs_plugin_loader_app_is_valid, state);
+
+       /* filter duplicates with priority */
        gs_app_list_filter (state->list, gs_plugin_loader_app_set_prio, plugin_loader);
+       gs_app_list_filter_duplicates (state->list, GS_APP_LIST_FILTER_FLAG_NONE);
 
        /* success */
        g_task_return_pointer (task, g_object_ref (state->list), (GDestroyNotify) g_object_unref);
@@ -1191,8 +1188,9 @@ gs_plugin_loader_get_distro_upgrades_thread_cb (GTask *task,
                return;
        }
 
-       /* filter package list */
-       gs_app_list_filter_duplicates (state->list);
+       /* filter duplicates with priority */
+       gs_app_list_filter (state->list, gs_plugin_loader_app_set_prio, plugin_loader);
+       gs_app_list_filter_duplicates (state->list, GS_APP_LIST_FILTER_FLAG_NONE);
 
        /* success */
        g_task_return_pointer (task, g_object_ref (state->list), (GDestroyNotify) g_object_unref);
@@ -1270,8 +1268,9 @@ gs_plugin_loader_get_unvoted_reviews_thread_cb (GTask *task,
                return;
        }
 
-       /* filter package list */
-       gs_app_list_filter_duplicates (state->list);
+       /* filter duplicates with priority */
+       gs_app_list_filter (state->list, gs_plugin_loader_app_set_prio, plugin_loader);
+       gs_app_list_filter_duplicates (state->list, GS_APP_LIST_FILTER_FLAG_NONE);
 
        /* success */
        g_task_return_pointer (task, g_object_ref (state->list), (GDestroyNotify) g_object_unref);
@@ -1349,8 +1348,9 @@ gs_plugin_loader_get_sources_thread_cb (GTask *task,
                return;
        }
 
-       /* filter package list */
-       gs_app_list_filter_duplicates (state->list);
+       /* filter duplicates with priority */
+       gs_app_list_filter (state->list, gs_plugin_loader_app_set_prio, plugin_loader);
+       gs_app_list_filter_duplicates (state->list, GS_APP_LIST_FILTER_FLAG_NONE);
 
        /* success */
        g_task_return_pointer (task, g_object_ref (state->list), (GDestroyNotify) g_object_unref);
@@ -1551,7 +1551,10 @@ gs_plugin_loader_get_popular_thread_cb (GTask *task,
        gs_app_list_filter (state->list, gs_plugin_loader_app_is_valid, state);
        gs_app_list_filter (state->list, gs_plugin_loader_filter_qt_for_gtk, NULL);
        gs_app_list_filter (state->list, gs_plugin_loader_get_app_is_compatible, plugin_loader);
+
+       /* filter duplicates with priority */
        gs_app_list_filter (state->list, gs_plugin_loader_app_set_prio, plugin_loader);
+       gs_app_list_filter_duplicates (state->list, GS_APP_LIST_FILTER_FLAG_PRIORITY);
 
        /* success */
        g_task_return_pointer (task, g_object_ref (state->list), (GDestroyNotify) g_object_unref);
@@ -1646,7 +1649,10 @@ gs_plugin_loader_get_featured_thread_cb (GTask *task,
                gs_app_list_filter (state->list, gs_plugin_loader_app_is_valid, state);
                gs_app_list_filter (state->list, gs_plugin_loader_get_app_is_compatible, plugin_loader);
        }
+
+       /* filter duplicates with priority */
        gs_app_list_filter (state->list, gs_plugin_loader_app_set_prio, plugin_loader);
+       gs_app_list_filter_duplicates (state->list, GS_APP_LIST_FILTER_FLAG_PRIORITY);
 
        /* success */
        g_task_return_pointer (task, g_object_ref (state->list), (GDestroyNotify) g_object_unref);
@@ -1864,11 +1870,15 @@ gs_plugin_loader_search_thread_cb (GTask *task,
        gs_plugin_loader_convert_unavailable (state->list, state->value);
 
        /* filter package list */
-       gs_app_list_filter_duplicates (state->list);
        gs_app_list_filter (state->list, gs_plugin_loader_app_is_valid, state);
        gs_app_list_filter (state->list, gs_plugin_loader_filter_qt_for_gtk, NULL);
        gs_app_list_filter (state->list, gs_plugin_loader_get_app_is_compatible, plugin_loader);
+
+       /* filter duplicates with priority */
        gs_app_list_filter (state->list, gs_plugin_loader_app_set_prio, plugin_loader);
+       gs_app_list_filter_duplicates (state->list, GS_APP_LIST_FILTER_FLAG_NONE);
+
+       /* too many */
        if (gs_app_list_length (state->list) > 500) {
                g_task_return_new_error (task,
                                         GS_PLUGIN_ERROR,
@@ -2027,12 +2037,16 @@ gs_plugin_loader_search_files_thread_cb (GTask *task,
        gs_plugin_loader_convert_unavailable (state->list, state->value);
 
        /* filter package list */
-       gs_app_list_filter_duplicates (state->list);
        gs_app_list_filter (state->list, gs_plugin_loader_app_is_valid, state);
        gs_app_list_filter (state->list, gs_plugin_loader_app_is_non_installed, NULL);
        gs_app_list_filter (state->list, gs_plugin_loader_filter_qt_for_gtk, NULL);
        gs_app_list_filter (state->list, gs_plugin_loader_get_app_is_compatible, plugin_loader);
+
+       /* filter duplicates with priority */
        gs_app_list_filter (state->list, gs_plugin_loader_app_set_prio, plugin_loader);
+       gs_app_list_filter_duplicates (state->list, GS_APP_LIST_FILTER_FLAG_NONE);
+
+       /* too many */
        if (gs_app_list_length (state->list) > 500) {
                g_task_return_new_error (task,
                                         GS_PLUGIN_ERROR,
@@ -2191,12 +2205,16 @@ gs_plugin_loader_search_what_provides_thread_cb (GTask *task,
        gs_plugin_loader_convert_unavailable (state->list, state->value);
 
        /* filter package list */
-       gs_app_list_filter_duplicates (state->list);
        gs_app_list_filter (state->list, gs_plugin_loader_app_is_valid, state);
        gs_app_list_filter (state->list, gs_plugin_loader_app_is_non_installed, NULL);
        gs_app_list_filter (state->list, gs_plugin_loader_filter_qt_for_gtk, NULL);
        gs_app_list_filter (state->list, gs_plugin_loader_get_app_is_compatible, plugin_loader);
+
+       /* filter duplicates with priority */
        gs_app_list_filter (state->list, gs_plugin_loader_app_set_prio, plugin_loader);
+       gs_app_list_filter_duplicates (state->list, GS_APP_LIST_FILTER_FLAG_NONE);
+
+       /* too many */
        if (gs_app_list_length (state->list) > 500) {
                g_task_return_new_error (task,
                                         GS_PLUGIN_ERROR,
@@ -2534,12 +2552,14 @@ gs_plugin_loader_get_category_apps_thread_cb (GTask *task,
        }
 
        /* filter package list */
-       gs_app_list_filter_duplicates (state->list);
        gs_app_list_filter (state->list, gs_plugin_loader_app_is_non_compulsory, NULL);
        gs_app_list_filter (state->list, gs_plugin_loader_app_is_valid, state);
        gs_app_list_filter (state->list, gs_plugin_loader_filter_qt_for_gtk, NULL);
        gs_app_list_filter (state->list, gs_plugin_loader_get_app_is_compatible, plugin_loader);
+
+       /* filter duplicates with priority */
        gs_app_list_filter (state->list, gs_plugin_loader_app_set_prio, plugin_loader);
+       gs_app_list_filter_duplicates (state->list, GS_APP_LIST_FILTER_FLAG_PRIORITY);
 
        /* sort, just in case the UI doesn't do this */
        gs_app_list_sort (state->list, gs_plugin_loader_app_sort_cb, NULL);
@@ -4367,7 +4387,8 @@ gs_plugin_loader_file_to_app_thread_cb (GTask *task,
        }
 
        /* filter package list */
-       gs_app_list_filter_duplicates (state->list);
+       gs_app_list_filter (state->list, gs_plugin_loader_app_set_prio, plugin_loader);
+       gs_app_list_filter_duplicates (state->list, GS_APP_LIST_FILTER_FLAG_PRIORITY);
 
        /* check the apps have an icon set */
        for (j = 0; j < gs_app_list_length (state->list); j++) {
diff --git a/src/gs-self-test.c b/src/gs-self-test.c
index f02fd2f..00c2908 100644
--- a/src/gs-self-test.c
+++ b/src/gs-self-test.c
@@ -141,9 +141,29 @@ gs_plugin_func (void)
        gs_app_set_id (app, "e");
        g_object_unref (app);
        g_assert_cmpint (gs_app_list_length (list_remove), ==, 2);
-       gs_app_list_filter_duplicates (list_remove);
+       gs_app_list_filter_duplicates (list_remove, GS_APP_LIST_FILTER_FLAG_NONE);
        g_assert_cmpint (gs_app_list_length (list_remove), ==, 1);
        g_object_unref (list_remove);
+
+       /* respect priority when deduplicating */
+       list = gs_app_list_new ();
+       app = gs_app_new ("foo:e");
+       gs_app_list_add (list, app);
+       gs_app_set_priority (app, 0);
+       g_object_unref (app);
+       app = gs_app_new ("bar:e");
+       gs_app_list_add (list, app);
+       gs_app_set_priority (app, 99);
+       g_object_unref (app);
+       app = gs_app_new ("baz:e");
+       gs_app_list_add (list, app);
+       gs_app_set_priority (app, 50);
+       g_object_unref (app);
+       g_assert_cmpint (gs_app_list_length (list), ==, 3);
+       gs_app_list_filter_duplicates (list, GS_APP_LIST_FILTER_FLAG_PRIORITY);
+       g_assert_cmpint (gs_app_list_length (list), ==, 1);
+       g_assert_cmpstr (gs_app_get_id (gs_app_list_index (list, 0)), ==, "bar:e");
+       g_object_unref (list);
 }
 
 static void