diff --git a/apps/desktop/src/components/objects/ObjectBrowser.vue b/apps/desktop/src/components/objects/ObjectBrowser.vue index bad871d1f8..01d577ec74 100644 --- a/apps/desktop/src/components/objects/ObjectBrowser.vue +++ b/apps/desktop/src/components/objects/ObjectBrowser.vue @@ -155,7 +155,7 @@ import { createSidePanelRequestGuard } from "@/lib/table/sidePanelRequestGuard"; import { runBatchTableTruncate } from "@/lib/table/batchTableTruncate"; import { tableColumnDefaultDisplayValue } from "@/lib/table/tableColumnDefaultPresentation"; import { gaussdbMTypeDisplayName } from "@/lib/table/postgresDataTypeHelp"; -import { cacheObjectBrowserRows, createObjectBrowserRowsCacheWriteToken, getCachedObjectBrowserRows, type ObjectBrowserRowsCacheScope, type ObjectBrowserRowsCacheWriteToken } from "@/lib/table/objectBrowserRowsCache"; +import { cacheObjectBrowserRows, createObjectBrowserRowsCacheWriteToken, getCachedObjectBrowserRowsForScaffold, type ObjectBrowserRowsCacheScope, type ObjectBrowserRowsCacheWriteToken } from "@/lib/table/objectBrowserRowsCache"; import { createObjectBrowserRowsLoadGuard, type ObjectBrowserRowsLoadHandle } from "@/lib/table/objectBrowserRowsLoadGuard"; import { loadObjectDdl, type ObjectDdlRequest } from "@/lib/metadata/objectDdlCache"; import { loadObjectMetadataFacet } from "@/lib/metadata/objectMetadataCache"; @@ -211,6 +211,8 @@ const sortKey = ref("name"); const sortDirection = ref("asc"); const loadingSchemas = ref(false); const loadingObjects = ref(false); +const refreshingObjects = ref(false); +const scaffoldRefreshError = ref(""); const sourceLoading = ref(false); const sourceContent = ref(""); const sourceError = ref(""); @@ -2815,22 +2817,57 @@ watch([() => props.initialEventName, () => props.initialEventOpenRequestId, () = openInitialEventIfNeeded(); }); -async function loadObjects(options?: { allowCached?: boolean }) { +async function loadObjects(options?: { allowCached?: boolean; preserveExistingRows?: boolean }) { error.value = ""; + // A new load supersedes any in-flight one, so reset the transient refresh flags + // on entry. A superseded request's finally() can no longer run (the guard's + // epoch moved on) and would otherwise leave refreshingObjects stuck spinning + // the toolbar icon — the newest request owns the spinner state from here. + loadingObjects.value = false; + refreshingObjects.value = false; + scaffoldRefreshError.value = ""; + // True when we are revalidating on top of visible rows (a stale-cache scaffold, or a + // same-instance refresh) — a failure then keeps the rows and raises a non-blocking + // banner instead of replacing the whole list with a full-area error. + let scaffoldRefresh = false; const schema = needsSchema.value ? selectedSchema.value || "" : props.database; const request = objectBrowserRowsLoadGuard.start(objectBrowserRowsCacheScope(schema)); const cacheWriteToken = createObjectBrowserRowsCacheWriteToken(request.scope); - if (options?.allowCached) { - const cachedRows = getCachedObjectBrowserRows(request.scope); - if (cachedRows) { - applyObjectBrowserRows(cachedRows); - finishObjectBrowserRowsLoad(); - return; + + // finishObjectBrowserRowsLoad() is idempotent, but keep the default-filter / + // viewport restores to a single run per load so the events-preferred-filter case + // doesn't leave preserveObjectFilterScrollOnce latched across an extra call. + let finished = false; + const finishOnce = () => { + if (finished) return; + finished = true; + finishObjectBrowserRowsLoad(); + }; + + const cached = options?.allowCached ? getCachedObjectBrowserRowsForScaffold(request.scope) : undefined; + if (cached) { + // Scaffold the last-known rows so returning to this tab shows them instantly + // instead of flashing an empty list. A fresh entry (< TTL) is authoritative on + // its own; a stale one is presented and then revalidated in the background. + applyObjectBrowserRows(cached.rows); + finishOnce(); + if (!cached.stale) return; + scaffoldRefresh = true; + refreshingObjects.value = true; + } else { + // No scaffold: first load in this scope, cache invalidated by a DDL mutation, + // or the caller wants a true reload. If the caller explicitly asked to keep the + // current rows (same-instance refresh) and rows exist, refresh without blanking. + const keepExisting = options?.preserveExistingRows && rows.value.length > 0; + if (keepExisting) { + scaffoldRefresh = true; + refreshingObjects.value = true; + } else { + loadingObjects.value = true; + rows.value = []; } } - loadingObjects.value = true; - rows.value = []; try { const nextRows = props.connection.db_type === "mongodb" ? await loadMongoObjectBrowserRows(request.scope.connectionId, request.scope.database) : await loadSqlObjectBrowserRows(request); if (!objectBrowserRowsLoadGuard.isCurrent(request)) return; @@ -2839,9 +2876,19 @@ async function loadObjects(options?: { allowCached?: boolean }) { if (props.connection.db_type !== "mongodb") void loadObjectStatistics(request, cacheWriteToken, cachedAt); } catch (e: any) { if (!objectBrowserRowsLoadGuard.isCurrent(request)) return; - error.value = translateBackendError(t, e); + // Keep visible rows on a background revalidate failure — surface a lightweight + // banner rather than replacing the scaffold (or same-instance refresh) list. + if (scaffoldRefresh) { + scaffoldRefreshError.value = translateBackendError(t, e); + } else { + error.value = translateBackendError(t, e); + } } finally { - if (objectBrowserRowsLoadGuard.isCurrent(request)) finishObjectBrowserRowsLoad(); + if (objectBrowserRowsLoadGuard.isCurrent(request)) { + loadingObjects.value = false; + refreshingObjects.value = false; + finishOnce(); + } } } @@ -2894,15 +2941,15 @@ function normalizeStatisticNumber(value: number | null | undefined): number | nu return typeof value === "number" && Number.isFinite(value) ? value : null; } -async function reload(options?: { allowCachedObjects?: boolean; contextEpoch?: number }) { +async function reload(options?: { allowCachedObjects?: boolean; contextEpoch?: number; preserveExistingRows?: boolean }) { const epoch = options?.contextEpoch ?? objectBrowserRowsLoadGuard.invalidate(); if (!(await loadSchemas(epoch))) return; if (!objectBrowserRowsLoadGuard.isEpochCurrent(epoch)) return; - await loadObjects({ allowCached: options?.allowCachedObjects }); + await loadObjects({ allowCached: options?.allowCachedObjects, preserveExistingRows: options?.preserveExistingRows }); } function refresh(): boolean { - void reload(); + void reload({ preserveExistingRows: true }); void refreshActiveTableInfo(); return true; } @@ -3302,7 +3349,7 @@ function getObjectBrowserMenuItems(item: ObjectBrowserRow): ContextMenuItem[] { +
+ + {{ scaffoldRefreshError }} +
{{ t("objects.loading") }} diff --git a/apps/desktop/src/components/objects/__tests__/ObjectBrowserMetadataRefresh.spec.ts b/apps/desktop/src/components/objects/__tests__/ObjectBrowserMetadataRefresh.spec.ts index 9fe3797163..cf1eeab727 100644 --- a/apps/desktop/src/components/objects/__tests__/ObjectBrowserMetadataRefresh.spec.ts +++ b/apps/desktop/src/components/objects/__tests__/ObjectBrowserMetadataRefresh.spec.ts @@ -20,7 +20,7 @@ describe("ObjectBrowser table metadata refresh", () => { it("refreshes the object list and the open table-info tab from the toolbar", () => { const refresh = functionBody("refresh"); - expect(refresh).toContain("void reload();"); + expect(refresh).toContain("void reload({ preserveExistingRows: true });"); expect(refresh).toContain("void refreshActiveTableInfo();"); expect(source).toContain('@click="refresh"'); }); diff --git a/apps/desktop/src/components/objects/__tests__/ObjectBrowserScaffoldRefresh.spec.ts b/apps/desktop/src/components/objects/__tests__/ObjectBrowserScaffoldRefresh.spec.ts new file mode 100644 index 0000000000..6c242a05ea --- /dev/null +++ b/apps/desktop/src/components/objects/__tests__/ObjectBrowserScaffoldRefresh.spec.ts @@ -0,0 +1,58 @@ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; + +const source = readFileSync(new URL("../ObjectBrowser.vue", import.meta.url), "utf8"); + +function functionBody(name: string): string { + const signature = new RegExp(`(?:async\\s+)?function\\s+${name}\\s*\\([^)]*\\)\\s*(?::\\s*[^\\{]+)?\\{`, "m").exec(source); + if (!signature) throw new Error(`Missing function ${name}`); + const bodyStart = signature.index + signature[0].length; + let depth = 1; + for (let index = bodyStart; index < source.length; index += 1) { + if (source[index] === "{") depth += 1; + else if (source[index] === "}") depth -= 1; + if (depth === 0) return source.slice(bodyStart, index); + } + throw new Error(`Unclosed function ${name}`); +} + +describe("ObjectBrowser scaffold refresh race", () => { + // Scope A hits stale cache and starts a background revalidate (refreshingObjects = true); + // before it settles, the user switches to scope B which hits fresh cache and early-returns. + // A's finally() can no longer run (the load guard's epoch moved on), so the flag must be + // cleared by the newest request (B) at entry — otherwise the toolbar icon spins forever. + it("resets the transient refresh flags at entry, before the cache decision", () => { + const body = functionBody("loadObjects"); + + const loadingReset = body.indexOf("loadingObjects.value = false;"); + const refreshingReset = body.indexOf("refreshingObjects.value = false;"); + const cachedBranch = body.indexOf("const cached ="); + + expect(loadingReset).toBeGreaterThanOrEqual(0); + expect(refreshingReset).toBeGreaterThanOrEqual(0); + // Both resets must precede the cached fresh/stale decision so the newest request + // owns the spinner state even when a superseded request's finally() cannot run. + expect(Math.min(loadingReset, refreshingReset)).toBeLessThan(cachedBranch); + // And neither flag may be turned back on before the branch decides the indicator. + expect(body.indexOf("refreshingObjects.value = true;")).toBeGreaterThan(cachedBranch); + expect(body.indexOf("loadingObjects.value = true;")).toBeGreaterThan(cachedBranch); + }); + + it("exposes a non-blocking refresh flag distinct from the blocking full-area load", () => { + expect(source).toContain("const refreshingObjects = ref(false);"); + expect(functionBody("loadObjects")).toContain("refreshingObjects.value = true;"); + expect(functionBody("loadObjects")).toContain("loadingObjects.value = true;"); + }); + + it("keeps visible rows on a background revalidate failure (scaffold-preserving error)", () => { + expect(source).toContain('const scaffoldRefreshError = ref("");'); + const body = functionBody("loadObjects"); + // A scaffold/refresh revalidate failure must route to the non-blocking banner + // (scaffoldRefreshError) and keep the visible rows, not replace them with the + // blocking full-area error. + expect(body).toContain("scaffoldRefresh = true;"); + expect(body).toContain("if (scaffoldRefresh) {"); + expect(body).toContain("scaffoldRefreshError.value = translateBackendError(t, e)"); + expect(source).toContain('v-if="scaffoldRefreshError"'); + }); +}); diff --git a/apps/desktop/src/lib/__tests__/table/contentAreaObjectBrowserCatalog.spec.ts b/apps/desktop/src/lib/__tests__/table/contentAreaObjectBrowserCatalog.spec.ts index a2099deeb9..d8a5ac025c 100644 --- a/apps/desktop/src/lib/__tests__/table/contentAreaObjectBrowserCatalog.spec.ts +++ b/apps/desktop/src/lib/__tests__/table/contentAreaObjectBrowserCatalog.spec.ts @@ -36,7 +36,7 @@ describe("ContentArea object browser refresh wiring", () => { }); it("exposes object and active table-info reloads as refresh", () => { - expect(objectBrowserSource).toMatch(/function refresh\(\): boolean \{\s+void reload\(\);\s+void refreshActiveTableInfo\(\);\s+return true;\s+\}/); + expect(objectBrowserSource).toMatch(/function refresh\(\): boolean \{\s+void reload\(\{ preserveExistingRows: true \}\);\s+void refreshActiveTableInfo\(\);\s+return true;\s+\}/); expect(objectBrowserSource).toContain("defineExpose({ focusSearch, refresh });"); }); diff --git a/apps/desktop/src/lib/metadata/__tests__/objectBrowserRowsCache.spec.ts b/apps/desktop/src/lib/metadata/__tests__/objectBrowserRowsCache.spec.ts index 93eee2a09f..77b96acc25 100644 --- a/apps/desktop/src/lib/metadata/__tests__/objectBrowserRowsCache.spec.ts +++ b/apps/desktop/src/lib/metadata/__tests__/objectBrowserRowsCache.spec.ts @@ -1,6 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { ObjectBrowserRow } from "@/lib/table/objectBrowserRows"; -import { cacheObjectBrowserRows, clearObjectBrowserRowsCache, createObjectBrowserRowsCacheWriteToken, getCachedObjectBrowserRows, invalidateObjectBrowserRowsCache } from "@/lib/table/objectBrowserRowsCache"; +import { cacheObjectBrowserRows, clearObjectBrowserRowsCache, createObjectBrowserRowsCacheWriteToken, getCachedObjectBrowserRows, getCachedObjectBrowserRowsForScaffold, invalidateObjectBrowserRowsCache } from "@/lib/table/objectBrowserRowsCache"; const row: ObjectBrowserRow = { id: "table:users", @@ -127,4 +127,43 @@ describe("objectBrowserRowsCache", () => { expect(invalidateObjectBrowserRowsCache({ ...scope, tableName: "users" })).toBe(1); expect(getCachedObjectBrowserRows(scope)).toBeUndefined(); }); + + describe("scaffold reads (allowStale)", () => { + it("returns a fresh hit marked stale=false before the TTL", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-07-28T00:00:00Z")); + const scope = { connectionId: "c1", database: "db", schema: "public" }; + cacheRows(scope); + + const hit = getCachedObjectBrowserRowsForScaffold(scope); + expect(hit).toBeDefined(); + expect(hit!.rows).toEqual([row]); + expect(hit!.stale).toBe(false); + expect(hit!.ageMs).toBe(0); + expect(hit!.cachedAt).toBe(Date.now()); + }); + + it("surfaces an expired entry as a stale scaffold instead of refusing it", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-07-28T00:00:00Z")); + const scope = { connectionId: "c1", database: "db", schema: "public" }; + cacheRows(scope); + + vi.advanceTimersByTime(30_001); + const hit = getCachedObjectBrowserRowsForScaffold(scope); + expect(hit).toBeDefined(); + expect(hit!.rows).toEqual([row]); + expect(hit!.stale).toBe(true); + expect(hit!.ageMs).toBeGreaterThan(30_000); + }); + + it("returns undefined when no scaffold is available (uncached or invalidated)", () => { + const scope = { connectionId: "c1", database: "db", schema: "public" }; + expect(getCachedObjectBrowserRowsForScaffold(scope)).toBeUndefined(); + + cacheRows(scope); + invalidateObjectBrowserRowsCache({ connectionId: "c1", database: "db", schema: "public" }); + expect(getCachedObjectBrowserRowsForScaffold(scope)).toBeUndefined(); + }); + }); }); diff --git a/apps/desktop/src/lib/table/objectBrowserRowsCache.ts b/apps/desktop/src/lib/table/objectBrowserRowsCache.ts index 3f89b8259f..2018598151 100644 --- a/apps/desktop/src/lib/table/objectBrowserRowsCache.ts +++ b/apps/desktop/src/lib/table/objectBrowserRowsCache.ts @@ -78,6 +78,28 @@ export function getCachedObjectBrowserRows(scope: ObjectBrowserRowsCacheScope): return hit ? cloneRows(hit.value) : undefined; } +export interface ObjectBrowserRowsCacheHit { + rows: ObjectBrowserRow[]; + stale: boolean; + ageMs: number; + cachedAt: number; +} + +/** + * Reads the cached rows for a scope as a **scaffold**, allowing stale entries + * (beyond the TTL) to be surfaced and refreshed in the background. This never + * adds a new cache entry or expands capacity — a stale hit is the same Map entry + * the TTL would otherwise refuse to return, only now with its freshness metadata. + * + * Carries `stale`/`ageMs`/`cachedAt` so the caller can decide: keep an entry + * under TTL as-is, or present it immediately then revalidate in the background. + */ +export function getCachedObjectBrowserRowsForScaffold(scope: ObjectBrowserRowsCacheScope): ObjectBrowserRowsCacheHit | undefined { + const hit = objectBrowserRowsCache.get(objectBrowserRowsScope(scope), { allowStale: true }); + if (!hit) return undefined; + return { rows: cloneRows(hit.value), stale: hit.stale, ageMs: hit.ageMs, cachedAt: hit.cachedAt }; +} + export function createObjectBrowserRowsCacheWriteToken(scope: ObjectBrowserRowsCacheScope): ObjectBrowserRowsCacheWriteToken { const frozenScope = Object.freeze({ ...scope }); return Object.freeze({ generation: objectBrowserRowsCacheGeneration(frozenScope).generation, scope: frozenScope });