diff --git a/apps/api/src/routes/__tests__/users.test.ts b/apps/api/src/routes/__tests__/users.test.ts index 7a388bc..d2ced06 100644 --- a/apps/api/src/routes/__tests__/users.test.ts +++ b/apps/api/src/routes/__tests__/users.test.ts @@ -271,23 +271,31 @@ describe("users routes", () => { const token = await createTestJWT({ sub: "user-1", githubId: "123", name: "User1" }); const app = await createTestApp(); const env = createTestEnv(); + const smallEnv = { ...env, MAX_CACHE_SIZE: "3" } as any; mockDb.select = vi.fn(() => makeQuery({ allResult: [{ id: "user-2", name: "Result 1" }] })); - // We send 1002 requests with unique queries to hit the limit - // (Wait, sending 1000 requests might be slow. Is there a better way? Let's just do it in a Promise.all or similar) - // Alternatively, we can test MAX_CACHE_SIZE by exporting it in test. Since we can't easily, we'll just loop. - const reqs = []; - for (let i = 0; i < 1002; i++) { - reqs.push(app.request(`/api/users/search?q=limit${i}`, { headers: { Authorization: `Bearer ${token}` } }, env as any)); + // Use a small cache size to keep the eviction test fast and deterministic. + for (let i = 0; i < 3; i++) { + await app.request(`/api/users/search?q=limit${i}`, { headers: { Authorization: `Bearer ${token}` } }, smallEnv); } - await Promise.all(reqs); - const finalReq = await app.request("/api/users/search?q=limittrigger", { headers: { Authorization: `Bearer ${token}` } }, env as any); - expect(finalReq.status).toBe(200); + const callCountAfterFill = mockDb.select.mock.calls.length; + + // cache hit should promote limit0 to MRU without new DB query + await app.request(`/api/users/search?q=limit0`, { headers: { Authorization: `Bearer ${token}` } }, smallEnv); + const callCountAfterPromotion = mockDb.select.mock.calls.length; + expect(callCountAfterPromotion).toBe(callCountAfterFill); + + // trigger eviction, then verify limit1 (now oldest) is evicted while limit0 stays cached + await app.request("/api/users/search?q=limittrigger", { headers: { Authorization: `Bearer ${token}` } }, smallEnv); + await app.request(`/api/users/search?q=limit1`, { headers: { Authorization: `Bearer ${token}` } }, smallEnv); + await app.request(`/api/users/search?q=limit0`, { headers: { Authorization: `Bearer ${token}` } }, smallEnv); + + expect(mockDb.select.mock.calls.length).toBe(callCountAfterPromotion + 2); }); - it("GET /api/users/:id returns 404 when the user does not exist (covered missed line 154)", async () => { + it("GET /api/users/:id returns 404 for missing user profile fetch", async () => { mockDb.select = vi.fn(() => makeQuery({ getResult: null })); const app = await createTestApp(); const env = createTestEnv(); diff --git a/apps/api/src/routes/users.ts b/apps/api/src/routes/users.ts index bd8c6ea..817a621 100644 --- a/apps/api/src/routes/users.ts +++ b/apps/api/src/routes/users.ts @@ -76,17 +76,28 @@ const MAX_CACHE_SIZE = 1000; function getCachedResults(key: string): any[] | null { const cached = searchCache.get(key); - if (cached && Date.now() - cached.timestamp < CACHE_TTL_MS) { - return cached.data; + if (cached) { + if (Date.now() - cached.timestamp < CACHE_TTL_MS) { + // Re-insert to move to the end (MRU) + searchCache.delete(key); + searchCache.set(key, cached); + return cached.data; + } + + searchCache.delete(key); } - searchCache.delete(key); return null; } -function setCachedResults(key: string, data: any[]) { +function setCachedResults(key: string, data: any[], maxSize?: number) { + const effectiveMaxSize = + typeof maxSize === "number" && Number.isFinite(maxSize) && maxSize > 0 + ? maxSize + : MAX_CACHE_SIZE; + if (searchCache.has(key)) { searchCache.delete(key); - } else if (searchCache.size >= MAX_CACHE_SIZE) { + } else if (searchCache.size >= effectiveMaxSize) { const oldestKey = searchCache.keys().next().value; if (oldestKey !== undefined) { searchCache.delete(oldestKey); @@ -131,7 +142,7 @@ usersRoute.get("/search", authMiddleware, async (c) => { .limit(10) .all(); - setCachedResults(cacheKey, results); + setCachedResults(cacheKey, results, c.env.MAX_CACHE_SIZE ? Number(c.env.MAX_CACHE_SIZE) : undefined); return c.json({ users: results }); }); diff --git a/apps/api/src/types.ts b/apps/api/src/types.ts index cb90cd3..d2da5dd 100644 --- a/apps/api/src/types.ts +++ b/apps/api/src/types.ts @@ -10,6 +10,7 @@ export type Env = { ALLOWED_ORIGINS?: string; ENABLE_TEST_AUTH?: string; TEST_AUTH_SECRET?: string; + MAX_CACHE_SIZE?: string; }; export type JwtPayload = { diff --git a/apps/web/src/app/papers/[id]/paper-detail-client.tsx b/apps/web/src/app/papers/[id]/paper-detail-client.tsx index d7a64d0..bf3246c 100644 --- a/apps/web/src/app/papers/[id]/paper-detail-client.tsx +++ b/apps/web/src/app/papers/[id]/paper-detail-client.tsx @@ -86,6 +86,8 @@ type PaperStats = { const isAbsoluteUrl = (url: string) => /^https?:\/\//i.test(url); +const STATS_FETCH_ERROR_MESSAGE = "統計情報の取得に失敗しました"; + const isValidExternalUrl = (urlStr: string) => { try { const url = new URL(urlStr); @@ -107,6 +109,7 @@ function formatStatsDateLabel(date: string) { export default function PaperDetailClient({ paperId }: PaperDetailClientProps) { const { user } = useAuth(); const trackedViewPaperIdRef = useRef(null); + const mountedRef = useRef(true); const [paper, setPaper] = useState(null); const [files, setFiles] = useState([]); @@ -137,18 +140,53 @@ export default function PaperDetailClient({ paperId }: PaperDetailClientProps) { const isAuthor = authors.some((a) => a.userId === user?.id); - const fetchStats = useCallback(async () => { - if (!paperId || !isAuthor) return; - try { - const res = await apiFetch(`/api/papers/${safePath(paperId)}/stats`); - if (res.ok) { + const fetchStats = useCallback( + async (options?: { withLoading?: boolean; isCancelled?: () => boolean }) => { + if (!paperId || !isAuthor) return; + + const canUpdateState = + typeof options?.isCancelled === "function" + ? () => mountedRef.current && !options.isCancelled?.() + : () => mountedRef.current; + + if (options?.withLoading ?? true) { + if (canUpdateState()) setStatsLoading(true); + } + if (canUpdateState()) setStatsError(""); + + try { + const res = await apiFetch(`/api/papers/${safePath(paperId)}/stats`); + + if (!res.ok) { + if (!canUpdateState()) return; + setStats(null); + if (res.status === 401) { + setStatsError("統計情報を取得するにはログインが必要です"); + } else if (res.status === 403) { + setStatsError("統計情報を閲覧する権限がありません"); + } else if (res.status === 404) { + setStatsError("統計情報が見つかりません"); + } else { + setStatsError(STATS_FETCH_ERROR_MESSAGE); + } + return; + } + const data = (await res.json()) as PaperStats; + if (!canUpdateState()) return; + setStats(data); + setStatsError(""); + } catch { + if (!canUpdateState()) return; + setStats(null); + setStatsError(STATS_FETCH_ERROR_MESSAGE); + } finally { + if (canUpdateState()) setStatsLoading(false); } - } catch { - // Ignore - } - }, [paperId, isAuthor]); + }, + [paperId, isAuthor], + ); const applyCountedView = useCallback(() => { setPaper((current) => { @@ -160,7 +198,7 @@ export default function PaperDetailClient({ paperId }: PaperDetailClientProps) { }); // Refresh full stats after a recorded view to ensure consistency - void fetchStats(); + void fetchStats({ withLoading: false }); }, [fetchStats]); const fetchPaper = useCallback(async () => { @@ -212,6 +250,12 @@ export default function PaperDetailClient({ paperId }: PaperDetailClientProps) { if (isUploader) fetchInvites(); }, [isUploader, fetchInvites]); + useEffect(() => { + return () => { + mountedRef.current = false; + }; + }, []); + useEffect(() => { if (!paper?.id || trackedViewPaperIdRef.current === paper.id) return; @@ -250,14 +294,11 @@ export default function PaperDetailClient({ paperId }: PaperDetailClientProps) { return; } - setStatsLoading(true); - setStatsError(""); - - fetchStats(); - // Also set loading false in fetchStats normally, but here we use useEffect style - setStatsLoading(false); + let cancelled = false; + void fetchStats({ isCancelled: () => cancelled }); return () => { + cancelled = true; }; }, [paper?.id, isAuthor, fetchStats]);