From 9ec68087ace7947f5f49c6b5b4a7c8f2b6197e7e Mon Sep 17 00:00:00 2001 From: Alan Ren Date: Thu, 11 Aug 2022 10:26:55 -0700 Subject: [PATCH] allow database name to be empty (#20221) * allow database to be empty * test changes * fix import * fix test cases * comment --- .../ui/editableDropdown/browser/dropdown.ts | 4 +++ .../contrib/query/browser/queryActions.ts | 26 +++++++++++++++---- .../query/test/browser/queryActions.test.ts | 23 ++++++++++------ .../query/test/browser/queryEditor.test.ts | 4 +-- 4 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/sql/base/browser/ui/editableDropdown/browser/dropdown.ts b/src/sql/base/browser/ui/editableDropdown/browser/dropdown.ts index 2d5127c302..23701e19d3 100644 --- a/src/sql/base/browser/ui/editableDropdown/browser/dropdown.ts +++ b/src/sql/base/browser/ui/editableDropdown/browser/dropdown.ts @@ -420,4 +420,8 @@ export class Dropdown extends Disposable implements IListVirtualDelegate public get selectList(): List { return this._selectList; } + + public get options(): IDropdownOptions { + return this._options; + } } diff --git a/src/sql/workbench/contrib/query/browser/queryActions.ts b/src/sql/workbench/contrib/query/browser/queryActions.ts index 1fc85d820b..9e97ff5384 100644 --- a/src/sql/workbench/contrib/query/browser/queryActions.ts +++ b/src/sql/workbench/contrib/query/browser/queryActions.ts @@ -32,7 +32,7 @@ import { Task } from 'sql/workbench/services/tasks/browser/tasksRegistry'; import { IObjectExplorerService } from 'sql/workbench/services/objectExplorer/browser/objectExplorerService'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { IQueryEditorService } from 'sql/workbench/services/queryEditor/common/queryEditorService'; -import { IConnectionProfile } from 'sql/platform/connection/common/interfaces'; +import { ConnectionOptionSpecialType, IConnectionProfile } from 'sql/platform/connection/common/interfaces'; import { getCurrentGlobalConnection } from 'sql/workbench/browser/taskUtilities'; import { ServicesAccessor, IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { CommandsRegistry, ICommandService } from 'vs/platform/commands/common/commands'; @@ -655,7 +655,8 @@ export class ListDatabasesActionItem extends Disposable implements IActionViewIt @IContextViewService contextViewProvider: IContextViewService, @IConnectionManagementService private readonly connectionManagementService: IConnectionManagementService, @INotificationService private readonly notificationService: INotificationService, - @ILogService private readonly logService: ILogService + @ILogService private readonly logService: ILogService, + @ICapabilitiesService private readonly capabilitiesService: ICapabilitiesService ) { super(); this._databaseListDropdown = $('.databaseListDropdown'); @@ -751,6 +752,8 @@ export class ListDatabasesActionItem extends Disposable implements IActionViewIt severity: Severity.Error, message: nls.localize('changeDatabase.failed', "Failed to change database") }); + } else { + this._dropdown.options.strictSelection = true; } }, error => { @@ -860,9 +863,16 @@ export class ListDatabasesActionItem extends Disposable implements IActionViewIt } private updateConnection(databaseName: string): void { - // Ignore if the database name is not provided, this happens when the query editor connection is changed to - // a provider that does not support database. - if (!databaseName) { + if (!this._editor?.input) { + return; + } + const profile = this.connectionManagementService.getConnectionProfile(this._editor.input.uri); + if (!profile) { + return; + } + const supportDatabase = !!(this.capabilitiesService.getCapabilities(profile.providerName).connection.connectionOptions?.find(option => option.specialValueType === ConnectionOptionSpecialType.databaseName)); + // Ignore if the provider does not support database. + if (!supportDatabase) { return; } this._isConnected = true; @@ -873,6 +883,12 @@ export class ListDatabasesActionItem extends Disposable implements IActionViewIt this._dropdown.value = databaseName; this._dropdown.values = [databaseName]; this._dropdown.enabled = true; + + // Set the strict selection to false so that it is allowed to not to have a database selected to begin with. + // e.g. MySQL allows server level query. + if (!databaseName) { + this._dropdown.options.strictSelection = false; + } this.getDatabaseNames().then(databaseNames => { this._dropdown.values = databaseNames; }).catch(onUnexpectedError); diff --git a/src/sql/workbench/contrib/query/test/browser/queryActions.test.ts b/src/sql/workbench/contrib/query/test/browser/queryActions.test.ts index c51302925c..0d0119e6d2 100644 --- a/src/sql/workbench/contrib/query/test/browser/queryActions.test.ts +++ b/src/sql/workbench/contrib/query/test/browser/queryActions.test.ts @@ -35,6 +35,8 @@ import { TestStorageService } from 'vs/workbench/test/common/workbenchTestServic import { IRange } from 'vs/editor/common/core/range'; import { ServerInfo } from 'azdata'; import { QueryEditorState } from 'sql/workbench/common/editor/query/queryEditorInput'; +import { TestCapabilitiesService } from 'sql/platform/capabilities/test/common/testCapabilitiesService'; +import { mssqlProviderName } from 'sql/platform/connection/common/constants'; suite('SQL QueryAction Tests', () => { @@ -46,6 +48,7 @@ suite('SQL QueryAction Tests', () => { let configurationService: TypeMoq.Mock; let queryModelService: TypeMoq.Mock; let connectionManagementService: TypeMoq.Mock; + let capabilitiesService: TestCapabilitiesService; setup(() => { @@ -86,6 +89,7 @@ suite('SQL QueryAction Tests', () => { testQueryInput.setup(x => x.uri).returns(() => testUri); testQueryInput.setup(x => x.runQuery(undefined)).callback(() => { calledRunQueryOnInput = true; }); testQueryInput.setup(x => x.state).returns(() => testQueryInputState.object); + capabilitiesService = new TestCapabilitiesService(); }); test('setClass sets child CSS class correctly', () => { @@ -486,12 +490,13 @@ suite('SQL QueryAction Tests', () => { connectionManagementService.setup(x => x.getServerInfo(TypeMoq.It.isAny())).returns(() => { serverMajorVersion: 12, serverEdition: 'Test' }); connectionManagementService.setup(x => x.onConnectionChanged).returns(() => Event.None); connectionManagementService.setup(x => x.getConnectionProfile(TypeMoq.It.isAny())).returns(() => { - databaseName: databaseName + databaseName: databaseName, + providerName: mssqlProviderName }); connectionManagementService.setup(x => x.changeDatabase(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString())).returns(() => Promise.resolve(true)); // If I query without having initialized anything, state should be clear - listItem = new ListDatabasesActionItem(editor.object, undefined, undefined, connectionManagementService.object, undefined, undefined); + listItem = new ListDatabasesActionItem(editor.object, undefined, undefined, connectionManagementService.object, undefined, undefined, capabilitiesService); assert.strictEqual(listItem.isEnabled(), false, 'do not expect dropdown enabled unless connected'); assert.strictEqual(listItem.currentDatabaseName, undefined, 'do not expect dropdown to have entries unless connected'); @@ -520,11 +525,11 @@ suite('SQL QueryAction Tests', () => { let databaseName = 'foobar'; connectionManagementService.setup(x => x.onConnectionChanged).returns(() => dbChangedEmitter.event); connectionManagementService.setup(x => x.getServerInfo(TypeMoq.It.isAny())).returns(() => { serverMajorVersion: 12, serverEdition: 'Test' }); - connectionManagementService.setup(x => x.getConnectionProfile(TypeMoq.It.isAny())).returns(() => { databaseName: databaseName }); + connectionManagementService.setup(x => x.getConnectionProfile(TypeMoq.It.isAny())).returns(() => { databaseName: databaseName, providerName: mssqlProviderName }); connectionManagementService.setup(x => x.changeDatabase(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString())).returns(() => Promise.resolve(true)); // ... Create a database dropdown that has been connected - let listItem = new ListDatabasesActionItem(editor.object, undefined, undefined, connectionManagementService.object, undefined, undefined); + let listItem = new ListDatabasesActionItem(editor.object, undefined, undefined, connectionManagementService.object, undefined, undefined, capabilitiesService); listItem.onConnected(); // If: I raise a connection changed event @@ -544,11 +549,11 @@ suite('SQL QueryAction Tests', () => { let databaseName = 'foobar'; connectionManagementService.setup(x => x.onConnectionChanged).returns(() => dbChangedEmitter.event); connectionManagementService.setup(x => x.getServerInfo(TypeMoq.It.isAny())).returns(() => { serverMajorVersion: 12, serverEdition: 'Test' }); - connectionManagementService.setup(x => x.getConnectionProfile(TypeMoq.It.isAny())).returns(() => { databaseName: databaseName }); + connectionManagementService.setup(x => x.getConnectionProfile(TypeMoq.It.isAny())).returns(() => { databaseName: databaseName, providerName: mssqlProviderName }); connectionManagementService.setup(x => x.changeDatabase(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString())).returns(() => Promise.resolve(true)); // ... Create a database dropdown that has been connected - let listItem = new ListDatabasesActionItem(editor.object, undefined, undefined, connectionManagementService.object, undefined, undefined); + let listItem = new ListDatabasesActionItem(editor.object, undefined, undefined, connectionManagementService.object, undefined, undefined, capabilitiesService); listItem.onConnected(); // If: I raise a connection changed event for the 'wrong' URI @@ -571,14 +576,16 @@ suite('SQL QueryAction Tests', () => { // ... Create mock connection management service connectionManagementService.setup(x => x.onConnectionChanged).returns(() => dbChangedEmitter.event); + connectionManagementService.setup(x => x.getConnectionProfile(TypeMoq.It.isAny())).returns(() => { databaseName: 'foobarbaz', providerName: mssqlProviderName }); // ... Create a database dropdown - let listItem = new ListDatabasesActionItem(editor.object, undefined, undefined, connectionManagementService.object, undefined, undefined); + let listItem = new ListDatabasesActionItem(editor.object, undefined, undefined, connectionManagementService.object, undefined, undefined, capabilitiesService); // If: I raise a connection changed event let eventParams = { connectionProfile: { - databaseName: 'foobarbaz' + databaseName: 'foobarbaz', + providerName: mssqlProviderName }, connectionUri: editor.object.input.uri }; diff --git a/src/sql/workbench/contrib/query/test/browser/queryEditor.test.ts b/src/sql/workbench/contrib/query/test/browser/queryEditor.test.ts index ef862bdd71..1c95b6b867 100644 --- a/src/sql/workbench/contrib/query/test/browser/queryEditor.test.ts +++ b/src/sql/workbench/contrib/query/test/browser/queryEditor.test.ts @@ -64,7 +64,7 @@ suite('SQL QueryEditor Tests', () => { instantiationService.setup(x => x.createInstance(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns((classDef, editor, action) => { if (classDef.ID) { if (classDef.ID === 'listDatabaseQueryActionItem') { - return new ListDatabasesActionItem(editor, action, undefined, connectionManagementService.object, undefined, undefined); + return new ListDatabasesActionItem(editor, action, undefined, connectionManagementService.object, undefined, undefined, undefined); } } // Default @@ -292,7 +292,7 @@ suite('SQL QueryEditor Tests', () => { queryActionInstantiationService.setup(x => x.createInstance(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())) .returns((definition, editor, action, selectBox) => { if (definition.ID === 'listDatabaseQueryActionItem') { - let item = new ListDatabasesActionItem(editor, action, undefined, connectionManagementService.object, undefined, undefined); + let item = new ListDatabasesActionItem(editor, action, undefined, connectionManagementService.object, undefined, undefined, undefined); return item; } // Default