diff --git a/extensions/notebook/src/intellisense/completionItemProvider.ts b/extensions/notebook/src/intellisense/completionItemProvider.ts index cd223983f0..af2cb3050e 100644 --- a/extensions/notebook/src/intellisense/completionItemProvider.ts +++ b/extensions/notebook/src/intellisense/completionItemProvider.ts @@ -7,14 +7,14 @@ import { nb } from 'azdata'; import * as vscode from 'vscode'; import { charCountToJsCountDiff, jsIndexToCharIndex } from './text'; -import { JupyterNotebookProvider } from '../jupyter/jupyterNotebookProvider'; +import { JupyterExecuteProvider } from '../jupyter/jupyterExecuteProvider'; import { JupyterSessionManager } from '../jupyter/jupyterSessionManager'; const timeoutMilliseconds = 3000; export class NotebookCompletionItemProvider implements vscode.CompletionItemProvider { - constructor(private _notebookProvider: JupyterNotebookProvider) { + constructor(private _notebookProvider: JupyterExecuteProvider) { } public provideCompletionItems(document: vscode.TextDocument, position: vscode.Position, token: vscode.CancellationToken, context: vscode.CompletionContext) @@ -45,9 +45,9 @@ export class NotebookCompletionItemProvider implements vscode.CompletionItemProv private async tryFindKernelForDocument(document: vscode.TextDocument, info: INewIntellisenseInfo): Promise { try { - let notebookManager = await this._notebookProvider.getNotebookManager(document.uri); - if (notebookManager) { - let sessionManager: JupyterSessionManager = (notebookManager.sessionManager); + let executeManager = await this._notebookProvider.getExecuteManager(document.uri); + if (executeManager) { + let sessionManager: JupyterSessionManager = (executeManager.sessionManager); let sessions = sessionManager.listRunning(); if (sessions && sessions.length > 0) { let session = sessions.find(session => session.path === info.notebook.uri.path); diff --git a/extensions/notebook/src/jupyter/jupyterController.ts b/extensions/notebook/src/jupyter/jupyterController.ts index 512ee254db..a809733de8 100644 --- a/extensions/notebook/src/jupyter/jupyterController.ts +++ b/extensions/notebook/src/jupyter/jupyterController.ts @@ -19,7 +19,6 @@ import { IPrompter, IQuestion, QuestionTypes } from '../prompts/question'; import { AppContext } from '../common/appContext'; import { LocalJupyterServerManager, ServerInstanceFactory } from './jupyterServerManager'; import { NotebookCompletionItemProvider } from '../intellisense/completionItemProvider'; -import { JupyterNotebookProvider } from './jupyterNotebookProvider'; import { ConfigurePythonWizard } from '../dialog/configurePython/configurePythonWizard'; import CodeAdapter from '../prompts/adapter'; import { ManagePackagesDialog } from '../dialog/managePackages/managePackagesDialog'; @@ -28,6 +27,7 @@ import { LocalPipPackageManageProvider } from './localPipPackageManageProvider'; import { LocalCondaPackageManageProvider } from './localCondaPackageManageProvider'; import { ManagePackagesDialogModel, ManagePackageDialogOptions } from '../dialog/managePackages/managePackagesDialogModel'; import { PyPiClient } from './pypiClient'; +import { JupyterExecuteProvider } from './jupyterExecuteProvider'; let untitledCounter = 0; @@ -37,7 +37,7 @@ export class JupyterController { private _packageManageProviders = new Map(); private prompter: IPrompter; - private _notebookProvider: JupyterNotebookProvider; + private _executeProvider: JupyterExecuteProvider; constructor(private appContext: AppContext) { this.prompter = new CodeAdapter(); @@ -47,8 +47,8 @@ export class JupyterController { return this.appContext && this.appContext.extensionContext; } - public get notebookProvider(): JupyterNotebookProvider { - return this._notebookProvider; + public get executeProvider(): JupyterExecuteProvider { + return this._executeProvider; } // PUBLIC METHODS ////////////////////////////////////////////////////// @@ -79,21 +79,19 @@ export class JupyterController { let supportedFileFilter: vscode.DocumentFilter[] = [ { scheme: 'untitled', language: '*' } ]; - this.registerNotebookProvider(); - this.extensionContext.subscriptions.push(vscode.languages.registerCompletionItemProvider(supportedFileFilter, new NotebookCompletionItemProvider(this._notebookProvider), '.')); - this.registerDefaultPackageManageProviders(); - return true; - } - - private registerNotebookProvider(): void { - this._notebookProvider = new JupyterNotebookProvider((documentUri: vscode.Uri) => new LocalJupyterServerManager({ + this._executeProvider = new JupyterExecuteProvider((documentUri: vscode.Uri) => new LocalJupyterServerManager({ documentPath: documentUri.fsPath, jupyterInstallation: this._jupyterInstallation, extensionContext: this.extensionContext, factory: this._serverInstanceFactory })); - azdata.nb.registerNotebookProvider(this._notebookProvider); + azdata.nb.registerExecuteProvider(this._executeProvider); + + this.extensionContext.subscriptions.push(vscode.languages.registerCompletionItemProvider(supportedFileFilter, new NotebookCompletionItemProvider(this._executeProvider), '.')); + + this.registerDefaultPackageManageProviders(); + return true; } private saveProfileAndCreateNotebook(profile: azdata.IConnectionProfile): Promise { diff --git a/extensions/notebook/src/jupyter/jupyterNotebookManager.ts b/extensions/notebook/src/jupyter/jupyterExecuteManager.ts similarity index 92% rename from extensions/notebook/src/jupyter/jupyterNotebookManager.ts rename to extensions/notebook/src/jupyter/jupyterExecuteManager.ts index 1043b853eb..001be1c66d 100644 --- a/extensions/notebook/src/jupyter/jupyterNotebookManager.ts +++ b/extensions/notebook/src/jupyter/jupyterExecuteManager.ts @@ -10,7 +10,7 @@ import { ServerConnection, SessionManager } from '@jupyterlab/services'; import { JupyterSessionManager } from './jupyterSessionManager'; import { LocalJupyterServerManager } from './jupyterServerManager'; -export class JupyterNotebookManager implements nb.NotebookManager, vscode.Disposable { +export class JupyterExecuteManager implements nb.ExecuteManager, vscode.Disposable { protected _serverSettings: ServerConnection.ISettings; private _sessionManager: JupyterSessionManager; @@ -21,9 +21,6 @@ export class JupyterNotebookManager implements nb.NotebookManager, vscode.Dispos this._sessionManager.installation = this._serverManager.instanceOptions.install; }); } - public get contentManager(): nb.ContentManager { - return undefined; - } public get sessionManager(): nb.SessionManager { return this._sessionManager; diff --git a/extensions/notebook/src/jupyter/jupyterNotebookProvider.ts b/extensions/notebook/src/jupyter/jupyterExecuteProvider.ts similarity index 81% rename from extensions/notebook/src/jupyter/jupyterNotebookProvider.ts rename to extensions/notebook/src/jupyter/jupyterExecuteProvider.ts index e615578254..9720580752 100644 --- a/extensions/notebook/src/jupyter/jupyterNotebookProvider.ts +++ b/extensions/notebook/src/jupyter/jupyterExecuteProvider.ts @@ -11,41 +11,41 @@ const localize = nls.loadMessageBundle(); import * as constants from '../common/constants'; import * as utils from '../common/utils'; -import { JupyterNotebookManager } from './jupyterNotebookManager'; +import { JupyterExecuteManager } from './jupyterExecuteManager'; import { LocalJupyterServerManager } from './jupyterServerManager'; import { JupyterSessionManager } from './jupyterSessionManager'; export type ServerManagerFactory = (documentUri: vscode.Uri) => LocalJupyterServerManager; -export class JupyterNotebookProvider implements nb.NotebookProvider { +export class JupyterExecuteProvider implements nb.NotebookExecuteProvider { readonly providerId: string = constants.jupyterNotebookProviderId; - private managerTracker = new Map(); + private executeManagerTracker = new Map(); constructor(private createServerManager: ServerManagerFactory) { } - public getNotebookManager(notebookUri: vscode.Uri): Thenable { + public getExecuteManager(notebookUri: vscode.Uri): Thenable { if (!notebookUri) { return Promise.reject(localize('errNotebookUriMissing', "A notebook path is required")); } - return Promise.resolve(this.doGetNotebookManager(notebookUri)); + return Promise.resolve(this.doGetExecuteManager(notebookUri)); } - public get notebookManagerCount(): number { - return this.managerTracker.size; + public get executeManagerCount(): number { + return this.executeManagerTracker.size; } - private doGetNotebookManager(notebookUri: vscode.Uri): nb.NotebookManager { + private doGetExecuteManager(notebookUri: vscode.Uri): nb.ExecuteManager { let baseFolder = this.transformToBaseFolder(notebookUri?.fsPath?.toString()); - let manager = this.managerTracker.get(baseFolder); + let manager = this.executeManagerTracker.get(baseFolder); if (!manager) { let baseFolderUri = vscode.Uri.file(baseFolder); if (!baseFolderUri) { baseFolderUri = notebookUri; } let serverManager = this.createServerManager(baseFolderUri); - manager = new JupyterNotebookManager(serverManager); - this.managerTracker.set(baseFolder, manager); + manager = new JupyterExecuteManager(serverManager); + this.executeManagerTracker.set(baseFolder, manager); } return manager; } @@ -56,7 +56,7 @@ export class JupyterNotebookProvider implements nb.NotebookProvider { return; } let baseFolder = this.transformToBaseFolder(notebookUri.fsPath.toString()); - let manager = this.managerTracker.get(baseFolder); + let manager = this.executeManagerTracker.get(baseFolder); if (manager) { let sessionManager = (manager.sessionManager as JupyterSessionManager); let session = sessionManager.listRunning().find(e => e.path === notebookUri.fsPath); @@ -70,7 +70,7 @@ export class JupyterNotebookProvider implements nb.NotebookProvider { const timeoutInMs = timeoutInMinutes * 60 * 1000; setTimeout(() => { if (sessionManager.listRunning().length === 0) { - this.managerTracker.delete(baseFolder); + this.executeManagerTracker.delete(baseFolder); manager.dispose(); } }, timeoutInMs); @@ -79,10 +79,6 @@ export class JupyterNotebookProvider implements nb.NotebookProvider { } } - public get standardKernels(): nb.IStandardKernel[] { - return []; - } - private transformToBaseFolder(notebookPath: string): string { let parsedPath = path.parse(notebookPath); let userHome = utils.getUserHome(); diff --git a/extensions/notebook/src/jupyter/jupyterSessionManager.ts b/extensions/notebook/src/jupyter/jupyterSessionManager.ts index 1ea09d2be8..c9b0a2a164 100644 --- a/extensions/notebook/src/jupyter/jupyterSessionManager.ts +++ b/extensions/notebook/src/jupyter/jupyterSessionManager.ts @@ -58,7 +58,7 @@ const configBase = { } }; -export class JupyterSessionManager implements nb.SessionManager { +export class JupyterSessionManager implements nb.SessionManager, vscode.Disposable { private _ready: Deferred; private _isReady: boolean; private _sessionManager: Session.IManager; diff --git a/extensions/notebook/src/jupyter/remoteContentManager.ts b/extensions/notebook/src/jupyter/remoteContentManager.ts deleted file mode 100644 index b271cabe7d..0000000000 --- a/extensions/notebook/src/jupyter/remoteContentManager.ts +++ /dev/null @@ -1,41 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the Source EULA. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import { nb } from 'azdata'; -import * as vscode from 'vscode'; -import { Contents } from '@jupyterlab/services'; - -export class RemoteContentManager implements nb.ContentManager { - - constructor(private contents: Contents.IManager) { - } - - public getNotebookContents(notebookUri: vscode.Uri): Thenable { - return this.getNotebookContentsAsync(notebookUri.fsPath); - } - - private async getNotebookContentsAsync(path: string): Promise { - if (!path) { - return undefined; - } - // Note: intentionally letting caller handle exceptions - let contentsModel = await this.contents.get(path); - if (!contentsModel) { - return undefined; - } - return contentsModel.content; - } - - public async save(notebookUri: vscode.Uri, notebook: nb.INotebookContents): Promise { - let path = notebookUri.fsPath; - await this.contents.save(path, { - path: path, - content: notebook, - type: 'notebook', - format: 'json' - }); - return notebook; - } -} diff --git a/extensions/notebook/src/test/model/completionItemProvider.test.ts b/extensions/notebook/src/test/model/completionItemProvider.test.ts index daa2e7ba17..93909756f9 100644 --- a/extensions/notebook/src/test/model/completionItemProvider.test.ts +++ b/extensions/notebook/src/test/model/completionItemProvider.test.ts @@ -9,19 +9,19 @@ import * as vscode from 'vscode'; import * as TypeMoq from 'typemoq'; import { NotebookCompletionItemProvider } from '../../intellisense/completionItemProvider'; -import { JupyterNotebookProvider } from '../../jupyter/jupyterNotebookProvider'; import { NotebookUtils } from '../../common/notebookUtils'; -import { JupyterNotebookManager } from '../../jupyter/jupyterNotebookManager'; +import { JupyterExecuteManager } from '../../jupyter/jupyterExecuteManager'; import { JupyterSessionManager, JupyterSession } from '../../jupyter/jupyterSessionManager'; import { LocalJupyterServerManager } from '../../jupyter/jupyterServerManager'; import { TestKernel } from '../common'; import { sleep } from '../common/testUtils'; +import { JupyterExecuteProvider } from '../../jupyter/jupyterExecuteProvider'; describe('Completion Item Provider', function () { let completionItemProvider: NotebookCompletionItemProvider; - let notebookProviderMock: TypeMoq.IMock; + let executeProviderMock: TypeMoq.IMock; let notebookUtils: NotebookUtils; - let notebookManager: JupyterNotebookManager; + let notebookManager: JupyterExecuteManager; let mockSessionManager: TypeMoq.IMock; let mockServerManager: TypeMoq.IMock; let mockJupyterSession: TypeMoq.IMock; @@ -45,10 +45,10 @@ describe('Completion Item Provider', function () { mockSessionManager = TypeMoq.Mock.ofType(); mockJupyterSession = TypeMoq.Mock.ofType(); kernel = new TestKernel(true, true); - notebookManager = new JupyterNotebookManager(mockServerManager.object, mockSessionManager.object); - notebookProviderMock = TypeMoq.Mock.ofType(); - notebookProviderMock.setup(n => n.getNotebookManager(TypeMoq.It.isAny())).returns(() => Promise.resolve(notebookManager)); - completionItemProvider = new NotebookCompletionItemProvider(notebookProviderMock.object); + notebookManager = new JupyterExecuteManager(mockServerManager.object, mockSessionManager.object); + executeProviderMock = TypeMoq.Mock.ofType(); + executeProviderMock.setup(n => n.getExecuteManager(TypeMoq.It.isAny())).returns(() => Promise.resolve(notebookManager)); + completionItemProvider = new NotebookCompletionItemProvider(executeProviderMock.object); }); it('should not return items when undefined passed in for every parameter', async () => { diff --git a/extensions/notebook/src/test/model/contentManagers.test.ts b/extensions/notebook/src/test/model/contentManagers.test.ts deleted file mode 100644 index dc2da1824e..0000000000 --- a/extensions/notebook/src/test/model/contentManagers.test.ts +++ /dev/null @@ -1,94 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the Source EULA. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import * as vscode from 'vscode'; -import * as should from 'should'; -import * as TypeMoq from 'typemoq'; -import * as path from 'path'; -import { ContentsManager, Contents } from '@jupyterlab/services'; -import { nb } from 'azdata'; -import 'mocha'; - -import { INotebook, CellTypes } from '../../contracts/content'; -import { RemoteContentManager } from '../../jupyter/remoteContentManager'; -import * as testUtils from '../common/testUtils'; - -let expectedNotebookContent: INotebook = { - cells: [{ - cell_type: CellTypes.Code, - source: 'insert into t1 values (c1, c2)', - metadata: { language: 'python' }, - execution_count: 1 - }], - metadata: { - kernelspec: { - name: 'mssql', - language: 'sql' - } - }, - nbformat: 5, - nbformat_minor: 0 -}; - -function verifyMatchesExpectedNotebook(notebook: nb.INotebookContents): void { - should(notebook.cells).have.length(1, 'Expected 1 cell'); - should(notebook.cells[0].cell_type).equal(CellTypes.Code); - should(notebook.cells[0].source).equal(expectedNotebookContent.cells[0].source); - should(notebook.metadata.kernelspec.name).equal(expectedNotebookContent.metadata.kernelspec.name); - should(notebook.nbformat).equal(expectedNotebookContent.nbformat); - should(notebook.nbformat_minor).equal(expectedNotebookContent.nbformat_minor); -} - -describe('Remote Content Manager', function (): void { - let mockJupyterManager = TypeMoq.Mock.ofType(ContentsManager); - let contentManager = new RemoteContentManager(mockJupyterManager.object); - - // TODO re-enable when we bring in usage of remote content managers / binders - // it('Should return undefined if path is undefined', async function(): Promise { - // let content = await contentManager.getNotebookContents(undefined); - // should(content).be.undefined(); - // // tslint:disable-next-line:no-null-keyword - // content = await contentManager.getNotebookContents(null); - // should(content).be.undefined(); - // content = await contentManager.getNotebookContents(vscode.Uri.file('')); - // should(content).be.undefined(); - // }); - - it('Should throw if API call throws', async function (): Promise { - let exception = new Error('Path was wrong'); - mockJupyterManager.setup(c => c.get(TypeMoq.It.isAny(), TypeMoq.It.isAny())).throws(exception); - await testUtils.assertThrowsAsync(async () => await contentManager.getNotebookContents(vscode.Uri.file('/path/doesnot/exist.ipynb')), undefined); - }); - it('Should return notebook contents parsed as INotebook when valid notebook file parsed', async function (): Promise { - // Given a valid request to the notebook server - let remotePath = '/remote/path/that/exists.ipynb'; - let contentsModel: Contents.IModel = { - name: path.basename(remotePath), - content: expectedNotebookContent, - path: remotePath, - type: 'notebook', - writable: false, - created: undefined, - last_modified: undefined, - mimetype: 'json', - format: 'json' - }; - mockJupyterManager.setup(c => c.get(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(contentsModel)); - // when I read the content - let notebook = await contentManager.getNotebookContents(vscode.Uri.file(remotePath)); - // then I expect notebook format to match - verifyMatchesExpectedNotebook(notebook); - }); - - it('Should return undefined if service does not return anything', async function (): Promise { - // Given a valid request to the notebook server - let remotePath = '/remote/path/that/does/not/exist.ipynb'; - mockJupyterManager.setup(c => c.get(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(undefined)); - // when I read the content - let notebook = await contentManager.getNotebookContents(vscode.Uri.file(remotePath)); - // then I expect notebook format to match - should(notebook).be.undefined(); - }); -}); diff --git a/extensions/notebook/src/test/model/notebookManager.test.ts b/extensions/notebook/src/test/model/executeManager.test.ts similarity index 73% rename from extensions/notebook/src/test/model/notebookManager.test.ts rename to extensions/notebook/src/test/model/executeManager.test.ts index 04b6566c59..d40da88a9b 100644 --- a/extensions/notebook/src/test/model/notebookManager.test.ts +++ b/extensions/notebook/src/test/model/executeManager.test.ts @@ -14,10 +14,10 @@ import { JupyterServerInstallation } from '../../jupyter/jupyterServerInstallati import { Deferred } from '../../common/promise'; import { MockExtensionContext } from '../common/stubs'; import { JupyterSessionManager } from '../../jupyter/jupyterSessionManager'; -import { JupyterNotebookManager } from '../../jupyter/jupyterNotebookManager'; +import { JupyterExecuteManager } from '../../jupyter/jupyterExecuteManager'; import { initInstallAndInstance } from './serverManager.test'; -describe('Jupyter Notebook Manager', function (): void { +describe('Jupyter Execute Manager', function (): void { const pythonKernelSpec: azdata.nb.IKernelSpec = { name: 'python3', display_name: 'Python 3' @@ -25,7 +25,7 @@ describe('Jupyter Notebook Manager', function (): void { let expectedPath = 'my/notebook.ipynb'; let serverManager: LocalJupyterServerManager; let sessionManager: JupyterSessionManager; - let notebookManager: JupyterNotebookManager; + let executeManager: JupyterExecuteManager; let deferredInstall: Deferred; let mockExtensionContext: MockExtensionContext; let mockFactory: TypeMoq.IMock; @@ -48,48 +48,37 @@ describe('Jupyter Notebook Manager', function (): void { serverManager = new LocalJupyterServerManager(serverManagerOptions); sessionManager = new JupyterSessionManager(); - notebookManager = new JupyterNotebookManager(serverManager, sessionManager); + executeManager = new JupyterExecuteManager(serverManager, sessionManager); }); it('Server settings should be set', async function (): Promise { - should(notebookManager.serverSettings).be.undefined(); + should(executeManager.serverSettings).be.undefined(); let expectedUri = vscode.Uri.parse('http://localhost:1234?token=abcdefghijk'); initInstallAndInstance(expectedUri, mockFactory); deferredInstall.resolve(); // When I start the server await serverManager.startServer(pythonKernelSpec); - should(notebookManager.serverSettings.baseUrl).equal('http://localhost:1234', 'Server settings did not match expected value'); + should(executeManager.serverSettings.baseUrl).equal('http://localhost:1234', 'Server settings did not match expected value'); }); it('Session Manager should exist', async function (): Promise { - should(notebookManager.sessionManager).deepEqual(sessionManager); + should(executeManager.sessionManager).deepEqual(sessionManager); }); it('Server Manager should exist', async function (): Promise { - should(notebookManager.serverManager).deepEqual(serverManager); - }); - - it('Content manager should always be undefined', async function (): Promise { - should(notebookManager.contentManager).be.undefined(); - let expectedUri = vscode.Uri.parse('http://localhost:1234?token=abcdefghijk'); - initInstallAndInstance(expectedUri, mockFactory); - deferredInstall.resolve(); - - // When I start the server - await serverManager.startServer(pythonKernelSpec); - should(notebookManager.contentManager).be.undefined(); + should(executeManager.serverManager).deepEqual(serverManager); }); it('Session and server managers should be shutdown/stopped on dispose', async function(): Promise { let sessionManager = TypeMoq.Mock.ofType(); let serverManager = TypeMoq.Mock.ofType(); - notebookManager = new JupyterNotebookManager(serverManager.object, sessionManager.object); + executeManager = new JupyterExecuteManager(serverManager.object, sessionManager.object); sessionManager.setup(s => s.shutdownAll()).returns(() => new Promise((resolve) => resolve())); serverManager.setup(s => s.stopServer()).returns(() => new Promise((resolve) => resolve())); - // After I dispose the notebook manager - notebookManager.dispose(); + // After I dispose the execute manager + executeManager.dispose(); // Session and server managers should be shutdown/stopped sessionManager.verify((s) => s.shutdownAll(), TypeMoq.Times.once()); diff --git a/extensions/notebook/src/test/model/jupyterController.test.ts b/extensions/notebook/src/test/model/jupyterController.test.ts index ee529c723b..128f2c4617 100644 --- a/extensions/notebook/src/test/model/jupyterController.test.ts +++ b/extensions/notebook/src/test/model/jupyterController.test.ts @@ -76,24 +76,23 @@ describe('Jupyter Controller', function () { it('Returns expected values from notebook provider', async () => { await controller.activate(); - should(controller.notebookProvider.standardKernels).deepEqual([], 'Notebook provider standard kernels should return empty array'); - should(controller.notebookProvider.providerId).equal('jupyter', 'Notebook provider should be jupyter'); - await should(controller.notebookProvider.getNotebookManager(undefined)).be.rejected(); - should(controller.notebookProvider.notebookManagerCount).equal(0); - controller.notebookProvider.handleNotebookClosed(undefined); + should(controller.executeProvider.providerId).equal('jupyter', 'Notebook provider should be jupyter'); + await should(controller.executeProvider.getExecuteManager(undefined)).be.rejected(); + should(controller.executeProvider.executeManagerCount).equal(0); + controller.executeProvider.handleNotebookClosed(undefined); }); - it('Returns notebook manager for real notebook editor', async () => { + it('Returns execute manager for real notebook editor', async () => { await controller.activate(); let notebookUtils = new NotebookUtils(); const notebookEditor = await notebookUtils.newNotebook(undefined); - let notebookManager = await controller.notebookProvider.getNotebookManager(notebookEditor.document.uri); - should(controller.notebookProvider.notebookManagerCount).equal(1); + let notebookManager = await controller.executeProvider.getExecuteManager(notebookEditor.document.uri); + should(controller.executeProvider.executeManagerCount).equal(1); // Session manager should not be immediately ready should(notebookManager.sessionManager.isReady).equal(false); // Session manager should not immediately have specs should(notebookManager.sessionManager.specs).equal(undefined); - controller.notebookProvider.handleNotebookClosed(notebookEditor.document.uri); + controller.executeProvider.handleNotebookClosed(notebookEditor.document.uri); }); }); diff --git a/src/sql/azdata.d.ts b/src/sql/azdata.d.ts index 5b517580ec..f83b568fdc 100644 --- a/src/sql/azdata.d.ts +++ b/src/sql/azdata.d.ts @@ -4919,23 +4919,8 @@ declare module 'azdata' { deleteCell(index: number): void; } - /** - * Register a notebook provider. The supported file types handled by this - * provider are defined in the `package.json: - * ```json - * { - * "contributes": { - * "notebook.providers": [{ - * "provider": "providername", - * "fileExtensions": ["FILEEXT"] - * }] - * } - * } - * ``` - * @param notebook provider - * @returns disposable - */ - export function registerNotebookProvider(provider: NotebookProvider): vscode.Disposable; + export function registerSerializationProvider(provider: NotebookSerializationProvider): vscode.Disposable; + export function registerExecuteProvider(provider: NotebookExecuteProvider): vscode.Disposable; export interface IStandardKernel { readonly name: string; @@ -4943,24 +4928,27 @@ declare module 'azdata' { readonly connectionProviderIds: string[]; } - export interface NotebookProvider { + export interface NotebookSerializationProvider { readonly providerId: string; - /** - * @deprecated standardKernels will be removed in an upcoming release. Standard kernel contribution - * should happen via JSON for extensions. Until this is removed, notebook providers can safely return an empty array. - */ - readonly standardKernels: IStandardKernel[]; - getNotebookManager(notebookUri: vscode.Uri): Thenable; + getSerializationManager(notebookUri: vscode.Uri): Thenable; + } + + export interface NotebookExecuteProvider { + readonly providerId: string; + getExecuteManager(notebookUri: vscode.Uri): Thenable; handleNotebookClosed(notebookUri: vscode.Uri): void; } - export interface NotebookManager { + export interface SerializationManager { /** * Manages reading and writing contents to/from files. * Files may be local or remote, with this manager giving them a chance to convert and migrate * from specific notebook file types to and from a standard type for this UI */ readonly contentManager: ContentManager; + } + + export interface ExecuteManager { /** * A SessionManager that handles starting, stopping and handling notifications around sessions. * Each notebook has 1 session associated with it, and the session is responsible @@ -5009,7 +4997,7 @@ declare module 'azdata' { /* Reads contents from a Uri representing a local or remote notebook and returns a * JSON object containing the cells and metadata about the notebook */ - getNotebookContents(notebookUri: vscode.Uri): Thenable; + deserializeNotebook(contents: string): Thenable; /** * Save a file. @@ -5021,7 +5009,7 @@ declare module 'azdata' { * @returns A thenable which resolves with the file content model when the * file is saved. */ - save(notebookUri: vscode.Uri, notebook: INotebookContents): Thenable; + serializeNotebook(notebook: INotebookContents): Thenable; } /** diff --git a/src/sql/workbench/api/browser/mainThreadNotebook.ts b/src/sql/workbench/api/browser/mainThreadNotebook.ts index c0d923ba43..4115f55322 100644 --- a/src/sql/workbench/api/browser/mainThreadNotebook.ts +++ b/src/sql/workbench/api/browser/mainThreadNotebook.ts @@ -11,8 +11,8 @@ import { IExtHostContext } from 'vs/workbench/api/common/extHost.protocol'; import { Event, Emitter } from 'vs/base/common/event'; import { URI } from 'vs/base/common/uri'; -import { INotebookService, INotebookProvider, INotebookManager } from 'sql/workbench/services/notebook/browser/notebookService'; -import { INotebookManagerDetails, INotebookSessionDetails, INotebookKernelDetails, FutureMessageType, INotebookFutureDetails, INotebookFutureDone } from 'sql/workbench/api/common/sqlExtHostTypes'; +import { INotebookService, IExecuteProvider, IExecuteManager, ISerializationProvider, ISerializationManager } from 'sql/workbench/services/notebook/browser/notebookService'; +import { IExecuteManagerDetails, INotebookSessionDetails, INotebookKernelDetails, FutureMessageType, INotebookFutureDetails, INotebookFutureDone, ISerializationManagerDetails } from 'sql/workbench/api/common/sqlExtHostTypes'; import { LocalContentManager } from 'sql/workbench/services/notebook/common/localContentManager'; import { Deferred } from 'sql/base/common/promise'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; @@ -22,7 +22,8 @@ import type { FutureInternal } from 'sql/workbench/services/notebook/browser/int export class MainThreadNotebook extends Disposable implements MainThreadNotebookShape { private _proxy: ExtHostNotebookShape; - private _providers = new Map(); + private _serializationProviders = new Map(); + private _executeProviders = new Map(); private _futures = new Map(); constructor( @@ -45,22 +46,41 @@ export class MainThreadNotebook extends Disposable implements MainThreadNotebook } //#region Extension host callable methods - public $registerNotebookProvider(providerId: string, handle: number): void { + public $registerSerializationProvider(providerId: string, handle: number): void { let proxy: Proxies = { main: this, ext: this._proxy }; - let notebookProvider = this.instantiationService.createInstance(NotebookProviderWrapper, proxy, providerId, handle); - this._providers.set(handle, notebookProvider); - this.notebookService.registerProvider(providerId, notebookProvider); + let notebookProvider = this.instantiationService.createInstance(SerializationProviderWrapper, proxy, providerId, handle); + this._serializationProviders.set(handle, notebookProvider); + this.notebookService.registerSerializationProvider(providerId, notebookProvider); } - public $unregisterNotebookProvider(handle: number): void { - let registration = this._providers.get(handle); + public $registerExecuteProvider(providerId: string, handle: number): void { + let proxy: Proxies = { + main: this, + ext: this._proxy + }; + let notebookProvider = this.instantiationService.createInstance(ExecuteProviderWrapper, proxy, providerId, handle); + this._executeProviders.set(handle, notebookProvider); + this.notebookService.registerExecuteProvider(providerId, notebookProvider); + } + + public $unregisterSerializationProvider(handle: number): void { + let registration = this._serializationProviders.get(handle); if (registration) { - this.notebookService.unregisterProvider(registration.providerId); + this.notebookService.unregisterSerializationProvider(registration.providerId); registration.dispose(); - this._providers.delete(handle); + this._serializationProviders.delete(handle); + } + } + + public $unregisterExecuteProvider(handle: number): void { + let registration = this._executeProviders.get(handle); + if (registration) { + this.notebookService.unregisterExecuteProvider(registration.providerId); + registration.dispose(); + this._executeProviders.delete(handle); } } @@ -86,8 +106,8 @@ interface Proxies { ext: ExtHostNotebookShape; } -class NotebookProviderWrapper extends Disposable implements INotebookProvider { - private _notebookUriToManagerMap = new Map(); +class SerializationProviderWrapper extends Disposable implements ISerializationProvider { + private _notebookUriToManagerMap = new Map(); constructor( private _proxy: Proxies, @@ -98,16 +118,45 @@ class NotebookProviderWrapper extends Disposable implements INotebookProvider { super(); } - getNotebookManager(notebookUri: URI): Thenable { + getSerializationManager(notebookUri: URI): Thenable { // TODO must call through to setup in the extension host - return this.doGetNotebookManager(notebookUri); + return this.doGetSerializationManager(notebookUri); } - private async doGetNotebookManager(notebookUri: URI): Promise { + private async doGetSerializationManager(notebookUri: URI): Promise { let uriString = notebookUri.toString(); let manager = this._notebookUriToManagerMap.get(uriString); if (!manager) { - manager = this.instantiationService.createInstance(NotebookManagerWrapper, this._proxy, this.providerId, notebookUri); + manager = this.instantiationService.createInstance(SerializationManagerWrapper, this._proxy, this.providerId, notebookUri); + await manager.initialize(this.providerHandle); + this._notebookUriToManagerMap.set(uriString, manager); + } + return manager; + } +} + +class ExecuteProviderWrapper extends Disposable implements IExecuteProvider { + private _notebookUriToManagerMap = new Map(); + + constructor( + private _proxy: Proxies, + public readonly providerId: string, + public readonly providerHandle: number, + @IInstantiationService private readonly instantiationService: IInstantiationService + ) { + super(); + } + + getExecuteManager(notebookUri: URI): Thenable { + // TODO must call through to setup in the extension host + return this.doGetExecuteManager(notebookUri); + } + + private async doGetExecuteManager(notebookUri: URI): Promise { + let uriString = notebookUri.toString(); + let manager = this._notebookUriToManagerMap.get(uriString); + if (!manager) { + manager = this.instantiationService.createInstance(ExecuteManagerWrapper, this._proxy, this.providerId, notebookUri); await manager.initialize(this.providerHandle); this._notebookUriToManagerMap.set(uriString, manager); } @@ -120,11 +169,9 @@ class NotebookProviderWrapper extends Disposable implements INotebookProvider { } } -class NotebookManagerWrapper implements INotebookManager { - private _sessionManager: azdata.nb.SessionManager; +class SerializationManagerWrapper implements ISerializationManager { private _contentManager: azdata.nb.ContentManager; - private _serverManager: azdata.nb.ServerManager; - private managerDetails: INotebookManagerDetails; + private managerDetails: ISerializationManagerDetails; constructor(private _proxy: Proxies, public readonly providerId: string, @@ -132,10 +179,35 @@ class NotebookManagerWrapper implements INotebookManager { @IInstantiationService private readonly instantiationService: IInstantiationService ) { } - public async initialize(providerHandle: number): Promise { - this.managerDetails = await this._proxy.ext.$getNotebookManager(providerHandle, this.notebookUri); + public async initialize(providerHandle: number): Promise { + this.managerDetails = await this._proxy.ext.$getSerializationManagerDetails(providerHandle, this.notebookUri); let managerHandle = this.managerDetails.handle; this._contentManager = this.managerDetails.hasContentManager ? new ContentManagerWrapper(managerHandle, this._proxy) : this.instantiationService.createInstance(LocalContentManager); + return this; + } + + public get contentManager(): azdata.nb.ContentManager { + return this._contentManager; + } + + public get managerHandle(): number { + return this.managerDetails.handle; + } +} + +class ExecuteManagerWrapper implements IExecuteManager { + private _sessionManager: azdata.nb.SessionManager; + private _serverManager: azdata.nb.ServerManager; + private managerDetails: IExecuteManagerDetails; + + constructor(private _proxy: Proxies, + public readonly providerId: string, + private notebookUri: URI + ) { } + + public async initialize(providerHandle: number): Promise { + this.managerDetails = await this._proxy.ext.$getExecuteManagerDetails(providerHandle, this.notebookUri); + let managerHandle = this.managerDetails.handle; this._serverManager = this.managerDetails.hasServerManager ? new ServerManagerWrapper(managerHandle, this._proxy) : undefined; this._sessionManager = new SessionManagerWrapper(managerHandle, this._proxy); return this; @@ -144,9 +216,6 @@ class NotebookManagerWrapper implements INotebookManager { public get sessionManager(): azdata.nb.SessionManager { return this._sessionManager; } - public get contentManager(): azdata.nb.ContentManager { - return this._contentManager; - } public get serverManager(): azdata.nb.ServerManager { return this._serverManager; } @@ -160,12 +229,12 @@ class ContentManagerWrapper implements azdata.nb.ContentManager { constructor(private handle: number, private _proxy: Proxies) { } - getNotebookContents(notebookUri: URI): Thenable { - return this._proxy.ext.$getNotebookContents(this.handle, notebookUri); + deserializeNotebook(contents: string): Thenable { + return this._proxy.ext.$deserializeNotebook(this.handle, contents); } - save(path: URI, notebook: azdata.nb.INotebookContents): Thenable { - return this._proxy.ext.$save(this.handle, path, notebook); + serializeNotebook(notebook: azdata.nb.INotebookContents): Thenable { + return this._proxy.ext.$serializeNotebook(this.handle, notebook); } } diff --git a/src/sql/workbench/api/common/extHostNotebook.ts b/src/sql/workbench/api/common/extHostNotebook.ts index 055f1b892c..0b630e138f 100644 --- a/src/sql/workbench/api/common/extHostNotebook.ts +++ b/src/sql/workbench/api/common/extHostNotebook.ts @@ -12,9 +12,9 @@ import { localize } from 'vs/nls'; import { URI, UriComponents } from 'vs/base/common/uri'; import { ExtHostNotebookShape, MainThreadNotebookShape, SqlMainContext } from 'sql/workbench/api/common/sqlExtHost.protocol'; -import { INotebookManagerDetails, INotebookSessionDetails, INotebookKernelDetails, INotebookFutureDetails, FutureMessageType } from 'sql/workbench/api/common/sqlExtHostTypes'; +import { IExecuteManagerDetails, INotebookSessionDetails, INotebookKernelDetails, INotebookFutureDetails, FutureMessageType, ISerializationManagerDetails } from 'sql/workbench/api/common/sqlExtHostTypes'; -type Adapter = azdata.nb.NotebookProvider | azdata.nb.NotebookManager | azdata.nb.ISession | azdata.nb.IKernel | azdata.nb.IFuture; +type Adapter = azdata.nb.NotebookSerializationProvider | azdata.nb.SerializationManager | azdata.nb.NotebookExecuteProvider | azdata.nb.ExecuteManager | azdata.nb.ISession | azdata.nb.IKernel | azdata.nb.IFuture; export class ExtHostNotebook implements ExtHostNotebookShape { private static _handlePool: number = 0; @@ -28,26 +28,40 @@ export class ExtHostNotebook implements ExtHostNotebookShape { } //#region APIs called by main thread - async $getNotebookManager(providerHandle: number, notebookUri: UriComponents): Promise { + async $getSerializationManagerDetails(providerHandle: number, notebookUri: UriComponents): Promise { let uri = URI.revive(notebookUri); let uriString = uri.toString(); - let adapter = this.findManagerForUri(uriString); + let adapter = this.findSerializationManagerForUri(uriString); if (!adapter) { - adapter = await this._withProvider(providerHandle, (provider) => { - return this.getOrCreateManager(provider, uri); + adapter = await this._withSerializationProvider(providerHandle, (provider) => { + return this.getOrCreateSerializationManager(provider, uri); + }); + } + + return { + handle: adapter.handle, + hasContentManager: !!adapter.contentManager + }; + } + async $getExecuteManagerDetails(providerHandle: number, notebookUri: UriComponents): Promise { + let uri = URI.revive(notebookUri); + let uriString = uri.toString(); + let adapter = this.findExecuteManagerForUri(uriString); + if (!adapter) { + adapter = await this._withExecuteProvider(providerHandle, (provider) => { + return this.getOrCreateExecuteManager(provider, uri); }); } return { handle: adapter.handle, - hasContentManager: !!adapter.contentManager, hasServerManager: !!adapter.serverManager }; } $handleNotebookClosed(notebookUri: UriComponents): void { let uri = URI.revive(notebookUri); let uriString = uri.toString(); - let manager = this.findManagerForUri(uriString); + let manager = this.findExecuteManagerForUri(uriString); if (manager) { manager.provider.handleNotebookClosed(uri); this._adapters.delete(manager.handle); @@ -62,12 +76,12 @@ export class ExtHostNotebook implements ExtHostNotebookShape { return this._withServerManager(managerHandle, (serverManager) => serverManager.stopServer()); } - $getNotebookContents(managerHandle: number, notebookUri: UriComponents): Thenable { - return this._withContentManager(managerHandle, (contentManager) => contentManager.getNotebookContents(URI.revive(notebookUri))); + $deserializeNotebook(managerHandle: number, contents: string): Thenable { + return this._withContentManager(managerHandle, (contentManager) => contentManager.deserializeNotebook(contents)); } - $save(managerHandle: number, notebookUri: UriComponents, notebook: azdata.nb.INotebookContents): Thenable { - return this._withContentManager(managerHandle, (contentManager) => contentManager.save(URI.revive(notebookUri), notebook)); + $serializeNotebook(managerHandle: number, notebook: azdata.nb.INotebookContents): Thenable { + return this._withContentManager(managerHandle, (contentManager) => contentManager.serializeNotebook(notebook)); } $refreshSpecs(managerHandle: number): Thenable { @@ -222,12 +236,21 @@ export class ExtHostNotebook implements ExtHostNotebookShape { //#endregion //#region APIs called by extensions - registerNotebookProvider(provider: azdata.nb.NotebookProvider): vscode.Disposable { + registerExecuteProvider(provider: azdata.nb.NotebookExecuteProvider): vscode.Disposable { if (!provider || !provider.providerId) { - throw new Error(localize('providerRequired', "A NotebookProvider with valid providerId must be passed to this method")); + throw new Error(localize('executeProviderRequired', "A NotebookExecuteProvider with valid providerId must be passed to this method")); } const handle = this._addNewAdapter(provider); - this._proxy.$registerNotebookProvider(provider.providerId, handle); + this._proxy.$registerExecuteProvider(provider.providerId, handle); + return this._createDisposable(handle); + } + + registerSerializationProvider(provider: azdata.nb.NotebookSerializationProvider): vscode.Disposable { + if (!provider || !provider.providerId) { + throw new Error(localize('serializationProviderRequired', "A NotebookSerializationProvider with valid providerId must be passed to this method")); + } + const handle = this._addNewAdapter(provider); + this._proxy.$registerSerializationProvider(provider.providerId, handle); return this._createDisposable(handle); } //#endregion @@ -245,8 +268,8 @@ export class ExtHostNotebook implements ExtHostNotebookShape { return matchingAdapters; } - private findManagerForUri(uriString: string): NotebookManagerAdapter { - for (let manager of this.getAdapters(NotebookManagerAdapter)) { + private findSerializationManagerForUri(uriString: string): SerializationManagerAdapter { + for (let manager of this.getAdapters(SerializationManagerAdapter)) { if (manager.uriString === uriString) { return manager; } @@ -254,10 +277,27 @@ export class ExtHostNotebook implements ExtHostNotebookShape { return undefined; } - private async getOrCreateManager(provider: azdata.nb.NotebookProvider, notebookUri: URI): Promise { - let manager = await provider.getNotebookManager(notebookUri); + private findExecuteManagerForUri(uriString: string): ExecuteManagerAdapter { + for (let manager of this.getAdapters(ExecuteManagerAdapter)) { + if (manager.uriString === uriString) { + return manager; + } + } + return undefined; + } + + private async getOrCreateSerializationManager(provider: azdata.nb.NotebookSerializationProvider, notebookUri: URI): Promise { + let manager = await provider.getSerializationManager(notebookUri); let uriString = notebookUri.toString(); - let adapter = new NotebookManagerAdapter(provider, manager, uriString); + let adapter = new SerializationManagerAdapter(provider, manager, uriString); + adapter.handle = this._addNewAdapter(adapter); + return adapter; + } + + private async getOrCreateExecuteManager(provider: azdata.nb.NotebookExecuteProvider, notebookUri: URI): Promise { + let manager = await provider.getExecuteManager(notebookUri); + let uriString = notebookUri.toString(); + let adapter = new ExecuteManagerAdapter(provider, manager, uriString); adapter.handle = this._addNewAdapter(adapter); return adapter; } @@ -272,23 +312,39 @@ export class ExtHostNotebook implements ExtHostNotebookShape { return ExtHostNotebook._handlePool++; } - private _withProvider(handle: number, callback: (provider: azdata.nb.NotebookProvider) => R | PromiseLike): Promise { - let provider = this._adapters.get(handle) as azdata.nb.NotebookProvider; + private _withSerializationProvider(handle: number, callback: (provider: azdata.nb.NotebookSerializationProvider) => SerializationManagerAdapter | PromiseLike): Promise { + let provider = this._adapters.get(handle) as azdata.nb.NotebookSerializationProvider; if (provider === undefined) { - return Promise.reject(new Error(localize('errNoProvider', "no notebook provider found"))); + return Promise.reject(new Error(localize('errNoSerializationProvider', "No notebook serialization provider found"))); } return Promise.resolve(callback(provider)); } - private _withNotebookManager(handle: number, callback: (manager: NotebookManagerAdapter) => R | PromiseLike): Promise { - let manager = this._adapters.get(handle) as NotebookManagerAdapter; - if (manager === undefined) { - return Promise.reject(new Error(localize('errNoManager', "No Manager found"))); + private _withExecuteProvider(handle: number, callback: (provider: azdata.nb.NotebookExecuteProvider) => ExecuteManagerAdapter | PromiseLike): Promise { + let provider = this._adapters.get(handle) as azdata.nb.NotebookExecuteProvider; + if (provider === undefined) { + return Promise.reject(new Error(localize('errNoExecuteProvider', "No notebook execute provider found"))); } - return this.callbackWithErrorWrap(callback, manager); + return Promise.resolve(callback(provider)); } - private async callbackWithErrorWrap(callback: (manager: NotebookManagerAdapter) => R | PromiseLike, manager: NotebookManagerAdapter): Promise { + private _withSerializationManager(handle: number, callback: (manager: SerializationManagerAdapter) => R | PromiseLike): Promise { + let manager = this._adapters.get(handle) as SerializationManagerAdapter; + if (manager === undefined) { + return Promise.reject(new Error(localize('errNoSerializationManager', "No serialization manager found"))); + } + return this.callbackWithErrorWrap(callback, manager); + } + + private _withExecuteManager(handle: number, callback: (manager: ExecuteManagerAdapter) => R | PromiseLike): Promise { + let manager = this._adapters.get(handle) as ExecuteManagerAdapter; + if (manager === undefined) { + return Promise.reject(new Error(localize('errNoExecuteManager', "No execute manager found"))); + } + return this.callbackWithErrorWrap(callback, manager); + } + + private async callbackWithErrorWrap(callback: (manager: A) => R | PromiseLike, manager: A): Promise { try { let value = await callback(manager); return value; @@ -298,7 +354,7 @@ export class ExtHostNotebook implements ExtHostNotebookShape { } private _withServerManager(handle: number, callback: (manager: azdata.nb.ServerManager) => PromiseLike): Promise { - return this._withNotebookManager(handle, (notebookManager) => { + return this._withExecuteManager(handle, (notebookManager) => { let serverManager = notebookManager.serverManager; if (!serverManager) { return Promise.reject(new Error(localize('noServerManager', "Notebook Manager for notebook {0} does not have a server manager. Cannot perform operations on it", notebookManager.uriString))); @@ -308,7 +364,7 @@ export class ExtHostNotebook implements ExtHostNotebookShape { } private _withContentManager(handle: number, callback: (manager: azdata.nb.ContentManager) => PromiseLike): Promise { - return this._withNotebookManager(handle, (notebookManager) => { + return this._withSerializationManager(handle, (notebookManager) => { let contentManager = notebookManager.contentManager; if (!contentManager) { return Promise.reject(new Error(localize('noContentManager', "Notebook Manager for notebook {0} does not have a content manager. Cannot perform operations on it", notebookManager.uriString))); @@ -318,7 +374,7 @@ export class ExtHostNotebook implements ExtHostNotebookShape { } private _withSessionManager(handle: number, callback: (manager: azdata.nb.SessionManager) => PromiseLike): Promise { - return this._withNotebookManager(handle, (notebookManager) => { + return this._withExecuteManager(handle, (notebookManager) => { let sessionManager = notebookManager.sessionManager; if (!sessionManager) { return Promise.reject(new Error(localize('noSessionManager', "Notebook Manager for notebook {0} does not have a session manager. Cannot perform operations on it", notebookManager.uriString))); @@ -344,12 +400,11 @@ export class ExtHostNotebook implements ExtHostNotebookShape { //#endregion } - -class NotebookManagerAdapter implements azdata.nb.NotebookManager { +class SerializationManagerAdapter implements azdata.nb.SerializationManager { public handle: number; constructor( - public readonly provider: azdata.nb.NotebookProvider, - private manager: azdata.nb.NotebookManager, + public readonly provider: azdata.nb.NotebookSerializationProvider, + private manager: azdata.nb.SerializationManager, public readonly uriString: string ) { } @@ -357,6 +412,16 @@ class NotebookManagerAdapter implements azdata.nb.NotebookManager { public get contentManager(): azdata.nb.ContentManager { return this.manager.contentManager; } +} + +class ExecuteManagerAdapter implements azdata.nb.ExecuteManager { + public handle: number; + constructor( + public readonly provider: azdata.nb.NotebookExecuteProvider, + private manager: azdata.nb.ExecuteManager, + public readonly uriString: string + ) { + } public get sessionManager(): azdata.nb.SessionManager { return this.manager.sessionManager; diff --git a/src/sql/workbench/api/common/extHostNotebookDocumentsAndEditors.ts b/src/sql/workbench/api/common/extHostNotebookDocumentsAndEditors.ts index da2a560b27..0e1cf5d2c6 100644 --- a/src/sql/workbench/api/common/extHostNotebookDocumentsAndEditors.ts +++ b/src/sql/workbench/api/common/extHostNotebookDocumentsAndEditors.ts @@ -247,7 +247,7 @@ export class ExtHostNotebookDocumentsAndEditors implements ExtHostNotebookDocume registerNavigationProvider(provider: azdata.nb.NavigationProvider): vscode.Disposable { if (!provider || !provider.providerId) { - throw new Error(localize('providerRequired', "A NotebookProvider with valid providerId must be passed to this method")); + throw new Error(localize('navigationProviderRequired', "A NavigationProvider with valid providerId must be passed to this method")); } const handle = this._addNewAdapter(provider); this._proxy.$registerNavigationProvider(provider.providerId, handle); diff --git a/src/sql/workbench/api/common/sqlExtHost.api.impl.ts b/src/sql/workbench/api/common/sqlExtHost.api.impl.ts index ce32a09f3c..134baedf80 100644 --- a/src/sql/workbench/api/common/sqlExtHost.api.impl.ts +++ b/src/sql/workbench/api/common/sqlExtHost.api.impl.ts @@ -546,8 +546,11 @@ export function createAdsApiFactory(accessor: ServicesAccessor): IAdsExtensionAp showNotebookDocument(uri: vscode.Uri, showOptions: azdata.nb.NotebookShowOptions) { return extHostNotebookDocumentsAndEditors.showNotebookDocument(uri, showOptions); }, - registerNotebookProvider(provider: azdata.nb.NotebookProvider): vscode.Disposable { - return extHostNotebook.registerNotebookProvider(provider); + registerSerializationProvider(provider: azdata.nb.NotebookSerializationProvider): vscode.Disposable { + return extHostNotebook.registerSerializationProvider(provider); + }, + registerExecuteProvider(provider: azdata.nb.NotebookExecuteProvider): vscode.Disposable { + return extHostNotebook.registerExecuteProvider(provider); }, registerNavigationProvider(provider: azdata.nb.NavigationProvider): vscode.Disposable { return extHostNotebookDocumentsAndEditors.registerNavigationProvider(provider); diff --git a/src/sql/workbench/api/common/sqlExtHost.protocol.ts b/src/sql/workbench/api/common/sqlExtHost.protocol.ts index 3ec3c186fd..af8c877dfa 100644 --- a/src/sql/workbench/api/common/sqlExtHost.protocol.ts +++ b/src/sql/workbench/api/common/sqlExtHost.protocol.ts @@ -18,9 +18,10 @@ import { ITreeComponentItem } from 'sql/workbench/common/views'; import { ITaskHandlerDescription } from 'sql/workbench/services/tasks/common/tasks'; import { IItemConfig, IComponentShape, IModelViewDialogDetails, IModelViewTabDetails, IModelViewButtonDetails, - IModelViewWizardDetails, IModelViewWizardPageDetails, INotebookManagerDetails, INotebookSessionDetails, + IModelViewWizardDetails, IModelViewWizardPageDetails, IExecuteManagerDetails, INotebookSessionDetails, INotebookKernelDetails, INotebookFutureDetails, FutureMessageType, INotebookFutureDone, ISingleNotebookEditOperation, - NotebookChangeKind + NotebookChangeKind, + ISerializationManagerDetails } from 'sql/workbench/api/common/sqlExtHostTypes'; import { IUndoStopOptions } from 'vs/workbench/api/common/extHost.protocol'; import { IExtensionDescription } from 'vs/platform/extensions/common/extensions'; @@ -863,7 +864,8 @@ export interface ExtHostNotebookShape { * Looks up a notebook manager for a given notebook URI * @returns handle of the manager to be used when sending */ - $getNotebookManager(providerHandle: number, notebookUri: UriComponents): Thenable; + $getSerializationManagerDetails(providerHandle: number, notebookUri: UriComponents): Thenable; + $getExecuteManagerDetails(providerHandle: number, notebookUri: UriComponents): Thenable; $handleNotebookClosed(notebookUri: UriComponents): void; // Server Manager APIs @@ -871,8 +873,8 @@ export interface ExtHostNotebookShape { $doStopServer(managerHandle: number): Thenable; // Content Manager APIs - $getNotebookContents(managerHandle: number, notebookUri: UriComponents): Thenable; - $save(managerHandle: number, notebookUri: UriComponents, notebook: azdata.nb.INotebookContents): Thenable; + $deserializeNotebook(managerHandle: number, contents: string): Thenable; + $serializeNotebook(managerHandle: number, notebook: azdata.nb.INotebookContents): Thenable; // Session Manager APIs $refreshSpecs(managerHandle: number): Thenable; @@ -899,8 +901,10 @@ export interface ExtHostNotebookShape { } export interface MainThreadNotebookShape extends IDisposable { - $registerNotebookProvider(providerId: string, handle: number): void; - $unregisterNotebookProvider(handle: number): void; + $registerSerializationProvider(providerId: string, handle: number): void; + $registerExecuteProvider(providerId: string, handle: number): void; + $unregisterSerializationProvider(handle: number): void; + $unregisterExecuteProvider(handle: number): void; $onFutureMessage(futureId: number, type: FutureMessageType, payload: azdata.nb.IMessage): void; $onFutureDone(futureId: number, done: INotebookFutureDone): void; } diff --git a/src/sql/workbench/api/common/sqlExtHostTypes.ts b/src/sql/workbench/api/common/sqlExtHostTypes.ts index 4e53faf150..402b375053 100644 --- a/src/sql/workbench/api/common/sqlExtHostTypes.ts +++ b/src/sql/workbench/api/common/sqlExtHostTypes.ts @@ -551,9 +551,13 @@ export class SqlThemeIcon { } } -export interface INotebookManagerDetails { +export interface ISerializationManagerDetails { handle: number; hasContentManager: boolean; +} + +export interface IExecuteManagerDetails { + handle: number; hasServerManager: boolean; } diff --git a/src/sql/workbench/common/constants.ts b/src/sql/workbench/common/constants.ts index 6fe1296d50..96375fad44 100644 --- a/src/sql/workbench/common/constants.ts +++ b/src/sql/workbench/common/constants.ts @@ -34,6 +34,8 @@ export const UNTITLED_QUERY_EDITOR_TYPEID = 'workbench.editorinputs.untitledQuer export const FILE_QUERY_EDITOR_TYPEID = 'workbench.editorinputs.fileQueryInput'; export const RESOURCE_VIEWER_TYPEID = 'workbench.editorinputs.resourceViewerInput'; +export const JUPYTER_PROVIDER_ID = 'jupyter'; + export const enum NotebookLanguage { Notebook = 'Notebook', Ipynb = 'ipynb' diff --git a/src/sql/workbench/contrib/notebook/browser/models/notebookInput.ts b/src/sql/workbench/contrib/notebook/browser/models/notebookInput.ts index 7c8d67a4bb..b58982244a 100644 --- a/src/sql/workbench/contrib/notebook/browser/models/notebookInput.ts +++ b/src/sql/workbench/contrib/notebook/browser/models/notebookInput.ts @@ -13,11 +13,10 @@ import { IStandardKernelWithProvider, getProvidersForFileName, getStandardKernel import { INotebookService, DEFAULT_NOTEBOOK_PROVIDER, IProviderInfo } from 'sql/workbench/services/notebook/browser/notebookService'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { ITextEditorModel, ITextModelService } from 'vs/editor/common/services/resolverService'; -import { INotebookModel, IContentManager, NotebookContentChange } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; +import { INotebookModel, IContentLoader, NotebookContentChange } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; import { TextFileEditorModel } from 'vs/workbench/services/textfile/common/textFileEditorModel'; import { Schemas } from 'vs/base/common/network'; import { ITextFileSaveOptions, ITextFileEditorModel } from 'vs/workbench/services/textfile/common/textfiles'; -import { LocalContentManager } from 'sql/workbench/services/notebook/common/localContentManager'; import { IConnectionProfile } from 'sql/platform/connection/common/interfaces'; import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; import { IDisposable } from 'vs/base/common/lifecycle'; @@ -37,6 +36,7 @@ import { NotebookModel } from 'sql/workbench/services/notebook/browser/models/no import { INotebookInput } from 'sql/workbench/services/notebook/browser/interface'; import { EditorModel } from 'vs/workbench/common/editor/editorModel'; import { EditorInput } from 'vs/workbench/common/editor/editorInput'; +import { LocalContentManager } from 'sql/workbench/services/notebook/common/localContentManager'; export type ModeViewSaveHandler = (handle: number) => Thenable; @@ -61,8 +61,8 @@ export class NotebookEditorModel extends EditorModel { notebook.modelReady.then((model) => { if (!this._changeEventsHookedUp) { this._changeEventsHookedUp = true; - this._register(model.kernelChanged(e => this.updateModel(undefined, NotebookChangeType.KernelChanged))); - this._register(model.contentChanged(e => this.updateModel(e, e.changeType))); + this._register(model.kernelChanged(e => this.updateModel(undefined, NotebookChangeType.KernelChanged).catch(e => onUnexpectedError(e)))); + this._register(model.contentChanged(e => this.updateModel(e, e.changeType).catch(e => onUnexpectedError(e)))); this._register(notebook.model.onActiveCellChanged((cell) => { if (cell) { this._notebookTextFileModel.activeCellGuid = cell.cellGuid; @@ -120,7 +120,7 @@ export class NotebookEditorModel extends EditorModel { this._onDidChangeDirty.fire(); } - public updateModel(contentChange?: NotebookContentChange, type?: NotebookChangeType): void { + public async updateModel(contentChange?: NotebookContentChange, type?: NotebookChangeType): Promise { // If text editor model is readonly, exit early as no changes need to occur on the model // Note: this follows what happens in fileCommands where update/save logic is skipped for readonly text editor models if (this.textEditorModel?.isReadonly()) { @@ -171,14 +171,14 @@ export class NotebookEditorModel extends EditorModel { if (editAppliedSuccessfully) { return; } - this.replaceEntireTextEditorModel(notebookModel, type); + await this.replaceEntireTextEditorModel(notebookModel, type); this._lastEditFullReplacement = true; } } } - public replaceEntireTextEditorModel(notebookModel: INotebookModel, type: NotebookChangeType) { - this._notebookTextFileModel.replaceEntireTextEditorModel(notebookModel, type, this.textEditorModel); + public replaceEntireTextEditorModel(notebookModel: INotebookModel, type: NotebookChangeType): Promise { + return this._notebookTextFileModel.replaceEntireTextEditorModel(notebookModel, type, this.textEditorModel); } private sendNotebookSerializationStateChange(): void { @@ -224,7 +224,7 @@ export abstract class NotebookInput extends EditorInput implements INotebookInpu private readonly _layoutChanged: Emitter = this._register(new Emitter()); private _model: NotebookEditorModel; private _untitledEditorModel: IUntitledTextEditorModel; - private _contentManager: IContentManager; + private _contentLoader: IContentLoader; private _providersLoaded: Promise; private _dirtyListener: IDisposable; private _notebookEditorOpenedTimestamp: number; @@ -274,11 +274,12 @@ export abstract class NotebookInput extends EditorInput implements INotebookInpu return this._notebookFindModel; } - public get contentManager(): IContentManager { - if (!this._contentManager) { - this._contentManager = this.instantiationService.createInstance(NotebookEditorContentManager, this); + public get contentLoader(): IContentLoader { + if (!this._contentLoader) { + let contentManager = this.instantiationService.createInstance(LocalContentManager); + this._contentLoader = this.instantiationService.createInstance(NotebookEditorContentLoader, this, contentManager); } - return this._contentManager; + return this._contentLoader; } public override getName(): string { @@ -292,6 +293,10 @@ export abstract class NotebookInput extends EditorInput implements INotebookInpu return EditorInputCapabilities.None; } + public get providersLoaded(): Promise { + return this._providersLoaded; + } + public async getProviderInfo(): Promise { await this._providersLoaded; return { @@ -313,14 +318,14 @@ export abstract class NotebookInput extends EditorInput implements INotebookInpu } override async save(groupId: number, options?: ITextFileSaveOptions): Promise { - this.updateModel(); + await this.updateModel(); let input = await this.textInput.save(groupId, options); await this.setTrustForNewEditor(input); return input; } override async saveAs(group: number, options?: ITextFileSaveOptions): Promise { - this.updateModel(); + await this.updateModel(); let input = await this.textInput.saveAs(group, options); await this.setTrustForNewEditor(input); return input; @@ -438,6 +443,8 @@ export abstract class NotebookInput extends EditorInput implements INotebookInpu let standardKernels = getStandardKernelsForProvider(provider, this.notebookService); this._standardKernels.push(...standardKernels); }); + let serializationProvider = await this.notebookService.getOrCreateSerializationManager(this._providerId, this._resource); + this._contentLoader = this.instantiationService.createInstance(NotebookEditorContentLoader, this, serializationProvider.contentManager); } } @@ -497,8 +504,8 @@ export abstract class NotebookInput extends EditorInput implements INotebookInpu } } - updateModel(): void { - this._model.updateModel(); + public updateModel(): Promise { + return this._model.updateModel(); } public override matches(otherInput: any): boolean { @@ -510,17 +517,14 @@ export abstract class NotebookInput extends EditorInput implements INotebookInpu } } -export class NotebookEditorContentManager implements IContentManager { +export class NotebookEditorContentLoader implements IContentLoader { constructor( private notebookInput: NotebookInput, - @IInstantiationService private readonly instantiationService: IInstantiationService) { + private contentManager: azdata.nb.ContentManager) { } async loadContent(): Promise { let notebookEditorModel = await this.notebookInput.resolve(); - let contentManager = this.instantiationService.createInstance(LocalContentManager); - let contents = await contentManager.loadFromContentString(notebookEditorModel.contentString); - return contents; + return this.contentManager.deserializeNotebook(notebookEditorModel.contentString); } - } diff --git a/src/sql/workbench/contrib/notebook/browser/models/notebookTextFileModel.ts b/src/sql/workbench/contrib/notebook/browser/models/notebookTextFileModel.ts index dc11625335..8539eb329b 100644 --- a/src/sql/workbench/contrib/notebook/browser/models/notebookTextFileModel.ts +++ b/src/sql/workbench/contrib/notebook/browser/models/notebookTextFileModel.ts @@ -200,8 +200,15 @@ export class NotebookTextFileModel { return false; } - public replaceEntireTextEditorModel(notebookModel: INotebookModel, type: NotebookChangeType, textEditorModel: ITextEditorModel) { - let content = JSON.stringify(notebookModel.toJSON(type), undefined, ' '); + public async replaceEntireTextEditorModel(notebookModel: INotebookModel, type: NotebookChangeType, textEditorModel: ITextEditorModel): Promise { + let content: string; + let notebookContents = notebookModel.toJSON(type); + let serializer = notebookModel.serializationManager; + if (serializer) { + content = await serializer.contentManager.serializeNotebook(notebookContents); + } else { + content = JSON.stringify(notebookContents, undefined, ' '); + } let model = textEditorModel.textEditorModel; let endLine = model.getLineCount(); let endCol = model.getLineMaxColumn(endLine); diff --git a/src/sql/workbench/contrib/notebook/browser/notebook.contribution.ts b/src/sql/workbench/contrib/notebook/browser/notebook.contribution.ts index 3fe76d216a..d4a36b4ac2 100644 --- a/src/sql/workbench/contrib/notebook/browser/notebook.contribution.ts +++ b/src/sql/workbench/contrib/notebook/browser/notebook.contribution.ts @@ -53,7 +53,7 @@ import { ImageMimeTypes, TextCellEditModes } from 'sql/workbench/services/notebo import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { NotebookInput } from 'sql/workbench/contrib/notebook/browser/models/notebookInput'; import { INotebookModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; -import { INotebookManager } from 'sql/workbench/services/notebook/browser/notebookService'; +import { IExecuteManager } from 'sql/workbench/services/notebook/browser/notebookService'; import { NotebookExplorerViewletViewsContribution } from 'sql/workbench/contrib/notebook/browser/notebookExplorer/notebookExplorerViewlet'; import { Disposable, DisposableStore } from 'vs/base/common/lifecycle'; import { ContributedEditorPriority, IEditorOverrideService } from 'vs/workbench/services/editor/common/editorOverrideService'; @@ -62,6 +62,7 @@ import { IModeService } from 'vs/editor/common/services/modeService'; import { ILogService } from 'vs/platform/log/common/log'; import { DiffEditorInput } from 'vs/workbench/common/editor/diffEditorInput'; import { useNewMarkdownRendererKey } from 'sql/workbench/contrib/notebook/common/notebookCommon'; +import { JUPYTER_PROVIDER_ID } from 'sql/workbench/common/constants'; Registry.as(EditorExtensions.EditorInputFactories) .registerEditorInputSerializer(FileNotebookInput.ID, FileNoteBookEditorInputSerializer); @@ -189,10 +190,10 @@ CommandsRegistry.registerCommand({ for (let editor of editors) { if (editor instanceof NotebookInput) { let model: INotebookModel = editor.notebookModel; - if (model.providerId === 'jupyter' && model.clientSession.isReady) { + if (model.providerId === JUPYTER_PROVIDER_ID && model.clientSession.isReady) { // Jupyter server needs to be restarted so that the correct Python installation is used if (!jupyterServerRestarted && restartJupyterServer) { - let jupyterNotebookManager: INotebookManager = model.notebookManagers.find(x => x.providerId === 'jupyter'); + let jupyterNotebookManager: IExecuteManager = model.executeManagers.find(x => x.providerId === JUPYTER_PROVIDER_ID); // Shutdown all current Jupyter sessions before stopping the server await jupyterNotebookManager.sessionManager.shutdownAll(); // Jupyter session manager needs to be disposed so that a new one is created with the new server info @@ -222,8 +223,8 @@ CommandsRegistry.registerCommand({ for (let editor of editors) { if (editor instanceof NotebookInput) { let model: INotebookModel = editor.notebookModel; - if (model?.providerId === 'jupyter') { - let jupyterNotebookManager: INotebookManager = model.notebookManagers.find(x => x.providerId === 'jupyter'); + if (model?.providerId === JUPYTER_PROVIDER_ID) { + let jupyterNotebookManager: IExecuteManager = model.executeManagers.find(x => x.providerId === JUPYTER_PROVIDER_ID); await jupyterNotebookManager.sessionManager.shutdownAll(); jupyterNotebookManager.sessionManager.dispose(); await jupyterNotebookManager.serverManager.stopServer(); diff --git a/src/sql/workbench/contrib/notebook/browser/notebookEditor.component.ts b/src/sql/workbench/contrib/notebook/browser/notebookEditor.component.ts index 0af61b2e5a..8cf8c90512 100644 --- a/src/sql/workbench/contrib/notebook/browser/notebookEditor.component.ts +++ b/src/sql/workbench/contrib/notebook/browser/notebookEditor.component.ts @@ -7,7 +7,7 @@ import { NotebookModel } from 'sql/workbench/services/notebook/browser/models/no import * as notebookUtils from 'sql/workbench/services/notebook/browser/models/notebookUtils'; import { AngularDisposable } from 'sql/base/browser/lifecycle'; import { IBootstrapParams } from 'sql/workbench/services/bootstrap/common/bootstrapParams'; -import { INotebookParams, INotebookService, INotebookManager, DEFAULT_NOTEBOOK_PROVIDER, SQL_NOTEBOOK_PROVIDER } from 'sql/workbench/services/notebook/browser/notebookService'; +import { INotebookParams, INotebookService, IExecuteManager, DEFAULT_NOTEBOOK_PROVIDER, SQL_NOTEBOOK_PROVIDER, ISerializationManager } from 'sql/workbench/services/notebook/browser/notebookService'; import { IConnectionManagementService } from 'sql/platform/connection/common/connectionManagement'; import { CellMagicMapper } from 'sql/workbench/contrib/notebook/browser/models/cellMagicMapper'; import { INotificationService } from 'vs/platform/notification/common/notification'; @@ -38,7 +38,8 @@ export const NOTEBOOKEDITOR_SELECTOR: string = 'notebookeditor-component'; export class NotebookEditorComponent extends AngularDisposable { private readonly defaultViewMode = ViewMode.Notebook; private profile: IConnectionProfile; - private notebookManagers: INotebookManager[] = []; + private serializationManagers: ISerializationManager[] = []; + private executeManagers: IExecuteManager[] = []; private _modelReadyDeferred = new Deferred(); public model: NotebookModel; @@ -80,7 +81,8 @@ export class NotebookEditorComponent extends AngularDisposable { private async doLoad(): Promise { await this.createModelAndLoadContents(); - await this.setNotebookManager(); + await this.setSerializationManager(); + await this.setExecuteManager(); await this.loadModel(); this.setActiveView(); @@ -93,21 +95,23 @@ export class NotebookEditorComponent extends AngularDisposable { await this.model.requestModelLoad(); this.detectChanges(); this.setContextKeyServiceWithProviderId(this.model.providerId); - await this.model.startSession(this.model.notebookManager, undefined, true); + await this.model.startSession(this.model.executeManager, undefined, true); this.fillInActionsForCurrentContext(); this.detectChanges(); } private async createModelAndLoadContents(): Promise { + let providerInfo = await this._notebookParams.providerInfo; let model = new NotebookModel({ factory: this.modelFactory, notebookUri: this._notebookParams.notebookUri, connectionService: this.connectionManagementService, notificationService: this.notificationService, - notebookManagers: this.notebookManagers, - contentManager: this._notebookParams.input.contentManager, + serializationManagers: this.serializationManagers, + executeManagers: this.executeManagers, + contentLoader: this._notebookParams.input.contentLoader, cellMagicMapper: new CellMagicMapper(this.notebookService.languageMagics), - providerId: 'sql', + providerId: providerInfo.providerId, defaultKernel: this._notebookParams.input.defaultKernel, layoutChanged: this._notebookParams.input.layoutChanged, capabilitiesService: this.capabilitiesService, @@ -132,11 +136,19 @@ export class NotebookEditorComponent extends AngularDisposable { this.detectChanges(); } - private async setNotebookManager(): Promise { + private async setSerializationManager(): Promise { let providerInfo = await this._notebookParams.providerInfo; for (let providerId of providerInfo.providers) { - let notebookManager = await this.notebookService.getOrCreateNotebookManager(providerId, this._notebookParams.notebookUri); - this.notebookManagers.push(notebookManager); + let manager = await this.notebookService.getOrCreateSerializationManager(providerId, this._notebookParams.notebookUri); + this.serializationManagers.push(manager); + } + } + + private async setExecuteManager(): Promise { + let providerInfo = await this._notebookParams.providerInfo; + for (let providerId of providerInfo.providers) { + let manager = await this.notebookService.getOrCreateExecuteManager(providerId, this._notebookParams.notebookUri); + this.executeManagers.push(manager); } } diff --git a/src/sql/workbench/contrib/notebook/test/browser/cellToolbarActions.test.ts b/src/sql/workbench/contrib/notebook/test/browser/cellToolbarActions.test.ts index bbdd4a7666..0abe606048 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/cellToolbarActions.test.ts +++ b/src/sql/workbench/contrib/notebook/test/browser/cellToolbarActions.test.ts @@ -22,14 +22,14 @@ import { IProductService } from 'vs/platform/product/common/productService'; import { Separator } from 'vs/base/common/actions'; import { INotebookModelOptions } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; import { NotebookModel } from 'sql/workbench/services/notebook/browser/models/notebookModel'; -import { NotebookEditorContentManager } from 'sql/workbench/contrib/notebook/browser/models/notebookInput'; +import { NotebookEditorContentLoader } from 'sql/workbench/contrib/notebook/browser/models/notebookInput'; import { URI } from 'vs/base/common/uri'; import { ModelFactory } from 'sql/workbench/services/notebook/browser/models/modelFactory'; import { CellTypes } from 'sql/workbench/services/notebook/common/contracts'; import { nb } from 'azdata'; import { InstantiationService } from 'vs/platform/instantiation/common/instantiationService'; import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection'; -import { NotebookManagerStub } from 'sql/workbench/contrib/notebook/test/stubs'; +import { ExecuteManagerStub, SerializationManagerStub } from 'sql/workbench/contrib/notebook/test/stubs'; suite('CellToolbarActions', function (): void { suite('removeDuplicatedAndStartingSeparators', function (): void { @@ -202,13 +202,14 @@ export async function createandLoadNotebookModel(codeContent?: nb.INotebookConte let serviceCollection = new ServiceCollection(); let instantiationService = new InstantiationService(serviceCollection, true); - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(codeContent ? codeContent : defaultCodeContent)); let defaultModelOptions: INotebookModelOptions = { notebookUri: URI.file('/some/path.ipynb'), factory: new ModelFactory(instantiationService), - notebookManagers: [new NotebookManagerStub()], - contentManager: mockContentManager.object, + serializationManagers: [new SerializationManagerStub()], + executeManagers: [new ExecuteManagerStub()], + contentLoader: mockContentManager.object, notificationService: undefined, connectionService: undefined, providerId: 'SQL', diff --git a/src/sql/workbench/contrib/notebook/test/browser/notebookInput.test.ts b/src/sql/workbench/contrib/notebook/test/browser/notebookInput.test.ts index 5749359283..2b87d375ec 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/notebookInput.test.ts +++ b/src/sql/workbench/contrib/notebook/test/browser/notebookInput.test.ts @@ -17,10 +17,11 @@ import { NodeStub, NotebookServiceStub } from 'sql/workbench/contrib/notebook/te import { basenameOrAuthority } from 'vs/base/common/resources'; import { UntitledTextEditorInput } from 'vs/workbench/services/untitled/common/untitledTextEditorInput'; import { IExtensionService, NullExtensionService } from 'vs/workbench/services/extensions/common/extensions'; -import { INotebookService, IProviderInfo } from 'sql/workbench/services/notebook/browser/notebookService'; +import { INotebookService, IProviderInfo, ISerializationManager } from 'sql/workbench/services/notebook/browser/notebookService'; import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; import { IUntitledTextEditorService } from 'vs/workbench/services/untitled/common/untitledTextEditorService'; import { EditorInputCapabilities } from 'vs/workbench/common/editor'; +import { LocalContentManager } from 'sql/workbench/services/notebook/common/localContentManager'; suite('Notebook Input', function (): void { const instantiationService = workbenchInstantiationService(); @@ -39,9 +40,14 @@ suite('Notebook Input', function (): void { name: 'TestName', displayName: 'TestDisplayName', connectionProviderIds: ['TestId'], - notebookProvider: 'TestProvider' + notebookProvider: testProvider }]; }); + let testManager: ISerializationManager = { + providerId: testProvider, + contentManager: instantiationService.createInstance(LocalContentManager) + }; + mockNotebookService.setup(s => s.getOrCreateSerializationManager(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(testManager)); (instantiationService as TestInstantiationService).stub(INotebookService, mockNotebookService.object); @@ -87,11 +93,11 @@ suite('Notebook Input', function (): void { // Notebook URI assert.deepStrictEqual(untitledNotebookInput.notebookUri, untitledUri); - // Content Manager + // Notebook editor timestamp assert.notStrictEqual(untitledNotebookInput.editorOpenedTimestamp, undefined); - // Notebook editor timestamp - assert.notStrictEqual(untitledNotebookInput.contentManager, undefined); + // Content Loader + assert.notStrictEqual(untitledNotebookInput.contentLoader, undefined); // Layout changed event assert.notStrictEqual(untitledNotebookInput.layoutChanged, undefined); 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 4e619dde4d..5f14a295e3 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/notebookService.test.ts +++ b/src/sql/workbench/contrib/notebook/test/browser/notebookService.test.ts @@ -12,10 +12,10 @@ 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 { INavigationProvider, INotebookEditor, IExecuteManager, INotebookParams, IExecuteProvider, NavigationProviders, SQL_NOTEBOOK_PROVIDER, unsavedBooksContextKey, ISerializationProvider, ISerializationManager } from 'sql/workbench/services/notebook/browser/notebookService'; +import { FailToSaveTrustState, NotebookService, NotebookServiceNoProviderRegistered, NotebookUriNotDefined, ExecuteProviderDescriptor, SerializationProviderDescriptor } 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 { INotebookProviderRegistry, NotebookProviderRegistryId, ProviderDescriptionRegistration } 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'; @@ -36,9 +36,15 @@ import { IUntitledTextEditorService } from 'vs/workbench/services/untitled/commo import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { TestConfigurationService } from 'sql/platform/connection/test/common/testConfigurationService'; -/** - * class to mock azdata.nb.ServerManager object - */ +class TestContentManager implements azdata.nb.ContentManager { + deserializeNotebook(contents: string): Thenable { + throw new Error('Method not implemented.'); + } + serializeNotebook(notebook: azdata.nb.INotebookContents): Thenable { + throw new Error('Method not implemented.'); + } +} + class TestServerManager implements azdata.nb.ServerManager { isStarted: boolean = true; //by default our mock creates ServerManager in started mode. onServerStarted: Event; @@ -48,14 +54,16 @@ class TestServerManager implements azdata.nb.ServerManager { async stopServer(): Promise { this.isStarted = false; } - } -/** - * TestNotebookManager - creates a NotebookManager object that helps keep track of state needed by testing - */ -class TestNotebookManager implements INotebookManager { - contentManager: undefined; +class TestSerializationManager implements ISerializationManager { + constructor( + public providerId: string = 'providerId1', + public contentManager: TestContentManager = new TestContentManager() + ) { } +} + +class TestExecuteManager implements IExecuteManager { sessionManager: undefined; constructor( public providerId: string = 'providerId1', @@ -63,16 +71,24 @@ class TestNotebookManager implements INotebookManager { ) { } } -/** - * * TestNotebookProvider - creates a NotebookManager object that helps keep track of state needed by testing - */ -class TestNotebookProvider implements INotebookProvider { +class TestSerializationProvider implements ISerializationProvider { constructor( public providerId: string = 'providerId1', - public manager: TestNotebookManager = new TestNotebookManager(providerId) + public manager: TestSerializationManager = new TestSerializationManager(providerId) ) { } - getNotebookManager(uri: URI): Thenable { + getSerializationManager(notebookUri: URI): Thenable { + return Promise.resolve(this.manager); + } +} + +class TestExecuteProvider implements IExecuteProvider { + constructor( + public providerId: string = 'providerId1', + public manager: TestExecuteManager = new TestExecuteManager(providerId) + ) { } + + getExecuteManager(uri: URI): Thenable { return Promise.resolve(this.manager); } @@ -82,9 +98,11 @@ class TestNotebookProvider implements INotebookProvider { } suite('ProviderDescriptor:', () => { - test('Verifies varies getter setters of Provider Descriptor', async () => { - const notebookProvider = {}; - const providerDescriptor = new ProviderDescriptor(notebookProvider); + test('Verifies various getters & setters for Serialization Provider Descriptor', async () => { + const providerId = 'TestId'; + const notebookProvider = {}; + const providerDescriptor = new SerializationProviderDescriptor(providerId, notebookProvider); + assert.strictEqual(providerDescriptor.providerId, providerId, 'providerDescriptor providerId should return correct provider ID'); 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`); @@ -92,9 +110,26 @@ suite('ProviderDescriptor:', () => { 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`); + assert.strictEqual(providerDescriptor.instance, undefined, `providerDescriptor 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`); + assert.strictEqual(providerDescriptor.instance, notebookProvider, `providerDescriptor instance should be instance: ${notebookProvider} that we explicitly set it to`); + }); + + test('Verifies various getters & setters for Execute Provider Descriptor', async () => { + const providerId = 'TestId'; + const notebookProvider = {}; + const providerDescriptor = new ExecuteProviderDescriptor(providerId, notebookProvider); + assert.strictEqual(providerDescriptor.providerId, providerId, 'providerDescriptor providerId should return correct provider ID'); + 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, `providerDescriptor instance should be undefined when we set it explicitly to undefined`); + providerDescriptor.instance = notebookProvider; + assert.strictEqual(providerDescriptor.instance, notebookProvider, `providerDescriptor instance should be instance: ${notebookProvider} that we explicitly set it to`); }); }); @@ -198,7 +233,7 @@ suite.skip('NotebookService:', function (): void { await notebookService.registrationComplete; assert.deepStrictEqual(notebookService.getProvidersForFileType('ipynb'), ['sql'], 'sql provider should be registered for ipynb extension'); - const otherProviderRegistration: NotebookProviderRegistration = { + const otherProviderRegistration: ProviderDescriptionRegistration = { fileExtensions: 'ipynb', standardKernels: { name: 'kernel1', @@ -208,8 +243,8 @@ suite.skip('NotebookService:', function (): void { provider: 'otherProvider' }; - const notebookRegistry = Registry.as(Extensions.NotebookProviderContribution); - notebookRegistry.registerNotebookProvider(otherProviderRegistration); + const notebookRegistry = Registry.as(NotebookProviderRegistryId); + notebookRegistry.registerProviderDescription(otherProviderRegistration); assert.deepStrictEqual(notebookService.getProvidersForFileType('ipynb'), ['sql', 'otherProvider'], 'otherProvider should also be registered for ipynb extension'); assert.deepStrictEqual(notebookService.getSupportedFileExtensions(), ['IPYNB'], 'Only IPYNB should be registered as supported file extension'); @@ -223,10 +258,10 @@ suite.skip('NotebookService:', function (): void { 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 () => { + test('verify that getOrCreateSerializationManager does not throw when extensionService.whenInstalledExtensionRegistered() throws', async () => { const providerId = 'providerId1'; - createRegisteredProviderWithManager({ notebookService, providerId }); - notebookService.registerProvider(providerId, undefined); + createExecuteProviderWithManager({ notebookService, providerId }); + notebookService.registerSerializationProvider(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); @@ -236,18 +271,34 @@ suite.skip('NotebookService:', function (): void { 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')); + await notebookService.getOrCreateSerializationManager(providerId, URI.parse('untitled:uri1')); logServiceMock.verifyAll(); }); + test('verify that getOrCreateExecuteManager does not throw when extensionService.whenInstalledExtensionRegistered() throws', async () => { + const providerId = 'providerId1'; + createExecuteProviderWithManager({ notebookService, providerId }); + notebookService.registerExecuteProvider(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); - test('verify that getOrCreateNotebookManager throws when no providers are registered', async () => { - const methodName = 'getOrCreateNotebookManager'; + 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.getOrCreateExecuteManager(providerId, URI.parse('untitled:uri1')); + logServiceMock.verifyAll(); + }); + + test('verify that getOrCreateSerializationManager throws when no providers are registered', async () => { + const methodName = 'getOrCreateSerializationManager'; // register the builtin sql provider to be undefined - notebookService.registerProvider(SQL_NOTEBOOK_PROVIDER, undefined); + notebookService.registerSerializationProvider(SQL_NOTEBOOK_PROVIDER, undefined); try { - await notebookService.getOrCreateNotebookManager('test', URI.parse('untitled:uri1')); + await notebookService.getOrCreateSerializationManager('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`); @@ -256,7 +307,29 @@ suite.skip('NotebookService:', function (): void { // 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')); + await notebookService.getOrCreateSerializationManager(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('verify that getOrCreateExecuteManager throws when no providers are registered', async () => { + const methodName = 'getOrCreateExecuteManager'; + + // register the builtin sql provider to be undefined + notebookService.registerExecuteProvider(SQL_NOTEBOOK_PROVIDER, undefined); + try { + await notebookService.getOrCreateExecuteManager('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.getOrCreateExecuteManager(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`); @@ -282,27 +355,50 @@ suite.skip('NotebookService:', function (): void { 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 () => { + test('verifies return value of getOrCreateSerializationManager', async () => { await notebookService.registrationComplete; try { - await notebookService.getOrCreateNotebookManager(SQL_NOTEBOOK_PROVIDER, undefined); + await notebookService.getOrCreateSerializationManager(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`); + assert.strictEqual((error as Error).message, NotebookUriNotDefined, `getOrCreateSerializationManager 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 { providerId: provider2Id, manager: provider2Manager } = createExecuteProviderWithManager({ 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`); + const result1 = await notebookService.getOrCreateSerializationManager(provider1Id, uri1); + const result2 = await notebookService.getOrCreateSerializationManager(provider2Id, uri2); + const result1Again = await notebookService.getOrCreateSerializationManager(provider1Id, uri1); + assert.strictEqual(result2, provider2Manager, `the serialization manager for the provider must be the one returned by getSerializationManager of the provider`); + assert.notStrictEqual(result1, result2, `different serialization managers should be returned for different uris`); + assert.strictEqual(result1, result1Again, `same serialization managers should be returned for same uri for builtin providers`); + const result2Again = await notebookService.getOrCreateSerializationManager(provider2Id, uri2); + assert.strictEqual(result2, result2Again, `same serialization managers should be returned for same uri for custom providers`); + }); + + test('verifies return value of getOrCreateExecuteManager', async () => { + await notebookService.registrationComplete; + try { + await notebookService.getOrCreateExecuteManager(SQL_NOTEBOOK_PROVIDER, undefined); + throw new Error('expected exception was not thrown'); + } catch (error) { + assert.strictEqual((error as Error).message, NotebookUriNotDefined, `getOrCreateExecuteManager must throw with UriNotDefined error, when a valid uri is not provided`); + } + const provider1Id = SQL_NOTEBOOK_PROVIDER; + const { providerId: provider2Id, manager: provider2Manager } = createExecuteProviderWithManager({ providerId: 'provider2Id', notebookService }); + + const uri1: URI = URI.parse(`untitled:test1`); + const uri2: URI = URI.parse(`untitled:test2`); + const result1 = await notebookService.getOrCreateExecuteManager(provider1Id, uri1); + const result2 = await notebookService.getOrCreateExecuteManager(provider2Id, uri2); + const result1Again = await notebookService.getOrCreateExecuteManager(provider1Id, uri1); + assert.strictEqual(result2, provider2Manager, `the execute manager for the provider must be the one returned by getExecuteManager of the provider`); + assert.notStrictEqual(result1, result2, `different execute managers should be returned for different uris`); + assert.strictEqual(result1, result1Again, `same execute managers should be returned for same uri for builtin providers`); + const result2Again = await notebookService.getOrCreateExecuteManager(provider2Id, uri2); + assert.strictEqual(result2, result2Again, `same execute managers should be returned for same uri for custom providers`); }); test('verifies add/remove/find/list/renameNotebookEditor methods and corresponding events', async () => { @@ -360,24 +456,43 @@ suite.skip('NotebookService:', function (): void { 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 () => { + test('test registration of a new serialization provider with multiple filetypes & kernels and verify that corresponding manager is returned by getOrCreateSerializationManager methods', async () => { const providerId = 'Jpeg'; - const notebookProviderRegistration = { + 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; + const notebookRegistry = Registry.as(NotebookProviderRegistryId); + notebookRegistry.registerProviderDescription(notebookProviderRegistration); + + const serializationManagerPromise = notebookService.getOrCreateSerializationManager(providerId, URI.parse('untitled:jpg')); + const serializationProviderInstance = createSerializationProviderWithManager({ notebookService, providerId }); + notebookService.registerSerializationProvider(providerId, serializationProviderInstance); + const serializationResult = await serializationManagerPromise; // verify result - assert.strictEqual(result, providerInstance.manager, `createRegisteredProviderWithManager must return the manager corresponding to INotebookProvider that we registered`); + assert.strictEqual(serializationResult, serializationProviderInstance.manager, `createSerializationProviderWithManager must return the serialization manager corresponding to ISerializationProvider that we registered`); }); + test('test registration of a new execute provider with multiple filetypes & kernels and verify that corresponding manager is returned by getOrCreateExecuteManager methods', async () => { + const providerId = 'Jpeg'; + const notebookProviderRegistration = { + provider: providerId, + fileExtensions: ['jpeg', 'jpg'], + standardKernels: [{ name: 'kernel1' }, { name: 'kernel2' }] + }; + const notebookRegistry = Registry.as(NotebookProviderRegistryId); + notebookRegistry.registerProviderDescription(notebookProviderRegistration); + + const executeManagerPromise = notebookService.getOrCreateExecuteManager(providerId, URI.parse('untitled:jpg')); + const executeProviderInstance = createExecuteProviderWithManager({ notebookService, providerId }); + notebookService.registerExecuteProvider(providerId, executeProviderInstance); + const executeResult = await executeManagerPromise; + + // verify result + assert.strictEqual(executeResult, executeProviderInstance.manager, `createExecuteProviderWithManager must return the execute manager corresponding to IExecuteProvider that we registered`); + }); test('verify that firing of extensionService.onDidUninstallExtension event calls removeContributedProvidersFromCache', async () => { const methodName = 'removeContributedProvidersFromCache'; @@ -480,7 +595,7 @@ suite.skip('NotebookService:', function (): void { } }); - suite(`serialization tests`, () => { + suite(`serialization state tests`, () => { for (const isTrusted of [true, false, undefined]) { for (const isModelTrusted of [true, false]) { if (isTrusted !== undefined && !isModelTrusted) { @@ -576,10 +691,11 @@ suite.skip('NotebookService:', function (): void { }); function unRegisterProviders(notebookService: NotebookService) { - const notebookRegistry = Registry.as(Extensions.NotebookProviderContribution); + const notebookRegistry = Registry.as(NotebookProviderRegistryId); // unregister all builtin providers - for (const providerContribution of notebookRegistry.providers) { - notebookService.unregisterProvider(providerContribution.provider); + for (const providerContribution of notebookRegistry.providerDescriptions) { + notebookService.unregisterSerializationProvider(providerContribution.provider); + notebookService.unregisterExecuteProvider(providerContribution.provider); } } @@ -595,21 +711,32 @@ function setTrustedSetup(notebookService: NotebookService) { 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); +function createSerializationProviderWithManager({ notebookService, providerId = 'providerId', testProviderManagers = undefined }: { providerId?: string; notebookService: NotebookService; testProviderManagers?: TestSerializationProvider[] }): TestSerializationProvider { + const provider = new TestSerializationProvider(providerId); + notebookService.registerSerializationProvider(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 { +function createExecuteProviderWithManager({ notebookService, providerId = 'providerId', testProviderManagers = undefined }: { providerId?: string; notebookService: NotebookService; testProviderManagers?: TestExecuteProvider[] }): TestExecuteProvider { + const provider = new TestExecuteProvider(providerId); + notebookService.registerExecuteProvider(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}`)); + createSerializationProviderWithManager({ providerId, notebookService, testProviderManagers }); + createExecuteProviderWithManager({ providerId, notebookService, testProviderManagers }); + await notebookService.getOrCreateSerializationManager(providerId, URI.parse(`${uriPrefix}${i}`)); + await notebookService.getOrCreateExecuteManager(providerId, URI.parse(`${uriPrefix}${i}`)); } return testProviderManagers; } diff --git a/src/sql/workbench/contrib/notebook/test/browser/notebookViewModel.test.ts b/src/sql/workbench/contrib/notebook/test/browser/notebookViewModel.test.ts index e32dab56fe..8004bf16cd 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/notebookViewModel.test.ts +++ b/src/sql/workbench/contrib/notebook/test/browser/notebookViewModel.test.ts @@ -11,7 +11,7 @@ import { INotificationService } from 'vs/platform/notification/common/notificati import { TestNotificationService } from 'vs/platform/notification/test/common/testNotificationService'; import { URI } from 'vs/base/common/uri'; -import { NotebookManagerStub } from 'sql/workbench/contrib/notebook/test/stubs'; +import { ExecuteManagerStub, SerializationManagerStub } from 'sql/workbench/contrib/notebook/test/stubs'; import { NotebookModel } from 'sql/workbench/services/notebook/browser/models/notebookModel'; import { ModelFactory } from 'sql/workbench/services/notebook/browser/models/modelFactory'; import { INotebookModelOptions } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; @@ -24,7 +24,7 @@ import { InstantiationService } from 'vs/platform/instantiation/common/instantia import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection'; import { NullLogService } from 'vs/platform/log/common/log'; import { TestConnectionManagementService } from 'sql/platform/connection/test/common/testConnectionManagementService'; -import { NotebookEditorContentManager } from 'sql/workbench/contrib/notebook/browser/models/notebookInput'; +import { NotebookEditorContentLoader } from 'sql/workbench/contrib/notebook/browser/models/notebookInput'; import { SessionManager } from 'sql/workbench/contrib/notebook/test/emptySessionClasses'; import { NullAdsTelemetryService } from 'sql/platform/telemetry/common/adsTelemetryService'; import { CellTypes } from 'sql/workbench/services/notebook/common/contracts'; @@ -79,7 +79,8 @@ let configurationService: IConfigurationService; suite('NotebookViewModel', function (): void { let defaultViewName = 'Default New View'; - let notebookManagers = [new NotebookManagerStub()]; + let serializationManagers = [new SerializationManagerStub()]; + let executeManagers = [new ExecuteManagerStub()]; let mockSessionManager: TypeMoq.Mock; let memento: TypeMoq.Mock; let queryConnectionService: TypeMoq.Mock; @@ -239,7 +240,7 @@ suite('NotebookViewModel', function (): void { function setupServices() { mockSessionManager = TypeMoq.Mock.ofType(SessionManager); - notebookManagers[0].sessionManager = mockSessionManager.object; + executeManagers[0].sessionManager = mockSessionManager.object; notificationService = TypeMoq.Mock.ofType(TestNotificationService, TypeMoq.MockBehavior.Loose); capabilitiesService = TypeMoq.Mock.ofType(TestCapabilitiesService); memento = TypeMoq.Mock.ofType(Memento, TypeMoq.MockBehavior.Loose, ''); @@ -252,8 +253,9 @@ suite('NotebookViewModel', function (): void { defaultModelOptions = { notebookUri: defaultUri, factory: new ModelFactory(instantiationService), - notebookManagers, - contentManager: undefined, + serializationManagers: serializationManagers, + executeManagers: executeManagers, + contentLoader: undefined, notificationService: notificationService.object, connectionService: queryConnectionService.object, providerId: 'SQL', @@ -265,9 +267,9 @@ suite('NotebookViewModel', function (): void { } async function initializeNotebookViewsExtension(contents: nb.INotebookContents): Promise { - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(contents)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); await model.loadContents(); diff --git a/src/sql/workbench/contrib/notebook/test/browser/notebookViewsActions.test.ts b/src/sql/workbench/contrib/notebook/test/browser/notebookViewsActions.test.ts index 1607757db7..a2ee031ece 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/notebookViewsActions.test.ts +++ b/src/sql/workbench/contrib/notebook/test/browser/notebookViewsActions.test.ts @@ -8,10 +8,10 @@ import { ICapabilitiesService } from 'sql/platform/capabilities/common/capabilit import { TestCapabilitiesService } from 'sql/platform/capabilities/test/common/testCapabilitiesService'; import { TestConnectionManagementService } from 'sql/platform/connection/test/common/testConnectionManagementService'; import { NullAdsTelemetryService } from 'sql/platform/telemetry/common/adsTelemetryService'; -import { NotebookEditorContentManager } from 'sql/workbench/contrib/notebook/browser/models/notebookInput'; +import { NotebookEditorContentLoader } from 'sql/workbench/contrib/notebook/browser/models/notebookInput'; import { DeleteViewAction, InsertCellAction } from 'sql/workbench/contrib/notebook/browser/notebookViews/notebookViewsActions'; import { SessionManager } from 'sql/workbench/contrib/notebook/test/emptySessionClasses'; -import { NotebookManagerStub } from 'sql/workbench/contrib/notebook/test/stubs'; +import { ExecuteManagerStub, SerializationManagerStub } from 'sql/workbench/contrib/notebook/test/stubs'; import { ModelFactory } from 'sql/workbench/services/notebook/browser/models/modelFactory'; import { ICellModel, INotebookModelOptions, ViewMode } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; import { NotebookModel } from 'sql/workbench/services/notebook/browser/models/notebookModel'; @@ -58,7 +58,8 @@ let initialNotebookContent: nb.INotebookContents = { suite('Notebook Views Actions', function (): void { let defaultViewName = 'Default New View'; - let notebookManagers = [new NotebookManagerStub()]; + let serializationManagers = [new SerializationManagerStub()]; + let executeManagers = [new ExecuteManagerStub()]; let mockSessionManager: TypeMoq.Mock; let memento: TypeMoq.Mock; let queryConnectionService: TypeMoq.Mock; @@ -164,7 +165,7 @@ suite('Notebook Views Actions', function (): void { function setupServices() { mockSessionManager = TypeMoq.Mock.ofType(SessionManager); - notebookManagers[0].sessionManager = mockSessionManager.object; + executeManagers[0].sessionManager = mockSessionManager.object; notificationService = TypeMoq.Mock.ofType(TestNotificationService, TypeMoq.MockBehavior.Loose); capabilitiesService = TypeMoq.Mock.ofType(TestCapabilitiesService); memento = TypeMoq.Mock.ofType(Memento, TypeMoq.MockBehavior.Loose, ''); @@ -177,8 +178,9 @@ suite('Notebook Views Actions', function (): void { defaultModelOptions = { notebookUri: defaultUri, factory: new ModelFactory(instantiationService), - notebookManagers, - contentManager: undefined, + serializationManagers: serializationManagers, + executeManagers: executeManagers, + contentLoader: undefined, notificationService: notificationService.object, connectionService: queryConnectionService.object, providerId: 'SQL', @@ -190,9 +192,9 @@ suite('Notebook Views Actions', function (): void { } async function initializeNotebookViewsExtension(contents: nb.INotebookContents): Promise { - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(contents)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); await model.loadContents(); diff --git a/src/sql/workbench/contrib/notebook/test/browser/notebookViewsExtension.test.ts b/src/sql/workbench/contrib/notebook/test/browser/notebookViewsExtension.test.ts index 0513db6a48..bf630b4445 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/notebookViewsExtension.test.ts +++ b/src/sql/workbench/contrib/notebook/test/browser/notebookViewsExtension.test.ts @@ -11,7 +11,7 @@ import { INotificationService } from 'vs/platform/notification/common/notificati import { TestNotificationService } from 'vs/platform/notification/test/common/testNotificationService'; import { URI } from 'vs/base/common/uri'; -import { NotebookManagerStub } from 'sql/workbench/contrib/notebook/test/stubs'; +import { ExecuteManagerStub, SerializationManagerStub } from 'sql/workbench/contrib/notebook/test/stubs'; import { NotebookModel } from 'sql/workbench/services/notebook/browser/models/notebookModel'; import { ModelFactory } from 'sql/workbench/services/notebook/browser/models/modelFactory'; import { INotebookModelOptions } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; @@ -24,7 +24,7 @@ import { InstantiationService } from 'vs/platform/instantiation/common/instantia import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection'; import { NullLogService } from 'vs/platform/log/common/log'; import { TestConnectionManagementService } from 'sql/platform/connection/test/common/testConnectionManagementService'; -import { NotebookEditorContentManager } from 'sql/workbench/contrib/notebook/browser/models/notebookInput'; +import { NotebookEditorContentLoader } from 'sql/workbench/contrib/notebook/browser/models/notebookInput'; import { SessionManager } from 'sql/workbench/contrib/notebook/test/emptySessionClasses'; import { NullAdsTelemetryService } from 'sql/platform/telemetry/common/adsTelemetryService'; import { CellTypes } from 'sql/workbench/services/notebook/common/contracts'; @@ -62,7 +62,8 @@ let configurationService: IConfigurationService; suite('NotebookViews', function (): void { let defaultViewName = 'Default New View'; - let notebookManagers = [new NotebookManagerStub()]; + let serializationManagers = [new SerializationManagerStub()]; + let executeManagers = [new ExecuteManagerStub()]; let mockSessionManager: TypeMoq.Mock; let memento: TypeMoq.Mock; let queryConnectionService: TypeMoq.Mock; @@ -131,7 +132,7 @@ suite('NotebookViews', function (): void { function setupServices() { mockSessionManager = TypeMoq.Mock.ofType(SessionManager); - notebookManagers[0].sessionManager = mockSessionManager.object; + executeManagers[0].sessionManager = mockSessionManager.object; notificationService = TypeMoq.Mock.ofType(TestNotificationService, TypeMoq.MockBehavior.Loose); capabilitiesService = TypeMoq.Mock.ofType(TestCapabilitiesService); memento = TypeMoq.Mock.ofType(Memento, TypeMoq.MockBehavior.Loose, ''); @@ -144,8 +145,9 @@ suite('NotebookViews', function (): void { defaultModelOptions = { notebookUri: defaultUri, factory: new ModelFactory(instantiationService), - notebookManagers, - contentManager: undefined, + serializationManagers: serializationManagers, + executeManagers: executeManagers, + contentLoader: undefined, notificationService: notificationService.object, connectionService: queryConnectionService.object, providerId: 'SQL', @@ -157,9 +159,9 @@ suite('NotebookViews', function (): void { } async function initializeExtension(): Promise { - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(initialNotebookContent)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); await model.loadContents(); diff --git a/src/sql/workbench/contrib/notebook/test/electron-browser/clientSession.test.ts b/src/sql/workbench/contrib/notebook/test/electron-browser/clientSession.test.ts index b8fb074a33..7bed27c6fa 100644 --- a/src/sql/workbench/contrib/notebook/test/electron-browser/clientSession.test.ts +++ b/src/sql/workbench/contrib/notebook/test/electron-browser/clientSession.test.ts @@ -13,12 +13,12 @@ import { URI } from 'vs/base/common/uri'; import { ClientSession } from 'sql/workbench/services/notebook/browser/models/clientSession'; import { SessionManager, EmptySession } from 'sql/workbench/contrib/notebook/test/emptySessionClasses'; -import { NotebookManagerStub, ServerManagerStub } from 'sql/workbench/contrib/notebook/test/stubs'; +import { ExecuteManagerStub, ServerManagerStub } from 'sql/workbench/contrib/notebook/test/stubs'; import { isUndefinedOrNull } from 'vs/base/common/types'; suite('Client Session', function (): void { let path = URI.file('my/notebook.ipynb'); - let notebookManager: NotebookManagerStub; + let notebookManager: ExecuteManagerStub; let serverManager: ServerManagerStub; let mockSessionManager: TypeMoq.Mock; let notificationService: TypeMoq.Mock; @@ -27,19 +27,19 @@ suite('Client Session', function (): void { setup(() => { serverManager = new ServerManagerStub(); mockSessionManager = TypeMoq.Mock.ofType(SessionManager); - notebookManager = new NotebookManagerStub(); + notebookManager = new ExecuteManagerStub(); notebookManager.serverManager = serverManager; notebookManager.sessionManager = mockSessionManager.object; notificationService = TypeMoq.Mock.ofType(TestNotificationService, TypeMoq.MockBehavior.Loose); session = new ClientSession({ - notebookManager: notebookManager, + executeManager: notebookManager, notebookUri: path, notificationService: notificationService.object, kernelSpec: { name: 'python', display_name: 'Python 3', language: 'python' } }); - let serverlessNotebookManager = new NotebookManagerStub(); + let serverlessNotebookManager = new ExecuteManagerStub(); serverlessNotebookManager.sessionManager = mockSessionManager.object; }); @@ -169,7 +169,7 @@ suite('Client Session', function (): void { let remoteSession = new ClientSession({ kernelSpec: { name: 'python', display_name: 'Python 3', language: 'python' }, - notebookManager: newNotebookManager, + executeManager: newNotebookManager, notebookUri: path, notificationService: notificationService.object }); diff --git a/src/sql/workbench/contrib/notebook/test/electron-browser/contentManagers.test.ts b/src/sql/workbench/contrib/notebook/test/electron-browser/contentManagers.test.ts index 65792d2fdb..ef03fdba5b 100644 --- a/src/sql/workbench/contrib/notebook/test/electron-browser/contentManagers.test.ts +++ b/src/sql/workbench/contrib/notebook/test/electron-browser/contentManagers.test.ts @@ -10,14 +10,12 @@ import * as fs from 'fs'; import * as pfs from 'vs/base/node/pfs'; import { URI } from 'vs/base/common/uri'; -import * as tempWrite from 'temp-write'; import { LocalContentManager } from 'sql/workbench/services/notebook/common/localContentManager'; import { CellTypes } from 'sql/workbench/services/notebook/common/contracts'; import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; import { TestFileService } from 'vs/workbench/test/browser/workbenchTestServices'; import { IFileService, IReadFileOptions, IFileContent, IWriteFileOptions, IFileStatWithMetadata } from 'vs/platform/files/common/files'; import { VSBuffer, VSBufferReadable } from 'vs/base/common/buffer'; -import { isUndefinedOrNull } from 'vs/base/common/types'; import { promisify } from 'util'; let expectedNotebookContent: nb.INotebookContents = { @@ -69,34 +67,15 @@ suite('Local Content Manager', function (): void { contentManager = instantiationService.createInstance(LocalContentManager); }); - test('Should return undefined if path is undefined', async function (): Promise { - let content = await contentManager.getNotebookContents(undefined); - assert(isUndefinedOrNull(content)); - // tslint:disable-next-line:no-null-keyword - content = await contentManager.getNotebookContents(null); - assert(isUndefinedOrNull(content)); - }); - - test('Should throw if file does not exist', async function (): Promise { - try { - await contentManager.getNotebookContents(URI.file('/path/doesnot/exist.ipynb')); - assert.fail('expected to throw'); - } catch (e) { } - }); test('Should return notebook contents parsed as INotebook when valid notebook file parsed', async function (): Promise { - // Given a file containing a valid notebook - let localFile = tempWrite.sync(notebookContentString, 'notebook.ipynb'); - // when I read the content - let notebook = await contentManager.getNotebookContents(URI.file(localFile)); + let notebook = await contentManager.deserializeNotebook(notebookContentString); // then I expect notebook format to match verifyMatchesExpectedNotebook(notebook); }); test('Should ignore invalid content in the notebook file', async function (): Promise { // Given a file containing a notebook with some garbage properties let invalidContent = notebookContentString + '\\nasddfdsafasdf'; - let localFile = tempWrite.sync(invalidContent, 'notebook.ipynb'); - // when I read the content - let notebook = await contentManager.getNotebookContents(URI.file(localFile)); + let notebook = await contentManager.deserializeNotebook(invalidContent); // then I expect notebook format to still be valid verifyMatchesExpectedNotebook(notebook); }); @@ -130,10 +109,8 @@ suite('Local Content Manager', function (): void { nbformat_minor: 2 }; let mimeContentString = JSON.stringify(mimeNotebook); - // Given a file containing a valid notebook with multiline mime type - let localFile = tempWrite.sync(mimeContentString, 'notebook.ipynb'); // when I read the content - let notebook = await contentManager.getNotebookContents(URI.file(localFile)); + let notebook = await contentManager.deserializeNotebook(mimeContentString); // then I expect output to have been normalized into a single string let displayOutput = notebook.cells[0].outputs[0]; assert.strictEqual(displayOutput.data['text/html'], '
'); @@ -141,7 +118,7 @@ suite('Local Content Manager', function (): void { test('Should create a new empty notebook if content is undefined', async function (): Promise { // verify that when loading content from an empty string, a new notebook is created. - let content = await contentManager.loadFromContentString(undefined); + let content = await contentManager.deserializeNotebook(undefined); assert.strictEqual(content.metadata, undefined, 'Verify that metadata is undefined'); // verify that the notebook is empty assert.strictEqual(content.cells.length, 0, 'Notebook should be empty, so the number of cells should be 0'); @@ -149,12 +126,22 @@ suite('Local Content Manager', function (): void { test('Should create a new empty notebook if content is an empty string', async function (): Promise { // verify that when loading content from an empty string, a new notebook is created. - let content = await contentManager.loadFromContentString(''); + let content = await contentManager.deserializeNotebook(''); assert.strictEqual(content.metadata, undefined, 'Verify that metadata is undefined'); // verify that the notebook is empty assert.strictEqual(content.cells.length, 0, 'Notebook should be empty, so the number of cells should be 0'); }); + test('Should return undefined if notebook contents are undefined', async function (): Promise { + let strContent = await contentManager.serializeNotebook(undefined); + assert.strictEqual(strContent, undefined); + }); + + test('Should return stringified version of notebook contents', async function (): Promise { + let strContent = await contentManager.serializeNotebook(expectedNotebookContent); + assert.strictEqual(strContent, JSON.stringify(expectedNotebookContent, undefined, ' ')); + }); + test('Should create a markdown cell', async function (): Promise { let expectedNotebookMarkdownContent: nb.INotebookContents = { cells: [{ @@ -173,7 +160,7 @@ suite('Local Content Manager', function (): void { }; let markdownNotebookContent = JSON.stringify(expectedNotebookMarkdownContent); // verify that notebooks support markdown cells - let notebook = await contentManager.loadFromContentString(markdownNotebookContent); + let notebook = await contentManager.deserializeNotebook(markdownNotebookContent); // assert that markdown cell is supported by // verifying the notebook matches the expectedNotebookMarkdownContent format assert.strictEqual(notebook.cells.length, 1, 'The number of cells should be equal to 1'); @@ -207,7 +194,7 @@ suite('Local Content Manager', function (): void { }; let streamOutputContent = JSON.stringify(expectedNotebookStreamOutputContent); // Verify that the stream output type is supported - let notebook = await contentManager.loadFromContentString(streamOutputContent); + let notebook = await contentManager.deserializeNotebook(streamOutputContent); assert.strictEqual(notebook.cells[0].outputs[0].output_type, 'stream', 'Cell output from notebook should be stream'); assert.strictEqual(notebook.cells[0].cell_type, expectedNotebookStreamOutputContent.cells[0].cell_type, 'Cell type of notebook should match the expectedNotebookStreamOutputContent'); }); diff --git a/src/sql/workbench/contrib/notebook/test/electron-browser/notebookEditorModel.test.ts b/src/sql/workbench/contrib/notebook/test/electron-browser/notebookEditorModel.test.ts index 27c11f735d..1a8da8e6aa 100644 --- a/src/sql/workbench/contrib/notebook/test/electron-browser/notebookEditorModel.test.ts +++ b/src/sql/workbench/contrib/notebook/test/electron-browser/notebookEditorModel.test.ts @@ -32,7 +32,7 @@ import { TestLifecycleService, TestTextFileService, workbenchInstantiationServic import { Range } from 'vs/editor/common/core/range'; import { nb } from 'azdata'; import { Emitter } from 'vs/base/common/event'; -import { INotebookEditor, INotebookManager } from 'sql/workbench/services/notebook/browser/notebookService'; +import { INotebookEditor, IExecuteManager, ISerializationManager } from 'sql/workbench/services/notebook/browser/notebookService'; import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService'; import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; import { IStorageService } from 'vs/platform/storage/common/storage'; @@ -50,9 +50,13 @@ class ServiceAccessor { } } -class NotebookManagerStub implements INotebookManager { +class SerializationManagerStub implements ISerializationManager { providerId: string; contentManager: nb.ContentManager; +} + +class ExecuteManagerStub implements IExecuteManager { + providerId: string; sessionManager: nb.SessionManager; serverManager: nb.ServerManager; } @@ -62,7 +66,8 @@ let defaultUri = URI.file('/some/path.ipynb'); // Note: these tests are intentionally written to be extremely brittle and break on any changes to notebook/cell serialization changes. // If any of these tests fail, it is likely that notebook editor rehydration will fail with cryptic JSON messages. suite('Notebook Editor Model', function (): void { - let notebookManagers = [new NotebookManagerStub()]; + let serializationManagers = [new SerializationManagerStub()]; + let executeManagers = [new ExecuteManagerStub()]; let notebookModel: NotebookModel; const instantiationService: IInstantiationService = workbenchInstantiationService(); let accessor: ServiceAccessor; @@ -156,8 +161,9 @@ suite('Notebook Editor Model', function (): void { defaultModelOptions = { notebookUri: defaultUri, factory: new ModelFactory(instantiationService), - notebookManagers, - contentManager: undefined, + serializationManagers: serializationManagers, + executeManagers: executeManagers, + contentLoader: undefined, notificationService: notificationService.object, connectionService: queryConnectionService.object, providerId: 'SQL', @@ -197,7 +203,7 @@ suite('Notebook Editor Model', function (): void { cells: [newCell], cellIndex: 0 }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); assert(notebookEditorModel.lastEditFullReplacement); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(8), ' "source": ['); @@ -221,7 +227,7 @@ suite('Notebook Editor Model', function (): void { cells: [newCell], cellIndex: 0 }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); assert(notebookEditorModel.lastEditFullReplacement); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(25), ' "execution_count": null'); @@ -233,7 +239,7 @@ suite('Notebook Editor Model', function (): void { cellIndex: 0 }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellExecuted); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellExecuted); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(8), ' "source": ['); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(12), ' "azdata_cell_guid": "' + newCell.cellGuid + '"'); @@ -250,7 +256,7 @@ suite('Notebook Editor Model', function (): void { cellIndex: 0 }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellExecuted); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellExecuted); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(25), ' "execution_count": 10'); assert(!notebookEditorModel.lastEditFullReplacement); @@ -261,7 +267,7 @@ suite('Notebook Editor Model', function (): void { cellIndex: 0 }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellExecuted); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellExecuted); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(25), ' "execution_count": 15'); assert(!notebookEditorModel.lastEditFullReplacement); @@ -272,7 +278,7 @@ suite('Notebook Editor Model', function (): void { cellIndex: 0 }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellExecuted); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellExecuted); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(25), ' "execution_count": 105'); assert(!notebookEditorModel.lastEditFullReplacement); }); @@ -289,7 +295,7 @@ suite('Notebook Editor Model', function (): void { cells: [newCell], cellIndex: 0 }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); assert(notebookEditorModel.lastEditFullReplacement); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(14), ' "outputs": ['); @@ -300,7 +306,7 @@ suite('Notebook Editor Model', function (): void { cellIndex: 0 }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellOutputCleared); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellOutputCleared); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(8), ' "source": ['); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(12), ' "azdata_cell_guid": "' + newCell.cellGuid + '"'); @@ -323,7 +329,7 @@ suite('Notebook Editor Model', function (): void { cells: [newCell], cellIndex: 0 }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); assert(notebookEditorModel.lastEditFullReplacement); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(14), ' "outputs": ['); @@ -342,7 +348,7 @@ suite('Notebook Editor Model', function (): void { } }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(8), ' "source": ['); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(9), ' "This is a test\\n",'); @@ -369,7 +375,7 @@ suite('Notebook Editor Model', function (): void { cells: [newCell], cellIndex: 0 }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); assert(notebookEditorModel.lastEditFullReplacement); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(14), ' "outputs": ['); @@ -388,7 +394,7 @@ suite('Notebook Editor Model', function (): void { } }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(8), ' "source": ['); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(9), ' "This is a test"'); @@ -413,7 +419,7 @@ suite('Notebook Editor Model', function (): void { cells: [newCell], cellIndex: 0 }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); assert(notebookEditorModel.lastEditFullReplacement); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(14), ' "outputs": ['); @@ -427,7 +433,7 @@ suite('Notebook Editor Model', function (): void { modelContentChangedEvent: undefined }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); assert(!notebookEditorModel.lastEditFullReplacement, 'should not do a full replacement for a source update'); @@ -453,7 +459,7 @@ suite('Notebook Editor Model', function (): void { cells: [newCell], cellIndex: 0 }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); assert(notebookEditorModel.lastEditFullReplacement); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(14), ' "outputs": ['); @@ -467,7 +473,7 @@ suite('Notebook Editor Model', function (): void { modelContentChangedEvent: undefined }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); assert(!notebookEditorModel.lastEditFullReplacement, 'should not do a full replacement for a source update'); @@ -494,7 +500,7 @@ suite('Notebook Editor Model', function (): void { cells: [newCell], cellIndex: 0 }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); assert(notebookEditorModel.lastEditFullReplacement); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(8), ' "source": ['); @@ -516,7 +522,7 @@ suite('Notebook Editor Model', function (): void { } }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); assert(!notebookEditorModel.lastEditFullReplacement); contentChange = { @@ -533,7 +539,7 @@ suite('Notebook Editor Model', function (): void { } }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(8), ' "source": ['); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(9), ' ""'); @@ -558,7 +564,7 @@ suite('Notebook Editor Model', function (): void { cells: [newCell], cellIndex: 0 }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(14), ' "outputs": ['); @@ -576,7 +582,7 @@ suite('Notebook Editor Model', function (): void { } }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); assert(!notebookEditorModel.lastEditFullReplacement); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(8), ' "source": ['); @@ -600,7 +606,7 @@ suite('Notebook Editor Model', function (): void { } }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); assert(!notebookEditorModel.lastEditFullReplacement); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(8), ' "source": ['); @@ -630,7 +636,7 @@ suite('Notebook Editor Model', function (): void { cells: [cell], cellIndex: 0 }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); assert(notebookEditorModel.lastEditFullReplacement); } @@ -647,7 +653,7 @@ suite('Notebook Editor Model', function (): void { } }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); assert(!notebookEditorModel.lastEditFullReplacement); for (let i = 0; i < 10; i++) { @@ -677,7 +683,7 @@ suite('Notebook Editor Model', function (): void { cells: [newCell], cellIndex: 0 }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); assert(notebookEditorModel.lastEditFullReplacement); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(14), ' "outputs": ['); @@ -689,7 +695,7 @@ suite('Notebook Editor Model', function (): void { cells: [newCell] }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellOutputUpdated); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellOutputUpdated); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(8), ' "source": ['); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(12), ' "azdata_cell_guid": "' + newCell.cellGuid + '"'); @@ -714,7 +720,7 @@ suite('Notebook Editor Model', function (): void { cells: [newCell], cellIndex: 0 }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); assert(notebookEditorModel.lastEditFullReplacement); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(14), ' "outputs": ['); @@ -728,7 +734,7 @@ suite('Notebook Editor Model', function (): void { cells: [newCell] }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellOutputUpdated); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellOutputUpdated); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(8), ' "source": ['); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(12), ' "azdata_cell_guid": "' + newCell.cellGuid + '"'); @@ -750,7 +756,7 @@ suite('Notebook Editor Model', function (): void { cells: [newCell] }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellOutputUpdated); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellOutputUpdated); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(32), ' "text": "test test test"'); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(33), ' }'); @@ -779,7 +785,7 @@ suite('Notebook Editor Model', function (): void { cellIndex: 0 }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); assert(notebookEditorModel.lastEditFullReplacement); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(14), ' "outputs": [],'); @@ -792,7 +798,7 @@ suite('Notebook Editor Model', function (): void { cells: [newCell] }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellOutputUpdated); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellOutputUpdated); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(8), ' "source": ['); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(12), ' "azdata_cell_guid": "' + newCell.cellGuid + '"'); @@ -810,9 +816,9 @@ suite('Notebook Editor Model', function (): void { notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); let newCell = notebookModel.addCell(CellTypes.Code); - setupTextEditorModelWithEmptyOutputs(notebookEditorModel, newCell); + await setupTextEditorModelWithEmptyOutputs(notebookEditorModel, newCell); - addTextToBeginningOfTextEditorModel(notebookEditorModel, newCell, '"This text is in quotes"'); + await addTextToBeginningOfTextEditorModel(notebookEditorModel, newCell, '"This text is in quotes"'); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(9), ' "\\"This text is in quotes\\""'); ensureStaticContentInOneLineCellIsCorrect(notebookEditorModel, newCell); @@ -825,9 +831,9 @@ suite('Notebook Editor Model', function (): void { notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); let newCell = notebookModel.addCell(CellTypes.Code); - setupTextEditorModelWithEmptyOutputs(notebookEditorModel, newCell); + await setupTextEditorModelWithEmptyOutputs(notebookEditorModel, newCell); - addTextToBeginningOfTextEditorModel(notebookEditorModel, newCell, '""""""""""'); + await addTextToBeginningOfTextEditorModel(notebookEditorModel, newCell, '""""""""""'); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(9), ' "\\"\\"\\"\\"\\"\\"\\"\\"\\"\\""'); ensureStaticContentInOneLineCellIsCorrect(notebookEditorModel, newCell); @@ -840,9 +846,9 @@ suite('Notebook Editor Model', function (): void { notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); let newCell = notebookModel.addCell(CellTypes.Code); - setupTextEditorModelWithEmptyOutputs(notebookEditorModel, newCell); + await setupTextEditorModelWithEmptyOutputs(notebookEditorModel, newCell); - addTextToBeginningOfTextEditorModel(notebookEditorModel, newCell, '\\\\\\\\\\'); + await addTextToBeginningOfTextEditorModel(notebookEditorModel, newCell, '\\\\\\\\\\'); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(9), ' "\\\\\\\\\\\\\\\\\\\\\"'); ensureStaticContentInOneLineCellIsCorrect(notebookEditorModel, newCell); @@ -855,9 +861,9 @@ suite('Notebook Editor Model', function (): void { notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); let newCell = notebookModel.addCell(CellTypes.Code); - setupTextEditorModelWithEmptyOutputs(notebookEditorModel, newCell); + await setupTextEditorModelWithEmptyOutputs(notebookEditorModel, newCell); - addTextToBeginningOfTextEditorModel(notebookEditorModel, newCell, '\"\"\"\"\"\"\"\"\"\"'); + await addTextToBeginningOfTextEditorModel(notebookEditorModel, newCell, '\"\"\"\"\"\"\"\"\"\"'); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(9), ' "\\\"\\\"\\\"\\\"\\\"\\\"\\\"\\\"\\\"\\\""'); ensureStaticContentInOneLineCellIsCorrect(notebookEditorModel, newCell); @@ -870,9 +876,9 @@ suite('Notebook Editor Model', function (): void { notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); let newCell = notebookModel.addCell(CellTypes.Code); - setupTextEditorModelWithEmptyOutputs(notebookEditorModel, newCell); + await setupTextEditorModelWithEmptyOutputs(notebookEditorModel, newCell); - addTextToBeginningOfTextEditorModel(notebookEditorModel, newCell, 'this is a long line in a cell test. Everything should serialize correctly! # Comments here: adding more tests is fun?'); + await addTextToBeginningOfTextEditorModel(notebookEditorModel, newCell, 'this is a long line in a cell test. Everything should serialize correctly! # Comments here: adding more tests is fun?'); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(9), ' "this is a long line in a cell test. Everything should serialize correctly! # Comments here: adding more tests is fun?"'); ensureStaticContentInOneLineCellIsCorrect(notebookEditorModel, newCell); @@ -885,9 +891,9 @@ suite('Notebook Editor Model', function (): void { notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); let newCell = notebookModel.addCell(CellTypes.Code); - setupTextEditorModelWithEmptyOutputs(notebookEditorModel, newCell); + await setupTextEditorModelWithEmptyOutputs(notebookEditorModel, newCell); - addTextToBeginningOfTextEditorModel(notebookEditorModel, newCell, '`~1!2@3#4$5%6^7&8*9(0)-_=+[{]}\\|;:",<.>/?\''); + await addTextToBeginningOfTextEditorModel(notebookEditorModel, newCell, '`~1!2@3#4$5%6^7&8*9(0)-_=+[{]}\\|;:",<.>/?\''); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(9), ' "`~1!2@3#4$5%6^7&8*9(0)-_=+[{]}\\\\|;:\\",<.>/?\'"'); ensureStaticContentInOneLineCellIsCorrect(notebookEditorModel, newCell); @@ -900,9 +906,9 @@ suite('Notebook Editor Model', function (): void { notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); let newCell = notebookModel.addCell(CellTypes.Code); - setupTextEditorModelWithEmptyOutputs(notebookEditorModel, newCell); + await setupTextEditorModelWithEmptyOutputs(notebookEditorModel, newCell); - addTextToBeginningOfTextEditorModel(notebookEditorModel, newCell, '\'\'\'\''); + await addTextToBeginningOfTextEditorModel(notebookEditorModel, newCell, '\'\'\'\''); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(9), ' "\'\'\'\'"'); ensureStaticContentInOneLineCellIsCorrect(notebookEditorModel, newCell); @@ -915,9 +921,9 @@ suite('Notebook Editor Model', function (): void { notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); let newCell = notebookModel.addCell(CellTypes.Code); - setupTextEditorModelWithEmptyOutputs(notebookEditorModel, newCell); + await setupTextEditorModelWithEmptyOutputs(notebookEditorModel, newCell); - addTextToBeginningOfTextEditorModel(notebookEditorModel, newCell, ''); + await addTextToBeginningOfTextEditorModel(notebookEditorModel, newCell, ''); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(9), ' ""'); ensureStaticContentInOneLineCellIsCorrect(notebookEditorModel, newCell); @@ -930,9 +936,9 @@ suite('Notebook Editor Model', function (): void { notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); let newCell = notebookModel.addCell(CellTypes.Code); - setupTextEditorModelWithEmptyOutputs(notebookEditorModel, newCell); + await setupTextEditorModelWithEmptyOutputs(notebookEditorModel, newCell); - addTextToBeginningOfTextEditorModel(notebookEditorModel, newCell, '"test"' + os.EOL + 'test""'); + await addTextToBeginningOfTextEditorModel(notebookEditorModel, newCell, '"test"' + os.EOL + 'test""'); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(9), ' "\\"test\\"\\n",'); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(8), ' "source": ['); @@ -952,9 +958,9 @@ suite('Notebook Editor Model', function (): void { notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); let newCell = notebookModel.addCell(CellTypes.Code); - setupTextEditorModelWithEmptyOutputs(notebookEditorModel, newCell); + await setupTextEditorModelWithEmptyOutputs(notebookEditorModel, newCell); - addTextToBeginningOfTextEditorModel(notebookEditorModel, newCell, '"""""test"' + os.EOL + '"""""""test\\""'); + await addTextToBeginningOfTextEditorModel(notebookEditorModel, newCell, '"""""test"' + os.EOL + '"""""""test\\""'); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(9), ' "\\"\\"\\"\\"\\"test\\"\\n",'); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(8), ' "source": ['); @@ -983,7 +989,7 @@ suite('Notebook Editor Model', function (): void { return new NotebookEditorModel(defaultUri, textFileEditorModel, mockNotebookService.object, testResourcePropertiesService); } - function setupTextEditorModelWithEmptyOutputs(notebookEditorModel: NotebookEditorModel, newCell: ICellModel) { + async function setupTextEditorModelWithEmptyOutputs(notebookEditorModel: NotebookEditorModel, newCell: ICellModel) { // clear outputs newCell['_outputs'] = []; @@ -993,13 +999,13 @@ suite('Notebook Editor Model', function (): void { cellIndex: 0 }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); assert(notebookEditorModel.lastEditFullReplacement); assert.strictEqual(notebookEditorModel.editorModel.textEditorModel.getLineContent(14), ' "outputs": [],'); } - function addTextToBeginningOfTextEditorModel(notebookEditorModel: NotebookEditorModel, newCell: ICellModel, textToAdd: string) { + async function addTextToBeginningOfTextEditorModel(notebookEditorModel: NotebookEditorModel, newCell: ICellModel, textToAdd: string) { let contentChange: NotebookContentChange = { changeType: NotebookChangeType.CellSourceUpdated, cells: [newCell], @@ -1014,7 +1020,7 @@ suite('Notebook Editor Model', function (): void { } }; - notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); + await notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); } function ensureStaticContentInOneLineCellIsCorrect(notebookEditorModel: NotebookEditorModel, newCell: ICellModel) { diff --git a/src/sql/workbench/contrib/notebook/test/electron-browser/notebookFindModel.test.ts b/src/sql/workbench/contrib/notebook/test/electron-browser/notebookFindModel.test.ts index 93412a6b3b..84a70698e6 100644 --- a/src/sql/workbench/contrib/notebook/test/electron-browser/notebookFindModel.test.ts +++ b/src/sql/workbench/contrib/notebook/test/electron-browser/notebookFindModel.test.ts @@ -7,7 +7,7 @@ import { nb } from 'azdata'; import * as assert from 'assert'; import { URI } from 'vs/base/common/uri'; -import { NotebookManagerStub } from 'sql/workbench/contrib/notebook/test/stubs'; +import { ExecuteManagerStub, SerializationManagerStub } from 'sql/workbench/contrib/notebook/test/stubs'; import { CellTypes } from 'sql/workbench/services/notebook/common/contracts'; import { IClientSession, INotebookModelOptions } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; import { NotebookModel } from 'sql/workbench/services/notebook/browser/models/notebookModel'; @@ -26,7 +26,7 @@ import { ServiceCollection } from 'vs/platform/instantiation/common/serviceColle import { InstantiationService } from 'vs/platform/instantiation/common/instantiationService'; import { ClientSession } from 'sql/workbench/services/notebook/browser/models/clientSession'; import { TestStorageService } from 'vs/workbench/test/common/workbenchTestServices'; -import { NotebookEditorContentManager } from 'sql/workbench/contrib/notebook/browser/models/notebookInput'; +import { NotebookEditorContentLoader } from 'sql/workbench/contrib/notebook/browser/models/notebookInput'; import { NotebookRange } from 'sql/workbench/services/notebook/browser/notebookService'; import { NotebookMarkdownRenderer } from 'sql/workbench/contrib/notebook/browser/outputs/notebookMarkdown'; import { NullAdsTelemetryService } from 'sql/platform/telemetry/common/adsTelemetryService'; @@ -67,8 +67,8 @@ let instantiationService: IInstantiationService; let serviceCollection = new ServiceCollection(); suite('Notebook Find Model', function (): void { - - let notebookManagers = [new NotebookManagerStub()]; + let serializationManagers = [new SerializationManagerStub()]; + let executeManagers = [new ExecuteManagerStub()]; let memento: TypeMoq.Mock; let queryConnectionService: TypeMoq.Mock; let defaultModelOptions: INotebookModelOptions; @@ -92,8 +92,9 @@ suite('Notebook Find Model', function (): void { defaultModelOptions = { notebookUri: defaultUri, factory: new ModelFactory(instantiationService), - notebookManagers, - contentManager: undefined, + serializationManagers: serializationManagers, + executeManagers: executeManagers, + contentLoader: undefined, notificationService: notificationService.object, connectionService: queryConnectionService.object, providerId: 'SQL', @@ -434,9 +435,9 @@ suite('Notebook Find Model', function (): void { async function initNotebookModel(contents: nb.INotebookContents): Promise { - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(contents)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; // Initialize the model model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); await model.loadContents(); diff --git a/src/sql/workbench/contrib/notebook/test/electron-browser/notebookModel.test.ts b/src/sql/workbench/contrib/notebook/test/electron-browser/notebookModel.test.ts index f2188a03a2..1bd215a6cd 100644 --- a/src/sql/workbench/contrib/notebook/test/electron-browser/notebookModel.test.ts +++ b/src/sql/workbench/contrib/notebook/test/electron-browser/notebookModel.test.ts @@ -12,7 +12,7 @@ import { INotificationService } from 'vs/platform/notification/common/notificati import { TestNotificationService } from 'vs/platform/notification/test/common/testNotificationService'; import { URI } from 'vs/base/common/uri'; -import { NotebookManagerStub } from 'sql/workbench/contrib/notebook/test/stubs'; +import { ExecuteManagerStub, SerializationManagerStub } from 'sql/workbench/contrib/notebook/test/stubs'; import { NotebookModel } from 'sql/workbench/services/notebook/browser/models/notebookModel'; import { ModelFactory } from 'sql/workbench/services/notebook/browser/models/modelFactory'; import { IClientSession, INotebookModelOptions, NotebookContentChange, IClientSessionOptions, ICellModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; @@ -29,7 +29,7 @@ import { ServiceCollection } from 'vs/platform/instantiation/common/serviceColle import { NullLogService } from 'vs/platform/log/common/log'; import { TestConnectionManagementService } from 'sql/platform/connection/test/common/testConnectionManagementService'; import { isUndefinedOrNull } from 'vs/base/common/types'; -import { NotebookEditorContentManager } from 'sql/workbench/contrib/notebook/browser/models/notebookInput'; +import { NotebookEditorContentLoader } from 'sql/workbench/contrib/notebook/browser/models/notebookInput'; import { SessionManager } from 'sql/workbench/contrib/notebook/test/emptySessionClasses'; import { mssqlProviderName } from 'sql/platform/connection/common/constants'; import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile'; @@ -138,7 +138,8 @@ let instantiationService: IInstantiationService; let configurationService: IConfigurationService; suite('notebook model', function (): void { - let notebookManagers = [new NotebookManagerStub()]; + let serializationManagers = [new SerializationManagerStub()]; + let executeManagers = [new ExecuteManagerStub()]; let mockSessionManager: TypeMoq.Mock; let memento: TypeMoq.Mock; let queryConnectionService: TypeMoq.Mock; @@ -146,7 +147,7 @@ suite('notebook model', function (): void { const logService = new NullLogService(); setup(() => { mockSessionManager = TypeMoq.Mock.ofType(SessionManager); - notebookManagers[0].sessionManager = mockSessionManager.object; + executeManagers[0].sessionManager = mockSessionManager.object; sessionReady = new Deferred(); notificationService = TypeMoq.Mock.ofType(TestNotificationService, TypeMoq.MockBehavior.Loose); capabilitiesService = new TestCapabilitiesService(); @@ -160,8 +161,9 @@ suite('notebook model', function (): void { defaultModelOptions = { notebookUri: defaultUri, factory: new ModelFactory(instantiationService), - notebookManagers, - contentManager: undefined, + serializationManagers: serializationManagers, + executeManagers: executeManagers, + contentLoader: undefined, notificationService: notificationService.object, connectionService: queryConnectionService.object, providerId: 'SQL', @@ -171,7 +173,7 @@ suite('notebook model', function (): void { capabilitiesService: capabilitiesService }; clientSessionOptions = { - notebookManager: defaultModelOptions.notebookManagers[0], + executeManager: defaultModelOptions.executeManagers[0], notebookUri: defaultModelOptions.notebookUri, notificationService: notificationService.object, kernelSpec: defaultModelOptions.defaultKernel @@ -200,9 +202,9 @@ suite('notebook model', function (): void { nbformat_minor: 5 }; - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(emptyNotebook)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; // When I initialize the model let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); await model.loadContents(); @@ -216,9 +218,9 @@ suite('notebook model', function (): void { test('Should use trusted state set in model load', async function (): Promise { // Given a notebook - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; // When I initialize the model let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); await model.loadContents(true); @@ -231,9 +233,9 @@ suite('notebook model', function (): void { test('Should throw if model load fails', async function (): Promise { // Given a call to get Contents fails let error = new Error('File not found'); - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.reject(error));//.throws(error); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; // When I initalize the model // Then it should throw @@ -245,9 +247,9 @@ suite('notebook model', function (): void { test('Should convert cell info to CellModels', async function (): Promise { // Given a notebook with 2 cells - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; // When I initalize the model let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); @@ -261,18 +263,18 @@ suite('notebook model', function (): void { test('Should handle multiple notebook managers', async function (): Promise { // Given a notebook with 2 cells - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; - let defaultNotebookManager = new NotebookManagerStub(); + let defaultNotebookManager = new ExecuteManagerStub(); defaultNotebookManager.providerId = 'SQL'; - let jupyterNotebookManager = new NotebookManagerStub(); + let jupyterNotebookManager = new ExecuteManagerStub(); jupyterNotebookManager.providerId = 'jupyter'; // Setup 2 notebook managers - defaultModelOptions.notebookManagers = [defaultNotebookManager, jupyterNotebookManager]; + defaultModelOptions.executeManagers = [defaultNotebookManager, jupyterNotebookManager]; // Change default notebook provider id to jupyter defaultModelOptions.providerId = 'jupyter'; @@ -282,7 +284,7 @@ suite('notebook model', function (): void { await model.loadContents(); // I expect the default provider to be jupyter - assert.strictEqual(model.notebookManager.providerId, 'jupyter', 'Notebook manager provider id incorrect'); + assert.strictEqual(model.executeManager.providerId, 'jupyter', 'Notebook manager provider id incorrect'); // Similarly, change default notebook provider id to SQL defaultModelOptions.providerId = 'SQL'; @@ -292,13 +294,13 @@ suite('notebook model', function (): void { await model.loadContents(); // I expect the default provider to be SQL - assert.strictEqual(model.notebookManager.providerId, 'SQL', 'Notebook manager provider id incorrect after 2nd model load'); + assert.strictEqual(model.executeManager.providerId, 'SQL', 'Notebook manager provider id incorrect after 2nd model load'); // Check that the getters return the correct values - assert.strictEqual(model.notebookManagers.length, 2, 'There should be 2 notebook managers'); - assert(!isUndefinedOrNull(model.getNotebookManager('SQL')), 'SQL notebook manager is not defined'); - assert(!isUndefinedOrNull(model.getNotebookManager('jupyter')), 'Jupyter notebook manager is not defined'); - assert(isUndefinedOrNull(model.getNotebookManager('foo')), 'foo notebook manager is incorrectly defined'); + assert.strictEqual(model.executeManagers.length, 2, 'There should be 2 notebook managers'); + assert(!isUndefinedOrNull(model.getExecuteManager('SQL')), 'SQL notebook manager is not defined'); + assert(!isUndefinedOrNull(model.getExecuteManager('jupyter')), 'Jupyter notebook manager is not defined'); + assert(isUndefinedOrNull(model.getExecuteManager('foo')), 'foo notebook manager is incorrectly defined'); // Check other properties to ensure that they're returning as expected // No server manager was passed into the notebook manager stub, so expect hasServerManager to return false @@ -308,9 +310,9 @@ suite('notebook model', function (): void { test('Should set active cell correctly', async function (): Promise { // Given a notebook with 2 cells - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; // When I initalize the model let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); @@ -364,10 +366,10 @@ suite('notebook model', function (): void { }); test('Should set notebook parameter and injected parameter cell correctly', async function (): Promise { - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedParameterizedNotebookContent)); defaultModelOptions.notebookUri = defaultUri; - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; // When I initialize the model let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); @@ -392,10 +394,10 @@ suite('notebook model', function (): void { }); test('Should set notebookUri parameters to new cell correctly', async function (): Promise { - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContentOneCell)); defaultModelOptions.notebookUri = notebookUriParams; - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; // When I initialize the model let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); @@ -412,14 +414,14 @@ suite('notebook model', function (): void { }); test('Should set notebookUri parameters to new cell after parameters cell correctly', async function (): Promise { - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); let expectedNotebookContentParameterCell = expectedNotebookContentOneCell; //Set the cell to be tagged as parameter cell expectedNotebookContentParameterCell.cells[0].metadata.tags = ['parameters']; mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContentParameterCell)); defaultModelOptions.notebookUri = notebookUriParams; - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; // When I initialize the model let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); @@ -436,11 +438,11 @@ suite('notebook model', function (): void { }); test('Should set notebookUri parameters to new cell after injected parameters cell correctly', async function (): Promise { - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedParameterizedNotebookContent)); defaultModelOptions.notebookUri = notebookUriParams; - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; // When I initialize the model let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); @@ -457,9 +459,9 @@ suite('notebook model', function (): void { }); test('Should move first cell below second cell correctly', async function (): Promise { - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; // When I initialize the model let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); @@ -477,9 +479,9 @@ suite('notebook model', function (): void { }); test('Should move second cell up above the first cell correctly', async function (): Promise { - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; // When I initialize the model let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); @@ -498,9 +500,9 @@ suite('notebook model', function (): void { test('Should delete cells correctly', async function (): Promise { // Given a notebook with 2 cells - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; // When I initalize the model let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); @@ -549,9 +551,9 @@ suite('notebook model', function (): void { }); test('Should notify cell on metadata change', async function (): Promise { - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; // When I initalize the model let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); @@ -566,9 +568,9 @@ suite('notebook model', function (): void { }); test('Should set cell language correctly after cell type conversion', async function (): Promise { - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); await model.loadContents(); @@ -594,9 +596,9 @@ suite('notebook model', function (): void { }); test('Should load contents but then go to error state if client session startup fails', async function (): Promise { - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContentOneCell)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; // Given I have a session that fails to start sessionReady.resolve(); @@ -608,7 +610,7 @@ suite('notebook model', function (): void { await model.requestModelLoad(); // starting client session fails at startSessionInstance due to: // Cannot set property 'defaultKernelLoaded' of undefined - await assert.rejects(async () => { await model.startSession(notebookManagers[0]); }); + await assert.rejects(async () => { await model.startSession(executeManagers[0]); }); // Then I expect load to succeed assert.strictEqual(model.cells.length, 1); assert(model.clientSession); @@ -623,15 +625,15 @@ suite('notebook model', function (): void { let model = await loadModelAndStartClientSession(expectedNotebookContent); assert.strictEqual(model.inErrorState, false); - assert.strictEqual(model.notebookManagers.length, 1); + assert.strictEqual(model.executeManagers.length, 1); assert.deepStrictEqual(model.clientSession, mockClientSession); }); test('Should notify on trust set', async function () { // Given a notebook that's been loaded - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); await model.requestModelLoad(); @@ -697,9 +699,9 @@ suite('notebook model', function (): void { expectedNotebookContent.metadata['custom-object'] = { prop1: 'value1', prop2: 'value2' }; // Given a notebook - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedNotebookContent)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; // When I initialize the model let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, undefined, queryConnectionService.object, configurationService); await model.loadContents(); @@ -808,9 +810,9 @@ suite('notebook model', function (): void { nbformat: 4, nbformat_minor: 5 }; - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(notebook)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; // And a matching connection profile let expectedConnectionProfile = { @@ -858,9 +860,9 @@ suite('notebook model', function (): void { nbformat: 4, nbformat_minor: 5 }; - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(notebook)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; // When I initialize the model let model = new NotebookModel(defaultModelOptions, undefined, logService, undefined, new NullAdsTelemetryService(), queryConnectionService.object, configurationService); @@ -885,9 +887,9 @@ suite('notebook model', function (): void { }); test('Should keep kernel alias as language info kernel alias name even if kernel spec is seralized as SQL', async function () { - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(expectedKernelAliasNotebookContentOneCell)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; queryConnectionService.setup(c => c.getActiveConnections(TypeMoq.It.isAny())).returns(() => null); @@ -904,9 +906,9 @@ suite('notebook model', function (): void { }); async function loadModelAndStartClientSession(notebookContent: nb.INotebookContents): Promise { - let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentManager); + let mockContentManager = TypeMoq.Mock.ofType(NotebookEditorContentLoader); mockContentManager.setup(c => c.loadContent()).returns(() => Promise.resolve(notebookContent)); - defaultModelOptions.contentManager = mockContentManager.object; + defaultModelOptions.contentLoader = mockContentManager.object; queryConnectionService.setup(c => c.getActiveConnections(TypeMoq.It.isAny())).returns(() => null); @@ -921,7 +923,7 @@ suite('notebook model', function (): void { await model.requestModelLoad(); - await model.startSession(notebookManagers[0]); + await model.startSession(executeManagers[0]); // Then I expect load to succeed assert(!isUndefinedOrNull(model.clientSession), 'clientSession should exist after session is started'); diff --git a/src/sql/workbench/contrib/notebook/test/stubs.ts b/src/sql/workbench/contrib/notebook/test/stubs.ts index 1d64e258d1..340582dc66 100644 --- a/src/sql/workbench/contrib/notebook/test/stubs.ts +++ b/src/sql/workbench/contrib/notebook/test/stubs.ts @@ -8,7 +8,7 @@ import * as vsEvent from 'vs/base/common/event'; import { INotebookModel, ICellModel, IClientSession, NotebookContentChange, ISingleNotebookEditOperation, MoveDirection, ViewMode } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; import { INotebookFindModel } from 'sql/workbench/contrib/notebook/browser/models/notebookFindModel'; import { NotebookChangeType, CellType } from 'sql/workbench/services/notebook/common/contracts'; -import { INotebookManager, INotebookService, INotebookEditor, ILanguageMagic, INotebookProvider, INavigationProvider, INotebookParams, INotebookSection, ICellEditorProvider, NotebookRange } from 'sql/workbench/services/notebook/browser/notebookService'; +import { IExecuteManager, INotebookService, INotebookEditor, ILanguageMagic, IExecuteProvider, INavigationProvider, INotebookParams, INotebookSection, ICellEditorProvider, NotebookRange, ISerializationProvider, ISerializationManager } from 'sql/workbench/services/notebook/browser/notebookService'; import { IStandardKernelWithProvider } from 'sql/workbench/services/notebook/browser/models/notebookUtils'; import { IModelDecorationsChangeAccessor } from 'vs/editor/common/model'; import { NotebookFindMatch } from 'sql/workbench/contrib/notebook/browser/find/notebookFindDecorations'; @@ -47,7 +47,10 @@ export class NotebookModelStub implements INotebookModel { get sessionLoadFinished(): Promise { throw new Error('method not implemented.'); } - get notebookManagers(): INotebookManager[] { + get serializationManager(): ISerializationManager { + throw new Error('method not implemented.'); + } + get executeManagers(): IExecuteManager[] { throw new Error('method not implemented.'); } get kernelChanged(): vsEvent.Event { @@ -197,9 +200,13 @@ export class NotebookFindModelStub implements INotebookFindModel { } } -export class NotebookManagerStub implements INotebookManager { +export class SerializationManagerStub implements ISerializationManager { providerId: string; contentManager: nb.ContentManager; +} + +export class ExecuteManagerStub implements IExecuteManager { + providerId: string; sessionManager: nb.SessionManager; serverManager: nb.ServerManager; } @@ -245,10 +252,16 @@ export class NotebookServiceStub implements INotebookService { setTrusted(notebookUri: URI, isTrusted: boolean): Promise { throw new Error('Method not implemented.'); } - registerProvider(providerId: string, provider: INotebookProvider): void { + registerSerializationProvider(providerId: string, provider: ISerializationProvider): void { throw new Error('Method not implemented.'); } - unregisterProvider(providerId: string): void { + registerExecuteProvider(providerId: string, provider: IExecuteProvider): void { + throw new Error('Method not implemented.'); + } + unregisterSerializationProvider(providerId: string): void { + throw new Error('Method not implemented.'); + } + unregisterExecuteProvider(providerId: string): void { throw new Error('Method not implemented.'); } registerNavigationProvider(provider: INavigationProvider): void { @@ -266,7 +279,10 @@ export class NotebookServiceStub implements INotebookService { getStandardKernelsForProvider(provider: string): nb.IStandardKernel[] { throw new Error('Method not implemented.'); } - getOrCreateNotebookManager(providerId: string, uri: URI): Thenable { + getOrCreateSerializationManager(providerId: string, uri: URI): Promise { + throw new Error('Method not implemented.'); + } + getOrCreateExecuteManager(providerId: string, uri: URI): Thenable { throw new Error('Method not implemented.'); } addNotebookEditor(editor: INotebookEditor): void { diff --git a/src/sql/workbench/services/notebook/browser/interface.ts b/src/sql/workbench/services/notebook/browser/interface.ts index 4fd7162556..b0d18b85e1 100644 --- a/src/sql/workbench/services/notebook/browser/interface.ts +++ b/src/sql/workbench/services/notebook/browser/interface.ts @@ -5,7 +5,7 @@ import * as azdata from 'azdata'; import { URI } from 'vs/base/common/uri'; import { Event } from 'vs/base/common/event'; -import { IContentManager } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; +import { IContentLoader } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; import { IStandardKernelWithProvider } from 'sql/workbench/services/notebook/browser/models/notebookUtils'; export interface INotebookInput { @@ -14,11 +14,12 @@ export interface INotebookInput { isDirty(): boolean; setDirty(boolean); readonly notebookUri: URI; - updateModel(): void; + updateModel(): Promise; readonly editorOpenedTimestamp: number; readonly layoutChanged: Event; - readonly contentManager: IContentManager; + readonly contentLoader: IContentLoader; readonly standardKernels: IStandardKernelWithProvider[]; + readonly providersLoaded: Promise; } export function isINotebookInput(value: any): value is INotebookInput { @@ -29,7 +30,7 @@ export function isINotebookInput(value: any): value is INotebookInput { typeof value.isDirty === 'function' && typeof value.layoutChanged === 'function' && typeof value.editorOpenedTimestamp === 'number' && - typeof value.contentManager === 'object' && + typeof value.contentLoader === 'object' && typeof value.standardKernels === 'object') { return true; } diff --git a/src/sql/workbench/services/notebook/browser/models/clientSession.ts b/src/sql/workbench/services/notebook/browser/models/clientSession.ts index 819d6f180f..5bcc165885 100644 --- a/src/sql/workbench/services/notebook/browser/models/clientSession.ts +++ b/src/sql/workbench/services/notebook/browser/models/clientSession.ts @@ -13,7 +13,7 @@ import { getErrorMessage } from 'vs/base/common/errors'; import { IClientSession, IClientSessionOptions } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; import { Deferred } from 'sql/base/common/promise'; -import { INotebookManager } from 'sql/workbench/services/notebook/browser/notebookService'; +import { IExecuteManager } from 'sql/workbench/services/notebook/browser/notebookService'; import { IConnectionProfile } from 'sql/platform/connection/common/interfaces'; type KernelChangeHandler = (kernel: nb.IKernelChangedArgs) => Promise; @@ -45,14 +45,14 @@ export class ClientSession implements IClientSession { private _serverLoadFinished: Promise = Promise.resolve(); private _session: nb.ISession | undefined; - private isServerStarted: boolean = false; - private notebookManager: INotebookManager; + private _isServerStarted: boolean = false; + private _executeManager: IExecuteManager; private _kernelConfigActions: ((kernelName: string) => Promise)[] = []; private _connectionId: string = ''; constructor(private options: IClientSessionOptions) { this._notebookUri = options.notebookUri; - this.notebookManager = options.notebookManager; + this._executeManager = options.executeManager; this._isReady = false; this._ready = new Deferred(); this._kernelChangeCompleted = new Deferred(); @@ -77,23 +77,23 @@ export class ClientSession implements IClientSession { } private async startServer(kernelSpec: nb.IKernelSpec): Promise { - let serverManager = this.notebookManager.serverManager; + let serverManager = this._executeManager.serverManager; if (serverManager) { await serverManager.startServer(kernelSpec); if (!serverManager.isStarted) { throw new Error(localize('ServerNotStarted', "Server did not start for unknown reason")); } - this.isServerStarted = serverManager.isStarted; + this._isServerStarted = serverManager.isStarted; } else { - this.isServerStarted = true; + this._isServerStarted = true; } } private async initializeSession(): Promise { await this._serverLoadFinished; - if (this.isServerStarted) { - if (!this.notebookManager.sessionManager.isReady) { - await this.notebookManager.sessionManager.ready; + if (this._isServerStarted) { + if (!this._executeManager.sessionManager.isReady) { + await this._executeManager.sessionManager.ready; } if (this._defaultKernel) { await this.startSessionInstance(this._defaultKernel.name); @@ -105,7 +105,7 @@ export class ClientSession implements IClientSession { let session: nb.ISession; try { // TODO #3164 should use URI instead of path for startNew - session = await this.notebookManager.sessionManager.startNew({ + session = await this._executeManager.sessionManager.startNew({ path: this.notebookUri.fsPath, kernelName: kernelName // TODO add kernel name if saved in the document @@ -115,7 +115,7 @@ export class ClientSession implements IClientSession { // TODO move registration if (err && err.response && err.response.status === 501) { this.options.notificationService.warn(localize('kernelRequiresConnection', "Kernel {0} was not found. The default kernel will be used instead.", kernelName)); - session = await this.notebookManager.sessionManager.startNew({ + session = await this._executeManager.sessionManager.startNew({ path: this.notebookUri.fsPath, kernelName: undefined }); @@ -304,8 +304,8 @@ export class ClientSession implements IClientSession { */ public async shutdown(): Promise { // Always try to shut down session - if (this._session && this._session.id && this.notebookManager && this.notebookManager.sessionManager) { - await this.notebookManager.sessionManager.shutdown(this._session.id); + if (this._session && this._session.id && this._executeManager && this._executeManager.sessionManager) { + await this._executeManager.sessionManager.shutdown(this._session.id); } } diff --git a/src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts b/src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts index 12b4eeafaa..53ce5c56bc 100644 --- a/src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts +++ b/src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts @@ -12,7 +12,7 @@ import { URI } from 'vs/base/common/uri'; import { INotificationService } from 'vs/platform/notification/common/notification'; import { CellType, NotebookChangeType } from 'sql/workbench/services/notebook/common/contracts'; -import { INotebookManager, ILanguageMagic } from 'sql/workbench/services/notebook/browser/notebookService'; +import { IExecuteManager, ILanguageMagic, ISerializationManager } from 'sql/workbench/services/notebook/browser/notebookService'; import { IConnectionProfile } from 'sql/platform/connection/common/interfaces'; import { IConnectionManagementService } from 'sql/platform/connection/common/connectionManagement'; import { IStandardKernelWithProvider } from 'sql/workbench/services/notebook/browser/models/notebookUtils'; @@ -42,7 +42,7 @@ export interface ISingleNotebookEditOperation { export interface IClientSessionOptions { notebookUri: URI; - notebookManager: INotebookManager; + executeManager: IExecuteManager; notificationService: INotificationService; kernelSpec: nb.IKernelSpec; } @@ -254,9 +254,14 @@ export interface INotebookModel { readonly language: string; /** - * All notebook managers applicable for a given notebook + * The current serialization manager applicable for a given notebook */ - readonly notebookManagers: INotebookManager[]; + readonly serializationManager: ISerializationManager | undefined; + + /** + * All execute managers applicable for a given notebook + */ + readonly executeManagers: IExecuteManager[]; /** * Event fired on first initialization of the kernel and @@ -551,7 +556,7 @@ export interface IModelFactory { createClientSession(options: IClientSessionOptions): IClientSession; } -export interface IContentManager { +export interface IContentLoader { /** * This is a specialized method intended to load for a default context - just the current Notebook's URI */ @@ -569,8 +574,9 @@ export interface INotebookModelOptions { */ factory: IModelFactory; - contentManager: IContentManager; - notebookManagers: INotebookManager[]; + contentLoader: IContentLoader; + serializationManagers: ISerializationManager[]; + executeManagers: IExecuteManager[]; providerId: string; defaultKernel: nb.IKernelSpec; cellMagicMapper: ICellMagicMapper; diff --git a/src/sql/workbench/services/notebook/browser/models/notebookModel.ts b/src/sql/workbench/services/notebook/browser/models/notebookModel.ts index 294c941f63..10b29642ec 100644 --- a/src/sql/workbench/services/notebook/browser/models/notebookModel.ts +++ b/src/sql/workbench/services/notebook/browser/models/notebookModel.ts @@ -14,7 +14,7 @@ import { NotebookChangeType, CellType, CellTypes } from 'sql/workbench/services/ import { KernelsLanguage, nbversion } from 'sql/workbench/services/notebook/common/notebookConstants'; import * as notebookUtils from 'sql/workbench/services/notebook/browser/models/notebookUtils'; import * as TelemetryKeys from 'sql/platform/telemetry/common/telemetryKeys'; -import { INotebookManager, SQL_NOTEBOOK_PROVIDER, DEFAULT_NOTEBOOK_PROVIDER } from 'sql/workbench/services/notebook/browser/notebookService'; +import { IExecuteManager, SQL_NOTEBOOK_PROVIDER, DEFAULT_NOTEBOOK_PROVIDER, ISerializationManager } from 'sql/workbench/services/notebook/browser/notebookService'; import { NotebookContexts } from 'sql/workbench/services/notebook/browser/models/notebookContexts'; import { IConnectionProfile } from 'sql/platform/connection/common/interfaces'; import { INotification, Severity, INotificationService } from 'vs/platform/notification/common/notification'; @@ -124,7 +124,7 @@ export class NotebookModel extends Disposable implements INotebookModel { ) { super(); - if (!_notebookOptions || !_notebookOptions.notebookUri || !_notebookOptions.notebookManagers) { + if (!_notebookOptions || !_notebookOptions.notebookUri || !_notebookOptions.executeManagers) { throw new Error('path or notebook service not defined'); } this._trustedMode = false; @@ -136,27 +136,43 @@ export class NotebookModel extends Disposable implements INotebookModel { this._defaultKernel = _notebookOptions.defaultKernel; } - public get notebookManagers(): INotebookManager[] { - let notebookManagers = this._notebookOptions.notebookManagers.filter(manager => manager.providerId !== DEFAULT_NOTEBOOK_PROVIDER); - if (!notebookManagers.length) { - return this._notebookOptions.notebookManagers; + private get serializationManagers(): ISerializationManager[] { + let managers = this._notebookOptions.serializationManagers.filter(manager => manager.providerId !== DEFAULT_NOTEBOOK_PROVIDER); + if (!managers.length) { + return this._notebookOptions.serializationManagers; } - return notebookManagers; + return managers; } - public get notebookManager(): INotebookManager | undefined { - let manager = this.notebookManagers.find(manager => manager.providerId === this._providerId); + public get serializationManager(): ISerializationManager | undefined { + let manager = this.serializationManagers.find(manager => manager.providerId === this._providerId); if (!manager) { - // Note: this seems like a less than ideal scenario. We should ideally pass in the "correct" provider ID and allow there to be a default, - // instead of assuming in the NotebookModel constructor that the option is either SQL or Jupyter - manager = this.notebookManagers.find(manager => manager.providerId === DEFAULT_NOTEBOOK_PROVIDER); + manager = this.serializationManagers.find(manager => manager.providerId === DEFAULT_NOTEBOOK_PROVIDER); } return manager; } - public getNotebookManager(providerId: string): INotebookManager | undefined { + public get executeManagers(): IExecuteManager[] { + let managers = this._notebookOptions.executeManagers.filter(manager => manager.providerId !== DEFAULT_NOTEBOOK_PROVIDER); + if (!managers.length) { + return this._notebookOptions.executeManagers; + } + return managers; + } + + public get executeManager(): IExecuteManager | undefined { + let manager = this.executeManagers.find(manager => manager.providerId === this._providerId); + if (!manager) { + // Note: this seems like a less than ideal scenario. We should ideally pass in the "correct" provider ID and allow there to be a default, + // instead of assuming in the NotebookModel constructor that the option is either SQL or Jupyter + manager = this.executeManagers.find(manager => manager.providerId === DEFAULT_NOTEBOOK_PROVIDER); + } + return manager; + } + + public getExecuteManager(providerId: string): IExecuteManager | undefined { if (providerId) { - return this.notebookManagers.find(manager => manager.providerId === providerId); + return this.executeManagers.find(manager => manager.providerId === providerId); } return undefined; } @@ -174,7 +190,7 @@ export class NotebookModel extends Disposable implements INotebookModel { public get hasServerManager(): boolean { // If the service has a server manager, then we can show the start button - return !!this.notebookManager?.serverManager; + return !!this.executeManager?.serverManager; } public get contentChanged(): Event { @@ -247,7 +263,7 @@ export class NotebookModel extends Disposable implements INotebookModel { defaultKernel: '', kernels: [] }; - this.notebookManagers.forEach(manager => { + this.executeManagers.forEach(manager => { if (manager.sessionManager && manager.sessionManager.specs && manager.sessionManager.specs.kernels) { manager.sessionManager.specs.kernels.forEach(kernel => { specs.kernels.push(kernel); @@ -399,8 +415,8 @@ export class NotebookModel extends Disposable implements INotebookModel { let contents: nb.INotebookContents | undefined; - if (this._notebookOptions && this._notebookOptions.contentManager) { - contents = await this._notebookOptions.contentManager.loadContent(); + if (this._notebookOptions && this._notebookOptions.contentLoader) { + contents = await this._notebookOptions.contentLoader.loadContent(); } let factory = this._notebookOptions.factory; // if cells already exist, create them with language info (if it is saved) @@ -693,7 +709,7 @@ export class NotebookModel extends Disposable implements INotebookModel { this._onErrorEmitter.fire({ message: error, severity: Severity.Error }); } - public async startSession(manager: INotebookManager, displayName?: string, setErrorStateOnFail?: boolean, kernelAlias?: string): Promise { + public async startSession(manager: IExecuteManager, displayName?: string, setErrorStateOnFail?: boolean, kernelAlias?: string): Promise { if (displayName) { let standardKernel = this._standardKernels.find(kernel => kernel.displayName === displayName); if (standardKernel) { @@ -703,7 +719,7 @@ export class NotebookModel extends Disposable implements INotebookModel { if (this._defaultKernel) { let clientSession = this._notebookOptions.factory.createClientSession({ notebookUri: this._notebookOptions.notebookUri, - notebookManager: manager, + executeManager: manager, notificationService: this._notebookOptions.notificationService, kernelSpec: this._defaultKernel }); @@ -750,7 +766,7 @@ export class NotebookModel extends Disposable implements INotebookModel { if (this._activeClientSession) { // Old active client sessions have already been shutdown by RESTART_JUPYTER_NOTEBOOK_SESSIONS command this._activeClientSession = undefined; - await this.startSession(this.notebookManager, this._selectedKernelDisplayName, true); + await this.startSession(this.executeManager, this._selectedKernelDisplayName, true); } } @@ -777,7 +793,7 @@ export class NotebookModel extends Disposable implements INotebookModel { if (provider && provider !== this._providerId) { this._providerId = provider; } else if (!provider) { - this.notebookOptions.notebookManagers.forEach(m => { + this.notebookOptions.executeManagers.forEach(m => { if (m.providerId !== SQL_NOTEBOOK_PROVIDER) { // We don't know which provider it is before that provider is chosen to query its specs. Choosing the "last" one registered. this._providerId = m.providerId; @@ -1002,7 +1018,7 @@ export class NotebookModel extends Disposable implements INotebookModel { // Ensure that the kernel we try to switch to is a valid kernel; if not, use the default let kernelSpecs = this.getKernelSpecs(); if (kernelSpecs && kernelSpecs.length > 0 && kernelSpecs.findIndex(k => k.display_name === spec.display_name) < 0) { - spec = kernelSpecs.find(spec => spec.name === this.notebookManager?.sessionManager.specs.defaultKernel); + spec = kernelSpecs.find(spec => spec.name === this.executeManager?.sessionManager.specs.defaultKernel); } } else { @@ -1099,11 +1115,11 @@ export class NotebookModel extends Disposable implements INotebookModel { } public getDisplayNameFromSpecName(kernel: nb.IKernel): string | undefined { - let specs = this.notebookManager?.sessionManager.specs; + let specs = this.executeManager?.sessionManager.specs; if (!specs || !specs.kernels) { return kernel.name; } - let newKernel = this.notebookManager.sessionManager.specs.kernels.find(k => k.name === kernel.name); + let newKernel = this.executeManager.sessionManager.specs.kernels.find(k => k.name === kernel.name); let newKernelDisplayName; if (newKernel) { newKernelDisplayName = newKernel.display_name; @@ -1202,7 +1218,7 @@ export class NotebookModel extends Disposable implements INotebookModel { this._onProviderIdChanged.fire(this._providerId); await this.shutdownActiveSession(); - let manager = this.getNotebookManager(providerId); + let manager = this.getExecuteManager(providerId); if (manager) { await this.startSession(manager, displayName, false, kernelAlias); } else { @@ -1225,8 +1241,8 @@ export class NotebookModel extends Disposable implements INotebookModel { return providerId; } } else { - if (this.notebookManagers?.length) { - return this.notebookManagers.map(m => m.providerId).find(p => p !== DEFAULT_NOTEBOOK_PROVIDER && p !== SQL_NOTEBOOK_PROVIDER); + if (this.executeManagers?.length) { + return this.executeManagers.map(m => m.providerId).find(p => p !== DEFAULT_NOTEBOOK_PROVIDER && p !== SQL_NOTEBOOK_PROVIDER); } } return undefined; @@ -1234,9 +1250,9 @@ export class NotebookModel extends Disposable implements INotebookModel { // Get kernel specs from current sessionManager private getKernelSpecs(): nb.IKernelSpec[] { - if (this.notebookManager && this.notebookManager.sessionManager && this.notebookManager.sessionManager.specs && - this.notebookManager.sessionManager.specs.kernels) { - return this.notebookManager.sessionManager.specs.kernels; + if (this.executeManager && this.executeManager.sessionManager && this.executeManager.sessionManager.specs && + this.executeManager.sessionManager.specs.kernels) { + return this.executeManager.sessionManager.specs.kernels; } return []; } diff --git a/src/sql/workbench/services/notebook/browser/notebookService.ts b/src/sql/workbench/services/notebook/browser/notebookService.ts index d48b4ef38f..408558566c 100644 --- a/src/sql/workbench/services/notebook/browser/notebookService.ts +++ b/src/sql/workbench/services/notebook/browser/notebookService.ts @@ -57,15 +57,14 @@ export interface INotebookService { readonly isRegistrationComplete: boolean; readonly registrationComplete: Promise; readonly languageMagics: ILanguageMagic[]; - /** - * Register a metadata provider - */ - registerProvider(providerId: string, provider: INotebookProvider): void; - /** - * Register a metadata provider - */ - unregisterProvider(providerId: string): void; + registerSerializationProvider(providerId: string, provider: ISerializationProvider): void; + + registerExecuteProvider(providerId: string, provider: IExecuteProvider): void; + + unregisterSerializationProvider(providerId: string): void; + + unregisterExecuteProvider(providerId: string): void; registerNavigationProvider(provider: INavigationProvider): void; @@ -77,14 +76,9 @@ export interface INotebookService { getStandardKernelsForProvider(provider: string): azdata.nb.IStandardKernel[]; - /** - * Initializes and returns a Notebook manager that can handle all important calls to open, display, and - * run cells in a notebook. - * @param providerId ID for the provider to be used to instantiate a backend notebook service - * @param uri URI for a notebook that is to be opened. Based on this an existing manager may be used, or - * a new one may need to be created - */ - getOrCreateNotebookManager(providerId: string, uri: URI): Thenable; + getOrCreateSerializationManager(providerId: string, uri: URI): Promise; + + getOrCreateExecuteManager(providerId: string, uri: URI): Thenable; addNotebookEditor(editor: INotebookEditor): void; @@ -148,15 +142,24 @@ export interface INotebookService { getUntitledUriPath(originalTitle: string): string; } -export interface INotebookProvider { +export interface IExecuteProvider { readonly providerId: string; - getNotebookManager(notebookUri: URI): Thenable; + getExecuteManager(notebookUri: URI): Thenable; handleNotebookClosed(notebookUri: URI): void; } -export interface INotebookManager { +export interface ISerializationProvider { + readonly providerId: string; + getSerializationManager(notebookUri: URI): Thenable; +} + +export interface ISerializationManager { providerId: string; readonly contentManager: azdata.nb.ContentManager; +} + +export interface IExecuteManager { + providerId: string; readonly sessionManager: azdata.nb.SessionManager; readonly serverManager: azdata.nb.ServerManager; } diff --git a/src/sql/workbench/services/notebook/browser/notebookServiceImpl.ts b/src/sql/workbench/services/notebook/browser/notebookServiceImpl.ts index ef2c0853b6..f069aed8a0 100644 --- a/src/sql/workbench/services/notebook/browser/notebookServiceImpl.ts +++ b/src/sql/workbench/services/notebook/browser/notebookServiceImpl.ts @@ -9,12 +9,12 @@ import { URI, UriComponents } from 'vs/base/common/uri'; import { Registry } from 'vs/platform/registry/common/platform'; import { - INotebookService, INotebookManager, INotebookProvider, - DEFAULT_NOTEBOOK_FILETYPE, INotebookEditor, SQL_NOTEBOOK_PROVIDER, INavigationProvider, ILanguageMagic, NavigationProviders, unsavedBooksContextKey + INotebookService, IExecuteManager, IExecuteProvider, + DEFAULT_NOTEBOOK_FILETYPE, INotebookEditor, SQL_NOTEBOOK_PROVIDER, INavigationProvider, ILanguageMagic, NavigationProviders, unsavedBooksContextKey, ISerializationProvider, ISerializationManager } from 'sql/workbench/services/notebook/browser/notebookService'; import { RenderMimeRegistry } from 'sql/workbench/services/notebook/browser/outputs/registry'; import { standardRendererFactories } from 'sql/workbench/services/notebook/browser/outputs/factories'; -import { Extensions, INotebookProviderRegistry, NotebookProviderRegistration } from 'sql/workbench/services/notebook/common/notebookRegistry'; +import { Extensions, INotebookProviderRegistry, NotebookProviderRegistryId, ProviderDescriptionRegistration } from 'sql/workbench/services/notebook/common/notebookRegistry'; import { Emitter, Event } from 'vs/base/common/event'; import { Memento } from 'vs/workbench/common/memento'; import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage'; @@ -26,7 +26,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { IQueryManagementService } from 'sql/workbench/services/query/common/queryManagement'; import { ICellModel } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; import { ILifecycleService } from 'vs/workbench/services/lifecycle/common/lifecycle'; -import { SqlNotebookProvider } from 'sql/workbench/services/notebook/browser/sql/sqlNotebookProvider'; +import { SqlExecuteProvider } from 'sql/workbench/services/notebook/browser/sql/sqlExecuteProvider'; import { IFileService, IFileStatWithMetadata } from 'vs/platform/files/common/files'; import { Schemas } from 'vs/base/common/network'; import { ILogService } from 'vs/platform/log/common/log'; @@ -50,8 +50,9 @@ import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editor import { IEditorInput, IEditorPane } from 'vs/workbench/common/editor'; import { isINotebookInput } from 'sql/workbench/services/notebook/browser/interface'; import { INotebookShowOptions } from 'sql/workbench/api/common/sqlExtHost.protocol'; -import { NotebookLanguage } from 'sql/workbench/common/constants'; +import { JUPYTER_PROVIDER_ID, NotebookLanguage } from 'sql/workbench/common/constants'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; +import { SqlSerializationProvider } from 'sql/workbench/services/notebook/browser/sql/sqlSerializationProvider'; const languageAssociationRegistry = Registry.as(LanguageAssociationExtensions.LanguageAssociations); @@ -65,7 +66,8 @@ interface NotebookProviderCache { } export interface NotebookProvidersMemento { - notebookProviderCache: NotebookProviderCache; + notebookSerializationProviderCache: NotebookProviderCache; + notebookExecuteProviderCache: NotebookProviderCache; } interface TrustedNotebookMetadata { @@ -80,24 +82,53 @@ export interface TrustedNotebooksMemento { trustedNotebooksCache: TrustedNotebookCache; } -const notebookRegistry = Registry.as(Extensions.NotebookProviderContribution); +const notebookRegistry = Registry.as(NotebookProviderRegistryId); -export class ProviderDescriptor { - private _instanceReady = new Deferred(); - constructor(private _instance?: INotebookProvider) { +export class SerializationProviderDescriptor { + private _instanceReady = new Deferred(); + constructor(private readonly _providerId: string, private _instance?: ISerializationProvider) { if (_instance) { this._instanceReady.resolve(_instance); } } - public get instanceReady(): Promise { + public get providerId(): string { + return this._providerId; + } + + public get instanceReady(): Promise { return this._instanceReady.promise; } - public get instance(): INotebookProvider { + public get instance(): ISerializationProvider | undefined { return this._instance; } - public set instance(value: INotebookProvider) { + public set instance(value: ISerializationProvider) { + this._instance = value; + this._instanceReady.resolve(value); + } +} + +export class ExecuteProviderDescriptor { + private _instanceReady = new Deferred(); + constructor(private readonly _providerId: string, private _instance?: IExecuteProvider) { + if (_instance) { + this._instanceReady.resolve(_instance); + } + } + + public get providerId(): string { + return this._providerId; + } + + public get instanceReady(): Promise { + return this._instanceReady.promise; + } + + public get instance(): IExecuteProvider | undefined { + return this._instance; + } + public set instance(value: IExecuteProvider) { this._instance = value; this._instanceReady.resolve(value); } @@ -113,14 +144,16 @@ export class NotebookService extends Disposable implements INotebookService { private _providersMemento: Memento; private _trustedNotebooksMemento: Memento; private _mimeRegistry: RenderMimeRegistry; - private _providers: Map = new Map(); + private _serializationProviders: Map = new Map(); + private _executeProviders: Map = new Map(); private _navigationProviders: Map = new Map(); - private _managersMap: Map = new Map(); + private _serializationManagersMap: Map = new Map(); + private _executeManagersMap: Map = new Map(); private _onNotebookEditorAdd = new Emitter(); private _onNotebookEditorRemove = new Emitter(); private _onNotebookEditorRename = new Emitter(); private _editors = new Map(); - private _fileToProviders = new Map(); + private _fileToProviderDescriptions = new Map(); private _providerToStandardKernels = new Map(); private _registrationComplete = new Deferred(); private _isRegistrationComplete = false; @@ -147,19 +180,24 @@ export class NotebookService extends Disposable implements INotebookService { super(); this._providersMemento = new Memento('notebookProviders', this._storageService); this._trustedNotebooksMemento = new Memento(TrustedNotebooksMementoId, this._storageService); - if (this._storageService !== undefined && this.providersMemento.notebookProviderCache === undefined) { - this.providersMemento.notebookProviderCache = {}; + if (this._storageService !== undefined) { + if (this.providersMemento.notebookSerializationProviderCache === undefined) { + this.providersMemento.notebookSerializationProviderCache = {}; + } + if (this.providersMemento.notebookExecuteProviderCache === undefined) { + this.providersMemento.notebookExecuteProviderCache = {}; + } } - this._register(notebookRegistry.onNewRegistration(this.updateRegisteredProviders, this)); - this.registerBuiltInProvider(); + this._register(notebookRegistry.onNewDescriptionRegistration(this.handleNewProviderDescriptions, this)); + this.registerBuiltInProviders(); // 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) { - notebookRegistry.providers.forEach(p => { + if (notebookRegistry.providerDescriptions.length > 0) { + notebookRegistry.providerDescriptions.forEach(p => { // Don't need to re-register SQL_NOTEBOOK_PROVIDER if (p.provider !== SQL_NOTEBOOK_PROVIDER) { - this.updateRegisteredProviders({ id: p.provider, registration: p }); + this.handleNewProviderDescriptions({ id: p.provider, registration: p }); } }); } @@ -266,12 +304,15 @@ export class NotebookService extends Disposable implements INotebookService { this._registrationComplete.resolve(); } - private updateRegisteredProviders(p: { id: string; registration: NotebookProviderRegistration }) { - let registration = p.registration; - - if (!this._providers.has(p.id)) { - this._providers.set(p.id, new ProviderDescriptor()); + private handleNewProviderDescriptions(p: { id: string; registration: ProviderDescriptionRegistration }) { + // Skip adding a Serialization descriptor for Jupyter, since we use the default notebook Serialization provider for jupyter + if (!this._serializationProviders.has(p.id) && p.id !== JUPYTER_PROVIDER_ID) { + this._serializationProviders.set(p.id, new SerializationProviderDescriptor(p.id)); } + if (!this._executeProviders.has(p.id)) { + this._executeProviders.set(p.id, new ExecuteProviderDescriptor(p.id)); + } + let registration = p.registration; if (registration.fileExtensions) { if (Array.isArray(registration.fileExtensions)) { for (let fileType of registration.fileExtensions) { @@ -287,18 +328,32 @@ export class NotebookService extends Disposable implements INotebookService { } } - registerProvider(providerId: string, instance: INotebookProvider): void { - let providerDescriptor = this._providers.get(providerId); + registerSerializationProvider(providerId: string, instance: ISerializationProvider): void { + let providerDescriptor = this._serializationProviders.get(providerId); if (providerDescriptor) { // Update, which will resolve the promise for anyone waiting on the instance to be registered providerDescriptor.instance = instance; } else { - this._providers.set(providerId, new ProviderDescriptor(instance)); + this._serializationProviders.set(providerId, new SerializationProviderDescriptor(providerId, instance)); } } - unregisterProvider(providerId: string): void { - this._providers.delete(providerId); + registerExecuteProvider(providerId: string, instance: IExecuteProvider): void { + let providerDescriptor = this._executeProviders.get(providerId); + if (providerDescriptor) { + // Update, which will resolve the promise for anyone waiting on the instance to be registered + providerDescriptor.instance = instance; + } else { + this._executeProviders.set(providerId, new ExecuteProviderDescriptor(providerId, instance)); + } + } + + unregisterSerializationProvider(providerId: string): void { + this._serializationProviders.delete(providerId); + } + + unregisterExecuteProvider(providerId: string): void { + this._executeProviders.delete(providerId); } registerNavigationProvider(provider: INavigationProvider): void { @@ -322,20 +377,20 @@ export class NotebookService extends Disposable implements INotebookService { return this._registrationComplete.promise; } - private addFileProvider(fileType: string, provider: NotebookProviderRegistration) { - let providers = this._fileToProviders.get(fileType.toUpperCase()); + private addFileProvider(fileType: string, provider: ProviderDescriptionRegistration) { + let providers = this._fileToProviderDescriptions.get(fileType.toUpperCase()); if (!providers) { providers = []; } providers.push(provider); - this._fileToProviders.set(fileType.toUpperCase(), providers); + this._fileToProviderDescriptions.set(fileType.toUpperCase(), providers); } // Standard kernels are contributed where a list of kernels are defined that can be shown // in the kernels dropdown list before a SessionManager has been started; this way, // every NotebookProvider doesn't need to have an active SessionManager in order to contribute // kernels to the dropdown - private addStandardKernels(provider: NotebookProviderRegistration) { + private addStandardKernels(provider: ProviderDescriptionRegistration) { let providerUpperCase = provider.provider.toUpperCase(); let standardKernels = this._providerToStandardKernels.get(providerUpperCase); if (!standardKernels) { @@ -356,12 +411,12 @@ export class NotebookService extends Disposable implements INotebookService { } getSupportedFileExtensions(): string[] { - return Array.from(this._fileToProviders.keys()); + return Array.from(this._fileToProviderDescriptions.keys()); } getProvidersForFileType(fileType: string): string[] { fileType = fileType.toUpperCase(); - let providers = this._fileToProviders.get(fileType); + let providers = this._fileToProviderDescriptions.get(fileType); return providers ? providers.map(provider => provider.provider) : undefined; } @@ -371,7 +426,7 @@ export class NotebookService extends Disposable implements INotebookService { } private shutdown(): void { - this._managersMap.forEach(manager => { + this._executeManagersMap.forEach(manager => { manager.forEach(m => { if (m.serverManager) { // TODO should this thenable be awaited? @@ -381,12 +436,12 @@ export class NotebookService extends Disposable implements INotebookService { }); } - async getOrCreateNotebookManager(providerId: string, uri: URI): Promise { + async getOrCreateSerializationManager(providerId: string, uri: URI): Promise { if (!uri) { throw new Error(NotebookUriNotDefined); } let uriString = uri.toString(); - let managers: INotebookManager[] = this._managersMap.get(uriString); + let managers: ISerializationManager[] = this._serializationManagersMap.get(uriString); // If manager already exists for a given notebook, return it if (managers) { let index = managers.findIndex(m => m.providerId === providerId); @@ -394,11 +449,32 @@ export class NotebookService extends Disposable implements INotebookService { return managers[index]; } } - let newManager = await this.doWithProvider(providerId, (provider) => provider.getNotebookManager(uri)); + let newManager = await this.doWithSerializationProvider(providerId, (provider) => provider.getSerializationManager(uri)); managers = managers || []; managers.push(newManager); - this._managersMap.set(uriString, managers); + this._serializationManagersMap.set(uriString, managers); + return newManager; + } + + async getOrCreateExecuteManager(providerId: string, uri: URI): Promise { + if (!uri) { + throw new Error(NotebookUriNotDefined); + } + let uriString = uri.toString(); + let managers: IExecuteManager[] = this._executeManagersMap.get(uriString); + // If manager already exists for a given notebook, return it + if (managers) { + let index = managers.findIndex(m => m.providerId === providerId); + if (index >= 0) { + return managers[index]; + } + } + let newManager = await this.doWithExecuteProvider(providerId, (provider) => provider.getExecuteManager(uri)); + + managers = managers || []; + managers.push(newManager); + this._executeManagersMap.set(uriString, managers); return newManager; } @@ -461,26 +537,32 @@ export class NotebookService extends Disposable implements INotebookService { private sendNotebookCloseToProvider(editor: INotebookEditor): void { let notebookUri = editor.notebookParams.notebookUri; let uriString = notebookUri.toString(); - let managers = this._managersMap.get(uriString); + let managers = this._executeManagersMap.get(uriString); if (managers) { // As we have a manager, we can assume provider is ready - this._managersMap.delete(uriString); + this._executeManagersMap.delete(uriString); managers.forEach(m => { - let provider = this._providers.get(m.providerId); - provider.instance.handleNotebookClosed(notebookUri); + let provider = this._executeProviders.get(m.providerId); + provider?.instance?.handleNotebookClosed(notebookUri); }); } } - private async doWithProvider(providerId: string, op: (provider: INotebookProvider) => Thenable): Promise { + private async doWithSerializationProvider(providerId: string, op: (provider: ISerializationProvider) => Thenable): Promise { // Make sure the provider exists before attempting to retrieve accounts - let provider: INotebookProvider = await this.getProviderInstance(providerId); + let provider: ISerializationProvider = await this.getSerializationProviderInstance(providerId); return op(provider); } - private async getProviderInstance(providerId: string, timeout?: number): Promise { - let providerDescriptor = this._providers.get(providerId); - let instance: INotebookProvider; + private async doWithExecuteProvider(providerId: string, op: (provider: IExecuteProvider) => Thenable): Promise { + // Make sure the provider exists before attempting to retrieve accounts + let provider: IExecuteProvider = await this.getExecuteProviderInstance(providerId); + return op(provider); + } + + private async getSerializationProviderInstance(providerId: string, timeout?: number): Promise { + let providerDescriptor = this._serializationProviders.get(providerId); + let instance: ISerializationProvider; // Try get from actual provider, waiting on its registration if (providerDescriptor) { @@ -491,7 +573,7 @@ export class NotebookService extends Disposable implements INotebookService { } catch (error) { this._logService.error(error); } - instance = await this.waitOnProviderAvailability(providerDescriptor, timeout); + instance = await this.waitOnSerializationProviderAvailability(providerDescriptor, timeout); } else { instance = providerDescriptor.instance; } @@ -499,7 +581,7 @@ export class NotebookService extends Disposable implements INotebookService { // Fall back to default (SQL) if this failed if (!instance) { - providerDescriptor = this._providers.get(SQL_NOTEBOOK_PROVIDER); + providerDescriptor = this._serializationProviders.get(SQL_NOTEBOOK_PROVIDER); instance = providerDescriptor ? providerDescriptor.instance : undefined; } @@ -510,12 +592,60 @@ export class NotebookService extends Disposable implements INotebookService { return instance; } - private waitOnProviderAvailability(providerDescriptor: ProviderDescriptor, timeout?: number): Promise { - // Wait up to 30 seconds for the provider to be registered - timeout = timeout ?? 30000; - let promises: Promise[] = [ + private async getExecuteProviderInstance(providerId: string, timeout?: number): Promise { + let providerDescriptor = this._executeProviders.get(providerId); + let instance: IExecuteProvider; + + // Try get from actual provider, waiting on its registration + if (providerDescriptor) { + if (!providerDescriptor.instance) { + // Await extension registration before awaiting provider registration + try { + await this._extensionService.whenInstalledExtensionsRegistered(); + } catch (error) { + this._logService.error(error); + } + instance = await this.waitOnExecuteProviderAvailability(providerDescriptor, timeout); + } else { + instance = providerDescriptor.instance; + } + } + + // Fall back to default (SQL) if this failed + if (!instance) { + providerDescriptor = this._executeProviders.get(SQL_NOTEBOOK_PROVIDER); + instance = providerDescriptor ? providerDescriptor.instance : undefined; + } + + // Should never happen, but if default wasn't registered we should throw + if (!instance) { + throw new Error(NotebookServiceNoProviderRegistered); + } + return instance; + } + + private waitOnSerializationProviderAvailability(providerDescriptor: SerializationProviderDescriptor, timeout?: number): Promise { + // Wait up to 10 seconds for the provider to be registered + timeout = timeout ?? 10000; + let promises: Promise[] = [ providerDescriptor.instanceReady, - new Promise((resolve, reject) => setTimeout(() => resolve(undefined), timeout)) + new Promise((resolve, reject) => setTimeout(() => { + this._serializationProviders.delete(providerDescriptor.providerId); + return resolve(undefined); + }, timeout)) + ]; + return Promise.race(promises); + } + + private waitOnExecuteProviderAvailability(providerDescriptor: ExecuteProviderDescriptor, timeout?: number): Promise { + // Wait up to 10 seconds for the provider to be registered + timeout = timeout ?? 10000; + let promises: Promise[] = [ + providerDescriptor.instanceReady, + new Promise((resolve, reject) => setTimeout(() => { + this._executeProviders.delete(providerDescriptor.providerId); + return resolve(undefined); + }, timeout)) ]; return Promise.race(promises); } @@ -543,36 +673,48 @@ export class NotebookService extends Disposable implements INotebookService { } private cleanupProviders(): void { - let knownProviders = Object.keys(notebookRegistry.providers); - let cache = this.providersMemento.notebookProviderCache; - for (let key in cache) { + let knownProviders = notebookRegistry.providerDescriptions.map(d => d.provider); + let executeCache = this.providersMemento.notebookExecuteProviderCache; + for (let key in executeCache) { if (!knownProviders.some(x => x === key)) { - this._providers.delete(key); - delete cache[key]; + this._executeProviders.delete(key); + delete executeCache[key]; + } + } + + let serializationCache = this.providersMemento.notebookSerializationProviderCache; + for (let key in serializationCache) { + if (!knownProviders.some(x => x === key)) { + this._serializationProviders.delete(key); + delete serializationCache[key]; } } } - private registerBuiltInProvider() { - let sqlProvider = new SqlNotebookProvider(this._instantiationService); - this.registerProvider(sqlProvider.providerId, sqlProvider); - notebookRegistry.registerNotebookProvider({ - provider: sqlProvider.providerId, + private registerBuiltInProviders() { + let serializationProvider = new SqlSerializationProvider(this._instantiationService); + this.registerSerializationProvider(serializationProvider.providerId, serializationProvider); + + let executeProvider = new SqlExecuteProvider(this._instantiationService); + this.registerExecuteProvider(executeProvider.providerId, executeProvider); + + notebookRegistry.registerProviderDescription({ + provider: serializationProvider.providerId, fileExtensions: DEFAULT_NOTEBOOK_FILETYPE, standardKernels: { name: notebookConstants.SQL, displayName: notebookConstants.SQL, connectionProviderIds: [notebookConstants.SQL_CONNECTION_PROVIDER] } }); } protected async removeContributedProvidersFromCache(identifier: IExtensionIdentifier, extensionService: IExtensionService): Promise { - const notebookProvider = 'notebookProvider'; 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]; + && extensionDescription.contributes[Extensions.NotebookProviderDescriptionContribution] + && extensionDescription.contributes[Extensions.NotebookProviderDescriptionContribution].providerId) { + let id = extensionDescription.contributes[Extensions.NotebookProviderDescriptionContribution].providerId; + delete this.providersMemento.notebookSerializationProviderCache[id]; + delete this.providersMemento.notebookExecuteProviderCache[id]; } } catch (err) { onUnexpectedError(err); diff --git a/src/sql/workbench/services/notebook/browser/sql/sqlNotebookManager.ts b/src/sql/workbench/services/notebook/browser/sql/sqlExecuteManager.ts similarity index 70% rename from src/sql/workbench/services/notebook/browser/sql/sqlNotebookManager.ts rename to src/sql/workbench/services/notebook/browser/sql/sqlExecuteManager.ts index b71131050b..a22678d36c 100644 --- a/src/sql/workbench/services/notebook/browser/sql/sqlNotebookManager.ts +++ b/src/sql/workbench/services/notebook/browser/sql/sqlExecuteManager.ts @@ -7,15 +7,12 @@ import { nb } from 'azdata'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { SQL_NOTEBOOK_PROVIDER } from 'sql/workbench/services/notebook/browser/notebookService'; -import { LocalContentManager } from 'sql/workbench/services/notebook/common/localContentManager'; import { SqlSessionManager } from 'sql/workbench/services/notebook/browser/sql/sqlSessionManager'; -export class SqlNotebookManager implements nb.NotebookManager { - private _contentManager: nb.ContentManager; +export class SqlExecuteManager implements nb.ExecuteManager { private _sessionManager: nb.SessionManager; constructor(instantiationService: IInstantiationService) { - this._contentManager = instantiationService.createInstance(LocalContentManager); this._sessionManager = new SqlSessionManager(instantiationService); } @@ -23,10 +20,6 @@ export class SqlNotebookManager implements nb.NotebookManager { return SQL_NOTEBOOK_PROVIDER; } - public get contentManager(): nb.ContentManager { - return this._contentManager; - } - public get serverManager(): nb.ServerManager | undefined { return undefined; } @@ -34,8 +27,4 @@ export class SqlNotebookManager implements nb.NotebookManager { public get sessionManager(): nb.SessionManager { return this._sessionManager; } - - public get standardKernels(): nb.IStandardKernel[] { - return []; - } } diff --git a/src/sql/workbench/services/notebook/browser/sql/sqlNotebookProvider.ts b/src/sql/workbench/services/notebook/browser/sql/sqlExecuteProvider.ts similarity index 55% rename from src/sql/workbench/services/notebook/browser/sql/sqlNotebookProvider.ts rename to src/sql/workbench/services/notebook/browser/sql/sqlExecuteProvider.ts index 8a5847468a..297cd20eaf 100644 --- a/src/sql/workbench/services/notebook/browser/sql/sqlNotebookProvider.ts +++ b/src/sql/workbench/services/notebook/browser/sql/sqlExecuteProvider.ts @@ -6,21 +6,21 @@ import { URI } from 'vs/base/common/uri'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; -import { INotebookManager, INotebookProvider, SQL_NOTEBOOK_PROVIDER } from 'sql/workbench/services/notebook/browser/notebookService'; -import { SqlNotebookManager } from 'sql/workbench/services/notebook/browser/sql/sqlNotebookManager'; +import { IExecuteManager, IExecuteProvider, SQL_NOTEBOOK_PROVIDER } from 'sql/workbench/services/notebook/browser/notebookService'; +import { SqlExecuteManager } from 'sql/workbench/services/notebook/browser/sql/sqlExecuteManager'; -export class SqlNotebookProvider implements INotebookProvider { - private manager: SqlNotebookManager; +export class SqlExecuteProvider implements IExecuteProvider { + private manager: SqlExecuteManager; - constructor(private _instantiationService: IInstantiationService) { - this.manager = new SqlNotebookManager(this._instantiationService); + constructor(instantiationService: IInstantiationService) { + this.manager = new SqlExecuteManager(instantiationService); } public get providerId(): string { return SQL_NOTEBOOK_PROVIDER; } - getNotebookManager(notebookUri: URI): Thenable { + getExecuteManager(notebookUri: URI): Thenable { return Promise.resolve(this.manager); } diff --git a/src/sql/workbench/services/notebook/browser/sql/sqlSerializationManager.ts b/src/sql/workbench/services/notebook/browser/sql/sqlSerializationManager.ts new file mode 100644 index 0000000000..4f6f5ae609 --- /dev/null +++ b/src/sql/workbench/services/notebook/browser/sql/sqlSerializationManager.ts @@ -0,0 +1,26 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the Source EULA. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { nb } from 'azdata'; + +import { ISerializationManager, SQL_NOTEBOOK_PROVIDER } from 'sql/workbench/services/notebook/browser/notebookService'; +import { LocalContentManager } from 'sql/workbench/services/notebook/common/localContentManager'; +import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; + +export class SqlSerializationManager implements ISerializationManager { + private _manager: LocalContentManager; + + constructor(instantiationService: IInstantiationService) { + this._manager = instantiationService.createInstance(LocalContentManager); + } + + public get providerId(): string { + return SQL_NOTEBOOK_PROVIDER; + } + + public get contentManager(): nb.ContentManager { + return this._manager; + } +} diff --git a/src/sql/workbench/services/notebook/browser/sql/sqlSerializationProvider.ts b/src/sql/workbench/services/notebook/browser/sql/sqlSerializationProvider.ts new file mode 100644 index 0000000000..4266ad7d38 --- /dev/null +++ b/src/sql/workbench/services/notebook/browser/sql/sqlSerializationProvider.ts @@ -0,0 +1,26 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the Source EULA. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { URI } from 'vs/base/common/uri'; + +import { ISerializationManager, ISerializationProvider, SQL_NOTEBOOK_PROVIDER } from 'sql/workbench/services/notebook/browser/notebookService'; +import { SqlSerializationManager } from 'sql/workbench/services/notebook/browser/sql/sqlSerializationManager'; +import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; + +export class SqlSerializationProvider implements ISerializationProvider { + private _manager: SqlSerializationManager; + + constructor(instantiationService: IInstantiationService) { + this._manager = new SqlSerializationManager(instantiationService); + } + + public get providerId(): string { + return SQL_NOTEBOOK_PROVIDER; + } + + getSerializationManager(notebookUri: URI): Thenable { + return Promise.resolve(this._manager); + } +} diff --git a/src/sql/workbench/services/notebook/common/localContentManager.ts b/src/sql/workbench/services/notebook/common/localContentManager.ts index 3aded613bb..e96a6142a4 100644 --- a/src/sql/workbench/services/notebook/common/localContentManager.ts +++ b/src/sql/workbench/services/notebook/common/localContentManager.ts @@ -8,81 +8,45 @@ import { nb } from 'azdata'; import * as json from 'vs/base/common/json'; -import { URI } from 'vs/base/common/uri'; import { localize } from 'vs/nls'; -import { IFileService } from 'vs/platform/files/common/files'; import { JSONObject } from 'sql/workbench/services/notebook/common/jsonext'; import { OutputTypes } from 'sql/workbench/services/notebook/common/contracts'; import { nbversion } from 'sql/workbench/services/notebook/common/notebookConstants'; import { nbformat } from 'sql/workbench/services/notebook/common/nbformat'; -import { VSBuffer } from 'vs/base/common/buffer'; type MimeBundle = { [key: string]: string | string[] | undefined }; export class LocalContentManager implements nb.ContentManager { - constructor(@IFileService private readonly fileService: IFileService) { } + constructor() { } - public async loadFromContentString(contentString: string): Promise { - let contents: JSONObject; - if (contentString === '' || contentString === undefined) { + public async deserializeNotebook(contents: string): Promise { + let jsonContents: JSONObject; + if (contents === '' || contents === undefined) { return v4.createEmptyNotebook(); } else { - contents = this.parseFromJson(contentString); + jsonContents = this.parseFromJson(contents); } - if (contents) { - if (contents.nbformat === 4) { - return v4.readNotebook(contents); - } else if (contents.nbformat === 3) { - return v3.readNotebook(contents); + if (jsonContents) { + if (jsonContents.nbformat === 4) { + return v4.readNotebook(jsonContents); + } else if (jsonContents.nbformat === 3) { + return v3.readNotebook(jsonContents); } - if (contents.nbformat) { - throw new TypeError(localize('nbformatNotRecognized', "nbformat v{0}.{1} not recognized", contents.nbformat as any, contents.nbformat_minor as any)); + if (jsonContents.nbformat) { + throw new TypeError(localize('nbformatNotRecognized', "nbformat v{0}.{1} not recognized", jsonContents.nbformat as any, jsonContents.nbformat_minor as any)); } } // else, fallthrough condition throw new TypeError(localize('nbNotSupported', "This file does not have a valid notebook format")); - } - public async getNotebookContents(notebookUri: URI): Promise { - if (!notebookUri) { - return undefined; - } - // Note: intentionally letting caller handle exceptions - let notebookFileBuffer = await this.fileService.readFile(notebookUri); - let stringContents = notebookFileBuffer.value.toString(); - let contents: JSONObject; - if (stringContents === '' || stringContents === undefined) { - // Empty? - return v4.createEmptyNotebook(); - } else { - contents = this.parseFromJson(stringContents); - } - - if (contents) { - if (contents.nbformat === 4) { - return v4.readNotebook(contents); - } else if (contents.nbformat === 3) { - return v3.readNotebook(contents); - } - if (contents.nbformat) { - throw new TypeError(localize('nbformatNotRecognized', "nbformat v{0}.{1} not recognized", contents.nbformat as any, contents.nbformat_minor as any)); - } - } - - // else, fallthrough condition - throw new TypeError(localize('nbNotSupported', "This file does not have a valid notebook format")); - - } - - public async save(notebookUri: URI, notebook: nb.INotebookContents): Promise { + public async serializeNotebook(notebook: nb.INotebookContents): Promise { // Convert to JSON with pretty-print functionality let contents = JSON.stringify(notebook, undefined, ' '); - await this.fileService.writeFile(notebookUri, VSBuffer.fromString(contents)); - return notebook; + return contents; } private parseFromJson(contentString: string): JSONObject { diff --git a/src/sql/workbench/services/notebook/common/notebookRegistry.ts b/src/sql/workbench/services/notebook/common/notebookRegistry.ts index e3256e0a18..909e9ad1b9 100644 --- a/src/sql/workbench/services/notebook/common/notebookRegistry.ts +++ b/src/sql/workbench/services/notebook/common/notebookRegistry.ts @@ -4,24 +4,26 @@ *--------------------------------------------------------------------------------------------*/ import { IJSONSchema } from 'vs/base/common/jsonSchema'; -import { ExtensionsRegistry, IExtensionPointUser } from 'vs/workbench/services/extensions/common/extensionsRegistry'; +import { ExtensionsRegistry } from 'vs/workbench/services/extensions/common/extensionsRegistry'; import { localize } from 'vs/nls'; import * as platform from 'vs/platform/registry/common/platform'; import * as azdata from 'azdata'; import { Event, Emitter } from 'vs/base/common/event'; +export const NotebookProviderRegistryId = 'notebooks.providers.registry'; + export const Extensions = { - NotebookProviderContribution: 'notebook.providers', + NotebookProviderDescriptionContribution: 'notebook.providers', NotebookLanguageMagicContribution: 'notebook.languagemagics' }; -export interface NotebookProviderRegistration { +export interface ProviderDescriptionRegistration { provider: string; fileExtensions: string | string[]; standardKernels: azdata.nb.IStandardKernel | azdata.nb.IStandardKernel[]; } -let notebookProviderType: IJSONSchema = { +let providerDescriptionType: IJSONSchema = { type: 'object', default: { provider: '', fileExtensions: [], standardKernels: [] }, properties: { @@ -86,13 +88,13 @@ let notebookProviderType: IJSONSchema = { } }; -let notebookContrib: IJSONSchema = { - description: localize('vscode.extension.contributes.notebook.providers', "Contributes notebook providers."), +let providerDescriptionContrib: IJSONSchema = { + description: localize('vscode.extension.contributes.notebook.providersDescriptions', "Contributes notebook provider descriptions."), oneOf: [ - notebookProviderType, + providerDescriptionType, { type: 'array', - items: notebookProviderType + items: providerDescriptionType } ] }; @@ -146,81 +148,69 @@ export interface NotebookLanguageMagicRegistration { } export interface INotebookProviderRegistry { - readonly providers: NotebookProviderRegistration[]; + readonly providerDescriptions: ProviderDescriptionRegistration[]; readonly languageMagics: NotebookLanguageMagicRegistration[]; - readonly onNewRegistration: Event<{ id: string, registration: NotebookProviderRegistration }>; - registerNotebookProvider(provider: NotebookProviderRegistration): void; + readonly onNewDescriptionRegistration: Event<{ id: string, registration: ProviderDescriptionRegistration }>; + + registerProviderDescription(provider: ProviderDescriptionRegistration): void; registerNotebookLanguageMagic(magic: NotebookLanguageMagicRegistration): void; } class NotebookProviderRegistry implements INotebookProviderRegistry { - private providerIdToRegistration = new Map(); - private magicToRegistration = new Map(); - private _onNewRegistration = new Emitter<{ id: string, registration: NotebookProviderRegistration }>(); - public readonly onNewRegistration: Event<{ id: string, registration: NotebookProviderRegistration }> = this._onNewRegistration.event; + private _providerDescriptionRegistration = new Map(); + private _magicToRegistration = new Map(); - registerNotebookProvider(registration: NotebookProviderRegistration): void { - // Note: this method intentionally overrides default provider for a file type. - // This means that any built-in provider will be overridden by registered extensions - this.providerIdToRegistration.set(registration.provider, registration); - this._onNewRegistration.fire({ id: registration.provider, registration: registration }); + private _onNewDescriptionRegistration = new Emitter<{ id: string, registration: ProviderDescriptionRegistration }>(); + public readonly onNewDescriptionRegistration: Event<{ id: string, registration: ProviderDescriptionRegistration }> = this._onNewDescriptionRegistration.event; + + registerProviderDescription(registration: ProviderDescriptionRegistration): void { + this._providerDescriptionRegistration.set(registration.provider, registration); + this._onNewDescriptionRegistration.fire({ id: registration.provider, registration: registration }); } - public get providers(): NotebookProviderRegistration[] { - let registrationArray: NotebookProviderRegistration[] = []; - this.providerIdToRegistration.forEach(p => registrationArray.push(p)); + public get providerDescriptions(): ProviderDescriptionRegistration[] { + let registrationArray: ProviderDescriptionRegistration[] = []; + this._providerDescriptionRegistration.forEach(p => registrationArray.push(p)); return registrationArray; } registerNotebookLanguageMagic(magicRegistration: NotebookLanguageMagicRegistration): void { - this.magicToRegistration.set(magicRegistration.magic, magicRegistration); + this._magicToRegistration.set(magicRegistration.magic, magicRegistration); } public get languageMagics(): NotebookLanguageMagicRegistration[] { let registrationArray: NotebookLanguageMagicRegistration[] = []; - this.magicToRegistration.forEach(p => registrationArray.push(p)); + this._magicToRegistration.forEach(p => registrationArray.push(p)); return registrationArray; } - } const notebookProviderRegistry = new NotebookProviderRegistry(); -platform.Registry.add(Extensions.NotebookProviderContribution, notebookProviderRegistry); - - -ExtensionsRegistry.registerExtensionPoint({ extensionPoint: Extensions.NotebookProviderContribution, jsonSchema: notebookContrib }).setHandler(extensions => { - - function handleExtension(contrib: NotebookProviderRegistration, extension: IExtensionPointUser) { - notebookProviderRegistry.registerNotebookProvider(contrib); - } +platform.Registry.add(NotebookProviderRegistryId, notebookProviderRegistry); +ExtensionsRegistry.registerExtensionPoint({ extensionPoint: Extensions.NotebookProviderDescriptionContribution, jsonSchema: providerDescriptionContrib }).setHandler(extensions => { for (let extension of extensions) { const { value } = extension; if (Array.isArray(value)) { for (let command of value) { - handleExtension(command, extension); + notebookProviderRegistry.registerProviderDescription(command); } } else { - handleExtension(value, extension); + notebookProviderRegistry.registerProviderDescription(value); } } }); ExtensionsRegistry.registerExtensionPoint({ extensionPoint: Extensions.NotebookLanguageMagicContribution, jsonSchema: languageMagicContrib }).setHandler(extensions => { - - function handleExtension(contrib: NotebookLanguageMagicRegistration, extension: IExtensionPointUser) { - notebookProviderRegistry.registerNotebookLanguageMagic(contrib); - } - for (let extension of extensions) { const { value } = extension; if (Array.isArray(value)) { for (let command of value) { - handleExtension(command, extension); + notebookProviderRegistry.registerNotebookLanguageMagic(command); } } else { - handleExtension(value, extension); + notebookProviderRegistry.registerNotebookLanguageMagic(value); } } }); diff --git a/src/sql/workbench/test/electron-browser/api/exthostNotebook.test.ts b/src/sql/workbench/test/electron-browser/api/exthostNotebook.test.ts index 348bf02a39..41274c7e91 100644 --- a/src/sql/workbench/test/electron-browser/api/exthostNotebook.test.ts +++ b/src/sql/workbench/test/electron-browser/api/exthostNotebook.test.ts @@ -13,19 +13,21 @@ import { IMainContext } from 'vs/workbench/api/common/extHost.protocol'; import { ExtHostNotebook } from 'sql/workbench/api/common/extHostNotebook'; import { MainThreadNotebookShape } from 'sql/workbench/api/common/sqlExtHost.protocol'; -import { INotebookManagerDetails } from 'sql/workbench/api/common/sqlExtHostTypes'; -import { mssqlProviderName } from 'sql/platform/connection/common/constants'; +import { IExecuteManagerDetails, ISerializationManagerDetails } from 'sql/workbench/api/common/sqlExtHostTypes'; suite('ExtHostNotebook Tests', () => { let extHostNotebook: ExtHostNotebook; let mockProxy: TypeMoq.Mock; let notebookUri: URI; - let notebookProviderMock: TypeMoq.Mock; + let serializationProviderMock: TypeMoq.Mock; + let executeProviderMock: TypeMoq.Mock; setup(() => { mockProxy = TypeMoq.Mock.ofInstance({ - $registerNotebookProvider: (providerId, handle) => undefined, - $unregisterNotebookProvider: (handle) => undefined, + $registerSerializationProvider: (providerId, handle) => undefined, + $registerExecuteProvider: (providerId, handle) => undefined, + $unregisterSerializationProvider: (handle) => undefined, + $unregisterExecuteProvider: (handle) => undefined, dispose: () => undefined }); let mainContext = { @@ -33,71 +35,115 @@ suite('ExtHostNotebook Tests', () => { }; extHostNotebook = new ExtHostNotebook(mainContext); notebookUri = URI.parse('file:/user/default/my.ipynb'); - notebookProviderMock = TypeMoq.Mock.ofType(NotebookProviderStub, TypeMoq.MockBehavior.Loose); - notebookProviderMock.callBase = true; + serializationProviderMock = TypeMoq.Mock.ofType(SerializationProviderStub, TypeMoq.MockBehavior.Loose); + serializationProviderMock.callBase = true; + executeProviderMock = TypeMoq.Mock.ofType(ExecuteProviderStub, TypeMoq.MockBehavior.Loose); + executeProviderMock.callBase = true; }); - suite('getNotebookManager', () => { - test('Should throw if no matching provider is defined', async () => { + suite('get notebook managers', () => { + test('Should throw if no matching serialization provider is defined', async () => { try { - await extHostNotebook.$getNotebookManager(-1, notebookUri); + await extHostNotebook.$getSerializationManagerDetails(-1, notebookUri); + assert.fail('expected to throw'); + } catch (e) { } + }); + test('Should throw if no matching execute provider is defined', async () => { + try { + await extHostNotebook.$getExecuteManagerDetails(-1, notebookUri); assert.fail('expected to throw'); } catch (e) { } }); suite('with provider', () => { - let providerHandle: number = -1; + let serializationProviderHandle: number = -1; + let executeProviderHandle: number = -1; setup(() => { mockProxy.setup(p => - p.$registerNotebookProvider(TypeMoq.It.isValue(notebookProviderMock.object.providerId), TypeMoq.It.isAnyNumber())) + p.$registerSerializationProvider(TypeMoq.It.isValue(serializationProviderMock.object.providerId), TypeMoq.It.isAnyNumber())) .returns((providerId, handle) => { - providerHandle = handle; + serializationProviderHandle = handle; + return undefined; + }); + mockProxy.setup(p => + p.$registerExecuteProvider(TypeMoq.It.isValue(executeProviderMock.object.providerId), TypeMoq.It.isAnyNumber())) + .returns((providerId, handle) => { + executeProviderHandle = handle; return undefined; }); // Register the provider so we can test behavior with this present - extHostNotebook.registerNotebookProvider(notebookProviderMock.object); + extHostNotebook.registerSerializationProvider(serializationProviderMock.object); + extHostNotebook.registerExecuteProvider(executeProviderMock.object); }); - test('Should return a notebook manager with correct info on content and server manager existence', async () => { + test('Should return a serialization manager with correct info on content manager existence', async () => { // Given the provider returns a manager with no - let expectedManager = new NotebookManagerStub(); - notebookProviderMock.setup(p => p.getNotebookManager(TypeMoq.It.isAny())).returns(() => Promise.resolve(expectedManager)); + let expectedManager = new SerializationManagerStub(); + serializationProviderMock.setup(p => p.getSerializationManager(TypeMoq.It.isAny())).returns(() => Promise.resolve(expectedManager)); // When I call through using the handle provided during registration - let managerDetails: INotebookManagerDetails = await extHostNotebook.$getNotebookManager(providerHandle, notebookUri); + let managerDetails: ISerializationManagerDetails = await extHostNotebook.$getSerializationManagerDetails(serializationProviderHandle, notebookUri); // Then I expect the same manager to be returned assert.ok(managerDetails.hasContentManager === false, 'Expect no content manager defined'); + assert.ok(managerDetails.handle > 0, 'Expect a valid handle defined'); + }); + + test('Should return an execute manager with correct info on whether server manager exists', async () => { + // An execute manager with no server manager + let expectedManager = new ExecuteManagerStub(); + executeProviderMock.setup(p => p.getExecuteManager(TypeMoq.It.isAny())).returns(() => Promise.resolve(expectedManager)); + + // When I call through using the handle provided during registration + let managerDetails: IExecuteManagerDetails = await extHostNotebook.$getExecuteManagerDetails(executeProviderHandle, notebookUri); + + // Then I expect the same manager to be returned assert.ok(managerDetails.hasServerManager === false, 'Expect no server manager defined'); assert.ok(managerDetails.handle > 0, 'Expect a valid handle defined'); }); - test('Should have a unique handle for each notebook URI', async () => { + test('Should have a unique serialization provider handle for each notebook URI', async () => { // Given the we request 2 URIs - let expectedManager = new NotebookManagerStub(); - notebookProviderMock.setup(p => p.getNotebookManager(TypeMoq.It.isAny())).returns(() => Promise.resolve(expectedManager)); + let expectedManager = new SerializationManagerStub(); + serializationProviderMock.setup(p => p.getSerializationManager(TypeMoq.It.isAny())).returns(() => Promise.resolve(expectedManager)); // When I call through using the handle provided during registration - let originalManagerDetails = await extHostNotebook.$getNotebookManager(providerHandle, notebookUri); - let differentDetails = await extHostNotebook.$getNotebookManager(providerHandle, URI.parse('file://other/file.ipynb')); - let sameDetails = await extHostNotebook.$getNotebookManager(providerHandle, notebookUri); + let originalManagerDetails = await extHostNotebook.$getSerializationManagerDetails(serializationProviderHandle, notebookUri); + let differentDetails = await extHostNotebook.$getSerializationManagerDetails(serializationProviderHandle, URI.parse('file://other/file.ipynb')); + let sameDetails = await extHostNotebook.$getSerializationManagerDetails(serializationProviderHandle, notebookUri); // Then I expect the 2 different handles in the managers returned. // This is because we can't easily track identity of the managers, so just track which one is assigned to // a notebook by the handle ID assert.notStrictEqual(originalManagerDetails.handle, differentDetails.handle, 'Should have unique handle for each manager'); assert.strictEqual(originalManagerDetails.handle, sameDetails.handle, 'Should have same handle when same URI is passed in'); + }); + test('Should have a unique execute provider handle for each notebook URI', async () => { + // Given the we request 2 URIs + let expectedManager = new ExecuteManagerStub(); + executeProviderMock.setup(p => p.getExecuteManager(TypeMoq.It.isAny())).returns(() => Promise.resolve(expectedManager)); + + // When I call through using the handle provided during registration + let originalManagerDetails = await extHostNotebook.$getExecuteManagerDetails(executeProviderHandle, notebookUri); + let differentDetails = await extHostNotebook.$getExecuteManagerDetails(executeProviderHandle, URI.parse('file://other/file.ipynb')); + let sameDetails = await extHostNotebook.$getExecuteManagerDetails(executeProviderHandle, notebookUri); + + // Then I expect the 2 different handles in the managers returned. + // This is because we can't easily track identity of the managers, so just track which one is assigned to + // a notebook by the handle ID + assert.notStrictEqual(originalManagerDetails.handle, differentDetails.handle, 'Should have unique handle for each manager'); + assert.strictEqual(originalManagerDetails.handle, sameDetails.handle, 'Should have same handle when same URI is passed in'); }); }); }); - suite('registerNotebookProvider', () => { + suite('registerSerializationProvider', () => { let savedHandle: number = -1; setup(() => { mockProxy.setup(p => - p.$registerNotebookProvider(TypeMoq.It.isValue(notebookProviderMock.object.providerId), TypeMoq.It.isAnyNumber())) + p.$registerSerializationProvider(TypeMoq.It.isValue(serializationProviderMock.object.providerId), TypeMoq.It.isAnyNumber())) .returns((providerId, handle) => { savedHandle = handle; return undefined; @@ -105,28 +151,61 @@ suite('ExtHostNotebook Tests', () => { }); test('Should register with a new handle to the proxy', () => { - extHostNotebook.registerNotebookProvider(notebookProviderMock.object); + extHostNotebook.registerSerializationProvider(serializationProviderMock.object); mockProxy.verify(p => - p.$registerNotebookProvider(TypeMoq.It.isValue(notebookProviderMock.object.providerId), + p.$registerSerializationProvider(TypeMoq.It.isValue(serializationProviderMock.object.providerId), TypeMoq.It.isAnyNumber()), TypeMoq.Times.once()); // It shouldn't unregister until requested - mockProxy.verify(p => p.$unregisterNotebookProvider(TypeMoq.It.isValue(savedHandle)), TypeMoq.Times.never()); - + mockProxy.verify(p => p.$unregisterSerializationProvider(TypeMoq.It.isValue(savedHandle)), TypeMoq.Times.never()); }); test('Should not call unregister on disposing', () => { - let disposable = extHostNotebook.registerNotebookProvider(notebookProviderMock.object); + let disposable = extHostNotebook.registerSerializationProvider(serializationProviderMock.object); disposable.dispose(); - mockProxy.verify(p => p.$unregisterNotebookProvider(TypeMoq.It.isValue(savedHandle)), TypeMoq.Times.never()); + mockProxy.verify(p => p.$unregisterSerializationProvider(TypeMoq.It.isValue(savedHandle)), TypeMoq.Times.never()); + }); + }); + + suite('registerExecuteProvider', () => { + let savedHandle: number = -1; + setup(() => { + mockProxy.setup(p => + p.$registerExecuteProvider(TypeMoq.It.isValue(executeProviderMock.object.providerId), TypeMoq.It.isAnyNumber())) + .returns((providerId, handle) => { + savedHandle = handle; + return undefined; + }); + }); + + test('Should register with a new handle to the proxy', () => { + extHostNotebook.registerExecuteProvider(executeProviderMock.object); + mockProxy.verify(p => + p.$registerExecuteProvider(TypeMoq.It.isValue(executeProviderMock.object.providerId), + TypeMoq.It.isAnyNumber()), TypeMoq.Times.once()); + // It shouldn't unregister until requested + mockProxy.verify(p => p.$unregisterExecuteProvider(TypeMoq.It.isValue(savedHandle)), TypeMoq.Times.never()); + }); + + test('Should not call unregister on disposing', () => { + let disposable = extHostNotebook.registerExecuteProvider(executeProviderMock.object); + disposable.dispose(); + mockProxy.verify(p => p.$unregisterExecuteProvider(TypeMoq.It.isValue(savedHandle)), TypeMoq.Times.never()); }); }); }); -class NotebookProviderStub implements azdata.nb.NotebookProvider { +class SerializationProviderStub implements azdata.nb.NotebookSerializationProvider { providerId: string = 'TestProvider'; - standardKernels: azdata.nb.IStandardKernel[] = [{ name: 'fakeKernel', displayName: 'fakeKernel', connectionProviderIds: [mssqlProviderName] }]; - getNotebookManager(notebookUri: vscode.Uri): Thenable { + getSerializationManager(notebookUri: vscode.Uri): Thenable { + throw new Error('Method not implemented.'); + } +} + +class ExecuteProviderStub implements azdata.nb.NotebookExecuteProvider { + providerId: string = 'TestProvider'; + + getExecuteManager(notebookUri: vscode.Uri): Thenable { throw new Error('Method not implemented.'); } handleNotebookClosed(notebookUri: vscode.Uri): void { @@ -134,11 +213,13 @@ class NotebookProviderStub implements azdata.nb.NotebookProvider { } } -class NotebookManagerStub implements azdata.nb.NotebookManager { +class SerializationManagerStub implements azdata.nb.SerializationManager { get contentManager(): azdata.nb.ContentManager { return undefined; } +} +class ExecuteManagerStub implements azdata.nb.ExecuteManager { get sessionManager(): azdata.nb.SessionManager { return undefined; } diff --git a/src/sql/workbench/test/electron-browser/api/mainThreadNotebook.test.ts b/src/sql/workbench/test/electron-browser/api/mainThreadNotebook.test.ts index 73383c2496..02e29de0e0 100644 --- a/src/sql/workbench/test/electron-browser/api/mainThreadNotebook.test.ts +++ b/src/sql/workbench/test/electron-browser/api/mainThreadNotebook.test.ts @@ -12,8 +12,8 @@ import { IExtHostContext } from 'vs/workbench/api/common/extHost.protocol'; import { MainThreadNotebook } from 'sql/workbench/api/browser/mainThreadNotebook'; import { NotebookService } from 'sql/workbench/services/notebook/browser/notebookServiceImpl'; -import { INotebookProvider } from 'sql/workbench/services/notebook/browser/notebookService'; -import { INotebookManagerDetails, INotebookSessionDetails, INotebookKernelDetails, INotebookFutureDetails } from 'sql/workbench/api/common/sqlExtHostTypes'; +import { IExecuteProvider, ISerializationProvider } from 'sql/workbench/services/notebook/browser/notebookService'; +import { IExecuteManagerDetails, INotebookSessionDetails, INotebookKernelDetails, INotebookFutureDetails, ISerializationManagerDetails } from 'sql/workbench/api/common/sqlExtHostTypes'; import { LocalContentManager } from 'sql/workbench/services/notebook/common/localContentManager'; import { TestLifecycleService } from 'vs/workbench/test/browser/workbenchTestServices'; import { MockContextKeyService } from 'vs/platform/keybinding/test/common/mockKeybindingService'; @@ -57,80 +57,123 @@ suite('MainThreadNotebook Tests', () => { mainThreadNotebook = new MainThreadNotebook(extContext, mockNotebookService.object, instantiationService); }); - suite('On registering a provider', () => { - let provider: INotebookProvider; + suite('On registering a serialization provider', () => { + let provider: ISerializationProvider; setup(() => { - mockNotebookService.setup(s => s.registerProvider(TypeMoq.It.isAnyString(), TypeMoq.It.isAny())).returns((id, providerImpl) => { + mockNotebookService.setup(s => s.registerSerializationProvider(TypeMoq.It.isAnyString(), TypeMoq.It.isAny())).returns((id, providerImpl) => { provider = providerImpl; }); }); test('should call through to notebook service', () => { // When I register a provider - mainThreadNotebook.$registerNotebookProvider(providerId, 1); + mainThreadNotebook.$registerSerializationProvider(providerId, 1); // Then I expect a provider implementation to be passed to the service - mockNotebookService.verify(s => s.registerProvider(TypeMoq.It.isAnyString(), TypeMoq.It.isAny()), TypeMoq.Times.once()); + mockNotebookService.verify(s => s.registerSerializationProvider(TypeMoq.It.isAnyString(), TypeMoq.It.isAny()), TypeMoq.Times.once()); assert.strictEqual(provider.providerId, providerId); }); test('should unregister in service', () => { // Given we have a provider - mainThreadNotebook.$registerNotebookProvider(providerId, 1); + mainThreadNotebook.$registerSerializationProvider(providerId, 1); // When I unregister a provider twice - mainThreadNotebook.$unregisterNotebookProvider(1); - mainThreadNotebook.$unregisterNotebookProvider(1); + mainThreadNotebook.$unregisterSerializationProvider(1); + mainThreadNotebook.$unregisterSerializationProvider(1); // Then I expect it to be unregistered in the service just 1 time - mockNotebookService.verify(s => s.unregisterProvider(TypeMoq.It.isValue(providerId)), TypeMoq.Times.once()); + mockNotebookService.verify(s => s.unregisterSerializationProvider(TypeMoq.It.isValue(providerId)), TypeMoq.Times.once()); }); }); - suite('getNotebookManager', () => { - let managerWithAllFeatures: INotebookManagerDetails; - let provider: INotebookProvider; - + suite('On registering an execute provider', () => { + let provider: IExecuteProvider; setup(() => { - managerWithAllFeatures = { - handle: 2, - hasContentManager: true, - hasServerManager: true - }; - mockNotebookService.setup(s => s.registerProvider(TypeMoq.It.isAnyString(), TypeMoq.It.isAny())).returns((id, providerImpl) => { + mockNotebookService.setup(s => s.registerExecuteProvider(TypeMoq.It.isAnyString(), TypeMoq.It.isAny())).returns((id, providerImpl) => { provider = providerImpl; }); - mainThreadNotebook.$registerNotebookProvider(providerId, 1); + }); + + test('should call through to notebook service', () => { + // When I register a provider + mainThreadNotebook.$registerExecuteProvider(providerId, 1); + // Then I expect a provider implementation to be passed to the service + mockNotebookService.verify(s => s.registerExecuteProvider(TypeMoq.It.isAnyString(), TypeMoq.It.isAny()), TypeMoq.Times.once()); + assert.strictEqual(provider.providerId, providerId); + }); + test('should unregister in service', () => { + // Given we have a provider + mainThreadNotebook.$registerExecuteProvider(providerId, 1); + // When I unregister a provider twice + mainThreadNotebook.$unregisterExecuteProvider(1); + mainThreadNotebook.$unregisterExecuteProvider(1); + // Then I expect it to be unregistered in the service just 1 time + mockNotebookService.verify(s => s.unregisterExecuteProvider(TypeMoq.It.isValue(providerId)), TypeMoq.Times.once()); + }); + }); + + suite('get notebook managers', () => { + let serializationManagerWithAllFeatures: ISerializationManagerDetails; + let executeManagerWithAllFeatures: IExecuteManagerDetails; + let serializationProvider: ISerializationProvider; + let executeProvider: IExecuteProvider; + + setup(() => { + serializationManagerWithAllFeatures = { + handle: 3, + hasContentManager: true, + }; + executeManagerWithAllFeatures = { + handle: 4, + hasServerManager: true + }; + mockNotebookService.setup(s => s.registerSerializationProvider(TypeMoq.It.isAnyString(), TypeMoq.It.isAny())).returns((id, providerImpl) => { + serializationProvider = providerImpl; + }); + mainThreadNotebook.$registerSerializationProvider(providerId, 1); + mockNotebookService.setup(s => s.registerExecuteProvider(TypeMoq.It.isAnyString(), TypeMoq.It.isAny())).returns((id, providerImpl) => { + executeProvider = providerImpl; + }); + mainThreadNotebook.$registerExecuteProvider(providerId, 2); // Always return empty specs in this test suite mockProxy.setup(p => p.$refreshSpecs(TypeMoq.It.isAnyNumber())).returns(() => Promise.resolve(undefined)); }); - test('should return manager with default content manager & undefined server manager if extension host has none', async () => { + test('should return execute manager with undefined server manager if extension host has none', async () => { // Given the extension provider doesn't have acontent or server manager - let details: INotebookManagerDetails = { + let details: IExecuteManagerDetails = { handle: 2, - hasContentManager: false, hasServerManager: false }; - mockProxy.setup(p => p.$getNotebookManager(TypeMoq.It.isAnyNumber(), TypeMoq.It.isValue(notebookUri))) + mockProxy.setup(p => p.$getExecuteManagerDetails(TypeMoq.It.isAnyNumber(), TypeMoq.It.isValue(notebookUri))) .returns(() => Promise.resolve(details)); // When I get the notebook manager - let manager = await provider.getNotebookManager(notebookUri); + let manager = await executeProvider.getExecuteManager(notebookUri); - // Then it should use the built-in content manager - assert.ok(manager.contentManager instanceof LocalContentManager); // And it should not define a server manager assert.strictEqual(manager.serverManager, undefined); }); - test('should return manager with a content & server manager if extension host has these', async () => { + test('should return serialization manager with a content manager if extension host has these', async () => { // Given the extension provider doesn't have acontent or server manager - mockProxy.setup(p => p.$getNotebookManager(TypeMoq.It.isAnyNumber(), TypeMoq.It.isValue(notebookUri))) - .returns(() => Promise.resolve(managerWithAllFeatures)); + mockProxy.setup(p => p.$getSerializationManagerDetails(TypeMoq.It.isAnyNumber(), TypeMoq.It.isValue(notebookUri))) + .returns(() => Promise.resolve(serializationManagerWithAllFeatures)); // When I get the notebook manager - let manager = await provider.getNotebookManager(notebookUri); + let manager = await serializationProvider.getSerializationManager(notebookUri); - // Then it shouldn't have wrappers for the content or server manager + // Then it shouldn't have wrappers for the content manager assert.ok(!(manager.contentManager instanceof LocalContentManager)); + }); + + test('should return execute manager with a server manager if extension host has these', async () => { + // Given the extension provider doesn't have a content or server manager + mockProxy.setup(p => p.$getExecuteManagerDetails(TypeMoq.It.isAnyNumber(), TypeMoq.It.isValue(notebookUri))) + .returns(() => Promise.resolve(executeManagerWithAllFeatures)); + + // When I get the notebook manager + let manager = await executeProvider.getExecuteManager(notebookUri); + + // Then it shouldn't have wrappers for the server manager assert.notStrictEqual(manager.serverManager, undefined); }); }); @@ -138,7 +181,10 @@ suite('MainThreadNotebook Tests', () => { }); class ExtHostNotebookStub implements ExtHostNotebookShape { - $getNotebookManager(providerHandle: number, notebookUri: UriComponents): Thenable { + $getSerializationManagerDetails(providerHandle: number, notebookUri: UriComponents): Thenable { + throw new Error('Method not implemented.'); + } + $getExecuteManagerDetails(providerHandle: number, notebookUri: UriComponents): Thenable { throw new Error('Method not implemented.'); } $handleNotebookClosed(notebookUri: UriComponents): void { @@ -150,10 +196,10 @@ class ExtHostNotebookStub implements ExtHostNotebookShape { $doStopServer(managerHandle: number): Thenable { throw new Error('Method not implemented.'); } - $getNotebookContents(managerHandle: number, notebookUri: UriComponents): Thenable { + $deserializeNotebook(managerHandle: number, contents: string): Thenable { throw new Error('Method not implemented.'); } - $save(managerHandle: number, notebookUri: UriComponents, notebook: azdata.nb.INotebookContents): Thenable { + $serializeNotebook(managerHandle: number, notebook: azdata.nb.INotebookContents): Thenable { throw new Error('Method not implemented.'); } $refreshSpecs(managerHandle: number): Thenable { diff --git a/src/vs/workbench/api/browser/mainThreadDocumentsAndEditors.ts b/src/vs/workbench/api/browser/mainThreadDocumentsAndEditors.ts index d728872e85..7090610dd7 100644 --- a/src/vs/workbench/api/browser/mainThreadDocumentsAndEditors.ts +++ b/src/vs/workbench/api/browser/mainThreadDocumentsAndEditors.ts @@ -31,6 +31,7 @@ import { IUriIdentityService } from 'vs/workbench/services/uriIdentity/common/ur import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService'; import { IPathService } from 'vs/workbench/services/path/common/pathService'; import { diffSets, diffMaps } from 'vs/base/common/collections'; +import { INotebookService } from 'sql/workbench/services/notebook/browser/notebookService'; class TextEditorSnapshot { @@ -301,14 +302,15 @@ export class MainThreadDocumentsAndEditors { @IWorkingCopyFileService workingCopyFileService: IWorkingCopyFileService, @IUriIdentityService uriIdentityService: IUriIdentityService, @IClipboardService private readonly _clipboardService: IClipboardService, - @IPathService pathService: IPathService + @IPathService pathService: IPathService, + @INotebookService private readonly _notebookService: INotebookService // {{SQL CARBON EDIT}} ) { this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostDocumentsAndEditors); this._mainThreadDocuments = this._toDispose.add(new MainThreadDocuments(this, extHostContext, this._modelService, this._textFileService, fileService, textModelResolverService, environmentService, uriIdentityService, workingCopyFileService, pathService)); extHostContext.set(MainContext.MainThreadDocuments, this._mainThreadDocuments); - const mainThreadTextEditors = this._toDispose.add(new MainThreadTextEditors(this, extHostContext, codeEditorService, bulkEditService, this._editorService, this._editorGroupService)); + const mainThreadTextEditors = this._toDispose.add(new MainThreadTextEditors(this, extHostContext, codeEditorService, bulkEditService, this._editorService, this._editorGroupService, this._notebookService)); extHostContext.set(MainContext.MainThreadTextEditors, mainThreadTextEditors); // It is expected that the ctor of the state computer calls our `_onDelta`. diff --git a/src/vs/workbench/api/browser/mainThreadEditors.ts b/src/vs/workbench/api/browser/mainThreadEditors.ts index 728bbac61b..c51d5cfc3c 100644 --- a/src/vs/workbench/api/browser/mainThreadEditors.ts +++ b/src/vs/workbench/api/browser/mainThreadEditors.ts @@ -27,6 +27,7 @@ import { IWorkingCopyService } from 'vs/workbench/services/workingCopy/common/wo import { revive } from 'vs/base/common/marshalling'; import { ResourceNotebookCellEdit } from 'vs/workbench/contrib/bulkEdit/browser/bulkCellEdits'; import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions'; +import { INotebookService } from 'sql/workbench/services/notebook/browser/notebookService'; // {{SQL CARBON EDIT}} export function reviveWorkspaceEditDto2(data: IWorkspaceEditDto | undefined): ResourceEdit[] { if (!data?.edits) { @@ -64,7 +65,8 @@ export class MainThreadTextEditors implements MainThreadTextEditorsShape { @ICodeEditorService private readonly _codeEditorService: ICodeEditorService, @IBulkEditService private readonly _bulkEditService: IBulkEditService, @IEditorService private readonly _editorService: IEditorService, - @IEditorGroupsService private readonly _editorGroupService: IEditorGroupsService + @IEditorGroupsService private readonly _editorGroupService: IEditorGroupsService, + @INotebookService private readonly _notebookService: INotebookService // {{SQL CARBON EDIT}} ) { this._instanceId = String(++MainThreadTextEditors.INSTANCE_COUNT); this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostEditors); @@ -136,6 +138,8 @@ export class MainThreadTextEditors implements MainThreadTextEditorsShape { async $tryShowTextDocument(resource: UriComponents, options: ITextDocumentShowOptions): Promise { const uri = URI.revive(resource); + let notebookFileTypes = this._notebookService.getSupportedFileExtensions().map(s => s.toLowerCase()); // {{SQL CARBON EDIT}} + const editorOptions: ITextEditorOptions = { preserveFocus: options.preserveFocus, pinned: options.pinned, @@ -143,7 +147,7 @@ export class MainThreadTextEditors implements MainThreadTextEditorsShape { // preserve pre 1.38 behaviour to not make group active when preserveFocus: true // but make sure to restore the editor to fix https://github.com/microsoft/vscode/issues/79633 activation: options.preserveFocus ? EditorActivation.RESTORE : undefined, - override: uri?.fsPath?.toLowerCase().endsWith('ipynb') || uri?.fsPath?.toLowerCase().endsWith('sql') ? undefined : EditorOverride.DISABLED // {{SQL CARBON EDIT}} + override: notebookFileTypes?.some(ext => uri?.fsPath?.toLowerCase().endsWith(ext)) || uri?.fsPath?.toLowerCase().endsWith('sql') ? undefined : EditorOverride.DISABLED // {{SQL CARBON EDIT}} }; const input: IResourceEditorInput = { diff --git a/src/vs/workbench/test/browser/api/mainThreadDocumentsAndEditors.test.ts b/src/vs/workbench/test/browser/api/mainThreadDocumentsAndEditors.test.ts index b122489ca8..f5fdf747f9 100644 --- a/src/vs/workbench/test/browser/api/mainThreadDocumentsAndEditors.test.ts +++ b/src/vs/workbench/test/browser/api/mainThreadDocumentsAndEditors.test.ts @@ -28,6 +28,7 @@ import { TestNotificationService } from 'vs/platform/notification/test/common/te import { TestTextResourcePropertiesService, TestWorkingCopyFileService } from 'vs/workbench/test/common/workbenchTestServices'; import { UriIdentityService } from 'vs/workbench/services/uriIdentity/common/uriIdentityService'; import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService'; +import { INotebookService } from 'sql/workbench/services/notebook/browser/notebookService'; suite('MainThreadDocumentsAndEditors', () => { @@ -100,7 +101,10 @@ suite('MainThreadDocumentsAndEditors', () => { return Promise.resolve('clipboard_contents'); } }, - new TestPathService() + new TestPathService(), + { // {{SQL CARBON EDIT}} + getSupportedFileExtensions: () => ['ipynb'] + } ); });