Make registering DBus.Properties for the mixin more declarative.

The interfaces now just have a gchar * for the name (this avoids having to
fill in computed values, the quark and GType, in class_init - everything
can be static initialization data) and the mixin walks the interface list
at runtime to tag the interface implementation with its corresponding
interface info.
Also, use different user_data for the getter and setter functions, so those
two functions don't have to be tightly coupled.

20080218122213-53eee-c73c34ee631e11c208e6b3020649001a1ba56c7f.gz

3 files changed, 83 insertions, 37 deletions

diff --git a/telepathy-glib/dbus-properties-mixin.c b/telepathy-glib/dbus-properties-mixin.c
index 37fc50e29..f2d2d0b32 100644
--- a/telepathy-glib/dbus-properties-mixin.c
+++ b/telepathy-glib/dbus-properties-mixin.c
@@ -106,6 +106,7 @@ tp_dbus_properties_mixin_class_init (GObjectClass *cls,
   GType type = G_OBJECT_CLASS_TYPE (cls);
   TpDBusPropertiesMixinClass *mixin;
   TpDBusPropertiesMixinIfaceImpl *iface_impl;
+  GType *interfaces, *iface;
 
   g_return_if_fail (G_IS_OBJECT_CLASS (cls));
   g_return_if_fail (g_type_get_qdata (type, q) == NULL);
@@ -115,22 +116,41 @@ tp_dbus_properties_mixin_class_init (GObjectClass *cls,
 
   g_return_if_fail (mixin->interfaces != NULL);
 
+  interfaces = g_type_interfaces (type, NULL);
+
   for (iface_impl = mixin->interfaces;
-       iface_impl->dbus_interface != 0;
+       iface_impl->name != NULL;
        iface_impl++)
     {
-      TpDBusPropertiesMixinIfaceInfo *iface_info;
+      TpDBusPropertiesMixinIfaceInfo *iface_info = NULL;
       TpDBusPropertiesMixinPropImpl *prop_impl;
+      GQuark iface_quark = g_quark_try_string (iface_impl->name);
 
-      g_return_if_fail (G_TYPE_IS_INTERFACE (iface_impl->svc_interface));
       g_return_if_fail (iface_impl->props != NULL);
 
-      iface_info = tp_svc_interface_get_dbus_properties_info
-          (iface_impl->svc_interface);
+      /* no point bothering if there is no quark for the interface name */
+      if (iface_quark != 0)
+        {
+          for (iface = interfaces; *iface != 0; iface++)
+            {
+              iface_info = tp_svc_interface_get_dbus_properties_info (*iface);
+
+              if (iface_info != NULL &&
+                  iface_info->dbus_interface == iface_quark)
+                break;
+              else
+                iface_info = NULL;
+            }
+        }
+
+      if (iface_info == NULL)
+        {
+          g_critical ("%s tried to implement undefined interface %s",
+              g_type_name (type), iface_impl->name);
+          goto out;
+        }
 
-      g_return_if_fail (iface_info != NULL);
-      g_return_if_fail (iface_impl->dbus_interface ==
-          iface_info->dbus_interface);
+      iface_impl->mixin_priv = iface_info;
 
       for (prop_impl = iface_impl->props; prop_impl->name != NULL; prop_impl++)
         {
@@ -147,7 +167,10 @@ tp_dbus_properties_mixin_class_init (GObjectClass *cls,
                    prop_info++)
                 {
                   if (prop_info->name == name_quark)
-                    prop_impl->mixin_priv = prop_info;
+                    {
+                      prop_impl->mixin_priv = prop_info;
+                      break;
+                    }
                 }
             }
 
@@ -155,11 +178,14 @@ tp_dbus_properties_mixin_class_init (GObjectClass *cls,
             {
               g_critical ("%s tried to implement nonexistent property %s"
                   "on interface %s", g_type_name (type), prop_impl->name,
-                  g_quark_to_string (iface_impl->dbus_interface));
-              return;
+                  iface_impl->name);
+              goto out;
             }
         }
     }
+
+out:
+  g_free (interfaces);
 }
 
 static TpDBusPropertiesMixinIfaceImpl *
@@ -181,6 +207,7 @@ _tp_dbus_properties_mixin_find_iface_impl (GObject *self,
       gpointer offset = g_type_get_qdata (type, q);
       TpDBusPropertiesMixinClass *mixin;
       TpDBusPropertiesMixinIfaceImpl *iface_impl;
+      TpDBusPropertiesMixinIfaceInfo *iface_info;
 
       if (offset == NULL)
         continue;
@@ -189,10 +216,12 @@ _tp_dbus_properties_mixin_find_iface_impl (GObject *self,
           G_OBJECT_GET_CLASS (self), GPOINTER_TO_SIZE (offset));
 
       for (iface_impl = mixin->interfaces;
-           iface_impl->dbus_interface != 0;
+           iface_impl->name != NULL;
            iface_impl++)
         {
-          if (iface_impl->dbus_interface == iface_quark)
+          iface_info = iface_impl->mixin_priv;
+
+          if (iface_info->dbus_interface == iface_quark)
             return iface_impl;
         }
     }
@@ -233,6 +262,7 @@ _tp_dbus_properties_mixin_get (TpSvcDBusProperties *iface,
 {
   GObject *self = G_OBJECT (iface);
   TpDBusPropertiesMixinIfaceImpl *iface_impl;
+  TpDBusPropertiesMixinIfaceInfo *iface_info;
   TpDBusPropertiesMixinPropImpl *prop_impl;
   TpDBusPropertiesMixinPropInfo *prop_info;
   GValue value = { 0 };
@@ -243,6 +273,8 @@ _tp_dbus_properties_mixin_get (TpSvcDBusProperties *iface,
   if (iface_impl == NULL)
     return;
 
+  iface_info = iface_impl->mixin_priv;
+
   prop_impl = _tp_dbus_properties_mixin_find_prop_impl (iface_impl,
       property_name, context);
 
@@ -261,8 +293,8 @@ _tp_dbus_properties_mixin_get (TpSvcDBusProperties *iface,
     }
 
   g_value_init (&value, prop_info->type);
-  iface_impl->getter (self, iface_impl->dbus_interface,
-      prop_info->name, &value, prop_impl->data);
+  iface_impl->getter (self, iface_info->dbus_interface,
+      prop_info->name, &value, prop_impl->getter_data);
   tp_svc_dbus_properties_return_from_get (context, &value);
   g_value_unset (&value);
 }
@@ -274,6 +306,7 @@ _tp_dbus_properties_mixin_get_all (TpSvcDBusProperties *iface,
 {
   GObject *self = G_OBJECT (iface);
   TpDBusPropertiesMixinIfaceImpl *iface_impl;
+  TpDBusPropertiesMixinIfaceInfo *iface_info;
   TpDBusPropertiesMixinPropImpl *prop_impl;
   /* no key destructor needed - the keys are immortal */
   GHashTable *values = g_hash_table_new_full (g_str_hash, g_str_equal, NULL,
@@ -285,6 +318,8 @@ _tp_dbus_properties_mixin_get_all (TpSvcDBusProperties *iface,
   if (iface_impl == NULL)
     return;
 
+  iface_info = iface_impl->mixin_priv;
+
   for (prop_impl = iface_impl->props;
        prop_impl->name != NULL;
        prop_impl++)
@@ -296,8 +331,8 @@ _tp_dbus_properties_mixin_get_all (TpSvcDBusProperties *iface,
         continue;
 
       value = tp_g_value_slice_new (prop_info->type);
-      iface_impl->getter (self, iface_impl->dbus_interface,
-          prop_info->name, value, prop_impl->data);
+      iface_impl->getter (self, iface_info->dbus_interface,
+          prop_info->name, value, prop_impl->getter_data);
       g_hash_table_insert (values, (gchar *) prop_impl->name, value);
     }
 
@@ -314,6 +349,7 @@ _tp_dbus_properties_mixin_set (TpSvcDBusProperties *iface,
 {
   GObject *self = G_OBJECT (iface);
   TpDBusPropertiesMixinIfaceImpl *iface_impl;
+  TpDBusPropertiesMixinIfaceInfo *iface_info;
   TpDBusPropertiesMixinPropImpl *prop_impl;
   TpDBusPropertiesMixinPropInfo *prop_info;
   GValue copy = { 0 };
@@ -325,6 +361,8 @@ _tp_dbus_properties_mixin_set (TpSvcDBusProperties *iface,
   if (iface_impl == NULL)
     return;
 
+  iface_info = iface_impl->mixin_priv;
+
   prop_impl = _tp_dbus_properties_mixin_find_prop_impl (iface_impl,
       property_name, context);
 
@@ -359,8 +397,8 @@ _tp_dbus_properties_mixin_set (TpSvcDBusProperties *iface,
         }
     }
 
-  if (iface_impl->setter (self, iface_impl->dbus_interface,
-        prop_info->name, value, prop_impl->data, &error))
+  if (iface_impl->setter (self, iface_info->dbus_interface,
+        prop_info->name, value, prop_impl->setter_data, &error))
     {
       tp_svc_dbus_properties_return_from_get (context, value);
     }
diff --git a/telepathy-glib/dbus-properties-mixin.h b/telepathy-glib/dbus-properties-mixin.h
index d51f9503c..16f3d1759 100644
--- a/telepathy-glib/dbus-properties-mixin.h
+++ b/telepathy-glib/dbus-properties-mixin.h
@@ -54,28 +54,32 @@ void tp_svc_interface_set_dbus_properties_info (GType g_interface,
 /* ---- Concrete implementation (in GObject subclasses) ------------- */
 
 typedef void (*TpDBusPropertiesMixinGetter) (GObject *object,
-    GQuark interface, GQuark name, GValue *value, gpointer user_data);
+    GQuark interface, GQuark name, GValue *value, gpointer getter_data);
 
 typedef gboolean (*TpDBusPropertiesMixinSetter) (GObject *object,
-    GQuark interface, GQuark name, const GValue *value, gpointer user_data,
+    GQuark interface, GQuark name, const GValue *value, gpointer setter_data,
     GError **error);
 
 typedef struct {
     const gchar *name;
-    gpointer data;
-    gpointer _1;
+    gpointer getter_data;
+    gpointer setter_data;
+    /*<private>*/
+    GCallback _1;
+    GCallback _2;
     gpointer mixin_priv;
 } TpDBusPropertiesMixinPropImpl;
 
 typedef struct {
-    GQuark dbus_interface;
-    GType svc_interface;
+    const gchar *name;
     TpDBusPropertiesMixinGetter getter;
     TpDBusPropertiesMixinSetter setter;
     TpDBusPropertiesMixinPropImpl *props;
-    gpointer _1;
-    gpointer _2;
-    gpointer _3;
+    /*<private>*/
+    GCallback _1;
+    GCallback _2;
+    GCallback _3;
+    gpointer mixin_priv;
 } TpDBusPropertiesMixinIfaceImpl;
 
 struct _TpDBusPropertiesMixinClass {
diff --git a/tests/dbus/properties.c b/tests/dbus/properties.c
index d7ee0dda7..18c96f6a6 100644
--- a/tests/dbus/properties.c
+++ b/tests/dbus/properties.c
@@ -8,6 +8,7 @@
 #include <telepathy-glib/debug.h>
 #include <telepathy-glib/proxy.h>
 #include <telepathy-glib/svc-generic.h>
+#include <telepathy-glib/util.h>
 
 #include "_gen/svc.h"
 #include "tests/myassert.h"
@@ -57,6 +58,9 @@ prop_getter (GObject *object,
              GValue *value,
              gpointer user_data)
 {
+  MYASSERT (!tp_strdiff (user_data, "read") ||
+            !tp_strdiff (user_data, "full-access"),
+            "%s", (gchar *) user_data);
   g_value_set_uint (value, 42);
 }
 
@@ -69,6 +73,9 @@ prop_setter (GObject *object,
              GError **error)
 {
   MYASSERT (G_VALUE_HOLDS_UINT (value), "");
+  MYASSERT (!tp_strdiff (user_data, "FULL ACCESS") ||
+            !tp_strdiff (user_data, "BLACK HOLE"),
+            "%s", (gchar *) user_data);
   MYASSERT (g_value_get_uint (value) == 57, "%u", g_value_get_uint (value));
   return TRUE;
 }
@@ -77,20 +84,17 @@ static void
 test_properties_class_init (TestPropertiesClass *cls)
 {
   static TpDBusPropertiesMixinPropImpl with_properties_props[] = {
-        { "ReadOnly", "read" },
-        { "ReadWrite", "full-access" },
-        { "WriteOnly", "black-hole" },
+        { "ReadOnly", "read", "READ" },
+        { "ReadWrite", "full-access", "FULL ACCESS" },
+        { "WriteOnly", "black-hole", "BLACK HOLE" },
         { NULL }
   };
   static TpDBusPropertiesMixinIfaceImpl interfaces[] = {
-      { 0, 0, prop_getter, prop_setter, with_properties_props },
-      { 0 }
+      { "com.example.WithProperties", prop_getter, prop_setter,
+        with_properties_props },
+      { NULL }
   };
 
-  interfaces[0].dbus_interface =
-      g_quark_from_static_string ("com.example.WithProperties");
-  interfaces[0].svc_interface = TEST_TYPE_SVC_WITH_PROPERTIES;
-
   cls->props.interfaces = interfaces;
 
   tp_dbus_properties_mixin_class_init (G_OBJECT_CLASS (cls),