Revert "Connection Store Refactor (#4632)" (#4671)

This reverts commit 756f77063a.
2026-03-25 22:30:29 -04:00 · 2019-03-22 11:30:20 -07:00
parent 8d5f676039
commit f5c9174c2f
19 changed files with 1008 additions and 897 deletions
--- a/src/sqltest/parts/connection/connectionManagementService.test.ts
+++ b/src/sqltest/parts/connection/connectionManagementService.test.ts
@@ -99,7 +99,7 @@ suite('SQL ConnectionManagementService tests', () => {
 		connectionDialogService.setup(x => x.showDialog(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(none));
 		connectionDialogService.setup(x => x.showDialog(TypeMoq.It.isAny(), TypeMoq.It.isAny(), undefined, TypeMoq.It.isAny())).returns(() => Promise.resolve(none));

-		connectionStore.setup(x => x.addRecentConnection(TypeMoq.It.isAny())).returns(() => Promise.resolve());
+		connectionStore.setup(x => x.addActiveConnection(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve());
 		connectionStore.setup(x => x.saveProfile(TypeMoq.It.isAny())).returns(() => Promise.resolve(connectionProfile));
 		workbenchEditorService.setup(x => x.openEditor(undefined, TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(undefined));
 		connectionStore.setup(x => x.addSavedPassword(TypeMoq.It.is<IConnectionProfile>(
@@ -147,13 +147,16 @@ suite('SQL ConnectionManagementService tests', () => {

 	function createConnectionManagementService(): ConnectionManagementService {
 		let connectionManagementService = new ConnectionManagementService(
+			undefined,
 			connectionStore.object,
+			new TestStorageService(),
 			connectionDialogService.object,
 			undefined,
 			undefined,
 			workbenchEditorService.object,
 			undefined,
 			workspaceConfigurationServiceMock.object,
+			undefined,
 			capabilitiesService,
 			undefined,
 			editorGroupService.object,
--- a/src/sqltest/parts/connection/connectionStore.test.ts
+++ b/src/sqltest/parts/connection/connectionStore.test.ts
@@ -0,0 +1,508 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the Source EULA. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+'use strict';
+
+import * as TypeMoq from 'typemoq';
+import { ConnectionConfig } from 'sql/platform/connection/common/connectionConfig';
+import { IConnectionProfile } from 'sql/platform/connection/common/interfaces';
+import { WorkspaceConfigurationTestService } from 'sqltest/stubs/workspaceConfigurationTestService';
+import * as Constants from 'sql/platform/connection/common/constants';
+import { StorageTestService } from 'sqltest/stubs/storageTestService';
+import { ConnectionStore } from 'sql/platform/connection/common/connectionStore';
+import { CredentialsService } from 'sql/platform/credentials/common/credentialsService';
+import * as assert from 'assert';
+import { Memento } from 'vs/workbench/common/memento';
+import * as azdata from 'azdata';
+import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile';
+import { ConnectionProfileGroup, IConnectionProfileGroup } from 'sql/platform/connection/common/connectionProfileGroup';
+import { ConnectionOptionSpecialType, ServiceOptionType } from 'sql/workbench/api/common/sqlExtHostTypes';
+import { CapabilitiesTestService } from '../../stubs/capabilitiesTestService';
+import { ConnectionProviderProperties } from 'sql/workbench/parts/connection/common/connectionProviderExtension';
+
+suite('SQL ConnectionStore tests', () => {
+	let defaultNamedProfile: IConnectionProfile;
+	let context: TypeMoq.Mock<Memento>;
+	let credentialStore: TypeMoq.Mock<CredentialsService>;
+	let connectionConfig: TypeMoq.Mock<ConnectionConfig>;
+	let workspaceConfigurationServiceMock: TypeMoq.Mock<WorkspaceConfigurationTestService>;
+	let storageServiceMock: TypeMoq.Mock<StorageTestService>;
+	let capabilitiesService: CapabilitiesTestService;
+	let mementoArray: any = [];
+	let maxRecent = 5;
+	let msSQLCapabilities: ConnectionProviderProperties;
+	let provider2Capabilities: ConnectionProviderProperties;
+	let defaultNamedConnectionProfile: ConnectionProfile;
+
+	setup(() => {
+		defaultNamedProfile = Object.assign({}, {
+			connectionName: 'new name',
+			serverName: 'namedServer',
+			databaseName: 'bcd',
+			authenticationType: 'SqlLogin',
+			userName: 'cde',
+			password: 'asdf!@#$',
+			savePassword: true,
+			groupId: '',
+			groupFullName: '',
+			getOptionsKey: undefined,
+			matches: undefined,
+			providerName: 'MSSQL',
+			options: {},
+			saveProfile: true,
+			id: undefined
+		});
+
+		storageServiceMock = TypeMoq.Mock.ofType(StorageTestService);
+
+		let momento = new Memento('ConnectionManagement', storageServiceMock.object);
+		context = TypeMoq.Mock.ofInstance(momento);
+		context.setup(x => x.getMemento(TypeMoq.It.isAny())).returns(() => mementoArray);
+
+		credentialStore = TypeMoq.Mock.ofType(CredentialsService);
+		connectionConfig = TypeMoq.Mock.ofType(ConnectionConfig);
+
+		// setup configuration to return maxRecent for the #MRU items
+
+		let configResult: { [key: string]: any } = {};
+		configResult[Constants.configMaxRecentConnections] = maxRecent;
+
+		workspaceConfigurationServiceMock = TypeMoq.Mock.ofType(WorkspaceConfigurationTestService);
+		workspaceConfigurationServiceMock.setup(x => x.getValue(Constants.sqlConfigSectionName))
+			.returns(() => configResult);
+
+		capabilitiesService = new CapabilitiesTestService();
+		let connectionProvider: azdata.ConnectionOption[] = [
+			{
+				name: 'connectionName',
+				displayName: undefined,
+				description: undefined,
+				groupName: undefined,
+				categoryValues: undefined,
+				defaultValue: undefined,
+				isIdentity: true,
+				isRequired: true,
+				specialValueType: ConnectionOptionSpecialType.connectionName,
+				valueType: ServiceOptionType.string
+			},
+			{
+				name: 'serverName',
+				displayName: undefined,
+				description: undefined,
+				groupName: undefined,
+				categoryValues: undefined,
+				defaultValue: undefined,
+				isIdentity: true,
+				isRequired: true,
+				specialValueType: ConnectionOptionSpecialType.serverName,
+				valueType: ServiceOptionType.string
+			},
+			{
+				name: 'databaseName',
+				displayName: undefined,
+				description: undefined,
+				groupName: undefined,
+				categoryValues: undefined,
+				defaultValue: undefined,
+				isIdentity: true,
+				isRequired: true,
+				specialValueType: ConnectionOptionSpecialType.databaseName,
+				valueType: ServiceOptionType.string
+			},
+			{
+				name: 'userName',
+				displayName: undefined,
+				description: undefined,
+				groupName: undefined,
+				categoryValues: undefined,
+				defaultValue: undefined,
+				isIdentity: true,
+				isRequired: true,
+				specialValueType: ConnectionOptionSpecialType.userName,
+				valueType: ServiceOptionType.string
+			},
+			{
+				name: 'authenticationType',
+				displayName: undefined,
+				description: undefined,
+				groupName: undefined,
+				categoryValues: undefined,
+				defaultValue: undefined,
+				isIdentity: true,
+				isRequired: true,
+				specialValueType: ConnectionOptionSpecialType.authType,
+				valueType: ServiceOptionType.string
+			},
+			{
+				name: 'password',
+				displayName: undefined,
+				description: undefined,
+				groupName: undefined,
+				categoryValues: undefined,
+				defaultValue: undefined,
+				isIdentity: true,
+				isRequired: true,
+				specialValueType: ConnectionOptionSpecialType.password,
+				valueType: ServiceOptionType.string
+			}
+		];
+		msSQLCapabilities = {
+			providerId: 'MSSQL',
+			displayName: 'MSSQL',
+			connectionOptions: connectionProvider
+		};
+
+		provider2Capabilities = {
+			providerId: 'MSSQL',
+			displayName: 'MSSQL',
+			connectionOptions: connectionProvider
+		};
+		capabilitiesService.capabilities['MSSQL'] = { connection: msSQLCapabilities };
+		capabilitiesService.capabilities['Provider2'] = { connection: provider2Capabilities };
+		let groups: IConnectionProfileGroup[] = [
+			{
+				id: 'root',
+				name: 'root',
+				parentId: '',
+				color: '',
+				description: ''
+			},
+			{
+				id: 'g1',
+				name: 'g1',
+				parentId: 'root',
+				color: 'blue',
+				description: 'g1'
+			}
+		];
+		connectionConfig.setup(x => x.getAllGroups()).returns(() => groups);
+
+		defaultNamedConnectionProfile = new ConnectionProfile(capabilitiesService, defaultNamedProfile);
+	});
+
+	test('addActiveConnection should limit recent connection saves to the MaxRecentConnections amount', async () => {
+		// Given 5 is the max # creds
+		let numCreds = 6;
+
+		// setup memento for MRU to return a list we have access to
+		credentialStore.setup(x => x.saveCredential(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
+			.returns(() => Promise.resolve(true));
+
+		// When saving 4 connections
+		// Expect all of them to be saved even if size is limited to 3
+		let connectionStore = new ConnectionStore(context.object, workspaceConfigurationServiceMock.object,
+			credentialStore.object, capabilitiesService, connectionConfig.object);
+		for (let i = 0; i < numCreds; i++) {
+			let cred = Object.assign({}, defaultNamedProfile, { serverName: defaultNamedProfile.serverName + i });
+			let connectionProfile = new ConnectionProfile(capabilitiesService, cred);
+			await connectionStore.addActiveConnection(connectionProfile, true);
+
+			let current = connectionStore.getRecentlyUsedConnections();
+			if (i >= maxRecent) {
+				assert.equal(current.length, maxRecent, `expect only top ${maxRecent} creds to be saved to MRU`);
+			} else {
+				assert.equal(current.length, i + 1, `expect all credentials to be saved to MRU ${current.length}|${i + 1} `);
+			}
+			assert.equal(current[0].serverName, cred.serverName, 'Expect most recently saved item to be first in list');
+			assert.ok(!current[0].password);
+		}
+		credentialStore.verify(x => x.saveCredential(TypeMoq.It.isAny(), TypeMoq.It.isAny()), TypeMoq.Times.exactly(numCreds));
+		let recentConnections = connectionStore.getActiveConnections();
+		assert.equal(numCreds, recentConnections.length, `expect number of active connection ${numCreds}|${recentConnections.length} `);
+	});
+
+	test('addActiveConnection with addToMru as false should not add any recent connections', async () => {
+		// setup memento for MRU to return a list we have access to
+		credentialStore.setup(x => x.saveCredential(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
+			.returns(() => Promise.resolve(true));
+
+		const numCreds = 3;
+		let connectionStore = new ConnectionStore(context.object, workspaceConfigurationServiceMock.object,
+			credentialStore.object, capabilitiesService, connectionConfig.object);
+		connectionStore.clearActiveConnections();
+		connectionStore.clearRecentlyUsed();
+
+		for (let i = 0; i < 3; i++) {
+			let cred = Object.assign({}, defaultNamedProfile, { serverName: defaultNamedProfile.serverName + i });
+			let connectionProfile = new ConnectionProfile(capabilitiesService, cred);
+			await connectionStore.addActiveConnection(connectionProfile, false);
+
+			let recentConnections = connectionStore.getRecentlyUsedConnections();
+			let activeConnections = connectionStore.getActiveConnections();
+			assert.equal(recentConnections.length, 0, `expect no entries to be saved to MRU`);
+			assert.equal(activeConnections.length, i + 1, `expect all credentials to be saved to activeConnections ${activeConnections.length}|${i + 1} `);
+		}
+		credentialStore.verify(x => x.saveCredential(TypeMoq.It.isAny(), TypeMoq.It.isAny()), TypeMoq.Times.exactly(numCreds));
+	});
+
+	test('getRecentlyUsedConnections should return connection for given provider', () => {
+		let connectionStore = new ConnectionStore(context.object, workspaceConfigurationServiceMock.object,
+			credentialStore.object, capabilitiesService, connectionConfig.object);
+		let connections = connectionStore.getRecentlyUsedConnections(['Provider2']);
+		assert.notEqual(connections, undefined);
+		assert.equal(connections.every(c => c.providerName === 'Provider2'), true);
+	});
+
+	test('addActiveConnection should add same connection exactly once', async () => {
+		// setup memento for MRU to return a list we have access to
+		credentialStore.setup(x => x.saveCredential(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
+			.returns(() => Promise.resolve(true));
+
+		// Given we save the same connection twice
+		// Then expect the only 1 instance of that connection to be listed in the MRU
+		let connectionStore = new ConnectionStore(context.object, workspaceConfigurationServiceMock.object,
+			credentialStore.object, capabilitiesService, connectionConfig.object);
+		connectionStore.clearActiveConnections();
+		connectionStore.clearRecentlyUsed();
+		let cred = Object.assign({}, defaultNamedProfile, { serverName: defaultNamedProfile.serverName + 1 });
+		let connectionProfile = new ConnectionProfile(capabilitiesService, cred);
+		await connectionStore.addActiveConnection(defaultNamedConnectionProfile, true);
+		await connectionStore.addActiveConnection(connectionProfile, true);
+		await connectionStore.addActiveConnection(connectionProfile, true);
+
+		let recentConnections = connectionStore.getRecentlyUsedConnections();
+		assert.equal(recentConnections.length, 2, 'expect 2 unique credentials to have been added');
+		assert.equal(recentConnections[0].serverName, cred.serverName, 'Expect most recently saved item to be first in list');
+		assert.ok(!recentConnections[0].password);
+	});
+
+	test('addActiveConnection should save password to credential store', async () => {
+		// Setup credential store to capture credentials sent to it
+		let capturedCreds: any;
+		credentialStore.setup(x => x.saveCredential(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
+			.callback((cred: string, pass: any) => {
+				capturedCreds = {
+					'credentialId': cred,
+					'password': pass
+				};
+			})
+			.returns(() => Promise.resolve(true));
+
+		// Given we save 1 connection with password and multiple other connections without
+		let connectionStore = new ConnectionStore(context.object, workspaceConfigurationServiceMock.object,
+			credentialStore.object, capabilitiesService, connectionConfig.object);
+		connectionStore.clearActiveConnections();
+		connectionStore.clearRecentlyUsed();
+
+		let integratedCred = Object.assign({}, defaultNamedProfile, {
+			serverName: defaultNamedProfile.serverName + 'Integrated',
+			authenticationType: 'Integrated',
+			userName: '',
+			password: ''
+		});
+		let noPwdCred = Object.assign({}, defaultNamedProfile, {
+			serverName: defaultNamedProfile.serverName + 'NoPwd',
+			password: ''
+		});
+		let connectionProfile = new ConnectionProfile(capabilitiesService, defaultNamedProfile);
+
+		let expectedCredCount = 0;
+		expectedCredCount++;
+		// Connection with stored password
+		await connectionStore.addActiveConnection(connectionProfile, true);
+		let recentConnections = connectionStore.getActiveConnections();
+
+		// Then verify that saveCredential was called and correctly stored the password
+		credentialStore.verify(x => x.saveCredential(TypeMoq.It.isAny(), TypeMoq.It.isAny()), TypeMoq.Times.once());
+		assert.strictEqual(capturedCreds.password, defaultNamedProfile.password);
+		let credId: string = capturedCreds.credentialId;
+		assert.ok(credId.includes(ConnectionStore.CRED_PROFILE_USER), 'Expect credential to be marked as an Profile cred');
+		assert.ok(!recentConnections[0].password);
+
+		// Integrated auth
+		expectedCredCount++;
+		let integratedCredConnectionProfile = new ConnectionProfile(capabilitiesService, integratedCred);
+		await connectionStore.addActiveConnection(integratedCredConnectionProfile, true);
+
+		recentConnections = connectionStore.getActiveConnections();
+		// We shouldn't see an increase in the calls to saveCredential, but the MRU should be increased
+		credentialStore.verify(x => x.saveCredential(TypeMoq.It.isAny(), TypeMoq.It.isAny()), TypeMoq.Times.once());
+		assert.equal(recentConnections.length, expectedCredCount, `expect ${expectedCredCount} unique credentials to have been added`);
+
+		// Connection with blank (no) password
+		expectedCredCount++;
+		let noPwdCredConnectionProfile = new ConnectionProfile(capabilitiesService, noPwdCred);
+		await connectionStore.addActiveConnection(noPwdCredConnectionProfile, true);
+
+		recentConnections = connectionStore.getActiveConnections();
+		// We shouldn't see an increase in the calls to saveCredential, but the MRU should be increased
+		credentialStore.verify(x => x.saveCredential(TypeMoq.It.isAny(), TypeMoq.It.isAny()), TypeMoq.Times.once());
+		assert.equal(recentConnections.length, expectedCredCount, `expect ${expectedCredCount} unique credentials to have been added`);
+
+	});
+
+	test('can clear connections list', (done) => {
+		connectionConfig.setup(x => x.getConnections(TypeMoq.It.isAny())).returns(() => []);
+
+		let connectionStore = new ConnectionStore(context.object, workspaceConfigurationServiceMock.object,
+			credentialStore.object, capabilitiesService, connectionConfig.object);
+
+		// When we clear the connections list and get the list of available connection items
+		connectionStore.clearActiveConnections();
+		connectionStore.clearRecentlyUsed();
+		// Expect no connection items
+		let result = connectionStore.getActiveConnections();
+		let expectedCount = 0; // 1 for create connection profile
+		assert.equal(result.length, expectedCount);
+		result = connectionStore.getRecentlyUsedConnections();
+		assert.equal(result.length, expectedCount);
+		// Then test is complete
+		done();
+	});
+
+	test('isPasswordRequired should return true for MSSQL SqlLogin', () => {
+
+		let connectionStore = new ConnectionStore(context.object, workspaceConfigurationServiceMock.object,
+			credentialStore.object, capabilitiesService, connectionConfig.object);
+
+		let expected: boolean = true;
+		let actual = connectionStore.isPasswordRequired(defaultNamedProfile);
+
+		assert.equal(expected, actual);
+	});
+
+	test('isPasswordRequired should return true for MSSQL SqlLogin for connection profile object', () => {
+		let connectionStore = new ConnectionStore(context.object, workspaceConfigurationServiceMock.object,
+			credentialStore.object, capabilitiesService, connectionConfig.object);
+		let connectionProfile = new ConnectionProfile(capabilitiesService, defaultNamedProfile);
+		let expected: boolean = true;
+		let actual = connectionStore.isPasswordRequired(connectionProfile);
+
+		assert.equal(expected, actual);
+	});
+
+	test('isPasswordRequired should return false if the password is not required in capabilities', () => {
+		let providerName: string = 'providername';
+		let connectionProvider = msSQLCapabilities.connectionOptions.map(o => {
+			if (o.name === 'password') {
+				o.isRequired = false;
+			}
+			return o;
+		});
+		let providerCapabilities = {
+			providerId: providerName,
+			displayName: providerName,
+			connectionOptions: connectionProvider
+		};
+
+		capabilitiesService.capabilities[providerName] = { connection: providerCapabilities };
+
+		let connectionStore = new ConnectionStore(context.object, workspaceConfigurationServiceMock.object,
+			credentialStore.object, capabilitiesService, connectionConfig.object);
+		let connectionProfile: IConnectionProfile = Object.assign({}, defaultNamedProfile, { providerName: providerName });
+		let expected: boolean = false;
+		let actual = connectionStore.isPasswordRequired(connectionProfile);
+
+		assert.equal(expected, actual);
+	});
+
+	test('saveProfile should save the password after the profile is saved', done => {
+		let password: string = 'asdf!@#$';
+		let groupId: string = 'group id';
+		let connectionProfile: IConnectionProfile = Object.assign({}, defaultNamedProfile, { password: password });
+		let savedConnection: IConnectionProfile = Object.assign({}, connectionProfile, { groupId: groupId, password: '' });
+		connectionConfig.setup(x => x.addConnection(TypeMoq.It.isAny())).returns(() => Promise.resolve(savedConnection));
+		credentialStore.setup(x => x.saveCredential(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(true));
+
+		let connectionStore = new ConnectionStore(context.object, workspaceConfigurationServiceMock.object,
+			credentialStore.object, capabilitiesService, connectionConfig.object);
+
+		connectionStore.saveProfile(connectionProfile).then(profile => {
+			// add connection should be called with a profile without password
+			connectionConfig.verify(x => x.addConnection(TypeMoq.It.is<IConnectionProfile>(c => c.password === '')), TypeMoq.Times.once());
+			credentialStore.verify(x => x.saveCredential(TypeMoq.It.isAny(), TypeMoq.It.isAny()), TypeMoq.Times.once());
+			assert.equal(profile.password, password, 'The returned profile should still keep the password');
+			assert.equal(profile.groupId, groupId, 'Group id should be set in the profile');
+			done();
+		}).catch(err => {
+			assert.fail(err);
+			done(err);
+		});
+	});
+
+	test('addConnectionToMemento should not add duplicate items', () => {
+		let connectionStore = new ConnectionStore(context.object, workspaceConfigurationServiceMock.object,
+			credentialStore.object, capabilitiesService, connectionConfig.object);
+		let mementoKey = 'RECENT_CONNECTIONS2';
+		connectionStore.clearFromMemento(mementoKey);
+		let connectionProfile: IConnectionProfile = Object.assign({}, defaultNamedProfile);
+		connectionStore.addConnectionToMemento(connectionProfile, mementoKey);
+
+		connectionProfile = Object.assign({}, defaultNamedProfile, { authenticationType: 'Integrated', userName: '' });
+		connectionStore.addConnectionToMemento(connectionProfile, mementoKey);
+
+		let currentList = connectionStore.getConnectionsFromMemento(mementoKey);
+		assert.equal(currentList.length, 2, 'Adding same connection with different auth');
+
+		connectionProfile = Object.assign({}, defaultNamedProfile, { groupFullName: 'new group' });
+		connectionStore.addConnectionToMemento(connectionProfile, mementoKey);
+
+		currentList = connectionStore.getConnectionsFromMemento(mementoKey);
+		assert.equal(currentList.length, 3, 'Adding same connection with different group name');
+
+		connectionProfile = Object.assign({}, defaultNamedProfile,
+			{ groupFullName: defaultNamedProfile.groupFullName.toUpperCase() });
+		connectionStore.addConnectionToMemento(connectionProfile, mementoKey);
+
+		currentList = connectionStore.getConnectionsFromMemento(mementoKey);
+		assert.equal(currentList.length, 3, 'Adding same connection with same group name but uppercase');
+
+		connectionProfile = Object.assign({}, defaultNamedProfile,
+			{ groupFullName: '' });
+		connectionStore.addConnectionToMemento(connectionProfile, mementoKey);
+
+		currentList = connectionStore.getConnectionsFromMemento(mementoKey);
+		assert.equal(currentList.length, 3, 'Adding same connection with group empty string');
+
+		connectionProfile = Object.assign({}, defaultNamedProfile,
+			{ groupFullName: '/' });
+		connectionStore.addConnectionToMemento(connectionProfile, mementoKey);
+
+		currentList = connectionStore.getConnectionsFromMemento(mementoKey);
+		assert.equal(currentList.length, 3, 'Adding same connection with group /');
+	});
+
+	test('getGroupFromId returns undefined when there is no group with the given ID', () => {
+		let connectionStore = new ConnectionStore(context.object, workspaceConfigurationServiceMock.object,
+			credentialStore.object, capabilitiesService, connectionConfig.object);
+		let group = connectionStore.getGroupFromId('invalidId');
+		assert.equal(group, undefined, 'Returned group was not undefined when there was no group with the given ID');
+	});
+
+	test('getGroupFromId returns the group that has the given ID', () => {
+		// Set up the server groups with an additional group that contains a child group
+		let groups: IConnectionProfileGroup[] = connectionConfig.object.getAllGroups();
+		let parentGroupId = 'parentGroup';
+		let childGroupId = 'childGroup';
+		let parentGroup = new ConnectionProfileGroup(parentGroupId, undefined, parentGroupId, '', '');
+		let childGroup = new ConnectionProfileGroup(childGroupId, parentGroup, childGroupId, '', '');
+		groups.push(parentGroup, childGroup);
+		let newConnectionConfig = TypeMoq.Mock.ofType(ConnectionConfig);
+		newConnectionConfig.setup(x => x.getAllGroups()).returns(() => groups);
+		let connectionStore = new ConnectionStore(context.object, workspaceConfigurationServiceMock.object,
+			credentialStore.object, capabilitiesService, newConnectionConfig.object);
+
+		// If I look up the parent group using its ID, then I get back the correct group
+		let actualGroup = connectionStore.getGroupFromId(parentGroupId);
+		assert.equal(actualGroup.id, parentGroupId, 'Did not get the parent group when looking it up with its ID');
+
+		// If I look up the child group using its ID, then I get back the correct group
+		actualGroup = connectionStore.getGroupFromId(childGroupId);
+		assert.equal(actualGroup.id, childGroupId, 'Did not get the child group when looking it up with its ID');
+	});
+
+	test('getProfileWithoutPassword can return the profile without credentials in the password property or options dictionary', () => {
+		let connectionStore = new ConnectionStore(context.object, workspaceConfigurationServiceMock.object,
+			credentialStore.object, capabilitiesService, connectionConfig.object);
+		let profile = Object.assign({}, defaultNamedProfile);
+		profile.options['password'] = profile.password;
+		profile.id = 'testId';
+		let expectedProfile = Object.assign({}, profile);
+		expectedProfile.password = '';
+		expectedProfile.options['password'] = '';
+		expectedProfile = ConnectionProfile.fromIConnectionProfile(capabilitiesService, expectedProfile).toIConnectionProfile();
+		let profileWithoutCredentials = connectionStore.getProfileWithoutPassword(profile);
+		assert.deepEqual(profileWithoutCredentials.toIConnectionProfile(), expectedProfile);
+	});
+});
--- a/src/sqltest/parts/connection/connectionTreeActions.test.ts
+++ b/src/sqltest/parts/connection/connectionTreeActions.test.ts
@@ -18,6 +18,7 @@ import { TestConnectionManagementService } from 'sqltest/stubs/connectionManagem
 import { ErrorMessageServiceStub } from 'sqltest/stubs/errorMessageServiceStub';
 import { InstantiationService } from 'vs/platform/instantiation/common/instantiationService';
 import { ServerTreeView } from 'sql/parts/objectExplorer/viewlet/serverTreeView';
+import * as Constants from 'sql/platform/connection/common/constants';
 import * as  LocalizedConstants from 'sql/parts/connection/common/localizedConstants';
 import { ObjectExplorerService, ObjectExplorerNodeEventArgs } from 'sql/workbench/services/objectExplorer/common/objectExplorerService';
 import { TreeNode } from 'sql/parts/objectExplorer/common/treeNode';
@@ -31,7 +32,6 @@ import { ObjectExplorerActionsContext, ManageConnectionAction } from 'sql/parts/
 import { IConnectionResult, IConnectionParams } from 'sql/platform/connection/common/connectionManagement';
 import { TreeSelectionHandler } from 'sql/parts/objectExplorer/viewlet/treeSelectionHandler';
 import { CapabilitiesTestService } from 'sqltest/stubs/capabilitiesTestService';
-import { UNSAVED_GROUP_ID } from 'sql/platform/connection/common/constants';

 suite('SQL Connection Tree Action tests', () => {
 	let errorMessageService: TypeMoq.Mock<ErrorMessageServiceStub>;
@@ -328,7 +328,7 @@ suite('SQL Connection Tree Action tests', () => {
 			saveProfile: true,
 			id: 'testId'
 		});
-		connection.parent = new ConnectionProfileGroup(LocalizedConstants.unsavedGroupLabel, undefined, UNSAVED_GROUP_ID, undefined, undefined);
+		connection.parent = new ConnectionProfileGroup(LocalizedConstants.unsavedGroupLabel, undefined, Constants.unsavedGroupId, undefined, undefined);
 		let connectionAction: DeleteConnectionAction = new DeleteConnectionAction(DeleteConnectionAction.ID,
 			DeleteConnectionAction.DELETE_CONNECTION_LABEL,
 			connection,
@@ -513,4 +513,4 @@ suite('SQL Connection Tree Action tests', () => {
 		}).then(() => done(), (err) => done(err));
 	});

-});
+});
--- a/src/sqltest/parts/query/editor/queryActions.test.ts
+++ b/src/sqltest/parts/query/editor/queryActions.test.ts
@@ -124,7 +124,7 @@ suite('SQL QueryAction Tests', () => {
 			.returns(() => Promise.resolve(none));

 		// ... Mock "isConnected" in ConnectionManagementService
-		let connectionManagementService = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, {}, connectionDialogService.object);
+		let connectionManagementService = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, {}, {}, new TestStorageService(), connectionDialogService.object);
 		connectionManagementService.callBase = true;
 		connectionManagementService.setup(x => x.isConnected(TypeMoq.It.isAnyString())).returns(() => isConnected);

@@ -182,7 +182,7 @@ suite('SQL QueryAction Tests', () => {
 		queryEditor.setup(x => x.isSelectionEmpty()).returns(() => isSelectionEmpty);

 		// ... Mock "isConnected" in ConnectionManagementService
-		let connectionManagementService = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, {});
+		let connectionManagementService = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, {}, {}, new TestStorageService());
 		connectionManagementService.callBase = true;
 		connectionManagementService.setup(x => x.isConnected(TypeMoq.It.isAnyString())).returns(() => true);

@@ -249,7 +249,7 @@ suite('SQL QueryAction Tests', () => {
 		});

 		// ... Mock "isConnected" in ConnectionManagementService
-		let connectionManagementService = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, {}, connectionDialogService.object);
+		let connectionManagementService = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, {}, {}, new TestStorageService(), connectionDialogService.object);
 		connectionManagementService.callBase = true;
 		connectionManagementService.setup(x => x.isConnected(TypeMoq.It.isAnyString())).returns(() => isConnected);

@@ -315,7 +315,7 @@ suite('SQL QueryAction Tests', () => {
 		let calledCancelQuery: boolean = false;

 		// ... Mock "isConnected" in ConnectionManagementService
-		let connectionManagementService = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, {});
+		let connectionManagementService = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, {}, {}, new TestStorageService());
 		connectionManagementService.setup(x => x.isConnected(TypeMoq.It.isAnyString())).returns(() => isConnected);

 		// ... Mock QueryModelService
@@ -348,7 +348,7 @@ suite('SQL QueryAction Tests', () => {
 		let countCalledDisconnectEditor: number = 0;

 		// ... Mock "isConnected" and "disconnectEditor" in ConnectionManagementService
-		let connectionManagementService = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, {});
+		let connectionManagementService = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, {}, {}, new TestStorageService());
 		connectionManagementService.setup(x => x.isConnected(TypeMoq.It.isAnyString())).returns(() => isConnected);
 		connectionManagementService.setup(x => x.disconnectEditor(TypeMoq.It.isAny())).callback(() => {
 			countCalledDisconnectEditor++;
@@ -387,7 +387,7 @@ suite('SQL QueryAction Tests', () => {
 			.returns(() => Promise.resolve(none));

 		// ... Mock "isConnected" in ConnectionManagementService
-		let connectionManagementService = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, {}, connectionDialogService.object);
+		let connectionManagementService = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, {}, {}, new TestStorageService(), connectionDialogService.object);
 		connectionManagementService.callBase = true;
 		connectionManagementService.setup(x => x.isConnected(TypeMoq.It.isAnyString())).returns(() => isConnected);

@@ -433,7 +433,7 @@ suite('SQL QueryAction Tests', () => {
 			.returns(() => Promise.resolve(none));

 		// ... Mock "isConnected" in ConnectionManagementService
-		let connectionManagementService = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, {}, connectionDialogService.object);
+		let connectionManagementService = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, {}, {}, new TestStorageService(), connectionDialogService.object);
 		connectionManagementService.callBase = true;
 		connectionManagementService.setup(x => x.isConnected(TypeMoq.It.isAnyString())).returns(() => isConnected);

@@ -468,7 +468,7 @@ suite('SQL QueryAction Tests', () => {
 		let databaseName: string = undefined;

 		// ... Mock "isConnected" in ConnectionManagementService
-		let connectionManagementService = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, {});
+		let connectionManagementService = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, {}, {}, new TestStorageService());
 		connectionManagementService.callBase = true;
 		connectionManagementService.setup(x => x.isConnected(TypeMoq.It.isAnyString())).returns(() => isConnected);
 		connectionManagementService.setup(x => x.getConnectionProfile(TypeMoq.It.isAny())).returns(() => <IConnectionProfile>{
@@ -505,7 +505,7 @@ suite('SQL QueryAction Tests', () => {

 		// ... Create mock connection management service
 		let databaseName = 'foobar';
-		let cms = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, {});
+		let cms = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, {}, {}, new TestStorageService());
 		cms.callBase = true;
 		cms.setup(x => x.onConnectionChanged).returns(() => dbChangedEmitter.event);
 		cms.setup(x => x.getConnectionProfile(TypeMoq.It.isAny())).returns(() => <IConnectionProfile>{ databaseName: databaseName });
--- a/src/sqltest/stubs/credentialsTestStubs.ts
+++ b/src/sqltest/stubs/credentialsTestStubs.ts
@@ -36,15 +36,15 @@ export class CredentialsTestProvider implements azdata.CredentialProvider {
 export class CredentialsTestService implements ICredentialsService {
 	_serviceBrand: any;

-	saveCredential(credentialId: string, password: string): Promise<boolean> {
+	saveCredential(credentialId: string, password: string): Thenable<boolean> {
 		return undefined;
 	}

-	readCredential(credentialId: string): Promise<azdata.Credential> {
+	readCredential(credentialId: string): Thenable<azdata.Credential> {
 		return undefined;
 	}

-	deleteCredential(credentialId: string): Promise<boolean> {
+	deleteCredential(credentialId: string): Thenable<boolean> {
 		return undefined;
 	}

@@ -52,4 +52,4 @@ export class CredentialsTestService implements ICredentialsService {
 		return undefined;
 	}

-}
+}