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

@@ -4272,17 +4272,8 @@ validate_and_maybe_schedule_property_getset (GDBusConnection            *connect
                    &property_name,
                    NULL);
 
-
+  if (vtable == NULL)
-  if (is_get)
-    {
-      if (vtable == NULL || vtable->get_property == NULL)
     goto out;
-    }
-  else
-    {
-      if (vtable == NULL || vtable->set_property == 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);

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
+ * Since 2.38, if you want to handle getting/setting D-Bus properties
- * register an object with the <literal>org.freedesktop.DBus.Properties</literal>
+ * asynchronously, give %NULL as your get_property() or set_property()
- * D-Bus interface using g_dbus_connection_register_object().
+ * function.  The D-Bus call will be directed to your @method_call
+ * function, with the provided @interface_name set to
+ * <literal>"org.freedesktop.DBus.Properties"</literal>.
+ *
+ * The usual checks on the validity of the calls is performed.  For
+ * <literal>'Get'</literal> 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 <literal>'Set'</literal> calls, and
+ * the provided value is also checked for being the correct type.
+ *
+ * For both <literal>'Get'</literal> and <literal>'Set'</literal> 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
+ * <literal>'Get'</literal> and <literal>'GetAll'</literal> methods on
+ * the <literal>'org.freedesktop.DBus.Properties'</literal> interface in
+ * your @method_call function.  Note that the required return type of
+ * the <literal>'Get'</literal> call is <literal>(v)</literal>, not the
+ * type of the property.  <literal>'GetAll'</literal> expects a return
+ * value of type <literal>a{sv}<literal>.
+ *
+ * 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 <literal>'Set'</literal>
+ * call.  If implementing the call, you must return the value of type
+ * %G_VARIANT_TYPE_UNIT.
  *
  * Since: 2.26
  */

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

gio/tests/gdbus-export.c

@@ -1524,6 +1524,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
@@ -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 */