TpChannelDispatchOperation: make the external API GVariant-based

The implementation is really simplistic, and copies everything into
dbus-glib data types.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=77139
Reviewed-by: Xavier Claessens

5 files changed, 72 insertions, 37 deletions

diff --git a/telepathy-glib/base-client.c b/telepathy-glib/base-client.c
index d7252e89b..686e6c0d9 100644
--- a/telepathy-glib/base-client.c
+++ b/telepathy-glib/base-client.c
@@ -1899,7 +1899,9 @@ _tp_base_client_add_dispatch_operation (TpSvcClientApprover *iface,
 
   dispatch_operation =
       _tp_client_factory_ensure_channel_dispatch_operation (
-          self->priv->factory, dispatch_operation_path, properties, &error);
+          self->priv->factory, dispatch_operation_path,
+          tp_asv_to_vardict (properties), &error);
+
   if (dispatch_operation == NULL)
     {
       DEBUG ("Failed to create TpChannelDispatchOperation: %s", error->message);
diff --git a/telepathy-glib/channel-dispatch-operation.c b/telepathy-glib/channel-dispatch-operation.c
index 62b7f0c94..ef87ca3db 100644
--- a/telepathy-glib/channel-dispatch-operation.c
+++ b/telepathy-glib/channel-dispatch-operation.c
@@ -198,7 +198,9 @@ tp_channel_dispatch_operation_get_property (GObject *object,
       break;
 
     case PROP_CDO_PROPERTIES:
-      g_value_set_boxed (value, self->priv->immutable_properties);
+      /* consume floating ref */
+      g_value_set_variant (value,
+          tp_asv_to_vardict (self->priv->immutable_properties));
       break;
 
     default:
@@ -374,14 +376,19 @@ tp_channel_dispatch_operation_set_property (GObject *object,
 
       case PROP_CDO_PROPERTIES:
         {
-          GHashTable *asv = g_value_get_boxed (value);
+          GVariant *vardict = g_value_get_variant (value);
+          GHashTable *asv;
 
-          if (asv == NULL)
+          if (vardict == NULL)
             return;
 
+          /* This implementation is pretty stupid and does a lot of copying:
+           * we still work in terms of GHashTable<string,GValue> internally. */
+          asv = tp_asv_from_vardict (vardict);
           tp_g_hash_table_update (self->priv->immutable_properties,
               asv, (GBoxedCopyFunc) g_strdup,
               (GBoxedCopyFunc) tp_g_value_slice_dup);
+          g_hash_table_unref (asv);
         }
         break;
 
@@ -711,20 +718,19 @@ tp_channel_dispatch_operation_class_init (TpChannelDispatchOperationClass *klass
    * TpChannelDispatchOperation:cdo-properties:
    *
    * The immutable D-Bus properties of this ChannelDispatchOperation,
-   * represented by a #GHashTable where the keys are D-Bus
-   * interface name + "." + property name, and the values are #GValue instances.
+   * represented by a #GVariant of type %G_VARIANT_TYPE_VARDICT
+   * where the keys are D-Bus
+   * interface name + "." + property name.
    *
    * Read-only except during construction. If this is not provided
    * during construction, it is not guaranteed to be set until
    * tp_proxy_prepare_async() has finished preparing
    * %TP_CHANNEL_DISPATCH_OPERATION_FEATURE_CORE.
-   *
-   * Since: 0.11.5
    */
-  param_spec = g_param_spec_boxed ("cdo-properties",
+  param_spec = g_param_spec_variant ("cdo-properties",
       "Immutable D-Bus properties",
-      "A map D-Bus interface + \".\" + property name => GValue",
-      TP_HASH_TYPE_QUALIFIED_PROPERTY_VALUE_MAP,
+      "A map D-Bus interface + \".\" + property name => value",
+      G_VARIANT_TYPE_VARDICT, NULL,
       G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS);
   g_object_class_install_property (object_class,
       PROP_CDO_PROPERTIES, param_spec);
@@ -737,23 +743,29 @@ tp_channel_dispatch_operation_class_init (TpChannelDispatchOperationClass *klass
 TpChannelDispatchOperation *
 _tp_channel_dispatch_operation_new (TpClientFactory *factory,
     const gchar *object_path,
-    GHashTable *immutable_properties,
+    GVariant *immutable_properties,
     GError **error)
 {
-  TpChannelDispatchOperation *self;
-  gchar *unique_name;
+  TpChannelDispatchOperation *self = NULL;
+  gchar *unique_name = NULL;
 
   g_return_val_if_fail (factory != NULL, NULL);
+  g_return_val_if_fail (immutable_properties == NULL ||
+      g_variant_is_of_type (immutable_properties,
+        G_VARIANT_TYPE_VARDICT), NULL);
   g_return_val_if_fail (object_path != NULL, NULL);
   g_return_val_if_fail (error == NULL || *error == NULL, NULL);
 
+  if (immutable_properties != NULL)
+    g_variant_ref_sink (immutable_properties);
+
   if (!tp_dbus_check_valid_object_path (object_path, error))
-    return NULL;
+    goto finally;
 
   if (!_tp_dbus_connection_get_name_owner (
       tp_client_factory_get_dbus_connection (factory), -1,
       TP_CHANNEL_DISPATCHER_BUS_NAME, &unique_name, error))
-    return NULL;
+    goto finally;
 
   self = TP_CHANNEL_DISPATCH_OPERATION (g_object_new (
         TP_TYPE_CHANNEL_DISPATCH_OPERATION,
@@ -763,6 +775,10 @@ _tp_channel_dispatch_operation_new (TpClientFactory *factory,
         "factory", factory,
         NULL));
 
+finally:
+  if (immutable_properties != NULL)
+    g_variant_unref (immutable_properties);
+
   g_free (unique_name);
 
   return self;
diff --git a/telepathy-glib/client-factory-internal.h b/telepathy-glib/client-factory-internal.h
index 9466d275b..f62ad7a68 100644
--- a/telepathy-glib/client-factory-internal.h
+++ b/telepathy-glib/client-factory-internal.h
@@ -38,7 +38,7 @@ TpChannelRequest *_tp_client_factory_ensure_channel_request (
 TpChannelDispatchOperation *
 _tp_client_factory_ensure_channel_dispatch_operation (TpClientFactory *self,
     const gchar *object_path,
-    GHashTable *immutable_properties,
+    GVariant *immutable_properties,
     GError **error);
 
 TpAccount *_tp_account_new (TpClientFactory *factory,
@@ -65,7 +65,7 @@ TpChannelRequest *_tp_channel_request_new (
 TpChannelDispatchOperation *_tp_channel_dispatch_operation_new (
     TpClientFactory *factory,
     const gchar *object_path,
-    GHashTable *immutable_properties,
+    GVariant *immutable_properties,
     GError **error);
 
 TpProtocol * _tp_protocol_new (TpClientFactory *factory,
diff --git a/telepathy-glib/client-factory.c b/telepathy-glib/client-factory.c
index 0d6246777..eda138d38 100644
--- a/telepathy-glib/client-factory.c
+++ b/telepathy-glib/client-factory.c
@@ -1461,8 +1461,9 @@ _tp_client_factory_ensure_channel_request (TpClientFactory *self,
  * _tp_client_factory_ensure_channel_dispatch_operation:
  * @self: a #TpClientFactory object
  * @object_path: the object path of a channel dispatch operation
- * @immutable_properties: (transfer none) (element-type utf8 GObject.Value):
- *  the immutable properties of the channel dispatch operation
+ * @immutable_properties: (allow-none): the immutable properties of the channel
+ *  dispatch operation as %G_VARIANT_TYPE_VARDICT; ownership is taken
+ *  if floating
  * @error: Used to raise an error if @object_path is not valid
  *
  * Returns a #TpChannelDispatchOperation for @object_path.
@@ -1482,22 +1483,35 @@ TpChannelDispatchOperation *
 _tp_client_factory_ensure_channel_dispatch_operation (
     TpClientFactory *self,
     const gchar *object_path,
-    GHashTable *immutable_properties,
+    GVariant *immutable_properties,
     GError **error)
 {
-  TpChannelDispatchOperation *dispatch;
+  TpChannelDispatchOperation *dispatch = NULL;
 
   g_return_val_if_fail (TP_IS_CLIENT_FACTORY (self), NULL);
   g_return_val_if_fail (g_variant_is_object_path (object_path), NULL);
+  g_return_val_if_fail (immutable_properties == NULL ||
+      g_variant_is_of_type (immutable_properties,
+        G_VARIANT_TYPE_VARDICT), NULL);
+
+  if (immutable_properties != NULL)
+    g_variant_ref_sink (immutable_properties);
 
   dispatch = lookup_proxy (self, object_path);
   if (dispatch != NULL)
-    return g_object_ref (dispatch);
+    {
+      g_object_ref (dispatch);
+      goto finally;
+    }
 
   dispatch = _tp_channel_dispatch_operation_new (self, object_path,
       immutable_properties, error);
   insert_proxy (self, dispatch);
 
+finally:
+  if (immutable_properties != NULL)
+    g_variant_unref (immutable_properties);
+
   return dispatch;
 }
 
diff --git a/tests/dbus/channel-dispatch-operation.c b/tests/dbus/channel-dispatch-operation.c
index 2f2e2e854..67b004a58 100644
--- a/tests/dbus/channel-dispatch-operation.c
+++ b/tests/dbus/channel-dispatch-operation.c
@@ -179,26 +179,29 @@ teardown_services (Test *test,
 static TpChannelDispatchOperation *
 dispatch_operation_new (GDBusConnection *bus_connection,
     const gchar *object_path,
-    GHashTable *immutable_properties,
+    GHashTable *asv,
     GError **error)
 {
   TpChannelDispatchOperation *self;
   TpClientFactory *factory;
+  GVariant *immutable_properties;
 
   if (!tp_dbus_check_valid_object_path (object_path, error))
     return NULL;
 
-  if (immutable_properties == NULL)
-    immutable_properties = tp_asv_new (NULL, NULL);
+  if (asv == NULL)
+    immutable_properties = g_variant_new ("a{sv}", NULL);
   else
-    g_hash_table_ref (immutable_properties);
+    immutable_properties = tp_asv_to_vardict (asv);
+
+  g_variant_ref_sink (immutable_properties);
 
   factory = tp_client_factory_new (bus_connection);
   self = _tp_client_factory_ensure_channel_dispatch_operation (factory,
       object_path, immutable_properties, error);
 
   g_object_unref (factory);
-  g_hash_table_unref (immutable_properties);
+  g_variant_unref (immutable_properties);
 
   return self;
 }
@@ -306,7 +309,7 @@ check_immutable_properties (Test *test)
   TpConnection *conn;
   TpAccount *account;
   GStrv possible_handlers;
-  GHashTable *immutable_props;
+  GVariant *immutable_props;
 
   g_object_get (test->cdo,
       "connection", &conn,
@@ -347,16 +350,16 @@ check_immutable_properties (Test *test)
         POSSIBLE_HANDLERS[0]));
 
   /* immutable properties */
-  g_assert (tp_asv_get_object_path (immutable_props,
+  g_assert (tp_vardict_get_object_path (immutable_props,
         TP_PROP_CHANNEL_DISPATCH_OPERATION_CONNECTION) != NULL);
-  g_assert (tp_asv_get_object_path (immutable_props,
+  g_assert (tp_vardict_get_object_path (immutable_props,
         TP_PROP_CHANNEL_DISPATCH_OPERATION_ACCOUNT) != NULL);
-  g_assert (tp_asv_get_strv (immutable_props,
-        TP_PROP_CHANNEL_DISPATCH_OPERATION_POSSIBLE_HANDLERS) != NULL);
-  g_assert (tp_asv_get_strv (immutable_props,
-        TP_PROP_CHANNEL_DISPATCH_OPERATION_INTERFACES) != NULL);
-  g_assert_cmpuint (g_hash_table_size (immutable_props), ==, 6);
-  g_hash_table_unref (immutable_props);
+  g_assert (g_variant_lookup (immutable_props,
+        TP_PROP_CHANNEL_DISPATCH_OPERATION_POSSIBLE_HANDLERS, "@as", NULL));
+  g_assert (g_variant_lookup (immutable_props,
+        TP_PROP_CHANNEL_DISPATCH_OPERATION_INTERFACES, "@as", NULL));
+  g_assert_cmpuint (g_variant_n_children (immutable_props), ==, 6);
+  g_variant_unref (immutable_props);
 }
 
 static void