Bug 53167 - [salut] Loses track of services' capabilities after a new service appears
Summary: [salut] Loses track of services' capabilities after a new service appears
Status: RESOLVED FIXED
Alias: None
Product: Ytstenut
Classification: Unclassified
Component: plugins (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: ytstenut
URL:
Whiteboard: review+
Keywords: patch
Depends on:
Blocks:
 
Reported: 2012-08-06 15:31 UTC by Simon McVittie
Modified: 2012-08-06 18:41 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
ytst_caps_manager_represent_client: in Salut, remember caps, not just forms (4.33 KB, patch)
2012-08-06 15:32 UTC, Simon McVittie
Details | Splinter Review

Description Simon McVittie 2012-08-06 15:31:43 UTC
Salut has a problematic representation of ContactCapabilities (Bug #53166) which does not cache the capabilities of each known client. As a result, each channel manager is responsible for remembering its part of the cache instead.

The one in ytstenut-plugins remembers and unions the data forms describing Yts services, but not the set of capability URIs representing "interest" in PEP nodes. As a result, each Ytstenut connection will lose interest in the PEP nodes in which it should have been interested whenever it creates a new transient Client, such as the one in telepathy-glib used for request-and-handle.

The long-term solution is to fix Bug #53166 and rework the Salut plugin to be more like the Gabble plugin, but for now we can just extend the caching behaviour.
Comment 1 Simon McVittie 2012-08-06 15:32:19 UTC
Created attachment 65182 [details] [review]
ytst_caps_manager_represent_client: in Salut, remember caps,  not just forms

Otherwise we lose our interest in services' statuses whenever a new
Ytstenut client is added - even if it's just a temporary one for the
request-and-handle pattern.
Comment 2 Olli Salli 2012-08-06 18:15:33 UTC
Comment on attachment 65182 [details] [review]
ytst_caps_manager_represent_client: in Salut, remember caps,  not just forms

Review of attachment 65182 [details] [review]:
-----------------------------------------------------------------

OK, except for

::: plugin-base/caps-manager.c
@@ +179,5 @@
> +add_to_set (gpointer key,
> +    gpointer value,
> +    gpointer user_data)
> +{
> +  DEBUG ("%s", (const gchar *) key);

You probably didn't intend to leave...

@@ +188,5 @@
>  add_to_array (gpointer key,
>      gpointer value,
>      gpointer user_data)
>  {
> +  DEBUG ("%s", (const gchar *) key);

... these here? If you did, please at least make them more useful ("client X: Y interests" or alike?)
Comment 3 Simon McVittie 2012-08-06 18:41:41 UTC
Pushed, without the DEBUG().


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.