perf: drop intermediate allocs in WebDAV href encoding; fold group-list COUNT into one query
webdav encode_uri_path runs on every PROPFIND href and did
.map(...).collect::<Vec<_>>().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
This commit is contained in:
@@ -126,30 +126,26 @@ impl SubjectGroupRepository for SubjectGroupPgRepository {
|
|||||||
offset: u32,
|
offset: u32,
|
||||||
name_query: Option<&str>,
|
name_query: Option<&str>,
|
||||||
) -> Result<(Vec<SubjectGroup>, u64), SubjectGroupRepositoryError> {
|
) -> Result<(Vec<SubjectGroup>, u64), SubjectGroupRepositoryError> {
|
||||||
// Two queries: one for the page, one for the total count. The query
|
// Single query: the page plus `COUNT(*) OVER()` for the total matching
|
||||||
// is small and frequent; a window function would add complexity for
|
// count, folding what used to be a separate COUNT round-trip into one.
|
||||||
// no measurable win.
|
let (sql_page, pattern) = match name_query {
|
||||||
let (sql_page, sql_count, pattern) = match name_query {
|
Some(q) => (
|
||||||
Some(q) => {
|
"SELECT id, name, description, is_virtual, created_at, updated_at,
|
||||||
let pat = like_escape(q);
|
COUNT(*) OVER() AS total_count
|
||||||
(
|
FROM auth.subject_groups
|
||||||
"SELECT id, name, description, is_virtual, created_at, updated_at
|
WHERE name ILIKE $1
|
||||||
FROM auth.subject_groups
|
ORDER BY is_virtual DESC, name
|
||||||
WHERE name ILIKE $1
|
LIMIT $2 OFFSET $3"
|
||||||
ORDER BY is_virtual DESC, name
|
.to_string(),
|
||||||
LIMIT $2 OFFSET $3"
|
Some(like_escape(q)),
|
||||||
.to_string(),
|
),
|
||||||
"SELECT COUNT(*) FROM auth.subject_groups WHERE name ILIKE $1".to_string(),
|
|
||||||
Some(pat),
|
|
||||||
)
|
|
||||||
}
|
|
||||||
None => (
|
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
|
FROM auth.subject_groups
|
||||||
ORDER BY is_virtual DESC, name
|
ORDER BY is_virtual DESC, name
|
||||||
LIMIT $1 OFFSET $2"
|
LIMIT $1 OFFSET $2"
|
||||||
.to_string(),
|
.to_string(),
|
||||||
"SELECT COUNT(*) FROM auth.subject_groups".to_string(),
|
|
||||||
None,
|
None,
|
||||||
),
|
),
|
||||||
};
|
};
|
||||||
@@ -170,19 +166,9 @@ impl SubjectGroupRepository for SubjectGroupPgRepository {
|
|||||||
}
|
}
|
||||||
.map_err(|e| Self::map_sqlx_err("list page", e))?;
|
.map_err(|e| Self::map_sqlx_err("list page", e))?;
|
||||||
|
|
||||||
let total: i64 = if let Some(ref p) = pattern {
|
// total_count is identical in every row; 0 when the page is empty.
|
||||||
sqlx::query_scalar(&sql_count)
|
let total = rows.first().map_or(0, |r| r.get::<i64, _>("total_count")) as u64;
|
||||||
.bind(p)
|
Ok((rows.iter().map(Self::row_to_group).collect(), total))
|
||||||
.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))
|
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn list_with_counts(
|
async fn list_with_counts(
|
||||||
@@ -191,38 +177,35 @@ impl SubjectGroupRepository for SubjectGroupPgRepository {
|
|||||||
offset: u32,
|
offset: u32,
|
||||||
name_query: Option<&str>,
|
name_query: Option<&str>,
|
||||||
) -> Result<(Vec<(SubjectGroup, i64)>, u64), SubjectGroupRepositoryError> {
|
) -> Result<(Vec<(SubjectGroup, i64)>, u64), SubjectGroupRepositoryError> {
|
||||||
// Single SQL: groups + COUNT of direct members per group, via LEFT JOIN
|
// Single SQL: groups + per-group member COUNT via LEFT JOIN on
|
||||||
// on `auth.subject_group_members`. No N+1; one round-trip for the
|
// `auth.subject_group_members`, plus `COUNT(*) OVER()` for the total
|
||||||
// page, a second for the unfiltered total (matches `list`).
|
// group count. No N+1 and no separate COUNT round-trip — one query.
|
||||||
let (sql_page, sql_count, pattern) = match name_query {
|
let (sql_page, pattern) = match name_query {
|
||||||
Some(q) => {
|
Some(q) => (
|
||||||
let pat = like_escape(q);
|
"SELECT g.id, g.name, g.description, g.is_virtual,
|
||||||
(
|
g.created_at, g.updated_at,
|
||||||
"SELECT g.id, g.name, g.description, g.is_virtual,
|
COUNT(m.group_id) AS member_count,
|
||||||
g.created_at, g.updated_at,
|
COUNT(*) OVER() AS total_count
|
||||||
COUNT(m.group_id) AS member_count
|
FROM auth.subject_groups g
|
||||||
FROM auth.subject_groups g
|
LEFT JOIN auth.subject_group_members m ON m.group_id = g.id
|
||||||
LEFT JOIN auth.subject_group_members m ON m.group_id = g.id
|
WHERE g.name ILIKE $1
|
||||||
WHERE g.name ILIKE $1
|
GROUP BY g.id
|
||||||
GROUP BY g.id
|
ORDER BY g.is_virtual DESC, g.name
|
||||||
ORDER BY g.is_virtual DESC, g.name
|
LIMIT $2 OFFSET $3"
|
||||||
LIMIT $2 OFFSET $3"
|
.to_string(),
|
||||||
.to_string(),
|
Some(like_escape(q)),
|
||||||
"SELECT COUNT(*) FROM auth.subject_groups WHERE name ILIKE $1".to_string(),
|
),
|
||||||
Some(pat),
|
|
||||||
)
|
|
||||||
}
|
|
||||||
None => (
|
None => (
|
||||||
"SELECT g.id, g.name, g.description, g.is_virtual,
|
"SELECT g.id, g.name, g.description, g.is_virtual,
|
||||||
g.created_at, g.updated_at,
|
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
|
FROM auth.subject_groups g
|
||||||
LEFT JOIN auth.subject_group_members m ON m.group_id = g.id
|
LEFT JOIN auth.subject_group_members m ON m.group_id = g.id
|
||||||
GROUP BY g.id
|
GROUP BY g.id
|
||||||
ORDER BY g.is_virtual DESC, g.name
|
ORDER BY g.is_virtual DESC, g.name
|
||||||
LIMIT $1 OFFSET $2"
|
LIMIT $1 OFFSET $2"
|
||||||
.to_string(),
|
.to_string(),
|
||||||
"SELECT COUNT(*) FROM auth.subject_groups".to_string(),
|
|
||||||
None,
|
None,
|
||||||
),
|
),
|
||||||
};
|
};
|
||||||
@@ -243,24 +226,14 @@ impl SubjectGroupRepository for SubjectGroupPgRepository {
|
|||||||
}
|
}
|
||||||
.map_err(|e| Self::map_sqlx_err("list_with_counts page", e))?;
|
.map_err(|e| Self::map_sqlx_err("list_with_counts page", e))?;
|
||||||
|
|
||||||
let total: i64 = if let Some(ref p) = pattern {
|
// total_count is identical in every row; 0 when the page is empty.
|
||||||
sqlx::query_scalar(&sql_count)
|
let total = rows.first().map_or(0, |r| r.get::<i64, _>("total_count")) as u64;
|
||||||
.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))?;
|
|
||||||
|
|
||||||
let items = rows
|
let items = rows
|
||||||
.iter()
|
.iter()
|
||||||
.map(|r| (Self::row_to_group(r), r.get::<i64, _>("member_count")))
|
.map(|r| (Self::row_to_group(r), r.get::<i64, _>("member_count")))
|
||||||
.collect();
|
.collect();
|
||||||
|
|
||||||
Ok((items, total as u64))
|
Ok((items, total))
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn count_members(&self, id: Uuid) -> Result<i64, SubjectGroupRepositoryError> {
|
async fn count_members(&self, id: Uuid) -> Result<i64, SubjectGroupRepositoryError> {
|
||||||
|
|||||||
@@ -63,10 +63,23 @@ fn encode_path_segment(segment: &str) -> String {
|
|||||||
|
|
||||||
/// Percent-encode a full slash-separated path, encoding each segment individually.
|
/// Percent-encode a full slash-separated path, encoding each segment individually.
|
||||||
pub(crate) fn encode_uri_path(path: &str) -> String {
|
pub(crate) fn encode_uri_path(path: &str) -> String {
|
||||||
path.split('/')
|
use std::fmt::Write as _;
|
||||||
.map(encode_path_segment)
|
// `utf8_percent_encode` returns a `Display` adapter, so write each encoded
|
||||||
.collect::<Vec<_>>()
|
// segment straight into `out` — avoids a String per segment and the joined
|
||||||
.join("/")
|
// Vec the previous `.map(...).collect::<Vec<_>>().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 `<D:href>` value for a non-collection (file) resource.
|
/// Build the `<D:href>` value for a non-collection (file) resource.
|
||||||
|
|||||||
Reference in New Issue
Block a user