diff --git a/src/sql/workbench/contrib/notebook/browser/outputs/renderers.ts b/src/sql/workbench/contrib/notebook/browser/outputs/renderers.ts index 5faebef217..f7d7c1ec86 100644 --- a/src/sql/workbench/contrib/notebook/browser/outputs/renderers.ts +++ b/src/sql/workbench/contrib/notebook/browser/outputs/renderers.ts @@ -5,7 +5,6 @@ import { default as AnsiUp } from 'ansi_up'; import { IRenderMime } from 'sql/workbench/services/notebook/browser/outputs/renderMimeInterfaces'; -import { URLExt } from 'sql/workbench/contrib/notebook/common/models/url'; import { URI } from 'vs/base/common/uri'; @@ -645,7 +644,7 @@ namespace Private { if (path && path.length > 0) { isLocal = resolver && resolver.isLocal ? resolver.isLocal(path) - : URLExt.isLocal(path); + : !!URI.parse(path).scheme; } else { // Empty string, so default to local isLocal = true; diff --git a/src/sql/workbench/contrib/notebook/common/models/url.ts b/src/sql/workbench/contrib/notebook/common/models/url.ts deleted file mode 100644 index 2425e7000c..0000000000 --- a/src/sql/workbench/contrib/notebook/common/models/url.ts +++ /dev/null @@ -1,181 +0,0 @@ -// Copyright (c) Jupyter Development Team. -// Distributed under the terms of the Modified BSD License. - -import { URI } from 'vs/base/common/uri'; -import { JSONObject } from 'sql/workbench/services/notebook/common/jsonext'; - -/** - * The namespace for URL-related functions. - */ -export namespace URLExt { - /** - * Normalize a url. - */ - export function normalize(url: string): string { - return URI.parse(url).toString(); - } - - /** - * Join a sequence of url components and normalizes as in node `path.join`. - * - * @param parts - The url components. - * - * @returns the joined url. - */ - export function join(...parts: string[]): string { - parts = parts || []; - - // Isolate the top element. - const top = parts[0] || ''; - - // Check whether protocol shorthand is being used. - const shorthand = top.indexOf('//') === 0; - - // Parse the top element into a header collection. - const header = top.match(/(\w+)(:)(\/\/)?/); - const protocol = header && header[1]; - const colon = protocol && header[2]; - const slashes = colon && header[3]; - - // Construct the URL prefix. - const prefix = shorthand - ? '//' - : [protocol, colon, slashes].filter(str => str).join(''); - - // Construct the URL body omitting the prefix of the top value. - const body = [top.indexOf(prefix) === 0 ? top.replace(prefix, '') : top] - // Filter out top value if empty. - .filter(str => str) - // Remove leading slashes in all subsequent URL body elements. - .concat(parts.slice(1).map(str => str.replace(/^\//, ''))) - .join('/') - // Replace multiple slashes with one. - .replace(/\/+/g, '/'); - - return prefix + body; - } - - /** - * Encode the components of a multi-segment url. - * - * @param url - The url to encode. - * - * @returns the encoded url. - * - * #### Notes - * Preserves the `'/'` separators. - * Should not include the base url, since all parts are escaped. - */ - export function encodeParts(url: string): string { - return join(...url.split('/').map(encodeURIComponent)); - } - - /** - * Return a serialized object string suitable for a query. - * - * @param object - The source object. - * - * @returns an encoded url query. - * - * #### Notes - * Modified version of [stackoverflow](http://stackoverflow.com/a/30707423). - */ - export function objectToQueryString(value: JSONObject): string { - const keys = Object.keys(value); - - if (!keys.length) { - return ''; - } - - return ( - '?' + - keys - .map(key => { - const content = encodeURIComponent(String(value[key])); - return key + (content ? '=' + content : ''); - }) - .join('&') - ); - } - - /** - * Return a parsed object that represents the values in a query string. - */ - export function queryStringToObject( - value: string - ): { [key: string]: string } { - return value - .replace(/^\?/, '') - .split('&') - .reduce( - (acc, val) => { - const [key, value] = val.split('='); - acc[key] = decodeURIComponent(value || ''); - return acc; - }, - {} as { [key: string]: string } - ); - } - - /** - * Test whether the url is a local url. - * - * #### Notes - * This function returns `false` for any fully qualified url, including - * `data:`, `file:`, and `//` protocol URLs. - */ - export function isLocal(url: string): boolean { - // If if doesn't have a scheme such as file: or http:// it's local - return !!URI.parse(url).scheme; - } - - /** - * The interface for a URL object - */ - export interface IUrl { - /** - * The full URL string that was parsed with both the protocol and host - * components converted to lower-case. - */ - href?: string; - - /** - * Identifies the URL's lower-cased protocol scheme. - */ - protocol?: string; - - /** - * The full lower-cased host portion of the URL, including the port if - * specified. - */ - host?: string; - - /** - * The lower-cased host name portion of the host component without the - * port included. - */ - hostname?: string; - - /** - * The numeric port portion of the host component. - */ - port?: string; - - /** - * The entire path section of the URL. - */ - pathname?: string; - - /** - * The "fragment" portion of the URL including the leading ASCII hash - * `(#)` character - */ - hash?: string; - - /** - * The search element, including leading question mark (`'?'`), if any, - * of the URL. - */ - search?: string; - } -} 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 1a398a7a45..e672b74000 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 @@ -12,7 +12,7 @@ import { TestNotificationService } from 'vs/platform/notification/test/common/te import { URI } from 'vs/base/common/uri'; import { ClientSession } from 'sql/workbench/services/notebook/browser/models/clientSession'; -import { SessionManager } from 'sql/workbench/services/notebook/browser/sessionManager'; +import { SessionManager, EmptySession } from 'sql/workbench/services/notebook/browser/sessionManager'; import { NotebookManagerStub, ServerManagerStub } from 'sql/workbench/contrib/notebook/test/stubs'; import { isUndefinedOrNull } from 'vs/base/common/types'; @@ -36,7 +36,7 @@ suite('Client Session', function (): void { notebookManager: notebookManager, notebookUri: path, notificationService: notificationService.object, - kernelSpec: undefined + kernelSpec: { name: 'python', display_name: 'Python 3', language: 'python' } }); let serverlessNotebookManager = new NotebookManagerStub(); @@ -87,7 +87,12 @@ suite('Client Session', function (): void { serverManager.isStarted = true; resolve(); }); + let sessionMock = TypeMoq.Mock.ofType(EmptySession); + mockSessionManager.setup(s => s.ready).returns(() => Promise.resolve()); + mockSessionManager.setup(s => s.startNew(TypeMoq.It.isAny())).returns((options) => { + return Promise.resolve(sessionMock.object); + }); // When I call initialize await session.initialize(); @@ -128,50 +133,53 @@ suite('Client Session', function (): void { assert.equal(session.errorMessage, 'error'); }); - // test('Should start session automatically if kernel preference requests it', async function (): Promise { - // serverManager.isStarted = true; - // mockSessionManager.setup(s => s.ready).returns(() => Promise.resolve()); - // let sessionMock = TypeMoq.Mock.ofType(EmptySession); - // let startOptions: nb.ISessionOptions = undefined; - // mockSessionManager.setup(s => s.startNew(TypeMoq.It.isAny())).returns((options) => { - // startOptions = options; - // return Promise.resolve(sessionMock.object); - // }); + test('Should start session automatically if kernel preference requests it', async function (): Promise { + serverManager.isStarted = true; + mockSessionManager.setup(s => s.ready).returns(() => Promise.resolve()); + let sessionMock = TypeMoq.Mock.ofType(EmptySession); + let startOptions: nb.ISessionOptions = undefined; + mockSessionManager.setup(s => s.startNew(TypeMoq.It.isAny())).returns((options) => { + startOptions = options; + return Promise.resolve(sessionMock.object); + }); + await session.initialize(); - // // When I call initialize after defining kernel preferences - // session.kernelPreference = { - // shouldStart: true, - // name: 'python' - // }; - // await session.initialize(); + // Then + assert.equal(session.isReady, true, 'Session is not ready'); + assert.equal(session.isInErrorState, false, 'Session should not be in error state'); + assert.equal(startOptions.kernelName, 'python', 'Session not started with python by default'); + assert.equal(startOptions.path, path.fsPath, 'Session start path is incorrect'); + }); - // // Then - // should(session.isReady).be.true(); - // should(session.isInErrorState).be.false(); - // should(startOptions.kernelName).equal('python'); - // should(startOptions.path).equal(path.fsPath); - // }); + test('Should shutdown session even if no serverManager is set', async function (): Promise { + // Given a session against a remote server + let emptySession = new EmptySession({ + path: path.toString(), + kernelId: '1', + kernelName: 'python', + name: 'emptySession', + type: 'type' + }); - // test('Should shutdown session even if no serverManager is set', async function (): Promise { - // // Given a session against a remote server - // let expectedId = 'abc'; - // mockSessionManager.setup(s => s.isReady).returns(() => true); - // mockSessionManager.setup(s => s.shutdown(TypeMoq.It.isAny())).returns(() => Promise.resolve()); - // let sessionMock = TypeMoq.Mock.ofType(EmptySession); - // sessionMock.setup(s => s.id).returns(() => expectedId); - // mockSessionManager.setup(s => s.startNew(TypeMoq.It.isAny())).returns(() => Promise.resolve(sessionMock.object)); + mockSessionManager.setup(s => s.isReady).returns(() => true); + mockSessionManager.setup(s => s.shutdown(TypeMoq.It.isAny())).returns(() => Promise.resolve()); + mockSessionManager.setup(s => s.startNew(TypeMoq.It.isAny())).returns(() => Promise.resolve(emptySession)); + let newNotebookManager = notebookManager; + newNotebookManager.serverManager = undefined; - // remoteSession.kernelPreference = { - // shouldStart: true, - // name: 'python' - // }; - // await remoteSession.initialize(); + let remoteSession = new ClientSession({ + kernelSpec: { name: 'python', display_name: 'Python 3', language: 'python' }, + notebookManager: newNotebookManager, + notebookUri: path, + notificationService: notificationService.object + }); + await remoteSession.initialize(); - // // When I call shutdown - // await remoteSession.shutdown(); + // When I call shutdown + await remoteSession.shutdown(); - // // Then - // mockSessionManager.verify(s => s.shutdown(TypeMoq.It.isValue(expectedId)), TypeMoq.Times.once()); - // }); + // Then + mockSessionManager.verify(s => s.shutdown(TypeMoq.It.isAny()), TypeMoq.Times.once()); + }); }); diff --git a/src/sql/workbench/contrib/notebook/test/stubs.ts b/src/sql/workbench/contrib/notebook/test/stubs.ts index 6c0f7ed5d7..0cc0f3f7a5 100644 --- a/src/sql/workbench/contrib/notebook/test/stubs.ts +++ b/src/sql/workbench/contrib/notebook/test/stubs.ts @@ -5,7 +5,7 @@ import { nb, IConnectionProfile } from 'azdata'; import * as vsEvent from 'vs/base/common/event'; -import { INotebookModel, ICellModel, IClientSession, NotebookContentChange, IKernelPreference } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; +import { INotebookModel, ICellModel, IClientSession, NotebookContentChange } 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'; @@ -343,12 +343,6 @@ export class ClientSessionStub implements IClientSession { get kernelChangeCompleted(): Promise { throw new Error('Method not implemented.'); } - get kernelPreference(): IKernelPreference { - throw new Error('Method not implemented.'); - } - set kernelPreference(value: IKernelPreference) { - throw new Error('Method not implemented.'); - } get kernelDisplayName(): string { throw new Error('Method not implemented.'); } diff --git a/src/sql/workbench/services/notebook/browser/models/clientSession.ts b/src/sql/workbench/services/notebook/browser/models/clientSession.ts index 51bddf77af..47de63b190 100644 --- a/src/sql/workbench/services/notebook/browser/models/clientSession.ts +++ b/src/sql/workbench/services/notebook/browser/models/clientSession.ts @@ -11,7 +11,7 @@ import { Event, Emitter } from 'vs/base/common/event'; import { localize } from 'vs/nls'; import { getErrorMessage } from 'vs/base/common/errors'; -import { IClientSession, IKernelPreference, IClientSessionOptions } from 'sql/workbench/services/notebook/browser/models/modelInterfaces'; +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 { IConnectionProfile } from 'sql/platform/connection/common/interfaces'; @@ -35,7 +35,6 @@ export class ClientSession implements IClientSession { private _isReady: boolean; private _ready: Deferred; private _kernelChangeCompleted: Deferred; - private _kernelPreference: IKernelPreference; private _kernelDisplayName: string; private _errorMessage: string; private _cachedKernelSpec: nb.IKernelSpec; @@ -200,12 +199,6 @@ export class ClientSession implements IClientSession { public get kernelChangeCompleted(): Promise { return this._kernelChangeCompleted.promise; } - public get kernelPreference(): IKernelPreference { - return this._kernelPreference; - } - public set kernelPreference(value: IKernelPreference) { - this._kernelPreference = value; - } public get kernelDisplayName(): string { return this._kernelDisplayName; } diff --git a/src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts b/src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts index 2e2869f22b..0346e715a1 100644 --- a/src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts +++ b/src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts @@ -118,11 +118,6 @@ export interface IClientSession extends IDisposable { */ readonly kernelChangeCompleted: Promise; - /** - * The kernel preference. - */ - kernelPreference: IKernelPreference; - /** * The display name of the kernel. */ @@ -213,41 +208,6 @@ export interface IClientSession extends IDisposable { onKernelChanging(changeHandler: ((kernel: nb.IKernelChangedArgs) => Promise)): void; } -/** - * A kernel preference. - */ -export interface IKernelPreference { - /** - * The name of the kernel. - */ - readonly name?: string; - - /** - * The preferred kernel language. - */ - readonly language?: string; - - /** - * The id of an existing kernel. - */ - readonly id?: string; - - /** - * Whether to prefer starting a kernel. - */ - readonly shouldStart?: boolean; - - /** - * Whether a kernel can be started. - */ - readonly canStart?: boolean; - - /** - * Whether to auto-start the default kernel if no matching kernel is found. - */ - readonly autoStartDefault?: boolean; -} - export interface INotebookModel { /** * Cell List for this model