Remove disposable from connection (#22687)

* Remove disposable from connection

* Remove from group
This commit is contained in:
Charles Gagnon
2023-04-11 15:27:01 -07:00
committed by GitHub
parent 219bdabfb2
commit df88d881c5
11 changed files with 10 additions and 63 deletions

View File

@@ -87,9 +87,7 @@ export class ConnectionConfig {
// Remove the profile if already set
let sameProfileInList = profiles.find(value => {
const providerConnectionProfile = ConnectionProfile.createFromStoredProfile(value, this._capabilitiesService);
const match = matcher(providerConnectionProfile, connectionProfile);
providerConnectionProfile.dispose();
return match;
return matcher(providerConnectionProfile, connectionProfile);
});
if (sameProfileInList) {
let profileIndex = profiles.findIndex(value => value === sameProfileInList);

View File

@@ -4,7 +4,6 @@
*--------------------------------------------------------------------------------------------*/
import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile';
import { Disposable } from 'vs/base/common/lifecycle';
import { isUndefinedOrNull } from 'vs/base/common/types';
export interface INewConnectionProfileGroup {
@@ -19,7 +18,7 @@ export interface IConnectionProfileGroup extends INewConnectionProfileGroup {
id?: string;
}
export class ConnectionProfileGroup extends Disposable implements IConnectionProfileGroup {
export class ConnectionProfileGroup implements IConnectionProfileGroup {
private _childGroups: ConnectionProfileGroup[] = [];
private _childConnections: ConnectionProfile[] = [];
@@ -35,7 +34,6 @@ export class ConnectionProfileGroup extends Disposable implements IConnectionPro
public color?: string,
public description?: string
) {
super();
this.parentId = parent ? parent.id : undefined;
if (ConnectionProfileGroup.isRoot(this.name)) {
this.name = '';
@@ -146,7 +144,6 @@ export class ConnectionProfileGroup extends Disposable implements IConnectionPro
connections?.forEach((conn) => {
this._childConnections = this._childConnections.filter((curConn) => { return curConn.id !== conn.id; });
conn.parent = this;
this._register(conn);
this._childConnections.push(conn);
});
@@ -156,7 +153,6 @@ export class ConnectionProfileGroup extends Disposable implements IConnectionPro
groups?.forEach((group) => {
this._childGroups = this._childGroups.filter((grp) => { return group.id !== grp.id; });
group.parent = this;
this._register(group);
this._childGroups.push(group);
});
}

View File

@@ -181,10 +181,8 @@ export class ConnectionStore {
}
public getProfileWithoutPassword(conn: IConnectionProfile): ConnectionProfile {
let savedConn = ConnectionProfile.fromIConnectionProfile(this.capabilitiesService, conn);
let newSavedConn = savedConn.withoutPassword();
savedConn.dispose();
return newSavedConn;
const savedConn = ConnectionProfile.fromIConnectionProfile(this.capabilitiesService, conn);
return savedConn.withoutPassword();
}
/**
@@ -229,9 +227,7 @@ export class ConnectionStore {
list.unshift(savedProfile);
const profiles = list.filter(n => n !== undefined).map(c => c.toIConnectionProfile());
list.forEach(c => c.dispose());
return profiles;
return list.filter(n => n !== undefined).map(c => c.toIConnectionProfile());
}
private removeFromConnectionList(conn: IConnectionProfile, list: ConnectionProfile[]): IConnectionProfile[] {

View File

@@ -3,7 +3,6 @@
* Licensed under the Source EULA. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { Disposable } from 'vs/base/common/lifecycle';
import { isString } from 'vs/base/common/types';
import * as azdata from 'azdata';
@@ -14,7 +13,7 @@ import { localize } from 'vs/nls';
type SettableProperty = 'serverName' | 'authenticationType' | 'databaseName' | 'password' | 'connectionName' | 'userName';
export class ProviderConnectionInfo extends Disposable implements azdata.ConnectionInfo {
export class ProviderConnectionInfo implements azdata.ConnectionInfo {
options: { [name: string]: any } = {};
@@ -25,7 +24,6 @@ export class ProviderConnectionInfo extends Disposable implements azdata.Connect
protected capabilitiesService: ICapabilitiesService,
model: string | azdata.IConnectionProfile | azdata.connection.ConnectionProfile | undefined
) {
super();
// we can't really do a whole lot if we don't have a provider
if (model) {
this.providerName = isString(model) ? model : 'providerName' in model ? model.providerName : model.providerId;
@@ -69,14 +67,6 @@ export class ProviderConnectionInfo extends Disposable implements azdata.Connect
this._providerName = name;
}
public override dispose(): void {
// Notes:
// 1. It is not recommended to add disposables to this class considering the way we create and use the connection objects,
// there are places that the connection objects are not being disposed properly.
// 2. Remember to dispose the listeners added to this class.
super.dispose();
}
public clone(): ProviderConnectionInfo {
let instance = new ProviderConnectionInfo(this.capabilitiesService, this.providerName);
instance.options = Object.assign({}, this.options);

View File

@@ -420,7 +420,6 @@ export class ServerTreeView extends Disposable implements IServerTreeView {
if (profile) {
newProfile = profile;
}
groups.forEach(group => group.dispose());
}
const currentSelections = this._tree!.getSelection();

View File

@@ -150,10 +150,9 @@ export class ConnectionController implements IConnectionComponentController {
}
allGroups.push(Object.assign({}, this._connectionWidget.DefaultServerGroup, { id: defaultGroupId }));
allGroups.push(this._connectionWidget.NoneServerGroup);
if (connectionGroupRoot && connectionGroupRoot.length > 0) {
if (connectionGroupRoot?.length > 0) {
this.flattenGroups(connectionGroupRoot[0], allGroups);
}
connectionGroupRoot.forEach(cpg => cpg.dispose());
return allGroups;
}

View File

@@ -346,11 +346,7 @@ export class ConnectionDialogService implements IConnectionDialogService {
}
}
this._model.providerName = this._currentProviderType;
const previousModel = this._model;
this._model = new ConnectionProfile(this._capabilitiesService, this._model);
if (previousModel) {
previousModel.dispose();
}
if (this._inputModel && this._inputModel.options) {
this.uiController.showUiComponent(input.container,
this._inputModel.options.authTypeChanged);
@@ -367,10 +363,6 @@ export class ConnectionDialogService implements IConnectionDialogService {
private async handleFillInConnectionInputs(connectionInfo: IConnectionProfile): Promise<void> {
try {
const connectionWithPassword = await this.createModel(connectionInfo);
if (this._model) {
this._model.dispose();
}
this._model = connectionWithPassword;
this.uiController.fillInConnectionInputs(this._model);
}
@@ -390,9 +382,6 @@ export class ConnectionDialogService implements IConnectionDialogService {
}
private async updateModelServerCapabilities(model: IConnectionProfile) {
if (this._model) {
this._model.dispose();
}
this._model = await this.createModel(model);
if (this._model.providerName) {
this._currentProviderType = this._model.providerName;
@@ -504,9 +493,6 @@ export class ConnectionDialogService implements IConnectionDialogService {
this._connectionDialog.onFillinConnectionInputs((input) => this.handleFillInConnectionInputs(input as IConnectionProfile));
this._connectionDialog.onResetConnection(() => this.handleProviderOnResetConnection());
this._connectionDialog.render();
this._connectionDialog.onClosed(() => {
this._model?.dispose();
});
}
this._connectionDialog.newConnectionParams = params;
this._connectionDialog.updateProvider(this._providerNameToDisplayNameMap[this._currentProviderType]);
@@ -514,7 +500,6 @@ export class ConnectionDialogService implements IConnectionDialogService {
const recentConnections: ConnectionProfile[] = this._connectionManagementService.getRecentConnections(params.providers);
await this._connectionDialog.open(recentConnections.length > 0);
this.uiController.focusOnOpen();
recentConnections.forEach(conn => conn.dispose());
}
private showErrorDialog(severity: Severity, headerTitle: string, message: string, messageDetails?: string): void {

View File

@@ -356,14 +356,10 @@ export class ConnectionDialogWidget extends Modal {
actionProvider.onRecentConnectionRemoved(() => {
const recentConnections: ConnectionProfile[] = this.connectionManagementService.getRecentConnections();
this.open(recentConnections.length > 0).catch(err => this.logService.error(`Unexpected error opening connection widget after a recent connection was removed from action provider: ${err}`));
// We're just using the connections to determine if there are connections to show, dispose them right after to clean up their handlers
recentConnections.forEach(conn => conn.dispose());
});
controller.onRecentConnectionRemoved(() => {
const recentConnections: ConnectionProfile[] = this.connectionManagementService.getRecentConnections();
this.open(recentConnections.length > 0).catch(err => this.logService.error(`Unexpected error opening connection widget after a recent connection was removed from controller : ${err}`));
// We're just using the connections to determine if there are connections to show, dispose them right after to clean up their handlers
recentConnections.forEach(conn => conn.dispose());
});
this._recentConnectionTree = TreeCreationUtils.createConnectionTree(treeContainer, this.instantiationService, this._configurationService, localize('connectionDialog.recentConnections', "Recent Connections"), controller);
if (this._recentConnectionTree instanceof AsyncServerTree) {

View File

@@ -858,9 +858,7 @@ export class ConnectionManagementService extends Disposable implements IConnecti
public hasRegisteredServers(): boolean {
const groups: ConnectionProfileGroup[] = this.getConnectionGroups();
const hasRegisteredServers: boolean = this.doHasRegisteredServers(groups);
groups.forEach(cpg => cpg.dispose());
return hasRegisteredServers;
return this.doHasRegisteredServers(groups);
}
private doHasRegisteredServers(root: ConnectionProfileGroup[]): boolean {

View File

@@ -8,7 +8,7 @@ import { IListService, IWorkbenchAsyncDataTreeOptions, WorkbenchAsyncDataTree }
import { FuzzyScore } from 'vs/base/common/filters';
import { TreeNode } from 'sql/workbench/services/objectExplorer/common/treeNode';
import { ConnectionProfile } from 'sql/platform/connection/common/connectionProfile';
import { IAsyncDataTreeNode, IAsyncDataTreeUpdateChildrenOptions, IAsyncDataTreeViewState } from 'vs/base/browser/ui/tree/asyncDataTree';
import { IAsyncDataTreeNode, IAsyncDataTreeUpdateChildrenOptions } from 'vs/base/browser/ui/tree/asyncDataTree';
import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
import { IThemeService } from 'vs/platform/theme/common/themeService';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
@@ -57,13 +57,6 @@ export class AsyncServerTree extends WorkbenchAsyncDataTree<ConnectionProfileGro
})
}
// Overriding the setInput method to dispose the original input when a new input is set
override async setInput(input: ConnectionProfileGroup, viewState?: IAsyncDataTreeViewState): Promise<void> {
const originalInput = this.getInput();
await super.setInput(input, viewState);
originalInput?.dispose();
}
/**
* The original implementation of getDataNode compares refrences of the elements to find the node.
* This is not working for our case as we are creating new elements everytime we refresh the tree.

View File

@@ -151,10 +151,7 @@ export class TreeUpdateUtils {
public static getTreeInput(connectionManagementService: IConnectionManagementService, providers?: string[]): ConnectionProfileGroup | undefined {
const groups = connectionManagementService.getConnectionGroups(providers);
const input = groups.find(group => group.isRoot);
// Dispose of the unused groups to clean up their handlers
groups.filter(g => g !== input).forEach(g => g.dispose());
return input;
return groups.find(group => group.isRoot);
}
public static hasObjectExplorerNode(connection: ConnectionProfile, connectionManagementService: IConnectionManagementService): boolean {