perf: round 4 — one-pass row paths, drive-selector cache, CalDAV single-parse, streamed Azure, batched hydration
Nine benchmark-gated changes (benches/ROUND4.md; every one ships with a BEFORE/AFTER bench + equivalence gate, rollback rule as ROUND2/3): - Row→entity path build: one-pass StoragePath::from_folder_and_name / from_joined + normalize_storage_name_owned + alloc-free Display — 743→417 ns/file-row (1.78x), −5 allocs/row on every listing surface. - WebDAV drive-selector: per-user readable_cache (single-flight, 30 s TTL, explicit invalidation incl. membership + group changes) replaces the grants join per request — 441 µs → 0.8 µs (~550x), 0 queries warm. - CalDAV from_ical/update_ical_data: 8 full IcalParser runs per VEVENT → 1 (7.1x per PUT, 4.4x on 50-event imports); alloc-free split_vevents, chunk scan without the whole-body uppercase copy (1.4x), borrowed-key UID grouping (1.3x), REPORT props no longer cloned. - PROPFIND emit: partition Vecs dropped (single-pass 404 list) + stack rendered RFC 3339/2822 dates, sizes, quoted etags (common::fmt, chrono-byte-identical, sweep-tested) on both DAV surfaces — 1.22x per page, 17.9→12.0 allocs/row. - Grant-listing hydration: calendars/address books/playlists batch hydrate via = ANY($1) — 15 serial queries → 1 (~13x per sync poll). - user-flags cache: get→insert → try_get_with single-flight (32→1 queries per cold herd). - Azure downloads: whole-blob Vec buffering → streamed SDK pages — TTFB 349→4 ms (87x), peak heap 480→1.9 MiB (254x) on 256 MiB blobs; new OXICLOUD_AZURE_ENDPOINT_URL override (Azurite/bench hook). - Face indexing: unbounded per-image tokio::spawn → core-count semaphore, permit before blob read — peak heap 1175→176 MiB (6.7x). Checks: cargo fmt, clippy --all-features --all-targets -D warnings, cargo test --workspace (523 passed) + --features test_utils. hurl API suite and dockerized integration DB not runnable in this environment — left to CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017aJu9ghvuT8WqC31ZEGTBA
This commit is contained in:
Claude
@@ -250,25 +250,36 @@ impl CalendarEvent {
|
||||
* @return Result containing the new CalendarEvent or a domain error
|
||||
*/
|
||||
pub fn from_ical(calendar_id: Uuid, ical_data: String) -> Result<Self> {
|
||||
// This implementation would require a proper iCalendar parser
|
||||
// For brevity, we're using a simplified version here
|
||||
// Parse the body ONCE and read every property from the parsed
|
||||
// component. The previous shape funnelled each of the 8 property
|
||||
// lookups below through `extract_ical_property[_with_params]`,
|
||||
// which re-ran the full `IcalParser` (line unfolding + component
|
||||
// tree build) per property — 8 complete parses per VEVENT on
|
||||
// every CalDAV PUT / import. A missing-or-unparseable body maps
|
||||
// to the same "Missing SUMMARY" error the old first lookup
|
||||
// produced, preserving error parity.
|
||||
let event = Self::parse_first_vevent(&ical_data);
|
||||
|
||||
// Extract required fields from iCalendar data
|
||||
let summary = Self::extract_ical_property(&ical_data, "SUMMARY").ok_or_else(|| {
|
||||
DomainError::new(
|
||||
ErrorKind::InvalidInput,
|
||||
"CalendarEvent",
|
||||
"Missing SUMMARY in iCalendar data",
|
||||
)
|
||||
})?;
|
||||
// Extract required fields from the parsed component
|
||||
let summary = event
|
||||
.as_ref()
|
||||
.and_then(|e| Self::prop_value(e, "SUMMARY"))
|
||||
.ok_or_else(|| {
|
||||
DomainError::new(
|
||||
ErrorKind::InvalidInput,
|
||||
"CalendarEvent",
|
||||
"Missing SUMMARY in iCalendar data",
|
||||
)
|
||||
})?;
|
||||
let event = event.expect("prop_value returned Some, so the parse succeeded");
|
||||
|
||||
// DTSTART / DTEND: use the params-aware extractor so we can
|
||||
// detect `VALUE=DATE` (all-day) from the property parameters
|
||||
// rather than scanning the raw property line. The pre-parser-
|
||||
// rewrite substring scan couldn't see param-carrying lines at
|
||||
// all — see #528.
|
||||
let (dtstart_value, dtstart_params) =
|
||||
Self::extract_ical_property_with_params(&ical_data, "DTSTART").ok_or_else(|| {
|
||||
let (dtstart_value, dtstart_params) = Self::prop_with_params(&event, "DTSTART")
|
||||
.ok_or_else(|| {
|
||||
DomainError::new(
|
||||
ErrorKind::InvalidInput,
|
||||
"CalendarEvent",
|
||||
@@ -277,7 +288,7 @@ impl CalendarEvent {
|
||||
})?;
|
||||
|
||||
let (dtend_value, _dtend_params) =
|
||||
Self::extract_ical_property_with_params(&ical_data, "DTEND").ok_or_else(|| {
|
||||
Self::prop_with_params(&event, "DTEND").ok_or_else(|| {
|
||||
DomainError::new(
|
||||
ErrorKind::InvalidInput,
|
||||
"CalendarEvent",
|
||||
@@ -313,13 +324,13 @@ impl CalendarEvent {
|
||||
})?;
|
||||
|
||||
// Extract optional fields
|
||||
let description = Self::extract_ical_property(&ical_data, "DESCRIPTION");
|
||||
let location = Self::extract_ical_property(&ical_data, "LOCATION");
|
||||
let rrule = Self::extract_ical_property(&ical_data, "RRULE");
|
||||
let description = Self::prop_value(&event, "DESCRIPTION");
|
||||
let location = Self::prop_value(&event, "LOCATION");
|
||||
let rrule = Self::prop_value(&event, "RRULE");
|
||||
|
||||
// Extract UID or generate a new one
|
||||
let ical_uid = Self::extract_ical_property(&ical_data, "UID")
|
||||
.unwrap_or_else(|| Uuid::new_v4().to_string());
|
||||
let ical_uid =
|
||||
Self::prop_value(&event, "UID").unwrap_or_else(|| Uuid::new_v4().to_string());
|
||||
|
||||
// RECURRENCE-ID (RFC 5545 §3.8.4.4). When present, this VEVENT
|
||||
// is an override for a specific occurrence of a recurring
|
||||
@@ -329,17 +340,16 @@ impl CalendarEvent {
|
||||
// gets stored, just as a plain event (worst case a client sync
|
||||
// treats it as a new master, which the DB uniqueness will
|
||||
// refuse; better a persistence error than a silent split).
|
||||
let recurrence_id =
|
||||
match Self::extract_ical_property_with_params(&ical_data, "RECURRENCE-ID") {
|
||||
Some((value, params)) => {
|
||||
let is_date = params
|
||||
.get("VALUE")
|
||||
.map(|vs| vs.iter().any(|v| v.eq_ignore_ascii_case("DATE")))
|
||||
.unwrap_or(false);
|
||||
Self::parse_ical_datetime(&value, is_date).ok()
|
||||
}
|
||||
None => None,
|
||||
};
|
||||
let recurrence_id = match Self::prop_with_params(&event, "RECURRENCE-ID") {
|
||||
Some((value, params)) => {
|
||||
let is_date = params
|
||||
.get("VALUE")
|
||||
.map(|vs| vs.iter().any(|v| v.eq_ignore_ascii_case("DATE")))
|
||||
.unwrap_or(false);
|
||||
Self::parse_ical_datetime(&value, is_date).ok()
|
||||
}
|
||||
None => None,
|
||||
};
|
||||
|
||||
let now = Utc::now();
|
||||
|
||||
@@ -627,18 +637,28 @@ impl CalendarEvent {
|
||||
));
|
||||
}
|
||||
|
||||
// Extract and update properties from iCalendar data
|
||||
if let Some(summary) = Self::extract_ical_property(&ical_data, "SUMMARY") {
|
||||
// Parse the body ONCE and update every property from the parsed
|
||||
// component (same 8-parses→1 collapse as `from_ical`). An
|
||||
// unparseable body behaves exactly like the old per-property
|
||||
// lookups all returning `None`: optional fields clear, required
|
||||
// fields keep their previous values.
|
||||
let event = Self::parse_first_vevent(&ical_data);
|
||||
|
||||
if let Some(summary) = event.as_ref().and_then(|e| Self::prop_value(e, "SUMMARY")) {
|
||||
self.summary = summary;
|
||||
}
|
||||
|
||||
self.description = Self::extract_ical_property(&ical_data, "DESCRIPTION");
|
||||
self.location = Self::extract_ical_property(&ical_data, "LOCATION");
|
||||
self.description = event
|
||||
.as_ref()
|
||||
.and_then(|e| Self::prop_value(e, "DESCRIPTION"));
|
||||
self.location = event.as_ref().and_then(|e| Self::prop_value(e, "LOCATION"));
|
||||
|
||||
// Extract DTSTART with parameters — needed for the all-day
|
||||
// detection below AND for the DTSTART/DTEND datetime parsers
|
||||
// (they need to know whether the value is a date or a datetime).
|
||||
let dtstart_pair = Self::extract_ical_property_with_params(&ical_data, "DTSTART");
|
||||
let dtstart_pair = event
|
||||
.as_ref()
|
||||
.and_then(|e| Self::prop_with_params(e, "DTSTART"));
|
||||
let all_day = dtstart_pair
|
||||
.as_ref()
|
||||
.and_then(|(_v, params)| params.get("VALUE"))
|
||||
@@ -652,15 +672,17 @@ impl CalendarEvent {
|
||||
self.start_time = start_time;
|
||||
}
|
||||
|
||||
if let Some((value, _params)) = Self::extract_ical_property_with_params(&ical_data, "DTEND")
|
||||
if let Some((value, _params)) = event
|
||||
.as_ref()
|
||||
.and_then(|e| Self::prop_with_params(e, "DTEND"))
|
||||
&& let Ok(end_time) = Self::parse_ical_datetime(&value, all_day)
|
||||
{
|
||||
self.end_time = end_time;
|
||||
}
|
||||
|
||||
self.rrule = Self::extract_ical_property(&ical_data, "RRULE");
|
||||
self.rrule = event.as_ref().and_then(|e| Self::prop_value(e, "RRULE"));
|
||||
|
||||
if let Some(uid) = Self::extract_ical_property(&ical_data, "UID") {
|
||||
if let Some(uid) = event.as_ref().and_then(|e| Self::prop_value(e, "UID")) {
|
||||
self.ical_uid = uid;
|
||||
}
|
||||
|
||||
@@ -756,43 +778,74 @@ impl CalendarEvent {
|
||||
* @param property_name The name of the property to extract
|
||||
* @return Option containing the property value if found
|
||||
*/
|
||||
#[cfg(test)]
|
||||
fn extract_ical_property(ical_data: &str, property_name: &str) -> Option<String> {
|
||||
Self::extract_ical_property_with_params(ical_data, property_name).map(|(v, _p)| v)
|
||||
Self::prop_value(&Self::parse_first_vevent(ical_data)?, property_name)
|
||||
}
|
||||
|
||||
/// Extract a property's value AND parameter map. Same lookup rules
|
||||
/// as `extract_ical_property`; the second element is a map keyed by
|
||||
/// parameter name (`"VALUE"`, `"TZID"`, `"CN"`, …) whose value is
|
||||
/// the list of parameter values (parameters can be multi-valued —
|
||||
/// `MEMBER="mailto:a@x","mailto:b@x"` — hence the `Vec<String>`
|
||||
/// per key).
|
||||
///
|
||||
/// Callers that only need the value should use `extract_ical_property`;
|
||||
/// this variant is for DTSTART / DTEND / RECURRENCE-ID which need
|
||||
/// `VALUE=DATE` detection to distinguish all-day from timed events.
|
||||
/// Test-only sibling of [`Self::prop_with_params`] that parses the
|
||||
/// raw body first. Production callers (`from_ical`,
|
||||
/// `update_ical_data`) parse ONCE and use the by-reference helpers.
|
||||
#[cfg(test)]
|
||||
fn extract_ical_property_with_params(
|
||||
ical_data: &str,
|
||||
property_name: &str,
|
||||
) -> Option<(String, std::collections::HashMap<String, Vec<String>>)> {
|
||||
let event = Self::parse_first_vevent(ical_data)?;
|
||||
Self::prop_with_params(&Self::parse_first_vevent(ical_data)?, property_name)
|
||||
}
|
||||
|
||||
/// Read a property's trimmed value from an already-parsed VEVENT.
|
||||
///
|
||||
/// Value-only lookups skip the parameter-map build entirely; use
|
||||
/// [`Self::prop_with_params`] for DTSTART / DTEND / RECURRENCE-ID
|
||||
/// which need `VALUE=DATE` detection.
|
||||
///
|
||||
/// Returns `None` when the property is missing or its value is
|
||||
/// empty after trimming — the same rules the old per-property
|
||||
/// full-parse extractors applied.
|
||||
fn prop_value(
|
||||
event: &ical::parser::ical::component::IcalEvent,
|
||||
property_name: &str,
|
||||
) -> Option<String> {
|
||||
let prop = event
|
||||
.properties
|
||||
.into_iter()
|
||||
.iter()
|
||||
.find(|p| p.name.eq_ignore_ascii_case(property_name))?;
|
||||
let value = prop.value?;
|
||||
if value.trim().is_empty() {
|
||||
let trimmed = prop.value.as_deref()?.trim();
|
||||
if trimmed.is_empty() {
|
||||
return None;
|
||||
}
|
||||
Some(trimmed.to_string())
|
||||
}
|
||||
|
||||
/// Read a property's trimmed value AND parameter map from an
|
||||
/// already-parsed VEVENT. The map is keyed by parameter name
|
||||
/// (`"VALUE"`, `"TZID"`, `"CN"`, …) whose value is the list of
|
||||
/// parameter values (parameters can be multi-valued —
|
||||
/// `MEMBER="mailto:a@x","mailto:b@x"` — hence the `Vec<String>`
|
||||
/// per key).
|
||||
fn prop_with_params(
|
||||
event: &ical::parser::ical::component::IcalEvent,
|
||||
property_name: &str,
|
||||
) -> Option<(String, std::collections::HashMap<String, Vec<String>>)> {
|
||||
let prop = event
|
||||
.properties
|
||||
.iter()
|
||||
.find(|p| p.name.eq_ignore_ascii_case(property_name))?;
|
||||
let trimmed = prop.value.as_deref()?.trim();
|
||||
if trimmed.is_empty() {
|
||||
return None;
|
||||
}
|
||||
let mut params: std::collections::HashMap<String, Vec<String>> =
|
||||
std::collections::HashMap::new();
|
||||
if let Some(param_list) = prop.params {
|
||||
if let Some(param_list) = &prop.params {
|
||||
for (name, values) in param_list {
|
||||
// RFC 5545 property parameter names are ASCII case-insensitive.
|
||||
// Normalise to UPPER so callers key on a canonical form.
|
||||
params.insert(name.to_ascii_uppercase(), values);
|
||||
params.insert(name.to_ascii_uppercase(), values.clone());
|
||||
}
|
||||
}
|
||||
Some((value.trim().to_string(), params))
|
||||
Some((trimmed.to_string(), params))
|
||||
}
|
||||
|
||||
/// Parse a VCALENDAR body containing one or more VEVENT components
|
||||
@@ -847,6 +900,18 @@ impl CalendarEvent {
|
||||
let mut in_event = false;
|
||||
let mut current = String::new();
|
||||
|
||||
// Allocation-free case-insensitive prefix test. `to_ascii_uppercase`
|
||||
// maps ASCII bytes in place and leaves multi-byte chars untouched,
|
||||
// so "first N bytes uppercased equal TAG" ⇔ "first N bytes
|
||||
// ASCII-case-insensitively equal TAG"; `get(..N)` returning `None`
|
||||
// (char straddling the boundary) implies the prefix can't be the
|
||||
// all-ASCII tag. The old per-line `to_ascii_uppercase()` allocated
|
||||
// a String for every line of every uploaded body.
|
||||
fn starts_with_ci(line: &str, tag: &str) -> bool {
|
||||
line.get(..tag.len())
|
||||
.is_some_and(|p| p.eq_ignore_ascii_case(tag))
|
||||
}
|
||||
|
||||
for raw_line in ical_data.split('\n') {
|
||||
let line = raw_line.trim_end_matches('\r');
|
||||
// Match the tag ignoring case, allowing surrounding
|
||||
@@ -854,9 +919,9 @@ impl CalendarEvent {
|
||||
// continuations — the raw-line scan sees those but they
|
||||
// won't start with BEGIN/END so they slot through as
|
||||
// in-event content, which is correct).
|
||||
let upper = line.trim_start().to_ascii_uppercase();
|
||||
let tag_area = line.trim_start();
|
||||
|
||||
if upper.starts_with("BEGIN:VEVENT") {
|
||||
if starts_with_ci(tag_area, "BEGIN:VEVENT") {
|
||||
in_event = true;
|
||||
current.clear();
|
||||
}
|
||||
@@ -866,7 +931,7 @@ impl CalendarEvent {
|
||||
current.push_str("\r\n");
|
||||
}
|
||||
|
||||
if in_event && upper.starts_with("END:VEVENT") {
|
||||
if in_event && starts_with_ci(tag_area, "END:VEVENT") {
|
||||
blocks.push(std::mem::take(&mut current));
|
||||
in_event = false;
|
||||
}
|
||||
|
||||
+62
-11
@@ -1,7 +1,7 @@
|
||||
use uuid::Uuid;
|
||||
|
||||
use crate::domain::services::path_service::{
|
||||
StoragePath, normalize_storage_name, validate_storage_name,
|
||||
StoragePath, normalize_storage_name_owned, validate_storage_name,
|
||||
};
|
||||
|
||||
// Re-export entity errors from the centralized module
|
||||
@@ -122,7 +122,7 @@ impl File {
|
||||
mime_type: String,
|
||||
folder_id: Option<String>,
|
||||
) -> FileResult<Self> {
|
||||
let name = normalize_storage_name(&name);
|
||||
let name = normalize_storage_name_owned(name);
|
||||
if let Err(reason) = validate_storage_name(&name) {
|
||||
return Err(FileError::InvalidFileName(format!("{name}: {reason}")));
|
||||
}
|
||||
@@ -133,7 +133,7 @@ impl File {
|
||||
.as_secs();
|
||||
|
||||
// Store the path string for serialization compatibility
|
||||
let path_string = storage_path.to_string();
|
||||
let path_string = storage_path.to_path_string();
|
||||
|
||||
Ok(Self {
|
||||
id,
|
||||
@@ -160,13 +160,13 @@ impl File {
|
||||
created_at: u64,
|
||||
modified_at: u64,
|
||||
) -> FileResult<Self> {
|
||||
let name = normalize_storage_name(&name);
|
||||
let name = normalize_storage_name_owned(name);
|
||||
if let Err(reason) = validate_storage_name(&name) {
|
||||
return Err(FileError::InvalidFileName(format!("{name}: {reason}")));
|
||||
}
|
||||
|
||||
// Store the path string for serialization compatibility
|
||||
let path_string = storage_path.to_string();
|
||||
let path_string = storage_path.to_path_string();
|
||||
|
||||
Ok(Self {
|
||||
id,
|
||||
@@ -252,13 +252,64 @@ impl File {
|
||||
created_by: Option<Uuid>,
|
||||
updated_by: Option<Uuid>,
|
||||
) -> FileResult<Self> {
|
||||
let name = normalize_storage_name(&name);
|
||||
let name = normalize_storage_name_owned(name);
|
||||
if let Err(reason) = validate_storage_name(&name) {
|
||||
return Err(FileError::InvalidFileName(format!("{name}: {reason}")));
|
||||
}
|
||||
|
||||
// Store the path string for serialization compatibility
|
||||
let path_string = storage_path.to_string();
|
||||
let path_string = storage_path.to_path_string();
|
||||
|
||||
Ok(Self {
|
||||
id,
|
||||
name,
|
||||
storage_path,
|
||||
path_string,
|
||||
size,
|
||||
mime_type,
|
||||
folder_id,
|
||||
created_at,
|
||||
modified_at,
|
||||
blob_hash,
|
||||
created_by,
|
||||
updated_by,
|
||||
})
|
||||
}
|
||||
|
||||
/// PG-row constructor: the per-listing-row hot path.
|
||||
///
|
||||
/// Builds `storage_path` **and** `path_string` in one pass from the
|
||||
/// materialized folder path via
|
||||
/// [`StoragePath::from_folder_and_name`], instead of the old chain
|
||||
/// (`format!` temp → `from_string` split → `Display` re-join) that
|
||||
/// allocated the full path three times per row. The owned `name` is
|
||||
/// NFC-normalized without the always-copy of the borrowing variant
|
||||
/// (DB rows are NFC by invariant, so this is a zero-alloc check).
|
||||
///
|
||||
/// The path is built from the raw incoming name and the name field is
|
||||
/// normalized afterwards — the exact observable sequence of the old
|
||||
/// `make_file_path` + constructor pair, byte-identical for every
|
||||
/// input (for DB rows the two names coincide: stored names are NFC).
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub fn from_materialized_row(
|
||||
id: String,
|
||||
name: String,
|
||||
folder_path: Option<&str>,
|
||||
size: u64,
|
||||
mime_type: String,
|
||||
folder_id: Option<String>,
|
||||
created_at: u64,
|
||||
modified_at: u64,
|
||||
blob_hash: String,
|
||||
created_by: Option<Uuid>,
|
||||
updated_by: Option<Uuid>,
|
||||
) -> FileResult<Self> {
|
||||
let (storage_path, path_string) = StoragePath::from_folder_and_name(folder_path, &name);
|
||||
|
||||
let name = normalize_storage_name_owned(name);
|
||||
if let Err(reason) = validate_storage_name(&name) {
|
||||
return Err(FileError::InvalidFileName(format!("{name}: {reason}")));
|
||||
}
|
||||
|
||||
Ok(Self {
|
||||
id,
|
||||
@@ -442,7 +493,7 @@ impl File {
|
||||
// Create directly without validation to avoid errors in DTO
|
||||
// conversions. Still NFC-normalize so even DTO-reconstructed
|
||||
// entities maintain the storage invariant.
|
||||
let name = normalize_storage_name(&name);
|
||||
let name = normalize_storage_name_owned(name);
|
||||
|
||||
Self {
|
||||
id,
|
||||
@@ -466,7 +517,7 @@ impl File {
|
||||
|
||||
/// Creates a new version of the file with updated name
|
||||
pub fn with_name(mut self, new_name: String) -> FileResult<Self> {
|
||||
let new_name = normalize_storage_name(&new_name);
|
||||
let new_name = normalize_storage_name_owned(new_name);
|
||||
if let Err(reason) = validate_storage_name(&new_name) {
|
||||
return Err(FileError::InvalidFileName(format!("{new_name}: {reason}")));
|
||||
}
|
||||
@@ -485,7 +536,7 @@ impl File {
|
||||
// Consume `self` and mutate in place — only the path, name and mtime
|
||||
// change; id / mime_type / folder_id / blob_hash are carried over
|
||||
// without the per-field clone the old `&self` builder paid.
|
||||
self.path_string = new_storage_path.to_string();
|
||||
self.path_string = new_storage_path.to_path_string();
|
||||
self.storage_path = new_storage_path;
|
||||
self.name = new_name;
|
||||
self.modified_at = now;
|
||||
@@ -510,7 +561,7 @@ impl File {
|
||||
.as_secs();
|
||||
|
||||
// Consume `self`: only the path, folder_id and mtime change.
|
||||
self.path_string = new_storage_path.to_string();
|
||||
self.path_string = new_storage_path.to_path_string();
|
||||
self.storage_path = new_storage_path;
|
||||
self.folder_id = folder_id;
|
||||
self.modified_at = now;
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
use uuid::Uuid;
|
||||
|
||||
use crate::domain::services::path_service::{
|
||||
StoragePath, normalize_storage_name, validate_storage_name,
|
||||
StoragePath, normalize_storage_name_owned, validate_storage_name,
|
||||
};
|
||||
|
||||
// Re-export entity errors from the centralized module
|
||||
@@ -120,7 +120,7 @@ impl Folder {
|
||||
storage_path: StoragePath,
|
||||
parent_id: Option<String>,
|
||||
) -> FolderResult<Self> {
|
||||
let name = normalize_storage_name(&name);
|
||||
let name = normalize_storage_name_owned(name);
|
||||
if let Err(reason) = validate_storage_name(&name) {
|
||||
return Err(FolderError::InvalidFolderName(format!("{name}: {reason}")));
|
||||
}
|
||||
@@ -130,7 +130,7 @@ impl Folder {
|
||||
.unwrap_or_default()
|
||||
.as_secs();
|
||||
|
||||
let path_string = storage_path.to_string();
|
||||
let path_string = storage_path.to_path_string();
|
||||
|
||||
Ok(Self {
|
||||
id,
|
||||
@@ -221,12 +221,56 @@ impl Folder {
|
||||
created_by: Option<Uuid>,
|
||||
updated_by: Option<Uuid>,
|
||||
) -> FolderResult<Self> {
|
||||
let name = normalize_storage_name(&name);
|
||||
let name = normalize_storage_name_owned(name);
|
||||
if let Err(reason) = validate_storage_name(&name) {
|
||||
return Err(FolderError::InvalidFolderName(format!("{name}: {reason}")));
|
||||
}
|
||||
|
||||
let path_string = storage_path.to_string();
|
||||
let path_string = storage_path.to_path_string();
|
||||
|
||||
Ok(Self {
|
||||
id,
|
||||
name,
|
||||
storage_path,
|
||||
path_string,
|
||||
parent_id,
|
||||
drive_id,
|
||||
created_at,
|
||||
modified_at,
|
||||
tree_modified_at,
|
||||
created_by,
|
||||
updated_by,
|
||||
})
|
||||
}
|
||||
|
||||
/// PG-row constructor: the per-listing-row hot path.
|
||||
///
|
||||
/// Takes the materialized `storage.folders.path` column by value and
|
||||
/// splits it once via [`StoragePath::from_joined`] — when the stored
|
||||
/// path is already canonical (every row the repository writes), the
|
||||
/// input `String` is reused as `path_string` with zero copies,
|
||||
/// replacing the old `from_string` split + `Display` re-join pair.
|
||||
/// The owned `name` is NFC-normalized without the always-copy of the
|
||||
/// borrowing variant (DB rows are NFC by invariant).
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub fn from_materialized_row(
|
||||
id: String,
|
||||
name: String,
|
||||
path: String,
|
||||
parent_id: Option<String>,
|
||||
drive_id: Uuid,
|
||||
created_at: u64,
|
||||
modified_at: u64,
|
||||
tree_modified_at: u64,
|
||||
created_by: Option<Uuid>,
|
||||
updated_by: Option<Uuid>,
|
||||
) -> FolderResult<Self> {
|
||||
let name = normalize_storage_name_owned(name);
|
||||
if let Err(reason) = validate_storage_name(&name) {
|
||||
return Err(FolderError::InvalidFolderName(format!("{name}: {reason}")));
|
||||
}
|
||||
|
||||
let (storage_path, path_string) = StoragePath::from_joined(path);
|
||||
|
||||
Ok(Self {
|
||||
id,
|
||||
@@ -411,7 +455,7 @@ impl Folder {
|
||||
// round-trips lose the real rollup signal, so callers that
|
||||
// need a freshly-rolled-up etag must reload from the
|
||||
// repository.
|
||||
let name = normalize_storage_name(&name);
|
||||
let name = normalize_storage_name_owned(name);
|
||||
Self {
|
||||
id,
|
||||
name,
|
||||
@@ -437,7 +481,7 @@ impl Folder {
|
||||
|
||||
/// Creates a new version of the folder with updated name
|
||||
pub fn with_name(&self, new_name: String) -> FolderResult<Self> {
|
||||
let new_name = normalize_storage_name(&new_name);
|
||||
let new_name = normalize_storage_name_owned(new_name);
|
||||
if let Err(reason) = validate_storage_name(&new_name) {
|
||||
return Err(FolderError::InvalidFolderName(format!(
|
||||
"{new_name}: {reason}"
|
||||
@@ -452,7 +496,7 @@ impl Folder {
|
||||
};
|
||||
|
||||
// Update string representation
|
||||
let new_path_string = new_storage_path.to_string();
|
||||
let new_path_string = new_storage_path.to_path_string();
|
||||
|
||||
let now = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
@@ -492,7 +536,7 @@ impl Folder {
|
||||
};
|
||||
|
||||
// Update string representation
|
||||
let new_path_string = new_storage_path.to_string();
|
||||
let new_path_string = new_storage_path.to_path_string();
|
||||
|
||||
let now = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
|
||||
@@ -24,6 +24,14 @@ pub trait AddressBookRepository: Send + Sync + 'static {
|
||||
address_book: AddressBook,
|
||||
) -> AddressBookRepositoryResult<AddressBook>;
|
||||
async fn delete_address_book(&self, id: &Uuid) -> AddressBookRepositoryResult<()>;
|
||||
/// Batch sibling of `get_address_book_by_id`: one `= ANY($1)`
|
||||
/// round-trip for a page of grant-derived ids. Missing ids drop
|
||||
/// out; ordering is not guaranteed.
|
||||
async fn get_address_books_by_ids(
|
||||
&self,
|
||||
ids: &[Uuid],
|
||||
) -> AddressBookRepositoryResult<Vec<AddressBook>>;
|
||||
|
||||
async fn get_address_book_by_id(
|
||||
&self,
|
||||
id: &Uuid,
|
||||
|
||||
@@ -25,6 +25,12 @@ pub trait CalendarRepository: Send + Sync + 'static {
|
||||
/// Finds a calendar by its ID
|
||||
async fn find_calendar_by_id(&self, id: &Uuid) -> CalendarRepositoryResult<Calendar>;
|
||||
|
||||
/// Batch sibling of [`Self::find_calendar_by_id`]: one `= ANY($1)`
|
||||
/// round-trip for a page of grant-derived ids. Missing ids drop out
|
||||
/// (no per-id NotFound), matching the listing carve-out for
|
||||
/// deleted/trashed races. Ordering is not guaranteed.
|
||||
async fn find_calendars_by_ids(&self, ids: &[Uuid]) -> CalendarRepositoryResult<Vec<Calendar>>;
|
||||
|
||||
/// Lists all calendars owned by a specific user. Post-Round-3 the
|
||||
/// service layer prefers `authz.list_incoming_grants` (surfaces
|
||||
/// owned + shared in one union), but this direct lookup remains
|
||||
|
||||
@@ -13,6 +13,11 @@ pub trait PlaylistRepository: Send + Sync + 'static {
|
||||
|
||||
async fn find_playlist_by_id(&self, id: &Uuid) -> PlaylistRepositoryResult<Playlist>;
|
||||
|
||||
/// Batch sibling of [`Self::find_playlist_by_id`]: one `= ANY($1)`
|
||||
/// round-trip for a page of grant-derived ids. Missing ids drop
|
||||
/// out; ordering is not guaranteed.
|
||||
async fn find_playlists_by_ids(&self, ids: &[Uuid]) -> PlaylistRepositoryResult<Vec<Playlist>>;
|
||||
|
||||
async fn list_playlists_by_owner(
|
||||
&self,
|
||||
owner_id: Uuid,
|
||||
|
||||
@@ -40,6 +40,22 @@ pub fn normalize_storage_name(name: &str) -> String {
|
||||
name.nfc().collect()
|
||||
}
|
||||
|
||||
/// Owned-input sibling of [`normalize_storage_name`].
|
||||
///
|
||||
/// The borrowing variant must always allocate a fresh `String` even when
|
||||
/// the input is already NFC — which is every name loaded back from
|
||||
/// PostgreSQL (DB invariant) and every ASCII name. Callers that own the
|
||||
/// `String` (entity constructors receive `name: String` by value) were
|
||||
/// paying that copy only to drop the original immediately. This variant
|
||||
/// returns the input unchanged on the fast path: zero allocations per
|
||||
/// row on every listing (PROPFIND, photos timeline, search).
|
||||
pub fn normalize_storage_name_owned(name: String) -> String {
|
||||
if is_nfc_quick(name.chars()) == IsNormalized::Yes {
|
||||
return name;
|
||||
}
|
||||
name.nfc().collect()
|
||||
}
|
||||
|
||||
/// Validates a single file or folder name component.
|
||||
///
|
||||
/// Returns `Err` with a human-readable reason if the name is rejected.
|
||||
@@ -102,6 +118,88 @@ impl StoragePath {
|
||||
Self { segments }
|
||||
}
|
||||
|
||||
/// One-pass builder for PG listing rows: materialized folder path +
|
||||
/// file name → `(StoragePath, path_string)`.
|
||||
///
|
||||
/// Replaces the old per-row chain
|
||||
/// `StoragePath::from_string(&format!("{fp}/{name}"))` +
|
||||
/// `storage_path.to_string()`, which allocated a joined temporary,
|
||||
/// split it back into per-segment `String`s, and then re-joined those
|
||||
/// segments (via `join` + `write!`) into the `path_string` the DTOs
|
||||
/// actually serve. Here both representations are built in a single
|
||||
/// pass with exactly one `String` for the joined form and no
|
||||
/// intermediate temporaries.
|
||||
///
|
||||
/// Byte-equivalence with the old chain holds because concatenating
|
||||
/// with a `/` separator distributes over `split('/')`:
|
||||
/// `(fp + "/" + name).split('/') == fp.split('/') ⧺ name.split('/')`,
|
||||
/// and the joined form is exactly `Display`'s `/`-prefixed rendering
|
||||
/// of the surviving segments (root renders as `"/"`).
|
||||
pub fn from_folder_and_name(folder_path: Option<&str>, file_name: &str) -> (Self, String) {
|
||||
let fp = folder_path.unwrap_or("");
|
||||
// Upper bounds: every byte of both inputs survives at most once,
|
||||
// plus one leading '/' per segment (≤ segment count) — sizing to
|
||||
// input length + 2 covers the worst case without a second scan.
|
||||
let mut joined = String::with_capacity(fp.len() + file_name.len() + 2);
|
||||
let mut segments: Vec<String> =
|
||||
Vec::with_capacity(fp.bytes().filter(|&b| b == b'/').count() + 2);
|
||||
for seg in fp
|
||||
.split('/')
|
||||
.chain(file_name.split('/'))
|
||||
.filter(|s| Self::is_safe_segment(s))
|
||||
{
|
||||
joined.push('/');
|
||||
joined.push_str(seg);
|
||||
segments.push(seg.to_string());
|
||||
}
|
||||
if segments.is_empty() {
|
||||
joined.push('/');
|
||||
}
|
||||
(Self { segments }, joined)
|
||||
}
|
||||
|
||||
/// One-pass splitter for a pre-joined materialized path (the
|
||||
/// `storage.folders.path` column) → `(StoragePath, path_string)`.
|
||||
///
|
||||
/// When the input is already in canonical joined form (leading `/`,
|
||||
/// no empty/`.`/`..` segments, no trailing `/`) — which is every row
|
||||
/// the repository writes — the input `String` is reused as the
|
||||
/// `path_string` with zero copies. Non-canonical inputs fall back to
|
||||
/// the filtering rebuild and produce exactly what
|
||||
/// `from_string(&path).to_string()` used to.
|
||||
pub fn from_joined(path: String) -> (Self, String) {
|
||||
if Self::is_canonical_joined(&path) {
|
||||
let segments: Vec<String> = if path.len() == 1 {
|
||||
Vec::new()
|
||||
} else {
|
||||
path[1..].split('/').map(str::to_string).collect()
|
||||
};
|
||||
return (Self { segments }, path);
|
||||
}
|
||||
// Fallback: identical to the old from_string + to_string pair.
|
||||
let segments: Vec<String> = path
|
||||
.split('/')
|
||||
.filter(|s| Self::is_safe_segment(s))
|
||||
.map(str::to_string)
|
||||
.collect();
|
||||
let sp = Self { segments };
|
||||
let joined = sp.to_path_string();
|
||||
(sp, joined)
|
||||
}
|
||||
|
||||
/// `true` when `path` is exactly `Display`'s canonical rendering of
|
||||
/// its own segments: `"/"` alone, or `/seg(/seg)*` where every
|
||||
/// segment is safe. One scan, no allocations.
|
||||
fn is_canonical_joined(path: &str) -> bool {
|
||||
if path == "/" {
|
||||
return true;
|
||||
}
|
||||
if !path.starts_with('/') || path.ends_with('/') {
|
||||
return false;
|
||||
}
|
||||
path[1..].split('/').all(Self::is_safe_segment)
|
||||
}
|
||||
|
||||
/// Creates a path from a PathBuf
|
||||
pub fn from(path_buf: PathBuf) -> Self {
|
||||
let segments = path_buf
|
||||
@@ -152,14 +250,40 @@ impl StoragePath {
|
||||
impl std::fmt::Display for StoragePath {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
if self.segments.is_empty() {
|
||||
write!(f, "/")
|
||||
} else {
|
||||
write!(f, "/{}", self.segments.join("/"))
|
||||
return f.write_str("/");
|
||||
}
|
||||
// Write segments directly — the old `self.segments.join("/")`
|
||||
// allocated a full joined temporary inside every `format!`/
|
||||
// `to_string` of a path.
|
||||
for seg in &self.segments {
|
||||
f.write_str("/")?;
|
||||
f.write_str(seg)?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
impl StoragePath {
|
||||
/// The canonical joined form (`Display`'s output) in exactly one
|
||||
/// pre-sized allocation.
|
||||
///
|
||||
/// `to_string()` routes through `Display` into an unsized `String`
|
||||
/// that grows geometrically (multiple reallocs + copies for typical
|
||||
/// path lengths). Entity constructors call this once per row on
|
||||
/// every listing, so the sized single-alloc variant is the default
|
||||
/// there.
|
||||
pub fn to_path_string(&self) -> String {
|
||||
if self.segments.is_empty() {
|
||||
return "/".to_string();
|
||||
}
|
||||
let mut s = String::with_capacity(self.segments.iter().map(|seg| seg.len() + 1).sum());
|
||||
for seg in &self.segments {
|
||||
s.push('/');
|
||||
s.push_str(seg);
|
||||
}
|
||||
s
|
||||
}
|
||||
|
||||
/// Returns the path representation as a string
|
||||
pub fn as_str(&self) -> &str {
|
||||
// Note: The implementation should really store the string,
|
||||
|
||||
Reference in New Issue
Block a user