[Kde-pim] [PATCH] Akonadi::ItemSync customization

[Volker Krause]

On Friday 27 June 2008 21:44:07 Kevin Krammer wrote:
> Hi Dmitry,
>
> On Friday 27 June 2008, Dmitry Ivanov wrote:
> > Hello,
> >
> > The attached patch allows to customize the behaviour of 
> > Akonadi::ItemSync:
> >
> > 1. Allows subsclasses to "plug in" their own sync algorithms by
> > reimplementing virtual bool updateItem( const Item &local, Item &remote )
> > const
>
> I know that "local" and "remote" make sense in the resource context, but I
> would appreciate names that reflect more precisely that one item is the
> version already known to Akonadi and the other is the potentially changed
> one.
>
> I specifically renamed similar parameters to other methods because it
> always confused the hell out of me since if your mental context is the
> Akonadi connection, "local" indicated "from this process" and "remote"
> indicated "from the Akonadi server process".
>
> Probably more like "stored" or "old" or "current" instead of "local", no
> idea for "remote" alternatives.

newItem (since new is a keyword)? I agree that local/remote is really 
confusing since the meaning changes depending on your point of view.

> > 2. Allows to specify how much data ItemSync should fetch from the cache
> > when building a list of all items for the collection being synchronized,
> > by calling
> >    fetchScope() or setFetchScope().
>
> One suggestion:
>
>  void ItemSync::doStart()
>  {
> +  ItemFetchJob* job = new ItemFetchJob( d->mSyncCollection, this );
> +  job->fetchScope() = d->mFetchScope;
>
> IMHO job->setFetchScopr( d->mFetchScope ) would be cleaner.
>
> Anyway, since this is the last time we can do a BIC change at all, I am
> asking if this particular step of the synching is the only one that will
> ever need customization, e.g. I can imagine that a client already has an
> up-to-date item list around and might not need/want a separate item fetch.

After the discussions I recently had with Dmitry and the problems we already 
had with this in the IMAP resource I'm convinced by now that we need to 
completely rethink the whole item syncing part for 4.2. We probably end up 
with a new ItemSync2 class which is merely a basic interface (allowing all 
kinds of customization) and deprecating the old one. Since that's not used 
anywhere in the API, the impact would be minimal. Therefore I don't think we 
have to put too much API design efforts into ItemSync for 4.1.

So, API-wise the patch is fine IMHO, the only thing I'm a bit worried about 
are the subtle side-effects the moving of the ItemFetchJob might have. I hope 
to have time to test it tomorrow, there are some unit tests for itemsync 
which should find any job scheduling related isssues but probably need 
adjustments since ItemSync::exec() will behave differently now (not a problem 
in real applications, just breaks my test hacks ;-) ).

regards
Volker
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 194 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20080627/458ae3c1/attachment.sig>
-------------- next part --------------
_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/