Move and rename on servers that keep a UID in one calendar, or rename a calendar's URL

docs/design/API_DESIGN.md

@@ -395,7 +395,9 @@ class AsyncCollection:
 backend reported when it listed the collection. Assigning to them would
 have to write to the server as a side effect, so renaming is an explicit
 `set_name()`, which raises `UnsupportedError` where the backend cannot
-rename (a feed, a single `.ics` file). The same goes for items: there is no
+rename (a feed, a single `.ics` file). A server that names a calendar's URL
+after its display name (Zimbra) moves the calendar on a rename, and
+`set_name()` then updates `id` too. The same goes for items: there is no
 `collection.save()` without an argument, and no autosave (but see
 [§12](#12-beyond-the-funded-scope-implicit-synchronization)). This follows caldav, which settled on
 methods for anything that talks to the server. *Proposed by the author in
@@ -485,10 +487,21 @@ for `PARENT`, the children pointing back), as caldav's `get_relatives()`
 does. plann calls it 16 times, so it has to be here. Relatives outside
 this collection are looked up across the backend.
 
-**`move`** within one backend uses its own move where it has one
-(CalDAV `MOVE`); between backends it is `add` to the target, then `delete`
-from the source, declared `EMULATED` because it is not atomic: a failure
-between the two leaves the item in both places, never in neither.
+**`move`** within one backend uses its own move where it has one; between
+backends, and on CalDAV until caldav has a `move()` using WebDAV `MOVE`
+([caldav #841](https://github.com/python-caldav/caldav/issues/841)), it is
+`add` to the target, then `delete` from the source, declared `EMULATED`
+because it is not atomic: a failure between the two leaves the item in both
+places, never in neither. Where a server keeps a UID in one calendar only
+(caldav's `save.duplicate-uid.cross-calendar`), the capability's `order`
+says what to do instead: `delete-first` where the copy would be refused
+(CCS, Cyrus, Ox), putting the item back if the add then fails (and saying so
+on the error where that fails too), and `add-may-move` where saving the copy
+moves the original (Zimbra), checking that the target holds the copy before
+the original is deleted. `delete-first` needs the deleted UID to be reusable
+(`save-load.reuse-deleted-uid`); where it is not, `move` is `UNSUPPORTED`.
+An occurrence cannot be moved by deleting first: deleting it only excludes
+it from its series, whose UID stays.
 
 **How it fits Gitea:** a repository is a collection with `components =
 {TASK}`; `events()` returns an empty list rather than raising (there are

src/calendaring/_async/caldav.py

@@ -49,6 +49,10 @@
 SAFE_NAME = re.compile(r"^[A-Za-z0-9._@-]{1,200}$")
 
 
+def _url_of(calendar: Calendar) -> str:
+    return str(calendar.url).rstrip("/") + "/"
+
+
 def _name_of(href: object) -> str:
     return unquote(str(href).rstrip("/").rsplit("/", 1)[-1])
 
@@ -60,7 +64,7 @@ def __init__(
         self, backend: AsyncCalDAVBackend, calendar: Calendar, *, name: str | None, components: frozenset[Component]
     ) -> None:
         self.calendar = calendar
-        self.url = str(calendar.url).rstrip("/") + "/"
+        self.url = _url_of(calendar)
         super().__init__(
             backend,
             id=_name_of(self.url),
@@ -216,7 +220,17 @@ async def _delete(self, native_id: str, *, etag: str | None) -> None:
         )
 
     async def _set_name(self, name: str) -> None:
-        await self._backend._dav.set_display_name(self.calendar, name)
+        dav = self._backend._dav
+        await dav.set_display_name(self.calendar, name)
+        # Zimbra names a calendar's URL after its display name, so a rename moves the calendar.
+        listed = await dav.calendars()
+        if any(_url_of(c) == self.url for c in listed):
+            return
+        named = [c for c in listed if await dav.display_name(c) == name]
+        if len(named) == 1:  # with several of that name, which one it became is a guess
+            self.calendar = self.native = named[0]
+            self.url = _url_of(named[0])
+            self.id = _name_of(self.url)
 
     async def _delete_collection(self) -> None:
         await self._backend._dav.delete_calendar(self.calendar)

src/calendaring/_async/collection.py

@@ -29,6 +29,7 @@
 from ..capabilities import BACKEND_FEATURES, COMPONENT_FEATURE, Capabilities, Feature, Support
 from ..errors import (
     AlreadyExistsError,
+    BackendError,
     CalendaringError,
     ConflictError,
     InvalidDataError,
@@ -1047,7 +1048,11 @@ async def move(
     async def move(
         self, item: Item, target: AsyncCollection, *, loss: LossPolicy | None = None, verify: bool | None = None
     ) -> AsyncItem:
-        """Move to ``target``; between backends, add there and then delete here (``EMULATED``, not atomic)."""
+        """Move to ``target``; between backends, add there and then delete here (``EMULATED``, not atomic).
+
+        Where a UID may be in one calendar only, the ``move`` capability's ``order`` says what to do instead:
+        ``delete-first`` (the copy would be refused), or ``add-may-move`` (the copy may move the original).
+        """
         with self._context():
             self._require_writable(item)
             self._require(Feature.MOVE)
@@ -1060,12 +1065,38 @@ async def move(
                     return target._one(stored, written=True)
             if item.etag is not None and (await self._fetch(item.native_id)).etag != item.etag:
                 raise ConflictError(f"{item.native_id!r} changed since it was read")
+            # Where a UID may be in one calendar only, the server refuses the copy, or moves the original.
+            order = self.capabilities.details(Feature.MOVE).get("order")
+            if order == "delete-first":
+                if item.is_occurrence:  # deleting it writes an EXDATE: the UID stays here, and the copy is refused
+                    raise UnsupportedError(Feature.MOVE, "an occurrence, where the copy must be made after the delete")
+                await self.delete(item)
+                try:
+                    return await target.add(item.as_data(), loss=loss, verify=verify)
+                except CalendaringError as exc:
+                    try:
+                        await self.add(item.as_data(), loss=loss, verify=False)
+                    except CalendaringError:
+                        exc.add_note(
+                            f"{item.uid!r} was deleted here and could not be put back: it is in neither collection"
+                        )
+                    raise
             moved = await target.add(item.as_data(), loss=loss, verify=verify)
+            if order == "add-may-move":
+                try:
+                    await target._fetch(moved.native_id)  # type: ignore[arg-type]  # add() returns a stored item
+                except NotFoundError:
+                    raise BackendError(f"{item.uid!r} was not stored in {target.id!r}; it is still here") from None
             try:
                 await self.delete(item)
+            except NotFoundError:
+                if order != "add-may-move":
+                    raise
+                # the copy moved it: Zimbra still answers a read of the old URL, so only the delete tells
             except ConflictError:
-                with suppress(CalendaringError):  # best effort: the conflict is what to report
-                    await target.delete(moved, overwrite=True)
+                if order != "add-may-move":  # there, the copy may be the only one
+                    with suppress(CalendaringError):  # best effort: the conflict is what to report
+                        await target.delete(moved, overwrite=True)
                 raise
             return moved
 

src/calendaring/_caldav_features.py

@@ -64,7 +64,6 @@
     Feature.SEARCH_TIME_RANGE: E,
     Feature.SEARCH_TEXT: E,
     Feature.CHANGES: {"level": E, "method": "etag-listing"},
-    Feature.MOVE: {"level": E, "note": "added to the target, then deleted here"},
     Feature.IDENTITY_FOREIGN_ID: {"level": U, "note": "the UID is native"},
     Feature.RECURRENCE_EDIT_OCCURRENCE: {"level": U, "note": EDIT_ICALENDAR_PENDING},
     Feature.RECURRENCE_EDIT_THIS_AND_FUTURE: U,
@@ -133,6 +132,19 @@ def _table(features: Any) -> dict[Feature, Any]:
     return table
 
 
+def _move(features: Any) -> dict[str, Any]:
+    """Copy, then delete; unless the server keeps a UID in one calendar only (``save.duplicate-uid.cross-calendar``)."""
+    hint = features.is_supported("save.duplicate-uid.cross-calendar", dict) if _hinted(features) else {}
+    cross = str(hint.get("support", "unknown"))
+    if cross in ("full", "unknown"):
+        return {"level": E, "note": "added to the target, then deleted here"}
+    if cross == "unsupported" and hint.get("behaviour") != "silently-ignored":  # Zimbra moves the original
+        return {"level": E, "order": "add-may-move", "note": "added to the target, then deleted here if still there"}
+    if _level(features, "save-load.reuse-deleted-uid") is U:
+        return {"level": U, "note": "the UID can be neither in two calendars at once nor reused once deleted"}
+    return {"level": E, "order": "delete-first", "note": "deleted here, then added to the target"}
+
+
 def _changes(features: Any) -> dict[str, Any]:
     """A native change feed where the server has a reliable one; else the etag listing, which always works.
 
@@ -181,6 +193,7 @@ def collection_capabilities(features: Any, components: frozenset[Component]) ->
     """The server's table, narrowed to what the calendar says it holds."""
     table = {f: v for f, v in _table(features).items() if f not in BACKEND_FEATURES}
     table[Feature.CHANGES] = _changes(features)
+    table[Feature.MOVE] = _move(features)
     table[Feature.SEARCH_TIME_RANGE] = _time_range(features)
     for kind, (feature, name) in COMPONENT_FROM_SERVER.items():
         if kind in components:

tests/_async/test_caldav_backend.py

@@ -103,6 +103,59 @@ async def make_calendar(name: str, components: list[str]) -> Any:
         assert asked == [["VEVENT"]]
 
 
+async def test_a_rename_that_moves_the_calendar_follows_it(
+    collection: AsyncCollection, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Zimbra names a calendar's URL after its display name, so a rename moves the calendar."""
+    backend = collection._backend
+    moved_to = await backend.create_collection("moved-to", components=(Component.TASK,))
+    dav = backend._dav  # type: ignore[attr-defined]
+    calendars, set_display_name = dav.calendars, dav.set_display_name
+
+    async def renamed_elsewhere(calendar: Any, name: str) -> None:
+        await set_display_name(moved_to.calendar, name)  # type: ignore[attr-defined]
+
+    old_url = collection.url  # type: ignore[attr-defined]
+
+    async def without_the_old_url() -> list[Any]:
+        return [c for c in await calendars() if str(c.url).rstrip("/") + "/" != old_url]
+
+    monkeypatch.setattr(dav, "set_display_name", renamed_elsewhere)
+    monkeypatch.setattr(dav, "calendars", without_the_old_url)
+    old = collection.id
+    await collection.set_name("Renamed")
+    assert collection.id == moved_to.id != old
+    assert (await backend.collection(collection.id)).name == "Renamed"
+    monkeypatch.undo()
+    await (await backend.collection(old)).delete_collection()  # still there in this simulation
+
+
+async def test_a_rename_does_not_guess_between_calendars_of_the_same_name(
+    collection: AsyncCollection, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    backend = collection._backend
+    made = [await backend.create_collection(name, components=(Component.TASK,)) for name in ("one", "two")]
+    dav = backend._dav  # type: ignore[attr-defined]
+    calendars, set_display_name = dav.calendars, dav.set_display_name
+    old_url = collection.url  # type: ignore[attr-defined]
+
+    async def renamed_twice(calendar: Any, name: str) -> None:
+        for other in made:
+            await set_display_name(other.calendar, name)  # type: ignore[attr-defined]
+
+    async def without_the_old_url() -> list[Any]:
+        return [c for c in await calendars() if str(c.url).rstrip("/") + "/" != old_url]
+
+    monkeypatch.setattr(dav, "set_display_name", renamed_twice)
+    monkeypatch.setattr(dav, "calendars", without_the_old_url)
+    old = collection.id
+    await collection.set_name("Renamed")
+    assert collection.id == old
+    monkeypatch.undo()
+    for other in made:
+        await other.delete_collection()
+
+
 async def test_wire_bytes_are_sent_as_given(collection: AsyncCollection, monkeypatch: pytest.MonkeyPatch) -> None:
     # icalendar would fold this line.  Xandikos refolds what it stores, so what is checked is the PUT.
     client = collection._backend._dav.client  # type: ignore[attr-defined]