From f94795de22c8f0bb6ba3569a7d152945f2033736 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 16 Jun 2023 17:18:06 +1000 Subject: [PATCH 1/3] Notify providers when deleting Server Uri from MRU --- .../jupyterUriProviderRegistration.ts | 22 +++++++++++++++++-- ...upyterUriProviderRegistration.unit.test.ts | 13 +++++++++-- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/kernels/jupyter/connection/jupyterUriProviderRegistration.ts b/src/kernels/jupyter/connection/jupyterUriProviderRegistration.ts index e0dc50b01fb..6efb5dbb9ad 100644 --- a/src/kernels/jupyter/connection/jupyterUriProviderRegistration.ts +++ b/src/kernels/jupyter/connection/jupyterUriProviderRegistration.ts @@ -10,7 +10,12 @@ import * as localize from '../../../platform/common/utils/localize'; import { noop } from '../../../platform/common/utils/misc'; import { InvalidRemoteJupyterServerUriHandleError } from '../../errors/invalidRemoteJupyterServerUriHandleError'; import { computeServerId, generateUriFromRemoteProvider } from '../jupyterUtils'; -import { IInternalJupyterUriProvider, IJupyterUriProviderRegistration } from '../types'; +import { + IInternalJupyterUriProvider, + IJupyterServerUriEntry, + IJupyterServerUriStorage, + IJupyterUriProviderRegistration +} from '../types'; import { sendTelemetryEvent } from '../../../telemetry'; import { traceError } from '../../../platform/logging'; import { IJupyterServerUri, IJupyterUriProvider, JupyterServerUriHandle } from '../../../api'; @@ -35,10 +40,12 @@ export class JupyterUriProviderRegistration implements IJupyterUriProviderRegist constructor( @inject(IExtensions) private readonly extensions: IExtensions, @inject(IDisposableRegistry) disposables: IDisposableRegistry, - @inject(IMemento) @named(GLOBAL_MEMENTO) private readonly globalMemento: Memento + @inject(IMemento) @named(GLOBAL_MEMENTO) private readonly globalMemento: Memento, + @inject(IJupyterServerUriStorage) serverStorage: IJupyterServerUriStorage ) { disposables.push(this._onProvidersChanged); disposables.push(new Disposable(() => this._providers.forEach((p) => p.dispose()))); + disposables.push(serverStorage.onDidRemove(this.onDidRemoveServer, this)); } public async getProviders(): Promise> { @@ -96,6 +103,17 @@ export class JupyterUriProviderRegistration implements IJupyterUriProviderRegist return provider.getServerUri(handle); } + private onDidRemoveServer(e: IJupyterServerUriEntry[]) { + Promise.all( + e.map(async (s) => { + const provider = await this.getProvider(s.provider.id).catch(noop); + if (!provider || !provider.removeHandle) { + return; + } + await provider.removeHandle(s.provider.handle).catch(noop); + }) + ).catch(noop); + } private loadOtherExtensions(): Promise { if (!this.loadedOtherExtensionsPromise) { this.loadedOtherExtensionsPromise = this.loadOtherExtensionsImpl(); diff --git a/src/kernels/jupyter/connection/jupyterUriProviderRegistration.unit.test.ts b/src/kernels/jupyter/connection/jupyterUriProviderRegistration.unit.test.ts index f48f3088af1..61ab0b4c7b4 100644 --- a/src/kernels/jupyter/connection/jupyterUriProviderRegistration.unit.test.ts +++ b/src/kernels/jupyter/connection/jupyterUriProviderRegistration.unit.test.ts @@ -9,7 +9,7 @@ import * as vscode from 'vscode'; import { Extensions } from '../../../platform/common/application/extensions.node'; import { FileSystem } from '../../../platform/common/platform/fileSystem.node'; import { JupyterUriProviderRegistration } from './jupyterUriProviderRegistration'; -import { IInternalJupyterUriProvider } from '../types'; +import { IInternalJupyterUriProvider, IJupyterServerUriEntry, IJupyterServerUriStorage } from '../types'; import { IDisposable } from '../../../platform/common/types'; import { disposeAllDisposables } from '../../../platform/common/helpers'; import { IJupyterServerUri, JupyterServerUriHandle } from '../../../api'; @@ -67,7 +67,16 @@ suite('URI Picker', () => { const memento = mock(); when(memento.get(anything())).thenReturn([]); when(memento.get(anything(), anything())).thenReturn([]); - registration = new JupyterUriProviderRegistration(extensions, disposables, instance(memento)); + const uriStorage = mock(); + const onDidRemove = new vscode.EventEmitter(); + disposables.push(onDidRemove); + when(uriStorage.onDidRemove).thenReturn(onDidRemove.event); + registration = new JupyterUriProviderRegistration( + extensions, + disposables, + instance(memento), + instance(uriStorage) + ); providerIds.map(async (id) => { const extension = TypeMoq.Mock.ofType>(); const packageJson = TypeMoq.Mock.ofType(); From 5ca8a8f0d1e7a52a533841b8bf9d343347c1b40f Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 19 Jun 2023 07:37:06 +1000 Subject: [PATCH 2/3] Fix errors --- .../jupyter/connection/jupyterUriProviderRegistration.ts | 4 +++- .../connection/jupyterUriProviderRegistration.unit.test.ts | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/kernels/jupyter/connection/jupyterUriProviderRegistration.ts b/src/kernels/jupyter/connection/jupyterUriProviderRegistration.ts index 6efb5dbb9ad..01fef237bf6 100644 --- a/src/kernels/jupyter/connection/jupyterUriProviderRegistration.ts +++ b/src/kernels/jupyter/connection/jupyterUriProviderRegistration.ts @@ -20,6 +20,7 @@ import { sendTelemetryEvent } from '../../../telemetry'; import { traceError } from '../../../platform/logging'; import { IJupyterServerUri, IJupyterUriProvider, JupyterServerUriHandle } from '../../../api'; import { Disposables } from '../../../platform/common/utils'; +import { IServiceContainer } from '../../../platform/ioc/types'; const REGISTRATION_ID_EXTENSION_OWNER_MEMENTO_KEY = 'REGISTRATION_ID_EXTENSION_OWNER_MEMENTO_KEY'; @@ -41,10 +42,11 @@ export class JupyterUriProviderRegistration implements IJupyterUriProviderRegist @inject(IExtensions) private readonly extensions: IExtensions, @inject(IDisposableRegistry) disposables: IDisposableRegistry, @inject(IMemento) @named(GLOBAL_MEMENTO) private readonly globalMemento: Memento, - @inject(IJupyterServerUriStorage) serverStorage: IJupyterServerUriStorage + @inject(IServiceContainer) serviceContainer: IServiceContainer ) { disposables.push(this._onProvidersChanged); disposables.push(new Disposable(() => this._providers.forEach((p) => p.dispose()))); + const serverStorage = serviceContainer.get(IJupyterServerUriStorage); disposables.push(serverStorage.onDidRemove(this.onDidRemoveServer, this)); } diff --git a/src/kernels/jupyter/connection/jupyterUriProviderRegistration.unit.test.ts b/src/kernels/jupyter/connection/jupyterUriProviderRegistration.unit.test.ts index 61ab0b4c7b4..3cac3f6524c 100644 --- a/src/kernels/jupyter/connection/jupyterUriProviderRegistration.unit.test.ts +++ b/src/kernels/jupyter/connection/jupyterUriProviderRegistration.unit.test.ts @@ -13,6 +13,7 @@ import { IInternalJupyterUriProvider, IJupyterServerUriEntry, IJupyterServerUriS import { IDisposable } from '../../../platform/common/types'; import { disposeAllDisposables } from '../../../platform/common/helpers'; import { IJupyterServerUri, JupyterServerUriHandle } from '../../../api'; +import { IServiceContainer } from '../../../platform/ioc/types'; class MockProvider implements IInternalJupyterUriProvider { public get id() { @@ -71,11 +72,13 @@ suite('URI Picker', () => { const onDidRemove = new vscode.EventEmitter(); disposables.push(onDidRemove); when(uriStorage.onDidRemove).thenReturn(onDidRemove.event); + const serviceContainer = mock(); + when(serviceContainer.get(IJupyterServerUriStorage)).thenReturn(instance(uriStorage)); registration = new JupyterUriProviderRegistration( extensions, disposables, instance(memento), - instance(uriStorage) + instance(serviceContainer) ); providerIds.map(async (id) => { const extension = TypeMoq.Mock.ofType>(); From 403526f02fd66a4a5ed533f361433936207a31a5 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 23 Jun 2023 12:11:40 +1000 Subject: [PATCH 3/3] Notify providers when a server is deleted from MRU --- .../jupyterUriProviderRegistration.ts | 32 +- ...upyterUriProviderRegistration.unit.test.ts | 442 ++++++++++++------ src/kernels/jupyter/serviceRegistry.node.ts | 1 + src/kernels/jupyter/serviceRegistry.web.ts | 1 + src/kernels/jupyter/types.ts | 1 - 5 files changed, 319 insertions(+), 158 deletions(-) diff --git a/src/kernels/jupyter/connection/jupyterUriProviderRegistration.ts b/src/kernels/jupyter/connection/jupyterUriProviderRegistration.ts index 01fef237bf6..ff2cd275201 100644 --- a/src/kernels/jupyter/connection/jupyterUriProviderRegistration.ts +++ b/src/kernels/jupyter/connection/jupyterUriProviderRegistration.ts @@ -21,14 +21,18 @@ import { traceError } from '../../../platform/logging'; import { IJupyterServerUri, IJupyterUriProvider, JupyterServerUriHandle } from '../../../api'; import { Disposables } from '../../../platform/common/utils'; import { IServiceContainer } from '../../../platform/ioc/types'; +import { IExtensionSyncActivationService } from '../../../platform/activation/types'; -const REGISTRATION_ID_EXTENSION_OWNER_MEMENTO_KEY = 'REGISTRATION_ID_EXTENSION_OWNER_MEMENTO_KEY'; +export const REGISTRATION_ID_EXTENSION_OWNER_MEMENTO_KEY = 'REGISTRATION_ID_EXTENSION_OWNER_MEMENTO_KEY'; /** * Handles registration of 3rd party URI providers. */ @injectable() -export class JupyterUriProviderRegistration implements IJupyterUriProviderRegistration { +export class JupyterUriProviderRegistration + extends Disposables + implements IJupyterUriProviderRegistration, IExtensionSyncActivationService +{ private readonly _onProvidersChanged = new EventEmitter(); private loadedOtherExtensionsPromise: Promise | undefined; private _providers = new Map(); @@ -42,21 +46,18 @@ export class JupyterUriProviderRegistration implements IJupyterUriProviderRegist @inject(IExtensions) private readonly extensions: IExtensions, @inject(IDisposableRegistry) disposables: IDisposableRegistry, @inject(IMemento) @named(GLOBAL_MEMENTO) private readonly globalMemento: Memento, - @inject(IServiceContainer) serviceContainer: IServiceContainer + @inject(IServiceContainer) private readonly serviceContainer: IServiceContainer ) { - disposables.push(this._onProvidersChanged); - disposables.push(new Disposable(() => this._providers.forEach((p) => p.dispose()))); - const serverStorage = serviceContainer.get(IJupyterServerUriStorage); - disposables.push(serverStorage.onDidRemove(this.onDidRemoveServer, this)); + super(); + disposables.push(this); + this.disposables.push(this._onProvidersChanged); + this.disposables.push(new Disposable(() => this._providers.forEach((p) => p.dispose()))); } - public async getProviders(): Promise> { - await this.loadOtherExtensions(); - - // Other extensions should have registered in their activate callback - return Array.from(this.providers.values()); + public activate(): void { + const serverStorage = this.serviceContainer.get(IJupyterServerUriStorage); + this.disposables.push(serverStorage.onDidRemove(this.onDidRemoveServer, this)); } - public async getProvider(id: string): Promise { await this.loadOtherExtensions(); if (!this._providers.has(id)) { @@ -78,17 +79,18 @@ export class JupyterUriProviderRegistration implements IJupyterUriProviderRegist } this._onProvidersChanged.fire(); - return { + const disposable = { dispose: () => { this._providers.get(provider.id)?.dispose(); this._providers.delete(provider.id); this._onProvidersChanged.fire(); } }; + this.disposables.push(disposable); + return disposable; } public async getJupyterServerUri(id: string, handle: JupyterServerUriHandle): Promise { await this.loadOtherExtensions(); - const provider = this._providers.get(id); if (!provider) { traceError(`${localize.DataScience.unknownServerUri}. Provider Id=${id} and handle=${handle}`); diff --git a/src/kernels/jupyter/connection/jupyterUriProviderRegistration.unit.test.ts b/src/kernels/jupyter/connection/jupyterUriProviderRegistration.unit.test.ts index 3cac3f6524c..7ad4fc7aecb 100644 --- a/src/kernels/jupyter/connection/jupyterUriProviderRegistration.unit.test.ts +++ b/src/kernels/jupyter/connection/jupyterUriProviderRegistration.unit.test.ts @@ -1,163 +1,321 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { assert } from 'chai'; -import * as sinon from 'sinon'; -import { anything, instance, mock, when } from 'ts-mockito'; -import * as TypeMoq from 'typemoq'; -import * as vscode from 'vscode'; -import { Extensions } from '../../../platform/common/application/extensions.node'; -import { FileSystem } from '../../../platform/common/platform/fileSystem.node'; -import { JupyterUriProviderRegistration } from './jupyterUriProviderRegistration'; -import { IInternalJupyterUriProvider, IJupyterServerUriEntry, IJupyterServerUriStorage } from '../types'; -import { IDisposable } from '../../../platform/common/types'; +import * as fakeTimers from '@sinonjs/fake-timers'; +import { assert, use } from 'chai'; +import chaiAsPromised from 'chai-as-promised'; +import { anything, instance, mock, reset, verify, when } from 'ts-mockito'; +import { + JupyterUriProviderRegistration, + REGISTRATION_ID_EXTENSION_OWNER_MEMENTO_KEY +} from './jupyterUriProviderRegistration'; +import { IJupyterServerUriEntry, IJupyterServerUriStorage } from '../types'; +import { IDisposable, IExtensions } from '../../../platform/common/types'; import { disposeAllDisposables } from '../../../platform/common/helpers'; -import { IJupyterServerUri, JupyterServerUriHandle } from '../../../api'; +import { IJupyterServerUri, IJupyterUriProvider } from '../../../api'; import { IServiceContainer } from '../../../platform/ioc/types'; +import { Disposable, EventEmitter, Memento, QuickPickItem } from 'vscode'; +import { createEventHandler } from '../../../test/common'; +import { resolvableInstance } from '../../../test/datascience/helpers'; +import { computeServerId, generateUriFromRemoteProvider } from '../jupyterUtils'; +import { DataScience } from '../../../platform/common/utils/localize'; +use(chaiAsPromised); -class MockProvider implements IInternalJupyterUriProvider { - public get id() { - return this._id; - } - private currentBearer = 1; - private result: string = '1'; - public readonly extensionId: string = 'foo.bar'; - constructor(private readonly _id: string) { - // Id should be readonly - } - public getQuickPickEntryItems(): vscode.QuickPickItem[] { - return [{ label: 'Foo' }]; - } - public async handleQuickPick( - _item: vscode.QuickPickItem, - back: boolean - ): Promise { - return back ? 'back' : this.result; - } - public async getServerUri(handle: string): Promise { - if (handle === '1') { - return { - // eslint-disable-next-line - baseUrl: 'http://foobar:3000', - token: '', - displayName: 'dummy', - authorizationHeader: { Bearer: this.currentBearer.toString() } - }; - } - - throw new Error('Invalid server uri handle'); - } -} - -/* eslint-disable , @typescript-eslint/no-explicit-any */ -suite('URI Picker', () => { +suite('Uri Provider Registration', () => { const disposables: IDisposable[] = []; - teardown(() => { - sinon.restore(); - disposeAllDisposables(disposables); - }); - suiteSetup(() => sinon.restore()); - async function createRegistration(providerIds: string[]) { - let registration: JupyterUriProviderRegistration | undefined; - const extensionList: vscode.Extension[] = []; - const fileSystem = mock(FileSystem); - const allStub = sinon.stub(Extensions.prototype, 'all'); - allStub.callsFake(() => extensionList); - const extensions = new Extensions(instance(fileSystem)); - when(fileSystem.exists(anything())).thenResolve(false); - const memento = mock(); - when(memento.get(anything())).thenReturn([]); - when(memento.get(anything(), anything())).thenReturn([]); - const uriStorage = mock(); - const onDidRemove = new vscode.EventEmitter(); - disposables.push(onDidRemove); - when(uriStorage.onDidRemove).thenReturn(onDidRemove.event); - const serviceContainer = mock(); + let extensions: IExtensions; + let globalMemento: Memento; + let serviceContainer: IServiceContainer; + let uriStorage: IJupyterServerUriStorage; + let registration: JupyterUriProviderRegistration; + let onDidRemoveServer: EventEmitter; + let clock: fakeTimers.InstalledClock; + setup(async () => { + extensions = mock(); + globalMemento = mock(); + serviceContainer = mock(); + uriStorage = mock(); + onDidRemoveServer = new EventEmitter(); + when(globalMemento.get(REGISTRATION_ID_EXTENSION_OWNER_MEMENTO_KEY, anything())).thenCall( + (_, defaultValue) => defaultValue + ); + when(uriStorage.onDidRemove).thenReturn(onDidRemoveServer.event); when(serviceContainer.get(IJupyterServerUriStorage)).thenReturn(instance(uriStorage)); + when(extensions.all).thenReturn([]); + clock = fakeTimers.install(); + disposables.push(new Disposable(() => clock.uninstall())); + registration = new JupyterUriProviderRegistration( - extensions, + instance(extensions), disposables, - instance(memento), + instance(globalMemento), instance(serviceContainer) ); - providerIds.map(async (id) => { - const extension = TypeMoq.Mock.ofType>(); - const packageJson = TypeMoq.Mock.ofType(); - const contributes = TypeMoq.Mock.ofType(); - extension.setup((e) => e.packageJSON).returns(() => packageJson.object); - packageJson.setup((p) => p.contributes).returns(() => contributes.object); - contributes.setup((p) => p.pythonRemoteServerProvider).returns(() => [{ d: '' }]); - extension - .setup((e) => e.activate()) - .returns(() => { + registration.activate(); + await clock.runAllAsync(); + }); + teardown(() => disposeAllDisposables(disposables)); + test('No Providers registered', async () => { + assert.deepEqual(registration.providers, [], 'Providers should be empty'); + + await clock.runAllAsync(); + assert.deepEqual(registration.providers, [], 'Providers should be empty'); + }); + test('No Providers registered even after activating extension that seems to have a Jupyter Provider', async () => { + let activatedRightExtension = false; + let activatedWrongExtension = false; + when(extensions.all).thenReturn([ + { + activate: () => { + activatedRightExtension = true; return Promise.resolve(); - }); - extension.setup((e) => e.isActive).returns(() => false); - extensionList.push(extension.object); - registration?.registerProvider(new MockProvider(id), '1'); - }); - return registration; - } + }, + id: 'xyz', + isActive: false, + packageJSON: { contributes: { pythonRemoteServerProvider: {} } } as any + } as any, + { + activate: () => { + activatedWrongExtension = true; + return Promise.resolve(); + }, + id: 'xyz', + isActive: false + } as any + ]); + + assert.deepEqual(registration.providers, [], 'Providers should be empty'); + + await clock.runAllAsync(); + + assert.deepEqual(registration.providers, [], 'Providers should be empty'); + assert.strictEqual(activatedRightExtension, true, 'Extension should have been activated'); + assert.strictEqual(activatedWrongExtension, false, 'Extension should not have been activated'); + }); + test('Once a provider is registered trigger a change event', async () => { + const eventHandler = createEventHandler(registration, 'onDidChangeProviders', disposables); + + assert.deepEqual(registration.providers, [], 'Providers should be empty'); + + const provider1 = createAndRegisterJupyterUriProvider('ext', '1'); + + assert.strictEqual(registration.providers.length, 1); + assert.strictEqual(registration.providers[0].extensionId, 'ext'); + assert.strictEqual(registration.providers[0].id, '1'); + assert.strictEqual(eventHandler.count, 1); + + const provider2 = createAndRegisterJupyterUriProvider('ext', '2'); + assert.strictEqual(registration.providers.length, 2); + assert.strictEqual(eventHandler.count, 2); + + // Now remove provider 1 + provider1.disposable.dispose(); + + assert.strictEqual(registration.providers.length, 1); + assert.strictEqual(registration.providers[0].extensionId, 'ext'); + assert.strictEqual(registration.providers[0].id, '2'); + assert.strictEqual(eventHandler.count, 3); + + provider2.disposable.dispose(); + + assert.strictEqual(registration.providers.length, 0); + assert.strictEqual(eventHandler.count, 4); + }); + test('Cannot register the same provider twice', async () => { + const eventHandler = createEventHandler(registration, 'onDidChangeProviders', disposables); + + assert.deepEqual(registration.providers, [], 'Providers should be empty'); + + createAndRegisterJupyterUriProvider('ext', '1'); + + assert.strictEqual(registration.providers.length, 1); + assert.strictEqual(eventHandler.count, 1); + + assert.throws(() => createAndRegisterJupyterUriProvider('ext', '1')); + }); + test('Get a provider by id', async () => { + createAndRegisterJupyterUriProvider('a', '1'); + createAndRegisterJupyterUriProvider('b', '2'); + + assert.strictEqual((await registration.getProvider('1'))?.extensionId, 'a'); + assert.strictEqual((await registration.getProvider('2'))?.extensionId, 'b'); + assert.isUndefined(await registration.getProvider('3')); + }); + test('Throws an error when getting a server for an invalid item', async () => { + const { provider: provider1 } = createAndRegisterJupyterUriProvider('ext', 'a'); + const { provider: provider2 } = createAndRegisterJupyterUriProvider('ext', 'b'); + when(provider1.getHandles!()).thenResolve(['handle1', 'handle2']); + when(provider2.getHandles!()).thenResolve(['handlea', 'handleb']); - test('Simple', async () => { - const registration = await createRegistration(['1']); - const pickers = registration.providers; - assert.equal(pickers.length, 1, 'Default picker should be there'); - const quickPick = await pickers[0].getQuickPickEntryItems!(); - assert.equal(quickPick.length, 1, 'No quick pick items added'); - const handle = await pickers[0].handleQuickPick!(quickPick[0], false); - assert.ok(handle, 'Handle not set'); - const uri = await registration.getJupyterServerUri('1', handle!); - // eslint-disable-next-line - assert.equal(uri.baseUrl, 'http://foobar:3000', 'Base URL not found'); - assert.equal(uri.displayName, 'dummy', 'Display name not found'); + await assert.isRejected(registration.getJupyterServerUri('unknownId', 'unknownHandle')); + await assert.isRejected(registration.getJupyterServerUri('a', 'unknownHandle')); + await assert.isRejected(registration.getJupyterServerUri('b', 'unknownHandle')); }); - test('Back', async () => { - const registration = await createRegistration(['1']); - const pickers = registration.providers; - assert.equal(pickers.length, 1, 'Default picker should be there'); - const quickPick = await pickers[0].getQuickPickEntryItems!(); - assert.equal(quickPick.length, 1, 'No quick pick items added'); - const handle = await pickers[0].handleQuickPick!(quickPick[0], true); - assert.equal(handle, 'back', 'Should be sending back'); + test('Get a Jupyter Server by handle', async () => { + const { provider: provider1 } = createAndRegisterJupyterUriProvider('ext', 'a'); + const { provider: provider2 } = createAndRegisterJupyterUriProvider('ext', 'b'); + when(provider1.getHandles!()).thenResolve(['handle1', 'handle2']); + when(provider2.getHandles!()).thenResolve(['handlea', 'handleb']); + const serverForHandle1 = mock(); + when(serverForHandle1.baseUrl).thenReturn('http://server1/'); + when(serverForHandle1.displayName).thenReturn('Server 1'); + const serverForHandleB = mock(); + when(serverForHandleB.baseUrl).thenReturn('http://serverB/'); + when(serverForHandleB.displayName).thenReturn('Server B'); + when(provider1.getServerUri('handle1')).thenResolve(resolvableInstance(serverForHandle1)); + when(provider2.getServerUri('handleb')).thenResolve(resolvableInstance(serverForHandleB)); + + const server = await registration.getJupyterServerUri('a', 'handle1'); + assert.strictEqual(server.displayName, 'Server 1'); + assert.strictEqual(server, instance(serverForHandle1)); + + const server2 = await registration.getJupyterServerUri('b', 'handleb'); + assert.strictEqual(server2.displayName, 'Server B'); + assert.strictEqual(server2, instance(serverForHandleB)); }); - test('Error', async () => { - const registration = await createRegistration(['1']); - const pickers = registration.providers; - assert.equal(pickers.length, 1, 'Default picker should be there'); - const quickPick = await pickers[0].getQuickPickEntryItems!(); - assert.equal(quickPick.length, 1, 'No quick pick items added'); - try { - await registration.getJupyterServerUri('1', 'foobar'); - // eslint-disable-next-line - assert.fail('Should not get here'); - } catch { - // This means test passed. - } + test('Notify the provider when a server is deleted', async () => { + const { provider: provider1 } = createAndRegisterJupyterUriProvider('ext', 'a'); + const { provider: provider2 } = createAndRegisterJupyterUriProvider('ext', 'b'); + when(provider1.getHandles!()).thenResolve(['handle1', 'handle2']); + when(provider2.getHandles!()).thenResolve(['handlea', 'handleb']); + when(provider1.removeHandle!(anything())).thenResolve(); + when(provider2.removeHandle!(anything())).thenResolve(); + + const removedServer: IJupyterServerUriEntry = { + provider: { handle: 'handle2', id: 'a' }, + serverId: await computeServerId(generateUriFromRemoteProvider('a', 'handle2')), + time: Date.now(), + uri: 'http://handle1:8888/?token=1234', + displayName: 'Server for Handle2', + isValidated: false + }; + onDidRemoveServer.fire([removedServer]); + await clock.runAllAsync(); + + verify(provider1.removeHandle!('handle1')).never(); + verify(provider1.removeHandle!('handle2')).once(); + verify(provider2.removeHandle!(anything())).never(); }); - test('No picker call', async () => { - const registration = await createRegistration(['1']); - const uri = await registration.getJupyterServerUri('1', '1'); - // eslint-disable-next-line - assert.equal(uri.baseUrl, 'http://foobar:3000', 'Base URL not found'); + test('Verify the handles', async () => { + const { provider: mockProvider } = createAndRegisterJupyterUriProvider('a', '1'); + when(mockProvider.getHandles!()).thenResolve(['handle1', 'handle2']); + + const provider = await registration.getProvider('1'); + + assert.deepEqual(await provider!.getHandles!(), ['handle1', 'handle2']); }); - test('Two pickers', async () => { - const registration = await createRegistration(['1', '2']); - let uri = await registration.getJupyterServerUri('1', '1'); - // eslint-disable-next-line - assert.equal(uri.baseUrl, 'http://foobar:3000', 'Base URL not found'); - uri = await registration.getJupyterServerUri('2', '1'); - // eslint-disable-next-line - assert.equal(uri.baseUrl, 'http://foobar:3000', 'Base URL not found'); + test('Verify onDidChangeHandles is triggered', async () => { + const { provider: mockProvider, onDidChangeHandles } = createAndRegisterJupyterUriProvider('a', '1'); + when(mockProvider.getHandles!()).thenResolve(['handle1', 'handle2']); + + const provider = await registration.getProvider('1'); + const eventHandler = createEventHandler(provider!, 'onDidChangeHandles', disposables); + + onDidChangeHandles.fire(); + + assert.strictEqual(eventHandler.count, 1); }); - test('Two pickers with same id', async () => { - try { - const registration = await createRegistration(['1', '1']); - await registration.getJupyterServerUri('1', '1'); - // eslint-disable-next-line - assert.fail('Should have failed if calling with same picker'); - } catch { - // This means it passed - } + test('Verify not Quick Pick items are returned if there are none', async () => { + const { provider: mockProvider, onDidChangeHandles } = createAndRegisterJupyterUriProvider('a', '1'); + when(mockProvider.getQuickPickEntryItems).thenReturn(undefined); + + const provider = await registration.getProvider('1'); + + onDidChangeHandles.fire(); + + assert.deepEqual(await provider!.getQuickPickEntryItems!(), []); }); + test('Returns a list of the quick pick items', async () => { + const { provider: mockProvider1 } = createAndRegisterJupyterUriProvider('a', '1'); + const { provider: mockProvider2 } = createAndRegisterJupyterUriProvider('ext2', 'b'); + const quickPickItemsForHandle1: QuickPickItem[] = [ + { + label: 'Item 1' + }, + { label: 'Item 2' } + ]; + const quickPickItemsForHandle2: QuickPickItem[] = [ + { + label: 'Item X' + }, + { label: 'Item Y' } + ]; + when(mockProvider1.getQuickPickEntryItems!()).thenResolve(quickPickItemsForHandle1 as any); + when(mockProvider2.getQuickPickEntryItems!()).thenResolve(quickPickItemsForHandle2 as any); + + const provider1 = await registration.getProvider('1'); + const provider2 = await registration.getProvider('b'); + + assert.deepEqual( + await provider1!.getQuickPickEntryItems!(), + quickPickItemsForHandle1.map((item) => { + return { + ...item, + description: DataScience.uriProviderDescriptionFormat(item.description || '', 'a'), + original: item + }; + }) + ); + assert.deepEqual( + await provider2!.getQuickPickEntryItems!(), + quickPickItemsForHandle2.map((item) => { + return { + ...item, + description: DataScience.uriProviderDescriptionFormat(item.description || '', 'ext2'), + original: item + }; + }) + ); + }); + test('Handles the selection of a quick pick item', async () => { + const { provider: mockProvider1 } = createAndRegisterJupyterUriProvider('a', '1'); + const { provider: mockProvider2 } = createAndRegisterJupyterUriProvider('ext2', 'b'); + when(mockProvider1.handleQuickPick!(anything(), anything())).thenResolve(); + when(mockProvider2.handleQuickPick!(anything(), anything())).thenResolve(); + const quickPickItemsForHandle1: QuickPickItem[] = [ + { + label: 'Item 1' + }, + { label: 'Item 2' } + ]; + const quickPickItemsForHandle2: QuickPickItem[] = [ + { + label: 'Item X' + }, + { label: 'Item Y' } + ]; + when(mockProvider1.getQuickPickEntryItems!()).thenResolve(quickPickItemsForHandle1 as any); + when(mockProvider2.getQuickPickEntryItems!()).thenResolve(quickPickItemsForHandle2 as any); + + const provider1 = await registration.getProvider('1'); + const provider2 = await registration.getProvider('b'); + + await provider1?.handleQuickPick!( + { ...quickPickItemsForHandle1[0], original: quickPickItemsForHandle1[0] } as any, + false + ); + + verify(mockProvider1.handleQuickPick!(quickPickItemsForHandle1[0], false)).once(); + verify(mockProvider2.handleQuickPick!(anything(), anything())).never(); + reset(mockProvider1); + + await provider2?.handleQuickPick!( + { ...quickPickItemsForHandle2[1], original: quickPickItemsForHandle2[1] } as any, + true + ); + + verify(mockProvider1.handleQuickPick!(anything(), anything())).never(); + verify(mockProvider2.handleQuickPick!(quickPickItemsForHandle2[1], true)).once(); + }); + + function createAndRegisterJupyterUriProvider(extensionId: string, id: string, disposables: IDisposable[] = []) { + const provider = mock(); + const onDidChangeHandles = new EventEmitter(); + disposables.push(onDidChangeHandles); + when(provider.onDidChangeHandles).thenReturn(onDidChangeHandles.event); + when(provider.id).thenReturn(id); + + const disposable = registration.registerProvider(instance(provider), extensionId); + return { provider, disposable, onDidChangeHandles }; + } }); diff --git a/src/kernels/jupyter/serviceRegistry.node.ts b/src/kernels/jupyter/serviceRegistry.node.ts index 6db1f5e52e0..fc3c55aebe4 100644 --- a/src/kernels/jupyter/serviceRegistry.node.ts +++ b/src/kernels/jupyter/serviceRegistry.node.ts @@ -109,6 +109,7 @@ export function registerTypes(serviceManager: IServiceManager, _isDevMode: boole IJupyterUriProviderRegistration, JupyterUriProviderRegistration ); + serviceManager.addBinding(IJupyterUriProviderRegistration, IExtensionSyncActivationService); serviceManager.addSingleton(IJupyterServerUriStorage, JupyterServerUriStorage); serviceManager.addSingleton(INotebookStarter, JupyterServerStarter); serviceManager.addSingleton(IJupyterServerConnector, JupyterServerConnector); diff --git a/src/kernels/jupyter/serviceRegistry.web.ts b/src/kernels/jupyter/serviceRegistry.web.ts index bd5e309b814..9d49f5faf2c 100644 --- a/src/kernels/jupyter/serviceRegistry.web.ts +++ b/src/kernels/jupyter/serviceRegistry.web.ts @@ -49,6 +49,7 @@ export function registerTypes(serviceManager: IServiceManager, _isDevMode: boole IJupyterUriProviderRegistration, JupyterUriProviderRegistration ); + serviceManager.addBinding(IJupyterUriProviderRegistration, IExtensionSyncActivationService); serviceManager.addSingleton(IJupyterServerUriStorage, JupyterServerUriStorage); serviceManager.addSingleton(IKernelSessionFactory, KernelSessionFactory); serviceManager.addSingleton(JupyterKernelSessionFactory, JupyterKernelSessionFactory); diff --git a/src/kernels/jupyter/types.ts b/src/kernels/jupyter/types.ts index cb2c5c09d1b..e8f0f9a643f 100644 --- a/src/kernels/jupyter/types.ts +++ b/src/kernels/jupyter/types.ts @@ -174,7 +174,6 @@ export const IJupyterUriProviderRegistration = Symbol('IJupyterUriProviderRegist export interface IJupyterUriProviderRegistration { onDidChangeProviders: Event; readonly providers: ReadonlyArray; - getProviders(): Promise>; getProvider(id: string): Promise; registerProvider(provider: IJupyterUriProvider, extensionId: string): IDisposable; getJupyterServerUri(id: string, handle: JupyterServerUriHandle): Promise;