Skip to content

Commit 354b338

Browse files
tobixenclaude
andcommitted
fix: handle async client in add_organizer() no-arg path
add_organizer() without arguments called self.client.principal() directly. On an AsyncDAVClient that method is a coroutine function, so the call returned a coroutine object and the subsequent .get_vcal_address() raised AttributeError. Fix by delegating to _async_add_organizer() when is_async_client, following the same dual-mode pattern used by set_relation() and load(). Extract _set_organizer() to avoid duplicating the two-liner that pops and re-adds the ORGANIZER property. Add _async_get_vcal_address() to Principal so that _async_add_organizer() can await the property lookups (get_display_name, calendar-user-address-set, CalendarUserType) that get_vcal_address() performs synchronously. Also add missing unit tests: - test_add_organizer_principal_object: cover the Principal-object argument path (isinstance(organizer, _Principal)), which had no coverage. - test_add_organizer_explicit_arg_is_sync_safe_for_async_client: explicit arg is pure in-memory and must not require awaiting even for async clients. - test_add_organizer_no_arg_returns_coroutine_for_async_client: the no-arg path must return a coroutine (not raise AttributeError) for async clients. - test_add_organizer_no_arg_async_awaited_sets_organizer: awaiting that coroutine must actually set the ORGANIZER field. Add sync integration test testAddOrganizer to RepeatedFunctionalTestsBaseClass covering all three argument forms (no-arg, email string, vCalAddress). The no-arg sub-test is skipped on servers that do not expose scheduling.calendar-user-address-set. Add async integration test test_add_organizer_no_arg to AsyncFunctionalTestsBaseClass in tests/test_async_integration.py. It verifies that add_organizer() returns a coroutine for async clients, that awaiting it sets ORGANIZER, and that the resulting address matches the principal's own vCalAddress. Also skipped when scheduling.calendar-user-address-set is not supported. Note: the two callers that invoke add_organizer() without arguments (Calendar.freebusy_request and Calendar.save_with_invites in collection.py) are not themselves async-aware, so they will still malfunction when used via an async client — but that is a pre-existing limitation of those methods, not something introduced here. Prompt: The new logic in add_organizer, is it well-enough covered by test code? Is there a symmetry in the async and sync behaviour? Is there a symmetry between the sync and async tests? Fix the missing tests, including async tests and particularly a test to expose the async bug. Come with suggestions on how to solve the async problem. Followup-prompt: In the previous commit a fix was applied, but the commit message says that only the test framework was modified. Please amend the commit. Followup-prompt: The previous commit did not add any integration tests. Please add some integration tests (including async tests) and amend the commit. Followup-prompt: Keep the commit message in imperative mood. Followup-prompt: Async tests should be in tests/test_async_integration.py. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 916ef6d commit 354b338

File tree

5 files changed

+179
-1
lines changed

5 files changed

+179
-1
lines changed

caldav/calendarobjectresource.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,12 +182,18 @@ def add_organizer(self, organizer=None) -> None:
182182
* A plain email address string (``"mailto:"`` is prepended automatically)
183183
184184
Any pre-existing ORGANIZER field is removed before the new one is added.
185+
186+
For async clients, when *organizer* is omitted the method returns a
187+
coroutine that must be awaited. When an explicit *organizer* is supplied
188+
the method is always synchronous (pure in-memory, no network call).
185189
"""
186190
from .collection import Principal as _Principal ## avoid circular import
187191

188192
if organizer is None:
189193
if self.client is None:
190194
raise ValueError("Unexpected value None for self.client")
195+
if self.is_async_client:
196+
return self._async_add_organizer()
191197
organizer_obj = self.client.principal().get_vcal_address()
192198
elif isinstance(organizer, _Principal):
193199
organizer_obj = organizer.get_vcal_address()
@@ -201,6 +207,15 @@ def add_organizer(self, organizer=None) -> None:
201207
else:
202208
raise ValueError(f"Unsupported organizer type: {type(organizer)!r}")
203209

210+
self._set_organizer(organizer_obj)
211+
212+
async def _async_add_organizer(self) -> None:
213+
"""Async implementation of add_organizer() for async clients."""
214+
principal = await self.client.principal()
215+
self._set_organizer(await principal._async_get_vcal_address())
216+
217+
def _set_organizer(self, organizer_obj: vCalAddress) -> None:
218+
"""Write the ORGANIZER property onto the icalendar component (sync, no I/O)."""
204219
ievent = self.icalendar_component
205220
ievent.pop("organizer", None)
206221
ievent.add("organizer", organizer_obj)

caldav/collection.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,22 @@ def get_vcal_address(self) -> "vCalAddress":
396396
ret.params["cutype"] = vText(cutype)
397397
return ret
398398

399+
async def _async_get_vcal_address(self) -> "vCalAddress":
400+
"""Async counterpart of get_vcal_address() for use with AsyncDAVClient."""
401+
from icalendar import vCalAddress, vText
402+
403+
cn = await self.get_display_name()
404+
addresses_el = await self.get_property(cdav.CalendarUserAddressSet(), parse_props=False)
405+
if addresses_el is None:
406+
raise error.NotFoundError("No calendar user addresses given from server")
407+
assert not [x for x in addresses_el if x.tag != dav.Href().tag]
408+
addresses = sorted(list(addresses_el), key=lambda x: -int(x.get("preferred", 0)))
409+
cutype = await self.get_property(cdav.CalendarUserType())
410+
ret = vCalAddress(addresses[0].text)
411+
ret.params["cn"] = vText(cn)
412+
ret.params["cutype"] = vText(cutype)
413+
return ret
414+
399415
@property
400416
def calendar_home_set(self):
401417
if not self._calendar_home_set:

tests/test_async_integration.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,37 @@ async def test_todos_method(self, async_task_list: Any) -> None:
439439
assert len(todos) >= 1
440440
assert all(isinstance(t, AsyncTodo) for t in todos)
441441

442+
@pytest.mark.asyncio
443+
async def test_add_organizer_no_arg(self, async_client: Any, async_calendar: Any) -> None:
444+
"""add_organizer() without args returns a coroutine and sets ORGANIZER (issue #524).
445+
446+
Verifies the async fix: on an AsyncDAVClient principal() is a coroutine
447+
function, so add_organizer() must await it via _async_add_organizer()
448+
rather than calling it synchronously (which would raise AttributeError
449+
on the returned coroutine object).
450+
"""
451+
from caldav import Event
452+
453+
self.skip_unless_support("scheduling.calendar-user-address-set")
454+
455+
principal = await async_client.principal()
456+
expected_vcal = await principal._async_get_vcal_address()
457+
458+
event = Event(client=async_client, data=ev1(), parent=async_calendar)
459+
460+
## Must return a coroutine, not raise AttributeError
461+
coro = event.add_organizer()
462+
assert asyncio.iscoroutine(coro), (
463+
f"add_organizer() on async client must return a coroutine, got {type(coro)}"
464+
)
465+
await coro
466+
467+
org = event.icalendar_component.get("organizer")
468+
assert org is not None, "ORGANIZER must be set after awaiting add_organizer()"
469+
assert str(org) == str(expected_vcal), (
470+
f"ORGANIZER {org!r} should match the principal's address {expected_vcal!r}"
471+
)
472+
442473

443474
# ==================== Dynamic Test Class Generation ====================
444475
#

tests/test_caldav.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1263,6 +1263,46 @@ def testSchedulingInfo(self):
12631263
calendar_user_address_set = self.principal.calendar_user_address_set()
12641264
me_a_participant = self.principal.get_vcal_address()
12651265

1266+
def testAddOrganizer(self):
1267+
"""add_organizer() sets ORGANIZER from the current principal (issue #524).
1268+
1269+
Tests three paths:
1270+
- no-arg: principal is looked up from the client (requires
1271+
scheduling.calendar-user-address-set).
1272+
- explicit email string: pure in-memory, no server interaction.
1273+
- explicit vCalAddress: pure in-memory, no server interaction.
1274+
"""
1275+
from icalendar import vCalAddress
1276+
1277+
cal = self._fixCalendar()
1278+
event = Event(
1279+
client=self.caldav,
1280+
data=ev1,
1281+
parent=cal,
1282+
)
1283+
1284+
## ---- explicit string arg (pure in-memory, always runs) ----
1285+
event.add_organizer("organizer@example.com")
1286+
org = event.icalendar_component.get("organizer")
1287+
assert org is not None, "ORGANIZER should be set after add_organizer(string)"
1288+
assert "organizer@example.com" in str(org)
1289+
1290+
## ---- explicit vCalAddress arg ----
1291+
addr = vCalAddress("mailto:addr@example.com")
1292+
event.add_organizer(addr)
1293+
org = event.icalendar_component.get("organizer")
1294+
assert str(org) == "mailto:addr@example.com"
1295+
1296+
## ---- no-arg: uses current principal ----
1297+
self.skip_unless_support("scheduling.calendar-user-address-set")
1298+
event.add_organizer()
1299+
org = event.icalendar_component.get("organizer")
1300+
assert org is not None, "ORGANIZER should be set when add_organizer() uses principal"
1301+
principal_addresses = self.principal.calendar_user_address_set()
1302+
assert any(addr in str(org) for addr in principal_addresses), (
1303+
f"ORGANIZER {org!r} should contain one of the principal's addresses {principal_addresses!r}"
1304+
)
1305+
12661306
def testIssue399ChangeAttendeeStatusUsernameEmailFallback(self):
12671307
"""change_attendee_status() works when the attendee is identified
12681308
by the client username rather than calendar_user_address_set() (issue #399).

tests/test_caldav_unit.py

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2190,12 +2190,76 @@ def test_set_relation_returns_coroutine_for_async_client(self):
21902190
result.close()
21912191

21922192
def test_accept_invite_raises_not_implemented_for_async_client(self):
2193-
"""accept_invite() must raise NotImplementedError for async clients (not silently fail)."""
2193+
"""accept_invite() must raise Notimplemented for async clients (not silently fail)."""
21942194
client, calendar = self._make_async_client_and_calendar()
21952195
event = Event(client=client, url="/calendar/ev1.ics", data=ev1, parent=calendar)
21962196
with pytest.raises(NotImplementedError):
21972197
event.accept_invite()
21982198

2199+
def test_add_organizer_explicit_arg_is_sync_safe_for_async_client(self):
2200+
"""add_organizer(explicit_arg) is pure in-memory: no network call, no await needed.
2201+
It must work correctly even when the client is an async client."""
2202+
from icalendar import vCalAddress
2203+
2204+
client, calendar = self._make_async_client_and_calendar()
2205+
event = Event(client=client, url="/calendar/ev1.ics", data=ev1, parent=calendar)
2206+
event.add_organizer("organizer@example.com")
2207+
organizer = event.icalendar_component.get("organizer")
2208+
assert "organizer@example.com" in str(organizer)
2209+
2210+
def test_add_organizer_no_arg_returns_coroutine_for_async_client(self):
2211+
"""add_organizer() without args must return a coroutine for async clients.
2212+
2213+
Bug: today the code calls self.client.principal().get_vcal_address() without
2214+
checking is_async_client first. On an AsyncDAVClient, principal() is a
2215+
coroutine function, so principal() returns a coroutine object. Calling
2216+
.get_vcal_address() on that coroutine raises AttributeError instead of
2217+
returning a usable coroutine to the caller.
2218+
"""
2219+
import asyncio
2220+
2221+
from icalendar import vCalAddress
2222+
2223+
client, calendar = self._make_async_client_and_calendar()
2224+
event = Event(client=client, url="/calendar/ev1.ics", data=ev1, parent=calendar)
2225+
2226+
async def async_principal():
2227+
p = mock.MagicMock()
2228+
p._async_get_vcal_address = mock.AsyncMock(
2229+
return_value=vCalAddress("mailto:me@example.com")
2230+
)
2231+
return p
2232+
2233+
client.principal = async_principal
2234+
2235+
result = event.add_organizer()
2236+
assert asyncio.iscoroutine(result), (
2237+
f"expected a coroutine from add_organizer() on async client, got {type(result)}"
2238+
)
2239+
result.close()
2240+
2241+
def test_add_organizer_no_arg_async_awaited_sets_organizer(self):
2242+
"""Awaiting add_organizer() without args on async client must set ORGANIZER correctly."""
2243+
import asyncio
2244+
2245+
from icalendar import vCalAddress
2246+
2247+
client, calendar = self._make_async_client_and_calendar()
2248+
event = Event(client=client, url="/calendar/ev1.ics", data=ev1, parent=calendar)
2249+
2250+
async def async_principal():
2251+
p = mock.MagicMock()
2252+
p._async_get_vcal_address = mock.AsyncMock(
2253+
return_value=vCalAddress("mailto:me@example.com")
2254+
)
2255+
return p
2256+
2257+
client.principal = async_principal
2258+
2259+
asyncio.run(event.add_organizer())
2260+
organizer = event.icalendar_component.get("organizer")
2261+
assert str(organizer) == "mailto:me@example.com"
2262+
21992263

22002264
class TestRateLimitHelpers:
22012265
"""Unit tests for the shared rate-limit helper functions in caldav.lib.error."""
@@ -2867,6 +2931,18 @@ def test_add_organizer_no_arg_no_client_raises(self):
28672931
with pytest.raises(ValueError):
28682932
ev.add_organizer()
28692933

2934+
def test_add_organizer_principal_object(self):
2935+
"""Passing a Principal object directly calls get_vcal_address() on it."""
2936+
from icalendar import vCalAddress
2937+
2938+
ev = self._make_event()
2939+
mock_principal = mock.MagicMock(spec=Principal)
2940+
mock_principal.get_vcal_address.return_value = vCalAddress("mailto:organizer@example.com")
2941+
ev.add_organizer(mock_principal)
2942+
organizer = ev.icalendar_component.get("organizer")
2943+
assert str(organizer) == "mailto:organizer@example.com"
2944+
mock_principal.get_vcal_address.assert_called_once()
2945+
28702946

28712947
class TestChangeAttendeeStatusFallback:
28722948
"""Unit tests for change_attendee_status() fallback when calendar_user_address_set() is unavailable.

0 commit comments

Comments
 (0)