From de25229cdc2c8ba6c8c6a756c2b07834036bfcb0 Mon Sep 17 00:00:00 2001 From: zipg Date: Fri, 17 Jul 2026 15:30:12 +0800 Subject: [PATCH] fix(grid): refresh comments for restored data tabs --- .../composables/useSidebarDataOpenRuntime.ts | 110 +++++++++--------- .../sidebar/dataTabOpenPolicy.spec.ts | 24 +++- .../src/lib/sidebar/dataTabOpenPolicy.ts | 7 +- 3 files changed, 87 insertions(+), 54 deletions(-) diff --git a/apps/desktop/src/composables/useSidebarDataOpenRuntime.ts b/apps/desktop/src/composables/useSidebarDataOpenRuntime.ts index 0a891f9a2..4e684b397 100644 --- a/apps/desktop/src/composables/useSidebarDataOpenRuntime.ts +++ b/apps/desktop/src/composables/useSidebarDataOpenRuntime.ts @@ -7,7 +7,7 @@ import { uuid } from "@/lib/common/utils"; import { appendDebugLog, isDebugLoggingEnabled } from "@/lib/backend/debugLog"; import { effectiveDatabaseTypeForConnection, connectionObjectTreeNodeSchema, connectionObjectTreeQuerySchema } from "@/lib/database/jdbcDialect"; import { getCachedTableMetadata, loadTableMetadata, TABLE_METADATA_CACHE_TTL_MS, tableMetadataToDataTabMeta } from "@/lib/metadata/tableMetadataCache"; -import { canApplyDataTabMetadata, findExistingDataTabCandidate, type DataTabOpenMode } from "@/lib/sidebar/dataTabOpenPolicy"; +import { canApplyDataTabMetadata, dataTabMetadataNeedsRefresh, findExistingDataTabCandidate, type DataTabOpenMode } from "@/lib/sidebar/dataTabOpenPolicy"; import type { SidebarDataOpenRequest } from "@/lib/sidebar/sidebarDataOpenCoordinator"; import { hasTreeNodeDatabaseContext } from "@/lib/sidebar/treeNodeContext"; import { buildTableSelectSql } from "@/lib/table/tableSelectSql"; @@ -65,6 +65,57 @@ export function useSidebarDataOpenRuntime() { catalog: node.catalog, tableName: node.label, }; + const canApplyTableMetadata = (targetTabId: string) => + canApplyDataTabMetadata( + queryStore.tabs.find((tab) => tab.id === targetTabId), + dataTabTarget, + request?.signal, + ); + const refreshTableMetaInBackground = async (targetTabId: string, ensureConnected = false) => { + if (!config) return; + const metadataStartedAt = performance.now(); + openDataLog("info", "metadata:start", { + traceId, + elapsed: elapsed(), + }); + try { + if (ensureConnected) await connectionStore.ensureConnected(node.connectionId); + const loadedMetadata = await loadTableMetadata({ + connectionId: node.connectionId, + database: node.database, + schema: querySchema, + tableName: node.label, + tableType, + databaseType: metadataDatabaseType, + driverProfile: config.driver_profile || config.db_type, + catalog: node.catalog, + traceLogger: isDebugLoggingEnabled() ? (event) => openDataLog("debug", "metadata:trace", { sourceTraceId: traceId, ...event }) : undefined, + }); + if (!canApplyTableMetadata(targetTabId)) { + openDataLog("info", "metadata:stale", { + traceId, + tabId: targetTabId, + columnCount: loadedMetadata.metadata.columns.length, + elapsed: elapsed(), + }); + return; + } + const nextTableMeta = tableMetadataToDataTabMeta(loadedMetadata.metadata, tableSchema); + queryStore.setTableMeta(targetTabId, nextTableMeta); + openDataLog("info", "metadata:done", { + traceId, + tabId: targetTabId, + columnCount: nextTableMeta.columns.length, + primaryKeyCount: nextTableMeta.primaryKeys.length, + cacheStatus: loadedMetadata.cacheStatus, + ageMs: Math.round(loadedMetadata.ageMs), + elapsed: elapsed(), + metadataMs: Math.round(performance.now() - metadataStartedAt), + }); + } catch (error) { + openDataLog("warn", "metadata:error", { traceId, tabId: targetTabId, elapsed: elapsed(), error }); + } + }; const existingDataTabCandidate = findExistingDataTabCandidate(queryStore.tabs, dataTabTarget, { openMode, reuseDataTab: settingsStore.editorSettings.reuseDataTab }); const existingSameTableTab = existingDataTabCandidate?.match === "same-table" ? existingDataTabCandidate.tab : undefined; const resetReusedDataTabState = (tab: (typeof queryStore.tabs)[number]) => { @@ -93,6 +144,10 @@ export function useSidebarDataOpenRuntime() { if (existingSameTableTab && canActivateExistingDataTableTab(existingSameTableTab, { activateExecuting: false })) { queryStore.switchTab(existingSameTableTab.id); logPhase("existing-tab-activated", { table: node.label }); + if (dataTabMetadataNeedsRefresh(existingSameTableTab, DATA_TAB_METADATA_TTL_MS)) { + void refreshTableMetaInBackground(existingSameTableTab.id, true); + logPhase("metadata-started", { tabId: existingSameTableTab.id, reason: "existing-tab-stale" }); + } return; } @@ -171,12 +226,6 @@ export function useSidebarDataOpenRuntime() { // Helper to check if this openData call is still active (not superseded by a newer click) const isActive = () => (request?.isCurrent() ?? true) && queryStore.tabs.find((t) => t.id === tabId)?.executionId === openDataId; - const canApplyTableMetadata = () => - canApplyDataTabMetadata( - queryStore.tabs.find((t) => t.id === tabId), - dataTabTarget, - request?.signal, - ); try { openDataLog("info", "ensure-connected:start", { traceId, elapsed: elapsed() }); @@ -190,49 +239,6 @@ export function useSidebarDataOpenRuntime() { if (!config) throw new Error("Connection config not found"); const limit = tableOpenPageLimit(); - const refreshTableMetaInBackground = async () => { - const metadataStartedAt = performance.now(); - openDataLog("info", "metadata:start", { - traceId, - elapsed: elapsed(), - }); - try { - const loadedMetadata = await loadTableMetadata({ - connectionId: node.connectionId, - database: node.database, - schema: querySchema, - tableName: node.label, - tableType, - databaseType: metadataDatabaseType, - driverProfile: config.driver_profile || config.db_type, - catalog: node.catalog, - traceLogger: isDebugLoggingEnabled() ? (event) => openDataLog("debug", "metadata:trace", { sourceTraceId: traceId, ...event }) : undefined, - }); - if (!canApplyTableMetadata()) { - openDataLog("info", "metadata:stale", { - traceId, - tabId, - columnCount: loadedMetadata.metadata.columns.length, - elapsed: elapsed(), - }); - return; - } - const nextTableMeta = tableMetadataToDataTabMeta(loadedMetadata.metadata, tableSchema); - queryStore.setTableMeta(tabId, nextTableMeta); - openDataLog("info", "metadata:done", { - traceId, - tabId, - columnCount: nextTableMeta.columns.length, - primaryKeyCount: nextTableMeta.primaryKeys.length, - cacheStatus: loadedMetadata.cacheStatus, - ageMs: Math.round(loadedMetadata.ageMs), - elapsed: elapsed(), - metadataMs: Math.round(performance.now() - metadataStartedAt), - }); - } catch (error) { - openDataLog("warn", "metadata:error", { traceId, tabId, elapsed: elapsed(), error }); - } - }; const shouldRefreshTableMeta = !cachedTableMeta; if (cachedTableMeta) { openDataLog("info", "metadata:cache-hit", { @@ -289,8 +295,8 @@ export function useSidebarDataOpenRuntime() { }); openDataLog("info", "execute:done", { traceId, tabId, elapsed: elapsed() }); logPhase("execute-tab-sql", { tabId }); - if (shouldRefreshTableMeta && canApplyTableMetadata()) { - void refreshTableMetaInBackground(); + if (shouldRefreshTableMeta && canApplyTableMetadata(tabId)) { + void refreshTableMetaInBackground(tabId); logPhase("metadata-started", { tabId }); } } catch (e: any) { diff --git a/apps/desktop/src/lib/__tests__/sidebar/dataTabOpenPolicy.spec.ts b/apps/desktop/src/lib/__tests__/sidebar/dataTabOpenPolicy.spec.ts index b5d41c04c..b9d8e1471 100644 --- a/apps/desktop/src/lib/__tests__/sidebar/dataTabOpenPolicy.spec.ts +++ b/apps/desktop/src/lib/__tests__/sidebar/dataTabOpenPolicy.spec.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { canApplyDataTabMetadata, dataTabOpenModeFromTreeClick, findExistingDataTabCandidate } from "@/lib/sidebar/dataTabOpenPolicy"; +import { canApplyDataTabMetadata, dataTabMetadataNeedsRefresh, dataTabOpenModeFromTreeClick, findExistingDataTabCandidate } from "@/lib/sidebar/dataTabOpenPolicy"; import type { QueryTab } from "@/types/database"; function click(modifiers: Partial> = {}) { @@ -88,4 +88,26 @@ describe("dataTabOpenPolicy", () => { expect(canApplyDataTabMetadata(tab, usersTarget, new AbortController().signal)).toBe(false); }); + + it("refreshes missing, restored, and expired table metadata", () => { + const now = 100_000; + const ttl = 30_000; + const tab = dataTab("users", "users"); + + expect(dataTabMetadataNeedsRefresh(tab, ttl, now)).toBe(true); + + tab.tableMeta = { + schema: "public", + tableName: "users", + columns: [{ name: "id", data_type: "integer", is_nullable: false, column_default: null, is_primary_key: true, extra: null }], + primaryKeys: ["id"], + }; + expect(dataTabMetadataNeedsRefresh(tab, ttl, now)).toBe(true); + + tab.tableMetaUpdatedAt = now - ttl + 1; + expect(dataTabMetadataNeedsRefresh(tab, ttl, now)).toBe(false); + + tab.tableMetaUpdatedAt = now - ttl; + expect(dataTabMetadataNeedsRefresh(tab, ttl, now)).toBe(true); + }); }); diff --git a/apps/desktop/src/lib/sidebar/dataTabOpenPolicy.ts b/apps/desktop/src/lib/sidebar/dataTabOpenPolicy.ts index b94524fe0..da070245f 100644 --- a/apps/desktop/src/lib/sidebar/dataTabOpenPolicy.ts +++ b/apps/desktop/src/lib/sidebar/dataTabOpenPolicy.ts @@ -3,7 +3,7 @@ import type { QueryTab, TreeNodeType } from "@/types/database"; export type DataTabOpenMode = "default" | "new-tab"; -type DataTabLike = Pick; +type DataTabLike = Pick; export interface DataTabTarget { connectionId: string; @@ -41,6 +41,11 @@ export function canApplyDataTabMetadata(tab: DataTabLike | undefined, target: Da return signal?.aborted !== true && tab !== undefined && isSameTable(tab, target); } +export function dataTabMetadataNeedsRefresh(tab: DataTabLike, maxAgeMs: number, now = Date.now()): boolean { + if (!tab.tableMeta?.columns.length || tab.tableMetaUpdatedAt === undefined) return true; + return now - tab.tableMetaUpdatedAt >= maxAgeMs; +} + export function findExistingDataTabCandidate(tabs: T[], target: DataTabTarget, options: { openMode: DataTabOpenMode; reuseDataTab: boolean }): ExistingDataTabCandidate | undefined { if (options.openMode === "new-tab") return undefined;