fix(catalog): keep the live catalog truthful after every import path
Gitea Actions Runner Test / test-job (push) Successful in 0s
CI / check (push) Successful in 29s
CI / tests-integration (push) Failing after 1m43s
CI / tests-unit (push) Successful in 1m47s
CI / tests-ui (push) Successful in 2m33s
CI / preflight (push) Skipped
CI / deploy (push) Skipped
Gitea Actions Runner Test / test-job (push) Successful in 0s
CI / check (push) Successful in 29s
CI / tests-integration (push) Failing after 1m43s
CI / tests-unit (push) Successful in 1m47s
CI / tests-ui (push) Successful in 2m33s
CI / preflight (push) Skipped
CI / deploy (push) Skipped
The live catalog store only covered part of the import surface. A durable job settled, a sync queue drained, a .nitro upload or a clone run left the Studio rail and the stats bar showing pre-import numbers until the page was reloaded, and the Catalog Manager kept a second tree that never saw writes made elsewhere in the session. Every one of those paths now pulls the tree again, and the refresh carries the totals with it: importing writes catalog rows server-side, so the counts the store holds were stale for the rest of the session. - refreshCatalogTree shares one request between concurrent callers and queues a single follow-up read when a write lands mid-flight, so a burst of edits costs at most one extra read. - useFurnitureJobs treats its first payload as a baseline, so a page load no longer replays every past import as "just settled", and hands the settled jobs to the callback. - The Catalog Manager pushes its own mutations into the store and re-reads its active tab when the store changes. - The 30s unstable_cache on the admin totals is now tagged and invalidated from every catalog write, including the import worker, so it no longer survives an import even across a hard reload. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
1 parent
d73baf1458
commit
28ce0f911c
22 files changed
+445
-70
No files matched your search
@@ -7,6 +7,7 @@ import {
|
||||
type CreatedCatalogPage,
|
||||
EMPTY_CATALOG_DELTA,
|
||||
isEmptyCatalogDelta,
|
||||
normalizeCatalogTotals,
|
||||
normalizeTreePages,
|
||||
recomputeDepth,
|
||||
} from "./live-catalog-merge";
|
||||
@@ -238,3 +239,41 @@ describe("isEmptyCatalogDelta", () => {
|
||||
expect(isEmptyCatalogDelta(EMPTY_CATALOG_DELTA)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("normalizeCatalogTotals", () => {
|
||||
it("accepts a totals payload from the tree route", () => {
|
||||
expect(
|
||||
normalizeCatalogTotals({
|
||||
totalPages: 12,
|
||||
enabledPages: 9,
|
||||
totalItems: 400,
|
||||
}),
|
||||
).toEqual({ totalPages: 12, enabledPages: 9, totalItems: 400 });
|
||||
});
|
||||
|
||||
it("coerces string counts the way MariaDB drivers return them", () => {
|
||||
expect(
|
||||
normalizeCatalogTotals({
|
||||
totalPages: "12",
|
||||
enabledPages: "9",
|
||||
totalItems: "400",
|
||||
}),
|
||||
).toEqual({ totalPages: 12, enabledPages: 9, totalItems: 400 });
|
||||
});
|
||||
|
||||
it("accepts an explicit zeroed payload for an empty catalog", () => {
|
||||
expect(
|
||||
normalizeCatalogTotals({
|
||||
totalPages: 0,
|
||||
enabledPages: 0,
|
||||
totalItems: 0,
|
||||
}),
|
||||
).toEqual({ totalPages: 0, enabledPages: 0, totalItems: 0 });
|
||||
});
|
||||
|
||||
it("rejects a missing or partial payload so the store keeps its counts", () => {
|
||||
expect(normalizeCatalogTotals(undefined)).toBeNull();
|
||||
expect(normalizeCatalogTotals("nope")).toBeNull();
|
||||
expect(normalizeCatalogTotals({ totalPages: 3 })).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -51,6 +51,28 @@ export interface CatalogTotals {
|
||||
totalItems: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Coerce an API totals payload into CatalogTotals. An explicit all-zero payload
|
||||
* is valid (an empty catalog); a missing or partial one returns null so callers
|
||||
* keep whatever the store already holds.
|
||||
*/
|
||||
export function normalizeCatalogTotals(raw: unknown): CatalogTotals | null {
|
||||
if (!raw || typeof raw !== "object") return null;
|
||||
const row = raw as Record<string, unknown>;
|
||||
const { totalPages, totalItems, enabledPages } = row;
|
||||
if (
|
||||
totalPages === undefined ||
|
||||
totalItems === undefined ||
|
||||
enabledPages === undefined
|
||||
)
|
||||
return null;
|
||||
return {
|
||||
totalPages: toCount(totalPages),
|
||||
totalItems: toCount(totalItems),
|
||||
enabledPages: toCount(enabledPages),
|
||||
};
|
||||
}
|
||||
|
||||
export function isEmptyCatalogDelta(delta: CatalogTreeDelta): boolean {
|
||||
return (
|
||||
delta.pages.length === 0 &&
|
||||
|
||||
@@ -0,0 +1,169 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const adminFetch = vi.hoisted(() => vi.fn());
|
||||
vi.mock("@/lib/admin-fetch", () => ({ adminFetch }));
|
||||
|
||||
import {
|
||||
applyCatalogDelta,
|
||||
getLiveCatalogSnapshot,
|
||||
refreshCatalogTree,
|
||||
resetLiveCatalogForTests,
|
||||
seedCatalogTotals,
|
||||
} from "./use-live-catalog";
|
||||
|
||||
function ok(body: unknown) {
|
||||
return { ok: true, json: async () => body } as unknown as Response;
|
||||
}
|
||||
|
||||
function page(id: number, over: Record<string, unknown> = {}) {
|
||||
return {
|
||||
id,
|
||||
caption: `page ${id}`,
|
||||
parentId: -1,
|
||||
depth: 0,
|
||||
orderNum: 0,
|
||||
enabled: "1",
|
||||
visible: "1",
|
||||
iconImage: 0,
|
||||
iconColor: 0,
|
||||
pageLayout: "default_3x3",
|
||||
childCount: 0,
|
||||
itemCount: 0,
|
||||
...over,
|
||||
};
|
||||
}
|
||||
|
||||
describe("refreshCatalogTree", () => {
|
||||
beforeEach(() => {
|
||||
resetLiveCatalogForTests();
|
||||
adminFetch.mockReset();
|
||||
});
|
||||
|
||||
it("adopts the tree and the totals the route reports", async () => {
|
||||
seedCatalogTotals("normal", {
|
||||
totalPages: 1,
|
||||
enabledPages: 1,
|
||||
totalItems: 1,
|
||||
});
|
||||
adminFetch.mockResolvedValue(
|
||||
ok({
|
||||
pages: [page(1), page(2, { parentId: 1, itemCount: 7 })],
|
||||
totals: { totalPages: 2, enabledPages: 2, totalItems: 7 },
|
||||
}),
|
||||
);
|
||||
|
||||
await refreshCatalogTree();
|
||||
|
||||
const snapshot = getLiveCatalogSnapshot();
|
||||
expect(snapshot.tree.map((n) => n.id)).toEqual([1, 2]);
|
||||
expect(snapshot.totals.normal).toEqual({
|
||||
totalPages: 2,
|
||||
enabledPages: 2,
|
||||
totalItems: 7,
|
||||
});
|
||||
expect(snapshot.treeLoaded).toBe(true);
|
||||
});
|
||||
|
||||
it("replaces the counts an import delta had folded in", async () => {
|
||||
seedCatalogTotals("normal", {
|
||||
totalPages: 10,
|
||||
enabledPages: 10,
|
||||
totalItems: 100,
|
||||
});
|
||||
applyCatalogDelta({
|
||||
pages: [
|
||||
{
|
||||
id: 99,
|
||||
caption: "Chairs",
|
||||
parentId: -1,
|
||||
pageLayout: "default_3x3",
|
||||
iconImage: 0,
|
||||
iconColor: 0,
|
||||
orderNum: 0,
|
||||
enabled: "1",
|
||||
visible: "1",
|
||||
},
|
||||
],
|
||||
addedItems: [{ pageId: 99, count: 4 }],
|
||||
movedItems: [],
|
||||
});
|
||||
adminFetch.mockResolvedValue(
|
||||
ok({
|
||||
pages: [page(99)],
|
||||
totals: { totalPages: 11, enabledPages: 11, totalItems: 104 },
|
||||
}),
|
||||
);
|
||||
|
||||
await refreshCatalogTree();
|
||||
|
||||
expect(getLiveCatalogSnapshot().totals.normal).toEqual({
|
||||
totalPages: 11,
|
||||
enabledPages: 11,
|
||||
totalItems: 104,
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps the counts it holds when the route omits totals", async () => {
|
||||
seedCatalogTotals("normal", {
|
||||
totalPages: 3,
|
||||
enabledPages: 2,
|
||||
totalItems: 30,
|
||||
});
|
||||
adminFetch.mockResolvedValue(ok({ pages: [page(1)] }));
|
||||
|
||||
await refreshCatalogTree();
|
||||
|
||||
expect(getLiveCatalogSnapshot().totals.normal).toEqual({
|
||||
totalPages: 3,
|
||||
enabledPages: 2,
|
||||
totalItems: 30,
|
||||
});
|
||||
});
|
||||
|
||||
it("collapses concurrent callers into one read plus at most one follow-up", async () => {
|
||||
adminFetch.mockResolvedValue(ok({ pages: [] }));
|
||||
await Promise.all([
|
||||
refreshCatalogTree(),
|
||||
refreshCatalogTree(),
|
||||
refreshCatalogTree(),
|
||||
]);
|
||||
expect(adminFetch.mock.calls.length).toBeLessThanOrEqual(2);
|
||||
});
|
||||
|
||||
it("re-reads once for writes that land while a read is in flight", async () => {
|
||||
let release: (() => void) | undefined;
|
||||
const gate = new Promise<void>((resolve) => {
|
||||
release = resolve;
|
||||
});
|
||||
adminFetch
|
||||
.mockImplementationOnce(async () => {
|
||||
await gate;
|
||||
return ok({ pages: [page(1)] });
|
||||
})
|
||||
.mockResolvedValueOnce(ok({ pages: [page(1), page(2)] }));
|
||||
|
||||
const first = refreshCatalogTree();
|
||||
// A mutation lands while the first read is still open: its snapshot may
|
||||
// predate the write, so this caller asks for a follow-up read.
|
||||
const second = refreshCatalogTree();
|
||||
release?.();
|
||||
await Promise.all([first, second]);
|
||||
|
||||
expect(adminFetch).toHaveBeenCalledTimes(2);
|
||||
expect(getLiveCatalogSnapshot().tree.map((n) => n.id)).toEqual([1, 2]);
|
||||
});
|
||||
|
||||
it("leaves the previous snapshot alone when the read fails", async () => {
|
||||
adminFetch.mockResolvedValueOnce(ok({ pages: [page(1)] }));
|
||||
await refreshCatalogTree();
|
||||
adminFetch.mockResolvedValueOnce({
|
||||
ok: false,
|
||||
status: 500,
|
||||
json: async () => ({}),
|
||||
} as unknown as Response);
|
||||
|
||||
await refreshCatalogTree();
|
||||
|
||||
expect(getLiveCatalogSnapshot().tree.map((n) => n.id)).toEqual([1]);
|
||||
});
|
||||
});
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
type CatalogTotals,
|
||||
type CatalogTreeDelta,
|
||||
isEmptyCatalogDelta,
|
||||
normalizeCatalogTotals,
|
||||
normalizeTreePages,
|
||||
} from "./live-catalog-merge";
|
||||
|
||||
@@ -51,6 +52,11 @@ export function useLiveCatalog(): LiveCatalogSnapshot {
|
||||
return useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot);
|
||||
}
|
||||
|
||||
/** Non-React read of the same snapshot `useLiveCatalog` subscribes to. */
|
||||
export function getLiveCatalogSnapshot(): LiveCatalogSnapshot {
|
||||
return snapshot;
|
||||
}
|
||||
|
||||
// ── Reads ──────────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
@@ -91,28 +97,53 @@ async function loadTree(): Promise<void> {
|
||||
const res = await adminFetch("/api/admin/catalog/tree?mode=full");
|
||||
if (!res.ok) throw new Error(`Catalog tree refresh failed (${res.status})`);
|
||||
const data = await res.json();
|
||||
// The route answers with live totals alongside the tree: importing writes
|
||||
// catalog rows server-side, so the numbers the store holds would otherwise
|
||||
// stay pre-import for the rest of the session.
|
||||
const totals = normalizeCatalogTotals(data?.totals);
|
||||
emit({
|
||||
...snapshot,
|
||||
tree: normalizeTreePages(data?.pages),
|
||||
totals: {
|
||||
...snapshot.totals,
|
||||
normal: totals ?? snapshot.totals.normal,
|
||||
},
|
||||
treeLoaded: true,
|
||||
});
|
||||
}
|
||||
|
||||
let treeRequest: Promise<void> | null = null;
|
||||
let treeRequestQueued = false;
|
||||
|
||||
/**
|
||||
* Reload the whole tree over the API. Used where the server decides the shape of
|
||||
* the result — the furniture importers derive their category pages from
|
||||
* furnidata, so there is nothing for the client to predict. Still a plain
|
||||
* Reload the whole tree and totals over the API. Used where the server decides
|
||||
* the shape of the result — the furniture importers derive their category pages
|
||||
* from furnidata, so there is nothing for the client to predict. Still a plain
|
||||
* in-place data update: no route re-render, no remount, no lost editor state.
|
||||
*
|
||||
* Concurrent calls share one request, and a call that lands while one is in
|
||||
* flight queues a single follow-up read: the in-flight snapshot may predate the
|
||||
* write that triggered it. That keeps a burst of catalog mutations to one
|
||||
* extra read instead of one read each.
|
||||
*/
|
||||
export function refreshCatalogTree(): Promise<void> {
|
||||
if (treeRequest) return treeRequest;
|
||||
treeRequest = loadTree()
|
||||
.catch(() => undefined)
|
||||
.finally(() => {
|
||||
if (treeRequest) {
|
||||
treeRequestQueued = true;
|
||||
return treeRequest;
|
||||
}
|
||||
treeRequest = (async () => {
|
||||
try {
|
||||
do {
|
||||
treeRequestQueued = false;
|
||||
await loadTree();
|
||||
} while (treeRequestQueued);
|
||||
} catch {
|
||||
// A failed read keeps the previous snapshot; the next mount or
|
||||
// mutation tries again.
|
||||
} finally {
|
||||
treeRequest = null;
|
||||
});
|
||||
}
|
||||
})();
|
||||
return treeRequest;
|
||||
}
|
||||
|
||||
@@ -124,5 +155,6 @@ export function ensureCatalogTreeLoaded(): Promise<void> {
|
||||
/** Test seam: drop all live catalog state between cases. */
|
||||
export function resetLiveCatalogForTests(): void {
|
||||
treeRequest = null;
|
||||
treeRequestQueued = false;
|
||||
emit(EMPTY_SNAPSHOT);
|
||||
}
|
||||
@@ -0,0 +1,67 @@
|
||||
import "server-only";
|
||||
import { count, eq } from "drizzle-orm";
|
||||
import { revalidateTag, unstable_cache } from "next/cache";
|
||||
// Type-only import: the client merge module owns this shape, so the server
|
||||
// keeps exactly one definition of it.
|
||||
import type { CatalogTotals } from "@/features/catalog/client/live-catalog-merge";
|
||||
import {
|
||||
CatalogItems,
|
||||
CatalogItemsBc,
|
||||
CatalogPages,
|
||||
CatalogPagesBc,
|
||||
db,
|
||||
} from "@/lib/db";
|
||||
|
||||
export type { CatalogTotals };
|
||||
|
||||
/**
|
||||
* Cache tag for the admin catalog totals. Any path that writes catalog pages or
|
||||
* offers must invalidate it, otherwise the stats bar keeps serving the
|
||||
* pre-import numbers for the length of the TTL — including after a hard reload.
|
||||
*/
|
||||
export const CATALOG_TOTALS_TAG = "admin-catalog-totals";
|
||||
|
||||
export async function loadCatalogTotals(
|
||||
catalogType: "normal" | "bc",
|
||||
): Promise<CatalogTotals> {
|
||||
const pagesTable = catalogType === "bc" ? CatalogPagesBc : CatalogPages;
|
||||
const itemsTable = catalogType === "bc" ? CatalogItemsBc : CatalogItems;
|
||||
const [pages, items, enabled] = await Promise.all([
|
||||
db.select({ total: count() }).from(pagesTable),
|
||||
db.select({ total: count() }).from(itemsTable),
|
||||
db
|
||||
.select({ total: count() })
|
||||
.from(pagesTable)
|
||||
.where(eq(pagesTable.enabled, "1")),
|
||||
]);
|
||||
return {
|
||||
totalPages: Number(pages[0]?.total ?? 0),
|
||||
totalItems: Number(items[0]?.total ?? 0),
|
||||
enabledPages: Number(enabled[0]?.total ?? 0),
|
||||
};
|
||||
}
|
||||
|
||||
const getCachedTotals = unstable_cache(
|
||||
(catalogType: "normal" | "bc") => loadCatalogTotals(catalogType),
|
||||
[CATALOG_TOTALS_TAG],
|
||||
{ tags: [CATALOG_TOTALS_TAG], revalidate: 30 },
|
||||
);
|
||||
|
||||
export function getCachedCatalogTotals(
|
||||
catalogType: "normal" | "bc",
|
||||
): Promise<CatalogTotals> {
|
||||
return getCachedTotals(catalogType);
|
||||
}
|
||||
|
||||
/**
|
||||
* Best-effort invalidation for import workers and API routes, which run outside
|
||||
* a render. A failure here must never abort an import: the cache expires on its
|
||||
* own after 30 seconds.
|
||||
*/
|
||||
export function invalidateCatalogTotals(): void {
|
||||
try {
|
||||
revalidateTag(CATALOG_TOTALS_TAG, { expire: 0 });
|
||||
} catch {
|
||||
// Ignore — see above.
|
||||
}
|
||||
}
|
||||
@@ -5,6 +5,7 @@ import path from "node:path";
|
||||
import { logger } from "@/lib/logger";
|
||||
import { catalogStateRoot } from "@/lib/services/catalog-git-config";
|
||||
import { rcon } from "@/lib/services/rcon";
|
||||
import { invalidateCatalogTotals } from "./catalog-totals";
|
||||
export interface CatalogHotelStatus {
|
||||
sent: boolean;
|
||||
checkedAt: string;
|
||||
@@ -40,6 +41,9 @@ export async function sendCatalogUpdate(): Promise<CatalogHotelStatus> {
|
||||
error,
|
||||
});
|
||||
}
|
||||
// Callers reach here only after writing catalog rows, so the admin stats
|
||||
// totals must not keep serving the pre-write numbers from their 30s cache.
|
||||
invalidateCatalogTotals();
|
||||
const status = { sent, checkedAt: new Date().toISOString(), reference };
|
||||
const root = catalogStateRoot();
|
||||
const temp = path.join(root, `hotel-${randomUUID()}.tmp`);
|
||||
|
||||
Reference in new issue
Block a user