diff --git a/src/app/(app)/repositories/_components/repo-settings-tab.test.tsx b/src/app/(app)/repositories/_components/repo-settings-tab.test.tsx index db12d98e..60a30d35 100644 --- a/src/app/(app)/repositories/_components/repo-settings-tab.test.tsx +++ b/src/app/(app)/repositories/_components/repo-settings-tab.test.tsx @@ -37,9 +37,13 @@ vi.mock("sonner", () => ({ // Mock repositories API const mockUpdate = vi.fn(); +const mockGetCacheTtl = vi.fn(); +const mockSetCacheTtl = vi.fn(); vi.mock("@/lib/api/repositories", () => ({ repositoriesApi: { update: (...args: unknown[]) => mockUpdate(...args), + getCacheTtl: (...args: unknown[]) => mockGetCacheTtl(...args), + setCacheTtl: (...args: unknown[]) => mockSetCacheTtl(...args), }, })); @@ -751,3 +755,295 @@ describe("RepoSettingsTab - Quota unit switching", () => { expect(screen.getByText("You have unsaved changes")).toBeTruthy(); }); }); + +// --------------------------------------------------------------------------- +// Proxy Cache section (#448) -- shown only for Remote (proxy) repos. Pin +// the visibility gate, the editing flow that plugs into the existing +// hasChanges / Save / Discard workflow, and the inline-validation that +// mirrors the backend's validate_cache_ttl range (1..=2_592_000). +// --------------------------------------------------------------------------- + +const remoteRepo: Repository = { + ...baseRepo, + id: "repo-2", + key: "pypi-remote", + name: "PyPI Remote", + repo_type: "remote", + upstream_url: "https://pypi.org", +}; + +describe("RepoSettingsTab - Proxy Cache section (#448)", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockListPolicies.mockResolvedValue([]); + // Default to a stable success response so the Proxy Cache section + // can render its initial value -- per-test overrides via + // .mockResolvedValueOnce / .mockRejectedValueOnce. + mockGetCacheTtl.mockResolvedValue({ + repository_key: "pypi-remote", + cache_ttl_seconds: 86400, + }); + }); + + it("hides the Proxy Cache section for Local repos", () => { + render(, { + wrapper: createWrapper(), + }); + expect(screen.queryByText("Proxy Cache")).toBeNull(); + expect(screen.queryByLabelText("Cache TTL (seconds)")).toBeNull(); + // No GET should be issued for non-Remote repos -- the useQuery is + // gated on `enabled: isRemote` to avoid wasted round-trips. + expect(mockGetCacheTtl).not.toHaveBeenCalled(); + }); + + it("hides the Proxy Cache section for Virtual / Staging repos", () => { + const virtualRepo: Repository = { ...baseRepo, repo_type: "virtual" }; + render(, { + wrapper: createWrapper(), + }); + expect(screen.queryByText("Proxy Cache")).toBeNull(); + expect(mockGetCacheTtl).not.toHaveBeenCalled(); + }); + + it("renders the Proxy Cache section with the TTL fetched from the backend on Remote repos", async () => { + render(, { + wrapper: createWrapper(), + }); + expect(screen.getByText("Proxy Cache")).toBeTruthy(); + await waitFor(() => { + expect(screen.getByLabelText("Cache TTL (seconds)")).toHaveProperty( + "value", + "86400" + ); + }); + expect(mockGetCacheTtl).toHaveBeenCalledWith("pypi-remote"); + // The human-readable hint should display alongside the input -- 86400s + // is the backend's default fallback (artifact-keeper#917) and the + // helper picks the largest unit that gives an integer magnitude, so + // "1 day" rather than "24 hours". + expect(screen.getByText(/1 day/)).toBeTruthy(); + }); + + it("triggers the unsaved-changes bar when the TTL is edited", async () => { + const user = userEvent.setup(); + render(, { + wrapper: createWrapper(), + }); + await waitFor(() => { + expect(screen.getByLabelText("Cache TTL (seconds)")).toHaveProperty( + "value", + "86400" + ); + }); + + const input = screen.getByLabelText("Cache TTL (seconds)"); + await user.clear(input); + await user.type(input, "3600"); + + expect(screen.getByText("You have unsaved changes")).toBeTruthy(); + }); + + it("calls setCacheTtl on save and shows a success toast", async () => { + mockSetCacheTtl.mockResolvedValue({ + repository_key: "pypi-remote", + cache_ttl_seconds: 3600, + }); + const { toast } = await import("sonner"); + + const user = userEvent.setup(); + render(, { + wrapper: createWrapper(), + }); + await waitFor(() => { + expect(screen.getByLabelText("Cache TTL (seconds)")).toHaveProperty( + "value", + "86400" + ); + }); + + const input = screen.getByLabelText("Cache TTL (seconds)"); + await user.clear(input); + await user.type(input, "3600"); + + await user.click(screen.getByRole("button", { name: /save changes/i })); + + await waitFor(() => { + expect(mockSetCacheTtl).toHaveBeenCalledWith("pypi-remote", 3600); + }); + expect(toast.success).toHaveBeenCalledWith("Cache TTL saved"); + // The general-fields update mutation must NOT fire when only the + // TTL changed -- the two endpoints are independent and we don't want + // to send an empty PATCH that the audit log would record as a no-op + // edit. + expect(mockUpdate).not.toHaveBeenCalled(); + }); + + it("shows the inline error and disables Save for out-of-range TTL", async () => { + const user = userEvent.setup(); + render(, { + wrapper: createWrapper(), + }); + await waitFor(() => { + expect(screen.getByLabelText("Cache TTL (seconds)")).toHaveProperty( + "value", + "86400" + ); + }); + + const input = screen.getByLabelText("Cache TTL (seconds)"); + await user.clear(input); + // Above the backend max of 2,592,000 (30 days). + await user.type(input, "9999999"); + + expect( + screen.getByText( + /Must be a whole number between 1 and 2,592,000/i + ) + ).toBeTruthy(); + expect(input).toHaveProperty("ariaInvalid", "true"); + const save = screen.getByRole("button", { name: /save changes/i }); + expect(save).toHaveProperty("disabled", true); + }); + + it("programmatically associates the validation error with the input (#448 a11y)", async () => { + const user = userEvent.setup(); + render(, { + wrapper: createWrapper(), + }); + await waitFor(() => { + expect(screen.getByLabelText("Cache TTL (seconds)")).toHaveProperty( + "value", + "86400" + ); + }); + + const input = screen.getByLabelText("Cache TTL (seconds)"); + await user.clear(input); + await user.type(input, "9999999"); + + // The error must be exposed as a live alert and linked from the input + // via aria-describedby so screen-reader users hear the explanation, not + // just "invalid". Mirrors the reference pattern in #459. + const error = screen.getByRole("alert"); + expect(error.id).toBe("settings-cache-ttl-error"); + expect(input.getAttribute("aria-describedby")).toBe( + "settings-cache-ttl-error" + ); + + // When the value becomes valid again, the association is removed. + await user.clear(input); + await user.type(input, "3600"); + expect(screen.queryByRole("alert")).toBeNull(); + expect(input.getAttribute("aria-describedby")).toBeNull(); + }); + + it("disables Discard while a cache-TTL save is in flight (#448 review)", async () => { + // A never-resolving setCacheTtl keeps the mutation pending so we can + // assert the Discard button is guarded the same way Save is, preventing + // a Discard from racing an in-flight save. + let resolveSet: (() => void) | undefined; + mockSetCacheTtl.mockReturnValue( + new Promise((resolve) => { + resolveSet = resolve; + }) + ); + + const user = userEvent.setup(); + render(, { + wrapper: createWrapper(), + }); + await waitFor(() => { + expect(screen.getByLabelText("Cache TTL (seconds)")).toHaveProperty( + "value", + "86400" + ); + }); + + const input = screen.getByLabelText("Cache TTL (seconds)"); + await user.clear(input); + await user.type(input, "3600"); + await user.click(screen.getByRole("button", { name: /save changes/i })); + + await waitFor(() => { + expect( + screen.getByRole("button", { name: /discard/i }) + ).toHaveProperty("disabled", true); + }); + + // Let the in-flight mutation settle so the test doesn't leak a pending promise. + resolveSet?.(); + }); + + it("zero is rejected as out-of-range (backend min is 1)", async () => { + const user = userEvent.setup(); + render(, { + wrapper: createWrapper(), + }); + await waitFor(() => { + expect(screen.getByLabelText("Cache TTL (seconds)")).toHaveProperty( + "value", + "86400" + ); + }); + + const input = screen.getByLabelText("Cache TTL (seconds)"); + await user.clear(input); + await user.type(input, "0"); + + expect( + screen.getByText( + /Must be a whole number between 1 and 2,592,000/i + ) + ).toBeTruthy(); + }); + + it("Discard reverts the TTL override back to the fetched value", async () => { + const user = userEvent.setup(); + render(, { + wrapper: createWrapper(), + }); + await waitFor(() => { + expect(screen.getByLabelText("Cache TTL (seconds)")).toHaveProperty( + "value", + "86400" + ); + }); + + const input = screen.getByLabelText("Cache TTL (seconds)"); + await user.clear(input); + await user.type(input, "3600"); + expect(screen.getByText("You have unsaved changes")).toBeTruthy(); + + await user.click(screen.getByRole("button", { name: /discard/i })); + + expect(screen.queryByText("You have unsaved changes")).toBeNull(); + expect(input).toHaveProperty("value", "86400"); + }); + + it("shows an error toast when setCacheTtl fails (e.g. 503 from a misconfigured proxy)", async () => { + mockSetCacheTtl.mockRejectedValue( + new Error("proxy service not configured") + ); + const { toast } = await import("sonner"); + + const user = userEvent.setup(); + render(, { + wrapper: createWrapper(), + }); + await waitFor(() => { + expect(screen.getByLabelText("Cache TTL (seconds)")).toHaveProperty( + "value", + "86400" + ); + }); + + const input = screen.getByLabelText("Cache TTL (seconds)"); + await user.clear(input); + await user.type(input, "3600"); + await user.click(screen.getByRole("button", { name: /save changes/i })); + + await waitFor(() => { + expect(toast.error).toHaveBeenCalledWith("Failed to save cache TTL"); + }); + }); +}); diff --git a/src/app/(app)/repositories/_components/repo-settings-tab.tsx b/src/app/(app)/repositories/_components/repo-settings-tab.tsx index 63d30518..1e13c9b0 100644 --- a/src/app/(app)/repositories/_components/repo-settings-tab.tsx +++ b/src/app/(app)/repositories/_components/repo-settings-tab.tsx @@ -48,6 +48,37 @@ import { type QuotaUnit = "MB" | "GB"; +// Backend constraints from `validate_cache_ttl` in repositories.rs: 1s..=30d. +// The constants live here (not on the SDK) so the UI can show a clear inline +// validation error before submitting; the backend would otherwise reject with +// a 400 + opaque message. +const CACHE_TTL_MIN_SECONDS = 1; +const CACHE_TTL_MAX_SECONDS = 30 * 24 * 60 * 60; // 2,592,000 + +/** + * Format a TTL in seconds as a short human-readable hint + * ("24 hours", "1 day 6 hours", "30 minutes"). Used as a helper line under + * the TTL input so operators don't have to compute "is 86400 a sensible + * number?" in their head. + */ +function formatTtlHint(seconds: number): string { + if (!Number.isFinite(seconds) || seconds <= 0) return ""; + const day = 24 * 60 * 60; + const hour = 60 * 60; + const minute = 60; + const days = Math.floor(seconds / day); + const hours = Math.floor((seconds % day) / hour); + const minutes = Math.floor((seconds % hour) / minute); + const secs = seconds % minute; + + const parts: string[] = []; + if (days) parts.push(`${days} day${days === 1 ? "" : "s"}`); + if (hours) parts.push(`${hours} hour${hours === 1 ? "" : "s"}`); + if (minutes) parts.push(`${minutes} minute${minutes === 1 ? "" : "s"}`); + if (secs && parts.length === 0) parts.push(`${secs} second${secs === 1 ? "" : "s"}`); + return parts.join(" "); +} + export interface UpdateRepositoryFields { key?: string; name?: string; @@ -105,6 +136,38 @@ export function RepoSettingsTab({ repository }: RepoSettingsTabProps) { const quotaValue = quotaOverrides.value ?? quotaDefaults.value; const quotaUnit = quotaOverrides.unit ?? quotaDefaults.unit; + // -- Proxy cache TTL state (#448) -- + // Only meaningful for Remote (proxy) repos; the section is hidden for + // Local / Virtual / Staging because writes against those types are + // rejected upstream with 400 (see is_cache_ttl_configurable). We still + // run the GET unconditionally if the section is visible so the read uses + // the same code path the backend tests pin (#917). + const isRemote = repository.repo_type === "remote"; + const { data: cacheTtlData, isLoading: cacheTtlLoading } = useQuery({ + queryKey: ["cache-ttl", repository.key], + queryFn: () => repositoriesApi.getCacheTtl(repository.key), + enabled: isRemote, + }); + const currentCacheTtlSeconds = cacheTtlData?.cache_ttl_seconds; + // String-typed override so the input stays controlled while the user is + // typing (e.g. mid-edit "8" before they finish "86400") without snapping + // to the parsed number on every keystroke. + const [cacheTtlOverride, setCacheTtlOverride] = useState(undefined); + const cacheTtlInputValue = + cacheTtlOverride ?? + (currentCacheTtlSeconds != null ? String(currentCacheTtlSeconds) : ""); + const parsedCacheTtl = + cacheTtlInputValue.trim() === "" ? null : Number(cacheTtlInputValue); + const cacheTtlIsValid = + parsedCacheTtl != null && + Number.isInteger(parsedCacheTtl) && + parsedCacheTtl >= CACHE_TTL_MIN_SECONDS && + parsedCacheTtl <= CACHE_TTL_MAX_SECONDS; + const cacheTtlChanged = + isRemote && + cacheTtlOverride !== undefined && + parsedCacheTtl !== currentCacheTtlSeconds; + // Detect whether the form has unsaved changes const hasChanges = useMemo(() => { if (form.key !== repository.key) return true; @@ -114,8 +177,9 @@ export function RepoSettingsTab({ repository }: RepoSettingsTabProps) { const currentQuotaBytes = quotaToBytes(quotaValue, quotaUnit); const originalQuotaBytes = repository.quota_bytes ?? null; if (currentQuotaBytes !== originalQuotaBytes) return true; + if (cacheTtlChanged) return true; return false; - }, [form, quotaValue, quotaUnit, repository]); + }, [form, quotaValue, quotaUnit, repository, cacheTtlChanged]); // -- Save mutation -- const saveMutation = useMutation({ @@ -135,7 +199,26 @@ export function RepoSettingsTab({ repository }: RepoSettingsTabProps) { onError: mutationErrorToast("Failed to save repository settings"), }); - const handleSave = useCallback(() => { + // -- Cache TTL mutation (#448, separate endpoint from `update`) -- + const setCacheTtlMutation = useMutation({ + mutationFn: (seconds: number) => + repositoriesApi.setCacheTtl(repository.key, seconds), + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: ["cache-ttl", repository.key] }); + setCacheTtlOverride(undefined); + toast.success("Cache TTL saved"); + }, + onError: mutationErrorToast("Failed to save cache TTL"), + }); + + const handleSave = useCallback(async () => { + // The general-fields update and the cache-TTL update are two separate + // backend endpoints, so dispatch them independently. We deliberately do + // NOT short-circuit one on the other's failure: a bad TTL value + // shouldn't roll back a good name change, and the per-mutation toast + // already tells the operator which side failed. The promises are run + // in parallel because both are idempotent and the round-trips are + // independent. const fields: UpdateRepositoryFields = {}; if (form.name !== repository.name) fields.name = form.name; if (form.description !== (repository.description ?? "")) @@ -150,12 +233,34 @@ export function RepoSettingsTab({ repository }: RepoSettingsTabProps) { fields.quota_bytes = newQuota; } - saveMutation.mutate(fields); - }, [form, quotaValue, quotaUnit, repository, keyChanged, saveMutation]); + const promises: Promise[] = []; + if (Object.keys(fields).length > 0) { + promises.push(saveMutation.mutateAsync(fields)); + } + if (cacheTtlChanged && cacheTtlIsValid && parsedCacheTtl != null) { + promises.push(setCacheTtlMutation.mutateAsync(parsedCacheTtl)); + } + // Awaited via Promise.allSettled so a 4xx on one side doesn't surface + // as an unhandled rejection — each mutation already wired its own + // onError toast. + await Promise.allSettled(promises); + }, [ + form, + quotaValue, + quotaUnit, + repository, + keyChanged, + saveMutation, + cacheTtlChanged, + cacheTtlIsValid, + parsedCacheTtl, + setCacheTtlMutation, + ]); const handleDiscard = useCallback(() => { setOverrides({}); setQuotaOverrides({}); + setCacheTtlOverride(undefined); }, []); // -- Lifecycle policies -- @@ -343,6 +448,76 @@ export function RepoSettingsTab({ repository }: RepoSettingsTabProps) { + {/* -- Proxy Cache Section (#448, Remote-only) -- */} + {isRemote && ( + <> +
+

+ Proxy Cache +

+
+

+ How long the proxy keeps cached upstream artifacts before + re-validating against upstream. Applies repository-wide; per- + artifact eviction is available from the artifact details + dialog. +

+ +
+ + {cacheTtlLoading ? ( + + ) : ( + <> + setCacheTtlOverride(e.target.value)} + aria-invalid={ + cacheTtlOverride !== undefined && !cacheTtlIsValid + } + aria-describedby={ + cacheTtlOverride !== undefined && !cacheTtlIsValid + ? "settings-cache-ttl-error" + : undefined + } + /> +
+ + Range: {CACHE_TTL_MIN_SECONDS}s to{" "} + {CACHE_TTL_MAX_SECONDS.toLocaleString()}s (30 days) + + {parsedCacheTtl != null && cacheTtlIsValid && ( + + ≈ {formatTtlHint(parsedCacheTtl)} + + )} +
+ {cacheTtlOverride !== undefined && !cacheTtlIsValid && ( + + )} + + )} +
+
+
+ + + + )} + {/* -- Cleanup Policies Section -- */}
@@ -428,14 +603,24 @@ export function RepoSettingsTab({ repository }: RepoSettingsTabProps) { You have unsaved changes
-