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 619135 - Add photo support to Google Contacts backend
Add photo support to Google Contacts backend
Status: RESOLVED FIXED
Product: evolution-data-server
Classification: Platform
Component: Contacts
3.2.x (obsolete)
Other All
: Normal enhancement
: ---
Assigned To: evolution-addressbook-maintainers
Evolution QA team
evolution[google]
Depends on:
Blocks:
 
 
Reported: 2010-05-19 21:56 UTC by Philip Withnall
Modified: 2013-09-14 16:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add photo support to Google Contacts backend (29.36 KB, patch)
2011-07-10 15:18 UTC, Philip Withnall
committed Details | Review
Fix cancellation and progress reporting for cold-cache queries (6.73 KB, patch)
2011-08-16 23:42 UTC, Philip Withnall
committed Details | Review

Description Philip Withnall 2010-05-19 21:56:00 UTC
Add support for getting/setting contacts' photos with the Google Contacts backend.
Comment 1 Philip Withnall 2010-12-11 23:53:50 UTC
This will have to depend on libgdata 0.8.0, as there are a few problems with the contact photo API in libgdata 0.7.x, and 0.8.0 will break API to fix them.
Comment 2 Philip Withnall 2011-07-10 15:18:06 UTC
Created attachment 191630 [details] [review]
Add photo support to Google Contacts backend

Here's a patch which adds photo support. The code for it's only compiled if libgdata ≥ 0.9.0 is available; if not, everything should compile as before without photo support.
Comment 3 Philip Withnall 2011-07-10 15:22:34 UTC
Ah, there is one slight problem with this patch: it no longer reports the progress or allows cancellation of downloading all the contacts (e.g. with a cold cache). I'll fix this up once the first patch is reviewed and committed, though.
Comment 4 Milan Crha 2011-08-01 20:38:07 UTC
I didn't tested this, but looks mostly good. My minor comments:
a) chunk in e_book_backend_google_get_backend_property, after this change the array ends always with a comma. There was some cleanup some time ago to avoid this, as it introduces compiler warnings on certain systems/compilers, if I recall correctly. I suggest to add the new value as the pre-last one.

b) I would use assert/g_assert. It's not good to crash whole application when something doesn't work.

c) comparing against TRUE, as you have couple times "x == TRUE". This is mostly a personal preference, though there is one argument why to not compare against it. The expression will work for all cases where 'x' is assigned strictly to TRUE or FALSE, but if you have it assigned a boolean expression result, like "x = a && b;", then there is no guarantee that compiler uses same TRUE value as GLib defines it. I'll keep it up to you, whether you will change it or not.

Feel free to commit when you change above a) and b).
Comment 5 Philip Withnall 2011-08-01 22:29:48 UTC
Comment on attachment 191630 [details] [review]
Add photo support to Google Contacts backend

Fixed and merged, thanks.

commit 0c89996a87400fbc45b79271b0e9f72dc804af9c
Author: Philip Withnall <philip@tecnocode.co.uk>
Date:   Sun Jun 5 15:58:08 2011 +0100

    Bug 619135 — Add photo support to Google Contacts backend
    
    Add support for getting and setting photos on contacts from Google Contacts,
    including caching support.
    
    Closes: bgo#619135

 .../backends/google/e-book-backend-google.c        |  586 ++++++++++++++++++--
 1 files changed, 527 insertions(+), 59 deletions(-)
Comment 6 Philip Withnall 2011-08-01 22:30:17 UTC
(In reply to comment #3)
> Ah, there is one slight problem with this patch: it no longer reports the
> progress or allows cancellation of downloading all the contacts (e.g. with a
> cold cache). I'll fix this up once the first patch is reviewed and committed,
> though.

I'll keep the bug open until I get time to fix this.
Comment 7 Philip Withnall 2011-08-16 23:42:37 UTC
Created attachment 193999 [details] [review]
Fix cancellation and progress reporting for cold-cache queries

This patch re-adds cancellation and progress reporting support for queries for the list of contacts on a cold cache.
Comment 8 Milan Crha 2011-08-17 05:28:55 UTC
Looks good, please commit to sources. By the way, the configure.ac checks for libgdata 0.9.0, but if I compile against it then the build fails, only 0.9.1 works without any issue. Maybe you could fix this as well (by increasing dependency for HAVE_LIBGDATA_0_9).
Comment 9 Philip Withnall 2011-08-17 16:41:30 UTC
(In reply to comment #8)
> Looks good, please commit to sources. By the way, the configure.ac checks for
> libgdata 0.9.0, but if I compile against it then the build fails, only 0.9.1
> works without any issue. Maybe you could fix this as well (by increasing
> dependency for HAVE_LIBGDATA_0_9).

Merged, with a few additional tidy-ups to on_sequence_complete() and finish_operation().

EDS appears to already check for libgdata >= 0.9.1. Did you mean Evolution or EDS?

commit ee4952a4d5665a9c3d7819dabebb9eba4765245f
Author: Philip Withnall <philip@tecnocode.co.uk>
Date:   Wed Aug 17 17:38:39 2011 +0100

    google: Tidy up finish_operation()
    
    Merge on_sequence_complete() into it and ensure that all errors get
    propagated. See: bgo#619135

 .../backends/google/e-book-backend-google.c        |   58 ++++++++------------
 1 files changed, 22 insertions(+), 36 deletions(-)

commit f03645977340f1377264d1e9df8a651e0b9157ef
Author: Philip Withnall <philip@tecnocode.co.uk>
Date:   Wed Aug 17 00:41:10 2011 +0100

    google: Fix cancellation and progress reporting for cold-cache queries
    
    See: https://bugzilla.gnome.org/show_bug.cgi?id=619135#c3
    
    Closes: bgo#619135

 .../backends/google/e-book-backend-google.c        |  111 ++++++++++++++++++--
 1 files changed, 100 insertions(+), 11 deletions(-)
Comment 10 Milan Crha 2011-08-18 06:25:36 UTC
(In reply to comment #9)
> EDS appears to already check for libgdata >= 0.9.1. Did you mean Evolution or
> EDS?

I meant eds, that was the only one I was rebuilding for this patch. Anyway, it's nothing important if it's already there, as it's possible it was my fault.