[Bug 24061] TpAccount, TpAccountManager: add convenience API similar to libempathy's

[bugzilla-daemon at freedesktop.org]

http://bugs.freedesktop.org/show_bug.cgi?id=24061





--- Comment #1 from Simon McVittie <simon.mcvittie at collabora.co.uk>  2009-09-21 07:21:23 PST ---
API review:

> #define TP_ACCOUNT_MANAGER_FEATURE_CORE \
>   g_quark_from_static_string ("tp-account-manager-feature-core")

I'd have made this expand to a function call so the linker has already done the
hash table lookup, but, whatever. Same for Account.

> TpAccount *tp_account_manager_get_account_for_connection (
>     TpAccountManager *manager, TpConnection *connection);

Drop this, it can only work if all accounts are guaranteed to be cached, and if
we really need it we can add it later.

> TpAccount *tp_account_manager_get_account (TpAccountManager *manager,
>     const gchar *path);

General point: if it's not guaranteed to be ready, the docs should say so.
(It's not, afaics.)

I don't think get_account makes a great deal of sense, we should just have
ensure_account?

ensure_account should create an object even if the TpAM doesn't think the
account exists, to avoid races. The docs should probably say it does.

(TpQt4 equivalent of ensure_account is accountForPath)

> GList *tp_account_manager_get_accounts (TpAccountManager *manager);

> void tp_account_manager_request_global_presence (TpAccountManager *manager,
>     TpConnectionPresenceType type, const gchar *status, const gchar *message);

I'd prefer this called something like request_all_presences() or
set_all_requested_presences().

> TpConnectionPresenceType tp_account_manager_get_requested_global_presence (
>     TpAccountManager *manager, gchar **status, gchar **message);

Can this be removed? I'm not sure why we need it.

> TpConnectionPresenceType tp_account_manager_get_global_presence (
>     TpAccountManager *manager, gchar **status, gchar **message);

get_most_available_presence() please.

Document what happens if no accounts are enabled and valid (should be OFFLINE,
"offline", "").

Document the invariant that there exists at least one account with this
presence (this is never synthesized from multiple accounts' presences).

> void tp_account_manager_prepare_async (TpAccountManager *account,
>     GQuark* features, GAsyncReadyCallback callback, gpointer user_data);
> 
> gboolean tp_account_manager_prepare_finish (TpAccountManager *account,
>     GAsyncResult *result, GError **error);

become_ready_async()? Also in Account.

> gboolean tp_account_manager_set_features (TpAccountManager *account,
>     const GQuark* features);

Remove this, and make become_ready_async() callable with a NULL callback to get
the same result. Also in Account.

> const GQuark * tp_account_manager_get_features (TpAccountManager *account);

This should have the same semantics as tpqt4's ReadyObject, which has these
accessors:

* requestedFeatures() - features that the async operation started, and may or
may not have finished
* actualFeatures() - features that the async operation finished successfully
and the feature does really work (e.g. Connection has Avatars)
* missingFeatures() - features that the async operation finished, but the
feature can't work (e.g. Connection doesn't have Avatars)

and isReady() means that the proxy was not invalidated yet, and the feature is
in either the actual set or the missing set.

> gboolean tp_account_is_just_connected (TpAccount *account);

No, no, and furthermore, no. (Also, the corresponding function in Empathy
should be renamed to connected_recently() to avoid the ambiguity of whether
"just" means "recently" or "only".)

> TpConnection *tp_account_get_connection (TpAccount *account);

I think this should be allowed to return unready connections.

The fact that the "connection-ready" state in TpConnection implies CONNECTED is
unfortunate from a future-tp-glib point of view, and in future (when we have
the "interactive SASL authentication" D-Bus API) accounts' unconnected
connections will probably be rather more interesting.

> const gchar *tp_account_get_connection_manager (TpAccount *account);

telepathy-qt4 also has FeatureProtocolInfo; we should clone this bug as
"implement TpAccount FeatureProtocolInfo, which gets its protocol info from a
hidden TpConnectionManager" or something, as future work.

> TpConnectionStatus tp_account_get_connection_status (TpAccount *account);

Perhaps this should be TpConnectionStatus get_connection_status (TpA *, TpCSR
*maybe_reason) like in TpConnection?

> TpConnectionPresenceType tp_account_get_presence (TpAccount *account);
> const gchar *tp_account_get_status (TpAccount *account);
> const gchar *tp_account_get_status_message (TpAccount *account);

tp_account_get_current_presence(), please. It's very deliberate that Account
doesn't have a Presence member (MC 4's "presence" and "presence_actual" were
horrible).

Perhaps this should return a presence type, with status/message optionally
returned through pointers too? tp_account_get_current_presence_type() could
return the enum member if you think that's valuable.

> void tp_account_refresh_properties (TpAccount *account);

Should never be needed... if the AM crashes, TpAccount should recover
automatically when it notices the AM name come back, updating any accounts that
are still there and invalidating any accounts that turn out to have disappeared
(telepathy-qt4 implements this).

> const gchar *tp_account_get_unique_name (TpAccount *account);

Remove this. tp_proxy_get_object_path() suffices, and "unique name" is D-Bus
jargon already, with a different meaning.

I would not necessarily object to a method tp_account_get_object_path_tail() or
tp_account_get_identifier() or something, which returned the part of the object
path after the common prefix (this is what MC internally, and inappropriately,
calls the account's "unique name").


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.