python-caldav / caldav

Apache License 2.0
313 stars 91 forks source link

Overwriting CalendarObjectResource.data with new data as string #346

Open yuwash opened 7 months ago

yuwash commented 7 months ago

Although the data field has a setter, it’s rather difficult to use if you (apparently?) can’t just put a string into it, but it uses some internal model that I couldn’t figure out. The only bad hack that seems to get it updated is (context)

            try:
                save_ = caldav.Todo.save
                caldav.Todo.save = lambda *args, **kwargs: None
                temp_todo = davcalendar.add_todo(serialized)
            finally:
                caldav.Todo.save = save_
            existing_todo.data = temp_todo.data

Another related difficulty is though that python-caldav represents each VTODO as a complete VCALENDAR when getting Todo.data whilst the library I’m using for VTODO (ics) doesn’t. The serialized string here thus only contains the VTODO part. Still add_todo does seem to work.

tobixen commented 7 months ago

Although the data field has a setter, it’s rather difficult to use if you (apparently?) can’t just put a string into it,

That should always work, it would be nice to see a test case or a scenario where it doesn't work.

Obviously you should put valid icalendar data into it, but I don't remember if there is any validation done when setting the data field. I think not.

tobixen commented 7 months ago

I have never heard about the ics-py libary.

The library currently is supporting vobject and icalendar, it has setters and getters for vobject_instance, icalendar_instance (and also icalendar_component) . If the ics-py library is good, I would consider accepting a pull-request supporting the ics-py library in the same way.

There is some magic concerning those setters and getters, internally the state is always represented in one of those three formats, when it's needed with a different kind of format the data is converted.

This efficiently means that accessing the attribute causes side-effects, and that's bad. Just accessing event.icalendar_component or event.data may cause:

component = event.icalendar_component
if 'old description' in event.data:
    component['description'] = 'New description'
event.save()

Those attributes are also accessed internally in the library, causing the problems above to happen rather unexpectedly.

At some point I will redesign a bit and create a 2.0-interface (but I'm really serious on backward compability - everything that works under 1.0 should continue working until 3.0 is released). Most likely I will remove (deprecate) those attributes from the 2.0-interface and replace them with explicit methods for converting the data.

yuwash commented 7 months ago

Thank you very much for the detailed response! I was missing the part that python-caldav uses icalendar and VObject for representing ical objects. I just recently started with a project and chose ics because that project seems to be the most active one (VObject being stale since 5 years), but perhaps I’ll look into switching to icalendar to make the whole thing simpler. Unlike my initial assumption about the lack of a VCALENDAR layer in the string that I used, the minimal offline example did work as expected. Have to check again why it seems to have not updated Todo.data the last time I did it with a real online client.

>>> import caldav                                                                                                                                                       
>>> import types
>>> put_response = types.SimpleNamespace(status=204)                                                                                                                    
>>> client = types.SimpleNamespace(put=lambda url, body, headers: put_response)
>>> cal = caldav.Calendar(client=client, url="")               
>>> vtodo = "BEGIN:VTODO\nDTSTAMP:20231123T215645\nSUMMARY:Do something nice\nUID:bc7\nEND:VTODO\n"                                                                                   
>>> t = cal.add_todo(vtodo)
>>> t.data = "BEGIN:VTODO\nDTSTAMP:20231123T215645\nSUMMARY:Do something else\nUID:bc7\nEND:VTODO\n"  # Changed the summary
>>> t.data
'BEGIN:VTODO\nDTSTAMP:20231123T215645\nSUMMARY:Do something else\nUID:bc7\nEND:VTODO\n'
tobixen commented 7 months ago

vobject is stale, yes, it's historic reasons why vobject is supported. I've had multiple pull requests on replacing vobjects with the icalendar library, but couldn't do it due to backward compability - so event.instance will still yield a vobject. I came up with the vobject_instance and icalendar_instance properties so it's possible to choose, but vobject_instance will probably be deprecated at some point in the future.