[Kde-pim] Review Request: Allow more than one payload class per mime type in Akonadi::Item

[Volker Krause]

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

Ship it!


Very educating to read, obviously I can't find any mistakes in there :)

- Volker


On 2010-08-05 17:17:32, Marc Mutz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4910/
> -----------------------------------------------------------
> 
> (Updated 2010-08-05 17:17:32)
> 
> 
> Review request for KDE PIM, Allen Winter, Volker Krause, Bertjan Broeksema, and Sergio Martins.
> 
> 
> Summary
> -------
> 
> This patch makes it possible for Akonadi::Item to contain more than one parsed representation of a given payload, and convert between them.
> 
> This is a general framework designed to solve the immediate problem of keeping binary compatibility with older libakonadi-kde users while at the same time transitioning from KCal::Incidence to KCalCore::Incidence payloads. The design goals are:
> 1. Stay binary compatible (this still needs to be checked)
> 2. Stay source compatible (this is mostly the case; only adding Q_DECLARE_METATYPE for payloads is now required)
> 3. Allow a payload set as QSharedPointer<KCalCore::Incidence> to be retrieved as a boost::shared_ptr<KCal::Incidence>
> 4. Anticipate similar scenarios in the future (KMime2 was mentioned)
> 
> This is a rationale for the design decisions I made:
> 
> The old template code cannot be changed, since it's compiled into the applications using libakonadi-kde. So we have a new set of payload functions (hasPayload<T>(), payload<T>(), setPayload<T>()) that do not call payloadBase()/setPayloadBase() (thus leaving them as indicators of old code calling us), but the -V2 versions. These get qMetaTypeId() (and shared pointer id) passed, so that we can store that information along with the PayloadBase, and later know what the user wants to retrieve, so we can dish out the correct PayloadBase.
> 
> The problem, now, is that the old function did not pass that information along, so we need to reconstruct the information after the fact. That's where addToLegacyMetaTypeIdMap<T>() comes in: we assume that we will not have to deal with old serialiser plugins, but new ones. Those are supposed to tell us which meta type id the old payload was for a given mime type (I'm still undecided about whether that's really needed, or whether it suffices to treat the lagacy type as simply a separate type). We can use this information to test the incoming PayloadBase' typeName() against our expectations for legacy types, and, if we get a match, we clone the payload so we can dish it out quickly when we're asked for it by code that was recompiled, but still requests the old types.
> 
> When serialising an item, we don't really care about which of the parsed formats' plugin gets the job done, as we assume that the serialisations will be equivalent. For deserialisation, one plugin is marked as the default for each mime type (otherwise a random one is chosen, but not the legacy one, unless we find nothing else, this is done in order to defer loading the legacy plugin until absolutely necessary).
> 
> When we are asked for a payload in a shared pointer, and we don't find it, we check whether we have the same payload in "the other shared pointer" (this should be extended to allow for more than two shared pointer types (std::shared_ptr is 'round the corner...)). If we find that one, we try to clone the payload (not the PayloadBase, that one's type includes the shared pointer type). If that's not possible (b/c the payload T doesn't provide a virtual copy constructor T * T::clone() const), we fail.
> 
> When we are asked for a payload we don't have, but we have something else, we try to convert that something else into the requested type by serialising the something else and deserialising to the requested type.
> 
> To make all this work, the PayloadTraits were extended to assign each shared pointer type a unique sharedPointerId, and a set of traits were written to check for clone() methods, and to convert between shared pointers. On the TypePluginLoader front, the registry was extended to allow for more than one plugin per mimetype (PluginLoader itself is also affected). Desktop files describing akonadi_serializer plugins are now required to provide an X-Akonadi-Class entry with class names it supports. There are two special class names: "default", used to pick the default deserialiser, and "legacy", which indicates the plugin for the type used by KDE < 4.6.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdepim/runtime/plugins/CMakeLists.txt 1159370 
>   /trunk/KDE/kdepim/runtime/plugins/akonadi_serializer_addressee.desktop 1159370 
>   /trunk/KDE/kdepim/runtime/plugins/akonadi_serializer_bookmark.desktop 1159370 
>   /trunk/KDE/kdepim/runtime/plugins/akonadi_serializer_contactgroup.desktop 1159370 
>   /trunk/KDE/kdepim/runtime/plugins/akonadi_serializer_kcal.desktop 1159370 
>   /trunk/KDE/kdepim/runtime/plugins/akonadi_serializer_kcalcore.h PRE-CREATION 
>   /trunk/KDE/kdepim/runtime/plugins/akonadi_serializer_kcalcore.cpp PRE-CREATION 
>   /trunk/KDE/kdepim/runtime/plugins/akonadi_serializer_kcalcore.desktop PRE-CREATION 
>   /trunk/KDE/kdepim/runtime/plugins/akonadi_serializer_mail.desktop 1159370 
>   /trunk/KDE/kdepim/runtime/plugins/akonadi_serializer_microblog.desktop 1159370 
>   /trunk/KDE/kdepimlibs/akonadi/conflicthandling/conflictresolvedialog.cpp 1159076 
>   /trunk/KDE/kdepimlibs/akonadi/item.h 1159076 
>   /trunk/KDE/kdepimlibs/akonadi/item.cpp 1159076 
>   /trunk/KDE/kdepimlibs/akonadi/item_p.h 1159076 
>   /trunk/KDE/kdepimlibs/akonadi/itempayloadinternals_p.h 1159076 
>   /trunk/KDE/kdepimlibs/akonadi/itemserializer.cpp 1159076 
>   /trunk/KDE/kdepimlibs/akonadi/itemserializer_p.h 1159076 
>   /trunk/KDE/kdepimlibs/akonadi/pluginloader.cpp 1159076 
>   /trunk/KDE/kdepimlibs/akonadi/pluginloader_p.h 1159076 
>   /trunk/KDE/kdepimlibs/akonadi/tests/itemhydratest.h 1159076 
>   /trunk/KDE/kdepimlibs/akonadi/tests/itemhydratest.cpp 1159076 
>   /trunk/KDE/kdepimlibs/akonadi/tests/pluginloadertest.cpp 1159076 
>   /trunk/KDE/kdepimlibs/akonadi/typepluginloader.cpp 1159076 
>   /trunk/KDE/kdepimlibs/akonadi/typepluginloader_p.h 1159076 
> 
> Diff: http://reviewboard.kde.org/r/4910/diff
> 
> 
> Testing
> -------
> 
> - See itemhydratest for a unit test of shared pointer conversions.
> - For actual payload class conversion, I took the KCalCore serialiser from kdepim_kcalcore branch (new files in the diff, becuase reviewboard barfs on svn cp'ed files, but the diff in reality is much smaller), set it to be the default deserialiser for text/calendar and checked that akonadiconsole (which still uses KCal::Incidence only) can still display it.
> 
> 
> Thanks,
> 
> Marc
> 
>

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