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 656184 - Add is-quiescent property
Add is-quiescent property
Status: RESOLVED FIXED
Product: folks
Classification: Platform
Component: libfolks
git master
Other All
: Normal enhancement
: folks-0.6.2
Assigned To: folks-maint
folks-maint
: 658266 (view as bug list)
Depends on:
Blocks: 657063 657967
 
 
Reported: 2011-08-08 22:36 UTC by Philip Withnall
Modified: 2011-09-07 10:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add is-quiescent property (32.78 KB, patch)
2011-08-08 22:42 UTC, Philip Withnall
committed Details | Review
patch (2.50 KB, patch)
2011-09-05 21:28 UTC, Raul Gutierrez Segales
reviewed Details | Review

Description Philip Withnall 2011-08-08 22:36:20 UTC
Alex needs a way to know when folks thinks it's reached a quiescent state[1] so that he can know when to stop waiting for a given individual to appear in the aggregator (e.g. if requesting that specific individual).

This also has similar applications in the test suite, but I haven't done any work on that.

[1]: where by “quiescent” I mean that all of the backends which are installed when folks is first started are made available, and all their initial personas have been exposed by the relevant persona stores. In other words, a persona store has reached a quiescent state when the only future emissions of the personas-changed signal are expected to be in response to manual linking/unlinking operations, and personas appearing and disappearing at runtime (e.g. if they go offline).
Comment 1 Philip Withnall 2011-08-08 22:42:49 UTC
Created attachment 193453 [details] [review]
Add is-quiescent property

https://www.gitorious.org/folks/folks/commits/656184-is-quiescent

Here's a patch to do that. This has been tested with folks-inspect, and seems to work there. I haven't had a chance to modify gnome-contacts and test it with that. The documentation comments for the new properties could do with some attention, since I couldn't come up with anything better.
Comment 2 Travis Reitter 2011-08-09 14:57:23 UTC
I'll check this out for 0.6.1 (to avoid delaying this release any longer than it already has been).
Comment 3 Raul Gutierrez Segales 2011-08-29 12:46:56 UTC
Didn't make this release; punting to 0.6.2.
Comment 4 Philip Withnall 2011-09-01 17:35:34 UTC
(In reply to comment #2)
> I'll check this out for 0.6.1 (to avoid delaying this release any longer than
> it already has been).

How about checking it out for 0.6.2? :-)
Comment 5 Travis Reitter 2011-09-02 18:23:00 UTC
Review of attachment 193453 [details] [review]:

+* TODO — Add quiescence API

This should be TODONE

+      /* If this is the first contacts-added notification, assume we've reached
+       * a quiescent state. TODO: Verify that this is the case. */

Is this the case?

I asked in #evolution but didn't get a definitive answer. I suspect it depends upon the backend (and possibly the server). It's not the end of the world if this gets issued a little early (though obviously suboptimal).


backends/telepathy/lib/tpf-persona-store.vala
=======================

+  private void _check_is_quiescent ()

_notify_if_quiescent() would be a better name. This also happens in other source files.


(various source files)
+                  this.removed ();

It would be nice if these were in a separate, prior commit (though not critical).


Test cases would be nice (but aren't critical).

Other than that, it looks good.
Comment 6 Philip Withnall 2011-09-02 20:34:42 UTC
Comment on attachment 193453 [details] [review]
Add is-quiescent property

Committed with the fixes. Some brief testing indicates that my assumption about the eds backend's notifications about added contacts is correct.

commit 77ee30d3c8df1fab219ca7e0d1d08f6c263184c8
Author: Philip Withnall <philip@tecnocode.co.uk>
Date:   Fri Sep 2 21:15:35 2011 +0100

    Bug 656184 — Add is-quiescent property
    
    This allows clients to determine when the IndividualAggregator has reached
    a quiescent state — it's received and linked all of the personas it should
    do at startup, and from that point onwards will only relink personas in
    response to (for example), persona stores dynamically being added and removed
    at runtime.
    
    Closes: bgo#656184

 NEWS                                             |    3 +
 backends/eds/eds-backend.vala                    |   19 ++++
 backends/eds/lib/edsf-persona-store.vala         |   21 +++++
 backends/key-file/kf-backend.vala                |   19 ++++
 backends/key-file/kf-persona-store.vala          |   17 ++++
 backends/libsocialweb/lib/swf-persona-store.vala |   23 +++++
 backends/libsocialweb/sw-backend.vala            |   19 ++++
 backends/telepathy/lib/tpf-persona-store.vala    |   64 +++++++++++++-
 backends/telepathy/tp-backend.vala               |   19 ++++
 backends/tracker/lib/trf-persona-store.vala      |   18 ++++
 backends/tracker/tr-backend.vala                 |   20 ++++
 folks/backend-store.vala                         |    4 +-
 folks/backend.vala                               |   15 +++
 folks/individual-aggregator.vala                 |  101 +++++++++++++++++++++-
 folks/persona-store.vala                         |   13 +++
 15 files changed, 371 insertions(+), 4 deletions(-)

commit 823a6ae6b8bb63dbd50e5fae56793a20b1ee2d34
Author: Philip Withnall <philip@tecnocode.co.uk>
Date:   Fri Sep 2 21:15:08 2011 +0100

    eds/tracker: Ensure the PersonaStores are removed on error

 backends/eds/lib/edsf-persona-store.vala    |   12 ++++++++++++
 backends/tracker/lib/trf-persona-store.vala |    3 +++
 2 files changed, 15 insertions(+), 0 deletions(-)
Comment 7 Alexander Larsson 2011-09-05 09:32:43 UTC
I do get an is_quiescent notify on the aggregator, but before the individuals_changed signal for the new individuals has been sent out, so its not all that useful to me.
Comment 8 Travis Reitter 2011-09-05 16:44:41 UTC
See the description of bug #658266 for another case that needs to be handled before we close this bug.
Comment 9 Travis Reitter 2011-09-05 16:45:32 UTC
*** Bug 658266 has been marked as a duplicate of this bug. ***
Comment 10 Raul Gutierrez Segales 2011-09-05 16:47:21 UTC
(In reply to comment #7)
> I do get an is_quiescent notify on the aggregator, but before the
> individuals_changed signal for the new individuals has been sent out, so its
> not all that useful to me.

What PersonaStores do you have? I've got a bunch of Telepathy PersonaStores that never reach the is-quiescent state because this._got_stored_channel_members never gets to be true (though this._got_self_handle does).
Comment 11 Raul Gutierrez Segales 2011-09-05 18:16:57 UTC
So my Telepathy PersonaStores are not reaching is-quiescent state because the only path for this._got_stored_channel_members = true is either if the Store is disconnected or through _stored_channel_group_members_changed_detailed_cb ().

But in my case that callback is never called.
Comment 12 Philip Withnall 2011-09-05 18:55:39 UTC
I can't test it (since I wasn't having the problem you're having in the first place), but this might fix it:
https://www.gitorious.org/folks/folks/commit/9e0633853696d2dfea45f4db307bfbbebbed5277
Comment 13 Raul Gutierrez Segales 2011-09-05 21:07:26 UTC
(In reply to comment #12)
> I can't test it (since I wasn't having the problem you're having in the first
> place), but this might fix it:
> https://www.gitorious.org/folks/folks/commit/9e0633853696d2dfea45f4db307bfbbebbed5277

This fixes the problem for PersonaStores corresponding to Jabber accounts, the problem persist for Msn PersonaStores:

Persona store '/org/freedesktop/Telepathy/Account/butterfly/msn/foobar_40hotmail_2ecom0' with 305 personas:
  type-id               telepathy
  display-name          foobar@hotmail.com
  id                    /org/freedesktop/Telepathy/Account/butterfly/msn/rau_5fgutierrez_40hotmail_2ecom0
  personas              Set of 305 personas
  can-add-personas      FOLKS_MAYBE_BOOL_TRUE
  can-alias-personas    FOLKS_MAYBE_BOOL_TRUE
  can-group-personas    FOLKS_MAYBE_BOOL_TRUE
  can-remove-personas   FOLKS_MAYBE_BOOL_TRUE
  is-prepared           TRUE
  is-quiescent          FALSE
  is-writeable          FALSE

I guess we are missing one more path.
Comment 14 Philip Withnall 2011-09-05 21:13:20 UTC
Committed.

commit 0ee29949100e6211775bd98527740e535db97bbf
Author: Philip Withnall <philip@tecnocode.co.uk>
Date:   Mon Sep 5 19:53:45 2011 +0100

    telepathy: Potentially fix is-quiescent
    
    See: bgo#656184

 backends/telepathy/lib/tpf-persona-store.vala |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)
Comment 15 Raul Gutierrez Segales 2011-09-05 21:28:54 UTC
Created attachment 195736 [details] [review]
patch

Make libsocialweb PersonaStores immediately quiescient.
Comment 16 Travis Reitter 2011-09-05 21:43:59 UTC
Review of attachment 195736 [details] [review]:

+                           *        social client to see if its available

"its" -> "it's"

Also, please wrap this block comment like:

/* FIXME: for lsw Stores with 0 contacts or badly
 * configured (or not authenticated, etc) we are
 * condemned ...

Other than that, it seems fine to me.
Comment 17 Raul Gutierrez Segales 2011-09-05 21:54:48 UTC
Merged:

commit 81d9e2ca8c24d589c463d6ea7c637315122d3292
Author: Raul Gutierrez Segales <rgs@collabora.co.uk>
Date:   Mon Sep 5 22:25:56 2011 +0100

    libsocialweb: make PersonaStores immediately quiescent
    
    Because we don't know if a libsocialweb service has 0
    contacts or is not well configured or is not authenticated, etc.;
    we might end up forever for contacts to arrive and never
    achieve quiescence. Until libsocialweb provides us with more
    information about the status of service (is it configured and
    authenticated? How many contacts should we receive? etc.) we
    immediately declare it quiescent after preparing it.

Will follow up with the libsocialweb people to see if what we need to achieve quiescence is available or doable.
Comment 18 Raul Gutierrez Segales 2011-09-06 10:35:16 UTC
And here is the reason why msn stores don't reach quiescence :

[/org/freedesktop/Telepathy/Account/butterfly/msn/rau_5fgutierrez_40hotmail_2ecom0] Failed to add channel 'stored': TargetID stored not valid for type 3

Because we swallow errors when we add new channels:

http://git.gnome.org/browse/folks/tree/backends/telepathy/lib/tpf-persona-store.vala#n1705

we missed this one.

I get something similar for my SIP account:

[/org/freedesktop/Telepathy/Account/sofiasip/sip/_31167_40voip_2ecollabora_2eco_2euk0] Failed to add channel 'publish': Handle type not supported by this connection manager
[/org/freedesktop/Telepathy/Account/sofiasip/sip/_31167_40voip_2ecollabora_2eco_2euk0] Failed to add channel 'stored': Handle type not supported by this connection manager
[/org/freedesktop/Telepathy/Account/sofiasip/sip/_31167_40voip_2ecollabora_2eco_2euk0] Failed to add channel 'subscribe': Handle type not supported by this connection manager

Looking a way around this.
Comment 19 Raul Gutierrez Segales 2011-09-06 11:06:51 UTC
After checking out with Jonny the problem is butterfly not implementing http://telepathy.freedesktop.org/spec/Channel_Type_Contact_List.html.

We should either push CMs to implement this or figure out a way around to have working is-quiescent with telepathy stores.
Comment 20 Raul Gutierrez Segales 2011-09-06 12:14:51 UTC
I've pushed a workaround for broken CMs:

commit eff9dfdbd8f82167764a2064d0c4731c2cb678b7
Author: Raul Gutierrez Segales <rgs@collabora.co.uk>
Date:   Tue Sep 6 13:05:45 2011 +0100

    Workaround for Tpf.PersonaStores with no stored channels support
    
    Helps: https://bugzilla.gnome.org/show_bug.cgi?id=656184

diff --git a/backends/telepathy/lib/tpf-persona-store.vala b/backends/telepathy/lib/tpf-persona-store.vala
index 7388975..b8b9848 100644
--- a/backends/telepathy/lib/tpf-persona-store.vala
+++ b/backends/telepathy/lib/tpf-persona-store.vala
@@ -1706,6 +1706,16 @@ public class Tpf.PersonaStore : Folks.PersonaStore
         {
           debug ("Failed to add channel '%s': %s\n", name, e.message);
 
+          /* If the Connection doesn't support 'stored' channels we
+           * pretend we've received the stored channel members.
+           *
+           * When this happens it probably means the ConnectionManager doesn't
+           * implement the Channel.Type.ContactList interface.
+           *
+           * See: https://bugzilla.gnome.org/show_bug.cgi?id=656184 */
+           this._got_stored_channel_members = true;
+           this._notify_if_is_quiescent ();
+
           /* XXX: assuming there's no decent way to recover from this */
 
           return null;
Comment 21 Raul Gutierrez Segales 2011-09-07 10:25:01 UTC
I've filed a bug against libsocialweb so we can get the needed support to implement correct quiescence support in lsw PersonaStores:

https://bugzilla.gnome.org/show_bug.cgi?id=658445

So closing this.