Remote plasma api review.

[Aaron J. Seigo]

On Wednesday 08 July 2009, Rob Scheepmaker wrote:
> Hello all,
>
> With remote services, dataengines and pin pairing working now, another
> round of api review seemed useful. Things are becoming pretty concrete now.
> See the attached file for all new public classes. Not all these classes
> need to be in public api though. Three are convenience classes of which I'm
> not yet 100% sure if they should be included. These are:
>
> * DenyAllAuthorization & PinPairingAuthorization
> Both are implementations of AuthorizationInterface. Since not setting an
> AuthorizationInterface in your shell is potentially unsafe (a plugin can
> then do so), DenyAllAuthorization would avoid every shell, even those not
> interested in remote plasma stuff, needing to implement this interface.
> PinPairingAuthorization will fire a simple dialog for pin pairing. With
> these two default implementations most shells (except a future headless
> plasma shell) will have an implementation that suits their need.
>
> * PinPairingDialog
> A dialog that asks for a pin for pin pairing. This is used by
> PinPairingAuthorization.  This could be made private, but having it public
> might avoid some duplication in case we want to hide this dialog behind,
> say, a knotification.

once public, we can't make them private. but we can always make private things 
public. so default to private in all cases where there are no use cases 
implemented yet.

personally, what i'd expect is the ability to set the authorization interface 
either using an AuthorizationInterface* or by just calling 
AuthorizationManager::setAuthorizationInterface() with a enum as the 
parameter, with the enums mapping to these internal classes.

so a shell can either go "standard" or "custom".

the standards might include DenyAllAuthorization, PinPairingAuthorization, 
CertificateAuthorization? pin pairing should include certificate auth, i'd 
think, as well... but yes, then there's no need to make any of it public.

> About AccessManager:
> serviceForSource in
> dataengine however returns a Service directly, so to make that work I
> needed to add a serviceReady signal to Service anyway.

right ...

> ServiceAccessJob
> isn't really necesarry anymore. So I'm thinking of making accessService
> static in Service.

makes sense.

> Disadvantage is that for applets we'll still need to
> return a job because we can't really instantiate an applet without having
> obtained it first. So access to applets still need a place in AccessManager

yes, applets are quite different kinds of creatures in this case.

something else in AccessManager:

ServiceAccessJob* accessService(KUrl location) const;

the url is said to be, in the dox:

* @param location a valid JOLIE url

besides not wanting to really expose JOLIE too much in the api, this is not 
going to work out well for other modes of transit. same will go for retrieving 
applets.

while it's just fine to have only JOLIE created right now, i think we should 
keep the possibility open for other mechanisms. JOLIE may not be an option on 
some devices, particularly very small ones. so while plasma:// would be 
synonymous with "this is a JOLIE driven service" or "this is an applet we're 
going to grab using JOLIE", i do plan on writing an emb:// (or whatever) 
handler that does things in a rather more direct fashion between plasma and a 
device. the only major difference is the kind of service it returns (it would 
obviously speak a slightly different protocol) and how it retrieves the 
applet/service/etc.

now, this doesn't impact the external API at all, but the apidox should 
probably be changed and the backend code should remain flexible enough to have 
this added in future.

> I hate it if there are two
> methods for service access though (Service *serviceForSource and
> ServiceAccessJob *accessService). 

there are actually more than two; you can ask for a service from a local 
plugin as well using Plasma::Service::load. there's nothing that says services 
must be attached a DataEngine, after all.

that said, they all have completely different use cases, and in the case of 
serviceForSource, if it's a remote engine it should be using accessService in 
the background. so for the users of services today in plasma, it will be 
completely transparent and the API is just there as an implementation detail 
to them, no?

> Another option would be to have
> accessService in AccessManager and have it still return a Service *, but
> that would make it inconsistent with accessApplet.

i think the accessService should just return a Service*, since Service is 
(now, anyways :) fully async, and accessApplet should return a job because 
applets are _not_ async. it's not inconsistent really, it's just that the two 
kinds of objects behave very differently.

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Qt Software
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/plasma-devel/attachments/20090708/6cc0a5e2/attachment.sig