Bug 31001

Summary:	channel client factory: add API for TpChannel preparation
Product:	Telepathy	Reporter:	Guillaume Desmottes <guillaume.desmottes>
Component:	tp-glib	Assignee:	Guillaume Desmottes <guillaume.desmottes>
Status:	RESOLVED FIXED	QA Contact:	Telepathy bugs list <telepathy-bugs>
Severity:	enhancement
Priority:	medium	Keywords:	patch
Version:	unspecified
Hardware:	Other
OS:	All
URL:	http://git.Collabora.co.uk/?p=user/cassidy/telepathy-glib;a=shortlog;h=refs/heads/features-31001
Whiteboard:	review+
i915 platform:		i915 features:

Description Guillaume Desmottes 2010-10-20 05:24:35 UTC

On bug #29973 we considered 2 options regarding preparations of extra features:

A) Add a prepare async API on TpClientChannelFactoryInterface doing the preparation of a TpChannel

b) Add API to get a list of features to be prepared and let the user (TpBaseClient basically) doing the preparation.

The consensus seems to go for B) as TpBaseClient already has code to prepare the channels.
If that's ok I'll do the following changes:

-  Add GQuark *features tp_client_channel_factory_get_features (TpClientChannelFactoryInterface *self, TpChannel *self) implemented using a virtual function returning CORE as default implementation.
Or maybe we should return a more flexible structur to make it easier to merge it with TpBaseClient's features?

- Modify TpBaseClient to prepare the union of its features and the ones returned by _get_features(). I guess that's fine if we ask to prepare twice the same feature.

- Implement _get_features () inTpAutomaticProxyFactory  by returning CORE + TP_CHANNEL_FEATURE_GROUP if the channel implements Group.

- I'm not sure if we want to add API on TpClientChannelFactoryInterface to add features? It seems weird to add features on a factory which already created channel. And clients can still add feature on TpBaseClient or create their own factory if needed.

Comment 1 Simon McVittie 2010-10-20 05:42:25 UTC

(In reply to comment #0)
> -  Add GQuark *features tp_client_channel_factory_get_features
> (TpClientChannelFactoryInterface *self, TpChannel *self) implemented using a
> virtual function returning CORE as default implementation.
> Or maybe we should return a more flexible structur to make it easier to merge
> it with TpBaseClient's features?

I'm not sure what structure we'd return; perhaps a GArray?

> - Modify TpBaseClient to prepare the union of its features and the ones
> returned by _get_features(). I guess that's fine if we ask to prepare twice the
> same feature.

It should be OK, yes; preparing one "batch" which may contain duplicates is probably a little more efficient than two overlapping "batches".

> - Implement _get_features () inTpAutomaticProxyFactory  by returning CORE +
> TP_CHANNEL_FEATURE_GROUP if the channel implements Group.

IMO there's no need for this: just include TP_CHANNEL_FEATURE_GROUP unconditionally, and have the guarantee be "well, we tried".

> - I'm not sure if we want to add API on TpClientChannelFactoryInterface to add
> features? It seems weird to add features on a factory which already created
> channel. And clients can still add feature on TpBaseClient or create their own
> factory if needed.

Yes, I think leaving this out is the conservative route. You could always have an EmpathyProxyFactory subclass of TpAutomaticProxyFactory which just overrides this method, if you wanted.

Comment 2 Guillaume Desmottes 2010-10-21 05:55:10 UTC

Implemented in http://git.Collabora.co.uk/?p=user/cassidy/telepathy-glib;a=shortlog;h=refs/heads/features-31001

Comment 3 Simon McVittie 2010-10-21 06:11:18 UTC

> + * Once a channel has be created by a factory using

has been created

> + * Returns: (transfer container): a newly allocated #GArray

I think containers of integers are usually said to be "transfer full", since for non-freeable contents it's equivalent to "transfer container" anyway, and "full" is simpler to explain.

I think the name should say _dup_ (and the same for the underlying virtual method).

> +array_contain_feature (GArray *features,

array_contains_feature, and @features should be const

Comment 4 Guillaume Desmottes 2010-10-21 06:25:08 UTC

All fixed.

Comment 5 Simon McVittie 2010-10-21 06:27:27 UTC

go go go

Comment 6 Guillaume Desmottes 2010-10-21 06:29:10 UTC

Merged to master; will be in 0.13.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.