Skip to content

Commit 2a2d0ca

Browse files
tobixenclaude
andcommitted
fix: Improved async tests. Fixing bugs discovered.
Fixing: * Zimbra compatibility fix: omit DisplayName from MKCALENDAR body when create-calendar.set-displayname is unsupported (see below) * _async_put did not await the retry coroutine returned by _post_put. (Found when investigating Ox test breakages) * _async_get_object_by_uid was missing include_completed=True, which the sync get_object_by_uid always passes. (Found when investigating Ox test breakages) * _async_search_with_comptypes did not skip component types that the server does not support, unlike the sync _search_with_comptypes which already had this guard. (found when testing towards OX) Test code: * Add more fixtures and helpers for async tests * Add 7 async tests, mirroring the sync RepeatedFunctionalTestsBaseClass CRUD tests * Fixing up the tests to be more or less symmetric with the sync tests (and prettyfying the sync tests while being at it) * Rearranged the async testing fixtures to avoid Ox throwing errors when same event ended up in multiple calendars. prompt: An issue was created with `git bug bug new`, it has id 099db26 comment has id 009999d. Please start with Phase 1 and update the issue, but in a separate temporary worktree on a branch spun off from v3.2-preparations prompt: continue with phase 2 prompt: tests/test_async_integration.py::TestAsyncForXandikos::test_multi_get fails (...), fix multiget in collection.py to be async-aware, following the pattern in docs/design/ASYNC_DUAL_MODE.md followup-prompt: wait. calendar_multiget is deprecated and scheduled for deletion. Any test code exercising calendar_multiget can be removed or rewritten to use self.multiget instead. prompt: please help me investigate the tests/test_async_integration.py::TestAsyncForZimbra::test_lookup_event breakage. It's a known problem with zimbra that a GET towards an event URL frequently yields 404, but the .load()-method should have fallbacks to .multiget(), and the caldav-server-tester in ~/caldav-server-tester claims that the current versions of Zimbra have no problems with such GETs prompt: (lots of arguing with Claude) prompt: (trying a new claude session) With `pytest -k 'zimbra and lookup'` the sync test passes, while the async test fails. Why? There shouldn't be any difference there? (you've already investigated two hypotheses: "zimbra does not support quoted at-sign in the URL" and "the uid has been used on another calendar, therefore the GET returns 404", both hypotheses are wrong, and does not explain the asymmetry between sync and async) prompt: tests/test_async_integration.py::TestAsyncForOx::test_object_by_uid breaks followup-prompt: (when giving two options on how to fix the tests / test fixtures) Isn't it a third option here too - use a static UID, but different from well_known_1? I think we should make things as symmetric as possible and reuse the same calendar. It shouldn't be persistent, except for servers that doesn't support create/delete calendars. Mostly AI-generated, with some hands-on adjustments (AI-generating test code is fine, when done under good supervision - though I wonder a bit if I would have been able to do this faster without AI-help) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent c48ecbe commit 2a2d0ca

5 files changed

Lines changed: 360 additions & 36 deletions

File tree

caldav/calendarobjectresource.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1071,6 +1071,7 @@ async def _async_load_by_multiget(self) -> Self:
10711071
def _post_load_by_multiget(self, items):
10721072
if not items:
10731073
raise error.NotFoundError(self.url)
1074+
items = iter(items)
10741075
url_data = next(items, None)
10751076
if url_data is None:
10761077
## We shouldn't come here. Something is wrong.
@@ -1147,7 +1148,10 @@ def _put(self, retry_on_failure=True) -> "None | Coroutine[Any, Any, None]":
11471148

11481149
async def _async_put(self, headers, retry_on_failure=True):
11491150
r = await self.client.put(str(self.url), str(self.data), headers | ICALH)
1150-
return self._post_put(r, retry_on_failure)
1151+
result = self._post_put(r, retry_on_failure)
1152+
if result is not None:
1153+
# _post_put returned a retry coroutine (self._put(False) for async client)
1154+
await result
11511155

11521156
def _post_put(self, r, retry_on_failure):
11531157
if r.status == 412:

caldav/collection.py

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -723,7 +723,14 @@ def _create(
723723

724724
prop = dav.Prop()
725725
display_name = None
726-
if name:
726+
# Some servers (e.g. Zimbra) use the DisplayName from the MKCALENDAR body
727+
# as the calendar URL, ignoring the actual request path. When the server
728+
# does not support setting a separate display name, omit it from the body so
729+
# the request URL path is used as the calendar identifier.
730+
supports_displayname = not self.client or self.client.features.is_supported(
731+
"create-calendar.set-displayname"
732+
)
733+
if name and supports_displayname:
727734
display_name = dav.DisplayName(name)
728735
prop += [display_name]
729736
if supported_calendar_component_set:
@@ -747,7 +754,7 @@ def _create(
747754
# on setting the DisplayName on calendar creation
748755
# (DAViCal, Zimbra, ...). Doing an attempt on explicitly setting the
749756
# display name using PROPPATCH.
750-
if name:
757+
if display_name:
751758
try:
752759
self.set_properties([display_name])
753760
except Exception:
@@ -766,7 +773,7 @@ async def _async_create(self, path, mkcol, method, name, display_name) -> None:
766773
await self._query(root=mkcol, query_method=method, url=path, expected_return_value=201)
767774

768775
# COMPATIBILITY ISSUE - try to set display name explicitly
769-
if name:
776+
if display_name:
770777
try:
771778
await self.set_properties([display_name])
772779
except Exception:
@@ -1080,6 +1087,7 @@ def _multiget(self, event_urls: Iterable[URL], raise_notfound: bool = False) ->
10801087
"""
10811088
get multiple events' data.
10821089
TODO: Does it overlap the _request_report_build_resultlist method
1090+
## WARNING: async logic is duplicated in _async_multiget — mirror any changes there
10831091
"""
10841092
if self.url is None:
10851093
raise ValueError("Unexpected value None for self.url")
@@ -1098,28 +1106,33 @@ def _multiget(self, event_urls: Iterable[URL], raise_notfound: bool = False) ->
10981106
for r in results:
10991107
yield (r, results[r][cdav.CalendarData.tag])
11001108

1101-
## Replace the last lines with
1109+
def _post_multiget(self, results: Iterable[tuple[str, str]]) -> list[_CC]:
1110+
"""Post-processing shared by multiget and _async_multiget_objects."""
1111+
return [
1112+
self._calendar_comp_class_by_data(data)(
1113+
self.client,
1114+
# Quote path to handle servers returning unencoded spaces (e.g., Zimbra)
1115+
url=self.url.join(quote(unquote(str(url)), safe="/:@")),
1116+
data=data,
1117+
parent=self,
1118+
)
1119+
for url, data in results
1120+
]
1121+
11021122
def multiget(self, event_urls: Iterable[URL], raise_notfound: bool = False) -> Iterable[_CC]:
11031123
"""
11041124
get multiple events' data
11051125
TODO: Does it overlap the _request_report_build_resultlist method?
11061126
@author mtorange@gmail.com (refactored by Tobias)
11071127
"""
1108-
results = self._multiget(event_urls, raise_notfound=raise_notfound)
1109-
for url, data in results:
1110-
# Quote path to handle servers returning unencoded spaces (e.g., Zimbra)
1111-
quoted_url = quote(unquote(str(url)), safe="/:@")
1112-
yield self._calendar_comp_class_by_data(data)(
1113-
self.client,
1114-
url=self.url.join(quoted_url),
1115-
data=data,
1116-
parent=self,
1117-
)
1128+
if self.is_async_client:
1129+
return self._async_multiget_objects(event_urls, raise_notfound=raise_notfound)
1130+
return self._post_multiget(self._multiget(event_urls, raise_notfound=raise_notfound))
11181131

11191132
async def _async_multiget(
11201133
self, event_urls: Iterable[URL], raise_notfound: bool = False
11211134
) -> list[tuple[str, str]]:
1122-
"""Async version of _multiget — returns a list of (url, data) tuples."""
1135+
## WARNING: sync logic is duplicated in _multiget — mirror any changes there
11231136
if self.url is None:
11241137
raise ValueError("Unexpected value None for self.url")
11251138

@@ -1134,12 +1147,20 @@ async def _async_multiget(
11341147
raise error.NotFoundError(f"Status {status} in {href}")
11351148
return [(r, results[r][cdav.CalendarData.tag]) for r in results]
11361149

1150+
async def _async_multiget_objects(
1151+
self, event_urls: Iterable[URL], raise_notfound: bool = False
1152+
) -> list[_CC]:
1153+
"""Async version of multiget."""
1154+
return self._post_multiget(
1155+
await self._async_multiget(event_urls, raise_notfound=raise_notfound)
1156+
)
1157+
11371158
def calendar_multiget(self, *largs, **kwargs):
11381159
"""
11391160
get multiple events' data
11401161
@author mtorange@gmail.com
11411162
(refactored by Tobias)
1142-
This is for backward compatibility. It may be removed in 3.0 or later release.
1163+
This is for backward compatibility. It may be removed in a later release.
11431164
"""
11441165
return list(self.multiget(*largs, **kwargs))
11451166

@@ -1605,7 +1626,12 @@ async def _async_get_object_by_uid(
16051626
) -> "Event":
16061627
"""Async helper for get_object_by_uid()."""
16071628
items_found = await self.search(
1608-
uid=uid, comp_class=comp_class, xml=comp_filter, post_filter=True, _hacks="insist"
1629+
uid=uid,
1630+
comp_class=comp_class,
1631+
xml=comp_filter,
1632+
post_filter=True,
1633+
_hacks="insist",
1634+
include_completed=True,
16091635
)
16101636
items_found = [o for o in items_found if o.id == uid]
16111637

caldav/search.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -993,6 +993,10 @@ async def _async_search_with_comptypes(
993993
assert self.event is None and self.todo is None and self.journal is None
994994

995995
for comp_class in (Event, Todo, Journal):
996+
if not calendar.client.features.is_supported(
997+
f"save-load.{comp_class.__name__.lower()}"
998+
):
999+
continue
9961000
clone = replace(self)
9971001
clone.comp_class = comp_class
9981002
results = await clone.async_search(

0 commit comments

Comments
 (0)