diff --git a/docs/design/API_DESIGN.md b/docs/design/API_DESIGN.md index 970d763..df54a4a 100644 --- a/docs/design/API_DESIGN.md +++ b/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 diff --git a/src/calendaring/_async/caldav.py b/src/calendaring/_async/caldav.py index 5bdde04..6f0132d 100644 --- a/src/calendaring/_async/caldav.py +++ b/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) diff --git a/src/calendaring/_async/collection.py b/src/calendaring/_async/collection.py index e13681b..7be7bbc 100644 --- a/src/calendaring/_async/collection.py +++ b/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 diff --git a/src/calendaring/_caldav_features.py b/src/calendaring/_caldav_features.py index 71d2065..64f05b8 100644 --- a/src/calendaring/_caldav_features.py +++ b/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: diff --git a/src/calendaring/_sync/caldav.py b/src/calendaring/_sync/caldav.py index 3a2666f..99f9e6b 100644 --- a/src/calendaring/_sync/caldav.py +++ b/src/calendaring/_sync/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: CalDAVBackend, 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 @@ def _delete(self, native_id: str, *, etag: str | None) -> None: ) def _set_name(self, name: str) -> None: - self._backend._dav.set_display_name(self.calendar, name) + dav = self._backend._dav + dav.set_display_name(self.calendar, name) + # Zimbra names a calendar's URL after its display name, so a rename moves the calendar. + listed = dav.calendars() + if any(_url_of(c) == self.url for c in listed): + return + named = [c for c in listed if 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) def _delete_collection(self) -> None: self._backend._dav.delete_calendar(self.calendar) diff --git a/src/calendaring/_sync/collection.py b/src/calendaring/_sync/collection.py index e308840..a745dde 100644 --- a/src/calendaring/_sync/collection.py +++ b/src/calendaring/_sync/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 @@ def move( def move( self, item: Item, target: Collection, *, loss: LossPolicy | None = None, verify: bool | None = None ) -> SyncItem: - """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 @@ def move( return target._one(stored, written=True) if item.etag is not None and (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") + self.delete(item) + try: + return target.add(item.as_data(), loss=loss, verify=verify) + except CalendaringError as exc: + try: + 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 = target.add(item.as_data(), loss=loss, verify=verify) + if order == "add-may-move": + try: + 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: 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 - 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 + target.delete(moved, overwrite=True) raise return moved diff --git a/tests/_async/test_caldav_backend.py b/tests/_async/test_caldav_backend.py index d15aa26..56a4153 100644 --- a/tests/_async/test_caldav_backend.py +++ b/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] diff --git a/tests/_async/test_generic.py b/tests/_async/test_generic.py index c55d93e..4f93fc3 100644 --- a/tests/_async/test_generic.py +++ b/tests/_async/test_generic.py @@ -13,6 +13,7 @@ from icalendar_searcher import Searcher from calendaring import ( + AlreadyExistsError, AsyncBackend, AsyncCollection, AsyncTask, @@ -36,7 +37,7 @@ VerificationError, ) from calendaring.loss import WARN -from calendaring.memory import TRACKER, AsyncMemoryBackend, AsyncMemoryCollection, MemoryObject +from calendaring.memory import FULL, TRACKER, AsyncMemoryBackend, AsyncMemoryCollection, MemoryObject, Profile from calendaring.types import StoredObject @@ -236,6 +237,167 @@ class Backend(AsyncMemoryBackend): assert await target.search() == [] +def moving(order: str) -> Profile: + """A backend whose move needs ``order``: what the CalDAV backend declares from a server's hints.""" + return Profile( + f"move-{order}", + {**FULL.capabilities, Feature.MOVE: {"level": Support.EMULATED, "order": order}}, + FULL.components, + ) + + +def _uid(calendar: icalendar.Calendar) -> str: + return str(next(c for c in calendar.subcomponents if "UID" in c)["UID"]) + + +class OneUidPerAccount(AsyncMemoryCollection): + """Refuses a UID another calendar of the account holds, as CCS, Cyrus and Ox do.""" + + async def _create(self, calendar: icalendar.Calendar, *, native_id: str | None, wire: bytes | None) -> StoredObject: + for other in self._backend.store.calendars.values(): + if other is not self.store and any(o.uid == _uid(calendar) for o in other.objects.values()): + raise AlreadyExistsError("the UID is in another calendar", taken="uid") + return await super()._create(calendar, native_id=native_id, wire=wire) + + +class CopyIsMove(AsyncMemoryCollection): + """Saving a UID another calendar holds moves it here, as Zimbra does.""" + + async def _create(self, calendar: icalendar.Calendar, *, native_id: str | None, wire: bytes | None) -> StoredObject: + for other in self._backend.store.calendars.values(): + for obj in [o for o in other.objects.values() if o.uid == _uid(calendar)]: + del other.objects[obj.native_id] + return await super()._create(calendar, native_id=native_id, wire=wire) + + +@pytest.mark.parametrize( + ("collection_class", "order"), [(OneUidPerAccount, "delete-first"), (CopyIsMove, "add-may-move")] +) +async def test_a_move_where_a_uid_cannot_be_in_two_calendars( + collection_class: type[AsyncMemoryCollection], order: str +) -> None: + backend = backend_of(collection_class, order) + source = await backend.collection("default") + target = await backend.create_collection("target", components=[Component.TASK]) + task = await source.add_task(summary="renew the passport") + moved = await source.move(task, target) + assert moved.collection is target + assert await source.search() == [] + assert [t.uid for t in await target.tasks()] == [task.uid] + + +async def test_a_move_that_deleted_first_puts_the_item_back_when_the_copy_fails() -> None: + class Refusing(AsyncMemoryCollection): + async def _create( + self, calendar: icalendar.Calendar, *, native_id: str | None, wire: bytes | None + ) -> StoredObject: + if self.id == "target": + raise BackendError("refused") + return await super()._create(calendar, native_id=native_id, wire=wire) + + class Backend(AsyncMemoryBackend): + collection_class = Refusing + + backend = Backend(moving("delete-first")) + source = await backend.collection("default") + target = await backend.create_collection("target", components=[Component.TASK]) + task = await source.add_task(summary="renew the passport") + with pytest.raises(BackendError): + await source.move(task, target) + assert [t.uid for t in await source.tasks()] == [task.uid] + + +def backend_of(collection_class: type[AsyncMemoryCollection], order: str) -> AsyncMemoryBackend: + class Backend(AsyncMemoryBackend): + pass + + Backend.collection_class = collection_class + return Backend(moving(order)) + + +class DropsTheCopy(AsyncMemoryCollection): + """Answers a save of a UID another calendar holds as stored, and drops it (`silently-ignored`).""" + + async def _create(self, calendar: icalendar.Calendar, *, native_id: str | None, wire: bytes | None) -> StoredObject: + stored = await super()._create(calendar, native_id=native_id, wire=wire) + if any( + o.uid == _uid(calendar) + for c in self._backend.store.calendars.values() + if c is not self.store + for o in c.objects.values() + ): + del self.store.objects[stored.native_id] + return stored + + +async def test_a_dropped_copy_leaves_the_original_where_it_was() -> None: + backend = backend_of(DropsTheCopy, "add-may-move") + source = await backend.collection("default") + target = await backend.create_collection("target", components=[Component.TASK]) + task = await source.add_task(summary="renew the passport") + with pytest.raises(BackendError): + await source.move(task, target) + assert [t.uid for t in await source.tasks()] == [task.uid] + + +class CopiesThenConflicts(AsyncMemoryCollection): + """Keeps the copy, then refuses the delete of the original with a conflict.""" + + async def _delete(self, native_id: str, *, etag: str | None) -> None: + if self.id != "default": + return await super()._delete(native_id, etag=etag) + raise ConflictError(f"{native_id!r} changed since it was read") + + +async def test_a_conflict_after_a_copy_that_may_move_keeps_the_copy() -> None: + backend = backend_of(CopiesThenConflicts, "add-may-move") + source = await backend.collection("default") + target = await backend.create_collection("target", components=[Component.TASK]) + task = await source.add_task(summary="renew the passport") + with pytest.raises(ConflictError): + await source.move(task, target) + assert [t.uid for t in await target.tasks()] == [task.uid], "the copy may be the only one" + + +async def test_a_lost_item_is_reported_when_neither_copy_nor_restore_works() -> None: + class RefusesEverything(AsyncMemoryCollection): + refusing: ClassVar[bool] = False + + async def _create( + self, calendar: icalendar.Calendar, *, native_id: str | None, wire: bytes | None + ) -> StoredObject: + if self.refusing: + raise BackendError("refused") + return await super()._create(calendar, native_id=native_id, wire=wire) + + backend = backend_of(RefusesEverything, "delete-first") + source = await backend.collection("default") + target = await backend.create_collection("target", components=[Component.TASK]) + task = await source.add_task(summary="renew the passport") + RefusesEverything.refusing = True + with pytest.raises(BackendError) as info: + await source.move(task, target) + assert any(task.uid in note and "neither" in note for note in info.value.__notes__) + + +async def test_an_occurrence_is_not_moved_by_deleting_first() -> None: + backend = backend_of(OneUidPerAccount, "delete-first") + source = await backend.collection("default") + target = await backend.create_collection("target", components=[Component.EVENT]) + standup = Event.new( + summary="stand-up", start=datetime(2026, 11, 2, 9, tzinfo=UTC), end=datetime(2026, 11, 2, 10, tzinfo=UTC) + ) + standup.component.add("RRULE", {"FREQ": "DAILY", "COUNT": 3}) + await source.add(standup) + [occurrence, *_] = await source.events( + start=datetime(2026, 11, 2, tzinfo=UTC), end=datetime(2026, 11, 5, tzinfo=UTC), expand=True + ) + with pytest.raises(UnsupportedError): + await source.move(occurrence, target) + assert len(await source.events()) == 1 + assert await target.events() == [] + + async def test_a_token_from_another_backend_is_refused() -> None: work = await AsyncMemoryBackend(id="work").collection("default") home = await AsyncMemoryBackend(id="home").collection("default") diff --git a/tests/_sync/test_caldav_backend.py b/tests/_sync/test_caldav_backend.py index cdaae25..a77b78a 100644 --- a/tests/_sync/test_caldav_backend.py +++ b/tests/_sync/test_caldav_backend.py @@ -103,6 +103,59 @@ def make_calendar(name: str, components: list[str]) -> Any: assert asked == [["VEVENT"]] +def test_a_rename_that_moves_the_calendar_follows_it( + collection: Collection, 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 = 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 + + def renamed_elsewhere(calendar: Any, name: str) -> None: + set_display_name(moved_to.calendar, name) # type: ignore[attr-defined] + + old_url = collection.url # type: ignore[attr-defined] + + def without_the_old_url() -> list[Any]: + return [c for c in 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 + collection.set_name("Renamed") + assert collection.id == moved_to.id != old + assert (backend.collection(collection.id)).name == "Renamed" + monkeypatch.undo() + (backend.collection(old)).delete_collection() # still there in this simulation + + +def test_a_rename_does_not_guess_between_calendars_of_the_same_name( + collection: Collection, monkeypatch: pytest.MonkeyPatch +) -> None: + backend = collection._backend + made = [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] + + def renamed_twice(calendar: Any, name: str) -> None: + for other in made: + set_display_name(other.calendar, name) # type: ignore[attr-defined] + + def without_the_old_url() -> list[Any]: + return [c for c in 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 + collection.set_name("Renamed") + assert collection.id == old + monkeypatch.undo() + for other in made: + other.delete_collection() + + def test_wire_bytes_are_sent_as_given(collection: Collection, 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] diff --git a/tests/_sync/test_generic.py b/tests/_sync/test_generic.py index a9a204b..60a1c6e 100644 --- a/tests/_sync/test_generic.py +++ b/tests/_sync/test_generic.py @@ -13,6 +13,7 @@ from icalendar_searcher import Searcher from calendaring import ( + AlreadyExistsError, Backend, Collection, SyncTask, @@ -36,7 +37,7 @@ VerificationError, ) from calendaring.loss import WARN -from calendaring.memory import TRACKER, MemoryBackend, MemoryCollection, MemoryObject +from calendaring.memory import FULL, TRACKER, MemoryBackend, MemoryCollection, MemoryObject, Profile from calendaring.types import StoredObject @@ -236,6 +237,167 @@ class Backend(MemoryBackend): assert target.search() == [] +def moving(order: str) -> Profile: + """A backend whose move needs ``order``: what the CalDAV backend declares from a server's hints.""" + return Profile( + f"move-{order}", + {**FULL.capabilities, Feature.MOVE: {"level": Support.EMULATED, "order": order}}, + FULL.components, + ) + + +def _uid(calendar: icalendar.Calendar) -> str: + return str(next(c for c in calendar.subcomponents if "UID" in c)["UID"]) + + +class OneUidPerAccount(MemoryCollection): + """Refuses a UID another calendar of the account holds, as CCS, Cyrus and Ox do.""" + + def _create(self, calendar: icalendar.Calendar, *, native_id: str | None, wire: bytes | None) -> StoredObject: + for other in self._backend.store.calendars.values(): + if other is not self.store and any(o.uid == _uid(calendar) for o in other.objects.values()): + raise AlreadyExistsError("the UID is in another calendar", taken="uid") + return super()._create(calendar, native_id=native_id, wire=wire) + + +class CopyIsMove(MemoryCollection): + """Saving a UID another calendar holds moves it here, as Zimbra does.""" + + def _create(self, calendar: icalendar.Calendar, *, native_id: str | None, wire: bytes | None) -> StoredObject: + for other in self._backend.store.calendars.values(): + for obj in [o for o in other.objects.values() if o.uid == _uid(calendar)]: + del other.objects[obj.native_id] + return super()._create(calendar, native_id=native_id, wire=wire) + + +@pytest.mark.parametrize( + ("collection_class", "order"), [(OneUidPerAccount, "delete-first"), (CopyIsMove, "add-may-move")] +) +def test_a_move_where_a_uid_cannot_be_in_two_calendars( + collection_class: type[MemoryCollection], order: str +) -> None: + backend = backend_of(collection_class, order) + source = backend.collection("default") + target = backend.create_collection("target", components=[Component.TASK]) + task = source.add_task(summary="renew the passport") + moved = source.move(task, target) + assert moved.collection is target + assert source.search() == [] + assert [t.uid for t in target.tasks()] == [task.uid] + + +def test_a_move_that_deleted_first_puts_the_item_back_when_the_copy_fails() -> None: + class Refusing(MemoryCollection): + def _create( + self, calendar: icalendar.Calendar, *, native_id: str | None, wire: bytes | None + ) -> StoredObject: + if self.id == "target": + raise BackendError("refused") + return super()._create(calendar, native_id=native_id, wire=wire) + + class Backend(MemoryBackend): + collection_class = Refusing + + backend = Backend(moving("delete-first")) + source = backend.collection("default") + target = backend.create_collection("target", components=[Component.TASK]) + task = source.add_task(summary="renew the passport") + with pytest.raises(BackendError): + source.move(task, target) + assert [t.uid for t in source.tasks()] == [task.uid] + + +def backend_of(collection_class: type[MemoryCollection], order: str) -> MemoryBackend: + class Backend(MemoryBackend): + pass + + Backend.collection_class = collection_class + return Backend(moving(order)) + + +class DropsTheCopy(MemoryCollection): + """Answers a save of a UID another calendar holds as stored, and drops it (`silently-ignored`).""" + + def _create(self, calendar: icalendar.Calendar, *, native_id: str | None, wire: bytes | None) -> StoredObject: + stored = super()._create(calendar, native_id=native_id, wire=wire) + if any( + o.uid == _uid(calendar) + for c in self._backend.store.calendars.values() + if c is not self.store + for o in c.objects.values() + ): + del self.store.objects[stored.native_id] + return stored + + +def test_a_dropped_copy_leaves_the_original_where_it_was() -> None: + backend = backend_of(DropsTheCopy, "add-may-move") + source = backend.collection("default") + target = backend.create_collection("target", components=[Component.TASK]) + task = source.add_task(summary="renew the passport") + with pytest.raises(BackendError): + source.move(task, target) + assert [t.uid for t in source.tasks()] == [task.uid] + + +class CopiesThenConflicts(MemoryCollection): + """Keeps the copy, then refuses the delete of the original with a conflict.""" + + def _delete(self, native_id: str, *, etag: str | None) -> None: + if self.id != "default": + return super()._delete(native_id, etag=etag) + raise ConflictError(f"{native_id!r} changed since it was read") + + +def test_a_conflict_after_a_copy_that_may_move_keeps_the_copy() -> None: + backend = backend_of(CopiesThenConflicts, "add-may-move") + source = backend.collection("default") + target = backend.create_collection("target", components=[Component.TASK]) + task = source.add_task(summary="renew the passport") + with pytest.raises(ConflictError): + source.move(task, target) + assert [t.uid for t in target.tasks()] == [task.uid], "the copy may be the only one" + + +def test_a_lost_item_is_reported_when_neither_copy_nor_restore_works() -> None: + class RefusesEverything(MemoryCollection): + refusing: ClassVar[bool] = False + + def _create( + self, calendar: icalendar.Calendar, *, native_id: str | None, wire: bytes | None + ) -> StoredObject: + if self.refusing: + raise BackendError("refused") + return super()._create(calendar, native_id=native_id, wire=wire) + + backend = backend_of(RefusesEverything, "delete-first") + source = backend.collection("default") + target = backend.create_collection("target", components=[Component.TASK]) + task = source.add_task(summary="renew the passport") + RefusesEverything.refusing = True + with pytest.raises(BackendError) as info: + source.move(task, target) + assert any(task.uid in note and "neither" in note for note in info.value.__notes__) + + +def test_an_occurrence_is_not_moved_by_deleting_first() -> None: + backend = backend_of(OneUidPerAccount, "delete-first") + source = backend.collection("default") + target = backend.create_collection("target", components=[Component.EVENT]) + standup = Event.new( + summary="stand-up", start=datetime(2026, 11, 2, 9, tzinfo=UTC), end=datetime(2026, 11, 2, 10, tzinfo=UTC) + ) + standup.component.add("RRULE", {"FREQ": "DAILY", "COUNT": 3}) + source.add(standup) + [occurrence, *_] = source.events( + start=datetime(2026, 11, 2, tzinfo=UTC), end=datetime(2026, 11, 5, tzinfo=UTC), expand=True + ) + with pytest.raises(UnsupportedError): + source.move(occurrence, target) + assert len(source.events()) == 1 + assert target.events() == [] + + def test_a_token_from_another_backend_is_refused() -> None: work = MemoryBackend(id="work").collection("default") home = MemoryBackend(id="home").collection("default") diff --git a/tests/test_caldav_features.py b/tests/test_caldav_features.py index a4f9925..0b76224 100644 --- a/tests/test_caldav_features.py +++ b/tests/test_caldav_features.py @@ -161,3 +161,47 @@ def test_a_calendar_is_made_lossily_where_the_server_cannot_store_a_component() capability = backend_capabilities(server)[Feature.CREATE_COLLECTION] assert capability.level is Support.LOSSY assert "VJOURNAL" in (capability.note or "") + + +def move_of(**hints: str) -> Any: + return collection_capabilities(hinted(save_load="full", **hints), ALL)[Feature.MOVE] + + +def test_a_move_copies_then_deletes_where_a_uid_may_be_in_two_calendars() -> None: + move = move_of(**{"save.duplicate-uid.cross-calendar": "full"}) + assert move.level is Support.EMULATED + assert "order" not in move.details + + +def test_a_move_deletes_first_where_the_copy_would_be_refused() -> None: + # CCS, Cyrus, Ox: the same UID in a second calendar is an error + move = move_of(**{"save.duplicate-uid.cross-calendar": "ungraceful"}) + assert move.level is Support.EMULATED + assert move.details["order"] == "delete-first" + + +def test_a_move_checks_what_the_copy_did_where_the_server_moves_or_ignores_it() -> None: + # Zimbra moves the original when the copy is saved + move = move_of(**{"save.duplicate-uid.cross-calendar": "unsupported"}) + assert move.level is Support.EMULATED + assert move.details["order"] == "add-may-move" + + +def test_a_move_deletes_first_where_the_copy_would_be_dropped() -> None: + server = features( + { + "save-load": {"support": "full"}, + "save.duplicate-uid.cross-calendar": {"support": "unsupported", "behaviour": "silently-ignored"}, + } + ) + assert collection_capabilities(server, ALL)[Feature.MOVE].details["order"] == "delete-first" + + +def test_a_move_is_impossible_where_neither_a_copy_nor_a_reused_uid_is_allowed() -> None: + hints = {"save.duplicate-uid.cross-calendar": "ungraceful", "save-load.reuse-deleted-uid": "unsupported"} + assert move_of(**hints).level is Support.UNSUPPORTED + + +def test_a_copy_that_moves_needs_no_reused_uid() -> None: + move = move_of(**{"save.duplicate-uid.cross-calendar": "unsupported", "save-load.reuse-deleted-uid": "unsupported"}) + assert move.details["order"] == "add-may-move"