Skip to content

CalDAV adapter - #33

Merged
SashankBhamidi merged 4 commits into
mainfrom
feat/caldav-adapter
Oct 25, 2026
Merged

SashankBhamidi merged 4 commits into
mainfrom
feat/caldav-adapter

Conversation

@SashankBhamidi

Copy link
Copy Markdown
Member

Closes #10.

This brings in the CalDAV adapter, CalDAVAdapter and its async twin, behind a new caldav extra. I didn't want the sync and async versions drifting apart, so the whole sync flow is written once in adapters/_flow.py, and the two adapters only drive it differently. The JMAP adapter will sit on the same base.

For testing, I recorded real conversations with Radicale, Xandikos, Cyrus, Stalwart and Baïkal, and the unit tests replay those. The integration tests run the same scenarios live against all five, including a token that actually expires and a login that gets refused.

Along the way I had to go past the issue text in a few places:

  • python-caldav 3.6.0 landed with byte-exact raw_data, precondition="etag" and If-None-Match: * on save(no_overwrite=True), so I bumped the floor to it, and creates now go through save() instead of put().
  • python-caldav hands back the same member as %40 from a sync report and @ from a multiget, so object ids are now the href's path in one spelling.
  • A change refused for its UID stays pending for now, since the core only knows LogicalKeyTaken for creates.

The servers had some surprises too, all on the compatibility page. The one that worried me most: Baïkal accepts any token with its own prefix, even one it never issued, and simply returns no changes. A client holding such a token would miss changes without any error.

@tobixen, two python-caldav things I noticed:

  • sync-collection goes out with Depth: 1, though RFC 6578 section 3.2 only defines it for Depth: 0. Every server I tried answers it anyway.
  • A 507 from propfind comes back as a ResponseError with status set to None, so for now I'm reading it from the message.

One cost to know about: canonical fingerprints take around 2.4 s for 20,000 events, mostly inside icalendar's parser. I've added a benchmark scenario so we can keep an eye on it.

@SashankBhamidi
SashankBhamidi removed the request for review from stevepiercy October 23, 2026 18:01
@read-the-docs-community

read-the-docs-community Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

@codecov

codecov Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@tobixen

tobixen commented Oct 23, 2026

Copy link
Copy Markdown
Member

The servers had some surprises too, all on the compatibility page.

I'm adding those checks to the caldav-server-tester.

  • sync-collection goes out with Depth: 1, though RFC 6578 section 3.2 only defines it for Depth: 0. Every server I tried answers it anyway.

This has been fixed in a PR now.

  • A 507 from propfind comes back as a ResponseError with status set to None, so for now I'm reading it from the message.

I will look into this as well.

@tobixen tobixen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if the integration tests could be run towards arbitrary servers. I do have a list of servers I test the caldav library towards. I also have on my list to make the list exportable from the caldav library, making it easier to maintain it in one place and run different libraries towards it - see python-caldav/caldav#817

The text below the line is generated by Claude Opus 5.5


This is a lot of careful work, and the recorded tapes plus live scenarios across five servers are great. I reviewed c40782b against python-caldav 3.6.0. The 243 CalDAV unit tests pass. The points below come from reading the code; I haven't reproduced them. The first six look like real bugs. The rest are robustness and tidy-up points.

Comment thread src/calendaring_sync/adapters/caldav.py Outdated
Comment thread src/calendaring_sync/adapters/_flow.py Outdated
Comment thread src/calendaring_sync/adapters/caldav.py Outdated
Comment thread src/calendaring_sync/adapters/caldav.py Outdated
Comment thread src/calendaring_sync/adapters/caldav.py
Comment thread src/calendaring_sync/adapters/caldav.py
Comment thread src/calendaring_sync/adapters/caldav.py Outdated
Comment thread src/calendaring_sync/adapters/caldav.py Outdated
Comment thread src/calendaring_sync/adapters/caldav.py Outdated
Comment thread src/calendaring_sync/tests/test_caldav_unit.py Outdated
@SashankBhamidi

Copy link
Copy Markdown
Member Author

Thanks @tobixen, really useful review :) Fixed most of it in a710a61 and answered the rest in the threads.
Nice to see the Depth fix in python-caldav/caldav#818.
And running the integration tests against your server list sounds good, will pick it up once python-caldav/caldav#817 is in.

@tobixen tobixen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: AI-generated text below the line, generated by Claude Opus 5.5


@SashankBhamidi, the fixes in a710a61 look right to me, and the three open threads (weak ETags, a UID taken on an update, the 507 workaround) are fine as later work. Approving.

As I said I would, your page's checks are now probes in caldav-server-tester (python-caldav/caldav-server-tester#36, python-caldav/caldav-server-tester#37). The results are python-caldav compatibility hints (python-caldav/caldav#822), measured on 22 servers. Once that's released, a DAVClient configured with a server's name carries them as client.features, so the adapter can ask instead of hard-coding servers. These matter for the adapter:

sync-token.delete.created-and-deleted: Stalwart depends on what else changed, not on the version. On 0.16.11, a member created and deleted between two reports is left out unless a member that existed at the old token also changed in the same interval:

changes between the two reports reported
create e, delete e nothing
create e, delete e, create f f only
create e, delete e, change or delete an existing a e as 404, and a

That's why our first probe passed Stalwart and your page didn't. The probe now tests the short-lived member in an interval of its own. Xandikos and Bedework 5 never report it.

sync-token.recreated is broken on Nextcloud (and ecloud) and Synology. A member deleted and re-created at the same href is reported only as a 404, with no changed entry; I checked by hand that a GET answers 200 while the sync says 404. The adapter then marks an existing object REMOVED, and nothing brings it back short of a full listing or a later change. Worth a row. Stalwart's twice-reported case is a quirk.

sync-token.initial-sync: CCS lists the collection itself, like Stalwart; the adapter's filter covers both. Radicale lists deleted members with a 404.

delete.if-match is broken on Bedework (3 and 5), Zimbra, Posteo and SOGo: a DELETE whose If-Match no longer matches deletes the object anyway, so a conditional delete can remove a remote edit there.

delete.missing.if-match: Baïkal, Nextcloud, Davis, CCS, OX, Purelymail, Robur and Infomaniak answer a DELETE with If-Match on a missing member with 412. The adapter already leaves such a delete pending, so this only widens the "a tested server" sentence.

multiget.missing-href: Baïkal, Nextcloud, DAViCal, Davis and Infomaniak leave the href out of the multistatus, which the adapter handles. Posteo fails the whole calendar-multiget with a 404 instead, so a member removed between report and multiget would make each sync raise there.

save.etag.matches-stored: Radicale, Xandikos, CCS and Infomaniak reorder properties, SOGo adds CLASS, Zimbra and Bedework 5 add properties, and all of them still answer the PUT with a strong ETag (RFC 4791 section 5.3.4 forbids that). Clearing the version after a push, as the adapter does, is the right call.

multiget.limit, propfind.limit: Stalwart stops at WebDav.maxResults with a 507; caldav's Stalwart test container now runs with 50, so this shows up with few objects.

python-caldav, from 3.7.0:

  • ResponseError.status is set for failed replies (python-caldav/caldav@824e26e3), for the 507 thread.
  • sync-collection goes out with Depth: 0, so the Depth: 1 sentence in docs/explanation/adapters.rst needs a version.
  • Infomaniak adds an empty propstat with 418 to a removed member in a sync reply, which made python-caldav raise on every sync after a deletion: python-caldav/caldav#821.

@SashankBhamidi

Copy link
Copy Markdown
Member Author

Thanks @tobixen, the server-tester results helped a lot. I've handled them in this PR itself: a re-created member on Nextcloud and Synology is fetched now, Posteo's multiget 404 falls back to one member at a time, the 507 is read from the status, and the Depth sentence has a version. Your results are on the compatibility page too, credited to the tester. Could you have another look when you get time?

@tobixen

tobixen commented Oct 25, 2026

Copy link
Copy Markdown
Member

LGTM

@SashankBhamidi
SashankBhamidi merged commit 8407518 into main Oct 25, 2026
14 checks passed
@SashankBhamidi
SashankBhamidi deleted the feat/caldav-adapter branch October 25, 2026 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CalDAV adapter

2 participants