[Kde-pim] Review Request 109914: [IMAP] Use itemsRetrievedIncremental() when fetching missing bodies

[Kevin Ottens]

> On April 9, 2013, 6:24 a.m., Kevin Ottens wrote:
> > Any chance to get that logic unit tested? The original submitter for this behavior didn't unit test and it shows now.
> 
> Dan Vrátil wrote:
>     The code for retrieving missing bodies uses Akonadi::ItemFetchJob, which means the test would have to start a fake Akonadi server. I know there's an implementation in kdepimlibs, but I don't know how to reach it there from kdepim-runtime.
> 
> Dan Vrátil wrote:
>     It also means that the RetrieveItemsTask test does not work at all at the moment.
> 
> Andras Mantia wrote:
>     I didn't write a unit test as the whole tests with a fake server are unreliable for me (and cause configuration issues according to David). Neverthless we should fix that and have working and reliable unit tests.
>     The patch itself looks good for me.

You know that's no good excuse right? The resource is designed to not talk to the akonadi server at all in test mode. I'm pretty sure it could be unit tested today. I'm really unimpressed at the current layering violation introduced. No Akonadi jobs should be used in the resource tasks, this way it's properly modular and testable. It's not the case anymore for AddCollectionTask and RetrieveItemsTask.


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109914/#review30737
-----------------------------------------------------------


On April 9, 2013, 12:09 p.m., Dan Vrátil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109914/
> -----------------------------------------------------------
> 
> (Updated April 9, 2013, 12:09 p.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Description
> -------
> 
> After RetrieveItemsTask finishes retrieving new emails or missing bodies, it fetches flags of emails 1...firstFetchedEmail. But in case of missing bodies fetch, the firstFetchedEmail does not have to be the newest item in the folder, so flags of (firstFetchedEmail + 1)...lastEmailInFolder are never retrieved and calling itemsRetrievalDone() causes Akonadi to drop all the messages in this range, as described in the referenced bug.
> 
> Using itemsRetrievedIncremental() when retrieving missing bodies fixes the issue, and I think it also makes more sense in this context, because syncing flags should be only done during regular sync.
> 
> 
> This addresses bug 316521.
>     http://bugs.kde.org/show_bug.cgi?id=316521
> 
> 
> Diffs
> -----
> 
>   resources/imap/retrieveitemstask.cpp 5129297 
> 
> Diff: http://git.reviewboard.kde.org/r/109914/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Vrátil
> 
>

_______________________________________________
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/