Fix CalDAV/CardDAV multiget loading entire collections into memory
A REPORT multiget for a handful of resources previously listed the
whole calendar/address book (every row incl. ical_data/vcard) and
filtered by href in Rust with an O(N×M) substring scan. Large
collections paid full-table latency, RAM and DB CPU on every sync.
- Add find_events_by_ical_uids / get_contacts_by_uids through every
layer (domain repo trait → PG repo → storage port/adapter → use case
→ handler) using one indexed `= ANY($2)` query, mirroring the
existing single-UID lookups.
- Extract UIDs from multiget hrefs with a shared, tested
uid_from_multiget_href helper (percent-decoding, case-insensitive
extension strip, collection hrefs rejected). Exact UID matching also
removes the false positives the old substring filter allowed.
- CalDAV PROPFIND single-event path now uses the existing indexed
get_event_by_ical_uid instead of listing all events.
- Honour the already-documented limit/offset query params on
GET /api/address-books/{id}/contacts: optional ListQuery fields
thread through ContactUseCase::list_contacts to a paginated repo
query. Omitted params keep returning the full book (frontend and
DAV listing/sync paths unchanged).
https://claude.ai/code/session_0193Hff42gaA962wThxMGSd1
This commit is contained in:
@@ -8,3 +8,91 @@ pub mod webdav_adapter;
|
||||
mod caldav_adapter_test;
|
||||
#[cfg(test)]
|
||||
mod carddav_adapter_test;
|
||||
|
||||
/// Extract the resource UID from a DAV multiget `href`.
|
||||
///
|
||||
/// CalDAV/CardDAV multiget REPORTs address object resources by full href
|
||||
/// (e.g. `/caldav/{calendar_id}/{uid}.ics`, possibly with a username
|
||||
/// segment). The UID is the last path segment with the protocol
|
||||
/// `extension` (`.ics` / `.vcf`, matched case-insensitively) stripped,
|
||||
/// then percent-decoded — hrefs arrive in the XML body, so they have not
|
||||
/// gone through URL-path decoding, and clients may re-encode hrefs they
|
||||
/// previously read from the server.
|
||||
///
|
||||
/// Returns `None` for collection hrefs (empty last segment) or segments
|
||||
/// that are not valid UTF-8 after decoding.
|
||||
pub fn uid_from_multiget_href(href: &str, extension: &str) -> Option<String> {
|
||||
// A trailing slash denotes a collection, not an object resource.
|
||||
if href.ends_with('/') {
|
||||
return None;
|
||||
}
|
||||
let segment = href.rsplit('/').next()?;
|
||||
|
||||
// Case-insensitive ASCII extension strip; the matched tail is ASCII,
|
||||
// so the byte cut is guaranteed to land on a char boundary.
|
||||
let bytes = segment.as_bytes();
|
||||
let ext = extension.as_bytes();
|
||||
let segment =
|
||||
if bytes.len() >= ext.len() && bytes[bytes.len() - ext.len()..].eq_ignore_ascii_case(ext) {
|
||||
&segment[..segment.len() - ext.len()]
|
||||
} else {
|
||||
segment
|
||||
};
|
||||
|
||||
let decoded = percent_encoding::percent_decode_str(segment)
|
||||
.decode_utf8()
|
||||
.ok()?;
|
||||
let uid = decoded.trim();
|
||||
(!uid.is_empty()).then(|| uid.to_string())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod multiget_href_tests {
|
||||
use super::uid_from_multiget_href;
|
||||
|
||||
#[test]
|
||||
fn plain_caldav_href() {
|
||||
assert_eq!(
|
||||
uid_from_multiget_href("/caldav/abc-123/event-uid.ics", ".ics"),
|
||||
Some("event-uid".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn href_with_username_prefix() {
|
||||
assert_eq!(
|
||||
uid_from_multiget_href("/carddav/alice/book-1/uid-42.vcf", ".vcf"),
|
||||
Some("uid-42".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn uppercase_extension() {
|
||||
assert_eq!(
|
||||
uid_from_multiget_href("/caldav/abc/EVENT.ICS", ".ics"),
|
||||
Some("EVENT".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn percent_encoded_uid() {
|
||||
assert_eq!(
|
||||
uid_from_multiget_href("/caldav/abc/uid%40example.com.ics", ".ics"),
|
||||
Some("uid@example.com".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_extension_uses_whole_segment() {
|
||||
assert_eq!(
|
||||
uid_from_multiget_href("/caldav/abc/bare-uid", ".ics"),
|
||||
Some("bare-uid".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn collection_href_yields_none() {
|
||||
assert_eq!(uid_from_multiget_href("/caldav/abc/", ".ics"), None);
|
||||
assert_eq!(uid_from_multiget_href("", ".ics"), None);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -95,6 +95,14 @@ pub trait CalendarStoragePort: Send + Sync + 'static {
|
||||
calendar_id: &str,
|
||||
ical_uid: &str,
|
||||
) -> Result<Option<CalendarEventDto>, DomainError>;
|
||||
/// Indexed batch lookup by iCalendar UID (`ical_uid = ANY(...)`) — the
|
||||
/// CalDAV multiget REPORT must use this instead of listing the whole
|
||||
/// calendar (every row + its `ical_data`) and filtering client-side.
|
||||
async fn find_events_by_ical_uids(
|
||||
&self,
|
||||
calendar_id: &str,
|
||||
ical_uids: &[String],
|
||||
) -> Result<Vec<CalendarEventDto>, DomainError>;
|
||||
async fn list_events_by_calendar(
|
||||
&self,
|
||||
calendar_id: &str,
|
||||
@@ -197,6 +205,15 @@ pub trait CalendarUseCase: Send + Sync + 'static {
|
||||
ical_uid: &str,
|
||||
user_id: Uuid,
|
||||
) -> Result<Option<CalendarEventDto>, DomainError>;
|
||||
/// Resolve a batch of events by their iCalendar UIDs with a single
|
||||
/// indexed query. UIDs without a matching event are silently absent
|
||||
/// from the result (CalDAV multiget semantics).
|
||||
async fn get_events_by_ical_uids(
|
||||
&self,
|
||||
calendar_id: &str,
|
||||
ical_uids: &[String],
|
||||
user_id: Uuid,
|
||||
) -> Result<Vec<CalendarEventDto>, DomainError>;
|
||||
async fn list_events(
|
||||
&self,
|
||||
calendar_id: &str,
|
||||
|
||||
@@ -82,9 +82,25 @@ pub trait ContactUseCase: Send + Sync + 'static {
|
||||
uid: &str,
|
||||
user_id: Uuid,
|
||||
) -> Result<Option<ContactDto>, DomainError>;
|
||||
/// Resolve a batch of contacts by their vCard UIDs with a single
|
||||
/// indexed query (`uid = ANY(...)`) — the CardDAV multiget REPORT
|
||||
/// must use this instead of listing the whole address book and
|
||||
/// filtering client-side. UIDs without a matching contact are
|
||||
/// silently absent from the result.
|
||||
async fn get_contacts_by_uids(
|
||||
&self,
|
||||
address_book_id: &str,
|
||||
uids: &[String],
|
||||
user_id: Uuid,
|
||||
) -> Result<Vec<ContactDto>, DomainError>;
|
||||
/// List contacts in an address book. `limit`/`offset` bound the
|
||||
/// result for paginated callers (REST API); `None` returns the full
|
||||
/// book, which the CardDAV listing/sync paths rely on.
|
||||
async fn list_contacts(
|
||||
&self,
|
||||
address_book_id: &str,
|
||||
limit: Option<i64>,
|
||||
offset: Option<i64>,
|
||||
user_id: Uuid,
|
||||
) -> Result<Vec<ContactDto>, DomainError>;
|
||||
async fn search_contacts(
|
||||
|
||||
@@ -300,6 +300,32 @@ impl CalendarUseCase for CalendarService {
|
||||
.await
|
||||
}
|
||||
|
||||
async fn get_events_by_ical_uids(
|
||||
&self,
|
||||
calendar_id: &str,
|
||||
ical_uids: &[String],
|
||||
user_id: Uuid,
|
||||
) -> Result<Vec<CalendarEventDto>, DomainError> {
|
||||
let has_access = self
|
||||
.calendar_storage
|
||||
.check_calendar_access(calendar_id, user_id)
|
||||
.await?;
|
||||
let calendar = self.calendar_storage.get_calendar(calendar_id).await?;
|
||||
if !has_access && !calendar.is_public {
|
||||
return Err(DomainError::new(
|
||||
ErrorKind::AccessDenied,
|
||||
"Calendar",
|
||||
"You don't have permission to view events in this calendar",
|
||||
));
|
||||
}
|
||||
if ical_uids.is_empty() {
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
self.calendar_storage
|
||||
.find_events_by_ical_uids(calendar_id, ical_uids)
|
||||
.await
|
||||
}
|
||||
|
||||
async fn list_events(
|
||||
&self,
|
||||
calendar_id: &str,
|
||||
|
||||
@@ -796,9 +796,34 @@ impl ContactUseCase for ContactService {
|
||||
Ok(contact.map(ContactDto::from))
|
||||
}
|
||||
|
||||
async fn get_contacts_by_uids(
|
||||
&self,
|
||||
address_book_id: &str,
|
||||
uids: &[String],
|
||||
user_id: Uuid,
|
||||
) -> Result<Vec<ContactDto>, DomainError> {
|
||||
let id = Uuid::parse_str(address_book_id)
|
||||
.map_err(|_| DomainError::validation_error("Invalid address book ID format"))?;
|
||||
|
||||
// Check if user has access to the address book
|
||||
self.check_address_book_access(&id, &user_id).await?;
|
||||
|
||||
if uids.is_empty() {
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
|
||||
let contacts = self
|
||||
.contact_repository
|
||||
.get_contacts_by_uids(&id, uids)
|
||||
.await?;
|
||||
Ok(contacts.into_iter().map(ContactDto::from).collect())
|
||||
}
|
||||
|
||||
async fn list_contacts(
|
||||
&self,
|
||||
address_book_id: &str,
|
||||
limit: Option<i64>,
|
||||
offset: Option<i64>,
|
||||
user_id: Uuid,
|
||||
) -> Result<Vec<ContactDto>, DomainError> {
|
||||
let id = Uuid::parse_str(address_book_id)
|
||||
@@ -808,10 +833,17 @@ impl ContactUseCase for ContactService {
|
||||
self.check_address_book_access(&id, &user_id).await?;
|
||||
|
||||
// Get contacts
|
||||
let contacts = self
|
||||
.contact_repository
|
||||
.get_contacts_by_address_book(&id)
|
||||
.await?;
|
||||
let contacts = if limit.is_some() || offset.is_some() {
|
||||
let limit = limit.unwrap_or(100);
|
||||
let offset = offset.unwrap_or(0);
|
||||
self.contact_repository
|
||||
.get_contacts_by_address_book_paginated(&id, limit, offset)
|
||||
.await?
|
||||
} else {
|
||||
self.contact_repository
|
||||
.get_contacts_by_address_book(&id)
|
||||
.await?
|
||||
};
|
||||
let dtos = contacts.into_iter().map(ContactDto::from).collect();
|
||||
|
||||
Ok(dtos)
|
||||
@@ -1324,7 +1356,9 @@ impl StorageUseCase for ContactService {
|
||||
let user_id = Uuid::parse_str(user_id)
|
||||
.map_err(|_| DomainError::validation_error("Invalid user_id format"))?;
|
||||
|
||||
let result = self.list_contacts(address_book_id, user_id).await?;
|
||||
let result = self
|
||||
.list_contacts(address_book_id, None, None, user_id)
|
||||
.await?;
|
||||
Ok(serde_json::to_value(result).unwrap())
|
||||
}
|
||||
"search_contacts" => {
|
||||
|
||||
Reference in New Issue
Block a user