[gnome-software/630-flatpak-updates-are-stuck-in-installing-state-when-flatpak-update-was-already-done-outside: 2/2] flatpak: Refresh UI after changes made outside of Software

[Milan Crha <mcrha src gnome org>]

commit 0af09a109c7a99a03361c299ae6ffcd7b2dac497
Author: Milan Crha <mcrha redhat com>
Date:   Wed Mar 3 12:18:38 2021 +0100

    flatpak: Refresh UI after changes made outside of Software
    
    When the flatpak installation monitor claims it changed in the background,
    the memory plugin cache of the GsApp should be discarded and the information
    about installed/to-be-updated applications refreshed as well. The UI should
    be reloaded, to reflect the actual state, instead of showing stale data, aka
    the GsApp, which had been in the cache.
    
    Closes https://gitlab.gnome.org/GNOME/gnome-software/-/issues/630

 plugins/flatpak/gs-flatpak-transaction.c | 11 +++++--
 plugins/flatpak/gs-flatpak.c             | 45 +++++++++++++++++-----------
 plugins/flatpak/gs-self-test.c           | 51 +++++++++++++++++++++++++-------
 3 files changed, 76 insertions(+), 31 deletions(-)
---
diff --git a/plugins/flatpak/gs-flatpak-transaction.c b/plugins/flatpak/gs-flatpak-transaction.c
index 28dd912b5..08baef74b 100644
--- a/plugins/flatpak/gs-flatpak-transaction.c
+++ b/plugins/flatpak/gs-flatpak-transaction.c
@@ -175,9 +175,13 @@ _transaction_ready (FlatpakTransaction *transaction)
                        _transaction_operation_set_app (op, app);
                        /* if we're updating a component, then mark all the apps
                         * involved to ensure updating the button state */
-                       if (flatpak_transaction_operation_get_operation_type (op) ==
-                                       FLATPAK_TRANSACTION_OPERATION_UPDATE)
+                       if (flatpak_transaction_operation_get_operation_type (op) == 
FLATPAK_TRANSACTION_OPERATION_UPDATE) {
+                               if (gs_app_get_state (app) == GS_APP_STATE_UNKNOWN ||
+                                   gs_app_get_state (app) == GS_APP_STATE_INSTALLED)
+                                       gs_app_set_state (app, GS_APP_STATE_UPDATABLE_LIVE);
+
                                gs_app_set_state (app, GS_APP_STATE_INSTALLING);
+                       }
                }
 
 #if FLATPAK_CHECK_VERSION(1, 7, 3)
@@ -516,7 +520,8 @@ _transaction_new_operation (FlatpakTransaction *transaction,
                gs_app_set_state (app, GS_APP_STATE_INSTALLING);
                break;
        case FLATPAK_TRANSACTION_OPERATION_UPDATE:
-               if (gs_app_get_state (app) == GS_APP_STATE_UNKNOWN)
+               if (gs_app_get_state (app) == GS_APP_STATE_UNKNOWN ||
+                   gs_app_get_state (app) == GS_APP_STATE_INSTALLED)
                        gs_app_set_state (app, GS_APP_STATE_UPDATABLE_LIVE);
                gs_app_set_state (app, GS_APP_STATE_INSTALLING);
                break;
diff --git a/plugins/flatpak/gs-flatpak.c b/plugins/flatpak/gs-flatpak.c
index 128ba46b7..e1c407862 100644
--- a/plugins/flatpak/gs-flatpak.c
+++ b/plugins/flatpak/gs-flatpak.c
@@ -412,12 +412,15 @@ gs_plugin_flatpak_changed_cb (GFileMonitor *monitor,
        /* drop the installed refs cache */
        locker = g_mutex_locker_new (&self->installed_refs_mutex);
        g_clear_pointer (&self->installed_refs, g_ptr_array_unref);
-
        g_clear_pointer (&locker, g_mutex_locker_free);
 
        /* drop the remote title cache */
        locker = g_mutex_locker_new (&self->remote_title_mutex);
        g_hash_table_remove_all (self->remote_title);
+       g_clear_pointer (&locker, g_mutex_locker_free);
+
+       gs_plugin_cache_invalidate (self->plugin);
+       gs_plugin_reload (self->plugin);
 }
 
 static gboolean
@@ -1344,25 +1347,33 @@ gs_flatpak_ref_to_app (GsFlatpak *self, const gchar *ref,
                       GCancellable *cancellable, GError **error)
 {
        g_autoptr(GPtrArray) xremotes = NULL;
-       g_autoptr(GPtrArray) xrefs = NULL;
 
        g_return_val_if_fail (ref != NULL, NULL);
 
-       /* get all the installed apps (no network I/O) */
-       xrefs = flatpak_installation_list_installed_refs (self->installation,
-                                                         cancellable,
-                                                         error);
-       if (xrefs == NULL) {
-               gs_flatpak_error_convert (error);
-               return NULL;
+       g_mutex_lock (&self->installed_refs_mutex);
+
+       if (self->installed_refs == NULL) {
+               self->installed_refs = flatpak_installation_list_installed_refs (self->installation,
+                                                                cancellable, error);
+
+               if (self->installed_refs == NULL) {
+                       g_mutex_unlock (&self->installed_refs_mutex);
+                       gs_flatpak_error_convert (error);
+                       return NULL;
+               }
        }
-       for (guint i = 0; i < xrefs->len; i++) {
-               FlatpakInstalledRef *xref = g_ptr_array_index (xrefs, i);
+
+       for (guint i = 0; i < self->installed_refs->len; i++) {
+               FlatpakInstalledRef *xref = g_ptr_array_index (self->installed_refs, i);
                g_autofree gchar *ref_tmp = flatpak_ref_format_ref (FLATPAK_REF (xref));
-               if (g_strcmp0 (ref, ref_tmp) == 0)
+               if (g_strcmp0 (ref, ref_tmp) == 0) {
+                       g_mutex_unlock (&self->installed_refs_mutex);
                        return gs_flatpak_create_installed (self, xref, NULL, cancellable);
+               }
        }
 
+       g_mutex_unlock (&self->installed_refs_mutex);
+
        /* look at each remote xref */
        xremotes = flatpak_installation_list_remotes (self->installation,
                                                      cancellable, error);
@@ -1900,6 +1911,10 @@ gs_flatpak_refine_app_state_unlocked (GsFlatpak *self,
        if (!gs_refine_item_metadata (self, app, cancellable, error))
                return FALSE;
 
+       /* ensure origin set */
+       if (!gs_plugin_refine_item_origin (self, app, cancellable, error))
+               return FALSE;
+
        /* find the app using the origin and the ID */
        g_mutex_lock (&self->installed_refs_mutex);
 
@@ -1915,7 +1930,6 @@ gs_flatpak_refine_app_state_unlocked (GsFlatpak *self,
        }
 
        installed_refs = g_ptr_array_ref (self->installed_refs);
-       g_mutex_unlock (&self->installed_refs_mutex);
 
        for (guint i = 0; i < installed_refs->len; i++) {
                FlatpakInstalledRef *ref_tmp = g_ptr_array_index (installed_refs, i);
@@ -1931,6 +1945,7 @@ gs_flatpak_refine_app_state_unlocked (GsFlatpak *self,
                        break;
                }
        }
+       g_mutex_unlock (&self->installed_refs_mutex);
        if (ref != NULL) {
                g_debug ("marking %s as installed with flatpak",
                         gs_app_get_unique_id (app));
@@ -1949,10 +1964,6 @@ gs_flatpak_refine_app_state_unlocked (GsFlatpak *self,
                return TRUE;
        }
 
-       /* ensure origin set */
-       if (!gs_plugin_refine_item_origin (self, app, cancellable, error))
-               return FALSE;
-
        /* anything not installed just check the remote is still present */
        if (gs_app_get_state (app) == GS_APP_STATE_UNKNOWN &&
            gs_app_get_origin (app) != NULL) {
diff --git a/plugins/flatpak/gs-self-test.c b/plugins/flatpak/gs-self-test.c
index 08c5b6a9f..da0b406e7 100644
--- a/plugins/flatpak/gs-self-test.c
+++ b/plugins/flatpak/gs-self-test.c
@@ -1470,16 +1470,23 @@ gs_plugins_flatpak_app_update_func (GsPluginLoader *plugin_loader)
        g_assert_no_error (error);
        g_assert_true (list_updates != NULL);
 
-       /* make sure there are two entries */
+       /* make sure there is one entry */
        g_assert_cmpint (gs_app_list_length (list_updates), ==, 1);
        for (guint i = 0; i < gs_app_list_length (list_updates); i++) {
                app_tmp = gs_app_list_index (list_updates, i);
                g_debug ("got update %s", gs_app_get_unique_id (app_tmp));
        }
 
-       /* check they are the same GObject */
-       app_tmp = gs_app_list_lookup (list_updates, "*/flatpak/test/org.test.Chiron/*");
-       g_assert_true (app_tmp == app);
+       /* check that the runtime is not the update's one */
+       old_runtime = gs_app_get_runtime (app);
+       g_assert_true (old_runtime != NULL);
+       g_assert_cmpstr (gs_app_get_branch (old_runtime), !=, "new_master");
+
+       g_object_ref (old_runtime);
+
+       /* use the returned app, which can differ from that previous */
+       app = gs_app_list_lookup (list_updates, "*/flatpak/test/org.test.Chiron/*");
+       g_assert_nonnull (app);
        g_assert_cmpint (gs_app_get_state (app), ==, GS_APP_STATE_UPDATABLE_LIVE);
        g_assert_cmpstr (gs_app_get_update_details (app), ==, "Version 1.2.4:\nThis is best.\n\nVersion 
1.2.3:\nThis is better.");
        g_assert_cmpstr (gs_app_get_update_version (app), ==, "1.2.4");
@@ -1502,11 +1509,6 @@ gs_plugins_flatpak_app_update_func (GsPluginLoader *plugin_loader)
                                  G_CALLBACK (update_app_progress_notify_cb),
                                  &progress_cnt);
 
-       /* check that the runtime is not the update's one */
-       old_runtime = gs_app_get_runtime (app);
-       g_assert_true (old_runtime != NULL);
-       g_assert_cmpstr (gs_app_get_branch (old_runtime), !=, "new_master");
-
        /* use a mainloop so we get the events in the default context */
        g_object_unref (plugin_job);
        plugin_job = gs_plugin_job_newv (GS_PLUGIN_ACTION_UPDATE,
@@ -1584,6 +1586,8 @@ gs_plugins_flatpak_app_update_func (GsPluginLoader *plugin_loader)
        g_assert_no_error (error);
        g_assert_true (ret);
        g_assert_cmpint (gs_app_get_state (app_source), ==, GS_APP_STATE_UNAVAILABLE);
+
+       g_object_unref (old_runtime);
 }
 
 static void
@@ -1734,8 +1738,7 @@ gs_plugins_flatpak_runtime_extension_func (GsPluginLoader *plugin_loader)
        g_assert_null (app_tmp);
 
        /* check that the app has an update (it's affected by the extension's update) */
-       app_tmp = gs_app_list_lookup (list_updates, "*/flatpak/test/org.test.Chiron/*");
-       g_assert_true (app_tmp == app);
+       app = gs_app_list_lookup (list_updates, "*/flatpak/test/org.test.Chiron/*");
        g_assert_cmpint (gs_app_get_state (app), ==, GS_APP_STATE_UPDATABLE_LIVE);
 
        /* care about signals */
@@ -1779,6 +1782,12 @@ gs_plugins_flatpak_runtime_extension_func (GsPluginLoader *plugin_loader)
        g_assert_true (got_progress_installing);
        g_assert_cmpint (pending_app_changed_cnt, ==, 0);
 
+       /* The install refreshes GsApp-s cache, thus re-get the extension */
+       g_clear_object (&extension);
+       extension = gs_plugin_loader_app_create (plugin_loader,
+                       "user/flatpak/*/org.test.Chiron.Extension/master");
+       g_assert_nonnull (extension);
+
        /* check the extension's state after the update */
        g_assert_cmpint (gs_app_get_state (extension), ==, GS_APP_STATE_INSTALLED);
 
@@ -1788,8 +1797,28 @@ gs_plugins_flatpak_runtime_extension_func (GsPluginLoader *plugin_loader)
        g_signal_handler_disconnect (app, notify_state_id);
        g_signal_handler_disconnect (app, notify_progress_id);
 
+       g_clear_object (&list);
+       /* Reload the 'app', as it could change due to repo change */
+       g_object_unref (plugin_job);
+       plugin_job = gs_plugin_job_newv (GS_PLUGIN_ACTION_SEARCH,
+                                        "search", "Bingo",
+                                        "refine-flags", GS_PLUGIN_REFINE_FLAGS_DEFAULT |
+                                                        GS_PLUGIN_REFINE_FLAGS_REQUIRE_RUNTIME,
+                                        NULL);
+       list = gs_plugin_loader_job_process (plugin_loader, plugin_job, NULL, &error);
+       gs_test_flush_main_context ();
+       g_assert_no_error (error);
+       g_assert_nonnull (list);
+
+       /* make sure there is one entry, the flatpak app */
+       g_assert_cmpint (gs_app_list_length (list), ==, 1);
+       app = gs_app_list_index (list, 0);
+       g_assert_cmpstr (gs_app_get_id (app), ==, "org.test.Chiron");
+       g_assert_cmpint (gs_app_get_state (app), ==, GS_APP_STATE_INSTALLED);
+
        /* getting the runtime for later removal */
        runtime = gs_app_get_runtime (app);
+       g_assert_nonnull (runtime);
 
        /* remove the app */
        g_object_unref (plugin_job);