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 742472 - Freebase: Pass API key on all service requests
Freebase: Pass API key on all service requests
Status: RESOLVED FIXED
Product: libgdata
Classification: Platform
Component: General
git master
Other Linux
: Normal normal
: ---
Assigned To: libgdata-maint
libgdata-maint
Depends on:
Blocks:
 
 
Reported: 2015-01-06 17:45 UTC by Carlos Garnacho
Modified: 2015-03-05 16:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
freebase: Pass API key as a GET variable (2.65 KB, patch)
2015-01-06 17:47 UTC, Carlos Garnacho
needs-work Details | Review
tests: Indirectly test API keys (2.95 KB, patch)
2015-01-06 17:47 UTC, Carlos Garnacho
accepted-commit_now Details | Review

Description Carlos Garnacho 2015-01-06 17:45:44 UTC
A big oversight on my side... I'm attaching a patch to actually pass the API key as a GET parameter, as documented in https://developers.google.com/freebase/v1/parameters , and a patch to indirectly test that in the current freebase tests.

One thing I feel is that I'm maybe abusing append_query_headers(), as I'm modifying the message uri, not actually setting a header there.
Comment 1 Carlos Garnacho 2015-01-06 17:47:12 UTC
Created attachment 293957 [details] [review]
freebase: Pass API key as a GET variable

This needs including on every freebase request if a developer key is set
by the caller.
Comment 2 Carlos Garnacho 2015-01-06 17:47:17 UTC
Created attachment 293958 [details] [review]
tests: Indirectly test API keys

Set a fake one on the service, so it can be checked when comparing with
traces.
Comment 3 Philip Withnall 2015-01-07 00:21:51 UTC
Review of attachment 293957 [details] [review]:

This is the right approach, but there are a few niggles.

::: gdata/services/freebase/gdata-freebase-service.c
@@ +118,3 @@
+	g_assert (message != NULL);
+
+	if (priv->developer_key) {

Can you add a comment pointing to the documentation please: https://developers.google.com/freebase/v1/parameters?

@@ +125,3 @@
+			new_query = g_strdup_printf ("%s&key=%s", query, priv->developer_key);
+		} else {
+			new_query = g_strdup_printf ("key=%s", priv->developer_key);

The developer key needs to be URI-escaped here.

It’s probably simplest to use a GString and use g_string_append_uri_escaped().
Comment 4 Philip Withnall 2015-01-07 00:22:17 UTC
Review of attachment 293958 [details] [review]:

++
Comment 5 Carlos Garnacho 2015-03-05 16:39:19 UTC
Sorry, this slipped out of my mind... I'm pushing modified patches with the
suggested changes.

Attachment 293957 [details] pushed as 540e624 - freebase: Pass API key as a GET variable
Attachment 293958 [details] pushed as 044a2a9 - tests: Indirectly test API keys