Merge branch 'app-filter-unchanged' into 'master'

user-controls: Only save the app filter if it’s changed

See merge request pwithnall/malcontent!102

libmalcontent-ui/user-controls.c

@@ -104,7 +104,8 @@ struct _MctUserControls
   GDBusConnection *dbus_connection;  /* (owned) */
   GCancellable *cancellable; /* (owned) */
   MctManager   *manager; /* (owned) */
-  MctAppFilter *filter; /* (owned) (nullable) */
+  MctAppFilter *filter; /* (owned) (nullable); updated by the user of #MctUserControls */
+  MctAppFilter *last_saved_filter; /* (owned) (nullable); updated each time we internally time out and save the app filter */
   guint         selected_age; /* @oars_disabled_age to disable OARS */
 
   guint         blocklist_apps_source_id;
@@ -296,6 +297,7 @@ update_app_filter_from_user (MctUserControls *self)
 
   /* FIXME: make it asynchronous */
   g_clear_pointer (&self->filter, mct_app_filter_unref);
+  g_clear_pointer (&self->last_saved_filter, mct_app_filter_unref);
   self->filter = mct_manager_get_app_filter (self->manager,
                                              act_user_get_uid (self->user),
                                              MCT_MANAGER_GET_VALUE_FLAGS_NONE,
@@ -310,6 +312,8 @@ update_app_filter_from_user (MctUserControls *self)
       return;
     }
 
+  self->last_saved_filter = mct_app_filter_ref (self->filter);
+
   g_debug ("Retrieved new app filter for user '%s'", act_user_get_user_name (self->user));
 }
 
@@ -591,6 +595,15 @@ blocklist_apps_cb (gpointer data)
   mct_user_controls_build_app_filter (self, &builder);
   new_filter = mct_app_filter_builder_end (&builder);
 
+  /* Don’t bother saving the app filter (which could result in asking the user
+   * for admin permission) if it hasn’t changed. */
+  if (self->last_saved_filter != NULL &&
+      mct_app_filter_equal (new_filter, self->last_saved_filter))
+    {
+      g_debug ("Not saving app filter as it hasn’t changed");
+      return G_SOURCE_REMOVE;
+    }
+
   /* FIXME: should become asynchronous */
   mct_manager_set_app_filter (self->manager,
                               act_user_get_uid (self->user),
@@ -605,6 +618,10 @@ blocklist_apps_cb (gpointer data)
       setup_parental_control_settings (self);
     }
 
+  /* Update the cached copy */
+  mct_app_filter_unref (self->last_saved_filter);
+  self->last_saved_filter = g_steal_pointer (&new_filter);
+
   return G_SOURCE_REMOVE;
 }
 
@@ -816,6 +833,7 @@ mct_user_controls_finalize (GObject *object)
   g_clear_object (&self->permission);
 
   g_clear_pointer (&self->filter, mct_app_filter_unref);
+  g_clear_pointer (&self->last_saved_filter, mct_app_filter_unref);
   g_clear_object (&self->manager);
   g_clear_object (&self->dbus_connection);
 
@@ -1333,8 +1351,12 @@ mct_user_controls_set_app_filter (MctUserControls *self,
     return;
 
   g_clear_pointer (&self->filter, mct_app_filter_unref);
+  g_clear_pointer (&self->last_saved_filter, mct_app_filter_unref);
   if (app_filter != NULL)
-    self->filter = mct_app_filter_ref (app_filter);
+    {
+      self->filter = mct_app_filter_ref (app_filter);
+      self->last_saved_filter = mct_app_filter_ref (app_filter);
+    }
 
   g_debug ("Set new app filter from caller");
   setup_parental_control_settings (self);

libmalcontent/app-filter.c

@@ -755,6 +755,33 @@ mct_app_filter_deserialize (GVariant  *variant,
   return g_steal_pointer (&app_filter);
 }
 
+/**
+ * mct_app_filter_equal:
+ * @a: (not nullable): an #MctAppFilter
+ * @b: (not nullable): an #MctAppFilter
+ *
+ * Check whether app filters @a and @b are equal.
+ *
+ * Returns: %TRUE if @a and @b are equal, %FALSE otherwise
+ * Since: 0.10.0
+ */
+gboolean
+mct_app_filter_equal (MctAppFilter *a,
+                      MctAppFilter *b)
+{
+  g_return_val_if_fail (a != NULL, FALSE);
+  g_return_val_if_fail (a->ref_count >= 1, FALSE);
+  g_return_val_if_fail (b != NULL, FALSE);
+  g_return_val_if_fail (b->ref_count >= 1, FALSE);
+
+  return (a->user_id == b->user_id &&
+          a->app_list_type == b->app_list_type &&
+          a->allow_user_installation == b->allow_user_installation &&
+          a->allow_system_installation == b->allow_system_installation &&
+          g_strv_equal ((const gchar * const *) a->app_list, (const gchar * const *) b->app_list) &&
+          g_variant_equal (a->oars_ratings, b->oars_ratings));
+}
+
 /*
  * Actual implementation of #MctAppFilterBuilder.
  *

libmalcontent/app-filter.h

@@ -104,6 +104,9 @@ MctAppFilter *mct_app_filter_deserialize (GVariant      *variant,
                                           uid_t          user_id,
                                           GError       **error);
 
+gboolean mct_app_filter_equal (MctAppFilter *a,
+                               MctAppFilter *b);
+
 /**
  * MctAppFilterBuilder:
  *

libmalcontent/tests/app-filter.c

@@ -162,6 +162,65 @@ test_app_filter_deserialize_invalid (void)
     }
 }
 
+/* Test that mct_app_filter_equal() returns the correct results on various
+ * app filters. */
+static void
+test_app_filter_equal (void)
+{
+  g_auto(MctAppFilterBuilder) builder = MCT_APP_FILTER_BUILDER_INIT ();
+  MctAppFilter *equal_filters[2];
+  const gchar *unequal_filters_serialised[] =
+    {
+      "{ 'AppFilter': <(true, @as ['/usr/bin/gnome-software'])> }",
+      "{ 'AppFilter': <(false, @as ['/usr/bin/gnome-software'])> }",
+      "{ 'AllowUserInstallation': <true> }",
+      "{ 'AllowSystemInstallation': <true> }",
+      "{ 'OarsFilter': <('oars-1.1', { 'violence-cartoon': 'mild' })> }",
+    };
+  MctAppFilter *unequal_filters[G_N_ELEMENTS (unequal_filters_serialised)];
+
+  /* Build a couple of filters which are identical. */
+  equal_filters[0] = mct_app_filter_builder_end (&builder);
+
+  mct_app_filter_builder_init (&builder);
+  equal_filters[1] = mct_app_filter_builder_end (&builder);
+
+  /* And a load of filters which are not. */
+  for (gsize i = 0; i < G_N_ELEMENTS (unequal_filters_serialised); i++)
+    {
+      g_autoptr(GVariant) serialized = NULL;
+
+      serialized = g_variant_parse (NULL, unequal_filters_serialised[i], NULL, NULL, NULL);
+      g_assert (serialized != NULL);
+
+      unequal_filters[i] = mct_app_filter_deserialize (serialized, 1, NULL);
+      g_assert (unequal_filters[i] != NULL);
+    }
+
+  /* Test the equality checks on them all. */
+  for (gsize i = 0; i < G_N_ELEMENTS (equal_filters); i++)
+    for (gsize j = 0; j < G_N_ELEMENTS (equal_filters); j++)
+      g_assert_true (mct_app_filter_equal (equal_filters[i], equal_filters[j]));
+
+  for (gsize i = 0; i < G_N_ELEMENTS (unequal_filters); i++)
+    {
+      for (gsize j = 0; j < G_N_ELEMENTS (equal_filters); j++)
+        g_assert_false (mct_app_filter_equal (unequal_filters[i], equal_filters[j]));
+      for (gsize j = 0; j < G_N_ELEMENTS (unequal_filters); j++)
+        {
+          if (i != j)
+            g_assert_false (mct_app_filter_equal (unequal_filters[i], unequal_filters[j]));
+          else
+            g_assert_true (mct_app_filter_equal (unequal_filters[i], unequal_filters[j]));
+        }
+    }
+
+  for (gsize i = 0; i < G_N_ELEMENTS (equal_filters); i++)
+    mct_app_filter_unref (equal_filters[i]);
+  for (gsize i = 0; i < G_N_ELEMENTS (unequal_filters); i++)
+    mct_app_filter_unref (unequal_filters[i]);
+}
+
 /* Test that mct_app_filter_is_enabled() returns the correct results on various
  * app filters. */
 static void
@@ -1521,6 +1580,8 @@ main (int    argc,
   g_test_add_func ("/app-filter/deserialize", test_app_filter_deserialize);
   g_test_add_func ("/app-filter/deserialize/invalid", test_app_filter_deserialize_invalid);
 
+  g_test_add_func ("/app-filter/equal", test_app_filter_equal);
+
   g_test_add_func ("/app-filter/is-enabled", test_app_filter_is_enabled);
 
   g_test_add ("/app-filter/builder/stack/non-empty", BuilderFixture, NULL,