Notebooks: Only have One Connection in Attach To Dropdown (#8582)

* Have only one connection in attach to dropdown

* LGTM fixes

* Test fixes not unnecessarily changing context 2x

* PR Feedback

extensions/integration-tests/src/notebook.test.ts

@@ -9,8 +9,8 @@ import * as azdata from 'azdata';
 import * as vscode from 'vscode';
 import { isTestSetupCompleted } from './testContext';
 import { sqlNotebookContent, writeNotebookToFile, sqlKernelMetadata, getFileName, pySparkNotebookContent, pySparkKernelMetadata, pythonKernelMetadata, sqlNotebookMultipleCellsContent, notebookContentForCellLanguageTest, sqlKernelSpec, pythonKernelSpec, pySparkKernelSpec, CellTypes } from './notebook.util';
-import { getBdcServer, getConfigValue, EnvironmentVariable_PYTHON_PATH } from './testConfig';
-import { connectToServer, sleep } from './utils';
+import { getBdcServer, getConfigValue, EnvironmentVariable_PYTHON_PATH, TestServerProfile } from './testConfig';
+import { connectToServer, sleep, testServerProfileToIConnectionProfile } from './utils';
 import * as fs from 'fs';
 import { stressify } from 'adstest';
 import { isNullOrUndefined } from 'util';
@@ -392,15 +392,20 @@ class NotebookTester {
 	async openNotebook(content: azdata.nb.INotebookContents, kernelMetadata: any, testName: string, connectToDifferentServer?: boolean): Promise<azdata.nb.NotebookEditor> {
 		let notebookConfig = vscode.workspace.getConfiguration('notebook');
 		notebookConfig.update('pythonPath', getConfigValue(EnvironmentVariable_PYTHON_PATH), 1);
+		let server: TestServerProfile;
 		if (!connectToDifferentServer) {
-			let server = await getBdcServer();
+			server = await getBdcServer();
 			assert(server && server.serverName, 'No server could be found in openNotebook');
 			await connectToServer(server, 6000);
 		}
 		let notebookJson = Object.assign({}, content, { metadata: kernelMetadata });
 		let uri = writeNotebookToFile(notebookJson, testName);
 		console.log('Notebook uri ' + uri);
-		let notebook = await azdata.nb.showNotebookDocument(uri);
+		let nbShowOptions: azdata.nb.NotebookShowOptions;
+		if (server) {
+			nbShowOptions = { connectionProfile: testServerProfileToIConnectionProfile(server) };
+		}
+		let notebook = await azdata.nb.showNotebookDocument(uri, nbShowOptions);
 		return notebook;
 	}
 

extensions/integration-tests/src/utils.ts

@@ -271,3 +271,21 @@ export async function assertTableCreationResult(databaseName: string, schema: st
 		assert(result.columnInfo[0].columnName !== 'ErrorMessage', `Checking for table creation threw error ${result.rows[0][0].displayValue}`);
 	}
 }
+
+export function testServerProfileToIConnectionProfile(serverProfile: TestServerProfile): azdata.IConnectionProfile {
+	return {
+		serverName: serverProfile.serverName,
+		databaseName: serverProfile.database,
+		authenticationType: serverProfile.authenticationTypeName,
+		providerName: serverProfile.providerName,
+		connectionName: '',
+		userName: serverProfile.userName,
+		password: serverProfile.password,
+		savePassword: false,
+		groupFullName: undefined,
+		saveProfile: true,
+		id: undefined,
+		groupId: undefined,
+		options: {}
+	};
+}

src/sql/workbench/contrib/notebook/browser/cellViews/code.component.ts

@@ -38,6 +38,7 @@ import { UntitledTextEditorModel } from 'vs/workbench/common/editor/untitledText
 
 export const CODE_SELECTOR: string = 'code-component';
 const MARKDOWN_CLASS = 'markdown';
+const DEFAULT_OR_LOCAL_CONTEXT_ID = '-1';
 
 @Component({
 	selector: CODE_SELECTOR,
@@ -146,8 +147,8 @@ export class CodeComponent extends AngularDisposable implements OnInit, OnChange
 			let connectionService = this.connectionService;
 			if (!shouldConnect && connectionService && connectionService.isConnected(cellUri)) {
 				connectionService.disconnect(cellUri).catch(e => this.logService.error(e));
-			} else if (shouldConnect && this._model.activeConnection && this._model.activeConnection.id !== '-1') {
-				connectionService.connect(this._model.activeConnection, cellUri).catch(e => this.logService.error(e));
+			} else if (shouldConnect && this._model.context && this._model.context.id !== DEFAULT_OR_LOCAL_CONTEXT_ID) {
+				connectionService.connect(this._model.context, cellUri).catch(e => this.logService.error(e));
 			}
 		}
 	}

src/sql/workbench/contrib/notebook/browser/models/cell.ts

@@ -316,7 +316,7 @@ export class CellModel implements ICellModel {
 				this.sendNotification(notificationService, Severity.Info, localize('runCellCancelled', "Cell execution cancelled"));
 			} else {
 				// TODO update source based on editor component contents
-				if (kernel.requiresConnection && !this.notebookModel.activeConnection) {
+				if (kernel.requiresConnection && !this.notebookModel.context) {
 					let connected = await this.notebookModel.requestConnection();
 					if (!connected) {
 						return false;
@@ -509,11 +509,11 @@ export class CellModel implements ICellModel {
 				let result = output as nb.IDisplayResult;
 				if (result && result.data && result.data['text/html']) {
 					let model = this._options.notebook as NotebookModel;
-					if (model.activeConnection) {
-						let gatewayEndpointInfo = this.getGatewayEndpoint(model.activeConnection);
+					if (model.context) {
+						let gatewayEndpointInfo = this.getGatewayEndpoint(model.context);
 						if (gatewayEndpointInfo) {
 							let hostAndIp = notebookUtils.getHostAndPortFromEndpoint(gatewayEndpointInfo.endpoint);
-							let host = hostAndIp.host ? hostAndIp.host : model.activeConnection.serverName;
+							let host = hostAndIp.host ? hostAndIp.host : model.context.serverName;
 							let port = hostAndIp.port ? ':' + hostAndIp.port : defaultPort;
 							let html = result.data['text/html'];
 							// CTP 3.1 and earlier Spark link

src/sql/workbench/contrib/notebook/browser/models/modelInterfaces.ts

@@ -214,11 +214,6 @@ export interface IClientSession extends IDisposable {
 	onKernelChanging(changeHandler: ((kernel: nb.IKernelChangedArgs) => Promise<void>)): void;
 }
 
-export interface IDefaultConnection {
-	defaultConnection: ConnectionProfile;
-	otherConnections: ConnectionProfile[];
-}
-
 /**
  * A kernel preference.
  */
@@ -323,10 +318,10 @@ export interface INotebookModel {
 	readonly specs: nb.IAllKernels | undefined;
 
 	/**
-	 * The specs for available contexts, or undefined if these have
+	 * The specs for available context, or undefined if this has
 	 * not been loaded yet
 	 */
-	readonly contexts: IDefaultConnection | undefined;
+	readonly context: ConnectionProfile | undefined;
 
 	/**
 	 * Event fired on first initialization of the cells and

src/sql/workbench/contrib/notebook/browser/models/notebookContexts.ts

@@ -3,113 +3,44 @@
  *  Licensed under the Source EULA. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { nb } from 'azdata';
 
 import { localize } from 'vs/nls';
-import { IDefaultConnection } from 'sql/workbench/contrib/notebook/browser/models/modelInterfaces';
-import { IConnectionManagementService } from 'sql/platform/connection/common/connectionManagement';
 import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile';
-import { IConnectionProfile } from 'sql/platform/connection/common/interfaces';
 import { mssqlProviderName } from 'sql/platform/connection/common/constants';
-import { find } from 'vs/base/common/arrays';
+
 
 export class NotebookContexts {
 
-	public static get DefaultContext(): IDefaultConnection {
-		let defaultConnection: ConnectionProfile = <any>{
+	public static get DefaultContext(): ConnectionProfile {
+		return <any>{
 			providerName: mssqlProviderName,
 			id: '-1',
 			serverName: localize('selectConnection', "Select Connection")
 		};
-
-		return {
-			// default context if no other contexts are applicable
-			defaultConnection: defaultConnection,
-			otherConnections: [defaultConnection]
-		};
 	}
 
-	public static get LocalContext(): IDefaultConnection {
-		let localConnection: ConnectionProfile = <any>{
+	public static get LocalContext(): ConnectionProfile {
+		return <any>{
 			providerName: mssqlProviderName,
 			id: '-1',
 			serverName: localize('localhost', "localhost")
 		};
-
-		return {
-			// default context if no other contexts are applicable
-			defaultConnection: localConnection,
-			otherConnections: [localConnection]
-		};
 	}
 
 	/**
-	 * Get all of the applicable contexts for a given kernel
-	 * @param connectionService connection management service
+	 * Get the applicable context for a given kernel
+	 * @param context current connection profile
 	 * @param connProviderIds array of connection provider ids applicable for a kernel
-	 * @param kernelChangedArgs kernel changed args (both old and new kernel info)
-	 * @param profile current connection profile
-	 */
-	public static getContextsForKernel(connectionService: IConnectionManagementService, connProviderIds: string[], kernelChangedArgs?: nb.IKernelChangedArgs, profile?: IConnectionProfile): IDefaultConnection {
-		let connections: IDefaultConnection = this.DefaultContext;
-		if (!profile) {
-			if (!kernelChangedArgs || !kernelChangedArgs.newValue ||
-				(kernelChangedArgs.oldValue && kernelChangedArgs.newValue.id === kernelChangedArgs.oldValue.id)) {
-				// nothing to do, kernels are the same or new kernel is undefined
-				return connections;
-			}
-		}
-		if (kernelChangedArgs && kernelChangedArgs.newValue && kernelChangedArgs.newValue.name && connProviderIds.length < 1) {
-			return connections;
-		} else {
-			connections = this.getActiveContexts(connectionService, connProviderIds, profile);
-		}
-		return connections;
-	}
-
-	/**
-	 * Get all active contexts and sort them
-	 * @param connectionService connection service
-	 * @param connProviderIds array of applicable connection providers to filter connections
-	 * @param profile connection profile passed when launching notebook
 	 */
-	public static getActiveContexts(connectionService: IConnectionManagementService, connProviderIds: string[], profile?: IConnectionProfile): IDefaultConnection {
-		let defaultConnection: ConnectionProfile = NotebookContexts.DefaultContext.defaultConnection;
-		let activeConnections: ConnectionProfile[] = connectionService.getActiveConnections();
-		if (activeConnections && activeConnections.length > 0) {
-			activeConnections = activeConnections.filter(conn => conn.id !== '-1');
-		}
+	public static getContextForKernel(context: ConnectionProfile, connProviderIds: string[]): ConnectionProfile {
 		// If no connection provider ids exist for a given kernel, the attach to should show localhost
 		if (connProviderIds.length === 0) {
 			return NotebookContexts.LocalContext;
 		}
-		// If no active connections exist, show "Select connection" as the default value
-		if (activeConnections.length === 0) {
-			return NotebookContexts.DefaultContext;
+		if (context && context.providerName && connProviderIds.filter(p => p === context.providerName).length > 0) {
+			return context;
 		}
-		// Filter active connections by their provider ids to match kernel's supported connection providers
-		else if (activeConnections.length > 0) {
-			let connections = activeConnections.filter(connection => {
-				return connProviderIds.some(x => x === connection.providerName);
-			});
-			if (connections && connections.length > 0) {
-				defaultConnection = connections[0];
-				if (profile && profile.options) {
-					let matchingConn = find(connections, connection => connection.serverName === profile.serverName);
-					if (matchingConn) {
-						defaultConnection = matchingConn;
-					}
-				}
-			} else if (connections.length === 0) {
-				return NotebookContexts.DefaultContext;
-			}
-			activeConnections = [];
-			connections.forEach(connection => activeConnections.push(connection));
-		}
-
-		return {
-			otherConnections: activeConnections,
-			defaultConnection: defaultConnection
-		};
+		return NotebookContexts.DefaultContext;
 	}
+
 }