[Kde-pim] Review Request: iCal format fixes for CREATED and DTSTAMP

Christian Mollekopf chrigi_1 at fastmail.fm
Thu Dec 13 15:15:17 GMT 2012


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

(Updated Dec. 13, 2012, 3:15 p.m.)


Review request for KDEPIM, KDEPIM-Libraries and David Jarvie.


Changes
-------

I replaced the check for the product-id, which is the wrong version number to check, with a X-KDE-ICAL-IMPLEMENTATION-VERSION property, which contains the version of the kde ical implementation, and can therefore be used for proper backward compatiblity to older versions of this implementation. Of course we can't fix old files, so we still need to parse prodid to check if the file was potentially written by kcalcore, for future backward compatibility issues we will have proper means to identify the version though.

I also had to change the compat class to a decorator, so we're able to stack multiple compat classes, which is a requirement for this fix to work while not killing other backwards compat fixes such as CompatPre35 (so far only one compat class could be active, now multiple can).

Note that this patch broke some unittests due to the new X-KDE-ICAL-IMPLEMENTATION-VERSION property which is always written out.

Here's an event with the new property for your reference:
BEGIN:VCALENDAR?
PRODID:-//K Desktop Environment//NONSGML libkcal 4.3//EN?
VERSION:2.0?
X-KDE-ICAL-IMPLEMENTATION-VERSION:1.0?
BEGIN:VEVENT?
ORGANIZER:MAILTO:nobody at nowhere?
DTSTAMP:20031213T204152Z?
CREATED:20031213T204152Z?
UID:uid?
LAST-MODIFIED:20031213T204152Z?
SUMMARY:Holladiho?
DTSTART:20031213T071500Z?
TRANSP:OPAQUE?
END:VEVENT?
END:VCALENDAR?

The alternative fix would be to attach this version number to PRODID, which would work too as long as the actual ical implementation writes the version, and not the user of the library, but that would involve parsing the prodid string for version numbers and I think this way is cleaner.

I still need to check if akonadi also write this property out (I think it uses a hardcoded VCALENDAR and only write the VEVENT component using kcalcore).


Description
-------

This patch fixes the uses of CREATED and DTSTAMP in respect to RFC5545 (iCal) and RFC5546 (iTip).

CREATED was used as serialization timestamp, which is IMO wrong as it should preserve the creation date of the conceptual ical object. The dtstamp was written as creation-date but never read back, but it should be the last-modification-date without METHOD (iTip), and the time of serialization with a METHOD.

This patch fixes the following problems:
* dtstamp and created exchanged when writing any ical object
* created date is not preserved
* last-modification-date is not preserved
* vFreebusy iTip messages contains a created date (but must not)

As a sideeffect this patch also fixed the RecurNext* and RecurPrev* unittests.

This patch should be safe to apply to existing systems.


This addresses bugs 310448 and 310469.
    http://bugs.kde.org/show_bug.cgi?id=310448
    http://bugs.kde.org/show_bug.cgi?id=310469


Diffs (updated)
-----

  kcalcore/compat.h c74aa9d8037750e661bf7822b60ef01ab65b9a33 
  kcalcore/compat.cpp 9447fff30dfc9242bb891b4ad54902b37aba58ab 
  kcalcore/icalformat_p.h 128ff7ea0e5fe2aa5ed861854ee3b87da10bb551 
  kcalcore/icalformat_p.cpp 418a92a6e1f033d1110add754c4966436585dea2 
  kcalcore/tests/CMakeLists.txt 10f449dc3c83293b923c02dc6e429df5e60d3996 
  kcalcore/tests/testcreateddatecompat.h PRE-CREATION 
  kcalcore/tests/testcreateddatecompat.cpp PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/107480/diff/


Testing
-------

checked existing unit tests (all pass). Running the patch on my productive system now.


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