From 7318a850f8b02633011044c13ae104785dba1687 Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Sat, 4 Apr 2026 22:13:48 +0200 Subject: [PATCH 1/2] fix: remove search.text.by-uid compatibility feature There seems to be no servers allowing search by uid (basically a text search) without allowing other text searches and vice versa - so the search.text.by-uid "feature" is moot, and has been removed in this commit. get_object_by_uid() already has a client-side fallback via _hacks="insist" that fetches all objects and filters client-side when the server returns no results from text search. The search.text.by-uid guard in the tests was therefore no longer needed, and all skip_unless_support/is_supported checks in tests have been removed. Some tests now failed for other reasons, so the tests have been made a bit more robust. Closes https://github.com/python-caldav/caldav/issues/586 Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 4 ++ caldav/calendarobjectresource.py | 2 +- caldav/collection.py | 13 +++++-- caldav/compatibility_hints.py | 8 ++-- caldav/search.py | 10 ++++- tests/test_caldav.py | 61 ++++++++++++++----------------- tests/test_compatibility_hints.py | 10 ++--- 7 files changed, 57 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d153169a..1f2da59f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ This project should adhere to [Semantic Versioning](https://semver.org/spec/v2.0 ## [Unreleased] +### Removed + +* Compatibility feature `search.text.by-uid` has been removed. `get_object_by_uid()` already has a client-side fallback (via `_hacks="insist"`) that works on any server, so the guard was no longer needed. Closes https://github.com/python-caldav/caldav/issues/586 + ### Fixed * Reusing a `CalDAVSearcher` across multiple `search()` calls could yield inconsistent results: the first call would return only pending tasks (correct), but subsequent calls would change behaviour because `icalendar_searcher.Searcher.check_component()` mutated the `include_completed` field from `None` to `False` as a side-effect. Fixed by passing a copy with `include_completed` already resolved to `filter_search_results()`, leaving the original searcher object unchanged. Fixes https://github.com/python-caldav/caldav/issues/650 diff --git a/caldav/calendarobjectresource.py b/caldav/calendarobjectresource.py index a4d9c6c9..cc28f735 100644 --- a/caldav/calendarobjectresource.py +++ b/caldav/calendarobjectresource.py @@ -1125,7 +1125,7 @@ def save( def get_self(): from caldav.lib import error - uid = self.id or self.icalendar_component.get("uid") + uid = self.id if uid and self.parent: try: if not obj_type: diff --git a/caldav/collection.py b/caldav/collection.py index 5827f6cc..c19a4c7b 100644 --- a/caldav/collection.py +++ b/caldav/collection.py @@ -1508,7 +1508,12 @@ def get_object_by_uid( ## apply an exact match filter afterwards to preserve the semantics of ## this method (see testObjectByUID). items_found = self.search( - uid=uid, comp_class=comp_class, xml=comp_filter, post_filter=True, _hacks="insist" + uid=uid, + comp_class=comp_class, + xml=comp_filter, + post_filter=True, + _hacks="insist", + include_completed=True, ) items_found = [o for o in items_found if o.id == uid] @@ -1541,7 +1546,7 @@ def get_todo_by_uid(self, uid: str) -> "CalendarObjectResource": Returns the task with the given uid. See :meth:`get_object_by_uid` for more details. """ - return self.get_object_by_uid(uid, comp_filter=cdav.CompFilter("VTODO")) + return self.get_object_by_uid(uid, comp_class=Todo) def get_event_by_uid(self, uid: str) -> "CalendarObjectResource": """ @@ -1550,7 +1555,7 @@ def get_event_by_uid(self, uid: str) -> "CalendarObjectResource": Returns the event with the given uid. See :meth:`get_object_by_uid` for more details. """ - return self.get_object_by_uid(uid, comp_filter=cdav.CompFilter("VEVENT")) + return self.get_object_by_uid(uid, comp_class=Event) def get_journal_by_uid(self, uid: str) -> "CalendarObjectResource": """ @@ -1559,7 +1564,7 @@ def get_journal_by_uid(self, uid: str) -> "CalendarObjectResource": Returns the journal with the given uid. See :meth:`get_object_by_uid` for more details. """ - return self.get_object_by_uid(uid, comp_filter=cdav.CompFilter("VJOURNAL")) + return self.get_object_by_uid(uid, comp_class=Journal) ## Deprecated aliases - use get_*_by_uid instead diff --git a/caldav/compatibility_hints.py b/caldav/compatibility_hints.py index be9f506f..1e8f9938 100644 --- a/caldav/compatibility_hints.py +++ b/caldav/compatibility_hints.py @@ -221,9 +221,6 @@ class FeatureSet: "search.text.category.substring": { "description": "Substring search for category should work according to the RFC. I.e., search for mil should match family,finance", }, - "search.text.by-uid": { - "description": "The server supports searching for objects by UID property. When unsupported, calendar.get_object_by_uid(uid) will not work. This may be removed in the feature - the checker-script is not checking the right thing (check TODO-comments), probably search by uid is no special case for any server implementations" - }, "search.recurrences": { "description": "Support for recurrences in search" }, @@ -1034,6 +1031,7 @@ def dotted_feature_set_list(self, compact=False): 'old_flags': [ 'propfind_allprop_failure', 'duplicates_not_allowed', + 'no_relships' ## relships seems to work as long as it's one RELATED-TO-line, but as soon as there are multiple lines the implementation seems broken ], } @@ -1219,7 +1217,6 @@ def dotted_feature_set_list(self, compact=False): "search.time-range.todo": { "support": "unsupported" }, "search.time-range.alarm": {'support': 'unsupported'}, "search.text": { "support": "unsupported", "behaviour": "a text search ignores the filter and returns all elements" }, - "search.text.by-uid": { "support": "fragile", "behaviour": "Probably not supported, but my caldav-server-checker tool has issues with it at the moment" }, "search.comp-type.optional": { "support": "ungraceful" }, "search.recurrences.expanded.todo": { "support": "unsupported" }, "search.recurrences.expanded.event": { "support": "fragile" }, @@ -1495,6 +1492,9 @@ def dotted_feature_set_list(self, compact=False): 'principal-search.list-all': {'support': 'unsupported'}, ## Cross-calendar duplicate UID test fails (AuthorizationError creating second calendar) 'save.duplicate-uid.cross-calendar': {'support': 'ungraceful'}, + 'old_flags': [ + 'no_relships', + ], } # fmt: on diff --git a/caldav/search.py b/caldav/search.py index 856df01e..3661860a 100644 --- a/caldav/search.py +++ b/caldav/search.py @@ -233,6 +233,8 @@ def _search_impl( ): """Core search implementation as a generator yielding actions. + (TODO: refactoring beyond readability? Is this sane?) + This generator contains all the search logic and yields (action, data) tuples that the caller (sync or async) executes. Results are sent back via .send(). @@ -451,7 +453,7 @@ def _search_impl( ## we wouldn't waste so much time on repeated queries if self.todo and self.include_completed is False: clone = replace(self, include_completed=True) - clone.include_completed = True + clone.include_completed = True ## Why? Isn't this redundant? clone.expand = False if ( @@ -618,7 +620,7 @@ def search( :param calendar: Calendar to be searched (optional if searcher was created from a calendar via ``calendar.searcher()``) :param server_expand: Ask the CalDAV server to expand recurrences - :param split_expanded: Don't collect a recurrence set in one ical calendar + :param split_expanded: Send expanded recurrences as multiple objects :param props: CalDAV properties to send in the query :param xml: XML query to be sent to the server (string or elements) :param post_filter: Do client-side filtering after querying the server @@ -699,6 +701,10 @@ def _search_with_comptypes( assert self.event is None and self.todo is None and self.journal is None for comp_class in (Event, Todo, Journal): + if not calendar.client.features.is_supported( + f"save-load.{comp_class.__name__.lower()}" + ): + continue clone = replace(self) clone.comp_class = comp_class objects += clone.search( diff --git a/tests/test_caldav.py b/tests/test_caldav.py index e9a1b4a2..86967532 100644 --- a/tests/test_caldav.py +++ b/tests/test_caldav.py @@ -1116,7 +1116,7 @@ def testFindCalendarOwner(self): ## TODO: something should probably be asserted about the Owner def testIssue397(self): - self.skip_unless_support("search.text.by-uid") + self.skip_unless_support("save-load.event.recurrences.exception") cal = self._fixCalendar() cal.add_event( """BEGIN:VCALENDAR @@ -1289,7 +1289,6 @@ def testChangeAttendeeStatusWithEmailGiven(self): ) event.change_attendee_status(attendee="testuser@example.com", PARTSTAT="ACCEPTED") event.save() - self.skip_unless_support("search.text.by-uid") event = c.get_event_by_uid("test1") ## TODO: work in progress ... see https://github.com/python-caldav/caldav/issues/399 @@ -1456,7 +1455,6 @@ def testObjectByUID(self): """ It should be possible to save a task and retrieve it by uid """ - self.skip_unless_support("search.text.by-uid") c = self._fixCalendar(supported_calendar_component_set=["VTODO"]) c.add_todo(summary="Some test task with a well-known uid", uid="well_known_1") foo = c.get_object_by_uid("well_known_1") @@ -2228,7 +2226,6 @@ def testWrongPassword(self): def testCreateChildParent(self): self.skip_unless_support("save-load.event") self.skip_on_compatibility_flag("no_relships") - self.skip_unless_support("search.text.by-uid") c = self._fixCalendar(supported_calendar_component_set=["VEVENT"]) parent = c.add_event( dtstart=datetime(2022, 12, 26, 19, 15), @@ -2356,7 +2353,7 @@ def testSetDue(self): dtstart=datetime(2022, 12, 26, 19, 15, tzinfo=utc), due=datetime(2022, 12, 26, 20, 00, tzinfo=utc), summary="Some task", - uid="ctuid1", + uid="ctuid5", ) ## setting the due should ... set the due (surprise, surprise) @@ -2378,7 +2375,7 @@ def testSetDue(self): dtstart=datetime(2022, 12, 26, 19, 15, tzinfo=utc), duration=timedelta(minutes=15), summary="Some other task", - uid="ctuid2", + uid="ctuid6", ) some_other_todo.set_due(datetime(2022, 12, 26, 19, 45, tzinfo=utc), move_dtstart=True) assert some_other_todo.icalendar_component["DUE"].dt == datetime( @@ -2391,19 +2388,22 @@ def testSetDue(self): some_todo.save() self.skip_on_compatibility_flag("no_relships") - self.skip_unless_support("search.text.by-uid") + parent = c.add_todo( dtstart=datetime(2022, 12, 26, 19, 00, tzinfo=utc), due=datetime(2022, 12, 26, 21, 00, tzinfo=utc), summary="this is a parent test task", - uid="ctuid3", + uid="ctuid7", child=[some_todo.id], ) + ## The check_reverse_relations method is cheeky, + ## returning a list of non-behaving relations + ## (so it SHOULD return an empty list) assert not parent.check_reverse_relations() - ## The above updates the some_todo object on the server side, but the local object is not - ## updated ... until we reload it + ## The above updates the some_todo object on the server side, + ## but the local object is not updated ... until we reload it some_todo.load() ## This should work out (set the children due to some time before the parents due) @@ -2468,7 +2468,6 @@ def testCreateJournalListAndJournalEntry(self): j1 = c.add_journal(journal) journals = c.get_journals() assert len(journals) == 1 - self.skip_unless_support("search.text.by-uid") j1_ = c.get_journal_by_uid(j1.id) ## Direct comparison handles different line folding from different fetch methods assert j1_.get_icalendar_instance() == journals[0].get_icalendar_instance() @@ -2836,11 +2835,10 @@ def testTodoCompletion(self): # The historic todo-item can still be accessed todos = c.get_todos(include_completed=True) assert len(todos) == 3 - if self.is_supported("search.text.by-uid"): - t3_ = c.get_todo_by_uid(t3.id) - assert t3_.vobject_instance.vtodo.summary == t3.vobject_instance.vtodo.summary - assert t3_.vobject_instance.vtodo.uid == t3.vobject_instance.vtodo.uid - assert t3_.vobject_instance.vtodo.dtstart == t3.vobject_instance.vtodo.dtstart + t3_ = c.get_todo_by_uid(t3.id) + assert t3_.vobject_instance.vtodo.summary == t3.vobject_instance.vtodo.summary + assert t3_.vobject_instance.vtodo.uid == t3.vobject_instance.vtodo.uid + assert t3_.vobject_instance.vtodo.dtstart == t3.vobject_instance.vtodo.dtstart t2.delete() @@ -3073,10 +3071,9 @@ def testLookupEvent(self): e2 = c.event_by_url(e1.url) assert e2.vobject_instance.vevent.uid == e1.vobject_instance.vevent.uid assert e2.url == e1.url - if self.is_supported("search.text.by-uid"): - e3 = c.get_event_by_uid("20010712T182145Z-123401@example.com") - assert e3.vobject_instance.vevent.uid == e1.vobject_instance.vevent.uid - assert e3.url == e1.url + e3 = c.get_event_by_uid("20010712T182145Z-123401@example.com") + assert e3.vobject_instance.vevent.uid == e1.vobject_instance.vevent.uid + assert e3.url == e1.url # Knowing the URL of an event, we should be able to get to it # without going through a calendar object @@ -3100,10 +3097,9 @@ def testCreateOverwriteDeleteEvent(self): c = self._fixCalendar() assert c.url is not None - # attempts on updating/overwriting a non-existing event should fail (unless get_object_by_uid_is_broken): - if self.is_supported("search.text.by-uid"): - with pytest.raises(error.ConsistencyError): - c.add_event(ev1, no_create=True) + # attempts on updating/overwriting a non-existing event should fail: + with pytest.raises(error.ConsistencyError): + c.add_event(ev1, no_create=True) # no_create and no_overwrite is mutually exclusive, this will always # raise an error (unless the ical given is blank) @@ -3121,11 +3117,9 @@ def testCreateOverwriteDeleteEvent(self): assert t1.url is not None if not self.check_compatibility_flag("event_by_url_is_broken"): assert c.event_by_url(e1.url).url == e1.url - if self.is_supported("search.text.by-uid"): - assert c.get_event_by_uid(e1.id).url == e1.url + assert c.get_event_by_uid(e1.id).url == e1.url - ## no_create will not work unless get_object_by_uid works - no_create = self.is_supported("search.text.by-uid") + no_create = True ## add same event again. As it has same uid, it should be overwritten ## (but some calendars may throw a "409 Conflict") @@ -3155,13 +3149,12 @@ def testCreateOverwriteDeleteEvent(self): e3 = c.event_by_url(e1.url) assert e3.vobject_instance.vevent.summary.value == "Bastille Day Party!" - ## "no_overwrite" should throw a ConsistencyError. But it depends on get_object_by_uid. - if self.is_supported("search.text.by-uid"): + ## "no_overwrite" should throw a ConsistencyError. + with pytest.raises(error.ConsistencyError): + c.add_event(ev1, no_overwrite=True) + if todo_ok: with pytest.raises(error.ConsistencyError): - c.add_event(ev1, no_overwrite=True) - if todo_ok: - with pytest.raises(error.ConsistencyError): - c.add_todo(todo, no_overwrite=True) + c.add_todo(todo, no_overwrite=True) # delete event e1.delete() diff --git a/tests/test_compatibility_hints.py b/tests/test_compatibility_hints.py index 401b96e2..3277871f 100644 --- a/tests/test_compatibility_hints.py +++ b/tests/test_compatibility_hints.py @@ -375,8 +375,8 @@ def test_hierarchical_vs_independent_subfeatures(self) -> None: def test_intermediate_feature_derives_from_children(self) -> None: """Test that intermediate features (e.g. search.text) derive status from their children""" - # search.text has 5 children: case-sensitive, case-insensitive, - # substring, category, by-uid (none have explicit defaults) + # search.text has 4 direct children: case-sensitive, case-insensitive, + # substring, category (none have explicit defaults) # All children set with mixed statuses -> derive "unknown" fs = FeatureSet( @@ -384,8 +384,7 @@ def test_intermediate_feature_derives_from_children(self) -> None: "search.text.case-sensitive": {"support": "unsupported"}, "search.text.case-insensitive": {"support": "unsupported"}, "search.text.substring": {"support": "unsupported"}, - "search.text.category": {"support": "unsupported"}, - "search.text.by-uid": {"support": "fragile"}, + "search.text.category": {"support": "fragile"}, } ) assert not fs.is_supported("search.text") @@ -396,7 +395,7 @@ def test_intermediate_feature_derives_from_children(self) -> None: fs1b = FeatureSet( { "search.text.case-sensitive": {"support": "unsupported"}, - "search.text.by-uid": {"support": "fragile"}, + "search.text.category.substring": {"support": "fragile"}, } ) assert fs1b.is_supported("search.text") @@ -408,7 +407,6 @@ def test_intermediate_feature_derives_from_children(self) -> None: "search.text.case-insensitive": {"support": "unsupported"}, "search.text.substring": {"support": "unsupported"}, "search.text.category": {"support": "unsupported"}, - "search.text.by-uid": {"support": "unsupported"}, } ) assert not fs2.is_supported("search.text") From 7c79856bdd23130f35b2a0636a49563a9537b8ef Mon Sep 17 00:00:00 2001 From: Tobias Brox Date: Sun, 5 Apr 2026 20:30:53 +0200 Subject: [PATCH 2/2] ci: pin cyrus-docker-test-server to last known-good digest The cyrusimap/cyrus-docker-test-server image was rebuilt on 2026-04-03 with a multi-stage slim build that broke CalDAV startup: the httpd module never comes up on port 8080, causing the health check to fail before CI can even run. Pin to the digest that was current on 2026-03-30 (confirmed passing in GitHub Actions run 23748898521) until the upstream image is fixed. --- .github/workflows/tests.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 48555ffc..cfb20413 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -40,7 +40,10 @@ jobs: --health-retries 5 --health-start-period 60s cyrus: - image: ghcr.io/cyrusimap/cyrus-docker-test-server:latest + # Pinned to last known-good build (pre-April-2026 multi-stage rebuild + # that broke CalDAV startup). Unpin once upstream is fixed. + # Working digest confirmed in CI run 23748898521 (2026-03-30). + image: ghcr.io/cyrusimap/cyrus-docker-test-server@sha256:d639a9116691a7a1c875073486c419d60843e5ef8e32e65c5ef56283874dbf2c ports: - 8802:8080 - 8001:8001