[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

--- Comment #13 from Olli Salli <ollisal at gmail.com> 2010-08-18 07:44:40 PDT ---
(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.

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