max / makenotwork
8 files changed,
+189 insertions,
-68 deletions
| @@ -6,19 +6,6 @@ use sqlx::PgPool; | |||
| 6 | 6 | use super::{ItemId, ProjectId, UserId}; | |
| 7 | 7 | use crate::error::Result; | |
| 8 | 8 | ||
| 9 | - | /// Check if an item is in the user's cart. | |
| 10 | - | #[tracing::instrument(skip_all)] | |
| 11 | - | pub async fn is_in_cart(pool: &PgPool, user_id: UserId, item_id: ItemId) -> Result<bool> { | |
| 12 | - | let exists: bool = sqlx::query_scalar( | |
| 13 | - | "SELECT EXISTS(SELECT 1 FROM cart_items WHERE user_id = $1 AND item_id = $2)", | |
| 14 | - | ) | |
| 15 | - | .bind(user_id) | |
| 16 | - | .bind(item_id) | |
| 17 | - | .fetch_one(pool) | |
| 18 | - | .await?; | |
| 19 | - | ||
| 20 | - | Ok(exists) | |
| 21 | - | } | |
| 22 | 9 | ||
| 23 | 10 | /// Add an item to the user's cart (idempotent). | |
| 24 | 11 | #[tracing::instrument(skip_all)] |
| @@ -326,28 +326,6 @@ pub async fn reorder_collection_items( | |||
| 326 | 326 | Ok(()) | |
| 327 | 327 | } | |
| 328 | 328 | ||
| 329 | - | /// Count how many of a user's collections contain a specific item. | |
| 330 | - | #[tracing::instrument(skip_all)] | |
| 331 | - | pub async fn count_user_collections_containing_item( | |
| 332 | - | pool: &PgPool, | |
| 333 | - | user_id: UserId, | |
| 334 | - | item_id: ItemId, | |
| 335 | - | ) -> Result<i64> { | |
| 336 | - | let (count,): (i64,) = sqlx::query_as( | |
| 337 | - | r#" | |
| 338 | - | SELECT COUNT(*) FROM collections c | |
| 339 | - | JOIN collection_items ci ON ci.collection_id = c.id | |
| 340 | - | WHERE c.user_id = $1 AND ci.item_id = $2 | |
| 341 | - | "#, | |
| 342 | - | ) | |
| 343 | - | .bind(user_id) | |
| 344 | - | .bind(item_id) | |
| 345 | - | .fetch_one(pool) | |
| 346 | - | .await?; | |
| 347 | - | ||
| 348 | - | Ok(count) | |
| 349 | - | } | |
| 350 | - | ||
| 351 | 329 | /// Get a user's collections with membership state for a specific item. | |
| 352 | 330 | /// Returns (collection_id, title, is_in_collection) for the "add to collection" dropdown. | |
| 353 | 331 | #[tracing::instrument(skip_all)] |
| @@ -17,6 +17,45 @@ use super::models::*; | |||
| 17 | 17 | use super::{ItemId, MtThreadId, PriceCents, ProjectId, UserId}; | |
| 18 | 18 | use crate::error::Result; | |
| 19 | 19 | ||
| 20 | + | /// Per-viewer display flags for an item's store page (wishlisted / in-cart / | |
| 21 | + | /// number of the viewer's collections containing it). Display-only — none of | |
| 22 | + | /// these gate access. | |
| 23 | + | #[derive(Debug, Default)] | |
| 24 | + | pub struct ViewerItemFlags { | |
| 25 | + | pub is_wishlisted: bool, | |
| 26 | + | pub in_cart: bool, | |
| 27 | + | pub collection_count: i64, | |
| 28 | + | } | |
| 29 | + | ||
| 30 | + | /// Fetch a viewer's display flags for an item in ONE query instead of three | |
| 31 | + | /// sequential round-trips (wishlist EXISTS, cart EXISTS, collections COUNT). | |
| 32 | + | /// Fewer round-trips AND fewer pooled connections per item-page render — the | |
| 33 | + | /// collapse the public item page wants (vs. fanning the three out concurrently, | |
| 34 | + | /// which would add connection pressure on a hot public path). | |
| 35 | + | #[tracing::instrument(skip_all)] | |
| 36 | + | pub async fn get_viewer_item_flags( | |
| 37 | + | pool: &PgPool, | |
| 38 | + | user_id: UserId, | |
| 39 | + | item_id: ItemId, | |
| 40 | + | ) -> Result<ViewerItemFlags> { | |
| 41 | + | let (is_wishlisted, in_cart, collection_count) = sqlx::query_as::<_, (bool, bool, i64)>( | |
| 42 | + | r#" | |
| 43 | + | SELECT | |
| 44 | + | EXISTS(SELECT 1 FROM wishlists WHERE user_id = $1 AND item_id = $2), | |
| 45 | + | EXISTS(SELECT 1 FROM cart_items WHERE user_id = $1 AND item_id = $2), | |
| 46 | + | (SELECT COUNT(*) FROM collections c | |
| 47 | + | JOIN collection_items ci ON ci.collection_id = c.id | |
| 48 | + | WHERE c.user_id = $1 AND ci.item_id = $2) | |
| 49 | + | "#, | |
| 50 | + | ) | |
| 51 | + | .bind(user_id) | |
| 52 | + | .bind(item_id) | |
| 53 | + | .fetch_one(pool) | |
| 54 | + | .await?; | |
| 55 | + | ||
| 56 | + | Ok(ViewerItemFlags { is_wishlisted, in_cart, collection_count }) | |
| 57 | + | } | |
| 58 | + | ||
| 20 | 59 | /// Insert a new item into a project and return the created row. | |
| 21 | 60 | /// | |
| 22 | 61 | /// Auto-generates a URL-safe slug from the title. If the slug collides with |
| @@ -90,6 +90,44 @@ pub async fn list_collaborators( | |||
| 90 | 90 | Ok(rows) | |
| 91 | 91 | } | |
| 92 | 92 | ||
| 93 | + | /// A collaborator row tagged with its `repo_id`, for batched listing across | |
| 94 | + | /// many repos in a single query (see [`list_collaborators_for_repos`]). | |
| 95 | + | #[derive(Debug, FromRow)] | |
| 96 | + | pub struct CollaboratorForRepo { | |
| 97 | + | pub repo_id: GitRepoId, | |
| 98 | + | pub user_id: UserId, | |
| 99 | + | pub username: String, | |
| 100 | + | pub can_push: bool, | |
| 101 | + | } | |
| 102 | + | ||
| 103 | + | /// List collaborators for many repos in ONE query, each row tagged with its | |
| 104 | + | /// `repo_id` so the caller can group them. Replaces a per-repo `list_collaborators` | |
| 105 | + | /// loop (the N+1 in the project code tab). Returns rows ordered by repo then | |
| 106 | + | /// join time; an empty input short-circuits without a query. | |
| 107 | + | #[tracing::instrument(skip_all)] | |
| 108 | + | pub async fn list_collaborators_for_repos( | |
| 109 | + | pool: &PgPool, | |
| 110 | + | repo_ids: &[GitRepoId], | |
| 111 | + | ) -> Result<Vec<CollaboratorForRepo>> { | |
| 112 | + | if repo_ids.is_empty() { | |
| 113 | + | return Ok(Vec::new()); | |
| 114 | + | } | |
| 115 | + | let rows = sqlx::query_as::<_, CollaboratorForRepo>( | |
| 116 | + | r#" | |
| 117 | + | SELECT rc.repo_id, rc.user_id, u.username, rc.can_push | |
| 118 | + | FROM repo_collaborators rc | |
| 119 | + | JOIN users u ON u.id = rc.user_id | |
| 120 | + | WHERE rc.repo_id = ANY($1) | |
| 121 | + | ORDER BY rc.repo_id, rc.created_at ASC | |
| 122 | + | "#, | |
| 123 | + | ) | |
| 124 | + | .bind(repo_ids) | |
| 125 | + | .fetch_all(pool) | |
| 126 | + | .await?; | |
| 127 | + | ||
| 128 | + | Ok(rows) | |
| 129 | + | } | |
| 130 | + | ||
| 93 | 131 | /// Check if a user has push access to a repo (either owner or collaborator with can_push). | |
| 94 | 132 | #[tracing::instrument(skip_all)] | |
| 95 | 133 | pub async fn can_user_push( |
| @@ -468,25 +468,29 @@ pub(super) async fn project_tab_code( | |||
| 468 | 468 | let all_repos = db::git_repos::get_repos_by_user(&state.db, session_user.id).await.unwrap_or_default(); | |
| 469 | 469 | let available_repos: Vec<_> = all_repos.into_iter().filter(|r| r.project_id.is_none()).collect(); | |
| 470 | 470 | ||
| 471 | - | // Build linked repo views with collaborators | |
| 472 | - | let mut linked_repos = Vec::with_capacity(db_linked_repos.len()); | |
| 473 | - | for repo in &db_linked_repos { | |
| 474 | - | let collabs = db::repo_collaborators::list_collaborators(&state.db, repo.id) | |
| 475 | - | .await | |
| 476 | - | .unwrap_or_default(); | |
| 477 | - | linked_repos.push(LinkedRepoView { | |
| 478 | - | id: repo.id.to_string(), | |
| 479 | - | name: repo.name.clone(), | |
| 480 | - | collaborators: collabs | |
| 481 | - | .iter() | |
| 482 | - | .map(|c| RepoCollaboratorView { | |
| 483 | - | user_id: c.user_id.to_string(), | |
| 484 | - | username: c.username.clone(), | |
| 485 | - | can_push: c.can_push, | |
| 486 | - | }) | |
| 487 | - | .collect(), | |
| 471 | + | // Build linked repo views with collaborators. One batched query for all | |
| 472 | + | // repos' collaborators, grouped in memory — avoids the per-repo N+1. | |
| 473 | + | let repo_ids: Vec<_> = db_linked_repos.iter().map(|r| r.id).collect(); | |
| 474 | + | let mut collabs_by_repo: std::collections::HashMap<_, Vec<RepoCollaboratorView>> = | |
| 475 | + | std::collections::HashMap::new(); | |
| 476 | + | for c in db::repo_collaborators::list_collaborators_for_repos(&state.db, &repo_ids) | |
| 477 | + | .await | |
| 478 | + | .unwrap_or_default() | |
| 479 | + | { | |
| 480 | + | collabs_by_repo.entry(c.repo_id).or_default().push(RepoCollaboratorView { | |
| 481 | + | user_id: c.user_id.to_string(), | |
| 482 | + | username: c.username, | |
| 483 | + | can_push: c.can_push, | |
| 488 | 484 | }); | |
| 489 | 485 | } | |
| 486 | + | let linked_repos: Vec<LinkedRepoView> = db_linked_repos | |
| 487 | + | .iter() | |
| 488 | + | .map(|repo| LinkedRepoView { | |
| 489 | + | id: repo.id.to_string(), | |
| 490 | + | name: repo.name.clone(), | |
| 491 | + | collaborators: collabs_by_repo.remove(&repo.id).unwrap_or_default(), | |
| 492 | + | }) | |
| 493 | + | .collect(); | |
| 490 | 494 | ||
| 491 | 495 | let db_items = db::items::get_items_by_project(&state.db, db_project.id).await?; | |
| 492 | 496 | let project = Project::from_db(&db_project, db_items.len() as u32); |
| @@ -236,25 +236,19 @@ pub(crate) async fn render_item_page( | |||
| 236 | 236 | let db_sections = db::item_sections::list_by_item(&state.db, db_item.id).await?; | |
| 237 | 237 | let sections: Vec<ItemSection> = db_sections.iter().map(|s| ItemSection::from_db(s, db_project.user_id, cdn_base)).collect(); | |
| 238 | 238 | ||
| 239 | - | let is_wishlisted = if let Some(ref user) = maybe_user { | |
| 240 | - | db::wishlists::is_wishlisted(&state.db, user.id, db_item.id).await.unwrap_or(false) | |
| 241 | - | } else { | |
| 242 | - | false | |
| 243 | - | }; | |
| 244 | - | ||
| 245 | - | let in_cart = if let Some(ref user) = maybe_user { | |
| 246 | - | db::cart::is_in_cart(&state.db, user.id, db_item.id).await.unwrap_or(false) | |
| 247 | - | } else { | |
| 248 | - | false | |
| 249 | - | }; | |
| 250 | - | ||
| 251 | - | let collection_count = if let Some(ref user) = maybe_user { | |
| 252 | - | db::collections::count_user_collections_containing_item(&state.db, user.id, db_item.id) | |
| 239 | + | // Wishlist / cart / collection-count for the viewer, collapsed into one query | |
| 240 | + | // (was three sequential round-trips). Display-only; a failure falls back to | |
| 241 | + | // the empty default, matching the prior per-call error tolerance. | |
| 242 | + | let viewer_flags = if let Some(ref user) = maybe_user { | |
| 243 | + | db::items::get_viewer_item_flags(&state.db, user.id, db_item.id) | |
| 253 | 244 | .await | |
| 254 | - | .unwrap_or(0) as u32 | |
| 245 | + | .unwrap_or_default() | |
| 255 | 246 | } else { | |
| 256 | - | 0 | |
| 247 | + | db::items::ViewerItemFlags::default() | |
| 257 | 248 | }; | |
| 249 | + | let is_wishlisted = viewer_flags.is_wishlisted; | |
| 250 | + | let in_cart = viewer_flags.in_cart; | |
| 251 | + | let collection_count = viewer_flags.collection_count as u32; | |
| 258 | 252 | ||
| 259 | 253 | // Ordered gallery → carousel frames (additive to the cover image). Alt is | |
| 260 | 254 | // creator-optional; fall back to a title-based description rather than an |
| @@ -238,3 +238,41 @@ async fn update_cart_pwyw_above_cap_rejected() { | |||
| 238 | 238 | resp.status, resp.text | |
| 239 | 239 | ); | |
| 240 | 240 | } | |
| 241 | + | ||
| 242 | + | // PERF-2: the item page collapses wishlist/cart/collection-count into one query | |
| 243 | + | // (db::items::get_viewer_item_flags). Pin that the combined query reflects each | |
| 244 | + | // piece of state and stays scoped to the viewer. | |
| 245 | + | #[tokio::test] | |
| 246 | + | async fn viewer_item_flags_reflects_wishlist_cart_and_collections() { | |
| 247 | + | use makenotwork::db::items::get_viewer_item_flags; | |
| 248 | + | use makenotwork::db::ItemId; | |
| 249 | + | ||
| 250 | + | let mut h = TestHarness::new().await; | |
| 251 | + | let setup = h.create_creator_with_item("flagviewer", "digital", 500).await; | |
| 252 | + | let user_id = setup.user_id; | |
| 253 | + | let item = ItemId::from(uuid::Uuid::parse_str(&setup.item_id).unwrap()); | |
| 254 | + | ||
| 255 | + | // Nothing seeded yet. | |
| 256 | + | let f = get_viewer_item_flags(&h.db, user_id, item).await.unwrap(); | |
| 257 | + | assert!(!f.is_wishlisted && !f.in_cart && f.collection_count == 0, "empty state"); | |
| 258 | + | ||
| 259 | + | sqlx::query("INSERT INTO wishlists (user_id, item_id) VALUES ($1, $2::uuid)") | |
| 260 | + | .bind(user_id).bind(&setup.item_id).execute(&h.db).await.unwrap(); | |
| 261 | + | sqlx::query("INSERT INTO cart_items (user_id, item_id) VALUES ($1, $2::uuid)") | |
| 262 | + | .bind(user_id).bind(&setup.item_id).execute(&h.db).await.unwrap(); | |
| 263 | + | let collection_id: uuid::Uuid = sqlx::query_scalar( | |
| 264 | + | "INSERT INTO collections (user_id, slug, title) VALUES ($1, 'fav', 'Favorites') RETURNING id", | |
| 265 | + | ).bind(user_id).fetch_one(&h.db).await.unwrap(); | |
| 266 | + | sqlx::query("INSERT INTO collection_items (collection_id, item_id) VALUES ($1, $2::uuid)") | |
| 267 | + | .bind(collection_id).bind(&setup.item_id).execute(&h.db).await.unwrap(); | |
| 268 | + | ||
| 269 | + | let f = get_viewer_item_flags(&h.db, user_id, item).await.unwrap(); | |
| 270 | + | assert!(f.is_wishlisted, "wishlist flag"); | |
| 271 | + | assert!(f.in_cart, "cart flag"); | |
| 272 | + | assert_eq!(f.collection_count, 1, "collection count"); | |
| 273 | + | ||
| 274 | + | // A different viewer sees none of it (per-viewer scoping). | |
| 275 | + | let other = h.signup("flagother", "flagother@test.com", "password123").await; | |
| 276 | + | let f2 = get_viewer_item_flags(&h.db, other, item).await.unwrap(); | |
| 277 | + | assert!(!f2.is_wishlisted && !f2.in_cart && f2.collection_count == 0, "other viewer isolated"); | |
| 278 | + | } |
| @@ -308,3 +308,46 @@ async fn project_page_with_git_link() { | |||
| 308 | 308 | "Project page should render with git repos in DB" | |
| 309 | 309 | ); | |
| 310 | 310 | } | |
| 311 | + | ||
| 312 | + | // PERF-3: the project code tab batches all linked repos' collaborators into one | |
| 313 | + | // query (db::repo_collaborators::list_collaborators_for_repos) instead of a | |
| 314 | + | // per-repo N+1. Pin that the batched query returns every collaborator tagged | |
| 315 | + | // with the right repo_id, and that an empty input short-circuits. | |
| 316 | + | #[tokio::test] | |
| 317 | + | async fn list_collaborators_for_repos_groups_by_repo() { | |
| 318 | + | use makenotwork::db::repo_collaborators::list_collaborators_for_repos; | |
| 319 | + | use makenotwork::db::GitRepoId; | |
| 320 | + | ||
| 321 | + | let mut h = TestHarness::new().await; | |
| 322 | + | let owner = h.signup("repoowner", "repoowner@test.com", "password123").await; | |
| 323 | + | let alice = h.signup("collaba", "collaba@test.com", "password123").await; | |
| 324 | + | let bob = h.signup("collabb", "collabb@test.com", "password123").await; | |
| 325 | + | ||
| 326 | + | let repo1: uuid::Uuid = sqlx::query_scalar( | |
| 327 | + | "INSERT INTO git_repos (user_id, name) VALUES ($1, 'repo-one') RETURNING id", | |
| 328 | + | ).bind(owner).fetch_one(&h.db).await.unwrap(); | |
| 329 | + | let repo2: uuid::Uuid = sqlx::query_scalar( | |
| 330 | + | "INSERT INTO git_repos (user_id, name) VALUES ($1, 'repo-two') RETURNING id", | |
| 331 | + | ).bind(owner).fetch_one(&h.db).await.unwrap(); | |
| 332 | + | ||
| 333 | + | // repo1: alice (push) + bob (no push); repo2: bob (push). | |
| 334 | + | sqlx::query("INSERT INTO repo_collaborators (repo_id, user_id, can_push) VALUES ($1, $2, true)") | |
| 335 | + | .bind(repo1).bind(alice).execute(&h.db).await.unwrap(); | |
| 336 | + | sqlx::query("INSERT INTO repo_collaborators (repo_id, user_id, can_push) VALUES ($1, $2, false)") | |
| 337 | + | .bind(repo1).bind(bob).execute(&h.db).await.unwrap(); | |
| 338 | + | sqlx::query("INSERT INTO repo_collaborators (repo_id, user_id, can_push) VALUES ($1, $2, true)") | |
| 339 | + | .bind(repo2).bind(bob).execute(&h.db).await.unwrap(); | |
| 340 | + | ||
| 341 | + | let r1 = GitRepoId::from(repo1); | |
| 342 | + | let r2 = GitRepoId::from(repo2); | |
| 343 | + | let rows = list_collaborators_for_repos(&h.db, &[r1, r2]).await.unwrap(); | |
| 344 | + | assert_eq!(rows.len(), 3, "all collaborators across both repos in one query"); | |
| 345 | + | assert_eq!(rows.iter().filter(|c| c.repo_id == r1).count(), 2, "repo1 has two collaborators"); | |
| 346 | + | let repo2_collabs: Vec<_> = rows.iter().filter(|c| c.repo_id == r2).collect(); | |
| 347 | + | assert_eq!(repo2_collabs.len(), 1, "repo2 has one collaborator"); | |
| 348 | + | assert_eq!(repo2_collabs[0].username, "collabb"); | |
| 349 | + | assert!(repo2_collabs[0].can_push, "bob has push on repo2"); | |
| 350 | + | ||
| 351 | + | // Empty input short-circuits without a query. | |
| 352 | + | assert!(list_collaborators_for_repos(&h.db, &[]).await.unwrap().is_empty()); | |
| 353 | + | } |