fix(manager-core): F-P-001 batch app_user_roles lookups in users::list (N+1 → 1)
users::list page-fetched users then looped calling self.fetch_roles(cx.app_id, row.id) — one query per user row, with default limit 50 and max 500 — so the admin "users" page did 51 to 501 round-trips. The same fetch_roles is also called from verify_session_for_realtime on every authenticated SSE subscribe. Add AppUserRoleRepo::list_for_users(app_id, &[user_ids]) returning a HashMap<AppUserId, Vec<String>>; rewrite list to use it. Single query with `WHERE user_id = ANY($2)`. Empty-input fast path returns an empty map without touching Postgres. AUDIT.md anchor: F-P-001. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -47,6 +47,17 @@ pub trait AppUserRoleRepo: Send + Sync {
|
||||
app_id: AppId,
|
||||
user_id: AppUserId,
|
||||
) -> Result<Vec<String>, AppUserRoleRepoError>;
|
||||
|
||||
/// Batched variant of `list_for_user` — returns a map from user_id
|
||||
/// to that user's role list, in one round-trip. Used by paginated
|
||||
/// admin/script list endpoints to avoid an N+1.
|
||||
///
|
||||
/// Empty `user_ids` returns an empty map (zero queries).
|
||||
async fn list_for_users(
|
||||
&self,
|
||||
app_id: AppId,
|
||||
user_ids: &[AppUserId],
|
||||
) -> Result<std::collections::HashMap<AppUserId, Vec<String>>, AppUserRoleRepoError>;
|
||||
}
|
||||
|
||||
pub struct PostgresAppUserRoleRepo {
|
||||
@@ -129,4 +140,29 @@ impl AppUserRoleRepo for PostgresAppUserRoleRepo {
|
||||
.await?;
|
||||
Ok(rows.into_iter().map(|(s,)| s).collect())
|
||||
}
|
||||
|
||||
async fn list_for_users(
|
||||
&self,
|
||||
app_id: AppId,
|
||||
user_ids: &[AppUserId],
|
||||
) -> Result<std::collections::HashMap<AppUserId, Vec<String>>, AppUserRoleRepoError> {
|
||||
if user_ids.is_empty() {
|
||||
return Ok(std::collections::HashMap::new());
|
||||
}
|
||||
let ids: Vec<uuid::Uuid> = user_ids.iter().map(|u| u.into_inner()).collect();
|
||||
let rows: Vec<(uuid::Uuid, String)> = sqlx::query_as(
|
||||
"SELECT user_id, role FROM app_user_roles \
|
||||
WHERE app_id = $1 AND user_id = ANY($2) ORDER BY user_id, role",
|
||||
)
|
||||
.bind(app_id.into_inner())
|
||||
.bind(&ids)
|
||||
.fetch_all(&self.pool)
|
||||
.await?;
|
||||
let mut out: std::collections::HashMap<AppUserId, Vec<String>> =
|
||||
user_ids.iter().map(|id| (*id, Vec::new())).collect();
|
||||
for (uid, role) in rows {
|
||||
out.entry(AppUserId::from(uid)).or_default().push(role);
|
||||
}
|
||||
Ok(out)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -558,11 +558,22 @@ impl UsersService for UsersServiceImpl {
|
||||
)
|
||||
.await?;
|
||||
|
||||
let mut items = Vec::with_capacity(page.items.len());
|
||||
for row in page.items {
|
||||
let roles = self.fetch_roles(cx.app_id, row.id).await?;
|
||||
items.push(Self::to_app_user(row, roles));
|
||||
}
|
||||
// F-P-001: one batched roles lookup instead of N per-user
|
||||
// queries. With limit=500 this is 1 query vs 501.
|
||||
let user_ids: Vec<AppUserId> = page.items.iter().map(|r| r.id).collect();
|
||||
let mut roles_by_user = self
|
||||
.roles
|
||||
.list_for_users(cx.app_id, &user_ids)
|
||||
.await
|
||||
.map_err(|e| UsersError::Backend(e.to_string()))?;
|
||||
let items: Vec<AppUser> = page
|
||||
.items
|
||||
.into_iter()
|
||||
.map(|row| {
|
||||
let roles = roles_by_user.remove(&row.id).unwrap_or_default();
|
||||
Self::to_app_user(row, roles)
|
||||
})
|
||||
.collect();
|
||||
Ok(UsersListPage {
|
||||
items,
|
||||
next_cursor: page.next_cursor,
|
||||
|
||||
Reference in New Issue
Block a user