From 9293b93d75dfe8da782f30e5fbbf4e171c5faaaf Mon Sep 17 00:00:00 2001 From: marius Date: Thu, 29 Oct 2026 14:12:50 +0100 Subject: [PATCH 1/2] feat(frontend): cache administration page (#250) Add Administration -> Cache for site administrators, wired to the existing /api/cache endpoints the SPA never called: entry counts and size, hit rate and access time, recommendations with their priority, and the refresh, clean up, reset counters and clear actions. Clear all and Reset counters ask for confirmation; clean up only drops expired entries and does not. The page is built from /api/cache/stats and /recommendations only. The performance and health endpoints are left unused because hit_rate_percentage is a 0..1 fraction, memory_usage_mb is on the wrong scale and health.memory_usage is the entry count; this is documented in the API contract. Co-Authored-By: Claude Sonnet 5.5 --- docs/developer/http-api-contract.md | 14 + frontend/src/App.tsx | 2 + frontend/src/api/__tests__/cache.test.ts | 79 +++++ frontend/src/api/cache.ts | 64 ++++ frontend/src/api/client.ts | 1 + frontend/src/pages/CachePage.tsx | 290 ++++++++++++++++++ .../src/pages/__tests__/CachePage.test.tsx | 178 +++++++++++ frontend/src/pages/admin/AdminLayout.tsx | 1 + .../admin/__tests__/AdminLayout.test.tsx | 3 +- 9 files changed, 631 insertions(+), 1 deletion(-) create mode 100644 frontend/src/api/__tests__/cache.test.ts create mode 100644 frontend/src/api/cache.ts create mode 100644 frontend/src/pages/CachePage.tsx create mode 100644 frontend/src/pages/__tests__/CachePage.test.tsx diff --git a/docs/developer/http-api-contract.md b/docs/developer/http-api-contract.md index 79cdc045..6aa8f63c 100644 --- a/docs/developer/http-api-contract.md +++ b/docs/developer/http-api-contract.md @@ -174,6 +174,20 @@ Self-hosted only; requires a **global administrator**. | `GET` | `/api/admin/backup/info` | `{ attachments_available, attachment_files, attachment_bytes }`: what a backup with files would contain (`attachments_available: false` when attachment storage is not configured). **403** for non-admins, **410** like below. | | `POST` | `/api/admin/backup` | No body; needs the CSRF header. Query `attachments` (default `true`). Runs `pg_dump --no-owner --no-privileges` against `DATABASE_URL` and returns **200** with `Content-Type: application/gzip` and `Content-Disposition: attachment; filename="marreq-backup__.tar.gz"`: a tar.gz with `marreq-backup_<…>/database.sql` (plain SQL), `attachments/` (the attachment store, unless `attachments=false` or storage is not configured) and `manifest.json` (`format: "marreq.backup.v1"`, `includes_attachments`, `attachment_files`, `attachment_bytes`). Before issue #341 this was a `.sql.gz`. **403** for non-admins; **410** when the deployment mode disallows backups (`allows_database_backup: false` in `GET /api/meta/deployment`, i.e. cloud); **500** with the `pg_dump` error summary if the dump fails. Success and failure are recorded as `EXPORT` audit rows. The `pg_dump` binary can be overridden with `MARREQ_PG_DUMP`. The Compose nginx allows up to 15 minutes for this route. | +## Admin application cache + +Instance-wide (the in-memory cache of the running process); served by both `marreq-server` and `marreq-cloud`. The SPA page is Administration → Cache. Every route except `health` requires a **global administrator**; other authenticated users receive **403**. The `POST` routes take no body and need the CSRF header. + +| Method | Path | Description | +|--------|------|-------------| +| `GET` | `/api/cache/stats` | `{ total_entries, active_entries, expired_entries, hits, misses, total_requests, hit_rate, miss_rate, average_access_time_ns, total_access_time_ns, last_cleanup, cache_size_bytes }`. `hit_rate` and `miss_rate` are fractions in 0..1. This is the source the SPA uses. | +| `GET` | `/api/cache/recommendations` | `{ recommendations: string[], priority: "high" \| "medium" \| "low", analysis_timestamp }`. | +| `POST` | `/api/cache/clear` | Drops every entry. Response `{ message, timestamp }`. | +| `POST` | `/api/cache/cleanup` | Drops only the expired entries. Response `{ message, cleaned_entries, timestamp }`. | +| `POST` | `/api/cache/reset-counters` | Zeroes the hit/miss counters; entries are kept. Response `{ message, timestamp }`. | +| `GET` | `/api/cache/performance` | Derived metrics (JSON object). Not used by the SPA: `hit_rate_percentage` is the 0..1 fraction rounded to two decimals, not a percentage, and `memory_usage_mb` is not in megabytes. Use `stats` instead. | +| `GET` | `/api/cache/health` | **Public.** `{ status, total_entries, active_entries, expired_entries, memory_usage, cleanup_needed, last_cleanup }`. `memory_usage` is the entry count, and `last_cleanup` is the time of the request. | + ## User management Instance-wide. All routes require a **global administrator** (`users.is_admin`); other authenticated users receive **403**. Mutating routes need the CSRF header like any other SPA write. diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index f19e0cf2..86e088df 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -23,6 +23,7 @@ import AdminPage from '@/pages/AdminPage'; import SystemLogsPage from '@/pages/SystemLogsPage'; import LogAnalyticsPage from '@/pages/LogAnalyticsPage'; import BackupPage from '@/pages/BackupPage'; +import CachePage from '@/pages/CachePage'; import CreateRequirementPage from '@/pages/CreateRequirementPage'; import CreateVerificationPage from '@/pages/CreateVerificationPage'; import DashboardPage from '@/pages/DashboardPage'; @@ -134,6 +135,7 @@ export default function App() { } /> } /> } /> + } /> {/* Project workspace: /:projectSlug/... */} }> diff --git a/frontend/src/api/__tests__/cache.test.ts b/frontend/src/api/__tests__/cache.test.ts new file mode 100644 index 00000000..9787ca37 --- /dev/null +++ b/frontend/src/api/__tests__/cache.test.ts @@ -0,0 +1,79 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { CSRF, itSendsEachRequest, stubFetch } from '@/test/apiHarness'; +import { ApiError } from '../transport'; +import { + clearCache, + cleanupCache, + getCacheRecommendations, + getCacheStats, + resetCacheCounters, +} from '../cache'; + +const stats = { + total_entries: 10, + active_entries: 8, + expired_entries: 2, + hits: 30, + misses: 10, + total_requests: 40, + hit_rate: 0.75, + miss_rate: 0.25, + average_access_time_ns: 1200, + total_access_time_ns: 48000, + last_cleanup: '2026-10-29T08:00:00Z', + cache_size_bytes: 4096, +}; + +describe('cache API', () => { + afterEach(() => vi.unstubAllGlobals()); + + itSendsEachRequest([ + { + name: 'getCacheStats', + call: () => getCacheStats(), + method: 'GET', + url: '/api/cache/stats', + response: { json: stats }, + returns: stats, + }, + { + name: 'getCacheRecommendations', + call: () => getCacheRecommendations(), + method: 'GET', + url: '/api/cache/recommendations', + response: { + json: { recommendations: ['Run cleanup'], priority: 'low', analysis_timestamp: '2026-10-29T08:00:00Z' }, + }, + returns: { recommendations: ['Run cleanup'], priority: 'low', analysis_timestamp: '2026-10-29T08:00:00Z' }, + }, + { + name: 'clearCache', + call: () => clearCache(CSRF), + method: 'POST', + url: '/api/cache/clear', + response: { json: { message: 'Cache cleared successfully', timestamp: 't' } }, + returns: { message: 'Cache cleared successfully', timestamp: 't' }, + }, + { + name: 'cleanupCache returns the number of removed entries', + call: () => cleanupCache(CSRF), + method: 'POST', + url: '/api/cache/cleanup', + response: { json: { message: 'done', cleaned_entries: 2, timestamp: 't' } }, + returns: { message: 'done', cleaned_entries: 2, timestamp: 't' }, + }, + { + name: 'resetCacheCounters', + call: () => resetCacheCounters(CSRF), + method: 'POST', + url: '/api/cache/reset-counters', + response: { json: { message: 'reset', timestamp: 't' } }, + returns: { message: 'reset', timestamp: 't' }, + }, + ]); + + it('rejects with an ApiError for non-administrators', async () => { + stubFetch({ status: 403, json: { message: 'forbidden' } }); + await expect(getCacheStats()).rejects.toBeInstanceOf(ApiError); + }); +}); diff --git a/frontend/src/api/cache.ts b/frontend/src/api/cache.ts new file mode 100644 index 00000000..ce4a68e1 --- /dev/null +++ b/frontend/src/api/cache.ts @@ -0,0 +1,64 @@ +import { fetchJson } from './transport'; + +/** `GET /api/cache/stats` (site administrators only). Rates are fractions in 0..1. */ +export type CacheStats = { + total_entries: number; + active_entries: number; + expired_entries: number; + hits: number; + misses: number; + total_requests: number; + hit_rate: number; + miss_rate: number; + average_access_time_ns: number; + total_access_time_ns: number; + /** RFC 3339 timestamp. */ + last_cleanup: string; + cache_size_bytes: number; +}; + +export type CacheRecommendationPriority = 'high' | 'medium' | 'low'; + +export type CacheRecommendations = { + recommendations: string[]; + priority: CacheRecommendationPriority; + /** RFC 3339 timestamp. */ + analysis_timestamp: string; +}; + +/** Answer of the write endpoints; `cleaned_entries` only comes with a cleanup. */ +export type CacheActionResult = { + message: string; + timestamp: string; + cleaned_entries?: number; +}; + +export async function getCacheStats(): Promise { + return fetchJson('/api/cache/stats'); +} + +export async function getCacheRecommendations(): Promise { + return fetchJson('/api/cache/recommendations'); +} + +function cachePost(path: string, csrfToken: string): Promise { + return fetchJson(path, { + method: 'POST', + headers: { 'X-CSRF-Token': csrfToken }, + }); +} + +/** Drops every cached entry; the next requests rebuild it from the database. */ +export function clearCache(csrfToken: string): Promise { + return cachePost('/api/cache/clear', csrfToken); +} + +/** Drops only the expired entries. */ +export function cleanupCache(csrfToken: string): Promise { + return cachePost('/api/cache/cleanup', csrfToken); +} + +/** Zeroes the hit/miss counters without touching the cached entries. */ +export function resetCacheCounters(csrfToken: string): Promise { + return cachePost('/api/cache/reset-counters', csrfToken); +} diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index cbc959cd..3f20603f 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -17,6 +17,7 @@ export * from './meta'; export * from './imports'; export * from './exports'; export * from './logs'; +export * from './cache'; export * from './attachments'; export * from './verificationControl'; export * from './reports'; diff --git a/frontend/src/pages/CachePage.tsx b/frontend/src/pages/CachePage.tsx new file mode 100644 index 00000000..394ebdb2 --- /dev/null +++ b/frontend/src/pages/CachePage.tsx @@ -0,0 +1,290 @@ +import { useCallback, useEffect, useState } from 'react'; +import { + type CacheActionResult, + type CacheRecommendationPriority, + type CacheRecommendations, + type CacheStats, + cleanupCache, + clearCache, + getCacheRecommendations, + getCacheStats, + getCsrfToken, + listUsersOptional, + resetCacheCounters, +} from '@/api/client'; +import StitchPageHeader from '@/components/StitchPageHeader'; +import { useDashboard } from '@/context/DashboardContext'; +import { btnDanger, btnPrimary } from '@/pages/catalog/catalogUi'; +import { ADMIN_BREADCRUMB } from '@/pages/admin/adminArea'; +import { formatBytes } from '@/utils/formatBytes'; +import { parseUser } from '@/utils/parseUser'; + +const CLEAR_CONFIRM = + 'Clear all cache entries? This will temporarily slow down the application until the cache is rebuilt.'; +const RESET_CONFIRM = 'Reset the cache hit/miss counters? The cached entries are kept.'; + +const PRIORITY_BADGE: Record = { + high: 'border-red-500/30 bg-red-500/10 text-red-800 dark:text-red-100', + medium: 'border-amber-500/30 bg-amber-500/10 text-amber-900 dark:text-amber-100', + low: 'border-emerald-500/30 bg-emerald-500/10 text-emerald-900 dark:text-emerald-100', +}; + +const cardClass = 'rounded-xl border border-stitch-border bg-stitch-surface p-5 shadow-stitch space-y-3'; +const cardTitleClass = 'text-sm font-bold text-stitch-accent uppercase tracking-wide'; +const btnSecondary = + 'inline-flex items-center justify-center rounded-md border border-stitch-border px-4 py-2 text-xs font-bold uppercase tracking-wider text-stitch-accent hover:bg-stitch-higher disabled:opacity-50'; + +/** The API reports the average access time in nanoseconds. */ +function formatDuration(ns: number): string { + if (!Number.isFinite(ns) || ns < 0) return '—'; + if (ns < 1_000) return `${Math.round(ns)} ns`; + if (ns < 1_000_000) return `${(ns / 1_000).toFixed(1)} µs`; + return `${(ns / 1_000_000).toFixed(2)} ms`; +} + +/** `hit_rate` is a fraction in 0..1, not a percentage. */ +function formatRate(rate: number): string { + return Number.isFinite(rate) ? `${(rate * 100).toFixed(1)}%` : '—'; +} + +function formatTimestamp(value: string): string { + const date = new Date(value); + return Number.isNaN(date.getTime()) ? '—' : date.toLocaleString(); +} + +function Metric({ label, value }: { label: string; value: string | number }) { + return ( +
+
{label}
+
{value}
+
+ ); +} + +export default function CachePage() { + const { dashboard, csrfToken } = useDashboard(); + const me = parseUser(dashboard?.user); + + const [allowed, setAllowed] = useState(null); + const [stats, setStats] = useState(null); + const [advice, setAdvice] = useState(null); + const [busy, setBusy] = useState(false); + const [error, setError] = useState(null); + const [notice, setNotice] = useState(null); + + const load = useCallback(async () => { + const [nextStats, nextAdvice] = await Promise.all([ + getCacheStats(), + // The recommendations are advice only; the page still works without them. + getCacheRecommendations().catch(() => null), + ]); + setStats(nextStats); + setAdvice(nextAdvice); + }, []); + + useEffect(() => { + let alive = true; + // `/api/users` is admin-only, so it doubles as the access check (same as Backup). + listUsersOptional().then((users) => { + if (!alive) return; + setAllowed(users !== null); + if (users === null) return; + load().catch((e) => { + if (alive) setError(e instanceof Error ? e.message : 'Could not load the cache statistics'); + }); + }); + return () => { + alive = false; + }; + }, [load]); + + async function run( + action: (csrf: string) => Promise, + describe: (result: CacheActionResult) => string, + ) { + setBusy(true); + setError(null); + setNotice(null); + try { + const result = await action(csrfToken ?? (await getCsrfToken())); + setNotice(describe(result)); + await load(); + } catch (e) { + setError(e instanceof Error ? e.message : 'Cache action failed'); + } finally { + setBusy(false); + } + } + + function onRefresh() { + setBusy(true); + setError(null); + setNotice(null); + load() + .catch((e) => setError(e instanceof Error ? e.message : 'Could not load the cache statistics')) + .finally(() => setBusy(false)); + } + + function onCleanup() { + void run(cleanupCache, (r) => { + const n = r.cleaned_entries ?? 0; + return `Removed ${n} expired ${n === 1 ? 'entry' : 'entries'}.`; + }); + } + + function onClear() { + if (!window.confirm(CLEAR_CONFIRM)) return; + void run(clearCache, () => 'Cache cleared.'); + } + + function onResetCounters() { + if (!window.confirm(RESET_CONFIRM)) return; + void run(resetCacheCounters, () => 'Cache counters reset.'); + } + + if (allowed === null) { + return ( +
+ Loading… +
+ ); + } + + if (!allowed) { + return ( +
+ +
+ + lock + +

Access denied

+

+ Managing the application cache requires a global administrator account. You are signed + in as {me?.username ?? '?'}. +

+
+
+ ); + } + + return ( +
+ + + {error ? ( +
+ {error} +
+ ) : null} + {notice ? ( +
+ {notice} +
+ ) : null} + +
+
+

Entries

+ {stats ? ( +
+ + + + +
+ ) : ( +

No statistics available.

+ )} +
+ +
+

Performance

+ {stats ? ( + <> +
+ + + + + +
+

+ Last cleanup: {formatTimestamp(stats.last_cleanup)} +

+ + ) : ( +

No statistics available.

+ )} +
+ +
+
+

Recommendations

+ {advice ? ( + + {advice.priority} + + ) : null} +
+ {advice && advice.recommendations.length > 0 ? ( +
    + {advice.recommendations.map((text) => ( +
  • {text}
    • + ))} +
+ ) : ( +

No recommendations.

+ )} + {advice ? ( +

+ Analysis performed at {formatTimestamp(advice.analysis_timestamp)} +

+ ) : null} +
+ +
+

Actions

+

+ Clean up removes only expired entries. Clear all drops every entry, so the next + requests rebuild the cache from the database. Resetting the counters keeps the + entries. +

+
+ + + + +
+
+
+
+ ); +} diff --git a/frontend/src/pages/__tests__/CachePage.test.tsx b/frontend/src/pages/__tests__/CachePage.test.tsx new file mode 100644 index 00000000..4153869e --- /dev/null +++ b/frontend/src/pages/__tests__/CachePage.test.tsx @@ -0,0 +1,178 @@ +import { render, screen, waitFor, within } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { MemoryRouter, Route, Routes } from 'react-router-dom'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import CachePage from '../CachePage'; + +const mocks = vi.hoisted(() => ({ + listUsersOptional: vi.fn(), + getCsrfToken: vi.fn(), + getCacheStats: vi.fn(), + getCacheRecommendations: vi.fn(), + clearCache: vi.fn(), + cleanupCache: vi.fn(), + resetCacheCounters: vi.fn(), +})); + +vi.mock('@/api/client', () => mocks); + +vi.mock('@/context/DashboardContext', () => ({ + useDashboard: () => ({ + csrfToken: 'csrf-token', + dashboard: { user: { id: 1, username: 'alice', name: 'Alice', is_admin: true } }, + }), +})); + +const stats = { + total_entries: 10, + active_entries: 8, + expired_entries: 2, + hits: 30, + misses: 10, + total_requests: 40, + // A fraction, not a percentage: the page must show 75.0%, not 0.8%. + hit_rate: 0.75, + miss_rate: 0.25, + average_access_time_ns: 1200, + total_access_time_ns: 48000, + last_cleanup: '2026-10-29T08:00:00Z', + cache_size_bytes: 4096, +}; + +const advice = { + recommendations: ['Run a cache cleanup to drop expired entries'], + priority: 'medium', + analysis_timestamp: '2026-10-29T08:00:05Z', +}; + +function renderPage() { + return render( + + + } /> + + , + ); +} + +describe('CachePage', () => { + let confirm: ReturnType; + + beforeEach(() => { + vi.clearAllMocks(); + confirm = vi.spyOn(window, 'confirm').mockReturnValue(true); + mocks.listUsersOptional.mockResolvedValue([{ id: 1, username: 'alice' }]); + mocks.getCsrfToken.mockResolvedValue('fetched-csrf'); + mocks.getCacheStats.mockResolvedValue(stats); + mocks.getCacheRecommendations.mockResolvedValue(advice); + mocks.clearCache.mockResolvedValue({ message: 'Cache cleared successfully', timestamp: 't' }); + mocks.cleanupCache.mockResolvedValue({ message: 'done', cleaned_entries: 2, timestamp: 't' }); + mocks.resetCacheCounters.mockResolvedValue({ message: 'reset', timestamp: 't' }); + }); + + afterEach(() => confirm.mockRestore()); + + it('shows access denied for non-admins without calling the cache API', async () => { + mocks.listUsersOptional.mockResolvedValue(null); + renderPage(); + expect(await screen.findByText('Access denied')).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Clear all' })).not.toBeInTheDocument(); + expect(mocks.getCacheStats).not.toHaveBeenCalled(); + expect(mocks.getCacheRecommendations).not.toHaveBeenCalled(); + }); + + it('shows the entries, the performance figures and the recommendations', async () => { + renderPage(); + const entries = await screen.findByTestId('cache-entries'); + expect(within(entries).getByText('Total').nextSibling).toHaveTextContent('10'); + expect(within(entries).getByText('Expired').nextSibling).toHaveTextContent('2'); + expect(within(entries).getByText('Size').nextSibling).toHaveTextContent('4 KB'); + + const performance = screen.getByTestId('cache-performance'); + expect(within(performance).getByText('Hit rate').nextSibling).toHaveTextContent('75.0%'); + expect(within(performance).getByText('Requests').nextSibling).toHaveTextContent('40'); + expect(within(performance).getByText('Avg access time').nextSibling).toHaveTextContent('1.2 µs'); + + expect(screen.getByText('Run a cache cleanup to drop expired entries')).toBeInTheDocument(); + expect(screen.getByTestId('cache-priority')).toHaveTextContent('medium'); + }); + + it('still shows the statistics when the recommendations cannot be loaded', async () => { + mocks.getCacheRecommendations.mockRejectedValue(new Error('boom')); + renderPage(); + expect(await screen.findByTestId('cache-entries')).toBeInTheDocument(); + expect(screen.getByText('No recommendations.')).toBeInTheDocument(); + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + }); + + it('shows an error when the statistics cannot be loaded', async () => { + mocks.getCacheStats.mockRejectedValue(new Error('cache unavailable')); + renderPage(); + expect(await screen.findByRole('alert')).toHaveTextContent('cache unavailable'); + }); + + it('cleans up expired entries without asking and reports how many were removed', async () => { + renderPage(); + const user = userEvent.setup(); + await user.click(await screen.findByRole('button', { name: 'Clean up expired' })); + + expect(mocks.cleanupCache).toHaveBeenCalledWith('csrf-token'); + expect(confirm).not.toHaveBeenCalled(); + expect(await screen.findByRole('status')).toHaveTextContent('Removed 2 expired entries.'); + expect(mocks.getCacheStats).toHaveBeenCalledTimes(2); + }); + + it('does not clear the cache when the confirmation is declined', async () => { + confirm.mockReturnValue(false); + renderPage(); + const user = userEvent.setup(); + await user.click(await screen.findByRole('button', { name: 'Clear all' })); + + expect(confirm).toHaveBeenCalledWith(expect.stringContaining('temporarily slow down')); + expect(mocks.clearCache).not.toHaveBeenCalled(); + expect(mocks.getCacheStats).toHaveBeenCalledTimes(1); + }); + + it('clears the cache after confirmation and reloads the statistics', async () => { + renderPage(); + const user = userEvent.setup(); + await user.click(await screen.findByRole('button', { name: 'Clear all' })); + + expect(mocks.clearCache).toHaveBeenCalledWith('csrf-token'); + expect(await screen.findByRole('status')).toHaveTextContent('Cache cleared.'); + await waitFor(() => expect(mocks.getCacheStats).toHaveBeenCalledTimes(2)); + }); + + it('resets the counters only after confirmation', async () => { + confirm.mockReturnValueOnce(false); + renderPage(); + const user = userEvent.setup(); + const button = await screen.findByRole('button', { name: 'Reset counters' }); + + await user.click(button); + expect(mocks.resetCacheCounters).not.toHaveBeenCalled(); + + await user.click(button); + expect(mocks.resetCacheCounters).toHaveBeenCalledWith('csrf-token'); + expect(await screen.findByRole('status')).toHaveTextContent('Cache counters reset.'); + }); + + it('shows the backend error when an action fails and keeps the statistics', async () => { + mocks.clearCache.mockRejectedValue(new Error('forbidden')); + renderPage(); + const user = userEvent.setup(); + await user.click(await screen.findByRole('button', { name: 'Clear all' })); + + expect(await screen.findByRole('alert')).toHaveTextContent('forbidden'); + expect(screen.queryByRole('status')).not.toBeInTheDocument(); + expect(screen.getByTestId('cache-entries')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Clear all' })).toBeEnabled(); + }); + + it('refreshes the statistics on demand', async () => { + renderPage(); + const user = userEvent.setup(); + await user.click(await screen.findByRole('button', { name: 'Refresh' })); + await waitFor(() => expect(mocks.getCacheStats).toHaveBeenCalledTimes(2)); + }); +}); diff --git a/frontend/src/pages/admin/AdminLayout.tsx b/frontend/src/pages/admin/AdminLayout.tsx index 10753921..c1fec0eb 100644 --- a/frontend/src/pages/admin/AdminLayout.tsx +++ b/frontend/src/pages/admin/AdminLayout.tsx @@ -47,6 +47,7 @@ export default function AdminLayout() { { to: `${ADMIN_BASE}/logs`, label: 'System logs', icon: 'history', end: true }, { to: `${ADMIN_BASE}/logs/analytics`, label: 'Log analytics', icon: 'monitoring' }, { to: `${ADMIN_BASE}/backup`, label: 'Backup', icon: 'backup', hidden: !backupAllowed }, + { to: `${ADMIN_BASE}/cache`, label: 'Cache', icon: 'cached' }, ]; return ( diff --git a/frontend/src/pages/admin/__tests__/AdminLayout.test.tsx b/frontend/src/pages/admin/__tests__/AdminLayout.test.tsx index f27cf1ef..1608ef6a 100644 --- a/frontend/src/pages/admin/__tests__/AdminLayout.test.tsx +++ b/frontend/src/pages/admin/__tests__/AdminLayout.test.tsx @@ -21,6 +21,7 @@ function renderAt(entry: string | { pathname: string; state?: unknown }) { logs page

} /> analytics page

} /> backup page

} /> + cache page

} /> } /> @@ -42,7 +43,7 @@ describe('AdminLayout', () => { renderAt('/admin/logs'); expect(screen.getByText('logs page')).toBeInTheDocument(); const nav = screen.getByRole('navigation', { name: 'Administration sections' }); - await waitFor(() => expect(nav.querySelectorAll('a')).toHaveLength(4)); + await waitFor(() => expect(nav.querySelectorAll('a')).toHaveLength(5)); expect(screen.getByRole('link', { name: /system logs/i })).toHaveAttribute('aria-current', 'page'); expect(screen.getByRole('link', { name: 'Users' })).not.toHaveAttribute('aria-current'); }); From b2dfa4498b5826a08901c9589b842fa705e60fb6 Mon Sep 17 00:00:00 2001 From: marius Date: Thu, 29 Oct 2026 14:35:55 +0100 Subject: [PATCH 2/2] fix(cache): derive entry counts from the cache map Cache::stats() read active/expired counts from two AtomicU64 counters that drifted and underflowed: - cleanup() ran expired_entries.fetch_sub(n), but time-expired entries were never counted there, so the counter wrapped to u64::MAX. The background maintenance thread calls cleanup() every TTL, so this happened on its own. - get() on an expired entry subtracted from active_entries unconditionally, and reset_counters() zeroed both counters while the entries stayed, so the next expired read wrapped active_entries as well. - total_entries = active + expired then overflowed (a panic in debug builds, a silently wrong sum in release). Count active and expired entries from the map in stats(), in the same pass that already sums the size, and drop the counters. reset_counters() now only zeroes hits, misses and access time, as the API documents, and the entries keep being counted. Regression tests cover the cleanup and expired-read underflows, expired entries being counted before a cleanup, and reset_counters keeping entries. test_cache_stats_branches built its health states by writing the counters; it now creates real expired entries. Co-Authored-By: Claude Sonnet 5.5 --- .../src/repository/cache/repository.rs | 74 +---------- marreq-core/src/repository/cache/stats.rs | 125 ++++++++++++++---- 2 files changed, 101 insertions(+), 98 deletions(-) diff --git a/marreq-core/src/repository/cache/repository.rs b/marreq-core/src/repository/cache/repository.rs index 14949bba..e2b2430e 100644 --- a/marreq-core/src/repository/cache/repository.rs +++ b/marreq-core/src/repository/cache/repository.rs @@ -23,8 +23,6 @@ pub struct Cache { pub(super) hits: Arc, pub(super) misses: Arc, pub(super) total_access_time: Arc, - pub(super) active_entries: Arc, - pub(super) expired_entries: Arc, pub(super) last_cleanup: Arc>>, } @@ -43,8 +41,6 @@ impl Cache { hits: Arc::new(AtomicU64::new(0)), misses: Arc::new(AtomicU64::new(0)), total_access_time: Arc::new(AtomicU64::new(0)), - active_entries: Arc::new(AtomicU64::new(0)), - expired_entries: Arc::new(AtomicU64::new(0)), last_cleanup: Arc::new(Mutex::new(chrono::Utc::now())), } } @@ -83,9 +79,6 @@ impl Cache { None } Status::Expired => { - // Update counters: move from active to expired before removal - self.active_entries.fetch_sub(1, Ordering::Relaxed); - self.expired_entries.fetch_add(1, Ordering::Relaxed); self.misses.fetch_add(1, Ordering::Relaxed); self.remove(key); None @@ -110,54 +103,25 @@ impl Cache { expires_at: Instant::now() + ttl, }; - let mut data = self.data.write().unwrap(); - // Check if this key already exists to update counters correctly - let existed = data.contains_key(key); - data.insert(key.to_string(), entry); - - // Update active entries counter - if !existed { - self.active_entries.fetch_add(1, Ordering::Relaxed); - } + self.data.write().unwrap().insert(key.to_string(), entry); } /// Remove a key from cache pub fn remove(&self, key: &str) { - let mut data = self.data.write().unwrap(); - if let Some(entry) = data.remove(key) { - let now = Instant::now(); - - // Always decrement active count first - self.active_entries - .try_update(Ordering::Relaxed, Ordering::Relaxed, |v| v.checked_sub(1)) - .ok(); - - // If expired, also decrement expired count - if entry.expires_at <= now { - self.expired_entries - .try_update(Ordering::Relaxed, Ordering::Relaxed, |v| v.checked_sub(1)) - .ok(); - } - } + self.data.write().unwrap().remove(key); } /// Clear all cache entries pub fn clear(&self) { - let mut data = self.data.write().unwrap(); - data.clear(); - - // Reset entry counters - self.active_entries.store(0, Ordering::Relaxed); - self.expired_entries.store(0, Ordering::Relaxed); + self.data.write().unwrap().clear(); } - /// Reset performance counters + /// Reset the hit/miss/access-time counters. The cached entries stay, and + /// so do the entry counts, which are derived from them. pub fn reset_counters(&self) { self.hits.store(0, Ordering::Relaxed); self.misses.store(0, Ordering::Relaxed); self.total_access_time.store(0, Ordering::Relaxed); - self.active_entries.store(0, Ordering::Relaxed); - self.expired_entries.store(0, Ordering::Relaxed); } /// Clean up expired entries and return count of cleaned entries @@ -166,22 +130,10 @@ impl Cache { let now = Instant::now(); let initial_count = data.len(); - // Count expired entries before removal - let expired_count = data - .values() - .filter(|entry| entry.expires_at <= now) - .count(); - data.retain(|_, entry| entry.expires_at > now); let cleaned_count = initial_count - data.len(); - // Update counters for cleaned entries - if expired_count > 0 { - self.expired_entries - .fetch_sub(expired_count as u64, Ordering::Relaxed); - } - // Update last cleanup time let mut last_cleanup = self.last_cleanup.lock().unwrap(); *last_cleanup = chrono::Utc::now(); @@ -189,22 +141,6 @@ impl Cache { cleaned_count } - /// Sync counters with actual data (useful for maintenance and debugging) - pub fn sync_counters(&self) { - let data = self.data.read().unwrap(); - let now = Instant::now(); - - let actual_active = data.values().filter(|entry| entry.expires_at > now).count() as u64; - let actual_expired = data - .values() - .filter(|entry| entry.expires_at <= now) - .count() as u64; - - self.active_entries.store(actual_active, Ordering::Relaxed); - self.expired_entries - .store(actual_expired, Ordering::Relaxed); - } - /// Invalidate all project-related cache entries pub fn invalidate_project(&self, project_id: i32) { self.remove(&keys::Requirements::by_project(project_id)); diff --git a/marreq-core/src/repository/cache/stats.rs b/marreq-core/src/repository/cache/stats.rs index 0d71e2bd..6a064bae 100644 --- a/marreq-core/src/repository/cache/stats.rs +++ b/marreq-core/src/repository/cache/stats.rs @@ -6,7 +6,7 @@ use super::repository::Cache; use chrono; use serde::{Deserialize, Serialize}; use serde_json::json; -use std::time::Duration; +use std::time::{Duration, Instant}; /// Cache statistics #[derive(Debug, Clone, Serialize, Deserialize)] @@ -28,7 +28,7 @@ pub struct CacheStats { impl Cache { /// Get cache statistics pub fn stats(&self) -> CacheStats { - // Use atomic counters for O(1) performance instead of O(n) calculations + // Hit/miss/access-time use atomic counters, so reading them is O(1). let hits = self.hits.load(std::sync::atomic::Ordering::Relaxed); let misses = self.misses.load(std::sync::atomic::Ordering::Relaxed); let total_requests = hits + misses; @@ -47,24 +47,29 @@ impl Cache { ) }; - let active_entries = self - .active_entries - .load(std::sync::atomic::Ordering::Relaxed) as usize; - let expired_entries = self - .expired_entries - .load(std::sync::atomic::Ordering::Relaxed) as usize; + // Count the entries from the map itself. Entries expire by the clock + // without anything touching them, so incrementally maintained counters + // drift (and used to underflow); one pass under the read lock cannot. + let (active_entries, expired_entries, cache_size_bytes) = { + let data = self.data.read().unwrap(); + let now = Instant::now(); + let mut active = 0usize; + let mut expired = 0usize; + let mut size = 0usize; + for (key, entry) in data.iter() { + if entry.expires_at > now { + active += 1; + } else { + expired += 1; + } + size += key.len() + entry.data.len(); + } + (active, expired, size) + }; let total_entries = active_entries + expired_entries; let last_cleanup = *self.last_cleanup.lock().unwrap(); - // Only calculate cache size when needed (this is still O(n) but less frequent) - let cache_size_bytes = if total_entries > 0 { - let data = self.data.read().unwrap(); - data.iter().map(|(k, e)| k.len() + e.data.len()).sum() - } else { - 0 - }; - CacheStats { total_entries, active_entries, @@ -245,19 +250,71 @@ mod tests { let stats = cache.stats(); assert_eq!(stats.hits, 0); assert_eq!(stats.misses, 0); - assert_eq!(stats.active_entries, 0); + assert_eq!(stats.active_entries, 1); + } + + /// Entries that expire by the clock are never touched by `get`, so the + /// entry counts have to reflect what is really in the cache. + #[test] + fn test_expired_entries_are_counted_before_cleanup() { + let cache = Cache::new(300); + cache.set_with_ttl("old1", "v".to_string(), Duration::from_millis(1)); + cache.set_with_ttl("old2", "v".to_string(), Duration::from_millis(1)); + cache.set("keep", "v".to_string()); + thread::sleep(Duration::from_millis(10)); + let stats = cache.stats(); + assert_eq!(stats.total_entries, 3); + assert_eq!(stats.active_entries, 1); + assert_eq!(stats.expired_entries, 2); } + /// Regression: `cleanup` used to subtract from an unsigned counter that + /// had never been incremented, so `expired_entries` wrapped to u64::MAX. #[test] - fn test_sync_counters_updates_expired() { + fn test_cleanup_does_not_underflow_the_entry_counts() { let cache = Cache::new(300); - cache.set_with_ttl("active", "v".to_string(), Duration::from_secs(1)); - cache.set_with_ttl("expired", "v".to_string(), Duration::from_millis(1)); + cache.set_with_ttl("old1", "v".to_string(), Duration::from_millis(1)); + cache.set_with_ttl("old2", "v".to_string(), Duration::from_millis(1)); + cache.set("keep", "v".to_string()); thread::sleep(Duration::from_millis(10)); - cache.sync_counters(); + assert_eq!(cache.cleanup(), 2); let stats = cache.stats(); + assert_eq!(stats.total_entries, 1); assert_eq!(stats.active_entries, 1); - assert_eq!(stats.expired_entries, 1); + assert_eq!(stats.expired_entries, 0); + } + + /// Regression: reading an expired entry after `reset_counters` used to + /// subtract from a zeroed counter and wrap `active_entries`. + #[test] + fn test_reading_an_expired_entry_does_not_underflow_the_entry_counts() { + let cache = Cache::new(300); + cache.set_with_ttl("old", "v".to_string(), Duration::from_millis(1)); + cache.reset_counters(); + thread::sleep(Duration::from_millis(10)); + assert_eq!(cache.get("old"), None); + let stats = cache.stats(); + assert_eq!(stats.total_entries, 0); + assert_eq!(stats.active_entries, 0); + assert_eq!(stats.expired_entries, 0); + assert_eq!(stats.misses, 1); + } + + /// `reset_counters` only zeroes the performance counters; the cached + /// entries (and therefore the entry counts) stay. + #[test] + fn test_reset_counters_keeps_the_entries() { + let cache = Cache::new(300); + cache.set("a", "1".to_string()); + assert_eq!(cache.get("a"), Some("1".to_string())); + cache.reset_counters(); + let stats = cache.stats(); + assert_eq!(stats.hits, 0); + assert_eq!(stats.misses, 0); + assert_eq!(stats.total_requests, 0); + assert_eq!(stats.active_entries, 1); + assert_eq!(stats.total_entries, 1); + assert_eq!(cache.get("a"), Some("1".to_string())); } #[test] @@ -267,14 +324,12 @@ mod tests { cache.set("keep", "v".to_string()); cache.set_with_ttl("gone", "v".to_string(), Duration::from_millis(1)); thread::sleep(Duration::from_millis(10)); - cache.sync_counters(); let stats = cache.get_stats(); assert_eq!(stats["total_entries"].as_u64(), Some(2)); assert_eq!(stats["active_entries"].as_u64(), Some(1)); assert_eq!(stats["expired_entries"].as_u64(), Some(1)); assert_eq!(stats["cleanup_available"].as_bool(), Some(true)); cache.cleanup(); - cache.sync_counters(); let stats = cache.get_stats(); assert_eq!(stats["total_entries"].as_u64(), Some(1)); assert_eq!(stats["active_entries"].as_u64(), Some(1)); @@ -314,12 +369,26 @@ mod tests { cache.hits.store(1, Ordering::Relaxed); cache.misses.store(2, Ordering::Relaxed); cache.total_access_time.store(6_000_000, Ordering::Relaxed); - cache.active_entries.store(10, Ordering::Relaxed); - cache.expired_entries.store(11, Ordering::Relaxed); + // `get_performance` caches its own snapshot, so count the active entries + // instead of assuming how many there are. + let active = cache.stats().active_entries; + let add_expired = |from: usize, to: usize| { + for i in from..to { + cache.set_with_ttl( + &format!("old{i}"), + "v".to_string(), + Duration::from_millis(1), + ); + } + thread::sleep(Duration::from_millis(10)); + }; + // expired > active -> degraded + add_expired(0, active + 1); let health_degraded = cache.get_health(); assert_eq!(health_degraded["status"].as_str(), Some("degraded")); assert!(health_degraded["cleanup_needed"].as_bool().unwrap()); - cache.expired_entries.store(25, Ordering::Relaxed); + // expired > 2 * active -> warning + add_expired(active + 1, active * 2 + 1); let health_warning = cache.get_health(); assert_eq!(health_warning["status"].as_str(), Some("warning")); let recs_bad = cache.get_recommendations(); @@ -354,8 +423,6 @@ mod tests { cache.reset_counters(); cache.hits.store(10, Ordering::Relaxed); cache.total_access_time.store(10, Ordering::Relaxed); - cache.active_entries.store(1, Ordering::Relaxed); - cache.expired_entries.store(0, Ordering::Relaxed); let recs_good = cache.get_recommendations(); let recs_good_arr = recs_good["recommendations"].as_array().unwrap(); assert_eq!(recs_good_arr.len(), 1);