diff --git a/extensions/arc/src/common/utils.ts b/extensions/arc/src/common/utils.ts index 92bc63db5c..8338313a6f 100644 --- a/extensions/arc/src/common/utils.ts +++ b/extensions/arc/src/common/utils.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { ResourceType } from 'arc'; +import * as azdata from 'azdata'; import * as azurecore from 'azurecore'; import * as vscode from 'vscode'; import { ConnectionMode, IconPath, IconPathHelper } from '../constants'; @@ -300,6 +301,13 @@ export function convertToGibibyteString(value: string): string { return String(floatValue); } +/** + * Used to confirm if object is an azdata CheckBoxComponent + */ +export function instanceOfCheckBox(object: any): object is azdata.CheckBoxComponent { + return 'checked' in object; +} + /* * Throws an Error with given {@link message} unless {@link condition} is true. * This also tells the typescript compiler that the condition is 'truthy' in the remainder of the scope diff --git a/extensions/arc/src/models/postgresModel.ts b/extensions/arc/src/models/postgresModel.ts index a287abc45f..b4b2c3407b 100644 --- a/extensions/arc/src/models/postgresModel.ts +++ b/extensions/arc/src/models/postgresModel.ts @@ -33,13 +33,12 @@ export class PostgresModel extends ResourceModel { private readonly _azdataApi: azdataExt.IExtension; private readonly _onConfigUpdated = new vscode.EventEmitter(); - public readonly _onEngineSettingsUpdated = new vscode.EventEmitter(); public onConfigUpdated = this._onConfigUpdated.event; - public onEngineSettingsUpdated = this._onEngineSettingsUpdated.event; public configLastUpdated?: Date; public engineSettingsLastUpdated?: Date; private _refreshPromise?: Deferred; + private _engineSettingsPromise?: Deferred; constructor(_controllerModel: ControllerModel, private _pgInfo: PGResourceInfo, registration: Registration, private _treeDataProvider: AzureArcTreeDataProvider) { super(_controllerModel, _pgInfo, registration); @@ -132,55 +131,69 @@ export class PostgresModel extends ResourceModel { } public async getEngineSettings(): Promise { - if (!this._connectionProfile) { - await this.getConnectionProfile(); + // Only allow to get engine setting once at a time + if (this._engineSettingsPromise) { + return this._engineSettingsPromise.promise; } + this._engineSettingsPromise = new Deferred(); - // We haven't connected yet so do so now and then store the ID for the active connection - if (!this._activeConnectionId) { - const result = await azdata.connection.connect(this._connectionProfile!, false, false); - if (!result.connected) { - throw new Error(result.errorMessage); + try { + if (!this._connectionProfile) { + await this.getConnectionProfile(); } - this._activeConnectionId = result.connectionId; - } - // TODO Need to make separate calls for worker nodes and coordinator node - const provider = azdata.dataprotocol.getProvider(this._connectionProfile!.providerName, azdata.DataProviderType.QueryProvider); - const ownerUri = await azdata.connection.getUriForConnection(this._activeConnectionId); - - const engineSettings = await provider.runQueryAndReturn(ownerUri, 'select name, setting, short_desc,min_val, max_val, enumvals, vartype from pg_settings'); - if (!engineSettings) { - throw new Error('Could not fetch engine settings'); - } - - const skippedEngineSettings: String[] = [ - 'archive_command', 'archive_timeout', 'log_directory', 'log_file_mode', 'log_filename', 'restore_command', - 'shared_preload_libraries', 'synchronous_commit', 'ssl', 'unix_socket_permissions', 'wal_level' - ]; - - this.workerNodesEngineSettings = []; - - engineSettings.rows.forEach(row => { - let rowValues = row.map(c => c.displayValue); - let name = rowValues.shift(); - if (!skippedEngineSettings.includes(name!)) { - let result: EngineSettingsModel = { - parameterName: name, - value: rowValues.shift(), - description: rowValues.shift(), - min: rowValues.shift(), - max: rowValues.shift(), - options: rowValues.shift(), - type: rowValues.shift() - }; - - this.workerNodesEngineSettings.push(result); + // We haven't connected yet so do so now and then store the ID for the active connection + if (!this._activeConnectionId) { + const result = await azdata.connection.connect(this._connectionProfile!, false, false); + if (!result.connected) { + throw new Error(result.errorMessage); + } + this._activeConnectionId = result.connectionId; } - }); - this.engineSettingsLastUpdated = new Date(); - this._onEngineSettingsUpdated.fire(this.workerNodesEngineSettings); + // TODO Need to make separate calls for worker nodes and coordinator node + const provider = azdata.dataprotocol.getProvider(this._connectionProfile!.providerName, azdata.DataProviderType.QueryProvider); + const ownerUri = await azdata.connection.getUriForConnection(this._activeConnectionId); + + const engineSettings = await provider.runQueryAndReturn(ownerUri, 'select name, setting, short_desc,min_val, max_val, enumvals, vartype from pg_settings'); + if (!engineSettings) { + throw new Error('Could not fetch engine settings'); + } + + const skippedEngineSettings: String[] = [ + 'archive_command', 'archive_timeout', 'log_directory', 'log_file_mode', 'log_filename', 'restore_command', + 'shared_preload_libraries', 'synchronous_commit', 'ssl', 'unix_socket_permissions', 'wal_level' + ]; + + this.workerNodesEngineSettings = []; + + engineSettings.rows.forEach(row => { + let rowValues = row.map(c => c.displayValue); + let name = rowValues.shift(); + if (!skippedEngineSettings.includes(name!)) { + let result: EngineSettingsModel = { + parameterName: name, + value: rowValues.shift(), + description: rowValues.shift(), + min: rowValues.shift(), + max: rowValues.shift(), + options: rowValues.shift(), + type: rowValues.shift() + }; + + this.workerNodesEngineSettings.push(result); + } + }); + + + this.engineSettingsLastUpdated = new Date(); + this._engineSettingsPromise.resolve(); + } catch (err) { + this._engineSettingsPromise.reject(err); + throw err; + } finally { + this._engineSettingsPromise = undefined; + } } protected createConnectionProfile(): azdata.IConnectionProfile { diff --git a/extensions/arc/src/test/models/postgresModel.test.ts b/extensions/arc/src/test/models/postgresModel.test.ts index 685d32e0a9..9d3054a8e2 100644 --- a/extensions/arc/src/test/models/postgresModel.test.ts +++ b/extensions/arc/src/test/models/postgresModel.test.ts @@ -462,29 +462,6 @@ describe('PostgresModel', function (): void { should(postgresModel.engineSettingsLastUpdated).be.Date(); }); - it('Calls onEngineSettingsUpdated event after populating engine settings', async function (): Promise { - const connectionResultMock = TypeMoq.Mock.ofType(); - connectionResultMock.setup(x => x.connected).returns(() => true); - connectionResultMock.setup((x: any) => x.then).returns(() => undefined); - sinon.stub(azdata.connection, 'connect').returns(Promise.resolve(connectionResultMock.object)); - - const array: azdata.DbCellValue[][] = []; - - const executeMock = TypeMoq.Mock.ofType(); - executeMock.setup(x => x.rows).returns(() => array); - executeMock.setup((x: any) => x.then).returns(() => undefined); - - const providerMock = TypeMoq.Mock.ofType(); - providerMock.setup(x => x.runQueryAndReturn(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(async () => executeMock.object); - providerMock.setup((x: any) => x.then).returns(() => undefined); - sinon.stub(azdata.dataprotocol, 'getProvider').returns(providerMock.object); - - const onEngineSettingsUpdated = sinon.spy(vscode.EventEmitter.prototype, 'fire'); - - await postgresModel.getEngineSettings(); - sinon.assert.calledOnceWithExactly(onEngineSettingsUpdated, postgresModel.workerNodesEngineSettings); - }); - it('Populating engine settings skips certain parameters', async function (): Promise { const connectionResultMock = TypeMoq.Mock.ofType(); connectionResultMock.setup(x => x.connected).returns(() => true); diff --git a/extensions/arc/src/ui/dashboards/postgres/postgresParameters.ts b/extensions/arc/src/ui/dashboards/postgres/postgresParameters.ts index 5355fc5fc2..7b16926f30 100644 --- a/extensions/arc/src/ui/dashboards/postgres/postgresParameters.ts +++ b/extensions/arc/src/ui/dashboards/postgres/postgresParameters.ts @@ -11,7 +11,7 @@ import { UserCancelledError } from '../../../common/api'; import { IconPathHelper, cssStyles } from '../../../constants'; import { DashboardPage } from '../../components/dashboardPage'; import { EngineSettingsModel, PostgresModel } from '../../../models/postgresModel'; -import { debounce } from '../../../common/utils'; +import { debounce, instanceOfCheckBox } from '../../../common/utils'; export type ParametersModel = { parameterName: string, @@ -47,8 +47,7 @@ export abstract class PostgresParametersPage extends DashboardPage { this.initializeSearchBox(); this.disposables.push( - this._postgresModel.onConfigUpdated(() => this.eventuallyRunOnInitialized(() => this.handleServiceUpdated())), - this._postgresModel.onEngineSettingsUpdated(() => this.eventuallyRunOnInitialized(() => this.refreshParametersTable())) + this._postgresModel.onConfigUpdated(() => this.eventuallyRunOnInitialized(() => this.handleServiceUpdated())) ); } @@ -160,9 +159,9 @@ export abstract class PostgresParametersPage extends DashboardPage { throw err; } try { - await this._postgresModel.refresh(); + await this.callGetEngineSettings(); } catch (error) { - vscode.window.showErrorMessage(loc.refreshFailed(error)); + vscode.window.showErrorMessage(loc.fetchEngineSettingsFailed(this._postgresModel.info.name, error)); } } ); @@ -238,9 +237,9 @@ export abstract class PostgresParametersPage extends DashboardPage { } this.changedComponentValues.clear(); try { - await this._postgresModel.refresh(); + await this.callGetEngineSettings(); } catch (error) { - vscode.window.showErrorMessage(loc.refreshFailed(error)); + vscode.window.showErrorMessage(loc.fetchEngineSettingsFailed(this._postgresModel.info.name, error)); } } ); @@ -321,13 +320,19 @@ export abstract class PostgresParametersPage extends DashboardPage { this.searchBox.enabled = true; this.resetAllButton.enabled = true; this.parameterContainer.addItem(this._parametersTable!); - this.refreshParametersTable(); + this.refreshParametersTableValues(); } } private async callGetEngineSettings(): Promise { try { - await this._postgresModel.getEngineSettings(); + await this._postgresModel.getEngineSettings().then(() => { + if (this._parametersTable.data?.length !== 0) { + this.refreshParametersTableValues(); + } else { + this.populateParametersTable(); + } + }); } catch (error) { if (error instanceof UserCancelledError) { vscode.window.showWarningMessage(loc.pgConnectionRequired); @@ -415,9 +420,9 @@ export abstract class PostgresParametersPage extends DashboardPage { async (_progress, _token): Promise => { await this.resetParameter(engineSetting.parameterName!); try { - await this._postgresModel.refresh(); + await this.callGetEngineSettings(); } catch (error) { - vscode.window.showErrorMessage(loc.refreshFailed(error)); + vscode.window.showErrorMessage(loc.fetchEngineSettingsFailed(this._postgresModel.info.name, error)); } } ); @@ -573,10 +578,6 @@ export abstract class PostgresParametersPage extends DashboardPage { } private discardParametersTableChanges(): void { - let instanceOfCheckBox = function (object: any): object is azdata.CheckBoxComponent { - return 'checked' in object; - }; - this.changedComponentValues.forEach(v => { let param = this._parameters.find(p => p.parameterName === v); if (instanceOfCheckBox(param!.valueComponent)) { @@ -591,7 +592,7 @@ export abstract class PostgresParametersPage extends DashboardPage { }); } - private refreshParametersTable(): void { + private populateParametersTable(): void { this._parameters = this.engineSettings.map(parameter => this.createParameterComponents(parameter)); this._parametersTable.data = this._parameters.map(p => { @@ -607,14 +608,37 @@ export abstract class PostgresParametersPage extends DashboardPage { }); } + /** + * Checks if exisiting parameter values needs to be updated. + * Only updates exisiting parameters, will not add/remove parameters from the table. + */ + private refreshParametersTableValues(): void { + this.engineSettings.map(parameter => { + let param = this._parameters.find(p => p.parameterName === parameter.parameterName); + if (param) { + if (parameter.value !== param.originalValue) { + param.originalValue = parameter.value!; + + if (instanceOfCheckBox(param.valueComponent)) { + if (param.originalValue === 'on') { + param.valueComponent.checked = true; + } else { + param.valueComponent.checked = false; + } + } else { + param.valueComponent.value = parameter.value; + } + } + } + }); + } + private async handleServiceUpdated(): Promise { if (this._postgresModel.configLastUpdated && !this._postgresModel.engineSettingsLastUpdated) { this.connectToServerButton!.enabled = true; this.parametersTableLoading!.loading = false; } else if (this._postgresModel.engineSettingsLastUpdated) { await this.callGetEngineSettings(); - this.discardButton.enabled = false; - this.saveButton.enabled = false; } }