Allow recurrence objects to be changed. (#500)

* Fixes #379 - editing a recurrence object and saving it will do what it ought to do on all servers - change only that recurrence and nothing else
* The `save`-method now takes a boolean `all_recurrences` which 
* piggybacking some code refactoring, fixes #476

#500

Author: tobixen

CHANGELOG.md

@@ -14,10 +14,19 @@ In version 2.0, support for python 3.7 and python 3.8 will be officially dropped
 
 ## [1.6.0] - [Unreleased]
 
+### Added
+
+* New option `event.save(all_recurrences=True)` to edit the whole series when saving a modified recurrence.
+
 ### Fixed
 
+* Save single recurrence.  I can't find any information in the RFCs on this, but all servers I've tested does the wrong thing - when saving a single recurrence (with RECURRENCE-ID set but without RRULE), then the original event (or task) will be overwritten (and the RRULE disappear), which is most likely not what one wants.  New logic in place (with good test coverage) to ensure only the single instance is saved. Issue https://github.com/python-caldav/caldav/issues/379, pull request https://github.com/python-caldav/caldav/pull/500
 * Scheduling support.  It was work in progress many years ago, but uncompleted work was eventually committed to the project.  I managed to get a DAViCal test server up and running with three test accounts, ran through the tests, found quite some breakages, but managed to fix up.  https://github.com/python-caldav/caldav/pull/497
 
+### Refactoring
+
+* Partially tossed out all internal usage of vobject, https://github.com/python-caldav/caldav/issues/476.  Refactoring and removing unuseful code.  Parts of this work was accidentally committed directly to master, 2f61dc7adbe044eaf43d0d2c78ba96df09201542, the rest was piggybaced in through  https://github.com/python-caldav/caldav/pull/500.
+
 ## [1.5.0] - 2025-05-24
 
 Version 1.5 comes with support for alarms (searching for alarms if the server permits and easy interface for adding alamrs when creating events), lots of workarounds and fixes ensuring compatibility with various servers, refactored some code, and done some preparations for the upcoming server compatibility hints project.

caldav/calendarobjectresource.py

@@ -225,10 +225,6 @@ def expand_rrule(
             ):
                 continue
             if "RECURRENCE-ID" not in occurrence:
-                ## we should not get here?
-                import pdb
-
-                pdb.set_trace()
                 occurrence.add("RECURRENCE-ID", occurrence.get("DTSTART").dt)
             calendar.add_component(occurrence)
 
@@ -723,9 +719,7 @@ def _put(self, retry_on_failure=True):
                 raise error.PutError(errmsg(r))
 
     def _create(self, id=None, path=None, retry_on_failure=True) -> None:
-        ## We're efficiently running the icalendar code through the icalendar
-        ## library.  This may cause data modifications and may "unfix"
-        ## https://github.com/python-caldav/caldav/issues/43
+        ## TODO: Find a better method name
         self._find_id_path(id=id, path=path)
         self._put()
 
@@ -784,9 +778,10 @@ def save(
         obj_type: Optional[str] = None,
         increase_seqno: bool = True,
         if_schedule_tag_match: bool = False,
+        only_this_recurrence: bool = True,
+        all_recurrences: bool = False,
     ) -> Self:
-        """
-        Save the object, can be used for creation and update.
+        """Save the object, can be used for creation and update.
 
         no_overwrite and no_create will check if the object exists.
         Those two are mutually exclusive.  Some servers don't support
@@ -795,10 +790,36 @@ def save(
         obj_type is only used in conjunction with no_overwrite and
         no_create.
 
+        is_schedule_tag_match is currently ignored. (TODO - fix or remove)
+
+        The SEQUENCE should be increased when saving a new version of
+        the object.  If this behaviour is unwanted, then
+        increase_seqno should be set to False.  Also, if SEQUENCE is
+        not set, then this will be ignored.
+
+        The behaviour when saving a single recurrence object to the
+        server is as far as I can understand not defined in the RFCs,
+        but all servers I've tested against will overwrite the full
+        event with the recurrence instance (effectively deleting the
+        recurrence rule).  That's almost for sure not what the caller
+        intended.  only_this_recurrence and all_recurrences only
+        applies when trying to save a recurrence object.  They are by
+        nature mutually exclusive, but since only_this_recurrence is
+        True by default, it will be ignored if all_recurrences is set.
+
+        If you want to sent the recurrence as it is to the server,
+        you should set both all_recurrences and only_this_recurrence
+        to False.
+
         Returns:
          * self
 
         """
+        ## Rather than passing the icalendar data verbatimely, we're
+        ## efficiently running the icalendar code through the icalendar
+        ## library.  This may cause data modifications and may "unfix"
+        ## https://github.com/python-caldav/caldav/issues/43
+        ## TODO: think more about this
         if not obj_type:
             obj_type = self.__class__.__name__.lower()
         if (
@@ -812,6 +833,18 @@ def save(
 
         path = self.url.path if self.url else None
 
+        def get_self():
+            self.id = self.id or self.icalendar_component.get("uid")
+            if self.id:
+                try:
+                    if obj_type:
+                        return getattr(self.parent, "%s_by_uid" % obj_type)(self.id)
+                    else:
+                        return self.parent.object_by_uid(self.id)
+                except error.NotFoundError:
+                    return None
+            return None
+
         if no_overwrite or no_create:
             ## SECURITY TODO: path names on the server does not
             ## necessarily map cleanly to UUIDs.  We need to do quite
@@ -824,43 +857,86 @@ def save(
             ## to do a PUT instead of POST when creating new data).
             ## TODO: the "find id"-logic is duplicated in _create,
             ## should be refactored
-            if not self.id:
-                for component in self.vobject_instance.getChildren():
-                    if hasattr(component, "uid"):
-                        self.id = component.uid.value
+            existing = get_self()
             if not self.id and no_create:
                 raise error.ConsistencyError("no_create flag was set, but no ID given")
-            existing = None
-            ## some servers require one to explicitly search for the right kind of object.
-            ## todo: would arguably be nicer to verify the type of the object and take it from there
-            if not self.id:
-                methods = []
-            elif obj_type:
-                methods = (getattr(self.parent, "%s_by_uid" % obj_type),)
-            else:
-                methods = (
-                    self.parent.object_by_uid,
-                    self.parent.event_by_uid,
-                    self.parent.todo_by_uid,
-                    self.parent.journal_by_uid,
+            if no_overwrite and existing:
+                raise error.ConsistencyError(
+                    "no_overwrite flag was set, but object already exists"
                 )
-            for method in methods:
-                try:
-                    existing = method(self.id)
-                    if no_overwrite:
-                        raise error.ConsistencyError(
-                            "no_overwrite flag was set, but object already exists"
-                        )
-                    break
-                except error.NotFoundError:
-                    pass
 
             if no_create and not existing:
                 raise error.ConsistencyError(
                     "no_create flag was set, but object does not exists"
                 )
 
-        if increase_seqno and b"SEQUENCE" in to_wire(self.data):
+        ## Save a single recurrence-id and all calendars servers seems
+        ## to overwrite the full object, effectively deleting the
+        ## RRULE.  I can't find this behaviour specified in the RFC.
+        ## That's probably not what the caller intended intended.
+        if (
+            only_this_recurrence or all_recurrences
+        ) and "RECURRENCE-ID" in self.icalendar_component:
+            obj = get_self()  ## get the full object, not only the recurrence
+            ici = obj.icalendar_instance  # ical instance
+            if all_recurrences:
+                occ = obj.icalendar_component  ## original calendar component
+                ncc = self.icalendar_component.copy()  ## new calendar component
+                for prop in ["exdate", "exrule", "rdate", "rrule"]:
+                    if prop in occ:
+                        ncc[prop] = occ[prop]
+
+                ## dtstart_diff = how much we've moved the time
+                ## TODO: we may easily have timezone problems here and events shifting some hours ...
+                dtstart_diff = (
+                    ncc.start.astimezone() - ncc["recurrence-id"].dt.astimezone()
+                )
+                new_duration = ncc.duration
+                ncc.pop("dtstart")
+                ncc.add("dtstart", occ.start + dtstart_diff)
+                for ep in ("duration", "dtend"):
+                    if ep in ncc:
+                        ncc.pop(ep)
+                ncc.add("dtend", ncc.start + new_duration)
+                ncc.pop("recurrence-id")
+                s = ici.subcomponents
+
+                ## Replace the "root" subcomponent
+                comp_idxes = (
+                    i
+                    for i in range(0, len(s))
+                    if not isinstance(s[i], icalendar.Timezone)
+                )
+                comp_idx = next(comp_idxes)
+                s[comp_idx] = ncc
+
+                ## The recurrence-ids of all objects has to be
+                ## recalculated (this is probably not quite right.  If
+                ## we move the time of a daily meeting from 8 to 10,
+                ## then we need to do this.  If we move the date of
+                ## the first instance, then probably we shouldn't
+                ## ... oh well ... so many complications)
+                if dtstart_diff:
+                    for i in comp_idxes:
+                        rid = s[i].pop("recurrence-id")
+                        s[i].add("recurrence-id", rid.dt + dtstart_diff)
+
+                return obj.save(increase_seqno=increase_seqno)
+            if only_this_recurrence:
+                existing_idx = [
+                    i
+                    for i in range(0, len(ici.subcomponents))
+                    if ici.subcomponents[i].get("recurrence-id")
+                    == self.icalendar_component["recurrence-id"]
+                ]
+                error.assert_(len(existing_idx) <= 1)
+                if existing_idx:
+                    ici.subcomponents[existing_idx[0]] = self.icalendar_component
+                else:
+                    ici.add_component(self.icalendar_component)
+                return obj.save(increase_seqno=increase_seqno)
+
+        if "SEQUENCE" in self.icalendar_component:
             seqno = self.icalendar_component.pop("SEQUENCE", None)
             if seqno is not None:
                 self.icalendar_component.add("SEQUENCE", seqno + 1)

caldav/compatibility_hints.py

@@ -370,6 +370,10 @@
     "no_alarmsearch",
     "no_events_and_tasks_on_same_calendar",
 
+    ## TODO: I just discovered that when searching for a date some
+    ## years after a recurring daily event was made, the event does
+    ## not appear.
+
     ## extra features not specified in RFC5545
     "calendar_order",
     "calendar_color"

tests/test_caldav.py

@@ -2910,6 +2910,101 @@ def testRecurringDateWithExceptionSearch(self):
                 tzinfo=None
             ) == datetime(2024, 4, 25, 12, 30, 00)
 
+    def testEditSingleRecurrence(self):
+        """
+        It should be possible to fetch a single recurrence from
+        the calendar using search and expand, edit it and save it.
+        Only the recurrence should be edited, not the rest of the
+        event.
+        """
+        self.skip_on_compatibility_flag("no_recurring")
+        cal = self._fixCalendar()
+
+        ## Create a daily recurring event
+        cal.save_event(
+            uid="test1",
+            summary="daily test",
+            dtstart=datetime(2015, 1, 1, 8, 7, 6),
+            dtend=datetime(2015, 1, 1, 9, 7, 6),
+            rrule={"FREQ": "DAILY"},
+        )
+
+        def search(month):
+            """
+            Internal function to find one recurrence object
+            """
+            recurrence = cal.search(
+                event=True,
+                start=datetime(2015, month, 1),
+                end=datetime(2015, month, 2),
+                expand="client",  ## client will be default from 2.0
+            )
+            assert len(recurrence) == 1
+            return recurrence[0]
+
+        def summary_by_month(month):
+            return search(month).icalendar_component["summary"]
+
+        ## Search for a recurrence
+        recurrence = search(7)
+
+        ## Modify it and save it
+        recurrence.icalendar_component["summary"] = "half a year of daily testing"
+        recurrence.save()
+
+        ## Only one day should be affected
+        assert summary_by_month(6) == "daily test"
+        assert summary_by_month(7) == "half a year of daily testing"
+        assert summary_by_month(8) == "daily test"
+
+        ## let's try to set several recurrence exceptions
+        recurrence = search(2)
+        recurrence.icalendar_component["summary"] = "one month of daily testing"
+        recurrence.save()
+
+        assert summary_by_month(1) == "daily test"
+        assert summary_by_month(2) == "one month of daily testing"
+        assert summary_by_month(7) == "half a year of daily testing"
+
+        ## Changing any of the exceptions should also work
+        recurrence = search(7)
+        recurrence.icalendar_component["summary"] = "six months of daily testing"
+        recurrence.save()
+        assert summary_by_month(7) == "six months of daily testing"
+
+        ## this new feature does not workk on python 3.8.  We will soon enough
+        ## release 2.0 and shed the 3.8-dependency.  As for now, just skip the rest of the test.
+        if sys.version_info < (3, 9):
+            return
+
+        ## parameter all_recurrences should change all recurrences -
+        ## except February and July
+        recurrence = search(9)
+        recurrence.icalendar_component["summary"] = "daily testing"
+        recurrence.save(all_recurrences=True)
+        assert summary_by_month(1) == "daily testing"
+        assert summary_by_month(2) == "one month of daily testing"
+        assert summary_by_month(3) == "daily testing"
+        assert summary_by_month(7) == "six months of daily testing"
+
+        ## Last ... let's change the dtend and dtstart of the recurrence
+        recurrence = search(9)
+        recurrence.icalendar_component.pop("dtstart")
+        recurrence.icalendar_component.add("dtstart", datetime(2015, 9, 1, 8, 0, 0))
+        recurrence.icalendar_component.pop("dtend")
+        recurrence.icalendar_component.add("dtend", datetime(2015, 9, 1, 10, 0, 0))
+        recurrence.save(all_recurrences=True)
+
+        recurrence = search(8)
+        assert (
+            recurrence.icalendar_component.start.astimezone()
+            == datetime(2015, 8, 1, 8, 0, 0).astimezone()
+        )
+        assert (
+            recurrence.icalendar_component.end.astimezone()
+            == datetime(2015, 8, 1, 10, 0, 0).astimezone()
+        )
+
     def testOffsetURL(self):
         """
         pass a URL pointing to a calendar or a user to the DAVClient class,