Review request for KSecretsService components

[Albert Astals Cid]

A Dijous, 10 de novembre de 2011, Valentin Rusu vàreu escriure:
> On 11/10/2011 12:48 AM, Albert Astals Cid wrote:
> > A Dijous, 10 de novembre de 2011, Valentin Rusu vàreu escriure:
> >> Hello Albert,
> >> 
> >> Thanks for the thourough review.
> >> 
> >> On 11/09/2011 03:26 PM, Albert Astals Cid wrote:
> >>> This is scary, last time i used kwallet, i had to add a single line,
> >>> and now there are like a billion of classes?
> >> 
> >> Welcome to the asynchronous jobs world. I had the same opinion at
> >> start.
> >> And, to be honest, that's why it takes so long to implement, as I'm
> >> not
> >> working on it only during my spare time.
> > 
> > Are you saying you don't have anything as simple as this in the new API?
> > 
> > QString walletName = KWallet::Wallet::NetworkWallet();
> > KWallet::Wallet *wallet = KWallet::Wallet::openWallet(walletName,
> > parentwid); if ( !wallet->hasFolder( "KPdf" ) )
> > 
> >    wallet->createFolder( "KPdf" );
> > 
> > wallet->setFolder( "KPdf" );
> > wallet->readPassword( walletKey, retrievedPass )
> 
> Yes, your code isn't asynchrounous ! (boo)

Yeah well, kparts gives you
  virtual bool openUrl( const KUrl &url );
With that signature you are forced to either use synchronous code or create a 
nested event loop, pick your poison. 

> Well, I don't fully agree with this point of view, but in Randa they
> managed to convince me about the benefits.
> Take a look inside kwallet.cpp class to see how this new API should be used.
> Another example is the ksecrets API inside kdereview/ksecrets repository.

This answer makes me really scared in that it is not going to be 5 lines :-/

Ok, now i've looked at the API and I am not sure i like it, particulary 
nonQtlike seems the way of searching things by passing a map with a key/value 
structure of what you want to find.

I can see that it is very flexible, but string based api end up with people 
writing "key" instead of "Key" and they lose days debugging why it fails.

Also i'm a bit confused about the docu of 
SearchCollectionItemsJob * searchItems( const StringStringMap &attributes );
it says 
 * KSecretsService uses overall the following standard attributes:
 * "Label" : item's or collection's label
so i understand that "Label" is the only key of the map that you can use, and 
then the example says
 * StrinStringMap attrs;
 * attrs["Key"] = "";
and uses "Key" ? Does this mean "Key" is a valid attribute too and should be 
documented? Or should it say "Label"? Or i totally misunderstood the 
documentation?
Also note it says StrinStringMap, missing g.

Albert