Bug 655431 – add-persona test fails

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 655431 - add-persona test fails


Summary:	add-persona test fails


Status:	RESOLVED DUPLICATE of bug 655374

Product:	folks
Classification:	Platform
Component:	e-d-s backend
Version:	git master
Hardware:	Other Linux

Importance:	Normal normal
Target Milestone:	Unset
Assigned To:	Raul Gutierrez Segales
QA Contact:	Raul Gutierrez Segales

URL:
Whiteboard:

Depends on:
Blocks:

Reported:	2011-07-27 15:22 UTC by Raul Gutierrez Segales
Modified:	2011-07-29 20:04 UTC

See Also:
GNOME target:	---
GNOME version:	---

Description Raul Gutierrez Segales 2011-07-27 15:22:06 UTC

After commit :

commit 3862f876a9bef82f4fcf838a00c81ef5c57ae353
Author: Philip Withnall <philip@tecnocode.co.uk>
Date:   Mon Jun 13 10:54:23 2011 +0100

    Bug 650414 — Need better APIs to handle image data
    
    Change AvatarDetails.avatar to have type LoadableIcon. By introducing
    a libfolks-wide avatar cache, propagate this change to all the backends.
    
    This breaks the API of AvatarDetails.
    
    Closes: bgo#650414


tests/eds/add-persona fails with:

[rgs@andromeda eds]$ make check TESTS=add-persona
add-persona       add-persona.c     add-persona.o     add-persona.vala  
[rgs@andromeda eds]$ make check TESTS=add-persona
make  check-TESTS
make[1]: Entering directory `/debian/rgs/devel/folks/tests/eds'
/AddPersonaTests/test adding a persona to e-d-s : 
folks-CRITICAL **: folks_persona_build_uid: assertion `persona_id != NULL' failed

+ Trace 227915

#0 __libc_waitpid
at ../sysdeps/unix/sysv/linux/waitpid.c line 41
#1 g_on_error_stack_trace
at gbacktrace.c line 192
#2 g_logv
at gmessages.c line 527
#3 g_log
at gmessages.c line 573
#4 folks_persona_build_uid
at persona.c line 295
#5 _edsf_persona_store_set_contact_avatar_co
at edsf-persona-store.c line 2998
#6 edsf_persona_store_real_add_persona_from_details_co
at edsf-persona-store.c line 1559
#7 g_simple_async_result_complete
at gsimpleasyncresult.c line 749
#8 complete_in_idle_cb
at gsimpleasyncresult.c line 761
#9 g_main_dispatch
at gmain.c line 2500
#10 g_main_context_dispatch
at gmain.c line 3083
#11 g_main_context_iterate
at gmain.c line 3161
#12 g_main_loop_run
at gmain.c line 3369
#13 add_persona_tests_test_add_persona
at add-persona.c line 559
#14 test_case_run
at gtestutils.c line 1227
#15 g_test_run_suite_internal
at gtestutils.c line 1280
#16 g_test_run_suite_internal
at gtestutils.c line 1291
#17 g_test_run_suite
at gtestutils.c line 1336
#18 _vala_main
at add-persona.c line 1846
#19 __libc_start_main
at libc-start.c line 226
#20 _start


The reason it happens is cause we are trying to get an E.Contact's UID in backends/eds/lib/edsf-persona-store.vala:

  private async void _set_contact_avatar (E.Contact contact,
      LoadableIcon? avatar)
    {
      var uid = Folks.Persona.build_uid (BACKEND_NAME, this.id,
          (string) Edsf.Persona._get_property_from_contact (contact, "id"));
      .....

Calling _set_contact_avatar() assumes that the E.Contact already has a UID.. which is not the case when we call it from add_persona_from_details().

More to the point, we shouldn't add images to the Avatar Cache  as a result of the user setting the avatar property on a Persona or creating a new Persona, but as a consequence of e-d-s alerting us of a new Contact being created or a Contact being modified. This is the strategy we use with all property setters.

Comment 1 Raul Gutierrez Segales 2011-07-27 15:37:54 UTC

And more to the point, we are setting Contact's Photo via:

              // Cache the avatar so that it has a URI
              var uri = yield cache.store_avatar (uid, avatar);

              // Set the avatar on the contact
              var cp = new ContactPhoto ();
              cp.type = ContactPhotoType.URI;
              cp.set_uri (uri);

which is supported by e-d-s of course, but I think they mostly use inlined photos (a quick git grep in Evo  and e-d-s says so).

Comment 2 Travis Reitter 2011-07-29 20:04:03 UTC

(In reply to comment #1)
> And more to the point, we are setting Contact's Photo via:
> 
>               // Cache the avatar so that it has a URI
>               var uri = yield cache.store_avatar (uid, avatar);
> 
>               // Set the avatar on the contact
>               var cp = new ContactPhoto ();
>               cp.type = ContactPhotoType.URI;
>               cp.set_uri (uri);
> 
> which is supported by e-d-s of course, but I think they mostly use inlined
> photos (a quick git grep in Evo  and e-d-s says so).

In fact, I don't think we can safely do this, since it makes the EDS contact depend upon our cache -- which means it's no longer a cache (where we should be able to safely remove it at any point without data loss).

Comment 3 Travis Reitter 2011-07-29 20:04:46 UTC

Thanks for the bug report. This particular bug has already been reported into our bug tracking system, but please feel free to report any further bugs you find.

*** This bug has been marked as a duplicate of bug 655374 ***