Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 18 additions & 10 deletions apps/api/src/routes/__tests__/users.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
23 changes: 17 additions & 6 deletions apps/api/src/routes/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 });
});
Expand Down
1 change: 1 addition & 0 deletions apps/api/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
73 changes: 57 additions & 16 deletions apps/web/src/app/papers/[id]/paper-detail-client.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -107,6 +109,7 @@ function formatStatsDateLabel(date: string) {
export default function PaperDetailClient({ paperId }: PaperDetailClientProps) {
const { user } = useAuth();
const trackedViewPaperIdRef = useRef<string | null>(null);
const mountedRef = useRef(true);

const [paper, setPaper] = useState<Paper | null>(null);
const [files, setFiles] = useState<PaperFile[]>([]);
Expand Down Expand Up @@ -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) => {
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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]);

Expand Down
Loading