Compare commits

...

1 Commits

Author SHA1 Message Date
cesnimda 21db35d2b9 fix(perf): keep superseded company reads out of cache 2026-11-10 17:40:51 +01:00
3 changed files with 78 additions and 11 deletions
@@ -0,0 +1,9 @@
# 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.
@@ -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<typeof api>;
@@ -15,10 +15,20 @@ function CompaniesProbe() {
return <div>{companies.map((company) => company.name).join(", ") || "No companies"}</div>;
}
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<typeof api.get>);
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(<CompaniesProbe />);
await waitFor(() => expect(mockedApi.get).toHaveBeenCalledTimes(1));
invalidateCompaniesCache();
const finishFreshRead = holdCompanyRead();
const freshConsumer = render(<CompaniesProbe />);
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(<CompaniesProbe />);
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(<CompaniesProbe />);
const second = render(<CompaniesProbe />);
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(<CompaniesProbe />);
await waitFor(() => expect(mockedApi.get).toHaveBeenCalledTimes(1));
mockedApi.get.mockResolvedValueOnce({ data: [] } as Awaited<ReturnType<typeof api.get>>);
act(() => clearAuthClientState(false));
await waitFor(() => expect(mockedApi.get).toHaveBeenCalledTimes(2));
mockedApi.get.mockResolvedValueOnce({ data: [{ id: 1, name: "New session company" }] } as Awaited<ReturnType<typeof api.get>>);
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(<CompaniesProbe />);
expect(later.container).toHaveTextContent("New session company");
expect(mockedApi.get).toHaveBeenCalledTimes(3);
});
});
+1 -7
View File
@@ -31,7 +31,7 @@ async function fetchCompanies(): Promise<Company[]> {
const request = api
.get<Company[]>('/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());