[gnome-software/wip/hughsie/no-global-cache: 8/8] Remove the concept of a shared global app cache
- From: Richard Hughes <rhughes src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gnome-software/wip/hughsie/no-global-cache: 8/8] Remove the concept of a shared global app cache
- Date: Thu, 22 Feb 2018 16:56:44 +0000 (UTC)
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");
}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]