diff --git a/docs/how-to/convert.rst b/docs/how-to/convert.rst index 560b919..7fd3022 100644 --- a/docs/how-to/convert.rst +++ b/docs/how-to/convert.rst @@ -5,7 +5,7 @@ Convert iCalendar and JSCalendar ================================ -``ical_to_jscal`` and ``jscal_to_ical`` are the same conversion functions the client uses internally, available directly if you want to convert between formats without making any JMAP request. +``ical_to_jscal`` and ``jscal_to_ical`` are the same conversion functions :meth:`~calendaring_jmap.objects.calendar.JMAPCalendar.add_event` and :meth:`~calendaring_jmap.objects.calendar_object.JMAPCalendarObject.save` use internally, available directly if you want to convert between formats without making any JMAP request, or if you're calling :meth:`~calendaring_jmap.client.JMAPClient.create_event`/:meth:`~calendaring_jmap.client.JMAPClient.update_event` directly and starting from iCalendar rather than a JSCalendar dict. .. code-block:: python @@ -14,10 +14,10 @@ Convert iCalendar and JSCalendar jscal = ical_to_jscal(ical_str) ical_str_again = jscal_to_ical(jscal) -``ical_to_jscal`` processes the first VEVENT in the string; any sibling VEVENTs sharing the same UID with a RECURRENCE-ID are folded into the result's ``recurrenceOverrides`` map, and EXDATE lines become excluded override entries. Pass ``calendar_id`` if the result is going to a real ``CalendarEvent/set`` call, since the server needs ``calendarIds`` set on it: +``ical_to_jscal`` processes the first VEVENT in the string; any sibling VEVENTs sharing the same UID with a RECURRENCE-ID are folded into the result's ``recurrenceOverrides`` map, and EXDATE lines become excluded override entries. Pass ``calendar_id`` if you need the result's own ``calendarIds`` set and aren't passing it straight to ``create_event``/``send_invite``, which set ``calendarIds`` themselves from their own ``calendar_id`` argument regardless of what the dict already has: .. code-block:: python jscal = ical_to_jscal(ical_str, calendar_id=calendar_id) -Both functions raise ``ValueError`` on malformed input, the same errors :doc:`errors` describes for ``create_event`` and ``update_event``, since those methods call ``ical_to_jscal`` internally. See :doc:`../explanation/design` for the conversion layer's own design decisions. +Both functions raise ``ValueError`` on malformed input. ``add_event``/``save`` call ``ical_to_jscal``/``jscal_to_ical`` internally and surface the same errors; ``create_event``/``send_invite``/``update_event`` take a JSCalendar dict directly and raise nothing from this layer, since no conversion happens inside them. See :doc:`../explanation/design` for the conversion layer's own design decisions. diff --git a/docs/how-to/errors.rst b/docs/how-to/errors.rst index f5cc6f5..13a78d8 100644 --- a/docs/how-to/errors.rst +++ b/docs/how-to/errors.rst @@ -20,7 +20,7 @@ Catch specific error types ) try: - event_id = client.create_event(calendar_id, ical) + event_id, old_state, new_state = client.create_event(calendar_id, event) except JMAPAuthError: print("Authentication failed (401/403)") except JMAPCapabilityError: @@ -35,12 +35,12 @@ See :doc:`../reference/errors` for what each error class means and the full list Malformed iCalendar input ========================= -``create_event``, ``send_invite``, ``update_event``, and the calendar/object convenience methods that call them (``JMAPCalendar.add_event``, ``JMAPCalendarObject.save``) convert your iCalendar string to JSCalendar before sending it. If it's missing a mandatory property (``UID``, ``DTSTART``) or has a negative duration, they raise a plain ``ValueError``, not a ``JMAPError`` subclass. This happens client-side, before any request reaches the server. Catch it if you're constructing iCalendar strings by hand rather than passing through data a server already returned to you: +``create_event``, ``send_invite``, and ``update_event`` take a JSCalendar dict directly and raise nothing client-side on a malformed one; whether a missing required property comes back as the server's own ``invalidProperties`` (surfaced as :class:`~calendaring_jmap.error.JMAPMethodError`) depends on the server and the property. Confirmed live that Cyrus rejects a create missing ``uid``/``start`` with ``invalidProperties``, while Stalwart accepts it, generating its own ``uid`` and leaving the event with no ``start`` at all rather than rejecting the request. The ``ValueError`` case below applies to ``ical_to_jscal`` itself, and so to ``JMAPCalendar.add_event``/``JMAPCalendarObject.save``, which call it on your behalf: if the iCalendar string is missing a mandatory property (``UID``, ``DTSTART``) or has a negative duration, conversion raises a plain ``ValueError`` before any request reaches the server. Catch it if you're constructing iCalendar strings by hand rather than passing through data a server already returned to you: .. code-block:: python try: - event_id = client.create_event(calendar_id, ical) + event_id = cal.add_event(ical) except ValueError as e: print(f"Malformed iCalendar: {e}") except JMAPMethodError as e: diff --git a/docs/how-to/events.rst b/docs/how-to/events.rst index 7254de8..77fda99 100644 --- a/docs/how-to/events.rst +++ b/docs/how-to/events.rst @@ -5,7 +5,7 @@ Work with events ================ -Events are passed as iCalendar strings, so existing iCalendar-producing code works unchanged. +:meth:`~calendaring_jmap.objects.calendar.JMAPCalendar.add_event` and :meth:`~calendaring_jmap.objects.calendar_object.JMAPCalendarObject.save` take and produce iCalendar strings, so existing iCalendar-producing code works unchanged through them. The lower-level client methods (:meth:`~calendaring_jmap.client.JMAPClient.create_event`, :meth:`~calendaring_jmap.client.JMAPClient.send_invite`, :meth:`~calendaring_jmap.client.JMAPClient.update_event`) take a JSCalendar dict directly instead, the same shape :meth:`~calendaring_jmap.client.JMAPClient.get_event` hands back; convert with ``ical_to_jscal`` first if you're starting from iCalendar. Add an event ============ @@ -61,11 +61,11 @@ Edit the object in place, then call :meth:`~calendaring_jmap.objects.calendar_ob ``save()`` needs an object with its ``parent`` calendar set, since it uses that calendar's client to send the update. Objects from ``cal.get_object_by_uid`` and :meth:`~calendaring_jmap.objects.calendar.JMAPCalendar.search` have it set; objects from :meth:`~calendaring_jmap.client.JMAPClient.get_event` don't, since they aren't fetched through a calendar. It's also sync-only; on an async-backed calendar, call :meth:`~calendaring_jmap.async_client.AsyncJMAPClient.update_event` directly instead. -If you already have a full replacement iCalendar string, ``update_event`` also works directly on the client: +If you already have a full replacement iCalendar string, ``update_event`` also works directly on the client, once converted: .. code-block:: python - client.update_event(event_id, updated_ical_str) + client.update_event(event_id, ical_to_jscal(updated_ical_str)) Delete an event =============== diff --git a/docs/how-to/scheduling.rst b/docs/how-to/scheduling.rst index 0ebca93..ea0ee95 100644 --- a/docs/how-to/scheduling.rst +++ b/docs/how-to/scheduling.rst @@ -10,9 +10,9 @@ Send an invitation .. code-block:: python - event_id = client.send_invite(calendar_id, ical_str) + event_id, old_state, new_state = client.send_invite(calendar_id, event) -Identical to :meth:`~calendaring_jmap.client.JMAPClient.create_event`, except the server dispatches iTIP invitations to the event's participants (ATTENDEE/ORGANIZER properties in ``ical_str``). Use :meth:`~calendaring_jmap.client.JMAPClient.create_event` itself for an event with no participants to notify. +Identical to :meth:`~calendaring_jmap.client.JMAPClient.create_event`, except the server dispatches iTIP invitations to the event's participants (ATTENDEE/ORGANIZER properties, as a JSCalendar ``participants`` map). Use :meth:`~calendaring_jmap.client.JMAPClient.create_event` itself for an event with no participants to notify. Respond to an invitation ======================== diff --git a/news/67.breaking.1.rst b/news/67.breaking.1.rst new file mode 100644 index 0000000..f021425 --- /dev/null +++ b/news/67.breaking.1.rst @@ -0,0 +1 @@ +``create_event``, ``send_invite``, and ``update_event`` now take a JSCalendar dict instead of an iCalendar string. ``JMAPCalendar.add_event`` and ``JMAPCalendarObject.save`` are unaffected: they still take and produce iCalendar strings. diff --git a/news/67.breaking.2.rst b/news/67.breaking.2.rst new file mode 100644 index 0000000..dbc21b9 --- /dev/null +++ b/news/67.breaking.2.rst @@ -0,0 +1 @@ +``create_event`` and ``send_invite`` now return a 3-tuple ``(event_id, old_state, new_state)`` instead of just the event id. diff --git a/news/67.breaking.3.rst b/news/67.breaking.3.rst new file mode 100644 index 0000000..e8aee14 --- /dev/null +++ b/news/67.breaking.3.rst @@ -0,0 +1 @@ +``update_event`` now returns ``(old_state, new_state)`` instead of ``None``. diff --git a/src/calendaring_jmap/async_client.py b/src/calendaring_jmap/async_client.py index 872b257..dd3fb37 100644 --- a/src/calendaring_jmap/async_client.py +++ b/src/calendaring_jmap/async_client.py @@ -76,7 +76,6 @@ PARTICIPATION_STATUS_DECLINED, PARTICIPATION_STATUS_TENTATIVE, ) -from calendaring_jmap.convert import ical_to_jscal from calendaring_jmap.error import ( _DEFAULT_ERROR_TYPE, JMAPAuthError, @@ -439,13 +438,14 @@ async def set_default_alerts( async def _create_event_impl( self, calendar_id: str, - ical_str: str, + event: dict, account_id: str | None, send_scheduling_messages: bool, if_in_state: str | None = None, - ) -> str: + ) -> tuple[str, str, str]: session = await self._get_session() - jscal = ical_to_jscal(ical_str, calendar_id=calendar_id) + jscal = self._strip_server_managed_event_properties(event) + jscal["calendarIds"] = {calendar_id: True} target_account = self._resolve_account(session, account_id) call = build_event_set_create( target_account, @@ -454,22 +454,30 @@ async def _create_event_impl( if_in_state=if_in_state, ) responses = await self._request([call]) - return self._parse_create_response( + event_id = self._parse_create_response( responses, session.api_url, "CalendarEvent/set", parse_event_set ) + old_state, new_state = self._parse_set_state(responses, "CalendarEvent/set") + return event_id, old_state, new_state async def create_event( self, calendar_id: str, - ical_str: str, + event: dict, account_id: str | None = None, if_in_state: str | None = None, - ) -> str: - """Create a calendar event from an iCalendar string. + ) -> tuple[str, str, str]: + """Create a calendar event from a JSCalendar dict. Args: calendar_id: The JMAP calendar ID to create the event in. - ical_str: A VCALENDAR string representing the event. + event: A JSCalendar CalendarEvent dict (:rfc:`8984#section-2.1`), + e.g. one returned by :meth:`get_event`. Not mutated: a copy is made + before ``id``, ``baseEventId``, and ``isOrigin`` are stripped, + since draft-ietf-jmap-calendars documents all three as + server-set and immutable, and ``calendarIds`` is overwritten + with ``calendar_id``, replacing whatever calendar membership + ``event`` already had. account_id: The account owning ``calendar_id``; pass the owner's account here to add an event to a calendar shared with you (see ``get_calendars(account_id=...)``). @@ -482,17 +490,19 @@ async def create_event( unconditionally. Returns: - The server-assigned JMAP event ID. + A 3-tuple ``(event_id, old_state, new_state)``: the server-assigned + JMAP event ID, and the account's state immediately before and + after this write (:rfc:`8620#section-5.3`), confirmed live + present on every ``CalendarEvent/set`` response on both Cyrus + and Stalwart. Pass ``new_state`` as a later call's + ``if_in_state`` to require that no other write has landed since. Raises: JMAPMethodError: If the server rejects the create request. - ValueError: If ``ical_str`` is malformed in a way ``ical_to_jscal`` - rejects (missing ``UID``/``DTSTART``, a negative duration, - mismatched ``DTSTART``/``DTEND`` value types). """ return await self._create_event_impl( calendar_id, - ical_str, + event, account_id, send_scheduling_messages=False, if_in_state=if_in_state, @@ -501,37 +511,36 @@ async def create_event( async def send_invite( self, calendar_id: str, - ical_str: str, + event: dict, account_id: str | None = None, if_in_state: str | None = None, - ) -> str: + ) -> tuple[str, str, str]: """Create a calendar event and notify its participants. Identical to :meth:`create_event`, except the server actually - dispatches iTIP invitations to the event's participants (an iCalendar - string with ATTENDEE/ORGANIZER properties). ``create_event`` itself - never sends scheduling messages; use this instead when the event has + dispatches iTIP invitations to the event's participants (a JSCalendar + dict with ``participants`` set). ``create_event`` itself never sends + scheduling messages; use this instead when the event has participants who need to be notified. Args: calendar_id: The JMAP calendar ID to create the event in. - ical_str: A VCALENDAR string representing the event. + event: A JSCalendar CalendarEvent dict. See :meth:`create_event`. account_id: The account owning ``calendar_id``, same as :meth:`create_event`. if_in_state: See :meth:`create_event`. Returns: - The server-assigned JMAP event ID. + See :meth:`create_event`. Raises: JMAPMethodError: If the server rejects the create request, or with ``error_type == "noSupportedScheduleMethods"`` if a participant has no usable delivery method. - ValueError: Same conditions as :meth:`create_event`. """ return await self._create_event_impl( calendar_id, - ical_str, + event, account_id, send_scheduling_messages=True, if_in_state=if_in_state, @@ -722,15 +731,18 @@ async def tentatively_accept( async def update_event( self, event_id: str, - ical_str: str, + event: dict, account_id: str | None = None, if_in_state: str | None = None, - ) -> None: - """Update a calendar event from an iCalendar string. + ) -> tuple[str, str]: + """Update a calendar event from a JSCalendar dict. Args: event_id: The JMAP event ID to update. - ical_str: A VCALENDAR string with the updated event data. + event: A JSCalendar CalendarEvent dict with the updated event + data. See :meth:`~calendaring_jmap.client.JMAPClient.update_event` + for how absent properties are cleared, and for how + server-managed properties and ``calendarIds`` are handled. account_id: The account owning ``event_id``; pass the owner's account here to update an event on a calendar shared with you. @@ -744,15 +756,16 @@ async def update_event( null-key-dropping loop below, since those retries narrow the same patch rather than re-reading the account's state. + Returns: + A 2-tuple ``(old_state, new_state)``. See + :meth:`~calendaring_jmap.client.JMAPClient.update_event`. + Raises: JMAPMethodError: If the server rejects the update. - ValueError: If ``ical_str`` is malformed in a way ``ical_to_jscal`` - rejects (missing ``UID``/``DTSTART``, a negative duration, - mismatched ``DTSTART``/``DTEND`` value types). """ session = await self._get_session() target_account = self._resolve_account(session, account_id) - patch, nulled = self._build_event_update_patch(ical_str) + patch, nulled = self._build_event_update_patch(event) while True: responses = await self._request( [build_event_set_update(target_account, {event_id: patch}, if_in_state=if_in_state)] @@ -765,6 +778,7 @@ async def update_event( self._parse_update_response( responses, session.api_url, "CalendarEvent/set", parse_event_set, event_id ) + return self._parse_set_state(responses, "CalendarEvent/set") async def _search( self, @@ -1062,10 +1076,17 @@ async def get_objects_by_sync_token( server. Compare this against ``sync_token`` to detect a server that silently accepted an invalid or stale state instead of reporting an error (:rfc:`8620#section-5.2` does not require a - server to validate ``sinceState`` against a real prior state; - confirmed live that Cyrus echoes back whatever value it was - given, including one that was never actually issued, rather - than rejecting it). + server to validate ``sinceState`` against a real prior state). + Confirmed live that neither server validates consistently: + Cyrus rejects a non-numeric or empty ``sinceState`` with + ``cannotCalculateChanges``, but silently accepts and echoes + back any numeric-looking string, including one that was never + actually issued. Stalwart rejects nearly everything invalid + with ``invalidArguments``, except a string starting with the + literal character ``n``, which it accepts and normalizes + ``oldState`` down to just ``"n"`` regardless of what follows, + a quirk of its own internal state encoding rather than general + leniency. - ``new_state``: Pass to the next call to this method as ``sync_token``, whether or not ``has_more_changes`` is set. Named to match ``oldState``/``newState`` as a pair diff --git a/src/calendaring_jmap/client.py b/src/calendaring_jmap/client.py index e717e94..0cd7a28 100644 --- a/src/calendaring_jmap/client.py +++ b/src/calendaring_jmap/client.py @@ -83,7 +83,6 @@ TASK_CAPABILITY, UTC_DATETIME_FORMAT, ) -from calendaring_jmap.convert import ical_to_jscal from calendaring_jmap.convert._patch import _NULL_FOR_UPDATE from calendaring_jmap.convert._utils import _duration_to_timedelta from calendaring_jmap.error import ( @@ -456,16 +455,40 @@ def _busy_intervals_from_events(cls, events: list[dict]) -> list[BusyInterval]: return intervals @staticmethod - def _build_event_update_patch(ical_str: str) -> tuple[dict, frozenset[str]]: + def _strip_server_managed_event_properties(event: dict) -> dict: + """Return a shallow copy of ``event`` without server-managed properties. + + ``id`` and ``baseEventId`` are documented "immutable; server-set" in + draft-ietf-jmap-calendars' CalendarEvent property list; ``isOrigin`` + is documented "server-set" there too. Confirmed live that Stalwart + rejects a ``CalendarEvent/set`` create or update carrying any of + these with ``invalidProperties``/``"This property is immutable."``, + while Cyrus silently tolerates them; stripping unconditionally + avoids depending on which server a caller happens to be running + against. ``created``/``updated``/``sequence`` need no stripping: + confirmed live both servers silently overwrite them with their own + values regardless of what the client sends. + + Never mutates ``event`` itself: a caller passing in + :attr:`~calendaring_jmap.objects.calendar_object.JMAPCalendarObject.data` + should not see it altered by a write call made from it. + """ + copy = dict(event) + for key in ("id", "baseEventId", "isOrigin"): + copy.pop(key, None) + return copy + + @staticmethod + def _build_event_update_patch(event: dict) -> tuple[dict, frozenset[str]]: """Build a JSCalendar PatchObject for a ``CalendarEvent/set`` update. See ``update_event`` for why absent properties are explicitly nulled. Returns the patch together with the set of keys that were - null-injected purely for this cleanup (not present in the converted - iCalendar), so the caller can drop them if the server refuses to - null a property it does not support. + null-injected purely for this cleanup (not present in ``event``), + so the caller can drop them if the server refuses to null a + property it does not support. """ - patch = ical_to_jscal(ical_str) + patch = _JMAPClientBase._strip_server_managed_event_properties(event) patch.pop("uid", None) # uid is server-immutable after creation; patch must omit it nulled: set[str] = set() for key in _NULL_FOR_UPDATE: @@ -706,6 +729,22 @@ def _parse_event_query_response(responses: list) -> tuple[int, int | None]: return parse_event_query(resp_args) return 0, None + @staticmethod + def _parse_set_state(responses: list, set_method: str) -> tuple[str, str]: + """Return ``(old_state, new_state)`` from a ``*/set`` response. + + ``("", "")`` if no response for ``set_method`` is present in + ``responses``. Separate from :func:`~calendaring_jmap._methods.parse_set_response`, + which every object type's ``create``/``update``/``destroy`` parsing shares: + ``oldState``/``newState`` are only surfaced on the event write methods + right now, so reading them here keeps calendars, tasks, and push + subscriptions unaffected. + """ + for name, resp_args, _ in responses: + if name == set_method: + return resp_args.get("oldState", ""), resp_args.get("newState", "") + return "", "" + @staticmethod def _parse_availability_fallback_response(responses: list) -> list[BusyInterval]: """Parse the ``CalendarEvent/get`` response from @@ -1424,13 +1463,14 @@ def set_default_alerts( def _create_event_impl( self, calendar_id: str, - ical_str: str, + event: dict, account_id: str | None, send_scheduling_messages: bool, if_in_state: str | None = None, - ) -> str: + ) -> tuple[str, str, str]: session = self._get_session() - jscal = ical_to_jscal(ical_str, calendar_id=calendar_id) + jscal = self._strip_server_managed_event_properties(event) + jscal["calendarIds"] = {calendar_id: True} target_account = self._resolve_account(session, account_id) call = build_event_set_create( target_account, @@ -1439,22 +1479,30 @@ def _create_event_impl( if_in_state=if_in_state, ) responses = self._request([call]) - return self._parse_create_response( + event_id = self._parse_create_response( responses, session.api_url, "CalendarEvent/set", parse_event_set ) + old_state, new_state = self._parse_set_state(responses, "CalendarEvent/set") + return event_id, old_state, new_state def create_event( self, calendar_id: str, - ical_str: str, + event: dict, account_id: str | None = None, if_in_state: str | None = None, - ) -> str: - """Create a calendar event from an iCalendar string. + ) -> tuple[str, str, str]: + """Create a calendar event from a JSCalendar dict. Args: calendar_id: The JMAP calendar ID to create the event in. - ical_str: A VCALENDAR string representing the event. + event: A JSCalendar CalendarEvent dict (:rfc:`8984#section-2.1`), + e.g. one returned by :meth:`get_event`. Not mutated: a copy is made + before ``id``, ``baseEventId``, and ``isOrigin`` are stripped, + since draft-ietf-jmap-calendars documents all three as + server-set and immutable, and ``calendarIds`` is overwritten + with ``calendar_id``, replacing whatever calendar membership + ``event`` already had. account_id: The account owning ``calendar_id``; pass the owner's account here to add an event to a calendar shared with you (see ``get_calendars(account_id=...)``). @@ -1467,17 +1515,19 @@ def create_event( unconditionally. Returns: - The server-assigned JMAP event ID. + A 3-tuple ``(event_id, old_state, new_state)``: the server-assigned + JMAP event ID, and the account's state immediately before and + after this write (:rfc:`8620#section-5.3`), confirmed live + present on every ``CalendarEvent/set`` response on both Cyrus + and Stalwart. Pass ``new_state`` as a later call's + ``if_in_state`` to require that no other write has landed since. Raises: JMAPMethodError: If the server rejects the create request. - ValueError: If ``ical_str`` is malformed in a way ``ical_to_jscal`` - rejects (missing ``UID``/``DTSTART``, a negative duration, - mismatched ``DTSTART``/``DTEND`` value types). """ return self._create_event_impl( calendar_id, - ical_str, + event, account_id, send_scheduling_messages=False, if_in_state=if_in_state, @@ -1486,37 +1536,36 @@ def create_event( def send_invite( self, calendar_id: str, - ical_str: str, + event: dict, account_id: str | None = None, if_in_state: str | None = None, - ) -> str: + ) -> tuple[str, str, str]: """Create a calendar event and notify its participants. Identical to :meth:`create_event`, except the server actually - dispatches iTIP invitations to the event's participants (an iCalendar - string with ATTENDEE/ORGANIZER properties). ``create_event`` itself - never sends scheduling messages; use this instead when the event has + dispatches iTIP invitations to the event's participants (a JSCalendar + dict with ``participants`` set). ``create_event`` itself never sends + scheduling messages; use this instead when the event has participants who need to be notified. Args: calendar_id: The JMAP calendar ID to create the event in. - ical_str: A VCALENDAR string representing the event. + event: A JSCalendar CalendarEvent dict. See :meth:`create_event`. account_id: The account owning ``calendar_id``, same as :meth:`create_event`. if_in_state: See :meth:`create_event`. Returns: - The server-assigned JMAP event ID. + See :meth:`create_event`. Raises: JMAPMethodError: If the server rejects the create request, or with ``error_type == "noSupportedScheduleMethods"`` if a participant has no usable delivery method. - ValueError: Same conditions as :meth:`create_event`. """ return self._create_event_impl( calendar_id, - ical_str, + event, account_id, send_scheduling_messages=True, if_in_state=if_in_state, @@ -1820,21 +1869,28 @@ def tentatively_accept( def update_event( self, event_id: str, - ical_str: str, + event: dict, account_id: str | None = None, if_in_state: str | None = None, - ) -> None: - """Update a calendar event from an iCalendar string. + ) -> tuple[str, str]: + """Update a calendar event from a JSCalendar dict. A property present on the server's own copy but absent from - ``ical_str`` is explicitly cleared, not left alone: :rfc:`8620#section-5.3` + ``event`` is explicitly cleared, not left alone: :rfc:`8620#section-5.3` merge semantics mean an omitted property would otherwise be preserved rather than removed, so this nulls it to actually drop it server-side. Args: event_id: The JMAP event ID to update. - ical_str: A VCALENDAR string with the updated event data. + event: A JSCalendar CalendarEvent dict with the updated event + data, e.g. one returned by :meth:`get_event` and modified in + place. Not mutated: a copy is made before ``id``, + ``baseEventId``, and ``isOrigin`` are stripped (server-managed; + see :meth:`create_event`). ``calendarIds``, if present, is + sent through unchanged; this method has no ``calendar_id`` + argument to override it with, so moving an event between + calendars this way is left to the caller's own ``event``. account_id: The account owning ``event_id``; pass the owner's account here to update an event on a calendar shared with you. @@ -1848,15 +1904,19 @@ def update_event( null-key-dropping loop below, since those retries narrow the same patch rather than re-reading the account's state. + Returns: + A 2-tuple ``(old_state, new_state)``, the account's state + immediately before and after this write (:rfc:`8620#section-5.3`), + confirmed live present on every ``CalendarEvent/set`` response on + both Cyrus and Stalwart. Pass ``new_state`` as a later call's + ``if_in_state`` to require that no other write has landed since. + Raises: JMAPMethodError: If the server rejects the update. - ValueError: If ``ical_str`` is malformed in a way ``ical_to_jscal`` - rejects (missing ``UID``/``DTSTART``, a negative duration, - mismatched ``DTSTART``/``DTEND`` value types). """ session = self._get_session() target_account = self._resolve_account(session, account_id) - patch, nulled = self._build_event_update_patch(ical_str) + patch, nulled = self._build_event_update_patch(event) while True: responses = self._request( [build_event_set_update(target_account, {event_id: patch}, if_in_state=if_in_state)] @@ -1869,6 +1929,7 @@ def update_event( self._parse_update_response( responses, session.api_url, "CalendarEvent/set", parse_event_set, event_id ) + return self._parse_set_state(responses, "CalendarEvent/set") def _search( self, @@ -2431,10 +2492,17 @@ def get_objects_by_sync_token( server. Compare this against ``sync_token`` to detect a server that silently accepted an invalid or stale state instead of reporting an error (:rfc:`8620#section-5.2` does not require a - server to validate ``sinceState`` against a real prior state; - confirmed live that Cyrus echoes back whatever value it was - given, including one that was never actually issued, rather - than rejecting it). + server to validate ``sinceState`` against a real prior state). + Confirmed live that neither server validates consistently: + Cyrus rejects a non-numeric or empty ``sinceState`` with + ``cannotCalculateChanges``, but silently accepts and echoes + back any numeric-looking string, including one that was never + actually issued. Stalwart rejects nearly everything invalid + with ``invalidArguments``, except a string starting with the + literal character ``n``, which it accepts and normalizes + ``oldState`` down to just ``"n"`` regardless of what follows, + a quirk of its own internal state encoding rather than general + leniency. - ``new_state``: Pass to the next call to this method as ``sync_token``, whether or not ``has_more_changes`` is set. Named to match ``oldState``/``newState`` as a pair diff --git a/src/calendaring_jmap/objects/calendar.py b/src/calendaring_jmap/objects/calendar.py index 8f987ea..8d3f1f9 100644 --- a/src/calendaring_jmap/objects/calendar.py +++ b/src/calendaring_jmap/objects/calendar.py @@ -16,6 +16,7 @@ from typing import TYPE_CHECKING, Any, Generic, Literal, TypeVar, cast, overload from calendaring_jmap.constants import UTC_DATETIME_FORMAT +from calendaring_jmap.convert import ical_to_jscal from calendaring_jmap.objects.calendar_object import JMAPCalendarObject if TYPE_CHECKING: @@ -316,14 +317,21 @@ def add_event(self: Any, ical_str: str): Raises: JMAPMethodError: If the server rejects the create request. - ValueError: Same conditions as - :meth:`~calendaring_jmap.client.JMAPClient.create_event`. + ValueError: If ``ical_str`` is malformed in a way ``ical_to_jscal`` + rejects (missing ``UID``/``DTSTART``, a negative duration, + mismatched ``DTSTART``/``DTEND`` value types). """ if self._is_async: return self._async_add_event(ical_str) - return self._bound_client.create_event(self.id, ical_str, account_id=self._account_id) + jscal = ical_to_jscal(ical_str, calendar_id=self.id) + event_id, _, _ = self._bound_client.create_event( + self.id, jscal, account_id=self._account_id + ) + return event_id async def _async_add_event(self: JMAPCalendar[Literal[True]], ical_str: str) -> str: - return await self._bound_async_client.create_event( - self.id, ical_str, account_id=self._account_id + jscal = ical_to_jscal(ical_str, calendar_id=self.id) + event_id, _, _ = await self._bound_async_client.create_event( + self.id, jscal, account_id=self._account_id ) + return event_id diff --git a/src/calendaring_jmap/objects/calendar_object.py b/src/calendaring_jmap/objects/calendar_object.py index 1081566..c3564ea 100644 --- a/src/calendaring_jmap/objects/calendar_object.py +++ b/src/calendaring_jmap/objects/calendar_object.py @@ -17,7 +17,7 @@ import icalendar -from calendaring_jmap.convert import jscal_to_ical +from calendaring_jmap.convert import ical_to_jscal, jscal_to_ical from calendaring_jmap.error import JMAPMethodError if TYPE_CHECKING: @@ -90,17 +90,18 @@ def edit_icalendar_instance(self): def save(self) -> None: """Persist changes made via :meth:`edit_icalendar_instance` to the server. - Serialises the (possibly edited) icalendar object back to an iCalendar - string and calls ``update_event()`` on the parent calendar's client. + If :meth:`edit_icalendar_instance` was used, the (possibly edited) + icalendar object is converted back to a JSCalendar dict first. If + it wasn't, :attr:`data` is sent as-is, with no round trip through + iCalendar. Raises: JMAPMethodError: If no parent calendar is set (``parent`` is ``None``). RuntimeError: If called on an async-backed calendar. - ValueError: If an edit made via :meth:`edit_icalendar_instance` - leaves the object in a shape rejected by - :meth:`~calendaring_jmap.client.JMAPClient.update_event` - (e.g. removing ``UID``/``DTSTART``, or moving ``DTEND`` - before ``DTSTART``). + ValueError: If :meth:`edit_icalendar_instance` was used and the + edit leaves the object missing ``UID``/``DTSTART``, or with + ``DTEND`` before ``DTSTART``; raised by the conversion back + to JSCalendar, before ``update_event`` is ever called. """ if self.parent is None: raise JMAPMethodError(url="N/A", reason="Cannot save: no parent calendar is set") @@ -109,11 +110,9 @@ def save(self) -> None: "save() is not supported for async-backed calendars. " "Use await parent._client.update_event() directly." ) - ical_str = ( - self._ical_cache.to_ical().decode() + event = ( + ical_to_jscal(self._ical_cache.to_ical().decode()) if self._ical_cache is not None - else jscal_to_ical(self.data) - ) - self.parent._bound_client.update_event( - self.id, ical_str, account_id=self.parent._account_id + else self.data ) + self.parent._bound_client.update_event(self.id, event, account_id=self.parent._account_id) diff --git a/src/calendaring_jmap/tests/test_jmap_integration.py b/src/calendaring_jmap/tests/test_jmap_integration.py index 4fe941a..bc257e5 100644 --- a/src/calendaring_jmap/tests/test_jmap_integration.py +++ b/src/calendaring_jmap/tests/test_jmap_integration.py @@ -31,7 +31,7 @@ from calendaring_jmap._methods import parse_set_response from calendaring_jmap.client import _CONTACTS_USING, _PUSH_USING, _JMAPClientBase from calendaring_jmap.constants import CALENDAR_CAPABILITY, CONTACTS_CAPABILITY -from calendaring_jmap.convert import jscal_to_ical +from calendaring_jmap.convert import ical_to_jscal, jscal_to_ical from calendaring_jmap.error import JMAPMethodError from calendaring_jmap.session import fetch_session @@ -141,6 +141,21 @@ def _minimal_ical(title: str = "Test Event", start: datetime | None = None) -> s return _vevent_ical(title, start or _DEFAULT_ICAL_START, timedelta(hours=1)) +def _minimal_event( + title: str = "Test Event", start: datetime | None = None, calendar_id: str | None = None +) -> dict: + """``ical_to_jscal(_minimal_ical(...), calendar_id=...)`` in one call. + + Shared by every integration test that just needs a fresh, minimal + JSCalendar dict to pass to ``create_event``/``send_invite``/ + ``update_event``, the common case; a test exercising recurrence, + attachments, or other non-minimal shapes still builds its own + iCalendar string and converts it directly, since this helper only + covers the minimal case. + """ + return ical_to_jscal(_minimal_ical(title, start=start), calendar_id=calendar_id) + + def _recurring_ical( rrule: str, title: str = "Recurring Test Event", @@ -245,7 +260,9 @@ def calendar_id(client): @pytest.fixture def created_event_id(client, calendar_id): - event_id = client.create_event(calendar_id, _minimal_ical("Integration Test Event")) + event_id, _, _ = client.create_event( + calendar_id, _minimal_event("Integration Test Event", calendar_id=calendar_id) + ) yield event_id try: client.delete_event(event_id) @@ -267,8 +284,9 @@ async def async_calendar_id(async_client): @pytest_asyncio.fixture async def async_created_event_id(async_client, async_calendar_id): - event_id = await async_client.create_event( - async_calendar_id, _minimal_ical("Async Integration Test Event") + event_id, _, _ = await async_client.create_event( + async_calendar_id, + _minimal_event("Async Integration Test Event", calendar_id=async_calendar_id), ) yield event_id try: @@ -298,8 +316,9 @@ def stalwart_calendar_id(stalwart_client): @pytest.fixture def stalwart_event_id(stalwart_client, stalwart_calendar_id): - event_id = stalwart_client.create_event( - stalwart_calendar_id, _minimal_ical("Stalwart Test Event") + event_id, _, _ = stalwart_client.create_event( + stalwart_calendar_id, + _minimal_event("Stalwart Test Event", calendar_id=stalwart_calendar_id), ) yield event_id try: @@ -449,7 +468,9 @@ def test_create_update_delete_calendar(self, calendar_management_client): def test_delete_calendar_with_event_raises_calendar_has_event(self, calendar_management_client): cal_id = calendar_management_client.create_calendar("Calendar With Event") - calendar_management_client.create_event(cal_id, _minimal_ical("Blocking Event")) + calendar_management_client.create_event( + cal_id, _minimal_event("Blocking Event", calendar_id=cal_id) + ) try: with pytest.raises(JMAPMethodError) as exc_info: calendar_management_client.delete_calendar(cal_id) @@ -465,7 +486,9 @@ def test_delete_calendar_with_event_and_on_destroy_remove_events( self, calendar_management_client ): cal_id = calendar_management_client.create_calendar("Calendar With Event To Remove") - calendar_management_client.create_event(cal_id, _minimal_ical("Removable Event")) + calendar_management_client.create_event( + cal_id, _minimal_event("Removable Event", calendar_id=cal_id) + ) calendar_management_client.delete_calendar(cal_id, on_destroy_remove_events=True) calendars = calendar_management_client.get_calendars() assert not any(c.id == cal_id for c in calendars) @@ -580,8 +603,8 @@ def test_edit_and_delete_event_on_shared_calendar_targets_owner_account( calendar_management_client.share_calendar( cal_id, second_account_id, {"mayReadItems": True, "mayWriteAll": True} ) - event_id = calendar_management_client.create_event( - cal_id, _minimal_ical("Owner Created Event") + event_id, _, _ = calendar_management_client.create_event( + cal_id, _minimal_event("Owner Created Event", calendar_id=cal_id) ) second_view = second_account_client.get_calendars(account_id=owner_account_id) @@ -629,7 +652,9 @@ def test_send_invite_and_accept( try: calendar_management_client.share_calendar(cal_id, second_account_id, _SCHEDULING_RIGHTS) ical = _invite_ical(owner_email, second_account_email, title="Accept Me") - event_id = calendar_management_client.send_invite(cal_id, ical) + event_id, _, _ = calendar_management_client.send_invite( + cal_id, ical_to_jscal(ical, calendar_id=cal_id) + ) second_account_client.accept_invitation( event_id, second_account_email, account_id=owner_account_id @@ -658,7 +683,9 @@ def test_decline_invitation( try: calendar_management_client.share_calendar(cal_id, second_account_id, _SCHEDULING_RIGHTS) ical = _invite_ical(owner_email, second_account_email, title="Decline Me") - event_id = calendar_management_client.send_invite(cal_id, ical) + event_id, _, _ = calendar_management_client.send_invite( + cal_id, ical_to_jscal(ical, calendar_id=cal_id) + ) second_account_client.decline_invitation( event_id, second_account_email, account_id=owner_account_id @@ -683,7 +710,9 @@ def test_tentatively_accept_invitation( try: calendar_management_client.share_calendar(cal_id, second_account_id, _SCHEDULING_RIGHTS) ical = _invite_ical(owner_email, second_account_email, title="Maybe Me") - event_id = calendar_management_client.send_invite(cal_id, ical) + event_id, _, _ = calendar_management_client.send_invite( + cal_id, ical_to_jscal(ical, calendar_id=cal_id) + ) second_account_client.tentatively_accept( event_id, second_account_email, account_id=owner_account_id @@ -699,7 +728,9 @@ def test_accept_invitation_raises_when_own_email_not_a_participant( ): cal_id = calendar_management_client.create_calendar("No Participant Calendar") try: - event_id = calendar_management_client.create_event(cal_id, _minimal_ical("Solo Event")) + event_id, _, _ = calendar_management_client.create_event( + cal_id, _minimal_event("Solo Event", calendar_id=cal_id) + ) with pytest.raises(JMAPMethodError) as exc_info: calendar_management_client.accept_invitation(event_id, "nobody@example.invalid") assert exc_info.value.error_type == "notFound" @@ -725,7 +756,9 @@ def test_cancellation_from_organizer_removes_event_for_participant( try: calendar_management_client.share_calendar(cal_id, second_account_id, _SCHEDULING_RIGHTS) ical = _invite_ical(owner_email, second_account_email, title="Cancel Me") - event_id = calendar_management_client.send_invite(cal_id, ical) + event_id, _, _ = calendar_management_client.send_invite( + cal_id, ical_to_jscal(ical, calendar_id=cal_id) + ) second_account_client.accept_invitation( event_id, second_account_email, account_id=owner_account_id ) @@ -758,7 +791,9 @@ def test_no_counter_proposal_mechanism( try: calendar_management_client.share_calendar(cal_id, second_account_id, _SCHEDULING_RIGHTS) ical = _invite_ical(owner_email, second_account_email, title="Propose New Time") - event_id = calendar_management_client.send_invite(cal_id, ical) + event_id, _, _ = calendar_management_client.send_invite( + cal_id, ical_to_jscal(ical, calendar_id=cal_id) + ) second_account_client.accept_invitation( event_id, second_account_email, account_id=owner_account_id ) @@ -769,7 +804,9 @@ def test_no_counter_proposal_mechanism( title="Propose New Time", start=datetime(2030, 6, 1, 15, 0, tzinfo=timezone.utc), ) - second_account_client.update_event(event_id, proposed, account_id=owner_account_id) + second_account_client.update_event( + event_id, ical_to_jscal(proposed), account_id=owner_account_id + ) updated = calendar_management_client.get_event(event_id) assert updated.get_data()["start"] == "2030-06-01T15:00:00", ( @@ -790,14 +827,14 @@ def test_sequence_increments_on_substantive_update(self, calendar_management_cli pytest.skip("Stalwart does not increment sequence on CalendarEvent/set update") cal_id = calendar_management_client.create_calendar("Sequence Integration Calendar") try: - event_id = calendar_management_client.create_event( - cal_id, _minimal_ical("Sequence Start") + event_id, _, _ = calendar_management_client.create_event( + cal_id, _minimal_event("Sequence Start", calendar_id=cal_id) ) before = calendar_management_client.get_event(event_id) sequence_before = before.get_data().get("sequence", 0) calendar_management_client.update_event( - event_id, _minimal_ical("Sequence Changed Title") + event_id, _minimal_event("Sequence Changed Title") ) after = calendar_management_client.get_event(event_id) @@ -837,7 +874,9 @@ def _stale_state_after_bystander_write( it, not just the one it's actually testing. """ stale_state = event_client.get_sync_token() - other_event_id = event_client.create_event(calendar_id, _minimal_ical(bystander_title)) + other_event_id, _, _ = event_client.create_event( + calendar_id, _minimal_event(bystander_title, calendar_id=calendar_id) + ) return stale_state, other_event_id @@ -850,19 +889,24 @@ def test_event_create_get(self, event_client, event_created_id): assert "BEGIN:VCALENDAR" in ical def test_event_update(self, event_client, event_created_id): - event_client.update_event(event_created_id, _minimal_ical("Updated Title")) + event_client.update_event(event_created_id, _minimal_event("Updated Title")) obj = event_client.get_event(event_created_id) assert "Updated Title" in jscal_to_ical(obj.get_data()) def test_event_delete(self, event_client, event_calendar_id): - event_id = event_client.create_event(event_calendar_id, _minimal_ical("To Be Deleted")) + event_id, _, _ = event_client.create_event( + event_calendar_id, + _minimal_event("To Be Deleted", calendar_id=event_calendar_id), + ) event_client.delete_event(event_id) with pytest.raises(JMAPMethodError): event_client.get_event(event_id) def test_event_query_time_range(self, event_client, event_calendar_id): title = "Query Range Test Event" - event_id = event_client.create_event(event_calendar_id, _minimal_ical(title)) + event_id, _, _ = event_client.create_event( + event_calendar_id, _minimal_event(title, calendar_id=event_calendar_id) + ) try: results = event_client.search_events( calendar_id=event_calendar_id, @@ -876,7 +920,10 @@ def test_event_query_time_range(self, event_client, event_calendar_id): def test_event_sync(self, event_client, event_calendar_id): token_before = event_client.get_sync_token() - event_id = event_client.create_event(event_calendar_id, _minimal_ical("Sync Test Event")) + event_id, _, _ = event_client.create_event( + event_calendar_id, + _minimal_event("Sync Test Event", calendar_id=event_calendar_id), + ) try: added, _modified, _deleted, old_state, _new_state, _has_more = ( event_client.get_objects_by_sync_token(token_before) @@ -897,8 +944,9 @@ def test_get_sync_token_uses_given_account_id(self, event_client, server): def test_get_objects_by_sync_token_uses_given_account_id(self, event_client, event_calendar_id): session_account_id = event_client._get_session().account_id token_before = event_client.get_sync_token(account_id=session_account_id) - event_id = event_client.create_event( - event_calendar_id, _minimal_ical("Account Scoped Sync Test Event") + event_id, _, _ = event_client.create_event( + event_calendar_id, + _minimal_event("Account Scoped Sync Test Event", calendar_id=event_calendar_id), ) try: added, _modified, _deleted, old_state, _new_state, _has_more = ( @@ -912,14 +960,15 @@ def test_get_objects_by_sync_token_uses_given_account_id(self, event_client, eve event_client.delete_event(event_id) def test_update_event_with_correct_if_in_state_succeeds(self, event_client, event_calendar_id): - event_id = event_client.create_event( - event_calendar_id, _minimal_ical("ifInState Success Test Event") + event_id, _, _ = event_client.create_event( + event_calendar_id, + _minimal_event("ifInState Success Test Event", calendar_id=event_calendar_id), ) try: current_state = event_client.get_sync_token() event_client.update_event( event_id, - _minimal_ical("ifInState Success Test Event Updated"), + _minimal_event("ifInState Success Test Event Updated"), if_in_state=current_state, ) updated = jscal_to_ical(event_client.get_event(event_id).get_data()) @@ -935,8 +984,9 @@ def test_update_event_with_stale_if_in_state_raises_state_mismatch( confirmed here against a real server rather than only a mocked response, since this is the exact failure mode the calendaring-sync adapter depends on to detect a concurrent write.""" - event_id = event_client.create_event( - event_calendar_id, _minimal_ical("ifInState Mismatch Test Event") + event_id, _, _ = event_client.create_event( + event_calendar_id, + _minimal_event("ifInState Mismatch Test Event", calendar_id=event_calendar_id), ) try: stale_state, other_event_id = _stale_state_after_bystander_write( @@ -946,7 +996,7 @@ def test_update_event_with_stale_if_in_state_raises_state_mismatch( with pytest.raises(JMAPMethodError) as exc_info: event_client.update_event( event_id, - _minimal_ical("ifInState Mismatch Test Event Updated"), + _minimal_event("ifInState Mismatch Test Event Updated"), if_in_state=stale_state, ) assert exc_info.value.error_type == "stateMismatch" @@ -958,8 +1008,9 @@ def test_update_event_with_stale_if_in_state_raises_state_mismatch( def test_delete_event_with_stale_if_in_state_raises_state_mismatch( self, event_client, event_calendar_id ): - event_id = event_client.create_event( - event_calendar_id, _minimal_ical("ifInState Delete Mismatch Test Event") + event_id, _, _ = event_client.create_event( + event_calendar_id, + _minimal_event("ifInState Delete Mismatch Test Event", calendar_id=event_calendar_id), ) try: stale_state, other_event_id = _stale_state_after_bystander_write( @@ -984,7 +1035,10 @@ def test_get_objects_by_sync_token_pages_with_max_changes( recovers every change with none missed or duplicated.""" token_before = event_client.get_sync_token() event_ids = [ - event_client.create_event(event_calendar_id, _minimal_ical(f"Paged Sync Event {i}")) + event_client.create_event( + event_calendar_id, + _minimal_event(f"Paged Sync Event {i}", calendar_id=event_calendar_id), + )[0] for i in range(3) ] try: @@ -1017,7 +1071,9 @@ def test_recurring_event_create_get_update(self, event_client, event_calendar_id title="Weekly Standup", start=datetime(2026, 7, 1, 10, 0, 0, tzinfo=timezone.utc), ) - event_id = event_client.create_event(event_calendar_id, ical) + event_id, _, _ = event_client.create_event( + event_calendar_id, ical_to_jscal(ical, calendar_id=event_calendar_id) + ) try: obj = event_client.get_event(event_id) assert "recurrenceRule" in obj.get_data(), ( @@ -1026,7 +1082,7 @@ def test_recurring_event_create_get_update(self, event_client, event_calendar_id assert "RRULE" in jscal_to_ical(obj.get_data()) updated_ical = ical.replace("Weekly Standup", "Weekly Standup Renamed") - event_client.update_event(event_id, updated_ical) + event_client.update_event(event_id, ical_to_jscal(updated_ical)) updated = event_client.get_event(event_id) assert "recurrenceRule" in updated.get_data(), ( f"{server}: recurrence rule was lost after an unrelated update." @@ -1037,8 +1093,9 @@ def test_recurring_event_create_get_update(self, event_client, event_calendar_id def test_ical_roundtrip(self, event_client, event_calendar_id): start = datetime(2026, 7, 15, 9, 0, 0, tzinfo=timezone.utc) - event_id = event_client.create_event( - event_calendar_id, _minimal_ical("Roundtrip Event", start=start) + event_id, _, _ = event_client.create_event( + event_calendar_id, + _minimal_event("Roundtrip Event", start=start, calendar_id=event_calendar_id), ) try: fetched = jscal_to_ical(event_client.get_event(event_id).get_data()) @@ -1047,6 +1104,29 @@ def test_ical_roundtrip(self, event_client, event_calendar_id): finally: event_client.delete_event(event_id) + def test_jscalendar_write_back_unchanged_round_trips(self, event_client, event_calendar_id): + """An event read as a JSCalendar dict and written back unchanged + comes back equal, with no iCalendar round trip in between to lose + properties either way. ``updated`` is excluded from the comparison: + confirmed live that Cyrus bumps it on every ``CalendarEvent/set`` + call, including a no-op write, matching draft-ietf-jmap-calendars + section 5.9's "the server sets updated to the current time on + every create or update," not only on a change that alters + something.""" + event_id, _, _ = event_client.create_event( + event_calendar_id, + _minimal_event("Write Back Unchanged Event", calendar_id=event_calendar_id), + ) + try: + original = event_client.get_event(event_id).get_data() + event_client.update_event(event_id, original) + refetched = event_client.get_event(event_id).get_data() + assert {k: v for k, v in refetched.items() if k != "updated"} == { + k: v for k, v in original.items() if k != "updated" + } + finally: + event_client.delete_event(event_id) + def test_attach_property_roundtrip(self, event_client, event_calendar_id, server): """ical_to_jscal/jscal_to_ical convert ATTACH <-> links (rel: "enclosure"). On Cyrus, the fetched event's ATTACH is lost on the @@ -1059,7 +1139,9 @@ def test_attach_property_roundtrip(self, event_client, event_calendar_id, server timedelta(hours=1), extra_lines="ATTACH;FMTTYPE=application/pdf:https://example.com/report.pdf\r\n", ) - event_id = event_client.create_event(event_calendar_id, ical) + event_id, _, _ = event_client.create_event( + event_calendar_id, ical_to_jscal(ical, calendar_id=event_calendar_id) + ) try: fetched = jscal_to_ical(event_client.get_event(event_id).get_data()) if server == "cyrus": @@ -1086,7 +1168,9 @@ def test_url_property_roundtrip(self, event_client, event_calendar_id): timedelta(hours=1), extra_lines="URL:https://example.com/schedule.ics\r\n", ) - event_id = event_client.create_event(event_calendar_id, ical) + event_id, _, _ = event_client.create_event( + event_calendar_id, ical_to_jscal(ical, calendar_id=event_calendar_id) + ) try: fetched = jscal_to_ical(event_client.get_event(event_id).get_data()) assert "URL:https://example.com/schedule.ics" in fetched @@ -1116,14 +1200,18 @@ def test_virtual_location_description_roundtrip(self, event_client, event_calend ) if server == "cyrus": with pytest.raises(JMAPMethodError) as exc_info: - event_client.create_event(event_calendar_id, ical) + event_client.create_event( + event_calendar_id, ical_to_jscal(ical, calendar_id=event_calendar_id) + ) assert exc_info.value.error_type == "invalidProperties", ( "cyrus: if this now accepts VirtualLocation.description, " "Cyrus may have fixed this limitation; update this test " "accordingly." ) return - event_id = event_client.create_event(event_calendar_id, ical) + event_id, _, _ = event_client.create_event( + event_calendar_id, ical_to_jscal(ical, calendar_id=event_calendar_id) + ) try: data = event_client.get_event(event_id).get_data() vloc = next(iter(data["virtualLocations"].values())) @@ -1153,7 +1241,9 @@ def test_recurrence_override_combines_multiple_property_changes( "CONFERENCE;VALUE=URI;FEATURE=PHONE:tel:+1-555-555-5555\r\n" ), ) - event_id = event_client.create_event(event_calendar_id, ical) + event_id, _, _ = event_client.create_event( + event_calendar_id, ical_to_jscal(ical, calendar_id=event_calendar_id) + ) try: obj = event_client.get_event(event_id) data = obj.get_data() @@ -1214,7 +1304,9 @@ def test_recurrence_override_participant_and_alert_server_quirks( "BEGIN:VALARM\r\nACTION:DISPLAY\r\nTRIGGER:-PT15M\r\nEND:VALARM\r\n" ), ) - event_id = event_client.create_event(event_calendar_id, ical) + event_id, _, _ = event_client.create_event( + event_calendar_id, ical_to_jscal(ical, calendar_id=event_calendar_id) + ) try: obj = event_client.get_event(event_id) overrides = obj.get_data().get("recurrenceOverrides") or {} @@ -1262,7 +1354,9 @@ def test_recurrence_override_location_patch_resolves_when_master_has_its_own_loc master_extra_lines="LOCATION:Room 100\r\n", override_extra_lines="LOCATION:Room 200\r\n", ) - event_id = event_client.create_event(event_calendar_id, ical) + event_id, _, _ = event_client.create_event( + event_calendar_id, ical_to_jscal(ical, calendar_id=event_calendar_id) + ) try: obj = event_client.get_event(event_id) data = obj.get_data() @@ -1309,8 +1403,9 @@ def test_get_availability_returns_busy_interval_for_event( ): start = datetime(2026, 9, 21, 10, 0, 0, tzinfo=timezone.utc) session = event_client._get_session() - event_id = event_client.create_event( - event_calendar_id, _minimal_ical("Availability Test Event", start=start) + event_id, _, _ = event_client.create_event( + event_calendar_id, + _minimal_event("Availability Test Event", start=start, calendar_id=event_calendar_id), ) try: availability = event_client.get_availability( @@ -1333,7 +1428,10 @@ def test_get_availability_show_details(self, event_client, event_calendar_id, se title = "Details Test Event" start = datetime(2026, 9, 21, 14, 0, 0, tzinfo=timezone.utc) session = event_client._get_session() - event_id = event_client.create_event(event_calendar_id, _minimal_ical(title, start=start)) + event_id, _, _ = event_client.create_event( + event_calendar_id, + _minimal_event(title, start=start, calendar_id=event_calendar_id), + ) try: availability = event_client.get_availability( [session.account_id], @@ -1366,7 +1464,9 @@ def test_fallback_recurring_event_busy_intervals(self, event_client, event_calen duration=timedelta(minutes=30), ) session = event_client._get_session() - event_id = event_client.create_event(event_calendar_id, ical) + event_id, _, _ = event_client.create_event( + event_calendar_id, ical_to_jscal(ical, calendar_id=event_calendar_id) + ) try: intervals = event_client._get_availability_via_fallback( session.account_id, "2026-09-19T00:00:00", "2026-10-10T00:00:00" @@ -1393,15 +1493,16 @@ async def test_event_create_get(self, async_client, async_created_event_id): @pytest.mark.asyncio async def test_event_update(self, async_client, async_created_event_id): await async_client.update_event( - async_created_event_id, _minimal_ical("Async Updated Title") + async_created_event_id, _minimal_event("Async Updated Title") ) obj = await async_client.get_event(async_created_event_id) assert "Async Updated Title" in jscal_to_ical(obj.get_data()) @pytest.mark.asyncio async def test_event_delete(self, async_client, async_calendar_id): - event_id = await async_client.create_event( - async_calendar_id, _minimal_ical("Async To Be Deleted") + event_id, _, _ = await async_client.create_event( + async_calendar_id, + _minimal_event("Async To Be Deleted", calendar_id=async_calendar_id), ) await async_client.delete_event(event_id) with pytest.raises(JMAPMethodError): @@ -1424,8 +1525,9 @@ async def test_event_query_time_range( @pytest.mark.asyncio async def test_event_sync(self, async_client, async_calendar_id): token_before = await async_client.get_sync_token() - event_id = await async_client.create_event( - async_calendar_id, _minimal_ical("Async Sync Test Event") + event_id, _, _ = await async_client.create_event( + async_calendar_id, + _minimal_event("Async Sync Test Event", calendar_id=async_calendar_id), ) try: ( @@ -1444,8 +1546,9 @@ async def test_event_sync(self, async_client, async_calendar_id): @pytest.mark.asyncio async def test_ical_roundtrip(self, async_client, async_calendar_id): start = datetime(2026, 7, 15, 9, 0, 0, tzinfo=timezone.utc) - event_id = await async_client.create_event( - async_calendar_id, _minimal_ical("Async Roundtrip Event", start=start) + event_id, _, _ = await async_client.create_event( + async_calendar_id, + _minimal_event("Async Roundtrip Event", start=start, calendar_id=async_calendar_id), ) try: fetched = jscal_to_ical((await async_client.get_event(event_id)).get_data()) @@ -1460,8 +1563,11 @@ async def test_get_availability_returns_busy_interval_for_event( ): start = datetime(2026, 9, 21, 10, 0, 0, tzinfo=timezone.utc) session = await async_client._get_session() - event_id = await async_client.create_event( - async_calendar_id, _minimal_ical("Async Availability Test Event", start=start) + event_id, _, _ = await async_client.create_event( + async_calendar_id, + _minimal_event( + "Async Availability Test Event", start=start, calendar_id=async_calendar_id + ), ) try: availability = await async_client.get_availability( @@ -1506,8 +1612,9 @@ def test_download_unknown_blob_id_raises(self, event_client): def test_attach_to_event_persists_href(self, event_client, event_calendar_id, server): data = b"png-like binary content for attachment test" - event_id = event_client.create_event( - event_calendar_id, _minimal_ical("Attachment Test Event") + event_id, _, _ = event_client.create_event( + event_calendar_id, + _minimal_event("Attachment Test Event", calendar_id=event_calendar_id), ) try: blob_id = event_client.upload_attachment(data, "application/octet-stream") @@ -1532,8 +1639,9 @@ def test_get_event_attachments_finds_attachment(self, event_client, event_calend JMAPAttachment.is_attachment), so it finds nothing here even though attach_to_event succeeded; Stalwart finds the attachment.""" data = b"attachment content for get_event_attachments test" - event_id = event_client.create_event( - event_calendar_id, _minimal_ical("Attachment Filter Test Event") + event_id, _, _ = event_client.create_event( + event_calendar_id, + _minimal_event("Attachment Filter Test Event", calendar_id=event_calendar_id), ) try: blob_id = event_client.upload_attachment(data, "application/octet-stream") @@ -1574,8 +1682,9 @@ async def test_attach_to_event_persists_href(self, async_client, async_calendar_ """Cyrus-only class; asserts on the raw links data rather than get_event_attachments, see test_get_event_attachments_empty_on_cyrus_due_to_rel_bug.""" data = b"async binary content for attachment test" - event_id = await async_client.create_event( - async_calendar_id, _minimal_ical("Async Attachment Test Event") + event_id, _, _ = await async_client.create_event( + async_calendar_id, + _minimal_event("Async Attachment Test Event", calendar_id=async_calendar_id), ) try: blob_id = await async_client.upload_attachment(data, "application/octet-stream") @@ -1601,8 +1710,9 @@ async def test_get_event_attachments_empty_on_cyrus_due_to_rel_bug( self, async_client, async_calendar_id ): """See TestAttachmentIntegration.test_get_event_attachments_finds_attachment.""" - event_id = await async_client.create_event( - async_calendar_id, _minimal_ical("Async Attachment Filter Test Event") + event_id, _, _ = await async_client.create_event( + async_calendar_id, + _minimal_event("Async Attachment Filter Test Event", calendar_id=async_calendar_id), ) try: blob_id = await async_client.upload_attachment(b"data", "application/octet-stream") @@ -1896,7 +2006,9 @@ class TestRicherEventSearchIntegration: def test_has_attachment_true_finds_event_with_attachment( self, event_client, event_calendar_id, server ): - event_id = event_client.create_event(event_calendar_id, _attachment_ical()) + event_id, _, _ = event_client.create_event( + event_calendar_id, ical_to_jscal(_attachment_ical(), calendar_id=event_calendar_id) + ) try: results = event_client.search_events( calendar_id=event_calendar_id, has_attachment=True @@ -1920,7 +2032,9 @@ def test_has_attachment_false_excludes_event_with_attachment_on_stalwart( ): if server == "cyrus": pytest.skip("Cyrus drops rel: enclosure, so this event never registers as attached") - event_id = event_client.create_event(event_calendar_id, _attachment_ical()) + event_id, _, _ = event_client.create_event( + event_calendar_id, ical_to_jscal(_attachment_ical(), calendar_id=event_calendar_id) + ) try: results = event_client.search_events( calendar_id=event_calendar_id, has_attachment=False @@ -1937,9 +2051,12 @@ def test_chair_participant_matches_email_status_and_role_filters( participant_role together instead of three near-identical create/search/delete sequences.""" chair_email = "chair-filter-test@example.com" - event_id = event_client.create_event( + event_id, _, _ = event_client.create_event( event_calendar_id, - _invite_ical(owner_email, chair_email, role="CHAIR", partstat="ACCEPTED"), + ical_to_jscal( + _invite_ical(owner_email, chair_email, role="CHAIR", partstat="ACCEPTED"), + calendar_id=event_calendar_id, + ), ) try: by_email = event_client.search_events( @@ -1971,8 +2088,9 @@ def test_participant_role_attendee_confirmed_dropped( assertion should start failing, not silently pass, the day either server stops doing that.""" attendee_email = "attendee-role-gap-test@example.com" - event_id = event_client.create_event( - event_calendar_id, _invite_ical(owner_email, attendee_email) + event_id, _, _ = event_client.create_event( + event_calendar_id, + ical_to_jscal(_invite_ical(owner_email, attendee_email), calendar_id=event_calendar_id), ) try: results = event_client.search_events( @@ -1997,7 +2115,10 @@ def test_search_events_limit_caps_page_and_total_reflects_full_count( actually passed, not merely whether the server happened to include one.""" event_ids = [ - event_client.create_event(event_calendar_id, _minimal_ical(f"Paged Search Event {i}")) + event_client.create_event( + event_calendar_id, + _minimal_event(f"Paged Search Event {i}", calendar_id=event_calendar_id), + )[0] for i in range(3) ] try: diff --git a/src/calendaring_jmap/tests/test_jmap_unit.py b/src/calendaring_jmap/tests/test_jmap_unit.py index 16eee2e..632b61d 100644 --- a/src/calendaring_jmap/tests/test_jmap_unit.py +++ b/src/calendaring_jmap/tests/test_jmap_unit.py @@ -909,17 +909,19 @@ def test_calendar_get_object_by_uid_dispatches_to_async_when_async_backed(self): def test_calendar_add_event_dispatches_to_async_when_async_backed(self): mock_client = MagicMock() - mock_client.create_event = AsyncMock(return_value="ev-new-id") + mock_client.create_event = AsyncMock(return_value=("ev-new-id", "s0", "s1")) cal: JMAPCalendar[Literal[True]] = JMAPCalendar(id="cal1", name="Test") cal._client = mock_client cal._is_async = True import asyncio - result = asyncio.run(cal.add_event("BEGIN:VCALENDAR\r\nEND:VCALENDAR\r\n")) + result = asyncio.run(cal.add_event(self._MINIMAL_ICAL)) assert result == "ev-new-id" - mock_client.create_event.assert_awaited_once_with( - "cal1", "BEGIN:VCALENDAR\r\nEND:VCALENDAR\r\n", account_id=None - ) + mock_client.create_event.assert_awaited_once() + call_args = mock_client.create_event.call_args + assert call_args[0][0] == "cal1" + assert call_args[0][1].get("calendarIds") == {"cal1": True} + assert call_args.kwargs["account_id"] is None _MINIMAL_JSCAL_DICT = { @@ -973,9 +975,25 @@ def test_save_calls_update_event(self): mock_client.update_event.assert_called_once() call_args = mock_client.update_event.call_args assert call_args[0][0] == "ev-obj-1" - assert isinstance(call_args[0][1], str) + # edit_icalendar_instance() was used, so save() re-converts the + # (possibly edited) icalendar object back to JSCalendar. + assert isinstance(call_args[0][1], dict) assert call_args.kwargs["account_id"] is None + def test_save_sends_data_directly_without_edit(self): + mock_client = MagicMock() + mock_parent = JMAPCalendar(id="cal1", name="Test") + mock_parent._client = mock_client + + obj = JMAPCalendarObject(data=_MINIMAL_JSCAL_DICT, parent=mock_parent) + obj.save() + + mock_client.update_event.assert_called_once() + call_args = mock_client.update_event.call_args + # No edit_icalendar_instance() edit happened, so save() sends + # self.data straight through, no iCalendar round trip. + assert call_args[0][1] is _MINIMAL_JSCAL_DICT + def test_save_uses_parent_account_id_when_shared(self): mock_client = MagicMock() mock_parent = JMAPCalendar(id="cal1", name="Test") @@ -5535,6 +5553,20 @@ def _participant_event( } +def _event_with_stale_server_managed_properties(base_event: dict, event_id: str) -> dict: + """Return a copy of ``base_event`` with ``id``/``baseEventId``/``isOrigin`` + set to values a server would reject or treat as immutable, to confirm + ``create_event``/``update_event`` strip them before sending. Shared by + the sync and async strip tests for both methods, which otherwise build + this same poisoned dict four times over.""" + return { + **base_event, + "id": event_id, + "baseEventId": "stale-base-id", + "isOrigin": True, + } + + class TestJMAPClientEvents(_MockedClientMixin): _MINIMAL_ICAL = ( "BEGIN:VCALENDAR\r\n" @@ -5547,6 +5579,14 @@ class TestJMAPClientEvents(_MockedClientMixin): "END:VCALENDAR\r\n" ) + # JSCalendar equivalent of _MINIMAL_ICAL, what a caller passes to + # create_event/send_invite/update_event now that they take a dict. + # calendarIds points at a different calendar than any test below passes + # as calendar_id, so test_create_event_passes_calendar_id's assertion + # that the server call's calendarIds matches the calendar_id argument + # (not whatever was already in the dict) stays meaningful. + _MINIMAL_EVENT = ical_to_jscal(_MINIMAL_ICAL, calendar_id="original-calendar") + _RAW_EVENT = { "id": "ev1", "uid": "test-uid@example.com", @@ -5565,7 +5605,7 @@ def _get_response(self, items): def test_create_event_returns_server_id(self, monkeypatch): resp = self._set_response(created={"new-0": {"id": "sv-1"}}) client = _make_client_with_mocked_session(monkeypatch, resp) - event_id = client.create_event("cal1", self._MINIMAL_ICAL) + event_id, _, _ = client.create_event("cal1", self._MINIMAL_EVENT) assert event_id == "sv-1" def test_create_event_raises_on_failure(self, monkeypatch): @@ -5574,25 +5614,44 @@ def test_create_event_raises_on_failure(self, monkeypatch): ) client = _make_client_with_mocked_session(monkeypatch, resp) with pytest.raises(JMAPMethodError) as exc_info: - client.create_event("cal1", self._MINIMAL_ICAL) + client.create_event("cal1", self._MINIMAL_EVENT) assert exc_info.value.error_type == "invalidArguments" def test_create_event_raises_on_malformed_response(self, monkeypatch): resp = self._set_response(created={}, notCreated={}) client = _make_client_with_mocked_session(monkeypatch, resp) with pytest.raises(JMAPMethodError): - client.create_event("cal1", self._MINIMAL_ICAL) + client.create_event("cal1", self._MINIMAL_EVENT) def test_create_event_passes_calendar_id(self, monkeypatch): resp = self._set_response(created={"new-0": {"id": "sv-2"}}) client, captured = self._capturing_client(monkeypatch, resp) - client.create_event("my-calendar", self._MINIMAL_ICAL) + client.create_event("my-calendar", self._MINIMAL_EVENT) method_calls = captured["json"]["methodCalls"] create_args = method_calls[0][1] event_payload = create_args["create"]["new-0"] assert event_payload.get("calendarIds") == {"my-calendar": True} + def test_create_event_does_not_mutate_caller_dict(self, monkeypatch): + resp = self._set_response(created={"new-0": {"id": "sv-3"}}) + client, _ = self._capturing_client(monkeypatch, resp) + event = dict(self._MINIMAL_EVENT) + event["id"] = "stale-id-from-a-different-event" + before = dict(event) + client.create_event("my-calendar", event) + assert event == before + + def test_create_event_strips_server_managed_properties(self, monkeypatch): + resp = self._set_response(created={"new-0": {"id": "sv-4"}}) + client, captured = self._capturing_client(monkeypatch, resp) + event = _event_with_stale_server_managed_properties(self._MINIMAL_EVENT, "stale-id") + client.create_event("my-calendar", event) + event_payload = captured["json"]["methodCalls"][0][1]["create"]["new-0"] + assert "id" not in event_payload + assert "baseEventId" not in event_payload + assert "isOrigin" not in event_payload + def test_get_event_returns_ical(self, monkeypatch): raw_event = { "id": "ev1", @@ -5625,25 +5684,53 @@ def test_get_event_uses_given_account_id(self, monkeypatch): def test_update_event_success(self, monkeypatch): resp = self._set_response(updated={"ev1": None}) client = _make_client_with_mocked_session(monkeypatch, resp) - client.update_event("ev1", self._MINIMAL_ICAL) + client.update_event("ev1", self._MINIMAL_EVENT) def test_update_event_raises_on_failure(self, monkeypatch): resp = self._set_response(notUpdated={"ev1": {"type": "notFound"}}) client = _make_client_with_mocked_session(monkeypatch, resp) with pytest.raises(JMAPMethodError) as exc_info: - client.update_event("ev1", self._MINIMAL_ICAL) + client.update_event("ev1", self._MINIMAL_EVENT) assert exc_info.value.error_type == "notFound" def test_update_event_drops_uid_from_patch(self, monkeypatch): resp = self._set_response(updated={"ev1": None}) client, captured = self._capturing_client(monkeypatch, resp) - client.update_event("ev1", self._MINIMAL_ICAL) + client.update_event("ev1", self._MINIMAL_EVENT) method_calls = captured["json"]["methodCalls"] update_args = method_calls[0][1] patch = update_args["update"]["ev1"] assert "uid" not in patch + def test_update_event_does_not_mutate_caller_dict(self, monkeypatch): + resp = self._set_response(updated={"ev1": None}) + client, _ = self._capturing_client(monkeypatch, resp) + event = dict(self._MINIMAL_EVENT) + before = dict(event) + client.update_event("ev1", event) + assert event == before + + def test_update_event_strips_server_managed_properties(self, monkeypatch): + resp = self._set_response(updated={"ev1": None}) + client, captured = self._capturing_client(monkeypatch, resp) + event = _event_with_stale_server_managed_properties(self._MINIMAL_EVENT, "ev1") + client.update_event("ev1", event) + patch = captured["json"]["methodCalls"][0][1]["update"]["ev1"] + assert "id" not in patch + assert "baseEventId" not in patch + assert "isOrigin" not in patch + + def test_update_event_keeps_calendar_ids_unchanged(self, monkeypatch): + # update_event has no calendar_id argument to override calendarIds + # with, unlike create_event; whatever the caller's dict already has + # passes through as-is. + resp = self._set_response(updated={"ev1": None}) + client, captured = self._capturing_client(monkeypatch, resp) + client.update_event("ev1", self._MINIMAL_EVENT) + patch = captured["json"]["methodCalls"][0][1]["update"]["ev1"] + assert patch["calendarIds"] == {"original-calendar": True} + def test_update_event_nulls_removed_optional_properties(self, monkeypatch): # RFC 8620 section 3.3: absent keys in a PatchObject preserve the server value. # To actually delete a property the patch must set it to null. @@ -5660,7 +5747,7 @@ def test_update_event_nulls_removed_optional_properties(self, monkeypatch): resp = self._set_response(updated={"ev1": None}) client, captured = self._capturing_client(monkeypatch, resp) # First, pretend the event had a location (we don't need to call create; just update) - client.update_event("ev1", _ICAL_WITHOUT_LOCATION) + client.update_event("ev1", ical_to_jscal(_ICAL_WITHOUT_LOCATION)) patch = captured["json"]["methodCalls"][0][1]["update"]["ev1"] # The patch must contain explicit null for 'locations' to remove it from the server assert "locations" in patch @@ -5715,7 +5802,7 @@ def reject(prop): self._set_response(updated={"ev1": None}), ] client, captured = self._sequence_client(responses) - client.update_event("ev1", self._MINIMAL_ICAL) + client.update_event("ev1", self._MINIMAL_EVENT) assert len(captured["patches"]) == 3 # First attempt nulled both recurrence keys; the final accepted patch dropped them. @@ -5737,7 +5824,7 @@ def test_update_event_does_not_drop_explicitly_set_property(self, monkeypatch): ) client, captured = self._sequence_client([resp]) with pytest.raises(JMAPMethodError) as exc_info: - client.update_event("ev1", self._MINIMAL_ICAL) + client.update_event("ev1", self._MINIMAL_EVENT) assert exc_info.value.error_type == "invalidProperties" assert len(captured["patches"]) == 1 # no retry @@ -5756,7 +5843,7 @@ def test_delete_event_raises_on_failure(self, monkeypatch): def test_update_event_uses_given_account_id(self, monkeypatch): resp = self._set_response(updated={"ev1": None}) client, captured = self._capturing_client(monkeypatch, resp) - client.update_event("ev1", self._MINIMAL_ICAL, account_id="owner-account") + client.update_event("ev1", self._MINIMAL_EVENT, account_id="owner-account") update_args = captured["json"]["methodCalls"][0][1] assert update_args["accountId"] == "owner-account" @@ -5777,14 +5864,14 @@ def test_delete_event_sends_scheduling_messages(self, monkeypatch): def test_create_event_passes_if_in_state(self, monkeypatch): resp = self._set_response(created={"new-0": {"id": "sv-1"}}) client, captured = self._capturing_client(monkeypatch, resp) - client.create_event("cal1", self._MINIMAL_ICAL, if_in_state="state-1") + client.create_event("cal1", self._MINIMAL_EVENT, if_in_state="state-1") create_args = captured["json"]["methodCalls"][0][1] assert create_args["ifInState"] == "state-1" def test_create_event_omits_if_in_state_when_not_passed(self, monkeypatch): resp = self._set_response(created={"new-0": {"id": "sv-1"}}) client, captured = self._capturing_client(monkeypatch, resp) - client.create_event("cal1", self._MINIMAL_ICAL) + client.create_event("cal1", self._MINIMAL_EVENT) create_args = captured["json"]["methodCalls"][0][1] assert "ifInState" not in create_args @@ -5792,23 +5879,39 @@ def test_create_event_raises_state_mismatch(self, monkeypatch): resp = {"methodResponses": [["error", {"type": "stateMismatch"}, "ev-set-create-0"]]} client = _make_client_with_mocked_session(monkeypatch, resp) with pytest.raises(JMAPMethodError) as exc_info: - client.create_event("cal1", self._MINIMAL_ICAL, if_in_state="stale-state") + client.create_event("cal1", self._MINIMAL_EVENT, if_in_state="stale-state") assert exc_info.value.error_type == "stateMismatch" + def test_create_event_returns_old_and_new_state(self, monkeypatch): + resp = self._set_response( + created={"new-0": {"id": "sv-1"}}, oldState="state-0", newState="state-1" + ) + client = _make_client_with_mocked_session(monkeypatch, resp) + _, old_state, new_state = client.create_event("cal1", self._MINIMAL_EVENT) + assert old_state == "state-0" + assert new_state == "state-1" + def test_update_event_passes_if_in_state(self, monkeypatch): resp = self._set_response(updated={"ev1": None}) client, captured = self._capturing_client(monkeypatch, resp) - client.update_event("ev1", self._MINIMAL_ICAL, if_in_state="state-1") + client.update_event("ev1", self._MINIMAL_EVENT, if_in_state="state-1") update_args = captured["json"]["methodCalls"][0][1] assert update_args["ifInState"] == "state-1" def test_update_event_omits_if_in_state_when_not_passed(self, monkeypatch): resp = self._set_response(updated={"ev1": None}) client, captured = self._capturing_client(monkeypatch, resp) - client.update_event("ev1", self._MINIMAL_ICAL) + client.update_event("ev1", self._MINIMAL_EVENT) update_args = captured["json"]["methodCalls"][0][1] assert "ifInState" not in update_args + def test_update_event_returns_old_and_new_state(self, monkeypatch): + resp = self._set_response(updated={"ev1": None}, oldState="state-1", newState="state-2") + client = _make_client_with_mocked_session(monkeypatch, resp) + old_state, new_state = client.update_event("ev1", self._MINIMAL_EVENT) + assert old_state == "state-1" + assert new_state == "state-2" + def test_delete_event_passes_if_in_state(self, monkeypatch): resp = self._set_response(destroyed=["ev1"]) client, captured = self._capturing_client(monkeypatch, resp) @@ -5826,20 +5929,20 @@ def test_delete_event_omits_if_in_state_when_not_passed(self, monkeypatch): def test_send_invite_passes_if_in_state(self, monkeypatch): resp = self._set_response(created={"new-0": {"id": "sv-1"}}) client, captured = self._capturing_client(monkeypatch, resp) - client.send_invite("cal1", self._MINIMAL_ICAL, if_in_state="state-1") + client.send_invite("cal1", self._MINIMAL_EVENT, if_in_state="state-1") create_args = captured["json"]["methodCalls"][0][1] assert create_args["ifInState"] == "state-1" def test_send_invite_returns_server_id(self, monkeypatch): resp = self._set_response(created={"new-0": {"id": "sv-1"}}) client = _make_client_with_mocked_session(monkeypatch, resp) - event_id = client.send_invite("cal1", self._MINIMAL_ICAL) + event_id, _, _ = client.send_invite("cal1", self._MINIMAL_EVENT) assert event_id == "sv-1" def test_send_invite_sends_scheduling_messages(self, monkeypatch): resp = self._set_response(created={"new-0": {"id": "sv-1"}}) client, captured = self._capturing_client(monkeypatch, resp) - client.send_invite("cal1", self._MINIMAL_ICAL) + client.send_invite("cal1", self._MINIMAL_EVENT) create_args = captured["json"]["methodCalls"][0][1] assert create_args["sendSchedulingMessages"] is True @@ -5847,7 +5950,7 @@ def test_send_invite_raises_no_supported_schedule_methods(self, monkeypatch): resp = self._set_response(notCreated={"new-0": {"type": "noSupportedScheduleMethods"}}) client = _make_client_with_mocked_session(monkeypatch, resp) with pytest.raises(JMAPMethodError) as exc_info: - client.send_invite("cal1", self._MINIMAL_ICAL) + client.send_invite("cal1", self._MINIMAL_EVENT) assert exc_info.value.error_type == "noSupportedScheduleMethods" def _participant_event_response(self, own_email="me@example.com", participant_id="p1"): @@ -7949,6 +8052,10 @@ class TestAsyncJMAPClient: ] ) + # JSCalendar equivalent of _MINIMAL_ICAL, what a caller passes to + # create_event/send_invite/update_event now that they take a dict. + _MINIMAL_EVENT = ical_to_jscal(_MINIMAL_ICAL, calendar_id="original-calendar") + _RAW_EVENT = { "id": "ev-async-1", "uid": "async-test-uid@example.com", @@ -8173,7 +8280,7 @@ async def test_set_default_alerts_raises_on_failure(self, monkeypatch): async def test_create_event_returns_id(self, monkeypatch): resp = self._event_set_resp(created={"new-0": {"id": "ev-new-1"}}, notCreated={}) self._patch_async_session(monkeypatch, resp) - event_id = await self._make_client().create_event("cal1", self._MINIMAL_ICAL) + event_id, _, _ = await self._make_client().create_event("cal1", self._MINIMAL_EVENT) assert event_id == "ev-new-1" @pytest.mark.asyncio @@ -8181,7 +8288,7 @@ async def test_create_event_raises_on_failure(self, monkeypatch): resp = self._event_set_resp(created={}, notCreated={"new-0": {"type": "invalidArguments"}}) self._patch_async_session(monkeypatch, resp) with pytest.raises(JMAPMethodError) as exc_info: - await self._make_client().create_event("cal1", self._MINIMAL_ICAL) + await self._make_client().create_event("cal1", self._MINIMAL_EVENT) assert exc_info.value.error_type == "invalidArguments" @pytest.mark.asyncio @@ -8212,14 +8319,14 @@ async def test_get_event_uses_given_account_id(self, monkeypatch): async def test_update_event_success(self, monkeypatch): resp = self._event_set_resp(updated={"ev-async-1": None}, notUpdated={}) self._patch_async_session(monkeypatch, resp) - await self._make_client().update_event("ev-async-1", self._MINIMAL_ICAL) + await self._make_client().update_event("ev-async-1", self._MINIMAL_EVENT) @pytest.mark.asyncio async def test_update_event_raises_on_failure(self, monkeypatch): resp = self._event_set_resp(updated={}, notUpdated={"ev-async-1": {"type": "notFound"}}) self._patch_async_session(monkeypatch, resp) with pytest.raises(JMAPMethodError) as exc_info: - await self._make_client().update_event("ev-async-1", self._MINIMAL_ICAL) + await self._make_client().update_event("ev-async-1", self._MINIMAL_EVENT) assert exc_info.value.error_type == "notFound" @pytest.mark.asyncio @@ -8227,7 +8334,7 @@ async def test_update_event_uses_given_account_id(self, monkeypatch): resp = self._event_set_resp(updated={"ev-async-1": None}, notUpdated={}) mock_http = self._patch_async_session(monkeypatch, resp) await self._make_client().update_event( - "ev-async-1", self._MINIMAL_ICAL, account_id="owner-account" + "ev-async-1", self._MINIMAL_EVENT, account_id="owner-account" ) update_args = mock_http.post.call_args.kwargs["json"]["methodCalls"][0][1] assert update_args["accountId"] == "owner-account" @@ -8259,7 +8366,7 @@ async def test_update_event_passes_if_in_state(self, monkeypatch): resp = self._event_set_resp(updated={"ev-async-1": None}, notUpdated={}) mock_http = self._patch_async_session(monkeypatch, resp) await self._make_client().update_event( - "ev-async-1", self._MINIMAL_ICAL, if_in_state="state-1" + "ev-async-1", self._MINIMAL_EVENT, if_in_state="state-1" ) update_args = mock_http.post.call_args.kwargs["json"]["methodCalls"][0][1] assert update_args["ifInState"] == "state-1" @@ -8268,10 +8375,22 @@ async def test_update_event_passes_if_in_state(self, monkeypatch): async def test_update_event_omits_if_in_state_when_not_passed(self, monkeypatch): resp = self._event_set_resp(updated={"ev-async-1": None}, notUpdated={}) mock_http = self._patch_async_session(monkeypatch, resp) - await self._make_client().update_event("ev-async-1", self._MINIMAL_ICAL) + await self._make_client().update_event("ev-async-1", self._MINIMAL_EVENT) update_args = mock_http.post.call_args.kwargs["json"]["methodCalls"][0][1] assert "ifInState" not in update_args + @pytest.mark.asyncio + async def test_update_event_returns_old_and_new_state(self, monkeypatch): + resp = self._event_set_resp( + updated={"ev-async-1": None}, notUpdated={}, oldState="s1", newState="s2" + ) + self._patch_async_session(monkeypatch, resp) + old_state, new_state = await self._make_client().update_event( + "ev-async-1", self._MINIMAL_EVENT + ) + assert old_state == "s1" + assert new_state == "s2" + @pytest.mark.asyncio async def test_delete_event_passes_if_in_state(self, monkeypatch): resp = self._event_set_resp(destroyed=["ev-async-1"], notDestroyed={}) @@ -8292,15 +8411,25 @@ async def test_delete_event_omits_if_in_state_when_not_passed(self, monkeypatch) async def test_create_event_omits_if_in_state_when_not_passed(self, monkeypatch): resp = self._event_set_resp(created={"new-0": {"id": "sv-1"}}) mock_http = self._patch_async_session(monkeypatch, resp) - await self._make_client().create_event("cal1", self._MINIMAL_ICAL) + await self._make_client().create_event("cal1", self._MINIMAL_EVENT) create_args = mock_http.post.call_args.kwargs["json"]["methodCalls"][0][1] assert "ifInState" not in create_args + @pytest.mark.asyncio + async def test_create_event_returns_old_and_new_state(self, monkeypatch): + resp = self._event_set_resp(created={"new-0": {"id": "sv-1"}}, oldState="s0", newState="s1") + self._patch_async_session(monkeypatch, resp) + _, old_state, new_state = await self._make_client().create_event( + "cal1", self._MINIMAL_EVENT + ) + assert old_state == "s0" + assert new_state == "s1" + @pytest.mark.asyncio async def test_send_invite_passes_if_in_state(self, monkeypatch): resp = self._event_set_resp(created={"new-0": {"id": "sv-1"}}) mock_http = self._patch_async_session(monkeypatch, resp) - await self._make_client().send_invite("cal1", self._MINIMAL_ICAL, if_in_state="state-1") + await self._make_client().send_invite("cal1", self._MINIMAL_EVENT, if_in_state="state-1") create_args = mock_http.post.call_args.kwargs["json"]["methodCalls"][0][1] assert create_args["ifInState"] == "state-1" @@ -8310,7 +8439,7 @@ async def test_create_event_raises_state_mismatch(self, monkeypatch): self._patch_async_session(monkeypatch, resp) with pytest.raises(JMAPMethodError) as exc_info: await self._make_client().create_event( - "cal1", self._MINIMAL_ICAL, if_in_state="stale-state" + "cal1", self._MINIMAL_EVENT, if_in_state="stale-state" ) assert exc_info.value.error_type == "stateMismatch" @@ -8326,14 +8455,14 @@ async def test_delete_event_raises_on_failure(self, monkeypatch): async def test_send_invite_returns_id(self, monkeypatch): resp = self._event_set_resp(created={"new-0": {"id": "sv-async-1"}}, notCreated={}) self._patch_async_session(monkeypatch, resp) - event_id = await self._make_client().send_invite("cal1", self._MINIMAL_ICAL) + event_id, _, _ = await self._make_client().send_invite("cal1", self._MINIMAL_EVENT) assert event_id == "sv-async-1" @pytest.mark.asyncio async def test_send_invite_sends_scheduling_messages(self, monkeypatch): resp = self._event_set_resp(created={"new-0": {"id": "sv-async-1"}}, notCreated={}) mock_http = self._patch_async_session(monkeypatch, resp) - await self._make_client().send_invite("cal1", self._MINIMAL_ICAL) + await self._make_client().send_invite("cal1", self._MINIMAL_EVENT) create_args = mock_http.post.call_args.kwargs["json"]["methodCalls"][0][1] assert create_args["sendSchedulingMessages"] is True @@ -9126,20 +9255,69 @@ async def capturing_post(*args, **kwargs): async def test_create_event_passes_calendar_id(self, monkeypatch): resp = self._event_set_resp(created={"new-0": {"id": "ev-new-1"}}, notCreated={}) client, captured = self._capturing_async_session(monkeypatch, resp) - await client.create_event("my-cal", self._MINIMAL_ICAL) + await client.create_event("my-cal", self._MINIMAL_EVENT) create_args = captured["json"]["methodCalls"][0][1] new_event = create_args["create"]["new-0"] assert "my-cal" in new_event.get("calendarIds", {}) + @pytest.mark.asyncio + async def test_create_event_does_not_mutate_caller_dict(self, monkeypatch): + resp = self._event_set_resp(created={"new-0": {"id": "ev-new-1"}}, notCreated={}) + client, _ = self._capturing_async_session(monkeypatch, resp) + event = dict(self._MINIMAL_EVENT) + event["id"] = "stale-id-from-a-different-event" + before = dict(event) + await client.create_event("my-cal", event) + assert event == before + + @pytest.mark.asyncio + async def test_create_event_strips_server_managed_properties(self, monkeypatch): + resp = self._event_set_resp(created={"new-0": {"id": "ev-new-1"}}, notCreated={}) + client, captured = self._capturing_async_session(monkeypatch, resp) + event = _event_with_stale_server_managed_properties(self._MINIMAL_EVENT, "stale-id") + await client.create_event("my-cal", event) + event_payload = captured["json"]["methodCalls"][0][1]["create"]["new-0"] + assert "id" not in event_payload + assert "baseEventId" not in event_payload + assert "isOrigin" not in event_payload + @pytest.mark.asyncio async def test_update_event_drops_uid_from_patch(self, monkeypatch): resp = self._event_set_resp(updated={"ev-async-1": None}, notUpdated={}) client, captured = self._capturing_async_session(monkeypatch, resp) - await client.update_event("ev-async-1", self._MINIMAL_ICAL) + await client.update_event("ev-async-1", self._MINIMAL_EVENT) update_args = captured["json"]["methodCalls"][0][1] patch = update_args["update"]["ev-async-1"] assert "uid" not in patch + @pytest.mark.asyncio + async def test_update_event_does_not_mutate_caller_dict(self, monkeypatch): + resp = self._event_set_resp(updated={"ev-async-1": None}, notUpdated={}) + client, _ = self._capturing_async_session(monkeypatch, resp) + event = dict(self._MINIMAL_EVENT) + before = dict(event) + await client.update_event("ev-async-1", event) + assert event == before + + @pytest.mark.asyncio + async def test_update_event_strips_server_managed_properties(self, monkeypatch): + resp = self._event_set_resp(updated={"ev-async-1": None}, notUpdated={}) + client, captured = self._capturing_async_session(monkeypatch, resp) + event = _event_with_stale_server_managed_properties(self._MINIMAL_EVENT, "ev-async-1") + await client.update_event("ev-async-1", event) + patch = captured["json"]["methodCalls"][0][1]["update"]["ev-async-1"] + assert "id" not in patch + assert "baseEventId" not in patch + assert "isOrigin" not in patch + + @pytest.mark.asyncio + async def test_update_event_keeps_calendar_ids_unchanged(self, monkeypatch): + resp = self._event_set_resp(updated={"ev-async-1": None}, notUpdated={}) + client, captured = self._capturing_async_session(monkeypatch, resp) + await client.update_event("ev-async-1", self._MINIMAL_EVENT) + patch = captured["json"]["methodCalls"][0][1]["update"]["ev-async-1"] + assert patch["calendarIds"] == {"original-calendar": True} + @pytest.mark.asyncio async def test_search_events_passes_calendar_id_filter(self, monkeypatch): client, captured = self._capturing_async_session( @@ -10038,7 +10216,7 @@ async def post(*args, **kwargs): mock_http.post = post monkeypatch.setattr("calendaring_jmap.async_client.AsyncSession", lambda: mock_http) - await self._make_client().update_event("ev-async-1", self._MINIMAL_ICAL) + await self._make_client().update_event("ev-async-1", self._MINIMAL_EVENT) assert len(patches_seen) == 2 assert patches_seen[0]["recurrenceRule"] is None assert "recurrenceRule" not in patches_seen[1]