From f754c4e85b666686a374aa31f7d05fff739e9cde Mon Sep 17 00:00:00 2001 From: Ryan Lortie Date: Wed, 17 Apr 2013 09:30:15 -0400 Subject: [PATCH] GDBusConnection: allow async property handling The existing advice in the documentation to "simply" register the "org.freedesktop.DBus.Properties" interface if you want to handle properties asynchronously is pretty unreasonable. If you want to handle this interface you have to deal with all properties for all interfaces on the path, and you have to do all of the checking for yourself. You also have to provide your own introspection data. Introduce a new convention for dealing with properties asynchronously. If the user provides NULL for their get_property() or set_property() functions in the vtable and has properties registered then the properties are sent to the method_call() handler. We get lucky here that this function takes an "interface_name" parameter that we can set to "org.freedesktop.DBus.Properties". We also do the user the favour of setting the GDBusPropertyInfo on the GDBusMethodInvocation for their convenience (for much the same reasons as they might want the already-available GDBusMethodInfo). Add a testcase as well as a bunch of documentation about this new feature. https://bugzilla.gnome.org/show_bug.cgi?id=698375 --- gio/gdbusconnection.c | 53 ++++++++++--- gio/gdbusconnection.h | 35 ++++++++- gio/gdbusmethodinvocation.c | 19 +++++ gio/tests/gdbus-export.c | 176 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 268 insertions(+), 15 deletions(-) diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c index a745be4..8ea5c43 100644 --- a/gio/gdbusconnection.c +++ b/gio/gdbusconnection.c @@ -4272,17 +4272,8 @@ validate_and_maybe_schedule_property_getset (GDBusConnection *connect &property_name, NULL); - - if (is_get) - { - if (vtable == NULL || vtable->get_property == NULL) - goto out; - } - else - { - if (vtable == NULL || vtable->set_property == NULL) - goto out; - } + if (vtable == NULL) + goto out; /* Check that the property exists - if not fail with org.freedesktop.DBus.Error.InvalidArgs */ @@ -4350,6 +4341,32 @@ validate_and_maybe_schedule_property_getset (GDBusConnection *connect g_variant_unref (value); } + /* If the vtable pointer for get_property() resp. set_property() is + * NULL then dispatch the call via the method_call() handler. + */ + if (is_get) + { + if (vtable->get_property == NULL) + { + schedule_method_call (connection, message, registration_id, subtree_registration_id, + interface_info, NULL, property_info, g_dbus_message_get_body (message), + vtable, main_context, user_data); + handled = TRUE; + goto out; + } + } + else + { + if (vtable->set_property == NULL) + { + schedule_method_call (connection, message, registration_id, subtree_registration_id, + interface_info, NULL, property_info, g_dbus_message_get_body (message), + vtable, main_context, user_data); + handled = TRUE; + goto out; + } + } + /* ok, got the property info - call user code in an idle handler */ property_data = g_new0 (PropertyData, 1); property_data->connection = g_object_ref (connection); @@ -4538,9 +4555,21 @@ validate_and_maybe_schedule_property_get_all (GDBusConnection *connec handled = FALSE; - if (vtable == NULL || vtable->get_property == NULL) + if (vtable == NULL) goto out; + /* If the vtable pointer for get_property() is NULL, then dispatch the + * call via the method_call() handler. + */ + if (vtable->get_property == NULL) + { + schedule_method_call (connection, message, registration_id, subtree_registration_id, + interface_info, NULL, NULL, g_dbus_message_get_body (message), + vtable, main_context, user_data); + handled = TRUE; + goto out; + } + /* ok, got the property info - call user in an idle handler */ property_get_all_data = g_new0 (PropertyGetAllData, 1); property_get_all_data->connection = g_object_ref (connection); diff --git a/gio/gdbusconnection.h b/gio/gdbusconnection.h index 1325e37..02caedc 100644 --- a/gio/gdbusconnection.h +++ b/gio/gdbusconnection.h @@ -340,9 +340,38 @@ typedef gboolean (*GDBusInterfaceSetPropertyFunc) (GDBusConnection *conne * Virtual table for handling properties and method calls for a D-Bus * interface. * - * If you want to handle getting/setting D-Bus properties asynchronously, simply - * register an object with the org.freedesktop.DBus.Properties - * D-Bus interface using g_dbus_connection_register_object(). + * Since 2.38, if you want to handle getting/setting D-Bus properties + * asynchronously, give %NULL as your get_property() or set_property() + * function. The D-Bus call will be directed to your @method_call + * function, with the provided @interface_name set to + * "org.freedesktop.DBus.Properties". + * + * The usual checks on the validity of the calls is performed. For + * 'Get' calls, an error is automatically returned if + * the property does not exist or the permissions do not allow access. + * The same checks are performed for 'Set' calls, and + * the provided value is also checked for being the correct type. + * + * For both 'Get' and 'Set' calls, + * the #GDBusMethodInvocation passed to the method_call handler can be + * queried with g_dbus_method_invocation_get_property_info() to get a + * pointer to the #GDBusPropertyInfo of the property. + * + * If you have readable properties specified in your interface info, you + * must ensure that you either provide a non-%NULL @get_property() + * function or provide implementations of both the + * 'Get' and 'GetAll' methods on + * the 'org.freedesktop.DBus.Properties' interface in + * your @method_call function. Note that the required return type of + * the 'Get' call is (v), not the + * type of the property. 'GetAll' expects a return + * value of type a{sv}. + * + * If you have writable properties specified in your interface info, you + * must ensure that you either provide a non-%NULL @set_property() + * function or provide an implementation of the 'Set' + * call. If implementing the call, you must return the value of type + * %G_VARIANT_TYPE_UNIT. * * Since: 2.26 */ diff --git a/gio/gdbusmethodinvocation.c b/gio/gdbusmethodinvocation.c index 6ff0556..ad19d9d 100644 --- a/gio/gdbusmethodinvocation.c +++ b/gio/gdbusmethodinvocation.c @@ -166,6 +166,11 @@ g_dbus_method_invocation_get_object_path (GDBusMethodInvocation *invocation) * * Gets the name of the D-Bus interface the method was invoked on. * + * If this method call is a property Get, Set or GetAll call that has + * been redirected to the method call handler then + * "org.freedesktop.DBus.Properties" will be returned. See + * #GDBusInterfaceVTable for more information. + * * Returns: A string. Do not free, it is owned by @invocation. * * Since: 2.26 @@ -183,6 +188,11 @@ g_dbus_method_invocation_get_interface_name (GDBusMethodInvocation *invocation) * * Gets information about the method call, if any. * + * If this method invocation is a property Get, Set or GetAll call that + * has been redirected to the method call handler then %NULL will be + * returned. See g_dbus_method_invocation_get_property_info() and + * #GDBusInterfaceVTable for more information. + * * Returns: A #GDBusMethodInfo or %NULL. Do not free, it is owned by @invocation. * * Since: 2.26 @@ -201,6 +211,15 @@ g_dbus_method_invocation_get_method_info (GDBusMethodInvocation *invocation) * Gets information about the property that this method call is for, if * any. * + * This will only be set in the case of an invocation in response to a + * property Get or Set call that has been directed to the method call + * handler for an object on account of its property_get() or + * property_set() vtable pointers being unset. + * + * See #GDBusInterfaceVTable for more information. + * + * If the call was GetAll, %NULL will be returned. + * * Returns: (transfer none): a #GDBusPropertyInfo or %NULL * * Since: 2.38 diff --git a/gio/tests/gdbus-export.c b/gio/tests/gdbus-export.c index f4ce6ce..03e6c6c 100644 --- a/gio/tests/gdbus-export.c +++ b/gio/tests/gdbus-export.c @@ -1526,6 +1526,181 @@ test_registered_interfaces (void) /* ---------------------------------------------------------------------------------------------------- */ +static void +test_async_method_call (GDBusConnection *connection, + const gchar *sender, + const gchar *object_path, + const gchar *interface_name, + const gchar *method_name, + GVariant *parameters, + GDBusMethodInvocation *invocation, + gpointer user_data) +{ + const GDBusPropertyInfo *property; + + /* Strictly speaking, this function should also expect to receive + * method calls not on the org.freedesktop.DBus.Properties interface, + * but we don't do any during this testcase, so assert that. + */ + g_assert_cmpstr (interface_name, ==, "org.freedesktop.DBus.Properties"); + g_assert (g_dbus_method_invocation_get_method_info (invocation) == NULL); + + property = g_dbus_method_invocation_get_property_info (invocation); + + /* Do a whole lot of asserts to make sure that invalid calls are still + * getting properly rejected by GDBusConnection and that our + * environment is as we expect it to be. + */ + if (g_str_equal (method_name, "Get")) + { + const gchar *iface_name, *prop_name; + + g_variant_get (parameters, "(&s&s)", &iface_name, &prop_name); + g_assert_cmpstr (iface_name, ==, "org.example.Foo"); + g_assert (property != NULL); + g_assert_cmpstr (prop_name, ==, property->name); + g_assert (property->flags & G_DBUS_PROPERTY_INFO_FLAGS_READABLE); + g_dbus_method_invocation_return_value (invocation, g_variant_new ("(v)", g_variant_new_string (prop_name))); + } + + else if (g_str_equal (method_name, "Set")) + { + const gchar *iface_name, *prop_name; + GVariant *value; + + g_variant_get (parameters, "(&s&sv)", &iface_name, &prop_name, &value); + g_assert_cmpstr (iface_name, ==, "org.example.Foo"); + g_assert (property != NULL); + g_assert_cmpstr (prop_name, ==, property->name); + g_assert (property->flags & G_DBUS_PROPERTY_INFO_FLAGS_WRITABLE); + g_assert (g_variant_is_of_type (value, G_VARIANT_TYPE (property->signature))); + g_dbus_method_invocation_return_value (invocation, g_variant_new ("()")); + g_variant_unref (value); + } + + else if (g_str_equal (method_name, "GetAll")) + { + const gchar *iface_name; + + g_variant_get (parameters, "(&s)", &iface_name); + g_assert_cmpstr (iface_name, ==, "org.example.Foo"); + g_assert (property == NULL); + g_dbus_method_invocation_return_value (invocation, + g_variant_new_parsed ("({ 'PropertyUno': < 'uno' >," + " 'NotWritable': < 'notwrite' > },)")); + } + + else + g_assert_not_reached (); +} + +static gint outstanding_cases; + +static void +ensure_result_cb (GObject *source, + GAsyncResult *result, + gpointer user_data) +{ + GDBusConnection *connection = G_DBUS_CONNECTION (source); + GVariant *reply; + + reply = g_dbus_connection_call_finish (connection, result, NULL); + + if (user_data == NULL) + { + /* Expected an error */ + g_assert (reply == NULL); + } + else + { + /* Expected a reply of a particular format. */ + gchar *str; + + g_assert (reply != NULL); + str = g_variant_print (reply, TRUE); + g_assert_cmpstr (str, ==, (const gchar *) user_data); + g_free (str); + + g_variant_unref (reply); + } + + g_assert_cmpint (outstanding_cases, >, 0); + outstanding_cases--; +} + +static void +test_async_case (GDBusConnection *connection, + const gchar *expected_reply, + const gchar *method, + const gchar *format_string, + ...) +{ + va_list ap; + + va_start (ap, format_string); + + g_dbus_connection_call (connection, g_dbus_connection_get_unique_name (connection), "/foo", + "org.freedesktop.DBus.Properties", method, g_variant_new_va (format_string, NULL, &ap), + NULL, G_DBUS_CALL_FLAGS_NONE, -1, NULL, ensure_result_cb, (gpointer) expected_reply); + + va_end (ap); + + outstanding_cases++; +} + +static void +test_async_properties (void) +{ + GError *error = NULL; + guint registration_id; + static const GDBusInterfaceVTable vtable = { + test_async_method_call, NULL, NULL + }; + + c = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, &error); + g_assert_no_error (error); + g_assert (c != NULL); + + registration_id = g_dbus_connection_register_object (c, + "/foo", + (GDBusInterfaceInfo *) &foo_interface_info, + &vtable, NULL, NULL, &error); + g_assert_no_error (error); + g_assert (registration_id); + + test_async_case (c, NULL, "random", "()"); + + /* Test a variety of error cases */ + test_async_case (c, NULL, "Get", "(si)", "wrong signature", 5); + test_async_case (c, NULL, "Get", "(ss)", "org.example.WrongInterface", "zzz"); + test_async_case (c, NULL, "Get", "(ss)", "org.example.Foo", "NoSuchProperty"); + test_async_case (c, NULL, "Get", "(ss)", "org.example.Foo", "NotReadable"); + + test_async_case (c, NULL, "Set", "(si)", "wrong signature", 5); + test_async_case (c, NULL, "Set", "(ssv)", "org.example.WrongInterface", "zzz", g_variant_new_string ("")); + test_async_case (c, NULL, "Set", "(ssv)", "org.example.Foo", "NoSuchProperty", g_variant_new_string ("")); + test_async_case (c, NULL, "Set", "(ssv)", "org.example.Foo", "NotWritable", g_variant_new_string ("")); + test_async_case (c, NULL, "Set", "(ssv)", "org.example.Foo", "PropertyUno", g_variant_new_object_path ("/wrong")); + + test_async_case (c, NULL, "GetAll", "(si)", "wrong signature", 5); + test_async_case (c, NULL, "GetAll", "(s)", "org.example.WrongInterface"); + + /* Now do the proper things */ + test_async_case (c, "(<'PropertyUno'>,)", "Get", "(ss)", "org.example.Foo", "PropertyUno"); + test_async_case (c, "(<'NotWritable'>,)", "Get", "(ss)", "org.example.Foo", "NotWritable"); + test_async_case (c, "()", "Set", "(ssv)", "org.example.Foo", "PropertyUno", g_variant_new_string ("")); + test_async_case (c, "()", "Set", "(ssv)", "org.example.Foo", "NotReadable", g_variant_new_string ("")); + test_async_case (c, "({'PropertyUno': <'uno'>, 'NotWritable': <'notwrite'>},)", "GetAll", "(s)", "org.example.Foo"); + + while (outstanding_cases) + g_main_context_iteration (NULL, TRUE); + + g_dbus_connection_unregister_object (c, registration_id); + g_object_unref (c); +} + +/* ---------------------------------------------------------------------------------------------------- */ + int main (int argc, char *argv[]) @@ -1541,6 +1716,7 @@ main (int argc, g_test_add_func ("/gdbus/object-registration", test_object_registration); g_test_add_func ("/gdbus/registered-interfaces", test_registered_interfaces); + g_test_add_func ("/gdbus/async-properties", test_async_properties); /* TODO: check that we spit out correct introspection data */ /* TODO: check that registering a whole subtree works */ -- 2.7.4