From b0418a44308eab56446da74ee1a3b2a16e4099b9 Mon Sep 17 00:00:00 2001 From: Quick <31828688+Quick104@users.noreply.github.com> Date: Mon, 3 Aug 2026 21:51:50 -0400 Subject: [PATCH] fix(web): stop library state request loop (#536) * fix(web): stop library state request loop * fix(web): harden library state persistence * fix(web): reconcile rejected library state writes * fix(web): make library state persistence lossless * fix(web): serialize library state across remounts * fix(web): pin profile identity across auth refresh * fix(web): scope profile verification cleanup * fix(web): isolate library state by account --- web/src/api/client.test.ts | 98 +++ web/src/api/client.ts | 21 +- .../hooks/queries/libraryPageState.test.tsx | 806 ++++++++++++++++++ web/src/hooks/queries/libraryPageState.ts | 346 +++++++- web/src/hooks/queries/settingValues.ts | 59 +- .../queries/settingValuesMutations.test.tsx | 193 +++++ web/src/pages/LibraryPage.test.tsx | 337 ++++++++ web/src/pages/LibraryPage.tsx | 222 ++++- 8 files changed, 2027 insertions(+), 55 deletions(-) create mode 100644 web/src/hooks/queries/libraryPageState.test.tsx create mode 100644 web/src/hooks/queries/settingValuesMutations.test.tsx create mode 100644 web/src/pages/LibraryPage.test.tsx diff --git a/web/src/api/client.test.ts b/web/src/api/client.test.ts index d3f4a554..0ffa92f7 100644 --- a/web/src/api/client.test.ts +++ b/web/src/api/client.test.ts @@ -5,7 +5,10 @@ import { bootstrapAccessToken, getAccessToken, getPersonCatalogItems, + onProfileUnverified, setAccessToken, + setProfileId, + setProfileToken, setRefreshToken, } from "./client"; @@ -167,6 +170,101 @@ describe("apiBlob", () => { }); describe("api", () => { + it("keeps the originating profile isolated when retrying after token refresh", async () => { + const localStorageState = new Map(); + Object.defineProperty(globalThis, "localStorage", { + value: { + get length() { + return localStorageState.size; + }, + getItem: (key: string) => localStorageState.get(key) ?? null, + key: (index: number) => Array.from(localStorageState.keys())[index] ?? null, + setItem: (key: string, value: string) => { + localStorageState.set(key, value); + }, + removeItem: (key: string) => { + localStorageState.delete(key); + }, + clear: () => { + localStorageState.clear(); + }, + } satisfies Storage, + configurable: true, + }); + Object.defineProperty(globalThis, "sessionStorage", { + value: { + getItem: () => null, + setItem: () => {}, + removeItem: () => {}, + clear: () => {}, + }, + configurable: true, + }); + setAccessToken("old"); + setRefreshToken("old"); + setProfileId("profile-old"); + setProfileToken("old"); + const profileUnverified = vi.fn(); + onProfileUnverified(profileUnverified); + + let protectedRequestCount = 0; + const fetchMock = vi.fn(async (input) => { + const path = String(input); + if (path === "/api/v1/auth/refresh") { + // Model a household profile switch while refresh is in flight. + setProfileId("profile-new"); + setProfileToken("new"); + return new Response( + JSON.stringify({ + access_token: "new", + refresh_token: "new", + expires_in: 3600, + }), + { status: 200, headers: { "Content-Type": "application/json" } }, + ); + } + protectedRequestCount += 1; + return protectedRequestCount === 1 + ? new Response(null, { status: 401 }) + : new Response( + JSON.stringify({ + error: "profile_unverified", + message: "Profile verification required.", + }), + { status: 403, headers: { "Content-Type": "application/json" } }, + ); + }); + vi.stubGlobal("fetch", fetchMock); + + await expect( + api("/settings/values/ui.library_page_state?scope=profile_device", { + method: "PUT", + body: JSON.stringify({ value: { version: 1, libraries: {} } }), + }), + ).rejects.toMatchObject({ status: 403, code: "profile_unverified" }); + + const requestCalls = fetchMock.mock.calls.filter( + ([input]) => String(input) !== "/api/v1/auth/refresh", + ); + expect(requestCalls).toHaveLength(2); + const firstHeaders = requestCalls[0]?.[1]?.headers as Record; + const retryHeaders = requestCalls[1]?.[1]?.headers as Record; + expect(firstHeaders).toMatchObject({ + Authorization: "Bearer old", + "X-Profile-Id": "profile-old", + "X-Profile-Token": "old", + }); + expect(retryHeaders).toMatchObject({ + Authorization: "Bearer new", + "X-Profile-Id": "profile-old", + "X-Profile-Token": "old", + }); + expect(localStorage.getItem("profile_id")).toBe("profile-new"); + expect(localStorage.getItem("profile_token")).toBe("new"); + expect(profileUnverified).not.toHaveBeenCalled(); + onProfileUnverified(null); + }); + it("forwards AbortSignal from options to fetch", async () => { Object.defineProperty(globalThis, "sessionStorage", { value: { diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 6a66932b..bc4c55b6 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -343,6 +343,8 @@ export async function api(path: string, options: RequestInit = {}): Promise { const headers = buildApiHeaders(options); + const requestProfileId = headers["X-Profile-Id"] ?? null; + const requestProfileToken = headers["X-Profile-Token"] ?? null; let res = await fetch(`/api/v1${path}`, { ...options, headers }); @@ -355,14 +357,29 @@ export async function apiResponse(path: string, options: RequestInit = {}): Prom } const refreshed = await refreshPromise; if (refreshed) { - const refreshedHeaders = buildApiHeaders(options); + // Keep the profile and device identity captured for the original + // request. A household profile can change while refresh is pending; + // rebuilding every header here could replay an old-profile mutation + // under the newly selected profile. Only the refreshed account token + // is allowed to change for this retry. + const refreshedHeaders = { ...headers }; + if (accessToken) { + refreshedHeaders.Authorization = `Bearer ${accessToken}`; + } else { + delete refreshedHeaders.Authorization; + } res = await fetch(`/api/v1${path}`, { ...options, headers: refreshedHeaders }); } } if (!res.ok) { const parsed = await parseApiError(res); - if (res.status === 403 && parsed.apiErr.error === "profile_unverified") { + if ( + res.status === 403 && + parsed.apiErr.error === "profile_unverified" && + getProfileId() === requestProfileId && + getProfileToken() === requestProfileToken + ) { setProfileToken(null); profileUnverifiedListener?.(); } diff --git a/web/src/hooks/queries/libraryPageState.test.tsx b/web/src/hooks/queries/libraryPageState.test.tsx new file mode 100644 index 00000000..e0590cf9 --- /dev/null +++ b/web/src/hooks/queries/libraryPageState.test.tsx @@ -0,0 +1,806 @@ +import { act, renderHook, waitFor } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { ApiClientError } from "@/api/client"; +import { + libraryPageStateWriteRetryDelay, + shouldRetryLibraryPageStateWrite, + useLibraryPageStatePreference, +} from "./libraryPageState"; + +const mocks = vi.hoisted(() => ({ + mutate: vi.fn(), + mutateAsync: vi.fn(), + accountId: 1, + profileId: "profile-1", + preference: { + version: 1 as const, + libraries: { "3": { search: "tab=collections" } } as Record, + }, +})); + +vi.mock("@/utils/storage", () => ({ + storage: { + KEYS: { PROFILE_ID: "profile_id" }, + get: () => mocks.profileId, + }, +})); + +vi.mock("@/hooks/useAuth", () => ({ + useOptionalAuth: () => ({ user: { id: mocks.accountId } }), +})); + +vi.mock("@/hooks/queries/settingValues", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useEffectiveSettings: () => ({ + data: { + "ui.library_page_state": { + value: mocks.preference, + }, + "ui.remember_library_page_state": { value: true }, + }, + isLoading: false, + }), + useSetSettingValue: () => ({ + mutate: mocks.mutate, + mutateAsync: mocks.mutateAsync, + }), + }; +}); + +let profileSequence = 0; + +describe("useLibraryPageStatePreference", () => { + beforeEach(() => { + mocks.mutate.mockReset(); + mocks.mutateAsync.mockReset(); + profileSequence += 1; + mocks.accountId = profileSequence; + mocks.profileId = `profile-test-${profileSequence}`; + mocks.preference = { + version: 1, + libraries: { "3": { search: "tab=collections" } }, + }; + }); + + it("serializes whole-document saves and preserves queued library changes", async () => { + let resolveFirst: ((value: unknown) => void) | undefined; + mocks.mutateAsync + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirst = resolve; + }), + ) + .mockResolvedValueOnce({}); + const { result } = renderHook(() => useLibraryPageStatePreference()); + + act(() => { + void result.current.saveLibrarySearch(7, "tab=library&sort=year"); + void result.current.saveLibrarySearch(9, "tab=collections"); + }); + + await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledTimes(1)); + expect(mocks.mutate).not.toHaveBeenCalled(); + + await act(async () => { + resolveFirst?.({}); + await Promise.resolve(); + }); + await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledTimes(2)); + + expect(mocks.mutateAsync.mock.calls[0][0].value).toEqual({ + version: 1, + libraries: { + "3": { search: "tab=collections" }, + "7": { search: "tab=library&sort=year" }, + }, + }); + expect(mocks.mutateAsync.mock.calls[1][0].value).toEqual({ + version: 1, + libraries: { + "3": { search: "tab=collections" }, + "7": { search: "tab=library&sort=year" }, + "9": { search: "tab=collections" }, + }, + }); + }); + + it("allows the same desired state to retry after a failed write", async () => { + mocks.mutateAsync + .mockRejectedValueOnce(new ApiClientError(429, "rate_limited", "rate limited")) + .mockResolvedValueOnce({}); + const { result } = renderHook(() => useLibraryPageStatePreference()); + + await act(async () => { + await expect(result.current.saveLibrarySearch(7, "tab=library&sort=year")).rejects.toThrow( + "rate limited", + ); + }); + await act(async () => { + await result.current.saveLibrarySearch(7, "tab=library&sort=year"); + }); + + expect(mocks.mutateAsync).toHaveBeenCalledTimes(2); + }); + + it("retries the same desired state after an ambiguous write failure", async () => { + mocks.mutateAsync + .mockRejectedValueOnce(new TypeError("network connection lost")) + .mockResolvedValueOnce({}); + const { result } = renderHook(() => useLibraryPageStatePreference()); + + await act(async () => { + await expect(result.current.saveLibrarySearch(7, "tab=library&sort=year")).rejects.toThrow( + "network connection lost", + ); + }); + await act(async () => { + await result.current.saveLibrarySearch(7, "tab=library&sort=year"); + }); + + expect(mocks.mutateAsync).toHaveBeenCalledTimes(2); + }); + + it("removes a rejected value before applying later queued writes", async () => { + let rejectFirst: ((reason?: unknown) => void) | undefined; + mocks.mutateAsync + .mockImplementationOnce( + () => + new Promise((_, reject) => { + rejectFirst = reject; + }), + ) + .mockResolvedValueOnce({}); + const { result } = renderHook(() => useLibraryPageStatePreference()); + + const rejected = result.current.saveLibrarySearch(3, "tab=library&sort=year"); + const rejectedError = rejected.catch((error: unknown) => error); + const queued = result.current.saveLibrarySearch(9, "tab=collections"); + + await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledTimes(1)); + await act(async () => { + rejectFirst?.(new ApiClientError(429, "rate_limited", "rate limited")); + expect(await rejectedError).toEqual(new ApiClientError(429, "rate_limited", "rate limited")); + await queued; + }); + + expect(mocks.mutateAsync).toHaveBeenCalledTimes(2); + expect(mocks.mutateAsync.mock.calls[1][0].value).toEqual({ + version: 1, + libraries: { + "3": { search: "tab=collections" }, + "9": { search: "tab=collections" }, + }, + }); + }); + + it("preserves an ambiguous attempted write before advancing the queue", async () => { + let rejectFirst: ((reason?: unknown) => void) | undefined; + mocks.mutateAsync + .mockImplementationOnce( + () => + new Promise((_, reject) => { + rejectFirst = reject; + }), + ) + .mockResolvedValueOnce({}); + const { result } = renderHook(() => useLibraryPageStatePreference()); + + const ambiguous = result.current.saveLibrarySearch(7, "tab=library&sort=year"); + const ambiguousError = ambiguous.catch((error: unknown) => error); + const queued = result.current.saveLibrarySearch(9, "tab=collections"); + + await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledTimes(1)); + await act(async () => { + rejectFirst?.(new TypeError("response connection lost")); + expect(await ambiguousError).toEqual(new TypeError("response connection lost")); + await queued; + }); + + expect(mocks.mutateAsync.mock.calls[1][0].value).toEqual({ + version: 1, + libraries: { + "3": { search: "tab=collections" }, + "7": { search: "tab=library&sort=year" }, + "9": { search: "tab=collections" }, + }, + }); + }); + + it("treats a server error as ambiguous before advancing the queue", async () => { + mocks.mutateAsync + .mockRejectedValueOnce(new ApiClientError(503, "unavailable", "service unavailable")) + .mockResolvedValueOnce({}); + const { result } = renderHook(() => useLibraryPageStatePreference()); + + const ambiguous = result.current.saveLibrarySearch(7, "tab=library&sort=year"); + const ambiguousError = ambiguous.catch((error: unknown) => error); + const queued = result.current.saveLibrarySearch(9, "tab=collections"); + + await ambiguousError; + await queued; + + expect(mocks.mutateAsync.mock.calls[1][0].value).toEqual({ + version: 1, + libraries: { + "3": { search: "tab=collections" }, + "7": { search: "tab=library&sort=year" }, + "9": { search: "tab=collections" }, + }, + }); + }); + + it("merges an ambiguous write onto a deferred settings snapshot", async () => { + let rejectFirst: ((reason?: unknown) => void) | undefined; + mocks.mutateAsync + .mockImplementationOnce( + () => + new Promise((_, reject) => { + rejectFirst = reject; + }), + ) + .mockResolvedValueOnce({}); + const { result, rerender } = renderHook(() => useLibraryPageStatePreference()); + + const ambiguous = result.current.saveLibrarySearch(7, "tab=library&sort=year"); + const ambiguousError = ambiguous.catch((error: unknown) => error); + const queued = result.current.saveLibrarySearch(9, "tab=collections"); + + await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledTimes(1)); + act(() => { + mocks.preference = { + version: 1, + libraries: { + "3": { search: "tab=collections" }, + "11": { search: "tab=library&sort=title" }, + }, + }; + rerender(); + }); + await act(async () => { + rejectFirst?.(new TypeError("response connection lost")); + await ambiguousError; + await queued; + }); + + expect(mocks.mutateAsync.mock.calls[1][0].value).toEqual({ + version: 1, + libraries: { + "3": { search: "tab=collections" }, + "7": { search: "tab=library&sort=year" }, + "9": { search: "tab=collections" }, + "11": { search: "tab=library&sort=title" }, + }, + }); + }); + + it("merges a successful write onto a deferred settings snapshot", async () => { + let resolveFirst: ((value: unknown) => void) | undefined; + mocks.mutateAsync + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirst = resolve; + }), + ) + .mockResolvedValueOnce({}); + const { result, rerender } = renderHook(() => useLibraryPageStatePreference()); + + const first = result.current.saveLibrarySearch(7, "tab=library&sort=year"); + const queued = result.current.saveLibrarySearch(9, "tab=collections"); + + await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledTimes(1)); + act(() => { + mocks.preference = { + version: 1, + libraries: { + "3": { search: "tab=collections" }, + "11": { search: "tab=library&sort=title" }, + }, + }; + rerender(); + }); + await act(async () => { + resolveFirst?.({}); + await first; + await queued; + }); + + expect(mocks.mutateAsync.mock.calls[1][0].value).toEqual({ + version: 1, + libraries: { + "3": { search: "tab=collections" }, + "7": { search: "tab=library&sort=year" }, + "9": { search: "tab=collections" }, + "11": { search: "tab=library&sort=title" }, + }, + }); + }); + + it("keeps an earlier ambiguous edit when a stale snapshot arrives during the next write", async () => { + let rejectFirst: ((reason?: unknown) => void) | undefined; + let resolveSecond: ((value: unknown) => void) | undefined; + mocks.mutateAsync + .mockImplementationOnce( + () => + new Promise((_, reject) => { + rejectFirst = reject; + }), + ) + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveSecond = resolve; + }), + ) + .mockResolvedValueOnce({}); + const { result, rerender } = renderHook(() => useLibraryPageStatePreference()); + + const first = result.current.saveLibrarySearch(7, "tab=library&sort=year"); + const firstError = first.catch((error: unknown) => error); + const second = result.current.saveLibrarySearch(9, "tab=collections"); + + await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledTimes(1)); + await act(async () => { + rejectFirst?.(new TypeError("response connection lost")); + await firstError; + }); + await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledTimes(2)); + + act(() => { + mocks.preference = { + version: 1, + libraries: { + "3": { search: "tab=collections" }, + "11": { search: "tab=library&sort=title" }, + }, + }; + rerender(); + }); + await act(async () => { + resolveSecond?.({}); + await second; + }); + await act(async () => { + await result.current.saveLibrarySearch(13, "tab=library&sort=added"); + }); + + expect(mocks.mutateAsync.mock.calls[2][0].value).toEqual({ + version: 1, + libraries: { + "3": { search: "tab=collections" }, + "7": { search: "tab=library&sort=year" }, + "9": { search: "tab=collections" }, + "11": { search: "tab=library&sort=title" }, + "13": { search: "tab=library&sort=added" }, + }, + }); + }); + + it("keeps an earlier successful edit when a stale snapshot arrives during the next write", async () => { + let resolveFirst: ((value: unknown) => void) | undefined; + let resolveSecond: ((value: unknown) => void) | undefined; + mocks.mutateAsync + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirst = resolve; + }), + ) + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveSecond = resolve; + }), + ) + .mockResolvedValueOnce({}); + const { result, rerender } = renderHook(() => useLibraryPageStatePreference()); + + const first = result.current.saveLibrarySearch(7, "tab=library&sort=year"); + const second = result.current.saveLibrarySearch(9, "tab=collections"); + + await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledTimes(1)); + await act(async () => { + resolveFirst?.({}); + await first; + }); + await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledTimes(2)); + + act(() => { + mocks.preference = { + version: 1, + libraries: { + "3": { search: "tab=collections" }, + "11": { search: "tab=library&sort=title" }, + }, + }; + rerender(); + }); + await act(async () => { + resolveSecond?.({}); + await second; + }); + await act(async () => { + await result.current.saveLibrarySearch(13, "tab=library&sort=added"); + }); + + expect(mocks.mutateAsync.mock.calls[2][0].value).toEqual({ + version: 1, + libraries: { + "3": { search: "tab=collections" }, + "7": { search: "tab=library&sort=year" }, + "9": { search: "tab=collections" }, + "11": { search: "tab=library&sort=title" }, + "13": { search: "tab=library&sort=added" }, + }, + }); + }); + + it("keeps an ambiguous edit when reconciliation finishes before the next write", async () => { + mocks.mutateAsync + .mockRejectedValueOnce(new TypeError("response connection lost")) + .mockResolvedValueOnce({}); + const { result, rerender } = renderHook(() => useLibraryPageStatePreference()); + + await act(async () => { + await expect(result.current.saveLibrarySearch(7, "tab=library&sort=year")).rejects.toThrow( + "response connection lost", + ); + }); + act(() => { + mocks.preference = { + version: 1, + libraries: { + "3": { search: "tab=collections" }, + "11": { search: "tab=library&sort=title" }, + }, + }; + rerender(); + }); + await act(async () => { + await result.current.saveLibrarySearch(9, "tab=collections"); + }); + + expect(mocks.mutateAsync.mock.calls[1][0].value).toEqual({ + version: 1, + libraries: { + "3": { search: "tab=collections" }, + "7": { search: "tab=library&sort=year" }, + "9": { search: "tab=collections" }, + "11": { search: "tab=library&sort=title" }, + }, + }); + }); + + it("applies a deferred settings snapshot after a definitive write failure", async () => { + let rejectFirst: ((reason?: unknown) => void) | undefined; + mocks.mutateAsync + .mockImplementationOnce( + () => + new Promise((_, reject) => { + rejectFirst = reject; + }), + ) + .mockResolvedValueOnce({}); + const { result, rerender } = renderHook(() => useLibraryPageStatePreference()); + + const rejected = result.current.saveLibrarySearch(7, "tab=library&sort=year"); + const rejectedError = rejected.catch((error: unknown) => error); + const queued = result.current.saveLibrarySearch(9, "tab=collections"); + + await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledTimes(1)); + act(() => { + mocks.preference = { + version: 1, + libraries: { + "3": { search: "tab=collections" }, + "11": { search: "tab=library&sort=title" }, + }, + }; + rerender(); + }); + await act(async () => { + rejectFirst?.(new ApiClientError(429, "rate_limited", "rate limited")); + await rejectedError; + await queued; + }); + + expect(mocks.mutateAsync.mock.calls[1][0].value).toEqual({ + version: 1, + libraries: { + "3": { search: "tab=collections" }, + "9": { search: "tab=collections" }, + "11": { search: "tab=library&sort=title" }, + }, + }); + }); + + it("returns the promise for the matching in-flight save", async () => { + let resolveFirst: ((value: unknown) => void) | undefined; + mocks.mutateAsync + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirst = resolve; + }), + ) + .mockRejectedValueOnce(new ApiClientError(429, "rate_limited", "rate limited")); + const { result } = renderHook(() => useLibraryPageStatePreference()); + + const first = result.current.saveLibrarySearch(7, "tab=library&sort=year"); + const tail = result.current.saveLibrarySearch(9, "tab=collections"); + const coalesced = result.current.saveLibrarySearch(7, "tab=library&sort=year"); + const tailError = tail.catch((error: unknown) => error); + + expect(coalesced).toBe(first); + + await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledTimes(1)); + await act(async () => { + resolveFirst?.({}); + await first; + }); + + await expect(coalesced).resolves.toEqual({}); + expect(await tailError).toEqual(new ApiClientError(429, "rate_limited", "rate limited")); + expect(mocks.mutateAsync).toHaveBeenCalledTimes(2); + }); + + it("returns the rejected promise for a matching middle write", async () => { + let resolveFirst: ((value: unknown) => void) | undefined; + const rejected = new ApiClientError(429, "rate_limited", "rate limited"); + mocks.mutateAsync + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirst = resolve; + }), + ) + .mockRejectedValueOnce(rejected) + .mockResolvedValueOnce({}); + const { result } = renderHook(() => useLibraryPageStatePreference()); + + const first = result.current.saveLibrarySearch(7, "tab=library&sort=year"); + const middle = result.current.saveLibrarySearch(9, "tab=collections"); + void result.current.saveLibrarySearch(7, "tab=library&sort=title"); + const revisitedMiddle = result.current.saveLibrarySearch(9, "tab=collections"); + + expect(revisitedMiddle).toBe(middle); + await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledTimes(1)); + await act(async () => { + resolveFirst?.({}); + await first; + }); + + await expect(revisitedMiddle).rejects.toEqual(rejected); + await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledTimes(3)); + }); + + it("does not coalesce past a newer value for the same library", async () => { + let resolveFirst: ((value: unknown) => void) | undefined; + mocks.mutateAsync + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirst = resolve; + }), + ) + .mockResolvedValue({}); + const { result } = renderHook(() => useLibraryPageStatePreference()); + + const first = result.current.saveLibrarySearch(7, "tab=library&sort=year"); + void result.current.saveLibrarySearch(7, "tab=library&sort=title"); + const latest = result.current.saveLibrarySearch(7, "tab=library&sort=year"); + + expect(latest).not.toBe(first); + await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledTimes(1)); + await act(async () => { + resolveFirst?.({}); + await first; + await latest; + }); + + expect(mocks.mutateAsync).toHaveBeenCalledTimes(3); + expect(mocks.mutateAsync.mock.calls[2][0].value.libraries["7"]).toEqual({ + search: "tab=library&sort=year", + }); + }); + + it("does not share a cancellable queued write across hook instances", async () => { + let resolveBlocker: ((value: unknown) => void) | undefined; + mocks.mutateAsync + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveBlocker = resolve; + }), + ) + .mockResolvedValueOnce({}); + const firstHook = renderHook(() => useLibraryPageStatePreference()); + + const blocker = firstHook.result.current.saveLibrarySearch(5, "tab=collections"); + const cancelled = firstHook.result.current.saveLibrarySearch(7, "tab=library&sort=year"); + const cancellation = cancelled.catch((error: unknown) => error); + await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledTimes(1)); + + const secondHook = renderHook(() => useLibraryPageStatePreference()); + const retained = secondHook.result.current.saveLibrarySearch(7, "tab=library&sort=year"); + expect(retained).not.toBe(cancelled); + firstHook.unmount(); + + await act(async () => { + resolveBlocker?.({}); + await blocker; + await cancellation; + await retained; + }); + + expect(mocks.mutateAsync).toHaveBeenCalledTimes(2); + expect(mocks.mutateAsync.mock.calls[1][0].value.libraries["7"]).toEqual({ + search: "tab=library&sort=year", + }); + }); + + it("serializes whole-document saves across hook remounts", async () => { + let resolveFirst: ((value: unknown) => void) | undefined; + mocks.mutateAsync + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirst = resolve; + }), + ) + .mockResolvedValueOnce({}); + const firstHook = renderHook(() => useLibraryPageStatePreference()); + + const first = firstHook.result.current.saveLibrarySearch(7, "tab=library&sort=year"); + await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledTimes(1)); + firstHook.unmount(); + + const secondHook = renderHook(() => useLibraryPageStatePreference()); + const second = secondHook.result.current.saveLibrarySearch(9, "tab=collections"); + await Promise.resolve(); + expect(mocks.mutateAsync).toHaveBeenCalledTimes(1); + + await act(async () => { + resolveFirst?.({}); + await first; + await second; + }); + + expect(mocks.mutateAsync).toHaveBeenCalledTimes(2); + expect(mocks.mutateAsync.mock.calls[1][0].value).toEqual({ + version: 1, + libraries: { + "3": { search: "tab=collections" }, + "7": { search: "tab=library&sort=year" }, + "9": { search: "tab=collections" }, + }, + }); + }); + + it("cancels queued writes when the active profile changes", async () => { + let resolveFirst: ((value: unknown) => void) | undefined; + mocks.mutateAsync.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirst = resolve; + }), + ); + const { result, rerender } = renderHook(() => useLibraryPageStatePreference()); + + const first = result.current.saveLibrarySearch(7, "tab=library&sort=year"); + const queued = result.current.saveLibrarySearch(9, "tab=collections"); + const queuedError = queued.catch((error: unknown) => error); + + await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledTimes(1)); + act(() => { + mocks.profileId = `${mocks.profileId}-next`; + rerender(); + }); + await act(async () => { + resolveFirst?.({}); + await first; + }); + + const cancellation = await queuedError; + expect(cancellation).toEqual( + new Error("Library preference write cancelled because the active profile changed"), + ); + expect(shouldRetryLibraryPageStateWrite(cancellation)).toBe(true); + expect(libraryPageStateWriteRetryDelay(cancellation, 2_000)).toBe(0); + expect(mocks.mutateAsync).toHaveBeenCalledTimes(1); + }); + + it("does not carry an old account's overlay into the same profile id", async () => { + mocks.mutateAsync + .mockRejectedValueOnce(new TypeError("response connection lost")) + .mockResolvedValueOnce({}); + const { result, rerender } = renderHook(() => useLibraryPageStatePreference()); + + await act(async () => { + await expect(result.current.saveLibrarySearch(7, "tab=library&sort=year")).rejects.toThrow( + "response connection lost", + ); + }); + + act(() => { + mocks.accountId += 10_000; + rerender(); + }); + await act(async () => { + await result.current.saveLibrarySearch(9, "tab=collections"); + }); + + expect(mocks.mutateAsync.mock.calls[1][0].value).toEqual({ + version: 1, + libraries: { + "3": { search: "tab=collections" }, + "9": { search: "tab=collections" }, + }, + }); + }); + + it("preserves the local overlay when a queued write is cancelled before dispatch", async () => { + let rejectFirst: ((reason?: unknown) => void) | undefined; + mocks.mutateAsync + .mockImplementationOnce( + () => + new Promise((_, reject) => { + rejectFirst = reject; + }), + ) + .mockResolvedValueOnce({}); + const originalProfileId = mocks.profileId; + const { result, rerender } = renderHook(() => useLibraryPageStatePreference()); + + const first = result.current.saveLibrarySearch(7, "tab=library&sort=year"); + const firstError = first.catch((error: unknown) => error); + const cancelled = result.current.saveLibrarySearch(9, "tab=collections"); + const cancellation = cancelled.catch((error: unknown) => error); + + await waitFor(() => expect(mocks.mutateAsync).toHaveBeenCalledTimes(1)); + act(() => { + mocks.profileId = `${originalProfileId}-next`; + rerender(); + }); + await act(async () => { + rejectFirst?.(new TypeError("response connection lost")); + await firstError; + await cancellation; + }); + + act(() => { + mocks.profileId = originalProfileId; + rerender(); + }); + await act(async () => { + await result.current.saveLibrarySearch(13, "tab=library&sort=added"); + }); + + expect(mocks.mutateAsync.mock.calls[1][0].value).toEqual({ + version: 1, + libraries: { + "3": { search: "tab=collections" }, + "7": { search: "tab=library&sort=year" }, + "13": { search: "tab=library&sort=added" }, + }, + }); + }); + + it("honors a rate-limit retry hint while keeping retries bounded by the caller", () => { + const error = new ApiClientError(429, "rate_limit_exceeded", "rate limited"); + error.body = { retry_after: 30 }; + + expect(shouldRetryLibraryPageStateWrite(error)).toBe(true); + expect(libraryPageStateWriteRetryDelay(error, 2_000)).toBe(30_000); + expect( + libraryPageStateWriteRetryDelay( + new ApiClientError(422, "validation_failed", "invalid setting"), + 2_000, + ), + ).toBeNull(); + }); +}); diff --git a/web/src/hooks/queries/libraryPageState.ts b/web/src/hooks/queries/libraryPageState.ts index 54504e00..fe966386 100644 --- a/web/src/hooks/queries/libraryPageState.ts +++ b/web/src/hooks/queries/libraryPageState.ts @@ -1,5 +1,11 @@ -import { useCallback, useMemo } from "react"; -import { useEffectiveSettings, useSetSettingValue } from "@/hooks/queries/settingValues"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { ApiClientError } from "@/api/client"; +import { + isDefinitiveSettingMutationRejection, + useEffectiveSettings, + useSetSettingValue, +} from "@/hooks/queries/settingValues"; +import { useOptionalAuth } from "@/hooks/useAuth"; import type { SettingIdentity } from "@/hooks/queries/settingValues"; import { SETTING_KEYS } from "@/lib/settingsContract"; import { storage } from "@/utils/storage"; @@ -17,6 +23,43 @@ export interface LibraryPageStatePreference { libraries: Record; } +interface PreferenceWriteQueue { + ownerKey: string | null; + ownerProfileId: string | null; + subscribers: number; + resolvedPreference: LibraryPageStatePreference; + deferredPreference: LibraryPageStatePreference | null; + unconfirmedWrites: PendingPreferenceWrite[]; + pendingWrites: PendingPreferenceWrite[]; + writeChain: Promise; +} + +interface PreferenceWriteLease { + ownerKey: string | null; + active: boolean; +} + +interface PendingPreferenceWrite { + libraryId: number; + search: string; + lease: PreferenceWriteLease; + promise?: Promise; +} + +// The setting is a whole-document value. Keep one queue per account+profile +// for the lifetime of this browser context so an unmount/remount cannot let a +// newer document race an older in-flight PUT. Profile IDs are not globally +// unique, so the account must be part of the key to prevent logout/login from +// carrying an old account's overlay into a new account's document. +const preferenceWriteQueues = new Map(); + +function preferenceWriteOwnerKey( + accountId: number | null, + profileId: string | null, +): string | null { + return accountId === null || profileId === null ? null : JSON.stringify([accountId, profileId]); +} + /** * Accepts the canonical object value, the legacy JSON-string encoding, or * null/undefined (the contract default), and always lands on a usable @@ -84,32 +127,313 @@ function createEmptyLibraryPageStatePreference(): LibraryPageStatePreference { return { version: 1, libraries: {} }; } +function createPreferenceWriteQueue( + ownerKey: string | null, + ownerProfileId: string | null, + preference: LibraryPageStatePreference, +): PreferenceWriteQueue { + return { + ownerKey, + ownerProfileId, + subscribers: 0, + resolvedPreference: preference, + deferredPreference: null, + unconfirmedWrites: [], + pendingWrites: [], + writeChain: Promise.resolve(), + }; +} + +function getPreferenceWriteQueue( + ownerKey: string | null, + ownerProfileId: string | null, + preference: LibraryPageStatePreference, +): PreferenceWriteQueue { + if (ownerKey === null) { + return createPreferenceWriteQueue(null, ownerProfileId, preference); + } + const existing = preferenceWriteQueues.get(ownerKey); + if (existing !== undefined) { + return existing; + } + const queue = createPreferenceWriteQueue(ownerKey, ownerProfileId, preference); + preferenceWriteQueues.set(ownerKey, queue); + return queue; +} + +function applyPendingPreferenceWrites( + preference: LibraryPageStatePreference, + writes: PendingPreferenceWrite[], +): LibraryPageStatePreference { + return writes.reduce( + (next, write) => updateLibraryPageStatePreference(next, write.libraryId, write.search), + preference, + ); +} + +function appendPreferenceWrite( + writes: PendingPreferenceWrite[], + write: PendingPreferenceWrite, +): PendingPreferenceWrite[] { + return [...writes.filter((candidate) => candidate.libraryId !== write.libraryId), write]; +} + +function findMatchingPendingWrite( + writes: PendingPreferenceWrite[], + lease: PreferenceWriteLease, + libraryId: number, + search: string, +): PendingPreferenceWrite | undefined { + for (let index = writes.length - 1; index >= 0; index -= 1) { + const write = writes[index]; + if (write?.libraryId === libraryId) { + return write.lease === lease && write.lease.active && write.search === search + ? write + : undefined; + } + } + return undefined; +} + +function removeConfirmedPreferenceWrites( + preference: LibraryPageStatePreference, + writes: PendingPreferenceWrite[], +): PendingPreferenceWrite[] { + return writes.filter( + (write) => preference.libraries[String(write.libraryId)]?.search !== write.search, + ); +} + +function settlePreferenceWrite( + queue: PreferenceWriteQueue, + pendingWrite: PendingPreferenceWrite, + outcome: "success" | "ambiguous_failure" | "definitive_failure", + attemptedPreference?: LibraryPageStatePreference, + attemptedWrites: PendingPreferenceWrite[] = [], +) { + const authoritativePreference = queue.deferredPreference; + if (outcome === "success" || outcome === "ambiguous_failure") { + // A successful mutation can still be followed by a stale refetch. Keep + // every local edit in the overlay until an effective-settings snapshot + // actually contains it; ambiguous failures need the same protection. + queue.unconfirmedWrites = + authoritativePreference === null + ? attemptedWrites + : removeConfirmedPreferenceWrites(authoritativePreference, attemptedWrites); + } else if (authoritativePreference !== null) { + queue.unconfirmedWrites = removeConfirmedPreferenceWrites( + authoritativePreference, + queue.unconfirmedWrites, + ); + } + const resolvedBase = + authoritativePreference ?? + (outcome === "definitive_failure" ? queue.resolvedPreference : attemptedPreference) ?? + queue.resolvedPreference; + queue.resolvedPreference = applyPendingPreferenceWrites(resolvedBase, queue.unconfirmedWrites); + queue.deferredPreference = null; + queue.pendingWrites = queue.pendingWrites.filter((write) => write !== pendingWrite); +} + +class LibraryPreferenceWriteCancelledError extends Error {} + +function cancelledPreferenceWrite(): Error { + return new LibraryPreferenceWriteCancelledError( + "Library preference write cancelled because the active profile changed", + ); +} + +export function shouldRetryLibraryPageStateWrite(error: unknown): boolean { + // A profile switch cancels work owned by the old queue, but the same page + // state still needs to be submitted through the new profile's queue. + if (error instanceof LibraryPreferenceWriteCancelledError) { + return true; + } + // 408, 425, and 429 are definitive HTTP responses but transient request + // outcomes. Keep that retry decision separate from the queue's commit + // certainty decision so rate limiting cannot make a page state terminal. + if (error instanceof ApiClientError) { + return ( + error.status === 408 || error.status === 425 || error.status === 429 || error.status >= 500 + ); + } + return !isDefinitiveSettingMutationRejection(error); +} + +export function libraryPageStateWriteRetryDelay( + error: unknown, + fallbackDelayMs: number, +): number | null { + if (!shouldRetryLibraryPageStateWrite(error)) { + return null; + } + if (error instanceof LibraryPreferenceWriteCancelledError) { + return 0; + } + if (error instanceof ApiClientError && error.status === 429) { + const body = error.body; + const retryAfter = + body && typeof body === "object" && "retry_after" in body + ? (body as { retry_after?: unknown }).retry_after + : undefined; + if (typeof retryAfter === "number" && Number.isFinite(retryAfter) && retryAfter > 0) { + return Math.max(fallbackDelayMs, retryAfter * 1_000); + } + } + return fallbackDelayMs; +} + export function useLibraryPageStatePreference() { // The effective endpoint requires a profile header; before one is chosen // there is no saved state to restore and nowhere to save it. - const enabled = Boolean(storage.get(storage.KEYS.PROFILE_ID)); + const auth = useOptionalAuth(); + const activeAccountId = auth?.user?.id ?? null; + const activeProfileId = storage.get(storage.KEYS.PROFILE_ID); + const activeOwnerKey = preferenceWriteOwnerKey(activeAccountId, activeProfileId); + const enabled = activeOwnerKey !== null; const { data, isLoading } = useEffectiveSettings({ keys: PAGE_STATE_KEYS, enabled }); const mutation = useSetSettingValue(); - const { mutate } = mutation; + const { mutateAsync } = mutation; const stateValue = data?.[SETTING_KEYS.UI_LIBRARY_PAGE_STATE]?.value; const preference = useMemo(() => parseLibraryPageStatePreference(stateValue), [stateValue]); + // This setting is one last-write-wins document. Keep queued changes in the + // same document and send them in order so a slower request cannot restore an + // older library state over a newer one. + const [initialWriteQueue] = useState(() => + getPreferenceWriteQueue(activeOwnerKey, activeProfileId, preference), + ); + const [initialWriteLease] = useState(() => ({ + ownerKey: activeOwnerKey, + active: false, + })); + const writeQueueRef = useRef(initialWriteQueue); + const writeLeaseRef = useRef(initialWriteLease); + useEffect(() => { + let currentQueue = writeQueueRef.current; + let currentLease = writeLeaseRef.current; + if (currentQueue.ownerKey !== activeOwnerKey) { + currentLease.active = false; + // The following preference-sync effect supplies this owner's resolved + // document before callers' effects can enqueue a write. + currentQueue = getPreferenceWriteQueue( + activeOwnerKey, + activeProfileId, + createEmptyLibraryPageStatePreference(), + ); + currentLease = { ownerKey: activeOwnerKey, active: false }; + writeQueueRef.current = currentQueue; + writeLeaseRef.current = currentLease; + } + currentQueue.subscribers += 1; + currentLease.active = true; + + return () => { + currentLease.active = false; + currentQueue.subscribers = Math.max(0, currentQueue.subscribers - 1); + }; + }, [activeOwnerKey, activeProfileId]); + useEffect(() => { + const queue = writeQueueRef.current; + const lease = writeLeaseRef.current; + if (!lease.active || lease.ownerKey !== activeOwnerKey || queue.ownerKey !== activeOwnerKey) { + return; + } + if (queue.pendingWrites.length === 0) { + queue.unconfirmedWrites = removeConfirmedPreferenceWrites( + preference, + queue.unconfirmedWrites, + ); + queue.resolvedPreference = applyPendingPreferenceWrites(preference, queue.unconfirmedWrites); + queue.deferredPreference = null; + } else { + // A realtime update or mutation refetch can arrive while a local write + // is in flight. Retain the newest authoritative snapshot so a rejected + // local tail cannot make the next write erase it. + queue.unconfirmedWrites = removeConfirmedPreferenceWrites( + preference, + queue.unconfirmedWrites, + ); + queue.deferredPreference = preference; + } + }, [activeOwnerKey, preference]); // The contract default is true; anything but an explicit false keeps the // feature on, matching the legacy `!== "false"` reading. const rememberEnabled = data?.[SETTING_KEYS.UI_REMEMBER_LIBRARY_PAGE_STATE]?.value !== false; const saveLibrarySearch = useCallback( (libraryId: number, search: string) => { - const nextPreference = updateLibraryPageStatePreference(preference, libraryId, search); - mutate({ - key: SETTING_KEYS.UI_LIBRARY_PAGE_STATE, - value: nextPreference, - identity: DEVICE_SCOPE, - }); + const queue = writeQueueRef.current; + const lease = writeLeaseRef.current; + if ( + !lease.active || + lease.ownerKey !== activeOwnerKey || + queue.ownerKey === null || + queue.ownerKey !== activeOwnerKey + ) { + return Promise.reject(cancelledPreferenceWrite()); + } + const matchingWrite = findMatchingPendingWrite(queue.pendingWrites, lease, libraryId, search); + if (matchingWrite?.promise !== undefined) { + return matchingWrite.promise; + } + + const pendingWrite: PendingPreferenceWrite = { libraryId, search, lease }; + queue.pendingWrites.push(pendingWrite); + + let attemptedPreference: LibraryPageStatePreference | undefined; + let attemptedWrites: PendingPreferenceWrite[] = []; + let mutationStarted = false; + const write = queue.writeChain + .catch(() => undefined) + .then(() => { + if (!lease.active || storage.get(storage.KEYS.PROFILE_ID) !== queue.ownerProfileId) { + throw cancelledPreferenceWrite(); + } + attemptedPreference = updateLibraryPageStatePreference( + queue.resolvedPreference, + libraryId, + search, + ); + attemptedWrites = appendPreferenceWrite(queue.unconfirmedWrites, pendingWrite); + mutationStarted = true; + return mutateAsync({ + key: SETTING_KEYS.UI_LIBRARY_PAGE_STATE, + value: attemptedPreference, + identity: DEVICE_SCOPE, + }); + }); + pendingWrite.promise = write; + queue.writeChain = write.then( + (result) => { + settlePreferenceWrite( + queue, + pendingWrite, + "success", + attemptedPreference, + attemptedWrites, + ); + return result; + }, + (error: unknown) => { + settlePreferenceWrite( + queue, + pendingWrite, + !mutationStarted || isDefinitiveSettingMutationRejection(error) + ? "definitive_failure" + : "ambiguous_failure", + attemptedPreference, + attemptedWrites, + ); + return undefined; + }, + ); + return write; }, - [mutate, preference], + [activeOwnerKey, mutateAsync], ); return { + ownerKey: activeOwnerKey, isLoading: enabled && isLoading, preference, rememberEnabled, diff --git a/web/src/hooks/queries/settingValues.ts b/web/src/hooks/queries/settingValues.ts index 65353cdd..31e35442 100644 --- a/web/src/hooks/queries/settingValues.ts +++ b/web/src/hooks/queries/settingValues.ts @@ -1,6 +1,6 @@ import { useMemo } from "react"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { api } from "@/api/client"; +import { api, ApiClientError } from "@/api/client"; import { storage } from "@/utils/storage"; import { SETTING_DEFINITIONS, SETTINGS_REVISION, type SettingKey } from "@/lib/settingsContract"; import { useEventChannel } from "@/components/realtimeEventsContext"; @@ -183,6 +183,35 @@ export function useSettingValue( }; } +export function isDefinitiveSettingMutationRejection(error: unknown): boolean { + // Ordinary 4xx responses reject the request before applying it. A 408 or + // 5xx can be emitted by the server or a gateway after the mutation reached + // the handler, so those remain ambiguous and require reconciliation. + return ( + error instanceof ApiClientError && + error.status >= 400 && + error.status < 500 && + error.status !== 408 + ); +} + +function shouldReconcileAfterMutationError(error: unknown): boolean { + return !isDefinitiveSettingMutationRejection(error); +} + +function invalidateSettingValueQueries( + queryClient: ReturnType, + identity: SettingIdentity, +) { + void queryClient.invalidateQueries({ queryKey: [...settingsKeys.all, "values"] }); + // A device-scoped write changes that device's "how many things differ" + // count, which the device list shows. Without this the badge stays stale + // until the list's own staleTime expires. + if (identity.scope === "profile_device") { + void queryClient.invalidateQueries({ queryKey: deviceKeys.all }); + } +} + /** Write one value at one scope. */ export function useSetSettingValue() { const qc = useQueryClient(); @@ -208,13 +237,12 @@ export function useSetSettingValue() { }, body: JSON.stringify({ value }), }), - onSettled: (_data, _error, variables) => { - void qc.invalidateQueries({ queryKey: [...settingsKeys.all, "values"] }); - // A device-scoped write changes that device's "how many things differ" - // count, which the device list shows. Without this the badge stays stale - // until the list's own staleTime expires. - if (variables.identity.scope === "profile_device") { - void qc.invalidateQueries({ queryKey: deviceKeys.all }); + onSuccess: (_data, variables) => { + invalidateSettingValueQueries(qc, variables.identity); + }, + onError: (error, variables) => { + if (shouldReconcileAfterMutationError(error)) { + invalidateSettingValueQueries(qc, variables.identity); } }, }); @@ -227,10 +255,17 @@ export function useClearSettingValue() { return useMutation({ mutationFn: ({ key, identity }: { key: SettingKey; identity: SettingIdentity }) => api(`/settings/values/${key}?${identityQuery(identity)}`, { method: "DELETE" }), - onSettled: (_data, _error, variables) => { - void qc.invalidateQueries({ queryKey: [...settingsKeys.all, "values"] }); - if (variables.identity.scope === "profile_device") { - void qc.invalidateQueries({ queryKey: deviceKeys.all }); + onSuccess: (_data, variables) => { + invalidateSettingValueQueries(qc, variables.identity); + }, + onError: (error, variables) => { + // DELETE is idempotent for reset callers: a 404 means another client + // already cleared the value, so stale effective caches must catch up. + if ( + shouldReconcileAfterMutationError(error) || + (error instanceof ApiClientError && error.status === 404) + ) { + invalidateSettingValueQueries(qc, variables.identity); } }, }); diff --git a/web/src/hooks/queries/settingValuesMutations.test.tsx b/web/src/hooks/queries/settingValuesMutations.test.tsx new file mode 100644 index 00000000..b38e896e --- /dev/null +++ b/web/src/hooks/queries/settingValuesMutations.test.tsx @@ -0,0 +1,193 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { act, renderHook } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { ApiClientError } from "@/api/client"; +import { SETTING_KEYS } from "@/lib/settingsContract"; +import { deviceKeys, settingsKeys } from "./keys"; +import { useClearSettingValue, useSetSettingValue } from "./settingValues"; + +const apiMock = vi.hoisted(() => vi.fn()); +vi.mock("@/api/client", async () => { + const actual = await vi.importActual("@/api/client"); + return { ...actual, api: apiMock }; +}); + +function createHarness() { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + const wrapper = ({ children }: { children: ReactNode }) => ( + {children} + ); + return { queryClient, wrapper }; +} + +describe("typed setting mutations", () => { + beforeEach(() => { + apiMock.mockReset(); + }); + + it("does not invalidate effective settings after a definitive rejected write", async () => { + apiMock.mockRejectedValueOnce(new ApiClientError(429, "rate_limited", "rate limited")); + const { queryClient, wrapper } = createHarness(); + const invalidateQueries = vi.spyOn(queryClient, "invalidateQueries"); + const { result } = renderHook(() => useSetSettingValue(), { wrapper }); + + await act(async () => { + await expect( + result.current.mutateAsync({ + key: SETTING_KEYS.UI_LIBRARY_PAGE_STATE, + value: { version: 1, libraries: {} }, + identity: { scope: "profile_device" }, + }), + ).rejects.toThrow("rate limited"); + }); + + expect(invalidateQueries).not.toHaveBeenCalled(); + }); + + it("invalidates effective settings and device summaries after a successful device write", async () => { + apiMock.mockResolvedValueOnce({}); + const { queryClient, wrapper } = createHarness(); + const invalidateQueries = vi.spyOn(queryClient, "invalidateQueries"); + const { result } = renderHook(() => useSetSettingValue(), { wrapper }); + + await act(async () => { + await result.current.mutateAsync({ + key: SETTING_KEYS.UI_LIBRARY_PAGE_STATE, + value: { version: 1, libraries: {} }, + identity: { scope: "profile_device" }, + }); + }); + + expect(invalidateQueries).toHaveBeenCalledWith({ + queryKey: [...settingsKeys.all, "values"], + }); + expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: deviceKeys.all }); + }); + + it("reconciles effective settings and device summaries after an ambiguous write failure", async () => { + apiMock.mockRejectedValueOnce(new TypeError("network connection lost")); + const { queryClient, wrapper } = createHarness(); + const invalidateQueries = vi.spyOn(queryClient, "invalidateQueries"); + const { result } = renderHook(() => useSetSettingValue(), { wrapper }); + + await act(async () => { + await expect( + result.current.mutateAsync({ + key: SETTING_KEYS.UI_LIBRARY_PAGE_STATE, + value: { version: 1, libraries: {} }, + identity: { scope: "profile_device" }, + }), + ).rejects.toThrow("network connection lost"); + }); + + expect(invalidateQueries).toHaveBeenCalledWith({ + queryKey: [...settingsKeys.all, "values"], + }); + expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: deviceKeys.all }); + }); + + it("reconciles effective settings and device summaries after a server write failure", async () => { + apiMock.mockRejectedValueOnce(new ApiClientError(503, "unavailable", "service unavailable")); + const { queryClient, wrapper } = createHarness(); + const invalidateQueries = vi.spyOn(queryClient, "invalidateQueries"); + const { result } = renderHook(() => useSetSettingValue(), { wrapper }); + + await act(async () => { + await expect( + result.current.mutateAsync({ + key: SETTING_KEYS.UI_LIBRARY_PAGE_STATE, + value: { version: 1, libraries: {} }, + identity: { scope: "profile_device" }, + }), + ).rejects.toThrow("service unavailable"); + }); + + expect(invalidateQueries).toHaveBeenCalledWith({ + queryKey: [...settingsKeys.all, "values"], + }); + expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: deviceKeys.all }); + }); + + it("does not invalidate effective settings after a definitive rejected clear", async () => { + apiMock.mockRejectedValueOnce(new ApiClientError(429, "rate_limited", "rate limited")); + const { queryClient, wrapper } = createHarness(); + const invalidateQueries = vi.spyOn(queryClient, "invalidateQueries"); + const { result } = renderHook(() => useClearSettingValue(), { wrapper }); + + await act(async () => { + await expect( + result.current.mutateAsync({ + key: SETTING_KEYS.UI_LIBRARY_PAGE_STATE, + identity: { scope: "profile_device" }, + }), + ).rejects.toThrow("rate limited"); + }); + + expect(invalidateQueries).not.toHaveBeenCalled(); + }); + + it("reconciles effective settings and device summaries after an already-cleared response", async () => { + apiMock.mockRejectedValueOnce(new ApiClientError(404, "not_found", "setting not found")); + const { queryClient, wrapper } = createHarness(); + const invalidateQueries = vi.spyOn(queryClient, "invalidateQueries"); + const { result } = renderHook(() => useClearSettingValue(), { wrapper }); + + await act(async () => { + await expect( + result.current.mutateAsync({ + key: SETTING_KEYS.UI_LIBRARY_PAGE_STATE, + identity: { scope: "profile_device" }, + }), + ).rejects.toThrow("setting not found"); + }); + + expect(invalidateQueries).toHaveBeenCalledWith({ + queryKey: [...settingsKeys.all, "values"], + }); + expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: deviceKeys.all }); + }); + + it("invalidates effective settings and device summaries after a successful device clear", async () => { + apiMock.mockResolvedValueOnce({}); + const { queryClient, wrapper } = createHarness(); + const invalidateQueries = vi.spyOn(queryClient, "invalidateQueries"); + const { result } = renderHook(() => useClearSettingValue(), { wrapper }); + + await act(async () => { + await result.current.mutateAsync({ + key: SETTING_KEYS.UI_LIBRARY_PAGE_STATE, + identity: { scope: "profile_device" }, + }); + }); + + expect(invalidateQueries).toHaveBeenCalledWith({ + queryKey: [...settingsKeys.all, "values"], + }); + expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: deviceKeys.all }); + }); + + it("reconciles effective settings and device summaries after an ambiguous clear failure", async () => { + apiMock.mockRejectedValueOnce(new TypeError("response connection lost")); + const { queryClient, wrapper } = createHarness(); + const invalidateQueries = vi.spyOn(queryClient, "invalidateQueries"); + const { result } = renderHook(() => useClearSettingValue(), { wrapper }); + + await act(async () => { + await expect( + result.current.mutateAsync({ + key: SETTING_KEYS.UI_LIBRARY_PAGE_STATE, + identity: { scope: "profile_device" }, + }), + ).rejects.toThrow("response connection lost"); + }); + + expect(invalidateQueries).toHaveBeenCalledWith({ + queryKey: [...settingsKeys.all, "values"], + }); + expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: deviceKeys.all }); + }); +}); diff --git a/web/src/pages/LibraryPage.test.tsx b/web/src/pages/LibraryPage.test.tsx new file mode 100644 index 00000000..3f1b4bae --- /dev/null +++ b/web/src/pages/LibraryPage.test.tsx @@ -0,0 +1,337 @@ +import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { MemoryRouter, Route, Routes, useLocation } from "react-router"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + ownerKey: "profile-1", + rememberEnabled: true, + savedSearch: "tab=library" as string | undefined, + saveLibrarySearch: vi.fn<(libraryId: number, search: string) => Promise>(), +})); + +vi.mock("@/hooks/queries/libraries", () => ({ + useUserLibraries: () => ({ + data: [{ id: 7, name: "Movies", type: "movie", sort_order: 0 }], + isLoading: false, + }), +})); + +vi.mock("@/hooks/queries/libraryPageState", () => ({ + libraryPageStateWriteRetryDelay: (error: unknown, fallbackDelayMs: number) => { + const status = + typeof error === "object" && error !== null && "status" in error + ? (error as { status?: unknown }).status + : undefined; + if ( + typeof status === "number" && + status >= 400 && + status < 500 && + status !== 408 && + status !== 425 && + status !== 429 + ) { + return null; + } + return fallbackDelayMs; + }, + useLibraryPageStatePreference: () => ({ + ownerKey: mocks.ownerKey, + isLoading: false, + preference: { + version: 1, + libraries: mocks.savedSearch === undefined ? {} : { "7": { search: mocks.savedSearch } }, + }, + rememberEnabled: mocks.rememberEnabled, + saveLibrarySearch: mocks.saveLibrarySearch, + }), +})); + +vi.mock("@/hooks/useDocumentTitle", () => ({ + useDocumentTitle: vi.fn(), +})); + +vi.mock("@/components/LibraryHeader", () => ({ + default: () =>
Library header
, +})); + +vi.mock("@/components/ui/tabs", () => ({ + Tabs: ({ children }: { children: ReactNode }) =>
{children}
, + TabsContent: ({ children }: { children: ReactNode }) =>
{children}
, +})); + +vi.mock("./LibraryRecommended", () => ({ + default: ({ onHeroStateChange }: { onHeroStateChange: (rendered: boolean) => void }) => ( + + ), +})); + +vi.mock("./LibraryBrowse", () => ({ + default: () =>
Library browse
, +})); + +vi.mock("./LibraryCollections", () => ({ + default: () =>
Library collections
, +})); + +import LibraryPage from "./LibraryPage"; + +function LocationProbe() { + return {useLocation().search}; +} + +function page(initialEntry = "/libraries/7?tab=library&sort=year&order=desc") { + return ( + + + + } /> + + + ); +} + +function renderPage(initialEntry?: string) { + return render(page(initialEntry)); +} + +describe("LibraryPage saved state", () => { + beforeEach(() => { + mocks.ownerKey = "profile-1"; + mocks.rememberEnabled = true; + mocks.savedSearch = "tab=library"; + mocks.saveLibrarySearch.mockReset(); + mocks.saveLibrarySearch.mockResolvedValue(); + }); + + it("submits one save while the cached value remains stale across unrelated rerenders", async () => { + renderPage(); + + await waitFor(() => expect(mocks.saveLibrarySearch).toHaveBeenCalledTimes(1)); + const firstCall = mocks.saveLibrarySearch.mock.calls[0]; + expect(firstCall).toBeDefined(); + const [, canonicalSearch] = firstCall!; + + fireEvent.click(screen.getByRole("button", { name: "Show hero" })); + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(mocks.saveLibrarySearch).toHaveBeenCalledTimes(1); + expect(new URLSearchParams(canonicalSearch)).toEqual( + new URLSearchParams("tab=library&sort=year&order=desc"), + ); + }); + + it("retries the same canonical search after a failed save", async () => { + vi.useFakeTimers(); + mocks.saveLibrarySearch.mockRejectedValueOnce(new Error("rate limited")); + try { + const view = renderPage(); + + expect(mocks.saveLibrarySearch).toHaveBeenCalledTimes(1); + const firstResult = mocks.saveLibrarySearch.mock.results[0]?.value; + expect(firstResult).toBeDefined(); + await expect(firstResult).rejects.toThrow("rate limited"); + + await act(async () => { + vi.advanceTimersByTime(2_000); + await Promise.resolve(); + }); + + expect(mocks.saveLibrarySearch).toHaveBeenCalledTimes(2); + expect(mocks.saveLibrarySearch.mock.calls[1]).toEqual(mocks.saveLibrarySearch.mock.calls[0]); + const retryResult = mocks.saveLibrarySearch.mock.results[1]?.value; + expect(retryResult).toBeDefined(); + await expect(retryResult).resolves.toBeUndefined(); + + view.rerender(page()); + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(mocks.saveLibrarySearch).toHaveBeenCalledTimes(2); + } finally { + vi.useRealTimers(); + } + }); + + it("stops retrying after two failed automatic retries", async () => { + vi.useFakeTimers(); + mocks.saveLibrarySearch.mockRejectedValue(new Error("rate limited")); + try { + renderPage(); + + expect(mocks.saveLibrarySearch).toHaveBeenCalledTimes(1); + await mocks.saveLibrarySearch.mock.results[0]?.value.catch(() => undefined); + + await act(async () => { + vi.advanceTimersByTime(2_000); + await Promise.resolve(); + }); + expect(mocks.saveLibrarySearch).toHaveBeenCalledTimes(2); + await mocks.saveLibrarySearch.mock.results[1]?.value.catch(() => undefined); + + await act(async () => { + vi.advanceTimersByTime(5_000); + await Promise.resolve(); + }); + expect(mocks.saveLibrarySearch).toHaveBeenCalledTimes(3); + await mocks.saveLibrarySearch.mock.results[2]?.value.catch(() => undefined); + + await act(async () => { + vi.advanceTimersByTime(60_000); + await Promise.resolve(); + }); + expect(mocks.saveLibrarySearch).toHaveBeenCalledTimes(3); + } finally { + vi.useRealTimers(); + } + }); + + it("cancels a scheduled retry when the saved value acknowledges the write", async () => { + vi.useFakeTimers(); + mocks.saveLibrarySearch.mockRejectedValueOnce(new Error("response connection lost")); + try { + const view = renderPage(); + + expect(mocks.saveLibrarySearch).toHaveBeenCalledTimes(1); + await mocks.saveLibrarySearch.mock.results[0]?.value.catch(() => undefined); + const canonicalSearch = mocks.saveLibrarySearch.mock.calls[0]?.[1]; + expect(canonicalSearch).toBeDefined(); + + mocks.savedSearch = canonicalSearch; + view.rerender(page()); + await act(async () => { + vi.advanceTimersByTime(60_000); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(mocks.saveLibrarySearch).toHaveBeenCalledTimes(1); + } finally { + vi.useRealTimers(); + } + }); + + it("retries a transient rate-limit rejection without looping", async () => { + vi.useFakeTimers(); + const rateLimited = Object.assign(new Error("rate limited"), { status: 429 }); + mocks.saveLibrarySearch.mockRejectedValueOnce(rateLimited); + try { + renderPage(); + + expect(mocks.saveLibrarySearch).toHaveBeenCalledTimes(1); + await mocks.saveLibrarySearch.mock.results[0]?.value.catch(() => undefined); + + await act(async () => { + vi.advanceTimersByTime(2_000); + await Promise.resolve(); + }); + expect(mocks.saveLibrarySearch).toHaveBeenCalledTimes(2); + await expect(mocks.saveLibrarySearch.mock.results[1]?.value).resolves.toBeUndefined(); + + await act(async () => { + vi.advanceTimersByTime(60_000); + await Promise.resolve(); + }); + expect(mocks.saveLibrarySearch).toHaveBeenCalledTimes(2); + } finally { + vi.useRealTimers(); + } + }); + + it("does not carry a terminal retry marker into a different profile", async () => { + const rejected = Object.assign(new Error("invalid setting"), { status: 422 }); + mocks.saveLibrarySearch.mockRejectedValueOnce(rejected); + const view = renderPage(); + + expect(mocks.saveLibrarySearch).toHaveBeenCalledTimes(1); + await mocks.saveLibrarySearch.mock.results[0]?.value.catch(() => undefined); + + mocks.ownerKey = "profile-2"; + view.rerender(page()); + + await waitFor(() => expect(mocks.saveLibrarySearch).toHaveBeenCalledTimes(2)); + await expect(mocks.saveLibrarySearch.mock.results[1]?.value).resolves.toBeUndefined(); + }); + + it("does not carry an in-flight submission marker into a different profile", async () => { + let resolveFirst: (() => void) | undefined; + mocks.saveLibrarySearch.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirst = resolve; + }), + ); + const view = renderPage(); + + expect(mocks.saveLibrarySearch).toHaveBeenCalledTimes(1); + + mocks.ownerKey = "profile-2"; + view.rerender(page()); + + await waitFor(() => expect(mocks.saveLibrarySearch).toHaveBeenCalledTimes(2)); + await expect(mocks.saveLibrarySearch.mock.results[1]?.value).resolves.toBeUndefined(); + resolveFirst?.(); + await expect(mocks.saveLibrarySearch.mock.results[0]?.value).resolves.toBeUndefined(); + }); + + it("rehydrates URL state instead of copying it into a different profile", async () => { + mocks.savedSearch = "tab=collections"; + const view = renderPage("/libraries/7"); + + await waitFor(() => + expect(screen.getByTestId("location-search")).toHaveTextContent("?tab=collections"), + ); + expect(mocks.saveLibrarySearch).not.toHaveBeenCalled(); + + mocks.ownerKey = "profile-2"; + mocks.savedSearch = "tab=library&sort=year&order=desc"; + view.rerender(page("/libraries/7")); + + await waitFor(() => + expect(screen.getByTestId("location-search")).toHaveTextContent( + "?tab=library&sort=year&order=desc", + ), + ); + expect(mocks.saveLibrarySearch).not.toHaveBeenCalledWith(7, "tab=collections"); + }); + + it("clears inherited URL state when the new profile has no saved value", async () => { + mocks.savedSearch = "tab=collections"; + const view = renderPage("/libraries/7"); + + await waitFor(() => + expect(screen.getByTestId("location-search")).toHaveTextContent("?tab=collections"), + ); + + mocks.ownerKey = "profile-2"; + mocks.savedSearch = undefined; + view.rerender(page("/libraries/7")); + + await waitFor(() => expect(screen.getByTestId("location-search")).toBeEmptyDOMElement()); + expect(mocks.saveLibrarySearch).not.toHaveBeenCalledWith(7, "tab=collections"); + }); + + it("clears inherited URL state when the new profile disables remembering", async () => { + mocks.savedSearch = "tab=collections"; + const view = renderPage("/libraries/7"); + + await waitFor(() => + expect(screen.getByTestId("location-search")).toHaveTextContent("?tab=collections"), + ); + + mocks.ownerKey = "profile-2"; + mocks.rememberEnabled = false; + mocks.savedSearch = "tab=library&sort=year&order=desc"; + view.rerender(page("/libraries/7")); + + await waitFor(() => expect(screen.getByTestId("location-search")).toBeEmptyDOMElement()); + expect(mocks.saveLibrarySearch).not.toHaveBeenCalledWith(7, "tab=collections"); + }); +}); diff --git a/web/src/pages/LibraryPage.tsx b/web/src/pages/LibraryPage.tsx index 1fd01325..ee98a342 100644 --- a/web/src/pages/LibraryPage.tsx +++ b/web/src/pages/LibraryPage.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useRef, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { Link, useParams, useSearchParams } from "react-router"; import type { QueryDefinition } from "@/api/types"; import { Tabs, TabsContent } from "@/components/ui/tabs"; @@ -9,7 +9,10 @@ import { useDocumentTitle } from "@/hooks/useDocumentTitle"; import LibraryRecommended from "./LibraryRecommended"; import LibraryBrowse from "./LibraryBrowse"; import LibraryCollections from "./LibraryCollections"; -import { useLibraryPageStatePreference } from "@/hooks/queries/libraryPageState"; +import { + libraryPageStateWriteRetryDelay, + useLibraryPageStatePreference, +} from "@/hooks/queries/libraryPageState"; import { applySavedLibraryPageSearchParams, hasLibraryPageSearchParams, @@ -19,47 +22,86 @@ import { type LibraryBrowseType, } from "./libraryPageSearchParams"; +const LIBRARY_SAVE_RETRY_DELAYS_MS = [2_000, 5_000] as const; + +interface LibrarySaveRetry { + key: string; + failures: number; + ready: boolean; + timeout?: ReturnType; +} + +interface HydratedLibrarySearch { + ownerKey: string | null; + libraryId: number; + search: string; +} + export default function LibraryPage() { const { libraryId } = useParams<{ libraryId: string }>(); const [searchParams, setSearchParams] = useSearchParams(); const { data: libraries, isLoading } = useUserLibraries(); const { + ownerKey: libraryPageStateOwnerKey, isLoading: libraryPageStateLoading, preference: libraryPageStatePreference, rememberEnabled: rememberLibraryPageState, saveLibrarySearch, } = useLibraryPageStatePreference(); - const savedStateHydratedLibraryIdRef = useRef(null); + const [savedStateHydratedKey, setSavedStateHydratedKey] = useState(null); + const [hydratedLibrarySearch, setHydratedLibrarySearch] = useState( + null, + ); const applyingSavedSearchParamsRef = useRef(null); - const applyingSavedSearchParamsLibraryIdRef = useRef(null); + const applyingSavedSearchParamsKeyRef = useRef(null); + const submittedLibrarySearchRef = useRef<{ key: string } | null>(null); + const librarySaveRetryRef = useRef(null); + const [librarySaveRetryNonce, setLibrarySaveRetryNonce] = useState(0); + + useEffect(() => { + return () => { + const retry = librarySaveRetryRef.current; + if (retry?.timeout !== undefined) { + clearTimeout(retry.timeout); + } + }; + }, []); const id = Number(libraryId); + const libraryPageStateKey = `${libraryPageStateOwnerKey ?? "none"}:${id}`; const library = libraries?.find((l) => l.id === id); const libraryType = library?.type ?? ""; const savedLibrarySearch = Number.isFinite(id) && id > 0 ? libraryPageStatePreference.libraries[String(id)]?.search : undefined; + const currentLibrarySearch = serializeLibraryPageSearchParams(searchParams); + const hasInheritedHydratedSearch = + hydratedLibrarySearch !== null && + hydratedLibrarySearch.ownerKey !== libraryPageStateOwnerKey && + hydratedLibrarySearch.libraryId === id && + hydratedLibrarySearch.search === currentLibrarySearch; + const hasUnhydratedLibraryState = + Boolean(libraryType) && + Number.isFinite(id) && + id > 0 && + savedStateHydratedKey !== libraryPageStateKey; const shouldApplySavedLibrarySearch = - Boolean(libraryType) && - Number.isFinite(id) && - id > 0 && + hasUnhydratedLibraryState && !libraryPageStateLoading && - rememberLibraryPageState && - savedStateHydratedLibraryIdRef.current !== id && - savedLibrarySearch != null && - !hasLibraryPageSearchParams(searchParams) && - savedLibrarySearch !== serializeLibraryPageSearchParams(searchParams); + ((hasInheritedHydratedSearch && !rememberLibraryPageState) || + (rememberLibraryPageState && + (!hasLibraryPageSearchParams(searchParams) || hasInheritedHydratedSearch) && + ((savedLibrarySearch != null && savedLibrarySearch !== currentLibrarySearch) || + hasInheritedHydratedSearch))); const shouldWaitForSavedLibrarySearch = - Boolean(libraryType) && - Number.isFinite(id) && - id > 0 && + hasUnhydratedLibraryState && libraryPageStateLoading && - savedStateHydratedLibraryIdRef.current !== id && - !hasLibraryPageSearchParams(searchParams); - const { activeTab, browseType, queryDefinition } = parseLibraryPageState( - searchParams, - libraryType, + (!hasLibraryPageSearchParams(searchParams) || hasInheritedHydratedSearch); + const searchParamsKey = searchParams.toString(); + const { activeTab, browseType, queryDefinition } = useMemo( + () => parseLibraryPageState(new URLSearchParams(searchParamsKey), libraryType), + [libraryType, searchParamsKey], ); // Tracks whether LibraryRecommended is currently rendering a hero banner. @@ -74,45 +116,71 @@ export default function LibraryPage() { useDocumentTitle(library?.name ?? "Library"); + /* eslint-disable react-hooks/set-state-in-effect -- hydration provenance is synchronized with router state */ + useEffect(() => { + if ( + applyingSavedSearchParamsRef.current === null && + hydratedLibrarySearch?.ownerKey === libraryPageStateOwnerKey && + hydratedLibrarySearch.libraryId === id && + hydratedLibrarySearch.search !== currentLibrarySearch + ) { + // The URL no longer matches what this profile hydrated, so a later + // profile switch must treat it as an explicit user choice. + setHydratedLibrarySearch(null); + } + }, [currentLibrarySearch, hydratedLibrarySearch, id, libraryPageStateOwnerKey]); + useEffect(() => { if ( !libraryType || !Number.isFinite(id) || id <= 0 || libraryPageStateLoading || - !rememberLibraryPageState || - savedStateHydratedLibraryIdRef.current === id || + savedStateHydratedKey === libraryPageStateKey || !shouldApplySavedLibrarySearch ) { return; } - savedStateHydratedLibraryIdRef.current = id; - const nextSearchParams = applySavedLibraryPageSearchParams(searchParams, savedLibrarySearch); + setSavedStateHydratedKey(libraryPageStateKey); + const nextSearchParams = applySavedLibraryPageSearchParams( + searchParams, + rememberLibraryPageState ? (savedLibrarySearch ?? "") : "", + ); + const hydratedSearch = serializeLibraryPageSearchParams(nextSearchParams); + setHydratedLibrarySearch({ + ownerKey: libraryPageStateOwnerKey, + libraryId: id, + search: hydratedSearch, + }); if (nextSearchParams.toString() !== searchParams.toString()) { - applyingSavedSearchParamsRef.current = serializeLibraryPageSearchParams(nextSearchParams); - applyingSavedSearchParamsLibraryIdRef.current = id; + applyingSavedSearchParamsRef.current = hydratedSearch; + applyingSavedSearchParamsKeyRef.current = libraryPageStateKey; setSearchParams(nextSearchParams, { replace: true }); } }, [ id, + libraryPageStateKey, libraryPageStateLoading, + libraryPageStateOwnerKey, libraryType, rememberLibraryPageState, + savedStateHydratedKey, savedLibrarySearch, searchParams, setSearchParams, shouldApplySavedLibrarySearch, ]); + /* eslint-enable react-hooks/set-state-in-effect */ useEffect(() => { if (!libraryType || shouldApplySavedLibrarySearch) { return; } - if (applyingSavedSearchParamsLibraryIdRef.current !== id) { + if (applyingSavedSearchParamsKeyRef.current !== libraryPageStateKey) { applyingSavedSearchParamsRef.current = null; - applyingSavedSearchParamsLibraryIdRef.current = id; + applyingSavedSearchParamsKeyRef.current = libraryPageStateKey; } const normalizedSearchParams = updateLibraryPageSearchParams( @@ -127,6 +195,8 @@ export default function LibraryPage() { }, [ activeTab, browseType, + id, + libraryPageStateKey, queryDefinition, libraryType, searchParams, @@ -156,20 +226,112 @@ export default function LibraryPage() { } const canonicalSearch = serializeLibraryPageSearchParams(normalizedSearchParams); + const retryKey = `${libraryPageStateKey}:${canonicalSearch}`; + const pendingRetry = librarySaveRetryRef.current; + if (pendingRetry !== null && pendingRetry.key !== retryKey) { + if (pendingRetry.timeout !== undefined) { + clearTimeout(pendingRetry.timeout); + } + librarySaveRetryRef.current = null; + } if (applyingSavedSearchParamsRef.current != null) { if (applyingSavedSearchParamsRef.current === canonicalSearch) { applyingSavedSearchParamsRef.current = null; } return; } - if (savedLibrarySearch !== canonicalSearch) { - saveLibrarySearch(id, canonicalSearch); + // Mutation state and cache invalidation can rerender before the effective + // read catches up. Treat one canonical URL as one logical submission. + const submitted = submittedLibrarySearchRef.current; + if (savedLibrarySearch === canonicalSearch) { + const retry = librarySaveRetryRef.current; + if (retry?.key === retryKey) { + if (retry.timeout !== undefined) { + clearTimeout(retry.timeout); + } + librarySaveRetryRef.current = null; + } + if (submitted?.key === retryKey) { + submittedLibrarySearchRef.current = null; + } + return; } + if (submitted?.key === retryKey) { + return; + } + const retry = librarySaveRetryRef.current; + if (retry?.key === retryKey) { + if (!retry.ready) { + return; + } + retry.ready = false; + } + + const nextSubmission = { key: retryKey }; + submittedLibrarySearchRef.current = nextSubmission; + void saveLibrarySearch(id, canonicalSearch).then( + () => { + const completedRetry = librarySaveRetryRef.current; + if (completedRetry?.key === retryKey) { + if (completedRetry.timeout !== undefined) { + clearTimeout(completedRetry.timeout); + } + librarySaveRetryRef.current = null; + } + }, + (error: unknown) => { + if (submittedLibrarySearchRef.current !== nextSubmission) { + return; + } + submittedLibrarySearchRef.current = null; + + const previousRetry = librarySaveRetryRef.current; + const failures = previousRetry?.key === retryKey ? previousRetry.failures : 0; + if (failures >= LIBRARY_SAVE_RETRY_DELAYS_MS.length) { + librarySaveRetryRef.current = { + key: retryKey, + failures, + ready: false, + }; + return; + } + + const fallbackRetryDelay = LIBRARY_SAVE_RETRY_DELAYS_MS[failures]; + if (fallbackRetryDelay === undefined) { + return; + } + const retryDelay = libraryPageStateWriteRetryDelay(error, fallbackRetryDelay); + if (retryDelay === null) { + librarySaveRetryRef.current = { + key: retryKey, + failures: LIBRARY_SAVE_RETRY_DELAYS_MS.length, + ready: false, + }; + return; + } + const nextRetry: LibrarySaveRetry = { + key: retryKey, + failures: failures + 1, + ready: false, + }; + nextRetry.timeout = setTimeout(() => { + if (librarySaveRetryRef.current !== nextRetry) { + return; + } + nextRetry.timeout = undefined; + nextRetry.ready = true; + setLibrarySaveRetryNonce((nonce) => nonce + 1); + }, retryDelay); + librarySaveRetryRef.current = nextRetry; + }, + ); }, [ activeTab, browseType, id, libraryPageStateLoading, + libraryPageStateKey, + librarySaveRetryNonce, libraryType, queryDefinition, rememberLibraryPageState,