[gnome-software/mwleeds/fix-deprecated-install: 4/4] flatpak: Improve performance of ref/bundle processing

[Phaedrus Leeds <mwleeds src gnome org>]

commit da5c17baa8fcc234bb3dc7954c1bf404bedf5c40
Author: Phaedrus Leeds <mwleeds protonmail com>
Date:   Mon Nov 29 20:57:08 2021 -0800

    flatpak: Improve performance of ref/bundle processing
    
    This commit reworks the functions that handle flatpakref and flatpak
    bundle files. There are two cases to worry about: either (1) the thing
    is already installed or available from a configured remote, or (2) it's
    only available from the remote specified by the flatpakref/bundle. In
    the former case, the gs_flatpak_file_to_app_bundle() implementation
    isn't right since commit "flatpak: Avoid plugin cache for temp
    installation" since we will no longer get the installed/available app
    from the cache. And the gs_flatpak_file_to_app_ref() implementation
    isn't right because the installation is temporary so
    FLATPAK_ERROR_ALREADY_INSTALLED won't be returned by the transaction.
    
    Since there is already code in the calling function
    gs_plugin_flatpak_file_to_app_app_{bundle,ref} to handle the case where
    the app is installed/available, there's no point in going through the
    whole expensive process of refining the GsApp created in
    gs_flatpak_file_to_app_{bundle,ref} since it will just be discarded. So
    use the gs_flatpak_ functions just to get the ref, and then only if
    necessary call them again and do the expensive refine code path.

 plugins/flatpak/gs-flatpak.c        | 68 ++++++++++++++++---------------------
 plugins/flatpak/gs-flatpak.h        |  2 ++
 plugins/flatpak/gs-plugin-flatpak.c | 20 ++++++++---
 3 files changed, 48 insertions(+), 42 deletions(-)
---
diff --git a/plugins/flatpak/gs-flatpak.c b/plugins/flatpak/gs-flatpak.c
index f9f2f4e17..0cdc48add 100644
--- a/plugins/flatpak/gs-flatpak.c
+++ b/plugins/flatpak/gs-flatpak.c
@@ -3295,6 +3295,7 @@ gs_flatpak_app_remove_source (GsFlatpak *self,
 GsApp *
 gs_flatpak_file_to_app_bundle (GsFlatpak *self,
                               GFile *file,
+                              gboolean unrefined,
                               GCancellable *cancellable,
                               GError **error)
 {
@@ -3303,8 +3304,6 @@ gs_flatpak_file_to_app_bundle (GsFlatpak *self,
        g_autoptr(GBytes) metadata = NULL;
        g_autoptr(GsApp) app = NULL;
        g_autoptr(FlatpakBundleRef) xref_bundle = NULL;
-       g_autoptr(FlatpakInstalledRef) installed_ref = NULL;
-       const char *origin = NULL;
 
        /* load bundle */
        xref_bundle = flatpak_bundle_ref_new (file, error);
@@ -3314,23 +3313,11 @@ gs_flatpak_file_to_app_bundle (GsFlatpak *self,
                return NULL;
        }
 
-       /* get the origin if it's already installed */
-       installed_ref = flatpak_installation_get_installed_ref (self->installation,
-                                                               flatpak_ref_get_kind (FLATPAK_REF 
(xref_bundle)),
-                                                               flatpak_ref_get_name (FLATPAK_REF 
(xref_bundle)),
-                                                               flatpak_ref_get_arch (FLATPAK_REF 
(xref_bundle)),
-                                                               flatpak_ref_get_branch (FLATPAK_REF 
(xref_bundle)),
-                                                               NULL, NULL);
-       if (installed_ref != NULL)
-               origin = flatpak_installed_ref_get_origin (installed_ref);
-
        /* load metadata */
-       app = gs_flatpak_create_app (self, origin, FLATPAK_REF (xref_bundle), NULL, cancellable);
-       if (gs_app_get_state (app) == GS_APP_STATE_INSTALLED) {
-               if (gs_flatpak_app_get_ref_name (app) == NULL)
-                       gs_flatpak_set_metadata (self, app, FLATPAK_REF (xref_bundle));
+       app = gs_flatpak_create_app (self, NULL, FLATPAK_REF (xref_bundle), NULL, cancellable);
+       if (unrefined)
                return g_steal_pointer (&app);
-       }
+
        gs_flatpak_app_set_file_kind (app, GS_FLATPAK_APP_FILE_KIND_BUNDLE);
        gs_app_set_state (app, GS_APP_STATE_AVAILABLE_LOCAL);
        gs_app_set_size_installed (app, flatpak_bundle_ref_get_installed_size (xref_bundle));
@@ -3345,7 +3332,7 @@ gs_flatpak_file_to_app_bundle (GsFlatpak *self,
        /* load AppStream */
        appstream_gz = flatpak_bundle_ref_get_appstream (xref_bundle);
        if (appstream_gz != NULL) {
-               if (!gs_flatpak_refine_appstream_from_bytes (self, app, origin, installed_ref,
+               if (!gs_flatpak_refine_appstream_from_bytes (self, app, NULL, NULL,
                                                             appstream_gz,
                                                             GS_PLUGIN_REFINE_FLAGS_DEFAULT,
                                                             cancellable, error))
@@ -3421,6 +3408,7 @@ _txn_choose_remote_for_ref (FlatpakTransaction *transaction,
 GsApp *
 gs_flatpak_file_to_app_ref (GsFlatpak *self,
                            GFile *file,
+                           gboolean unrefined,
                            GCancellable *cancellable,
                            GError **error)
 {
@@ -3504,6 +3492,26 @@ gs_flatpak_file_to_app_ref (GsFlatpak *self,
                return NULL;
        }
 
+       if (unrefined) {
+               /* Note: we don't support non-default arch here but it's not a
+                * regression since we never have for a flatpakref
+                */
+               g_autofree char *app_ref = g_strdup_printf ("%s/%s/%s/%s",
+                                                           is_runtime ? "runtime" : "app",
+                                                           ref_name,
+                                                           flatpak_get_default_arch (),
+                                                           ref_branch);
+               parsed_ref = flatpak_ref_parse (app_ref, error);
+               if (parsed_ref == NULL) {
+                       gs_flatpak_error_convert (error);
+                       return NULL;
+               }
+
+               /* early return */
+               app = gs_flatpak_create_app (self, NULL, parsed_ref, NULL, cancellable);
+               return g_steal_pointer (&app);
+       }
+
        /* Add the remote (to the temporary installation) but abort the
         * transaction before it installs the app
         */
@@ -3524,31 +3532,15 @@ gs_flatpak_file_to_app_ref (GsFlatpak *self,
        success = flatpak_transaction_run (transaction, cancellable, &error_local);
        g_assert (!success); /* aborted in _txn_abort_on_ready */
 
-       if (!g_error_matches (error_local, FLATPAK_ERROR, FLATPAK_ERROR_ALREADY_INSTALLED) &&
-           !g_error_matches (error_local, FLATPAK_ERROR, FLATPAK_ERROR_ABORTED)) {
+       /* We don't check for FLATPAK_ERROR_ALREADY_INSTALLED here because it's
+        * a temporary installation
+        */
+       if (!g_error_matches (error_local, FLATPAK_ERROR, FLATPAK_ERROR_ABORTED)) {
                g_propagate_error (error, g_steal_pointer (&error_local));
                gs_flatpak_error_convert (error);
                return NULL;
        }
 
-       /* handle the already installed case */
-       if (g_error_matches (error_local, FLATPAK_ERROR, FLATPAK_ERROR_ALREADY_INSTALLED)) {
-               g_autoptr(FlatpakInstalledRef) installed_ref = NULL;
-               installed_ref = flatpak_installation_get_installed_ref (self->installation,
-                                                                       is_runtime ? FLATPAK_REF_KIND_RUNTIME 
: FLATPAK_REF_KIND_APP,
-                                                                       ref_name,
-                                                                       NULL, /* arch */
-                                                                       ref_branch,
-                                                                       cancellable,
-                                                                       error);
-               if (installed_ref == NULL) {
-                       gs_flatpak_error_convert (error);
-                       return NULL;
-               }
-               app = gs_flatpak_create_installed (self, installed_ref, NULL, cancellable);
-               return g_steal_pointer (&app);
-       }
-
        g_clear_error (&error_local);
 
        /* find the operation for the flatpakref */
diff --git a/plugins/flatpak/gs-flatpak.h b/plugins/flatpak/gs-flatpak.h
index 348a49783..ed27f79b5 100644
--- a/plugins/flatpak/gs-flatpak.h
+++ b/plugins/flatpak/gs-flatpak.h
@@ -88,10 +88,12 @@ gboolean    gs_flatpak_app_install_source   (GsFlatpak              *self,
                                                 GError                 **error);
 GsApp          *gs_flatpak_file_to_app_ref     (GsFlatpak              *self,
                                                 GFile                  *file,
+                                                gboolean                unrefined,
                                                 GCancellable           *cancellable,
                                                 GError                 **error);
 GsApp          *gs_flatpak_file_to_app_bundle  (GsFlatpak              *self,
                                                 GFile                  *file,
+                                                gboolean                unrefined,
                                                 GCancellable           *cancellable,
                                                 GError                 **error);
 GsApp          *gs_flatpak_find_source_by_url  (GsFlatpak              *self,
diff --git a/plugins/flatpak/gs-plugin-flatpak.c b/plugins/flatpak/gs-plugin-flatpak.c
index e5121276c..787139f90 100644
--- a/plugins/flatpak/gs-plugin-flatpak.c
+++ b/plugins/flatpak/gs-plugin-flatpak.c
@@ -1334,8 +1334,8 @@ gs_plugin_flatpak_file_to_app_bundle (GsPluginFlatpak  *self,
        if (flatpak_tmp == NULL)
                return NULL;
 
-       /* add object */
-       app = gs_flatpak_file_to_app_bundle (flatpak_tmp, file, cancellable, error);
+       /* First make a quick GsApp to get the ref */
+       app = gs_flatpak_file_to_app_bundle (flatpak_tmp, file, TRUE /* unrefined */, cancellable, error);
        if (app == NULL)
                return NULL;
 
@@ -1345,6 +1345,12 @@ gs_plugin_flatpak_file_to_app_bundle (GsPluginFlatpak  *self,
        if (app_tmp != NULL)
                return g_steal_pointer (&app_tmp);
 
+       /* If not installed/available, make a fully refined GsApp */
+       g_clear_object (&app);
+       app = gs_flatpak_file_to_app_bundle (flatpak_tmp, file, FALSE /* unrefined */, cancellable, error);
+       if (app == NULL)
+               return NULL;
+
        /* force this to be 'any' scope for installation */
        gs_app_set_scope (app, AS_COMPONENT_SCOPE_UNKNOWN);
 
@@ -1369,8 +1375,8 @@ gs_plugin_flatpak_file_to_app_ref (GsPluginFlatpak  *self,
        if (flatpak_tmp == NULL)
                return NULL;
 
-       /* add object */
-       app = gs_flatpak_file_to_app_ref (flatpak_tmp, file, cancellable, error);
+       /* First make a quick GsApp to get the ref */
+       app = gs_flatpak_file_to_app_ref (flatpak_tmp, file, TRUE /* unrefined */, cancellable, error);
        if (app == NULL)
                return NULL;
 
@@ -1380,6 +1386,12 @@ gs_plugin_flatpak_file_to_app_ref (GsPluginFlatpak  *self,
        if (app_tmp != NULL)
                return g_steal_pointer (&app_tmp);
 
+       /* If not installed/available, make a fully refined GsApp */
+       g_clear_object (&app);
+       app = gs_flatpak_file_to_app_ref (flatpak_tmp, file, FALSE /* unrefined */, cancellable, error);
+       if (app == NULL)
+               return NULL;
+
        /* force this to be 'any' scope for installation */
        gs_app_set_scope (app, AS_COMPONENT_SCOPE_UNKNOWN);