[Kde-pim] Review Request 112111: Extract the gid even if we're ignore payload is set.

Christian Mollekopf chrigi_1 at fastmail.fm
Mon Aug 19 10:53:52 BST 2013



> On Aug. 19, 2013, 7:42 a.m., Dan Vrátil wrote:
> > If I understand it correctly, the concern here is that when extracting the GID from payload you now have to set ignorePayload to false, which will cause the ItemModifyJob to upload the entire payload to Akonadi Server even though it's not necessary (you are only changing GID), and by removing the ignorePayload check you would be able to extract GID from payload while having ignorePayload set to true, right? I believe that's a good idea.
> > 
> > You probably want to replace the ignorePayload condition in getGid() by !item.loadedPayloadParts().isEmpty() to avoid querying TypePluginLoader and calling an extractor for an item without payload (I believe that was why in the original review Volker suggested using ignorePayload).
> > 
> > I would like to see ignoreGid() option too (with default set to true, only extractors would change it to false) to prevent GID extraction when fetching email bodies (you already have Message-ID extracted from headers). Fetching GID is disabled by default, so there's a chance that you would be in a situation when GID is not fetched (but stored on a server), so the ItemModifyJob would extract it and overwrite the one stored in Akonadi.

Yes, you understood correctly =)

I can add the check for no available payload parts, although I'm not sure this really has a performance impact, and could be avoided by the ignorePayload option in the cases it really has.

Given the GID is initially written by the createjob, the GID really never changes (it shouldn't), and we run the migration job as new extractors come along, it probably makes sense to ignore the gid by default to avoid it being written over and over again.


- Christian


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


On Aug. 16, 2013, 8:22 a.m., Christian Mollekopf wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112111/
> -----------------------------------------------------------
> 
> (Updated Aug. 16, 2013, 8:22 a.m.)
> 
> 
> Review request for KDEPIM-Libraries and Volker Krause.
> 
> 
> Description
> -------
> 
> Extract the gid even if we're ignore payload is set.
> 
> This is required so we can rewrite the gid without having set the payload again.
> As I'm not aware of any situation where we need to disable gid extraction by ignoring the payload, I'll just remove this for now.
> Otherwise an ignoreGid option would be a solution.
> 
> 
> Diffs
> -----
> 
>   akonadi/gid/gidextractor.cpp 63c1296f72afa1b439ba61754e3f881f7e0a9f3c 
>   akonadi/gid/gidextractor_p.h 95a6ead9eb9ce57b4a31366ed032af68681c87de 
>   akonadi/itemmodifyjob.cpp ed166a7181aa5051db8429b799a352f4ea7efe9d 
> 
> Diff: http://git.reviewboard.kde.org/r/112111/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Christian Mollekopf
> 
>

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


More information about the kde-pim mailing list