[gnome-software/1767-size-code-in-gsapp-can-double-count-runtimes] gs-app: Make sure dependencies size counts each app only once

[Milan Crha <mcrha src gnome org>]

commit 90d7d5c5ef06940c889c0d6769f4604960c89a2e
Author: Milan Crha <mcrha redhat com>
Date:   Mon May 23 11:50:07 2022 +0200

    gs-app: Make sure dependencies size counts each app only once
    
    The dependencies size could count runtimes or other dependencies
    multiple time, in case the dependencies share the same runtime
    or a dependency.
    
    Closes https://gitlab.gnome.org/GNOME/gnome-software/-/issues/1767

 lib/gs-app.c                 | 149 +++++++++++++++++++++++++++++--------------
 plugins/dummy/gs-self-test.c |  90 ++++++++++++++++++++++++++
 2 files changed, 191 insertions(+), 48 deletions(-)
---
diff --git a/lib/gs-app.c b/lib/gs-app.c
index ca6b6a3db..2776da0ff 100644
--- a/lib/gs-app.c
+++ b/lib/gs-app.c
@@ -3621,7 +3621,9 @@ gs_app_set_size_download (GsApp      *app,
  * @out_bytes will silently be clamped to %G_MAXUINT64.
  */
 static gboolean
-add_sizes (GsSizeType  a_type,
+add_sizes (GsApp      *app,
+           GHashTable *covered_uids,
+           GsSizeType  a_type,
            guint64     a_bytes,
            GsSizeType  b_type,
            guint64     b_bytes,
@@ -3631,6 +3633,17 @@ add_sizes (GsSizeType  a_type,
        g_return_val_if_fail (out_type != NULL, FALSE);
        g_return_val_if_fail (out_bytes != NULL, FALSE);
 
+       if (app != NULL && covered_uids != NULL) {
+               const gchar *id = gs_app_get_unique_id (app);
+               if (id == NULL)
+                       id = gs_app_get_id (app);
+               if (id != NULL) {
+                       if (g_hash_table_contains (covered_uids, id))
+                               return TRUE;
+                       g_hash_table_add (covered_uids, g_strdup (id));
+               }
+       }
+
        if (a_type == GS_SIZE_TYPE_VALID && b_type == GS_SIZE_TYPE_VALID) {
                *out_type = GS_SIZE_TYPE_VALID;
                if (!g_uint64_checked_add (out_bytes, a_bytes, b_bytes))
@@ -3644,25 +3657,10 @@ add_sizes (GsSizeType  a_type,
        return FALSE;
 }
 
-/**
- * gs_app_get_size_download_dependencies:
- * @app: A #GsApp
- * @size_bytes_out: (optional) (out caller-allocates): return location for
- *   the download size of dependencies, in bytes, or %NULL to ignore
- *
- * Get the value of #GsApp:size-download-dependencies-type and
- * #GsApp:size-download-dependencies.
- *
- * If this returns %GS_SIZE_TYPE_VALID, @size_bytes_out (if non-%NULL) will be
- * set to the download size of dependencies. Otherwise, its value will be
- * undefined.
- *
- * Returns: type of the download size of dependencies
- * Since: 43
- **/
-GsSizeType
-gs_app_get_size_download_dependencies (GsApp   *app,
-                                       guint64 *size_bytes_out)
+static GsSizeType
+get_size_download_dependencies (GsApp *app,
+                               guint64 *size_bytes_out,
+                               GHashTable *covered_uids)
 {
        GsAppPrivate *priv = gs_app_get_instance_private (app);
        GsSizeType size_type = GS_SIZE_TYPE_VALID;
@@ -3678,12 +3676,16 @@ gs_app_get_size_download_dependencies (GsApp   *app,
 
                runtime_size_download_type = gs_app_get_size_download (priv->runtime, 
&runtime_size_download_bytes);
 
-               if (add_sizes (size_type, size_bytes,
+               if (add_sizes (priv->runtime, covered_uids,
+                              size_type, size_bytes,
                               runtime_size_download_type, runtime_size_download_bytes,
                               &size_type, &size_bytes)) {
-                       runtime_size_download_dependencies_type = gs_app_get_size_download_dependencies 
(priv->runtime, &runtime_size_download_dependencies_bytes);
+                       runtime_size_download_dependencies_type = get_size_download_dependencies 
(priv->runtime,
+                                                                                                 
&runtime_size_download_dependencies_bytes,
+                                                                                                 
covered_uids);
 
-                       add_sizes (size_type, size_bytes,
+                       add_sizes (NULL, NULL,
+                                  size_type, size_bytes,
                                   runtime_size_download_dependencies_type, 
runtime_size_download_dependencies_bytes,
                                   &size_type, &size_bytes);
                }
@@ -3697,14 +3699,18 @@ gs_app_get_size_download_dependencies (GsApp   *app,
 
                related_size_download_type = gs_app_get_size_download (app_related, 
&related_size_download_bytes);
 
-               if (!add_sizes (size_type, size_bytes,
+               if (!add_sizes (app_related, covered_uids,
+                               size_type, size_bytes,
                                related_size_download_type, related_size_download_bytes,
                                &size_type, &size_bytes))
                        break;
 
-               related_size_download_dependencies_type = gs_app_get_size_download_dependencies (app_related, 
&related_size_download_dependencies_bytes);
+               related_size_download_dependencies_type = get_size_download_dependencies (app_related,
+                                                                                         
&related_size_download_dependencies_bytes,
+                                                                                         covered_uids);
 
-               if (!add_sizes (size_type, size_bytes,
+               if (!add_sizes (NULL, NULL,
+                               size_type, size_bytes,
                                related_size_download_dependencies_type, 
related_size_download_dependencies_bytes,
                                &size_type, &size_bytes))
                        break;
@@ -3716,6 +3722,35 @@ gs_app_get_size_download_dependencies (GsApp   *app,
        return size_type;
 }
 
+/**
+ * gs_app_get_size_download_dependencies:
+ * @app: A #GsApp
+ * @size_bytes_out: (optional) (out caller-allocates): return location for
+ *   the download size of dependencies, in bytes, or %NULL to ignore
+ *
+ * Get the value of #GsApp:size-download-dependencies-type and
+ * #GsApp:size-download-dependencies.
+ *
+ * If this returns %GS_SIZE_TYPE_VALID, @size_bytes_out (if non-%NULL) will be
+ * set to the download size of dependencies. Otherwise, its value will be
+ * undefined.
+ *
+ * Returns: type of the download size of dependencies
+ * Since: 43
+ **/
+GsSizeType
+gs_app_get_size_download_dependencies (GsApp   *app,
+                                       guint64 *size_bytes_out)
+{
+       g_autoptr(GHashTable) covered_uids = NULL;
+
+       g_return_val_if_fail (GS_IS_APP (app), GS_SIZE_TYPE_UNKNOWN);
+
+       covered_uids = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
+
+       return get_size_download_dependencies (app, size_bytes_out, covered_uids);
+}
+
 /**
  * gs_app_get_size_installed:
  * @app: a #GsApp
@@ -3779,25 +3814,10 @@ gs_app_set_size_installed (GsApp      *app,
        }
 }
 
-/**
- * gs_app_get_size_installed_dependencies:
- * @app: a #GsApp
- * @size_bytes_out: (optional) (out caller-allocates): return location for
- *   the installed size of dependencies, in bytes, or %NULL to ignore
- *
- * Get the values of #GsApp:size-installed-dependencies-type and
- * #GsApp:size-installed-dependencies.
- *
- * If this returns %GS_SIZE_TYPE_VALID, @size_bytes_out (if non-%NULL) will be
- * set to the installed size of dependencies. Otherwise, its value will be
- * undefined.
- *
- * Returns: type of the installed size of dependencies
- * Since: 43
- **/
-GsSizeType
-gs_app_get_size_installed_dependencies (GsApp   *app,
-                                        guint64 *size_bytes_out)
+static GsSizeType
+get_size_installed_dependencies (GsApp *app,
+                                guint64 *size_bytes_out,
+                                GHashTable *covered_uids)
 {
        GsAppPrivate *priv = gs_app_get_instance_private (app);
        GsSizeType size_type = GS_SIZE_TYPE_VALID;
@@ -3813,14 +3833,18 @@ gs_app_get_size_installed_dependencies (GsApp   *app,
 
                related_size_installed_type = gs_app_get_size_installed (app_related, 
&related_size_installed_bytes);
 
-               if (!add_sizes (size_type, size_bytes,
+               if (!add_sizes (app_related, covered_uids,
+                               size_type, size_bytes,
                                related_size_installed_type, related_size_installed_bytes,
                                &size_type, &size_bytes))
                        break;
 
-               related_size_installed_dependencies_type = gs_app_get_size_installed_dependencies 
(app_related, &related_size_installed_dependencies_bytes);
+               related_size_installed_dependencies_type = get_size_installed_dependencies (app_related,
+                                                                                           
&related_size_installed_dependencies_bytes,
+                                                                                           covered_uids);
 
-               if (!add_sizes (size_type, size_bytes,
+               if (!add_sizes (NULL, NULL,
+                               size_type, size_bytes,
                                related_size_installed_dependencies_type, 
related_size_installed_dependencies_bytes,
                                &size_type, &size_bytes))
                        break;
@@ -3832,6 +3856,35 @@ gs_app_get_size_installed_dependencies (GsApp   *app,
        return size_type;
 }
 
+/**
+ * gs_app_get_size_installed_dependencies:
+ * @app: a #GsApp
+ * @size_bytes_out: (optional) (out caller-allocates): return location for
+ *   the installed size of dependencies, in bytes, or %NULL to ignore
+ *
+ * Get the values of #GsApp:size-installed-dependencies-type and
+ * #GsApp:size-installed-dependencies.
+ *
+ * If this returns %GS_SIZE_TYPE_VALID, @size_bytes_out (if non-%NULL) will be
+ * set to the installed size of dependencies. Otherwise, its value will be
+ * undefined.
+ *
+ * Returns: type of the installed size of dependencies
+ * Since: 43
+ **/
+GsSizeType
+gs_app_get_size_installed_dependencies (GsApp   *app,
+                                        guint64 *size_bytes_out)
+{
+       g_autoptr(GHashTable) covered_uids = NULL;
+
+       g_return_val_if_fail (GS_IS_APP (app), GS_SIZE_TYPE_UNKNOWN);
+
+       covered_uids = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
+
+       return get_size_installed_dependencies (app, size_bytes_out, covered_uids);
+}
+
 /**
  * gs_app_get_size_user_data:
  * @app: A #GsApp
diff --git a/plugins/dummy/gs-self-test.c b/plugins/dummy/gs-self-test.c
index 671ca6287..e15a9815e 100644
--- a/plugins/dummy/gs-self-test.c
+++ b/plugins/dummy/gs-self-test.c
@@ -747,6 +747,93 @@ gs_plugins_dummy_limit_parallel_ops_func (GsPluginLoader *plugin_loader)
        gs_plugin_loader_set_max_parallel_ops (plugin_loader, 0);
 }
 
+static void
+gs_plugins_dummy_app_size_calc_func (GsPluginLoader *loader)
+{
+       g_autoptr(GsApp) app1 = NULL;
+       g_autoptr(GsApp) app2 = NULL;
+       g_autoptr(GsApp) runtime = NULL;
+       guint64 value = 0;
+
+       app1 = gs_app_new ("app1");
+       gs_app_set_state (app1, GS_APP_STATE_AVAILABLE);
+       gs_app_set_size_download (app1, GS_SIZE_TYPE_VALID, 1);
+       gs_app_set_size_installed (app1, GS_SIZE_TYPE_VALID, 1000);
+       g_assert_cmpint (gs_app_get_size_download (app1, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 1);
+       g_assert_cmpint (gs_app_get_size_download_dependencies (app1, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 0);
+       g_assert_cmpint (gs_app_get_size_installed (app1, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 1000);
+       g_assert_cmpint (gs_app_get_size_installed_dependencies (app1, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 0);
+
+       app2 = gs_app_new ("app2");
+       gs_app_set_state (app2, GS_APP_STATE_AVAILABLE);
+       gs_app_set_size_download (app2, GS_SIZE_TYPE_VALID, 20);
+       gs_app_set_size_installed (app2, GS_SIZE_TYPE_VALID, 20000);
+       g_assert_cmpint (gs_app_get_size_download (app2, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 20);
+       g_assert_cmpint (gs_app_get_size_download_dependencies (app2, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 0);
+       g_assert_cmpint (gs_app_get_size_installed (app2, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 20000);
+       g_assert_cmpint (gs_app_get_size_installed_dependencies (app2, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 0);
+
+       runtime = gs_app_new ("runtime");
+       gs_app_set_state (runtime, GS_APP_STATE_AVAILABLE);
+       gs_app_set_size_download (runtime, GS_SIZE_TYPE_VALID, 300);
+       gs_app_set_size_installed (runtime, GS_SIZE_TYPE_VALID, 300000);
+       g_assert_cmpint (gs_app_get_size_download (runtime, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 300);
+       g_assert_cmpint (gs_app_get_size_download_dependencies (runtime, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 0);
+       g_assert_cmpint (gs_app_get_size_installed (runtime, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 300000);
+       g_assert_cmpint (gs_app_get_size_installed_dependencies (runtime, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 0);
+
+       gs_app_set_runtime (app1, runtime);
+       g_assert_cmpint (gs_app_get_size_download (app1, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 1);
+       g_assert_cmpint (gs_app_get_size_download_dependencies (app1, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 300);
+       g_assert_cmpint (gs_app_get_size_installed (app1, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 1000);
+       g_assert_cmpint (gs_app_get_size_installed_dependencies (app1, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 0);
+
+       gs_app_set_runtime (app2, runtime);
+       g_assert_cmpint (gs_app_get_size_download (app2, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 20);
+       g_assert_cmpint (gs_app_get_size_download_dependencies (app2, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 300);
+       g_assert_cmpint (gs_app_get_size_installed (app2, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 20000);
+       g_assert_cmpint (gs_app_get_size_installed_dependencies (app2, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 0);
+
+       gs_app_add_related (app1, app2);
+       g_assert_cmpint (gs_app_get_size_download (app1, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 1);
+       g_assert_cmpint (gs_app_get_size_download_dependencies (app1, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 320);
+       g_assert_cmpint (gs_app_get_size_installed (app1, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 1000);
+       g_assert_cmpint (gs_app_get_size_installed_dependencies (app1, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 20000);
+
+       g_assert_cmpint (gs_app_get_size_download (app2, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 20);
+       g_assert_cmpint (gs_app_get_size_download_dependencies (app2, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 300);
+       g_assert_cmpint (gs_app_get_size_installed (app2, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 20000);
+       g_assert_cmpint (gs_app_get_size_installed_dependencies (app2, &value), ==, GS_SIZE_TYPE_VALID);
+       g_assert_cmpint (value, ==, 0);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -905,6 +992,9 @@ main (int argc, char **argv)
        g_test_add_data_func ("/gnome-software/plugins/dummy/limit-parallel-ops",
                              plugin_loader,
                              (GTestDataFunc) gs_plugins_dummy_limit_parallel_ops_func);
+       g_test_add_data_func ("/gnome-software/plugins/dummy/app-size-calc",
+                             plugin_loader,
+                             (GTestDataFunc) gs_plugins_dummy_app_size_calc_func);
        retval = g_test_run ();
 
        /* Clean up. */