extensions tslint cleanup/Connection config refactor (#4370)

* various clean ups * formatting * remove linting * formatting * IConfigurationService is even better * messing with connection config tests * update tests * formatting * foramtting * remove unused code * add more tests
2026-02-13 19:48:37 -05:00 · 2019-03-20 11:59:07 -07:00
parent 6cb7153bdd
commit 4472764f3a
27 changed files with 1019 additions and 1319 deletions
--- a/src/sql/platform/connection/common/connectionConfig.ts
+++ b/src/sql/platform/connection/common/connectionConfig.ts
@@ -4,19 +4,16 @@
 *--------------------------------------------------------------------------------------------*/

 'use strict';
-import * as Constants from './constants';
-import * as Utils from './utils';
-import { IConnectionProfile, IConnectionProfileStore } from './interfaces';
-import { IConnectionConfig } from './iconnectionConfig';
-import { ConnectionProfileGroup, IConnectionProfileGroup } from './connectionProfileGroup';
-import { ConfigurationTarget, IConfigurationService } from 'vs/platform/configuration/common/configuration';
-import { ConnectionProfile } from './connectionProfile';
 import { ICapabilitiesService } from 'sql/platform/capabilities/common/capabilitiesService';
-import * as azdata from 'azdata';
-import * as nls from 'vs/nls';
-
+import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile';
+import { ConnectionProfileGroup, IConnectionProfileGroup } from 'sql/platform/connection/common/connectionProfileGroup';
+import * as Constants from 'sql/platform/connection/common/constants';
+import { IConnectionConfig } from 'sql/platform/connection/common/iconnectionConfig';
+import { IConnectionProfile, IConnectionProfileStore } from 'sql/platform/connection/common/interfaces';
+import * as Utils from 'sql/platform/connection/common/utils';
 import { generateUuid } from 'vs/base/common/uuid';
-import { ConfigurationEditingService, IConfigurationValue } from 'vs/workbench/services/configuration/common/configurationEditingService';
+import * as nls from 'vs/nls';
+import { ConfigurationTarget, IConfigurationService } from 'vs/platform/configuration/common/configuration';

 export interface ISaveGroupResult {
 	groups: IConnectionProfileGroup[];
@@ -28,12 +25,8 @@ export interface ISaveGroupResult {
 */
 export class ConnectionConfig implements IConnectionConfig {

-	/**
-	 * Constructor.
-	 */
 	public constructor(
-		private _configurationEditService: ConfigurationEditingService,
-		private _workspaceConfigurationService: IConfigurationService,
+		private configurationService: IConfigurationService,
 		private _capabilitiesService: ICapabilitiesService
 	) { }

@@ -43,15 +36,14 @@ export class ConnectionConfig implements IConnectionConfig {
 	public getAllGroups(): IConnectionProfileGroup[] {

 		let allGroups: IConnectionProfileGroup[] = [];
-		let userGroups = this.getConfiguration(Constants.connectionGroupsArrayName).user as IConnectionProfileGroup[];
-		let workspaceGroups = this.getConfiguration(Constants.connectionGroupsArrayName).workspace as IConnectionProfileGroup[];
+		let { user, workspace } = this.configurationService.inspect<IConnectionProfileGroup[]>(Constants.connectionGroupsArrayName);

-		if (userGroups) {
-			if (workspaceGroups) {
-				userGroups = userGroups.filter(x => workspaceGroups.find(f => this.isSameGroupName(f, x)) === undefined);
-				allGroups = allGroups.concat(workspaceGroups);
+		if (user) {
+			if (workspace) {
+				user = user.filter(x => workspace.find(f => this.isSameGroupName(f, x)) === undefined);
+				allGroups = allGroups.concat(workspace);
 			}
-			allGroups = allGroups.concat(userGroups);
+			allGroups = allGroups.concat(user);
 		}
 		allGroups = allGroups.map(g => {
 			if (g.parentId === '' || !g.parentId) {
@@ -66,39 +58,35 @@ export class ConnectionConfig implements IConnectionConfig {
 	 * Add a new connection to the connection config.
 	 */
 	public addConnection(profile: IConnectionProfile): Promise<IConnectionProfile> {
-		return new Promise<IConnectionProfile>((resolve, reject) => {
-			if (profile.saveProfile) {
-				this.addGroupFromProfile(profile).then(groupId => {
-					let profiles = this._workspaceConfigurationService.inspect<IConnectionProfileStore[]>(Constants.connectionsArrayName).user;
-					if (!profiles) {
-						profiles = [];
-					}
+		if (profile.saveProfile) {
+			return this.addGroupFromProfile(profile).then(groupId => {
+				let profiles = this.configurationService.inspect<IConnectionProfileStore[]>(Constants.connectionsArrayName).user;
+				if (!profiles) {
+					profiles = [];
+				}

-					let connectionProfile = this.getConnectionProfileInstance(profile, groupId);
-					let newProfile = ConnectionProfile.convertToProfileStore(this._capabilitiesService, connectionProfile);
+				let connectionProfile = this.getConnectionProfileInstance(profile, groupId);
+				let newProfile = ConnectionProfile.convertToProfileStore(this._capabilitiesService, connectionProfile);

-					// Remove the profile if already set
-					var sameProfileInList = profiles.find(value => {
-						let providerConnectionProfile = ConnectionProfile.createFromStoredProfile(value, this._capabilitiesService);
-						return providerConnectionProfile.matches(connectionProfile);
-					});
-					if (sameProfileInList) {
-						let profileIndex = profiles.findIndex(value => value === sameProfileInList);
-						newProfile.id = sameProfileInList.id;
-						connectionProfile.id = sameProfileInList.id;
-						profiles[profileIndex] = newProfile;
-					} else {
-						profiles.push(newProfile);
-					}
-
-					this.writeConfiguration(Constants.connectionsArrayName, profiles).then(() => {
-						resolve(connectionProfile);
-					}).catch(err => {
-						reject(err);
-					});
+				// Remove the profile if already set
+				var sameProfileInList = profiles.find(value => {
+					let providerConnectionProfile = ConnectionProfile.createFromStoredProfile(value, this._capabilitiesService);
+					return providerConnectionProfile.matches(connectionProfile);
 				});
-			}
-		});
+				if (sameProfileInList) {
+					let profileIndex = profiles.findIndex(value => value === sameProfileInList);
+					newProfile.id = sameProfileInList.id;
+					connectionProfile.id = sameProfileInList.id;
+					profiles[profileIndex] = newProfile;
+				} else {
+					profiles.push(newProfile);
+				}
+
+				return this.configurationService.updateValue(Constants.connectionsArrayName, profiles, ConfigurationTarget.USER).then(() => connectionProfile);
+			});
+		} else {
+			return Promise.resolve(profile);
+		}
 	}

 	private getConnectionProfileInstance(profile: IConnectionProfile, groupId: string): ConnectionProfile {
@@ -115,21 +103,15 @@ export class ConnectionConfig implements IConnectionConfig {
 	 * @param groupName
 	 */
 	public addGroupFromProfile(profile: IConnectionProfile): Promise<string> {
-		return new Promise<string>((resolve, reject) => {
-			if (profile.groupId && profile.groupId !== Utils.defaultGroupId) {
-				resolve(profile.groupId);
-			} else {
-				let groups = this._workspaceConfigurationService.inspect<IConnectionProfileGroup[]>(Constants.connectionGroupsArrayName).user;
-				let result = this.saveGroup(groups, profile.groupFullName, undefined, undefined);
-				groups = result.groups;
+		if (profile.groupId && profile.groupId !== Utils.defaultGroupId) {
+			return Promise.resolve(profile.groupId);
+		} else {
+			let groups = this.configurationService.inspect<IConnectionProfileGroup[]>(Constants.connectionGroupsArrayName).user;
+			let result = this.saveGroup(groups, profile.groupFullName, undefined, undefined);
+			groups = result.groups;

-				this.writeConfiguration(Constants.connectionGroupsArrayName, groups).then(() => {
-					resolve(result.newGroupId);
-				}).catch(err => {
-					reject(err);
-				});
-			}
-		});
+			return this.configurationService.updateValue(Constants.connectionGroupsArrayName, groups, ConfigurationTarget.USER).then(() => result.newGroupId);
+		}
 	}

 	/**
@@ -137,41 +119,35 @@ export class ConnectionConfig implements IConnectionConfig {
 	 * @param groupName
 	 */
 	public addGroup(profileGroup: IConnectionProfileGroup): Promise<string> {
-		return new Promise<string>((resolve, reject) => {
-			if (profileGroup.id) {
-				resolve(profileGroup.id);
+		if (profileGroup.id) {
+			return Promise.resolve(profileGroup.id);
+		} else {
+			let groups = this.configurationService.inspect<IConnectionProfileGroup[]>(Constants.connectionGroupsArrayName).user;
+			let sameNameGroup = groups ? groups.find(group => group.name === profileGroup.name) : undefined;
+			if (sameNameGroup) {
+				let errMessage: string = nls.localize('invalidServerName', "A server group with the same name already exists.");
+				return Promise.reject(errMessage);
 			} else {
-				let groups = this._workspaceConfigurationService.inspect<IConnectionProfileGroup[]>(Constants.connectionGroupsArrayName).user;
-				let sameNameGroup = groups ? groups.find(group => group.name === profileGroup.name) : undefined;
-				if (sameNameGroup) {
-					let errMessage: string = nls.localize('invalidServerName', "A server group with the same name already exists.");
-					reject(errMessage);
-				} else {
-					let result = this.saveGroup(groups, profileGroup.name, profileGroup.color, profileGroup.description);
-					groups = result.groups;
+				let result = this.saveGroup(groups, profileGroup.name, profileGroup.color, profileGroup.description);
+				groups = result.groups;

-					this.writeConfiguration(Constants.connectionGroupsArrayName, groups).then(() => {
-						resolve(result.newGroupId);
-					}).catch(err => {
-						reject(err);
-					});
-				}
+				return this.configurationService.updateValue(Constants.connectionGroupsArrayName, groups, ConfigurationTarget.USER).then(() => result.newGroupId);
 			}
-		});
+		}
 	}

 	private getConnectionProfilesForTarget(configTarget: ConfigurationTarget): IConnectionProfileStore[] {
-		let configs = this.getConfiguration(Constants.connectionsArrayName);
+		let configs = this.configurationService.inspect<IConnectionProfileStore[]>(Constants.connectionsArrayName);
 		let profiles: IConnectionProfileStore[];
 		if (configs) {
 			if (configTarget === ConfigurationTarget.USER) {
-				profiles = <IConnectionProfileStore[]>configs.user;
+				profiles = configs.user;
 			} else if (configTarget === ConfigurationTarget.WORKSPACE) {
-				profiles = <IConnectionProfileStore[]>configs.workspace;
+				profiles = configs.workspace;
 			}
 			if (profiles) {
 				if (this.fixConnectionIds(profiles)) {
-					this.writeConfiguration(Constants.connectionsArrayName, profiles, configTarget);
+					this.configurationService.updateValue(Constants.connectionsArrayName, profiles, configTarget);
 				}
 			} else {
 				profiles = [];
@@ -185,7 +161,7 @@ export class ConnectionConfig implements IConnectionConfig {
 	 * Replace duplicate ids with new ones. Sets id for the profiles without id
 	 * @param profiles
 	 */
-	public fixConnectionIds(profiles: IConnectionProfileStore[]): boolean {
+	private fixConnectionIds(profiles: IConnectionProfileStore[]): boolean {
 		let idsCache: { [label: string]: boolean } = {};
 		let changed: boolean = false;
 		for (var index = 0; index < profiles.length; index++) {
@@ -239,7 +215,7 @@ export class ConnectionConfig implements IConnectionConfig {
 	 */
 	public deleteConnection(profile: ConnectionProfile): Promise<void> {
 		// Get all connections in the settings
-		let profiles = this._workspaceConfigurationService.inspect<IConnectionProfileStore[]>(Constants.connectionsArrayName).user;
+		let profiles = this.configurationService.inspect<IConnectionProfileStore[]>(Constants.connectionsArrayName).user;
 		// Remove the profile from the connections
 		profiles = profiles.filter(value => {
 			let providerConnectionProfile = ConnectionProfile.createFromStoredProfile(value, this._capabilitiesService);
@@ -247,7 +223,7 @@ export class ConnectionConfig implements IConnectionConfig {
 		});

 		// Write connections back to settings
-		return this.writeConfiguration(Constants.connectionsArrayName, profiles);
+		return this.configurationService.updateValue(Constants.connectionsArrayName, profiles, ConfigurationTarget.USER);
 	}

 	/**
@@ -260,7 +236,7 @@ export class ConnectionConfig implements IConnectionConfig {
 		// Add selected group to subgroups list
 		subgroups.push(group);
 		// Get all connections in the settings
-		let profiles = this._workspaceConfigurationService.inspect<IConnectionProfileStore[]>(Constants.connectionsArrayName).user;
+		let profiles = this.configurationService.inspect<IConnectionProfileStore[]>(Constants.connectionsArrayName).user;
 		// Remove the profiles from the connections
 		profiles = profiles.filter(value => {
 			let providerConnectionProfile = ConnectionProfile.createFromStoredProfile(value, this._capabilitiesService);
@@ -268,32 +244,29 @@ export class ConnectionConfig implements IConnectionConfig {
 		});

 		// Get all groups in the settings
-		let groups = this._workspaceConfigurationService.inspect<IConnectionProfileGroup[]>(Constants.connectionGroupsArrayName).user;
+		let groups = this.configurationService.inspect<IConnectionProfileGroup[]>(Constants.connectionGroupsArrayName).user;
 		// Remove subgroups in the settings
 		groups = groups.filter((grp) => {
 			return !subgroups.some((item) => item.id === grp.id);
 		});
-		return new Promise<void>((resolve, reject) => {
-			this.writeConfiguration(Constants.connectionsArrayName, profiles).then(() => {
-				this.writeConfiguration(Constants.connectionGroupsArrayName, groups).then(() => {
-					resolve();
-				}).catch(() => reject());
-			}).catch(() => reject());
-		});
+		return Promise.all([
+			this.configurationService.updateValue(Constants.connectionsArrayName, profiles, ConfigurationTarget.USER),
+			this.configurationService.updateValue(Constants.connectionGroupsArrayName, groups, ConfigurationTarget.USER)
+		]).then(() => Promise.resolve());
 	}

 	/**
 	 * Moves the source group under the target group.
 	 */
 	public changeGroupIdForConnectionGroup(source: ConnectionProfileGroup, target: ConnectionProfileGroup): Promise<void> {
-		let groups = this._workspaceConfigurationService.inspect<IConnectionProfileGroup[]>(Constants.connectionGroupsArrayName).user;
+		let groups = this.configurationService.inspect<IConnectionProfileGroup[]>(Constants.connectionGroupsArrayName).user;
 		groups = groups.map(g => {
 			if (g.id === source.id) {
 				g.parentId = target.id;
 			}
 			return g;
 		});
-		return this.writeConfiguration(Constants.connectionGroupsArrayName, groups);
+		return this.configurationService.updateValue(Constants.connectionGroupsArrayName, groups, ConfigurationTarget.USER);
 	}

 	/**
@@ -310,53 +283,40 @@ export class ConnectionConfig implements IConnectionConfig {
 	 * Moves the connection under the target group with the new ID.
 	 */
 	private changeGroupIdForConnectionInSettings(profile: ConnectionProfile, newGroupID: string, target: ConfigurationTarget = ConfigurationTarget.USER): Promise<void> {
-		return new Promise<void>((resolve, reject) => {
-			let profiles = target === ConfigurationTarget.USER ? this._workspaceConfigurationService.inspect<IConnectionProfileStore[]>(Constants.connectionsArrayName).user :
-				this._workspaceConfigurationService.inspect<IConnectionProfileStore[]>(Constants.connectionsArrayName).workspace;
-			if (profiles) {
-				if (profile.parent && profile.parent.id === Constants.unsavedGroupId) {
-					profile.groupId = newGroupID;
-					profiles.push(ConnectionProfile.convertToProfileStore(this._capabilitiesService, profile));
-				} else {
-					profiles.forEach((value) => {
-						let configProf = ConnectionProfile.createFromStoredProfile(value, this._capabilitiesService);
-						if (configProf.getOptionsKey() === profile.getOptionsKey()) {
-							value.groupId = newGroupID;
-						}
-					});
-				}
-
-				this.writeConfiguration(Constants.connectionsArrayName, profiles, target).then(result => {
-					resolve();
-				}).catch(error => {
-					reject(error);
-				});
+		let profiles = target === ConfigurationTarget.USER ? this.configurationService.inspect<IConnectionProfileStore[]>(Constants.connectionsArrayName).user :
+			this.configurationService.inspect<IConnectionProfileStore[]>(Constants.connectionsArrayName).workspace;
+		if (profiles) {
+			if (profile.parent && profile.parent.id === Constants.unsavedGroupId) {
+				profile.groupId = newGroupID;
+				profiles.push(ConnectionProfile.convertToProfileStore(this._capabilitiesService, profile));
 			} else {
-				resolve();
+				profiles.forEach((value) => {
+					let configProf = ConnectionProfile.createFromStoredProfile(value, this._capabilitiesService);
+					if (configProf.getOptionsKey() === profile.getOptionsKey()) {
+						value.groupId = newGroupID;
+					}
+				});
 			}
-		});
+
+			return this.configurationService.updateValue(Constants.connectionsArrayName, profiles, target);
+		} else {
+			return Promise.resolve();
+		}
 	}

 	/**
 	 * Moves the connection under the target group with the new ID.
 	 */
 	public changeGroupIdForConnection(profile: ConnectionProfile, newGroupID: string): Promise<void> {
-		return new Promise<void>((resolve, reject) => {
-			if (!this.canChangeConnectionConfig(profile, newGroupID)) {
-				// Same connection already exists in this group
-				reject('Same connection already exists in the group');
-			} else {
-				this.changeGroupIdForConnectionInSettings(profile, newGroupID, ConfigurationTarget.USER).then(result1 => {
-					this.changeGroupIdForConnectionInSettings(profile, newGroupID, ConfigurationTarget.WORKSPACE).then(result2 => {
-						resolve();
-					}).catch(error2 => {
-						reject(error2);
-					});
-				}).catch(error1 => {
-					reject(error1);
-				});
-			}
-		});
+		if (!this.canChangeConnectionConfig(profile, newGroupID)) {
+			// Same connection already exists in this group
+			return Promise.reject('Same connection already exists in the group');
+		} else {
+			return Promise.all([
+				this.changeGroupIdForConnectionInSettings(profile, newGroupID, ConfigurationTarget.USER),
+				this.changeGroupIdForConnectionInSettings(profile, newGroupID, ConfigurationTarget.WORKSPACE)
+			]).then(() => Promise.resolve());
+		}
 	}

 	public saveGroup(groups: IConnectionProfileGroup[], groupFullName: string, color: string, description: string): ISaveGroupResult {
@@ -367,7 +327,7 @@ export class ConnectionConfig implements IConnectionConfig {
 	}

 	public editGroup(source: ConnectionProfileGroup): Promise<void> {
-		let groups = this._workspaceConfigurationService.inspect<IConnectionProfileGroup[]>(Constants.connectionGroupsArrayName).user;
+		let groups = this.configurationService.inspect<IConnectionProfileGroup[]>(Constants.connectionGroupsArrayName).user;
 		let sameNameGroup = groups ? groups.find(group => group.name === source.name && group.id !== source.id) : undefined;
 		if (sameNameGroup) {
 			let errMessage: string = nls.localize('invalidServerName', "A server group with the same name already exists.");
@@ -382,7 +342,7 @@ export class ConnectionConfig implements IConnectionConfig {
 			}
 			return g;
 		});
-		return this.writeConfiguration(Constants.connectionGroupsArrayName, groups);
+		return this.configurationService.updateValue(Constants.connectionGroupsArrayName, groups, ConfigurationTarget.USER);
 	}

 	private isSameGroupName(group1: IConnectionProfileGroup, group2: IConnectionProfileGroup): boolean {
@@ -441,40 +401,4 @@ export class ConnectionConfig implements IConnectionConfig {
 		};
 		return groupResult;
 	}
-
-	/**
-	 * Get all profiles from the parsed settings file.
-	 * This is public for testing only.
-	 * @param parsedSettingsFile an object representing the parsed contents of the settings file.
-	 * @returns the set of connection profiles found in the parsed settings file.
-	 */
-	private getConfiguration(key: string): any {
-		let configs: any;
-		configs = this._workspaceConfigurationService.inspect<IConnectionProfileStore[] | IConnectionProfileGroup[] | azdata.DataProtocolServerCapabilities[]>(key);
-		return configs;
-	}
-
-	/**
-	 * Replace existing profiles in the settings file with a new set of profiles.
-	 * @param parsedSettingsFile an object representing the parsed contents of the settings file.
-	 * @param profiles the set of profiles to insert into the settings file.
-	 */
-	private writeConfiguration(
-		key: string,
-		profiles: IConnectionProfileStore[] | IConnectionProfileGroup[] | azdata.DataProtocolServerCapabilities[],
-		target: ConfigurationTarget = ConfigurationTarget.USER): Promise<void> {
-		return new Promise<void>((resolve, reject) => {
-			let configValue: IConfigurationValue = {
-				key: key,
-				value: profiles
-			};
-			this._configurationEditService.writeConfiguration(target, configValue).then(result => {
-				this._workspaceConfigurationService.reloadConfiguration().then(() => {
-					resolve();
-				});
-			}, (error => {
-				reject(error);
-			}));
-		});
-	}
 }
--- a/src/sql/platform/connection/common/connectionInfo.ts
+++ b/src/sql/platform/connection/common/connectionInfo.ts
@@ -5,17 +5,17 @@

 'use strict';

-import Interfaces = require('./interfaces');
+import * as interfaces from 'sql/platform/connection/common/interfaces';

 /**
 * Sets sensible defaults for key connection properties, especially
 * if connection to Azure
 *
 * @export connectionInfo/fixupConnectionCredentials
- * @param {Interfaces.IConnectionCredentials} connCreds connection to be fixed up
- * @returns {Interfaces.IConnectionCredentials} the updated connection
+ * @param {interfaces.IConnectionCredentials} connCreds connection to be fixed up
+ * @returns {interfaces.IConnectionCredentials} the updated connection
 */
-export function fixupConnectionCredentials(connCreds: Interfaces.IConnectionProfile): Interfaces.IConnectionProfile {
+export function fixupConnectionCredentials(connCreds: interfaces.IConnectionProfile): interfaces.IConnectionProfile {
 	if (!connCreds.serverName) {
 		connCreds.serverName = '';
 	}
@@ -33,4 +33,3 @@ export function fixupConnectionCredentials(connCreds: Interfaces.IConnectionProf
 	}
 	return connCreds;
 }
-
--- a/src/sql/platform/connection/common/connectionManagement.ts
+++ b/src/sql/platform/connection/common/connectionManagement.ts
@@ -11,7 +11,7 @@ import * as azdata from 'azdata';
 import { IConnectionProfileGroup, ConnectionProfileGroup } from 'sql/platform/connection/common/connectionProfileGroup';
 import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile';
 import { IConnectionProfile } from 'sql/platform/connection/common/interfaces';
-import { ConnectionManagementInfo } from './connectionManagementInfo';
+import { ConnectionManagementInfo } from 'sql/platform/connection/common/connectionManagementInfo';
 import { IServerGroupDialogCallbacks } from 'sql/platform/serverGroup/common/serverGroupController';

 export const VIEWLET_ID = 'workbench.view.connections';
@@ -331,4 +331,4 @@ export enum TaskStatus {
 export interface IConnectionParams {
 	connectionUri: string;
 	connectionProfile: IConnectionProfile;
-}
+}
--- a/src/sql/platform/connection/common/connectionManagementInfo.ts
+++ b/src/sql/platform/connection/common/connectionManagementInfo.ts
@@ -70,4 +70,4 @@ export class ConnectionManagementInfo {
 	 * Owner uri assigned to the connection
 	 */
 	public ownerUri: string;
-}
+}
--- a/src/sql/platform/connection/common/connectionManagementService.ts
+++ b/src/sql/platform/connection/common/connectionManagementService.ts
@@ -56,7 +56,6 @@ import { IQuickInputService } from 'vs/platform/quickinput/common/quickInput';
 import { IConnectionDialogService } from 'sql/workbench/services/connection/common/connectionDialogService';
 import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService';
 import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
-import { ConfigurationEditingService } from 'vs/workbench/services/configuration/common/configurationEditingService';

 export class ConnectionManagementService extends Disposable implements IConnectionManagementService {

@@ -77,8 +76,6 @@ export class ConnectionManagementService extends Disposable implements IConnecti
 	private _onLanguageFlavorChanged = new Emitter<azdata.DidChangeLanguageFlavorParams>();
 	private _connectionGlobalStatus = new ConnectionGlobalStatus(this._statusBarService);

-	private _configurationEditService: ConfigurationEditingService;
-
 	constructor(
 		private _connectionMemento: Memento,
 		private _connectionStore: ConnectionStore,
@@ -99,17 +96,14 @@ export class ConnectionManagementService extends Disposable implements IConnecti
 		@IAccountManagementService private _accountManagementService: IAccountManagementService
 	) {
 		super();
-		if (this._instantiationService) {
-			this._configurationEditService = this._instantiationService.createInstance(ConfigurationEditingService);
-		}

 		// _connectionMemento and _connectionStore are in constructor to enable this class to be more testable
 		if (!this._connectionMemento) {
 			this._connectionMemento = new Memento('ConnectionManagement', _storageService);
 		}
 		if (!this._connectionStore) {
-			this._connectionStore = new ConnectionStore(_storageService, this._connectionMemento,
-				this._configurationEditService, this._configurationService, this._credentialsService, this._capabilitiesService);
+			this._connectionStore = new ConnectionStore(this._connectionMemento,
+				this._configurationService, this._credentialsService, this._capabilitiesService);
 		}

 		// Register Statusbar item
--- a/src/sql/platform/connection/common/connectionProfile.ts
+++ b/src/sql/platform/connection/common/connectionProfile.ts
@@ -5,7 +5,7 @@

 'use strict';

-import { ConnectionProfileGroup } from './connectionProfileGroup';
+import { ConnectionProfileGroup } from 'sql/platform/connection/common/connectionProfileGroup';
 import * as azdata from 'azdata';
 import { ProviderConnectionInfo } from 'sql/platform/connection/common/providerConnectionInfo';
 import * as interfaces from 'sql/platform/connection/common/interfaces';
--- a/src/sql/platform/connection/common/connectionProfileGroup.ts
+++ b/src/sql/platform/connection/common/connectionProfileGroup.ts
@@ -5,7 +5,7 @@

 'use strict';

-import { ConnectionProfile } from './connectionProfile';
+import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile';

 export interface IConnectionProfileGroup {
 	id: string;
@@ -216,4 +216,4 @@ export class ConnectionProfileGroup implements IConnectionProfileGroup {
 		}
 		return subgroups;
 	}
-}
+}
--- a/src/sql/platform/connection/common/connectionStatusManager.ts
+++ b/src/sql/platform/connection/common/connectionStatusManager.ts
@@ -4,22 +4,20 @@
 *--------------------------------------------------------------------------------------------*/
 'use strict';

-import { ConnectionManagementInfo } from './connectionManagementInfo';
+import { ConnectionManagementInfo } from 'sql/platform/connection/common/connectionManagementInfo';
 import { ICapabilitiesService } from 'sql/platform/capabilities/common/capabilitiesService';
 import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile';
-import { IConnectionProfile } from './interfaces';
-import * as Utils from './utils';
+import { IConnectionProfile } from 'sql/platform/connection/common/interfaces';
+import * as Utils from 'sql/platform/connection/common/utils';
 import * as azdata from 'azdata';
 import { StopWatch } from 'vs/base/common/stopwatch';

 export class ConnectionStatusManager {

 	private _connections: { [id: string]: ConnectionManagementInfo };
-	private _providerCapabilitiesMap: { [providerName: string]: azdata.DataProtocolServerCapabilities };

 	constructor( @ICapabilitiesService private _capabilitiesService: ICapabilitiesService) {
 		this._connections = {};
-		this._providerCapabilitiesMap = {};
 	}

 	public findConnection(uri: string): ConnectionManagementInfo {
@@ -207,4 +205,4 @@ export class ConnectionStatusManager {
 		}
 		return profiles;
 	}
-}
+}
--- a/src/sql/platform/connection/common/connectionStore.ts
+++ b/src/sql/platform/connection/common/connectionStore.ts
@@ -5,19 +5,18 @@

 'use strict';

-import * as Constants from './constants';
-import * as ConnInfo from './connectionInfo';
-import { ConnectionProfile } from '../common/connectionProfile';
+import * as Constants from 'sql/platform/connection/common/constants';
+import * as ConnInfo from 'sql/platform/connection/common/connectionInfo';
+import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile';
 import { IConnectionProfile } from 'sql/platform/connection/common/interfaces';
 import { ICredentialsService } from 'sql/platform/credentials/common/credentialsService';
-import { IConnectionConfig } from './iconnectionConfig';
-import { ConnectionConfig } from './connectionConfig';
+import { IConnectionConfig } from 'sql/platform/connection/common/iconnectionConfig';
+import { ConnectionConfig } from 'sql/platform/connection/common/connectionConfig';
 import { Memento } from 'vs/workbench/common/memento';
-import { IStorageService, StorageScope } from 'vs/platform/storage/common/storage';
+import { StorageScope } from 'vs/platform/storage/common/storage';
 import { ConnectionProfileGroup, IConnectionProfileGroup } from './connectionProfileGroup';
 import { ICapabilitiesService } from 'sql/platform/capabilities/common/capabilitiesService';
 import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
-import { ConfigurationEditingService } from 'vs/workbench/services/configuration/common/configurationEditingService';

 const MAX_CONNECTIONS_DEFAULT = 25;

@@ -33,9 +32,7 @@ export class ConnectionStore {
 	private _groupFullNameToIdMap: { [groupId: string]: string };

 	constructor(
-		private _storageService: IStorageService,
 		private _context: Memento,
-		private _configurationEditService: ConfigurationEditingService,
 		private _configurationService: IConfigurationService,
 		private _credentialService: ICredentialsService,
 		private _capabilitiesService: ICapabilitiesService,
@@ -47,8 +44,7 @@ export class ConnectionStore {
 		this._groupIdToFullNameMap = {};
 		this._groupFullNameToIdMap = {};
 		if (!this._connectionConfig) {
-			this._connectionConfig = new ConnectionConfig(this._configurationEditService,
-				this._configurationService, this._capabilitiesService);
+			this._connectionConfig = new ConnectionConfig(this._configurationService, this._capabilitiesService);
 		}
 	}

--- a/src/sql/platform/connection/common/constants.ts
+++ b/src/sql/platform/connection/common/constants.ts
@@ -34,4 +34,3 @@ export const passwordChars = '***************';
 export const sqlLogin = 'SqlLogin';
 export const integrated = 'Integrated';
 export const azureMFA = 'AzureMFA';
-
--- a/src/sql/platform/connection/common/iconnectionConfig.ts
+++ b/src/sql/platform/connection/common/iconnectionConfig.ts
@@ -5,10 +5,9 @@

 'use strict';

-import { IConnectionProfile } from './interfaces';
-import { IConnectionProfileGroup, ConnectionProfileGroup } from './connectionProfileGroup';
-import { ConnectionProfile } from './connectionProfile';
-import * as azdata from 'azdata';
+import { IConnectionProfile } from 'sql/platform/connection/common/interfaces';
+import { IConnectionProfileGroup, ConnectionProfileGroup } from 'sql/platform/connection/common/connectionProfileGroup';
+import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile';

 /**
 * Interface for a configuration file that stores connection profiles.
--- a/src/sql/platform/connection/common/utils.ts
+++ b/src/sql/platform/connection/common/utils.ts
@@ -5,9 +5,9 @@

 'use strict';

-import { IConnectionProfile } from './interfaces';
-import { ConnectionProfile } from './connectionProfile';
-import { ConnectionProfileGroup } from './connectionProfileGroup';
+import { IConnectionProfile } from 'sql/platform/connection/common/interfaces';
+import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile';
+import { ConnectionProfileGroup } from 'sql/platform/connection/common/connectionProfileGroup';

 // CONSTANTS //////////////////////////////////////////////////////////////////////////////////////
 const msInH = 3.6e6;
--- a/src/sql/platform/connection/test/common/connectionConfig.test.ts
+++ b/src/sql/platform/connection/test/common/connectionConfig.test.ts
@@ -0,0 +1,737 @@
+/*---------------------------------------------------------------------------------------------
+ *  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, ISaveGroupResult } from 'sql/platform/connection/common/connectionConfig';
+import { IConnectionProfile, IConnectionProfileStore } from 'sql/platform/connection/common/interfaces';
+import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile';
+import { ConfigurationTarget } from 'vs/platform/configuration/common/configuration';
+import * as Constants from 'sql/platform/connection/common/constants';
+import { IConnectionProfileGroup, ConnectionProfileGroup } from 'sql/platform/connection/common/connectionProfileGroup';
+import * as assert from 'assert';
+import { ProviderFeatures, ICapabilitiesService } from 'sql/platform/capabilities/common/capabilitiesService';
+import * as azdata from 'azdata';
+import { Emitter } from 'vs/base/common/event';
+import { ConnectionOptionSpecialType, ServiceOptionType } from 'sql/workbench/api/common/sqlExtHostTypes';
+import { CapabilitiesTestService } from 'sqltest/stubs/capabilitiesTestService';
+import { TestConfigurationService } from 'sql/platform/connection/test/common/testConfigurationService';
+import { deepFreeze, deepClone } from 'vs/base/common/objects';
+import { isUndefinedOrNull } from 'vs/base/common/types';
+import { ConnectionManagementInfo } from 'sql/platform/connection/common/connectionManagementInfo';
+
+suite('ConnectionConfig', () => {
+	let capabilitiesService: TypeMoq.Mock<ICapabilitiesService>;
+	let msSQLCapabilities: ProviderFeatures;
+	let capabilities: ProviderFeatures[];
+	let onCapabilitiesRegistered = new Emitter<ProviderFeatures>();
+
+	const testGroups = deepFreeze([
+		{
+			name: 'g1',
+			id: 'g1',
+			parentId: 'ROOT',
+			color: 'pink',
+			description: 'g1'
+		},
+		{
+			name: 'g1-1',
+			id: 'g1-1',
+			parentId: 'g1',
+			color: 'blue',
+			description: 'g1-1'
+		},
+		{
+			name: 'ROOT',
+			id: 'ROOT',
+			parentId: '',
+			color: 'white',
+			description: 'ROOT'
+		},
+		{
+			name: 'g2',
+			id: 'g2',
+			parentId: 'ROOT',
+			color: 'green',
+			description: 'g2'
+		},
+		{
+			name: 'g2-1',
+			id: 'g2-1',
+			parentId: 'g2',
+			color: 'yellow',
+			description: 'g2'
+		},
+		{
+			name: 'g3',
+			id: 'g3',
+			parentId: '',
+			color: 'orange',
+			description: 'g3'
+		},
+		{
+			name: 'g3-1',
+			id: 'g3-1',
+			parentId: 'g3',
+			color: 'purple',
+			description: 'g3-1'
+		}
+	]);
+
+	const testConnections: IConnectionProfileStore[] = deepFreeze([
+		{
+			options: {
+				serverName: 'server1',
+				databaseName: 'database',
+				userName: 'user',
+				password: 'password',
+				authenticationType: ''
+			},
+			providerName: 'MSSQL',
+			groupId: 'test',
+			savePassword: true,
+			id: 'server1'
+		},
+		{
+			options: {
+				serverName: 'server2',
+				databaseName: 'database',
+				userName: 'user',
+				password: 'password',
+				authenticationType: ''
+			},
+			providerName: 'MSSQL',
+			groupId: 'test',
+			savePassword: true,
+			id: 'server2'
+		},
+		{
+			options: {
+				serverName: 'server3',
+				databaseName: 'database',
+				userName: 'user',
+				password: 'password',
+				authenticationType: ''
+			},
+			providerName: 'MSSQL',
+			groupId: 'g3',
+			savePassword: true,
+			id: 'server3'
+		}
+	]);
+
+	setup(() => {
+		capabilitiesService = TypeMoq.Mock.ofType(CapabilitiesTestService);
+		capabilities = [];
+		let connectionProvider: azdata.ConnectionProviderOptions = {
+			options: [
+				{
+					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 = {
+			connection: {
+				providerId: 'MSSQL',
+				displayName: 'MSSQL',
+				connectionOptions: connectionProvider.options
+			}
+		};
+		capabilities.push(msSQLCapabilities);
+
+		capabilitiesService.setup(x => x.getCapabilities('MSSQL')).returns(() => msSQLCapabilities);
+		(capabilitiesService.object as any).onCapabilitiesRegistered = onCapabilitiesRegistered.event;
+	});
+
+	function groupsAreEqual(groups1: IConnectionProfileGroup[], groups2: IConnectionProfileGroup[]): boolean {
+		if (!groups1 && !groups2) {
+			return true;
+		} else if ((!groups1 && groups2 && groups2.length === 0) || (!groups2 && groups1 && groups1.length === 0)) {
+			return true;
+		}
+
+		if (groups1.length !== groups2.length) {
+			return false;
+		}
+
+		for (let group of groups1) {
+			let g2 = groups2.find(g => g.name === group.name);
+			// if we couldn't find the group it means they must not be equal
+			if (!g2) {
+				return false;
+			}
+
+			// weird way to verify that each group appears the same number of times in each array
+			let result = groupsAreEqual(groups1.filter(a => a.parentId === group.id), groups2.filter(b => b.parentId === g2.id));
+			if (!result) {
+				return false;
+			}
+
+		}
+
+		return true;
+	}
+
+	test('getAllGroups should merge user and workspace settings correctly', () => {
+		let configurationService = new TestConfigurationService();
+		configurationService.updateValue(Constants.connectionGroupsArrayName, deepClone(testGroups).slice(0, 3), ConfigurationTarget.USER);
+		// we intentionally overlap these values with the expectation that the function to should return each group once
+		configurationService.updateValue(Constants.connectionGroupsArrayName, deepClone(testGroups).slice(2, testGroups.length), ConfigurationTarget.WORKSPACE);
+
+		let config = new ConnectionConfig(configurationService, capabilitiesService.object);
+		let allGroups = config.getAllGroups();
+
+		assert.equal(allGroups.length, testGroups.length, 'did not meet the expected length');
+		assert.ok(groupsAreEqual(allGroups, testGroups), 'the groups returned did not match expectation');
+	});
+
+	test('addConnection should add the new profile to user settings if does not exist', async () => {
+		let newProfile: IConnectionProfile = {
+			serverName: 'new server',
+			databaseName: 'database',
+			userName: 'user',
+			password: 'password',
+			authenticationType: '',
+			savePassword: true,
+			groupFullName: undefined,
+			groupId: undefined,
+			getOptionsKey: undefined,
+			matches: undefined,
+			providerName: 'MSSQL',
+			options: {},
+			saveProfile: true,
+			id: undefined,
+			connectionName: undefined
+		};
+
+		let configurationService = new TestConfigurationService();
+		configurationService.updateValue(Constants.connectionGroupsArrayName, deepClone(testGroups), ConfigurationTarget.USER);
+		configurationService.updateValue(Constants.connectionsArrayName, deepClone(testConnections), ConfigurationTarget.USER);
+		let connectionProfile = new ConnectionProfile(capabilitiesService.object, newProfile);
+		connectionProfile.options['databaseDisplayName'] = 'database';
+		let config = new ConnectionConfig(configurationService, capabilitiesService.object);
+		let savedConnectionProfile = await config.addConnection(connectionProfile);
+
+		assert.ok(!isUndefinedOrNull(savedConnectionProfile.id));
+		assert.equal(configurationService.inspect<IConnectionProfileStore[]>(Constants.connectionsArrayName).user.length, testConnections.length + 1);
+	});
+
+	test('addConnection should not add the new profile to user settings if already exists', async () => {
+		let existingConnection = testConnections[0];
+		let newProfile: IConnectionProfile = {
+			serverName: existingConnection.options['serverName'],
+			databaseName: existingConnection.options['databaseName'],
+			userName: existingConnection.options['userName'],
+			password: existingConnection.options['password'],
+			authenticationType: existingConnection.options['authenticationType'],
+			groupId: existingConnection.groupId,
+			savePassword: true,
+			groupFullName: undefined,
+			getOptionsKey: undefined,
+			matches: undefined,
+			providerName: 'MSSQL',
+			options: {},
+			saveProfile: true,
+			id: undefined,
+			connectionName: undefined
+		};
+
+		let configurationService = new TestConfigurationService();
+		configurationService.updateValue(Constants.connectionGroupsArrayName, deepClone(testGroups), ConfigurationTarget.USER);
+		configurationService.updateValue(Constants.connectionsArrayName, deepClone(testConnections), ConfigurationTarget.USER);
+
+		let connectionProfile = new ConnectionProfile(capabilitiesService.object, newProfile);
+		connectionProfile.options['databaseDisplayName'] = existingConnection.options['databaseName'];
+
+		let config = new ConnectionConfig(configurationService, capabilitiesService.object);
+		let savedConnectionProfile = await config.addConnection(connectionProfile);
+
+		assert.equal(savedConnectionProfile.id, existingConnection.id);
+		assert.equal(configurationService.inspect<IConnectionProfileStore[]>(Constants.connectionsArrayName).user.length, testConnections.length);
+	});
+
+	test('addConnection should add the new group to user settings if does not exist', async () => {
+		let newProfile: IConnectionProfile = {
+			serverName: 'new server',
+			databaseName: 'database',
+			userName: 'user',
+			password: 'password',
+			authenticationType: '',
+			savePassword: true,
+			groupFullName: 'g2/g2-2',
+			groupId: undefined,
+			getOptionsKey: undefined,
+			matches: undefined,
+			providerName: 'MSSQL',
+			options: {},
+			saveProfile: true,
+			id: undefined,
+			connectionName: undefined
+		};
+
+		let configurationService = new TestConfigurationService();
+		configurationService.updateValue(Constants.connectionGroupsArrayName, deepClone(testGroups), ConfigurationTarget.USER);
+		configurationService.updateValue(Constants.connectionsArrayName, deepClone(testConnections), ConfigurationTarget.USER);
+
+		let connectionProfile = new ConnectionProfile(capabilitiesService.object, newProfile);
+		let config = new ConnectionConfig(configurationService, capabilitiesService.object);
+		await config.addConnection(connectionProfile);
+
+		assert.equal(configurationService.inspect<IConnectionProfileStore[]>(Constants.connectionsArrayName).user.length, testConnections.length + 1);
+		assert.equal(configurationService.inspect<IConnectionProfileStore[]>(Constants.connectionGroupsArrayName).user.length, testGroups.length + 1);
+	});
+
+	test('getConnections should return connections from user and workspace settings given getWorkspaceConnections set to true', () => {
+		let configurationService = new TestConfigurationService();
+		configurationService.updateValue(Constants.connectionsArrayName, deepClone(testConnections).slice(0, 1), ConfigurationTarget.USER);
+		configurationService.updateValue(Constants.connectionsArrayName, deepClone(testConnections).slice(1, testConnections.length), ConfigurationTarget.WORKSPACE);
+
+		let config = new ConnectionConfig(configurationService, capabilitiesService.object);
+		let allConnections = config.getConnections(true);
+		assert.equal(allConnections.length, testConnections.length);
+	});
+
+	test('getConnections should return connections from user settings given getWorkspaceConnections set to false', () => {
+		let configurationService = new TestConfigurationService();
+		configurationService.updateValue(Constants.connectionsArrayName, deepClone(testConnections).slice(0, 2), ConfigurationTarget.USER);
+		configurationService.updateValue(Constants.connectionsArrayName, deepClone(testConnections).slice(2, testConnections.length), ConfigurationTarget.WORKSPACE);
+
+		let config = new ConnectionConfig(configurationService, capabilitiesService.object);
+		let allConnections = config.getConnections(false);
+		assert.equal(allConnections.length, 2);
+	});
+
+	test('getConnections should return connections with a valid id', () => {
+		let workspaceConnections = deepClone(testConnections).map(c => {
+			c.id = c.options['serverName'];
+			return c;
+		});
+		let userConnections = deepClone(testConnections).map(c => {
+			c.id = undefined;
+			return c;
+		});
+		let configurationService = new TestConfigurationService();
+		configurationService.updateValue(Constants.connectionsArrayName, userConnections, ConfigurationTarget.USER);
+		configurationService.updateValue(Constants.connectionsArrayName, workspaceConnections, ConfigurationTarget.WORKSPACE);
+
+		let config = new ConnectionConfig(configurationService, capabilitiesService.object);
+		let allConnections = config.getConnections(false);
+		assert.equal(allConnections.length, testConnections.length);
+		allConnections.forEach(connection => {
+			let userConnection = testConnections.find(u => u.options['serverName'] === connection.serverName);
+			if (userConnection !== undefined) {
+				assert.notEqual(connection.id, connection.getOptionsKey());
+				assert.ok(!isUndefinedOrNull(connection.id));
+			} else {
+				let workspaceConnection = workspaceConnections.find(u => u.options['serverName'] === connection.serverName);
+				assert.notEqual(connection.id, connection.getOptionsKey());
+				assert.equal(workspaceConnection.id, connection.id);
+			}
+		});
+	});
+
+	test('saveGroup should save the new groups to tree and return the id of the last group name', () => {
+		let config = new ConnectionConfig(undefined, undefined);
+		let groups: IConnectionProfileGroup[] = deepClone(testGroups);
+		let newGroups: string = 'ROOT/g1/g1-1/new-group/new-group2';
+		let color: string = 'red';
+
+		let result: ISaveGroupResult = config.saveGroup(groups, newGroups, color, newGroups);
+		assert.ok(!isUndefinedOrNull(result));
+		assert.equal(result.groups.length, testGroups.length + 2, 'The result groups length is invalid');
+		let newGroup = result.groups.find(g => g.name === 'new-group2');
+		assert.equal(result.newGroupId, newGroup.id, 'The groups id is invalid');
+	});
+
+	test('saveGroup should only add the groups that are not in the tree', () => {
+		let config = new ConnectionConfig(undefined, undefined);
+		let groups: IConnectionProfileGroup[] = deepClone(testGroups);
+		let newGroups: string = 'ROOT/g2/g2-5';
+		let color: string = 'red';
+
+		let result: ISaveGroupResult = config.saveGroup(groups, newGroups, color, newGroups);
+		assert.ok(!isUndefinedOrNull(result));
+		assert.equal(result.groups.length, testGroups.length + 1, 'The result groups length is invalid');
+		let newGroup = result.groups.find(g => g.name === 'g2-5');
+		assert.equal(result.newGroupId, newGroup.id, 'The groups id is invalid');
+	});
+
+	test('saveGroup should not add any new group if tree already has all the groups in the full path', () => {
+		let config = new ConnectionConfig(undefined, undefined);
+		let groups: IConnectionProfileGroup[] = deepClone(testGroups);
+		let newGroups: string = 'ROOT/g2/g2-1';
+		let color: string = 'red';
+
+		let result: ISaveGroupResult = config.saveGroup(groups, newGroups, color, newGroups);
+		assert.ok(!isUndefinedOrNull(result));
+		assert.equal(result.groups.length, testGroups.length, 'The result groups length is invalid');
+		let newGroup = result.groups.find(g => g.name === 'g2-1');
+		assert.equal(result.newGroupId, newGroup.id, 'The groups id is invalid');
+	});
+
+	test('deleteConnection should remove the connection from config', async () => {
+		let newProfile: IConnectionProfile = {
+			serverName: 'server3',
+			databaseName: 'database',
+			userName: 'user',
+			password: 'password',
+			authenticationType: '',
+			savePassword: true,
+			groupFullName: 'g3',
+			groupId: 'g3',
+			getOptionsKey: undefined,
+			matches: undefined,
+			providerName: 'MSSQL',
+			options: {},
+			saveProfile: true,
+			id: undefined,
+			connectionName: undefined
+		};
+		let configurationService = new TestConfigurationService();
+		configurationService.updateValue(Constants.connectionsArrayName, deepClone(testConnections), ConfigurationTarget.USER);
+
+		let connectionProfile = new ConnectionProfile(capabilitiesService.object, newProfile);
+		connectionProfile.options['databaseDisplayName'] = 'database';
+
+		let config = new ConnectionConfig(configurationService, capabilitiesService.object);
+		await config.deleteConnection(connectionProfile);
+
+		assert.equal(configurationService.inspect<IConnectionProfileStore[]>(Constants.connectionsArrayName).user.length, testConnections.length - 1);
+	});
+
+	test('deleteConnectionGroup should remove the children connections and subgroups from config', async () => {
+		let newProfile: IConnectionProfile = {
+			serverName: 'server3',
+			databaseName: 'database',
+			userName: 'user',
+			password: 'password',
+			authenticationType: '',
+			savePassword: true,
+			groupFullName: 'g3',
+			groupId: 'g3',
+			getOptionsKey: undefined,
+			matches: undefined,
+			providerName: 'MSSQL',
+			options: {},
+			saveProfile: true,
+			id: undefined,
+			connectionName: undefined
+		};
+		let configurationService = new TestConfigurationService();
+		configurationService.updateValue(Constants.connectionsArrayName, deepClone(testConnections), ConfigurationTarget.USER);
+		configurationService.updateValue(Constants.connectionGroupsArrayName, deepClone(testGroups), ConfigurationTarget.USER);
+
+		let connectionProfile = new ConnectionProfile(capabilitiesService.object, newProfile);
+		connectionProfile.options['databaseDisplayName'] = 'database';
+
+		let connectionProfileGroup = new ConnectionProfileGroup('g3', undefined, 'g3', undefined, undefined);
+		let childGroup = new ConnectionProfileGroup('g3-1', connectionProfileGroup, 'g3-1', undefined, undefined);
+		connectionProfileGroup.addGroups([childGroup]);
+		connectionProfileGroup.addConnections([connectionProfile]);
+
+		let config = new ConnectionConfig(configurationService, capabilitiesService.object);
+		await config.deleteGroup(connectionProfileGroup);
+
+		assert.equal(configurationService.inspect<IConnectionProfileStore[]>(Constants.connectionsArrayName).user.length, testConnections.length - 1);
+		assert.equal(configurationService.inspect<IConnectionProfileGroup[]>(Constants.connectionGroupsArrayName).user.length, testGroups.length - 2);
+	});
+
+	test('deleteConnection should not throw error for connection not in config', async () => {
+		let newProfile: IConnectionProfile = {
+			serverName: 'connectionNotThere',
+			databaseName: 'database',
+			userName: 'user',
+			password: 'password',
+			authenticationType: '',
+			savePassword: true,
+			groupFullName: 'g3',
+			groupId: 'newid',
+			getOptionsKey: undefined,
+			matches: undefined,
+			providerName: 'MSSQL',
+			options: {},
+			saveProfile: true,
+			id: undefined,
+			connectionName: undefined
+		};
+		let configurationService = new TestConfigurationService();
+		configurationService.updateValue(Constants.connectionsArrayName, deepClone(testConnections), ConfigurationTarget.USER);
+
+		let connectionProfile = new ConnectionProfile(capabilitiesService.object, newProfile);
+		let config = new ConnectionConfig(configurationService, capabilitiesService.object);
+		await config.deleteConnection(connectionProfile);
+
+		assert.equal(configurationService.inspect<IConnectionProfileStore[]>(Constants.connectionsArrayName).user.length, testConnections.length);
+	});
+
+	test('renameGroup should change group name', async () => {
+		let configurationService = new TestConfigurationService();
+		configurationService.updateValue(Constants.connectionGroupsArrayName, deepClone(testGroups), ConfigurationTarget.USER);
+
+		let connectionProfileGroup = new ConnectionProfileGroup('g-renamed', undefined, 'g2', undefined, undefined);
+		let config = new ConnectionConfig(configurationService, capabilitiesService.object);
+		await config.editGroup(connectionProfileGroup);
+
+		let editedGroups = configurationService.inspect<IConnectionProfileGroup[]>(Constants.connectionGroupsArrayName).user;
+
+		assert.equal(editedGroups.length, testGroups.length);
+		let editedGroup = editedGroups.find(group => group.id === 'g2');
+		assert.ok(!isUndefinedOrNull(editedGroup));
+		assert.equal(editedGroup.name, 'g-renamed');
+	});
+
+	test('edit group should throw if there is a confliction', async () => {
+		let configurationService = new TestConfigurationService();
+		configurationService.updateValue(Constants.connectionGroupsArrayName, deepClone(testGroups), ConfigurationTarget.USER);
+
+		let sameNameGroup = new ConnectionProfileGroup('g3', undefined, 'g2', undefined, undefined);
+		let config = new ConnectionConfig(configurationService, capabilitiesService.object);
+
+		try {
+			await config.editGroup(sameNameGroup);
+			assert.fail();
+		} catch (e) {
+			let groups = configurationService.inspect<IConnectionProfileGroup[]>(Constants.connectionGroupsArrayName).user;
+			let originalGroup = groups.find(g => g.id === 'g2');
+			assert.ok(!isUndefinedOrNull(originalGroup));
+			assert.equal(originalGroup.name, 'g2');
+		}
+	});
+
+	test('change group(parent) for connection group', async () => {
+		let configurationService = new TestConfigurationService();
+		configurationService.updateValue(Constants.connectionGroupsArrayName, deepClone(testGroups), ConfigurationTarget.USER);
+
+		let sourceProfileGroup = new ConnectionProfileGroup('g2', undefined, 'g2', undefined, undefined);
+		let targetProfileGroup = new ConnectionProfileGroup('g3', undefined, 'g3', undefined, undefined);
+		let config = new ConnectionConfig(configurationService, capabilitiesService.object);
+		await config.changeGroupIdForConnectionGroup(sourceProfileGroup, targetProfileGroup);
+
+		let editedGroups = configurationService.inspect<IConnectionProfileGroup[]>(Constants.connectionGroupsArrayName).user;
+
+		assert.equal(editedGroups.length, testGroups.length);
+		let editedGroup = editedGroups.find(group => group.id === 'g2');
+		assert.ok(!isUndefinedOrNull(editedGroup));
+		assert.equal(editedGroup.parentId, 'g3');
+	});
+
+
+	test('change group for connection with conflict should throw', async () => {
+		let changingProfile: IConnectionProfile = {
+			serverName: 'server3',
+			databaseName: 'database',
+			userName: 'user',
+			password: 'password',
+			authenticationType: '',
+			savePassword: true,
+			groupFullName: 'g3',
+			groupId: 'g3',
+			getOptionsKey: () => { return 'connectionId'; },
+			matches: undefined,
+			providerName: 'MSSQL',
+			options: {},
+			saveProfile: true,
+			id: 'server3-2',
+			connectionName: undefined
+		};
+		let existingProfile = ConnectionProfile.convertToProfileStore(capabilitiesService.object, {
+			serverName: 'server3',
+			databaseName: 'database',
+			userName: 'user',
+			password: 'password',
+			authenticationType: '',
+			savePassword: true,
+			groupFullName: 'test',
+			groupId: 'test',
+			getOptionsKey: () => { return 'connectionId'; },
+			matches: undefined,
+			providerName: 'MSSQL',
+			options: {},
+			saveProfile: true,
+			id: 'server3',
+			connectionName: undefined
+		});
+
+		let _testConnections = deepClone(testConnections).concat([existingProfile, changingProfile]);
+
+		let configurationService = new TestConfigurationService();
+		configurationService.updateValue(Constants.connectionsArrayName, _testConnections, ConfigurationTarget.USER);
+
+		let connectionProfile = new ConnectionProfile(capabilitiesService.object, changingProfile);
+
+		let config = new ConnectionConfig(configurationService, capabilitiesService.object);
+		try {
+			await config.changeGroupIdForConnection(connectionProfile, 'test');
+			assert.fail();
+		} catch (e) {
+			let editedConnections = configurationService.inspect<IConnectionProfileStore[]>(Constants.connectionsArrayName).user;
+			// two
+			assert.equal(editedConnections.length, _testConnections.length);
+			let editedConnection = editedConnections.find(con => con.id === 'server3-2');
+			assert.ok(!isUndefinedOrNull(editedConnection));
+			assert.equal(editedConnection.groupId, 'g3');
+		}
+	});
+
+	test('change group(parent) for connection', async () => {
+		let newProfile: IConnectionProfile = {
+			serverName: 'server3',
+			databaseName: 'database',
+			userName: 'user',
+			password: 'password',
+			authenticationType: '',
+			savePassword: true,
+			groupFullName: 'g3',
+			groupId: 'g3',
+			getOptionsKey: () => { return 'connectionId'; },
+			matches: undefined,
+			providerName: 'MSSQL',
+			options: {},
+			saveProfile: true,
+			id: 'server3',
+			connectionName: undefined
+		};
+
+		let configurationService = new TestConfigurationService();
+		configurationService.updateValue(Constants.connectionsArrayName, deepClone(testConnections), ConfigurationTarget.USER);
+
+		let connectionProfile = new ConnectionProfile(capabilitiesService.object, newProfile);
+		let newId = 'newid';
+
+		let config = new ConnectionConfig(configurationService, capabilitiesService.object);
+		await config.changeGroupIdForConnection(connectionProfile, newId);
+
+		let editedConnections = configurationService.inspect<IConnectionProfileStore[]>(Constants.connectionsArrayName).user;
+		assert.equal(editedConnections.length, testConnections.length);
+		let editedConnection = editedConnections.find(con => con.id === 'server3');
+		assert.ok(!isUndefinedOrNull(editedConnection));
+		assert.equal(editedConnection.groupId, 'newid');
+	});
+
+	test('addConnection should not move the connection when editing', async () => {
+		// Set up the connection config
+		let configurationService = new TestConfigurationService();
+		configurationService.updateValue(Constants.connectionsArrayName, deepClone(testConnections), ConfigurationTarget.USER);
+		configurationService.updateValue(Constants.connectionGroupsArrayName, deepClone(testGroups), ConfigurationTarget.USER);
+		let config = new ConnectionConfig(configurationService, capabilitiesService.object);
+
+		// Clone a connection and modify an option
+		const connectionIndex = 1;
+		const optionKey = 'testOption';
+		const optionValue = 'testValue';
+		let allConnections = config.getConnections(false);
+		let oldLength = allConnections.length;
+		let connectionToEdit = allConnections[connectionIndex].clone();
+		connectionToEdit.options[optionKey] = optionValue;
+		await config.addConnection(connectionToEdit);
+
+		// Get the connection and verify that it is in the same place and has been updated
+		let newConnections = config.getConnections(false);
+		assert.equal(newConnections.length, oldLength);
+		let editedConnection = newConnections[connectionIndex];
+		assert.equal(editedConnection.getOptionsKey(), connectionToEdit.getOptionsKey());
+		assert.equal(editedConnection.options[optionKey], optionValue);
+	});
+
+	test('addgroup works', async () => {
+		let newGroup: IConnectionProfileGroup = {
+			id: undefined,
+			parentId: undefined,
+			name: 'new group',
+			color: 'red',
+			description: 'new group'
+		};
+		let configurationService = new TestConfigurationService();
+		configurationService.updateValue(Constants.connectionGroupsArrayName, deepClone(testGroups), ConfigurationTarget.USER);
+
+		let config = new ConnectionConfig(configurationService, capabilitiesService.object);
+
+		await config.addGroup(newGroup);
+
+		let editGroups = configurationService.inspect<IConnectionProfileGroup[]>(Constants.connectionGroupsArrayName).user;
+
+		assert.equal(editGroups.length, testGroups.length + 1);
+	});
+
+	test('addGroup rejects if group name already exists', async () => {
+		let existingGroupName: IConnectionProfileGroup = {
+			id: undefined,
+			parentId: undefined,
+			name: 'g2',
+			color: 'red',
+			description: 'new group'
+		};
+		let configurationService = new TestConfigurationService();
+		configurationService.updateValue(Constants.connectionGroupsArrayName, deepClone(testGroups), ConfigurationTarget.USER);
+
+		const config = new ConnectionConfig(configurationService, capabilitiesService.object);
+		try {
+			await config.addGroup(existingGroupName);
+			assert.fail();
+		} catch (e) {
+			let editGroups = configurationService.inspect<IConnectionProfileGroup[]>(Constants.connectionGroupsArrayName).user;
+
+			assert.equal(editGroups.length, testGroups.length);
+		}
+	});
+});
--- a/src/sql/platform/connection/test/common/testConfigurationService.ts
+++ b/src/sql/platform/connection/test/common/testConfigurationService.ts
@@ -0,0 +1,79 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the Source EULA. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import { getConfigurationKeys, IConfigurationOverrides, IConfigurationService, getConfigurationValue, isConfigurationOverrides, ConfigurationTarget } from 'vs/platform/configuration/common/configuration';
+
+export class TestConfigurationService implements IConfigurationService {
+	public _serviceBrand: any;
+
+	private configuration = {
+		user: {},
+		workspace: {}
+	};
+
+	public reloadConfiguration<T>(): Promise<T> {
+		return Promise.resolve(this.getValue());
+	}
+
+	public getValue(arg1?: any, arg2?: any): any {
+		let configuration;
+		configuration = configuration ? configuration : this.configuration;
+		if (arg1 && typeof arg1 === 'string') {
+			return getConfigurationValue(configuration, arg1);
+		}
+		return configuration;
+	}
+
+	public updateValue(key: string, value: any, target?: any): Promise<void> {
+		let _target = (target as ConfigurationTarget) === ConfigurationTarget.USER ? 'user' : 'workspace';
+		let keyArray = key.split('.');
+		let targetObject = this.configuration[_target];
+		for (let i = 0; i < keyArray.length; i++) {
+			if (i === keyArray.length - 1) {
+				targetObject[keyArray[i]] = value;
+			} else {
+				if (!targetObject[keyArray[i]]) {
+					targetObject[keyArray[i]] = {};
+				}
+				targetObject = targetObject[keyArray[i]];
+			}
+		}
+		return Promise.resolve(void 0);
+	}
+
+	public onDidChangeConfiguration() {
+		return { dispose() { } };
+	}
+
+	public inspect<T>(key: string, overrides?: IConfigurationOverrides): {
+		default: T,
+		user: T,
+		workspace?: T,
+		workspaceFolder?: T
+		value: T,
+	} {
+
+		return {
+			value: getConfigurationValue<T>(this.configuration.user, key),
+			default: undefined,
+			user: getConfigurationValue<T>(this.configuration.user, key),
+			workspace: getConfigurationValue<T>(this.configuration.workspace, key),
+			workspaceFolder: undefined
+		};
+	}
+
+	public keys() {
+		return {
+			default: getConfigurationKeys(),
+			user: Object.keys(this.configuration),
+			workspace: [],
+			workspaceFolder: []
+		};
+	}
+
+	public getConfigurationData() {
+		return null;
+	}
+}