From 0d71b28cc1d8d8de1938793ca90601c576da0b87 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 13 Nov 2024 07:47:18 -0600 Subject: [PATCH 01/13] Toast on connection error (152) --- src/controllers/ConnectionController.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/controllers/ConnectionController.ts b/src/controllers/ConnectionController.ts index cc78ce05..ff806495 100644 --- a/src/controllers/ConnectionController.ts +++ b/src/controllers/ConnectionController.ts @@ -153,6 +153,7 @@ export class ConnectionController implements Disposable { // disconnect from it. if (err instanceof UnsupportedConsoleTypeError && newConnectionUrl) { this._serverManager.disconnectFromServer(newConnectionUrl); + this._toaster.error(err.message); } throw err; From 4a379932f68a6bdf967467ae2289c3b5f9fe88f4 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 13 Nov 2024 09:38:55 -0600 Subject: [PATCH 02/13] Subscription cleanup (152) --- src/services/DhcService.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/services/DhcService.ts b/src/services/DhcService.ts index cab2920e..572fede8 100644 --- a/src/services/DhcService.ts +++ b/src/services/DhcService.ts @@ -178,7 +178,7 @@ export class DhcService implements IDhcService { try { const { cn, session } = await this.initSessionPromise; - cn.subscribeToFieldUpdates(changes => { + const fieldUpdateSubscription = cn.subscribeToFieldUpdates(changes => { this.panelService.updateVariables( this.serverUrl, changes as VariableChanges @@ -198,15 +198,15 @@ export class DhcService implements IDhcService { panelVariablesToUpdate ); }); + this.subscriptions.push(fieldUpdateSubscription); // TODO: Use constant 'disconnect' event name - this.subscriptions.push( - cn.addEventListener('disconnect', () => { - this.clearCaches(); - }) - ); + const disconnectSubscription = cn.addEventListener('disconnect', () => { + this.clearCaches(); + }); + this.subscriptions.push(disconnectSubscription); - session.onLogMessage(logItem => { + const logMessageSubscription = session.onLogMessage(logItem => { // TODO: Should this pull log level from config somewhere? if (logItem.logLevel !== 'INFO') { const date = new Date(logItem.micros / 1000); @@ -217,6 +217,7 @@ export class DhcService implements IDhcService { ); } }); + this.subscriptions.push(logMessageSubscription); } catch (err) {} } From ce59e524fb351ee68c6c5b04a5f4135d0c82582d Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 13 Nov 2024 09:41:16 -0600 Subject: [PATCH 03/13] Properly dispose panel references (152) --- src/controllers/ExtensionController.ts | 1 + src/services/PanelService.ts | 6 ++++++ src/types/serviceTypes.d.ts | 1 + 3 files changed, 8 insertions(+) diff --git a/src/controllers/ExtensionController.ts b/src/controllers/ExtensionController.ts index 9bc1e5d8..0c55dea4 100644 --- a/src/controllers/ExtensionController.ts +++ b/src/controllers/ExtensionController.ts @@ -382,6 +382,7 @@ export class ExtensionController implements Disposable { this._serverManager.onDidDisconnect( serverUrl => { + this._panelService?.clearServerData(serverUrl); this._outputChannel?.appendLine( `Disconnected from server: '${serverUrl}'.` ); diff --git a/src/services/PanelService.ts b/src/services/PanelService.ts index 097de023..5414a55c 100644 --- a/src/services/PanelService.ts +++ b/src/services/PanelService.ts @@ -22,8 +22,14 @@ export class PanelService implements IPanelService, Disposable { private readonly _cnPanelMap: URLMap; private readonly _cnVariableMap: URLMap; + clearServerData = (url: URL): void => { + this._cnPanelMap.delete(url); + this._cnVariableMap.delete(url); + }; + dispose = async (): Promise => { this._cnPanelMap.clear(); + this._cnVariableMap.clear(); }; /** diff --git a/src/types/serviceTypes.d.ts b/src/types/serviceTypes.d.ts index a0c4263a..368eb23e 100644 --- a/src/types/serviceTypes.d.ts +++ b/src/types/serviceTypes.d.ts @@ -110,6 +110,7 @@ export type IDheServiceFactory = IFactory; export interface IPanelService extends Disposable { readonly onDidUpdate: vscode.Event; + clearServerData: (url: URL) => void; getPanelUrls: () => URL[]; getPanelVariables: (url: URL) => VariableDefintion[]; getPanelOrThrow: (url: URL, variableId: VariableID) => vscode.WebviewPanel; From cdc940765a01ec25a5d757a53c28c3db39b94bcc Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 13 Nov 2024 09:53:45 -0600 Subject: [PATCH 04/13] Comment (152) --- src/services/PanelService.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/services/PanelService.ts b/src/services/PanelService.ts index 5414a55c..f185897b 100644 --- a/src/services/PanelService.ts +++ b/src/services/PanelService.ts @@ -22,6 +22,10 @@ export class PanelService implements IPanelService, Disposable { private readonly _cnPanelMap: URLMap; private readonly _cnVariableMap: URLMap; + /** + * Clear panel data for the given connection url. + * @param url The connection url. + */ clearServerData = (url: URL): void => { this._cnPanelMap.delete(url); this._cnVariableMap.delete(url); From 9cfb5b73fc5a77cd102b68ff990d3c692029a889 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 13 Nov 2024 09:54:59 -0600 Subject: [PATCH 05/13] Comment (152) --- src/services/PanelService.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/services/PanelService.ts b/src/services/PanelService.ts index f185897b..d654d357 100644 --- a/src/services/PanelService.ts +++ b/src/services/PanelService.ts @@ -31,6 +31,9 @@ export class PanelService implements IPanelService, Disposable { this._cnVariableMap.delete(url); }; + /** + * Cleanup resources. + */ dispose = async (): Promise => { this._cnPanelMap.clear(); this._cnVariableMap.clear(); From e46760ebc6726860dffbc2c513dcfc789214d4f5 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 13 Nov 2024 10:26:38 -0600 Subject: [PATCH 06/13] Deleted unused EventDispatcher (152) --- src/services/EventDispatcher.ts | 47 --------------------------------- src/types/serviceTypes.d.ts | 12 --------- 2 files changed, 59 deletions(-) delete mode 100644 src/services/EventDispatcher.ts diff --git a/src/services/EventDispatcher.ts b/src/services/EventDispatcher.ts deleted file mode 100644 index 54f02f60..00000000 --- a/src/services/EventDispatcher.ts +++ /dev/null @@ -1,47 +0,0 @@ -import type { - EventListenerT, - IEventDispatcher, - UnsubscribeEventListener, -} from '../types'; - -/** - * General purpose event dispatcher for events that can be subscribed to. - * @deprecated Use `vscode.EventEmitter` instead. - */ -export class EventDispatcher - implements IEventDispatcher -{ - private listeners: Map> = new Map(); - - /** - * Register an event listener for a given event name. - * @param eventName The name of the event to listen for. - * @param listener The event listener to register. - * @returns A function that can be called to unsubscribe the event listener. - */ - addEventListener = ( - eventName: TEventName, - listener: EventListenerT - ): UnsubscribeEventListener => { - if (!this.listeners.has(eventName)) { - this.listeners.set(eventName, new Set()); - } - - this.listeners.get(eventName)?.add(listener); - - return () => { - this.listeners.get(eventName)?.delete(listener); - }; - }; - - /** - * Dispatch an event to all registered listeners. - * @param eventName The name of the event to dispatch. - * @param event The event to dispatch to all listeners - */ - dispatchEvent = (eventName: TEventName, event?: TEvent): void => { - this.listeners.get(eventName)?.forEach(listener => { - listener(event); - }); - }; -} diff --git a/src/types/serviceTypes.d.ts b/src/types/serviceTypes.d.ts index 368eb23e..d9bd6f57 100644 --- a/src/types/serviceTypes.d.ts +++ b/src/types/serviceTypes.d.ts @@ -75,18 +75,6 @@ export interface IDheService extends ConnectionState, Disposable { deleteWorker: (workerUrl: WorkerURL) => Promise; } -/** - * @deprecated Use `vscode.EventEmitter` instead. - */ -export interface IEventDispatcher { - addEventListener: ( - eventName: TEventName, - listener: EventListenerT - ) => UnsubscribeEventListener; - - dispatchEvent: (eventName: TEventName, event?: TEvent) => void; -} - export interface IFactory { create: (...args: TArgs) => T; } From 34a6ea49fe6b6fa07e46c9c9ab768036b92c6dbc Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 13 Nov 2024 10:26:52 -0600 Subject: [PATCH 07/13] Dispose eventemitters (152) --- src/controllers/ExtensionController.ts | 6 +++++- src/providers/RunCommandCodeLensProvider.ts | 7 ++++++- src/providers/TreeDataProviderBase.ts | 8 ++++++-- src/services/PanelService.ts | 1 + src/services/SerializedKeyMap.ts | 8 +++++++- src/services/ServerManager.ts | 11 +++++++++-- src/services/cache/ByURLAsyncCache.ts | 2 ++ 7 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/controllers/ExtensionController.ts b/src/controllers/ExtensionController.ts index 0c55dea4..c057b438 100644 --- a/src/controllers/ExtensionController.ts +++ b/src/controllers/ExtensionController.ts @@ -161,6 +161,7 @@ export class ExtensionController implements Disposable { const codelensProvider = new RunCommandCodeLensProvider(); this._context.subscriptions.push( + codelensProvider, vscode.languages.registerCodeLensProvider('groovy', codelensProvider), vscode.languages.registerCodeLensProvider('python', codelensProvider) ); @@ -530,7 +531,10 @@ export class ExtensionController implements Disposable { this._context.subscriptions.push( this._serverTreeView, this._serverConnectionTreeView, - this._serverConnectionPanelTreeView + this._serverConnectionPanelTreeView, + this._serverTreeProvider, + this._serverConnectionTreeProvider, + this._serverConnectionPanelTreeProvider ); }; diff --git a/src/providers/RunCommandCodeLensProvider.ts b/src/providers/RunCommandCodeLensProvider.ts index b3471908..ae648925 100644 --- a/src/providers/RunCommandCodeLensProvider.ts +++ b/src/providers/RunCommandCodeLensProvider.ts @@ -1,11 +1,12 @@ import * as vscode from 'vscode'; import { ICON_ID } from '../common'; +import type { Disposable } from '../types'; /** * Provides inline editor code lenses for running Deephaven code. */ export class RunCommandCodeLensProvider - implements vscode.CodeLensProvider + implements vscode.CodeLensProvider, Disposable { constructor() { vscode.workspace.onDidChangeConfiguration(() => { @@ -22,6 +23,10 @@ export class RunCommandCodeLensProvider readonly onDidChangeCodeLenses: vscode.Event = this._onDidChangeCodeLenses.event; + dispose = async (): Promise => { + this._onDidChangeCodeLenses.dispose(); + }; + provideCodeLenses( document: vscode.TextDocument, _token: vscode.CancellationToken diff --git a/src/providers/TreeDataProviderBase.ts b/src/providers/TreeDataProviderBase.ts index f790421b..71bbcc9f 100644 --- a/src/providers/TreeDataProviderBase.ts +++ b/src/providers/TreeDataProviderBase.ts @@ -1,11 +1,11 @@ import * as vscode from 'vscode'; -import type { IServerManager } from '../types'; +import type { Disposable, IServerManager } from '../types'; /** * Base class for tree view data providers. */ export abstract class TreeDataProviderBase - implements vscode.TreeDataProvider + implements vscode.TreeDataProvider, Disposable { constructor(serverManager: IServerManager) { this.serverManager = serverManager; @@ -27,6 +27,10 @@ export abstract class TreeDataProviderBase abstract getChildren(element?: T | undefined): vscode.ProviderResult; + dispose = async (): Promise => { + this._onDidChangeTreeData.dispose(); + }; + refresh(): void { this._onDidChangeTreeData.fire(); } diff --git a/src/services/PanelService.ts b/src/services/PanelService.ts index d654d357..30bd67bf 100644 --- a/src/services/PanelService.ts +++ b/src/services/PanelService.ts @@ -37,6 +37,7 @@ export class PanelService implements IPanelService, Disposable { dispose = async (): Promise => { this._cnPanelMap.clear(); this._cnVariableMap.clear(); + this._onDidUpdate.dispose(); }; /** diff --git a/src/services/SerializedKeyMap.ts b/src/services/SerializedKeyMap.ts index 3b592a01..cb291664 100644 --- a/src/services/SerializedKeyMap.ts +++ b/src/services/SerializedKeyMap.ts @@ -1,4 +1,5 @@ import * as vscode from 'vscode'; +import type { Disposable } from '../types'; /** * Base class for Maps that need to store their keys as serialized string values @@ -10,7 +11,7 @@ import * as vscode from 'vscode'; * // New reference on every call * e.g. deserializeKey = (key: string) => new URL(key) */ -export abstract class SerializedKeyMap { +export abstract class SerializedKeyMap implements Disposable { constructor(); constructor(entries: readonly (readonly [TKey, TValue])[] | null); constructor(entries?: readonly (readonly [TKey, TValue])[] | null) { @@ -40,6 +41,11 @@ export abstract class SerializedKeyMap { keys.forEach(key => this._onDidChange.fire(key)); } + dispose = async (): Promise => { + this._map.clear(); + this._onDidChange.dispose(); + }; + get(key: TKey): TValue | undefined { return this._map.get(this.serializeKey(key)); } diff --git a/src/services/ServerManager.ts b/src/services/ServerManager.ts index de13a235..53d4f6dd 100644 --- a/src/services/ServerManager.ts +++ b/src/services/ServerManager.ts @@ -100,6 +100,15 @@ export class ServerManager implements IServerManager { canStartServer: boolean; + async dispose(): Promise { + this._onDidConnect.dispose(); + this._onDidDisconnect.dispose(); + this._onDidLoadConfig.dispose(); + this._onDidServerStatusChange.dispose(); + this._onDidRegisterEditor.dispose(); + this._onDidUpdate.dispose(); + } + loadServerConfig = async (): Promise => { // We want to keep any existing managed servers that aren't overridden by // the latest config so we don't lose the PSKs that were generated when @@ -625,6 +634,4 @@ export class ServerManager implements IServerManager { this._hasEverUpdatedStatus = true; }; - - async dispose(): Promise {} } diff --git a/src/services/cache/ByURLAsyncCache.ts b/src/services/cache/ByURLAsyncCache.ts index d2e1f797..64f10c92 100644 --- a/src/services/cache/ByURLAsyncCache.ts +++ b/src/services/cache/ByURLAsyncCache.ts @@ -36,6 +36,8 @@ export class ByURLAsyncCache }; dispose = async (): Promise => { + this._onDidInvalidate.dispose(); + const promises = [...this._promiseMap.values()]; this._promiseMap.clear(); From 9603cf9c8188abfe8f4996760e1e6e88a0e09113 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 13 Nov 2024 10:51:00 -0600 Subject: [PATCH 08/13] Resource disposal (152) --- src/controllers/ExtensionController.ts | 3 +++ src/services/DheService.ts | 6 ++++-- src/services/PanelService.ts | 7 +++++-- src/services/SerializedKeyMap.ts | 15 ++++++++++++++- src/services/ServerManager.ts | 12 ++++++++++-- src/services/cache/ByURLAsyncCache.ts | 15 +-------------- 6 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/controllers/ExtensionController.ts b/src/controllers/ExtensionController.ts index c057b438..55f28d52 100644 --- a/src/controllers/ExtensionController.ts +++ b/src/controllers/ExtensionController.ts @@ -345,7 +345,10 @@ export class ExtensionController implements Disposable { }; this._coreClientCache = new URLMap(); + this._context.subscriptions.push(this._coreClientCache); + this._dheClientCache = new URLMap(); + this._context.subscriptions.push(this._dheClientCache); this._panelService = new PanelService(); this._context.subscriptions.push(this._panelService); diff --git a/src/services/DheService.ts b/src/services/DheService.ts index 50dafde2..dc5e805a 100644 --- a/src/services/DheService.ts +++ b/src/services/DheService.ts @@ -256,8 +256,10 @@ export class DheService implements IDheService { const querySerials = [...this._querySerialSet]; this._querySerialSet.clear(); - this._workerInfoMap.clear(); - await this._disposeQueries(querySerials); + await Promise.all([ + this._workerInfoMap.dispose(), + this._disposeQueries(querySerials), + ]); }; } diff --git a/src/services/PanelService.ts b/src/services/PanelService.ts index 30bd67bf..42b6036b 100644 --- a/src/services/PanelService.ts +++ b/src/services/PanelService.ts @@ -35,9 +35,12 @@ export class PanelService implements IPanelService, Disposable { * Cleanup resources. */ dispose = async (): Promise => { - this._cnPanelMap.clear(); - this._cnVariableMap.clear(); this._onDidUpdate.dispose(); + + await Promise.all([ + this._cnPanelMap.dispose(), + this._cnVariableMap.dispose(), + ]); }; /** diff --git a/src/services/SerializedKeyMap.ts b/src/services/SerializedKeyMap.ts index cb291664..e097583a 100644 --- a/src/services/SerializedKeyMap.ts +++ b/src/services/SerializedKeyMap.ts @@ -1,5 +1,6 @@ import * as vscode from 'vscode'; import type { Disposable } from '../types'; +import { isDisposable } from '../util'; /** * Base class for Maps that need to store their keys as serialized string values @@ -42,8 +43,20 @@ export abstract class SerializedKeyMap implements Disposable { } dispose = async (): Promise => { - this._map.clear(); this._onDidChange.dispose(); + + const promises = [...this._map.values()]; + this._map.clear(); + + const disposing = promises.map(async maybePromise => { + // If value is a Promise, it has to be resolved before it can be disposed. + const resolved = await maybePromise; + if (isDisposable(resolved)) { + await resolved.dispose(); + } + }); + + await Promise.all(disposing); }; get(key: TKey): TValue | undefined { diff --git a/src/services/ServerManager.ts b/src/services/ServerManager.ts index 53d4f6dd..a87ebc9b 100644 --- a/src/services/ServerManager.ts +++ b/src/services/ServerManager.ts @@ -100,14 +100,21 @@ export class ServerManager implements IServerManager { canStartServer: boolean; - async dispose(): Promise { + dispose = async (): Promise => { this._onDidConnect.dispose(); this._onDidDisconnect.dispose(); this._onDidLoadConfig.dispose(); this._onDidServerStatusChange.dispose(); this._onDidRegisterEditor.dispose(); this._onDidUpdate.dispose(); - } + + await Promise.all([ + this._connectionMap.dispose(), + this._serverMap.dispose(), + this._uriConnectionsMap.dispose(), + this._workerURLToServerURLMap.dispose(), + ]); + }; loadServerConfig = async (): Promise => { // We want to keep any existing managed servers that aren't overridden by @@ -243,6 +250,7 @@ export class ServerManager implements IServerManager { this._onDidUpdate.fire(); if (!(await connection.initSession())) { + connection.dispose(); this._connectionMap.delete(serverUrl); return null; } diff --git a/src/services/cache/ByURLAsyncCache.ts b/src/services/cache/ByURLAsyncCache.ts index 64f10c92..8f230a54 100644 --- a/src/services/cache/ByURLAsyncCache.ts +++ b/src/services/cache/ByURLAsyncCache.ts @@ -1,6 +1,5 @@ import * as vscode from 'vscode'; import type { IAsyncCacheService } from '../../types'; -import { isDisposable } from '../../util'; import { URLMap } from '../URLMap'; /** @@ -37,18 +36,6 @@ export class ByURLAsyncCache dispose = async (): Promise => { this._onDidInvalidate.dispose(); - - const promises = [...this._promiseMap.values()]; - this._promiseMap.clear(); - - // Values have to be resolved before they can be disposed. - const disposing = promises.map(async promise => { - const resolved = await promise; - if (isDisposable(resolved)) { - await resolved.dispose(); - } - }); - - await Promise.all(disposing); + await this._promiseMap.dispose(); }; } From fb3ce3f21797588f44da67d0b9e931e935021b3d Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 13 Nov 2024 11:16:53 -0600 Subject: [PATCH 09/13] client disposal (152) --- src/controllers/ExtensionController.ts | 30 ++++++++++++++++++++------ src/types/serviceTypes.d.ts | 4 ++-- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/controllers/ExtensionController.ts b/src/controllers/ExtensionController.ts index 55f28d52..0955ff35 100644 --- a/src/controllers/ExtensionController.ts +++ b/src/controllers/ExtensionController.ts @@ -330,24 +330,42 @@ export class ExtensionController implements Disposable { this._coreClientFactory = async ( url: URL - ): Promise => { + ): Promise => { assertDefined(this._coreJsApiCache, 'coreJsApiCache'); const dhc = await this._coreJsApiCache.get(url); - return new dhc.CoreClient(url.toString()) as CoreUnauthenticatedClient; + + const client = new dhc.CoreClient( + url.toString() + ) as CoreUnauthenticatedClient; + + // Attach a dispose method so that client caches can dispose of the client + return Object.assign(client, { + dispose: async () => { + client.disconnect(); + }, + }); }; this._dheClientFactory = async ( url: URL - ): Promise => { + ): Promise => { assertDefined(this._dheJsApiCache, 'dheJsApiCache'); const dhe = await this._dheJsApiCache.get(url); - return createDheClient(dhe, getWsUrl(url)); + + const client = await createDheClient(dhe, getWsUrl(url)); + + // Attach a dispose method so that client caches can dispose of the client + return Object.assign(client, { + dispose: async () => { + client.disconnect(); + }, + }); }; - this._coreClientCache = new URLMap(); + this._coreClientCache = new URLMap(); this._context.subscriptions.push(this._coreClientCache); - this._dheClientCache = new URLMap(); + this._dheClientCache = new URLMap(); this._context.subscriptions.push(this._dheClientCache); this._panelService = new PanelService(); diff --git a/src/types/serviceTypes.d.ts b/src/types/serviceTypes.d.ts index d9bd6f57..e45b3838 100644 --- a/src/types/serviceTypes.d.ts +++ b/src/types/serviceTypes.d.ts @@ -81,7 +81,7 @@ export interface IFactory { export type ICoreClientFactory = ( serverUrl: URL -) => Promise; +) => Promise; /** * Factory for creating IDhService instances. @@ -92,7 +92,7 @@ export type IDhcServiceFactory = IFactory< >; export type IDheClientFactory = ( serverUrl: URL -) => Promise; +) => Promise; export type IDheServiceFactory = IFactory; export interface IPanelService extends Disposable { From a6078e0145c419f9b5fc515fe3004193dbf982af Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 13 Nov 2024 11:26:15 -0600 Subject: [PATCH 10/13] Types (152) --- src/controllers/ExtensionController.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/controllers/ExtensionController.ts b/src/controllers/ExtensionController.ts index 0955ff35..fc6cbd96 100644 --- a/src/controllers/ExtensionController.ts +++ b/src/controllers/ExtensionController.ts @@ -117,11 +117,14 @@ export class ExtensionController implements Disposable { readonly _config: IConfigService; private _connectionController: ConnectionController | null = null; - private _coreClientCache: URLMap | null = null; + private _coreClientCache: URLMap< + CoreAuthenticatedClient & Disposable + > | null = null; private _coreClientFactory: ICoreClientFactory | null = null; private _coreJsApiCache: IAsyncCacheService | null = null; - private _dheClientCache: URLMap | null = null; + private _dheClientCache: URLMap | null = + null; private _dheClientFactory: IDheClientFactory | null = null; private _dheServiceCache: IAsyncCacheService | null = null; private _panelController: PanelController | null = null; @@ -362,10 +365,10 @@ export class ExtensionController implements Disposable { }); }; - this._coreClientCache = new URLMap(); + this._coreClientCache = new URLMap(); this._context.subscriptions.push(this._coreClientCache); - this._dheClientCache = new URLMap(); + this._dheClientCache = new URLMap(); this._context.subscriptions.push(this._dheClientCache); this._panelService = new PanelService(); From 8bedb799779c46c14611b042919df8f49f175720 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 13 Nov 2024 12:19:04 -0600 Subject: [PATCH 11/13] Fixed circular dependency failing test (152) --- .../ServerConnectionPanelTreeProvider.ts | 6 +- src/services/PollingService.spec.ts | 88 +++++++++++++++++++ src/services/PollingService.ts | 61 +++++++++++++ src/services/consoleTypeUtils.ts | 81 +++++++++++++++++ src/services/index.ts | 1 + src/util/index.ts | 1 + src/util/promiseUtils.spec.ts | 73 +-------------- src/util/promiseUtils.ts | 62 ------------- src/util/serverUtils.ts | 65 -------------- src/util/treeViewUtils.spec.ts | 5 +- src/util/treeViewUtils.ts | 16 ++-- 11 files changed, 250 insertions(+), 209 deletions(-) create mode 100644 src/services/PollingService.spec.ts create mode 100644 src/services/consoleTypeUtils.ts diff --git a/src/providers/ServerConnectionPanelTreeProvider.ts b/src/providers/ServerConnectionPanelTreeProvider.ts index b8647adb..8404953a 100644 --- a/src/providers/ServerConnectionPanelTreeProvider.ts +++ b/src/providers/ServerConnectionPanelTreeProvider.ts @@ -11,6 +11,7 @@ import { getPanelVariableTreeItem, sortByStringProp, } from '../util'; +import { getFirstSupportedConsoleType } from '../services'; export class ServerConnectionPanelTreeProvider extends TreeDataProviderBase { constructor(serverManager: IServerManager, panelService: IPanelService) { @@ -31,7 +32,10 @@ export class ServerConnectionPanelTreeProvider extends TreeDataProviderBase { + vi.clearAllMocks(); + vi.useFakeTimers(); +}); + +afterAll(() => { + vi.useRealTimers(); +}); + +const resolved = vi.fn().mockName('resolved'); +const rejected = vi.fn().mockName('rejected'); + +describe('pollUntilTrue', () => { + const poll = vi.fn<() => Promise>().mockName('poll'); + + it('should resolve when poll function returns true', async () => { + const intervalMs = 100; + poll.mockResolvedValue(false); + + const { promise } = pollUntilTrue(poll, intervalMs); + promise.then(resolved); + + // Initial polling call that is scheduled via setTimeout(..., 0) + await vi.advanceTimersToNextTimerAsync(); + expect(poll).toHaveBeenCalledTimes(1); + expect(resolved).not.toHaveBeenCalled(); + + // 2nd poll (after first intervalMs) + await vi.advanceTimersByTimeAsync(intervalMs); + expect(poll).toHaveBeenCalledTimes(2); + expect(resolved).not.toHaveBeenCalled(); + + poll.mockResolvedValue(true); + + // 3rd poll + await vi.advanceTimersByTimeAsync(intervalMs); + expect(poll).toHaveBeenCalledTimes(3); + expect(resolved).toHaveBeenCalledWith(true); + + // Advance intervalMs. No more polling expected since resolved + await vi.advanceTimersByTimeAsync(intervalMs); + expect(poll).toHaveBeenCalledTimes(3); + expect(resolved).toHaveBeenCalledOnce(); + }); + + it('should cancel polling if timeout exceeded', async () => { + const intervalMs = 100; + const timeoutMs = 1000; + + poll.mockResolvedValue(false); + + const { promise } = pollUntilTrue(poll, intervalMs, timeoutMs); + promise.then(resolved).catch(rejected); + + expect(resolved).not.toHaveBeenCalled(); + expect(rejected).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(timeoutMs); + + expect(resolved).not.toHaveBeenCalled(); + expect(rejected).toHaveBeenCalledWith(new Error('Polling cancelled')); + }); + + it('should cancel polling if cancel explicitly called', async () => { + const intervalMs = 100; + + poll.mockResolvedValue(false); + + const { promise, cancel } = pollUntilTrue(poll, intervalMs); + promise.then(resolved).catch(rejected); + + expect(resolved).not.toHaveBeenCalled(); + expect(rejected).not.toHaveBeenCalled(); + + cancel(); + + await vi.advanceTimersToNextTimerAsync(); + + expect(resolved).not.toHaveBeenCalled(); + expect(rejected).toHaveBeenCalledWith(new Error('Polling cancelled')); + }); +}); diff --git a/src/services/PollingService.ts b/src/services/PollingService.ts index 9363dd6f..5bc5c9e7 100644 --- a/src/services/PollingService.ts +++ b/src/services/PollingService.ts @@ -1,4 +1,5 @@ import type { Disposable } from '../types'; +import { withResolvers, type PromiseWithCancel } from '../util'; export type Runner = () => Promise; @@ -56,3 +57,63 @@ export class PollingService implements Disposable { this.stop(); }; } + +/** + * Call the given poll function at an interval. + * - If the poll result resolves to true, stop polling and resolve the promise. + * - If the poll function throws, stop polling and reject the promise. + * @param poll + * @param intervalMs + * @param timeoutMs + * @returns Promise that resolves when the poll function returns true + a `reject` + * function that can be used to cancel the polling. + */ +export function pollUntilTrue( + poll: () => Promise, + intervalMs: number, + timeoutMs?: number +): PromiseWithCancel { + const { promise, resolve, reject } = withResolvers(); + + let timeout: NodeJS.Timeout; + const poller = new PollingService(); + + /** Stop polling and resolve / reject promise */ + function resolveOrReject(trueOrError: true | Error): void { + poller.stop(); + clearTimeout(timeout); + + if (trueOrError === true) { + resolve(trueOrError); + } else { + reject(trueOrError); + } + } + + /** Cancel polling */ + const cancel = (): void => { + resolveOrReject(new Error('Polling cancelled')); + }; + + if (timeoutMs != null) { + timeout = setTimeout(() => { + cancel(); + }, timeoutMs); + } + + poller.start(async () => { + try { + const isTrue = await poll(); + if (isTrue) { + resolveOrReject(true); + } + } catch (err) { + resolveOrReject(err instanceof Error ? err : new Error(String(err))); + } + }, intervalMs); + + return { + promise, + cancel, + }; +} diff --git a/src/services/consoleTypeUtils.ts b/src/services/consoleTypeUtils.ts new file mode 100644 index 00000000..f062ef9d --- /dev/null +++ b/src/services/consoleTypeUtils.ts @@ -0,0 +1,81 @@ +import type { + ConnectionState, + ConsoleType, + ServerConnectionPanelNode, +} from '../types'; +import { isInstanceOf } from '../util'; +import DhcService from './DhcService'; + +/** + * Get connections supporting the given console type. + * @param connections Connections to filter + * @param consoleType Console type to filter by + * @returns Connections supporting the given console type + */ +export async function getConnectionsForConsoleType( + connections: ConnectionState[], + consoleType: ConsoleType +): Promise { + const filteredConnections: ConnectionState[] = []; + + for await (const connection of iterateConnectionsForConsoleType( + connections, + consoleType + )) { + filteredConnections.push(connection); + } + + return filteredConnections; +} + +export async function getFirstSupportedConsoleType( + connectionOrVariable: ServerConnectionPanelNode +): Promise { + const [consoleType] = + isInstanceOf(connectionOrVariable, DhcService) && + connectionOrVariable.isInitialized + ? await connectionOrVariable.getConsoleTypes() + : []; + + return consoleType; +} + +/** + * Get the first connection supporting the given console type. + * @param connections Connections to filter + * @param consoleType Console type to filter by + * @returns First connection supporting the given console type + */ +export async function getFirstConnectionForConsoleType( + connections: ConnectionState[], + consoleType: ConsoleType +): Promise { + const first = await iterateConnectionsForConsoleType( + connections, + consoleType + ).next(); + + return first.value ?? null; +} + +/** + * Lazy async iterator that yields all connections supporting the given console + * type. + * @param connections Connections to iterate + * @param consoleType Console type to filter by + * @returns Async iterator for connections supporting the given console type + */ +export async function* iterateConnectionsForConsoleType( + connections: ConnectionState[], + consoleType: ConsoleType +): AsyncGenerator { + for (const connection of connections) { + const isConsoleTypeSupported = + isInstanceOf(connection, DhcService) && + (await connection.supportsConsoleType(consoleType)); + + if (isConsoleTypeSupported) { + yield connection; + } + } +} diff --git a/src/services/index.ts b/src/services/index.ts index 26053866..3446998e 100644 --- a/src/services/index.ts +++ b/src/services/index.ts @@ -1,5 +1,6 @@ export * from './cache'; export * from './ConfigService'; +export * from './consoleTypeUtils'; export * from './DhcService'; export * from './DheService'; export * from './PanelService'; diff --git a/src/util/index.ts b/src/util/index.ts index e9389a37..7ff2bbf5 100644 --- a/src/util/index.ts +++ b/src/util/index.ts @@ -6,6 +6,7 @@ export * from './isDisposable'; export * from './Logger'; export * from './OutputChannelWithHistory'; export * from './panelUtils'; +export * from './promiseUtils'; export * from './selectionUtils'; export * from './serverUtils'; export * from './testUtils'; diff --git a/src/util/promiseUtils.spec.ts b/src/util/promiseUtils.spec.ts index 6230a852..146d2e87 100644 --- a/src/util/promiseUtils.spec.ts +++ b/src/util/promiseUtils.spec.ts @@ -1,5 +1,5 @@ import { beforeEach, describe, it, expect, vi, afterAll } from 'vitest'; -import { pollUntilTrue, waitFor, withResolvers } from './promiseUtils'; +import { waitFor, withResolvers } from './promiseUtils'; // See __mocks__/vscode.ts for the mock implementation vi.mock('vscode'); @@ -57,74 +57,3 @@ describe('withResolvers', () => { expect(rejected).toHaveBeenCalledWith('Some Error'); }); }); - -describe('pollUntilTrue', () => { - const poll = vi.fn<() => Promise>().mockName('poll'); - - it('should resolve when poll function returns true', async () => { - const intervalMs = 100; - poll.mockResolvedValue(false); - - const { promise } = pollUntilTrue(poll, intervalMs); - promise.then(resolved); - - // Initial polling call that is scheduled via setTimeout(..., 0) - await vi.advanceTimersToNextTimerAsync(); - expect(poll).toHaveBeenCalledTimes(1); - expect(resolved).not.toHaveBeenCalled(); - - // 2nd poll (after first intervalMs) - await vi.advanceTimersByTimeAsync(intervalMs); - expect(poll).toHaveBeenCalledTimes(2); - expect(resolved).not.toHaveBeenCalled(); - - poll.mockResolvedValue(true); - - // 3rd poll - await vi.advanceTimersByTimeAsync(intervalMs); - expect(poll).toHaveBeenCalledTimes(3); - expect(resolved).toHaveBeenCalledWith(true); - - // Advance intervalMs. No more polling expected since resolved - await vi.advanceTimersByTimeAsync(intervalMs); - expect(poll).toHaveBeenCalledTimes(3); - expect(resolved).toHaveBeenCalledOnce(); - }); - - it('should cancel polling if timeout exceeded', async () => { - const intervalMs = 100; - const timeoutMs = 1000; - - poll.mockResolvedValue(false); - - const { promise } = pollUntilTrue(poll, intervalMs, timeoutMs); - promise.then(resolved).catch(rejected); - - expect(resolved).not.toHaveBeenCalled(); - expect(rejected).not.toHaveBeenCalled(); - - await vi.advanceTimersByTimeAsync(timeoutMs); - - expect(resolved).not.toHaveBeenCalled(); - expect(rejected).toHaveBeenCalledWith(new Error('Polling cancelled')); - }); - - it('should cancel polling if cancel explicitly called', async () => { - const intervalMs = 100; - - poll.mockResolvedValue(false); - - const { promise, cancel } = pollUntilTrue(poll, intervalMs); - promise.then(resolved).catch(rejected); - - expect(resolved).not.toHaveBeenCalled(); - expect(rejected).not.toHaveBeenCalled(); - - cancel(); - - await vi.advanceTimersToNextTimerAsync(); - - expect(resolved).not.toHaveBeenCalled(); - expect(rejected).toHaveBeenCalledWith(new Error('Polling cancelled')); - }); -}); diff --git a/src/util/promiseUtils.ts b/src/util/promiseUtils.ts index 9de9d6e2..eac7dd88 100644 --- a/src/util/promiseUtils.ts +++ b/src/util/promiseUtils.ts @@ -1,5 +1,3 @@ -import { PollingService } from '../services'; - export interface PromiseWithResolvers { promise: Promise; resolve: (value: T | PromiseLike) => void; @@ -39,63 +37,3 @@ export function withResolvers(): PromiseWithResolvers { reject, }; } - -/** - * Call the given poll function at an interval. - * - If the poll result resolves to true, stop polling and resolve the promise. - * - If the poll function throws, stop polling and reject the promise. - * @param poll - * @param intervalMs - * @param timeoutMs - * @returns Promise that resolves when the poll function returns true + a `reject` - * function that can be used to cancel the polling. - */ -export function pollUntilTrue( - poll: () => Promise, - intervalMs: number, - timeoutMs?: number -): PromiseWithCancel { - const { promise, resolve, reject } = withResolvers(); - - let timeout: NodeJS.Timeout; - const poller = new PollingService(); - - /** Stop polling and resolve / reject promise */ - function resolveOrReject(trueOrError: true | Error): void { - poller.stop(); - clearTimeout(timeout); - - if (trueOrError === true) { - resolve(trueOrError); - } else { - reject(trueOrError); - } - } - - /** Cancel polling */ - const cancel = (): void => { - resolveOrReject(new Error('Polling cancelled')); - }; - - if (timeoutMs != null) { - timeout = setTimeout(() => { - cancel(); - }, timeoutMs); - } - - poller.start(async () => { - try { - const isTrue = await poll(); - if (isTrue) { - resolveOrReject(true); - } - } catch (err) { - resolveOrReject(err instanceof Error ? err : new Error(String(err))); - } - }, intervalMs); - - return { - promise, - cancel, - }; -} diff --git a/src/util/serverUtils.ts b/src/util/serverUtils.ts index e54dd8d2..cdd6f2b9 100644 --- a/src/util/serverUtils.ts +++ b/src/util/serverUtils.ts @@ -5,12 +5,9 @@ import type { ConsoleType, Port, ServerConnectionConfig, - ConnectionState, } from '../types'; import { PIP_SERVER_STATUS_DIRECTORY, SERVER_LANGUAGE_SET } from '../common'; import { getTempDir } from './tmpUtils'; -import { DhcService } from '../services'; -import { isInstanceOf } from './isInstanceOf'; /** * Get initial server states based on server configs. @@ -32,28 +29,6 @@ export function getInitialServerStates( })); } -/** - * Get connections supporting the given console type. - * @param connections Connections to filter - * @param consoleType Console type to filter by - * @returns Connections supporting the given console type - */ -export async function getConnectionsForConsoleType( - connections: ConnectionState[], - consoleType: ConsoleType -): Promise { - const filteredConnections: ConnectionState[] = []; - - for await (const connection of iterateConnectionsForConsoleType( - connections, - consoleType - )) { - filteredConnections.push(connection); - } - - return filteredConnections; -} - /** * If the given value is a valid console type, return it, otherwise return undefined. * @param maybeConsoleType @@ -68,24 +43,6 @@ export function getConsoleType( : undefined; } -/** - * Get the first connection supporting the given console type. - * @param connections Connections to filter - * @param consoleType Console type to filter by - * @returns First connection supporting the given console type - */ -export async function getFirstConnectionForConsoleType( - connections: ConnectionState[], - consoleType: ConsoleType -): Promise { - const first = await iterateConnectionsForConsoleType( - connections, - consoleType - ).next(); - - return first.value ?? null; -} - /** * Get the pip server URL for the given port. * @param port The port number to create a URL for @@ -118,28 +75,6 @@ export function isSupportedLanguageId( return SERVER_LANGUAGE_SET.has(maybeSupported as ConsoleType); } -/** - * Lazy async iterator that yields all connections supporting the given console - * type. - * @param connections Connections to iterate - * @param consoleType Console type to filter by - * @returns Async iterator for connections supporting the given console type - */ -export async function* iterateConnectionsForConsoleType( - connections: ConnectionState[], - consoleType: ConsoleType -): AsyncGenerator { - for (const connection of connections) { - const isConsoleTypeSupported = - isInstanceOf(connection, DhcService) && - (await connection.supportsConsoleType(consoleType)); - - if (isConsoleTypeSupported) { - yield connection; - } - } -} - /** * Parse a port string into a number. * @param portStr Numeric port string to parse diff --git a/src/util/treeViewUtils.spec.ts b/src/util/treeViewUtils.spec.ts index 36768725..f558670a 100644 --- a/src/util/treeViewUtils.spec.ts +++ b/src/util/treeViewUtils.spec.ts @@ -59,7 +59,10 @@ describe('getPanelConnectionTreeItem', () => { vi.mocked(isInstanceOf).mockReturnValue(true); - const actual = await getPanelConnectionTreeItem(connection); + const actual = await getPanelConnectionTreeItem(connection, async () => { + const [consoleType] = await getConsoleTypes(); + return isInitialized ? consoleType : undefined; + }); expect(actual).toMatchSnapshot(); } ); diff --git a/src/util/treeViewUtils.ts b/src/util/treeViewUtils.ts index a5aa6da3..5223b793 100644 --- a/src/util/treeViewUtils.ts +++ b/src/util/treeViewUtils.ts @@ -1,6 +1,7 @@ import * as vscode from 'vscode'; import type { ConnectionState, + ConsoleType, ServerGroupState, ServerState, VariableDefintion, @@ -12,8 +13,6 @@ import { SERVER_TREE_ITEM_CONTEXT, type ServerTreeItemContextValue, } from '../common'; -import { DhcService } from '../services'; -import { isInstanceOf } from './isInstanceOf'; /** * Get a tree item vscode.ThemeIcon for a variable type. @@ -49,15 +48,16 @@ export function getVariableIconPath( /** * Get `TreeItem` for a panel connection. - * @param connection + * @param connection Connection state + * @param getConsoleType Function to get the console type for the connection. */ export async function getPanelConnectionTreeItem( - connection: ConnectionState + connection: ConnectionState, + getConsoleType: ( + connection: ConnectionState + ) => Promise ): Promise { - const [consoleType] = - isInstanceOf(connection, DhcService) && connection.isInitialized - ? await connection.getConsoleTypes() - : []; + const consoleType = await getConsoleType(connection); return { label: new URL(connection.serverUrl.toString()).host, From b60bf122c7137dccf23b86f0127563565147e394 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 13 Nov 2024 12:24:42 -0600 Subject: [PATCH 12/13] Updated imports (152) --- src/controllers/ConnectionController.ts | 2 +- src/controllers/PipServerController.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/controllers/ConnectionController.ts b/src/controllers/ConnectionController.ts index ff806495..018cbad5 100644 --- a/src/controllers/ConnectionController.ts +++ b/src/controllers/ConnectionController.ts @@ -13,7 +13,6 @@ import { createConnectionQuickPick, createConnectionQuickPickOptions, createConnectStatusBarItem, - getConnectionsForConsoleType, getConsoleType, getEditorForUri, isSupportedLanguageId, @@ -21,6 +20,7 @@ import { updateConnectionStatusBarItem, } from '../util'; import { UnsupportedConsoleTypeError } from '../common'; +import { getConnectionsForConsoleType } from '../services'; const logger = new Logger('ConnectionController'); diff --git a/src/controllers/PipServerController.ts b/src/controllers/PipServerController.ts index 303c281c..5e910c9d 100644 --- a/src/controllers/PipServerController.ts +++ b/src/controllers/PipServerController.ts @@ -13,8 +13,9 @@ import { PIP_SERVER_SUPPORTED_PLATFORMS, PYTHON_ENV_WAIT, } from '../common'; -import { pollUntilTrue, waitFor } from '../util/promiseUtils'; +import { waitFor } from '../util'; import { isDhcServerRunning } from '../dh/dhc'; +import { pollUntilTrue } from '../services'; const logger = new Logger('PipServerController'); From 3c1c5ac2feecce83e50ec54c6fee64dbb7eec965 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 13 Nov 2024 12:31:24 -0600 Subject: [PATCH 13/13] Fixed build issue + broken package json script (152) --- package.json | 2 +- src/dh/dhc.ts | 10 ---------- src/types/global.d.ts | 11 +++++++++++ src/types/index.ts | 1 + 4 files changed, 13 insertions(+), 11 deletions(-) create mode 100644 src/types/global.d.ts diff --git a/package.json b/package.json index f27f8cec..6e85d101 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "test:lint": "eslint . --ext ts", "test:unit": "vitest --reporter=default --reporter=junit --outputFile=./test-reports/vitest.junit.xml", "test": "npm run test:unit", - "vscode:prepublish": "compile:prod", + "vscode:prepublish": "npm run compile:prod", "watch:esbuild": "node scripts/esbuild.js --watch", "watch:tsc": "tsc --build ./tsconfig.json --watch", "watch": "npm-run-all -p watch:*" diff --git a/src/dh/dhc.ts b/src/dh/dhc.ts index 3aba031e..743b362b 100644 --- a/src/dh/dhc.ts +++ b/src/dh/dhc.ts @@ -20,16 +20,6 @@ export type ConnectionAndSession = { session: TSession; }; -// TODO: https://github.com/deephaven/deephaven-core/issues/5911 to address the -// underlying issue of jsapi-types being unaware of `dhinternal`. Once that is -// addressed, this can be removed. -declare global { - // eslint-disable-next-line no-unused-vars - namespace dhinternal.io.deephaven.proto.ticket_pb { - export type TypedTicket = unknown; - } -} - /** * Download the DH Core jsapi from a running server and return the `dh` object. * @param serverUrl URL of the DH Core server to download the api from. diff --git a/src/types/global.d.ts b/src/types/global.d.ts new file mode 100644 index 00000000..e2d68c59 --- /dev/null +++ b/src/types/global.d.ts @@ -0,0 +1,11 @@ +// TODO: https://github.com/deephaven/deephaven-core/issues/5911 to address the +// underlying issue of jsapi-types being unaware of `dhinternal`. Once that is +// addressed, this can be removed. +declare global { + // eslint-disable-next-line no-unused-vars + namespace dhinternal.io.deephaven.proto.ticket_pb { + export type TypedTicket = unknown; + } +} + +export {}; diff --git a/src/types/index.ts b/src/types/index.ts index c741bafc..1f25af54 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -1,4 +1,5 @@ export type * from './commonTypes'; +export type * from './global'; export type * from './serviceTypes'; export type * from './treeViewTypes'; export type * from './uiTypes';