From 4481cd13f0f36a560754df948a40cec106c65284 Mon Sep 17 00:00:00 2001 From: zipg Date: Thu, 16 Jul 2026 14:39:09 +0800 Subject: [PATCH] fix(sidebar): preserve primary key metadata after table loading --- .../src/components/sidebar/TreeItem.vue | 36 +++++++++---------- .../sidebar/dataTabOpenPolicy.spec.ts | 25 ++++++++++++- .../sidebarDataOpenCoordinator.spec.ts | 11 ++++++ .../src/lib/sidebar/dataTabOpenPolicy.ts | 4 +++ 4 files changed, 56 insertions(+), 20 deletions(-) diff --git a/apps/desktop/src/components/sidebar/TreeItem.vue b/apps/desktop/src/components/sidebar/TreeItem.vue index b0c7af0057..a220eb444a 100644 --- a/apps/desktop/src/components/sidebar/TreeItem.vue +++ b/apps/desktop/src/components/sidebar/TreeItem.vue @@ -104,7 +104,7 @@ import { usesTreeSchemaMode, } from "@/lib/database/databaseCapabilities"; import { copyNameForTreeNode, isDocumentBrowserTreeNode, objectSourceKindForTreeNode, shouldRunTreeNodeRowAction, treeNodeRowAction, treeNodeRowDoubleClickAction } from "@/lib/sidebar/treeNodeClick"; -import { dataTabOpenModeFromTreeClick, findExistingDataTabCandidate, type DataTabOpenMode } from "@/lib/sidebar/dataTabOpenPolicy"; +import { canApplyDataTabMetadata, dataTabOpenModeFromTreeClick, findExistingDataTabCandidate, type DataTabOpenMode } from "@/lib/sidebar/dataTabOpenPolicy"; import { isCopySidebarSelectionShortcut, isEditSidebarConnectionShortcut, isPasteSidebarSelectionShortcut } from "@/lib/editor/keyboardShortcuts"; import { formatSqlInsert } from "@/lib/export/exportFormats"; import { joinExportedDdls } from "@/lib/export/ddlExport"; @@ -1397,17 +1397,14 @@ async function openData(node: TreeNode, request?: SidebarDataOpenRequest, openMo const querySchema = config ? connectionObjectTreeQuerySchema(config, node.database, tableSchema) : (tableSchema ?? ""); const effectiveDbType = effectiveDatabaseTypeForConnection(config); const metadataDatabaseType = effectiveDbType || config?.db_type || ""; - const existingDataTabCandidate = findExistingDataTabCandidate( - queryStore.tabs, - { - connectionId: node.connectionId, - database: node.database, - schema: tableSchema, - catalog: node.catalog, - tableName: node.label, - }, - { openMode, reuseDataTab: settingsStore.editorSettings.reuseDataTab }, - ); + const dataTabTarget = { + connectionId: node.connectionId, + database: node.database, + schema: tableSchema, + catalog: node.catalog, + tableName: node.label, + }; + 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]) => { tab.title = node.label; @@ -1513,11 +1510,12 @@ async function openData(node: TreeNode, request?: SidebarDataOpenRequest, openMo // 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 isCurrentDataTab = () => { - if (!(request?.isCurrent() ?? true)) return false; - const current = queryStore.tabs.find((t) => t.id === tabId); - return current?.mode === "data" && current.connectionId === node.connectionId && current.database === node.database && current.schema === tableSchema && current.title === node.label; - }; + const canApplyTableMetadata = () => + canApplyDataTabMetadata( + queryStore.tabs.find((t) => t.id === tabId), + dataTabTarget, + request?.signal, + ); try { openDataLog("info", "ensure-connected:start", { traceId, elapsed: elapsed() }); @@ -1549,7 +1547,7 @@ async function openData(node: TreeNode, request?: SidebarDataOpenRequest, openMo catalog: node.catalog, traceLogger: isDebugLoggingEnabled() ? (event) => openDataLog("debug", "metadata:trace", { sourceTraceId: traceId, ...event }) : undefined, }); - if (!isCurrentDataTab()) { + if (!canApplyTableMetadata()) { openDataLog("info", "metadata:stale", { traceId, tabId, @@ -1630,7 +1628,7 @@ async function openData(node: TreeNode, request?: SidebarDataOpenRequest, openMo }); openDataLog("info", "execute:done", { traceId, tabId, elapsed: elapsed() }); logPhase("execute-tab-sql", { tabId }); - if (shouldRefreshTableMeta && isCurrentDataTab()) { + if (shouldRefreshTableMeta && canApplyTableMetadata()) { void refreshTableMetaInBackground(); logPhase("metadata-started", { tabId }); } diff --git a/apps/desktop/src/lib/__tests__/sidebar/dataTabOpenPolicy.spec.ts b/apps/desktop/src/lib/__tests__/sidebar/dataTabOpenPolicy.spec.ts index 4ec23b1ccc..b5d41c04c5 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 { dataTabOpenModeFromTreeClick, findExistingDataTabCandidate } from "@/lib/sidebar/dataTabOpenPolicy"; +import { canApplyDataTabMetadata, dataTabOpenModeFromTreeClick, findExistingDataTabCandidate } from "@/lib/sidebar/dataTabOpenPolicy"; import type { QueryTab } from "@/types/database"; function click(modifiers: Partial> = {}) { @@ -65,4 +65,27 @@ describe("dataTabOpenPolicy", () => { expect(findExistingDataTabCandidate([otherTable], usersTarget, { openMode: "default", reuseDataTab: true })).toEqual({ tab: otherTable, match: "database" }); expect(findExistingDataTabCandidate([otherTable], usersTarget, { openMode: "default", reuseDataTab: false })).toBeUndefined(); }); + + it("allows metadata to update a tab that still points to the requested table", () => { + const tab = dataTab("users", "users"); + tab.tableMeta = { schema: "public", tableName: "users", columns: [], primaryKeys: [] }; + + expect(canApplyDataTabMetadata(tab, usersTarget, new AbortController().signal)).toBe(true); + }); + + it("rejects metadata after its request is cancelled", () => { + const tab = dataTab("users", "users"); + tab.tableMeta = { schema: "public", tableName: "users", columns: [], primaryKeys: [] }; + const controller = new AbortController(); + controller.abort(); + + expect(canApplyDataTabMetadata(tab, usersTarget, controller.signal)).toBe(false); + }); + + it("rejects stale metadata after a reusable tab switches to another table", () => { + const tab = dataTab("reused", "orders"); + tab.tableMeta = { schema: "public", tableName: "orders", columns: [], primaryKeys: [] }; + + expect(canApplyDataTabMetadata(tab, usersTarget, new AbortController().signal)).toBe(false); + }); }); diff --git a/apps/desktop/src/lib/__tests__/sidebar/sidebarDataOpenCoordinator.spec.ts b/apps/desktop/src/lib/__tests__/sidebar/sidebarDataOpenCoordinator.spec.ts index 8b678ab380..85d35dbcc8 100644 --- a/apps/desktop/src/lib/__tests__/sidebar/sidebarDataOpenCoordinator.spec.ts +++ b/apps/desktop/src/lib/__tests__/sidebar/sidebarDataOpenCoordinator.spec.ts @@ -138,4 +138,15 @@ describe("sidebarDataOpenCoordinator", () => { expect(signal?.aborted).toBe(true); }); + + it("marks a completed request non-current without aborting its signal", async () => { + let requestState: { isCurrent: () => boolean; signal: AbortSignal } | undefined; + + runSidebarDataOpenImmediately({ connectionKey: "conn" }, (request) => { + requestState = request; + }); + + await vi.waitFor(() => expect(requestState?.isCurrent()).toBe(false)); + expect(requestState?.signal.aborted).toBe(false); + }); }); diff --git a/apps/desktop/src/lib/sidebar/dataTabOpenPolicy.ts b/apps/desktop/src/lib/sidebar/dataTabOpenPolicy.ts index 4933094385..b94524fe04 100644 --- a/apps/desktop/src/lib/sidebar/dataTabOpenPolicy.ts +++ b/apps/desktop/src/lib/sidebar/dataTabOpenPolicy.ts @@ -37,6 +37,10 @@ function isSameTable(tab: DataTabLike, target: DataTabTarget): boolean { return isSameDatabase(tab, target) && (tab.tableMeta?.catalog || "") === (target.catalog || "") && (tab.schema || "") === (target.schema || "") && (tab.tableMeta?.tableName || tab.title) === target.tableName; } +export function canApplyDataTabMetadata(tab: DataTabLike | undefined, target: DataTabTarget, signal?: AbortSignal): boolean { + return signal?.aborted !== true && tab !== undefined && isSameTable(tab, target); +} + export function findExistingDataTabCandidate(tabs: T[], target: DataTabTarget, options: { openMode: DataTabOpenMode; reuseDataTab: boolean }): ExistingDataTabCandidate | undefined { if (options.openMode === "new-tab") return undefined;