Re: GDBus API Questions; was: GDBus/GVariant plans for next GLib release

[Mikkel Kamstrup Erlandsen <mikkel kamstrup gmail com>]

2009/10/28 Mikkel Kamstrup Erlandsen <mikkel kamstrup gmail com>:
> 2009/10/28 Mikkel Kamstrup Erlandsen <mikkel kamstrup gmail com>:
>> 2009/10/27 Mikkel Kamstrup Erlandsen <mikkel kamstrup gmail com>:
>>> 2009/10/27 David Zeuthen <david fubar dk>:
>>>> Hey Mikkel,
>>>>
>>>> On Mon, 2009-10-26 at 23:52 +0100, Mikkel Kamstrup Erlandsen wrote:
>>>>> > I just looked over the newly introduced
>>>>> > g_dbus_connection_register_subtree() and related data structures, and
>>>>> > I think it will fit very nicely with what I am going to need. All in
>>>>> > all it looks really sweet, good work.
>>>>> >
>>>>> > One thing though is that as I read it objects in a subtree must be
>>>>> > known before method calls are accepted to them? For my use case in
>>>>> > Zeitgeist I was hoping that I could completely get rid of a "Manager"
>>>>> > type of interface, and just implicitly create objects in the tree
>>>>> > whenever calls where made to them. This does not look possible as it
>>>>> > stands?
>>>>> >
>>>>> > Maybe allowing '*' as a wildcard node name in the subtree enumeration function?
>>>>
>>>> I'm actually a bit wary of introducing this kind of functionality -
>>>> mainly, I guess, because it screws with the notion that a D-Bus service
>>>> export a set of objects. In particular it makes it hard to
>>>> debug/introspect the service - for example, in extreme abuses of such a
>>>> feature (not the use-case you are suggesting though), you can't really
>>>> use e.g. d-feet to get an idea of what kind of objects are exported and
>>>> known by the service.
>>>>
>>>> The subtree functionality is really just for performance hacks - the
>>>> intended use is to avoid creating a huge amount of objects. For example,
>>>> one use case is export the subtree /org/foo/Project/processes/<pid>
>>>> where <pid> is, say, a UNIX process id. With the subtree handler, no
>>>> object creation over is necessary.
>>>>
>>>> Anyway, your original use case does seem sound and reasonable - it
>>>> reduces overhead insofar that the client saves a round-trip to a
>>>> hypothetical Manager.CreateObject() method that would be needed if we
>>>> didn't have this. It does make it less intuitive insofar that remote
>>>> object creation is this magical thing with automatically appearing
>>>> nodes... but I guess that's fine.
>>>
>>> Not to mention the built in race condition you have if the exported
>>> objects can also be deleted via the manager. All client apps would
>>> have to precede all calls to an object by a Create() message (and it
>>> would still be racy). Anyway, enough about this :-) I think it is
>>> probably a fragment of my REST/web background where such patterns are
>>> more common.
>>>
>>>>> I had a stab at this myself. The wildcard idea seemed like a bad one,
>>>>> so I instead added another gboolean param to
>>>>> g_dbus_connection_register_subtree(), @is_dynamic.
>>>>>
>>>>> If is_dynamic is TRUE then objects need not be in the enumerated list
>>>>> of objects in order to be introspected and dispatched. Pretty simple.
>>>>>
>>>>> No matter the simplicity I still managed to screw up one of the unit
>>>>> tests. I will fix it and add some specific tests for the dynamic case
>>>>> if you give the "go" for this David.
>>>>
>>>> Sounds good to me. I'd prefer a GDBusSubtreeFlags flag enumeration with
>>>> a G_DBUS_SUBTREE_FLAGS_DISPATCH_TO_UNENUMERATED_NODES member instead of
>>>> @is_dynamic. Just for future proofing and all. Maybe it would also be
>>>> nice to pass a "gboolean enumerated" to @introspect and @dispatch that
>>>> indicates whether the node was enumerated (or not).
>>>
>>> That sounds like a better idea. I'll have another stab at this tonight
>>> if the kids allow me :-)
>>
>> They did :-)
>>
>> Attached is a series of patches (0001 should be identical to my
>> previous) implementing what you describe, except adding the gboolean
>> enumerated arg to @introspect and @dispatch. I will get around to
>> that.
>>
>> The failing unit tests I described was really just me b0rking up the
>> linking between some installed gdbus components and my devel version.
>> I have them running now[1] and I added some testing for the dynamic
>> objects as well (see 0003).
>
> Some more tests to go with the other patches attached.

And of course the attachment :-S

-- 
Cheers,
Mikkel

From 9fe0126b9d74dd260e1d3ef3969afe94f8b92fa9 Mon Sep 17 00:00:00 2001
From: Mikkel Kamstrup Erlandsen <mikkel kamstrup gmail com>
Date: Wed, 28 Oct 2009 20:01:52 +0100
Subject: [PATCH 4/4] test calls to non-existing objects

Add a test for calls to a non-existing object which is then dynamically
spawned on the server
---
 gdbus/tests/export.c |   75 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 74 insertions(+), 1 deletions(-)

diff --git a/gdbus/tests/export.c b/gdbus/tests/export.c
index be911e2..fabc0c3 100644
--- a/gdbus/tests/export.c
+++ b/gdbus/tests/export.c
@@ -375,6 +375,66 @@ count_interfaces (GDBusConnection *c,
   return ret;
 }
 
+static void
+dyna_create_callback (GDBusProxy   *proxy,
+                      GAsyncResult  *res,
+                      gpointer      user_data)
+{
+  GVariant *result;
+  GError *error;
+
+  error = NULL;
+  result = g_dbus_proxy_invoke_method_finish (proxy,
+                                              res,
+                                              &error);
+  g_assert_no_error (error);
+  g_assert (result != NULL);
+  g_variant_unref (result);
+
+  g_main_loop_quit (loop);
+}
+
+/* Dynamically create @object_name under /foo/dyna */
+static void
+dyna_create (GDBusConnection *c,
+             const gchar     *object_name)
+{
+  GError *error;
+  GDBusProxy *proxy;
+  gchar *object_path;
+
+  object_path = g_strconcat ("/foo/dyna/", object_name, NULL);
+  
+  error = NULL;
+  proxy = g_dbus_proxy_new_sync (c,
+                                 G_TYPE_DBUS_PROXY,
+                                 G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES |
+                                 G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS,
+                                 g_dbus_connection_get_unique_name (c),
+                                 object_path,
+                                 "org.example.Dyna",
+                                 NULL,
+                                 &error);
+  g_assert_no_error (error);
+  g_assert (proxy != NULL);
+
+  /* do this async to avoid libdbus-1 deadlocks */
+  g_dbus_proxy_invoke_method (proxy,
+                              "DynaCyber",
+                              g_variant_new ("()"),
+                              -1,
+                              NULL,
+                              (GAsyncReadyCallback) dyna_create_callback,
+                              NULL);
+  g_main_loop_run (loop);
+
+  g_assert_no_error (error);
+
+  g_object_unref (proxy);
+  g_free (object_path);
+
+  return;
+}
 
 typedef struct
 {
@@ -861,7 +921,7 @@ test_object_registration (void)
   g_strfreev (nodes);
   g_assert_cmpint (count_interfaces (c, "/foo/dyna"), ==, 4);
 
-  /* Install three nodes in the dynamic subtree via the backdoor and
+  /* Install three nodes in the dynamic subtree via the dyna_data backdoor and
    * assert that they show up correctly in the introspection data */
   g_ptr_array_add (dyna_data, "lol");
   g_ptr_array_add (dyna_data, "cat");
@@ -876,6 +936,19 @@ test_object_registration (void)
   g_assert_cmpint (count_interfaces (c, "/foo/dyna/lol"), ==, 4);
   g_assert_cmpint (count_interfaces (c, "/foo/dyna/cat"), ==, 4);
   g_assert_cmpint (count_interfaces (c, "/foo/dyna/cheezburger"), ==, 4);
+
+  /* Call a non-existing object path and assert that it has been created */
+  dyna_create (c, "dynamicallycreated");
+  nodes = get_nodes_at (c, "/foo/dyna");
+  g_assert (nodes != NULL);
+  g_assert_cmpint (g_strv_length (nodes), ==, 4);
+  g_assert_cmpstr (nodes[0], ==, "lol");
+  g_assert_cmpstr (nodes[1], ==, "cat");
+  g_assert_cmpstr (nodes[2], ==, "cheezburger");
+  g_assert_cmpstr (nodes[3], ==, "dynamicallycreated");
+  g_strfreev (nodes);
+  g_assert_cmpint (count_interfaces (c, "/foo/dyna/dynamicallycreated"), ==, 4);
+
   
   /* now check that the object hierarachy is properly generated... yes, it's a bit
    * perverse that we round-trip to the bus to introspect ourselves ;-)
-- 
1.6.3.3