From 2b0b529b21f5039687fe0e068ef445697a58a79f Mon Sep 17 00:00:00 2001 From: Arvind Ranasaria Date: Fri, 7 Aug 2020 16:47:10 -0700 Subject: [PATCH] Add controller display name (#11675) * Add controller display name * pr feedback * revert unneeded change * pr feedback --- extensions/arc/src/common/utils.ts | 2 +- extensions/arc/src/localizedConstants.ts | 10 +++-- extensions/arc/src/models/controllerModel.ts | 10 ++++- extensions/arc/src/test/common/utils.test.ts | 2 +- .../dialogs/connectControllerDialog.test.ts | 40 ++++++++++++++----- .../ui/tree/azureArcTreeDataProvider.test.ts | 16 ++++---- .../src/ui/dialogs/connectControllerDialog.ts | 21 +++++++--- .../arc/src/ui/tree/controllerTreeNode.ts | 4 +- 8 files changed, 74 insertions(+), 31 deletions(-) diff --git a/extensions/arc/src/common/utils.ts b/extensions/arc/src/common/utils.ts index 77bbdfd69c..b26208ffde 100644 --- a/extensions/arc/src/common/utils.ts +++ b/extensions/arc/src/common/utils.ts @@ -98,7 +98,7 @@ export function getDatabaseStateDisplayText(state: string): string { case 'SUSPECT': return loc.suspect; case 'EMERGENCY': - return loc.emergecy; + return loc.emergency; } return state; } diff --git a/extensions/arc/src/localizedConstants.ts b/extensions/arc/src/localizedConstants.ts index f4fc62bff8..6e18a583bf 100644 --- a/extensions/arc/src/localizedConstants.ts +++ b/extensions/arc/src/localizedConstants.ts @@ -74,6 +74,8 @@ export const loading = localize('arc.loading', "Loading..."); export const refreshToEnterCredentials = localize('arc.refreshToEnterCredentials', "Refresh node to enter credentials"); export const connectToController = localize('arc.connectToController', "Connect to Existing Controller"); export const controllerUrl = localize('arc.controllerUrl', "Controller URL"); +export const controllerName = localize('arc.controllerName', "Name"); +export const defaultControllerName = localize('arc.defaultControllerName', "arc-dc"); export const username = localize('arc.username', "Username"); export const password = localize('arc.password', "Password"); export const rememberPassword = localize('arc.rememberPassword', "Remember Password"); @@ -87,7 +89,7 @@ export const restoring = localize('arc.restoring', "Restoring"); export const recovering = localize('arc.recovering', "Recovering"); export const recoveryPending = localize('arc.recoveringPending', "Recovery Pending"); export const suspect = localize('arc.suspect', "Suspect"); -export const emergecy = localize('arc.emergecy', "Emergecy"); +export const emergency = localize('arc.emergency', "Emergency"); // Postgres constants export const coordinatorEndpoint = localize('arc.coordinatorEndpoint', "Coordinator endpoint"); @@ -142,9 +144,9 @@ export function openDashboardFailed(error: any): string { return localize('arc.o export function resourceDeletionFailed(name: string, error: any): string { return localize('arc.resourceDeletionFailed', "Failed to delete resource {0}. {1}", name, getErrorMessage(error)); } export function databaseCreationFailed(name: string, error: any): string { return localize('arc.databaseCreationFailed', "Failed to create database {0}. {1}", name, getErrorMessage(error)); } export function connectToControllerFailed(url: string, error: any): string { return localize('arc.connectToControllerFailed', "Could not connect to controller {0}. {1}", url, getErrorMessage(error)); } -export function fetchStatusFailed(name: string, error: any): string { return localize('arc.fetchStatusFailed', "An unexpected error occured retrieving the status for resource '{0}'. {1}", name, getErrorMessage(error)); } -export function fetchEndpointsFailed(name: string, error: any): string { return localize('arc.fetchEndpointsFailed', "An unexpected error occured retrieving the endpoints for '{0}'. {1}", name, getErrorMessage(error)); } -export function fetchRegistrationsFailed(name: string, error: any): string { return localize('arc.fetchRegistrationsFailed', "An unexpected error occured retrieving the registrations for '{0}'. {1}", name, getErrorMessage(error)); } +export function fetchStatusFailed(name: string, error: any): string { return localize('arc.fetchStatusFailed', "An unexpected error occurred retrieving the status for resource '{0}'. {1}", name, getErrorMessage(error)); } +export function fetchEndpointsFailed(name: string, error: any): string { return localize('arc.fetchEndpointsFailed', "An unexpected error occurred retrieving the endpoints for '{0}'. {1}", name, getErrorMessage(error)); } +export function fetchRegistrationsFailed(name: string, error: any): string { return localize('arc.fetchRegistrationsFailed', "An unexpected error occurred retrieving the registrations for '{0}'. {1}", name, getErrorMessage(error)); } export function couldNotFindRegistration(namespace: string, name: string) { return localize('arc.couldNotFindRegistration', "Could not find controller registration for {0} ({1})", name, namespace); } export function resourceDeletionWarning(namespace: string, name: string): string { return localize('arc.resourceDeletionWarning', "Warning! Deleting a resource is permanent and cannot be undone. To delete the resource '{0}.{1}' type the name '{1}' below to proceed.", namespace, name); } export function invalidResourceDeletionName(name: string): string { return localize('arc.invalidResourceDeletionName', "The value '{0}' does not match the instance name. Try again or press escape to exit", name); } diff --git a/extensions/arc/src/models/controllerModel.ts b/extensions/arc/src/models/controllerModel.ts index 16f0e4f970..738aac7d9a 100644 --- a/extensions/arc/src/models/controllerModel.ts +++ b/extensions/arc/src/models/controllerModel.ts @@ -14,6 +14,7 @@ import * as loc from '../localizedConstants'; export type ControllerInfo = { url: string, + name: string, username: string, rememberPassword: boolean, resources: ResourceInfo[] @@ -74,7 +75,7 @@ export class ControllerModel { if (password && !promptReconnect) { this.setAuthentication(new BasicAuth(this.info.username, password)); } else { - // No password yet or we want to reprompt for credentials so prompt for it from the user + // No password yet or we want to re-prompt for credentials so prompt for it from the user const dialog = new ConnectToControllerDialog(this.treeDataProvider); dialog.showDialog(this.info, password); const model = await dialog.waitForClose(); @@ -182,6 +183,13 @@ export class ControllerModel { this._registrationRouter.setDefaultAuthentication(auth); this._sqlInstanceRouter.setDefaultAuthentication(auth); } + + /** + * property to for use a display label for this controller + */ + public get label(): string { + return `${this.info.name} (${this.info.url})`; + } } /** diff --git a/extensions/arc/src/test/common/utils.test.ts b/extensions/arc/src/test/common/utils.test.ts index a72c9d781b..44d72407dc 100644 --- a/extensions/arc/src/test/common/utils.test.ts +++ b/extensions/arc/src/test/common/utils.test.ts @@ -116,7 +116,7 @@ describe('getDatabaseStateDisplayText Method Tests', function () { should(getDatabaseStateDisplayText('RECOVERING')).equal(loc.recovering); should(getDatabaseStateDisplayText('RECOVERY PENDING')).equal(loc.recoveryPending); should(getDatabaseStateDisplayText('SUSPECT')).equal(loc.suspect); - should(getDatabaseStateDisplayText('EMERGENCY')).equal(loc.emergecy); + should(getDatabaseStateDisplayText('EMERGENCY')).equal(loc.emergency); }); it('State should stay the same for unknown value', function (): void { diff --git a/extensions/arc/src/test/ui/dialogs/connectControllerDialog.test.ts b/extensions/arc/src/test/ui/dialogs/connectControllerDialog.test.ts index 117458f3c8..d5fb0d24ff 100644 --- a/extensions/arc/src/test/ui/dialogs/connectControllerDialog.test.ts +++ b/extensions/arc/src/test/ui/dialogs/connectControllerDialog.test.ts @@ -5,8 +5,9 @@ import * as should from 'should'; import * as sinon from 'sinon'; +import { ControllerInfo, ControllerModel, Registration } from '../../../models/controllerModel'; import { ConnectToControllerDialog } from '../../../ui/dialogs/connectControllerDialog'; -import { ControllerInfo, ControllerModel } from '../../../models/controllerModel'; +import * as loc from '../../../localizedConstants'; describe('ConnectControllerDialog', function (): void { afterEach(function (): void { @@ -29,7 +30,7 @@ describe('ConnectControllerDialog', function (): void { it('validate returns false if controller refresh fails', async function (): Promise { sinon.stub(ControllerModel.prototype, 'refresh').returns(Promise.reject('Controller refresh failed')); const connectControllerDialog = new ConnectToControllerDialog(undefined!); - const info = { url: 'https://127.0.0.1:30080', username: 'sa', rememberPassword: true, resources: [] }; + const info = { url: 'https://127.0.0.1:30080', name: 'my-arc', username: 'sa', rememberPassword: true, resources: [] }; connectControllerDialog.showDialog(info, 'pwd'); await connectControllerDialog.isInitialized; const validateResult = await connectControllerDialog.validate(); @@ -38,34 +39,55 @@ describe('ConnectControllerDialog', function (): void { it('validate replaces http with https', async function (): Promise { await validateConnectControllerDialog( - { url: 'http://127.0.0.1:30081', username: 'sa', rememberPassword: true, resources: [] }, + { url: 'http://127.0.0.1:30081', name: 'my-arc', username: 'sa', rememberPassword: true, resources: [] }, 'https://127.0.0.1:30081'); }); it('validate appends https if missing', async function (): Promise { - await validateConnectControllerDialog({ url: '127.0.0.1:30080', username: 'sa', rememberPassword: true, resources: [] }, + await validateConnectControllerDialog({ url: '127.0.0.1:30080', name: 'my-arc', username: 'sa', rememberPassword: true, resources: [] }, 'https://127.0.0.1:30080'); }); it('validate appends default port if missing', async function (): Promise { - await validateConnectControllerDialog({ url: 'https://127.0.0.1', username: 'sa', rememberPassword: true, resources: [] }, + await validateConnectControllerDialog({ url: 'https://127.0.0.1', name: 'my-arc', username: 'sa', rememberPassword: true, resources: [] }, 'https://127.0.0.1:30080'); }); it('validate appends both port and https if missing', async function (): Promise { - await validateConnectControllerDialog({ url: '127.0.0.1', username: 'sa', rememberPassword: true, resources: [] }, + await validateConnectControllerDialog({ url: '127.0.0.1', name: 'my-arc', username: 'sa', rememberPassword: true, resources: [] }, 'https://127.0.0.1:30080'); }); + + for (const name of ['', undefined]) { + it(`validate display name gets set to arc instance name for user chosen name of:${name}`, async function (): Promise { + await validateConnectControllerDialog( + { url: 'http://127.0.0.1:30081', name: name!, username: 'sa', rememberPassword: true, resources: [] }, + 'https://127.0.0.1:30081'); + }); + } + + it(`validate display name gets set to default data controller name for user chosen name of:'' and instanceName in explicably returned as undefined from the controller endpoint`, async function (): Promise { + await validateConnectControllerDialog( + { url: 'http://127.0.0.1:30081', name: '', username: 'sa', rememberPassword: true, resources: [] }, + 'https://127.0.0.1:30081', + undefined); + }); }); -async function validateConnectControllerDialog(info: ControllerInfo, expectedUrl: string): Promise { - // For first set of tests just stub out refresh calls - we'll test that separately - sinon.stub(ControllerModel.prototype, 'refresh').returns(Promise.resolve()); +async function validateConnectControllerDialog(info: ControllerInfo, expectedUrl: string, arcInstanceName: string = 'arc-instance'): Promise { + const expectedControllerInfoName = info.name || arcInstanceName || loc.defaultControllerName; const connectControllerDialog = new ConnectToControllerDialog(undefined!); + // Stub out refresh calls to controllerModel - we'll test those separately + sinon.stub(ControllerModel.prototype, 'refresh').returns(Promise.resolve()); + // stub out controller registration response to return a known instanceName for the dc. + sinon.stub(ControllerModel.prototype, 'controllerRegistration').get(() => { + return { instanceName: arcInstanceName }; + }); connectControllerDialog.showDialog(info, 'pwd'); await connectControllerDialog.isInitialized; const validateResult = await connectControllerDialog.validate(); should(validateResult).be.true('Validation should have returned true'); const model = await connectControllerDialog.waitForClose(); should(model?.controllerModel.info.url).equal(expectedUrl); + should(model?.controllerModel.info.name).equal(expectedControllerInfoName); } diff --git a/extensions/arc/src/test/ui/tree/azureArcTreeDataProvider.test.ts b/extensions/arc/src/test/ui/tree/azureArcTreeDataProvider.test.ts index ddfdda6d54..263a4f2e56 100644 --- a/extensions/arc/src/test/ui/tree/azureArcTreeDataProvider.test.ts +++ b/extensions/arc/src/test/ui/tree/azureArcTreeDataProvider.test.ts @@ -3,12 +3,12 @@ * Licensed under the Source EULA. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import * as vscode from 'vscode'; -import * as should from 'should'; import 'mocha'; +import * as should from 'should'; import * as TypeMoq from 'typemoq'; -import { AzureArcTreeDataProvider } from '../../../ui/tree/azureArcTreeDataProvider'; +import * as vscode from 'vscode'; import { ControllerModel } from '../../../models/controllerModel'; +import { AzureArcTreeDataProvider } from '../../../ui/tree/azureArcTreeDataProvider'; import { ControllerTreeNode } from '../../../ui/tree/controllerTreeNode'; describe('AzureArcTreeDataProvider tests', function (): void { @@ -43,7 +43,7 @@ describe('AzureArcTreeDataProvider tests', function (): void { treeDataProvider['_loading'] = false; let children = await treeDataProvider.getChildren(); should(children.length).equal(0, 'There initially shouldn\'t be any children'); - const controllerModel = new ControllerModel(treeDataProvider, { url: '127.0.0.1', username: 'sa', rememberPassword: true, resources: [] }); + const controllerModel = new ControllerModel(treeDataProvider, { url: '127.0.0.1', name: 'my-arc', username: 'sa', rememberPassword: true, resources: [] }); await treeDataProvider.addOrUpdateController(controllerModel, ''); should(children.length).equal(1, 'Controller node should be added correctly'); await treeDataProvider.addOrUpdateController(controllerModel, ''); @@ -54,11 +54,11 @@ describe('AzureArcTreeDataProvider tests', function (): void { treeDataProvider['_loading'] = false; let children = await treeDataProvider.getChildren(); should(children.length).equal(0, 'There initially shouldn\'t be any children'); - const controllerModel = new ControllerModel(treeDataProvider, { url: '127.0.0.1', username: 'sa', rememberPassword: true, resources: [] }); + const controllerModel = new ControllerModel(treeDataProvider, { url: '127.0.0.1', name: 'my-arc', username: 'sa', rememberPassword: true, resources: [] }); await treeDataProvider.addOrUpdateController(controllerModel, ''); should(children.length).equal(1, 'Controller node should be added correctly'); should((children[0]).model.info.rememberPassword).be.true('Info was not set correctly initially'); - const controllerModel2 = new ControllerModel(treeDataProvider, { url: '127.0.0.1', username: 'sa', rememberPassword: false, resources: [] }); + const controllerModel2 = new ControllerModel(treeDataProvider, { url: '127.0.0.1', name: 'my-arc', username: 'sa', rememberPassword: false, resources: [] }); await treeDataProvider.addOrUpdateController(controllerModel2, ''); should(children.length).equal(1, 'Shouldn\'t add duplicate controller node'); should((children[0]).model.info.rememberPassword).be.false('Info was not updated correctly'); @@ -82,8 +82,8 @@ describe('AzureArcTreeDataProvider tests', function (): void { describe('removeController', function (): void { it('removing a controller should work as expected', async function (): Promise { treeDataProvider['_loading'] = false; - const controllerModel = new ControllerModel(treeDataProvider, { url: '127.0.0.1', username: 'sa', rememberPassword: true, resources: [] }); - const controllerModel2 = new ControllerModel(treeDataProvider, { url: '127.0.0.2', username: 'cloudsa', rememberPassword: true, resources: [] }); + const controllerModel = new ControllerModel(treeDataProvider, { url: '127.0.0.1', name: 'my-arc', username: 'sa', rememberPassword: true, resources: [] }); + const controllerModel2 = new ControllerModel(treeDataProvider, { url: '127.0.0.2', name: 'my-arc', username: 'cloudsa', rememberPassword: true, resources: [] }); await treeDataProvider.addOrUpdateController(controllerModel, ''); await treeDataProvider.addOrUpdateController(controllerModel2, ''); const children = (await treeDataProvider.getChildren()); diff --git a/extensions/arc/src/ui/dialogs/connectControllerDialog.ts b/extensions/arc/src/ui/dialogs/connectControllerDialog.ts index e466ca620f..849890055c 100644 --- a/extensions/arc/src/ui/dialogs/connectControllerDialog.ts +++ b/extensions/arc/src/ui/dialogs/connectControllerDialog.ts @@ -5,18 +5,18 @@ import * as azdata from 'azdata'; import * as vscode from 'vscode'; -import * as loc from '../../localizedConstants'; -import { AzureArcTreeDataProvider } from '../tree/azureArcTreeDataProvider'; -import { ControllerModel, ControllerInfo } from '../../models/controllerModel'; import { Deferred } from '../../common/promise'; +import * as loc from '../../localizedConstants'; +import { ControllerInfo, ControllerModel } from '../../models/controllerModel'; import { InitializingComponent } from '../components/initializingComponent'; +import { AzureArcTreeDataProvider } from '../tree/azureArcTreeDataProvider'; export type ConnectToControllerDialogModel = { controllerModel: ControllerModel, password: string }; - export class ConnectToControllerDialog extends InitializingComponent { private modelBuilder!: azdata.ModelBuilder; private urlInputBox!: azdata.InputBoxComponent; + private nameInputBox!: azdata.InputBoxComponent; private usernameInputBox!: azdata.InputBoxComponent; private passwordInputBox!: azdata.InputBoxComponent; private rememberPwCheckBox!: azdata.CheckBoxComponent; @@ -39,6 +39,10 @@ export class ConnectToControllerDialog extends InitializingComponent { // If we have a model then we're editing an existing connection so don't let them modify the URL readOnly: !!controllerInfo }).component(); + this.nameInputBox = this.modelBuilder.inputBox() + .withProperties({ + value: controllerInfo?.name + }).component(); this.usernameInputBox = this.modelBuilder.inputBox() .withProperties({ value: controllerInfo?.username @@ -62,6 +66,10 @@ export class ConnectToControllerDialog extends InitializingComponent { component: this.urlInputBox, title: loc.controllerUrl, required: true + }, { + component: this.nameInputBox, + title: loc.controllerName, + required: false }, { component: this.usernameInputBox, title: loc.username, @@ -108,14 +116,17 @@ export class ConnectToControllerDialog extends InitializingComponent { } const controllerInfo: ControllerInfo = { url: url, + name: this.nameInputBox.value ?? '', username: this.usernameInputBox.value, rememberPassword: this.rememberPwCheckBox.checked ?? false, resources: [] }; const controllerModel = new ControllerModel(this._treeDataProvider, controllerInfo, this.passwordInputBox.value); try { - // Validate that we can connect to the controller + // Validate that we can connect to the controller, this also populates the controllerRegistration from the connection response. await controllerModel.refresh(false); + // default info.name to the name of the controller instance if the user did not specify their own and to a pre-canned default if for some weird reason controller endpoint returned instanceName is also not a valid value + controllerModel.info.name = controllerModel.info.name || controllerModel.controllerRegistration?.instanceName || loc.defaultControllerName; } catch (err) { vscode.window.showErrorMessage(loc.connectToControllerFailed(this.urlInputBox.value, err)); return false; diff --git a/extensions/arc/src/ui/tree/controllerTreeNode.ts b/extensions/arc/src/ui/tree/controllerTreeNode.ts index 7c3b5e9080..38d94eefbb 100644 --- a/extensions/arc/src/ui/tree/controllerTreeNode.ts +++ b/extensions/arc/src/ui/tree/controllerTreeNode.ts @@ -28,7 +28,7 @@ export class ControllerTreeNode extends TreeNode { private _childrenRefreshPromise = new Deferred(); constructor(public model: ControllerModel, private _context: vscode.ExtensionContext, private _treeDataProvider: AzureArcTreeDataProvider) { - super(model.info.url, vscode.TreeItemCollapsibleState.Collapsed, ResourceType.dataControllers); + super(model.label, vscode.TreeItemCollapsibleState.Collapsed, ResourceType.dataControllers); model.onRegistrationsUpdated(registrations => this.refreshChildren(registrations)); } @@ -49,7 +49,7 @@ export class ControllerTreeNode extends TreeNode { } // Couldn't get the children and TreeView doesn't have a way to collapse a node // in a way that will refetch its children when expanded again so instead we - // display a tempory node that will prompt the user to re-enter credentials + // display a temporary node that will prompt the user to re-enter credentials return [new RefreshTreeNode(this)]; } }