From a1ab53709496b5d7823f70e9522cd8a21762e8d6 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra <13396919+cheenamalhotra@users.noreply.github.com> Date: Wed, 7 Jun 2023 00:37:49 -0700 Subject: [PATCH] Fix broken native command line argument support (#23312) --- .../electron-browser/commandLine.ts | 68 ++----------------- .../test/electron-browser/commandLine.test.ts | 49 ++++++++++--- src/vs/platform/environment/common/argv.ts | 61 +++++++++++++++-- src/vs/platform/environment/node/argv.ts | 17 +++-- 4 files changed, 112 insertions(+), 83 deletions(-) diff --git a/src/sql/workbench/contrib/commandLine/electron-browser/commandLine.ts b/src/sql/workbench/contrib/commandLine/electron-browser/commandLine.ts index 41e5d8d594..39fac595fd 100644 --- a/src/sql/workbench/contrib/commandLine/electron-browser/commandLine.ts +++ b/src/sql/workbench/contrib/commandLine/electron-browser/commandLine.ts @@ -30,64 +30,6 @@ import { IDialogService } from 'vs/platform/dialogs/common/dialogs'; import { IEnvironmentService, INativeEnvironmentService } from 'vs/platform/environment/common/environment'; import { NativeParsedArgs } from 'vs/platform/environment/common/argv'; -export interface SqlArgs { - /** - * Used to determine file paths to be opened with SQL Editor. - * If provided, we connect the given profile to to it. - * More than one files can be passed to connect to provided profile. - */ - _?: string[]; - /** - * Provide authenticationType to be used. - * accepted values: AzureMFA, SqlLogin, Integrated, etc. - */ - authenticationType?: string - /** - * Name of database - */ - database?: string; - /** - * Name of server - */ - server?: string; - /** - * User name/email address - */ - user?: string; - /** - * Operation to perform: - * accepted values: connect, openConnectionDialog - */ - command?: string; - /** - * Name of connection provider, - * accepted values: mssql (by default), pgsql, etc. - */ - provider?: string; - /** - * Deprecated - used by SSMS - authenticationType should be used instead - */ - aad?: boolean; - /** - * Deprecated - used by SSMS - authenticationType should be used instead. - */ - integrated?: boolean; - /** - * Whether or not to show dashboard - * accepted values: true, false (by default). - */ - showDashboard?: boolean; - /** - * Supports providing applicationName that will be used for connection profile app name. - */ - applicationName?: string; - /** - * Supports providing advanced connection properties that providers support. - * Value must be a json object containing key-value pairs in format: '{"key1":"value1","key2":"value2",...}' - */ - connectionProperties?: string; -} - //#region decorators type PathHandler = (uri: URI) => Promise; @@ -153,7 +95,7 @@ export class CommandLineWorkbenchContribution implements IWorkbenchContribution, // (null, commandName) => Launch the command with a null connection. If the command implementation needs a connection, it will need to create it. // (serverName, null) => Connect object explorer and open a new query editor if no file names are passed. If file names are passed, connect their editors to the server. // (null, null) => Prompt for a connection unless there are registered servers - public async processCommandLine(args: SqlArgs): Promise { + public async processCommandLine(args: NativeParsedArgs): Promise { let profile: IConnectionProfile = undefined; let commandName = undefined; if (args) { @@ -296,7 +238,7 @@ export class CommandLineWorkbenchContribution implements IWorkbenchContribution, return true; } - private async confirmConnect(args: SqlArgs): Promise { + private async confirmConnect(args: NativeParsedArgs): Promise { let detail = args && args.server ? localize('connectServerDetail', "This will connect to server {0}", args.server) : ''; const result = await this.dialogService.confirm({ message: localize('confirmConnect', "Are you sure you want to connect?"), @@ -311,8 +253,8 @@ export class CommandLineWorkbenchContribution implements IWorkbenchContribution, return false; } - private parseProtocolArgs(uri: URI): SqlArgs { - let args: SqlArgs = querystring.parse(uri.query); + private parseProtocolArgs(uri: URI): NativeParsedArgs { + let args: NativeParsedArgs = querystring.parse(uri.query); // Clear out command, not supporting arbitrary command via this path args.command = undefined; return args; @@ -336,7 +278,7 @@ export class CommandLineWorkbenchContribution implements IWorkbenchContribution, } } - private readProfileFromArgs(args: SqlArgs) { + private readProfileFromArgs(args: NativeParsedArgs) { let profile = new ConnectionProfile(this._capabilitiesService, null); // We want connection store to use any matching password it finds profile.savePassword = true; diff --git a/src/sql/workbench/contrib/commandLine/test/electron-browser/commandLine.test.ts b/src/sql/workbench/contrib/commandLine/test/electron-browser/commandLine.test.ts index 41ebc82140..6717c65dff 100644 --- a/src/sql/workbench/contrib/commandLine/test/electron-browser/commandLine.test.ts +++ b/src/sql/workbench/contrib/commandLine/test/electron-browser/commandLine.test.ts @@ -8,7 +8,7 @@ import * as TypeMoq from 'typemoq'; import * as azdata from 'azdata'; import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile'; import { ConnectionProfileGroup } from 'sql/platform/connection/common/connectionProfileGroup'; -import { CommandLineWorkbenchContribution, SqlArgs } from 'sql/workbench/contrib/commandLine/electron-browser/commandLine'; +import { CommandLineWorkbenchContribution } from 'sql/workbench/contrib/commandLine/electron-browser/commandLine'; import * as Constants from 'sql/platform/connection/common/constants'; import { ICapabilitiesService } from 'sql/platform/capabilities/common/capabilitiesService'; import { TestCapabilitiesService } from 'sql/platform/capabilities/test/common/testCapabilitiesService'; @@ -35,11 +35,24 @@ import { FileQueryEditorInput } from 'sql/workbench/contrib/query/browser/fileQu import { TestDialogService } from 'vs/platform/dialogs/test/common/testDialogService'; import { NativeParsedArgs } from 'vs/platform/environment/common/argv'; -class TestParsedArgs implements NativeParsedArgs, SqlArgs { +class TestParsedArgs implements NativeParsedArgs { [arg: string]: any; _: string[]; - database?: string; + + // Start: SQL Args + aad?: boolean; + applicationName?: string; + authenticationType?: string; command?: string; + connectionProperties?: string; + database?: string; + integrated?: boolean; + provider?: string; + server?: string; + showDashboard?: boolean; + user?: string; + // End: SQL Args + debugBrkPluginHost?: string; debugBrkSearch?: string; debugId?: string; @@ -77,7 +90,6 @@ class TestParsedArgs implements NativeParsedArgs, SqlArgs { 'prof-startup'?: boolean; 'prof-startup-prefix'?: string; 'reuse-window'?: boolean; - server?: string; 'show-versions'?: boolean; 'skip-add-to-recently-opened'?: boolean; 'skip-getting-started'?: boolean; @@ -87,16 +99,12 @@ class TestParsedArgs implements NativeParsedArgs, SqlArgs { 'uninstall-extension'?: string[]; 'unity-launch'?: boolean; // Always open a new window, except if opening the first window or opening a file or folder as part of the launch. 'upload-logs'?: string; - user?: string; 'user-data-dir'?: string; _urls?: string[]; verbose?: boolean; version?: boolean; wait?: boolean; waitMarkerFilePath?: string; - authenticationType?: string; - applicationName?: string; - connectionProperties?: string; } suite('commandLineService tests', () => { @@ -220,6 +228,30 @@ suite('commandLineService tests', () => { connectionManagementService.verifyAll(); }); + test('processCommandLine shows dashboard when requested', async () => { + const connectionManagementService: TypeMoq.Mock + = TypeMoq.Mock.ofType(TestConnectionManagementService, TypeMoq.MockBehavior.Strict); + + const args: TestParsedArgs = new TestParsedArgs(); + args.server = 'myserver'; + args.database = 'mydatabase'; + args.user = 'myuser'; + args.showDashboard = true; + args.authenticationType = Constants.AuthenticationType.SqlLogin; + + connectionManagementService.setup((c) => c.showConnectionDialog()).verifiable(TypeMoq.Times.never()); + connectionManagementService.setup(c => c.showDashboard(TypeMoq.It.isAny())).verifiable(TypeMoq.Times.atMostOnce()); + connectionManagementService.setup(c => c.hasRegisteredServers()).returns(() => true).verifiable(TypeMoq.Times.atMostOnce()); + connectionManagementService.setup(c => c.getConnectionGroups(TypeMoq.It.isAny())).returns(() => []); + let originalProfile: IConnectionProfile = undefined; + connectionManagementService.setup(c => c.getConnectionProfileById(TypeMoq.It.isAnyString())).returns(() => originalProfile); + const configurationService = getConfigurationServiceMock(true); + const logService = new NullLogService(); + let contribution = getCommandLineContribution(connectionManagementService.object, configurationService.object, capabilitiesService, undefined, undefined, logService); + await contribution.processCommandLine(args); + connectionManagementService.verifyAll(); + }); + test('processCommandLine loads advanced options in args', async () => { const connectionManagementService: TypeMoq.Mock = TypeMoq.Mock.ofType(TestConnectionManagementService, TypeMoq.MockBehavior.Strict); @@ -284,7 +316,6 @@ suite('commandLineService tests', () => { test('processCommandLine invokes a command with a profile parameter when a server is passed', async () => { - const connectionManagementService: TypeMoq.Mock = TypeMoq.Mock.ofType(TestConnectionManagementService, TypeMoq.MockBehavior.Strict); diff --git a/src/vs/platform/environment/common/argv.ts b/src/vs/platform/environment/common/argv.ts index 981b28951b..be405dbcfb 100644 --- a/src/vs/platform/environment/common/argv.ts +++ b/src/vs/platform/environment/common/argv.ts @@ -7,7 +7,15 @@ * A list of command line arguments we support natively. */ export interface NativeParsedArgs { - _: string[]; + /** + * {{ SQL CARBON EDIT}} Start + * Optional for Azure Data Studio to support URI conversion. + * Used to determine file paths to be opened with SQL Editor. + * If provided, we connect the given profile to to it. + * More than one files can be passed to connect to provided profile. + */ + _?: string[]; + /** {{ SQL CARBON EDIT}} End */ 'folder-uri'?: string[]; // undefined or array of 1 or more 'file-uri'?: string[]; // undefined or array of 1 or more _urls?: string[]; @@ -93,12 +101,55 @@ export interface NativeParsedArgs { 'locate-shell-integration-path'?: string; // {{SQL CARBON EDIT}} Start + /** + * Deprecated - used by SSMS - authenticationType should be used instead + */ aad?: boolean; - database?: string; - integrated?: boolean; - server?: string; - user?: string; + /** + * Supports providing applicationName that will be used for connection profile app name. + */ + applicationName?: string; + /** + * Provide authenticationType to be used. + * accepted values: AzureMFA, SqlLogin, Integrated, etc. + */ + authenticationType?: string + /** + * Operation to perform: + * accepted values: connect, openConnectionDialog + */ command?: string; + /** + * Supports providing advanced connection properties that providers support. + * Value must be a json object containing key-value pairs in format: '{"key1":"value1","key2":"value2",...}' + */ + connectionProperties?: string; + /** + * Name of database + */ + database?: string; + /** + * Deprecated - used by SSMS - authenticationType should be used instead. + */ + integrated?: boolean; + /** + * Name of connection provider, + * accepted values: mssql (by default), pgsql, etc. + */ + provider?: string; + /** + * Name of server + */ + server?: string; + /** + * Whether or not to show dashboard + * accepted values: true, false (by default). + */ + showDashboard?: boolean; + /** + * User name/email address + */ + user?: string; // {{SQL CARBON EDIT}} End // chromium command line args: https://electronjs.org/docs/all#supported-chrome-command-line-switches diff --git a/src/vs/platform/environment/node/argv.ts b/src/vs/platform/environment/node/argv.ts index f551ace76a..ba09de3456 100644 --- a/src/vs/platform/environment/node/argv.ts +++ b/src/vs/platform/environment/node/argv.ts @@ -131,12 +131,17 @@ export const OPTIONS: OptionDescriptions> = { 'locate-shell-integration-path': { type: 'string', args: ['bash', 'pwsh', 'zsh'] }, // {{SQL CARBON EDIT}} Start - 'command': { type: 'string', alias: 'c', cat: 'o', args: 'command-name', description: localize('commandParameter', 'Name of command to run') }, - 'database': { type: 'string', alias: 'D', cat: 'o', args: 'database', description: localize('databaseParameter', 'Database name') }, - 'server': { type: 'string', alias: 'S', cat: 'o', args: 'server', description: localize('serverParameter', 'Server name') }, - 'user': { type: 'string', alias: 'U', cat: 'o', args: 'user-name', description: localize('userParameter', 'User name for server connection') }, - 'aad': { type: 'boolean', cat: 'o', description: localize('aadParameter', 'Use Azure Active Directory authentication for server connection') }, - 'integrated': { type: 'boolean', alias: 'E', cat: 'o', description: localize('integratedAuthParameter', 'Use Integrated authentication for server connection') }, + 'aad': { type: 'boolean', cat: 'o', description: localize('aadParameter', 'Use Azure Active Directory authentication, this option is depcrecated - use \'authenticationType\' instead.') }, + 'applicationName': { type: 'string', alias: 'A', cat: 'o', allowEmptyValue: true, description: localize('applicationNameParameter', 'Supports providing applicationName that will be used for connection profile app name.') }, + 'authenticationType': { type: 'string', alias: 'T', cat: 'o', allowEmptyValue: true, deprecates: ['aad', 'integrated'], description: localize('authenticationTypeParameter', 'Provide authentication mode to be used. Accepted values: AzureMFA, SqlLogin, Integrated, etc.') }, + 'command': { type: 'string', alias: 'c', cat: 'o', args: 'command-name', allowEmptyValue: true, description: localize('commandParameter', 'Name of command to run, accepted values: connect, openConnectionDialog') }, + 'connectionProperties': { type: 'string', alias: 'Z', cat: 'o', allowEmptyValue: true, description: localize('connectionPropsParameter', `Supports providing advanced connection properties that providers support. Value must be a json object containing key-value pairs in format: '{"key1":"value1"}'`) }, + 'database': { type: 'string', alias: 'D', cat: 'o', args: 'database', allowEmptyValue: true, description: localize('databaseParameter', 'Name of database') }, + 'integrated': { type: 'boolean', alias: 'E', cat: 'o', description: localize('integratedAuthParameter', 'Use Integrated authentication, this option is depcrecated - use \'authenticationType\' instead.') }, + 'provider': { type: 'string', alias: 'P', cat: 'o', allowEmptyValue: true, description: localize('providerParameter', 'Connection provider to use, e.g. MSSQL, PGSQL, etc. ') }, + 'server': { type: 'string', alias: 'S', cat: 'o', args: 'server', allowEmptyValue: true, description: localize('serverParameter', 'Name of target server or host name.') }, + 'showDashboard': { type: 'boolean', cat: 'o', description: localize('showDashboardParameter', 'Whether or not to show dashboard on connection, false by default.') }, + 'user': { type: 'string', alias: 'U', cat: 'o', args: 'user-name', allowEmptyValue: true, description: localize('userParameter', 'User name/email address') }, // {{SQL CARBON EDIT}} - End // chromium flags