GNOME Bugzilla – Bug 680436
net: Add possibility to set custom HTTP headers on requests
Last modified: 2012-09-18 09:18:05 UTC
Created attachment 219475 [details] [review] suggested fix Add a grl_net_wc_request_async_full function that takes a string,string hash with header → value mapping
Review of attachment 219475 [details] [review]: Usually the developer that needs to set custom headers will need to set just one header, 2 at most. The point is that to do it, basically, developer first need to create a hash table, then set the header (as saying most of cases will just add one entry), invoke the function and free the hash table. IMHO, too many steps for something that should be simpler, and specially easy to read. I was thinking that something similar to g_object_set() would be more appropriate and easy to read: in the same function you set up each header with its value. The API I have in mind is: void grl_net_wc_request_with_headers_async (GrlNet *self, const gchar *uri, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data, const gchar *header, ...); An example of calling this function would be: grl_net_wc_request_with_headers_async (wc, "http://myuri", cancellable, on_callback, myuser_data, "header1", "value1", "header2", "value2", NULL);
Still, vaargs are bad for bindings/introspection
Yes, not very good for introspection. But that problem is also in other parts of Grilo that, I think, must be addressed in the future. Anyway, IIRC (and correct me if I'm wrong), it is possible to provide an alternative method for the binding case that works like you have proposed (using a hashtable), and that would be used instead of varargs version when making the bindings. Still, if above doesn't work, I still prefer the varargs version. And check in future how to solve all the introspection/bindings problem in Grilo in one shot.
Created attachment 223988 [details] [review] Updated patch which includes a varargs version which delegates to the (renamed) hash version from the previous patch.
Created attachment 223990 [details] [review] Patch v3 - Introspection magic Same as version 2, but with added introspection magic to make the hash version shadow the varargs version in GI.
Review of attachment 223990 [details] [review]: ::: libs/net/grl-net-wc.c @@ +433,3 @@ + cancellable, + callback, + user_data); Shouldn't free the hashtable? ::: libs/net/grl-net-wc.h @@ +122,3 @@ gpointer user_data); +void grl_net_wc_request_async_with_headers_hash (GrlNetWc *self, Let's keep the standard use "_async" as the suffix. So rename it to grl_net_wc_request_with_headers_hash_async() @@ +129,3 @@ + gpointer user_data); + +void grl_net_wc_request_async_with_headers (GrlNetWc *self, Same here: rename it to grl_net_wc_request_with_headers_async()
Created attachment 224491 [details] [review] v4: Fix leak and address other comments net: Add possibility to add arbitrary HTTP headers Some webservices need custom HTTP headers for requests; this patch adds a function to schedule requests with those special headers. Under contract for Canonical Ltd.
commit dd619934612a98493ee8ab6b76eb883acc52cd9c Author: Jens Georg <jensg@openismus.com> Date: Thu Jul 12 22:44:47 2012 +0200 net: Add possibility to add arbitrary HTTP headers Some webservices need custom HTTP headers for requests; this patch adds a function to schedule requests with those special headers. Under contract for Canonical Ltd. https://bugzilla.gnome.org/show_bug.cgi?id=680436 libs/net/grl-net-private.h | 1 + libs/net/grl-net-soup-stable.c | 13 +++++++++++++ libs/net/grl-net-soup-unstable.c | 19 +++++++++++++++++++
(In reply to comment #8) > commit dd619934612a98493ee8ab6b76eb883acc52cd9c Wrong commit commit bf186d0626220b8515cbf2ce2809e404ce6f0d28 Author: Jens Georg <jensg@openismus.com> Date: Thu Jul 12 22:44:47 2012 +0200 net: Add possibility to add arbitrary HTTP headers Some webservices need custom HTTP headers for requests; this patch adds a function to schedule requests with those special headers. Under contract for Canonical Ltd. https://bugzilla.gnome.org/show_bug.cgi?id=680436 Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com> libs/net/grl-net-private.h | 1 + libs/net/grl-net-soup-stable.c | 13 +++++++++++++ libs/net/grl-net-soup-unstable.c | 19 +++++++++++++++++++ libs/net/grl-net-wc.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- libs/net/grl-net-wc.h | 16 ++++++++++++++++ 5 files changed, 143 insertions(+), 3 deletions(-)