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'); }); 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);