From ec8ddebc30f52685fd14e535a6a6dcbda569203d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 13:29:05 +0000 Subject: [PATCH] perf: drop intermediate allocs in WebDAV href encoding; fold group-list COUNT into one query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit webdav encode_uri_path runs on every PROPFIND href and did .map(...).collect::>().join("/"), allocating a String per segment plus a joined Vec. Write each utf8_percent_encode Display adapter straight into a single preallocated String. Behavior is identical (split on '/', encode each segment, join with '/'), including leading/trailing-slash edge cases. subject_group list / list_with_counts each issued a second SELECT COUNT(*) round-trip for the total. Fold it into the page query via COUNT(*) OVER() — the pattern folder_db_repository already uses — halving the round-trips. total_count is read from the first row and is 0 on an empty page, matching folder_db_repository's documented convention. https://claude.ai/code/session_01UtfkS3nZF1vrF5jNAps6wV --- .../pg/subject_group_pg_repository.rs | 109 +++++++----------- src/interfaces/api/handlers/webdav_handler.rs | 21 +++- 2 files changed, 58 insertions(+), 72 deletions(-) diff --git a/src/infrastructure/repositories/pg/subject_group_pg_repository.rs b/src/infrastructure/repositories/pg/subject_group_pg_repository.rs index 39bdc61c..7484a155 100644 --- a/src/infrastructure/repositories/pg/subject_group_pg_repository.rs +++ b/src/infrastructure/repositories/pg/subject_group_pg_repository.rs @@ -126,30 +126,26 @@ impl SubjectGroupRepository for SubjectGroupPgRepository { offset: u32, name_query: Option<&str>, ) -> Result<(Vec, u64), SubjectGroupRepositoryError> { - // Two queries: one for the page, one for the total count. The query - // is small and frequent; a window function would add complexity for - // no measurable win. - let (sql_page, sql_count, pattern) = match name_query { - Some(q) => { - let pat = like_escape(q); - ( - "SELECT id, name, description, is_virtual, created_at, updated_at - FROM auth.subject_groups - WHERE name ILIKE $1 - ORDER BY is_virtual DESC, name - LIMIT $2 OFFSET $3" - .to_string(), - "SELECT COUNT(*) FROM auth.subject_groups WHERE name ILIKE $1".to_string(), - Some(pat), - ) - } + // Single query: the page plus `COUNT(*) OVER()` for the total matching + // count, folding what used to be a separate COUNT round-trip into one. + let (sql_page, pattern) = match name_query { + Some(q) => ( + "SELECT id, name, description, is_virtual, created_at, updated_at, + COUNT(*) OVER() AS total_count + FROM auth.subject_groups + WHERE name ILIKE $1 + ORDER BY is_virtual DESC, name + LIMIT $2 OFFSET $3" + .to_string(), + Some(like_escape(q)), + ), None => ( - "SELECT id, name, description, is_virtual, created_at, updated_at + "SELECT id, name, description, is_virtual, created_at, updated_at, + COUNT(*) OVER() AS total_count FROM auth.subject_groups ORDER BY is_virtual DESC, name LIMIT $1 OFFSET $2" .to_string(), - "SELECT COUNT(*) FROM auth.subject_groups".to_string(), None, ), }; @@ -170,19 +166,9 @@ impl SubjectGroupRepository for SubjectGroupPgRepository { } .map_err(|e| Self::map_sqlx_err("list page", e))?; - let total: i64 = if let Some(ref p) = pattern { - sqlx::query_scalar(&sql_count) - .bind(p) - .fetch_one(self.pool.as_ref()) - .await - } else { - sqlx::query_scalar(&sql_count) - .fetch_one(self.pool.as_ref()) - .await - } - .map_err(|e| Self::map_sqlx_err("list count", e))?; - - Ok((rows.iter().map(Self::row_to_group).collect(), total as u64)) + // total_count is identical in every row; 0 when the page is empty. + let total = rows.first().map_or(0, |r| r.get::("total_count")) as u64; + Ok((rows.iter().map(Self::row_to_group).collect(), total)) } async fn list_with_counts( @@ -191,38 +177,35 @@ impl SubjectGroupRepository for SubjectGroupPgRepository { offset: u32, name_query: Option<&str>, ) -> Result<(Vec<(SubjectGroup, i64)>, u64), SubjectGroupRepositoryError> { - // Single SQL: groups + COUNT of direct members per group, via LEFT JOIN - // on `auth.subject_group_members`. No N+1; one round-trip for the - // page, a second for the unfiltered total (matches `list`). - let (sql_page, sql_count, pattern) = match name_query { - Some(q) => { - let pat = like_escape(q); - ( - "SELECT g.id, g.name, g.description, g.is_virtual, - g.created_at, g.updated_at, - COUNT(m.group_id) AS member_count - FROM auth.subject_groups g - LEFT JOIN auth.subject_group_members m ON m.group_id = g.id - WHERE g.name ILIKE $1 - GROUP BY g.id - ORDER BY g.is_virtual DESC, g.name - LIMIT $2 OFFSET $3" - .to_string(), - "SELECT COUNT(*) FROM auth.subject_groups WHERE name ILIKE $1".to_string(), - Some(pat), - ) - } + // Single SQL: groups + per-group member COUNT via LEFT JOIN on + // `auth.subject_group_members`, plus `COUNT(*) OVER()` for the total + // group count. No N+1 and no separate COUNT round-trip — one query. + let (sql_page, pattern) = match name_query { + Some(q) => ( + "SELECT g.id, g.name, g.description, g.is_virtual, + g.created_at, g.updated_at, + COUNT(m.group_id) AS member_count, + COUNT(*) OVER() AS total_count + FROM auth.subject_groups g + LEFT JOIN auth.subject_group_members m ON m.group_id = g.id + WHERE g.name ILIKE $1 + GROUP BY g.id + ORDER BY g.is_virtual DESC, g.name + LIMIT $2 OFFSET $3" + .to_string(), + Some(like_escape(q)), + ), None => ( "SELECT g.id, g.name, g.description, g.is_virtual, g.created_at, g.updated_at, - COUNT(m.group_id) AS member_count + COUNT(m.group_id) AS member_count, + COUNT(*) OVER() AS total_count FROM auth.subject_groups g LEFT JOIN auth.subject_group_members m ON m.group_id = g.id GROUP BY g.id ORDER BY g.is_virtual DESC, g.name LIMIT $1 OFFSET $2" .to_string(), - "SELECT COUNT(*) FROM auth.subject_groups".to_string(), None, ), }; @@ -243,24 +226,14 @@ impl SubjectGroupRepository for SubjectGroupPgRepository { } .map_err(|e| Self::map_sqlx_err("list_with_counts page", e))?; - let total: i64 = if let Some(ref p) = pattern { - sqlx::query_scalar(&sql_count) - .bind(p) - .fetch_one(self.pool.as_ref()) - .await - } else { - sqlx::query_scalar(&sql_count) - .fetch_one(self.pool.as_ref()) - .await - } - .map_err(|e| Self::map_sqlx_err("list_with_counts total", e))?; - + // total_count is identical in every row; 0 when the page is empty. + let total = rows.first().map_or(0, |r| r.get::("total_count")) as u64; let items = rows .iter() .map(|r| (Self::row_to_group(r), r.get::("member_count"))) .collect(); - Ok((items, total as u64)) + Ok((items, total)) } async fn count_members(&self, id: Uuid) -> Result { diff --git a/src/interfaces/api/handlers/webdav_handler.rs b/src/interfaces/api/handlers/webdav_handler.rs index ef339dff..779efe7b 100644 --- a/src/interfaces/api/handlers/webdav_handler.rs +++ b/src/interfaces/api/handlers/webdav_handler.rs @@ -63,10 +63,23 @@ fn encode_path_segment(segment: &str) -> String { /// Percent-encode a full slash-separated path, encoding each segment individually. pub(crate) fn encode_uri_path(path: &str) -> String { - path.split('/') - .map(encode_path_segment) - .collect::>() - .join("/") + use std::fmt::Write as _; + // `utf8_percent_encode` returns a `Display` adapter, so write each encoded + // segment straight into `out` — avoids a String per segment and the joined + // Vec the previous `.map(...).collect::>().join("/")` allocated on + // every PROPFIND href. + let mut out = String::with_capacity(path.len() + 8); + for (i, segment) in path.split('/').enumerate() { + if i > 0 { + out.push('/'); + } + let _ = write!( + out, + "{}", + utf8_percent_encode(segment, PATH_SEGMENT_ENCODE_SET) + ); + } + out } /// Build the `` value for a non-collection (file) resource.