[Bug 29451] Too many asynchronous setup steps needed for basic usage

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=29451

Olli Salli <ollisal at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         Depends on|29609                       |

--- Comment #3 from Olli Salli <ollisal at gmail.com> 2010-08-17 06:41:12 PDT ---
Let's decide on the semantics level by level, starting with Account(Manager),
and bordering Connection.

The current situation in tp-qt4 is that all signals and methods returning
Accounts in AccountManager return base account classes, with FeatureCore always
ready. The account-filter-by-rcc branch from bug #29090 adds a
AccountManager::FeatureFilterByCapabilities feature, which makes
Account::FeatureCapabilities ready on existing, but not newly created accounts
for filtering purposes (?!). (Said Account feature gets RCC from either an
online connection or CM protocol info).

Also, tp-qt4 currently has no pseudo-singleton AccountManager, or any other
suitable link-up between ClientRegistrar and an AM. Client channel handling
currently always creates new Account and Connection objects.

This has some problems:
 a) Implementing "pass-through" features like said FilterByCapabilities is
reportedly hard to get right for all cases
 b) If application code wants more Account features than just Core (and
possibly Capabilities), it needs to call becomeReady() on all of the Accounts,
one by one, including any new ones from the newAccount() signal, and wait for
the operation to finish
 c) Due to always supplying newly created Account and Connection objects in
Client channel handling, whether an AccountManager already would have those
ready or not, leads to yet more becomeReady steps needed, including the Core
feature on those Accounts. Also, contacts and handles from two different
Connection objects aren't interchangeable. Arguably, this could be partially
solved by doing the comparisons per-object-path instead of
per-proxy-object-address, but contacts from connection B created for handling
would still go invalid when the Connection B is destroyed, even if the existing
Connection A with the same actual object path survives, which without extra
precautions is when the handling method returns). This is bug #29609.
 d) One can't specify the subclass of Account to construct in any case - even
by subclassing AccountManager itself, actually. Subclassability was one of the
original design goals for tp-qt4, but tbh I'm not sure if ever really cared
about it - do we?
 e) The connection objects returned by Account::connection() have no guarantees
on features ready, so becomeReady() calls are always needed for that
Connection. As much as possible, I'd like to postpone discussing Connection
stuff to after the Account improvements have been prototyped though.

So, approach 1) to solve these: (gently extrapolated and ported from
Guillaume's basic tp-glib idea in bug #29529 to tp-qt4 terms)
 - include knowledge of which Features are for which class somehow in the
Feature itself
 - make AccountManager readifying code pass any Account features given in
AccountManager::becomeReady() to the sub-accounts and wait for them to become
ready in those before becoming ready itself
 - ditto for Connection features, although this would probably be handled
recursively by a similar implementation in the Account
 - maybe it's actually: In AM, use whatever features you recognize, otherwise
pass them to contained objects, and ditto for them

This would solve a) by making FeatureFilterByCapabilities unnecessary, just
allowing passing Account::FeatureCapabilities to AM::bR(). b) would be solved
by definition. c) is already half-solved in tp-glib by using the bus-wide
singleton AM proxy in the handling code BUT only if there's a suitable existing
Account and Connection guaranteed by the application, hence the last caveat
below. e) would be, recursively, solved.

Caveats (reiterating and expanding on previous discussion):
 - d) is not solved, because the subclass can't be supplied - but do we care?
 - being able to pass "contained" object features to becomeReady() might need
API/ABI breaks at least in tp-qt4 - not sure though, and we are heading towards
one soon anyway.
 - if the application hasn't requested and made ready a Connection using the
Account in question yet, one still needs to make Connections ready in the
application when doing channel handling (because it'll be newly created, or
made ready on a bus-wide singleton with some default set of features (core), if
there has been time to do so). This will probably happen often in handlers in
non-monolithic applications, where the application (eg. a tube client) is only
started and begins introspecting objects by service activation on the handler
method.

Another approach 2), formalizing on the hand-waving I did with Andre last week
and somewhat inscribed in the bug description is "object factories":
 - application specifies on an AccountFactory object (or implements its own
AccountFactory subclass and registers it)
 - the factory either has API to specify a subclass factory function and a
QList of Features, or determines these in a specialized subclass implementation
 - the factory has a create() (virtual) method which AccountManager calls when
it needs an Account object
   - constructs the desired Account subclass
   - calls becomeReady with the set features
 - AccountManager waits on a signal that all of the features the factory
requested are now ready [1]
 - we can make the factory instance / its settings either settable on a
specific AccountManager, bus-connection globals or library globals, it's not
that different but requires special consideration on the Client adaptor linkup

This solves a) if we document that "if you want to filter by capabilities,
enable FeatureCapabilities on the AccountFactory", b) by definition, although
only for a "base set" of features the entire application needs -
domain-specific / optional features should still be upgraded with becomeReady,
d) by definition and e) by definition if we do the same for a ConnectionFactory

Issue c) might be solved by this depending on improvements to the channel
handling API. If we make it somehow possible to specify the AccountManager
instance to co-operate with on ClientRegistrar / the Adaptors (AbstractClient*
is just a thin base class with no actual functionality!!!), we don't need a
(bus-wide) singleton AM or factories to enable constructing Accounts /
Connections with the desired factory settings. This kind of link-up would also
allow the Account convenience channel request methods to be improved to allow
waiting for the requested channel to arrive, bypassing the usual handling
mechanism.

Caveats:
 - if some part of the application wants to access Accounts before all of their
features some other part of the application requested are ready, we must add
another "giveMeAccountsEvenIfTheyAren'tReady" feature - or more logically,
instead add AM::FeatureWaitForAccountsToBeReady which would be specified by
default, leading to the [1] behavior described above, but can be left out, such
that AM::becomeReady() will then return for that request before the accounts
are ready - currently Core is always waited for, maybe this is a suitable
middle-ground
 - if a feature that only becomes ready when a connection is Connected is set
to be guaranteedly ready on a Connection, this makes not-yet-connected
Connections invisible, which makes for example handling authentication channels
impossible as Simon pointed out in the previous comment. Actually, there are no
such features on Tp::Connection - all features can be ready in any connection
state, but are re-introspected when the state changes, and signaling the state
change always waits for all previously requested features to be ready. (So we
actually have had a guarantee somewhat resembling the ones I'm suggesting here
for a long time - whenever statusChanged is emitted, any features previously
requested are ready for the new status)
 - doesn't allow for as fine-grained control on when a given module gets the
object to use as using becomeReady everywhere - modules requesting a lot of
features can delay modules having requested less features getting the object.
I'm not sure if this is a problem though - I see features more as means to
avoid extra D-Bus calls and signal connections (both of which will be
equivalent in either case) than means to get some part of logic running
slightly earlier (which it might not, anyway, if it isn't scheduled before the
rest completes). Of course, if tp-qt4 had features like said FEATURE_CONNECTED
this would be a bigger problem, but it doesn't (by design - I think waiting for
a statusChanged signal with the aforementioned feature guarantees is rather
sufficient)

So, the main difference lies in the channel handling area - being able to
specify, in advance, which features should be ready on ALL of the object levels
(account, connection, channel) avoids all becomeReady round-trips in most
use-cases even for the objects which need to be created from scratch, these
being:
 - ALWAYS: Channel objects (it's a new channel after all)
 - USUALLY: Contacts on the channel (could be specified per-channel-class which
features to make ready on the channel's contacts - note that falling back to
the id can be implemented just fine extending the Contact::actualFeatures()
semantics)
 - SOMETIMES: Connection objects, if AM + Account hasn't been able to (or not
asked to) make it ready yet, or is not there at all (it's a bus-activated
handler and the handling method is the first one called)
 - SOMETIMES: Account objects, if there is no AM, similarly to ^^

Comments, please.

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