From 41a3c8661cf86c17157227bdd7fe723b05241d31 Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Tue, 31 May 2022 21:32:44 -0700 Subject: [PATCH] Fix GetConnection (#19579) --- .../connection/common/connectionManagement.ts | 2 -- .../browser/mainThreadConnectionManagement.ts | 2 +- .../contrib/assessment/browser/asmtActions.ts | 13 ++++++++++--- .../assessment/test/browser/asmtActions.test.ts | 17 ++++++++++++++--- .../browser/connectionManagementService.ts | 14 -------------- .../browser/connectionManagementService.test.ts | 4 ++-- 6 files changed, 27 insertions(+), 25 deletions(-) diff --git a/src/sql/platform/connection/common/connectionManagement.ts b/src/sql/platform/connection/common/connectionManagement.ts index 2c7f43a17d..9f27e5d65a 100644 --- a/src/sql/platform/connection/common/connectionManagement.ts +++ b/src/sql/platform/connection/common/connectionManagement.ts @@ -321,8 +321,6 @@ export interface IConnectionManagementService { * @returns array of connections */ getConnections(activeConnectionsOnly?: boolean): ConnectionProfile[]; - - getConnection(uri: string): ConnectionProfile; } export enum RunQueryOnConnectionMode { diff --git a/src/sql/workbench/api/browser/mainThreadConnectionManagement.ts b/src/sql/workbench/api/browser/mainThreadConnectionManagement.ts index 031febf268..13049b6feb 100644 --- a/src/sql/workbench/api/browser/mainThreadConnectionManagement.ts +++ b/src/sql/workbench/api/browser/mainThreadConnectionManagement.ts @@ -96,7 +96,7 @@ export class MainThreadConnectionManagement extends Disposable implements MainTh } public $getConnection(uri: string): Thenable { - let profile = this._connectionManagementService.getConnection(uri); + const profile = this._connectionManagementService.getConnectionProfile(uri); if (!profile) { return Promise.resolve(undefined); } diff --git a/src/sql/workbench/contrib/assessment/browser/asmtActions.ts b/src/sql/workbench/contrib/assessment/browser/asmtActions.ts index 27ccbcbf9b..f3b8f2a57f 100644 --- a/src/sql/workbench/contrib/assessment/browser/asmtActions.ts +++ b/src/sql/workbench/contrib/assessment/browser/asmtActions.ts @@ -22,6 +22,8 @@ import * as path from 'vs/base/common/path'; import { HTMLReportBuilder } from 'sql/workbench/contrib/assessment/common/htmlReportGenerator'; import Severity from 'vs/base/common/severity'; import { INotificationService } from 'vs/platform/notification/common/notification'; +import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile'; +import { ICapabilitiesService } from 'sql/platform/capabilities/common/capabilitiesService'; export interface SqlAssessmentResultInfo { result: SqlAssessmentResult; @@ -54,6 +56,7 @@ abstract class AsmtServerAction extends Action { label: string, private asmtType: AssessmentType, @IConnectionManagementService private _connectionManagement: IConnectionManagementService, + @ICapabilitiesService private _capabilitiesService: ICapabilitiesService, @ILogService protected _logService: ILogService, @IAdsTelemetryService protected _telemetryService: IAdsTelemetryService ) { @@ -66,7 +69,7 @@ abstract class AsmtServerAction extends Action { context.component.showProgress(this.asmtType); let serverResults = this.getServerItems(context.ownerUri); let connectionUri: string = this._connectionManagement.getConnectionUriFromId(context.connectionId); - let connection = this._connectionManagement.getConnection(connectionUri); + let connection = this._connectionManagement.getConnectionProfile(connectionUri); let databaseListResult = this._connectionManagement.listDatabases(connectionUri); context.component.showInitialResults(await serverResults, this.asmtType); let dbList = await databaseListResult; @@ -76,7 +79,8 @@ abstract class AsmtServerAction extends Action { break; } let dbName = dbList.databaseNames[nDbName]; - let newUri = await this._connectionManagement.connectIfNotConnected(connection.cloneWithDatabase(dbName).clone()); + const profile = ConnectionProfile.fromIConnectionProfile(this._capabilitiesService, connection); + let newUri = await this._connectionManagement.connectIfNotConnected(profile.cloneWithDatabase(dbName).clone()); this._logService.info(`Database ${dbName} assessment started`); let dbResult = await this.getDatabaseItems(newUri); @@ -101,6 +105,7 @@ export class AsmtServerSelectItemsAction extends AsmtServerAction { constructor( @IConnectionManagementService _connectionManagement: IConnectionManagementService, + @ICapabilitiesService _capabilitiesService: ICapabilitiesService, @ILogService _logService: ILogService, @IAssessmentService private _assessmentService: IAssessmentService, @IAdsTelemetryService _telemetryService: IAdsTelemetryService @@ -108,6 +113,7 @@ export class AsmtServerSelectItemsAction extends AsmtServerAction { super(AsmtServerSelectItemsAction.ID, AsmtServerSelectItemsAction.LABEL, AssessmentType.AvailableRules, _connectionManagement, + _capabilitiesService, _logService, _telemetryService); } @@ -150,11 +156,12 @@ export class AsmtServerInvokeItemsAction extends AsmtServerAction { constructor( @IConnectionManagementService _connectionManagement: IConnectionManagementService, + @ICapabilitiesService _capabilitiesService: ICapabilitiesService, @ILogService _logService: ILogService, @IAssessmentService private _assessmentService: IAssessmentService, @IAdsTelemetryService _telemetryService: IAdsTelemetryService ) { - super(AsmtServerInvokeItemsAction.ID, AsmtServerInvokeItemsAction.LABEL, AssessmentType.InvokeAssessment, _connectionManagement, _logService, _telemetryService); + super(AsmtServerInvokeItemsAction.ID, AsmtServerInvokeItemsAction.LABEL, AssessmentType.InvokeAssessment, _connectionManagement, _capabilitiesService, _logService, _telemetryService); } getServerItems(ownerUri: string): Thenable { this._logService.info(`Requesting server items`); diff --git a/src/sql/workbench/contrib/assessment/test/browser/asmtActions.test.ts b/src/sql/workbench/contrib/assessment/test/browser/asmtActions.test.ts index 1018c8d54b..edc12c119a 100644 --- a/src/sql/workbench/contrib/assessment/test/browser/asmtActions.test.ts +++ b/src/sql/workbench/contrib/assessment/test/browser/asmtActions.test.ts @@ -31,6 +31,7 @@ import { TestFileService, TestEnvironmentService, TestFileDialogService } from ' import { TestNotificationService } from 'vs/platform/notification/test/common/testNotificationService'; import { URI } from 'vs/base/common/uri'; import { IFileService } from 'vs/platform/files/common/files'; +import { TestCapabilitiesService } from 'sql/platform/capabilities/test/common/testCapabilitiesService'; /** * Class to test Assessment Management Actions */ @@ -120,7 +121,7 @@ suite('Assessment Actions', () => { let connectionManagementService = TypeMoq.Mock.ofType(TestConnectionManagementService); connectionManagementService.setup(c => c.listDatabases(TypeMoq.It.isAny())).returns(() => Promise.resolve(dbListResult)); connectionManagementService.setup(c => c.getConnectionUriFromId(TypeMoq.It.isAny())).returns(() => ''); - connectionManagementService.setup(c => c.getConnection(TypeMoq.It.isAny())).returns(() => connectionProfile.object); + connectionManagementService.setup(c => c.getConnectionProfile(TypeMoq.It.isAny())).returns(() => connectionProfile.object); connectionManagementService.setup(c => c.connectIfNotConnected(TypeMoq.It.isAny())).returns(() => Promise.resolve('')); return connectionManagementService; @@ -133,7 +134,12 @@ suite('Assessment Actions', () => { const connectionManagementService = createConnectionManagementService(dbListResult); - const action = new AsmtServerSelectItemsAction(connectionManagementService.object, new NullLogService(), mockAssessmentService.object, new NullAdsTelemetryService()); + const action = new AsmtServerSelectItemsAction( + connectionManagementService.object, + new TestCapabilitiesService(), + new NullLogService(), + mockAssessmentService.object, + new NullAdsTelemetryService()); assert.strictEqual(action.id, AsmtServerSelectItemsAction.ID, 'Get Server Rules id action mismatch'); assert.strictEqual(action.label, AsmtServerSelectItemsAction.LABEL, 'Get Server Rules label action mismatch'); @@ -156,7 +162,12 @@ suite('Assessment Actions', () => { const connectionManagementService = createConnectionManagementService(dbListResult); - const action = new AsmtServerInvokeItemsAction(connectionManagementService.object, new NullLogService(), mockAssessmentService.object, new NullAdsTelemetryService()); + const action = new AsmtServerInvokeItemsAction( + connectionManagementService.object, + new TestCapabilitiesService(), + new NullLogService(), + mockAssessmentService.object, + new NullAdsTelemetryService()); assert.strictEqual(action.id, AsmtServerInvokeItemsAction.ID, 'Invoke Server Assessment id action mismatch'); assert.strictEqual(action.label, AsmtServerInvokeItemsAction.LABEL, 'Invoke Server Assessment label action mismatch'); diff --git a/src/sql/workbench/services/connection/browser/connectionManagementService.ts b/src/sql/workbench/services/connection/browser/connectionManagementService.ts index c43b791126..0ce3d8a639 100644 --- a/src/sql/workbench/services/connection/browser/connectionManagementService.ts +++ b/src/sql/workbench/services/connection/browser/connectionManagementService.ts @@ -1634,20 +1634,6 @@ export class ConnectionManagementService extends Disposable implements IConnecti return connections; } - public getConnection(uri: string): ConnectionProfile { - const connections = this.getActiveConnections(); - if (connections) { - for (let connection of connections) { - let connectionUri = this.getConnectionUriFromId(connection.id); - if (connectionUri === uri) { - return connection; - } - } - } - - return undefined; - } - private getConnectionsInGroup(group: ConnectionProfileGroup): ConnectionProfile[] { const connections = []; if (group) { diff --git a/src/sql/workbench/services/connection/test/browser/connectionManagementService.test.ts b/src/sql/workbench/services/connection/test/browser/connectionManagementService.test.ts index 9cc99f0202..0f77accbfd 100644 --- a/src/sql/workbench/services/connection/test/browser/connectionManagementService.test.ts +++ b/src/sql/workbench/services/connection/test/browser/connectionManagementService.test.ts @@ -781,8 +781,8 @@ suite('SQL ConnectionManagementService tests', () => { await connect(uri, options, true, profile); // invalid uri check. - assert.strictEqual(connectionManagementService.getConnection(badString), undefined); - let returnedProfile = connectionManagementService.getConnection(uri); + assert.strictEqual(connectionManagementService.getConnectionProfile(badString), undefined); + let returnedProfile = connectionManagementService.getConnectionProfile(uri); assert.strictEqual(returnedProfile.groupFullName, profile.groupFullName); assert.strictEqual(returnedProfile.groupId, profile.groupId); });