GNOME Bugzilla – Bug 697175
Grilo's flickr plugin using OAuth
Last modified: 2013-05-28 15:11:13 UTC
Created attachment 240478 [details] [review] Flickr plugin Hey, I wrote a patch for grilo's Flickr plugin to use OAuth. Can you, please, review the code? Tks
Created attachment 240479 [details] [review] Grilo changes for flickr's plugin (and grilo-test-ui) The other part of patch - I added two functions into grl-config and rewrote the grilo-test-ui
Sorry for the delay. I'll take a look to these patches along this week.
Review of attachment 240479 [details] [review]: ::: configure.ac @@ -152,1 +153,2 @@ AM_CONDITIONAL(BUILD_GRILO_TEST_UI, test "x$HAVE_GTK" = "xyes") +PKG_CHECK_MODULES(OAUTH, oauth) I understand that oauth in grilo is needed to build grilo-test-ui. In this case, enable-test-ui should check if GTK+ and oauth are present.
So, basicallly this?: +PKG_CHECK_MODULES(OAUTH, oauth, [HAVE_OAUTH=yes], [HAVE_OAUTH=no]) + AC_ARG_ENABLE(test-ui, AS_HELP_STRING([--enable-test-ui], [Build Test UI (default: auto)]), @@ -143,6 +145,9 @@ AC_ARG_ENABLE(test-ui, if test "x$HAVE_GTK" = "xno"; then AC_MSG_ERROR([gtk+-3.0 >= 3.0 not found, install it or use fi + if test "x$HAVE_OAUTH" = "xno"; then + AC_MSG_ERROR([OAuth not found, install it or use --disable- + fi If so, should I merge it with the old patch or attache it as a new one?
1) *attach it as a new one ;) 2) and also change this: AM_CONDITIONAL(BUILD_GRILO_TEST_UI, test "x$HAVE_GTK" = "xyes") to AM_CONDITIONAL(BUILD_GRILO_TEST_UI, test "x$HAVE_GTK" = "xyes" -a "x$HAVE_OAUTH" = "xyes")
Yes, better attach a new one replacing the older. Also, can you rebase the patches against master? I had some problems to apply one of the patches.
Created attachment 242120 [details] [review] Changes for grilo and test-ui (In reply to comment #1) > Created an attachment (id=240479) [details] [review] > Grilo changes for flickr's plugin (and grilo-test-ui) > > The other part of patch - I added two functions into grl-config and rewrote the > grilo-test-ui (In reply to comment #6) > Yes, better attach a new one replacing the older. > > Also, can you rebase the patches against master? I had some problems to apply > one of the patches. Here's the patch rebased against master, including proposed changes from comment #3
Created attachment 242121 [details] [review] New version of patch 240478 (In reply to comment #0) > Created an attachment (id=240478) [details] [review] > Flickr plugin > > Hey, > I wrote a patch for grilo's Flickr plugin to use OAuth. > Can you, please, review the code? > > Tks (In reply to comment #6) > Yes, better attach a new one replacing the older. > > Also, can you rebase the patches against master? I had some problems to apply > one of the patches. Here's the plugin patch rebased against master. There was a conflict with a new patch, now it should be ok.
Great. I've split the core patch into two commits: one for adding the grl_config_get/set_api_token_secret() functions, and another for the changes in test-ui to use the new Flickr version.
Comment on attachment 242120 [details] [review] Changes for grilo and test-ui commit 21339e9723f62aee1c68bc341773388df52bc2b6 Author: Marek Chalupa <mchalupa@redhat.com> Date: Mon Apr 22 09:19:25 2013 +0200 test-ui: use new flickr plugin flickr-auth.[ch] was replaced by their oauth alternatives and main.c was rewritten to use them. Into authenticate flickr popup window was added text entry to enter the verifier given by flickr. https://bugzilla.gnome.org/show_bug.cgi?id=697175 Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com> configure.ac | 8 +++++- tools/grilo-test-ui/Makefile.am | 6 +++-- tools/grilo-test-ui/flickr-auth.c | 257 ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- tools/grilo-test-ui/flickr-auth.h | 43 ------------------------------- tools/grilo-test-ui/flickr-oauth.c | 317 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/grilo-test-ui/flickr-oauth.h | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/grilo-test-ui/main.c | 44 ++++++++++++++++++++++---------- 7 files changed, 449 insertions(+), 316 deletions(-) commit b590157c45a14eb95c54091a75eb109cf1bcf438 Author: Marek Chalupa <mchalupa@redhat.com> Date: Mon Apr 22 09:19:25 2013 +0200 core: add grl_config_get/set_api_token_secret() Two new functions added to grl-config.[ch], needed by oauth authentication proccess. https://bugzilla.gnome.org/show_bug.cgi?id=697175 Signed-off-by: Juan A. Suarez Romero <jasuarez@igalia.com> src/data/grl-config.c | 35 +++++++++++++++++++++++++++++++++++ src/data/grl-config.h | 21 +++++++++++++-------- 2 files changed, 48 insertions(+), 8 deletions(-)
commit d1ba25ddc6f1381a33e64c4d978816ed762031e9 Author: Marek Chalupa <mchalupa@redhat.com> Date: Mon Apr 22 11:05:48 2013 +0200 flickr: use liboauth In new files flickr-oauth.[ch] are functions providing signing and creating request to flickr api, either non-authorized or authorized via OAuth. In gflickr.c are new wrapper functions for abovementioned functions (just for simplicity, e.g.: flickroauth_create_api_url (6 args..) --> create_url (3 args) etc. ) and the API in this file is modified to use these function. Old authentication functions was deleted. GFlickr object was modified to use oauth token and token secret. In the same manner was modified grl-flicker.c Into configure.ac and Makefile.am was written appropriate changes. https://bugzilla.gnome.org/show_bug.cgi?id=697175 configure.ac | 11 +++-- src/flickr/Makefile.am | 4 +- src/flickr/flickr-oauth.c | 166 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/flickr/flickr-oauth.h | 56 ++++++++++++++++++++++ src/flickr/gflickr.c | 602 +++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- src/flickr/gflickr.h | 18 +++----- src/flickr/grl-flickr.c | 47 +++++++++++++------ 7 files changed, 382 insertions(+), 522 deletions(-)