[Telepathy] tp-qt4 Observer, Approver, Handler high-level API proposal
Andre Moreira Magalhaes
andre.magalhaes at collabora.co.uk
Fri Apr 3 11:15:59 PDT 2009
Simon McVittie wrote:
> On Fri, 03 Apr 2009 at 13:00:55 -0300, Andre Moreira Magalhaes wrote:
>> void setOperationFinished(uint operationHandle);
>> void setOperationFinishedWithError(uint operationHandle,
>> const QString &errorName, const QString &errorMessage);
> Ewwwwww. We have opaque typed pointers. Can we please use them? Even if they're
> secretly just pointer-sized integers that have been cast to Something*.
I don't see the need to that, we can typedef OperationHandle uint if
that's more pleasant.
> I'm not convinced that the base class is hugely useful - the meaning of
> returning from the initial D-Bus method varies, and we should have a method
> name that indicates what it's doing.
The base class is useful in the sense that we don't replicated the
filters methods in all subclasses, it's just a thin class and does not
Let me elaborate on asynchronous and finished/finishedWithError usage:
All D-Bus methods can fail, some race condition for example, so enabling
them to fail is not something we should avoid IMHO.
Also the spec for Observer/Approver/Handler is still DRAFT, with some
FIXME/TODO's items, and the idea is that it can change at any time, so I
decided to make all methods able to fail, in case we add error support
in the future.
Regarding async usage:
Some methods probably don't need to be async, but we never know what the
app will do in order to implement the method, so allowing the app to
implement the method async is OK. Also it's better to make all methods
call finished/finishedWithError when finished then doing it case by
case, so we have a pattern.
This will be documented, and I don't see the reason not to do it.
>> const QVariantMap &observerInfo) = 0;
> It'd be good if there was some way to access the contents of this dict in a
> better way, as we define meanings for bits of it; maybe the virtual method
> could call back into the base class somehow.
> When observing channels, returning from ObserveChannels indicates that you
> have done all your initial setup and are now ready for those channels to be
> handled by a Handler (e.g. a logger would connect to MessageReceived and
> download the message backlog before saying "yes"). As a result, it needs to
> be able to happen asynchronously.
>> virtual void addDispatchOperation(uint operationHandle,
>> const QString &dispatchOperationObjectPath,
>> const QVariantMap &dispatchOperationProperties) = 0;
> This should take a DispatchOperationPtr to a dispatch operation that is
> guaranteed to already be ready (the QVariantMap should be enough to ensure
DispatchOperation class is on the TODO list.
> It's not yet clear to me what returning from AddDispatchOperation signifies,
> or whether it needs to be asynchronous.
>> virtual void handleChannels(uint operationHandle,
>> const AccountPtr &account,
>> const ConnectionPtr &connection,
>> const QList<ChannelPtr> &channels,
>> const QStringList &requestsSatisfiedObjectPaths,
>> const QDateTime &userActionTime) = 0;
> What does Qt use to do focus-stealing prevention in its equivalent of
> gtk_window_present_with_time()? That's the purpose of UserActionTime, so
> whatever representation Qt wants, that's what userActionTime should be.
Need to check.
> Returning successfully from HandleChannels means you have accepted your
> responsibility to handle the channels. Returning an error means you are unable
> to handle them, and the ChannelDispatcher should close them or fail over to
> another Handler or something. I'm not yet sure whether we need to allow
> the library user to make HandleChannels fail, and whether this needs to be
>> virtual void addRequest(const QString &requestObjectPath,
>> const QVariantMap &requestProperties) = 0;
> This should probably take a ChannelRequestPtr that's guaranteed to be ready?
ChannelRequest class is on the TODO list.
> Perhaps we could have some sort of opt-in mechanism so only clients that want
> to care about requests will get this method called, so they can avoid the
> overhead of creating the ChannelRequest?
I will add enableListenRequests(bool)
> Returning successfully or unsuccessfully from AddRequest is basically
> irrelevant, if I remember the spec correctly: the ChannelDispatcher will
> probably just emit debug output if you failed. Accordingly, I think we can
> return from the D-Bus method as soon as the virtual method returns, and not
> give the user a way to make it fail.
>> virtual void removeFailedRequest(const QString &requestObjectPath,
>> const QString &error, const QString &message) = 0;
> Returning successfully or unsuccessfully is like AddRequest, IIRC.
> Probably the BaseHandler should cache a ChannelRequestPtr for each request that
> we've been told about and that has not either been satisfied or removed,
> so we could get a ChannelRequestPtr here that can be compared for equality
> with the one from addRequest.
Yep, when ChannelRequest appear, a cache will be useful.
>> // clientName is the name to be used
>> ClientRegister(const QString &clientName, bool unique = false,
>> QObject *parent = 0);
>> QString clientName() const;
>> bool addObserverClient(ObserverClient *client);
>> bool addApproverClient(ApproverClient *client);
>> bool addHandlerClient(HandlerClient *client);
>> bool registerClients();
> I'm not sure whether we should block on RequestName like this. Probably we
> should, since it's only a round trip to the dbus-daemon.
I don't see the need to make registerClients async, but if you prefer it
can return a PendingOperation.
> Can't we have this happen at construct time?
Yeah, we can, But I was thinking if we will be able to add filters at
We have 2 solutions:
- Receive the filters in the constructor
- Have a addFilters instead of setFilters
Which one do you prefer?
>> - Write wrappers for ChannelRequest.DRAFT and ChannelDispatchOperation.DRAFT
> Yes. My current work on Mission Control should get these D-Bus APIs closer to
> being final.
More information about the telepathy