[dconf] DConfChangeset: implement dir resets properly

[Allison Ryan Lortie <desrt src gnome org>]

commit c6423fa1cacb9fb3178016e712be9323a743bce4
Author: Allison Ryan Lortie <desrt desrt ca>
Date:   Mon Nov 30 16:40:25 2015 +0000

    DConfChangeset: implement dir resets properly
    
    If a dir is reset against a DConfChangeset then the result ought to be
    that all keys under that dir read as NULL (until such a time as they are
    set to a new value).
    
    This is consistent with the (existing) behaviour that a key will read as
    NULL if it, itself, was reset.
    
    In order to make that efficient, we create a separate GHashTable to
    serve as a cache of all of the directories that have been reset and
    iterate it whenever we do a key lookup that doesn't have a direct hit.
    
    We update (and expand) the test case to reflect this new reality -- the
    tests actually had a case that relied on the inconsistent behaviour.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=744678

 common/dconf-changeset.c |   57 ++++++++++++++++++++++++++++++++++++---------
 tests/changeset.c        |   27 +++++++++++++++++++--
 2 files changed, 69 insertions(+), 15 deletions(-)
---
diff --git a/common/dconf-changeset.c b/common/dconf-changeset.c
index 89f340c..eb370d6 100644
--- a/common/dconf-changeset.c
+++ b/common/dconf-changeset.c
@@ -54,6 +54,7 @@
 struct _DConfChangeset
 {
   GHashTable *table;
+  GHashTable *dir_resets;
   guint is_database : 1;
   guint is_sealed : 1;
   gint ref_count;
@@ -157,6 +158,9 @@ dconf_changeset_unref (DConfChangeset *changeset)
 
       g_hash_table_unref (changeset->table);
 
+      if (changeset->dir_resets)
+        g_hash_table_unref (changeset->dir_resets);
+
       g_slice_free (DConfChangeset, changeset);
     }
 }
@@ -177,6 +181,21 @@ dconf_changeset_ref (DConfChangeset *changeset)
   return changeset;
 }
 
+void
+dconf_changeset_record_dir_reset (DConfChangeset *changeset,
+                                  const gchar    *dir)
+{
+  g_return_if_fail (dconf_is_dir (dir, NULL));
+  g_return_if_fail (!changeset->is_database);
+  g_return_if_fail (!changeset->is_sealed);
+
+  if (!changeset->dir_resets)
+    changeset->dir_resets = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
+
+  g_hash_table_insert (changeset->table, g_strdup (dir), NULL);
+  g_hash_table_add (changeset->dir_resets, g_strdup (dir));
+}
+
 /**
  * dconf_changeset_set:
  * @changeset: a #DConfChangeset
@@ -217,7 +236,7 @@ dconf_changeset_set (DConfChangeset *changeset,
 
       /* If this is a non-database then record the reset itself. */
       if (!changeset->is_database)
-        g_hash_table_insert (changeset->table, g_strdup (path), NULL);
+        dconf_changeset_record_dir_reset (changeset, path);
     }
 
   /* ...or a value reset */
@@ -263,7 +282,26 @@ dconf_changeset_get (DConfChangeset  *changeset,
   gpointer tmp;
 
   if (!g_hash_table_lookup_extended (changeset->table, key, NULL, &tmp))
-    return FALSE;
+    {
+      /* Did not find an exact match, so check for dir resets */
+      if (changeset->dir_resets)
+        {
+          GHashTableIter iter;
+          gpointer dir;
+
+          g_hash_table_iter_init (&iter, changeset->dir_resets);
+          while (g_hash_table_iter_next (&iter, &dir, NULL))
+            if (g_str_has_prefix (key, dir))
+              {
+                if (value)
+                  *value = NULL;
+
+                return TRUE;
+              }
+        }
+
+      return FALSE;
+    }
 
   if (value)
     *value = tmp ? g_variant_ref (tmp) : NULL;
@@ -624,16 +662,11 @@ dconf_changeset_deserialise (GVariant *serialised)
        *
        * If we get an invalid case, just fall through and ignore it.
        */
-      if (value == NULL)
-        {
-          if (dconf_is_path (key, NULL))
-            g_hash_table_insert (changeset->table, g_strdup (key), NULL);
-        }
-      else
-        {
-          if (dconf_is_key (key, NULL))
-            g_hash_table_insert (changeset->table, g_strdup (key), g_variant_ref (value));
-        }
+      if (dconf_is_key (key, NULL))
+        g_hash_table_insert (changeset->table, g_strdup (key), value ? g_variant_ref (value) : NULL);
+
+      else if (dconf_is_dir (key, NULL) && value == NULL)
+        dconf_changeset_record_dir_reset (changeset, key);
     }
 
   return changeset;
diff --git a/tests/changeset.c b/tests/changeset.c
index 90b8de6..5f046df 100644
--- a/tests/changeset.c
+++ b/tests/changeset.c
@@ -281,37 +281,58 @@ static void
 test_reset (void)
 {
   DConfChangeset *changeset;
+  GVariant *value;
 
   changeset = dconf_changeset_new ();
   g_assert (!dconf_changeset_get (changeset, "/value/a", NULL));
+  g_assert (!dconf_changeset_get (changeset, "/value/a", &value));
+  /* value was not set */
 
   /* set a value */
   dconf_changeset_set (changeset, "/value/a", g_variant_new_boolean (TRUE));
   g_assert (dconf_changeset_get (changeset, "/value/a", NULL));
+  g_assert (dconf_changeset_get (changeset, "/value/a", &value));
+  g_assert (value != NULL);
+  g_variant_unref (value);
 
   /* record the reset */
   dconf_changeset_set (changeset, "/value/", NULL);
-  g_assert (!dconf_changeset_get (changeset, "/value/a", NULL));
+  g_assert (dconf_changeset_get (changeset, "/value/a", NULL));
+  g_assert (dconf_changeset_get (changeset, "/value/a", &value));
+  g_assert (value == NULL);
 
   /* write it back */
   dconf_changeset_set (changeset, "/value/a", g_variant_new_boolean (TRUE));
   g_assert (dconf_changeset_get (changeset, "/value/a", NULL));
+  g_assert (dconf_changeset_get (changeset, "/value/a", &value));
+  g_assert (value != NULL);
+  g_variant_unref (value);
 
   /* reset again */
   dconf_changeset_set (changeset, "/value/", NULL);
-  g_assert (!dconf_changeset_get (changeset, "/value/a", NULL));
+  g_assert (dconf_changeset_get (changeset, "/value/a", NULL));
+  g_assert (dconf_changeset_get (changeset, "/value/a", &value));
+  g_assert (value == NULL);
 
   /* write again */
   dconf_changeset_set (changeset, "/value/a", g_variant_new_boolean (TRUE));
   g_assert (dconf_changeset_get (changeset, "/value/a", NULL));
+  g_assert (dconf_changeset_get (changeset, "/value/a", &value));
+  g_assert (value != NULL);
+  g_variant_unref (value);
 
   /* reset a different way */
-  dconf_changeset_set (changeset, "/value/a", g_variant_new_boolean (TRUE));
+  dconf_changeset_set (changeset, "/value/a", NULL);
   g_assert (dconf_changeset_get (changeset, "/value/a", NULL));
+  g_assert (dconf_changeset_get (changeset, "/value/a", &value));
+  g_assert (value == NULL);
 
   /* write last time */
   dconf_changeset_set (changeset, "/value/a", g_variant_new_boolean (TRUE));
   g_assert (dconf_changeset_get (changeset, "/value/a", NULL));
+  g_assert (dconf_changeset_get (changeset, "/value/a", &value));
+  g_assert (value != NULL);
+  g_variant_unref (value);
 
   dconf_changeset_unref (changeset);
 }