compatibility fix: date-ranged searches without comptype#682
Draft
tobixen wants to merge 10 commits into
Draft
Conversation
A CALDAV:time-range filter is only valid inside a comp-filter for VEVENT/VTODO/VJOURNAL/VFREEBUSY/VALARM (RFC4791 section 9.7), never directly under VCALENDAR. When search() was called with a time-range but no component type, the library emitted an illegal query that SabreDAV-based servers (Baikal, Nextcloud, ...) reject with HTTP 400 "You cannot add time-range filters on the VCALENDAR component". This worked previously only because lenient servers tolerated it. Fixes: - new feature search.time-range.comp-type.optional (default: unsupported, which is fully RFC-compliant and not a server defect). When not supported, a comp-type-less time-range search is split into one query per component type. - reactive fallback: if the feature is configured as supported but the server still rejects the query, retry by splitting per component type. - driver fix: search()'s generator driver now feeds exceptions raised while executing an action back INTO the generator via gen.throw(), so the search logic's own try/except branches (the #681 fallback, the per-object load error handling, the backward-compat report retry) are no longer dead code. Async driver mirror and integration tests follow separately. #681 prompt: look into github issue #681 - any suggestions? followup-prompt: So I think we need: 1) split the search.comp-type.optional test in caldav-server-tester, 2) explicit test without comp-type but with date-range, 3) run twice with patched config, 4) workaround in code. followup-prompt: deal with sync tests and logic first, then async. All sync integration tests should be mirrored in the async integration tests. followup-prompt: we should have test code for the driver fix, too Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> AI Prompts: claude-sonnet-4-6: I closed `git bug bug show e44ee06` aka #667 as I have the impression that the work there is done. Please verify. claude-sonnet-4-6: look into github issue #681 - any suggestions? claude-sonnet-4-6: Don't we have any integration tests doing `cal.search(start=..., end=...)`, or perhaps it passes because of `'search.comp-type.optional': {'support': 'ungraceful'}` in the compatibility matrix? claude-sonnet-4-6: So I think we need: 1) in the caldav-server-tester, the test for `search.comp-type.optional` must be split - perhaps a separate `search.time-range.comp-type.optional` (with comments that False is completely fine according to the RFC)) - and the search logic in the caldav library must be fixed similarly. 2) We need an explicit test that does a search without comp-type but with date-range, 3) for servers not supporting search.time-range.comp-type.optional said integration test should be run twice, once with the server feature configuration patched up. The latter test is supposed to fail at baikla. 4) We need a workaround in the code so that such errors will be caught and handled, causing the test to pass.
search(..., compatibility_workarounds=False) disables every server- compatibility workaround in _search_impl (comp-type splitting, filter rewriting, sliding-window time-range injection, fallback retries, the todo pending-task multi-query) and sends the query the searcher describes verbatim as a single REPORT. This lets the server-compatibility checker observe raw server behaviour instead of the worked-around behaviour - needed now that the issue #681 fix makes the library rewrite comp-type-less time-range queries. The flag is stored as a dataclass field so it propagates to clones via dataclasses.replace(); the search() parameter defaults to None so internal recursive calls leave the searcher's setting untouched. #681 prompt: I suggest adding a flag to the search function to supress any comaptibility-workarounds Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…r.search - Calendar.search() now accepts and forwards compatibility_workarounds to the searcher (previously it was swallowed into **searchargs and wrongly turned into a bogus property filter). - async_search() gains the same compatibility_workarounds parameter for symmetry (async tests/driver mirror still to come). - new integration test testSearchWithoutCompTypeWithDateRange: a comp-type-less time-range search must work, including a second run with search.time-range.comp-type.optional forced on to exercise the reactive HTTP-400 fallback. Verified passing against Baikal (SabreDAV 4.7.0, which rejects the raw query with the issue #681 error). #681 followup-prompt: I suggest adding a flag to the search function to supress any comaptibility-workarounds Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- async_search()'s generator driver now mirrors the sync driver: it feeds exceptions raised while executing an action back into the generator via gen.throw(), so the search logic's own try/except branches (the issue #681 reactive time-range fallback, per-object load error handling) work in the async path too. - new async integration test test_search_without_comptype_with_date_range mirrors the sync testSearchWithoutCompTypeWithDateRange. Verified passing against Baikal and Nextcloud (both SabreDAV-based and both reject the raw comp-type-less time-range query with the issue #681 error). #681 followup-prompt: deal with sync tests and logic first, then async. All sync integration tests should be mirrored in the async integration tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> AI Prompts: claude-sonnet-4-6: continue
#681 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Now that the comp-type.optional probe no longer carries a time-range, the observed values for several servers changed (the old "ungraceful" readings were artifacts of a checker bug where the probe sent a time-range that SabreDAV-likes reject): - nextcloud, cyrus: search.comp-type.optional ungraceful -> full - sogo: search.comp-type.optional ungraceful -> unsupported (returns nothing without a comp-type) and search.time-range.comp-type.optional -> full (works when a time-range is present) - bedework, zimbra: search.time-range.comp-type.optional -> full Verified: testCheckCompatibility passes for all five servers. #681 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> AI Prompts: claude-sonnet-4-6: `pytest -k 'compat and xandikos'` fails, radicale fails similarly. I find it a bit weird if search.comp-type.optional is ungraceful while search.time-range.comp-type.optional is supported. Please investigate. ERROR root:checks_base.py:70 Server checker found something unexpected for search.comp-type.optional. Expected: {'support': 'full'}, observed: {'support': 'ungraceful'} ERROR root:checks_base.py:70 Server checker found something unexpected for search.time-range.comp-type.optional. Expected: {'support': 'unsupported'}, observed: {'support': 'full'} claude-sonnet-4-6: <task-notification> <task-id>bwekpm388</task-id> <tool-use-id>toolu_012V75iTS2hPpbKUqyipHxTW</tool-use-id> <output-file>/tmp/claude-7385/-home-tobias-caldav/a1535ac2-6269-4140-8902-1a870106aaab/tasks/bwekpm388.output</output-file> <status>completed</status> <summary>Background command "Run all testCheckCompatibility tests" completed (exit code 0)</summary> </task-notification> claude-sonnet-4-6: continue calibrating compatibility_hints for issue #681 from the b93fha8id compat run results claude-sonnet-4-6: <task-notification> <task-id>blvu1dml7</task-id> <tool-use-id>toolu_014LYExUYBbQ4j7RmHvkqn8d</tool-use-id> <output-file>/tmp/claude-7385/-home-tobias-caldav/a1535ac2-6269-4140-8902-1a870106aaab/tasks/blvu1dml7.output</output-file> <status>completed</status> <summary>Background command "Re-run 5 servers' compat after hint calibration" completed (exit code 0)</summary> </task-notification>
Follow-up to the earlier calibration. The first pass marked SOGo, Bedework and Zimbra as supporting search.time-range.comp-type.optional, but the checker probe only verified the query did not error - SOGo and Bedework silently return NOTHING for a comp-type-less time-range query, which is not real support. With the probe now requiring that a known in-range event is actually returned, the verified picture is: - genuinely supported (event returned): xandikos, radicale, davical, zimbra -> search.time-range.comp-type.optional = full - silently returns nothing: sogo, bedework -> unsupported (the default) - rejects with HTTP 400 (SabreDAV): nextcloud, cyrus, baikal, davis, ... -> unsupported (the default) Verified: testCheckCompatibility passes for all servers (Zimbra's remaining failure is an unrelated, pre-existing flake on scheduling.auto-schedule). #681 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> AI Prompts: claude-sonnet-4-6: continue: verify the 5-server compat re-run (blvu1dml7) passed, then commit the compatibility_hints calibration and the list() checker fix for issue #681 claude-sonnet-4-6: Some servers does not support comp-type.optional, but do support time-range.comp-type.optional - that sounds odd, please verify that it's actually true. claude-sonnet-4-6: <task-notification> <task-id>bofybhimi</task-id> <tool-use-id>toolu_0152eaafMhsKhM2UKPXJGtWz</tool-use-id> <output-file>/tmp/claude-7385/-home-tobias-caldav/a1535ac2-6269-4140-8902-1a870106aaab/tasks/bofybhimi.output</output-file> <status>completed</status> <summary>Background command "Re-run servers with corrected probe" completed (exit code 0)</summary> </task-notification> claude-sonnet-4-6: <task-notification> <task-id>bg939r71a</task-id> <tool-use-id>toolu_01VWyJoCZybKHUM5nUE16WP2</tool-use-id> <output-file>/tmp/claude-7385/-home-tobias-caldav/a1535ac2-6269-4140-8902-1a870106aaab/tasks/bg939r71a.output</output-file> <status>completed</status> <summary>Background command "Verify import and run full compat suite" completed (exit code 0)</summary> </task-notification> AI Prompts: claude-sonnet-4-6: what is the connection details for the nextcloud docker server? (please give it in json format, for inclusion in my calendar config file) claude-sonnet-4-6: Fix the README claude-sonnet-4-6: continue: read bg939r71a full compat results; if all green commit the corrected hints (caldav) and the stricter probe (server-tester) for issue #681; otherwise fix the flagged server hints
Generalises the issue #681 fix from time-range filters to property (prop-filter) filters. A search with a property filter (CATEGORIES, SUMMARY, ...) but no component type put the prop-filter directly under the VCALENDAR comp-filter, where it matches VCALENDAR's own properties - which do not include component properties like CATEGORIES - so servers (Xandikos, SabreDAV, ...) silently returned nothing. The library now splits such a search into one query per component type, governed by the new feature search.text.comp-type.optional (default unsupported - verified to be the universal case, since a prop-filter under VCALENDAR is meaningless on every tested server). The reactive per-component-type fallback is likewise extended to property filters for servers that reject (rather than silently ignore) the query. Adds unit tests, sync (testSearchWithoutCompTypeWithCategory) and async integration tests. Verified against Baikal and Xandikos; full testCheckCompatibility matrix unaffected (no server needs calibration). #681 prompt: Now the same issue applies when filtering i.e. on CATEGORIES and other attributes. The VCALENDAR does not have such a property, so at least xandikos will not find anything is searching for a specific category but without a compfilter. We need a check for this, too. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> AI Prompts: claude-sonnet-4-6: Now the same issue applies when filtering i.e. on CATEGORIES and other attributes. The VCALENDAR does not have such a property, so at least xandikos will not find anything is searching for a specific category but without a compfilter. We need a check for this, too.
CI failure: test_compatibility_hints.test_intermediate_feature_derives_from_children broke because the intermediate grouping nodes search.time-range.comp-type and search.text.comp-type (added so find_feature could walk the dotted parents of ...comp-type.optional) had no explicit default, so they were counted as subfeatures of search.time-range / search.text and polluted those parents' subfeature-derivation. Fix: use a single dotted segment "comp-type-optional" (dash, not dot), so the features are direct children of search.time-range / search.text and need no intermediate grouping node: search.time-range.comp-type.optional -> search.time-range.comp-type-optional search.text.comp-type.optional -> search.text.comp-type-optional The grouping nodes are removed. The original search.comp-type.optional feature is untouched. #681 prompt: actually, the proper fix is probably to replace a dot with a dash. comp-type-optional instead of comp-type.optional. At all three places. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> AI Prompts: claude-sonnet-4-6: run the full compat matrix once more to confirm green claude-sonnet-4-6: <task-notification> <task-id>b09i5stdr</task-id> <tool-use-id>toolu_01PhuKFr8hK8KSG9Dh9jULse</tool-use-id> <output-file>/tmp/claude-7385/-home-tobias-caldav/a1535ac2-6269-4140-8902-1a870106aaab/tasks/b09i5stdr.output</output-file> <status>completed</status> <summary>Background command "Full compat matrix run" completed (exit code 0)</summary> </task-notification> claude-sonnet-4-6: continue: read b09i5stdr full compat matrix results and report whether all servers are green claude-sonnet-4-6: github runs failed
…tion CI failure on Cyrus: testSearchWithoutCompTypeWithDateRange's second run forced search.time-range.comp-type-optional ON and asserted the event was returned, relying on the reactive HTTP-400 fallback. But that fallback only recovers from a ReportError (SabreDAV's 400) - Cyrus instead returns nothing (CI) or 403 (locally), so the forced run either found nothing or crashed. The forced "Run 2" is only meaningful where the raw comp-type-less time-range query raises a ReportError (Baikal, Nextcloud). The test now probes the raw behaviour first and only runs Run 2 in that case; other servers exercise just the proactive split (Run 1). The reactive fallback itself remains covered deterministically by the unit test. #681 Co-Authored-By: Claude Opus 4.8 <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.
This should fix issue #681.
AI-generated code, with very little human review - changes should be examinated carefully, git messages should probably be reweritten, need to test it thoroughly, etc, hence it's in "draft" mode as for now.