From faf259fcc2012ed17d2f2cbadd9fb90b21a48ee1 Mon Sep 17 00:00:00 2001 From: soddygo Date: Tue, 15 Sep 2026 12:21:18 +0800 Subject: [PATCH] fix(metadata): await object cache invalidation on connection refresh --- .../sidebar/SidebarTreeRuntimeHost.vue | 14 ++ ...TreeRuntimeHost.objectCacheRefresh.spec.ts | 31 +++ apps/desktop/src/i18n/locales/en.ts | 1 + apps/desktop/src/i18n/locales/es.ts | 1 + apps/desktop/src/i18n/locales/it.ts | 1 + apps/desktop/src/i18n/locales/ja.ts | 1 + apps/desktop/src/i18n/locales/ko.ts | 1 + apps/desktop/src/i18n/locales/pt-BR.ts | 1 + apps/desktop/src/i18n/locales/zh-CN.ts | 1 + apps/desktop/src/i18n/locales/zh-TW.ts | 1 + .../metadata/__tests__/objectDdlCache.spec.ts | 153 ++++++++++++++ .../__tests__/objectMetadataCache.spec.ts | 29 +++ .../metadata/objectCacheInvalidationError.ts | 41 ++++ .../src/lib/metadata/objectDdlCache.ts | 34 +-- .../src/lib/metadata/objectMetadataCache.ts | 33 +-- .../__tests__/connectionStore.refresh.spec.ts | 200 ++++++++++++++++++ apps/desktop/src/stores/connectionStore.ts | 25 ++- 17 files changed, 536 insertions(+), 32 deletions(-) create mode 100644 apps/desktop/src/components/sidebar/__tests__/SidebarTreeRuntimeHost.objectCacheRefresh.spec.ts create mode 100644 apps/desktop/src/lib/metadata/objectCacheInvalidationError.ts diff --git a/apps/desktop/src/components/sidebar/SidebarTreeRuntimeHost.vue b/apps/desktop/src/components/sidebar/SidebarTreeRuntimeHost.vue index cb9583e77d..036ab72e5a 100644 --- a/apps/desktop/src/components/sidebar/SidebarTreeRuntimeHost.vue +++ b/apps/desktop/src/components/sidebar/SidebarTreeRuntimeHost.vue @@ -162,6 +162,7 @@ import { formatSqlForDisplay, sqlFormatDialectForDbType } from "@/lib/sql/sqlFor import { omitDdlIdentifierQuotes } from "@/lib/sql/ddlDisplay"; import { getTableStructureCapabilities } from "@/lib/table/tableStructureCapabilities"; import { connectionObjectTreeNodeSchema, connectionObjectTreeQuerySchema, connectionTableSqlSchema, connectionUsesDatabaseObjectTreeMode, effectiveDatabaseTypeForConnection, tableStructureDatabaseTypeForConnection } from "@/lib/database/jdbcDialect"; +import { isObjectCacheInvalidationError } from "@/lib/metadata/objectCacheInvalidationError"; import { hasTreeNodeDatabaseContext } from "@/lib/sidebar/treeNodeContext"; import { defaultPasteTableMode, @@ -2191,6 +2192,19 @@ function openElasticsearchIndexMetadata(kind: ElasticsearchIndexMetadataKind) { async function refresh() { const node = activeNode.value; + if (node.type === "connection" && node.connectionId) { + try { + await connectionStore.refreshConnectionTreeNode(node); + } catch (e: any) { + if (isObjectCacheInvalidationError(e)) { + toast(t("connection.objectCacheRefreshFailed", { message: translateBackendError(t, e) }), 5000); + return; + } + toast(t("connection.connectFailed", { message: translateBackendError(t, e) }), 5000); + openDriverStoreForInstallError(e?.message || String(e), node); + } + return; + } try { await connectionStore.refreshTreeNode(node); } catch (e: any) { diff --git a/apps/desktop/src/components/sidebar/__tests__/SidebarTreeRuntimeHost.objectCacheRefresh.spec.ts b/apps/desktop/src/components/sidebar/__tests__/SidebarTreeRuntimeHost.objectCacheRefresh.spec.ts new file mode 100644 index 0000000000..ff616d8129 --- /dev/null +++ b/apps/desktop/src/components/sidebar/__tests__/SidebarTreeRuntimeHost.objectCacheRefresh.spec.ts @@ -0,0 +1,31 @@ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; + +const source = readFileSync(new URL("../SidebarTreeRuntimeHost.vue", import.meta.url), "utf8"); + +/** + * Secondary evidence for the connection-node refresh wiring. The primary + * behavioral coverage lives in connectionStore.refresh.spec.ts and the + * metadata cache specs; this guards the component-side routing and error + * classification without mounting the full sidebar tree host. + */ +describe("SidebarTreeRuntimeHost connection refresh cache invalidation", () => { + it("routes connection-node refresh through the awaited strict store method", () => { + expect(source).toMatch(/if \(node\.type === "connection" && node\.connectionId\) \{\s*try \{\s*await connectionStore\.refreshConnectionTreeNode\(node\);/); + }); + + it("reports object cache failures distinctly without opening the driver store", () => { + const connectionBranch = source.slice(source.indexOf("await connectionStore.refreshConnectionTreeNode(node);"), source.indexOf("await connectionStore.refreshTreeNode(node);", source.indexOf("await connectionStore.refreshConnectionTreeNode(node);"))); + const cacheFailureBlock = connectionBranch.match(/if \(isObjectCacheInvalidationError\(e\)\) \{[\s\S]*?\n\s*return;\s*\n/); + expect(cacheFailureBlock).not.toBeNull(); + expect(cacheFailureBlock![0]).toContain('t("connection.objectCacheRefreshFailed"'); + expect(cacheFailureBlock![0]).not.toContain("openDriverStoreForInstallError"); + }); + + it("keeps the regular refresh path and its connection-error handling for other nodes", () => { + const refresh = source.slice(source.indexOf("async function refresh()"), source.indexOf("async function copyName()")); + const nonConnection = refresh.slice(refresh.indexOf("await connectionStore.refreshTreeNode(node);")); + expect(nonConnection).toMatch(/toast\(t\("connection\.connectFailed"/); + expect(nonConnection).toContain("openDriverStoreForInstallError"); + }); +}); diff --git a/apps/desktop/src/i18n/locales/en.ts b/apps/desktop/src/i18n/locales/en.ts index d12dc6d06e..079473d175 100644 --- a/apps/desktop/src/i18n/locales/en.ts +++ b/apps/desktop/src/i18n/locales/en.ts @@ -1204,6 +1204,7 @@ export default { connecting: "Connecting to {name}...", connectSuccess: "Connected to {name}", connectFailed: "Connection failed: {message}", + objectCacheRefreshFailed: "Object cache refresh failed: {message}", driverNotInstalled: "{driver} driver is not installed. Please install it from the Driver Manager.", phoenixDriverNotInstalled: "The Apache Phoenix JDBC driver is not installed. Install it from the Driver Manager, then retry.", jreNotInstalled: "JRE {jre} runtime is not installed. Please install it from the Driver Manager.", diff --git a/apps/desktop/src/i18n/locales/es.ts b/apps/desktop/src/i18n/locales/es.ts index 6bc0467672..a99b4e2a85 100644 --- a/apps/desktop/src/i18n/locales/es.ts +++ b/apps/desktop/src/i18n/locales/es.ts @@ -1403,6 +1403,7 @@ export default withEnglishFallback({ pluginSshKeyPickerPlaceholder: "Seleccionar clave SSH local…", pluginSshKeyPickerEmpty: "No se encontraron claves", pluginSshKeyEncrypted: "Cifrado", + objectCacheRefreshFailed: "Error al actualizar la caché de objetos: {message}", }, editor: { duckdbDraining: "La consulta anterior de DuckDB aún se está deteniendo. Vuelva a intentarlo en breve.", diff --git a/apps/desktop/src/i18n/locales/it.ts b/apps/desktop/src/i18n/locales/it.ts index 9f232d200a..f10972900b 100644 --- a/apps/desktop/src/i18n/locales/it.ts +++ b/apps/desktop/src/i18n/locales/it.ts @@ -1401,6 +1401,7 @@ export default withEnglishFallback({ pluginSshKeyPickerPlaceholder: "Seleziona chiave SSH locale…", pluginSshKeyPickerEmpty: "Nessuna chiave trovata", pluginSshKeyEncrypted: "Crittografata", + objectCacheRefreshFailed: "Aggiornamento della cache degli oggetti non riuscito: {message}", }, editor: { duckdbDraining: "La query DuckDB precedente è ancora in fase di arresto. Riprova a breve.", diff --git a/apps/desktop/src/i18n/locales/ja.ts b/apps/desktop/src/i18n/locales/ja.ts index 0e0c709edc..c966fe0f25 100644 --- a/apps/desktop/src/i18n/locales/ja.ts +++ b/apps/desktop/src/i18n/locales/ja.ts @@ -1422,6 +1422,7 @@ export default withEnglishFallback({ pluginSshKeyPickerPlaceholder: "ローカルの SSH キーを選択…", pluginSshKeyPickerEmpty: "キーが見つかりません", pluginSshKeyEncrypted: "暗号化済み", + objectCacheRefreshFailed: "オブジェクトキャッシュの更新に失敗しました：{message}", }, editor: { duckdbDraining: "前回の DuckDB クエリはまだ停止処理中です。しばらくしてから再試行してください。", diff --git a/apps/desktop/src/i18n/locales/ko.ts b/apps/desktop/src/i18n/locales/ko.ts index 7800de0634..150b94f981 100644 --- a/apps/desktop/src/i18n/locales/ko.ts +++ b/apps/desktop/src/i18n/locales/ko.ts @@ -1320,6 +1320,7 @@ export default withEnglishFallback({ pluginSshKeyPickerPlaceholder: "로컬 SSH 키 선택…", pluginSshKeyPickerEmpty: "키를 찾을 수 없음", pluginSshKeyEncrypted: "암호화됨", + objectCacheRefreshFailed: "개체 캐시 새로 고침 실패: {message}", }, editor: { duckdbDraining: "이전 DuckDB 쿼리가 아직 중지 중입니다. 잠시 후 다시 시도해 주세요.", diff --git a/apps/desktop/src/i18n/locales/pt-BR.ts b/apps/desktop/src/i18n/locales/pt-BR.ts index 35a6fe9b13..47dfacb8d6 100644 --- a/apps/desktop/src/i18n/locales/pt-BR.ts +++ b/apps/desktop/src/i18n/locales/pt-BR.ts @@ -1402,6 +1402,7 @@ export default withEnglishFallback({ pluginSshKeyPickerPlaceholder: "Selecionar chave SSH local…", pluginSshKeyPickerEmpty: "Nenhuma chave encontrada", pluginSshKeyEncrypted: "Criptografada", + objectCacheRefreshFailed: "Falha ao atualizar o cache de objetos: {message}", }, editor: { duckdbDraining: "A consulta anterior do DuckDB ainda está sendo interrompida. Tente novamente em breve.", diff --git a/apps/desktop/src/i18n/locales/zh-CN.ts b/apps/desktop/src/i18n/locales/zh-CN.ts index e596ff3043..33201f0f57 100644 --- a/apps/desktop/src/i18n/locales/zh-CN.ts +++ b/apps/desktop/src/i18n/locales/zh-CN.ts @@ -1122,6 +1122,7 @@ export default withEnglishFallback({ sshTotpCancelled: "已取消输入一次性验证码", connectSuccess: "已连接 {name}", connectFailed: "连接失败：{message}", + objectCacheRefreshFailed: "对象缓存刷新失败：{message}", driverNotInstalled: "{driver} 驱动未安装，请在驱动管理器中安装。", phoenixDriverNotInstalled: "Apache Phoenix JDBC 驱动未安装，请在驱动管理器中安装后重试。", jreNotInstalled: "JRE {jre} 运行环境未安装，请在驱动管理器中安装。", diff --git a/apps/desktop/src/i18n/locales/zh-TW.ts b/apps/desktop/src/i18n/locales/zh-TW.ts index d7ded4a219..cdebc02a43 100644 --- a/apps/desktop/src/i18n/locales/zh-TW.ts +++ b/apps/desktop/src/i18n/locales/zh-TW.ts @@ -978,6 +978,7 @@ export default withEnglishFallback({ connecting: "正在連線 {name}……", connectSuccess: "已連線 {name}", connectFailed: "連線失敗：{message}", + objectCacheRefreshFailed: "物件快取重新整理失敗：{message}", driverNotInstalled: "{driver} 驅動程式未安裝，請在驅動程式管理器中安裝。", phoenixDriverNotInstalled: "Apache Phoenix JDBC 驅動程式未安裝，請在驅動程式管理器中安裝後再試。", jreNotInstalled: "JRE {jre} 執行環境未安裝，請在驅動程式管理器中安裝。", diff --git a/apps/desktop/src/lib/metadata/__tests__/objectDdlCache.spec.ts b/apps/desktop/src/lib/metadata/__tests__/objectDdlCache.spec.ts index 25c3257804..c2b4815043 100644 --- a/apps/desktop/src/lib/metadata/__tests__/objectDdlCache.spec.ts +++ b/apps/desktop/src/lib/metadata/__tests__/objectDdlCache.spec.ts @@ -11,6 +11,7 @@ const mocks = vi.hoisted(() => ({ vi.mock("@/lib/backend/api", () => mocks); import { cancelObjectDdlLoadsForConnection, getObjectDdlCacheDebugStateForTests, invalidateObjectDdl, invalidateObjectDdlCache, loadObjectDdl, objectDdlCacheKey } from "@/lib/metadata/objectDdlCache"; +import { isObjectCacheInvalidationError } from "@/lib/metadata/objectCacheInvalidationError"; import { clearMetadataRuntimeCache } from "@/lib/metadata/metadataRuntimeCache"; const request = { connectionId: "c1", database: "app", schema: "public", tableName: "users", catalog: "analytics" } as const; @@ -263,4 +264,156 @@ describe("objectDdlCache", () => { expect(mocks.getTableDisplayDdl).toHaveBeenCalledTimes(2); expect(mocks.deleteSchemaCachePrefix).not.toHaveBeenCalledWith("object-ddl:v1:c1:"); }); + + describe("strict connection-level invalidation", () => { + const connectionMatch = { connectionId: "c1" }; + + it.each(["object-ddl:v1:c1:", "object-meta:v1:c1:"])("waits for the other namespace when %s fails", async (failedPrefix) => { + let release!: () => void; + const pending = new Promise((resolve) => { + release = resolve; + }); + mocks.deleteSchemaCachePrefix.mockImplementation((prefix: string) => (prefix === failedPrefix ? Promise.reject(new Error("locked")) : pending)); + let settled = false; + const outcome = invalidateObjectDdlCache(connectionMatch, { strict: true }).then( + () => { + settled = true; + return undefined; + }, + (error: unknown) => { + settled = true; + return error; + }, + ); + await vi.waitFor(() => expect(mocks.deleteSchemaCachePrefix).toHaveBeenCalledTimes(2)); + // Drain the rejection handlers, without releasing the other deletion. + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(settled).toBe(false); + release(); + const failure = await outcome; + expect(isObjectCacheInvalidationError(failure)).toBe(true); + expect(failure).toMatchObject({ scope: failedPrefix, message: "locked" }); + }); + + function objectPrefixes(): string[] { + return mocks.deleteSchemaCachePrefix.mock.calls.map(([prefix]) => prefix as string); + } + + function rejectAllDeletions(error: Error) { + let rejectGate: (reason: Error) => void = () => {}; + const gate = new Promise((_resolve, reject) => { + rejectGate = reject; + }); + mocks.deleteSchemaCachePrefix.mockImplementation(() => + gate.then( + () => undefined, + () => { + throw error; + }, + ), + ); + return () => rejectGate(error); + } + + it("rejects with a marked object cache error when the persisted deletion fails", async () => { + mocks.deleteSchemaCachePrefix.mockRejectedValue(new Error("sqlite locked")); + + const failure = await invalidateObjectDdlCache(connectionMatch, { strict: true }).then( + () => undefined, + (error: unknown) => error, + ); + expect(failure).toBeInstanceOf(Error); + expect(isObjectCacheInvalidationError(failure)).toBe(true); + expect((failure as Error).message).toBe("sqlite locked"); + // Both the DDL and the object-metadata namespace deletions were attempted. + expect(objectPrefixes()).toContain("object-ddl:v1:c1:"); + expect(objectPrefixes()).toContain("object-meta:v1:c1:"); + }); + + it("keeps default invalidation best-effort when the persisted deletion fails", async () => { + mocks.deleteSchemaCachePrefix.mockRejectedValue(new Error("sqlite locked")); + + await expect(invalidateObjectDdlCache(connectionMatch)).resolves.toBeUndefined(); + expect(objectPrefixes()).toEqual(["object-ddl:v1:c1:", "object-meta:v1:c1:"]); + }); + + it("still rejects in strict mode when a deletion rejects without a reason", async () => { + // A bare Promise.reject() has reason undefined; settled status, not the + // reason's value, must decide failure. + mocks.deleteSchemaCachePrefix.mockRejectedValue(undefined as never); + + const failure = await invalidateObjectDdlCache(connectionMatch, { strict: true }).then( + () => undefined, + (error: unknown) => error, + ); + expect(isObjectCacheInvalidationError(failure)).toBe(true); + }); + + it("lets a strict call sharing a pending best-effort deletion observe the failure", async () => { + const failAll = rejectAllDeletions(new Error("disk full")); + + const bestEffort = invalidateObjectDdlCache(connectionMatch); + const strict = invalidateObjectDdlCache(connectionMatch, { strict: true }); + // The strict call must reuse the in-flight deletions instead of starting new ones. + await vi.waitFor(() => expect(objectPrefixes()).toEqual(["object-ddl:v1:c1:", "object-meta:v1:c1:"])); + + failAll(); + await expect(bestEffort).resolves.toBeUndefined(); + const failure = await strict.then( + () => undefined, + (error: unknown) => error, + ); + expect(isObjectCacheInvalidationError(failure)).toBe(true); + expect((failure as Error).message).toBe("disk full"); + }); + + it("retries the persisted deletion on the next strict invalidation after a failure", async () => { + mocks.deleteSchemaCachePrefix.mockRejectedValueOnce(new Error("locked")).mockRejectedValueOnce(new Error("locked")); + + await expect(invalidateObjectDdlCache(connectionMatch, { strict: true })).rejects.toThrow("locked"); + + await expect(invalidateObjectDdlCache(connectionMatch, { strict: true })).resolves.toBeUndefined(); + const prefixes = objectPrefixes(); + expect(prefixes.filter((prefix) => prefix === "object-ddl:v1:c1:")).toHaveLength(2); + expect(prefixes.filter((prefix) => prefix === "object-meta:v1:c1:")).toHaveLength(2); + }); + + it("keeps non-force reads working when a concurrent deletion fails", async () => { + const failAll = rejectAllDeletions(new Error("locked")); + mocks.getTableDisplayDdl.mockResolvedValue("fresh ddl"); + + const invalidation = invalidateObjectDdlCache({ connectionId: request.connectionId, database: request.database, schema: request.schema, tableName: request.tableName }); + const read = loadObjectDdl(request); + failAll(); + await expect(read).resolves.toEqual({ ddl: "fresh ddl", cacheStatus: "remote" }); + await expect(invalidation).resolves.toBeUndefined(); + }); + + it("force refresh proceeds when the persisted deletion fails", async () => { + mocks.deleteSchemaCachePrefix.mockRejectedValue(new Error("locked")); + mocks.getTableDisplayDdl.mockResolvedValue("new ddl"); + + await expect(loadObjectDdl(request, { force: true })).resolves.toEqual({ ddl: "new ddl", cacheStatus: "remote" }); + }); + + it("keeps object-level invalidation best-effort when the deletion fails", async () => { + mocks.deleteSchemaCachePrefix.mockRejectedValue(new Error("locked")); + + await expect(invalidateObjectDdl(request)).resolves.toBeUndefined(); + }); + + it("encodes connection ids and isolates similar ids in persisted deletions", async () => { + mocks.persisted.set("object-ddl:v1:conn10:db:public:t::TABLE:", { version: 1, cachedAt: new Date().toISOString(), ddl: "conn10 ddl" }); + + await invalidateObjectDdlCache({ connectionId: "conn1" }); + + expect(objectPrefixes()).toContain("object-ddl:v1:conn1:"); + expect(objectPrefixes()).toContain("object-meta:v1:conn1:"); + expect([...mocks.persisted.keys()]).toContain("object-ddl:v1:conn10:db:public:t::TABLE:"); + + await invalidateObjectDdlCache({ connectionId: "a:b c%中" }); + expect(mocks.deleteSchemaCachePrefix).toHaveBeenCalledWith("object-ddl:v1:a%3Ab%20c%25%E4%B8%AD:"); + expect(mocks.deleteSchemaCachePrefix).toHaveBeenCalledWith("object-meta:v1:a%3Ab%20c%25%E4%B8%AD:"); + }); + }); }); diff --git a/apps/desktop/src/lib/metadata/__tests__/objectMetadataCache.spec.ts b/apps/desktop/src/lib/metadata/__tests__/objectMetadataCache.spec.ts index 23c8f94d45..ec63fe15ce 100644 --- a/apps/desktop/src/lib/metadata/__tests__/objectMetadataCache.spec.ts +++ b/apps/desktop/src/lib/metadata/__tests__/objectMetadataCache.spec.ts @@ -10,6 +10,7 @@ const mocks = vi.hoisted(() => ({ vi.mock("@/lib/backend/api", () => mocks); import { cancelObjectMetadataLoadsForConnection, getObjectMetadataCacheDebugStateForTests, invalidateObjectMetadataCache, loadObjectMetadataFacet } from "@/lib/metadata/objectMetadataCache"; +import { isObjectCacheInvalidationError } from "@/lib/metadata/objectCacheInvalidationError"; import { clearMetadataRuntimeCache } from "@/lib/metadata/metadataRuntimeCache"; const request = { connectionId: "c1", database: "app", schema: "public", tableName: "users", catalog: "analytics" } as const; @@ -252,4 +253,32 @@ describe("objectMetadataCache", () => { expect(loader).toHaveBeenCalledTimes(2); expect(mocks.deleteSchemaCachePrefix).not.toHaveBeenCalledWith("object-meta:v1:c1:"); }); + + describe("strict invalidation", () => { + it("rejects with a marked object cache error in strict mode and stays best-effort by default", async () => { + mocks.deleteSchemaCachePrefix.mockRejectedValue(new Error("sqlite locked")); + + const failure = await invalidateObjectMetadataCache({ connectionId: "c1" }, { strict: true }).then( + () => undefined, + (error: unknown) => error, + ); + expect(failure).toBeInstanceOf(Error); + expect(isObjectCacheInvalidationError(failure)).toBe(true); + expect((failure as Error).message).toBe("sqlite locked"); + expect(mocks.deleteSchemaCachePrefix).toHaveBeenCalledWith("object-meta:v1:c1:"); + + mocks.deleteSchemaCachePrefix.mockClear(); + mocks.deleteSchemaCachePrefix.mockRejectedValue(new Error("sqlite locked")); + await expect(invalidateObjectMetadataCache({ connectionId: "c1" })).resolves.toBeUndefined(); + expect(mocks.deleteSchemaCachePrefix).toHaveBeenCalledWith("object-meta:v1:c1:"); + }); + + it("force facet loads tolerate a failed persisted deletion", async () => { + mocks.deleteSchemaCachePrefix.mockRejectedValue(new Error("locked")); + const loader = vi.fn().mockResolvedValue(["fresh"]); + + await expect(loadObjectMetadataFacet(request, "columns", loader, { force: true })).resolves.toEqual({ value: ["fresh"], cacheStatus: "remote" }); + expect(loader).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/apps/desktop/src/lib/metadata/objectCacheInvalidationError.ts b/apps/desktop/src/lib/metadata/objectCacheInvalidationError.ts new file mode 100644 index 0000000000..b3e9e42636 --- /dev/null +++ b/apps/desktop/src/lib/metadata/objectCacheInvalidationError.ts @@ -0,0 +1,41 @@ +/** + * Error marker for strict object-cache invalidation failures. + * + * Best-effort invalidation callers keep swallowing persistence errors, so the + * only signal a strict caller (connection-node refresh) can rely on is an + * explicitly marked error. Detection is name-based rather than instanceof so + * duplicated module instances (tests reset modules) still match. + */ + +export interface ObjectCacheInvalidationOptions { + /** Propagate persistence deletion failures instead of swallowing them. */ + strict?: boolean; +} + +const OBJECT_CACHE_INVALIDATION_ERROR_NAME = "ObjectCacheInvalidationError"; + +export class ObjectCacheInvalidationError extends Error { + readonly code = "OBJECT_CACHE_INVALIDATION_FAILED"; + /** Cache prefix whose persisted deletion failed. */ + readonly scope: string; + /** Original error thrown by the deletion. */ + readonly reason: unknown; + + constructor(message: string, scope: string, reason?: unknown) { + super(message); + this.name = OBJECT_CACHE_INVALIDATION_ERROR_NAME; + this.scope = scope; + this.reason = reason; + } +} + +export function isObjectCacheInvalidationError(value: unknown): value is ObjectCacheInvalidationError { + return value instanceof Error && value.name === OBJECT_CACHE_INVALIDATION_ERROR_NAME; +} + +/** Wrap a deletion failure, passing through an already-marked error unchanged. */ +export function toObjectCacheInvalidationError(error: unknown, scope: string): ObjectCacheInvalidationError { + if (isObjectCacheInvalidationError(error)) return error; + const message = error instanceof Error ? error.message : String(error); + return new ObjectCacheInvalidationError(message, scope, error); +} diff --git a/apps/desktop/src/lib/metadata/objectDdlCache.ts b/apps/desktop/src/lib/metadata/objectDdlCache.ts index 3e78c0e31d..f6d67fa002 100644 --- a/apps/desktop/src/lib/metadata/objectDdlCache.ts +++ b/apps/desktop/src/lib/metadata/objectDdlCache.ts @@ -3,6 +3,7 @@ import { ActiveCacheReadTracker } from "./activeCacheReadTracker"; import type { ObjectSourceKind } from "@/types/database"; import type { MetadataCacheInvalidation } from "./metadataResultCache"; import { invalidateObjectMetadataCache } from "./objectMetadataCache"; +import { toObjectCacheInvalidationError, type ObjectCacheInvalidationOptions } from "./objectCacheInvalidationError"; import { getMetadataRuntimeCache, invalidateMetadataRuntimeCachePrefix, recordMetadataCacheL2Hit, recordMetadataCacheRemoteMiss, setMetadataRuntimeCache } from "./metadataRuntimeCache"; const OBJECT_DDL_CACHE_PREFIX = "object-ddl:v1"; @@ -56,14 +57,6 @@ async function saveSchemaCacheSafe(cacheKey: string, payload: unknown): Promise< } } -async function deleteSchemaCachePrefixSafe(prefix: string): Promise { - try { - await api.deleteSchemaCachePrefix(prefix); - } catch { - // Cache invalidation is best effort when running with a reduced backend. - } -} - function cacheSegment(value: string | undefined): string { return encodeURIComponent(value ?? ""); } @@ -94,7 +87,9 @@ function decodeCachedDdl(payload: unknown): string | null { } async function waitForPendingInvalidations(cacheKey: string): Promise { - const pending = [...pendingInvalidations.entries()].filter(([prefix]) => cacheKey.startsWith(prefix)).map(([, promise]) => promise); + // Readers only wait for pending deletions to settle; the failure itself is + // reported by the caller that started the invalidation. + const pending = [...pendingInvalidations.entries()].filter(([prefix]) => cacheKey.startsWith(prefix)).map(([, promise]) => promise.catch(() => undefined)); if (pending.length) await Promise.all(pending); } @@ -129,7 +124,9 @@ function invalidatePersistedPrefix(prefix: string): Promise { if (existing) return existing; const deletion = (async () => { await waitForPendingWrites(prefix); - await deleteSchemaCachePrefixSafe(prefix); + // Deletion failures propagate so strict callers can observe them; + // best-effort callers catch at their own boundary. + await api.deleteSchemaCachePrefix(prefix); })().finally(() => { if (pendingInvalidations.get(prefix) === deletion) pendingInvalidations.delete(prefix); }); @@ -174,7 +171,8 @@ export async function loadObjectDdl(request: ObjectDdlRequest, options?: { force invalidateMetadataRuntimeCachePrefix(cacheKey); const priorInvalidations = waitForPendingInvalidations(cacheKey); const deletion = invalidatePersistedPrefix(cacheKey); - await Promise.all([priorInvalidations, deletion]); + // A failed prefix deletion must not fail the force load itself. + await Promise.allSettled([priorInvalidations, deletion]); } else { await waitForPendingInvalidations(cacheKey); } @@ -203,14 +201,22 @@ export async function loadObjectDdl(request: ObjectDdlRequest, options?: { force return { ddl: await loadRemoteDdl(request, cacheKey, options?.force === true), cacheStatus: "remote" }; } -export async function invalidateObjectDdlCache(match: MetadataCacheInvalidation): Promise { +export async function invalidateObjectDdlCache(match: MetadataCacheInvalidation, options?: ObjectCacheInvalidationOptions): Promise { const prefix = invalidationPrefix(match); activeCacheReads.invalidatePrefix(prefix); for (const cacheKey of [...remoteLoads.keys()]) if (cacheKey.startsWith(prefix)) invalidateRemoteLoad(cacheKey); invalidateMetadataRuntimeCachePrefix(prefix); const deletion = invalidatePersistedPrefix(prefix); - await Promise.all([deletion, invalidateObjectMetadataCache(match)]); + const metadataInvalidation = invalidateObjectMetadataCache(match, options); + // Wait for both namespace deletions to settle before deciding, so a strict + // caller never reports failure while the other deletion is still in flight. + const [ddlSettled, metaSettled] = await Promise.allSettled([deletion, metadataInvalidation]); + if (!options?.strict) return; + // Judge by settled status, never by the rejection reason's value: a bare + // Promise.reject() has reason undefined and must still count as a failure. + if (ddlSettled.status === "rejected") throw toObjectCacheInvalidationError(ddlSettled.reason, prefix); + if (metaSettled.status === "rejected") throw toObjectCacheInvalidationError(metaSettled.reason, prefix); } export async function invalidateObjectDdl(request: ObjectDdlRequest): Promise { @@ -218,7 +224,7 @@ export async function invalidateObjectDdl(request: ObjectDdlRequest): Promise { - try { - await api.deleteSchemaCachePrefix(prefix); - } catch { - // Cache invalidation is best effort when running with a reduced backend. - } -} - export type ObjectMetadataFacet = "columns" | "indexes" | "foreign-keys" | "constraints" | "triggers" | "comment" | "owner"; function cacheSegment(value: string | undefined): string { @@ -81,7 +74,9 @@ function decodeEnvelope(payload: unknown): { hit: true; value: T } | undefine } async function waitForPendingInvalidations(cacheKey: string): Promise { - const pending = [...pendingInvalidations.entries()].filter(([prefix]) => cacheKey.startsWith(prefix)).map(([, promise]) => promise); + // Readers only wait for pending deletions to settle; the failure itself is + // reported by the caller that started the invalidation. + const pending = [...pendingInvalidations.entries()].filter(([prefix]) => cacheKey.startsWith(prefix)).map(([, promise]) => promise.catch(() => undefined)); if (pending.length) await Promise.all(pending); } @@ -116,7 +111,9 @@ function invalidatePersistedPrefix(prefix: string): Promise { if (existing) return existing; const deletion = (async () => { await waitForPendingWrites(prefix); - await deleteSchemaCachePrefixSafe(prefix); + // Deletion failures propagate so strict callers can observe them; + // best-effort callers catch at their own boundary. + await api.deleteSchemaCachePrefix(prefix); })().finally(() => { if (pendingInvalidations.get(prefix) === deletion) pendingInvalidations.delete(prefix); }); @@ -134,7 +131,8 @@ export async function loadObjectMetadataFacet(request: ObjectDdlRequest, face invalidateMetadataRuntimeCachePrefix(cacheKey); const priorInvalidations = waitForPendingInvalidations(cacheKey); const deletion = invalidatePersistedPrefix(cacheKey); - await Promise.all([priorInvalidations, deletion]); + // A failed prefix deletion must not fail the force load itself. + await Promise.allSettled([priorInvalidations, deletion]); } else { await waitForPendingInvalidations(cacheKey); } @@ -188,13 +186,22 @@ export async function loadObjectMetadataFacet(request: ObjectDdlRequest, face return { value: await entry.promise, cacheStatus: "remote" }; } -export async function invalidateObjectMetadataCache(match: MetadataCacheInvalidation): Promise { +export async function invalidateObjectMetadataCache(match: MetadataCacheInvalidation, options?: ObjectCacheInvalidationOptions): Promise { const prefix = invalidationPrefix(match); activeCacheReads.invalidatePrefix(prefix); for (const cacheKey of [...inFlightLoads.keys()]) if (cacheKey.startsWith(prefix)) invalidateInFlightLoad(cacheKey); invalidateMetadataRuntimeCachePrefix(prefix); - return invalidatePersistedPrefix(prefix); + const deletion = invalidatePersistedPrefix(prefix); + if (!options?.strict) { + await deletion.catch(() => undefined); + return; + } + try { + await deletion; + } catch (error) { + throw toObjectCacheInvalidationError(error, prefix); + } } /** Invalidate active loads for a disconnected connection without deleting its persisted snapshot. */ diff --git a/apps/desktop/src/stores/__tests__/connectionStore.refresh.spec.ts b/apps/desktop/src/stores/__tests__/connectionStore.refresh.spec.ts index f9e9cae3d4..b31aad3cd7 100644 --- a/apps/desktop/src/stores/__tests__/connectionStore.refresh.spec.ts +++ b/apps/desktop/src/stores/__tests__/connectionStore.refresh.spec.ts @@ -1,5 +1,6 @@ import { createPinia, setActivePinia } from "pinia"; import { beforeEach, describe, expect, it, vi } from "vitest"; +import { isObjectCacheInvalidationError } from "@/lib/metadata/objectCacheInvalidationError"; import type { ConnectionConfig, TreeNode } from "@/types/database"; function installLocalStorage() { @@ -167,3 +168,202 @@ describe("connectionStore tree refresh state", () => { expect(store.completionCacheRevision("pg-1", "app")).toBeGreaterThan(0); }); }); + +describe("connection node refresh awaits object cache invalidation", () => { + function installConnectionApiMocks(options?: { deleteSchemaCachePrefix?: ReturnType; getColumns?: ReturnType; getTableDisplayDdl?: ReturnType; listDatabases?: ReturnType; listSchemaInfos?: ReturnType }) { + vi.doMock("@/lib/backend/tauriRuntime", () => ({ isTauriRuntime: () => false })); + vi.doMock("@/lib/backend/api", () => ({ + checkConnectionHealth: vi.fn().mockResolvedValue(undefined), + deleteSchemaCachePrefix: options?.deleteSchemaCachePrefix ?? vi.fn().mockResolvedValue(undefined), + getColumns: options?.getColumns ?? vi.fn().mockResolvedValue([]), + getTableDisplayDdl: options?.getTableDisplayDdl ?? vi.fn().mockResolvedValue("CREATE TABLE users (id int)"), + listDatabases: options?.listDatabases ?? vi.fn().mockResolvedValue([{ name: "app", comment: null }]), + listInstalledAgents: vi.fn().mockResolvedValue([]), + listObjects: vi.fn().mockResolvedValue([]), + listSchemaInfos: options?.listSchemaInfos ?? vi.fn().mockResolvedValue([{ name: "public", comment: null }]), + listTables: vi.fn().mockResolvedValue([]), + loadSchemaCache: vi.fn().mockResolvedValue(null), + saveConnections: vi.fn().mockResolvedValue(undefined), + saveSchemaCache: vi.fn().mockResolvedValue(undefined), + saveSidebarLayout: vi.fn().mockResolvedValue(undefined), + })); + } + + /** A database node under the connection node: refresh reloads it via listSchemaInfos. */ + function databaseNode(): TreeNode { + return { id: "pg-1:app", label: "app", type: "database", connectionId: "pg-1", database: "app", isExpanded: true, children: [] }; + } + + async function createStoreWithConnection(connectionId = "pg-1") { + const { useConnectionStore } = await import("@/stores/connectionStore"); + const { useSettingsStore } = await import("@/stores/settingsStore"); + const store = useConnectionStore(); + useSettingsStore().editorSettings.sidebarObjectDisplay = "grouped"; + const connection: ConnectionConfig = { + ...postgresConnection(), + id: connectionId, + }; + store.connections = [connection]; + store.connectedIds.add(connection.id); + const connectionNode: TreeNode = { + id: connection.id, + label: connection.name, + type: "connection", + connectionId: connection.id, + isExpanded: true, + children: [], + }; + store.treeNodes = [connectionNode]; + return { store, connectionNode }; + } + + function objectCacheDeletePrefixes(mock: ReturnType): string[] { + return mock.mock.calls.filter(([prefix]) => (prefix as string).startsWith("object-")).map(([prefix]) => prefix as string); + } + + /** A prefix-deletion mock whose object-cache deletions share one deferred gate. */ + function gatedObjectCacheDeletes() { + const deleteSchemaCachePrefix = vi.fn((prefix: string) => { + if (!prefix.startsWith("object-")) return Promise.resolve(); + return gate.then(() => undefined); + }); + let resolveGate!: () => void; + const gate = new Promise((resolve) => { + resolveGate = resolve; + }); + return { deleteSchemaCachePrefix, releaseObjectCacheDeletes: () => resolveGate() }; + } + + beforeEach(() => { + vi.resetModules(); + vi.unstubAllGlobals(); + installLocalStorage(); + setActivePinia(createPinia()); + }); + + it("blocks the tree reload until both object cache deletions settle", async () => { + const { deleteSchemaCachePrefix, releaseObjectCacheDeletes } = gatedObjectCacheDeletes(); + const listDatabases = vi.fn().mockResolvedValue([{ name: "app", comment: null }]); + installConnectionApiMocks({ deleteSchemaCachePrefix, listDatabases }); + const { store, connectionNode } = await createStoreWithConnection(); + + let settled = false; + const refresh = store.refreshConnectionTreeNode(connectionNode).finally(() => { + settled = true; + }); + await vi.waitFor(() => expect(objectCacheDeletePrefixes(deleteSchemaCachePrefix).length).toBe(2)); + expect(settled).toBe(false); + expect(listDatabases).not.toHaveBeenCalled(); + + releaseObjectCacheDeletes(); + await refresh; + expect(listDatabases).toHaveBeenCalledTimes(1); + expect(listDatabases).toHaveBeenCalledWith("pg-1"); + }); + + it("issues exactly one object-cache deletion per namespace on a connection refresh", async () => { + const deleteSchemaCachePrefix = vi.fn().mockResolvedValue(undefined); + installConnectionApiMocks({ deleteSchemaCachePrefix }); + const { store, connectionNode } = await createStoreWithConnection(); + + await store.refreshConnectionTreeNode(connectionNode); + + expect(objectCacheDeletePrefixes(deleteSchemaCachePrefix).sort()).toEqual(["object-ddl:v1:pg-1:", "object-meta:v1:pg-1:"]); + }); + + it("lets concurrent connection refreshes share the pending deletions", async () => { + const { deleteSchemaCachePrefix, releaseObjectCacheDeletes } = gatedObjectCacheDeletes(); + installConnectionApiMocks({ deleteSchemaCachePrefix }); + const { store, connectionNode } = await createStoreWithConnection(); + + const first = store.refreshConnectionTreeNode(connectionNode); + const second = store.refreshConnectionTreeNode(connectionNode); + await vi.waitFor(() => expect(objectCacheDeletePrefixes(deleteSchemaCachePrefix).length).toBe(2)); + + releaseObjectCacheDeletes(); + await Promise.all([first, second]); + expect(objectCacheDeletePrefixes(deleteSchemaCachePrefix)).toHaveLength(2); + }); + + it("rejects with the object cache error and skips the tree reload when deletion fails", async () => { + const deleteSchemaCachePrefix = vi.fn((prefix: string) => (prefix.startsWith("object-") ? Promise.reject(new Error("sqlite locked")) : Promise.resolve())); + const listDatabases = vi.fn().mockResolvedValue([{ name: "app", comment: null }]); + installConnectionApiMocks({ deleteSchemaCachePrefix, listDatabases }); + const { store, connectionNode } = await createStoreWithConnection(); + const previousChildren = connectionNode.children; + + const failure = await store.refreshConnectionTreeNode(connectionNode).then( + () => undefined, + (error: unknown) => error, + ); + expect(isObjectCacheInvalidationError(failure)).toBe(true); + expect((failure as Error).message).toBe("sqlite locked"); + expect(listDatabases).not.toHaveBeenCalled(); + expect(connectionNode.children).toBe(previousChildren); + }); + + it("retries the deletion and completes the tree reload on a subsequent refresh", async () => { + let objectDeletionFailures = 2; + const deleteSchemaCachePrefix = vi.fn((prefix: string) => { + if (!prefix.startsWith("object-")) return Promise.resolve(); + return objectDeletionFailures-- > 0 ? Promise.reject(new Error("locked")) : Promise.resolve(); + }); + const listDatabases = vi.fn().mockResolvedValue([{ name: "app", comment: null }]); + installConnectionApiMocks({ deleteSchemaCachePrefix, listDatabases }); + const { store, connectionNode } = await createStoreWithConnection(); + + await expect(store.refreshConnectionTreeNode(connectionNode)).rejects.toThrow("locked"); + expect(listDatabases).not.toHaveBeenCalled(); + + await expect(store.refreshConnectionTreeNode(connectionNode)).resolves.toBeUndefined(); + expect(listDatabases).toHaveBeenCalledTimes(1); + }); + + it("encodes connection ids in the invalidated object cache prefixes", async () => { + const deleteSchemaCachePrefix = vi.fn().mockResolvedValue(undefined); + installConnectionApiMocks({ deleteSchemaCachePrefix }); + const { store, connectionNode } = await createStoreWithConnection("pg:1"); + + await store.refreshConnectionTreeNode(connectionNode); + + expect(objectCacheDeletePrefixes(deleteSchemaCachePrefix).sort()).toEqual(["object-ddl:v1:pg%3A1:", "object-meta:v1:pg%3A1:"]); + }); + + it("keeps non-connection node refresh best-effort when the object deletion fails", async () => { + const deleteSchemaCachePrefix = vi.fn((prefix: string) => (prefix.startsWith("object-") ? Promise.reject(new Error("locked")) : Promise.resolve())); + const listSchemaInfos = vi.fn().mockResolvedValue([{ name: "public", comment: null }]); + installConnectionApiMocks({ deleteSchemaCachePrefix, listSchemaInfos }); + const { store } = await createStoreWithConnection(); + const database = databaseNode(); + store.treeNodes[0]!.children = [database]; + + await expect(store.refreshTreeNode(database)).resolves.toBeUndefined(); + // The tree still reloads even though the object-cache deletion failed. + expect(listSchemaInfos).toHaveBeenCalledTimes(1); + expect(objectCacheDeletePrefixes(deleteSchemaCachePrefix).sort()).toEqual(["object-ddl:v1:pg-1:app:", "object-meta:v1:pg-1:app:"]); + }); + + it("does not fetch object DDL or column metadata during a connection refresh", async () => { + const deleteSchemaCachePrefix = vi.fn().mockResolvedValue(undefined); + const getTableDisplayDdl = vi.fn(); + const getColumns = vi.fn(); + installConnectionApiMocks({ deleteSchemaCachePrefix, getTableDisplayDdl, getColumns }); + const { store, connectionNode } = await createStoreWithConnection(); + + await store.refreshConnectionTreeNode(connectionNode); + + expect(getTableDisplayDdl).not.toHaveBeenCalled(); + expect(getColumns).not.toHaveBeenCalled(); + }); + + it("routes non-connection nodes to the regular refresh path", async () => { + const deleteSchemaCachePrefix = vi.fn().mockResolvedValue(undefined); + installConnectionApiMocks({ deleteSchemaCachePrefix }); + const { store } = await createStoreWithConnection(); + const database = databaseNode(); + store.treeNodes[0]!.children = [database]; + + await expect(store.refreshConnectionTreeNode(database)).resolves.toBeUndefined(); + expect(objectCacheDeletePrefixes(deleteSchemaCachePrefix).sort()).toEqual(["object-ddl:v1:pg-1:app:", "object-meta:v1:pg-1:app:"]); + }); +}); diff --git a/apps/desktop/src/stores/connectionStore.ts b/apps/desktop/src/stores/connectionStore.ts index b537848c52..d1cb7435f8 100644 --- a/apps/desktop/src/stores/connectionStore.ts +++ b/apps/desktop/src/stores/connectionStore.ts @@ -2272,7 +2272,7 @@ export const useConnectionStore = defineStore("connection", () => { }); } - function invalidateMetadataCachesForNode(node: TreeNode) { + function invalidateMetadataCachesForNode(node: TreeNode, options?: { skipObjectCacheInvalidation?: boolean }) { if (!node.connectionId) return; const tableName = node.tableName || (node.type === "table" || node.type === "view" || node.type === "materialized_view" || node.type === "mongo-collection" || node.type === "dynamodb-table" ? node.label : undefined); const match = { @@ -2282,7 +2282,9 @@ export const useConnectionStore = defineStore("connection", () => { tableName, }; invalidateMetadataCaches(match); - void invalidateObjectDdlCache(match); + // Connection-node refresh already awaited a strict object-cache invalidation; + // skip the fire-and-forget duplicate so no late async deletion trails it. + if (!options?.skipObjectCacheInvalidation) void invalidateObjectDdlCache(match); } function invalidateMetadataCache(connectionId: string, database?: string, schema?: string, tableName?: string) { @@ -7054,9 +7056,9 @@ export const useConnectionStore = defineStore("connection", () => { } } - async function refreshTreeNode(node: TreeNode) { + async function refreshTreeNode(node: TreeNode, options?: { skipObjectCacheInvalidation?: boolean }) { invalidateCompletionCachesForNode(node); - invalidateMetadataCachesForNode(node); + invalidateMetadataCachesForNode(node, options); if (objectTypesForGroupNode(node.type)) { clearLoadedChildrenCache(node.id, { deletePersisted: false }); await loadObjectGroupChildren(node, { force: true }); @@ -7067,7 +7069,7 @@ export const useConnectionStore = defineStore("connection", () => { const parentId = objectGroupRefreshParentId(node); const parentNode = parentId ? findNode(treeNodes.value, parentId) : null; if (parentNode) { - await refreshTreeNode(parentNode); + await refreshTreeNode(parentNode, options); return; } @@ -7119,6 +7121,18 @@ export const useConnectionStore = defineStore("connection", () => { } } + /** + * Connection-node refresh: fully invalidate the connection's object caches + * (DDL + object metadata, memory and persisted) and surface deletion + * failures before the tree reload runs. Tree-reload failures propagate + * unmarked and keep the original connect-failure handling. + */ + async function refreshConnectionTreeNode(node: TreeNode): Promise { + if (node.type !== "connection" || !node.connectionId) return refreshTreeNode(node); + await invalidateObjectDdlCache({ connectionId: node.connectionId }, { strict: true }); + await refreshTreeNode(node, { skipObjectCacheInvalidation: true }); + } + async function refreshTreeNodeForTableNameFilter(node: TreeNode, scopeKey: string, revision: number) { invalidateMetadataCachesForNode(node); if (objectTypesForGroupNode(node.type)) { @@ -9223,6 +9237,7 @@ export const useConnectionStore = defineStore("connection", () => { collapseAllTreeNodes, refreshSidebarObjectPagination, refreshTreeNode, + refreshConnectionTreeNode, refreshDatabaseTreeNode, refreshObjectListTreeNode, connectedIds,