GNOME Bugzilla – Bug 632940
Add API for converting JSON to GVariant and vice-versa
Last modified: 2012-03-13 22:08:36 UTC
Bug is about adding new API to convert from a JsonNode to a GVariant and vice-versa. My particular use case is needing to interface GDBus APIs using JSON. Other use case could be using a GVariant in contexts not supporting binary data (like Javascript). I suspect some (configurable?) data type mapping would be required when converting from JSON to GVariant, to disambiguate cases like Number which has several candidates in GVariant.
if we adhere to the strictest (and most portable) interpretation of the JSON spec, then we need: - int64 for all integer numbers - double for all floating point numbers - string - boolean and arrays/dictionaries. everything that does not conform has to be converted, using the usual GValue↔GVariant mapping functions. obviously, unlike GVariant and DBus, JSON cannot accept signatures for "structures" (i.e. something like "(uuisss)" would not be acceptable). strictly speaking, a valid array in JSON would be represented as a "av", and a valid dictionary as a "a{sv}", so I'm not entirely sure of the convenience of a generic API. in any case, patches are welcome.
I started working on a patch for this. Converting GVariant to JSON is pretty straight and unambiguous. In the case of JSON to GVariant, my idea to disambiguate is to provide a GVariant signature together with the JSON tree, and build the GVariant in conformance with the signature, not with the types in the JSON nodes. A GError is returned if the signature is not a valid type string or if the JSON structure doesn't comply with it. API would be something like this: JsonNode *json_node_from_gvariant (GVariant *variant); GVariant *json_node_to_gvariant (JsonNode *node, const gchar *signature, GError *error); I'm have not found any source of ambiguity with this approach, and should allow general purpose conversion, provided the API user knows the expected signature (something pretty reasonable to assert in D-Bus at least). I'm not sure whether we should allow calling 'to_gvariant()' passing a NULL signature, in which case generic (likely not useful) conversion is done directly from the data type in JSON nodes. I'm in favor of disallowing that and make 'signature' argument required. My conversion table from GVariant to JSON is this: * boolean <=> JSON boolean * byte, int16, uint16, int32, uint32, int64, uint64, handle <=> JSON int64 * string, object path, signature <=> JSON string * double => JSON double * variant <=> JSON sub-tree (recursing into the content of the variant) * array, tuple <=> JSON array * maybe <=> JSON node of the actual type, or JSON null * dict entry alone or array of dict entries <=> JSON object One tricky issue I see is mapping GVariant dictionary entries that doesn't have string keys (like "{nv}"). As the key is always a simple type, my idea is to convert it to string to conform the JSON object key (like -1 => "-1", true => "true", 1.4E002 => "1.4E002", etc). Then, when converting back to GVariant, and having the expected signature, the reverse process is done and the string key is converted to the actual type before creating the GVariant dict entry.
(In reply to comment #2) > > GVariant *json_node_to_gvariant (JsonNode *node, > const gchar *signature, > GError *error); > It is of course 'GError **error' instead of just '*error'.
I have completed the GVariant to JSON conversion. It can be checked out from http://gitorious.org/eventdance/json-glib-elima/commits/gvariant for early review. A test is included. After finishing the other part, I'm gonna squash and upload patch here.
I have pushed the JSON to GVariant conversion. API currently is: JsonNode * json_from_gvariant (GVariant *variant); gchar * json_data_from_gvariant (GVariant *variant, gsize *size); GVariant * json_to_gvariant (JsonNode *json_node, const gchar *signature, GError **error); GVariant * json_data_to_gvariant (const gchar *json, gssize length, const gchar *signature, GError **error); I did find a source of ambiguity though, and is related with GVariant of type variant. Problem is that when you have a variant with 'v' signature, the signature of the actual variant inside is lost, so converting a complete JSON tree to GVariant 'v' is possible, but there is no way to constrain the JSON node types. My solution to this is simply that when the conversion finds a variant type, from there on it continues building the GVariant structure using the data type of the JSON nodes. In this scenario, the conversion table is: JSON | GVariant ------------------------------------- string | string (s) int64 | int64 (x) boolean | boolean (b) double | double (d) array | array of variants (av) object | dictionary of string-variant (a{sv}) null | maybe variant (mv) For example, the JSON "['foo', true, 10, {'bar': 'baz'}]" as a GVariant with signature v, produces "<[<'foo'>, <true>, <gint64 10>, {'bar': <'baz'>}]>". Pending stuff: - Add doc and annotations to public API. - Complete gvariant-test to include some error cases. Feedback and more tests cases are very welcomed. cheers
Created attachment 176146 [details] [review] initial patch providing the new API Attached is the patch resulting from a complete squash of my 'gvariant' branch of json-glib (comment 4), to facilitate reviewing. Unit tests and GTK-DOC docs are included. I published the generated documentation at http://people.igalia.com/elima/json-glib-elima/index.html, in case you want to review it and don't feel like building it. It is a rather big diff so I understand reviewing could take time. I really want to help ease the process of pushing this in, so please feel free to contact me on GNOME IRC to quickly discuss any part of the patch and/or to propose changes. Don't hesitate to nitpick on anything you could feel uncomfortable with.
Review of attachment 176146 [details] [review]: thanks for the patch. generally, I think it's a solid initial approach; kudos for writing an extensive test unit: it's much appreciated. there are some issues in the documentation, and the API doesn't fit in with the rest of the JSON-to-GLib-types integration that is currently exposed. ::: doc/reference/json-glib-sections.txt @@ +249,3 @@ <SECTION> +<FILE>json-gvariant</FILE> +<TITLE>JSON-GVariant conversion</TITLE> the title should go inline in the file, using @Title ::: json-glib/json-gvariant.c @@ +26,3 @@ +/** + * SECTION:json-gvariant + * @short_description: Converts from JSON to GVariant and vice-versa missing: @Title: I'd use "JSON GVariant Integration" @@ +28,3 @@ + * @short_description: Converts from JSON to GVariant and vice-versa + * + * Use #json_from_gvariant and #json_data_from_gvariant to convert from wrong gtk-doc tags (here and elsewhere). functions are linked using their name plus "()", so this should read: Use json_from_gvariant() and json_data_from_gvariant() [...] the # is used for structs and % for macros and enumeration values. @@ +184,3 @@ + + case G_VARIANT_CLASS_DOUBLE: + str = g_strdup_printf ("%lf", g_variant_get_double (variant)); this will be a problem in various locales. use g_ascii_formatd() instead. @@ +257,3 @@ + * + * Return value: (transfer full): A #JsonNode representing the root of the + * JSON data structure obtained from @variant missin "Since: 0.14" annotation. ::: json-glib/json-gvariant.h @@ +33,3 @@ +gchar * json_data_from_gvariant (GVariant *variant, + gsize *size); + to fit the naming scheme used by json-glib for gobjects and gboxeds, I'd use: gchar * json_gvariant_serialize (GVariant *variant, gsize *size); JsonNode *json_gvariant_deserialize (GVariant *variant); and their relative string versions. the important bit is namespacing under "json_gvariant_*" and not under "json_*".
Created attachment 176360 [details] [review] my previous patch with fixes Emmanuele, thanks a lot for reviewing my patch promptly. Here goes a second patch fixing all the issues you commented. API methods were renamed to: GVariant -> JSON - json_gvariant_serialize() - json_gvariant_serialize_data() JSON -> GVariant - json_gvariant_deserialize() - json_gvariant_deserialize_data() I you have a more specific suggestion I'll be happy to update them again. The generated documentation can be reviewed at http://people.igalia.com/elima/json-glib-elima/index.html.
the new patch looks pretty good to me. let's get it merged, and we can work on this on master.
on second thought, there are a couple of issues with the patch :-) • compiler warnings for format strings; in order to portably convert gint64 and guint64 to string you should use G_GINT64_FORMAT and G_GUINT64_FORMAT; • G_VARIANT_CLASS_DICTIONARY is a custom extension, but I fail to see its correct use; • the test suite fails here: ERROR:gvariant-test.c:165:test_gvariant_to_json: assertion failed (test_case->json_data == json_data): ("{ \"999888777666\" : true, \"0\" : false }" == "{ \"3456364994\" : true, \"0\" : false }") FAIL GTester: last random seed: R02S16e956ee5687d00699e7e20e1362a382 which is the gint64 format string issue; if I fix that, then I get this failure: *** JSON data: { "999888777666" : true, "0" : false } *** GVariant signature:a{tb} ** ERROR:gvariant-test.c:191:test_json_to_gvariant: assertion failed (error == NULL): Invalid string value converting to GVariant (g-io-error-quark, 35)
Created attachment 179380 [details] [review] patch with more fixes > • compiler warnings for format strings; in order to portably convert gint64 and > guint64 to string you should use G_GINT64_FORMAT and G_GUINT64_FORMAT; Fixed. I now use G_GINT64_FORMAT and G_GUINT64_FORMAT for conversion and also replaced all "strtoXX" for "g_ascii_strtoXX". > • G_VARIANT_CLASS_DICTIONARY is a custom extension, but I fail to see its > correct use; I have defined G_VARIANT_CLASS_DICTIONARY only for internal use. I was not aware that it is an accepted extension. I define it in line 87 of json-gvariant.c and it is used strictly in my private routines to differentiate a single dictionary entry from an array of dictionary entries. It is just a convenience macro to provide simplicity in the internal code. > • the test suite fails here: > > ERROR:gvariant-test.c:165:test_gvariant_to_json: assertion failed > (test_case->json_data == json_data): ("{ \"999888777666\" : true, \"0\" : false > }" == "{ \"3456364994\" : true, \"0\" : false }") > FAIL > GTester: last random seed: R02S16e956ee5687d00699e7e20e1362a382 I'm not getting this error right now, but honestly I have only tested it in a 64 bits debian. Is it still failing after this patch? cheers
Review of attachment 179380 [details] [review]: the patch looks good now, and the test suite passes without warnings. there are still some coding style considerations, and the issue of a compiler warning, which I can fix myself. so, let's get this in. :-)
attachment 179380 [details] [review] pushed to master
[Fixing Default QA assignee for json-glib - see 613232#c1. Sorry for bugmail noise.]