Fixes/improvements for SsmsMin extension (#5495)

* Fix server name in URN for Azure servers and update menu when clauses

* Use setStatusBarMessage instead of constructing our own. Remove server name from URN - it's not necessary.

* Clean up unused code
This commit is contained in:
Charles Gagnon
2019-05-17 09:38:15 -07:00
committed by GitHub
parent fe17955fa1
commit 94061fa634
3 changed files with 24 additions and 36 deletions

View File

@@ -55,7 +55,12 @@
}, },
{ {
"command": "adminToolExtWin.launchSsmsMinPropertiesDialog", "command": "adminToolExtWin.launchSsmsMinPropertiesDialog",
"when": "isWindows && connectionProvider == MSSQL && nodeType && nodeType =~ /^(Server|Database|Table|Column|Index|Statistic|View|ServerLevelLogin|ServerLevelServerRole|ServerLevelCredential|ServerLevelServerAudit|ServerLevelServerAuditSpecification|StoredProcedure|ScalarValuedFunction|TableValuedFunction|AggregateFunction|Synonym|Assembly|UserDefinedDataType|UserDefinedType|UserDefinedTableType|Sequence|User|DatabaseRole|ApplicationRole|Schema|SecurityPolicy|ServerLevelLinkedServer)$/", "when": "isWindows && connectionProvider == MSSQL && serverInfo && !isCloud && nodeType && nodeType == Server",
"group": "z-AdminToolExt@2"
},
{
"command": "adminToolExtWin.launchSsmsMinPropertiesDialog",
"when": "isWindows && connectionProvider == MSSQL && serverInfo && nodeType && nodeType =~ /^(Database|Table|Column|Index|Statistic|View|ServerLevelLogin|ServerLevelServerRole|ServerLevelCredential|ServerLevelServerAudit|ServerLevelServerAuditSpecification|StoredProcedure|ScalarValuedFunction|TableValuedFunction|AggregateFunction|Synonym|Assembly|UserDefinedDataType|UserDefinedType|UserDefinedTableType|Sequence|User|DatabaseRole|ApplicationRole|Schema|SecurityPolicy|ServerLevelLinkedServer)$/",
"group": "z-AdminToolExt@2" "group": "z-AdminToolExt@2"
} }
] ]

View File

@@ -10,7 +10,6 @@ import * as vscode from 'vscode';
import { Telemetry } from './telemetry'; import { Telemetry } from './telemetry';
import { doubleEscapeSingleQuotes, backEscapeDoubleQuotes } from './utils'; import { doubleEscapeSingleQuotes, backEscapeDoubleQuotes } from './utils';
import { ChildProcess, exec } from 'child_process'; import { ChildProcess, exec } from 'child_process';
const localize = nls.loadMessageBundle(); const localize = nls.loadMessageBundle();
const ssmsMinVer = JSON.parse(JSON.stringify(require('./config.json'))).version; const ssmsMinVer = JSON.parse(JSON.stringify(require('./config.json'))).version;
@@ -144,23 +143,6 @@ async function launchSsmsDialog(action: string, connectionContext: azdata.Object
return; return;
} }
// Currently Azure isn't supported by the SSMS server properties dialog
const serverInfo = await azdata.connection.getServerInfo(connectionContext.connectionProfile.id);
if (serverInfo && serverInfo.isCloud) {
vscode.window.showErrorMessage(localize('adminToolExtWin.invalidEngineType', 'This option is not currently available for this engine type.'));
return;
}
// Note - this is a temporary fix for the issue that currently the connection API doesn't allow retrieving credentials for a disconnected
// node. So until that's fixed we'll prevent users from attempting to launch SsmsMin on a disconnected node.
// We also aren't able to hide the menu item on disconnected nodes because we currently don't have a contextKey for the connected status
// of a node.
const activeConnections = await azdata.connection.getActiveConnections();
if (!activeConnections.some(conn => conn.connectionId === connectionContext.connectionProfile.id)) {
vscode.window.showErrorMessage(localize('adminToolExtWin.notConnected', 'This option requires a connected node - please connect and try again.'));
return;
}
let oeNode: azdata.objectexplorer.ObjectExplorerNode; let oeNode: azdata.objectexplorer.ObjectExplorerNode;
// Server node is a Connection node and so doesn't have the NodeInfo // Server node is a Connection node and so doesn't have the NodeInfo
if (connectionContext.isConnectionNode) { if (connectionContext.isConnectionNode) {
@@ -174,7 +156,7 @@ async function launchSsmsDialog(action: string, connectionContext: azdata.Object
return; return;
} }
const urn: string = await buildUrn(connectionContext.connectionProfile.serverName, oeNode); const urn: string = await buildUrn(oeNode);
let password: string = connectionContext.connectionProfile.password; let password: string = connectionContext.connectionProfile.password;
if (!password || password === '') { if (!password || password === '') {
@@ -194,6 +176,9 @@ async function launchSsmsDialog(action: string, connectionContext: azdata.Object
const args = buildSsmsMinCommandArgs(params); const args = buildSsmsMinCommandArgs(params);
Telemetry.sendTelemetryEvent('LaunchSsmsDialog', { 'action': action }); Telemetry.sendTelemetryEvent('LaunchSsmsDialog', { 'action': action });
vscode.window.setStatusBarMessage(localize('adminToolExtWin.launchingDialogStatus', 'Launching dialog...'), 3000);
// This will be an async call since we pass in the callback // This will be an async call since we pass in the callback
const proc: ChildProcess = exec( const proc: ChildProcess = exec(
/*command*/ `"${exePath}" ${args}`, /*command*/ `"${exePath}" ${args}`,
@@ -232,17 +217,16 @@ export function buildSsmsMinCommandArgs(params: LaunchSsmsDialogParams): string
return `${params.action ? '-a "' + backEscapeDoubleQuotes(params.action) + '"' : ''}\ return `${params.action ? '-a "' + backEscapeDoubleQuotes(params.action) + '"' : ''}\
${params.server ? ' -S "' + backEscapeDoubleQuotes(params.server) + '"' : ''}\ ${params.server ? ' -S "' + backEscapeDoubleQuotes(params.server) + '"' : ''}\
${params.database ? ' -D "' + backEscapeDoubleQuotes(params.database) + '"' : ''}\ ${params.database ? ' -D "' + backEscapeDoubleQuotes(params.database) + '"' : ''}\
${params.useAad !== true && params.user ? ' -U "' + backEscapeDoubleQuotes(params.user) + '"' : ''}\ ${params.user ? ' -U "' + backEscapeDoubleQuotes(params.user) + '"' : ''}\
${params.useAad === true ? ' -G' : ''}\ ${params.useAad === true ? ' -G' : ''}\
${params.urn ? ' -u "' + backEscapeDoubleQuotes(params.urn) + '"' : ''}`; ${params.urn ? ' -u "' + backEscapeDoubleQuotes(params.urn) + '"' : ''}`;
} }
/** /**
* Builds the URN string for a given ObjectExplorerNode in the form understood by SsmsMin * Builds the URN string for a given ObjectExplorerNode in the form understood by SsmsMin
* @param serverName The name of the Server to use for the Server segment
* @param node The node to get the URN of * @param node The node to get the URN of
*/ */
export async function buildUrn(serverName: string, node: azdata.objectexplorer.ObjectExplorerNode): Promise<string> { export async function buildUrn(node: azdata.objectexplorer.ObjectExplorerNode): Promise<string> {
let urnNodes: string[] = []; let urnNodes: string[] = [];
while (node) { while (node) {
// Server is special since it's a connection node - always add it as the root // Server is special since it's a connection node - always add it as the root
@@ -258,5 +242,6 @@ export async function buildUrn(serverName: string, node: azdata.objectexplorer.O
} }
node = await node.getParent(); node = await node.getParent();
} }
return [`Server[@Name='${doubleEscapeSingleQuotes(serverName)}']`].concat(urnNodes).join('/');
return ['Server'].concat(urnNodes).join('/');
} }

View File

@@ -36,8 +36,8 @@ describe('buildSsmsMinCommandArgs Method Tests', () => {
urn: 'Server\\Database\\Table' urn: 'Server\\Database\\Table'
}; };
const args = buildSsmsMinCommandArgs(params); const args = buildSsmsMinCommandArgs(params);
// User is omitted since UseAAD is true
should(args).equal('-a "myAction" -S "myServer" -D "myDatabase" -G -u "Server\\Database\\Table"'); should(args).equal('-a "myAction" -S "myServer" -D "myDatabase" -U "user" -G -u "Server\\Database\\Table"');
}); });
it('Should be built correctly and names escaped correctly', function (): void { it('Should be built correctly and names escaped correctly', function (): void {
@@ -50,8 +50,8 @@ describe('buildSsmsMinCommandArgs Method Tests', () => {
urn: 'Server\\Database[\'myDatabase\'\'"/\\[]tricky\']\\Table["myTable\'""/\\[]tricky"]' urn: 'Server\\Database[\'myDatabase\'\'"/\\[]tricky\']\\Table["myTable\'""/\\[]tricky"]'
}; };
const args = buildSsmsMinCommandArgs(params); const args = buildSsmsMinCommandArgs(params);
// User is omitted since UseAAD is true
should(args).equal('-a "myAction\'\\"/\\[]tricky" -S "myServer\'\\"/\\[]tricky" -D "myDatabase\'\\"/\\[]tricky" -G -u "Server\\Database[\'myDatabase\'\'\\"/\\[]tricky\']\\Table[\\"myTable\'\\"\\"/\\[]tricky\\"]"'); should(args).equal('-a "myAction\'\\"/\\[]tricky" -S "myServer\'\\"/\\[]tricky" -D "myDatabase\'\\"/\\[]tricky" -U "user\'\\"/\\[]tricky" -G -u "Server\\Database[\'myDatabase\'\'\\"/\\[]tricky\']\\Table[\\"myTable\'\\"\\"/\\[]tricky\\"]"');
}); });
it('Should be built correctly with only action and server', function (): void { it('Should be built correctly with only action and server', function (): void {
@@ -65,8 +65,6 @@ describe('buildSsmsMinCommandArgs Method Tests', () => {
}); });
}); });
const serverName = 'My\'Server';
const escapedServerName = doubleEscapeSingleQuotes(serverName);
const dbName = 'My\'Db'; const dbName = 'My\'Db';
const escapedDbName = doubleEscapeSingleQuotes(dbName); const escapedDbName = doubleEscapeSingleQuotes(dbName);
const dbSchema = 'db\'sch'; const dbSchema = 'db\'sch';
@@ -78,21 +76,21 @@ const escapedTableSchema = doubleEscapeSingleQuotes(tableSchema);
describe('buildUrn Method Tests', () => { describe('buildUrn Method Tests', () => {
it('Urn should be correct with just server', async function (): Promise<void> { it('Urn should be correct with just server', async function (): Promise<void> {
should(await buildUrn(serverName, undefined)).equal(`Server[@Name=\'${escapedServerName}\']`); should(await buildUrn(undefined)).equal('Server');
}); });
it('Urn should be correct with Server and only Databases folder', async function (): Promise<void> { it('Urn should be correct with Server and only Databases folder', async function (): Promise<void> {
const leafNode: ExtHostObjectExplorerNodeStub = const leafNode: ExtHostObjectExplorerNodeStub =
new ExtHostObjectExplorerNodeStub('Databases', undefined, 'Folder', undefined); new ExtHostObjectExplorerNodeStub('Databases', undefined, 'Folder', undefined);
should(await buildUrn(serverName, leafNode)).equal(`Server[@Name='${escapedServerName}']`); should(await buildUrn(leafNode)).equal('Server');
}); });
it('Urn should be correct with Server and Database node', async function (): Promise<void> { it('Urn should be correct with Server and Database node', async function (): Promise<void> {
const leafNode: ExtHostObjectExplorerNodeStub = const leafNode: ExtHostObjectExplorerNodeStub =
new ExtHostObjectExplorerNodeStub('Databases', undefined, 'Folder', undefined) new ExtHostObjectExplorerNodeStub('Databases', undefined, 'Folder', undefined)
.createChild(dbName, dbSchema, 'Database'); .createChild(dbName, dbSchema, 'Database');
should(await buildUrn(serverName, leafNode)).equal( should(await buildUrn(leafNode)).equal(
`Server[@Name='${escapedServerName}']/Database[@Name='${escapedDbName}' and @Schema='${escapedDbSchema}']`); `Server/Database[@Name='${escapedDbName}' and @Schema='${escapedDbSchema}']`);
}); });
it('Urn should be correct with Multiple levels of Nodes', async function (): Promise<void> { it('Urn should be correct with Multiple levels of Nodes', async function (): Promise<void> {
@@ -101,8 +99,8 @@ describe('buildUrn Method Tests', () => {
.createChild(dbName, dbSchema, 'Database') .createChild(dbName, dbSchema, 'Database')
.createChild('Tables', undefined, 'Folder') .createChild('Tables', undefined, 'Folder')
.createChild(tableName, tableSchema, 'Table'); .createChild(tableName, tableSchema, 'Table');
should(await buildUrn(serverName, rootNode)).equal( should(await buildUrn(rootNode)).equal(
`Server[@Name='${escapedServerName}']/Database[@Name='${escapedDbName}' and @Schema='${escapedDbSchema}']/Table[@Name='${escapedTableName}' and @Schema='${escapedTableSchema}']`); `Server/Database[@Name='${escapedDbName}' and @Schema='${escapedDbSchema}']/Table[@Name='${escapedTableName}' and @Schema='${escapedTableSchema}']`);
}); });
}); });