After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 632940 - Add API for converting JSON to GVariant and vice-versa
Add API for converting JSON to GVariant and vice-versa
Status: RESOLVED FIXED
Product: json-glib
Classification: Core
Component: Generator
git master
Other Linux
: Normal normal
: ---
Assigned To: json-glib-maint
json-glib-maint
Depends on:
Blocks:
 
 
Reported: 2010-10-22 22:39 UTC by Eduardo Lima Mitev
Modified: 2012-03-13 22:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
initial patch providing the new API (48.14 KB, patch)
2010-12-09 19:37 UTC, Eduardo Lima Mitev
needs-work Details | Review
my previous patch with fixes (48.78 KB, patch)
2010-12-13 19:32 UTC, Eduardo Lima Mitev
none Details | Review
patch with more fixes (48.89 KB, patch)
2011-01-26 16:41 UTC, Eduardo Lima Mitev
committed Details | Review

Description Eduardo Lima Mitev 2010-10-22 22:39:57 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.
Comment 1 Emmanuele Bassi (:ebassi) 2010-11-04 03:35:06 UTC
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.
Comment 2 Eduardo Lima Mitev 2010-11-04 09:40:43 UTC
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.
Comment 3 Eduardo Lima Mitev 2010-11-04 09:43:22 UTC
(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'.
Comment 4 Eduardo Lima Mitev 2010-11-09 16:09:46 UTC
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.
Comment 5 Eduardo Lima Mitev 2010-11-11 18:01:29 UTC
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
Comment 6 Eduardo Lima Mitev 2010-12-09 19:37:19 UTC
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.
Comment 7 Emmanuele Bassi (:ebassi) 2010-12-10 12:17:56 UTC
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_*".
Comment 8 Eduardo Lima Mitev 2010-12-13 19:32:00 UTC
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.
Comment 9 Emmanuele Bassi (:ebassi) 2011-01-10 11:57:18 UTC
the new patch looks pretty good to me.

let's get it merged, and we can work on this on master.
Comment 10 Emmanuele Bassi (:ebassi) 2011-01-10 12:17:10 UTC
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)
Comment 11 Eduardo Lima Mitev 2011-01-26 16:41:24 UTC
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
Comment 12 Emmanuele Bassi (:ebassi) 2011-01-26 17:02:31 UTC
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. :-)
Comment 13 Emmanuele Bassi (:ebassi) 2011-01-26 17:03:52 UTC
attachment 179380 [details] [review] pushed to master
Comment 14 André Klapper 2012-03-13 22:08:36 UTC
[Fixing Default QA assignee for json-glib - see 613232#c1. Sorry for bugmail noise.]