Reassigning this to the general component since it affects telepathy-glib too.

(In reply to comment #0)
>  * allow specifying which Account features should be ready in any Account an
> AccountManager gives you, and make AccountManager make them ready before
> signaling accountCreated() and signaling itself being ready in the initial
> accounts download on AM::becomeReady() -> no need to do Account::becomeReady()
> on accounts from the AM

One possible model would be to signal the "creation" as soon as possible, or as soon as Core is ready, and then signal *again* when the desired features are ready.

>  * allow specifying which Connection features (on which Connection subclass)
> should be made ready before signaling Account::haveConnectionChanged(true) ->
> no need to do Connection::becomeReady() on whatever Account::connection() gives

This is a particularly nasty case. Some modules want to see a Connection as soon as it pops up, others don't want to care until the CONNECTED feature is ready; we can't just wait for the upper bound, in case the Connection *can't* become connected until someone has interacted with it. Consider pre-connection TLS channels, popping up to ask "this certificate looks suspicious, is it OK?", or pre-connection authentication channels, popping up to ask for your password - it's very important that their Handlers can run before CONNECTED.

>  * allow specifying which Contact features should always be ready for a contact
> signaled as having joined a Channel (this should be documented as dangerous if
> used with too many features - if the contact in question leaves the channel
> before we finish introspecting every feature, the operation may fail and we
> won't necessarily even know who it was)

This is a danger, yes. I'd rather have the API make this situation impossible when used with a sufficiently good connection manager - for instance, the Contact could just appear anyway, with its alias/avatar/hat colour missing, as long as it has its handle and identifier.

(This is tied up in considerations about handle lifetimes, 

>  * allow specifying which Channel features are always interesting for any given
> channel class, so that Channels given to you when you're a Client or when you
> request one in general are always ready with the desired features (requires
> extending ChannelFactory to accept dynamically overriding which Channel
> subtypes to construct, and which features to begin making ready for a given set
> of immutable properties making up the channel class)
> 
> I'll file the corresponding sub-bugs as dependencies when I have time to do so.
> Suggestions, especially turning in more superfluous setup step suspects kindly
> welcome. The suggestions above can be implemented in a backwards-compatible
> fashion, but we should identify beneficial candidates for an API/ABI break too.
> 
> The basic assumption above is that each application has a basic set of features
> they're always interested in, and will want for every object. Stuff that an
> application is only potentially interested in (menu option to show eg. avatars
> for chat participants, the user opening a "more gory details on your friend"
> expander widget, and so on) can always be "upgraded" afterwards on top of the
> base set in the current fashion (becomeReady() with more features,
> upgradeContacts()). Also, in more sophisticated applications, whatever can be
> lazily updated to the view, should be lazily updated after an upgrade operation
> finishes, and should be documented as the recommended action in tp-qt4 docs.
> 
> One final point is - whatever we do, this shouldn't counteract trying to avoid
> extra D-Bus state download and signal connections to uninteresting information
> (forcing Features on etc). Also, we shouldn't hamper subclassability. Even if
> Account constructs Connection objects for you you should be able to specify the
> Connection subclass and desired Connection features, etc. This could be
> accomplished in general using a factory pattern, where you specify a factory
> function returning the desired subclass and the features to always make ready
> in constructed objects.
> 
> To keep the factories simple, they should only begin the process of making the
> desired Features ready (and do the other misc manipulation on the constructed
> object). The constructed object should be returned immediately. The object (eg.
> AM) which requested constructing the object (the Account) should then wait for
> the object to get ready, with no further interaction with the factory for that
> object.

Urgh, pressed [commit] too soon. More:

>  * allow specifying which Contact features should always be ready for a contact
> signaled as having joined a Channel (this should be documented as dangerous if
> used with too many features - if the contact in question leaves the channel
> before we finish introspecting every feature, the operation may fail and we
> won't necessarily even know who it was)

This is a danger, yes. I'd rather have the API make this situation impossible
when used with a sufficiently good connection manager - for instance, the
Contact could just appear anyway, with its alias/avatar/hat colour missing, as
long as it has its handle and identifier.

(This is tied up in considerations about handle lifetimes, Bug #23155.)

>  * allow specifying which Channel features are always interesting for any given
> channel class, so that Channels given to you when you're a Client or when you
> request one in general are always ready with the desired features (requires
> extending ChannelFactory to accept dynamically overriding which Channel
> subtypes to construct, and which features to begin making ready for a given set
> of immutable properties making up the channel class)

I don't think the precise thing you're suggesting here is quite the right approach, but it's close. I've been thinking about a similar case in telepathy-glib, and was plotting giving each TpBaseClient an optional ChannelFactory, so instead of setting things up as library-global state (which is inherently dangerous if you have uncooperative modules), you set things up per TpBaseClient and rely on everything that shares TpBaseClient being suitably cooperative.

(Tp::AbstractClient would be your equivalent; Tp::ClientRegistry is not.)

> Also, we shouldn't hamper subclassability. Even if
> Account constructs Connection objects for you you should be able to specify the
> Connection subclass and desired Connection features, etc.

This is tricky if you encourage a pattern in which the whole process shares an AccountManager pseudo-singleton (really one AccountManager per D-Bus connection), which is something we probably do want, because sharing an AccountManager means you share Accounts, which means you share Connections, which means you can safely share Contacts.

Perhaps the rules should be "specifying a subclass factory on the pseudo-singleton AccountManager is an error" and "if you want to get given subclasses automatically, you have to operate in your own little world, starting from your own AccountManager".

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.

(In reply to comment #3)
>  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

There's a tension between this, and wanting new Accounts to pop up as soon as possible because it's signalling an event that, really, already happened. Perhaps we want two signals, newAccount and newlyReadyAccount...

>  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?

It's worth keeping in mind, but not necessarily as a/the primary goal.

>  - maybe it's actually: In AM, use whatever features you recognize, otherwise
> pass them to contained objects, and ditto for them

I'm not sure I like this meme: it makes it impossible to warn API users if they supplied a Feature that makes no sense, and will have no effect. Silent failure to act considered unhelpful :-)

>  - 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.

Can we approach this from another angle? In a Client, don't call the user-supplied HandleChannels, ObserveChannels or AddDispatchOperation callback (vmethod? slot? whatever this is in Qt-land) until certain features are ready.

This solves the problem for every feature except Contact features in every app that is only a Client, which I think will actually cover a lot of practical use-cases, including all Tubes apps (which, long-term, we hope will be the widest class of application).

It's also safe for a telepathy-qt4 Client to have a flag for "don't invoke me until the Connection is CONNECTED" (even though telepathy-qt4 doesn't normally do this), because the Client knows whether it's useful on disconnected Connections (most aren't).

It doesn't solve anything for apps that start from the AccountManager and work downwards - account management UIs, status choosers, contact lists, and that sort of "furniture" - but it's a start.

> Another approach 2), formalizing on the hand-waving I did with Andre last week
> and somewhat inscribed in the bug description is "object factories"

This seems like something we eventually want for subclassability, even if we don't want it for feature preparation.

>  - we can make the factory instance / its settings either settable on a
> specific AccountManager, bus-connection globals or library globals

No library globals, please; that breaks the case of two unrelated plugins in a non-Telepathy-aware plugin host (e.g. Maemo 5 status menu, gnome-panel, GEdit), each of which wants to use Telepathy and subclass stuff. Attaching a factory to an AccountManager instance could be a good way, though?

> 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.

I do like this. With hindsight, we should have probably made TpBaseClient's constructor take a TpAccountManager, not a TpDBusDaemon, so that it can behave similarly; I'll sort that out.

> 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.

This is a Qt equivalent of Bug #13422 (tp_account_channel_request_create_and_handle_async()), achieved by creating a Handler behind the scenes and making it preferred, and it's probably worth using a similar API:

http://telepathy.freedesktop.org/doc/telepathy-glib/telepathy-glib-account-channel-request.html

What you already have is equivalent to Bug #29456, tp_account_channel_request_create_async().

> 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

Or, modules that only want to care about connected accounts can listen for accountConnected instead of accountAdded?

>  - 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.

Specifying features per (TpBase|Tp::Abstract)Client solves this: if you handle authentication channels in your TpBaseClient, and you demand TP_CONNECTION_FEATURE_CONNECTED, then you're silly.

> 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)

Indeed, telepathy-glib features and telepathy-qt4 features don't correspond 1:1.

I'm a bit wary of the telepathy-qt4 design at the moment, because I've been investigating ContactList API for telepathy-glib. The contact list can be downloaded asynchronously after CONNECTED, can take time to download (in XMPP, multiple-minute lag is apparently entirely possible), and not signalling StatusChanged(CONNECTED) until the contact list has been retrieved, because a module over there -> asked for it before connecting, doesn't seem entirely desirable...

The sketch I have at the moment is:

TP_CONNECTION_FEATURE_CONTACT_LIST_INFO
    Status/capability flags (CanChangeContactList, ContactListState etc.)
    which are always available after you have CONNECTED
    Implies TP_CONNECTION_FEATURE_CONNECTED

TP_CONNECTION_FEATURE_CONTACT_LIST
    The actual contact list, with TpContact objects (with ID, handle
    and subscription states only, at the moment), and change notification
    Implies TP_CONNECTION_FEATURE_CONNECTED and ...CONTACT_LIST_INFO, and
    also implies contact-list-state == TP_CONTACT_LIST_STATE_SUCCESS

I haven't done anything about contact features yet. I was wondering whether to have a list of features to which you can only add (asynchronously) and never remove; after the async addition has finished, each TpContact on the contact list has all the features, and subsequently-added contacts aren't announced until they too have all of those features.

(This only works because all the contact features are "fast", though...)

(In reply to comment #4)
> With hindsight, we should have probably made TpBaseClient's
> constructor take a TpAccountManager, not a TpDBusDaemon, so that it can behave
> similarly; I'll sort that out.

Bug #29614 has a branch.

Thanks for your input. Judging the alternatives, I'm settling towards (lost this writeup already once thanks to a kernel crash, good times):
 - specify Channel subclass and features through the Client API, it's the only place where one gets those anyway
   - so a ChannelFactory (made more dynamic from the current one) per Client API "big object" (this may change from ClientRegistrar / the AbstractClient instance to *something* whenever I have time to think about fitting the requestAndHandle API in the picture, probably similarly to Tp-Glib though so I'll just call it Client for now)
 - allow specifying Account / Connection (subclass and) features both through AM and the Client API
   - tube clients and alike have Client or sometimes both, account editing, presence etc applications/widgets have just AM, while bigger applications likely have both, we should support all of these (and it's quite simple to do so)
   - AM options must include "don't make any Connections ready please" for account editing UIs etc which don't quite care about them
   - Client API delays calling the application until the objects are ready
   - new AM features {Accounts,Connections}Ready
   - AM delays becomeReady(FeatureAccountsReady) until the accounts are ready (currently delays AM::FeatureCore until the Account::FeatureCore is ready for all accounts, do we still want this even if the feature isn't specified? otherwise backwards compat which we're about to break anyway suffers unless we have becomeReady auto-add that feature and have an extra becomeReadyEarlyGoer variant)
   - AM delays becomeReady(FeatureConnectionsReady) until both the contained accounts and their connections are ready
   - new signal newReadyAccount(AccountPtr) which is delayed until the account in question AND its connection, if any, are ready, newAccount retains current semantics BUT still see my desperate arguments against this below
     - I don't think we need newReadyAccountButNotItsConnection - as previously explained, in tp-qt4 this is not at odds with eg. preconnection auth channels and I don't think the contact lists either
 - everything is object instance-specific, no library-global or bus-connection-wide state
 - allow setting an AM to use on the Client API "big object", whatever it is in the end
   - in this scenario, AM only waits for features specified on it to be ready, while the Client first gets objects from the AM and upgrades any extra features set on it before handing the objects over to the application approving/handling/observing function implementation
     - allows for more granularity, specifying some extra features only for accounts / connections the application handles channels from which sounds useful
   - Client always waits the AM and sub-objects to be ready before attempting to look-up Account/Connection objects to share - them being ready and a channel to handle coming for an account/connection we don't see through the AM is probably warning() zone
   - for tube clients operating without an AM, we probably want the following though:
     - implement weak-pointer cache of Accounts constructed for previous channel handling invocations
       - and Connections? probably not, and should just document that the application should always store the Account rather than just the Connection, as the Account already has a strong-ref to the Connection
     - if the application currently has an Account / Connection pair from a previous channel, this avoids state re-download and enables handle/contact interoperability between the two channels' contexts

The above leads me to the conclusion that AccountFactory and ConnectionFactory should actually be one object (Called AccountObjectFactory? constructing Connections is always Account-specific using it), because they always exist in the same place (AM and/or Client). ChannelFactory should be separate though, as it has somewhat different API due to being driven by the immutable properties, and only exists in Client.

Open issue: at which stages of the program's lifetime do we want to allow setting the subclasses and features?
 - AM semantics for setting features could be either
   a) just once, before calling becomeReady(Feature*sReady) on it for the first time, this way it can be synchronous, and it's guaranteed non-co-operative plugins etc can't add harmful extra delays semi-globally because just the host application can do it
    or
   b) always, and make it upgrade existing objects - this way it'll have to be a PendingOperation - is more flexible but also more dangerous? I'm not entirely convinced the latter holds true though
    - make it add-only though to not confuse somebody having set it earlier on with new objects not having their desired features, and document that if the application wants say an "show avatars" menu toggle, it shouldn't use this and instead upgrade objects in the old-fashioned way if that is enabled
    - for convenience, the AM becomeReady(SubObjects) calls should wait for these requested pendingoperations to be ready so it doesn't have to be done separately when initially setting up the object, similarly to a)
 - but subclass definitely can't be settable after the first object constructed in either case
 - Client semantics should be "new Client operations are delayed accordingly to the new settings, with no direct effect in Accounts and Connections given in earlier operations" (not direct as in: except through the fact that it can be the same object due to sharing)
   - can set features whenever convenient, subclass only before first object is constructed - this means we should document it should be set before the Client is registered on the bus
   - the Client Acc/Conn factory is unused if an AM is linked to it, except for the additional features to wait for, because all objects are available from the AM anyway so there is no conflict between the subclasses specified in the two factories

This general approach still might seem at odds with your concerns, so I'll try and defend my position next:

(In reply to comment #4)
> (In reply to comment #3)
> >  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
> 
> There's a tension between this, and wanting new Accounts to pop up as soon as
> possible because it's signalling an event that, really, already happened.
> Perhaps we want two signals, newAccount and newlyReadyAccount...

Well, I added the separate signal to my plan, but I'm still not convinced the not-quite-ready-for-the-use-you-asked-for-yet Account is useful, because tp-qt4 Account features are all semi-fast (neither downloading the Avatar over D-Bus nor reading .manager files / querying the CM for ProtocolInfo does take that long ie. make any network round trips). Again, tp-qt4 Features are designed to allow cutting down state download and signal connections by not specifying them, not really waiting for any slow-slow (network or user action dependent) events to happen by specifying them. We tend to have change notification signals for those.

In other words, DO WE HAVE AN USE CASE!!ONE (an actual tp-qt4 one, with no hypothetical added slow features)? My concern is that we'd end up applications upgrading accounts again and again with the same features like they're doing currently if we have *any* API which doesn't guarantee the requested features do be ready. Then again, maybe if we finally grew that ABOVEMEREMORTALS API annotation we could use that to tack those parts to an out-of-reach advanced section and just hint people towards them if we sense they need it for their corner-case.

> 
> >  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?
> 
> It's worth keeping in mind, but not necessarily as a/the primary goal.

The big-object-instance-specific-factories approach described above allows us to initially leave out the subclass specification but add it later on.

> 
> >  - maybe it's actually: In AM, use whatever features you recognize, otherwise
> > pass them to contained objects, and ditto for them
> 
> I'm not sure I like this meme: it makes it impossible to warn API users if they
> supplied a Feature that makes no sense, and will have no effect. Silent failure
> to act considered unhelpful :-)
> 

It could whine about features still unrecognized in the penultimate object, but yeah, I don't like it either.

> >  - 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.
> 
> Can we approach this from another angle? In a Client, don't call the
> user-supplied HandleChannels, ObserveChannels or AddDispatchOperation callback
> (vmethod? slot? whatever this is in Qt-land) until certain features are ready.

Yeah, this was the original intention, and present in the refinement in this post.

> 
> This solves the problem for every feature except Contact features in every app
> that is only a Client, which I think will actually cover a lot of practical
> use-cases, including all Tubes apps (which, long-term, we hope will be the
> widest class of application).
> 
> It's also safe for a telepathy-qt4 Client to have a flag for "don't invoke me
> until the Connection is CONNECTED" (even though telepathy-qt4 doesn't normally
> do this), because the Client knows whether it's useful on disconnected
> Connections (most aren't).

Yeah, instead of having a FeatureConnected this we could allow specifying this just for the Client, with no effect on accessing the Connections through the AM.

> 
> It doesn't solve anything for apps that start from the AccountManager and work
> downwards - account management UIs, status choosers, contact lists, and that
> sort of "furniture" - but it's a start.
> 

This posts' refinement does, by using the exact same AccountObjectFactory API in both AM and Client, with the documented interaction between the two as described in case both are used.

> > Another approach 2), formalizing on the hand-waving I did with Andre last week
> > and somewhat inscribed in the bug description is "object factories"
> 
> This seems like something we eventually want for subclassability, even if we
> don't want it for feature preparation.
> 

Yeah, so let's go all the way and specify both with it. This also gives us a natural location to add any other automatic manipulations on the objects beyond requesting Features on them if we come up with any (say, enable Contact features on all Connection's ContactManagers constructed with it if we add something of the sort to ContactManager).

> >  - we can make the factory instance / its settings either settable on a
> > specific AccountManager, bus-connection globals or library globals
> 
> No library globals, please; that breaks the case of two unrelated plugins in a
> non-Telepathy-aware plugin host (e.g. Maemo 5 status menu, gnome-panel, GEdit),
> each of which wants to use Telepathy and subclass stuff. Attaching a factory to
> an AccountManager instance could be a good way, though?

Yeah, in case there's no Telepathy common ground between the plugins (like there would be eg. in plugins for a chat window app which already uses Tp for the main functionality) they will each construct their own AM instances, which solves the problem if we go for what I propose.

> 
> > 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.
> 
> I do like this. With hindsight, we should have probably made TpBaseClient's
> constructor take a TpAccountManager, not a TpDBusDaemon, so that it can behave
> similarly; I'll sort that out.

++

> 
> > 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.
> 
> This is a Qt equivalent of Bug #13422
> (tp_account_channel_request_create_and_handle_async()), achieved by creating a
> Handler behind the scenes and making it preferred, and it's probably worth
> using a similar API:
> 
> http://telepathy.freedesktop.org/doc/telepathy-glib/telepathy-glib-account-channel-request.html

I really must check out how this API fits into my proposed scheme next, will do today.

> 
> What you already have is equivalent to Bug #29456,
> tp_account_channel_request_create_async().
> 

Yeah, thankfully there's no interaction between object construction and that variant.

> > 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
> 
> Or, modules that only want to care about connected accounts can listen for
> accountConnected instead of accountAdded?

The above scheme does add the Feature AND at least for now the separate signals.

> 
> >  - 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.
> 
> Specifying features per (TpBase|Tp::Abstract)Client solves this: if you handle
> authentication channels in your TpBaseClient, and you demand
> TP_CONNECTION_FEATURE_CONNECTED, then you're silly.

++

> 
> > 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)
> 
> Indeed, telepathy-glib features and telepathy-qt4 features don't correspond
> 1:1.
> 
> I'm a bit wary of the telepathy-qt4 design at the moment, because I've been
> investigating ContactList API for telepathy-glib. The contact list can be
> downloaded asynchronously after CONNECTED, can take time to download (in XMPP,
> multiple-minute lag is apparently entirely possible), and not signalling
> StatusChanged(CONNECTED) until the contact list has been retrieved, because a
> module over there -> asked for it before connecting, doesn't seem entirely
> desirable...
> 
> The sketch I have at the moment is:
> 
> TP_CONNECTION_FEATURE_CONTACT_LIST_INFO
>     Status/capability flags (CanChangeContactList, ContactListState etc.)
>     which are always available after you have CONNECTED
>     Implies TP_CONNECTION_FEATURE_CONNECTED
> 
> TP_CONNECTION_FEATURE_CONTACT_LIST
>     The actual contact list, with TpContact objects (with ID, handle
>     and subscription states only, at the moment), and change notification
>     Implies TP_CONNECTION_FEATURE_CONNECTED and ...CONTACT_LIST_INFO, and
>     also implies contact-list-state == TP_CONTACT_LIST_STATE_SUCCESS

I think currently tp-qt4 would go Ready when it has the Info and emit allKnownContactsChanged when the contacts are actually fetched. The clients need to listen for new contacts added anyway if eg. another client adds a contact.

> 
> I haven't done anything about contact features yet. I was wondering whether to
> have a list of features to which you can only add (asynchronously) and never
> remove; after the async addition has finished, each TpContact on the contact
> list has all the features, and subsequently-added contacts aren't announced
> until they too have all of those features.
> 
> (This only works because all the contact features are "fast", though...)

This is equivalent to what I propose for the tp-qt4 Account, Connection and Channel features too, given that tp-qt4 Account and Connection features are also "fast" (ie. don't wait for network / user events to happen). Otherwise, I'd like to postpone the contact features discussion for now. Having a factory for connection objects allows us to add API auto-enabling contact features on the relevant Connection's ContactManagers so I think we're future proof for that.

(In reply to comment #6)
>    - AM options must include "don't make any Connections ready please" for
> account editing UIs etc which don't quite care about them

I don't like the sound of negative Features. At the moment, all Features are positive, which means that sharing an object can only ever slow you down.

>    - Client API delays calling the application until the objects are ready

In telepathy-glib, it already waits for CORE on Account, Connection, Channel (but not necessarily ChannelRequest and ChannelDispatchOperation). I implemented this in a telepathy-glib branch; it's not very difficult.

>    - new AM features {Accounts,Connections}Ready

I hope that means "core-ready"? Perhaps the name could indicate that better - AM::Feature::AccountCore?

>  - everything is object instance-specific, no library-global or
> bus-connection-wide state

Yay! :-)

>    - Client always waits the AM and sub-objects to be ready before attempting
> to look-up Account/Connection objects to share - them being ready and a channel
> to handle coming for an account/connection we don't see through the AM is
> probably warning() zone

Actually, I think this can happen under normal use, if the message sequence is unlucky. This is why we have tp_account_ensure_connection(), for instance.

> Open issue: at which stages of the program's lifetime do we want to allow
> setting the subclasses

You can only set the factory at, or just after, construction of the owning object, would be my vote.

I think the factory (choice-of-subclass mechanism) and the choice of initially-prepared features should probably be distinct?

> and features?
>  - AM semantics for setting features could be either
>    a) just once, before calling becomeReady(Feature*sReady) on it for the first
> time, this way it can be synchronous, and it's guaranteed non-co-operative
> plugins etc can't add harmful extra delays semi-globally because just the host
> application can do it
>     or
>    b) always, and make it upgrade existing objects - this way it'll have to be
> a PendingOperation - is more flexible but also more dangerous? I'm not entirely
> convinced the latter holds true though
>     - make it add-only though to not confuse somebody having set it earlier on
> with new objects not having their desired features, and document that if the
> application wants say an "show avatars" menu toggle, it shouldn't use this and
> instead upgrade objects in the old-fashioned way if that is enabled
>     - for convenience, the AM becomeReady(SubObjects) calls should wait for
> these requested pendingoperations to be ready so it doesn't have to be done
> separately when initially setting up the object, similarly to a)
>  - but subclass definitely can't be settable after the first object constructed
> in either case


>  - Client semantics should be "new Client operations are delayed accordingly to
> the new settings, with no direct effect in Accounts and Connections given in
> earlier operations" (not direct as in: except through the fact that it can be
> the same object due to sharing)
>    - can set features whenever convenient, subclass only before first object is
> constructed - this means we should document it should be set before the Client
> is registered on the bus
>    - the Client Acc/Conn factory is unused if an AM is linked to it, except for
> the additional features to wait for, because all objects are available from the
> AM anyway so there is no conflict between the subclasses specified in the two
> factories

In telepathy-glib I made it "you can only add features, and you can only do that until you export the Client on the bus with tp_base_client_register()". I could relax that later if there's a need but it seems a good conservative default.

> This general approach still might seem at odds with your concerns, so I'll try
> and defend my position next:

Time to press [Commit] before reading/responding, so I won't lose this comment :-)

> and features?
>  - AM semantics for setting features could be either
>    a) just once, before calling becomeReady(Feature*sReady) on it for the first
> time, this way it can be synchronous, and it's guaranteed non-co-operative
> plugins etc can't add harmful extra delays semi-globally because just the host
> application can do it

(Variation of this: at construct-time)

> (In reply to comment #4)
> Well, I added the separate signal to my plan, but I'm still not convinced the
> not-quite-ready-for-the-use-you-asked-for-yet Account is useful, because tp-qt4
> Account features are all semi-fast

Right, if you document very clearly that all Account features now and forever must be at least semi-fast, then this is sufficient, and we don't have a use-case for slow preparation.

I think there's value in having some features not be fast, though; the as-yet-hypothetical TP_CONNECTION_FEATURE_CONTACT_LIST ought to wait until the server has actually returned the initial contact list, which can apparently take multiple minutes on shaky cellular connections.

It's case-by-case by class, really:

- on AM and Account, you're right that there's no reason why we'd have any slow features, because Accounts are locally-stored; unless there are features that are really shorthand for a Connection feature, in which case the below applies

- on Connection, telepathy-glib has a CONNECTED feature, mostly for historical reasons (the old "ready" boolean was "too big"); but in any case it seems useful to be able to wait for CONNECTED-and-a-bunch-of-features in a single transaction. Contact lists ought to be a feature that makes round-trips, probably, too.

- on Channel, I'm not sure

- on contacts, so far we've avoided slow features by design; the Alias feature doesn't really mean "I have the alias", but "I've made some effort towards getting the alias, and if I immediately had it to hand, I've returned it".

> > It's worth keeping in mind, but not necessarily as a/the primary goal.
> 
> The big-object-instance-specific-factories approach described above allows us
> to initially leave out the subclass specification but add it later on.

Yes. Good :-)

> > It's also safe for a telepathy-qt4 Client to have a flag for "don't invoke me
> > until the Connection is CONNECTED" (even though telepathy-qt4 doesn't normally
> > do this), because the Client knows whether it's useful on disconnected
> > Connections (most aren't).
> 
> Yeah, instead of having a FeatureConnected this we could allow specifying this
> just for the Client, with no effect on accessing the Connections through the
> AM.

Straw man: imagine that TpConnection didn't have the CONNECTED feature. Instead of putting it in its feature set, TpBaseClient could have had:

/* must be called before _register() */
tp_base_client_require_connected_connection (TpBaseClient *);

You could do a Qt equivalent of that?

> > TP_CONNECTION_FEATURE_CONTACT_LIST_INFO
> >     Status/capability flags (CanChangeContactList, ContactListState etc.)
> >     which are always available after you have CONNECTED
> >     Implies TP_CONNECTION_FEATURE_CONNECTED
> > 
> > TP_CONNECTION_FEATURE_CONTACT_LIST
> >     The actual contact list, with TpContact objects (with ID, handle
> >     and subscription states only, at the moment), and change notification
> >     Implies TP_CONNECTION_FEATURE_CONNECTED and ...CONTACT_LIST_INFO, and
> >     also implies contact-list-state == TP_CONTACT_LIST_STATE_SUCCESS
> 
> I think currently tp-qt4 would go Ready when it has the Info and emit
> allKnownContactsChanged when the contacts are actually fetched. The clients
> need to listen for new contacts added anyway if eg. another client adds a
> contact.

Right. That somewhat brings us back to the multiple-transactions problem, though: you wait for the features, but then you can't draw your contact chooser until you have *also* had allKnownContactsChanged; but you can't just ignore the preparation and rely on getting allKnownContactsChanged, because if the contact list was already ready, you might wait forever?

Ideally, you want a single GAsyncResult/Tp::PendingOperation round-trip between "I have a connection, how is it formed?" and "draw the contact chooser". TP_CONNECTION_FEATURE_CONTACT_LIST as described there would be that single round-trip, at the cost that it's a "slow feature" that might not work well.

Perhaps I should just assume that you won't want to wait for more than one unrelated "slow feature", and special-case them. It could have API like this:

/**
 * equivalent to: tp_proxy_prepare_async (self, features) followed by
 * waiting for the contact list and waiting for them all to have the
 * desired features
 */
tp_connection_await_contact_list_async (TpConnection *, const GQuark conn_features[], const TpContactFeature *contact_features)

(In reply to comment #9)
> I think there's value in having some features not be fast, though; the
> as-yet-hypothetical TP_CONNECTION_FEATURE_CONTACT_LIST ought to wait until the
> server has actually returned the initial contact list, which can apparently
> take multiple minutes on shaky cellular connections.

Maybe this should be a separate object in the bindings dangling off the Tp[::]Connection, which can be prepared separately. (Indeed, there already is a separate object for this in TpQt4, no?)

</drive-by>

I've inspected the requestAndHandle situation, and the tp-glib model of exporting a temporary Client with no filters seems like the approach to take. It will fit in with the semantics I suggested - there is no application-visible Client object (well, a relevant one anyway) so the only place to specify features will be the AM which the Account belongs to, which IS most logical indeed. However, this brings up another issue - should it be supported to "start" from an Tp::Account without getting it either from an AM or a Client? If it should, there won't be any means to specify the desired connection features.

A possible solution:
 - keep the Account and Connection factories separate
 - have just the ConnectionFactory in Account
 - have AM specify its own ConnectionFactory for all sub-accounts

Then again, is this needed? AFAICS all apps DO either use AM or get a single account/connection at a time given to them when observing/handling/approving.

(In reply to comment #10)
> (In reply to comment #9)
> > I think there's value in having some features not be fast, though; the
> > as-yet-hypothetical TP_CONNECTION_FEATURE_CONTACT_LIST ought to wait until the
> > server has actually returned the initial contact list, which can apparently
> > take multiple minutes on shaky cellular connections.
> 
> Maybe this should be a separate object in the bindings dangling off the
> Tp[::]Connection, which can be prepared separately. (Indeed, there already is a
> separate object for this in TpQt4, no?)
> 
> </drive-by>

Yeah, ContactManager is there, but actually it's made "ready" as part of making the Connection ready, with no way to prepare it further independently. Nothing stops us from making it a full-blown ReadyObject with some features of its own in an API/ABI break, though. To not counter-act what we're doing here, Connection should make its "core" (the current functionality) ready as part of making itself ready, though :)

(In reply to comment #7)
> (In reply to comment #6)
> >    - AM options must include "don't make any Connections ready please" for
> > account editing UIs etc which don't quite care about them
> 
> I don't like the sound of negative Features. At the moment, all Features are
> positive, which means that sharing an object can only ever slow you down.

I didn't exactly mean this should be a negative feature, rather a positive-as-usual FeatureConnectionsReady should be added, which can be left out by eg. account config UIs for which the Connections aren't useful.

> 
> >    - Client API delays calling the application until the objects are ready
> 
> In telepathy-glib, it already waits for CORE on Account, Connection, Channel
> (but not necessarily ChannelRequest and ChannelDispatchOperation). I
> implemented this in a telepathy-glib branch; it's not very difficult.

Yeah, and adding the user-specified features to the becomeReady / prepare_async call doesn't make it any harder :)

> 
> >    - new AM features {Accounts,Connections}Ready
> 
> I hope that means "core-ready"? Perhaps the name could indicate that better -
> AM::Feature::AccountCore?
> 

No, that means whatever-is-set-to-be-always-ready on the factory should be ready! The current behavior without specifying any features to AM::becomeReady() is to make the Account core ready, and nothing on the Connections within - specifying Account(Feature)sReady in the becomeReady() would make it only finish after the set features are ready on the accounts, and similarly for the connection case.

The point here is to enable access to the AM / the Account core / the Account features but not a Connection, even if some other module has asked for a lot of features and is waiting for AM::becomeReady(ConnectionFeaturesReady) - which you previously explained would be useful.

> >    - Client always waits the AM and sub-objects to be ready before attempting
> > to look-up Account/Connection objects to share - them being ready and a channel
> > to handle coming for an account/connection we don't see through the AM is
> > probably warning() zone
> 
> Actually, I think this can happen under normal use, if the message sequence is
> unlucky. This is why we have tp_account_ensure_connection(), for instance.
> 

Doesn't waiting for the AM to become ready (with FeatureAccountFeaturesReady), which in turn waits for the account features to become ready already sufficiently re-order the messages such that this can't happen?

Or actually... if the AM and existing accounts are already ready, but a new account is added, and we first get a channel to handle for it, and only then the corresponding AccountValidityChanged or some other silliness, this can indeed happen. In this case there wouldn't be even an account to ensure a connection for on the AM yet, though - so should there actually always be a weak-pointer cache in use, even if the Client has an AccountManager to use, but with the AM also having access to said cache to be able to use the account and/or connection the *Client* created when it gets the signal? This cache can't be bus-wide global either, otherwise we're back to the non-co-operative plugins problem due to excessive sharing.

> > Open issue: at which stages of the program's lifetime do we want to allow
> > setting the subclasses
> 
> You can only set the factory at, or just after, construction of the owning
> object, would be my vote.
> 
> I think the factory (choice-of-subclass mechanism) and the choice of
> initially-prepared features should probably be distinct?
> 

I think we're confusing two things. I'd like to have the AccountFactory, ConnectionFactory, ChannelFactory objects etc be non-subclassable objects created by the AM / Account / Client - not a subclass supplied from the application.

The subclass selection mechanism would then be setConstructorFunction() or alike on said Factory object - C++ makes creating such constructor functions eg. as template specifications functions sufficiently easy (much less boilerplate than having a custom Factory subclass just to get new YourFavoriteTypeHere).

That said, I think the setDesiredFeatures() methods should also live on said *Factory objects, mostly because of how ChannelFactory would work - you specify a constructor function per set of immutable Channel properties, so you'll also want to keep specifying desired features for a set of immutable properties at the same place. For consistency, I'd do this for all of the factories, even if what they construct doesn't depend on anything in the corresponding D-Bus object at all (always the same subclass and same features).

> 
> In telepathy-glib I made it "you can only add features, and you can only do
> that until you export the Client on the bus with tp_base_client_register()". I
> could relax that later if there's a need but it seems a good conservative
> default.

Agreed, I can't think of an use-case for being able to add more features between successive Client invocations, so I believe I'll take this route too.

> 
> > This general approach still might seem at odds with your concerns, so I'll try
> > and defend my position next:
> 
> Time to press [Commit] before reading/responding, so I won't lose this comment
> :-)

Ditto.

(In reply to comment #9)
> Right, if you document very clearly that all Account features now and forever
> must be at least semi-fast, then this is sufficient, and we don't have a
> use-case for slow preparation.
> 
> I think there's value in having some features not be fast, though; the
> as-yet-hypothetical TP_CONNECTION_FEATURE_CONTACT_LIST ought to wait until the
> server has actually returned the initial contact list, which can apparently
> take multiple minutes on shaky cellular connections.

What about having an extra attribute for features "isReallySlow" - which could then be used to filter out with a warning such features (currently none) when setting them to be automatically readified in a factory?

> 
> It's case-by-case by class, really:
> 
> - on AM and Account, you're right that there's no reason why we'd have any slow
> features, because Accounts are locally-stored; unless there are features that
> are really shorthand for a Connection feature, in which case the below applies
> 

If we are able to specify Connection features on the AccountManager API directly, we don't need pass-through features -> all Account and AM features will be fetching from local store only.

> - on Connection, telepathy-glib has a CONNECTED feature, mostly for historical
> reasons (the old "ready" boolean was "too big"); but in any case it seems
> useful to be able to wait for CONNECTED-and-a-bunch-of-features in a single
> transaction. Contact lists ought to be a feature that makes round-trips,
> probably, too.
> 
> - on Channel, I'm not sure

We could make these slowFeatures then, if the need for any emerges.
 
> 
> - on contacts, so far we've avoided slow features by design; the Alias feature
> doesn't really mean "I have the alias", but "I've made some effort towards
> getting the alias, and if I immediately had it to hand, I've returned it".
> 

I think this is sensible - the application needs to listen for any changes the remote contact makes anyway, so it can listen to actually having properly fetched the data using the same mechanism at zero cost.

> 
> Straw man: imagine that TpConnection didn't have the CONNECTED feature. Instead
> of putting it in its feature set, TpBaseClient could have had:
> 
> /* must be called before _register() */
> tp_base_client_require_connected_connection (TpBaseClient *);
> 
> You could do a Qt equivalent of that?

Exactly.

> 
> Right. That somewhat brings us back to the multiple-transactions problem,
> though: you wait for the features, but then you can't draw your contact chooser
> until you have *also* had allKnownContactsChanged; but you can't just ignore
> the preparation and rely on getting allKnownContactsChanged, because if the
> contact list was already ready, you might wait forever?

allKnownContactsChanged is the same signal that's emitted if some other app adds a contact, or a remote contact adds you (in addition to the presence blahblah requested signals), so the app needs to listen to it to lazily update new contacts to view anyway. The use I'm suggesting is:

1) connect to allKnownContactsChanged
2) get whatever currently is in allKnownContacts (what the CM & tp-qt4 have been able to report up to that moment, which is usually ~everything, but can be ~zero if the contacts slowly download from the server)
3) show the contacts chooser / friend presence list / contact list editor / whatever
4) if allKnownContactsChanged is emitted, update the widget accordingly

Isn't this a quite common pattern? Like, get the pending messages, or don't get any, but in either case get any new messages by a signal? So, I'm not proposing the application always wants for X allKnownContactsChanged invocations, or any, before displaying the roster.

> 
> Ideally, you want a single GAsyncResult/Tp::PendingOperation round-trip between
> "I have a connection, how is it formed?" and "draw the contact chooser".
> TP_CONNECTION_FEATURE_CONTACT_LIST as described there would be that single
> round-trip, at the cost that it's a "slow feature" that might not work well.

I'm trying for "I have Telepathy" (AM really) - when can I show and do whatever's interesting to my application at all of its levels, and can continue doing so when events happen / new objects appear?

That said, we can still add such slow features if we filter them out (force flag?) appropriately for the (often) app-wide factories.

> 
> Perhaps I should just assume that you won't want to wait for more than one
> unrelated "slow feature", and special-case them. It could have API like this:
> 
> /**
>  * equivalent to: tp_proxy_prepare_async (self, features) followed by
>  * waiting for the contact list and waiting for them all to have the
>  * desired features
>  */
> tp_connection_await_contact_list_async (TpConnection *, const GQuark
> conn_features[], const TpContactFeature *contact_features)

Maybe. We could do this in tp-qt4 easily using the PendingComposite class if the slow features are PendingOperations.

(In reply to comment #14)
> Isn't this a quite common pattern? Like, get the pending messages, or don't get
> any, but in either case get any new messages by a signal? So, I'm not proposing
> the application always wants for X allKnownContactsChanged invocations, or any,
> before displaying the roster.

"this quite a", "application always waits for"

(In reply to comment #13)
> 
> I think we're confusing two things. I'd like to have the AccountFactory,
> ConnectionFactory, ChannelFactory objects etc be non-subclassable objects
> created by the AM / Account / Client - not a subclass supplied from the
> application.
> 
> The subclass selection mechanism would then be setConstructorFunction() or
> alike on said Factory object - C++ makes creating such constructor functions
> eg. as template specifications functions sufficiently easy (much less
> boilerplate than having a custom Factory subclass just to get new
> YourFavoriteTypeHere).
> 
> That said, I think the setDesiredFeatures() methods should also live on said
> *Factory objects, mostly because of how ChannelFactory would work - you specify
> a constructor function per set of immutable Channel properties, so you'll also
> want to keep specifying desired features for a set of immutable properties at
> the same place. For consistency, I'd do this for all of the factories, even if
> what they construct doesn't depend on anything in the corresponding D-Bus
> object at all (always the same subclass and same features).
> 

Agreed with Andre over IRC that the Factory objects should be passed to the Account / AM / AbstractClient constructors, and not changed afterwards, and they should essentially just be tuples of (subclass to construct, list of features to request).

So we'll need new constructors for them, but should retain the old ones for backwards compat/convenience. The old ones should pass (base class, no features) (except for Account, for which core should be passed) for all to preserve semantics. Also, I believe we should make them "backwards" with default arguments, a bit like:
AbstractClient::AbstractClient(Other arguments, const ChannelFactory &chanF = ChannelFactory::theOneWithTpQt4StockSubClassesAndNoFeatures(), const ConnectionFactory &connF = ConnectionFactory::theOneWithTpConnectionAndNoFeatures(), const AccountFactory &accF = AccountFactory::theOneWithTpAccountAndCore())
because probably the smallest level (first here) is tampered with first. This also necessitates adding all three as empty stubs even if we start the implementation from just one of them in a branch as I believe we should.

Afterwards, the factories should be accessible with a const accessor to prevent further tampering. We CAN add mutating API later on though, if needed.

FWIW, Account, Connection and ChannelFactories all now in place with full capabilities and used everywhere in tp-qt4 0.3.13/0.3.14, so from the tp-qt4 POV this is done.

Of course, it could still be debated whether to include the analogous (but completely different) mechanism for Contacts.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/telepathy/telepathy-spec/issues/161.