Skip to content

Commit ab9f880

Browse files
tobixenclaude
andcommitted
fix: remove search.text.by-uid compatibility feature
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 was therefore no longer needed, and all skip_unless_support/is_supported checks in tests have been removed. Closes #586 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 04d3fa5 commit ab9f880

File tree

7 files changed

+57
-51
lines changed

7 files changed

+57
-51
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ This project should adhere to [Semantic Versioning](https://semver.org/spec/v2.0
1414

1515
## [Unreleased]
1616

17+
### Removed
18+
19+
* 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
20+
1721
### Fixed
1822

1923
* 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

caldav/calendarobjectresource.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1125,7 +1125,7 @@ def save(
11251125
def get_self():
11261126
from caldav.lib import error
11271127

1128-
uid = self.id or self.icalendar_component.get("uid")
1128+
uid = self.id
11291129
if uid and self.parent:
11301130
try:
11311131
if not obj_type:

caldav/collection.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1508,7 +1508,12 @@ def get_object_by_uid(
15081508
## apply an exact match filter afterwards to preserve the semantics of
15091509
## this method (see testObjectByUID).
15101510
items_found = self.search(
1511-
uid=uid, comp_class=comp_class, xml=comp_filter, post_filter=True, _hacks="insist"
1511+
uid=uid,
1512+
comp_class=comp_class,
1513+
xml=comp_filter,
1514+
post_filter=True,
1515+
_hacks="insist",
1516+
include_completed=True,
15121517
)
15131518
items_found = [o for o in items_found if o.id == uid]
15141519

@@ -1541,7 +1546,7 @@ def get_todo_by_uid(self, uid: str) -> "CalendarObjectResource":
15411546
Returns the task with the given uid.
15421547
See :meth:`get_object_by_uid` for more details.
15431548
"""
1544-
return self.get_object_by_uid(uid, comp_filter=cdav.CompFilter("VTODO"))
1549+
return self.get_object_by_uid(uid, comp_class=Todo)
15451550

15461551
def get_event_by_uid(self, uid: str) -> "CalendarObjectResource":
15471552
"""
@@ -1550,7 +1555,7 @@ def get_event_by_uid(self, uid: str) -> "CalendarObjectResource":
15501555
Returns the event with the given uid.
15511556
See :meth:`get_object_by_uid` for more details.
15521557
"""
1553-
return self.get_object_by_uid(uid, comp_filter=cdav.CompFilter("VEVENT"))
1558+
return self.get_object_by_uid(uid, comp_class=Event)
15541559

15551560
def get_journal_by_uid(self, uid: str) -> "CalendarObjectResource":
15561561
"""
@@ -1559,7 +1564,7 @@ def get_journal_by_uid(self, uid: str) -> "CalendarObjectResource":
15591564
Returns the journal with the given uid.
15601565
See :meth:`get_object_by_uid` for more details.
15611566
"""
1562-
return self.get_object_by_uid(uid, comp_filter=cdav.CompFilter("VJOURNAL"))
1567+
return self.get_object_by_uid(uid, comp_class=Journal)
15631568

15641569
## Deprecated aliases - use get_*_by_uid instead
15651570

caldav/compatibility_hints.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -221,9 +221,6 @@ class FeatureSet:
221221
"search.text.category.substring": {
222222
"description": "Substring search for category should work according to the RFC. I.e., search for mil should match family,finance",
223223
},
224-
"search.text.by-uid": {
225-
"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"
226-
},
227224
"search.recurrences": {
228225
"description": "Support for recurrences in search"
229226
},
@@ -1034,6 +1031,7 @@ def dotted_feature_set_list(self, compact=False):
10341031
'old_flags': [
10351032
'propfind_allprop_failure',
10361033
'duplicates_not_allowed',
1034+
'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
10371035
],
10381036

10391037
}
@@ -1219,7 +1217,6 @@ def dotted_feature_set_list(self, compact=False):
12191217
"search.time-range.todo": { "support": "unsupported" },
12201218
"search.time-range.alarm": {'support': 'unsupported'},
12211219
"search.text": { "support": "unsupported", "behaviour": "a text search ignores the filter and returns all elements" },
1222-
"search.text.by-uid": { "support": "fragile", "behaviour": "Probably not supported, but my caldav-server-checker tool has issues with it at the moment" },
12231220
"search.comp-type.optional": { "support": "ungraceful" },
12241221
"search.recurrences.expanded.todo": { "support": "unsupported" },
12251222
"search.recurrences.expanded.event": { "support": "fragile" },
@@ -1495,6 +1492,9 @@ def dotted_feature_set_list(self, compact=False):
14951492
'principal-search.list-all': {'support': 'unsupported'},
14961493
## Cross-calendar duplicate UID test fails (AuthorizationError creating second calendar)
14971494
'save.duplicate-uid.cross-calendar': {'support': 'ungraceful'},
1495+
'old_flags': [
1496+
'no_relships',
1497+
],
14981498
}
14991499

15001500
# fmt: on

caldav/search.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,8 @@ def _search_impl(
233233
):
234234
"""Core search implementation as a generator yielding actions.
235235
236+
(TODO: refactoring beyond readability? Is this sane?)
237+
236238
This generator contains all the search logic and yields (action, data) tuples
237239
that the caller (sync or async) executes. Results are sent back via .send().
238240
@@ -451,7 +453,7 @@ def _search_impl(
451453
## we wouldn't waste so much time on repeated queries
452454
if self.todo and self.include_completed is False:
453455
clone = replace(self, include_completed=True)
454-
clone.include_completed = True
456+
clone.include_completed = True ## Why? Isn't this redundant?
455457
clone.expand = False
456458

457459
if (
@@ -618,7 +620,7 @@ def search(
618620
:param calendar: Calendar to be searched (optional if searcher was created
619621
from a calendar via ``calendar.searcher()``)
620622
:param server_expand: Ask the CalDAV server to expand recurrences
621-
:param split_expanded: Don't collect a recurrence set in one ical calendar
623+
:param split_expanded: Send expanded recurrences as multiple objects
622624
:param props: CalDAV properties to send in the query
623625
:param xml: XML query to be sent to the server (string or elements)
624626
:param post_filter: Do client-side filtering after querying the server
@@ -699,6 +701,10 @@ def _search_with_comptypes(
699701
assert self.event is None and self.todo is None and self.journal is None
700702

701703
for comp_class in (Event, Todo, Journal):
704+
if not calendar.client.features.is_supported(
705+
f"save-load.{comp_class.__name__.lower()}"
706+
):
707+
continue
702708
clone = replace(self)
703709
clone.comp_class = comp_class
704710
objects += clone.search(

tests/test_caldav.py

Lines changed: 27 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1116,7 +1116,7 @@ def testFindCalendarOwner(self):
11161116
## TODO: something should probably be asserted about the Owner
11171117

11181118
def testIssue397(self):
1119-
self.skip_unless_support("search.text.by-uid")
1119+
self.skip_unless_support("save-load.event.recurrences.exception")
11201120
cal = self._fixCalendar()
11211121
cal.add_event(
11221122
"""BEGIN:VCALENDAR
@@ -1289,7 +1289,6 @@ def testChangeAttendeeStatusWithEmailGiven(self):
12891289
)
12901290
event.change_attendee_status(attendee="testuser@example.com", PARTSTAT="ACCEPTED")
12911291
event.save()
1292-
self.skip_unless_support("search.text.by-uid")
12931292
event = c.get_event_by_uid("test1")
12941293
## TODO: work in progress ... see https://github.com/python-caldav/caldav/issues/399
12951294

@@ -1456,7 +1455,6 @@ def testObjectByUID(self):
14561455
"""
14571456
It should be possible to save a task and retrieve it by uid
14581457
"""
1459-
self.skip_unless_support("search.text.by-uid")
14601458
c = self._fixCalendar(supported_calendar_component_set=["VTODO"])
14611459
c.add_todo(summary="Some test task with a well-known uid", uid="well_known_1")
14621460
foo = c.get_object_by_uid("well_known_1")
@@ -2228,7 +2226,6 @@ def testWrongPassword(self):
22282226
def testCreateChildParent(self):
22292227
self.skip_unless_support("save-load.event")
22302228
self.skip_on_compatibility_flag("no_relships")
2231-
self.skip_unless_support("search.text.by-uid")
22322229
c = self._fixCalendar(supported_calendar_component_set=["VEVENT"])
22332230
parent = c.add_event(
22342231
dtstart=datetime(2022, 12, 26, 19, 15),
@@ -2356,7 +2353,7 @@ def testSetDue(self):
23562353
dtstart=datetime(2022, 12, 26, 19, 15, tzinfo=utc),
23572354
due=datetime(2022, 12, 26, 20, 00, tzinfo=utc),
23582355
summary="Some task",
2359-
uid="ctuid1",
2356+
uid="ctuid5",
23602357
)
23612358

23622359
## setting the due should ... set the due (surprise, surprise)
@@ -2378,7 +2375,7 @@ def testSetDue(self):
23782375
dtstart=datetime(2022, 12, 26, 19, 15, tzinfo=utc),
23792376
duration=timedelta(minutes=15),
23802377
summary="Some other task",
2381-
uid="ctuid2",
2378+
uid="ctuid6",
23822379
)
23832380
some_other_todo.set_due(datetime(2022, 12, 26, 19, 45, tzinfo=utc), move_dtstart=True)
23842381
assert some_other_todo.icalendar_component["DUE"].dt == datetime(
@@ -2391,19 +2388,22 @@ def testSetDue(self):
23912388
some_todo.save()
23922389

23932390
self.skip_on_compatibility_flag("no_relships")
2394-
self.skip_unless_support("search.text.by-uid")
2391+
23952392
parent = c.add_todo(
23962393
dtstart=datetime(2022, 12, 26, 19, 00, tzinfo=utc),
23972394
due=datetime(2022, 12, 26, 21, 00, tzinfo=utc),
23982395
summary="this is a parent test task",
2399-
uid="ctuid3",
2396+
uid="ctuid7",
24002397
child=[some_todo.id],
24012398
)
24022399

2400+
## The check_reverse_relations method is cheeky,
2401+
## returning a list of non-behaving relations
2402+
## (so it SHOULD return an empty list)
24032403
assert not parent.check_reverse_relations()
24042404

2405-
## The above updates the some_todo object on the server side, but the local object is not
2406-
## updated ... until we reload it
2405+
## The above updates the some_todo object on the server side,
2406+
## but the local object is not updated ... until we reload it
24072407
some_todo.load()
24082408

24092409
## This should work out (set the children due to some time before the parents due)
@@ -2468,7 +2468,6 @@ def testCreateJournalListAndJournalEntry(self):
24682468
j1 = c.add_journal(journal)
24692469
journals = c.get_journals()
24702470
assert len(journals) == 1
2471-
self.skip_unless_support("search.text.by-uid")
24722471
j1_ = c.get_journal_by_uid(j1.id)
24732472
## Direct comparison handles different line folding from different fetch methods
24742473
assert j1_.get_icalendar_instance() == journals[0].get_icalendar_instance()
@@ -2836,11 +2835,10 @@ def testTodoCompletion(self):
28362835
# The historic todo-item can still be accessed
28372836
todos = c.get_todos(include_completed=True)
28382837
assert len(todos) == 3
2839-
if self.is_supported("search.text.by-uid"):
2840-
t3_ = c.get_todo_by_uid(t3.id)
2841-
assert t3_.vobject_instance.vtodo.summary == t3.vobject_instance.vtodo.summary
2842-
assert t3_.vobject_instance.vtodo.uid == t3.vobject_instance.vtodo.uid
2843-
assert t3_.vobject_instance.vtodo.dtstart == t3.vobject_instance.vtodo.dtstart
2838+
t3_ = c.get_todo_by_uid(t3.id)
2839+
assert t3_.vobject_instance.vtodo.summary == t3.vobject_instance.vtodo.summary
2840+
assert t3_.vobject_instance.vtodo.uid == t3.vobject_instance.vtodo.uid
2841+
assert t3_.vobject_instance.vtodo.dtstart == t3.vobject_instance.vtodo.dtstart
28442842

28452843
t2.delete()
28462844

@@ -3073,10 +3071,9 @@ def testLookupEvent(self):
30733071
e2 = c.event_by_url(e1.url)
30743072
assert e2.vobject_instance.vevent.uid == e1.vobject_instance.vevent.uid
30753073
assert e2.url == e1.url
3076-
if self.is_supported("search.text.by-uid"):
3077-
e3 = c.get_event_by_uid("20010712T182145Z-123401@example.com")
3078-
assert e3.vobject_instance.vevent.uid == e1.vobject_instance.vevent.uid
3079-
assert e3.url == e1.url
3074+
e3 = c.get_event_by_uid("20010712T182145Z-123401@example.com")
3075+
assert e3.vobject_instance.vevent.uid == e1.vobject_instance.vevent.uid
3076+
assert e3.url == e1.url
30803077

30813078
# Knowing the URL of an event, we should be able to get to it
30823079
# without going through a calendar object
@@ -3100,10 +3097,9 @@ def testCreateOverwriteDeleteEvent(self):
31003097
c = self._fixCalendar()
31013098
assert c.url is not None
31023099

3103-
# attempts on updating/overwriting a non-existing event should fail (unless get_object_by_uid_is_broken):
3104-
if self.is_supported("search.text.by-uid"):
3105-
with pytest.raises(error.ConsistencyError):
3106-
c.add_event(ev1, no_create=True)
3100+
# attempts on updating/overwriting a non-existing event should fail:
3101+
with pytest.raises(error.ConsistencyError):
3102+
c.add_event(ev1, no_create=True)
31073103

31083104
# no_create and no_overwrite is mutually exclusive, this will always
31093105
# raise an error (unless the ical given is blank)
@@ -3121,11 +3117,9 @@ def testCreateOverwriteDeleteEvent(self):
31213117
assert t1.url is not None
31223118
if not self.check_compatibility_flag("event_by_url_is_broken"):
31233119
assert c.event_by_url(e1.url).url == e1.url
3124-
if self.is_supported("search.text.by-uid"):
3125-
assert c.get_event_by_uid(e1.id).url == e1.url
3120+
assert c.get_event_by_uid(e1.id).url == e1.url
31263121

3127-
## no_create will not work unless get_object_by_uid works
3128-
no_create = self.is_supported("search.text.by-uid")
3122+
no_create = True
31293123

31303124
## add same event again. As it has same uid, it should be overwritten
31313125
## (but some calendars may throw a "409 Conflict")
@@ -3155,13 +3149,12 @@ def testCreateOverwriteDeleteEvent(self):
31553149
e3 = c.event_by_url(e1.url)
31563150
assert e3.vobject_instance.vevent.summary.value == "Bastille Day Party!"
31573151

3158-
## "no_overwrite" should throw a ConsistencyError. But it depends on get_object_by_uid.
3159-
if self.is_supported("search.text.by-uid"):
3152+
## "no_overwrite" should throw a ConsistencyError.
3153+
with pytest.raises(error.ConsistencyError):
3154+
c.add_event(ev1, no_overwrite=True)
3155+
if todo_ok:
31603156
with pytest.raises(error.ConsistencyError):
3161-
c.add_event(ev1, no_overwrite=True)
3162-
if todo_ok:
3163-
with pytest.raises(error.ConsistencyError):
3164-
c.add_todo(todo, no_overwrite=True)
3157+
c.add_todo(todo, no_overwrite=True)
31653158

31663159
# delete event
31673160
e1.delete()

tests/test_compatibility_hints.py

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -375,17 +375,16 @@ def test_hierarchical_vs_independent_subfeatures(self) -> None:
375375

376376
def test_intermediate_feature_derives_from_children(self) -> None:
377377
"""Test that intermediate features (e.g. search.text) derive status from their children"""
378-
# search.text has 5 children: case-sensitive, case-insensitive,
379-
# substring, category, by-uid (none have explicit defaults)
378+
# search.text has 4 direct children: case-sensitive, case-insensitive,
379+
# substring, category (none have explicit defaults)
380380

381381
# All children set with mixed statuses -> derive "unknown"
382382
fs = FeatureSet(
383383
{
384384
"search.text.case-sensitive": {"support": "unsupported"},
385385
"search.text.case-insensitive": {"support": "unsupported"},
386386
"search.text.substring": {"support": "unsupported"},
387-
"search.text.category": {"support": "unsupported"},
388-
"search.text.by-uid": {"support": "fragile"},
387+
"search.text.category": {"support": "fragile"},
389388
}
390389
)
391390
assert not fs.is_supported("search.text")
@@ -396,7 +395,7 @@ def test_intermediate_feature_derives_from_children(self) -> None:
396395
fs1b = FeatureSet(
397396
{
398397
"search.text.case-sensitive": {"support": "unsupported"},
399-
"search.text.by-uid": {"support": "fragile"},
398+
"search.text.category.substring": {"support": "fragile"},
400399
}
401400
)
402401
assert fs1b.is_supported("search.text")
@@ -408,7 +407,6 @@ def test_intermediate_feature_derives_from_children(self) -> None:
408407
"search.text.case-insensitive": {"support": "unsupported"},
409408
"search.text.substring": {"support": "unsupported"},
410409
"search.text.category": {"support": "unsupported"},
411-
"search.text.by-uid": {"support": "unsupported"},
412410
}
413411
)
414412
assert not fs2.is_supported("search.text")

0 commit comments

Comments
 (0)