From 26b0df15862c7e1afe01912fe19dae229a042c6e Mon Sep 17 00:00:00 2001 From: "Benjin Dubishar (from Dev Box)" Date: Tue, 7 Jan 2025 13:43:46 -0800 Subject: [PATCH 01/17] populating missing IDs for connections and groups --- package.json | 37 ++++++ package.nls.json | 1 + src/connectionconfig/connectionconfig.ts | 149 +++++++++++++++++------ src/constants/constants.ts | 1 + src/models/connectionProfile.ts | 2 + src/models/interfaces.ts | 10 ++ 6 files changed, 163 insertions(+), 37 deletions(-) diff --git a/package.json b/package.json index ca608d47fc..4ab9e72f92 100644 --- a/package.json +++ b/package.json @@ -1005,12 +1005,49 @@ "description": "%mssql.maxRecentConnections%", "scope": "window" }, + "mssql.connectionGroups": { + "type": "array", + "description": "%mssql.connectionGroups%", + "items": { + "type": "object", + "properties": { + "name": { + "type": "string", + "description": "%mssql.connectionGroup.name%" + }, + "id": { + "type": "string", + "description": "%mssql.connectionGroup.id%" + }, + "color": { + "type": ["string", null], + "description": "%mssql.connectionGroup.color%" + }, + "description": { + "type": ["string", null], + "description": "%mssql.connectionGroup.description%" + }, + "parentId": { + "type": ["string", null], + "description": "%mssql.connectionGroup.parentId%" + } + } + } + }, "mssql.connections": { "type": "array", "description": "%mssql.connections%", "items": { "type": "object", "properties": { + "id": { + "type": "string", + "description": "%mssql.connection.id%" + }, + "groupId": { + "type": "string", + "description": "%mssql.connection.groupId%" + }, "server": { "type": "string", "default": "{{put-server-name-here}}", diff --git a/package.nls.json b/package.nls.json index e20a47735b..5008d37758 100644 --- a/package.nls.json +++ b/package.nls.json @@ -46,6 +46,7 @@ "mssql.logDebugInfo":"[Optional] Log debug output to the VS Code console (Help -> Toggle Developer Tools)", "mssql.maxRecentConnections":"The maximum number of recently used connections to store in the connection list.", "mssql.connections":"Connection profiles defined in 'User Settings' are shown under 'MS SQL: Connect' command in the command palette.", +"mssql.connectionGroups":"Connection groups", "mssql.connection.server":"[Required] Specify the server name to connect to. Use 'hostname instance' or '.database.windows.net' for Azure SQL Database.", "mssql.connection.database":"[Optional] Specify the database name to connect to. If database is not specified, the default user database setting is used, typically 'master'.", "mssql.connection.user":"[Optional] Specify the user name for SQL Server authentication. If user name is not specified, when you connect, you will be asked again.", diff --git a/src/connectionconfig/connectionconfig.ts b/src/connectionconfig/connectionconfig.ts index 980fcb0db4..309fcc3793 100644 --- a/src/connectionconfig/connectionconfig.ts +++ b/src/connectionconfig/connectionconfig.ts @@ -6,14 +6,19 @@ import * as Constants from "../constants/constants"; import * as LocalizedConstants from "../constants/locConstants"; import * as Utils from "../models/utils"; -import { IConnectionProfile } from "../models/interfaces"; +import { IConnectionGroup, IConnectionProfile } from "../models/interfaces"; import { IConnectionConfig } from "./iconnectionconfig"; import VscodeWrapper from "../controllers/vscodeWrapper"; +import { Deferred } from "../protocol"; /** * Implements connection profile file storage. */ export class ConnectionConfig implements IConnectionConfig { + initialized: Deferred = new Deferred(); + + RootGroupName: string = "ROOT"; + /** * Constructor. */ @@ -21,6 +26,7 @@ export class ConnectionConfig implements IConnectionConfig { if (!this.vscodeWrapper) { this.vscodeWrapper = new VscodeWrapper(); } + void this.assignMissingIds(); } private get vscodeWrapper(): VscodeWrapper { @@ -31,6 +37,53 @@ export class ConnectionConfig implements IConnectionConfig { this._vscodeWrapper = value; } + private async assignMissingIds(): Promise { + // Connection groups + const groups: IConnectionGroup[] = this.getGroupsFromSettings(); + + // ensure each group has an id + groups.forEach((group) => { + if (!group.id) { + group.id = Utils.generateGuid(); + } + }); + + // ensure ROOT group exists + let rootGroup = groups.find( + (group) => group.name === this.RootGroupName, + ); + if (!rootGroup) { + rootGroup = { + name: this.RootGroupName, + id: Utils.generateGuid(), + }; + groups.push(rootGroup); + } + + // Connection profiles + const profiles: IConnectionProfile[] = this.getProfilesFromSettings(); + + // ensure each profile has an id + profiles.forEach((profile) => { + if (!profile.id) { + profile.id = Utils.generateGuid(); + } + }); + + // ensure each profile is in a group + profiles.forEach((profile) => { + if (!profile.groupId) { + profile.groupId = rootGroup.id; + } + }); + + // Save the changes to settings + await this.writeConnectionGroupsToSettings(groups); + await this.writeProfilesToSettings(profiles); + + this.initialized.resolve(); + } + /** * Add a new connection to the connection config. */ @@ -55,31 +108,17 @@ export class ConnectionConfig implements IConnectionConfig { getWorkspaceConnections: boolean, ): IConnectionProfile[] { let profiles: IConnectionProfile[] = []; - let compareProfileFunc = (a, b) => { - // Sort by profile name if available, otherwise fall back to server name or connection string - let nameA = a.profileName - ? a.profileName - : a.server - ? a.server - : a.connectionString; - let nameB = b.profileName - ? b.profileName - : b.server - ? b.server - : b.connectionString; - return nameA.localeCompare(nameB); - }; // Read from user settings let userProfiles = this.getProfilesFromSettings(); - userProfiles.sort(compareProfileFunc); + userProfiles.sort(this.compareConnectionProfile); profiles = profiles.concat(userProfiles); if (getWorkspaceConnections) { // Read from workspace settings let workspaceProfiles = this.getProfilesFromSettings(false); - workspaceProfiles.sort(compareProfileFunc); + workspaceProfiles.sort(this.compareConnectionProfile); profiles = profiles.concat(workspaceProfiles); } @@ -124,39 +163,44 @@ export class ConnectionConfig implements IConnectionConfig { /** * Get all profiles from the settings. * This is public for testing only. - * @param global When `true` profiles come from user settings, otherwise from workspace settings + * @param global When `true` profiles come from user settings, otherwise from workspace settings. Default is `true`. * @returns the set of connection profiles found in the settings. */ public getProfilesFromSettings( global: boolean = true, ): IConnectionProfile[] { + return this.getArrayFromSettings( + Constants.connectionsArrayName, + global, + ); + } + + public getGroupsFromSettings(global: boolean = true): IConnectionGroup[] { + return this.getArrayFromSettings( + Constants.connectionGroupsArrayName, + global, + ); + } + + private getArrayFromSettings( + configSection: string, + global: boolean = true, + ): T[] { let configuration = this._vscodeWrapper.getConfiguration( Constants.extensionName, this._vscodeWrapper.activeTextEditorUri, ); - let profiles: IConnectionProfile[] = []; - let configValue = configuration.inspect( - Constants.connectionsArrayName, - ); + let configValue = configuration.inspect(configSection); if (global) { - profiles = configValue.globalValue; + // only return the global values if that's what's requested + return configValue.globalValue || []; } else { - profiles = configValue.workspaceValue; - if (profiles !== undefined) { - profiles = profiles.concat( - configValue.workspaceFolderValue || [], - ); - } else { - profiles = configValue.workspaceFolderValue; - } + // otherwise, return the combination of the workspace and workspace folder values + return (configValue.workspaceValue || []).concat( + configValue.workspaceFolderValue || [], + ); } - - if (profiles === undefined) { - profiles = []; - } - - return profiles; } /** @@ -173,4 +217,35 @@ export class ConnectionConfig implements IConnectionConfig { profiles, ); } + + /** + * Replace existing connection groups in the user settings with a new set of connection groups. + * @param connGroups the set of connection groups to insert into the settings file. + */ + private async writeConnectionGroupsToSettings( + connGroups: IConnectionGroup[], + ): Promise { + // Save the file + await this._vscodeWrapper.setConfiguration( + Constants.extensionName, + Constants.connectionGroupsArrayName, + connGroups, + ); + } + + /** Sort by profile name if available, otherwise fall back to server name or connection string */ + private compareConnectionProfile(connA, connB) { + let nameA = connA.profileName + ? connA.profileName + : connA.server + ? connA.server + : connA.connectionString; + let nameB = connB.profileName + ? connB.profileName + : connB.server + ? connB.server + : connB.connectionString; + + return nameA.localeCompare(nameB); + } } diff --git a/src/constants/constants.ts b/src/constants/constants.ts index 5c33f56d65..411005af07 100644 --- a/src/constants/constants.ts +++ b/src/constants/constants.ts @@ -17,6 +17,7 @@ export const connectionApplicationName = "vscode-mssql"; export const outputChannelName = "MSSQL"; export const connectionConfigFilename = "settings.json"; export const connectionsArrayName = "connections"; +export const connectionGroupsArrayName = "connectionGroups"; export const disconnectedServerLabel = "disconnectedServer"; export const serverLabel = "Server"; export const folderLabel = "Folder"; diff --git a/src/models/connectionProfile.ts b/src/models/connectionProfile.ts index e527a1cda7..407c050459 100644 --- a/src/models/connectionProfile.ts +++ b/src/models/connectionProfile.ts @@ -36,6 +36,8 @@ export class ConnectionProfile implements IConnectionProfile { public profileName: string; + public id: string; + public groupId: string; public savePassword: boolean; public emptyPasswordInput: boolean; public azureAuthType: AzureAuthType; diff --git a/src/models/interfaces.ts b/src/models/interfaces.ts index 6b5c7d3daf..8ce8594838 100644 --- a/src/models/interfaces.ts +++ b/src/models/interfaces.ts @@ -58,6 +58,8 @@ export const contentTypes = [ // optional name and details on whether password should be saved export interface IConnectionProfile extends vscodeMssql.IConnectionInfo { profileName: string; + id: string; + groupId: string; savePassword: boolean; emptyPasswordInput: boolean; azureAuthType: AzureAuthType; @@ -66,6 +68,14 @@ export interface IConnectionProfile extends vscodeMssql.IConnectionInfo { isAzureActiveDirectory(): boolean; } +export interface IConnectionGroup { + id: string; + name: string; + parentId?: string; + color?: string; + description?: string; +} + export enum CredentialsQuickPickItemType { Profile, Mru, From f6e5c1b4b804ed5ceaaa8a353a681d5771b1cefd Mon Sep 17 00:00:00 2001 From: "Benjin Dubishar (from Dev Box)" Date: Tue, 7 Jan 2025 13:44:57 -0800 Subject: [PATCH 02/17] loc --- localization/xliff/vscode-mssql.xlf | 3 +++ 1 file changed, 3 insertions(+) diff --git a/localization/xliff/vscode-mssql.xlf b/localization/xliff/vscode-mssql.xlf index 3687e4c005..9544d61ac5 100644 --- a/localization/xliff/vscode-mssql.xlf +++ b/localization/xliff/vscode-mssql.xlf @@ -1569,6 +1569,9 @@ Connect + + Connection groups + Connection profiles defined in 'User Settings' are shown under 'MS SQL: Connect' command in the command palette. From 794893a9b4f71a9d53b8c4d48c44d707185e3bf7 Mon Sep 17 00:00:00 2001 From: "Benjin Dubishar (from Dev Box)" Date: Tue, 7 Jan 2025 16:15:15 -0800 Subject: [PATCH 03/17] adding description text for new config props --- package.nls.json | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/package.nls.json b/package.nls.json index 5008d37758..fa6ad25559 100644 --- a/package.nls.json +++ b/package.nls.json @@ -80,6 +80,13 @@ "mssql.connection.profileName":"[Optional] Specify a custom name for this connection profile to easily browse and search in the command palette of Visual Studio Code.", "mssql.connection.savePassword":"[Optional] When set to 'true', the password for SQL Server authentication is saved in the secure store of your operating system such as KeyChain in MacOS or Secure Store in Windows.", "mssql.connection.emptyPasswordInput":"[Optional] Indicates whether this profile has an empty password explicitly set", +"mssql.connection.id":"The unique identifier for this connection profile.", +"mssql.connection.groupId":"The unique identifier for the connection group this connection profile belongs to.", +"mssql.connectionGroup.name":"The name of the connection group.", +"mssql.connectionGroup.id":"The unique identifier for the connection group.", +"mssql.connectionGroup.color":"The color of the connection group.", +"mssql.connectionGroup.description":"The description of the connection group.", +"mssql.connectionGroup.parentId":"The unique identifier for the parent connection group.", "mssql.enableSqlAuthenticationProvider":"Enables use of the Sql Authentication Provider for 'Microsoft Entra Id Interactive' authentication mode when user selects 'AzureMFA' authentication. This enables Server-side resource endpoint integration when fetching access tokens. This option is only supported for 'MSAL' Authentication Library. Please restart Visual Studio Code after changing this option.", "mssql.enableConnectionPooling":"Enables connection pooling to improve overall connectivity performance. This setting is enabled by default. Visual Studio Code is required to be relaunched when the value is changed. To clear pooled connections, run the command: 'MS SQL: Clear Pooled Connections'", "mssql.shortcuts":"Shortcuts related to the results window", From a33d03bfda364887d009bd698af8ba316d7e77dd Mon Sep 17 00:00:00 2001 From: "Benjin Dubishar (from Dev Box)" Date: Wed, 8 Jan 2025 09:47:17 -0800 Subject: [PATCH 04/17] more loc --- localization/xliff/vscode-mssql.xlf | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/localization/xliff/vscode-mssql.xlf b/localization/xliff/vscode-mssql.xlf index 9544d61ac5..8207546380 100644 --- a/localization/xliff/vscode-mssql.xlf +++ b/localization/xliff/vscode-mssql.xlf @@ -1857,12 +1857,33 @@ Start Query History Capture + + The color of the connection group. + + + The description of the connection group. + The maximum number of recently used connections to store in the connection list. + + The name of the connection group. + The timeout in seconds for expanding a node in Object Explorer. The default value is 45 seconds. + + The unique identifier for the connection group this connection profile belongs to. + + + The unique identifier for the connection group. + + + The unique identifier for the parent connection group. + + + The unique identifier for this connection profile. + Toggle SQLCMD Mode From ff134d41b27ff51998adb8cbe67b74fcf00d542d Mon Sep 17 00:00:00 2001 From: "Benjin Dubishar (from Dev Box)" Date: Wed, 8 Jan 2025 09:51:31 -0800 Subject: [PATCH 05/17] removing quickpick connection typing --- .../connectionDialogWebviewController.ts | 28 +- src/controllers/connectionManager.ts | 4 +- src/controllers/mainController.ts | 4 +- src/models/connectionInfo.ts | 16 +- src/models/connectionStore.ts | 241 +++++++++++------- src/models/interfaces.ts | 11 +- src/models/utils.ts | 2 +- src/objectExplorer/objectExplorerService.ts | 119 +++++---- test/unit/utils.test.ts | 14 +- typings/vscode-mssql.d.ts | 2 +- 10 files changed, 244 insertions(+), 197 deletions(-) diff --git a/src/connectionconfig/connectionDialogWebviewController.ts b/src/connectionconfig/connectionDialogWebviewController.ts index 88742d3873..7af7aca39f 100644 --- a/src/connectionconfig/connectionDialogWebviewController.ts +++ b/src/connectionconfig/connectionDialogWebviewController.ts @@ -64,8 +64,8 @@ import { getErrorMessage } from "../utils/utils"; import { l10n } from "vscode"; import { CredentialsQuickPickItemType, - IConnectionCredentialsQuickPickItem, IConnectionProfile, + IConnectionProfileWithSource, } from "../models/interfaces"; import { IAccount } from "../models/contracts/azure"; import { @@ -564,24 +564,18 @@ export class ConnectionDialogWebviewController extends ReactWebviewPanelControll savedConnections: IConnectionDialogProfile[]; recentConnections: IConnectionDialogProfile[]; }> { - const unsortedConnections: IConnectionCredentialsQuickPickItem[] = - this._mainController.connectionManager.connectionStore.loadAllConnections( - true /* addRecentConnections */, + const unsortedConnections: IConnectionProfileWithSource[] = + this._mainController.connectionManager.connectionStore.readAllConnections( + true /* includeRecentConnections */, ); - const savedConnections = unsortedConnections - .filter( - (c) => - c.quickPickItemType === - CredentialsQuickPickItemType.Profile, - ) - .map((c) => c.connectionCreds); - - const recentConnections = unsortedConnections - .filter( - (c) => c.quickPickItemType === CredentialsQuickPickItemType.Mru, - ) - .map((c) => c.connectionCreds); + const savedConnections = unsortedConnections.filter( + (c) => c.profileSource === CredentialsQuickPickItemType.Profile, + ); + + const recentConnections = unsortedConnections.filter( + (c) => c.profileSource === CredentialsQuickPickItemType.Mru, + ); sendActionEvent( TelemetryViews.ConnectionDialog, diff --git a/src/controllers/connectionManager.ts b/src/controllers/connectionManager.ts index 82fe0198fb..94adcb52ad 100644 --- a/src/controllers/connectionManager.ts +++ b/src/controllers/connectionManager.ts @@ -312,7 +312,7 @@ export default class ConnectionManager { (uri) => this._connections[uri].credentials, ); for (let connectedCredential of connectedCredentials) { - if (Utils.isSameConnection(credential, connectedCredential)) { + if (Utils.isSameConnectionInfo(credential, connectedCredential)) { return true; } } @@ -322,7 +322,7 @@ export default class ConnectionManager { public getUriForConnection(connection: IConnectionInfo): string { for (let uri of Object.keys(this._connections)) { if ( - Utils.isSameConnection( + Utils.isSameConnectionInfo( this._connections[uri].credentials, connection, ) diff --git a/src/controllers/mainController.ts b/src/controllers/mainController.ts index b23c65875d..b9c5c97fde 100644 --- a/src/controllers/mainController.ts +++ b/src/controllers/mainController.ts @@ -2136,7 +2136,7 @@ export default class MainController implements vscode.Disposable { let staleConnections = objectExplorerConnections.filter( (oeConn) => { return !userConnections.some((userConn) => - Utils.isSameConnection(oeConn, userConn), + Utils.isSameConnectionInfo(oeConn, userConn), ); }, ); @@ -2170,7 +2170,7 @@ export default class MainController implements vscode.Disposable { // if a connection(s) was/were manually added let newConnections = userConnections.filter((userConn) => { return !objectExplorerConnections.some((oeConn) => - Utils.isSameConnection(userConn, oeConn), + Utils.isSameConnectionInfo(userConn, oeConn), ); }); for (let conn of newConnections) { diff --git a/src/models/connectionInfo.ts b/src/models/connectionInfo.ts index bf626b541d..b7f558184d 100644 --- a/src/models/connectionInfo.ts +++ b/src/models/connectionInfo.ts @@ -94,22 +94,22 @@ function isAzureDatabase(server: string): boolean { * Gets a label describing a connection in the picklist UI * * @export connectionInfo/getPicklistLabel - * @param connCreds connection to create a label for + * @param connection connection to create a label for * @param itemType type of quickpick item to display - this influences the icon shown to the user * @returns user readable label */ -export function getPicklistLabel( - connCreds: IConnectionInfo, - itemType: Interfaces.CredentialsQuickPickItemType, +export function getSimpleConnectionDisplayName( + connection: IConnectionInfo, ): string { - let profile: Interfaces.IConnectionProfile = < - Interfaces.IConnectionProfile - >connCreds; + let profile: Interfaces.IConnectionProfile = + connection as Interfaces.IConnectionProfile; if (profile.profileName) { return profile.profileName; } else { - return connCreds.server ? connCreds.server : connCreds.connectionString; + return connection.server + ? connection.server + : connection.connectionString; } } diff --git a/src/models/connectionStore.ts b/src/models/connectionStore.ts index 869ece4d3d..7730a0b1b5 100644 --- a/src/models/connectionStore.ts +++ b/src/models/connectionStore.ts @@ -15,6 +15,7 @@ import { IConnectionCredentialsQuickPickItem, CredentialsQuickPickItemType, AuthenticationTypes, + IConnectionProfileWithSource, } from "../models/interfaces"; import { ICredentialStore } from "../credentialstore/icredentialstore"; import { IConnectionConfig } from "../connectionconfig/iconnectionconfig"; @@ -184,7 +185,7 @@ export class ConnectionStore { */ public getPickListItems(): IConnectionCredentialsQuickPickItem[] { let pickListItems: IConnectionCredentialsQuickPickItem[] = - this.loadAllConnections(false); + this.getConnectionQuickpickItems(false); pickListItems.push({ label: `$(add) ${LocalizedConstants.CreateProfileFromConnectionsListLabel}`, connectionCreds: undefined, @@ -577,7 +578,7 @@ export class ConnectionStore { itemType: CredentialsQuickPickItemType, ): IConnectionCredentialsQuickPickItem { return { - label: ConnInfo.getPicklistLabel(item, itemType), + label: ConnInfo.getSimpleConnectionDisplayName(item), description: ConnInfo.getPicklistDescription(item), detail: ConnInfo.getPicklistDetails(item), connectionCreds: item, @@ -613,91 +614,133 @@ export class ConnectionStore { await this.saveProfile(profile); } - // Load connections from user preferences - public loadAllConnections( - addRecentConnections: boolean = false, - ): IConnectionCredentialsQuickPickItem[] { - let quickPickItems: IConnectionCredentialsQuickPickItem[] = []; + public readAllConnections( + includeRecentConnections: boolean = false, + ): IConnectionProfileWithSource[] { + let connResults: IConnectionProfileWithSource[] = []; - // Read recently used items from a memento - let recentConnections = []; - if (addRecentConnections) { - recentConnections = this.getConnectionsFromGlobalState( - Constants.configRecentConnections, - ); - } + const configConnections = this._connectionConfig + .getConnections(true) + .map((c) => { + const conn = c as IConnectionProfileWithSource; + conn.profileSource = CredentialsQuickPickItemType.Profile; + return conn; + }); - // Load connections from user preferences - // Per this https://code.visualstudio.com/Docs/customization/userandworkspace - // Connections defined in workspace scope are unioned with the Connections defined in user scope - let profilesInConfiguration = - this._connectionConfig.getConnections(true); - - // Remove any duplicates that are in both recent connections and the user settings - let profilesInRecentConnectionsList: number[] = []; - profilesInConfiguration = profilesInConfiguration.filter((profile) => { - for (let index = 0; index < recentConnections.length; index++) { - if ( - Utils.isSameProfile( - profile, - recentConnections[index], - ) - ) { - if ( - Utils.isSameConnection( - profile, - recentConnections[index], - ) - ) { - // The MRU item should reflect the current profile's settings from user preferences if it is still the same database - ConnInfo.fixupConnectionCredentials(profile); - recentConnections[index] = Object.assign({}, profile); - profilesInRecentConnectionsList.push(index); - } - return false; - } - } - return true; - }); + connResults = connResults.concat(configConnections); - // Ensure that MRU items which are actually profiles are labeled as such - let recentConnectionsItems = this.mapToQuickPickItems( - recentConnections, - CredentialsQuickPickItemType.Mru, - ); - for (let index of profilesInRecentConnectionsList) { - recentConnectionsItems[index].quickPickItemType = - CredentialsQuickPickItemType.Profile; + if (includeRecentConnections) { + const recentConnections = this.getRecentlyUsedConnections().map( + (c) => { + const conn = c as IConnectionProfileWithSource; + conn.profileSource = CredentialsQuickPickItemType.Mru; + return conn; + }, + ); + + connResults = connResults.concat(recentConnections); } - quickPickItems = quickPickItems.concat(recentConnectionsItems); - quickPickItems = quickPickItems.concat( - this.mapToQuickPickItems( - profilesInConfiguration, - CredentialsQuickPickItemType.Profile, - ), - ); + // TODO re-add deduplication logic from old method - // Return all connections - return quickPickItems; + return connResults; } - private getConnectionsFromGlobalState( - configName: string, - ): T[] { - let connections: T[] = []; - // read from the global state - let configValues = this._context.globalState.get(configName); - this.addConnections(connections, configValues); - return connections; + public getConnectionQuickpickItems( + includeRecentConnections: boolean = false, + ): IConnectionCredentialsQuickPickItem[] { + let output: IConnectionCredentialsQuickPickItem[] = []; + const connections = this.readAllConnections(includeRecentConnections); + + output = connections.map((c) => { + return this.createQuickPickItem(c, c.profileSource); + }); + + return output; } - private mapToQuickPickItems( - connections: IConnectionInfo[], - itemType: CredentialsQuickPickItemType, + // Load connections from user preferences + public loadAllConnections( + addRecentConnections: boolean = false, ): IConnectionCredentialsQuickPickItem[] { - return connections.map((c) => this.createQuickPickItem(c, itemType)); - } + throw new Error("Method not implemented."); + // let connResults: IConnectionCredentialsQuickPickItem[] = []; + + // // Read recently used items from a memento + // let recentConnections: IConnectionProfile[] = []; + // if (addRecentConnections) { + // recentConnections = + // this.getConnectionsFromGlobalState( + // Constants.configRecentConnections, + // ); + // } + + // // Load connections from user preferences + // // Per this https://code.visualstudio.com/Docs/customization/userandworkspace + // // Connections defined in workspace scope are unioned with the Connections defined in user scope + // let profilesInConfiguration = + // this._connectionConfig.getConnections(true); + + // // Remove any duplicates that are in both recent connections and the user settings + // let profilesInRecentConnectionsList: number[] = []; + // profilesInConfiguration = profilesInConfiguration.filter((profile) => { + // for (let index = 0; index < recentConnections.length; index++) { + // if (Utils.isSameProfile(profile, recentConnections[index])) { + // if ( + // Utils.isSameConnection( + // profile, + // recentConnections[index], + // ) + // ) { + // // The MRU item should reflect the current profile's settings from user preferences if it is still the same database + // ConnInfo.fixupConnectionCredentials(profile); + // recentConnections[index] = Object.assign({}, profile); + // profilesInRecentConnectionsList.push(index); + // } + // return false; // config profile is also in recent connections + // } + // } + // return true; // config profile is not in recent connections + // }); + + // // Ensure that MRU items which are actually profiles are labeled as such + // let recentConnectionsItems = this.mapToQuickPickItems( + // recentConnections, + // CredentialsQuickPickItemType.Mru, + // ); + // for (let index of profilesInRecentConnectionsList) { + // recentConnectionsItems[index].quickPickItemType = + // CredentialsQuickPickItemType.Profile; + // } + + // connResults = connResults.concat(recentConnectionsItems); + // connResults = connResults.concat( + // this.mapToQuickPickItems( + // profilesInConfiguration, + // CredentialsQuickPickItemType.Profile, + // ), + // ); + + // // Return all connections + // return connResults; + } + + // private getConnectionsFromGlobalState( + // configName: string, + // ): T[] { + // let connections: T[] = []; + // // read from the global state + // let configValues = this._context.globalState.get(configName); + // this.addConnections(connections, configValues); + // return connections; + // } + + // private mapToQuickPickItems( + // connections: IConnectionInfo[], + // itemType: CredentialsQuickPickItemType, + // ): IConnectionCredentialsQuickPickItem[] { + // return connections.map((c) => this.createQuickPickItem(c, itemType)); + // } private loadProfiles( loadWorkspaceProfiles: boolean, @@ -710,29 +753,29 @@ export class ConnectionStore { return quickPickItems; } - private addConnections( - connections: IConnectionInfo[], - configValues: IConnectionInfo[], - ): void { - if (configValues) { - for (let index = 0; index < configValues.length; index++) { - let element = configValues[index]; - if ( - element.server && - element.server.trim() && - !element.server.trim().startsWith("{{") - ) { - let connection = - ConnInfo.fixupConnectionCredentials(element); - connections.push(connection); - } else { - Utils.logDebug( - `Missing server name in user preferences connection: index ( ${index} ): ${element.toString()}`, - ); - } - } - } - } + // private addConnections( + // connections: IConnectionInfo[], + // configValues: IConnectionInfo[], + // ): void { + // if (configValues) { + // for (let index = 0; index < configValues.length; index++) { + // let element = configValues[index]; + // if ( + // element.server && + // element.server.trim() && + // !element.server.trim().startsWith("{{") + // ) { + // let connection = + // ConnInfo.fixupConnectionCredentials(element); + // connections.push(connection); + // } else { + // Utils.logDebug( + // `Missing server name in user preferences connection: index ( ${index} ): ${element.toString()}`, + // ); + // } + // } + // } + // } private getMaxRecentConnectionsCount(): number { let config = this._vscodeWrapper.getConfiguration( diff --git a/src/models/interfaces.ts b/src/models/interfaces.ts index 8ce8594838..97d75142e4 100644 --- a/src/models/interfaces.ts +++ b/src/models/interfaces.ts @@ -54,8 +54,10 @@ export const contentTypes = [ Constants.localizedTexts, ]; -// A Connection Profile contains all the properties of connection credentials, with additional -// optional name and details on whether password should be saved +/** + * Extension of IConnectionInfo that adds metadata relevant to the MSSQL extension's handling of connection profiles, + * such as the profile name, connection ID, connection group ID, and whether the password should be saved. + */ export interface IConnectionProfile extends vscodeMssql.IConnectionInfo { profileName: string; id: string; @@ -81,6 +83,11 @@ export enum CredentialsQuickPickItemType { Mru, NewConnection, } + +export interface IConnectionProfileWithSource extends IConnectionProfile { + profileSource: CredentialsQuickPickItemType; +} + export interface IConnectionCredentialsQuickPickItem extends vscode.QuickPickItem { connectionCreds: vscodeMssql.IConnectionInfo; diff --git a/src/models/utils.ts b/src/models/utils.ts index f42db0b1e5..59c5902101 100644 --- a/src/models/utils.ts +++ b/src/models/utils.ts @@ -324,7 +324,7 @@ export function isSameProfile( * @param expectedConn the connection to try to match * @returns boolean that is true if the connections match */ -export function isSameConnection( +export function isSameConnectionInfo( conn: IConnectionInfo, expectedConn: IConnectionInfo, ): boolean { diff --git a/src/objectExplorer/objectExplorerService.ts b/src/objectExplorer/objectExplorerService.ts index 8c8bee6b3c..172133ddf4 100644 --- a/src/objectExplorer/objectExplorerService.ts +++ b/src/objectExplorer/objectExplorerService.ts @@ -47,7 +47,7 @@ import { IConnectionInfo } from "vscode-mssql"; import { sendActionEvent } from "../telemetry/telemetry"; import { IAccount } from "../models/contracts/azure"; import * as AzureConstants from "../azure/constants"; -import { getConnectionDisplayName } from "../models/connectionInfo"; +import * as ConnInfo from "../models/connectionInfo"; import { TelemetryActions, TelemetryViews, @@ -129,24 +129,25 @@ export class ObjectExplorerService { // if no node label, check if it has a name in saved profiles // in case this call came from new query let savedConnections = - this._connectionManager.connectionStore.loadAllConnections(); + this._connectionManager.connectionStore.readAllConnections(); let nodeConnection = this._sessionIdToConnectionCredentialsMap.get( result.sessionId, ); for (let connection of savedConnections) { if ( - Utils.isSameConnection( - connection.connectionCreds, - nodeConnection, - ) + Utils.isSameConnectionInfo(connection, nodeConnection) ) { - // if it's not the defaul label + // if it's not the default label if ( - connection.label !== - connection.connectionCreds.server + ConnInfo.getSimpleConnectionDisplayName( + connection, + ) !== connection.server ) { - nodeLabel = connection.label; + nodeLabel = + ConnInfo.getSimpleConnectionDisplayName( + connection, + ); } break; } @@ -159,7 +160,7 @@ export class ObjectExplorerService { self._currentNode.sessionId === result.sessionId ) { nodeLabel = !nodeLabel - ? getConnectionDisplayName( + ? ConnInfo.getConnectionDisplayName( self._currentNode.connectionInfo, ) : nodeLabel; @@ -173,7 +174,7 @@ export class ObjectExplorerService { ); } else { nodeLabel = !nodeLabel - ? getConnectionDisplayName(nodeConnection) + ? ConnInfo.getConnectionDisplayName(nodeConnection) : nodeLabel; node = TreeNodeInfo.fromNodeInfo( result.rootNode, @@ -443,7 +444,7 @@ export class ObjectExplorerService { } for (let rootTreeNode of this._rootTreeNodeArray) { if ( - Utils.isSameConnection( + Utils.isSameConnectionInfo( node.connectionInfo, rootTreeNode.connectionInfo, ) && @@ -497,16 +498,13 @@ export class ObjectExplorerService { */ private getSavedConnections(): void { let savedConnections = - this._connectionManager.connectionStore.loadAllConnections(); + this._connectionManager.connectionStore.readAllConnections(); for (const conn of savedConnections) { let nodeLabel = - conn.label === conn.connectionCreds.server - ? getConnectionDisplayName(conn.connectionCreds) - : conn.label; - this._nodePathToNodeLabelMap.set( - conn.connectionCreds.server, - nodeLabel, - ); + ConnInfo.getSimpleConnectionDisplayName(conn) === conn.server + ? ConnInfo.getConnectionDisplayName(conn) + : ConnInfo.getSimpleConnectionDisplayName(conn); + this._nodePathToNodeLabelMap.set(conn.server, nodeLabel); let node = new TreeNodeInfo( nodeLabel, { @@ -520,7 +518,7 @@ export class ObjectExplorerService { undefined, Constants.disconnectedServerLabel, undefined, - conn.connectionCreds, + conn, undefined, undefined, ); @@ -635,7 +633,7 @@ export class ObjectExplorerService { // retrieve saved connections first when opening object explorer // for the first time let savedConnections = - this._connectionManager.connectionStore.loadAllConnections(); + this._connectionManager.connectionStore.readAllConnections(); // if there are no saved connections // show the add connection node if (savedConnections.length === 0) { @@ -660,7 +658,7 @@ export class ObjectExplorerService { * Create an OE session for the given connection credentials * otherwise prompt the user to select a connection to make an * OE out of - * @param connectionCredentials Connection Credentials for a node + * @param connectionProfile Connection Credentials for a node */ public async createSession( promise: Deferred, @@ -679,32 +677,38 @@ export class ObjectExplorerService { this._connectionManager.getServerInfo(connectionCredentials), ); } + if (connectionCredentials) { + const connectionProfile = + connectionCredentials as IConnectionProfile; + + if (!connectionProfile.id) { + connectionProfile.id = Utils.generateGuid(); + } + // connection string based credential - if (connectionCredentials.connectionString) { - if ( - (connectionCredentials as IConnectionProfile).savePassword - ) { + if (connectionProfile.connectionString) { + if ((connectionProfile as IConnectionProfile).savePassword) { // look up connection string let connectionString = await this._connectionManager.connectionStore.lookupPassword( - connectionCredentials, + connectionProfile, true, ); - connectionCredentials.connectionString = connectionString; + connectionProfile.connectionString = connectionString; } } else { if ( ConnectionCredentials.isPasswordBasedCredential( - connectionCredentials, + connectionProfile, ) ) { // show password prompt if SQL Login and password isn't saved - let password = connectionCredentials.password; + let password = connectionProfile.password; if (Utils.isEmpty(password)) { // if password isn't saved if ( - !(connectionCredentials) + !(connectionProfile) .savePassword ) { // prompt for password @@ -718,72 +722,67 @@ export class ObjectExplorerService { // look up saved password password = await this._connectionManager.connectionStore.lookupPassword( - connectionCredentials, + connectionProfile, ); if ( - connectionCredentials.authenticationType !== + connectionProfile.authenticationType !== Constants.azureMfa ) { - connectionCredentials.azureAccountToken = - undefined; + connectionProfile.azureAccountToken = undefined; } } - connectionCredentials.password = password; + connectionProfile.password = password; } } else if ( - connectionCredentials.authenticationType === + connectionProfile.authenticationType === Utils.authTypeToString(AuthenticationTypes.Integrated) ) { - connectionCredentials.azureAccountToken = undefined; + connectionProfile.azureAccountToken = undefined; } else if ( - connectionCredentials.authenticationType === - Constants.azureMfa + connectionProfile.authenticationType === Constants.azureMfa ) { let azureController = this._connectionManager.azureController; let account = this._connectionManager.accountStore.getAccount( - connectionCredentials.accountId, + connectionProfile.accountId, ); let needsRefresh = false; if (!account) { needsRefresh = true; } else if (azureController.isSqlAuthProviderEnabled()) { - connectionCredentials.user = + connectionProfile.user = account.displayInfo.displayName; - connectionCredentials.email = account.displayInfo.email; + connectionProfile.email = account.displayInfo.email; // Update profile after updating user/email await this._connectionManager.connectionUI.saveProfile( - connectionCredentials as IConnectionProfile, + connectionProfile as IConnectionProfile, ); if (!azureController.isAccountInCache(account)) { needsRefresh = true; } } if ( - !connectionCredentials.azureAccountToken && + !connectionProfile.azureAccountToken && (!azureController.isSqlAuthProviderEnabled() || needsRefresh) ) { - void this.refreshAccount( - account, - connectionCredentials, - ); + void this.refreshAccount(account, connectionProfile); } } } const connectionDetails = ConnectionCredentials.createConnectionDetails( - connectionCredentials, + connectionProfile, ); - if ((connectionCredentials as IConnectionProfile).profileName) { + if ((connectionProfile as IConnectionProfile).profileName) { this._nodePathToNodeLabelMap.set( // using the server name as the key because that's what the rest of the OE service expects. // TODO: this service should be refactored to use something guaranteed to be unique across all connections, // but that likely involves a larger refactor of connection management. - connectionCredentials.server, - (connectionCredentials as IConnectionProfile).profileName, + connectionProfile.server, + (connectionProfile as IConnectionProfile).profileName, ); } @@ -795,7 +794,7 @@ export class ObjectExplorerService { if (response) { this._sessionIdToConnectionCredentialsMap.set( response.sessionId, - connectionCredentials, + connectionProfile, ); this._sessionIdToPromiseMap.set(response.sessionId, promise); return response.sessionId; @@ -939,7 +938,7 @@ export class ObjectExplorerService { ): Promise { for (let conn of connections) { for (let node of this._rootTreeNodeArray) { - if (Utils.isSameConnection(node.connectionInfo, conn)) { + if (Utils.isSameConnectionInfo(node.connectionInfo, conn)) { await this.removeObjectExplorerNode(node); } } @@ -980,9 +979,9 @@ export class ObjectExplorerService { } public addDisconnectedNode(connectionCredentials: IConnectionInfo): void { - const label = (connectionCredentials).profileName - ? (connectionCredentials).profileName - : getConnectionDisplayName(connectionCredentials); + const label = (connectionCredentials as IConnectionProfile).profileName + ? (connectionCredentials as IConnectionProfile).profileName + : ConnInfo.getConnectionDisplayName(connectionCredentials); const node = new TreeNodeInfo( label, { diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.ts index c54fdb0856..920f9e93aa 100644 --- a/test/unit/utils.test.ts +++ b/test/unit/utils.test.ts @@ -68,29 +68,33 @@ suite("Utility Tests - isSameConnection", () => { }); test("should return true for matching non-connectionstring connections", () => { - expect(Utils.isSameConnection(connection1, connection2)).to.equal(true); + expect(Utils.isSameConnectionInfo(connection1, connection2)).to.equal( + true, + ); }); test("should return false for non-matching non-connectionstring connections", () => { connection2.server = "some-other-server"; - expect(Utils.isSameConnection(connection1, connection2)).to.equal( + expect(Utils.isSameConnectionInfo(connection1, connection2)).to.equal( false, ); }); test("should return true for matching connectionstring connections", () => { - expect(Utils.isSameConnection(connection3, connection4)).to.equal(true); + expect(Utils.isSameConnectionInfo(connection3, connection4)).to.equal( + true, + ); }); test("should return false for non-matching connectionstring connections", () => { connection4.connectionString = "Server=some-other-server"; - expect(Utils.isSameConnection(connection3, connection4)).to.equal( + expect(Utils.isSameConnectionInfo(connection3, connection4)).to.equal( false, ); }); test("should return false for connectionstring and non-connectionstring connections", () => { - expect(Utils.isSameConnection(connection1, connection3)).to.equal( + expect(Utils.isSameConnectionInfo(connection1, connection3)).to.equal( false, ); }); diff --git a/typings/vscode-mssql.d.ts b/typings/vscode-mssql.d.ts index 1510362b91..359db21e79 100644 --- a/typings/vscode-mssql.d.ts +++ b/typings/vscode-mssql.d.ts @@ -233,7 +233,7 @@ declare module 'vscode-mssql' { } /** - * Information about a database connection + * Information about a database connection necessary for connecting to a database. */ export interface IConnectionInfo { /** From 4b8d1b3fafc4491a3a5d9a60b0249d018fdf2a7d Mon Sep 17 00:00:00 2001 From: "Benjin Dubishar (from Dev Box)" Date: Mon, 13 Jan 2025 17:24:34 -0800 Subject: [PATCH 06/17] correcting map names --- src/objectExplorer/objectExplorerService.ts | 52 ++++++++++----------- src/objectExplorer/objectExplorerUtils.ts | 1 + src/views/connectionUI.ts | 15 +++++- 3 files changed, 39 insertions(+), 29 deletions(-) diff --git a/src/objectExplorer/objectExplorerService.ts b/src/objectExplorer/objectExplorerService.ts index 172133ddf4..d26ee142e3 100644 --- a/src/objectExplorer/objectExplorerService.ts +++ b/src/objectExplorer/objectExplorerService.ts @@ -66,9 +66,9 @@ export class ObjectExplorerService { private _client: SqlToolsServiceClient; private _currentNode: TreeNodeInfo; private _treeNodeToChildrenMap: Map; - private _nodePathToNodeLabelMap: Map; + private _connectionProfileIdToNodeLabelMap: Map; private _rootTreeNodeArray: Array; - private _sessionIdToConnectionCredentialsMap: Map; + private _sessionIdToConnectionProfileMap: Map; private _expandParamsToTreeNodeInfoMap: Map; // Deferred promise maps @@ -88,11 +88,11 @@ export class ObjectExplorerService { vscode.TreeItem[] >(); this._rootTreeNodeArray = new Array(); - this._sessionIdToConnectionCredentialsMap = new Map< + this._sessionIdToConnectionProfileMap = new Map< string, - IConnectionInfo + IConnectionProfile >(); - this._nodePathToNodeLabelMap = new Map(); + this._connectionProfileIdToNodeLabelMap = new Map(); this._sessionIdToPromiseMap = new Map< string, Deferred @@ -123,17 +123,19 @@ export class ObjectExplorerService { self.currentNode = getParentNode(self.currentNode); } if (result.success) { - let nodeLabel = this._nodePathToNodeLabelMap.get( - result.rootNode.nodePath, - ); + const connectionId = this._sessionIdToConnectionProfileMap.get( + result.sessionId, + ).id; + + let nodeLabel = + this._connectionProfileIdToNodeLabelMap.get(connectionId); // if no node label, check if it has a name in saved profiles // in case this call came from new query let savedConnections = this._connectionManager.connectionStore.readAllConnections(); - let nodeConnection = - this._sessionIdToConnectionCredentialsMap.get( - result.sessionId, - ); + let nodeConnection = this._sessionIdToConnectionProfileMap.get( + result.sessionId, + ); for (let connection of savedConnections) { if ( Utils.isSameConnectionInfo(connection, nodeConnection) @@ -362,10 +364,9 @@ export class ObjectExplorerService { const self = this; const handler = (result: ExpandResponse) => { if (result && result.nodes) { - const credentials = - self._sessionIdToConnectionCredentialsMap.get( - result.sessionId, - ); + const credentials = self._sessionIdToConnectionProfileMap.get( + result.sessionId, + ); const expandParams: ExpandParams = { sessionId: result.sessionId, nodePath: result.nodePath, @@ -504,7 +505,7 @@ export class ObjectExplorerService { ConnInfo.getSimpleConnectionDisplayName(conn) === conn.server ? ConnInfo.getConnectionDisplayName(conn) : ConnInfo.getSimpleConnectionDisplayName(conn); - this._nodePathToNodeLabelMap.set(conn.server, nodeLabel); + this._connectionProfileIdToNodeLabelMap.set(conn.id, nodeLabel); let node = new TreeNodeInfo( nodeLabel, { @@ -777,11 +778,8 @@ export class ObjectExplorerService { ); if ((connectionProfile as IConnectionProfile).profileName) { - this._nodePathToNodeLabelMap.set( - // using the server name as the key because that's what the rest of the OE service expects. - // TODO: this service should be refactored to use something guaranteed to be unique across all connections, - // but that likely involves a larger refactor of connection management. - connectionProfile.server, + this._connectionProfileIdToNodeLabelMap.set( + connectionProfile.id, (connectionProfile as IConnectionProfile).profileName, ); } @@ -792,7 +790,7 @@ export class ObjectExplorerService { connectionDetails, ); if (response) { - this._sessionIdToConnectionCredentialsMap.set( + this._sessionIdToConnectionProfileMap.set( response.sessionId, connectionProfile, ); @@ -860,8 +858,8 @@ export class ObjectExplorerService { } } public getConnectionCredentials(sessionId: string): IConnectionInfo { - if (this._sessionIdToConnectionCredentialsMap.has(sessionId)) { - return this._sessionIdToConnectionCredentialsMap.get(sessionId); + if (this._sessionIdToConnectionProfileMap.has(sessionId)) { + return this._sessionIdToConnectionProfileMap.get(sessionId); } return undefined; } @@ -917,7 +915,7 @@ export class ObjectExplorerService { new ConnectTreeNode(this._currentNode), ]); } - this._nodePathToNodeLabelMap.delete(node.nodePath); + this._connectionProfileIdToNodeLabelMap.delete(node.nodePath); // TODO: update this to use connection id this.cleanNodeChildren(node); sendActionEvent( TelemetryViews.ObjectExplorer, @@ -1017,7 +1015,7 @@ export class ObjectExplorerService { closeSessionParams, ); if (response && response.success) { - this._sessionIdToConnectionCredentialsMap.delete( + this._sessionIdToConnectionProfileMap.delete( response.sessionId, ); if (this._sessionIdToPromiseMap.has(node.sessionId)) { diff --git a/src/objectExplorer/objectExplorerUtils.ts b/src/objectExplorer/objectExplorerUtils.ts index f0afddf435..93f7d75bd2 100644 --- a/src/objectExplorer/objectExplorerUtils.ts +++ b/src/objectExplorer/objectExplorerUtils.ts @@ -40,6 +40,7 @@ export class ObjectExplorerUtils { return ObjectExplorerUtils.getNodeUriFromProfile(profile); } + // TODO: this function emulates one in STS; replace with call to STS to avoid mixups public static getNodeUriFromProfile(profile: IConnectionProfile): string { let uri: string; if (profile.connectionString) { diff --git a/src/views/connectionUI.ts b/src/views/connectionUI.ts index ec5bc12099..7a4abd7682 100644 --- a/src/views/connectionUI.ts +++ b/src/views/connectionUI.ts @@ -719,13 +719,19 @@ export class ConnectionUI { } private async promptForCreateProfile(): Promise { - return await ConnectionProfile.createProfile( + const profile = await ConnectionProfile.createProfile( this._prompter, this._connectionStore, this._context, this.connectionManager.azureController, this._accountStore, ); + + if (profile.id === undefined) { + profile.id = Utils.generateGuid(); + } + + return profile; } private async promptToRetryAndSaveProfile( @@ -756,7 +762,7 @@ export class ConnectionUI { LocalizedConstants.retryLabel, ); if (result === LocalizedConstants.retryLabel) { - return await ConnectionProfile.createProfile( + const newProfile = await ConnectionProfile.createProfile( this._prompter, this._connectionStore, this._context, @@ -764,6 +770,11 @@ export class ConnectionUI { this._accountStore, profile, ); + if (newProfile.id === undefined) { + newProfile.id = Utils.generateGuid(); + } + + return newProfile; } else { // user cancelled the prompt - throw error so that we know user cancelled throw new CancelError(); From 94d8bbbe5f89e254c563e59b27f07a3b27f72b23 Mon Sep 17 00:00:00 2001 From: "Benjin Dubishar (from Dev Box)" Date: Tue, 14 Jan 2025 14:39:59 -0800 Subject: [PATCH 07/17] everyone swapped to sessionID as key --- src/languageservice/serviceclient.ts | 7 +- .../objectExplorer/getSessionIdRequest.ts | 29 ++++ src/objectExplorer/objectExplorerService.ts | 158 ++++++++++-------- src/views/connectionUI.ts | 2 +- 4 files changed, 124 insertions(+), 72 deletions(-) create mode 100644 src/models/contracts/objectExplorer/getSessionIdRequest.ts diff --git a/src/languageservice/serviceclient.ts b/src/languageservice/serviceclient.ts index 7d73e8e85d..7eee0d8893 100644 --- a/src/languageservice/serviceclient.ts +++ b/src/languageservice/serviceclient.ts @@ -31,7 +31,7 @@ import StatusView from "../views/statusView"; import * as LanguageServiceContracts from "../models/contracts/languageService"; import { IConfig } from "../languageservice/interfaces"; import { exists } from "../utils/utils"; -import { env } from "process"; +// import { env } from "process"; import { getAppDataPath, getEnableConnectionPoolingConfig, @@ -39,7 +39,7 @@ import { } from "../azure/utils"; import { serviceName } from "../azure/constants"; -const STS_OVERRIDE_ENV_VAR = "MSSQL_SQLTOOLSSERVICE"; +// const STS_OVERRIDE_ENV_VAR = "MSSQL_SQLTOOLSSERVICE"; let _channel: vscode.OutputChannel = undefined; @@ -360,7 +360,8 @@ export default class SqlToolsServiceClient { // This env var is used to override the base install location of STS - primarily to be used for debugging scenarios. try { const exeFiles = this._config.getSqlToolsExecutableFiles(); - const stsRootPath = env[STS_OVERRIDE_ENV_VAR]; + const stsRootPath = + "C:\\Users\\benjind\\Source\\sqltoolsservice\\src\\Microsoft.SqlTools.ServiceLayer\\bin\\Release\\net8.0\\publish"; //env[STS_OVERRIDE_ENV_VAR]; if (stsRootPath) { for (const exeFile of exeFiles) { const serverFullPath = path.join(stsRootPath, exeFile); diff --git a/src/models/contracts/objectExplorer/getSessionIdRequest.ts b/src/models/contracts/objectExplorer/getSessionIdRequest.ts new file mode 100644 index 0000000000..834ca492a0 --- /dev/null +++ b/src/models/contracts/objectExplorer/getSessionIdRequest.ts @@ -0,0 +1,29 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { RequestType } from "vscode-languageclient"; +import { ConnectionDetails } from "vscode-mssql"; + +// Create session request message callback declaration +export namespace GetSessionIdRequest { + export const type = new RequestType< + ConnectionDetails, + GetSessionIdResponse, + void, + void + >("objectexplorer/getsessionid"); +} + +/** + * Contains a sessionId to be used when requesting + * expansion of nodes + */ +export class GetSessionIdResponse { + /** + * Unique Id to use when sending any requests for objects in the tree + * under the node + */ + public sessionId: string; +} diff --git a/src/objectExplorer/objectExplorerService.ts b/src/objectExplorer/objectExplorerService.ts index d26ee142e3..82b59b5ae1 100644 --- a/src/objectExplorer/objectExplorerService.ts +++ b/src/objectExplorer/objectExplorerService.ts @@ -52,6 +52,10 @@ import { TelemetryActions, TelemetryViews, } from "../sharedInterfaces/telemetry"; +import { + GetSessionIdRequest, + GetSessionIdResponse, +} from "../models/contracts/objectExplorer/getSessionIdRequest"; function getParentNode(node: TreeNodeType): TreeNodeInfo { node = node.parentNode; @@ -66,7 +70,7 @@ export class ObjectExplorerService { private _client: SqlToolsServiceClient; private _currentNode: TreeNodeInfo; private _treeNodeToChildrenMap: Map; - private _connectionProfileIdToNodeLabelMap: Map; + private _sessionIdToNodeLabelMap: Map; private _rootTreeNodeArray: Array; private _sessionIdToConnectionProfileMap: Map; private _expandParamsToTreeNodeInfoMap: Map; @@ -92,7 +96,7 @@ export class ObjectExplorerService { string, IConnectionProfile >(); - this._connectionProfileIdToNodeLabelMap = new Map(); + this._sessionIdToNodeLabelMap = new Map(); this._sessionIdToPromiseMap = new Map< string, Deferred @@ -123,37 +127,19 @@ export class ObjectExplorerService { self.currentNode = getParentNode(self.currentNode); } if (result.success) { - const connectionId = this._sessionIdToConnectionProfileMap.get( - result.sessionId, - ).id; - let nodeLabel = - this._connectionProfileIdToNodeLabelMap.get(connectionId); + this._sessionIdToNodeLabelMap.get(result.sessionId) ?? + ConnInfo.getConnectionDisplayName( + self._currentNode.connectionInfo, + ); // if no node label, check if it has a name in saved profiles // in case this call came from new query - let savedConnections = - this._connectionManager.connectionStore.readAllConnections(); + // let savedConnections = + // this._connectionManager.connectionStore.readAllConnections(); let nodeConnection = this._sessionIdToConnectionProfileMap.get( result.sessionId, ); - for (let connection of savedConnections) { - if ( - Utils.isSameConnectionInfo(connection, nodeConnection) - ) { - // if it's not the default label - if ( - ConnInfo.getSimpleConnectionDisplayName( - connection, - ) !== connection.server - ) { - nodeLabel = - ConnInfo.getSimpleConnectionDisplayName( - connection, - ); - } - break; - } - } + // set connection and other things let node: TreeNodeInfo; @@ -161,11 +147,6 @@ export class ObjectExplorerService { self._currentNode && self._currentNode.sessionId === result.sessionId ) { - nodeLabel = !nodeLabel - ? ConnInfo.getConnectionDisplayName( - self._currentNode.connectionInfo, - ) - : nodeLabel; node = TreeNodeInfo.fromNodeInfo( result.rootNode, result.sessionId, @@ -175,9 +156,6 @@ export class ObjectExplorerService { Constants.serverLabel, ); } else { - nodeLabel = !nodeLabel - ? ConnInfo.getConnectionDisplayName(nodeConnection) - : nodeLabel; node = TreeNodeInfo.fromNodeInfo( result.rootNode, result.sessionId, @@ -188,7 +166,7 @@ export class ObjectExplorerService { ); } // make a connection if not connected already - const nodeUri = ObjectExplorerUtils.getNodeUri(node); + const nodeUri = this.getNodeIdentifier(node); if ( !this._connectionManager.isConnected(nodeUri) && !this._connectionManager.isConnecting(nodeUri) @@ -247,8 +225,8 @@ export class ObjectExplorerService { handleFirewallResult.result && handleFirewallResult.ipAddress ) { - const nodeUri = ObjectExplorerUtils.getNodeUri( - self._currentNode, + const nodeUri = this.getNodeIdentifier( + self.currentNode, ); const profile = ( self._currentNode.connectionInfo @@ -300,7 +278,7 @@ export class ObjectExplorerService { ): Promise { node.connectionInfo = profile; this.updateNode(node); - let fileUri = ObjectExplorerUtils.getNodeUri(node); + let fileUri = this.getNodeIdentifier(node); if ( await this._connectionManager.connectionStore.saveProfile( profile as IConnectionProfile, @@ -360,6 +338,8 @@ export class ObjectExplorerService { return undefined; } + // TODO: call/handle refactor? + private handleExpandSessionNotification(): NotificationHandler { const self = this; const handler = (result: ExpandResponse) => { @@ -497,7 +477,7 @@ export class ObjectExplorerService { /** * Get nodes from saved connections */ - private getSavedConnections(): void { + private async getSavedConnections(): Promise { let savedConnections = this._connectionManager.connectionStore.readAllConnections(); for (const conn of savedConnections) { @@ -505,7 +485,17 @@ export class ObjectExplorerService { ConnInfo.getSimpleConnectionDisplayName(conn) === conn.server ? ConnInfo.getConnectionDisplayName(conn) : ConnInfo.getSimpleConnectionDisplayName(conn); - this._connectionProfileIdToNodeLabelMap.set(conn.id, nodeLabel); + + const connectionDetails = + ConnectionCredentials.createConnectionDetails(conn); + + const response: CreateSessionResponse = + await this._connectionManager.client.sendRequest( + GetSessionIdRequest.type, + connectionDetails, + ); + + this._sessionIdToNodeLabelMap.set(response.sessionId, nodeLabel); let node = new TreeNodeInfo( nodeLabel, { @@ -645,7 +635,7 @@ export class ObjectExplorerService { if (!this._objectExplorerProvider.objectExplorerExists) { // if there are actually saved connections this._rootTreeNodeArray = []; - this.getSavedConnections(); + await this.getSavedConnections(); this._objectExplorerProvider.objectExplorerExists = true; return this.sortByServerName(this._rootTreeNodeArray); } else { @@ -664,7 +654,7 @@ export class ObjectExplorerService { public async createSession( promise: Deferred, connectionCredentials?: IConnectionInfo, - context?: vscode.ExtensionContext, + _context?: vscode.ExtensionContext, ): Promise { if (!connectionCredentials) { const connectionUI = this._connectionManager.connectionUI; @@ -777,9 +767,15 @@ export class ObjectExplorerService { connectionProfile, ); + const sessionIdResponse: GetSessionIdResponse = + await this._connectionManager.client.sendRequest( + GetSessionIdRequest.type, + connectionDetails, + ); + if ((connectionProfile as IConnectionProfile).profileName) { - this._connectionProfileIdToNodeLabelMap.set( - connectionProfile.id, + this._sessionIdToNodeLabelMap.set( + sessionIdResponse.sessionId, (connectionProfile as IConnectionProfile).profileName, ); } @@ -869,7 +865,7 @@ export class ObjectExplorerService { isDisconnect: boolean = false, ): Promise { await this.closeSession(node); - const nodeUri = ObjectExplorerUtils.getNodeUri(node); + const nodeUri = this.getNodeIdentifier(node); await this._connectionManager.disconnect(nodeUri); if (!isDisconnect) { const index = this._rootTreeNodeArray.indexOf(node, 0); @@ -885,7 +881,7 @@ export class ObjectExplorerService { subType: "", }; node.sessionId = undefined; - if (!(node.connectionInfo).savePassword) { + if (!(node.connectionInfo as IConnectionProfile).savePassword) { node.connectionInfo.password = ""; } const label = @@ -915,7 +911,18 @@ export class ObjectExplorerService { new ConnectTreeNode(this._currentNode), ]); } - this._connectionProfileIdToNodeLabelMap.delete(node.nodePath); // TODO: update this to use connection id + + const connectionDetails = ConnectionCredentials.createConnectionDetails( + node.connectionInfo, + ); + + const sessionIdResponse: GetSessionIdResponse = + await this._connectionManager.client.sendRequest( + GetSessionIdRequest.type, + connectionDetails, + ); + + this._sessionIdToNodeLabelMap.delete(sessionIdResponse.sessionId); this.cleanNodeChildren(node); sendActionEvent( TelemetryViews.ObjectExplorer, @@ -1005,29 +1012,44 @@ export class ObjectExplorerService { * @param node */ public async closeSession(node: TreeNodeInfo): Promise { - if (node.sessionId) { - const closeSessionParams: CloseSessionParams = { - sessionId: node.sessionId, - }; - const response: CloseSessionResponse = - await this._connectionManager.client.sendRequest( - CloseSessionRequest.type, - closeSessionParams, - ); - if (response && response.success) { - this._sessionIdToConnectionProfileMap.delete( - response.sessionId, + if (!node.sessionId) { + return; + } + + const closeSessionParams: CloseSessionParams = { + sessionId: node.sessionId, + }; + const response: CloseSessionResponse = + await this._connectionManager.client.sendRequest( + CloseSessionRequest.type, + closeSessionParams, + ); + + if (response && response.success) { + if (response.sessionId !== node.sessionId) { + this._client.logger.error( + "Session ID mismatch in closeSession() response", ); - if (this._sessionIdToPromiseMap.has(node.sessionId)) { - this._sessionIdToPromiseMap.delete(node.sessionId); - } - const nodeUri = ObjectExplorerUtils.getNodeUri(node); - await this._connectionManager.disconnect(nodeUri); - this.cleanNodeChildren(node); - return; } + + this._sessionIdToConnectionProfileMap.delete(node.sessionId); + this._sessionIdToPromiseMap.delete(node.sessionId); + + const nodeUri = this.getNodeIdentifier(node); + await this._connectionManager.disconnect(nodeUri); + this.cleanNodeChildren(node); + + return; + } + } + + private getNodeIdentifier(node: TreeNodeInfo): string { + if (node.sessionId) { + return node.sessionId; + } else { + this._client.logger.error("Node does not have a session ID"); + return ObjectExplorerUtils.getNodeUri(node); // TODO: can this removed entirely? ideally, every node has a session ID associated with it } - return; } /** Getters */ diff --git a/src/views/connectionUI.ts b/src/views/connectionUI.ts index 7a4abd7682..7d0841dced 100644 --- a/src/views/connectionUI.ts +++ b/src/views/connectionUI.ts @@ -678,7 +678,7 @@ export class ConnectionUI { * false otherwise */ public async handleFirewallError( - uri: string, + _uri: string, profile: IConnectionProfile, ipAddress: string, ): Promise { From 39b5c0e4e8ef239f478098a7e07be652524d944f Mon Sep 17 00:00:00 2001 From: "Benjin Dubishar (from Dev Box)" Date: Tue, 14 Jan 2025 14:56:53 -0800 Subject: [PATCH 08/17] correction string --- src/objectExplorer/objectExplorerService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/objectExplorer/objectExplorerService.ts b/src/objectExplorer/objectExplorerService.ts index 82b59b5ae1..317d66b58a 100644 --- a/src/objectExplorer/objectExplorerService.ts +++ b/src/objectExplorer/objectExplorerService.ts @@ -196,7 +196,7 @@ export class ObjectExplorerService { errorNumber = result.errorNumber; } if (result.errorMessage) { - error += ` : ${result.errorMessage}`; + error += `: ${result.errorMessage}`; } if ( From cda4ed72ff0706aaabf94d16eb496490e209b628 Mon Sep 17 00:00:00 2001 From: "Benjin Dubishar (from Dev Box)" Date: Mon, 10 Feb 2025 11:20:13 -0800 Subject: [PATCH 09/17] reverting serviceclient changes --- src/languageservice/serviceclient.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/languageservice/serviceclient.ts b/src/languageservice/serviceclient.ts index 7eee0d8893..7d73e8e85d 100644 --- a/src/languageservice/serviceclient.ts +++ b/src/languageservice/serviceclient.ts @@ -31,7 +31,7 @@ import StatusView from "../views/statusView"; import * as LanguageServiceContracts from "../models/contracts/languageService"; import { IConfig } from "../languageservice/interfaces"; import { exists } from "../utils/utils"; -// import { env } from "process"; +import { env } from "process"; import { getAppDataPath, getEnableConnectionPoolingConfig, @@ -39,7 +39,7 @@ import { } from "../azure/utils"; import { serviceName } from "../azure/constants"; -// const STS_OVERRIDE_ENV_VAR = "MSSQL_SQLTOOLSSERVICE"; +const STS_OVERRIDE_ENV_VAR = "MSSQL_SQLTOOLSSERVICE"; let _channel: vscode.OutputChannel = undefined; @@ -360,8 +360,7 @@ export default class SqlToolsServiceClient { // This env var is used to override the base install location of STS - primarily to be used for debugging scenarios. try { const exeFiles = this._config.getSqlToolsExecutableFiles(); - const stsRootPath = - "C:\\Users\\benjind\\Source\\sqltoolsservice\\src\\Microsoft.SqlTools.ServiceLayer\\bin\\Release\\net8.0\\publish"; //env[STS_OVERRIDE_ENV_VAR]; + const stsRootPath = env[STS_OVERRIDE_ENV_VAR]; if (stsRootPath) { for (const exeFile of exeFiles) { const serverFullPath = path.join(stsRootPath, exeFile); From f7bcd88f0b4f638a32f36c7a746f21417bcdfd46 Mon Sep 17 00:00:00 2001 From: "Benjin Dubishar (from Dev Box)" Date: Tue, 11 Feb 2025 14:36:20 -0800 Subject: [PATCH 10/17] cleaning up disconnected node type --- src/constants/constants.ts | 2 +- src/controllers/connectionManager.ts | 2 + src/controllers/mainController.ts | 3 +- src/objectExplorer/objectExplorerService.ts | 51 ++++++++------------- src/objectExplorer/objectExplorerUtils.ts | 4 +- 5 files changed, 27 insertions(+), 35 deletions(-) diff --git a/src/constants/constants.ts b/src/constants/constants.ts index e833ac2f76..6f930d0a2e 100644 --- a/src/constants/constants.ts +++ b/src/constants/constants.ts @@ -18,7 +18,7 @@ export const outputChannelName = "MSSQL"; export const connectionConfigFilename = "settings.json"; export const connectionsArrayName = "connections"; export const connectionGroupsArrayName = "connectionGroups"; -export const disconnectedServerLabel = "disconnectedServer"; +export const disconnectedServerNodeType = "disconnectedServer"; export const serverLabel = "Server"; export const folderLabel = "Folder"; export const cmdRunQuery = "mssql.runQuery"; diff --git a/src/controllers/connectionManager.ts b/src/controllers/connectionManager.ts index 8229525e47..9260f3292e 100644 --- a/src/controllers/connectionManager.ts +++ b/src/controllers/connectionManager.ts @@ -1389,6 +1389,7 @@ export default class ConnectionManager { throw new Error(LocalizedConstants.serverNameMissing); } + // Refresh Azure auth token if necessary if (connectionCreds.authenticationType === Constants.azureMfa) { if ( AzureController.isTokenInValid( @@ -1450,6 +1451,7 @@ export default class ConnectionManager { } } + // Fetch the connection string from the connection store if necessary if ( connectionCreds.connectionString?.includes( ConnectionStore.CRED_PREFIX, diff --git a/src/controllers/mainController.ts b/src/controllers/mainController.ts index 7e33362500..4933b4f273 100644 --- a/src/controllers/mainController.ts +++ b/src/controllers/mainController.ts @@ -1116,7 +1116,8 @@ export default class MainController implements vscode.Disposable { return; } else if ( node.context.type === Constants.serverLabel || - node.context.type === Constants.disconnectedServerLabel + node.context.type === + Constants.disconnectedServerNodeType ) { const label = typeof node.label === "string" diff --git a/src/objectExplorer/objectExplorerService.ts b/src/objectExplorer/objectExplorerService.ts index f3943a586e..b809ee1013 100644 --- a/src/objectExplorer/objectExplorerService.ts +++ b/src/objectExplorer/objectExplorerService.ts @@ -43,7 +43,7 @@ import * as Utils from "../models/utils"; import { ConnectionCredentials } from "../models/connectionCredentials"; import { ConnectionProfile } from "../models/connectionProfile"; import providerSettings from "../azure/providerSettings"; -import { IConnectionInfo } from "vscode-mssql"; +import { IConnectionInfo, TreeNodeContextValue } from "vscode-mssql"; import { sendActionEvent } from "../telemetry/telemetry"; import { IAccount } from "../models/contracts/azure"; import * as AzureConstants from "../azure/constants"; @@ -477,7 +477,7 @@ export class ObjectExplorerService { /** * Get nodes from saved connections */ - private async getSavedConnections(): Promise { + private async addSavedNodesConnectionsToRoot(): Promise { let savedConnections = this._connectionManager.connectionStore.readAllConnections(); for (const conn of savedConnections) { @@ -498,16 +498,11 @@ export class ObjectExplorerService { this._sessionIdToNodeLabelMap.set(response.sessionId, nodeLabel); let node = new TreeNodeInfo( nodeLabel, - { - type: Constants.disconnectedServerLabel, - filterable: false, - hasFilters: false, - subType: "", - }, + ObjectExplorerService.disconnectedNodeContextValue, TreeItemCollapsibleState.Collapsed, undefined, undefined, - Constants.disconnectedServerLabel, + Constants.disconnectedServerNodeType, undefined, conn, undefined, @@ -517,6 +512,15 @@ export class ObjectExplorerService { } } + private static get disconnectedNodeContextValue(): TreeNodeContextValue { + return { + type: Constants.disconnectedServerNodeType, + filterable: false, + hasFilters: false, + subType: "", + }; + } + /** * Clean up expansion promises for a node * @param node The selected node @@ -635,7 +639,7 @@ export class ObjectExplorerService { if (!this._objectExplorerProvider.objectExplorerExists) { // if there are actually saved connections this._rootTreeNodeArray = []; - await this.getSavedConnections(); + await this.addSavedNodesConnectionsToRoot(); this._objectExplorerProvider.objectExplorerExists = true; return this.sortByServerName(this._rootTreeNodeArray); } else { @@ -873,13 +877,8 @@ export class ObjectExplorerService { this._rootTreeNodeArray.splice(index, 1); } } else { - node.nodeType = Constants.disconnectedServerLabel; - node.context = { - type: Constants.disconnectedServerLabel, - filterable: false, - hasFilters: false, - subType: "", - }; + node.nodeType = Constants.disconnectedServerNodeType; + node.context = ObjectExplorerService.disconnectedNodeContextValue; node.sessionId = undefined; if (!(node.connectionInfo as IConnectionProfile).savePassword) { node.connectionInfo.password = ""; @@ -889,16 +888,11 @@ export class ObjectExplorerService { // make a new node to show disconnected behavior let disconnectedNode = new TreeNodeInfo( label, - { - type: Constants.disconnectedServerLabel, - filterable: false, - hasFilters: false, - subType: "", - }, + ObjectExplorerService.disconnectedNodeContextValue, node.collapsibleState, node.nodePath, node.nodeStatus, - Constants.disconnectedServerLabel, + Constants.disconnectedServerNodeType, undefined, node.connectionInfo, node.parentNode, @@ -990,16 +984,11 @@ export class ObjectExplorerService { : ConnInfo.getConnectionDisplayName(connectionCredentials); const node = new TreeNodeInfo( label, - { - type: Constants.disconnectedServerLabel, - filterable: false, - hasFilters: false, - subType: "", - }, + ObjectExplorerService.disconnectedNodeContextValue, vscode.TreeItemCollapsibleState.Collapsed, undefined, undefined, - Constants.disconnectedServerLabel, + Constants.disconnectedServerNodeType, undefined, connectionCredentials, undefined, diff --git a/src/objectExplorer/objectExplorerUtils.ts b/src/objectExplorer/objectExplorerUtils.ts index 93f7d75bd2..e3e08b65cd 100644 --- a/src/objectExplorer/objectExplorerUtils.ts +++ b/src/objectExplorer/objectExplorerUtils.ts @@ -19,7 +19,7 @@ export class ObjectExplorerUtils { public static iconPath(label: string): string { if (label) { - if (label === Constants.disconnectedServerLabel) { + if (label === Constants.disconnectedServerNodeType) { // if disconnected label = `${Constants.serverLabel}_red`; } else if (label === Constants.serverLabel) { @@ -68,7 +68,7 @@ export class ObjectExplorerUtils { // We're on a server node so just use the database directly from the connection string if ( node.nodeType === Constants.serverLabel || - node.nodeType === Constants.disconnectedServerLabel + node.nodeType === Constants.disconnectedServerNodeType ) { return node.connectionInfo.database; } From 4f9ae4ac4f9bc71a05c5f4e851dc7d22864fc020 Mon Sep 17 00:00:00 2001 From: "Benjin Dubishar (from Dev Box)" Date: Wed, 12 Feb 2025 15:27:22 -0800 Subject: [PATCH 11/17] deduping entra token checks --- src/azure/azureController.ts | 8 +- .../connectionDialogWebviewController.ts | 3 +- src/controllers/connectionManager.ts | 213 +++++++----------- src/models/connectionStore.ts | 107 --------- src/views/connectionUI.ts | 71 +++--- test/unit/azureController.test.ts | 30 +-- 6 files changed, 139 insertions(+), 293 deletions(-) diff --git a/src/azure/azureController.ts b/src/azure/azureController.ts index 59a367be80..d935988eaf 100644 --- a/src/azure/azureController.ts +++ b/src/azure/azureController.ts @@ -222,7 +222,7 @@ export abstract class AzureController { ): Promise { if ( session?.account && - AzureController.isTokenInValid( + !AzureController.isTokenValid( session.token!.token, session.token!.expiresOn, ) @@ -245,12 +245,12 @@ export abstract class AzureController { } /** - * Returns true if token is invalid or expired + * Returns false if token is invalid or expired * @param token Token * @param token expiry */ - public static isTokenInValid(token: string, expiresOn?: number): boolean { - return !token || AzureController.isTokenExpired(expiresOn); + public static isTokenValid(token: string, expiresOn?: number): boolean { + return !!token && !AzureController.isTokenExpired(expiresOn); } /** diff --git a/src/connectionconfig/connectionDialogWebviewController.ts b/src/connectionconfig/connectionDialogWebviewController.ts index a6255a9dae..f6c40c0774 100644 --- a/src/connectionconfig/connectionDialogWebviewController.ts +++ b/src/connectionconfig/connectionDialogWebviewController.ts @@ -996,10 +996,11 @@ export class ConnectionDialogWebviewController extends ReactWebviewPanelControll account, undefined, ); - const isTokenExpired = AzureController.isTokenInValid( + const isTokenExpired = !AzureController.isTokenValid( session.token, session.expiresOn, ); + if (isTokenExpired) { actionButtons.push({ label: refreshTokenLabel, diff --git a/src/controllers/connectionManager.ts b/src/controllers/connectionManager.ts index 9260f3292e..2b1a2bad9a 100644 --- a/src/controllers/connectionManager.ts +++ b/src/controllers/connectionManager.ts @@ -1030,7 +1030,7 @@ export default class ConnectionManager { ); return true; } - const flavor = await this._connectionUI.promptLanguageFlavor(); + const flavor = await this.connectionUI.promptLanguageFlavor(); if (!flavor) { return false; } @@ -1196,7 +1196,81 @@ export default class ConnectionManager { return creds; } - // create a new connection with the connectionCreds provided + /** + * Checks the Entra token's validity, and refreshes if necessary. + * Does nothing if connection is not using Entra auth. + */ + public async confirmEntraTokenValidity(connectionInfo: IConnectionInfo) { + if (connectionInfo.authenticationType !== Constants.azureMfa) { + // Connection not using Entra auth, nothing to validate + return; + } + + if ( + AzureController.isTokenValid( + connectionInfo.azureAccountToken, + connectionInfo.expiresOn, + ) + ) { + // Token not expired, nothing to refresh + return; + } + + let account: IAccount; + let profile: ConnectionProfile; + + if (connectionInfo.accountId) { + account = this.accountStore.getAccount(connectionInfo.accountId); + profile = new ConnectionProfile(connectionInfo); + } else { + throw new Error(LocalizedConstants.cannotConnect); + } + + if (!account) { + throw new Error(LocalizedConstants.msgAccountNotFound); + } + + // Always set username + connectionInfo.user = account.displayInfo.displayName; + connectionInfo.email = account.displayInfo.email; + profile.user = account.displayInfo.displayName; + profile.email = account.displayInfo.email; + + const azureAccountToken = await this.azureController.refreshAccessToken( + account, + this.accountStore, + profile.tenantId, + providerSettings.resources.databaseResource!, + ); + + if (!azureAccountToken) { + let errorMessage = LocalizedConstants.msgAccountRefreshFailed; + let refreshResult = await this.vscodeWrapper.showErrorMessage( + errorMessage, + LocalizedConstants.refreshTokenLabel, + ); + if (refreshResult === LocalizedConstants.refreshTokenLabel) { + await this.azureController.populateAccountProperties( + profile, + this.accountStore, + providerSettings.resources.databaseResource!, + ); + } else { + throw new Error(LocalizedConstants.cannotConnect); + } + } else { + connectionInfo.azureAccountToken = azureAccountToken.token; + connectionInfo.expiresOn = azureAccountToken.expiresOn; + } + } + + /** + * create a new connection with the connectionCreds provided + * @param fileUri + * @param connectionCreds ConnectionInfo to connect with + * @param promise Deferred promise to resolve when connection is complete + * @returns true if connection is successful, false otherwise + */ public async connect( fileUri: string, connectionCreds: IConnectionInfo, @@ -1208,82 +1282,18 @@ export default class ConnectionManager { title: LocalizedConstants.connectProgressNoticationTitle, cancellable: false, }, - async (_progress, _token) => { + async (_progress, _cancellationToken) => { if ( !connectionCreds.server && !connectionCreds.connectionString ) { throw new Error(LocalizedConstants.serverNameMissing); } - // Check if the azure account token is present before sending connect request (only with SQL Auth Provider is not enabled.) + if (connectionCreds.authenticationType === Constants.azureMfa) { - if ( - AzureController.isTokenInValid( - connectionCreds.azureAccountToken, - connectionCreds.expiresOn, - ) - ) { - let account: IAccount; - let profile: ConnectionProfile; - if (connectionCreds.accountId) { - account = this.accountStore.getAccount( - connectionCreds.accountId, - ); - profile = new ConnectionProfile(connectionCreds); - } else { - throw new Error(LocalizedConstants.cannotConnect); - } - if (account) { - // Always set username - connectionCreds.user = - account.displayInfo.displayName; - connectionCreds.email = account.displayInfo.email; - profile.user = account.displayInfo.displayName; - profile.email = account.displayInfo.email; - let azureAccountToken = - await this.azureController.refreshAccessToken( - account!, - this.accountStore, - profile.tenantId, - providerSettings.resources - .databaseResource!, - ); - if (!azureAccountToken) { - let errorMessage = - LocalizedConstants.msgAccountRefreshFailed; - let refreshResult = - await this.vscodeWrapper.showErrorMessage( - errorMessage, - LocalizedConstants.refreshTokenLabel, - ); - if ( - refreshResult === - LocalizedConstants.refreshTokenLabel - ) { - await this.azureController.populateAccountProperties( - profile, - this.accountStore, - providerSettings.resources - .databaseResource!, - ); - } else { - throw new Error( - LocalizedConstants.cannotConnect, - ); - } - } else { - connectionCreds.azureAccountToken = - azureAccountToken.token; - connectionCreds.expiresOn = - azureAccountToken.expiresOn; - } - } else { - throw new Error( - LocalizedConstants.msgAccountNotFound, - ); - } - } + await this.confirmEntraTokenValidity(connectionCreds); } + let connectionPromise = new Promise( async (resolve, reject) => { if ( @@ -1389,66 +1399,9 @@ export default class ConnectionManager { throw new Error(LocalizedConstants.serverNameMissing); } - // Refresh Azure auth token if necessary + // Check if the azure account token is present before sending connect request if (connectionCreds.authenticationType === Constants.azureMfa) { - if ( - AzureController.isTokenInValid( - connectionCreds.azureAccountToken, - connectionCreds.expiresOn, - ) - ) { - let account: IAccount; - let profile: ConnectionProfile; - if (connectionCreds.accountId) { - account = this.accountStore.getAccount( - connectionCreds.accountId, - ); - profile = new ConnectionProfile(connectionCreds); - } else { - throw new Error(LocalizedConstants.cannotConnect); - } - if (account) { - // Always set username - connectionCreds.user = account.displayInfo.displayName; - connectionCreds.email = account.displayInfo.email; - profile.user = account.displayInfo.displayName; - profile.email = account.displayInfo.email; - let azureAccountToken = - await this.azureController.refreshAccessToken( - account!, - this.accountStore, - profile.tenantId, - providerSettings.resources.databaseResource!, - ); - if (!azureAccountToken) { - let errorMessage = - LocalizedConstants.msgAccountRefreshFailed; - let refreshResult = - await this.vscodeWrapper.showErrorMessage( - errorMessage, - LocalizedConstants.refreshTokenLabel, - ); - if ( - refreshResult === - LocalizedConstants.refreshTokenLabel - ) { - await this.azureController.populateAccountProperties( - profile, - this.accountStore, - providerSettings.resources.databaseResource!, - ); - } else { - throw new Error(LocalizedConstants.cannotConnect); - } - } else { - connectionCreds.azureAccountToken = - azureAccountToken.token; - connectionCreds.expiresOn = azureAccountToken.expiresOn; - } - } else { - throw new Error(LocalizedConstants.msgAccountNotFound); - } - } + await this.confirmEntraTokenValidity(connectionCreds); } // Fetch the connection string from the connection store if necessary @@ -1474,7 +1427,7 @@ export default class ConnectionManager { connectionInfo.credentials = connectionCreds; connectionInfo.connecting = true; // Setup the handler for the connection complete notification to call - connectionInfo.connectHandler = (connectResult, error) => {}; + connectionInfo.connectHandler = (connectionResult, error) => {}; this._connections[uri] = connectionInfo; // Note: must call flavor changed before connecting, or the timer showing an animation doesn't occur diff --git a/src/models/connectionStore.ts b/src/models/connectionStore.ts index 7730a0b1b5..f4d98ee082 100644 --- a/src/models/connectionStore.ts +++ b/src/models/connectionStore.ts @@ -659,89 +659,6 @@ export class ConnectionStore { return output; } - // Load connections from user preferences - public loadAllConnections( - addRecentConnections: boolean = false, - ): IConnectionCredentialsQuickPickItem[] { - throw new Error("Method not implemented."); - // let connResults: IConnectionCredentialsQuickPickItem[] = []; - - // // Read recently used items from a memento - // let recentConnections: IConnectionProfile[] = []; - // if (addRecentConnections) { - // recentConnections = - // this.getConnectionsFromGlobalState( - // Constants.configRecentConnections, - // ); - // } - - // // Load connections from user preferences - // // Per this https://code.visualstudio.com/Docs/customization/userandworkspace - // // Connections defined in workspace scope are unioned with the Connections defined in user scope - // let profilesInConfiguration = - // this._connectionConfig.getConnections(true); - - // // Remove any duplicates that are in both recent connections and the user settings - // let profilesInRecentConnectionsList: number[] = []; - // profilesInConfiguration = profilesInConfiguration.filter((profile) => { - // for (let index = 0; index < recentConnections.length; index++) { - // if (Utils.isSameProfile(profile, recentConnections[index])) { - // if ( - // Utils.isSameConnection( - // profile, - // recentConnections[index], - // ) - // ) { - // // The MRU item should reflect the current profile's settings from user preferences if it is still the same database - // ConnInfo.fixupConnectionCredentials(profile); - // recentConnections[index] = Object.assign({}, profile); - // profilesInRecentConnectionsList.push(index); - // } - // return false; // config profile is also in recent connections - // } - // } - // return true; // config profile is not in recent connections - // }); - - // // Ensure that MRU items which are actually profiles are labeled as such - // let recentConnectionsItems = this.mapToQuickPickItems( - // recentConnections, - // CredentialsQuickPickItemType.Mru, - // ); - // for (let index of profilesInRecentConnectionsList) { - // recentConnectionsItems[index].quickPickItemType = - // CredentialsQuickPickItemType.Profile; - // } - - // connResults = connResults.concat(recentConnectionsItems); - // connResults = connResults.concat( - // this.mapToQuickPickItems( - // profilesInConfiguration, - // CredentialsQuickPickItemType.Profile, - // ), - // ); - - // // Return all connections - // return connResults; - } - - // private getConnectionsFromGlobalState( - // configName: string, - // ): T[] { - // let connections: T[] = []; - // // read from the global state - // let configValues = this._context.globalState.get(configName); - // this.addConnections(connections, configValues); - // return connections; - // } - - // private mapToQuickPickItems( - // connections: IConnectionInfo[], - // itemType: CredentialsQuickPickItemType, - // ): IConnectionCredentialsQuickPickItem[] { - // return connections.map((c) => this.createQuickPickItem(c, itemType)); - // } - private loadProfiles( loadWorkspaceProfiles: boolean, ): IConnectionCredentialsQuickPickItem[] { @@ -753,30 +670,6 @@ export class ConnectionStore { return quickPickItems; } - // private addConnections( - // connections: IConnectionInfo[], - // configValues: IConnectionInfo[], - // ): void { - // if (configValues) { - // for (let index = 0; index < configValues.length; index++) { - // let element = configValues[index]; - // if ( - // element.server && - // element.server.trim() && - // !element.server.trim().startsWith("{{") - // ) { - // let connection = - // ConnInfo.fixupConnectionCredentials(element); - // connections.push(connection); - // } else { - // Utils.logDebug( - // `Missing server name in user preferences connection: index ( ${index} ): ${element.toString()}`, - // ); - // } - // } - // } - // } - private getMaxRecentConnectionsCount(): number { let config = this._vscodeWrapper.getConfiguration( Constants.extensionConfigSectionName, diff --git a/src/views/connectionUI.ts b/src/views/connectionUI.ts index 7d0841dced..9852de231b 100644 --- a/src/views/connectionUI.ts +++ b/src/views/connectionUI.ts @@ -601,43 +601,42 @@ export class ConnectionUI { if (!uri || !this.vscodeWrapper.isEditingSqlFile) { uri = ObjectExplorerUtils.getNodeUriFromProfile(profile); } - return await this.connectionManager - .connect(uri, profile) - .then(async (result) => { - if (result) { - // Success! save it - return await this.saveProfile(profile); - } else { - // Check whether the error was for firewall rule or not - if ( - this.connectionManager.failedUriToFirewallIpMap.has(uri) - ) { - let success = await this.addFirewallRule(uri, profile); - if (success) { - return await this.validateAndSaveProfile(profile); - } - return undefined; - } else if ( - this.connectionManager.failedUriToSSLMap.has(uri) - ) { - // SSL error - let updatedConn = - await this.connectionManager.handleSSLError( - uri, - profile, - ); - if (updatedConn) { - return await this.validateAndSaveProfile( - updatedConn as IConnectionProfile, - ); - } - return undefined; - } else { - // Normal connection error! Let the user try again, prefilling values that they already entered - return await this.promptToRetryAndSaveProfile(profile); - } + + const success = await this.connectionManager.connect(uri, profile); + + if (success) { + // Success! save it + return await this.saveProfile(profile); + } else { + // Check whether the error was for firewall rule or not + if ( + this.connectionManager.failedUriToFirewallIpMap.has(uri) + ) { + let success = await this.addFirewallRule(uri, profile); + if (success) { + return await this.validateAndSaveProfile(profile); } - }); + return undefined; + } else if ( + this.connectionManager.failedUriToSSLMap.has(uri) + ) { + // SSL error + let updatedConn = + await this.connectionManager.handleSSLError( + uri, + profile, + ); + if (updatedConn) { + return await this.validateAndSaveProfile( + updatedConn as IConnectionProfile, + ); + } + return undefined; + } else { + // Normal connection error! Let the user try again, prefilling values that they already entered + return await this.promptToRetryAndSaveProfile(profile); + } + } } /** diff --git a/test/unit/azureController.test.ts b/test/unit/azureController.test.ts index 38603e05ab..555836e778 100644 --- a/test/unit/azureController.test.ts +++ b/test/unit/azureController.test.ts @@ -9,39 +9,39 @@ import * as assert from "assert"; suite("Azure Controller Tests", () => { const currentTime = Date.now() / 1000; - test("isTokenInValid should return true for undefined token", () => { - const actual = AzureController.isTokenInValid(undefined, currentTime); - const expected = true; + test("isTokenValid should return false for undefined token", () => { + const actual = AzureController.isTokenValid(undefined, currentTime); + const expected = false; assert.strictEqual(actual, expected); }); - test("isTokenInValid should return true for empty token", () => { - const actual = AzureController.isTokenInValid("", currentTime); - const expected = true; + test("isTokenValid should return false for empty token", () => { + const actual = AzureController.isTokenValid("", currentTime); + const expected = false; assert.strictEqual(actual, expected); }); - test("isTokenInValid should return true for undefined expiresOn", () => { - const actual = AzureController.isTokenInValid("token", undefined); - const expected = true; + test("isTokenValid should return false for undefined expiresOn", () => { + const actual = AzureController.isTokenValid("token", undefined); + const expected = false; assert.strictEqual(actual, expected); }); - test("isTokenInValid should return true for expired token", () => { - const actual = AzureController.isTokenInValid( + test("isTokenValid should return false for expired token", () => { + const actual = AzureController.isTokenValid( "token", currentTime - 4 * 60, ); - const expected = true; + const expected = false; assert.strictEqual(actual, expected); }); - test("isTokenInValid should return false for valid token", () => { - const actual = AzureController.isTokenInValid( + test("isTokenValid should return true for valid token", () => { + const actual = AzureController.isTokenValid( "token", currentTime + 3 * 60, ); - const expected = false; + const expected = true; assert.strictEqual(actual, expected); }); }); From d38dfbf39466d1811917f3eca91917ec874d1f2d Mon Sep 17 00:00:00 2001 From: "Benjin Dubishar (from Dev Box)" Date: Thu, 13 Feb 2025 11:36:12 -0800 Subject: [PATCH 12/17] spacing --- src/views/connectionUI.ts | 1 + test/unit/connectionUI.test.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/src/views/connectionUI.ts b/src/views/connectionUI.ts index 9852de231b..148e52b0de 100644 --- a/src/views/connectionUI.ts +++ b/src/views/connectionUI.ts @@ -113,6 +113,7 @@ export class ConnectionUI { connectionProfileQuickPick.ignoreFocusOut = ignoreFocusOut; connectionProfileQuickPick.canSelectMany = false; connectionProfileQuickPick.busy = false; + connectionProfileQuickPick.show(); connectionProfileQuickPick.onDidChangeSelection((selection) => { if (selection[0]) { diff --git a/test/unit/connectionUI.test.ts b/test/unit/connectionUI.test.ts index 728adfbe67..4bd8f505d0 100644 --- a/test/unit/connectionUI.test.ts +++ b/test/unit/connectionUI.test.ts @@ -121,6 +121,7 @@ suite("Connection UI tests", () => { quickPickMock .setup((q) => q.onDidChangeSelection) .returns(() => onDidChangeSelectionEventEmitter.event); + // createProfile prompter stub prompter .setup((p) => p.prompt(TypeMoq.It.isAny(), true)) From 1a0e20b004997990e5d20ab035cf28f75f57df6b Mon Sep 17 00:00:00 2001 From: "Benjin Dubishar (from Dev Box)" Date: Thu, 13 Feb 2025 13:16:47 -0800 Subject: [PATCH 13/17] fixing test --- src/connectionconfig/connectionconfig.ts | 5 ++- src/models/connectionProfile.ts | 42 +++++++++++++++--------- src/views/connectionUI.ts | 7 ---- test/unit/connectionUI.test.ts | 16 ++++----- 4 files changed, 36 insertions(+), 34 deletions(-) diff --git a/src/connectionconfig/connectionconfig.ts b/src/connectionconfig/connectionconfig.ts index 309fcc3793..83ae2ca5d9 100644 --- a/src/connectionconfig/connectionconfig.ts +++ b/src/connectionconfig/connectionconfig.ts @@ -10,6 +10,7 @@ import { IConnectionGroup, IConnectionProfile } from "../models/interfaces"; import { IConnectionConfig } from "./iconnectionconfig"; import VscodeWrapper from "../controllers/vscodeWrapper"; import { Deferred } from "../protocol"; +import { ConnectionProfile } from "../models/connectionProfile"; /** * Implements connection profile file storage. @@ -65,9 +66,7 @@ export class ConnectionConfig implements IConnectionConfig { // ensure each profile has an id profiles.forEach((profile) => { - if (!profile.id) { - profile.id = Utils.generateGuid(); - } + ConnectionProfile.addIdIfMissing(profile); }); // ensure each profile is in a group diff --git a/src/models/connectionProfile.ts b/src/models/connectionProfile.ts index 407c050459..e43a4b76c4 100644 --- a/src/models/connectionProfile.ts +++ b/src/models/connectionProfile.ts @@ -75,7 +75,7 @@ export class ConnectionProfile azureController: AzureController, accountStore?: AccountStore, defaultProfileValues?: IConnectionProfile, - ): Promise { + ): Promise { let profile: ConnectionProfile = new ConnectionProfile(); // Ensure all core properties are entered let authOptions: INameValueChoice[] = @@ -198,21 +198,31 @@ export class ConnectionProfile }, ); - return prompter.prompt(questions, true).then(async (answers) => { - if (answers && profile.isValidProfile()) { - sendActionEvent( - TelemetryViews.ConnectionPrompt, - TelemetryActions.CreateConnectionResult, - { - authenticationType: profile.authenticationType, - passwordSaved: profile.savePassword ? "true" : "false", - }, - ); - return profile; - } - // returning undefined to indicate failure to create the profile - return undefined; - }); + const answers = await prompter.prompt(questions, true); + + if (answers && profile.isValidProfile()) { + sendActionEvent( + TelemetryViews.ConnectionPrompt, + TelemetryActions.CreateConnectionResult, + { + authenticationType: profile.authenticationType, + passwordSaved: profile.savePassword ? "true" : "false", + }, + ); + + ConnectionProfile.addIdIfMissing(profile); + + return profile; + } + + // returning undefined to indicate failure to create the profile + return undefined; + } + + public static addIdIfMissing(profile: IConnectionProfile): void { + if (profile && profile.id === undefined) { + profile.id = utils.generateGuid(); + } } // Assumption: having connection string or server + profile name indicates all requirements were met diff --git a/src/views/connectionUI.ts b/src/views/connectionUI.ts index 148e52b0de..c2dccb25a4 100644 --- a/src/views/connectionUI.ts +++ b/src/views/connectionUI.ts @@ -727,10 +727,6 @@ export class ConnectionUI { this._accountStore, ); - if (profile.id === undefined) { - profile.id = Utils.generateGuid(); - } - return profile; } @@ -770,9 +766,6 @@ export class ConnectionUI { this._accountStore, profile, ); - if (newProfile.id === undefined) { - newProfile.id = Utils.generateGuid(); - } return newProfile; } else { diff --git a/test/unit/connectionUI.test.ts b/test/unit/connectionUI.test.ts index 4bd8f505d0..300528f970 100644 --- a/test/unit/connectionUI.test.ts +++ b/test/unit/connectionUI.test.ts @@ -325,7 +325,7 @@ suite("Connection UI tests", () => { ); }); - test("promptForRetryCreateProfile should show an error message and create profile", () => { + test("promptForRetryCreateProfile should show an error message and create profile", async () => { let profile = new ConnectionProfile(); let mockConnection = { connectionString: "test" }; vscodeWrapper @@ -336,13 +336,13 @@ suite("Connection UI tests", () => { prompter .setup((p) => p.prompt(TypeMoq.It.isAny(), true)) .returns(() => Promise.resolve(mockConnection)); - return connectionUI.promptForRetryCreateProfile(profile).then(() => { - vscodeWrapper.verify( - (v) => - v.showErrorMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny()), - TypeMoq.Times.once(), - ); - }); + + await connectionUI.promptForRetryCreateProfile(profile); + + vscodeWrapper.verify( + (v) => v.showErrorMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny()), + TypeMoq.Times.once(), + ); }); test("createProfileWithDifferentCredentials should prompt to recreate connection", () => { From 5b6ea66f8941f2b14f7aa8d27f689168006d0501 Mon Sep 17 00:00:00 2001 From: "Benjin Dubishar (from Dev Box)" Date: Fri, 14 Feb 2025 07:51:18 -0800 Subject: [PATCH 14/17] fix up test --- test/unit/mainController.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/unit/mainController.test.ts b/test/unit/mainController.test.ts index 1732d0333f..8e75317a63 100644 --- a/test/unit/mainController.test.ts +++ b/test/unit/mainController.test.ts @@ -179,11 +179,12 @@ suite("MainController Tests", function () { // Cause event time out (above 10 ms should work) setTimeout(() => { void mainController.onDidCloseTextDocument(document); + try { connectionManager.verify( (x) => x.transferFileConnection( - TypeMoq.It.isAny(), + TypeMoq.It.is((x) => !x.endsWith("settings.json")), // exclude changes to settings.json because MainController setup adds missing mssql connection settings TypeMoq.It.isAny(), ), TypeMoq.Times.never(), From dc4949c64add59bc7781fb37ef68124d36a1534b Mon Sep 17 00:00:00 2001 From: "Benjin Dubishar (from Dev Box)" Date: Fri, 14 Feb 2025 07:55:58 -0800 Subject: [PATCH 15/17] adding typing to compare func --- src/connectionconfig/connectionconfig.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/connectionconfig/connectionconfig.ts b/src/connectionconfig/connectionconfig.ts index 83ae2ca5d9..03a4c39d41 100644 --- a/src/connectionconfig/connectionconfig.ts +++ b/src/connectionconfig/connectionconfig.ts @@ -232,14 +232,17 @@ export class ConnectionConfig implements IConnectionConfig { ); } - /** Sort by profile name if available, otherwise fall back to server name or connection string */ - private compareConnectionProfile(connA, connB) { - let nameA = connA.profileName + /** Compare function for sorting by profile name if available, otherwise fall back to server name or connection string */ + private compareConnectionProfile( + connA: IConnectionProfile, + connB: IConnectionProfile, + ): number { + const nameA = connA.profileName ? connA.profileName : connA.server ? connA.server : connA.connectionString; - let nameB = connB.profileName + const nameB = connB.profileName ? connB.profileName : connB.server ? connB.server From 8be9d2b88dfea1a5d8c804561b0a94756515cc3a Mon Sep 17 00:00:00 2001 From: "Benjin Dubishar (from Dev Box)" Date: Fri, 14 Feb 2025 08:24:36 -0800 Subject: [PATCH 16/17] ignore settings.json in test for both ends of transfer --- test/unit/mainController.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/unit/mainController.test.ts b/test/unit/mainController.test.ts index 8e75317a63..92fe63f89e 100644 --- a/test/unit/mainController.test.ts +++ b/test/unit/mainController.test.ts @@ -184,8 +184,9 @@ suite("MainController Tests", function () { connectionManager.verify( (x) => x.transferFileConnection( - TypeMoq.It.is((x) => !x.endsWith("settings.json")), // exclude changes to settings.json because MainController setup adds missing mssql connection settings - TypeMoq.It.isAny(), + // ignore changes to settings.json because MainController setup adds missing mssql connection settings + TypeMoq.It.is((x) => !x.endsWith("settings.json")), + TypeMoq.It.is((x) => !x.endsWith("settings.json")), ), TypeMoq.Times.never(), ); From 6ca9c8bc76e3a21fec5518b04622c1dbf34348ad Mon Sep 17 00:00:00 2001 From: Benjin Dubishar Date: Tue, 18 Feb 2025 07:37:27 -0800 Subject: [PATCH 17/17] PR cleanup --- src/connectionconfig/connectionconfig.ts | 8 +++----- src/objectExplorer/objectExplorerService.ts | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/connectionconfig/connectionconfig.ts b/src/connectionconfig/connectionconfig.ts index 03a4c39d41..66b4571fcb 100644 --- a/src/connectionconfig/connectionconfig.ts +++ b/src/connectionconfig/connectionconfig.ts @@ -61,16 +61,14 @@ export class ConnectionConfig implements IConnectionConfig { groups.push(rootGroup); } - // Connection profiles + // Clean up connection profiles const profiles: IConnectionProfile[] = this.getProfilesFromSettings(); - // ensure each profile has an id profiles.forEach((profile) => { + // ensure each profile has an id ConnectionProfile.addIdIfMissing(profile); - }); - // ensure each profile is in a group - profiles.forEach((profile) => { + // ensure each profile is in a group if (!profile.groupId) { profile.groupId = rootGroup.id; } diff --git a/src/objectExplorer/objectExplorerService.ts b/src/objectExplorer/objectExplorerService.ts index b809ee1013..1b90022278 100644 --- a/src/objectExplorer/objectExplorerService.ts +++ b/src/objectExplorer/objectExplorerService.ts @@ -703,7 +703,7 @@ export class ObjectExplorerService { if (Utils.isEmpty(password)) { // if password isn't saved if ( - !(connectionProfile) + !(connectionProfile as IConnectionProfile) .savePassword ) { // prompt for password