Skip to content

Commit eae3cac

Browse files
tobixenclaude
andcommitted
refactor: extract _calendar_is_accessible helper, return bool from _try_make_calendar
Introduced a `_calendar_is_accessible(cal)` static method on `CheckMakeDeleteCalendar` that probes a calendar by calling `.events()` and catches `DAVError` (which covers `NotFoundError` 404, `AuthorizationError` 403, and `ReportError` 500). This replaces three inline try/except blocks that had inconsistent exception coverage — one caught `Exception` (too broad), two caught only `NotFoundError` (too narrow). The trigger was a server that returns HTTP 500 on a REPORT request for a non-existent calendar, reported at pycalendar/calendar-cli#114 (comment) Also simplified `_try_make_calendar` to return a plain `bool` instead of a `(bool, exception_or_None)` tuple; callers only ever used `[0]`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 4b35681 commit eae3cac

1 file changed

Lines changed: 34 additions & 26 deletions

File tree

src/caldav_server_tester/checks.py

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,19 @@ class CheckMakeDeleteCalendar(Check):
8282
}
8383
depends_on = {CheckGetCurrentUserPrincipal}
8484

85+
@staticmethod
86+
def _calendar_is_accessible(cal) -> bool:
87+
"""Probe whether a calendar is accessible by calling events().
88+
89+
Returns True if events() succeeds, False if the server returns any DAV
90+
error (404 Not Found, 403 Forbidden, 500 Internal Server Error, etc.).
91+
"""
92+
try:
93+
cal.events()
94+
return True
95+
except DAVError:
96+
return False
97+
8598
def _try_make_calendar(self, cal_id, **kwargs):
8699
"""
87100
Does some attempts on creating and deleting calendars, and sets some
@@ -125,29 +138,23 @@ def _try_make_calendar(self, cal_id, **kwargs):
125138
except Exception:
126139
self.set_feature("create-calendar.set-displayname", False)
127140

128-
except DAVError as e:
141+
except DAVError:
129142
## calendar creation created an exception. Maybe the calendar exists?
130-
## in any case, return exception
131143
cal = self.checker.principal.calendar(cal_id=cal_id)
132-
try:
133-
cal.events()
134-
except Exception:
144+
if not self._calendar_is_accessible(cal):
135145
cal = None
136146
if not cal:
137147
## cal not made and does not exist, exception thrown.
138-
## Caller to decide why the calendar was not made
139-
return (False, e)
148+
return False
140149

141150
assert cal
142151

143152
try:
144153
## Use DAVObject.delete directly to bypass Calendar.delete()
145154
## workarounds - we want to test the server's raw DELETE behavior
146155
DAVObject.delete(cal)
147-
try:
148-
cal = self.checker.principal.calendar(cal_id=cal_id)
149-
events = cal.events()
150-
except NotFoundError:
156+
cal = self.checker.principal.calendar(cal_id=cal_id)
157+
if not self._calendar_is_accessible(cal):
151158
cal = None
152159
## Delete throw no exceptions, but was the calendar deleted?
153160
if not cal or self.checker.features_checked.is_supported("create-calendar.auto"):
@@ -160,24 +167,22 @@ def _try_make_calendar(self, cal_id, **kwargs):
160167
## Calendar not deleted.
161168
## Perhaps the server needs some time to delete the calendar
162169
time.sleep(10)
163-
try:
164-
cal = self.checker.principal.calendar(cal_id=cal_id)
165-
assert cal
166-
cal.events()
170+
cal = self.checker.principal.calendar(cal_id=cal_id)
171+
if self._calendar_is_accessible(cal):
167172
## Calendar not deleted, but no exception thrown.
168173
## Perhaps it's a "move to thrashbin"-regime on the server
169174
self.set_feature(
170175
"delete-calendar",
171176
{"support": "unknown", "behaviour": "move to trashbin?"},
172177
)
173-
except NotFoundError as e:
178+
else:
174179
## Calendar was deleted, it just took some time.
175180
self.set_feature(
176181
"delete-calendar",
177182
{"support": "fragile", "behaviour": "delayed deletion"},
178183
)
179-
return (calmade, e)
180-
return (calmade, None)
184+
return calmade
185+
return calmade
181186
except DAVError as e:
182187
time.sleep(10)
183188
try:
@@ -191,7 +196,7 @@ def _try_make_calendar(self, cal_id, **kwargs):
191196
)
192197
except DAVError as e2:
193198
self.set_feature("delete-calendar", False)
194-
return (calmade, None)
199+
return calmade
195200

196201
def _run_check(self):
197202
try:
@@ -201,7 +206,10 @@ def _run_check(self):
201206
except (
202207
NotFoundError,
203208
AuthorizationError,
204-
): ## robur throws a 403 .. and that's ok
209+
ReportError,
210+
): ## robur throws a 403 .. and that's ok.
211+
## Some unknown server throws 500 internal server error ... well, we should survive that, too
212+
## (perhaps we should do an `except Exception` instead?)
205213
self.set_feature("create-calendar.auto", False)
206214

207215
## Check on "no_default_calendar" flag
@@ -213,37 +221,37 @@ def _run_check(self):
213221
self.set_feature("get-current-user-principal.has-calendar", False)
214222

215223
makeret = self._try_make_calendar(name="Yep", cal_id="caldav-server-checker-mkdel-test")
216-
if makeret[0]:
224+
if makeret:
217225
## calendar created
218226
## TODO: this is a lie - we haven't really verified this, only on second script run we will be sure
219227
if self.checker.features_checked.is_supported("delete-calendar"):
220228
self.set_feature("delete-calendar.free-namespace", True)
221229
return
222230
makeret = self._try_make_calendar(name=str(uuid.uuid4()), cal_id="pythoncaldav-test")
223-
if makeret[0]:
231+
if makeret:
224232
self.set_feature("create-calendar.set-displayname", True)
225233
self.set_feature("delete-calendar.free-namespace", False)
226234
return
227235
makeret = self._try_make_calendar(cal_id="pythoncaldav-test")
228-
if makeret[0]:
236+
if makeret:
229237
self.set_feature("create-calendar.set-displayname", False)
230238
if self.checker.features_checked.is_supported("delete-calendar"):
231239
self.set_feature("delete-calendar.free-namespace", True)
232240
return
233241
unique_id1 = "testcalendar-" + str(uuid.uuid4())
234242
makeret = self._try_make_calendar(cal_id=unique_id1, name=str(uuid.uuid4()))
235-
if makeret[0]:
243+
if makeret:
236244
self.set_feature("delete-calendar.free-namespace", False)
237245
self.set_feature("create-calendar.set-displayname", True)
238246
return
239247
unique_id = "testcalendar-" + str(uuid.uuid4())
240248
makeret = self._try_make_calendar(cal_id=unique_id)
241-
if makeret[0]:
249+
if makeret:
242250
self.set_feature("create-calendar.set-displayname", False)
243251
self.set_feature("delete-calendar.free-namespace", False)
244252
return
245253
makeret = self._try_make_calendar(cal_id=unique_id, method="mkcol")
246-
if makeret[0]:
254+
if makeret:
247255
self.set_feature("create-calendar", {"support": "quirk", "behaviour": "mkcol-required"})
248256
else:
249257
self.set_feature("create-calendar", False)

0 commit comments

Comments
 (0)