diff --git a/CHANGELOG.md b/CHANGELOG.md index 9696dd9b..db7dd1f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,7 +40,7 @@ The JMAP support was declared experimental in 3.0, hence the changes below are d ### Fixed * The async connection-abort workaround now tries the existing one-shot Basic-auth fallback when its probe returns a bare 401 over TLS. Unsafe or unsuccessful probes still propagate the original connection error. See https://github.com/python-caldav/caldav/issues/717. - +* `search(start=..., end=..., expand=True)` without `event=True` (or another component type) returned only the first occurrence of a recurring event - a regression in v3.3.0. The comp-type-less search is split into one query per component type, and the results were deduplicated by URL, which all expanded occurrences of an event share. Passing `event=True` or `compatibility_workarounds=False` avoided it. See https://github.com/python-caldav/caldav/issues/722. * `Principal.get_vcal_address()` raised `IndexError: list index out of range` when the server returned an empty `calendar-user-address-set`. It now falls back to the principal URL, as RFC 6638 section 2.4.1 provides for a user with no well-defined calendar user address. `add_organizer()` and `add_attendee()` go through the same method, so they were affected too. Seen on Xandikos 0.4.7, which advertises `calendar-auto-schedule` and serves schedule-inbox/outbox, but leaves the address set empty. `change_attendee_status()` accepts that same URL back, so an event the library invited a principal to can still have its PARTSTAT changed. A property that is *absent* still raises `NotFoundError`; per the same section that means the user is not enabled for scheduling. En passant, the Xandikos profile is regraded for 0.4.7: scheduling is no longer declared unsupported, and `create-calendar.with-supported-component-types` no longer unsupported either, so `is_supported()` may answer differently with `features: xandikos` configured. diff --git a/caldav/search.py b/caldav/search.py index 061a336c..a179963d 100644 --- a/caldav/search.py +++ b/caldav/search.py @@ -235,19 +235,26 @@ def _build_search_xml_query( return (root, comp_class) -def _dedup_by_url(matches: list) -> list: +def _dedup_resources(matches: list) -> list: """Drop repeated resources, keeping the first occurrence and the order. A search that is split into several server queries can return the same resource more than once: the include-completed split issues overlapping queries, and in a comp-type split a resource that legally holds both a VEVENT and a VTODO matches two of the three queries. + + The RECURRENCE-ID is part of the key: with ``expand=True`` every + occurrence of a recurring event is a separate object carrying the URL of + the resource, and those must all be kept. + See https://github.com/python-caldav/caldav/issues/722 """ objects = [] seen = set() for item in matches: - if item.url not in seen: - seen.add(item.url) + recurrence_id = item.icalendar_component.get("RECURRENCE-ID") + key = (item.url, recurrence_id.to_ical() if recurrence_id is not None else None) + if key not in seen: + seen.add(key) objects.append(item) return objects @@ -760,7 +767,7 @@ def _search_impl( (clone, calendar, server_expand, False, props, xml, None, _hacks), ) - objects = _dedup_by_url(matches) + objects = _dedup_resources(matches) else: orig_xml = xml @@ -1062,7 +1069,7 @@ def _search_with_comptypes( objects += clone.search( calendar, server_expand, split_expanded, props, xml, post_filter, _hacks ) - return self.sort(_dedup_by_url(objects)) + return self.sort(_dedup_resources(objects)) async def async_search( self, @@ -1162,7 +1169,7 @@ async def _async_search_with_comptypes( calendar, server_expand, split_expanded, props, xml, post_filter, _hacks ) objects.extend(results) - return self.sort(_dedup_by_url(objects)) + return self.sort(_dedup_resources(objects)) def filter( self, diff --git a/docs/source/http-libraries.rst b/docs/source/http-libraries.rst index 1d662a0b..17b901c4 100644 --- a/docs/source/http-libraries.rst +++ b/docs/source/http-libraries.rst @@ -1,7 +1,7 @@ HTTP Library Configuration ========================== -As of v3.x, **niquests** is the preferred, recommended and supported library for HTTP communication. niquests is a backwards-compatible fork of the requests library. It's a modern HTTP library with support for HTTP/2 and HTTP/3 and many other things. +As of v3.x, **niquests** is the preferred, recommended and supported library for HTTP communication. niquests is a backwards-compatible fork of the requests library. It's a modern HTTP library with support for HTTP/2 and HTTP/3 and many other things. It works for me - but there may be some sharp edges. Due to popular demand, fallbacks to **requests** and to the **httpx** family (httpx, httpxyz, httpx2) exist. diff --git a/tests/test_search.py b/tests/test_search.py index ea1f1e42..33ff7cd3 100644 --- a/tests/test_search.py +++ b/tests/test_search.py @@ -896,6 +896,47 @@ def mock_is_supported(feat, type_=bool): calendar._request_report_build_resultlist.assert_called_once_with(full_xml, None, None) +class TestDedupResources: + """The helper that merges the results of a split search.""" + + def test_regenerated_copies_of_one_resource_collapse( + self, mock_client: DAVClient, mock_url: str + ) -> None: + """Two sub-queries can return the same resource with content the + server regenerated in between (a fresh DTSTAMP, say); that is still + one resource.""" + from caldav.search import _dedup_resources + + first = Event(client=mock_client, url=mock_url, data=SIMPLE_EVENT) + second = Event( + client=mock_client, + url=mock_url, + data=SIMPLE_EVENT.replace("DTSTAMP:20240101T120000Z", "DTSTAMP:20240102T120000Z"), + ) + + assert _dedup_resources([first, second]) == [first] + + def test_occurrences_of_one_resource_are_kept( + self, mock_client: DAVClient, mock_url: str + ) -> None: + """Expanded occurrences share the URL but differ in RECURRENCE-ID.""" + from caldav.search import _dedup_resources + + occurrences = [ + Event( + client=mock_client, + url=mock_url, + data=SIMPLE_EVENT.replace( + "DTSTART:20240615T140000Z", + f"DTSTART:202406{day}T140000Z\nRECURRENCE-ID:202406{day}T140000Z", + ), + ) + for day in (15, 22) + ] + + assert _dedup_resources(occurrences + occurrences) == occurrences + + class TestCompTypeLessSearchSplit: """Gate findings F10 and F11: the comp-type split path. @@ -1028,6 +1069,57 @@ def rep(xml, comp_cls, props=None): ## split into one query per component type (VEVENT/VTODO/VJOURNAL) assert len(calls) == 3 + @staticmethod + def _expanded_split_calendar(mock_client: DAVClient, mock_url: str, report) -> mock.Mock: + """A calendar whose comp-type split finds RECURRING_EVENT in the + VEVENT query only. `report` is `mock.Mock` or `mock.AsyncMock`.""" + from caldav.compatibility_hints import FeatureSet + + mock_client.features = FeatureSet(None) + calendar = mock.Mock() + calendar.client = mock_client + + def rep(xml, comp_cls, props=None): + if comp_cls is not Event: + return (mock.Mock(), []) + return (mock.Mock(), [Event(client=mock_client, url=mock_url, data=RECURRING_EVENT)]) + + calendar._request_report_build_resultlist = report(side_effect=rep) + calendar._async_batch_load_objects = mock.AsyncMock() + return calendar + + _JUNE_2024 = { + "start": datetime(2024, 6, 1, tzinfo=timezone.utc), + "end": datetime(2024, 7, 1, tzinfo=timezone.utc), + "expand": True, + } + + @staticmethod + def _assert_three_distinct_occurrences(result: list) -> None: + assert all("RRULE" not in o.data for o in result) + starts = sorted(o.icalendar_component["DTSTART"].dt for o in result) + assert starts == [datetime(2024, 6, day, 10, tzinfo=timezone.utc) for day in (1, 8, 15)] + + def test_untyped_expanded_search_keeps_all_occurrences( + self, mock_client: DAVClient, mock_url: str + ) -> None: + """https://github.com/python-caldav/caldav/issues/722: expanded + occurrences share the URL of their resource, so deduplicating the + comp-type split by URL alone dropped all but the first occurrence.""" + calendar = self._expanded_split_calendar(mock_client, mock_url, mock.Mock) + result = CalDAVSearcher(**self._JUNE_2024).search(calendar) + self._assert_three_distinct_occurrences(result) + + def test_untyped_expanded_async_search_keeps_all_occurrences( + self, mock_client: DAVClient, mock_url: str + ) -> None: + """Async twin of the issue-722 test above.""" + import asyncio + + calendar = self._expanded_split_calendar(mock_client, mock_url, mock.AsyncMock) + result = asyncio.run(CalDAVSearcher(**self._JUNE_2024).async_search(calendar)) + self._assert_three_distinct_occurrences(result) + def test_reactive_workaround_on_vcalendar_timerange_rejection( self, mock_client: DAVClient, mock_url: str ) -> None: