[gnome-software/wip/hughsie/no-global-cache: 8/8] Remove the concept of a shared global app cache

[Richard Hughes <rhughes src gnome org>]

commit dc70f6800735736c65120b789fcb401ba7925099
Author: Richard Hughes <richard hughsie com>
Date:   Thu Feb 22 16:48:26 2018 +0000

    Remove the concept of a shared global app cache
    
    This was leading to far too many hard-to-debug bugs, and was a significant
    source of confusion in various parts of the codebase. Allowing one plugin to
    access a GsApp managed by another plugin was sometimes useful, but came at a
    huge cognitive cost when working out when the unique-id was "good enough" to
    add a GsApp to the shared pool.
    
    Using the _refine_wildcard() vfunc like originally intended allows us an easy
    way to 'get' a GsApp just from an optionally-globbed unique-id string.

 lib/gs-plugin-loader.c              | 40 ++++++++++++++---------
 lib/gs-plugin-private.h             |  2 --
 lib/gs-plugin.c                     | 59 +---------------------------------
 lib/gs-self-test.c                  | 37 ----------------------
 plugins/core/gs-plugin-appstream.c  |  3 --
 plugins/core/gs-self-test.c         | 63 -------------------------------------
 plugins/dummy/gs-plugin-dummy.c     |  3 --
 plugins/flatpak/gs-plugin-flatpak.c |  3 --
 plugins/snap/gs-plugin-snap.c       |  3 --
 9 files changed, 26 insertions(+), 187 deletions(-)
---
diff --git a/lib/gs-plugin-loader.c b/lib/gs-plugin-loader.c
index 6ae7a546..128dda2d 100644
--- a/lib/gs-plugin-loader.c
+++ b/lib/gs-plugin-loader.c
@@ -45,7 +45,6 @@ typedef struct
        GPtrArray               *locations;
        gchar                   *locale;
        gchar                   *language;
-       GsAppList               *global_cache;
        AsProfile               *profile;
        SoupSession             *soup_session;
        GPtrArray               *auth_array;
@@ -2173,7 +2172,6 @@ gs_plugin_loader_open_plugin (GsPluginLoader *plugin_loader,
        gs_plugin_set_locale (plugin, priv->locale);
        gs_plugin_set_language (plugin, priv->language);
        gs_plugin_set_scale (plugin, gs_plugin_loader_get_scale (plugin_loader));
-       gs_plugin_set_global_cache (plugin, priv->global_cache);
        gs_plugin_set_network_monitor (plugin, priv->network_monitor);
        g_debug ("opened plugin %s: %s", filename, gs_plugin_get_name (plugin));
 
@@ -2277,7 +2275,6 @@ gs_plugin_loader_clear_caches (GsPluginLoader *plugin_loader)
                GsPlugin *plugin = g_ptr_array_index (priv->plugins, i);
                gs_plugin_cache_invalidate (plugin);
        }
-       gs_app_list_remove_all (priv->global_cache);
 }
 
 /**
@@ -2727,7 +2724,6 @@ gs_plugin_loader_finalize (GObject *object)
        g_ptr_array_unref (priv->locations);
        g_free (priv->locale);
        g_free (priv->language);
-       g_object_unref (priv->global_cache);
        g_ptr_array_unref (priv->file_monitors);
        g_hash_table_unref (priv->events_by_id);
        g_hash_table_unref (priv->disallow_updates);
@@ -2829,7 +2825,6 @@ gs_plugin_loader_init (GsPluginLoader *plugin_loader)
        guint i;
 
        priv->scale = 1;
-       priv->global_cache = gs_app_list_new ();
        priv->plugins = g_ptr_array_new_with_free_func ((GDestroyNotify) g_object_unref);
        priv->pending_apps = g_ptr_array_new_with_free_func ((GFreeFunc) g_object_unref);
        priv->queued_ops_pool = g_thread_pool_new (gs_plugin_loader_process_in_thread_pool_cb,
@@ -3719,19 +3714,34 @@ gs_plugin_loader_get_plugin_supported (GsPluginLoader *plugin_loader,
 GsApp *
 gs_plugin_loader_app_create (GsPluginLoader *plugin_loader, const gchar *unique_id)
 {
-       GsPluginLoaderPrivate *priv = gs_plugin_loader_get_instance_private (plugin_loader);
-       GsApp *app;
-
-       /* already exists */
-       app = gs_app_list_lookup (priv->global_cache, unique_id);
-       if (app != NULL)
-               return g_object_ref (app);
+       g_autoptr(GError) error = NULL;
+       g_autoptr(GsApp) app = NULL;
+       g_autoptr(GsAppList) list = gs_app_list_new ();
+       g_autoptr(GsPluginJob) plugin_job = NULL;
+       g_autoptr(GsPluginLoaderHelper) helper = NULL;
 
-       /* create and add */
+       /* use the plugin loader to convert a wildcard app*/
        app = gs_app_new (NULL);
+       gs_app_add_quirk (app, AS_APP_QUIRK_MATCH_ANY_PREFIX);
        gs_app_set_from_unique_id (app, unique_id);
-       gs_app_list_add (priv->global_cache, app);
-       return app;
+       gs_app_list_add (list, app);
+       plugin_job = gs_plugin_job_newv (GS_PLUGIN_ACTION_REFINE, NULL);
+       helper = gs_plugin_loader_helper_new (plugin_loader, plugin_job);
+       if (!gs_plugin_loader_run_refine (helper, list, NULL, &error)) {
+               g_error ("%s", error->message);
+               return NULL;
+       }
+
+       /* return the first returned app that's not a wildcard */
+       for (guint i = 0; i < gs_app_list_length (list); i++) {
+               GsApp *app_tmp = gs_app_list_index (list, i);
+               if (!gs_app_has_quirk (app_tmp, AS_APP_QUIRK_MATCH_ANY_PREFIX))
+                       return g_object_ref (app_tmp);
+       }
+
+       /* does not exist */
+       g_warning ("failed to create an app for %s", unique_id);
+       return NULL;
 }
 
 /**
diff --git a/lib/gs-plugin-private.h b/lib/gs-plugin-private.h
index 63be809e..3ed9805f 100644
--- a/lib/gs-plugin-private.h
+++ b/lib/gs-plugin-private.h
@@ -61,8 +61,6 @@ void           gs_plugin_set_profile                  (GsPlugin       *plugin,
                                                         AsProfile      *profile);
 void            gs_plugin_set_auth_array               (GsPlugin       *plugin,
                                                         GPtrArray      *auth_array);
-void            gs_plugin_set_global_cache             (GsPlugin       *plugin,
-                                                        GsAppList      *global_cache);
 void            gs_plugin_set_running_other            (GsPlugin       *plugin,
                                                         gboolean        running_other);
 GPtrArray      *gs_plugin_get_rules                    (GsPlugin       *plugin,
diff --git a/lib/gs-plugin.c b/lib/gs-plugin.c
index 298bdc04..f3ce5d0a 100644
--- a/lib/gs-plugin.c
+++ b/lib/gs-plugin.c
@@ -69,7 +69,6 @@ typedef struct
        GsPluginData            *data;                  /* for gs-plugin-{name}.c */
        GsPluginFlags            flags;
        SoupSession             *soup_session;
-       GsAppList               *global_cache;
        GPtrArray               *rules[GS_PLUGIN_RULE_LAST];
        GHashTable              *vfuncs;                /* string:pointer */
        GMutex                   vfuncs_mutex;
@@ -229,8 +228,6 @@ gs_plugin_finalize (GObject *object)
                g_ptr_array_unref (priv->auth_array);
        if (priv->soup_session != NULL)
                g_object_unref (priv->soup_session);
-       if (priv->global_cache != NULL)
-               g_object_unref (priv->global_cache);
        if (priv->network_monitor != NULL)
                g_object_unref (priv->network_monitor);
        g_hash_table_unref (priv->cache);
@@ -791,22 +788,6 @@ gs_plugin_set_soup_session (GsPlugin *plugin, SoupSession *soup_session)
        g_set_object (&priv->soup_session, soup_session);
 }
 
-/**
- * gs_plugin_set_global_cache:
- * @plugin: a #GsPlugin
- * @global_cache: a #GsAppList
- *
- * Sets the global cache that plugins can opt to use.
- *
- * Since: 3.22
- **/
-void
-gs_plugin_set_global_cache (GsPlugin *plugin, GsAppList *global_cache)
-{
-       GsPluginPrivate *priv = gs_plugin_get_instance_private (plugin);
-       g_set_object (&priv->global_cache, global_cache);
-}
-
 /**
  * gs_plugin_set_network_monitor:
  * @plugin: a #GsPlugin
@@ -1509,17 +1490,7 @@ gs_plugin_cache_lookup (GsPlugin *plugin, const gchar *key)
        g_return_val_if_fail (key != NULL, NULL);
 
        locker = g_mutex_locker_new (&priv->cache_mutex);
-
-       /* global, so using a unique_id */
-       if (gs_plugin_has_flags (plugin, GS_PLUGIN_FLAGS_GLOBAL_CACHE)) {
-               if (!as_utils_unique_id_valid (key)) {
-                       g_critical ("key %s is not a unique_id", key);
-                       return NULL;
-               }
-               app = gs_app_list_lookup (priv->global_cache, key);
-       } else {
-               app = g_hash_table_lookup (priv->cache, key);
-       }
+       app = g_hash_table_lookup (priv->cache, key);
        if (app == NULL)
                return NULL;
        return g_object_ref (app);
@@ -1544,19 +1515,6 @@ gs_plugin_cache_remove (GsPlugin *plugin, const gchar *key)
        g_return_if_fail (key != NULL);
 
        locker = g_mutex_locker_new (&priv->cache_mutex);
-
-       /* global, so using internal unique_id */
-       if (gs_plugin_has_flags (plugin, GS_PLUGIN_FLAGS_GLOBAL_CACHE)) {
-               GsApp *app_tmp;
-               if (!as_utils_unique_id_valid (key)) {
-                       g_critical ("key %s is not a unique_id", key);
-                       return;
-               }
-               app_tmp = gs_app_list_lookup (priv->global_cache, key);
-               if (app_tmp != NULL)
-                       gs_app_list_remove (priv->global_cache, app_tmp);
-               return;
-       }
        g_hash_table_remove (priv->cache, key);
 }
 
@@ -1588,21 +1546,6 @@ gs_plugin_cache_add (GsPlugin *plugin, const gchar *key, GsApp *app)
 
        g_return_if_fail (key != NULL);
 
-       /* global, so using internal unique_id */
-       if (gs_plugin_has_flags (plugin, GS_PLUGIN_FLAGS_GLOBAL_CACHE)) {
-               if (!as_utils_unique_id_valid (key)) {
-                       g_critical ("key %s is not a unique_id", key);
-                       return;
-               }
-               if (gs_app_has_quirk (app, AS_APP_QUIRK_MATCH_ANY_PREFIX)) {
-                       g_critical ("not adding wildcard to the global cache: %s",
-                                   gs_app_get_unique_id (app));
-                       return;
-               }
-               gs_app_list_add (priv->global_cache, app);
-               return;
-       }
-
        if (g_hash_table_lookup (priv->cache, key) == app)
                return;
        g_hash_table_insert (priv->cache, g_strdup (key), g_object_ref (app));
diff --git a/lib/gs-self-test.c b/lib/gs-self-test.c
index 4e7ae1da..fb13e766 100644
--- a/lib/gs-self-test.c
+++ b/lib/gs-self-test.c
@@ -191,42 +191,6 @@ gs_plugin_download_rewrite_func (void)
        g_assert (css != NULL);
 }
 
-static void
-gs_plugin_global_cache_func (void)
-{
-       const gchar *unique_id;
-       g_autoptr(GsPlugin) plugin1 = NULL;
-       g_autoptr(GsPlugin) plugin2 = NULL;
-       g_autoptr(GsAppList) list = gs_app_list_new ();
-       g_autoptr(GsApp) app = gs_app_new ("gimp.desktop");
-       g_autoptr(GsApp) app1 = NULL;
-       g_autoptr(GsApp) app2 = NULL;
-
-       plugin1 = gs_plugin_new ();
-       gs_plugin_set_global_cache (plugin1, list);
-
-       plugin2 = gs_plugin_new ();
-       gs_plugin_set_global_cache (plugin2, list);
-
-       /* both plugins not opted into the global cache */
-       unique_id = gs_app_get_unique_id (app);
-       gs_plugin_cache_add (plugin1, unique_id, app);
-       g_assert (gs_plugin_cache_lookup (plugin2, unique_id) == NULL);
-       app1 = gs_plugin_cache_lookup (plugin1, unique_id);
-       g_assert (app1 != NULL);
-
-       /* one plugin opted in */
-       gs_plugin_add_flags (plugin1, GS_PLUGIN_FLAGS_GLOBAL_CACHE);
-       gs_plugin_cache_add (plugin1, unique_id, app);
-       g_assert (gs_plugin_cache_lookup (plugin2, unique_id) == NULL);
-
-       /* both plugins opted in */
-       gs_plugin_add_flags (plugin2, GS_PLUGIN_FLAGS_GLOBAL_CACHE);
-       gs_plugin_cache_add (plugin1, unique_id, app);
-       app2 = gs_plugin_cache_lookup (plugin2, unique_id);
-       g_assert (app2 != NULL);
-}
-
 static void
 gs_plugin_func (void)
 {
@@ -668,7 +632,6 @@ main (int argc, char **argv)
        g_test_add_func ("/gnome-software/lib/app{thread}", gs_app_thread_func);
        g_test_add_func ("/gnome-software/lib/plugin", gs_plugin_func);
        g_test_add_func ("/gnome-software/lib/plugin{download-rewrite}", gs_plugin_download_rewrite_func);
-       g_test_add_func ("/gnome-software/lib/plugin{global-cache}", gs_plugin_global_cache_func);
        g_test_add_func ("/gnome-software/lib/auth{secret}", gs_auth_secret_func);
 
        return g_test_run ();
diff --git a/plugins/core/gs-plugin-appstream.c b/plugins/core/gs-plugin-appstream.c
index bc97caeb..2c2a2da3 100644
--- a/plugins/core/gs-plugin-appstream.c
+++ b/plugins/core/gs-plugin-appstream.c
@@ -178,9 +178,6 @@ gs_plugin_initialize (GsPlugin *plugin)
                                   AS_APP_SEARCH_MATCH_KEYWORD |
                                   AS_APP_SEARCH_MATCH_ID);
 
-       /* set plugin flags */
-       gs_plugin_add_flags (plugin, GS_PLUGIN_FLAGS_GLOBAL_CACHE);
-
        /* need package name */
        gs_plugin_add_rule (plugin, GS_PLUGIN_RULE_RUN_AFTER, "dpkg");
 
diff --git a/plugins/core/gs-self-test.c b/plugins/core/gs-self-test.c
index 8a438659..66ab4fd2 100644
--- a/plugins/core/gs-self-test.c
+++ b/plugins/core/gs-self-test.c
@@ -26,66 +26,6 @@
 #include "gs-appstream.h"
 #include "gs-test.h"
 
-static void
-gs_plugins_core_app_creation_func (GsPluginLoader *plugin_loader)
-{
-       AsApp *as_app = NULL;
-       GsPlugin *plugin;
-       gboolean ret;
-       g_autoptr(GsApp) app = NULL;
-       g_autoptr(GsApp) app2 = NULL;
-       g_autoptr(GsApp) cached_app = NULL;
-       g_autoptr(GsApp) cached_app2 = NULL;
-       g_autoptr(AsStore) store = NULL;
-       g_autoptr(GError) error = NULL;
-       const gchar *test_icon_root = g_getenv ("GS_SELF_TEST_APPSTREAM_ICON_ROOT");
-       g_autofree gchar *xml = g_strdup ("<?xml version=\"1.0\"?>\n"
-                                         "<components version=\"0.9\">\n"
-                                         "  <component type=\"desktop\">\n"
-                                         "    <id>demeter.desktop</id>\n"
-                                         "    <name>Demeter</name>\n"
-                                         "    <summary>An agriculture application</summary>\n"
-                                         "  </component>\n"
-                                         "</components>\n");
-
-       /* drop all caches */
-       gs_plugin_loader_setup_again (plugin_loader);
-
-       app = gs_plugin_loader_app_create (plugin_loader,
-                                          "*/*/*/desktop/demeter.desktop/*");
-       gs_app_add_quirk (app, AS_APP_QUIRK_MATCH_ANY_PREFIX);
-
-       cached_app = gs_plugin_loader_app_create (plugin_loader,
-                                                 "*/*/*/desktop/demeter.desktop/*");
-
-       g_assert (app == cached_app);
-
-       /* Make sure the app still has the match-any-prefix quirk*/
-       g_assert(gs_app_has_quirk (app, AS_APP_QUIRK_MATCH_ANY_PREFIX));
-
-       /* Ensure gs_appstream creates a new app when a matching one is cached but
-        * has the match-any-prefix quirk */
-       store = as_store_new ();
-       ret = as_store_from_xml (store, xml, test_icon_root, &error);
-       g_assert_no_error (error);
-       g_assert (ret);
-
-       as_app = as_store_get_app_by_id (store, "demeter.desktop");
-       g_assert (as_app != NULL);
-
-       plugin = gs_plugin_loader_find_plugin (plugin_loader, "appstream");
-       g_assert (plugin != NULL);
-
-       app2 = gs_appstream_create_app (plugin, as_app, NULL);
-       g_assert (app2 != NULL);
-       g_assert (cached_app != app2);
-       g_assert (!gs_app_has_quirk (app2, AS_APP_QUIRK_MATCH_ANY_PREFIX));
-
-       cached_app2 = gs_plugin_loader_app_create (plugin_loader,
-                                                  "*/*/*/desktop/demeter.desktop/*");
-       g_assert (cached_app2 == app2);
-}
-
 static void
 gs_plugins_core_search_repo_name_func (GsPluginLoader *plugin_loader)
 {
@@ -237,9 +177,6 @@ main (int argc, char **argv)
        g_test_add_data_func ("/gnome-software/plugins/core/search-repo-name",
                              plugin_loader,
                              (GTestDataFunc) gs_plugins_core_search_repo_name_func);
-       g_test_add_data_func ("/gnome-software/plugins/core/app-creation",
-                             plugin_loader,
-                             (GTestDataFunc) gs_plugins_core_app_creation_func);
        g_test_add_data_func ("/gnome-software/plugins/core/os-release",
                              plugin_loader,
                              (GTestDataFunc) gs_plugins_core_os_release_func);
diff --git a/plugins/dummy/gs-plugin-dummy.c b/plugins/dummy/gs-plugin-dummy.c
index fbd322cf..c338b0e6 100644
--- a/plugins/dummy/gs-plugin-dummy.c
+++ b/plugins/dummy/gs-plugin-dummy.c
@@ -61,9 +61,6 @@ gs_plugin_initialize (GsPlugin *plugin)
                return;
        }
 
-       /* set plugin flags */
-       gs_plugin_add_flags (plugin, GS_PLUGIN_FLAGS_GLOBAL_CACHE);
-
        /* toggle this */
        if (g_getenv ("GS_SELF_TEST_TOGGLE_ALLOW_UPDATES") != NULL) {
                priv->allow_updates_id = g_timeout_add_seconds (10,
diff --git a/plugins/flatpak/gs-plugin-flatpak.c b/plugins/flatpak/gs-plugin-flatpak.c
index 572d6314..8b629905 100644
--- a/plugins/flatpak/gs-plugin-flatpak.c
+++ b/plugins/flatpak/gs-plugin-flatpak.c
@@ -57,9 +57,6 @@ gs_plugin_initialize (GsPlugin *plugin)
        gs_plugin_add_rule (plugin, GS_PLUGIN_RULE_CONFLICTS, "flatpak-system");
        gs_plugin_add_rule (plugin, GS_PLUGIN_RULE_CONFLICTS, "flatpak-user");
 
-       /* set plugin flags */
-       gs_plugin_add_flags (plugin, GS_PLUGIN_FLAGS_GLOBAL_CACHE);
-
        /* getting app properties from appstream is quicker */
        gs_plugin_add_rule (plugin, GS_PLUGIN_RULE_RUN_AFTER, "appstream");
 
diff --git a/plugins/snap/gs-plugin-snap.c b/plugins/snap/gs-plugin-snap.c
index c67bd9f9..5591d6ee 100644
--- a/plugins/snap/gs-plugin-snap.c
+++ b/plugins/snap/gs-plugin-snap.c
@@ -90,9 +90,6 @@ gs_plugin_initialize (GsPlugin *plugin)
        /* Override hardcoded popular apps */
        gs_plugin_add_rule (plugin, GS_PLUGIN_RULE_RUN_BEFORE, "hardcoded-popular");
 
-       /* set plugin flags */
-       gs_plugin_add_flags (plugin, GS_PLUGIN_FLAGS_GLOBAL_CACHE);
-
        /* set name of MetaInfo file */
        gs_plugin_set_appstream_id (plugin, "org.gnome.Software.Plugin.Snap");
 }