From 5303419eec6f5a4eac99f8d8e9e2618d1e862a34 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sun, 7 Jun 2026 20:09:31 +0200 Subject: [PATCH] =?UTF-8?q?fix(manager-core):=20F-P-001=20batch=20app=5Fus?= =?UTF-8?q?er=5Froles=20lookups=20in=20users::list=20(N+1=20=E2=86=92=201)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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>; 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) --- crates/manager-core/src/app_user_role_repo.rs | 36 +++++++++++++++++++ crates/manager-core/src/users_service.rs | 21 ++++++++--- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/crates/manager-core/src/app_user_role_repo.rs b/crates/manager-core/src/app_user_role_repo.rs index e1f609f..2646834 100644 --- a/crates/manager-core/src/app_user_role_repo.rs +++ b/crates/manager-core/src/app_user_role_repo.rs @@ -47,6 +47,17 @@ pub trait AppUserRoleRepo: Send + Sync { app_id: AppId, user_id: AppUserId, ) -> Result, 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>, 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>, AppUserRoleRepoError> { + if user_ids.is_empty() { + return Ok(std::collections::HashMap::new()); + } + let ids: Vec = 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> = + 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) + } } diff --git a/crates/manager-core/src/users_service.rs b/crates/manager-core/src/users_service.rs index 8de7f37..d03c241 100644 --- a/crates/manager-core/src/users_service.rs +++ b/crates/manager-core/src/users_service.rs @@ -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 = 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 = 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,