diff --git a/src/sql/workbench/contrib/notebook/test/browser/notebookEditor.test.ts b/src/sql/workbench/contrib/notebook/test/browser/notebookEditor.test.ts index 420132ddd1..41d6b7523b 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/notebookEditor.test.ts +++ b/src/sql/workbench/contrib/notebook/test/browser/notebookEditor.test.ts @@ -17,7 +17,7 @@ import * as stubs from 'sql/workbench/contrib/notebook/test/stubs'; import { NotebookEditorStub } from 'sql/workbench/contrib/notebook/test/testCommon'; import { CellModel } from 'sql/workbench/services/notebook/browser/models/cell'; import { ICellModel, NotebookContentChange } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; -import { INotebookEditor, INotebookService, NotebookRange } from 'sql/workbench/services/notebook/browser/notebookService'; +import { INotebookEditor, INotebookParams, INotebookService, NotebookRange } from 'sql/workbench/services/notebook/browser/notebookService'; import { NotebookService } from 'sql/workbench/services/notebook/browser/notebookServiceImpl'; import { NotebookChangeType } from 'sql/workbench/services/notebook/common/contracts'; import * as TypeMoq from 'typemoq'; @@ -123,8 +123,7 @@ suite('Test class NotebookEditor:', () => { testTitle, untitledUri, untitledTextInput, undefined, instantiationService, notebookService, extensionService ); - const testNotebookEditor = new NotebookEditorStub({ cellGuid: cellTextEditorGuid, editor: queryTextEditor, model: notebookModel }); - testNotebookEditor.id = untitledNotebookInput.notebookUri.toString(); + const testNotebookEditor = new NotebookEditorStub({ cellGuid: cellTextEditorGuid, editor: queryTextEditor, model: notebookModel, notebookParams: { notebookUri: untitledNotebookInput.notebookUri } }); notebookService.addNotebookEditor(testNotebookEditor); notebookEditor.clearInput(); await notebookEditor.setInput(untitledNotebookInput, EditorOptions.create({ pinned: true })); @@ -238,7 +237,6 @@ suite('Test class NotebookEditor:', () => { const oldRange = {} as NotebookRange; const iNotebookEditorMock = TypeMoq.Mock.ofInstance(notebookEditorStub); iNotebookEditorMock.callBase = true; //by default forward all call call to the underlying object - iNotebookEditorMock.object.id = untitledNotebookInput.notebookUri.toString(); iNotebookEditorMock .setup(x => x.deltaDecorations(TypeMoq.It.isAny(), TypeMoq.It.isAny())) .callback((newDecorationRange: NotebookRange, oldDecorationRange: NotebookRange) => { @@ -718,8 +716,7 @@ function setupServices(arg: { workbenchThemeService?: WorkbenchThemeService, ins instantiationService.get(IEditorService), instantiationService.get(IConfigurationService) ); - const notebookEditorStub = new NotebookEditorStub({ cellGuid: cellTextEditorGuid, editor: queryTextEditor, model: new NotebookModelStub() }); - notebookEditorStub.id = untitledNotebookInput.notebookUri.toString(); + const notebookEditorStub = new NotebookEditorStub({ cellGuid: cellTextEditorGuid, editor: queryTextEditor, model: new NotebookModelStub(), notebookParams: { notebookUri: untitledNotebookInput.notebookUri } }); notebookService.addNotebookEditor(notebookEditorStub); return { instantiationService, workbenchThemeService, notebookService, testTitle, extensionService, cellTextEditorGuid, queryTextEditor, untitledNotebookInput, notebookEditorStub }; } diff --git a/src/sql/workbench/contrib/notebook/test/browser/notebookService.test.ts b/src/sql/workbench/contrib/notebook/test/browser/notebookService.test.ts index c1571a6f4b..63cc39663f 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/notebookService.test.ts +++ b/src/sql/workbench/contrib/notebook/test/browser/notebookService.test.ts @@ -4,52 +4,162 @@ *--------------------------------------------------------------------------------------------*/ import * as assert from 'assert'; -import { NotebookService } from 'sql/workbench/services/notebook/browser/notebookServiceImpl'; -import { TestLifecycleService, TestFileService } from 'vs/workbench/test/browser/workbenchTestServices'; -import { TestStorageService, TestExtensionService } from 'vs/workbench/test/common/workbenchTestServices'; -import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; -import { IExtensionManagementService, InstallExtensionEvent, DidInstallExtensionEvent, IExtensionIdentifier, DidUninstallExtensionEvent } from 'vs/platform/extensionManagement/common/extensionManagement'; -import { ExtensionManagementService } from 'vs/workbench/services/extensionManagement/common/extensionManagementService'; -import { NullLogService } from 'vs/platform/log/common/log'; +import * as azdata from 'azdata'; +import * as sinon from 'sinon'; +import { Deferred } from 'sql/base/common/promise'; import { NBTestQueryManagementService } from 'sql/workbench/contrib/notebook/test/nbTestQueryManagementService'; -import { INotebookService } from 'sql/workbench/services/notebook/browser/notebookService'; -import { Emitter } from 'vs/base/common/event'; -import { Registry } from 'vs/platform/registry/common/platform'; -import { NotebookProviderRegistration, INotebookProviderRegistry, Extensions } from 'sql/workbench/services/notebook/common/notebookRegistry'; +import { NotebookModelStub } from 'sql/workbench/contrib/notebook/test/stubs'; +import { NotebookEditorStub } from 'sql/workbench/contrib/notebook/test/testCommon'; +import { notebookConstants } from 'sql/workbench/services/notebook/browser/interfaces'; +import { ICellModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; +import { INavigationProvider, INotebookEditor, INotebookManager, INotebookParams, INotebookProvider, NavigationProviders, SQL_NOTEBOOK_PROVIDER, unsavedBooksContextKey } from 'sql/workbench/services/notebook/browser/notebookService'; +import { FailToSaveTrustState, NotebookService, NotebookServiceNoProviderRegistered, NotebookUriNotDefined, ProviderDescriptor } from 'sql/workbench/services/notebook/browser/notebookServiceImpl'; +import { NotebookChangeType } from 'sql/workbench/services/notebook/common/contracts'; +import { Extensions, INotebookProviderRegistry, NotebookProviderRegistration } from 'sql/workbench/services/notebook/common/notebookRegistry'; +import * as TypeMoq from 'typemoq'; +import { errorHandler, onUnexpectedError } from 'vs/base/common/errors'; +import { Emitter, Event } from 'vs/base/common/event'; +import { URI } from 'vs/base/common/uri'; +import { DidInstallExtensionEvent, DidUninstallExtensionEvent, IExtensionIdentifier, IExtensionManagementService, InstallExtensionEvent } from 'vs/platform/extensionManagement/common/extensionManagement'; +import { ExtensionIdentifier, IExtensionDescription } from 'vs/platform/extensions/common/extensions'; +import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; import { MockContextKeyService } from 'vs/platform/keybinding/test/common/mockKeybindingService'; +import { NullLogService } from 'vs/platform/log/common/log'; +import { Registry } from 'vs/platform/registry/common/platform'; +import { ExtensionManagementService } from 'vs/workbench/services/extensionManagement/common/extensionManagementService'; +import { TestFileService, TestLifecycleService } from 'vs/workbench/test/browser/workbenchTestServices'; +import { TestExtensionService, TestStorageService } from 'vs/workbench/test/common/workbenchTestServices'; -suite('Notebook Service Tests', function (): void { - let notebookService: INotebookService; +/** + * class to mock azdata.nb.ServerManager object + */ +class TestServerManager implements azdata.nb.ServerManager { + isStarted: boolean = true; //by default our mock creates ServerManager in started mode. + onServerStarted: Event; + async startServer(kernelSpec: azdata.nb.IKernelSpec): Promise { + this.isStarted = true; + } + async stopServer(): Promise { + this.isStarted = false; + } - let installEvent: Emitter, - didInstallEvent: Emitter, - uninstallEvent: Emitter, - didUninstallEvent: Emitter; +} + +/** + * TestNotebookManager - creates a NotebookManager object that helps keep track of state needed by testing + */ +class TestNotebookManager implements INotebookManager { + contentManager: undefined; + sessionManager: undefined; + constructor( + public providerId: string = 'providerId1', + public serverManager: TestServerManager = new TestServerManager() + ) { } +} + +/** + * * TestNotebookProvider - creates a NotebookManager object that helps keep track of state needed by testing + */ +class TestNotebookProvider implements INotebookProvider { + constructor( + public providerId: string = 'providerId1', + public manager: TestNotebookManager = new TestNotebookManager(providerId) + ) { } + + getNotebookManager(uri: URI): Thenable { + return Promise.resolve(this.manager); + } + + handleNotebookClosed(_notebookUri: URI): void { + // do nothing + } +} + +suite('ProviderDescriptor:', () => { + test('Verifies varies getter setters of Provider Descriptor', async () => { + const notebookProvider = {}; + const providerDescriptor = new ProviderDescriptor(notebookProvider); + assert.strictEqual(providerDescriptor.instance, notebookProvider, `providerDescriptor instance should be the value passed into the constructor`); + const providerInstancePromise = providerDescriptor.instanceReady; + assert.notStrictEqual(providerInstancePromise, undefined, `providerDescriptor instanceReady should not return an undefined promise object`); + const result = await providerInstancePromise; + assert.strictEqual(result, notebookProvider, `instanceReady property of the providerDescriptor should resolve with notebookProvider object that it was constructed with`); + + providerDescriptor.instance = undefined; + assert.strictEqual(providerDescriptor.instance, undefined, `provider.Descriptor instance should be undefined when we set it explicitly to undefined`); + providerDescriptor.instance = notebookProvider; + assert.strictEqual(providerDescriptor.instance, notebookProvider, `provider.Descriptor instance should be instance: ${notebookProvider} that we explicitly set it to`); + }); +}); + +suite('NotebookService:', function (): void { + let notebookService: NotebookService; + let lifecycleService: TestLifecycleService; + let storageService: TestStorageService; + let extensionService: TestExtensionService; + let fileService: TestFileService; + let logService: NullLogService; + let logServiceMock: TypeMoq.Mock; + let contextService: MockContextKeyService; + let queryManagementService: NBTestQueryManagementService; + let instantiationService: TestInstantiationService; + let extensionManagementService: IExtensionManagementService; + let extensionServiceMock: TypeMoq.Mock; + let testNo = 0; + let sandbox: sinon.SinonSandbox; + + let installExtensionEmitter: Emitter, + didInstallExtensionEmitter: Emitter, + uninstallExtensionEmitter: Emitter, + didUninstallExtensionEmitter: Emitter; setup(() => { - const lifecycleService = new TestLifecycleService(); - const storageService = new TestStorageService(); - const extensionService = new TestExtensionService(); - const fileService = new TestFileService(); - const logService = new NullLogService(); - const contextService = new MockContextKeyService(); - const queryManagementService = new NBTestQueryManagementService(); + testNo++; + lifecycleService = new TestLifecycleService(); + storageService = new TestStorageService(); - const instantiationService = new TestInstantiationService(); + extensionService = new TestExtensionService(); + extensionServiceMock = TypeMoq.Mock.ofInstance(extensionService); + extensionServiceMock.callBase = true; - installEvent = new Emitter(); - didInstallEvent = new Emitter(); - uninstallEvent = new Emitter(); - didUninstallEvent = new Emitter(); + fileService = new TestFileService(); + logService = new NullLogService(); + logServiceMock = TypeMoq.Mock.ofInstance(logService); + logServiceMock.callBase = true; + + contextService = new MockContextKeyService(); + queryManagementService = new NBTestQueryManagementService(); + instantiationService = new TestInstantiationService(); + + installExtensionEmitter = new Emitter(); + didInstallExtensionEmitter = new Emitter(); + uninstallExtensionEmitter = new Emitter(); + didUninstallExtensionEmitter = new Emitter(); instantiationService.stub(IExtensionManagementService, ExtensionManagementService); - instantiationService.stub(IExtensionManagementService, 'onInstallExtension', installEvent.event); - instantiationService.stub(IExtensionManagementService, 'onDidInstallExtension', didInstallEvent.event); - instantiationService.stub(IExtensionManagementService, 'onUninstallExtension', uninstallEvent.event); - instantiationService.stub(IExtensionManagementService, 'onDidUninstallExtension', didUninstallEvent.event); - const extensionManagementService = instantiationService.get(IExtensionManagementService); + instantiationService.stub(IExtensionManagementService, 'onInstallExtension', installExtensionEmitter.event); + instantiationService.stub(IExtensionManagementService, 'onDidInstallExtension', didInstallExtensionEmitter.event); + instantiationService.stub(IExtensionManagementService, 'onUninstallExtension', uninstallExtensionEmitter.event); + instantiationService.stub(IExtensionManagementService, 'onDidUninstallExtension', didUninstallExtensionEmitter.event); + extensionManagementService = instantiationService.get(IExtensionManagementService); - notebookService = new NotebookService(lifecycleService, storageService, extensionService, extensionManagementService, instantiationService, fileService, logService, queryManagementService, contextService); + notebookService = new NotebookService(lifecycleService, storageService, extensionServiceMock.object, extensionManagementService, instantiationService, fileService, logServiceMock.object, queryManagementService, contextService); + sandbox = sinon.sandbox.create(); + }); + + teardown(() => { + sandbox.restore(); + }); + + test(`verify that setTrusted logs message and does not throw when storageService.store throws`, async () => { + const saveError = new Error(`exception encountered while storing`); + sandbox.stub(storageService, 'store').throws(saveError); + logServiceMock.setup(x => x.trace(TypeMoq.It.isAnyString())).returns((message: string) => { + assert.ok(message.startsWith(FailToSaveTrustState), `the traced log message must start with ${FailToSaveTrustState}`); + }).verifiable(TypeMoq.Times.once()); + const notebookUri = setTrustedSetup(notebookService); + await notebookService.setTrusted(notebookUri, true); + logServiceMock.verifyAll(); }); test('Validate default properties on create', async function (): Promise { @@ -60,11 +170,12 @@ suite('Notebook Service Tests', function (): void { assert.equal(notebookService.getStandardKernelsForProvider('sql').length, 1, 'SQL kernel should be provided by default'); assert.equal(notebookService.getStandardKernelsForProvider('otherProvider'), undefined, 'Other provider should not have kernels since it has not been added as a provider'); assert.deepEqual(notebookService.getSupportedFileExtensions(), ['IPYNB'], 'IPYNB file extension should be supported by default'); + await notebookService.registrationComplete; + assert.ok(notebookService.isRegistrationComplete, `notebookService.isRegistrationComplete should be true once its registrationComplete promise is resolved`); }); test('Validate another provider added successfully', async function (): Promise { await notebookService.registrationComplete; - assert.equal(notebookService.isRegistrationComplete, true, 'Registration should be complete since sql provider exists in queryManagementService'); assert.deepEqual(notebookService.getProvidersForFileType('ipynb'), ['sql'], 'sql provider should be registered for ipynb extension'); const otherProviderRegistration: NotebookProviderRegistration = { @@ -85,4 +196,391 @@ suite('Notebook Service Tests', function (): void { assert.equal(notebookService.getStandardKernelsForProvider('otherProvider').length, 1, 'otherProvider kernel info could not be found'); assert.deepEqual(notebookService.getStandardKernelsForProvider('otherProvider')[0], otherProviderRegistration.standardKernels, 'otherProviderRegistration standard kernels does not match'); }); + + test('tests that dispose() method calls dispose on underlying disposable objects exactly once', async () => { + await notebookService.registrationComplete; + notebookService.dispose(); + assert.strictEqual(notebookService['_store']['_isDisposed'], true, `underlying disposable store object should be disposed state`); + }); + + test('verify that getOrCreateNotebookManager does not throw when extensionService.whenInstalledExtensionRegistered() throws', async () => { + const providerId = 'providerId1'; + createRegisteredProviderWithManager({ notebookService, providerId }); + notebookService.registerProvider(providerId, undefined); + //verify method under test logs error and does not throw when extensionService.whenInstalledExtensionRegistered() throws + const error: Error = new Error('Extension Registration Failed'); + extensionServiceMock.setup(x => x.whenInstalledExtensionsRegistered()).throws(error); + + logServiceMock.setup(x => x.error(TypeMoq.It.isAny())) + .returns((_error: string | Error, ...args: any[]) => { + assert.strictEqual(_error, error, `error object passed to logService.error call must be the one thrown from whenInstalledExtensionsRegistered call`); + }) + .verifiable(TypeMoq.Times.once()); + await notebookService.getOrCreateNotebookManager(providerId, URI.parse('untitled:uri1')); + logServiceMock.verifyAll(); + }); + + + test('verify that getOrCreateNotebookManager throws when no providers are registered', async () => { + const methodName = 'getOrCreateNotebookManager'; + + // register the builtin sql provider to be undefined + notebookService.registerProvider(SQL_NOTEBOOK_PROVIDER, undefined); + try { + await notebookService.getOrCreateNotebookManager('test', URI.parse('untitled:uri1')); + throw Error(`${methodName} did not throw as was expected`); + } catch (error) { + assert.strictEqual((error as Error).message, NotebookServiceNoProviderRegistered, `${methodName} should throw error with message:${NotebookServiceNoProviderRegistered}' when no providers are registered`); + } + + // when even the default provider is not registered, method under test throws exception + unRegisterProviders(notebookService); + try { + await notebookService.getOrCreateNotebookManager(SQL_NOTEBOOK_PROVIDER, URI.parse('untitled:uri1')); + throw Error(`${methodName} did not throw as was expected`); + } catch (error) { + assert.strictEqual((error as Error).message, NotebookServiceNoProviderRegistered, `${methodName} should throw error with message:${NotebookServiceNoProviderRegistered}' when no providers are registered`); + } + }); + + test('test register/get of navigationProviders', async () => { + const methodName = 'getNavigationProvider'; + assert.strictEqual(notebookService.getNavigationProvider(), undefined, `${methodName} returns undefined with no providers registered`); + + let providerId = NavigationProviders.NotebooksNavigator; + let provider1 = { providerId: providerId }; + notebookService.registerNavigationProvider(provider1); + let result = notebookService.getNavigationProvider(); + assert.strictEqual(result, provider1, `${methodName} must return the provider that we registered with netbookService for the provider id: ${provider1.providerId}`); + + providerId = NavigationProviders.ProvidedBooksNavigator; + provider1 = { providerId: providerId }; + notebookService.registerNavigationProvider(provider1); + contextService.createKey(unsavedBooksContextKey, {}); + result = notebookService.getNavigationProvider(); + + assert.strictEqual(result, provider1, `${methodName} must return the provider that we registered with netbookService for the provider id: ${provider1.providerId}`); + }); + + test('verifies return value of getOrCreateNotebookManager', async () => { + await notebookService.registrationComplete; + try { + await notebookService.getOrCreateNotebookManager(SQL_NOTEBOOK_PROVIDER, undefined); + throw new Error('expected exception was not thrown'); + } catch (error) { + assert.strictEqual((error as Error).message, NotebookUriNotDefined, `getOrCreateNotebookManager must throw with UriNotDefined error, when a valid uri is not provided`); + } + const provider1Id = SQL_NOTEBOOK_PROVIDER; + const { providerId: provider2Id, manager: provider2Manager } = createRegisteredProviderWithManager({ providerId: 'provider2Id', notebookService }); + + const uri1: URI = URI.parse(`untitled:test1`); + const uri2: URI = URI.parse(`untitled:test2`); + const result1 = await notebookService.getOrCreateNotebookManager(provider1Id, uri1); + const result2 = await notebookService.getOrCreateNotebookManager(provider2Id, uri2); + const result1Again = await notebookService.getOrCreateNotebookManager(provider1Id, uri1); + assert.strictEqual(result2, provider2Manager, `the notebook manager for the provider must be the one returned by getNotebookManager of the provider`); + assert.notStrictEqual(result1, result2, `different notebookManagers should be returned for different uris`); + assert.strictEqual(result1, result1Again, `same notebookManagers should be returned for same uri for builtin providers`); + const result2Again = await notebookService.getOrCreateNotebookManager(provider2Id, uri2); + assert.strictEqual(result2, result2Again, `same notebookManagers should be returned for same uri for custom providers`); + }); + + test('verifies add/remove/find/list/renameNotebookEditor methods and corresponding events', async () => { + assert.strictEqual(notebookService.findNotebookEditor(undefined), undefined, `findNotebookEditor should return undefined for undefined input`); + + let editorsAdded = 0; + let editorsRemoved = 0; + let editorsRenamed = 0; + + const editor1Uri = URI.parse('id1'); + const editor1 = new NotebookEditorStub({ + notebookParams: { + notebookUri: editor1Uri + } + }); + // add provider managers for the old and the new uri. + await addManagers({ notebookService, prefix: 'id', uriPrefix: 'id' }); + await addManagers({ notebookService, prefix: 'newId', uriPrefix: 'newId' }); + notebookService.onNotebookEditorAdd((e: INotebookEditor) => { + assert.strictEqual(e, editor1, `onNotebookEditorAdd event should fire with the INotebookEditor object that we passed to addNotebookEditor call`); + editorsAdded++; + }); + + notebookService.onNotebookEditorRemove((e: INotebookEditor) => { + assert.strictEqual(e, editor1, `onNotebookEditorRemove event should fire with the INotebookEditor object that we passed to removeNotebookEditor call`); + editorsRemoved++; + }); + + notebookService.onNotebookEditorRename((e: INotebookEditor) => { + assert.strictEqual(e, editor1, `onNotebookEditorRename event should fire with the INotebookEditor object that we passed to renameNotebookEditor call`); + editorsRenamed++; + }); + notebookService.addNotebookEditor(editor1); + assert.strictEqual(editorsAdded, 1, `onNotebookEditorAdd should have been called that increments editorsAdded value`); + const newEditor1Uri = URI.parse('newId1'); + notebookService.renameNotebookEditor(editor1Uri, newEditor1Uri, editor1); + assert.strictEqual(editorsRenamed, 1, `onNotebookEditorRename should have been called that increments editorsRenamed value`); + const resultOld = notebookService.findNotebookEditor(editor1Uri); + assert.strictEqual(resultOld, undefined, `findNotebookEditor should return undefined when searching with oldUri`); + const resultNew = notebookService.findNotebookEditor(newEditor1Uri); + assert.strictEqual(resultNew, editor1, `findNotebookEditor should return the editor when searching with the latest Uri`); + let editorList = notebookService.listNotebookEditors(); + assert.strictEqual(editorList.length, 1, `the editor list should just have 1 item`); + assert.strictEqual(editorList[0], editor1, `the first element in the editor list should be the one that we added`); + notebookService.removeNotebookEditor(editor1); + assert.strictEqual(editorsRemoved, 1, `onNotebookEditorRemove should have been called that increments editorsRemoved value`); + editorList = notebookService.listNotebookEditors(); + assert.strictEqual(editorList.length, 0, `the editor list should be empty after we remove the only item that we added`); + + //Remove editor when there are managers already populated + await addManagers({ notebookService, prefix: 'newId', uriPrefix: 'newId' }); + editorsRemoved = 0; editorsAdded = 0; + notebookService.addNotebookEditor(editor1); + notebookService.removeNotebookEditor(editor1); + assert.strictEqual(editorsRemoved, 1, `onNotebookEditorRemove should have been called that increments editorsRemoved value`); + }); + + test('test registration of a new provider with multiple filetypes & kernels and verify that corresponding manager is returned by getOrCreateNotebookManager', async () => { + const providerId = 'Jpeg'; + const notebookProviderRegistration = { + provider: providerId, + fileExtensions: ['jpeg', 'jpg'], + standardKernels: [{ name: 'kernel1' }, { name: 'kernel2' }] + }; + const notebookRegistry = Registry.as(Extensions.NotebookProviderContribution); + notebookRegistry.registerNotebookProvider(notebookProviderRegistration); + const managerPromise = notebookService.getOrCreateNotebookManager(providerId, URI.parse('untitled:jpg')); + const providerInstance = createRegisteredProviderWithManager({ notebookService, providerId }); + notebookService.registerProvider(providerId, providerInstance); + const result = await managerPromise; + + // verify result + assert.strictEqual(result, providerInstance.manager, `createRegisteredProviderWithManager must return the manager corresponding to INotebookProvider that we registered`); + }); + + + test('verify that firing of extensionService.onDidUninstallExtension event calls removeContributedProvidersFromCache', async () => { + const methodName = 'removeContributedProvidersFromCache'; + await notebookService.registrationComplete; + const providerId = 'providerId1'; + extensionServiceMock.setup(x => x.getExtensions()).returns(() => { + return Promise.resolve([ + { + name: 'testExtension', + publisher: 'Test', + version: '1.0.0', + engines: { + vscode: 'vscode-engine' + }, + identifier: new ExtensionIdentifier('id1'), + contributes: { + notebookProvider: { + providerId: providerId + } + }, + isBuiltin: false, + isUnderDevelopment: true, + extensionLocation: URI.parse('extLocation1'), + enableProposedApi: false, + forceReload: true + } + ]); + }); + const extensionIdentifier = ({ + identifier: { + id: 'id1' + } + }); + const targetMethodSpy = sandbox.spy(notebookService, methodName); + didUninstallExtensionEmitter.fire(extensionIdentifier); + assert.ok(targetMethodSpy.calledWithExactly(extensionIdentifier.identifier, extensionServiceMock.object), `call arguments to ${methodName} should be ${extensionIdentifier.identifier} & ${extensionServiceMock.object}`); + assert.ok(targetMethodSpy.calledOnce, `${methodName} should be called exactly once`); + }); + + test('verify removeContributedProvidersFromCache raises unExpectedError and does not throw', async () => { + const methodName = 'removeContributedProvidersFromCache'; + const onUnexpectedErrorVerifier = (error: any) => { + assert.ok(error instanceof Error, `${onUnexpectedError} must be passed an instance of ${Error}`); + assert.strictEqual((error as Error).message, `Cannot read property 'find' of undefined`, `Error text must be "Cannot read property 'find' of undefined"" when extensionService.getExtensions() returns undefined`); + }; + errorHandler.setUnexpectedErrorHandler(onUnexpectedErrorVerifier); + await notebookService.registrationComplete; + extensionServiceMock.setup(x => x.getExtensions()).returns(() => undefined); + const extensionIdentifier = ({ + identifier: { + id: 'id1' + } + }); + const targetMethodSpy = sandbox.spy(notebookService, methodName); + // the following call will encounter an exception internally with extensionService.getExtensions() returning undefined. + didUninstallExtensionEmitter.fire(extensionIdentifier); + assert.ok(targetMethodSpy.calledWithExactly(extensionIdentifier.identifier, extensionServiceMock.object), `call arguments to ${methodName} should be ${extensionIdentifier.identifier} & ${extensionServiceMock.object}`); + }); + + test('verify that firing of queryManagementService.onHandlerAdded event completes registration', async () => { + await notebookService.registrationComplete; + queryManagementService.onHandlerAddedEmitter.fire(SQL_NOTEBOOK_PROVIDER); + const connectionTypes = queryManagementService.getRegisteredProviders(); + const kernels = notebookService.getStandardKernelsForProvider(SQL_NOTEBOOK_PROVIDER); + for (const kernel of kernels) { + assert.strictEqual(kernel.name, notebookConstants.SQL, `kernel name for standard kernels should be ${notebookConstants.SQL}`); + assert.strictEqual(kernel.displayName, notebookConstants.SQL, `kernel displayName for standard kernels should be ${notebookConstants.SQL}`); + assert.deepStrictEqual(kernel.connectionProviderIds, connectionTypes, `kernel's connectionProviderIds for standard kernels should be connectionType:${JSON.stringify(connectionTypes)}`); + } + }); + + test('verify that notebookService constructor raises error and does not throw when an exception occurs', async () => { + let unexpectedErrorCalled = false; + let unexpectedErrorPromise: Deferred = new Deferred(); + const onUnexpectedErrorVerifier = async (error: any) => { + unexpectedErrorCalled = true; + assert.ok(error instanceof Error, `onUnexpectedError must be passed an instance of Error`); + assert.strictEqual((error as Error).message, `Cannot read property 'getRegisteredProviders' of undefined`, `Error text must be "Cannot read property 'getRegisteredProviders' of undefined" when queryManagementService is undefined`); + unexpectedErrorPromise.resolve(); + }; + errorHandler.setUnexpectedErrorHandler(onUnexpectedErrorVerifier); + // The following call throws an exception internally with queryManagementService parameter being undefined. + new NotebookService(lifecycleService, storageService, extensionService, extensionManagementService, instantiationService, fileService, logService, /* queryManagementService */ undefined, contextService); + await unexpectedErrorPromise; + assert.strictEqual(unexpectedErrorCalled, true, `onUnexpectedError must be have been raised when queryManagementService is undefined when calling NotebookService constructor`); + }); + + test('verify that shutdown executes in response to lifecycleService.onWillShutdown to stop all serverManager objects', async () => { + await notebookService.registrationComplete; + // no managers case + lifecycleService.fireShutdown(); + + // actual shutdown with managers defined + const testProviders = await addManagers({ notebookService, count: 4 }); + lifecycleService.fireShutdown(); + // verify that all managers are in shutdown state. + for (const provider of testProviders) { + assert.strictEqual(provider.manager.serverManager.isStarted, false, `Test Manager:${JSON.stringify(provider.manager)} should be in stopped state after shutdown has been called`); + } + }); + + suite(`serialization tests`, () => { + for (const isTrusted of [true, false, undefined]) { + for (const isModelTrusted of [true, false]) { + if (isTrusted !== undefined && !isModelTrusted) { + continue; // if isTrusted is true or false then we need to do only one case of isModelTrusted value and we are arbitrarily choose true as isModelTrusted does not matter in that case. + } + test(`verify serializeNotebookStateCache serializes correctly when notebook:isTrusted:${isTrusted} && notebookModel:isTrusted:${isModelTrusted}`, async () => { + const notebookUri = URI.parse('uri' + testNo); //Generate a unique notebookUri for each test so that information stored by serializeNotebookStateChange is unique for each test. + const model = new NotebookModelStub(); + const modelMock = TypeMoq.Mock.ofInstance(model); + modelMock.callBase = true; + const notebookEditor = new NotebookEditorStub({ + notebookParams: { + notebookUri: notebookUri, + }, + model: modelMock.object + }); + notebookEditor.model.trustedMode = isModelTrusted; + notebookService.addNotebookEditor(notebookEditor); + const changeType = NotebookChangeType.Saved; + const cellModel = {}; + modelMock.setup(x => x.serializationStateChanged(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns((_changeType: NotebookChangeType, _cellModel?: ICellModel) => { + assert.strictEqual(_changeType, changeType, `changeType value sent to INotebookModel.serializationStateChange should be the one passed to NotebookService.serializeNotebookStateChange`); + assert.strictEqual(_cellModel, cellModel, `cellModel value sent to INotebookModel.serializationStateChange should be the one passed to NotebookService.serializeNotebookStateChange`); + }) + .verifiable(TypeMoq.Times.once()); + await notebookService.serializeNotebookStateChange(notebookUri, changeType, cellModel, isTrusted); + modelMock.verifyAll(); + }); + } + } + }); + + test(`verify isNotebookTrustCached when notebook was not previously trusted`, async () => { + assert.strictEqual(await notebookService.isNotebookTrustCached(URI.parse('untitled:foo'), /* isDirty */ false), true, `untitled notebooks are always trust cached`); + const notebookUri = URI.parse('id-NeverTrusted'); + const notebookEditor = new NotebookEditorStub({ + notebookParams: { + notebookUri: notebookUri, + }, + model: new NotebookModelStub() + }); + notebookService.addNotebookEditor(notebookEditor); + const result = await notebookService.isNotebookTrustCached(notebookUri, true); + assert.strictEqual(result, false, 'isNotebookTrustCached returns false when notebookUri was not previously cached'); + }); + + for (const isTrusted of [true, false]) { + for (const isDirty of [true, false]) { + if (isDirty && !isTrusted) { + continue; + } + test(`verify setTrusted & isNotebookTrustCached calls for isDirty=${isDirty}, isTrusted=${isTrusted}`, async () => { + const notebookUri = setTrustedSetup(notebookService); + await notebookService.setTrusted(notebookUri, isTrusted); + const result = await notebookService.isNotebookTrustCached(notebookUri, isDirty); + if (isDirty) { + assert.strictEqual(result, true, 'isNotebookTrustCached returns true when notebookUri was previously cached'); + } + }); + } + } + + test(`verify that navigateTo call calls navigateToSection on the editor corresponding to the URI passed in`, async () => { + const notebookUri = URI.parse('id1'); + const notebookEditor = new NotebookEditorStub({ + notebookParams: { + notebookUri: notebookUri, + }, + model: new NotebookModelStub() + }); + const sectionId = 'section1'; + const mock = TypeMoq.Mock.ofInstance(notebookEditor); + mock.callBase = true; + notebookService.addNotebookEditor(mock.object); + mock.setup(x => x.navigateToSection(TypeMoq.It.isAnyString())) + .returns((_sectionId: string) => { + assert.strictEqual(_sectionId, sectionId, `sectionId passed into the notebookEditor.navigateToSection must be the one that we passed into notebookService.navigateTo method`); + }) + .verifiable(TypeMoq.Times.once()); + notebookService.navigateTo(notebookUri, sectionId); + mock.verifyAll(); + + }); }); + +function unRegisterProviders(notebookService: NotebookService) { + const notebookRegistry = Registry.as(Extensions.NotebookProviderContribution); + // unregister all builtin providers + for (const providerContribution of notebookRegistry.providers) { + notebookService.unregisterProvider(providerContribution.provider); + } +} + +function setTrustedSetup(notebookService: NotebookService) { + const notebookUri = URI.parse('id1'); + const notebookEditor = new NotebookEditorStub({ + notebookParams: { + notebookUri: notebookUri, + }, + model: new NotebookModelStub() + }); + notebookService.addNotebookEditor(notebookEditor); + return notebookUri; +} + +function createRegisteredProviderWithManager({ notebookService, providerId = 'providerId', testProviderManagers = undefined }: { providerId?: string; notebookService: NotebookService; testProviderManagers?: TestNotebookProvider[] }): TestNotebookProvider { + const provider = new TestNotebookProvider(providerId); + notebookService.registerProvider(providerId, provider); + if (testProviderManagers !== undefined) { + testProviderManagers.push(provider); + } + return provider; +} + +async function addManagers({ notebookService, prefix = 'providerId', uriPrefix = 'id', count = 1 }: { notebookService: NotebookService; prefix?: string; uriPrefix?: string; count?: number; }): Promise { + const testProviderManagers = []; + for (let i: number = 1; i <= count; i++) { + const providerId = `${prefix}${i}`; + createRegisteredProviderWithManager({ providerId, notebookService, testProviderManagers }); + await notebookService.getOrCreateNotebookManager(providerId, URI.parse(`${uriPrefix}${i}`)); + } + return testProviderManagers; +} diff --git a/src/sql/workbench/contrib/notebook/test/nbTestQueryManagementService.ts b/src/sql/workbench/contrib/notebook/test/nbTestQueryManagementService.ts index 68cd3551ee..74886e6585 100644 --- a/src/sql/workbench/contrib/notebook/test/nbTestQueryManagementService.ts +++ b/src/sql/workbench/contrib/notebook/test/nbTestQueryManagementService.ts @@ -7,8 +7,8 @@ import { TestQueryManagementService } from 'sql/workbench/services/query/test/co import { Event, Emitter } from 'vs/base/common/event'; export class NBTestQueryManagementService extends TestQueryManagementService { - readonly _onHandlerAdded = new Emitter(); - onHandlerAdded: Event = this._onHandlerAdded.event; + onHandlerAddedEmitter = new Emitter(); + onHandlerAdded: Event = this.onHandlerAddedEmitter.event; getRegisteredProviders(): string[] { return ['sql']; diff --git a/src/sql/workbench/contrib/notebook/test/stubs.ts b/src/sql/workbench/contrib/notebook/test/stubs.ts index 676b9d9ff3..b4529105fd 100644 --- a/src/sql/workbench/contrib/notebook/test/stubs.ts +++ b/src/sql/workbench/contrib/notebook/test/stubs.ts @@ -106,12 +106,12 @@ export class NotebookModelStub implements INotebookModel { throw new Error('method not implemented.'); } get onProviderIdChange(): vsEvent.Event { - throw new Error('method not impelemented.'); + throw new Error('method not implemented.'); } toJSON(): nb.INotebookContents { throw new Error('Method not implemented.'); } - serializationStateChanged(changeType: NotebookChangeType): void { + serializationStateChanged(changeType: NotebookChangeType, cell?: ICellModel): void { throw new Error('Method not implemented.'); } get onActiveCellChanged(): vsEvent.Event { diff --git a/src/sql/workbench/contrib/notebook/test/testCommon.ts b/src/sql/workbench/contrib/notebook/test/testCommon.ts index bff9560662..03933b0768 100644 --- a/src/sql/workbench/contrib/notebook/test/testCommon.ts +++ b/src/sql/workbench/contrib/notebook/test/testCommon.ts @@ -6,6 +6,7 @@ import { QueryTextEditor } from 'sql/workbench/browser/modelComponents/queryTextEditor'; import * as stubs from 'sql/workbench/contrib/notebook/test/stubs'; import { INotebookModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; +import { INotebookParams } from 'sql/workbench/services/notebook/browser/notebookService'; import * as dom from 'vs/base/browser/dom'; import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; @@ -18,20 +19,21 @@ import { TestStorageService } from 'vs/workbench/test/common/workbenchTestServic // Leave both undefined when you want the underlying object(s) to have an undefined editor. export class NotebookEditorStub extends stubs.NotebookEditorStub { cellEditors: CellEditorProviderStub[]; - private _model: INotebookModel | undefined; + model: INotebookModel | undefined; - get model(): INotebookModel | undefined { - return this._model; + get id(): string { + return this.notebookParams?.notebookUri?.toString(); } get modelReady(): Promise { - return Promise.resolve(this._model); + return Promise.resolve(this.model); } // Normally one needs to provide either the editor or the instantiationService as the constructor parameter - constructor({ cellGuid, instantiationService, editor, model }: { cellGuid?: string; instantiationService?: IInstantiationService; editor?: QueryTextEditor; model?: INotebookModel } = {}) { + constructor({ cellGuid, instantiationService, editor, model, notebookParams }: { cellGuid?: string; instantiationService?: IInstantiationService; editor?: QueryTextEditor; model?: INotebookModel, notebookParams?: INotebookParams } = {}) { super(); - this._model = model; + this.model = model; + this.notebookParams = notebookParams; this.cellEditors = [new CellEditorProviderStub({ cellGuid: cellGuid, instantiationService: instantiationService, editor: editor })]; } } diff --git a/src/sql/workbench/services/notebook/browser/notebookService.ts b/src/sql/workbench/services/notebook/browser/notebookService.ts index 46800ac684..dc9d1f868d 100644 --- a/src/sql/workbench/services/notebook/browser/notebookService.ts +++ b/src/sql/workbench/services/notebook/browser/notebookService.ts @@ -124,7 +124,7 @@ export interface INotebookService { navigateTo(notebookUri: URI, sectionId: string): void; /** - * Sets the trusted mode for the sepcified notebook. + * Sets the trusted mode for the specified notebook. * @param notebookUri URI of the notebook to navigate to * @param isTrusted True if notebook is to be set to trusted, false otherwise. */ diff --git a/src/sql/workbench/services/notebook/browser/notebookServiceImpl.ts b/src/sql/workbench/services/notebook/browser/notebookServiceImpl.ts index 9bdac99513..fc2a570171 100644 --- a/src/sql/workbench/services/notebook/browser/notebookServiceImpl.ts +++ b/src/sql/workbench/services/notebook/browser/notebookServiceImpl.ts @@ -20,7 +20,7 @@ import { Memento } from 'vs/workbench/common/memento'; import { IStorageService, StorageScope } from 'vs/platform/storage/common/storage'; import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; import { IExtensionManagementService, IExtensionIdentifier } from 'vs/platform/extensionManagement/common/extensionManagement'; -import { Disposable, IDisposable } from 'vs/base/common/lifecycle'; +import { Disposable } from 'vs/base/common/lifecycle'; import { Deferred } from 'sql/base/common/promise'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IQueryManagementService } from 'sql/workbench/services/query/common/queryManagement'; @@ -33,7 +33,6 @@ import { Schemas } from 'vs/base/common/network'; import { ILogService } from 'vs/platform/log/common/log'; import { toErrorMessage } from 'vs/base/common/errorMessage'; import { NotebookChangeType } from 'sql/workbench/services/notebook/common/contracts'; -import { find, firstIndex } from 'vs/base/common/arrays'; import { onUnexpectedError } from 'vs/base/common/errors'; import { notebookConstants } from 'sql/workbench/services/notebook/browser/interfaces'; import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; @@ -47,7 +46,7 @@ interface NotebookProviderCache { [id: string]: NotebookProviderProperties; } -interface NotebookProvidersMemento { +export interface NotebookProvidersMemento { notebookProviderCache: NotebookProviderCache; } @@ -59,13 +58,13 @@ interface TrustedNotebookCache { [uri: string]: TrustedNotebookMetadata; } -interface TrustedNotebooksMemento { +export interface TrustedNotebooksMemento { trustedNotebooksCache: TrustedNotebookCache; } const notebookRegistry = Registry.as(Extensions.NotebookProviderContribution); -class ProviderDescriptor { +export class ProviderDescriptor { private _instanceReady = new Deferred(); constructor(private _instance?: INotebookProvider) { if (_instance) { @@ -86,6 +85,10 @@ class ProviderDescriptor { } } +export const NotebookUriNotDefined = localize('notebookUriNotDefined', "No URI was passed when creating a notebook manager"); +export const NotebookServiceNoProviderRegistered = localize('notebookServiceNoProvider', "Notebook provider does not exist"); +export const FailToSaveTrustState = 'Failed to save trust state to cache'; +export const TrustedNotebooksMementoId = 'notebooks.trusted'; export class NotebookService extends Disposable implements INotebookService { _serviceBrand: undefined; @@ -97,14 +100,12 @@ export class NotebookService extends Disposable implements INotebookService { private _managersMap: Map = new Map(); private _onNotebookEditorAdd = new Emitter(); private _onNotebookEditorRemove = new Emitter(); - private _onCellChanged = new Emitter(); private _onNotebookEditorRename = new Emitter(); private _editors = new Map(); private _fileToProviders = new Map(); private _providerToStandardKernels = new Map(); private _registrationComplete = new Deferred(); private _isRegistrationComplete = false; - private _themeParticipant: IDisposable; private _trustedCacheQueue: URI[] = []; private _unTrustedCacheQueue: URI[] = []; @@ -121,10 +122,13 @@ export class NotebookService extends Disposable implements INotebookService { ) { super(); this._providersMemento = new Memento('notebookProviders', this._storageService); - this._trustedNotebooksMemento = new Memento('notebooks.trusted', this._storageService); - + this._trustedNotebooksMemento = new Memento(TrustedNotebooksMementoId, this._storageService); + if (this._storageService !== undefined && this.providersMemento.notebookProviderCache === undefined) { + this.providersMemento.notebookProviderCache = {}; + } this._register(notebookRegistry.onNewRegistration(this.updateRegisteredProviders, this)); this.registerBuiltInProvider(); + // If a provider has been already registered, the onNewRegistration event will not have a listener attached yet // So, explicitly updating registered providers here. if (notebookRegistry.providers.length > 0) { @@ -145,13 +149,13 @@ export class NotebookService extends Disposable implements INotebookService { this.updateSQLRegistrationWithConnectionProviders(); } - this._register(this._queryManagementService.onHandlerAdded((queryType) => { + this._register(this._queryManagementService.onHandlerAdded((_queryType) => { this.updateSQLRegistrationWithConnectionProviders(); })); }).catch(err => onUnexpectedError(err)); } if (extensionManagementService) { - this._register(extensionManagementService.onDidUninstallExtension(({ identifier }) => this.removeContributedProvidersFromCache(identifier, this._extensionService))); + this._register(extensionManagementService.onDidUninstallExtension(async ({ identifier }) => await this.removeContributedProvidersFromCache(identifier, this._extensionService))); } lifecycleService.onWillShutdown(() => this.shutdown()); @@ -159,18 +163,15 @@ export class NotebookService extends Disposable implements INotebookService { public dispose(): void { super.dispose(); - if (this._themeParticipant) { - this._themeParticipant.dispose(); - } } private updateSQLRegistrationWithConnectionProviders() { // Update the SQL extension - let sqlNotebookProvider = this._providerToStandardKernels.get(notebookConstants.SQL); - if (sqlNotebookProvider) { + let sqlNotebookKernels = this._providerToStandardKernels.get(notebookConstants.SQL); + if (sqlNotebookKernels) { let sqlConnectionTypes = this._queryManagementService.getRegisteredProviders(); - let provider = find(sqlNotebookProvider, p => p.name === notebookConstants.SQL); - if (provider) { + let kernel = sqlNotebookKernels.find(p => p.name === notebookConstants.SQL); + if (kernel) { this._providerToStandardKernels.set(notebookConstants.SQL, [{ name: notebookConstants.SQL, displayName: notebookConstants.SQL, @@ -295,14 +296,14 @@ export class NotebookService extends Disposable implements INotebookService { async getOrCreateNotebookManager(providerId: string, uri: URI): Promise { if (!uri) { - throw new Error(localize('notebookUriNotDefined', "No URI was passed when creating a notebook manager")); + throw new Error(NotebookUriNotDefined); } let uriString = uri.toString(); let managers: INotebookManager[] = this._managersMap.get(uriString); // If manager already exists for a given notebook, return it if (managers) { - let index = firstIndex(managers, m => m.providerId === providerId); - if (index && index >= 0) { + let index = managers.findIndex(m => m.providerId === providerId); + if (index >= 0) { return managers[index]; } } @@ -317,12 +318,10 @@ export class NotebookService extends Disposable implements INotebookService { get onNotebookEditorAdd(): Event { return this._onNotebookEditorAdd.event; } + get onNotebookEditorRemove(): Event { return this._onNotebookEditorRemove.event; } - get onCellChanged(): Event { - return this._onCellChanged.event; - } get onNotebookEditorRename(): Event { return this._onNotebookEditorRename.event; @@ -341,18 +340,12 @@ export class NotebookService extends Disposable implements INotebookService { this.sendNotebookCloseToProvider(editor); } - listNotebookEditors(): INotebookEditor[] { - let editors = []; - this._editors.forEach(e => editors.push(e)); - return editors; - } - findNotebookEditor(notebookUri: URI): INotebookEditor | undefined { if (!notebookUri) { return undefined; } let uriString = notebookUri.toString(); - let editor = find(this.listNotebookEditors(), n => n.id === uriString); + let editor = this.listNotebookEditors().find(n => n.id === uriString); return editor; } @@ -360,12 +353,18 @@ export class NotebookService extends Disposable implements INotebookService { let oldUriKey = oldUri.toString(); if (this._editors.has(oldUriKey)) { this._editors.delete(oldUriKey); - currentEditor.notebookParams.notebookUri = newUri; - this._editors.set(newUri.toString(), currentEditor); + currentEditor.notebookParams.notebookUri = newUri; //currentEditor.id gets this value as a string + this._editors.set(currentEditor.id, currentEditor); this._onNotebookEditorRename.fire(currentEditor); } } + listNotebookEditors(): INotebookEditor[] { + let editors = []; + this._editors.forEach(e => editors.push(e)); + return editors; + } + get languageMagics(): ILanguageMagic[] { return notebookRegistry.languageMagics; } @@ -375,11 +374,11 @@ export class NotebookService extends Disposable implements INotebookService { private sendNotebookCloseToProvider(editor: INotebookEditor): void { let notebookUri = editor.notebookParams.notebookUri; let uriString = notebookUri.toString(); - let manager = this._managersMap.get(uriString); - if (manager) { + let managers = this._managersMap.get(uriString); + if (managers) { // As we have a manager, we can assume provider is ready this._managersMap.delete(uriString); - manager.forEach(m => { + managers.forEach(m => { let provider = this._providers.get(m.providerId); provider.instance.handleNotebookClosed(notebookUri); }); @@ -405,7 +404,7 @@ export class NotebookService extends Disposable implements INotebookService { } catch (error) { this._logService.error(error); } - instance = await this.waitOnProviderAvailability(providerDescriptor); + instance = await this.waitOnProviderAvailability(providerDescriptor, timeout); } else { instance = providerDescriptor.instance; } @@ -419,14 +418,14 @@ export class NotebookService extends Disposable implements INotebookService { // Should never happen, but if default wasn't registered we should throw if (!instance) { - throw new Error(localize('notebookServiceNoProvider', "Notebook provider does not exist")); + throw new Error(NotebookServiceNoProviderRegistered); } return instance; } private waitOnProviderAvailability(providerDescriptor: ProviderDescriptor, timeout?: number): Promise { // Wait up to 30 seconds for the provider to be registered - timeout = timeout || 30000; + timeout = timeout ?? 30000; let promises: Promise[] = [ providerDescriptor.instanceReady, new Promise((resolve, reject) => setTimeout(() => resolve(), timeout)) @@ -437,7 +436,7 @@ export class NotebookService extends Disposable implements INotebookService { //Returns an instantiation of RenderMimeRegistry class getMimeRegistry(): RenderMimeRegistry { if (!this._mimeRegistry) { - return new RenderMimeRegistry({ + this._mimeRegistry = new RenderMimeRegistry({ initialFactories: standardRendererFactories }); } @@ -477,17 +476,20 @@ export class NotebookService extends Disposable implements INotebookService { }); } - private removeContributedProvidersFromCache(identifier: IExtensionIdentifier, extensionService: IExtensionService): void { + protected async removeContributedProvidersFromCache(identifier: IExtensionIdentifier, extensionService: IExtensionService): Promise { const notebookProvider = 'notebookProvider'; - extensionService.getExtensions().then(i => { - let extension = find(i, c => c.identifier.value.toLowerCase() === identifier.id.toLowerCase()); - if (extension && extension.contributes - && extension.contributes[notebookProvider] - && extension.contributes[notebookProvider].providerId) { - let id = extension.contributes[notebookProvider].providerId; + try { + const extensionDescriptions = await extensionService.getExtensions(); + let extensionDescription = extensionDescriptions.find(c => c.identifier.value.toLowerCase() === identifier.id.toLowerCase()); + if (extensionDescription && extensionDescription.contributes + && extensionDescription.contributes[notebookProvider] //'notebookProvider' isn't a field defined on IExtensionContributions so contributes[notebookProvider] is 'any'. TODO: Code cleanup + && extensionDescription.contributes[notebookProvider].providerId) { + let id = extensionDescription.contributes[notebookProvider].providerId; delete this.providersMemento.notebookProviderCache[id]; } - }).catch(err => onUnexpectedError(err)); + } catch (err) { + onUnexpectedError(err); + } } async isNotebookTrustCached(notebookUri: URI, isDirty: boolean): Promise { @@ -532,7 +534,7 @@ export class NotebookService extends Disposable implements INotebookService { // 3. Not already saving (e.g. isn't in the queue to be cached) // 4. Notebook is trusted. Don't need to save state of untrusted notebooks let notebookUriString = notebookUri.toString(); - if (changeType === NotebookChangeType.Saved && firstIndex(this._trustedCacheQueue, uri => uri.toString() === notebookUriString) < 0) { + if (changeType === NotebookChangeType.Saved && this._trustedCacheQueue.findIndex(uri => uri.toString() === notebookUriString) < 0) { if (isTrusted) { this._trustedCacheQueue.push(notebookUri); await this.updateTrustedCache(); @@ -541,7 +543,7 @@ export class NotebookService extends Disposable implements INotebookService { await this.updateTrustedCache(); } else { // Only save as trusted if the associated notebook model is trusted - let notebook = find(this.listNotebookEditors(), n => n.id === notebookUriString); + let notebook = this.listNotebookEditors().find(n => n.id === notebookUriString); if (notebook && notebook.model) { if (notebook.model.trustedMode) { this._trustedCacheQueue.push(notebookUri); @@ -590,7 +592,7 @@ export class NotebookService extends Disposable implements INotebookService { let items = this._unTrustedCacheQueue; this._unTrustedCacheQueue = []; let trustedCache = this.trustedNotebooksMemento.trustedNotebooksCache; - //Remove the trusted intry from the cache + //Remove the trusted entry from the cache for (let i = 0; i < items.length; i++) { if (trustedCache[items[i].toString()]) { trustedCache[items[i].toString()] = null; @@ -600,7 +602,7 @@ export class NotebookService extends Disposable implements INotebookService { } } catch (err) { if (this._logService) { - this._logService.trace(`Failed to save trust state to cache: ${toErrorMessage(err)}`); + this._logService.trace(`${FailToSaveTrustState}: ${toErrorMessage(err)}`); } } }