diff --git a/src/apps/cli/commands/runCommand.ts b/src/apps/cli/commands/runCommand.ts index c1961500..0c2e320d 100644 --- a/src/apps/cli/commands/runCommand.ts +++ b/src/apps/cli/commands/runCommand.ts @@ -58,7 +58,11 @@ async function verifyRemoteState( standardIo.writeStderr(`[Verification] Failed to connect to remote CouchDB: ${dbRet}\n`); return false; } - milestone = await dbRet.db.get(MILESTONE_DOCID); + try { + milestone = await dbRet.db.get(MILESTONE_DOCID); + } finally { + await dbRet.db.close(); + } } else if (settings.remoteType === REMOTE_MINIO) { milestone = await (replicator as LiveSyncJournalReplicator).client.downloadJson("_00000000-milestone.json"); } diff --git a/src/apps/cli/commands/runCommand.unit.spec.ts b/src/apps/cli/commands/runCommand.unit.spec.ts index f3301575..d48510e1 100644 --- a/src/apps/cli/commands/runCommand.unit.spec.ts +++ b/src/apps/cli/commands/runCommand.unit.spec.ts @@ -708,6 +708,18 @@ describe("runCommand abnormal cases", () => { describe("mark-resolved and unlock-remote commands", () => { it("mark-resolved without args runs on active database", async () => { const core = createCoreMock(); + const remoteDatabase = { + close: vi.fn(async () => undefined), + get: vi.fn(async () => ({ + locked: false, + accepted_nodes: ["test-node-id"], + })), + }; + core.services.replicator.getActiveReplicator.mockReturnValueOnce({ + nodeid: "test-node-id", + initializeDatabaseForReplication: vi.fn(async () => undefined), + connectRemoteCouchDBWithSetting: vi.fn(async () => ({ db: remoteDatabase })), + }); const result = await runCommand(makeOptions("mark-resolved", []), { ...context, core, @@ -715,6 +727,7 @@ describe("runCommand abnormal cases", () => { expect(result).toBe(true); expect(core.services.replication.markResolved).toHaveBeenCalledTimes(1); expect(core.services.control.applySettings).not.toHaveBeenCalled(); + expect(remoteDatabase.close).toHaveBeenCalledOnce(); }); it("mark-resolved with remote-id temporarily activates it and runs markResolved", async () => { diff --git a/src/features/LocalDatabaseMainte/CmdLocalDatabaseMainte.ts b/src/features/LocalDatabaseMainte/CmdLocalDatabaseMainte.ts index 4c25400d..932436c2 100644 --- a/src/features/LocalDatabaseMainte/CmdLocalDatabaseMainte.ts +++ b/src/features/LocalDatabaseMainte/CmdLocalDatabaseMainte.ts @@ -747,29 +747,33 @@ Success: ${successCount}, Errored: ${errored}`; this._notice(`Failed to connect to remote for compaction. ${remote}`, "gc-compact"); return; } - const compactResult = await remote.db.compact({ - interval: 1000, - }); - // Probably no need to wait, but just in case. - let timeout = 2 * 60 * 1000; // 2 minutes - for (;;) { - const status = await remote.db.info(); - if ("compact_running" in status && status?.compact_running) { - this._notice("Compaction in progress on remote database...", "gc-compact"); - await delay(2000); - timeout -= 2000; - if (timeout <= 0) { - this._notice("Compaction on remote database timed out.", "gc-compact"); - return; + try { + const compactResult = await remote.db.compact({ + interval: 1000, + }); + // Probably no need to wait, but just in case. + let timeout = 2 * 60 * 1000; // 2 minutes + for (;;) { + const status = await remote.db.info(); + if ("compact_running" in status && status?.compact_running) { + this._notice("Compaction in progress on remote database...", "gc-compact"); + await delay(2000); + timeout -= 2000; + if (timeout <= 0) { + this._notice("Compaction on remote database timed out.", "gc-compact"); + return; + } + } else { + break; } - } else { - break; } - } - if (compactResult && "ok" in compactResult) { - this._notice("Compaction on remote database completed successfully.", "gc-compact"); - } else { - this._notice("Compaction on remote database failed.", "gc-compact"); + if (compactResult && "ok" in compactResult) { + this._notice("Compaction on remote database completed successfully.", "gc-compact"); + } else { + this._notice("Compaction on remote database failed.", "gc-compact"); + } + } finally { + await remote.db.close(); } } diff --git a/src/features/LocalDatabaseMainte/CmdLocalDatabaseMainte.unit.spec.ts b/src/features/LocalDatabaseMainte/CmdLocalDatabaseMainte.unit.spec.ts index 3ecc4180..6e64bb02 100644 --- a/src/features/LocalDatabaseMainte/CmdLocalDatabaseMainte.unit.spec.ts +++ b/src/features/LocalDatabaseMainte/CmdLocalDatabaseMainte.unit.spec.ts @@ -222,6 +222,7 @@ describe("LocalDatabaseMaintenance Garbage Collection V3", () => { const remoteDatabase = { compact: vi.fn(async () => ({ ok: true })), info: vi.fn(async () => ({ compact_running: true })), + close: vi.fn(async () => undefined), }; Object.assign(maintenance, { core: { @@ -243,6 +244,7 @@ describe("LocalDatabaseMaintenance Garbage Collection V3", () => { "Compaction on remote database completed successfully.", "gc-compact" ); + expect(remoteDatabase.close).toHaveBeenCalledOnce(); }); it.each([ diff --git a/src/modules/core/ModuleReplicator.ts b/src/modules/core/ModuleReplicator.ts index 7479df4c..f7aed4cc 100644 --- a/src/modules/core/ModuleReplicator.ts +++ b/src/modules/core/ModuleReplicator.ts @@ -187,27 +187,31 @@ Even if you choose to clean up, you will see this option again if you exit Obsid return false; } - await purgeUnreferencedChunks(this.localDatabase.localDatabase, false); - this.localDatabase.clearCaches(); - // Perform the synchronisation once. - const replicated = await this.services.replicator.runFiniteReplicationActivity( - () => this.core.replicator.openReplication(this.settings, false, showMessage, true), - { label: "replication" } - ); - if (replicated) { - await balanceChunkPurgedDBs(this.localDatabase.localDatabase, remoteDB.db); + try { await purgeUnreferencedChunks(this.localDatabase.localDatabase, false); this.localDatabase.clearCaches(); - await this.services.replicator.getActiveReplicator()?.markRemoteResolved(this.settings); - Logger( - "The local database has been cleaned up.", - showMessage ? LOG_LEVEL_NOTICE : LOG_LEVEL_INFO - ); - } else { - Logger( - "Replication has been cancelled. Please try it again.", - showMessage ? LOG_LEVEL_NOTICE : LOG_LEVEL_INFO + // Perform the synchronisation once. + const replicated = await this.services.replicator.runFiniteReplicationActivity( + () => this.core.replicator.openReplication(this.settings, false, showMessage, true), + { label: "replication" } ); + if (replicated) { + await balanceChunkPurgedDBs(this.localDatabase.localDatabase, remoteDB.db); + await purgeUnreferencedChunks(this.localDatabase.localDatabase, false); + this.localDatabase.clearCaches(); + await this.services.replicator.getActiveReplicator()?.markRemoteResolved(this.settings); + Logger( + "The local database has been cleaned up.", + showMessage ? LOG_LEVEL_NOTICE : LOG_LEVEL_INFO + ); + } else { + Logger( + "Replication has been cancelled. Please try it again.", + showMessage ? LOG_LEVEL_NOTICE : LOG_LEVEL_INFO + ); + } + } finally { + await remoteDB.db.close(); } }, { label: "database-cleanup" } diff --git a/src/modules/core/ModuleReplicator.unit.spec.ts b/src/modules/core/ModuleReplicator.unit.spec.ts index d98e1fc5..668c94bc 100644 --- a/src/modules/core/ModuleReplicator.unit.spec.ts +++ b/src/modules/core/ModuleReplicator.unit.spec.ts @@ -128,8 +128,11 @@ describe("compatibility: cleaned-remote reconciliation for IndexedDB clients", ( }); const runFiniteReplicationActivity = vi.fn(async (task: () => unknown) => await task()); const openReplication = vi.fn(async () => true); + const remoteDatabase = { + close: vi.fn(async () => undefined), + }; const activeReplicator = Object.assign(new LiveSyncCouchDBReplicator({} as any), { - connectRemoteCouchDBWithSetting: vi.fn(async () => ({ db: {} })), + connectRemoteCouchDBWithSetting: vi.fn(async () => ({ db: remoteDatabase })), markRemoteResolved: vi.fn(async () => undefined), }); const services = { @@ -177,5 +180,9 @@ describe("compatibility: cleaned-remote reconciliation for IndexedDB clients", ( expect(openReplication).toHaveBeenCalledOnce(); expect(openReplication.mock.invocationCallOrder[0]).toBeLessThan(activityFinished.mock.invocationCallOrder[0]); expect(chunkMocks.balanceChunkPurgedDBs).toHaveBeenCalledOnce(); + expect(remoteDatabase.close).toHaveBeenCalledOnce(); + expect(remoteDatabase.close.mock.invocationCallOrder[0]).toBeLessThan( + activityFinished.mock.invocationCallOrder[0] + ); }); }); diff --git a/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.ts b/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.ts index 8e6e04c0..dbb7b239 100644 --- a/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.ts +++ b/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.ts @@ -547,7 +547,8 @@ export class ObsidianLiveSyncSettingTab extends PluginSettingTab { if (typeof db === "string") { Logger($msg("obsidianLiveSyncSettingTab.logCheckPassphraseFailed", { db }), LOG_LEVEL_NOTICE); return false; - } else { + } + try { if (await checkSyncInfo(db.db)) { // Logger($msg("obsidianLiveSyncSettingTab.logDatabaseConnected"), LOG_LEVEL_NOTICE); return true; @@ -555,6 +556,8 @@ export class ObsidianLiveSyncSettingTab extends PluginSettingTab { Logger($msg("obsidianLiveSyncSettingTab.logPassphraseNotCompatible"), LOG_LEVEL_NOTICE); return false; } + } finally { + await db.db.close(); } }; isPassphraseValid = async () => { diff --git a/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.unit.spec.ts b/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.unit.spec.ts new file mode 100644 index 00000000..9b79a865 --- /dev/null +++ b/src/modules/features/SettingDialogue/ObsidianLiveSyncSettingTab.unit.spec.ts @@ -0,0 +1,75 @@ +import { describe, expect, it, vi } from "vitest"; +import { DEFAULT_SETTINGS, REMOTE_COUCHDB } from "@vrtmrz/livesync-commonlib/compat/common/types"; + +const negotiationMocks = vi.hoisted(() => ({ + checkSyncInfo: vi.fn(async () => true), +})); + +vi.mock("@/deps.ts", () => ({ + App: class {}, + Component: class {}, + PluginSettingTab: class {}, +})); +vi.mock("@/main.ts", () => ({ default: class {} })); +vi.mock("@/common/events.ts", () => ({ + EVENT_REQUEST_RELOAD_SETTING_TAB: "request-reload-setting-tab", + eventHub: { onEvent: vi.fn() }, +})); +vi.mock("@vrtmrz/livesync-commonlib/compat/pouchdb/negotiation", () => negotiationMocks); +vi.mock("@vrtmrz/livesync-commonlib/compat/replication/couchdb/LiveSyncReplicator", () => ({ + LiveSyncCouchDBReplicator: class {}, +})); +vi.mock("./LiveSyncSetting.ts", () => ({ LiveSyncSetting: class {} })); +vi.mock("./SettingPane.ts", () => ({ + enableOnly: vi.fn(() => vi.fn()), + setLevelClass: vi.fn(), + setStyle: vi.fn(), + visibleOnly: vi.fn(() => vi.fn()), +})); +vi.mock("./PaneChangeLog.ts", () => ({ paneChangeLog: vi.fn() })); +vi.mock("./PaneSetup.ts", () => ({ paneSetup: vi.fn() })); +vi.mock("./PaneGeneral.ts", () => ({ paneGeneral: vi.fn() })); +vi.mock("./PaneRemoteConfig.ts", () => ({ paneRemoteConfig: vi.fn() })); +vi.mock("./PaneSelector.ts", () => ({ paneSelector: vi.fn() })); +vi.mock("./PaneSyncSettings.ts", () => ({ paneSyncSettings: vi.fn() })); +vi.mock("./PaneCustomisationSync.ts", () => ({ paneCustomisationSync: vi.fn() })); +vi.mock("./PaneHatch.ts", () => ({ paneHatch: vi.fn() })); +vi.mock("./PaneAdvanced.ts", () => ({ paneAdvanced: vi.fn() })); +vi.mock("./PanePowerUsers.ts", () => ({ panePowerUsers: vi.fn() })); +vi.mock("./PanePatches.ts", () => ({ panePatches: vi.fn() })); +vi.mock("./PaneMaintenance.ts", () => ({ paneMaintenance: vi.fn() })); + +import { LiveSyncCouchDBReplicator } from "@vrtmrz/livesync-commonlib/compat/replication/couchdb/LiveSyncReplicator"; +import { ObsidianLiveSyncSettingTab } from "./ObsidianLiveSyncSettingTab"; + +describe("ObsidianLiveSyncSettingTab passphrase verification", () => { + it("closes the finite remote connection after checking synchronisation information", async () => { + const remoteDatabase = { + close: vi.fn(async () => undefined), + }; + const replicator = Object.assign(new LiveSyncCouchDBReplicator({} as never), { + connectRemoteCouchDBWithSetting: vi.fn(async () => ({ db: remoteDatabase })), + }); + const plugin = { + app: {}, + core: { + services: { + API: { isMobile: vi.fn(() => false) }, + replicator: { getNewReplicator: vi.fn(() => replicator) }, + }, + }, + }; + const tab = new ObsidianLiveSyncSettingTab({} as never, plugin as never); + Object.assign(tab, { + _editingSettings: { + ...DEFAULT_SETTINGS, + remoteType: REMOTE_COUCHDB, + }, + }); + + await expect(tab.checkWorkingPassphrase()).resolves.toBe(true); + + expect(negotiationMocks.checkSyncInfo).toHaveBeenCalledWith(remoteDatabase); + expect(remoteDatabase.close).toHaveBeenCalledOnce(); + }); +}); diff --git a/src/modules/features/SetupWizard/dialogs/couchDBConnectionProbe.ts b/src/modules/features/SetupWizard/dialogs/couchDBConnectionProbe.ts index 50b0dd64..dcd3d90b 100644 --- a/src/modules/features/SetupWizard/dialogs/couchDBConnectionProbe.ts +++ b/src/modules/features/SetupWizard/dialogs/couchDBConnectionProbe.ts @@ -8,7 +8,7 @@ export type CouchDBConnectionProbeResult = { ok: true } | { ok: false; reason: s type CouchDBConnectionResult = | string | { - db: unknown; + db: { close(): Promise }; info: unknown; }; @@ -50,7 +50,11 @@ export async function probeCouchDBConnection( if (typeof result === "string") { return { ok: false, reason: result }; } - return { ok: true }; + try { + return { ok: true }; + } finally { + await result.db.close(); + } } export function isValidCouchDBServerURL(value: string): boolean { diff --git a/src/modules/features/SetupWizard/dialogs/couchDBConnectionProbe.unit.spec.ts b/src/modules/features/SetupWizard/dialogs/couchDBConnectionProbe.unit.spec.ts index e54659e8..ef19ecd0 100644 --- a/src/modules/features/SetupWizard/dialogs/couchDBConnectionProbe.unit.spec.ts +++ b/src/modules/features/SetupWizard/dialogs/couchDBConnectionProbe.unit.spec.ts @@ -14,8 +14,9 @@ describe("CouchDB setup connection policy", () => { ] as const)( "%s can %s without changing the Commonlib connection contract", async (createIfMissing, _description) => { + const close = vi.fn(async () => undefined); const connectRemoteCouchDBWithSetting = vi.fn(async () => ({ - db: {}, + db: { close }, info: { db_name: "notes" }, })); const replicator = { @@ -27,6 +28,7 @@ describe("CouchDB setup connection policy", () => { await expect(probeCouchDBConnection(replicator, settings, createIfMissing)).resolves.toEqual({ ok: true }); expect(connectRemoteCouchDBWithSetting).toHaveBeenCalledWith(settings, false, createIfMissing, false); expect(replicator.tryConnectRemote).not.toHaveBeenCalled(); + expect(close).toHaveBeenCalledOnce(); } );