Bug 27835 - Avoid being notified about Connection features nobody cares about
Summary: Avoid being notified about Connection features nobody cares about
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-spec (show other bugs)
Version: git master
Hardware: Other All
: medium enhancement
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL: http://git.collabora.co.uk/?p=user/sm...
Whiteboard: adds stable API, implementation exists
Keywords: patch
Depends on:
Blocks: 13349 27948
  Show dependency treegraph
 
Reported: 2010-04-26 06:43 UTC by Simon McVittie
Modified: 2010-10-18 06:54 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Simon McVittie 2010-04-26 06:43:13 UTC
MailNotification defines Subscribe and Unsubscribe methods to avoid doing lots of work (and network traffic) if no currently-running client actually cares about mail notifications.

Location doesn't do that, but it should, to avoid being spammed with Azimuth users' locations if no UI is actually displaying them. (Azimuth is an N900 app which advertises Location via Telepathy.)

Also, the Subscribe and Unsubscribe methods' names are horribly generic, causing problems for bindings that don't namespace things properly.

I propose to add AddClientInterest and RemoveClientInterest methods to the Connection, which would obsolete those methods and be usable for general interfaces. Here's the HTML:

http://people.freedesktop.org/~smcv/telepathy-spec-client_interests/spec/org.freedesktop.Telepathy.Connection.html#org.freedesktop.Telepathy.Connection.AddClientInterest

http://people.freedesktop.org/~smcv/telepathy-spec-client_interests/spec/org.freedesktop.Telepathy.Connection.html#org.freedesktop.Telepathy.Connection.RemoveClientInterest

http://people.freedesktop.org/~smcv/telepathy-spec-client_interests/spec/org.freedesktop.Telepathy.Connection.Interface.Location.html (see last paragraphs of: Description, GetLocations and /location)

http://people.freedesktop.org/~smcv/telepathy-spec-client_interests/spec/org.freedesktop.Telepathy.Connection.Interface.MailNotification.DRAFT.html (see Description, Subscribe, Unsubscribe)

ACI, RCI could go in Connection.FUTURE if we want to try them out, but to be particularly useful they need implementing in telepathy-glib anyway.

Proposed telepathy-glib API:

* implement ACI, RCI unconditionally
* have an API in TpBaseConnectionClass to declare which interfaces need to track "interest counts"
* have an API for Location to call: void tp_base_connection_add_client_interest (TpBaseConnection *, const gchar *unique_name, const gchar *interface);
* emit signals TpBaseConnection::clients-interested::$INTERFACE, TpBaseConnection::clients-uninterested::$INTERFACE on transitions between 0 and 1 total interest (we'd have to check that all D-Bus interface name characters are allowed in a signal detail)
Comment 1 Guillaume Desmottes 2010-04-26 06:55:37 UTC
I like the idea but if I understand the spec correctly that means that the client has to be registered on the bus as well? This could easily be done using the future TpBaseClient helpers but doesn't fit for, say, a map application displaying only positions of contacts: the app doesn't observe/approve/handle any channel type so can't be a Telepathy.Client.
This could easily be solved by allowing clients not being an observer/approver/handler.
Comment 2 Simon McVittie 2010-04-26 07:01:03 UTC
(In reply to comment #1)
> I like the idea but if I understand the spec correctly that means that the
> client has to be registered on the bus as well?

No, it's a unique name, like :1.42 - every client has one, and the service can retrieve it with dbus_g_method_get_sender() (or the language's equivalent) without having to be told explicitly.
Comment 3 Simon McVittie 2010-05-03 09:11:59 UTC
(In reply to comment #0)
> Proposed telepathy-glib API:
> 
> * implement ACI, RCI unconditionally
> * have an API in TpBaseConnectionClass to declare which interfaces need to
> track "interest counts"
> * have an API for Location to call: void tp_base_connection_add_client_interest
> (TpBaseConnection *, const gchar *unique_name, const gchar *interface);
> * emit signals TpBaseConnection::clients-interested::$INTERFACE,
> TpBaseConnection::clients-uninterested::$INTERFACE on transitions between 0 and
> 1 total interest (we'd have to check that all D-Bus interface name characters
> are allowed in a signal detail)

I implemented all of this in Bug #27948, except for the API for Location to call into.

In Bug #27948, if you try to Remove an interest you didn't have in the first place, no error is raised. I don't think this is a problem, because in practice, what are you going to do about it? - actually, I think Add and Remove should both be called with no reply (fire-and-forget).

I now wonder whether the Add D-Bus method should become singular (rationale: in practice, nobody's going to subscribe to multiple things at once).

The Remove method should probably stay plural, so a hypothetical future TpConnection can release all of its subscriptions at once.
Comment 4 Simon McVittie 2010-05-03 10:02:25 UTC
(In reply to comment #3)
> I implemented all of this in Bug #27948, except for the API for Location to
> call into.

I've now added this too.
Comment 5 Simon McVittie 2010-05-03 10:43:45 UTC
We should perhaps allow the strings to be either interfaces or tokens, rather than restricting to only interfaces as we currently do, to allow an interface to have more than one token.

The implementation in Bug #27948 doesn't care, and will act on arbitrary strings (some of the terminology in the API is wrong, but that's cosmetic and could be fixed quickly).
Comment 6 Simon McVittie 2010-05-04 07:12:30 UTC
Proposed telepathy-glib client side, which I haven't implemented yet:

15:08 < smcv> Zdra: if you request the LOCATION feature on at least one 
              TpContact, then its parent TpConnection will be interested in 
              Location until destroyed, is the idea
Comment 7 Simon McVittie 2010-05-17 10:24:13 UTC
(In reply to comment #6)
> Proposed telepathy-glib client side, which I haven't implemented yet:
> 
> 15:08 < smcv> Zdra: if you request the LOCATION feature on at least one 
>               TpContact, then its parent TpConnection will be interested in 
>               Location until destroyed, is the idea

Now implemented on Bug #27948.
Comment 8 Simon McVittie 2010-06-29 04:40:57 UTC
(In reply to comment #3)
> In Bug #27948, if you try to Remove an interest you didn't have in the first
> place, no error is raised. I don't think this is a problem, because in
> practice, what are you going to do about it? - actually, I think Add and Remove
> should both be called with no reply (fire-and-forget).

I've removed the error from the spec and defined the method as "never fails".

> I now wonder whether the Add D-Bus method should become singular (rationale: in
> practice, nobody's going to subscribe to multiple things at once).
> 
> The Remove method should probably stay plural, so a hypothetical future
> TpConnection can release all of its subscriptions at once.

Remove needs to stay plural, so I've left them both plural for symmetry.

(In reply to comment #5)
> We should perhaps allow the strings to be either interfaces or tokens, rather
> than restricting to only interfaces as we currently do, to allow an interface
> to have more than one token.

Done.
Comment 9 Guillaume Desmottes 2010-08-05 06:14:31 UTC
Spec looks sane to me.
Maybe we should have a way to formerly define interest tokens in the spec so they could be listed in their own subsection as the Contact Attributes. That's pretty convenient to easily see the tokens defined in an interface.
Comment 10 Simon McVittie 2010-08-10 06:39:24 UTC
(In reply to comment #9)
> Maybe we should have a way to formerly define interest tokens in the spec so
> they could be listed in their own subsection as the Contact Attributes. That's
> pretty convenient to easily see the tokens defined in an interface.

I'm inclined to defer this until we actually define one :-P

(At the moment, no interface needs a token other than "all of this interface", and it seems sensible to use the interface name itself for that; unless reviewers think Location and MailNotification should use .../location and .../mail-notification?)
Comment 11 Simon McVittie 2010-09-03 08:37:32 UTC
Sjoerd, Nicolas: I believe you wanted this API for Location and MailNotification respectively. Any thoughts on it? Do you think it's good to merge/insta-undraft?
Comment 12 Nicolas Dufresne 2010-09-03 08:55:42 UTC
(In reply to comment #11)
> Sjoerd, Nicolas: I believe you wanted this API for Location and
> MailNotification respectively. Any thoughts on it? Do you think it's good to
> merge/insta-undraft?

Just re-read the spec, found no issue, +1 for merge/insta-undraft.
Comment 13 Simon McVittie 2010-10-18 06:47:01 UTC
Branch updated based on Sjoerd's comments on IRC.
Comment 14 Simon McVittie 2010-10-18 06:54:19 UTC
r+ from Sjoerd on IRC, fixed in git for 0.21.3.


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.