Skip to content

Always store X-KDE-VOLATILE-XXX properties as volatile.

Damien Caliste requested to merge dcaliste/kcalendarcore:volatile into master

When setting custom a property with CustomProperties::setNonKDECustomProperty, save properties starting with X-KDE-VOLATILE- into the mVolatileProperties member. This is symetrical with removeNonKDECustomProperty().

The problem of always saving to mProperties member whatever the property name when using the setNonKDECustomProperty() API, is that in icalformat_p.cpp in writeCustomProperties() line 685, the volatile properties are not written during the iCal serialisation, which is fine per se. Then, let's imagine that we later on parse these data and create a new object from them. The two objects are not equal anymore (using the operator==()) because in the original object the X-KDE-VOLATILE-XXX property is part of the mProperties member and thus is used during the comparison.

This MR may not be the best way to fix this issue of not being reentrant after serialisation. What do you think ? The use case that raised this issue in SailfishOS is this one:

  • volatile information is added to an incidence with a proper setCustomProperty() API call.
  • the incidence is stored in a local database and properties are all stored using the ::properties() API (which lists also the volatile ones), using a (key,value,parameter) triplet.
  • later on, incidence from the database are read and all properties triplet are set using a call to setNonKDECustomProperty().

I understand that patching our database code to actually call either setCustomProperty() or setNonKDECustomProperty() depending on property name is an option. But I think, it could be safer to use the proposed patch, because there is no check done on property name when calling setNonKDECustomProperty() while internally isVolatile() is a check done on name.

What do you think ?

Merge request reports