From 1773dede25f668d7da9775788d52a45f32430cac Mon Sep 17 00:00:00 2001 From: Aditya Bist Date: Fri, 31 May 2019 11:14:37 -0700 Subject: [PATCH] CMS fit and finish (#5542) * cms connections dont save * added value to enum * remove refresh and update provider name for cms * removed ownerUri from saved connection and contributed to array * removed owneruri * ownerUri not needed any more * removed AAD from cms * initial review * changed comments * add back saveProfile option for connectionProfile * review fixes and other UI improvements * fixed auth * added cms integration tests * added constants * removed utils from apiwrapper * changed connection type name * review comments * clearer code and addressed reviews --- extensions/cms/package.json | 34 ++- extensions/cms/package.nls.json | 14 +- .../cms/resources/dark/folder_inverse.svg | 1 + extensions/cms/src/apiWrapper.ts | 157 +------------- extensions/cms/src/appContext.ts | 8 +- .../src/cmsResource/cmsResourceCommands.ts | 52 +++-- extensions/cms/src/cmsResource/constants.ts | 4 +- .../cms/src/cmsResource/tree/baseTreeNodes.ts | 6 +- .../cmsResource/tree/cmsResourceTreeNode.ts | 11 +- .../tree/registeredServerTreeNode.ts | 6 +- .../cmsResource/tree/serverGroupTreeNode.ts | 6 +- .../cms/src/cmsResource/tree/treeProvider.ts | 22 +- extensions/cms/src/cmsUtils.ts | 202 ++++++++++++++++++ extensions/cms/src/extension.ts | 4 +- .../tree/cmsResourceTreeNode.test.ts | 4 +- .../tree/registeredServerTreeNode.test.ts | 5 +- .../tree/serverGroupTreeNode.test.ts | 5 +- .../cmsResource/tree/treeProvider.test.ts | 20 +- extensions/integration-tests/src/cms.test.ts | 156 ++++++++++++++ extensions/integration-tests/src/utils.ts | 8 +- .../connection/common/connectionManagement.ts | 3 +- .../node/mainThreadConnectionManagement.ts | 15 +- .../workbench/api/node/sqlExtHost.api.impl.ts | 4 +- .../nodeActions.contribution.ts | 3 +- .../connection/browser/cmsConnectionWidget.ts | 14 +- .../browser/connectionDialogService.ts | 49 +++-- .../browser/connectionDialogWidget.ts | 11 +- 27 files changed, 546 insertions(+), 278 deletions(-) create mode 100644 extensions/cms/resources/dark/folder_inverse.svg create mode 100644 extensions/cms/src/cmsUtils.ts create mode 100644 extensions/integration-tests/src/cms.test.ts diff --git a/extensions/cms/package.json b/extensions/cms/package.json index 37c332433f..cb007d23c6 100644 --- a/extensions/cms/package.json +++ b/extensions/cms/package.json @@ -24,8 +24,8 @@ "type": "object", "title": "%cms.title%", "properties": { - "cms.cmsServers": { - "type": "object" + "centralManagementServers.servers": { + "type": "array" } } }, @@ -518,7 +518,7 @@ { "command": "cms.resource.deleteRegisteredServer", "title": "%cms.resource.deleteRegisteredServer.title%", - "when": "viewItem == cms.resource.itemType.registeredServer" + "when": "viewItem == cms.resource.itemType.databaseServer" }, { "command": "cms.resource.addRegisteredServer", @@ -536,18 +536,8 @@ "when": "viewItem == cms.resource.itemType.serverGroup" }, { - "command": "cms.resource.refreshServerGroup", - "title": "%cms.resource.refreshServerGroup.title%", - "when": "viewItem == cms.resource.itemType.serverGroup", - "icon": { - "light": "resources/light/refresh.svg", - "dark": "resources/dark/refresh_inverse.svg" - } - }, - { - "command": "cms.resource.refreshCmsServer", - "title": "%cms.resource.refreshCmsServer.title%", - "when": "viewItem == cms.resource.itemType.cmsNodeContainer", + "command": "cms.resource.refresh", + "title": "%cms.resource.refresh.title%", "icon": { "light": "resources/light/refresh.svg", "dark": "resources/dark/refresh_inverse.svg" @@ -585,7 +575,7 @@ "view/item/context": [ { "command": "cms.resource.deleteRegisteredServer", - "when": "viewItem == cms.resource.itemType.registeredServer", + "when": "viewItem == cms.resource.itemType.databaseServer", "group": "navigation@2" }, { @@ -600,7 +590,7 @@ }, { "command": "cms.resource.addRegisteredServer", - "when": "viewItem == cms.resource.itemType.cmsNodeContainer", + "when": "viewItem == cms.resource.itemType.databaseServerContainer", "group": "navigation@10" }, { @@ -610,22 +600,22 @@ }, { "command": "cms.resource.addServerGroup", - "when": "viewItem == cms.resource.itemType.cmsNodeContainer", + "when": "viewItem == cms.resource.itemType.databaseServerContainer", "group": "navigation@10" }, { - "command": "cms.resource.refreshServerGroup", + "command": "cms.resource.refresh", "when": "viewItem == cms.resource.itemType.serverGroup", "group": "navigation@10" }, { - "command": "cms.resource.refreshCmsServer", - "when": "viewItem == cms.resource.itemType.cmsNodeContainer", + "command": "cms.resource.refresh", + "when": "viewItem == cms.resource.itemType.databaseServerContainer", "group": "navigation@10" }, { "command": "cms.resource.deleteCmsServer", - "when": "viewItem == cms.resource.itemType.cmsNodeContainer", + "when": "viewItem == cms.resource.itemType.databaseServerContainer", "group": "navigation@10" } ] diff --git a/extensions/cms/package.nls.json b/extensions/cms/package.nls.json index 423fd76bd1..ca46fbbfe6 100644 --- a/extensions/cms/package.nls.json +++ b/extensions/cms/package.nls.json @@ -2,16 +2,16 @@ "cms.displayName": "CMS", "cms.description": "Central Management Servers", "cms.title": "Central Management Servers", - "cms.connectionProvider.displayName": "Microsoft SQL Server - CMS", + "cms.connectionProvider.displayName": "Microsoft SQL Server", "cms.resource.explorer.title": "Central Management Servers", + "cms.resource.refresh.title": "Refresh", "cms.resource.refreshServerGroup.title": "Refresh Server Group", - "cms.resource.refreshCmsServer.title": "Refresh Central Management Server", - "cms.resource.deleteRegisteredServer.title": "Delete Registered Server", - "cms.resource.addRegisteredServer.title": "Add Registered Server", - "cms.resource.deleteServerGroup.title": "Delete Server Group", - "cms.resource.addServerGroup.title": "Add Server Group", + "cms.resource.deleteRegisteredServer.title": "Delete", + "cms.resource.addRegisteredServer.title": "New Server Registration...", + "cms.resource.deleteServerGroup.title": "Delete", + "cms.resource.addServerGroup.title": "New Server Group...", "cms.resource.registerCmsServer.title": "Add Central Management Server", - "cms.resource.deleteCmsServer.title": "Delete Central Management Server", + "cms.resource.deleteCmsServer.title": "Delete", "cms.configuration.title": "MSSQL configuration", "cms.query.displayBitAsNumber": "Should BIT columns be displayed as numbers (1 or 0)? If false, BIT columns will be displayed as 'true' or 'false'", diff --git a/extensions/cms/resources/dark/folder_inverse.svg b/extensions/cms/resources/dark/folder_inverse.svg new file mode 100644 index 0000000000..f94d427cb1 --- /dev/null +++ b/extensions/cms/resources/dark/folder_inverse.svg @@ -0,0 +1 @@ +folder_inverse_16x16 \ No newline at end of file diff --git a/extensions/cms/src/apiWrapper.ts b/extensions/cms/src/apiWrapper.ts index eaec2bd00c..45a9041c55 100644 --- a/extensions/cms/src/apiWrapper.ts +++ b/extensions/cms/src/apiWrapper.ts @@ -12,6 +12,8 @@ import * as Utils from './cmsResource/utils'; import { ICmsResourceNodeInfo } from './cmsResource/tree/baseTreeNodes'; const localize = nls.loadMessageBundle(); +const cmsProvider: string = 'MSSQL-CMS'; +const mssqlProvider: string = 'MSSQL'; /** * Wrapper class to act as a facade over VSCode and Data APIs and allow us to test / mock callbacks into @@ -102,11 +104,11 @@ export class ApiWrapper { * @param resource The optional URI, as a URI object or a string, to use to get resource-scoped configurations */ public getConfiguration(): vscode.WorkspaceConfiguration { - return vscode.workspace.getConfiguration('cms'); + return vscode.workspace.getConfiguration('centralManagementServers'); } public async setConfiguration(value: any): Promise { - await vscode.workspace.getConfiguration('cms').update('cmsServers', value, true); + await vscode.workspace.getConfiguration('centralManagementServers').update('servers', value, true); } /** @@ -156,7 +158,7 @@ export class ApiWrapper { } public showWarningMessage(message: string, ...items: string[]): Thenable { - return vscode.window.showWarningMessage(message, ...items); + return vscode.window.showWarningMessage(message, { modal: true }, ...items); } public showInformationMessage(message: string, ...items: string[]): Thenable { @@ -178,153 +180,4 @@ export class ApiWrapper { public registerCompletionItemProvider(selector: vscode.DocumentSelector, provider: vscode.CompletionItemProvider, ...triggerCharacters: string[]): vscode.Disposable { return vscode.languages.registerCompletionItemProvider(selector, provider, ...triggerCharacters); } - - // Connection APIs - public openConnectionDialog(providers: string[], initialConnectionProfile?: azdata.IConnectionProfile, connectionCompletionOptions?: azdata.IConnectionCompletionOptions): Thenable { - return azdata.connection.openConnectionDialog(providers, initialConnectionProfile, connectionCompletionOptions); - } - - // CMS APIs - public async getCmsService(): Promise { - if (!this._cmsService) { - let extensionApi: mssql.MssqlExtensionApi = vscode.extensions.getExtension('Microsoft.mssql').exports; - this._cmsService = await extensionApi.getCmsServiceProvider(); - } - return this._cmsService; - } - - public async getRegisteredServers(ownerUri: string, relativePath: string): Promise { - return this.getCmsService().then((service) => { - return service.getRegisteredServers(ownerUri, relativePath).then((result) => { - if (result && result.registeredServersList && result.registeredServersList) { - return result; - } - }); - }); - } - - public async createCmsServer(connection: azdata.connection.Connection, - name: string, description: string): Promise { - let provider = await this.getCmsService(); - connection.providerName = connection.providerName === 'MSSQL-CMS' ? 'MSSQL' : connection.providerName; - let ownerUri = await azdata.connection.getUriForConnection(connection.connectionId); - if (!ownerUri) { - // Make a connection if it's not already connected - await azdata.connection.connect(Utils.toConnectionProfile(connection), false, false).then(async (result) => { - ownerUri = await azdata.connection.getUriForConnection(result.connectionId); - }); - } - return provider.createCmsServer(name, description, connection, ownerUri).then((result) => { - if (result) { - return result; - } - }); - } - - public async deleteCmsServer(cmsServer: any) { - let config = this.getConfiguration(); - if (config && config.cmsServers) { - let newServers = config.cmsServers.filter((cachedServer) => { - return cachedServer.name !== cmsServer; - }); - await this.setConfiguration(newServers); - this._registeredCmsServers = this._registeredCmsServers.filter((cachedServer) => { - return cachedServer.name !== cmsServer; - }); - } - } - - public cacheRegisteredCmsServer(name: string, description: string, ownerUri: string, connection: azdata.connection.Connection): void { - if (!this._registeredCmsServers) { - this._registeredCmsServers = []; - } - let cmsServerNode: ICmsResourceNodeInfo = { - name: name, - description: description, - ownerUri: ownerUri, - connection: connection - }; - this._registeredCmsServers.push(cmsServerNode); - } - - public async addRegisteredServer(relativePath: string, ownerUri: string, - parentServerName?: string): Promise { - let provider = await this.getCmsService(); - // Initial profile to disallow SQL Login without - // changing provider. - let initialProfile: azdata.IConnectionProfile = { - connectionName: undefined, - serverName: undefined, - databaseName: undefined, - userName: undefined, - password: undefined, - authenticationType: undefined, - savePassword: undefined, - groupFullName: undefined, - groupId: undefined, - providerName: undefined, - saveProfile: undefined, - id: undefined, - options: { - authTypeChanged: true - } - }; - return this.openConnectionDialog(['MSSQL-CMS'], initialProfile, undefined).then((connection) => { - if (connection && connection.options) { - if (connection.options.server === parentServerName) { - // error out for same server registration - let errorText = localize('cms.errors.sameServerUnderCms', 'You cannot add a shared registered server with the same name as the Configuration Server'); - this.showErrorMessage(errorText); - return; - } else { - return provider.addRegisteredServer(ownerUri, relativePath, - connection.options.registeredServerName, connection.options.registeredServerDescription, connection).then((result) => { - if (result) { - return connection.options.server; - } - }); - } - - } - }); - } - - public async removeRegisteredServer(registeredServerName: string, relativePath: string, ownerUri: string): Promise { - let provider = await this.getCmsService(); - return provider.removeRegisteredServer(ownerUri, relativePath, registeredServerName).then((result) => { - return result; - }); - } - - public async addServerGroup(groupName: string, groupDescription: string, relativePath: string, ownerUri: string): Promise { - let provider = await this.getCmsService(); - return provider.addServerGroup(ownerUri, relativePath, groupName, groupDescription).then((result) => { - return result; - }); - } - - public async removeServerGroup(groupName: string, relativePath: string, ownerUri: string): Promise { - let provider = await this.getCmsService(); - return provider.removeServerGroup(ownerUri, relativePath, groupName).then((result) => { - return result; - }); - } - - // Getters - - public get registeredCmsServers(): ICmsResourceNodeInfo[] { - return this._registeredCmsServers; - } - - public get connection(): Thenable { - return this.openConnectionDialog(['MSSQL-CMS'], undefined, undefined).then((connection) => { - if (connection) { - // remove group ID from connection if a user chose connection - // from the recent connections list - connection.options['groupId'] = null; - connection.providerName = 'MSSQL'; - return connection; - } - }); - } } \ No newline at end of file diff --git a/extensions/cms/src/appContext.ts b/extensions/cms/src/appContext.ts index d5b7c7c2fa..4dda8406d6 100644 --- a/extensions/cms/src/appContext.ts +++ b/extensions/cms/src/appContext.ts @@ -7,13 +7,19 @@ import * as vscode from 'vscode'; import { ApiWrapper } from './apiWrapper'; +import { CmsUtils } from './cmsUtils'; /** * Global context for the application */ export class AppContext { - constructor(public readonly extensionContext: vscode.ExtensionContext, public readonly apiWrapper: ApiWrapper) { + constructor( + public readonly extensionContext: vscode.ExtensionContext, + public readonly apiWrapper: ApiWrapper, + public readonly cmsUtils: CmsUtils + ) { this.apiWrapper = apiWrapper || new ApiWrapper(); + this.cmsUtils = cmsUtils || new CmsUtils(); } } diff --git a/extensions/cms/src/cmsResource/cmsResourceCommands.ts b/extensions/cms/src/cmsResource/cmsResourceCommands.ts index 33d7cf3946..453b4985b1 100644 --- a/extensions/cms/src/cmsResource/cmsResourceCommands.ts +++ b/extensions/cms/src/cmsResource/cmsResourceCommands.ts @@ -22,12 +22,12 @@ export function registerCmsServerCommand(appContext: AppContext, tree: CmsResour if (node && !(node instanceof CmsResourceEmptyTreeNode)) { return; } - await appContext.apiWrapper.connection.then(async (connection) => { + await appContext.cmsUtils.connection.then(async (connection) => { if (connection && connection.options) { let registeredCmsServerName = connection.options.registeredServerName ? connection.options.registeredServerName : connection.options.server; // check if a CMS with the same name is registered or not - let cachedServers = appContext.apiWrapper.registeredCmsServers; + let cachedServers = appContext.cmsUtils.registeredCmsServers; let serverExists: boolean = false; if (cachedServers) { serverExists = cachedServers.some((server) => { @@ -42,7 +42,7 @@ export function registerCmsServerCommand(appContext: AppContext, tree: CmsResour // remove server description from connection uri connection.options.registeredCmsServerDescription = null; let ownerUri = await azdata.connection.getUriForConnection(connection.connectionId); - appContext.apiWrapper.cacheRegisteredCmsServer(registeredCmsServerName, registeredCmsServerDescription, ownerUri, connection); + appContext.cmsUtils.cacheRegisteredCmsServer(registeredCmsServerName, registeredCmsServerDescription, ownerUri, connection); tree.notifyNodeChanged(undefined); } else { // error out for same server name @@ -61,7 +61,7 @@ export function deleteCmsServerCommand(appContext: AppContext, tree: CmsResource if (!(node instanceof CmsResourceTreeNode)) { return; } - await appContext.apiWrapper.deleteCmsServer(node.name); + await appContext.cmsUtils.deleteCmsServer(node.name); tree.isSystemInitialized = false; tree.notifyNodeChanged(undefined); }); @@ -75,9 +75,9 @@ export function addRegisteredServerCommand(appContext: AppContext, tree: CmsReso } let relativePath = node instanceof CmsResourceTreeNode ? '' : node.relativePath; let serverName = node instanceof CmsResourceTreeNode ? node.connection.options.server : null; - await appContext.apiWrapper.addRegisteredServer(relativePath, node.ownerUri, serverName).then((result) => { + await appContext.cmsUtils.addRegisteredServer(relativePath, node.ownerUri, serverName).then((result) => { if (result) { - tree.notifyNodeChanged(undefined); + tree.notifyNodeChanged(node); } }); }); @@ -89,11 +89,18 @@ export function deleteRegisteredServerCommand(appContext: AppContext, tree: CmsR if (!(node instanceof RegisteredServerTreeNode)) { return; } - appContext.apiWrapper.removeRegisteredServer(node.name, node.relativePath, node.ownerUri).then((result) => { - if (result) { - tree.notifyNodeChanged(undefined); - } - }); + appContext.apiWrapper.showWarningMessage( + `${localize('cms.confirmDeleteServer', 'Are you sure you want to delete')} ${node.name}?`, + localize('cms.yes', 'Yes'), + localize('cms.no', 'No')).then((result) => { + if (result && result === localize('cms.yes', 'Yes')) { + appContext.cmsUtils.removeRegisteredServer(node.name, node.relativePath, node.ownerUri).then((result) => { + if (result) { + tree.notifyNodeChanged(node.parent); + } + }); + } + }); }); } @@ -144,9 +151,9 @@ export function addServerGroupCommand(appContext: AppContext, tree: CmsResourceT groupExists = true; } if (!groupExists) { - appContext.apiWrapper.addServerGroup(serverGroupName, serverDescription, path, node.ownerUri).then((result) => { + appContext.cmsUtils.addServerGroup(serverGroupName, serverDescription, path, node.ownerUri).then((result) => { if (result) { - tree.notifyNodeChanged(undefined); + tree.notifyNodeChanged(node); } }); } else { @@ -165,11 +172,18 @@ export function deleteServerGroupCommand(appContext: AppContext, tree: CmsResour if (!(node instanceof ServerGroupTreeNode)) { return; } - appContext.apiWrapper.removeServerGroup(node.name, node.relativePath, node.ownerUri).then((result) => { - if (result) { - tree.notifyNodeChanged(undefined); - } - }); + appContext.apiWrapper.showWarningMessage( + `${localize('cms.confirmDeleteGroup', 'Are you sure you want to delete')} ${node.name}?`, + localize('cms.yes', 'Yes'), + localize('cms.no', 'No')).then((result) => { + if (result && result === localize('cms.yes', 'Yes')) { + appContext.cmsUtils.removeServerGroup(node.name, node.relativePath, node.ownerUri).then((result) => { + if (result) { + tree.notifyNodeChanged(node.parent); + } + }); + } + }); }); } @@ -179,7 +193,7 @@ export function refreshCommand(appContext: AppContext, tree: CmsResourceTreeProv if (!node) { return; } - tree.notifyNodeChanged(undefined); + tree.notifyNodeChanged(node); }); } diff --git a/extensions/cms/src/cmsResource/constants.ts b/extensions/cms/src/cmsResource/constants.ts index 1547aa9f7c..6f5ff53b73 100644 --- a/extensions/cms/src/cmsResource/constants.ts +++ b/extensions/cms/src/cmsResource/constants.ts @@ -8,7 +8,7 @@ export enum CmsResourceItemType { cmsMessageNodeContainer = 'cms.resource.itemType.cmsMessageNodeContainer', cmsEmptyNodeContainer = 'cms.resource.itemType.cmsEmptyNodeContainer', - cmsNodeContainer = 'cms.resource.itemType.cmsNodeContainer', - registeredServer = 'cms.resource.itemType.registeredServer', + cmsNodeContainer = 'cms.resource.itemType.databaseServerContainer', + registeredServer = 'cms.resource.itemType.databaseServer', serverGroup = 'cms.resource.itemType.serverGroup' } \ No newline at end of file diff --git a/extensions/cms/src/cmsResource/tree/baseTreeNodes.ts b/extensions/cms/src/cmsResource/tree/baseTreeNodes.ts index 9394c32222..78c623d536 100644 --- a/extensions/cms/src/cmsResource/tree/baseTreeNodes.ts +++ b/extensions/cms/src/cmsResource/tree/baseTreeNodes.ts @@ -8,19 +8,23 @@ import * as azdata from 'azdata'; import { AppContext } from '../../appContext'; import { TreeNode } from '../treeNode'; import { ICmsResourceTreeChangeHandler } from './treeChangeHandler'; +import { generateGuid } from '../utils'; export abstract class CmsResourceTreeNodeBase extends TreeNode { + protected _id: string = undefined; + public constructor( private _name: string, private _description: string, - private _ownerUri: string, + protected _ownerUri: string, public readonly appContext: AppContext, public readonly treeChangeHandler: ICmsResourceTreeChangeHandler, parent: TreeNode ) { super(); this.parent = parent; + this._id = generateGuid(); } public get name(): string { diff --git a/extensions/cms/src/cmsResource/tree/cmsResourceTreeNode.ts b/extensions/cms/src/cmsResource/tree/cmsResourceTreeNode.ts index 2c95156c61..b34ad08b49 100644 --- a/extensions/cms/src/cmsResource/tree/cmsResourceTreeNode.ts +++ b/extensions/cms/src/cmsResource/tree/cmsResourceTreeNode.ts @@ -20,7 +20,6 @@ const localize = nls.loadMessageBundle(); export class CmsResourceTreeNode extends CmsResourceTreeNodeBase { - private _id: string = undefined; private _serverGroupNodes: ServerGroupTreeNode[] = []; public constructor( @@ -33,13 +32,15 @@ export class CmsResourceTreeNode extends CmsResourceTreeNodeBase { parent: TreeNode ) { super(name, description, ownerUri, appContext, treeChangeHandler, parent); - this._id = `cms_cmsServer_${this.name}`; } public async getChildren(): Promise { try { - let nodes: TreeNode[] = []; - return this.appContext.apiWrapper.createCmsServer(this.connection, this.name, this.description).then(async (result) => { + let nodes: CmsResourceTreeNodeBase[] = []; + if (!this.ownerUri) { + this._ownerUri = await this.appContext.cmsUtils.getUriForConnection(this.connection); + } + return this.appContext.cmsUtils.createCmsServer(this.connection, this.name, this.description).then((result) => { if (result) { if (result.registeredServersList) { result.registeredServersList.forEach((registeredServer) => { @@ -70,7 +71,7 @@ export class CmsResourceTreeNode extends CmsResourceTreeNodeBase { } } if (nodes.length > 0) { - return nodes; + return nodes.sort((node1, node2) => node1.name > node2.name ? 1 : -1); } else { return [CmsResourceMessageTreeNode.create(CmsResourceTreeNode.noResourcesLabel, undefined)]; } diff --git a/extensions/cms/src/cmsResource/tree/registeredServerTreeNode.ts b/extensions/cms/src/cmsResource/tree/registeredServerTreeNode.ts index 5f085be4d0..2acf5842b1 100644 --- a/extensions/cms/src/cmsResource/tree/registeredServerTreeNode.ts +++ b/extensions/cms/src/cmsResource/tree/registeredServerTreeNode.ts @@ -11,12 +11,9 @@ import { CmsResourceItemType } from '../constants'; import { CmsResourceTreeNodeBase } from './baseTreeNodes'; import { AppContext } from '../../appContext'; import { ICmsResourceTreeChangeHandler } from './treeChangeHandler'; -import { generateGuid } from '../utils'; export class RegisteredServerTreeNode extends CmsResourceTreeNodeBase { - private _id: string = undefined; - constructor( name: string, description: string, @@ -28,7 +25,6 @@ export class RegisteredServerTreeNode extends CmsResourceTreeNodeBase { parent: TreeNode ) { super(name, description, ownerUri, appContext, treeChangeHandler, parent); - this._id = `cms_registeredServer_${this.name ? this.name : this.serverName}`; } public async getChildren(): Promise { @@ -37,7 +33,7 @@ export class RegisteredServerTreeNode extends CmsResourceTreeNodeBase { public getTreeItem(): azdata.TreeItem | Promise { let payload = { - id: generateGuid(), + id: this._id, connectionName: this.name ? this.name : this.serverName, serverName: this.serverName, databaseName: '', diff --git a/extensions/cms/src/cmsResource/tree/serverGroupTreeNode.ts b/extensions/cms/src/cmsResource/tree/serverGroupTreeNode.ts index 310b1bd25f..f7f4cd6e01 100644 --- a/extensions/cms/src/cmsResource/tree/serverGroupTreeNode.ts +++ b/extensions/cms/src/cmsResource/tree/serverGroupTreeNode.ts @@ -18,7 +18,6 @@ import { CmsResourceTreeNode } from './cmsResourceTreeNode'; export class ServerGroupTreeNode extends CmsResourceTreeNodeBase { - private _id: string = undefined; private _serverGroupNodes: ServerGroupTreeNode[] = []; constructor( @@ -31,12 +30,11 @@ export class ServerGroupTreeNode extends CmsResourceTreeNodeBase { parent: TreeNode ) { super(name, description, ownerUri, appContext, treeChangeHandler, parent); - this._id = `cms_serverGroup_${this.name}`; } public getChildren(): TreeNode[] | Promise { try { let nodes = []; - return this.appContext.apiWrapper.getRegisteredServers(this.ownerUri, this.relativePath).then((result) => { + return this.appContext.cmsUtils.getRegisteredServers(this.ownerUri, this.relativePath).then((result) => { if (result) { if (result.registeredServersList) { result.registeredServersList.forEach((registeredServer) => { @@ -86,7 +84,7 @@ export class ServerGroupTreeNode extends CmsResourceTreeNodeBase { item.id = this._id; item.tooltip = this.description; item.iconPath = { - dark: this.appContext.extensionContext.asAbsolutePath('resources/light/folder.svg'), + dark: this.appContext.extensionContext.asAbsolutePath('resources/dark/folder_inverse.svg'), light: this.appContext.extensionContext.asAbsolutePath('resources/light/folder.svg') }; return item; diff --git a/extensions/cms/src/cmsResource/tree/treeProvider.ts b/extensions/cms/src/cmsResource/tree/treeProvider.ts index 90ca066182..d6fa2bb47e 100644 --- a/extensions/cms/src/cmsResource/tree/treeProvider.ts +++ b/extensions/cms/src/cmsResource/tree/treeProvider.ts @@ -35,18 +35,18 @@ export class CmsResourceTreeProvider implements TreeDataProvider, ICms // Call to collect all locally saved CMS servers // to determine whether the system has been initialized. let cmsConfig = this._appContext.apiWrapper.getConfiguration(); - let cachedServers = cmsConfig ? cmsConfig.cmsServers : []; + let cachedServers = cmsConfig.servers ? cmsConfig.servers : []; if (cachedServers && cachedServers.length > 0) { let servers = []; - cachedServers.forEach((server) => { + cachedServers.forEach(async (server) => { servers.push(new CmsResourceTreeNode( server.name, server.description, - server.ownerUri, + undefined, server.connection, this._appContext, this, null)); - this.appContext.apiWrapper.cacheRegisteredCmsServer(server.name, server.description, - server.ownerUri, server.connection); + this.appContext.cmsUtils.cacheRegisteredCmsServer(server.name, server.description, + undefined, server.connection); }); return servers; } @@ -59,11 +59,16 @@ export class CmsResourceTreeProvider implements TreeDataProvider, ICms return [CmsResourceMessageTreeNode.create(CmsResourceTreeProvider.loadingLabel, undefined)]; } try { - let registeredCmsServers = this.appContext.apiWrapper.registeredCmsServers; + let registeredCmsServers = this.appContext.cmsUtils.registeredCmsServers; if (registeredCmsServers && registeredCmsServers.length > 0) { this.isSystemInitialized = true; // save the CMS Servers for future use - await this._appContext.apiWrapper.setConfiguration(registeredCmsServers); + let toSaveCmsServers = JSON.parse(JSON.stringify(registeredCmsServers)); + toSaveCmsServers.forEach(server => { + server.ownerUri = undefined, + server.connection.options.password = ''; + }); + await this._appContext.apiWrapper.setConfiguration(toSaveCmsServers); return registeredCmsServers.map((server) => { return new CmsResourceTreeNode( server.name, @@ -71,10 +76,9 @@ export class CmsResourceTreeProvider implements TreeDataProvider, ICms server.ownerUri, server.connection, this._appContext, this, null); - }); + }).sort((a, b) => a.name.localeCompare(b.name)); } else { return [new CmsResourceEmptyTreeNode()]; - } } catch (error) { return [new CmsResourceEmptyTreeNode()]; diff --git a/extensions/cms/src/cmsUtils.ts b/extensions/cms/src/cmsUtils.ts new file mode 100644 index 0000000000..6afff0b444 --- /dev/null +++ b/extensions/cms/src/cmsUtils.ts @@ -0,0 +1,202 @@ +/*--------------------------------------------------------------------------------------------- + * 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 nls from 'vscode-nls'; +import * as vscode from 'vscode'; +import * as azdata from 'azdata'; +import * as mssql from '../../mssql/src/api/mssqlapis'; +import * as Utils from './cmsResource/utils'; +import { ICmsResourceNodeInfo } from './cmsResource/tree/baseTreeNodes'; + +const localize = nls.loadMessageBundle(); +const cmsProvider: string = 'MSSQL-CMS'; +const mssqlProvider: string = 'MSSQL'; + +/** + * Wrapper class to act as a facade over VSCode and Data APIs and allow us to test / mock callbacks into + * this API from our code + * + * @export + * ApiWrapper + */ +export class CmsUtils { + + private _cmsService: mssql.CmsService; + private _registeredCmsServers: ICmsResourceNodeInfo[]; + + public showErrorMessage(message: string, ...items: string[]): Thenable { + return vscode.window.showErrorMessage(message, ...items); + } + + /** + * Get the configuration for a extensionName + * @param extensionName The string name of the extension to get the configuration for + * @param resource The optional URI, as a URI object or a string, to use to get resource-scoped configurations + */ + public getConfiguration(): vscode.WorkspaceConfiguration { + return vscode.workspace.getConfiguration('centralManagementServers'); + } + + public async setConfiguration(value: any): Promise { + await vscode.workspace.getConfiguration('centralManagementServers').update('servers', value, true); + } + + // Connection APIs + public openConnectionDialog(providers: string[], initialConnectionProfile?: azdata.IConnectionProfile, connectionCompletionOptions?: azdata.IConnectionCompletionOptions): Thenable { + return azdata.connection.openConnectionDialog(providers, initialConnectionProfile, connectionCompletionOptions); + } + + public async getUriForConnection(connection: azdata.connection.Connection): Promise { + let ownerUri = await azdata.connection.getUriForConnection(connection.connectionId); + if (!ownerUri) { + // Make a connection if it's not already connected + await azdata.connection.connect(Utils.toConnectionProfile(connection), false, false).then(async (result) => { + ownerUri = await azdata.connection.getUriForConnection(result.connectionId); + }); + } + return ownerUri; + } + + // CMS APIs + public async getCmsService(): Promise { + if (!this._cmsService) { + let extensionApi: mssql.MssqlExtensionApi = vscode.extensions.getExtension('Microsoft.mssql').exports; + this._cmsService = await extensionApi.getCmsServiceProvider(); + } + return this._cmsService; + } + + public async getRegisteredServers(ownerUri: string, relativePath: string): Promise { + return this.getCmsService().then((service) => { + return service.getRegisteredServers(ownerUri, relativePath).then((result) => { + if (result && result.registeredServersList && result.registeredServersList) { + return result; + } + }); + }); + } + + public async createCmsServer(connection: azdata.connection.Connection, + name: string, description: string): Promise { + let provider = await this.getCmsService(); + connection.providerName = connection.providerName === cmsProvider ? mssqlProvider : connection.providerName; + let ownerUri = await azdata.connection.getUriForConnection(connection.connectionId); + if (!ownerUri) { + // Make a connection if it's not already connected + await azdata.connection.connect(Utils.toConnectionProfile(connection), false, false).then(async (result) => { + ownerUri = await azdata.connection.getUriForConnection(result.connectionId); + }); + } + return provider.createCmsServer(name, description, connection, ownerUri).then((result) => { + if (result) { + return result; + } + }); + } + + public async deleteCmsServer(cmsServer: any): Promise { + let config = this.getConfiguration(); + if (config && config.servers) { + let newServers = config.servers.filter((cachedServer) => { + return cachedServer.name !== cmsServer; + }); + await this.setConfiguration(newServers); + this._registeredCmsServers = this._registeredCmsServers.filter((cachedServer) => { + return cachedServer.name !== cmsServer; + }); + } + } + + public cacheRegisteredCmsServer(name: string, description: string, ownerUri: string, connection: azdata.connection.Connection): void { + if (!this._registeredCmsServers) { + this._registeredCmsServers = []; + } + let cmsServerNode: ICmsResourceNodeInfo = { + name: name, + description: description, + connection: connection, + ownerUri: ownerUri + }; + this._registeredCmsServers.push(cmsServerNode); + } + + public async addRegisteredServer(relativePath: string, ownerUri: string, + parentServerName?: string): Promise { + let provider = await this.getCmsService(); + // Initial profile to disallow SQL Login without + // changing provider. + let initialProfile: azdata.IConnectionProfile = { + connectionName: undefined, + serverName: undefined, + databaseName: undefined, + userName: undefined, + password: undefined, + authenticationType: undefined, + savePassword: undefined, + groupFullName: undefined, + groupId: undefined, + providerName: undefined, + saveProfile: undefined, + id: undefined, + options: { + authTypeChanged: true + } + }; + return this.openConnectionDialog([cmsProvider], initialProfile, { saveConnection: false }).then(async (connection) => { + if (connection && connection.options) { + if (connection.options.server === parentServerName) { + // error out for same server registration + let errorText = localize('cms.errors.sameServerUnderCms', 'You cannot add a shared registered server with the same name as the Configuration Server'); + this.showErrorMessage(errorText); + return false; + } else { + let registeredServerName = connection.options.registeredServerName === '' ? connection.options.server : connection.options.registeredServerName; + let result = await provider.addRegisteredServer(ownerUri, relativePath, registeredServerName, connection.options.registeredServerDescription, connection); + return result; + } + + } + }); + } + + public async removeRegisteredServer(registeredServerName: string, relativePath: string, ownerUri: string): Promise { + let provider = await this.getCmsService(); + return provider.removeRegisteredServer(ownerUri, relativePath, registeredServerName).then((result) => { + return result; + }); + } + + public async addServerGroup(groupName: string, groupDescription: string, relativePath: string, ownerUri: string): Promise { + let provider = await this.getCmsService(); + return provider.addServerGroup(ownerUri, relativePath, groupName, groupDescription).then((result) => { + return result; + }); + } + + public async removeServerGroup(groupName: string, relativePath: string, ownerUri: string): Promise { + let provider = await this.getCmsService(); + return provider.removeServerGroup(ownerUri, relativePath, groupName).then((result) => { + return result; + }); + } + + // Getters + public get registeredCmsServers(): ICmsResourceNodeInfo[] { + return this._registeredCmsServers; + } + + public get connection(): Thenable { + return this.openConnectionDialog([cmsProvider], undefined, { saveConnection: false }).then((connection) => { + if (connection) { + // remove group ID from connection if a user chose connection + // from the recent connections list + connection.options['groupId'] = null; + connection.providerName = mssqlProvider; + return connection; + } + }); + } +} \ No newline at end of file diff --git a/extensions/cms/src/extension.ts b/extensions/cms/src/extension.ts index 8c5527cc23..07ec5be83e 100644 --- a/extensions/cms/src/extension.ts +++ b/extensions/cms/src/extension.ts @@ -10,6 +10,7 @@ import CmsResourceController from './controllers/cmsResourceController'; import { AppContext } from './appContext'; import ControllerBase from './controllers/controllerBase'; import { ApiWrapper } from './apiWrapper'; +import { CmsUtils } from './cmsUtils'; let controllers: ControllerBase[] = []; @@ -17,7 +18,8 @@ let controllers: ControllerBase[] = []; // your extension is activated the very first time the command is executed export function activate(extensionContext: vscode.ExtensionContext) { const apiWrapper = new ApiWrapper(); - let appContext = new AppContext(extensionContext, apiWrapper); + const cmsUtils = new CmsUtils(); + let appContext = new AppContext(extensionContext, apiWrapper, cmsUtils); let activations: Thenable[] = []; const cmsResourceController = new CmsResourceController(appContext); diff --git a/extensions/cms/src/test/cmsResource/tree/cmsResourceTreeNode.test.ts b/extensions/cms/src/test/cmsResource/tree/cmsResourceTreeNode.test.ts index 27705f2f43..35bb47e7be 100644 --- a/extensions/cms/src/test/cmsResource/tree/cmsResourceTreeNode.test.ts +++ b/extensions/cms/src/test/cmsResource/tree/cmsResourceTreeNode.test.ts @@ -15,12 +15,14 @@ import { CmsResourceItemType } from '../../../cmsResource/constants'; import { ServerGroupTreeNode } from '../../../cmsResource/tree/serverGroupTreeNode'; import { ICmsResourceTreeChangeHandler } from '../../../cmsResource/tree/treeChangeHandler'; import { cmsResource } from '../../../cmsResource/cms-resource'; +import { CmsUtils } from '../../../cmsUtils'; // Mock services let mockAppContext: AppContext; let mockExtensionContext: TypeMoq.IMock; let mockApiWrapper: TypeMoq.IMock; +let mockCmsUtils: TypeMoq.IMock; let mockTreeChangeHandler: TypeMoq.IMock; let mockResourceTreeDataProvider1: TypeMoq.IMock; @@ -30,7 +32,7 @@ describe('ServerGroupTreeNode.info', function(): void { beforeEach(() => { mockExtensionContext = TypeMoq.Mock.ofType(); mockApiWrapper = TypeMoq.Mock.ofType(); - mockAppContext = new AppContext(mockExtensionContext.object, mockApiWrapper.object); + mockAppContext = new AppContext(mockExtensionContext.object, mockApiWrapper.object, mockCmsUtils.object); mockTreeChangeHandler = TypeMoq.Mock.ofType(); mockResourceTreeDataProvider1 = TypeMoq.Mock.ofType(); mockResourceTreeDataProvider1.setup((o) => o.getChildren()).returns(() => Promise.resolve([TypeMoq.Mock.ofType().object])); diff --git a/extensions/cms/src/test/cmsResource/tree/registeredServerTreeNode.test.ts b/extensions/cms/src/test/cmsResource/tree/registeredServerTreeNode.test.ts index 621705cdbe..d015ef77cf 100644 --- a/extensions/cms/src/test/cmsResource/tree/registeredServerTreeNode.test.ts +++ b/extensions/cms/src/test/cmsResource/tree/registeredServerTreeNode.test.ts @@ -10,17 +10,18 @@ import * as vscode from 'vscode'; import 'mocha'; import { AppContext } from '../../../appContext'; import { ApiWrapper } from '../../../apiWrapper'; - import { CmsResourceItemType } from '../../../cmsResource/constants'; import { RegisteredServerTreeNode } from '../../../cmsResource/tree/registeredServerTreeNode'; import { ICmsResourceTreeChangeHandler } from '../../../cmsResource/tree/treeChangeHandler'; import { cmsResource } from '../../../cmsResource/cms-resource'; +import { CmsUtils } from '../../../cmsUtils'; // Mock services let mockAppContext: AppContext; let mockExtensionContext: TypeMoq.IMock; let mockApiWrapper: TypeMoq.IMock; +let mockCmsUtils: TypeMoq.IMock; let mockTreeChangeHandler: TypeMoq.IMock; let mockResourceTreeDataProvider1: TypeMoq.IMock; @@ -30,7 +31,7 @@ describe('RegisteredServerTreeNode.info', function(): void { beforeEach(() => { mockExtensionContext = TypeMoq.Mock.ofType(); mockApiWrapper = TypeMoq.Mock.ofType(); - mockAppContext = new AppContext(mockExtensionContext.object, mockApiWrapper.object); + mockAppContext = new AppContext(mockExtensionContext.object, mockApiWrapper.object, mockCmsUtils.object); mockTreeChangeHandler = TypeMoq.Mock.ofType(); mockResourceTreeDataProvider1 = TypeMoq.Mock.ofType(); mockResourceTreeDataProvider1.setup((o) => o.getChildren()).returns(() => Promise.resolve([TypeMoq.Mock.ofType().object])); diff --git a/extensions/cms/src/test/cmsResource/tree/serverGroupTreeNode.test.ts b/extensions/cms/src/test/cmsResource/tree/serverGroupTreeNode.test.ts index 27705f2f43..5f45f3bdd7 100644 --- a/extensions/cms/src/test/cmsResource/tree/serverGroupTreeNode.test.ts +++ b/extensions/cms/src/test/cmsResource/tree/serverGroupTreeNode.test.ts @@ -10,17 +10,18 @@ import * as vscode from 'vscode'; import 'mocha'; import { AppContext } from '../../../appContext'; import { ApiWrapper } from '../../../apiWrapper'; - import { CmsResourceItemType } from '../../../cmsResource/constants'; import { ServerGroupTreeNode } from '../../../cmsResource/tree/serverGroupTreeNode'; import { ICmsResourceTreeChangeHandler } from '../../../cmsResource/tree/treeChangeHandler'; import { cmsResource } from '../../../cmsResource/cms-resource'; +import { CmsUtils } from '../../../cmsUtils'; // Mock services let mockAppContext: AppContext; let mockExtensionContext: TypeMoq.IMock; let mockApiWrapper: TypeMoq.IMock; +let mockCmsUtils: TypeMoq.IMock; let mockTreeChangeHandler: TypeMoq.IMock; let mockResourceTreeDataProvider1: TypeMoq.IMock; @@ -30,7 +31,7 @@ describe('ServerGroupTreeNode.info', function(): void { beforeEach(() => { mockExtensionContext = TypeMoq.Mock.ofType(); mockApiWrapper = TypeMoq.Mock.ofType(); - mockAppContext = new AppContext(mockExtensionContext.object, mockApiWrapper.object); + mockAppContext = new AppContext(mockExtensionContext.object, mockApiWrapper.object, mockCmsUtils.object); mockTreeChangeHandler = TypeMoq.Mock.ofType(); mockResourceTreeDataProvider1 = TypeMoq.Mock.ofType(); mockResourceTreeDataProvider1.setup((o) => o.getChildren()).returns(() => Promise.resolve([TypeMoq.Mock.ofType().object])); diff --git a/extensions/cms/src/test/cmsResource/tree/treeProvider.test.ts b/extensions/cms/src/test/cmsResource/tree/treeProvider.test.ts index 61095ddcb8..44fb55cc97 100644 --- a/extensions/cms/src/test/cmsResource/tree/treeProvider.test.ts +++ b/extensions/cms/src/test/cmsResource/tree/treeProvider.test.ts @@ -4,22 +4,22 @@ *--------------------------------------------------------------------------------------------*/ 'use strict'; - +import 'mocha'; import * as vscode from 'vscode'; import * as should from 'should'; import * as TypeMoq from 'typemoq'; -import 'mocha'; import { AppContext } from '../../../appContext'; import { ApiWrapper } from '../../../apiWrapper'; - import { CmsResourceTreeProvider } from '../../../cmsResource/tree/treeProvider'; import { CmsResourceMessageTreeNode } from '../../../cmsResource/messageTreeNode'; import { CmsResourceEmptyTreeNode } from '../../../cmsResource/tree/cmsResourceEmptyTreeNode'; import { CmsResourceTreeNode } from '../../../cmsResource/tree/cmsResourceTreeNode'; +import { CmsUtils } from '../../../cmsUtils'; // Mock services let mockAppContext: AppContext; let mockExtensionContext: TypeMoq.IMock; +let mockCmsUtils: TypeMoq.IMock; let mockApiWrapper: TypeMoq.IMock; @@ -27,7 +27,7 @@ describe('CmsResourceTreeProvider.getChildren', function (): void { beforeEach(() => { mockExtensionContext = TypeMoq.Mock.ofType(); mockApiWrapper = TypeMoq.Mock.ofType(); - mockAppContext = new AppContext(mockExtensionContext.object, mockApiWrapper.object); + mockAppContext = new AppContext(mockExtensionContext.object, mockApiWrapper.object, mockCmsUtils.object); }); it('Should not be initialized.', async function (): Promise { @@ -39,19 +39,19 @@ describe('CmsResourceTreeProvider.getChildren', function (): void { should.equal(children[0] instanceof CmsResourceMessageTreeNode, true); }); - it('Should not be loading after initialized.'), async function (): Promise { + it('Should not be loading after initialized.', async function (): Promise { const treeProvider = new CmsResourceTreeProvider(mockAppContext); treeProvider.isSystemInitialized = true; should.equal(true, treeProvider.isSystemInitialized); - mockApiWrapper.setup(x => x.registeredCmsServers).returns(null); + mockCmsUtils.setup(x => x.registeredCmsServers).returns(null); const children = await treeProvider.getChildren(undefined); should.equal(children[0] instanceof CmsResourceEmptyTreeNode, true); - }; + }); - it('Should show CMS nodes if there are cached servers'), async function (): Promise { + it('Should show CMS nodes if there are cached servers', async function (): Promise { const treeProvider = new CmsResourceTreeProvider(mockAppContext); treeProvider.isSystemInitialized = true; - mockApiWrapper.setup(x => x.registeredCmsServers).returns(() => { + mockCmsUtils.setup(x => x.registeredCmsServers).returns(() => { return [{ name: 'name', description: 'description', @@ -61,5 +61,5 @@ describe('CmsResourceTreeProvider.getChildren', function (): void { }); const children = await treeProvider.getChildren(undefined); should.equal(children[0] instanceof CmsResourceTreeNode, true); - }; + }); }); diff --git a/extensions/integration-tests/src/cms.test.ts b/extensions/integration-tests/src/cms.test.ts new file mode 100644 index 0000000000..240e67c29e --- /dev/null +++ b/extensions/integration-tests/src/cms.test.ts @@ -0,0 +1,156 @@ +/*--------------------------------------------------------------------------------------------- + * 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 'mocha'; +import * as vscode from 'vscode'; +import * as azdata from 'azdata'; +import * as mssql from '../../mssql/src/api/mssqlapis'; +import * as utils from './utils'; +import { context } from './testContext'; +import assert = require('assert'); +import { getStandaloneServer, TestServerProfile, getBdcServer } from './testConfig'; + +let cmsService: mssql.CmsService; +let server: TestServerProfile; +let connectionId: string; +let ownerUri: string; +const SERVER_CONNECTION_TIMEOUT: number = 3000; + +if (context.RunTest) { + suite('CMS integration test suite', () => { + + setup(async function () { + // Set up CMS provider + if (!cmsService) { + let api: mssql.MssqlExtensionApi = vscode.extensions.getExtension('Microsoft.mssql').exports; + cmsService = await api.getCmsServiceProvider(); + assert(cmsService !== undefined); + } + + // Set up connection + if (!server) { + server = await getStandaloneServer(); + connectionId = await utils.connectToServer(server, SERVER_CONNECTION_TIMEOUT); + ownerUri = await azdata.connection.getConnectionString(connectionId, true); + console.log('Start CMS tests'); + } + if (!ownerUri) { + ownerUri = await azdata.connection.getConnectionString(connectionId, true); + } + }); + + test('Create CMS Server', async function () { + // Should fail + let failedResult = await cmsService.createCmsServer(undefined, 'test_description', undefined, ownerUri); + assert(failedResult === undefined, 'Cannot add a CMS server without a name or connection'); + + let connection = { + serverName: server.serverName, + userName: server.userName, + password: server.password, + authenticationType: server.authenticationTypeName, + database: server.database, + provider: server.provider, + version: server.version, + engineType: server.engineType, + options: {} + }; + + // Should create a CMS Server + let result = await cmsService.createCmsServer('test_cms', 'test_description', connection, ownerUri); + assert(result !== undefined, 'CMS server created successfully'); + }); + + test('Add and delete registered group to/from CMS server', async function () { + // Should fail + let failedResult = await cmsService.addServerGroup(ownerUri, '', undefined, 'test_description'); + assert(failedResult === undefined, 'Cannot add a server group without a name'); + + // Should create a server group + let result = await cmsService.addServerGroup(ownerUri, '', 'test_group', 'test_description'); + assert(result === true, 'Server group added to CMS server successfully'); + + // Shouldn't be able to create a new server group with same name + let repeatResult = await cmsService.addServerGroup(ownerUri, '', 'test_group', 'test_description'); + assert(repeatResult === undefined, 'Cannot add a server group with existing name'); + + let cmsResources = await cmsService.getRegisteredServers(ownerUri, ''); + assert(cmsResources.registeredServerGroups.length === 1, 'A server group was added successfully'); + + // Should remove the server group we added above + let deleteResult = await cmsService.removeServerGroup(ownerUri, '', 'test_group'); + assert(deleteResult === true, 'Server group removed from CMS server successfully'); + + cmsResources = await cmsService.getRegisteredServers(ownerUri, ''); + assert(cmsResources.registeredServerGroups.length === 0, 'The server group was removed successfully'); + }); + + test('Add and delete registered server to/from CMS server', async function () { + // Should fail + let failedResult = await cmsService.addRegisteredServer(ownerUri, '', undefined, 'test_description', undefined); + assert(failedResult === undefined, 'Cannot add a registered without a name or connection'); + + let bdcServer = await getBdcServer(); + let bdcConnection = { + serverName: bdcServer.serverName, + userName: bdcServer.userName, + password: bdcServer.password, + authenticationType: bdcServer.authenticationTypeName, + database: bdcServer.database, + provider: bdcServer.provider, + version: bdcServer.version, + engineType: bdcServer.engineType, + options: { } + }; + + // Should create a registered server + let result = await cmsService.addRegisteredServer(ownerUri, '', 'test_registered_server', 'test_description', bdcConnection); + assert(result === true, 'Registered server added to CMS server successfully'); + + // Shouldn't be able to create a new registered server with same name + let repeatResult = await cmsService.addRegisteredServer(ownerUri, '', 'test_registered_server', 'test_description', bdcConnection); + assert(repeatResult === undefined, 'Cannot add a registered server with existing name'); + + // Should remove the registered server we added above + let deleteResult = await cmsService.removeRegisteredServer(ownerUri, '', 'test_registered_server'); + assert(deleteResult === true, 'Registered server added to CMS server successfully'); + }); + + test('Add and delete registered server to/from server group', async function () { + + // Should create a server group + let result = await cmsService.addServerGroup(ownerUri, '', 'test_group', 'test_description'); + assert(result === true, 'Server group added to CMS server successfully'); + + // Make sure server group is created + let cmsResources = await cmsService.getRegisteredServers(ownerUri, ''); + assert(cmsResources.registeredServerGroups.length === 1, 'The server group was added successfully'); + + // Should create a registered server under the group + let bdcServer = await getBdcServer(); + let bdcConnection = { + serverName: bdcServer.serverName, + userName: bdcServer.userName, + password: bdcServer.password, + authenticationType: bdcServer.authenticationTypeName, + database: bdcServer.database, + provider: bdcServer.provider, + version: bdcServer.version, + engineType: bdcServer.engineType, + options: { } + }; + let relativePath = cmsResources.registeredServerGroups[0].relativePath; + result = await cmsService.addRegisteredServer(ownerUri, relativePath, 'test_registered_server_2', 'test_description', bdcConnection); + assert(result === true, 'Registered server added to server group'); + + // Should remove the server group we added above + let deleteResult = await cmsService.removeServerGroup(ownerUri, '', 'test_group'); + assert(deleteResult === true, 'Server group deleted from CMS server successfully'); + }); + }); +} + diff --git a/extensions/integration-tests/src/utils.ts b/extensions/integration-tests/src/utils.ts index 43ed9ba9d0..daeac42589 100644 --- a/extensions/integration-tests/src/utils.ts +++ b/extensions/integration-tests/src/utils.ts @@ -8,7 +8,12 @@ import * as azdata from 'azdata'; import * as vscode from 'vscode'; import { TestServerProfile } from './testConfig'; -export async function connectToServer(server: TestServerProfile, timeout: number = 3000) { +/** + * @param server test connection profile + * @param timeout optional timeout parameter + * Returns connection id for a new connection + */ +export async function connectToServer(server: TestServerProfile, timeout: number = 3000): Promise { let connectionProfile: azdata.IConnectionProfile = { serverName: server.serverName, databaseName: server.database, @@ -31,6 +36,7 @@ export async function connectToServer(server: TestServerProfile, timeout: number //workaround //wait for OE to load await new Promise(c => setTimeout(c, timeout)); + return result.connectionId; } export async function ensureConnectionViewOpened() { diff --git a/src/sql/platform/connection/common/connectionManagement.ts b/src/sql/platform/connection/common/connectionManagement.ts index 4e163ea25a..13c7d92316 100644 --- a/src/sql/platform/connection/common/connectionManagement.ts +++ b/src/sql/platform/connection/common/connectionManagement.ts @@ -312,7 +312,8 @@ export interface IConnectableInput { export enum ConnectionType { default = 0, - editor = 1 + editor = 1, + temporary = 2 } export enum MetadataType { diff --git a/src/sql/workbench/api/node/mainThreadConnectionManagement.ts b/src/sql/workbench/api/node/mainThreadConnectionManagement.ts index 7aed6fb49f..d363d9f834 100644 --- a/src/sql/workbench/api/node/mainThreadConnectionManagement.ts +++ b/src/sql/workbench/api/node/mainThreadConnectionManagement.ts @@ -7,7 +7,7 @@ import { SqlExtHostContext, SqlMainContext, ExtHostConnectionManagementShape, Ma import * as azdata from 'azdata'; import { IExtHostContext } from 'vs/workbench/api/common/extHost.protocol'; import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers'; -import { IConnectionManagementService } from 'sql/platform/connection/common/connectionManagement'; +import { IConnectionManagementService, ConnectionType } from 'sql/platform/connection/common/connectionManagement'; import { IObjectExplorerService } from 'sql/workbench/services/objectExplorer/common/objectExplorerService'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import * as TaskUtilities from 'sql/workbench/common/taskUtilities'; @@ -60,15 +60,23 @@ export class MainThreadConnectionManagement implements MainThreadConnectionManag } public async $openConnectionDialog(providers: string[], initialConnectionProfile?: IConnectionProfile, connectionCompletionOptions?: azdata.IConnectionCompletionOptions): Promise { + // Here we default to ConnectionType.editor which saves the connecton in the connection store by default + let connectionType = ConnectionType.editor; + + // If the API call explicitly set saveConnection to false, set it to ConnectionType.extension + // which doesn't save the connection by default + if (connectionCompletionOptions && !connectionCompletionOptions.saveConnection) { + connectionType = ConnectionType.temporary; + } let connectionProfile = await this._connectionDialogService.openDialogAndWait(this._connectionManagementService, - { connectionType: 1, providers: providers }, initialConnectionProfile, undefined); + { connectionType: connectionType, providers: providers }, initialConnectionProfile, undefined); const connection = connectionProfile ? { connectionId: connectionProfile.id, options: connectionProfile.options, providerName: connectionProfile.providerName } : undefined; - if (connectionCompletionOptions) { + if (connectionCompletionOptions && connectionCompletionOptions.saveConnection) { // Somehow, connectionProfile.saveProfile is false even if initialConnectionProfile.saveProfile is true, reset the flag here. connectionProfile.saveProfile = initialConnectionProfile.saveProfile; await this._connectionManagementService.connectAndSaveProfile(connectionProfile, undefined, { @@ -79,7 +87,6 @@ export class MainThreadConnectionManagement implements MainThreadConnectionManag showFirewallRuleOnError: isUndefinedOrNull(connectionCompletionOptions.showFirewallRuleOnError) ? true : connectionCompletionOptions.showFirewallRuleOnError }); } - return connection; } diff --git a/src/sql/workbench/api/node/sqlExtHost.api.impl.ts b/src/sql/workbench/api/node/sqlExtHost.api.impl.ts index 9025de63c9..1e8edcfd49 100644 --- a/src/sql/workbench/api/node/sqlExtHost.api.impl.ts +++ b/src/sql/workbench/api/node/sqlExtHost.api.impl.ts @@ -121,8 +121,8 @@ export function createApiFactory( getUriForConnection(connectionId: string): Thenable { return extHostConnectionManagement.$getUriForConnection(connectionId); }, - connect(connectionProfile: azdata.IConnectionProfile): Thenable { - return extHostConnectionManagement.$connect(connectionProfile); + connect(connectionProfile: azdata.IConnectionProfile, saveConnection: boolean, showDashboard: boolean): Thenable { + return extHostConnectionManagement.$connect(connectionProfile, saveConnection, showDashboard); } }; diff --git a/src/sql/workbench/parts/dataExplorer/electron-browser/nodeActions.contribution.ts b/src/sql/workbench/parts/dataExplorer/electron-browser/nodeActions.contribution.ts index 544a8cc770..abb5f79861 100644 --- a/src/sql/workbench/parts/dataExplorer/electron-browser/nodeActions.contribution.ts +++ b/src/sql/workbench/parts/dataExplorer/electron-browser/nodeActions.contribution.ts @@ -55,5 +55,6 @@ MenuRegistry.appendMenuItem(MenuId.DataExplorerContext, { command: { id: REFRESH_COMMAND_ID, title: localize('refresh', 'Refresh') - } + }, + when: NodeContextKey.IsConnected }); diff --git a/src/sql/workbench/services/connection/browser/cmsConnectionWidget.ts b/src/sql/workbench/services/connection/browser/cmsConnectionWidget.ts index c1d862ea79..359b14d589 100644 --- a/src/sql/workbench/services/connection/browser/cmsConnectionWidget.ts +++ b/src/sql/workbench/services/connection/browser/cmsConnectionWidget.ts @@ -97,10 +97,18 @@ export class CmsConnectionWidget extends ConnectionWidget { let newAuthTypes = authTypeOption.categoryValues; // True when opening a CMS dialog to add a registered server if (authTypeChanged) { - // Need to filter out SQL Login because registered servers don't support it - newAuthTypes = authTypeOption.categoryValues.filter((option) => option.name !== AuthenticationType.SqlLogin); + // Registered Servers only support Integrated Auth + newAuthTypes = authTypeOption.categoryValues.filter((option) => option.name === AuthenticationType.Integrated); + this._authTypeSelectBox.setOptions(newAuthTypes.map(c => c.displayName), 0); authTypeOption.defaultValue = AuthenticationType.Integrated; - this._authTypeSelectBox.setOptions(newAuthTypes.map(c => c.displayName)); + } else { + // CMS supports all auth types + if (OS === OperatingSystem.Windows) { + authTypeOption.defaultValue = this.getAuthTypeDisplayName(AuthenticationType.Integrated); + } else { + authTypeOption.defaultValue = this.getAuthTypeDisplayName(AuthenticationType.SqlLogin); + } + this._authTypeSelectBox.setOptions(authTypeOption.categoryValues.map(c => c.displayName), 1); } } diff --git a/src/sql/workbench/services/connection/browser/connectionDialogService.ts b/src/sql/workbench/services/connection/browser/connectionDialogService.ts index b7804f014f..b5098fd07e 100644 --- a/src/sql/workbench/services/connection/browser/connectionDialogService.ts +++ b/src/sql/workbench/services/connection/browser/connectionDialogService.ts @@ -61,14 +61,14 @@ export class ConnectionDialogService implements IConnectionDialogService { _serviceBrand: any; private _connectionDialog: ConnectionDialogWidget; - private _connectionControllerMap: { [providerDisplayName: string]: IConnectionComponentController } = {}; + private _connectionControllerMap: { [providerName: string]: IConnectionComponentController } = {}; private _model: ConnectionProfile; private _params: INewConnectionParams; private _options: IConnectionCompletionOptions; private _inputModel: IConnectionProfile; private _providerNameToDisplayNameMap: { [providerDisplayName: string]: string } = {}; private _providerTypes: string[] = []; - private _currentProviderType: string = 'Microsoft SQL Server'; + private _currentProviderType: string = Constants.mssqlProviderName; private _previousProviderType: string = undefined; private _connecting: boolean = false; private _connectionErrorTitle = localize('connectionError', 'Connection error'); @@ -171,8 +171,8 @@ export class ConnectionDialogService implements IConnectionDialogService { profile.serverName = trim(profile.serverName); // append the port to the server name for SQL Server connections - if (this.getCurrentProviderName() === Constants.mssqlProviderName || - this.getCurrentProviderName() === Constants.cmsProviderName) { + if (this._currentProviderType === Constants.mssqlProviderName || + this._currentProviderType === Constants.cmsProviderName) { let portPropertyName: string = 'port'; let portOption: string = profile.options[portPropertyName]; if (portOption && portOption.indexOf(',') === -1) { @@ -235,14 +235,15 @@ export class ConnectionDialogService implements IConnectionDialogService { return Promise.resolve(); } let fromEditor = params && params.connectionType === ConnectionType.editor; + let isTemporaryConnection = params && params.connectionType === ConnectionType.temporary; let uri: string = undefined; if (fromEditor && params && params.input) { uri = params.input.uri; } let options: IConnectionCompletionOptions = this._options || { params: params, - saveTheConnection: true, - showDashboard: params && params.showDashboard !== undefined ? params.showDashboard : !fromEditor, + saveTheConnection: !isTemporaryConnection, + showDashboard: params && params.showDashboard !== undefined ? params.showDashboard : !fromEditor && !isTemporaryConnection, showConnectionDialogOnError: false, showFirewallRuleOnError: true }; @@ -269,7 +270,7 @@ export class ConnectionDialogService implements IConnectionDialogService { private get uiController(): IConnectionComponentController { // Find the provider name from the selected provider type, or throw an error if it does not correspond to a known provider - let providerName = this.getCurrentProviderName(); + let providerName = this._currentProviderType; if (!providerName) { throw Error('Invalid provider type'); } @@ -302,9 +303,23 @@ export class ConnectionDialogService implements IConnectionDialogService { private handleShowUiComponent(input: OnShowUIResponse) { if (input.selectedProviderType) { - this._currentProviderType = input.selectedProviderType; + // If the call is for specific providers + let isParamProvider: boolean = false; + if (this._params && this._params.providers) { + this._params.providers.forEach((provider) => { + if (input.selectedProviderType === this._providerNameToDisplayNameMap[provider]) { + isParamProvider = true; + this._currentProviderType = provider; + } + }); + } + if (!isParamProvider) { + this._currentProviderType = Object.keys(this._providerNameToDisplayNameMap).find((key) => + this._providerNameToDisplayNameMap[key] === input.selectedProviderType + ); + } } - this._model.providerName = this.getCurrentProviderName(); + this._model.providerName = this._currentProviderType; this._model = new ConnectionProfile(this._capabilitiesService, this._model); if (this._inputModel && this._inputModel.options) { @@ -339,9 +354,9 @@ export class ConnectionDialogService implements IConnectionDialogService { private updateModelServerCapabilities(model: IConnectionProfile) { this._model = this.createModel(model); if (this._model.providerName) { - this._currentProviderType = this._providerNameToDisplayNameMap[this._model.providerName]; + this._currentProviderType = this._model.providerName; if (this._connectionDialog) { - this._connectionDialog.updateProvider(this._currentProviderType); + this._connectionDialog.updateProvider(this._providerNameToDisplayNameMap[this._currentProviderType]); } } } @@ -404,7 +419,8 @@ export class ConnectionDialogService implements IConnectionDialogService { return new Promise((resolve, reject) => { this.updateModelServerCapabilities(model); // If connecting from a query editor set "save connection" to false - if (params && params.input && params.connectionType === ConnectionType.editor) { + if (params && (params.input && params.connectionType === ConnectionType.editor || + params.connectionType === ConnectionType.temporary)) { this._model.saveProfile = false; } @@ -416,7 +432,6 @@ export class ConnectionDialogService implements IConnectionDialogService { }); } - private doShowDialog(params: INewConnectionParams): Promise { if (!this._connectionDialog) { this._connectionDialog = this._instantiationService.createInstance(ConnectionDialogWidget, this._providerTypes, this._providerNameToDisplayNameMap[this._model.providerName], this._providerNameToDisplayNameMap); @@ -438,7 +453,7 @@ export class ConnectionDialogService implements IConnectionDialogService { // if provider changed if ((this._previousProviderType !== this._currentProviderType) || // or if currentProvider not set correctly yet - !(this._currentProviderType === Constants.cmsProviderDisplayName && params.providers && params.providers.length > 1)) { + !(this._currentProviderType === Constants.cmsProviderName && params.providers && params.providers.length > 1)) { this._previousProviderType = undefined; this._connectionDialog.updateProvider(this._providerNameToDisplayNameMap[this.getDefaultProviderName()]); } else { @@ -450,12 +465,6 @@ export class ConnectionDialogService implements IConnectionDialogService { }); } - private getCurrentProviderName(): string { - return Object.keys(this._providerNameToDisplayNameMap).find(providerName => { - return this._currentProviderType === this._providerNameToDisplayNameMap[providerName]; - }); - } - private showErrorDialog(severity: Severity, headerTitle: string, message: string, messageDetails?: string): void { // Kerberos errors are currently very hard to understand, so adding handling of these to solve the common scenario // note that ideally we would have an extensible service to handle errors by error code and provider, but for now diff --git a/src/sql/workbench/services/connection/browser/connectionDialogWidget.ts b/src/sql/workbench/services/connection/browser/connectionDialogWidget.ts index b646b5b2fa..5e103556da 100644 --- a/src/sql/workbench/services/connection/browser/connectionDialogWidget.ts +++ b/src/sql/workbench/services/connection/browser/connectionDialogWidget.ts @@ -119,12 +119,17 @@ export class ConnectionDialogWidget extends Modal { if (this._newConnectionParams && this._newConnectionParams.providers) { const validProviderNames = Object.keys(this.providerNameToDisplayNameMap).filter(x => this.includeProvider(x, this._newConnectionParams)); if (validProviderNames && validProviderNames.length > 0) { - filteredProviderTypes = filteredProviderTypes.filter(x => validProviderNames.find(v => this.providerNameToDisplayNameMap[v] === x) !== undefined); + filteredProviderTypes = filteredProviderTypes.filter(x => validProviderNames.find( + v => this.providerNameToDisplayNameMap[v] === x) !== undefined + ); } } else { - filteredProviderTypes = filteredProviderTypes.filter(x => x !== Constants.cmsProviderDisplayName); + filteredProviderTypes = filteredProviderTypes.filter(x => x !== Constants.cmsProviderName); } - this._providerTypeSelectBox.setOptions(filteredProviderTypes); + this._providerTypeSelectBox.setOptions(filteredProviderTypes.filter((providerType, index) => + // Remove duplicate listings + filteredProviderTypes.indexOf(providerType) === index) + ); } private includeProvider(providerName: string, params?: INewConnectionParams): Boolean {