Skip to content

Commit

Permalink
Add error logging when fetching account provider accounts (#13082)
Browse files Browse the repository at this point in the history
* Add error logging when fetching account provider accounts

* fix test

* fix tests #2
  • Loading branch information
Charles-Gagnon committed Oct 26, 2020
1 parent 791dee1 commit c2bd11f
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 19 deletions.
38 changes: 22 additions & 16 deletions src/sql/platform/accounts/common/accountViewModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { Event, Emitter } from 'vs/base/common/event';
import { IAccountManagementService } from 'sql/platform/accounts/common/interfaces';
import { AccountProviderAddedEventParams, UpdateAccountListEventParams } from 'sql/platform/accounts/common/eventTypes';
import { coalesce } from 'vs/base/common/arrays';
import { ILogService } from 'vs/platform/log/common/log';

/**
* View model for account dialog
Expand All @@ -23,7 +24,7 @@ export class AccountViewModel {
private _updateAccountListEmitter: Emitter<UpdateAccountListEventParams>;
public get updateAccountListEvent(): Event<UpdateAccountListEventParams> { return this._updateAccountListEmitter.event; }

constructor(@IAccountManagementService private _accountManagementService: IAccountManagementService) {
constructor(@IAccountManagementService private _accountManagementService: IAccountManagementService, @ILogService private _logService: ILogService) {
// Create our event emitters
this._addProviderEmitter = new Emitter<AccountProviderAddedEventParams>();
this._removeProviderEmitter = new Emitter<azdata.AccountProviderMetadata>();
Expand All @@ -41,24 +42,29 @@ export class AccountViewModel {
* and fires an event after each provider/accounts has been loaded.
*
*/
public initialize(): Promise<AccountProviderAddedEventParams[]> {
public async initialize(): Promise<AccountProviderAddedEventParams[]> {
// Load a baseline of the account provider metadata and accounts
// 1) Get all the providers from the account management service
// 2) For each provider, get the accounts
// 3) Build parameters to add a provider and return it
return this._accountManagementService.getAccountProviderMetadata()
.then(providers => {
const promises = providers.map(provider => {
return this._accountManagementService.getAccountsForProvider(provider.id)
.then(accounts => <AccountProviderAddedEventParams>{
addedProvider: provider,
initialAccounts: accounts
}, () => undefined);
});
return Promise.all(promises).then(accounts => coalesce(accounts));
}, () => {
/* Swallow failures and just pretend we don't have any providers */
return [];
});
try {
const metadata = await this._accountManagementService.getAccountProviderMetadata();
const accounts = await Promise.all(metadata.map(async providerMetadata => {
try {
const accounts = await this._accountManagementService.getAccountsForProvider(providerMetadata.id);
return <AccountProviderAddedEventParams>{
addedProvider: providerMetadata,
initialAccounts: accounts
};
} catch (err) {
this._logService.warn(`Error getting accounts for provider ${providerMetadata.id} : ${err}`);
return undefined;
}
}));
return coalesce(accounts);
} catch (err) {
this._logService.warn(`Error getting account provider metadata : ${err}`);
return [];
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { AccountViewModel } from 'sql/platform/accounts/common/accountViewModel'
import { AccountProviderAddedEventParams, UpdateAccountListEventParams } from 'sql/platform/accounts/common/eventTypes';
import { TestAccountManagementService } from 'sql/platform/accounts/test/common/testAccountManagementService';
import { EventVerifierSingle } from 'sql/base/test/common/event';
import { NullLogService } from 'vs/platform/log/common/log';

// SUITE STATE /////////////////////////////////////////////////////////////
let mockAddProviderEmitter: Emitter<AccountProviderAddedEventParams>;
Expand Down Expand Up @@ -63,7 +64,7 @@ suite('Account Management Dialog ViewModel Tests', () => {
test('Construction - Events are properly defined', () => {
// If: I create an account viewmodel
let mockAccountManagementService = getMockAccountManagementService(false, false);
let vm = new AccountViewModel(mockAccountManagementService.object);
let vm = new AccountViewModel(mockAccountManagementService.object, new NullLogService());

// Then:
// ... All the events for the view models should be properly initialized
Expand Down Expand Up @@ -195,7 +196,7 @@ function getViewModel(
evRemove: EventVerifierSingle<azdata.AccountProviderMetadata>,
evUpdate: EventVerifierSingle<UpdateAccountListEventParams>
): AccountViewModel {
let vm = new AccountViewModel(ams);
let vm = new AccountViewModel(ams, new NullLogService());
vm.addProviderEvent(evAdd.eventHandler);
vm.removeProviderEvent(evRemove.eventHandler);
vm.updateAccountListEvent(evUpdate.eventHandler);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { TestErrorMessageService } from 'sql/platform/errorMessage/test/common/t
import { InstantiationService } from 'vs/platform/instantiation/common/instantiationService';
import { AccountListRenderer } from 'sql/workbench/services/accountManagement/browser/accountListRenderer';
import { MockContextKeyService } from 'vs/platform/keybinding/test/common/mockKeybindingService';
import { NullLogService } from 'vs/platform/log/common/log';

// TESTS ///////////////////////////////////////////////////////////////////
suite('Account Management Dialog Controller Tests', () => {
Expand Down Expand Up @@ -70,7 +71,7 @@ suite('Account Management Dialog Controller Tests', () => {

function createInstantiationService(addAccountFailureEmitter?: Emitter<string>): InstantiationService {
// Create a mock account dialog view model
let accountViewModel = new AccountViewModel(new TestAccountManagementService());
let accountViewModel = new AccountViewModel(new TestAccountManagementService(), new NullLogService());
let mockAccountViewModel = TypeMoq.Mock.ofInstance(accountViewModel);
let mockEvent = new Emitter<any>();
mockAccountViewModel.setup(x => x.addProviderEvent).returns(() => mockEvent.event);
Expand Down

0 comments on commit c2bd11f

Please sign in to comment.