perf(listing): return per-item is_favorite/is_shared, drop client badge fetches
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M8Vb9QHmLZnEMzHz7MrFy6
This commit is contained in:
@@ -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<FolderListing>;
|
||||
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 : []
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -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<Set<string>>(new Set());
|
||||
#shared = $state<Set<string>>(new Set());
|
||||
#loaded = false;
|
||||
#inflight: Promise<void> | 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<void> {
|
||||
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();
|
||||
@@ -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<typeof fetchFavoritesPage>
|
||||
>;
|
||||
const sharePage = (...ids: string[]) =>
|
||||
({ items: ids.map((id) => ({ resource: { id } })) }) as unknown as Awaited<
|
||||
ReturnType<typeof fetchMyShares>
|
||||
>;
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user