Conversation
tobixen
force-pushed
the
v3.0-dev
branch
3 times, most recently
from
31c86fd to
4f8b895
Compare
tobixen
force-pushed
the
v3.0-dev
branch
2 times, most recently
from
4433264 to
70fe8f1
Compare
Must-fix items: - xml_parsers.py: fix NameError in parse_calendar_multiget_response (missing _ prefix) - compatibility_hints.py: replace breakpoint() with ValueError for unknown feature type - calendarobjectresource.py: fix _set_deprecated_vobject_instance calling getter instead of setter - calendarobject_ops.py: replace uuid.uuid1() with uuid.uuid4() to avoid MAC address leak - davclient.py: narrow bare except clauses (TypeError for teardown, Exception for check_dav_support) - async_davclient.py: add url.unauth() to strip credentials from stored URL (prevents log leaks) Should-fix items: - async_davclient.py: add httpx-compatible _HttpxBearerAuth class; use it for bearer on httpx path - response.py: guard tree[0] access with len(tree) > 0 check - davobject.py: replace production-unsafe assert with explicit ValueError - calendar_ops.py, collection.py: add usedforsecurity=False to MD5 calls for FIPS compliance Co-Authored-By: Claude <noreply@anthropic.com>
Mark all "Must Fix" and "Should Fix" items as done (commit ff6db30). Add status column to bugs table. Move two remaining async issues into the v3.1+ list. Reword the executive summary accordingly. Co-Authored-By: Claude <noreply@anthropic.com>
- davobject.py: Extract _build_xml_body() and _build_propfind_root() helpers; replace four identical XML-serialisation blocks and two identical propfind-root-builder blocks with calls to these helpers. _query_properties / _async_query_properties each reduce to one line. - collection.py CalendarSet.get_calendars: Migrate sync path from the old children()/_expand_simple_props() API to the protocol layer (client.propfind + response.results), matching the async path. Add _calendars_from_results() as the single shared result processor; both get_calendars and _async_calendars now reduce to two lines each. - base_client.py: Add _build_principal_search_query() and _parse_principal_search_response() so that both DAVClient and AsyncDAVClient.search_principals share all build/parse logic. Hoist get_events() and get_todos() into BaseDAVClient — the non-async wrappers work for both clients because calling an async search_calendar() without await returns an awaitable coroutine. - davclient.py / async_davclient.py: Shrink search_principals to 4 lines each (body = build; response = report; status check; parse). Remove the now-redundant get_events and get_todos overrides. Net: -177 lines, 0 new tests needed (existing 460 all pass). Co-Authored-By: Claude <noreply@anthropic.com>
…7c51c9) Mark recommendation 11 as done; update §5 duplication tables to show which items are fixed and revised line-count estimates. Co-Authored-By: Claude <noreply@anthropic.com>
…cation Dead code (xml_builders.py): - Delete _build_freebusy_query_body, _build_mkcol_body, _to_utc_date_string — confirmed zero callers across the codebase. The _to_utc_date_string in xml_builders was a never-wired duplicate of the one in elements/cdav.py. CalendarInfo name collision (protocol/types.py, protocol/__init__.py): - Delete the CalendarInfo dataclass from protocol/types.py and remove it from __init__.py. It was never instantiated anywhere; the real CalendarInfo lives in operations/calendarset_ops.py (different fields, actively used). response.py / xml_parsers.py duplication: - Extract _normalize_href() into xml_parsers.py, consolidating the identical %2540 Confluence workaround + absolute-URL-to-path conversion from both _parse_response_element (xml_parsers) and _parse_response (response.py). - _parse_response_element now calls _normalize_href (4 lines -> 1). - response.py._strip_to_multistatus now delegates to the xml_parsers _strip_to_multistatus function instead of reimplementing it. - response.py.validate_status now delegates to xml_parsers._validate_status. - response.py._parse_response now calls _normalize_href; the trailing absolute-URL conversion block is removed (now inside _normalize_href). - Drop the unused unquote and URL imports from response.py. The propstat-loop duplication (_find_objects_and_props vs _extract_properties) is intentionally left: _find_objects_and_props must return raw _Element objects for expand_simple_props() to work, while _extract_properties returns plain Python values. Converging them would require a public API change. All 460 unit tests pass. Co-Authored-By: Claude <noreply@anthropic.com>
…ixes Mark §1.1 issues 2, 3, 4 as fixed; upgrade response.py rating from 6 to 7. Co-Authored-By: Claude <noreply@anthropic.com>
…e counts - §1.1 table: update line counts for types.py (221), xml_builders.py (346), xml_parsers.py (467); bump xml_parsers.py rating from 5/10 to 6/10 - §5.3: replace one-liner with a table showing which of the five duplicated pieces are now fixed vs still open, and why - §8: strike through item 16 (dead code removed); mark item 12 as partially done with a pointer to §5.3 - Appendix A: update line counts for response.py, types.py, xml_builders.py, xml_parsers.py to reflect current state Co-Authored-By: Claude <noreply@anthropic.com>
The pattern quote(uid.replace("/", "%2F")) was duplicated in both
_generate_url() and _find_id_and_path() in calendarobject_ops.py.
Extracted into a single _quote_uid() helper, both callers now use it.
Co-Authored-By: Claude <noreply@anthropic.com>
… return Stalwart (https://stalw.art/) is an all-in-one mail & collaboration server that added CalDAV/CardDAV support in 2024/2025. Adds the full Docker test server integration: - tests/docker-test-servers/stalwart/: docker-compose.yml (port 8809), start.sh, stop.sh, setup_stalwart.sh (creates domain + user via REST API) - tests/test_servers/docker.py: StalwartTestServer class - caldav/compatibility_hints.py: empty stalwart hints dict (to be filled after test runs) - tests/caldav_test_servers.yaml.example: stalwart entry Setup notes: - Admin password set via STALWART_ADMIN_PASSWORD env var - Domain must be created before user (POST /api/principal) - CalDAV home: /dav/cal/<username>/ - Startup takes ~25s; healthcheck start_period accounts for this Also fixes a missing `return` in the expand_simple_props wrapper introduced by the _expand_simple_props refactor — without it every calendar-query REPORT result was discarded (returned None), causing TypeError on iteration. Co-Authored-By: Claude <noreply@anthropic.com>
…resource.py Co-Authored-By: Claude <noreply@anthropic.com>
…import
Removes the last inline quote(uid.replace("/", "%2F")) in favour of the
shared _quote_uid() helper from calendarobject_ops.
Co-Authored-By: Claude <noreply@anthropic.com>
…n sync
Sync (davclient.py) bugs fixed:
- Auto-detect used `if rate_limit:` on the dict returned by is_supported(),
which is truthy even for the default {'enable': False}. Changed to
`if rate_limit and rate_limit.get('enable'):`
- `rate_limit_time_slept > self.rate_limit_max_sleep` crashed with TypeError
when rate_limit_max_sleep is None. Guarded with `is not None` check.
Async (async_davclient.py) brought to parity:
- rate_limit_handle now Optional[bool] = None with same features-based
auto-detect as sync.
- request() gains rate_limit_time_slept: float = 0 and the same adaptive
backoff loop: each retry adds half the already-slept time; stops when
rate_limit_max_sleep is exceeded. Retries recursively via self.request()
rather than calling _async_request() directly (enabling multi-retry).
Tests added for both clients:
- test_rate_limit_adaptive_sleep_increases_on_repeated_retries
- test_rate_limit_max_sleep_stops_adaptive_retries
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…on pattern
- caldav/search.py: rename module-level instance `TypesFactory = TypesFactory()`
to `_icalendar_types` to avoid shadowing the imported class name
- caldav/config.py: change `return None` to `return {}` when no config file
is found in default locations, for consistency with all other return paths
- caldav/calendarobjectresource.py: replace try/except StopIteration in
load_by_multiget() with next(gen, None) sentinel pattern — using StopIteration
for control flow is deprecated since PEP 479 (Python 3.7+)
- docs/design/V3_CODE_REVIEW.md: update code review notes to reflect fixes
Co-Authored-By: Claude <noreply@anthropic.com>
The sync and async variants were identical except for await. Inline the three-line body at each call site and delete both methods. Co-Authored-By: Claude <noreply@anthropic.com>
…_async_load - collection.py: delete Principal._async_get_property override — it had an incompatible signature (missing use_cached and **passthrough) and reimplemented PROPFIND logic that the parent DAVObject._async_get_property already handles correctly. Deleting the override restores LSP compliance. - collection.py: add Calendar._async_multiget() as the async counterpart to _multiget(), returning a list of (url, data) tuples. - calendarobjectresource.py: add _async_load_by_multiget() using the new _async_multiget(), mirroring sync load_by_multiget(). - calendarobjectresource.py: update _async_load() to match sync load() fallback behaviour — on NotFoundError: try multiget first, then re-fetch by UID; on other exceptions: try multiget (previously just re-raised). - docs/design/V3_CODE_REVIEW.md: mark both items as fixed. Co-Authored-By: Claude <noreply@anthropic.com>
Covers changes since 3.0.0a2: rate-limit bug fixes (sync + async), async multiget fallback, Principal._async_get_property fix, URL quoting fix, expand_simple_props fix, and Stalwart test server support. I'm also upgrading the icalendar dependency to icalendar7. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lots of minor commits, most of them trying to get all the integration tests to pass