From 6bfdbd9db72e06f18f8aa9aaf57b025bff55ce5b Mon Sep 17 00:00:00 2001 From: cesnimda Date: Tue, 10 Nov 2026 17:44:51 +0100 Subject: [PATCH] perf(companies): integrate verified owner cache invalidation --- docs/performance/PERF-01_COMPANIES_CACHE.md | 11 +++ .../evidence/perf-company-cache/LINT.txt | 1 + .../evidence/perf-company-cache/TESTS.txt | Bin 0 -> 1362 bytes .../evidence/perf-company-cache/TYPECHECK.txt | Bin 0 -> 150 bytes .../src/account-scoped-state.test.tsx | 72 +++++++++++++++++- job-tracker-ui/src/hooks/useCompanies.ts | 8 +- 6 files changed, 81 insertions(+), 11 deletions(-) create mode 100644 docs/performance/PERF-01_COMPANIES_CACHE.md create mode 100644 docs/ui-modernisation/evidence/perf-company-cache/LINT.txt create mode 100644 docs/ui-modernisation/evidence/perf-company-cache/TESTS.txt create mode 100644 docs/ui-modernisation/evidence/perf-company-cache/TYPECHECK.txt diff --git a/docs/performance/PERF-01_COMPANIES_CACHE.md b/docs/performance/PERF-01_COMPANIES_CACHE.md new file mode 100644 index 00000000..c088c090 --- /dev/null +++ b/docs/performance/PERF-01_COMPANIES_CACHE.md @@ -0,0 +1,11 @@ +# PERF-01 Companies cache + +Base `fd524a4b9d55d671d4f9c93134057d42f9cd6373`. The browser's Companies cache is scoped to the current owner. A response that started before invalidation could finish after its replacement and overwrite the fresh list. A pre-logout response could also write after signing back in as the same account. Separately, a mounting consumer could publish its initial empty array as cached data before the network request finished, so another consumer could miss the shared result. + +Only the current in-flight response can now populate the cache. It must still match the active owner. The resource-state effect no longer writes a second copy into the shared cache. Existing readers, invalidation callers, request URL and server authorisation are unchanged. + +Before the fix, the three added cases failed on the stale post-edit list, a missed shared response and the stale pre-logout list. After the fix, the two simultaneous readers both received the shared result with exactly one request. The post-invalidation check used exactly two requests across the old and new generations, with no extra request on a warm mount. Re-login and account switching checks also passed. + +**Verification:** 16 tests across the account-scoped state, Edit Job revision and theme state suites passed. Targeted lint and TypeScript passed. The test worktree reused the installed dependency tree from the matching A3 lockfile. These are mock request counts, with no browser or production latency measurement. + +The A3 integration used the exact source and regression delta from `21db35d2b965fbda2ddf5b419ac2646fcae05517`. Both input files matched its parent at A3 base `bca750446f1069eb55d648be838be192d70c2d30`. The fresh A3 run passed 37 tests across account-scoped state, JobTable density and Jobs integration, plus type-check and scoped lint. These runs overlap. [A3 receipt](../ui-modernisation/evidence/perf-company-cache/TESTS.txt) diff --git a/docs/ui-modernisation/evidence/perf-company-cache/LINT.txt b/docs/ui-modernisation/evidence/perf-company-cache/LINT.txt new file mode 100644 index 00000000..05963bf7 --- /dev/null +++ b/docs/ui-modernisation/evidence/perf-company-cache/LINT.txt @@ -0,0 +1 @@ +PASSED ESLint with zero warnings for useCompanies.ts and account-scoped-state.test.tsx diff --git a/docs/ui-modernisation/evidence/perf-company-cache/TESTS.txt b/docs/ui-modernisation/evidence/perf-company-cache/TESTS.txt new file mode 100644 index 0000000000000000000000000000000000000000..acb9134a87358a4e57ae37fd2451b047d9c892f7 GIT binary patch literal 1362 zcmd6nOG_L<6ot<^kpIxQkciI22Pnxxl8AyEF^;=LWNh2XsPmw!d_l-hkAA0m+8!T> zm`x~dcis27Rrl8I-}ic_b!9r$NJnZZs#I6G&Q!2#=|X*d(W2T~f)=riXae8Hv*I(X zTlidPsBP#QafUj^!|`(BTNAMfi5nB=IB^jRM{VE7_Aco90j@6i&arCYRS+qFEb8V3 zuYm0(R)wzE^;7mjy;Na@A0c^~({-7K=$)wzT|=2ZL)LsHZw3}eERZh2<@!YxW#Viz zk8Bp2(FU@g+C?{-&^B==iUGW&z7QK7Uy3u!b-E{YLrcL7eZcDJb+bdM==sw+7P|=7?1H3#9jdhj7 z-+in+Q5k4GZ`Fi3&I&ZfGq7Bhky{oMa^slx4=HzZKDJ$ZHiD%OV{|~*C0C%2eJV|? zYUV9#^7X05oj;w+`&II_wah#52`(gbd#Q520zSd!{jk2NHcKZI(;RuOdRV^w-S|EA Qp1uCvsJ-6yZdB8C2RH`nmjD0& literal 0 HcmV?d00001 diff --git a/docs/ui-modernisation/evidence/perf-company-cache/TYPECHECK.txt b/docs/ui-modernisation/evidence/perf-company-cache/TYPECHECK.txt new file mode 100644 index 0000000000000000000000000000000000000000..922304313e494a8e3a29322d2934e0370c471f93 GIT binary patch literal 150 zcmXwyOAf*?3`E}=sdvZ)g31LD8xB&URD{rv8nfW?g-Ha9v8^}u__KE$IOBM*aKmG4 z3RmjG|Gq>CaQ%*m@DG GIaOcV;~41x literal 0 HcmV?d00001 diff --git a/job-tracker-ui/src/account-scoped-state.test.tsx b/job-tracker-ui/src/account-scoped-state.test.tsx index aafe970e..4a76eaf6 100644 --- a/job-tracker-ui/src/account-scoped-state.test.tsx +++ b/job-tracker-ui/src/account-scoped-state.test.tsx @@ -1,11 +1,11 @@ import React from "react"; import "@testing-library/jest-dom"; -import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; import { api } from "./api"; -import { setAuthUserKey } from "./auth"; +import { clearAuthClientState, setAuthUserKey } from "./auth"; import SavedViewsMenu from "./components/SavedViewsMenu"; -import { useCompanies } from "./hooks/useCompanies"; +import { invalidateCompaniesCache, useCompanies } from "./hooks/useCompanies"; import { I18nProvider } from "./i18n/I18nProvider"; const mockedApi = api as jest.Mocked; @@ -15,10 +15,20 @@ function CompaniesProbe() { return
{companies.map((company) => company.name).join(", ") || "No companies"}
; } +function holdCompanyRead() { + let finish!: (value: { data: { id: number; name: string }[] }) => void; + const response = new Promise<{ data: { id: number; name: string }[] }>(resolve => { + finish = resolve; + }); + mockedApi.get.mockReturnValueOnce(response as ReturnType); + return (name: string) => finish({ data: [{ id: 1, name }] }); +} + describe("account-scoped browser state", () => { beforeEach(() => { window.localStorage.clear(); jest.clearAllMocks(); + invalidateCompaniesCache(); }); it("keeps saved views private to the account that created them", async () => { @@ -54,9 +64,63 @@ describe("account-scoped browser state", () => { expect(await screen.findByText("User A Company")).toBeInTheDocument(); mockedApi.get.mockResolvedValueOnce({ data: [{ id: 2, name: "User B Company" }] } as any); - setAuthUserKey("user-b", false); + act(() => setAuthUserKey("user-b", false)); await waitFor(() => expect(screen.getByText("User B Company")).toBeInTheDocument()); expect(screen.queryByText("User A Company")).not.toBeInTheDocument(); }); + + it("does not let a pre-edit company response restore stale cache", async () => { + setAuthUserKey("user-a", false); + const finishOldRead = holdCompanyRead(); + const oldConsumer = render(); + await waitFor(() => expect(mockedApi.get).toHaveBeenCalledTimes(1)); + + invalidateCompaniesCache(); + const finishFreshRead = holdCompanyRead(); + const freshConsumer = render(); + expect(mockedApi.get).toHaveBeenCalledTimes(2); + await act(async () => finishFreshRead("Current company")); + expect(freshConsumer.container).toHaveTextContent("Current company"); + + await act(async () => finishOldRead("Before the edit")); + oldConsumer.unmount(); + freshConsumer.unmount(); + const later = render(); + expect(later.container).toHaveTextContent("Current company"); + expect(mockedApi.get).toHaveBeenCalledTimes(2); + }); + + it("shares one pending company read between callers without treating initial empty state as cached", async () => { + setAuthUserKey("user-a", false); + const finishRead = holdCompanyRead(); + const first = render(); + const second = render(); + expect(mockedApi.get).toHaveBeenCalledTimes(1); + + await act(async () => finishRead("Shared company")); + expect(first.container).toHaveTextContent("Shared company"); + expect(second.container).toHaveTextContent("Shared company"); + expect(mockedApi.get).toHaveBeenCalledTimes(1); + }); + + it("does not restore a pending company response from before logout after signing back in", async () => { + setAuthUserKey("user-a", false); + const finishOldRead = holdCompanyRead(); + const oldConsumer = render(); + await waitFor(() => expect(mockedApi.get).toHaveBeenCalledTimes(1)); + + mockedApi.get.mockResolvedValueOnce({ data: [] } as Awaited>); + act(() => clearAuthClientState(false)); + await waitFor(() => expect(mockedApi.get).toHaveBeenCalledTimes(2)); + mockedApi.get.mockResolvedValueOnce({ data: [{ id: 1, name: "New session company" }] } as Awaited>); + act(() => setAuthUserKey("user-a", false)); + expect(await screen.findByText("New session company")).toBeInTheDocument(); + + await act(async () => finishOldRead("Previous session company")); + oldConsumer.unmount(); + const later = render(); + expect(later.container).toHaveTextContent("New session company"); + expect(mockedApi.get).toHaveBeenCalledTimes(3); + }); }); diff --git a/job-tracker-ui/src/hooks/useCompanies.ts b/job-tracker-ui/src/hooks/useCompanies.ts index bcab6ccb..0bcf7fd9 100644 --- a/job-tracker-ui/src/hooks/useCompanies.ts +++ b/job-tracker-ui/src/hooks/useCompanies.ts @@ -31,7 +31,7 @@ async function fetchCompanies(): Promise { const request = api .get('/companies') .then((r) => { - if (cacheOwner === owner && getAuthUserKey() === owner) cachedCompanies = r.data; + if (inflight === request && cacheOwner === owner && getAuthUserKey() === owner) cachedCompanies = r.data; return r.data; }) .finally(() => { @@ -61,12 +61,6 @@ export function useCompanies(): { deps: [cacheBust], }); - useEffect(() => { - if (!resource.error) { - cachedCompanies = resource.data; - } - }, [resource.data, resource.error]); - useEffect(() => { const reloadForAccount = () => { resetForOwner(getAuthUserKey());