[dconf: 3/11] engine: merge _read and _read_user_value()

[Allison Ryan Lortie <desrt src gnome org>]

commit a4139891fa874197e945fe409698e16a5ba35ae3
Author: Allison Ryan Lortie <desrt desrt ca>
Date:   Wed Dec 16 14:52:19 2015 +0000

    engine: merge _read and _read_user_value()
    
    Delete the separate dconf_engine_read_user_value() and merge its
    functionality into dconf_engine_read() by adding a flags field.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=759128

 client/dconf-client.c            |    4 +-
 common/dconf-enums.h             |    6 +++
 engine/dconf-engine.c            |   89 +++++++++++---------------------------
 engine/dconf-engine.h            |    7 +--
 gsettings/dconfsettingsbackend.c |    6 +-
 tests/engine.c                   |   24 +++++-----
 6 files changed, 51 insertions(+), 85 deletions(-)
---
diff --git a/client/dconf-client.c b/client/dconf-client.c
index 2af7dfa..e614256 100644
--- a/client/dconf-client.c
+++ b/client/dconf-client.c
@@ -242,7 +242,7 @@ dconf_client_read (DConfClient *client,
 {
   g_return_val_if_fail (DCONF_IS_CLIENT (client), NULL);
 
-  return dconf_engine_read (client->engine, NULL, key);
+  return dconf_engine_read (client->engine, DCONF_READ_FLAGS_NONE, NULL, key);
 }
 
 /* This provides a "read through" queue that resets all of the keys.
@@ -296,7 +296,7 @@ dconf_client_read_default (DConfClient *client,
 {
   g_return_val_if_fail (DCONF_IS_CLIENT (client), NULL);
 
-  return dconf_engine_read (client->engine, dconf_client_get_reset_queue (), key);
+  return dconf_engine_read (client->engine, DCONF_READ_FLAGS_NONE, dconf_client_get_reset_queue (), key);
 }
 
 /**
diff --git a/common/dconf-enums.h b/common/dconf-enums.h
index 9f08211..a2ac08a 100644
--- a/common/dconf-enums.h
+++ b/common/dconf-enums.h
@@ -32,4 +32,10 @@ typedef enum
   DCONF_ERROR_NOT_WRITABLE
 } DConfError;
 
+typedef enum
+{
+  DCONF_READ_FLAGS_NONE                 = 0,
+  DCONF_READ_USER_VALUE                 = (1u << 1)
+} DConfReadFlags;
+
 #endif /* __dconf_error_h__ */
diff --git a/engine/dconf-engine.c b/engine/dconf-engine.c
index 1b2770e..ebd8ede 100644
--- a/engine/dconf-engine.c
+++ b/engine/dconf-engine.c
@@ -444,9 +444,10 @@ dconf_engine_find_key_in_queue (GQueue       *queue,
 }
 
 GVariant *
-dconf_engine_read (DConfEngine *engine,
-                   GQueue      *read_through,
-                   const gchar *key)
+dconf_engine_read (DConfEngine    *engine,
+                   DConfReadFlags  flags,
+                   GQueue         *read_through,
+                   const gchar    *key)
 {
   GVariant *value = NULL;
   gint lock_level = 0;
@@ -476,6 +477,13 @@ dconf_engine_read (DConfEngine *engine,
    *   This statement includes read_through and queued changes.  If a
    *   lock is found, we will ignore those.
    *
+   * With respect to flags:
+   *
+   *   If DCONF_READ_USER_VALUE is given then we completely ignore all
+   *   locks, returning the user value all the time, even if it is not
+   *   visible (because of a lock).  This includes any pending value
+   *   that is in the read_through or pending queues.
+   *
    * With respect to read_through and queued changed:
    *
    *   We only consider read_through and queued changes in the event
@@ -554,12 +562,13 @@ dconf_engine_read (DConfEngine *engine,
    *
    * Note: i > 0 (strictly).  Ignore locks for source #0.
    */
-  for (i = engine->n_sources - 1; i > 0; i--)
-    if (engine->sources[i]->locks && gvdb_table_has_value (engine->sources[i]->locks, key))
-      {
-        lock_level = i;
-        break;
-      }
+  if (~flags & DCONF_READ_USER_VALUE)
+    for (i = engine->n_sources - 1; i > 0; i--)
+      if (engine->sources[i]->locks && gvdb_table_has_value (engine->sources[i]->locks, key))
+        {
+          lock_level = i;
+          break;
+        }
 
   /* Only do steps 2 to 4 if we have no locks and we have a writable source. */
   if (!lock_level && engine->n_sources != 0 && engine->sources[0]->writable)
@@ -600,61 +609,15 @@ dconf_engine_read (DConfEngine *engine,
     }
 
   /* Step 5.  Check the remaining sources, until value != NULL. */
-  for (i = lock_level; value == NULL && i < engine->n_sources; i++)
-    {
-      if (engine->sources[i]->values == NULL)
-        continue;
-
-      if ((value = gvdb_table_get_value (engine->sources[i]->values, key)))
-        break;
-    }
-
-  dconf_engine_release_sources (engine);
-
-  return value;
-}
-
-GVariant *
-dconf_engine_read_user_value (DConfEngine *engine,
-                              GQueue      *read_through,
-                              const gchar *key)
-{
-  gboolean found_key = FALSE;
-  GVariant *value = NULL;
-
-  /* This is a simplified version of the above.  We get to ignore locks
-   * and system-level settings.
-   *
-   * NB: we may find "NULL", which is why we have a separate variable.
-   */
-
-  /* Ignore the queues if we don't have a writable database */
-  if (engine->n_sources == 0 || !engine->sources[0]->writable)
-    return NULL;
-
-  dconf_engine_acquire_sources (engine);
-
-  /* First check read-through */
-  if (read_through)
-    found_key = dconf_engine_find_key_in_queue (read_through, key, &value);
-
-  /* Next pending/in-flight */
-  if (!found_key)
-    {
-      dconf_engine_lock_queues (engine);
-
-      /* Check the pending queue first because those were submitted
-       * more recently.
-       */
-      found_key = dconf_engine_find_key_in_queue (&engine->pending, key, &value) ||
-                  dconf_engine_find_key_in_queue (&engine->in_flight, key, &value);
-
-      dconf_engine_unlock_queues (engine);
-    }
+  if (~flags & DCONF_READ_USER_VALUE)
+    for (i = lock_level; value == NULL && i < engine->n_sources; i++)
+      {
+        if (engine->sources[i]->values == NULL)
+          continue;
 
-  /* Finally, check the user database */
-  if (!found_key && engine->sources[0]->values)
-    value = gvdb_table_get_value (engine->sources[0]->values, key);
+        if ((value = gvdb_table_get_value (engine->sources[i]->values, key)))
+          break;
+      }
 
   dconf_engine_release_sources (engine);
 
diff --git a/engine/dconf-engine.h b/engine/dconf-engine.h
index 5c34bbd..e166d53 100644
--- a/engine/dconf-engine.h
+++ b/engine/dconf-engine.h
@@ -21,6 +21,7 @@
 #define __dconf_engine_h__
 
 #include "../common/dconf-changeset.h"
+#include "../common/dconf-enums.h"
 
 #include <gio/gio.h>
 
@@ -118,11 +119,7 @@ gchar **                dconf_engine_list_locks                         (DConfEn
 
 G_GNUC_INTERNAL
 GVariant *              dconf_engine_read                               (DConfEngine             *engine,
-                                                                         GQueue                  
*read_through,
-                                                                         const gchar             *key);
-
-G_GNUC_INTERNAL
-GVariant *              dconf_engine_read_user_value                    (DConfEngine             *engine,
+                                                                         DConfReadFlags           flags,
                                                                          GQueue                  
*read_through,
                                                                          const gchar             *key);
 
diff --git a/gsettings/dconfsettingsbackend.c b/gsettings/dconfsettingsbackend.c
index 1b9aa16..0e96f5e 100644
--- a/gsettings/dconfsettingsbackend.c
+++ b/gsettings/dconfsettingsbackend.c
@@ -53,12 +53,12 @@ dconf_settings_backend_read (GSettingsBackend   *backend,
       /* Mark the key as having been reset when trying to do the read... */
       read_through = g_queue_new ();
       g_queue_push_tail (read_through, dconf_changeset_new_write (key, NULL));
-      value = dconf_engine_read (dcsb->engine, read_through, key);
+      value = dconf_engine_read (dcsb->engine, DCONF_READ_FLAGS_NONE, read_through, key);
       g_queue_free_full (read_through, (GDestroyNotify) dconf_changeset_unref);
     }
 
   else
-    value = dconf_engine_read (dcsb->engine, NULL, key);
+    value = dconf_engine_read (dcsb->engine, DCONF_READ_FLAGS_NONE, NULL, key);
 
   return value;
 }
@@ -70,7 +70,7 @@ dconf_settings_backend_read_user_value (GSettingsBackend   *backend,
 {
   DConfSettingsBackend *dcsb = (DConfSettingsBackend *) backend;
 
-  return dconf_engine_read_user_value (dcsb->engine, NULL, key);
+  return dconf_engine_read (dcsb->engine, DCONF_READ_USER_VALUE, NULL, key);
 }
 
 static gboolean
diff --git a/tests/engine.c b/tests/engine.c
index 60b4d5e..a804b9a 100644
--- a/tests/engine.c
+++ b/tests/engine.c
@@ -791,7 +791,7 @@ check_read (DConfEngine *engine,
       database_state /= 7;
     }
 
-  value = dconf_engine_read (engine, NULL, "/value");
+  value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "/value");
 
   if (expected != -1)
     {
@@ -829,7 +829,7 @@ check_read (DConfEngine *engine,
             our_expected = 123;
         }
 
-      value = dconf_engine_read (engine, &read_through_queues[i], "/value");
+      value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, &read_through_queues[i], "/value");
 
       if (our_expected != -1)
         {
@@ -862,7 +862,7 @@ check_read (DConfEngine *engine,
    *
    * Note: we do not consider locks.
    */
-  value = dconf_engine_read_user_value (engine, NULL, "/value");
+  value = dconf_engine_read (engine, DCONF_READ_USER_VALUE, NULL, "/value");
   if (value)
     {
       g_assert (first_contents && !(first_contents & 1) && !(source_types & 1));
@@ -892,7 +892,7 @@ check_read (DConfEngine *engine,
        * We see these values regardless of writability.  We do however
        * ensure that we have a writable database as the first one.
        */
-      value = dconf_engine_read_user_value (engine, &read_through_queues[i], "/value");
+      value = dconf_engine_read (engine, DCONF_READ_USER_VALUE, &read_through_queues[i], "/value");
 
       /* If we have no first source, or the first source is non-user
        * than we should always do nothing (since we can't queue changes
@@ -1336,7 +1336,7 @@ test_change_fast (void)
   g_string_set_size (change_log, 0);
 
   /* Verify that the value is set */
-  value = dconf_engine_read (engine, NULL, "/value");
+  value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "/value");
   g_assert_cmpstr (g_variant_get_string (value, NULL), ==, "value");
   g_variant_unref (value);
 
@@ -1349,7 +1349,7 @@ test_change_fast (void)
   g_string_set_size (change_log, 0);
 
   /* Verify that the value became unset due to the failure */
-  value = dconf_engine_read (engine, NULL, "value");
+  value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "value");
   g_assert (value == NULL);
 
   /* Now try a successful write */
@@ -1365,7 +1365,7 @@ test_change_fast (void)
   g_string_set_size (change_log, 0);
 
   /* Verify that the value is set */
-  value = dconf_engine_read (engine, NULL, "/value");
+  value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "/value");
   g_assert_cmpstr (g_variant_get_string (value, NULL), ==, "value");
   g_variant_unref (value);
 
@@ -1378,7 +1378,7 @@ test_change_fast (void)
 
   /* Verify that the value became unset due to the in-flight queue
    * clearing... */
-  value = dconf_engine_read (engine, NULL, "value");
+  value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "value");
   g_assert (value == NULL);
 
   /* Do that all again for a changeset with more than one item */
@@ -1389,7 +1389,7 @@ test_change_fast (void)
   g_assert (success);
   g_assert_cmpstr (change_log->str, ==, "/:2:to-reset,value:nil;");
   g_string_set_size (change_log, 0);
-  value = dconf_engine_read (engine, NULL, "/value");
+  value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "/value");
   g_assert_cmpstr (g_variant_get_string (value, NULL), ==, "value");
   g_variant_unref (value);
   g_test_expect_message ("dconf", G_LOG_LEVEL_WARNING, "failed to commit changes to dconf: something 
failed");
@@ -1398,7 +1398,7 @@ test_change_fast (void)
   g_clear_error (&error);
   g_assert_cmpstr (change_log->str, ==, "/:2:to-reset,value:nil;");
   g_string_set_size (change_log, 0);
-  value = dconf_engine_read (engine, NULL, "value");
+  value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "value");
   g_assert (value == NULL);
   dconf_mock_dbus_assert_no_async ();
   g_assert_cmpstr (change_log->str, ==, "");
@@ -1407,14 +1407,14 @@ test_change_fast (void)
   g_assert (success);
   g_assert_cmpstr (change_log->str, ==, "/:2:to-reset,value:nil;");
   g_string_set_size (change_log, 0);
-  value = dconf_engine_read (engine, NULL, "/value");
+  value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "/value");
   g_assert_cmpstr (g_variant_get_string (value, NULL), ==, "value");
   g_variant_unref (value);
   error = g_error_new_literal (G_FILE_ERROR, G_FILE_ERROR_NOENT, "something failed");
   dconf_mock_dbus_async_reply (g_variant_new ("(s)", "tag"), NULL);
   g_clear_error (&error);
   g_assert_cmpstr (change_log->str, ==, "");
-  value = dconf_engine_read (engine, NULL, "value");
+  value = dconf_engine_read (engine, DCONF_READ_FLAGS_NONE, NULL, "value");
   g_assert (value == NULL);
 
   dconf_engine_unref (engine);