[gnome-software/gnome-3-20] Do not set invalid application states on error
- From: Richard Hughes <rhughes src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gnome-software/gnome-3-20] Do not set invalid application states on error
- Date: Mon, 18 Apr 2016 09:47:53 +0000 (UTC)
commit 49713dcaa97e5fb0e772b0e470cf8703257e8790
Author: Richard Hughes <richard hughsie com>
Date: Sun Apr 17 11:32:40 2016 +0100
Do not set invalid application states on error
Now plugins can fail without stopping the loader we have to make them
responsible for setting the correct non-transient states when completed.
Provide a helper called gs_app_set_state_recover() which can be used to un-set
any transient states in the error handler.
src/gs-app.c | 27 ++++++++++++
src/gs-app.h | 1 +
src/gs-plugin-loader.c | 60 +++++++++------------------
src/gs-self-test.c | 41 ++++++++++++++++++
src/plugins/gs-plugin-dummy.c | 51 +++++++++++++++++++++++
src/plugins/gs-plugin-fedora-tagger-usage.c | 4 ++
src/plugins/gs-plugin-fwupd.c | 4 +-
src/plugins/gs-plugin-packagekit.c | 37 ++++++++++++++---
src/plugins/gs-plugin-xdg-app.c | 41 +++++++++++++-----
9 files changed, 208 insertions(+), 58 deletions(-)
---
diff --git a/src/gs-app.c b/src/gs-app.c
index f6df7c1..8c53514 100644
--- a/src/gs-app.c
+++ b/src/gs-app.c
@@ -89,6 +89,7 @@ struct _GsApp
guint64 size;
AsAppKind kind;
AsAppState state;
+ AsAppState state_recover;
guint progress;
GHashTable *metadata;
GdkPixbuf *pixbuf;
@@ -436,6 +437,19 @@ gs_app_get_progress (GsApp *app)
}
/**
+ * gs_app_set_state_recover:
+ *
+ * Sets the application state to the last status value that was not
+ * transient.
+ */
+void
+gs_app_set_state_recover (GsApp *app)
+{
+ if (app->state_recover != AS_APP_STATE_UNKNOWN)
+ gs_app_set_state (app, app->state_recover);
+}
+
+/**
* gs_app_set_state_internal:
*/
static gboolean
@@ -540,6 +554,19 @@ gs_app_set_state_internal (GsApp *app, AsAppState state)
state == AS_APP_STATE_AVAILABLE)
app->install_date = 0;
+ /* save this to simplify error handling in the plugins */
+ switch (state) {
+ case AS_APP_STATE_INSTALLING:
+ case AS_APP_STATE_REMOVING:
+ /* transient, so ignore */
+ break;
+ default:
+ g_debug ("non-transient state now %s",
+ as_app_state_to_string (state));
+ app->state_recover = state;
+ break;
+ }
+
return TRUE;
}
diff --git a/src/gs-app.h b/src/gs-app.h
index 79becf7..bab93c3 100644
--- a/src/gs-app.h
+++ b/src/gs-app.h
@@ -99,6 +99,7 @@ void gs_app_set_kind (GsApp *app,
AsAppState gs_app_get_state (GsApp *app);
void gs_app_set_state (GsApp *app,
AsAppState state);
+void gs_app_set_state_recover (GsApp *app);
guint gs_app_get_progress (GsApp *app);
void gs_app_set_progress (GsApp *app,
guint percentage);
diff --git a/src/gs-plugin-loader.c b/src/gs-plugin-loader.c
index 8d63855..c1ac319 100644
--- a/src/gs-plugin-loader.c
+++ b/src/gs-plugin-loader.c
@@ -74,8 +74,6 @@ typedef struct {
GsCategory *category;
GsApp *app;
GsReview *review;
- AsAppState state_success;
- AsAppState state_failure;
} GsPluginLoaderAsyncState;
static void
@@ -2319,16 +2317,12 @@ gs_plugin_loader_app_action_thread_cb (GTask *task,
cancellable,
&error);
if (ret) {
- if (state->state_success != AS_APP_STATE_UNKNOWN) {
- gs_app_set_state (state->app, state->state_success);
- addons = gs_app_get_addons (state->app);
- for (i = 0; i < addons->len; i++) {
- GsApp *addon = g_ptr_array_index (addons, i);
- if (gs_app_get_to_be_installed (addon)) {
- gs_app_set_state (addon, state->state_success);
- gs_app_set_to_be_installed (addon, FALSE);
- }
- }
+ /* unstage addons */
+ addons = gs_app_get_addons (state->app);
+ for (i = 0; i < addons->len; i++) {
+ GsApp *addon = g_ptr_array_index (addons, i);
+ if (gs_app_get_to_be_installed (addon))
+ gs_app_set_to_be_installed (addon, FALSE);
}
/* refine again to make sure we pick up new source id */
@@ -2345,20 +2339,22 @@ gs_plugin_loader_app_action_thread_cb (GTask *task,
g_task_return_error (task, error);
}
} else {
- if (state->state_failure != AS_APP_STATE_UNKNOWN) {
- gs_app_set_state (state->app, state->state_failure);
- addons = gs_app_get_addons (state->app);
- for (i = 0; i < addons->len; i++) {
- GsApp *addon = g_ptr_array_index (addons, i);
- if (gs_app_get_to_be_installed (addon)) {
- gs_app_set_state (addon, state->state_failure);
- gs_app_set_to_be_installed (addon, FALSE);
- }
- }
- }
g_task_return_error (task, error);
}
+ /* check the app is not still in an action state */
+ switch (gs_app_get_state (state->app)) {
+ case AS_APP_STATE_INSTALLING:
+ case AS_APP_STATE_REMOVING:
+ g_warning ("application %s left in %s state",
+ gs_app_get_id (state->app),
+ as_app_state_to_string (gs_app_get_state (state->app)));
+ gs_app_set_state (state->app, AS_APP_STATE_UNKNOWN);
+ break;
+ default:
+ break;
+ }
+
/* remove from list */
g_mutex_lock (&priv->pending_apps_mutex);
g_ptr_array_remove (priv->pending_apps, state->app);
@@ -2635,43 +2631,27 @@ gs_plugin_loader_app_action_async (GsPluginLoader *plugin_loader,
switch (action) {
case GS_PLUGIN_LOADER_ACTION_INSTALL:
state->function_name = "gs_plugin_app_install";
- state->state_success = AS_APP_STATE_INSTALLED;
- state->state_failure = AS_APP_STATE_AVAILABLE;
break;
case GS_PLUGIN_LOADER_ACTION_REMOVE:
state->function_name = "gs_plugin_app_remove";
- state->state_success = AS_APP_STATE_AVAILABLE;
- state->state_failure = AS_APP_STATE_INSTALLED;
break;
case GS_PLUGIN_LOADER_ACTION_SET_RATING:
state->function_name = "gs_plugin_app_set_rating";
- state->state_success = AS_APP_STATE_UNKNOWN;
- state->state_failure = AS_APP_STATE_UNKNOWN;
break;
case GS_PLUGIN_LOADER_ACTION_UPDATE:
state->function_name = "gs_plugin_app_update";
- state->state_success = AS_APP_STATE_INSTALLED;
- state->state_failure = AS_APP_STATE_UPDATABLE_LIVE;
break;
case GS_PLUGIN_LOADER_ACTION_UPGRADE_DOWNLOAD:
state->function_name = "gs_plugin_app_upgrade_download";
- state->state_success = AS_APP_STATE_UPDATABLE;
- state->state_failure = AS_APP_STATE_AVAILABLE;
break;
case GS_PLUGIN_LOADER_ACTION_UPGRADE_TRIGGER:
state->function_name = "gs_plugin_app_upgrade_trigger";
- state->state_success = AS_APP_STATE_UNKNOWN;
- state->state_failure = AS_APP_STATE_UNKNOWN;
break;
case GS_PLUGIN_LOADER_ACTION_LAUNCH:
state->function_name = "gs_plugin_launch";
- state->state_success = AS_APP_STATE_UNKNOWN;
- state->state_failure = AS_APP_STATE_UNKNOWN;
break;
case GS_PLUGIN_LOADER_ACTION_OFFLINE_UPDATE_CANCEL:
state->function_name = "gs_plugin_offline_update_cancel";
- state->state_success = AS_APP_STATE_UNKNOWN;
- state->state_failure = AS_APP_STATE_UNKNOWN;
break;
default:
g_assert_not_reached ();
@@ -3444,7 +3424,7 @@ gs_plugin_loader_set_network_status (GsPluginLoader *plugin_loader,
if (priv->online == online)
return;
- g_debug ("*** Network status change: %s", online ? "online" : "offline");
+ g_debug ("network status change: %s", online ? "online" : "offline");
priv->online = online;
diff --git a/src/gs-self-test.c b/src/gs-self-test.c
index d058e78..154907e 100644
--- a/src/gs-self-test.c
+++ b/src/gs-self-test.c
@@ -144,6 +144,14 @@ gs_app_func (void)
g_assert_cmpstr (gs_app_get_name (app), ==, "dave");
gs_app_set_name (app, GS_APP_QUALITY_HIGHEST, "hugh");
g_assert_cmpstr (gs_app_get_name (app), ==, "hugh");
+
+ /* check non-transient state saving */
+ gs_app_set_state (app, AS_APP_STATE_INSTALLED);
+ g_assert_cmpint (gs_app_get_state (app), ==, AS_APP_STATE_INSTALLED);
+ gs_app_set_state (app, AS_APP_STATE_REMOVING);
+ g_assert_cmpint (gs_app_get_state (app), ==, AS_APP_STATE_REMOVING);
+ gs_app_set_state_recover (app); // simulate an error
+ g_assert_cmpint (gs_app_get_state (app), ==, AS_APP_STATE_INSTALLED);
}
static guint _status_changed_cnt = 0;
@@ -157,6 +165,35 @@ gs_plugin_loader_status_changed_cb (GsPluginLoader *plugin_loader,
_status_changed_cnt++;
}
+static void
+gs_plugin_loader_install_func (GsPluginLoader *plugin_loader)
+{
+ gboolean ret;
+ g_autoptr(GsApp) app = NULL;
+ g_autoptr(GError) error = NULL;
+
+ /* install */
+ app = gs_app_new ("chiron.desktop");
+ gs_app_set_management_plugin (app, "dummy");
+ gs_app_set_state (app, AS_APP_STATE_AVAILABLE);
+ ret = gs_plugin_loader_app_action (plugin_loader, app,
+ GS_PLUGIN_LOADER_ACTION_INSTALL,
+ NULL,
+ &error);
+ g_assert_no_error (error);
+ g_assert (ret);
+ g_assert_cmpint (gs_app_get_state (app), ==, AS_APP_STATE_INSTALLED);
+
+ /* remove -- we're really testing for return code UNKNOWN,
+ * but dummy::refine() sets it */
+ ret = gs_plugin_loader_app_action (plugin_loader, app,
+ GS_PLUGIN_LOADER_ACTION_REMOVE,
+ NULL,
+ &error);
+ g_assert_no_error (error);
+ g_assert (ret);
+ g_assert_cmpint (gs_app_get_state (app), ==, AS_APP_STATE_INSTALLED);
+}
static void
gs_plugin_loader_refine_func (GsPluginLoader *plugin_loader)
@@ -456,6 +493,7 @@ main (int argc, char **argv)
/* we can only load this once per process */
plugin_loader = gs_plugin_loader_new ();
+ gs_plugin_loader_set_network_status (plugin_loader, TRUE);
g_signal_connect (plugin_loader, "status-changed",
G_CALLBACK (gs_plugin_loader_status_changed_cb), NULL);
gs_plugin_loader_set_location (plugin_loader, "./plugins/.libs");
@@ -474,6 +512,9 @@ main (int argc, char **argv)
g_test_add_data_func ("/gnome-software/plugin-loader{search}",
plugin_loader,
(GTestDataFunc) gs_plugin_loader_search_func);
+ g_test_add_data_func ("/gnome-software/plugin-loader{install}",
+ plugin_loader,
+ (GTestDataFunc) gs_plugin_loader_install_func);
g_test_add_data_func ("/gnome-software/plugin-loader{installed}",
plugin_loader,
(GTestDataFunc) gs_plugin_loader_installed_func);
diff --git a/src/plugins/gs-plugin-dummy.c b/src/plugins/gs-plugin-dummy.c
index 91a93a6..4e10b4f 100644
--- a/src/plugins/gs-plugin-dummy.c
+++ b/src/plugins/gs-plugin-dummy.c
@@ -234,6 +234,56 @@ gs_plugin_add_installed (GsPlugin *plugin,
}
/**
+ * gs_plugin_app_remove:
+ */
+gboolean
+gs_plugin_app_remove (GsPlugin *plugin,
+ GsApp *app,
+ GCancellable *cancellable,
+ GError **error)
+{
+ /* only process this app if was created by this plugin */
+ if (g_strcmp0 (gs_app_get_management_plugin (app), plugin->name) != 0)
+ return TRUE;
+
+ /* remove app */
+ if (g_strcmp0 (gs_app_get_id (app), "chiron.desktop") == 0) {
+ gs_app_set_state (app, AS_APP_STATE_REMOVING);
+ if (!gs_plugin_dummy_delay (plugin, app, 500, cancellable, error)) {
+ gs_app_set_state_recover (app);
+ return FALSE;
+ }
+ gs_app_set_state (app, AS_APP_STATE_UNKNOWN);
+ }
+ return TRUE;
+}
+
+/**
+ * gs_plugin_app_install:
+ */
+gboolean
+gs_plugin_app_install (GsPlugin *plugin,
+ GsApp *app,
+ GCancellable *cancellable,
+ GError **error)
+{
+ /* only process this app if was created by this plugin */
+ if (g_strcmp0 (gs_app_get_management_plugin (app), plugin->name) != 0)
+ return TRUE;
+
+ /* install app */
+ if (g_strcmp0 (gs_app_get_id (app), "chiron.desktop") == 0) {
+ gs_app_set_state (app, AS_APP_STATE_INSTALLING);
+ if (!gs_plugin_dummy_delay (plugin, app, 500, cancellable, error)) {
+ gs_app_set_state_recover (app);
+ return FALSE;
+ }
+ gs_app_set_state (app, AS_APP_STATE_INSTALLED);
+ }
+ return TRUE;
+}
+
+/**
* gs_plugin_refine_app:
*/
gboolean
@@ -406,6 +456,7 @@ gs_plugin_app_upgrade_download (GsPlugin *plugin, GsApp *app,
gs_app_set_state (app, AS_APP_STATE_INSTALLING);
if (!gs_plugin_dummy_delay (plugin, app, 5000, cancellable, error))
return FALSE;
+ gs_app_set_state (app, AS_APP_STATE_UPDATABLE);
return TRUE;
}
diff --git a/src/plugins/gs-plugin-fedora-tagger-usage.c b/src/plugins/gs-plugin-fedora-tagger-usage.c
index a71f8a5..e2665ab 100644
--- a/src/plugins/gs-plugin-fedora-tagger-usage.c
+++ b/src/plugins/gs-plugin-fedora-tagger-usage.c
@@ -164,6 +164,8 @@ gs_plugin_app_install (GsPlugin *plugin,
GCancellable *cancellable,
GError **error)
{
+ if (gs_app_get_state (app) != AS_APP_STATE_INSTALLED)
+ return TRUE;
return gs_plugin_app_set_usage_app (plugin, app, TRUE, error);
}
@@ -176,5 +178,7 @@ gs_plugin_app_remove (GsPlugin *plugin,
GCancellable *cancellable,
GError **error)
{
+ if (gs_app_get_state (app) != AS_APP_STATE_AVAILABLE)
+ return TRUE;
return gs_plugin_app_set_usage_app (plugin, app, FALSE, error);
}
diff --git a/src/plugins/gs-plugin-fwupd.c b/src/plugins/gs-plugin-fwupd.c
index 65c39e6..ff4b75a 100644
--- a/src/plugins/gs-plugin-fwupd.c
+++ b/src/plugins/gs-plugin-fwupd.c
@@ -927,8 +927,10 @@ gs_plugin_fwupd_install (GsPlugin *plugin,
gs_app_set_state (app, AS_APP_STATE_INSTALLING);
if (!gs_plugin_fwupd_upgrade (plugin, filename, FWUPD_DEVICE_ID_ANY, offline,
- cancellable, error))
+ cancellable, error)) {
+ gs_app_set_state_recover (app);
return FALSE;
+ }
gs_app_set_state (app, AS_APP_STATE_INSTALLED);
return TRUE;
}
diff --git a/src/plugins/gs-plugin-packagekit.c b/src/plugins/gs-plugin-packagekit.c
index 1d9735a..b301d8b 100644
--- a/src/plugins/gs-plugin-packagekit.c
+++ b/src/plugins/gs-plugin-packagekit.c
@@ -364,13 +364,15 @@ gs_plugin_app_install (GsPlugin *plugin,
gs_plugin_packagekit_progress_cb, &data,
error);
if (results == NULL) {
- gs_app_set_state (app, AS_APP_STATE_AVAILABLE);
+ gs_app_set_state_recover (app);
return FALSE;
}
+ /* state is known */
+ gs_app_set_state (app, AS_APP_STATE_INSTALLED);
+
/* no longer valid */
gs_app_clear_source_ids (app);
- gs_app_set_state (app, AS_APP_STATE_INSTALLED);
return TRUE;
}
@@ -430,8 +432,14 @@ gs_plugin_app_install (GsPlugin *plugin,
cancellable,
gs_plugin_packagekit_progress_cb, &data,
error);
- if (results == NULL)
+ if (results == NULL) {
+ gs_app_set_state_recover (app);
return FALSE;
+ }
+
+ /* state is known */
+ gs_app_set_state (app, AS_APP_STATE_INSTALLED);
+
break;
case AS_APP_STATE_AVAILABLE_LOCAL:
package_id = gs_app_get_metadata_item (app, "packagekit::local-filename");
@@ -449,8 +457,13 @@ gs_plugin_app_install (GsPlugin *plugin,
cancellable,
gs_plugin_packagekit_progress_cb, &data,
error);
- if (results == NULL)
+ if (results == NULL) {
+ gs_app_set_state_recover (app);
return FALSE;
+ }
+
+ /* state is known */
+ gs_app_set_state (app, AS_APP_STATE_INSTALLED);
/* get the new icon from the package */
gs_app_set_metadata (app, "packagekit::local-filename", NULL);
@@ -610,8 +623,13 @@ gs_plugin_app_remove (GsPlugin *plugin,
cancellable,
gs_plugin_packagekit_progress_cb, &data,
error);
- if (results == NULL)
+ if (results == NULL) {
+ gs_app_set_state_recover (app);
return FALSE;
+ }
+
+ /* state is not known: we don't know if we can re-install this app */
+ gs_app_set_state (app, AS_APP_STATE_UNKNOWN);
/* no longer valid */
gs_app_clear_source_ids (app);
@@ -669,7 +687,14 @@ gs_plugin_app_upgrade_download (GsPlugin *plugin,
cancellable,
gs_plugin_packagekit_progress_cb, &data,
error);
- return results != NULL;
+ if (results == NULL) {
+ gs_app_set_state_recover (app);
+ return FALSE;
+ }
+
+ /* state is known */
+ gs_app_set_state (app, AS_APP_STATE_UPDATABLE);
+ return TRUE;
}
/**
diff --git a/src/plugins/gs-plugin-xdg-app.c b/src/plugins/gs-plugin-xdg-app.c
index 340c4b2..ddc9ead 100644
--- a/src/plugins/gs-plugin-xdg-app.c
+++ b/src/plugins/gs-plugin-xdg-app.c
@@ -1208,13 +1208,20 @@ gs_plugin_app_remove (GsPlugin *plugin,
/* remove */
gs_app_set_state (app, AS_APP_STATE_REMOVING);
- return xdg_app_installation_uninstall (plugin->priv->installation,
- XDG_APP_REF_KIND_APP,
- gs_app_get_xdgapp_name (app),
- gs_app_get_xdgapp_arch (app),
- gs_app_get_xdgapp_branch (app),
- gs_plugin_xdg_app_progress_cb, &helper,
- cancellable, error);
+ if (!xdg_app_installation_uninstall (plugin->priv->installation,
+ XDG_APP_REF_KIND_APP,
+ gs_app_get_xdgapp_name (app),
+ gs_app_get_xdgapp_arch (app),
+ gs_app_get_xdgapp_branch (app),
+ gs_plugin_xdg_app_progress_cb, &helper,
+ cancellable, error)) {
+ gs_app_set_state_recover (app);
+ return FALSE;
+ }
+
+ /* state is not known: we don't know if we can re-install this app */
+ gs_app_set_state (app, AS_APP_STATE_UNKNOWN);
+ return TRUE;
}
/**
@@ -1279,7 +1286,7 @@ gs_plugin_app_install (GsPlugin *plugin,
gs_plugin_xdg_app_progress_cb, &helper,
cancellable, error);
if (xref == NULL) {
- gs_app_set_state (runtime, AS_APP_STATE_AVAILABLE);
+ gs_app_set_state_recover (runtime);
return FALSE;
}
gs_app_set_state (runtime, AS_APP_STATE_INSTALLED);
@@ -1309,9 +1316,14 @@ gs_plugin_app_install (GsPlugin *plugin,
gs_plugin_xdg_app_progress_cb, &helper,
cancellable, error);
}
+ if (xref == NULL) {
+ gs_app_set_state_recover (app);
+ return FALSE;
+ }
- /* now the main application */
- return xref != NULL;
+ /* state is known */
+ gs_app_set_state (app, AS_APP_STATE_INSTALLED);
+ return TRUE;
}
/**
@@ -1346,7 +1358,14 @@ gs_plugin_app_update (GsPlugin *plugin,
gs_app_get_xdgapp_branch (app),
gs_plugin_xdg_app_progress_cb, &helper,
cancellable, error);
- return xref != NULL;
+ if (xref == NULL) {
+ gs_app_set_state_recover (app);
+ return FALSE;
+ }
+
+ /* state is known */
+ gs_app_set_state (app, AS_APP_STATE_INSTALLED);
+ return TRUE;
}
/**
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
