From 9ccaeef0abc960ae77c4d776a04bdba231879ebf Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 19 Jun 2026 15:21:01 +0000 Subject: [PATCH] perf(listing): return per-item is_favorite/is_shared, drop client badge fetches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The folder listing now carries the favorite/share badge state for exactly the items it returns, so the files browser stops fetching favorites and outgoing shares separately. This removes the last per-navigation badge round-trips AND fixes the correctness hole of the previous approaches: badges were derived from only the first 200 global favorites / shares, so a favorited or shared item outside that window showed no badge. Now every listed item is correct, and the work is scoped to the items on screen. Backend (`GET /api/folders/{id}/listing`): - `FolderListingDto` gains `favorite_ids` and `shared_ids` (sorted) — listing- level metadata, so no churn to the many FileDto/FolderDto constructors. - The handler computes both with two batched, index-backed queries run concurrently: `FavoritesService::favorited_ids` (auth.user_favorites, ANY) and `PgAclEngine::shared_resource_ids` (storage.role_grants by granted_by + ANY, which already covers public links as 'token' grants — same membership the /grants/outgoing/resources endpoint exposes). Both fold into the ETag. - Public-share browsing passes empty sets (anonymous, read-only context). Frontend: - `listFolder` reads `favorite_ids` / `shared_ids`; the files view seeds local badge sets straight from the listing and updates them optimistically on favorite toggle / batch / share creation (via ShareDialog's `onshared`). - Removes the session `badges` store + its fetches entirely — the listing is now the single, authoritative, fetch-free source. Net: favorite/share badges cost zero extra client requests per navigation and are correct regardless of how many favorites/shares the user has. Validated: cargo check + clippy -D warnings (backend; integration tests need Postgres, unavailable here), frontend npm run check + unit tests, and a headless render of the real files route (list + grid) with the new flags present — no errors. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01M8Vb9QHmLZnEMzHz7MrFy6 --- frontend/src/lib/api/endpoints/folders.ts | 15 +++- frontend/src/lib/stores/badges.svelte.ts | 73 ------------------ frontend/src/lib/stores/badges.test.ts | 75 ------------------- .../src/routes/files/[...path]/+page.svelte | 60 +++++++++------ src/application/dtos/folder_listing_dto.rs | 7 ++ src/application/services/favorites_service.rs | 11 +++ .../services/share_browse_service.rs | 4 + src/infrastructure/services/pg_acl_engine.rs | 29 +++++++ src/interfaces/api/handlers/folder_handler.rs | 56 +++++++++++++- 9 files changed, 156 insertions(+), 174 deletions(-) delete mode 100644 frontend/src/lib/stores/badges.svelte.ts delete mode 100644 frontend/src/lib/stores/badges.test.ts diff --git a/frontend/src/lib/api/endpoints/folders.ts b/frontend/src/lib/api/endpoints/folders.ts index 308ac07f..f2863b01 100644 --- a/frontend/src/lib/api/endpoints/folders.ts +++ b/frontend/src/lib/api/endpoints/folders.ts @@ -13,6 +13,10 @@ const NO_CACHE: RequestInit = { export interface FolderListing { folders: FolderItem[]; files: FileItem[]; + /** Ids in this listing the caller has favorited (server-computed badge set). */ + favoriteIds: string[]; + /** Ids in this listing the caller has an outgoing share/grant on. */ + sharedIds: string[]; } /** Top-level folders for the user; the first entry is the home folder. */ @@ -37,10 +41,17 @@ export async function listFolder(folderId: string, forceRefresh = false): Promis const res = await apiFetch(url, { credentials: 'same-origin', cache: 'no-store', headers }); if (res.status === 403) throw Object.assign(new Error('Forbidden'), { status: 403 }); if (!res.ok) throw new Error(`listing failed: ${res.status}`); - const listing = (await res.json()) as Partial; + const listing = (await res.json()) as { + folders?: FolderItem[]; + files?: FileItem[]; + favorite_ids?: string[]; + shared_ids?: string[]; + }; return { folders: Array.isArray(listing.folders) ? listing.folders : [], - files: Array.isArray(listing.files) ? listing.files : [] + files: Array.isArray(listing.files) ? listing.files : [], + favoriteIds: Array.isArray(listing.favorite_ids) ? listing.favorite_ids : [], + sharedIds: Array.isArray(listing.shared_ids) ? listing.shared_ids : [] }; } diff --git a/frontend/src/lib/stores/badges.svelte.ts b/frontend/src/lib/stores/badges.svelte.ts deleted file mode 100644 index 8b081ee0..00000000 --- a/frontend/src/lib/stores/badges.svelte.ts +++ /dev/null @@ -1,73 +0,0 @@ -/** - * Session-scoped favorite / outgoing-share badge sets. - * - * The files browser shows a star (favorite) and a link (shared) badge per row. - * Previously every folder navigation re-fetched the first 200 favorites AND the - * first 200 shares — two round-trips per navigation, for data that barely - * changes. This caches both id sets once per session (`ensureLoaded`, deduped) - * and keeps them in sync via optimistic mutations from the views that toggle - * them, so navigating folders costs zero extra requests. - * - * (The 200-item ceiling is inherited from the previous implementation; the truly - * complete fix is to have the listing endpoint return per-item flags, a backend - * change tracked separately.) - */ -import { fetchFavoritesPage } from '$lib/api/endpoints/favorites'; -import { fetchMyShares } from '$lib/api/endpoints/grants'; - -class BadgesStore { - #favorites = $state>(new Set()); - #shared = $state>(new Set()); - #loaded = false; - #inflight: Promise | null = null; - - isFavorite(id: string): boolean { - return this.#favorites.has(id); - } - - isShared(id: string): boolean { - return this.#shared.has(id); - } - - /** Load both id sets once per session. Concurrent callers share one fetch. */ - ensureLoaded(): Promise { - if (this.#loaded) return Promise.resolve(); - if (this.#inflight) return this.#inflight; - this.#inflight = (async () => { - const [favs, shares] = await Promise.all([ - fetchFavoritesPage({ limit: 200 }).catch(() => null), - fetchMyShares({ limit: 200 }).catch(() => null) - ]); - if (favs) this.#favorites = new Set(favs.items.map((f) => f.resource.id)); - if (shares) this.#shared = new Set(shares.items.map((s) => s.resource.id)); - this.#loaded = true; - this.#inflight = null; - })(); - return this.#inflight; - } - - /** Optimistically reflect a favorite toggle (no refetch). */ - setFavorite(id: string, on: boolean): void { - if (on === this.#favorites.has(id)) return; - const next = new Set(this.#favorites); - if (on) next.add(id); - else next.delete(id); - this.#favorites = next; - } - - /** Mark an item as having an outgoing share (after one is created). */ - markShared(id: string): void { - if (this.#shared.has(id)) return; - this.#shared = new Set(this.#shared).add(id); - } - - /** Drop the cache (e.g. on logout) so the next session reloads fresh. */ - reset(): void { - this.#favorites = new Set(); - this.#shared = new Set(); - this.#loaded = false; - this.#inflight = null; - } -} - -export const badges = new BadgesStore(); diff --git a/frontend/src/lib/stores/badges.test.ts b/frontend/src/lib/stores/badges.test.ts deleted file mode 100644 index 0bfc3f26..00000000 --- a/frontend/src/lib/stores/badges.test.ts +++ /dev/null @@ -1,75 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; - -vi.mock('$lib/api/endpoints/favorites', () => ({ fetchFavoritesPage: vi.fn() })); -vi.mock('$lib/api/endpoints/grants', () => ({ fetchMyShares: vi.fn() })); - -import { fetchFavoritesPage } from '$lib/api/endpoints/favorites'; -import { fetchMyShares } from '$lib/api/endpoints/grants'; -import { badges } from './badges.svelte'; - -const favPage = (...ids: string[]) => - ({ items: ids.map((id) => ({ resource: { id } })) }) as unknown as Awaited< - ReturnType - >; -const sharePage = (...ids: string[]) => - ({ items: ids.map((id) => ({ resource: { id } })) }) as unknown as Awaited< - ReturnType - >; - -beforeEach(() => { - vi.clearAllMocks(); - vi.mocked(fetchFavoritesPage).mockResolvedValue(favPage('f1', 'f2')); - vi.mocked(fetchMyShares).mockResolvedValue(sharePage('s1')); - badges.reset(); -}); - -describe('badges store', () => { - it('loads once and serves every later navigation from cache', async () => { - // Five "folder navigations" each call ensureLoaded. - for (let i = 0; i < 5; i++) await badges.ensureLoaded(); - - expect(fetchFavoritesPage).toHaveBeenCalledTimes(1); - expect(fetchMyShares).toHaveBeenCalledTimes(1); - expect(badges.isFavorite('f1')).toBe(true); - expect(badges.isFavorite('f2')).toBe(true); - expect(badges.isShared('s1')).toBe(true); - expect(badges.isFavorite('nope')).toBe(false); - }); - - it('collapses concurrent loads into a single fetch', async () => { - await Promise.all([ - badges.ensureLoaded(), - badges.ensureLoaded(), - badges.ensureLoaded(), - badges.ensureLoaded() - ]); - expect(fetchFavoritesPage).toHaveBeenCalledTimes(1); - expect(fetchMyShares).toHaveBeenCalledTimes(1); - }); - - it('reflects favorite toggles optimistically without refetching', async () => { - await badges.ensureLoaded(); - badges.setFavorite('x', true); - expect(badges.isFavorite('x')).toBe(true); - badges.setFavorite('x', false); - expect(badges.isFavorite('x')).toBe(false); - // No extra network for optimistic updates. - expect(fetchFavoritesPage).toHaveBeenCalledTimes(1); - }); - - it('marks an item shared after a share is created', async () => { - await badges.ensureLoaded(); - expect(badges.isShared('new')).toBe(false); - badges.markShared('new'); - expect(badges.isShared('new')).toBe(true); - }); - - it('reset() clears the cache and allows a fresh reload', async () => { - await badges.ensureLoaded(); - expect(fetchFavoritesPage).toHaveBeenCalledTimes(1); - badges.reset(); - expect(badges.isFavorite('f1')).toBe(false); - await badges.ensureLoaded(); - expect(fetchFavoritesPage).toHaveBeenCalledTimes(2); - }); -}); diff --git a/frontend/src/routes/files/[...path]/+page.svelte b/frontend/src/routes/files/[...path]/+page.svelte index 3290fd01..09842a51 100644 --- a/frontend/src/routes/files/[...path]/+page.svelte +++ b/frontend/src/routes/files/[...path]/+page.svelte @@ -39,7 +39,6 @@ import WopiEditor from '$lib/components/WopiEditor.svelte'; import { t } from '$lib/i18n/index.svelte'; import { confirmDialog, promptDialog } from '$lib/stores/dialogs.svelte'; - import { badges } from '$lib/stores/badges.svelte'; import { files as filesStore } from '$lib/stores/files.svelte'; import { session } from '$lib/stores/session.svelte'; import { ui } from '$lib/stores/ui.svelte'; @@ -58,7 +57,7 @@ // /files → home root; /files/a/b → folder b inside a inside home. const pathSegments = $derived((page.params.path ?? '').split('/').filter((s) => s.length > 0)); - let listing = $state({ folders: [], files: [] }); + let listing = $state({ folders: [], files: [], favoriteIds: [], sharedIds: [] }); let crumbs = $state>([]); let currentId = $state(null); let loading = $state(false); @@ -80,6 +79,12 @@ let actionTarget = $state(null); let moveItems = $state(null); + // Favorite + shared badge sets for the current folder, seeded directly from + // the listing response (server-computed, scoped to these items — no extra + // per-navigation fetch) and updated optimistically on mutation. + let favoriteIds = $state>(new Set()); + let sharedIds = $state>(new Set()); + function openMove(kind: ItemType, id: string, name: string) { actionTarget = { id, name, kind }; moveItems = null; @@ -98,15 +103,21 @@ } async function toggleFavorite(kind: ItemType, id: string) { - const isFav = badges.isFavorite(id); + const isFav = favoriteIds.has(id); // Optimistic toggle, reverted on failure. - badges.setFavorite(id, !isFav); + const next = new Set(favoriteIds); + if (isFav) next.delete(id); + else next.add(id); + favoriteIds = next; try { if (isFav) await removeFavorite(kind, id); else await addFavorite(kind, id); } catch (e) { errorToast(e); - badges.setFavorite(id, isFav); + const reverted = new Set(favoriteIds); + if (isFav) reverted.add(id); + else reverted.delete(id); + favoriteIds = reverted; } } @@ -147,7 +158,8 @@ const [data, trail] = await Promise.all([listFolder(folderId), buildCrumbs(pathSegments)]); listing = data; crumbs = trail; - void badges.ensureLoaded(); + favoriteIds = new Set(data.favoriteIds); + sharedIds = new Set(data.sharedIds); maybeOpenDeepLink(); } catch (e) { // 403 → friendly message rather than the raw "Forbidden" error string. @@ -423,7 +435,7 @@ /** Batch add the selection to favorites — single /api/favorites/batch call. */ async function batchFavorites() { - const items = selectionTargets().filter((it) => !badges.isFavorite(it.id)); + const items = selectionTargets().filter((it) => !favoriteIds.has(it.id)); if (items.length === 0) { ui.notify(t('files.already_favorites', 'All selected items are already favorites'), 'info'); clearSelection(); @@ -439,7 +451,7 @@ }) }); if (!res.ok) throw new Error(`Server returned ${res.status}`); - for (const it of items) badges.setFavorite(it.id, true); + favoriteIds = new Set([...favoriteIds, ...items.map((it) => it.id)]); ui.notify(t('files.added_favorites', 'Added to favorites'), 'success'); clearSelection(); } catch (e) { @@ -1196,13 +1208,13 @@
{folder.name} - {#if badges.isFavorite(folder.id)}
{/if} - {#if badges.isShared(folder.id)}
@@ -1219,15 +1231,15 @@
diff --git a/src/application/dtos/folder_listing_dto.rs b/src/application/dtos/folder_listing_dto.rs index bdfc242f..7380ce85 100644 --- a/src/application/dtos/folder_listing_dto.rs +++ b/src/application/dtos/folder_listing_dto.rs @@ -12,4 +12,11 @@ pub struct FolderListingDto { pub folders: Vec, /// Files inside the requested folder pub files: Vec, + /// Ids (folders + files in this listing) the caller has favorited. Lets the + /// client render star badges without a separate per-navigation favorites + /// fetch. Sorted for a stable response / ETag. + pub favorite_ids: Vec, + /// Ids in this listing the caller has an outgoing share/grant on (incl. + /// public links). Sorted for a stable response / ETag. + pub shared_ids: Vec, } diff --git a/src/application/services/favorites_service.rs b/src/application/services/favorites_service.rs index 6ca17c54..e8970b42 100644 --- a/src/application/services/favorites_service.rs +++ b/src/application/services/favorites_service.rs @@ -27,6 +27,17 @@ impl FavoritesService { pub fn new(repo: Arc) -> Self { Self { repo } } + + /// Subset of `(item_id, item_type)` pairs the user has favorited — used to + /// stamp star badges onto a folder listing in one batched query (no N+1, no + /// global page fetch). + pub async fn favorited_ids( + &self, + user_id: Uuid, + items: &[(&str, &str)], + ) -> Result> { + self.repo.batch_check_favorites(user_id, items).await + } } impl FavoritesUseCase for FavoritesService { diff --git a/src/application/services/share_browse_service.rs b/src/application/services/share_browse_service.rs index 64f0bd92..a68d6685 100644 --- a/src/application/services/share_browse_service.rs +++ b/src/application/services/share_browse_service.rs @@ -186,6 +186,10 @@ impl ShareBrowseService { Ok(FolderListingDto { folders: folders_res?, files: files_res?, + // Public-share browsing is an anonymous, read-only context — no + // per-caller favorite/share badges apply. + favorite_ids: Vec::new(), + shared_ids: Vec::new(), }) } } diff --git a/src/infrastructure/services/pg_acl_engine.rs b/src/infrastructure/services/pg_acl_engine.rs index 652d54e3..bc02550a 100644 --- a/src/infrastructure/services/pg_acl_engine.rs +++ b/src/infrastructure/services/pg_acl_engine.rs @@ -105,6 +105,35 @@ impl PgAclEngine { } } + /// Subset of `resource_ids` the caller has shared — i.e. has any outgoing + /// role grant on (a `user`/`group` grant or a `token` grant, the latter + /// being a public link). One batched query, mirroring the membership the + /// `/grants/outgoing/resources` endpoint exposes; used to stamp "shared" + /// badges onto a folder listing without a per-navigation grants fetch. + pub async fn shared_resource_ids( + &self, + granted_by: Uuid, + resource_ids: &[Uuid], + ) -> Result, DomainError> { + if resource_ids.is_empty() { + return Ok(HashSet::new()); + } + let rows: Vec<(Uuid,)> = sqlx::query_as( + r#" + SELECT DISTINCT resource_id + FROM storage.role_grants + WHERE granted_by = $1 + AND resource_id = ANY($2) + "#, + ) + .bind(granted_by) + .bind(resource_ids) + .fetch_all(self.pool.as_ref()) + .await + .map_err(|e| DomainError::internal_error("PgAcl", format!("shared_resource_ids: {e}")))?; + Ok(rows.into_iter().map(|(id,)| id).collect()) + } + /// Creates a stub instance for tests that need to construct services /// without a real PostgreSQL pool. Connecting to the lazy pool will /// fail at runtime — only safe in tests that exercise types, not actual diff --git a/src/interfaces/api/handlers/folder_handler.rs b/src/interfaces/api/handlers/folder_handler.rs index da113be5..ff1160b7 100644 --- a/src/interfaces/api/handlers/folder_handler.rs +++ b/src/interfaces/api/handlers/folder_handler.rs @@ -169,6 +169,8 @@ impl FolderHandler { fn compute_listing_etag( folders: &[crate::application::dtos::folder_dto::FolderDto], files: &[crate::application::dtos::file_dto::FileDto], + favorite_ids: &[String], + shared_ids: &[String], ) -> String { let max_mod = folders .iter() @@ -180,6 +182,10 @@ impl FolderHandler { let mut hasher = std::collections::hash_map::DefaultHasher::new(); max_mod.hash(&mut hasher); count.hash(&mut hasher); + // Badge state is part of the representation — fold it in (both slices are + // sorted, so the hash is stable) so a favorite/share change busts the ETag. + favorite_ids.hash(&mut hasher); + shared_ids.hash(&mut hasher); format!("\"{:x}\"", hasher.finish()) } @@ -205,7 +211,48 @@ impl FolderHandler { match (folders_result, files_result) { (Ok(folders), Ok(files)) => { - let etag = Self::compute_listing_etag(&folders, &files); + // Badge enrichment for this listing: which items the caller has + // favorited / shared. Two batched, index-backed queries (run + // concurrently) replace the client's old per-navigation global + // favorites + outgoing-shares fetches — correct (no 200-item + // ceiling) and scoped to just the items on screen. + let fav_pairs: Vec<(&str, &str)> = folders + .iter() + .map(|f| (f.id.as_str(), "folder")) + .chain(files.iter().map(|f| (f.id.as_str(), "file"))) + .collect(); + let resource_uuids: Vec = folders + .iter() + .map(|f| f.id.as_str()) + .chain(files.iter().map(|f| f.id.as_str())) + .filter_map(|s| uuid::Uuid::parse_str(s).ok()) + .collect(); + + let (favorited, shared) = tokio::join!( + async { + match &state.favorites_service { + Some(svc) => svc + .favorited_ids(auth_user.id, &fav_pairs) + .await + .unwrap_or_default(), + None => Default::default(), + } + }, + state + .authorization + .shared_resource_ids(auth_user.id, &resource_uuids) + ); + + let mut favorite_ids: Vec = favorited.into_iter().collect(); + favorite_ids.sort(); + let mut shared_ids: Vec = shared + .unwrap_or_default() + .into_iter() + .map(|u| u.to_string()) + .collect(); + shared_ids.sort(); + + let etag = Self::compute_listing_etag(&folders, &files, &favorite_ids, &shared_ids); // 304 Not Modified if the client already has this version if let Some(inm) = headers.get(header::IF_NONE_MATCH) @@ -219,7 +266,12 @@ impl FolderHandler { .unwrap() .into_response(); } - let listing = FolderListingDto { folders, files }; + let listing = FolderListingDto { + folders, + files, + favorite_ids, + shared_ids, + }; let mut resp = (StatusCode::OK, Json(listing)).into_response(); resp.headers_mut() .insert(header::ETAG, header::HeaderValue::from_str(&etag).unwrap());