[glibmm] Glib::Variant: Improve handling of object paths and signatures

[Kjell Ahlstedt <kjellahl src gnome org>]

commit 6641ac99f95e02c7ef9e4259f6b6b8af1dcbb3f4
Author: Kjell Ahlstedt <kjell ahlstedt bredband net>
Date:   Mon Apr 13 18:57:26 2015 +0200

    Glib::Variant: Improve handling of object paths and signatures
    
    * glib/src/variant.[ccg|hg]: Add VariantBase::is_castable_to() and
    VariantContainerBase::get_iter(). Accept casts of complicated types
    containing object paths and DBus type signatures to Variant<> types
    containing Glib::ustring and std::string. Change some get(), get_child()
    and get_iter() methods similarly.
    * tests/glibmm_variant/main.cc: Add test_dynamic_cast_composite_types().
    Bug #747508.

 glib/src/variant.ccg         |  167 ++++++++++++++++++++++++++++--------------
 glib/src/variant.hg          |   94 +++++++++++-------------
 tests/glibmm_variant/main.cc |   59 +++++++++++++++
 3 files changed, 213 insertions(+), 107 deletions(-)
---
diff --git a/glib/src/variant.ccg b/glib/src/variant.ccg
index a7c5352..fe83540 100644
--- a/glib/src/variant.ccg
+++ b/glib/src/variant.ccg
@@ -56,6 +56,53 @@ void VariantBase::byteswap(VariantBase& result) const
   result.init(g_value); // g_value is already referenced.
 }
 
+bool VariantBase::is_castable_to(const VariantType& supertype) const
+{
+  const std::string subtype_string = get_type_string();
+  const std::string supertype_string = supertype.get_string();
+
+  // The following code is similar to g_variant_type_is_subtype_of(), with
+  // these differences:
+  // - Both types are assumed to be definite types (no indefinite types,
+  //   no 'r', '?' or '*').
+  // - VARIANT_TYPE_OBJECT_PATH (o) and VARIANT_TYPE_SIGNATURE (g) can be cast to
+  //   VARIANT_TYPE_STRING (s), the type of Glib::ustring.
+  // - VARIANT_TYPE_STRING (s), VARIANT_TYPE_OBJECT_PATH (o) and
+  //   VARIANT_TYPE_SIGNATURE (g) can be cast to
+  //   VARIANT_TYPE_BYTESTRING (ay), the type of std::string.
+
+  std::size_t subtype_index = 0;
+  std::size_t supertype_index = 0;
+  const std::size_t supertype_size = supertype_string.size();
+  while (supertype_index < supertype_size)
+  {
+    const char supertype_char = supertype_string[supertype_index++];
+    const char subtype_char = subtype_string[subtype_index++];
+
+    if (supertype_char == subtype_char)
+      continue;
+
+    switch (supertype_char)
+    {
+    case 's':
+      if (!(subtype_char == 'o' || subtype_char == 'g'))
+        return false;
+      break;
+
+    case 'a':
+      if (!(supertype_string[supertype_index] == 'y' &&
+         (subtype_char == 's' || subtype_char == 'o' || subtype_char == 'g')))
+        return false;
+      supertype_index++;
+      break;
+
+    default:
+      return false;
+    }
+  }
+  return true;
+}
+
 
 VariantStringBase::VariantStringBase()
 : VariantBase()
@@ -140,7 +187,7 @@ void VariantContainerBase::get_child(VariantBase& child, gsize index) const
 {
   if(index >= g_variant_n_children(gobject_))
     throw std::out_of_range(
-      "VariantContainerBase::get(): Index out of bounds.");
+      "VariantContainerBase::get_child(): Index out of bounds.");
 
   GVariant* const gvariant = g_variant_get_child_value(gobject_, index);
   child.init(gvariant);
@@ -180,6 +227,22 @@ bool VariantContainerBase::get_maybe(VariantBase& maybe) const
     return false;
 }
 
+VariantIter VariantContainerBase::get_iter(const VariantType& container_variant_type) const
+{
+  // Get the GVariantIter.
+  // If the underlying GVariant can be cast to the type of the container,
+  // use the actual type (which may differ from container_variant_type, if
+  // the GVariant contains strings, object paths or DBus type signatures),
+  // otherwise let g_variant_get() report what's wrong with the type.
+  GVariantIter* g_iter = 0;
+  g_variant_get(const_cast<GVariant*>(gobj()),
+    is_castable_to(container_variant_type) ?
+      get_type_string().c_str() : container_variant_type.get_string().c_str(),
+    &g_iter);
+
+  return VariantIter(g_iter);
+}
+
 /****************** Specializations ***********************************/
 
 VariantBase::operator const void*() const
@@ -197,6 +260,7 @@ void VariantBase::init(const GVariant* cobject, bool take_a_reference)
     g_variant_ref(gobject_);
 }
 
+/*--------------------Variant<VariantBase>---------------------*/
 
 Variant<VariantBase>::Variant()
 : VariantContainerBase()
@@ -226,6 +290,7 @@ void Variant<VariantBase>::get(VariantBase& variant) const
   variant.init(gvariant);
 }
 
+/*--------------------Variant<Glib::ustring>---------------------*/
 
 Variant<Glib::ustring>::Variant()
 : VariantStringBase()
@@ -281,6 +346,8 @@ throw(std::bad_cast)
   }
 }
 
+/*--------------------Variant<std::string>---------------------*/
+
 Variant<std::string>::Variant()
 : VariantStringBase()
 {
@@ -338,12 +405,14 @@ std::string Variant<std::string>::get() const
   const char* pch = 0;
   if(vtype.equal(VARIANT_TYPE_BYTESTRING))
     pch = g_variant_get_bytestring(gobject_);
-  else //g_variant_get_string() cna handle strings, object paths, and signatures.
+  else //g_variant_get_string() can handle strings, object paths, and signatures.
     pch = g_variant_get_string(gobject_, 0);
 
   return convert_const_gchar_ptr_to_stdstring(pch);
 }
 
+/*--------------------Variant< std::vector<Glib::ustring> >---------------------*/
+
 typedef std::vector<Glib::ustring> type_vec_ustring;
 
 Variant<type_vec_ustring>::Variant()
@@ -394,48 +463,42 @@ Variant<type_vec_ustring>::create(const type_vec_ustring& data)
 
 Glib::ustring Variant<type_vec_ustring>::get_child(gsize index) const
 {
-  gsize n_elements = 0;
-
-  const gchar** array = g_variant_get_strv(const_cast<GVariant*>(gobj()),
-    &n_elements);
-
-  if(index >= n_elements)
+  if (index >= g_variant_n_children(const_cast<GVariant*>(gobj())))
     throw std::out_of_range(
-      "Variant< std::vector<Glib::ustring> >::get(): Index out of bounds.");
+      "Variant< std::vector<Glib::ustring> >::get_child(): Index out of bounds.");
 
-  Glib::ustring const result(array[index]);
-  g_free(array);
-  return result;
+  GVariant* gvariant =
+    g_variant_get_child_value(const_cast<GVariant*>(gobj()), index);
+
+  return Glib::Variant<Glib::ustring>(gvariant).get();
 }
 
 type_vec_ustring Variant<type_vec_ustring>::get() const
 {
-  gsize n_elements = 0;
+  // g_variant_get_strv() can be used only if the type is VARIANT_TYPE_STRING_ARRAY,
+  // but the Variant can alternatively hold an array of object paths or DBus type signatures.
+  type_vec_ustring result;
+
+  gsize n_children = g_variant_n_children(const_cast<GVariant*>(gobj()));
 
-  const gchar** array = g_variant_get_strv(const_cast<GVariant*>(gobj()),
-    &n_elements);
+  for (gsize i = 0; i < n_children; i++)
+  {
+    GVariant* gvariant =
+      g_variant_get_child_value(const_cast<GVariant*>(gobj()), i);
+
+    result.push_back(Glib::Variant<Glib::ustring>(gvariant).get());
+  }
 
-  type_vec_ustring const result(array, array + n_elements);
-  g_free(array);
   return result;
 }
 
 VariantIter Variant<type_vec_ustring>::get_iter() const
 {
-  // Get the variant type of the elements.
-  VariantType element_variant_type = Variant<Glib::ustring>::variant_type();
-
-  // Get the variant type of the array.
-  VariantType array_variant_type = Variant<type_vec_ustring>::variant_type();
-
-  // Get the GVariantIter.
-  GVariantIter* g_iter = 0;
-  g_variant_get(const_cast<GVariant*>(gobj()),
-    array_variant_type.get_string().c_str(), &g_iter);
-
-  return VariantIter(g_iter);
+  return VariantContainerBase::get_iter(variant_type());
 }
 
+/*--------------------Variant< std::vector<std::string> >---------------------*/
+
 typedef std::vector<std::string> type_vec_string;
 
 Variant<type_vec_string>::Variant()
@@ -505,46 +568,38 @@ Variant<type_vec_string>::create_from_object_paths(const type_vec_string& data)
 
 std::string Variant<type_vec_string>::get_child(gsize index) const
 {
-  gsize n_elements = 0;
-
-  const gchar** array =
-    g_variant_get_bytestring_array(const_cast<GVariant*>(gobj()), &n_elements);
-
-  if(index >= n_elements)
+  if (index >= g_variant_n_children(const_cast<GVariant*>(gobj())))
     throw std::out_of_range(
-      "Variant< std::vector<std::string> >::get(): Index out of bounds.");
+      "Variant< std::vector<std::string> >::get_child(): Index out of bounds.");
 
-  std::string const result(array[index]);
-  g_free(array);
-  return result;
+  GVariant* gvariant =
+    g_variant_get_child_value(const_cast<GVariant*>(gobj()), index);
+
+  return Glib::Variant<std::string>(gvariant).get();
 }
 
 type_vec_string Variant<type_vec_string>::get() const
 {
-  gsize n_elements = 0;
+  // g_variant_get_bytestring_array() can be used only if the type is VARIANT_TYPE_BYTESTRING_ARRAY,
+  // but the Variant can alternatively hold an array of strings, object paths or DBus type signatures.
+  type_vec_string result;
 
-  const gchar** array =
-    g_variant_get_bytestring_array(const_cast<GVariant*>(gobj()), &n_elements);
+  gsize n_children = g_variant_n_children(const_cast<GVariant*>(gobj()));
+
+  for (gsize i = 0; i < n_children; i++)
+  {
+    GVariant* gvariant =
+      g_variant_get_child_value(const_cast<GVariant*>(gobj()), i);
+
+    result.push_back(Glib::Variant<std::string>(gvariant).get());
+  }
 
-  type_vec_string const result(array, array + n_elements);
-  g_free(array);
   return result;
 }
 
 VariantIter Variant<type_vec_string>::get_iter() const
 {
-  // Get the variant type of the elements.
-  const VariantType element_variant_type = Variant<std::string>::variant_type();
-
-  // Get the variant type of the array.
-  const VariantType array_variant_type = Variant<type_vec_string>::variant_type();
-
-  // Get the GVariantIter.
-  GVariantIter* g_iter = 0;
-  g_variant_get(const_cast<GVariant*>(gobj()),
-    array_variant_type.get_string().c_str(), &g_iter);
-
-  return VariantIter(g_iter);
+  return VariantContainerBase::get_iter(variant_type());
 }
 
 } // namespace Glib
diff --git a/glib/src/variant.hg b/glib/src/variant.hg
index be750d0..5bf3af4 100644
--- a/glib/src/variant.hg
+++ b/glib/src/variant.hg
@@ -213,6 +213,23 @@ public:
 
    _IGNORE(g_variant_dict_new)
 
+protected:
+#ifndef DOXYGEN_SHOULD_SKIP_THIS
+  /** Used by cast_dynamic().
+   * In addition to an exact match, the following casts are possible:
+   * - VARIANT_TYPE_OBJECT_PATH and VARIANT_TYPE_SIGNATURE can be cast to
+   *   VARIANT_TYPE_STRING (Glib::ustring).
+   * - VARIANT_TYPE_STRING, VARIANT_TYPE_OBJECT_PATH and VARIANT_TYPE_SIGNATURE
+   *   can be cast to VARIANT_TYPE_BYTESTRING (std::string).
+   *
+   * These casts are possible also when they are parts of a more complicated type.
+   * E.g. in Variant<std::map<Glib::ustring, std::vector<std::string> > > the map's keys
+   * can be VARIANT_TYPE_OBJECT_PATH and the vector's elements can be VARIANT_TYPE_SIGNATURE.
+   * @newin{2,46}
+   */
+  bool is_castable_to(const VariantType& supertype) const;
+#endif //DOXYGEN_SHOULD_SKIP_THIS
+
 private:
   /** Relational operators are deleted to prevent invalid conversion
    * to const void*.
@@ -243,7 +260,7 @@ throw(std::bad_cast)
   {
     return V_CastTo();
   }
-  if(v.is_of_type(V_CastTo::variant_type()))
+  if(v.is_castable_to(V_CastTo::variant_type()))
   {
     return V_CastTo(const_cast<GVariant*>(v.gobj()), true);
   }
@@ -259,8 +276,7 @@ throw(std::bad_cast)
  */
 class VariantStringBase : public VariantBase
 {
-  // Trick gmmproc into thinking this is derived from GVariant to wrap a
-  // some methods.
+  // Trick gmmproc into thinking this is derived from GVariant to wrap some methods.
   _CLASS_GENERIC(VariantStringBase, GVariant)
 
 public:
@@ -315,8 +331,7 @@ public:
  */
 class VariantContainerBase : public VariantBase
 {
-  // Trick gmmproc into thinking this is derived from GVariant to wrap a
-  // some methods.
+  // Trick gmmproc into thinking this is derived from GVariant to wrap some methods.
   _CLASS_GENERIC(VariantContainerBase, GVariant)
 
 public:
@@ -380,7 +395,7 @@ public:
   */
 
   /** If this is a maybe-typed instance, extract its value. If the value is
-   * Nothing, then this function returns <tt>0</tt>.
+   * Nothing, then this function returns <tt>false</tt>.
    *
    * @param maybe A place in which to return the value (the value may be
    * <tt>0</tt>).
@@ -388,6 +403,14 @@ public:
    */
   bool get_maybe(VariantBase& maybe) const;
   _IGNORE(g_variant_get_maybe)
+
+protected:
+#ifndef DOXYGEN_SHOULD_SKIP_THIS
+  /** Used by get_iter() in the subclasses.
+   * @newin{2,46}
+   */
+  VariantIter get_iter(const VariantType& container_variant_type) const;
+#endif //DOXYGEN_SHOULD_SKIP_THIS
 };
 
 template<>
@@ -405,10 +428,6 @@ public:
   typedef T CppType;
 };
 
-// Each specialization has (or should have) a variant_type() method that gets
-// the type. So the C g_variant_get_type() function can be ignored.
-_IGNORE(g_variant_get_type)
-
 /****************** Specializations ***********************************/
 
 /** Specialization of Variant containing a VariantBase.
@@ -418,8 +437,7 @@ _IGNORE(g_variant_get_type)
 template<>
 class Variant<VariantBase> : public VariantContainerBase
 {
-  // Trick gmmproc into thinking this is derived from GVariant to wrap a
-  // some methods.
+  // Trick gmmproc into thinking this is derived from GVariant to wrap some methods.
   _CLASS_GENERIC(Variant<VariantBase>, GVariant)
 
 public:
@@ -506,15 +524,14 @@ public:
 };
 
 /** Specialization of Variant containing a Glib::ustring, for variants of type
- * string, bytestring, object path, or signature.
+ * string, object path, or signature.
  * @newin{2,28}
  * @ingroup Variant
  */
 template<>
 class Variant<Glib::ustring> : public VariantStringBase
 {
-  // Trick gmmproc into thinking this is derived from GVariant to wrap a
-  // some methods.
+  // Trick gmmproc into thinking this is derived from GVariant to wrap some methods.
   _CLASS_GENERIC(Variant<Glib::ustring>, GVariant)
 public:
   typedef char*                 CType;
@@ -553,12 +570,13 @@ public:
   _IGNORE(g_variant_get_string, g_variant_dup_string)
 };
 
+//TODO: When we can break ABI, remove this template specialization.
 template<>
 Variant<Glib::ustring> VariantBase::cast_dynamic< Variant<Glib::ustring> >(const VariantBase& v)
 throw(std::bad_cast);
 
 /** Specialization of Variant containing a std::string, for variants of type
- * bytestring, object path, or signature.
+ * bytestring, string, object path, or signature.
  * See also Variant<Glib::ustring> for UTF-8 strings.
  * @newin{2,28}
  * @ingroup Variant
@@ -566,8 +584,7 @@ throw(std::bad_cast);
 template<>
 class Variant<std::string> : public VariantStringBase
 {
-  // Trick gmmproc into thinking this is derived from GVariant to wrap a
-  // some methods.
+  // Trick gmmproc into thinking this is derived from GVariant to wrap some methods.
   _CLASS_GENERIC(Variant<std::string>, GVariant)
 public:
   typedef char*                 CType;
@@ -602,6 +619,7 @@ public:
   _IGNORE(g_variant_get_bytestring, g_variant_dup_bytestring)
 };
 
+//TODO: When we can break ABI, remove this template specialization.
 template<>
 Variant<std::string> VariantBase::cast_dynamic< Variant<std::string> >(const VariantBase& v)
 throw(std::bad_cast);
@@ -1030,11 +1048,11 @@ Variant< std::pair<K, V> >::create(const std::pair<K, V>& data)
 template<class K, class V>
 std::pair<K, V> Variant< std::pair<K, V> >::get() const
 {
-  // Get the key (the first element of the this VariantContainerBase).
+  // Get the key (the first element of this VariantContainerBase).
   Variant<K> key;
   VariantContainerBase::get_child(key, 0);
 
-  // Get the value (the second element of the this VariantContainerBase).
+  // Get the value (the second element of this VariantContainerBase).
   Variant<V> value;
   VariantContainerBase::get_child(value, 1);
 
@@ -1059,9 +1077,6 @@ template<class T>
 Variant< std::vector<T> >
 Variant< std::vector<T> >::create(const std::vector<T>& data)
 {
-  // Get the variant type of the elements.
-  VariantType element_variant_type = Variant<T>::variant_type();
-
   // Get the variant type of the array.
   VariantType array_variant_type = Variant< std::vector<T> >::variant_type();
 
@@ -1079,7 +1094,7 @@ Variant< std::vector<T> >::create(const std::vector<T>& data)
   // Create the variant using the builder.
   Variant< std::vector<T> > result =
     Variant< std::vector<T> >(g_variant_new(
-      reinterpret_cast<gchar*>(array_variant_type.gobj()), builder));
+      reinterpret_cast<const gchar*>(array_variant_type.gobj()), builder));
 
   g_variant_builder_unref(builder);
 
@@ -1126,18 +1141,7 @@ std::vector<T> Variant< std::vector<T> >::get() const
 template<class T>
 VariantIter Variant< std::vector<T> >::get_iter() const
 {
-  // Get the variant type of the elements.
-  VariantType element_variant_type = Variant<T>::variant_type();
-
-  // Get the variant type of the array.
-  VariantType array_variant_type = Variant< std::vector<T> >::variant_type();
-
-  // Get the GVariantIter.
-  GVariantIter* g_iter = 0;
-  g_variant_get(const_cast<GVariant*>(gobj()),
-    reinterpret_cast<gchar*>(array_variant_type.gobj()), &g_iter);
-
-  return VariantIter(g_iter);
+  return VariantContainerBase::get_iter(variant_type());
 }
 
 /*---------------------Variant< std::map<K, V> > --------------------*/
@@ -1166,7 +1170,7 @@ Variant< std::map<K, V> >::create(const std::map<K, V>& data)
   // Create a GVariantBuilder to build the array.
   GVariantBuilder* builder = g_variant_builder_new(array_variant_type.gobj());
 
-  // Add the elements of the vector into the builder.
+  // Add the elements of the map into the builder.
   for(typename std::map<K, V>::const_iterator iter = data.begin();
     iter != data.end(); iter++)
   {
@@ -1178,7 +1182,7 @@ Variant< std::map<K, V> >::create(const std::map<K, V>& data)
 
   // Create the variant using the builder.
   Variant< std::map<K, V> > result = Variant< std::map<K, V> >(g_variant_new(
-    reinterpret_cast<gchar*>(array_variant_type.gobj()), builder));
+    reinterpret_cast<const gchar*>(array_variant_type.gobj()), builder));
 
   g_variant_builder_unref(builder);
 
@@ -1236,19 +1240,7 @@ std::map<K, V> Variant< std::map<K, V> >::get() const
 template<class K, class V>
 VariantIter Variant< std::map<K, V> >::get_iter() const
 {
-  // Get the variant type of the elements.
-  VariantType element_variant_type =
-    Variant< std::pair<K, V> >::variant_type();
-
-  // Get the variant type of the array.
-  VariantType array_variant_type = Variant< std::map<K, V> >::variant_type();
-
-  // Get the GVariantIter.
-  GVariantIter* g_iter = 0;
-  g_variant_get(const_cast<GVariant*>(gobj()),
-    reinterpret_cast<gchar*>(array_variant_type.gobj()), &g_iter);
-
-  return VariantIter(g_iter);
+  return VariantContainerBase::get_iter(variant_type());
 }
 
 } // namespace Glib
diff --git a/tests/glibmm_variant/main.cc b/tests/glibmm_variant/main.cc
index 086719f..484cb8f 100644
--- a/tests/glibmm_variant/main.cc
+++ b/tests/glibmm_variant/main.cc
@@ -276,6 +276,64 @@ static void test_dynamic_cast_string_types()
   }
 }
 
+// Test casting a complicated type, containing an object path and a DBus type signature.
+void test_dynamic_cast_composite_types()
+{
+  // Build a GVaraint of type a{oag}, and cast it to
+  // Glib::Variant<std::map<Glib::ustring, std::vector<std::string>>>.
+  // 'o' is VARIANT_TYPE_OBJECT_PATH and 'g' is VARIANT_TYPE_SIGNATURE.
+
+  GVariantBuilder dict_builder;
+  GVariantBuilder array_builder;
+  g_variant_builder_init(&dict_builder, G_VARIANT_TYPE("a{oag}"));
+
+  g_variant_builder_init(&array_builder, G_VARIANT_TYPE("ag"));
+  g_variant_builder_add(&array_builder, "g","id");
+  g_variant_builder_add(&array_builder, "g","isi");
+  g_variant_builder_add(&array_builder, "g","ia{si}");
+  g_variant_builder_add(&dict_builder, "{oag}", "/remote/object/path1", &array_builder);
+
+  g_variant_builder_init(&array_builder, G_VARIANT_TYPE("ag"));
+  g_variant_builder_add(&array_builder, "g","i(d)");
+  g_variant_builder_add(&array_builder, "g","i(si)");
+  g_variant_builder_add(&dict_builder, "{oag}", "/remote/object/path2", &array_builder);
+
+  Glib::VariantBase cppdict(g_variant_builder_end(&dict_builder));
+
+  try
+  {
+    typedef std::map<Glib::ustring, std::vector<std::string> > composite_type;
+    Glib::Variant<composite_type> derived =
+      Glib::VariantBase::cast_dynamic<Glib::Variant<composite_type> >(cppdict);
+
+    ostr << "Cast composite type (get_type_string()=" << derived.get_type_string()
+         << ", variant_type().get_string()=" << derived.variant_type().get_string() << "): ";
+    composite_type var = derived.get();
+    for (composite_type::const_iterator iter1 = var.begin(); iter1 != var.end(); ++iter1)
+    {
+      ostr << "\n  " << iter1->first << ":";
+      const std::vector<std::string>& vec = iter1->second;
+      for (std::vector<std::string>::const_iterator iter2 = vec.begin(); iter2 != vec.end(); ++iter2)
+        ostr << "  " << *iter2;
+    }
+    ostr << std::endl;
+  }
+  catch (const std::bad_cast& e)
+  {
+    g_assert_not_reached();
+  }
+
+  try
+  {
+    Glib::Variant<std::map<Glib::ustring, std::string > > derived =
+      Glib::VariantBase::cast_dynamic<Glib::Variant<std::map<Glib::ustring, std::string> > >(cppdict);
+    g_assert_not_reached();
+  }
+  catch (const std::bad_cast& e)
+  {
+  }
+}
+
 static void test_dynamic_cast()
 {
   Glib::Variant<int> v1 = Glib::Variant<int>::create(10);
@@ -359,6 +417,7 @@ static void test_dynamic_cast()
 
   test_dynamic_cast_ustring_types();
   test_dynamic_cast_string_types();
+  test_dynamic_cast_composite_types();
 }
 
 static GLogLevelFlags