debt: lazy creation of services in config (#160082)

* debt: lazy creation of services in config

* spell check

* use same instance of ConfigurationEditing

* fix unit tests
This commit is contained in:
Sandeep Somavarapu
2022-09-05 23:39:20 +02:00
committed by GitHub
parent 4542a16b0b
commit 1872bc1f7e
5 changed files with 31 additions and 38 deletions
@@ -13,7 +13,6 @@ import { ConfigurationModel, ConfigurationModelParser, ConfigurationParseOptions
import { WorkspaceConfigurationModelParser, StandaloneConfigurationModelParser } from 'vs/workbench/services/configuration/common/configurationModels';
import { TASKS_CONFIGURATION_KEY, FOLDER_SETTINGS_NAME, LAUNCH_CONFIGURATION_KEY, IConfigurationCache, ConfigurationKey, REMOTE_MACHINE_SCOPES, FOLDER_SCOPES, WORKSPACE_SCOPES } from 'vs/workbench/services/configuration/common/configuration';
import { IStoredWorkspaceFolder } from 'vs/platform/workspaces/common/workspaces';
import { JSONEditingService } from 'vs/workbench/services/configuration/common/jsonEditingService';
import { WorkbenchState, IWorkspaceFolder, IWorkspaceIdentifier } from 'vs/platform/workspace/common/workspace';
import { ConfigurationScope, Extensions, IConfigurationRegistry, OVERRIDE_PROPERTY_REGEX } from 'vs/platform/configuration/common/configurationRegistry';
import { equals } from 'vs/base/common/objects';
@@ -27,6 +26,7 @@ import { Registry } from 'vs/platform/registry/common/platform';
import { IBrowserWorkbenchEnvironmentService } from 'vs/workbench/services/environment/browser/environmentService';
import { isObject } from 'vs/base/common/types';
import { DefaultConfiguration as BaseDefaultConfiguration } from 'vs/platform/configuration/common/configurations';
import { IJSONEditingService } from 'vs/workbench/services/configuration/common/jsonEditing';
export class DefaultConfiguration extends BaseDefaultConfiguration {
@@ -629,7 +629,7 @@ export class WorkspaceConfiguration extends Disposable {
return this._workspaceConfiguration.getFolders();
}
setFolders(folders: IStoredWorkspaceFolder[], jsonEditingService: JSONEditingService): Promise<void> {
setFolders(folders: IStoredWorkspaceFolder[], jsonEditingService: IJSONEditingService): Promise<void> {
if (this._workspaceIdentifier) {
return jsonEditingService.write(this._workspaceIdentifier.configPath, [{ path: ['folders'], value: folders }], true)
.then(() => this.reload());
@@ -20,9 +20,8 @@ import { Registry } from 'vs/platform/registry/common/platform';
import { IConfigurationRegistry, Extensions, allSettings, windowSettings, resourceSettings, applicationSettings, machineSettings, machineOverridableSettings, ConfigurationScope, IConfigurationPropertySchema, keyFromOverrideIdentifiers, OVERRIDE_PROPERTY_PATTERN, resourceLanguageSettingsSchemaId, configurationDefaultsSchemaId } from 'vs/platform/configuration/common/configurationRegistry';
import { IStoredWorkspaceFolder, isStoredWorkspaceFolder, IWorkspaceFolderCreationData, getStoredWorkspaceFolder, toWorkspaceFolders } from 'vs/platform/workspaces/common/workspaces';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { ConfigurationEditingService, EditableConfigurationTarget } from 'vs/workbench/services/configuration/common/configurationEditingService';
import { ConfigurationEditing, EditableConfigurationTarget } from 'vs/workbench/services/configuration/common/configurationEditing';
import { WorkspaceConfiguration, FolderConfiguration, RemoteUserConfiguration, UserConfiguration, DefaultConfiguration } from 'vs/workbench/services/configuration/browser/configuration';
import { JSONEditingService } from 'vs/workbench/services/configuration/common/jsonEditingService';
import { IJSONSchema, IJSONSchemaMap } from 'vs/base/common/jsonSchema';
import { mark } from 'vs/base/common/performance';
import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService';
@@ -45,6 +44,7 @@ import { IPolicyService, NullPolicyService } from 'vs/platform/policy/common/pol
import { IUserDataProfile, IUserDataProfilesService } from 'vs/platform/userDataProfile/common/userDataProfile';
import { updateIgnoredSettings } from 'vs/platform/userDataSync/common/settingsMerge';
import { VSBuffer } from 'vs/base/common/buffer';
import { IJSONEditingService } from 'vs/workbench/services/configuration/common/jsonEditing';
function getLocalUserConfigurationScopes(userDataProfile: IUserDataProfile, hasRemote: boolean): ConfigurationScope[] | undefined {
return userDataProfile.isDefault
@@ -100,11 +100,8 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat
private readonly configurationRegistry: IConfigurationRegistry;
// TODO@sandeep debt with cyclic dependencies
private configurationEditingService!: ConfigurationEditingService;
private jsonEditingService!: JSONEditingService;
private cyclicDependencyReady!: Function;
private cyclicDependency = new Promise<void>(resolve => this.cyclicDependencyReady = resolve);
private instantiationService: IInstantiationService | undefined;
private configurationEditing: ConfigurationEditing | undefined;
constructor(
{ remoteAuthority, configurationCache }: { remoteAuthority?: string; configurationCache: IConfigurationCache },
@@ -112,7 +109,7 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat
private readonly userDataProfileService: IUserDataProfileService,
private readonly userDataProfilesService: IUserDataProfilesService,
private readonly fileService: IFileService,
remoteAgentService: IRemoteAgentService,
private readonly remoteAgentService: IRemoteAgentService,
private readonly uriIdentityService: IUriIdentityService,
private readonly logService: ILogService,
policyService: IPolicyService
@@ -207,7 +204,6 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat
}
public async updateFolders(foldersToAdd: IWorkspaceFolderCreationData[], foldersToRemove: URI[], index?: number): Promise<void> {
await this.cyclicDependency;
return this.workspaceEditingQueue.queue(() => this.doUpdateFolders(foldersToAdd, foldersToRemove, index));
}
@@ -303,8 +299,11 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat
}
private async setFolders(folders: IStoredWorkspaceFolder[]): Promise<void> {
await this.cyclicDependency;
await this.workspaceConfiguration.setFolders(folders, this.jsonEditingService);
if (!this.instantiationService) {
throw new Error('Cannot update workspace folders because workspace service is not yet ready to accept writes.');
}
await this.instantiationService.invokeFunction(accessor => this.workspaceConfiguration.setFolders(folders, accessor.get(IJSONEditingService)));
return this.onWorkspaceConfigurationChanged(false);
}
@@ -333,7 +332,6 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat
updateValue(key: string, value: any, target: ConfigurationTarget): Promise<void>;
updateValue(key: string, value: any, overrides: IConfigurationOverrides | IConfigurationUpdateOverrides, target: ConfigurationTarget, donotNotifyError?: boolean): Promise<void>;
async updateValue(key: string, value: any, arg3?: any, arg4?: any, donotNotifyError?: any): Promise<void> {
await this.cyclicDependency;
const overrides: IConfigurationUpdateOverrides | undefined = isConfigurationUpdateOverrides(arg3) ? arg3
: isConfigurationOverrides(arg3) ? { resource: arg3.resource, overrideIdentifiers: arg3.overrideIdentifier ? [arg3.overrideIdentifier] : undefined } : undefined;
const target: ConfigurationTarget | undefined = overrides ? arg4 : arg3;
@@ -482,14 +480,7 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat
}
acquireInstantiationService(instantiationService: IInstantiationService): void {
this.configurationEditingService = instantiationService.createInstance(ConfigurationEditingService);
this.jsonEditingService = instantiationService.createInstance(JSONEditingService);
if (this.cyclicDependencyReady) {
this.cyclicDependencyReady();
} else {
this.cyclicDependency = Promise.resolve(undefined);
}
this.instantiationService = instantiationService;
}
private async createWorkspace(arg: IAnyWorkspaceIdentifier): Promise<Workspace> {
@@ -981,6 +972,10 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat
}
private async writeConfigurationValue(key: string, value: any, target: ConfigurationTarget, overrides: IConfigurationUpdateOverrides | undefined, donotNotifyError: boolean): Promise<void> {
if (!this.instantiationService) {
throw new Error('Cannot write configuration because the configuration service is not yet ready to accept writes.');
}
if (target === ConfigurationTarget.DEFAULT) {
throw new Error('Invalid configuration target');
}
@@ -1001,7 +996,9 @@ export class WorkspaceService extends Disposable implements IWorkbenchConfigurat
throw new Error('Invalid configuration target');
}
await this.configurationEditingService.writeConfiguration(editableConfigurationTarget, { key, value }, { scopes: overrides, donotNotifyError });
// Use same instance of ConfigurationEditing to make sure all writes go through the same queue
this.configurationEditing = this.configurationEditing ?? this.instantiationService.createInstance(ConfigurationEditing, (await this.remoteAgentService.getEnvironment())?.settingsPath ?? null);
await this.configurationEditing.writeConfiguration(editableConfigurationTarget, { key, value }, { scopes: overrides, donotNotifyError });
switch (editableConfigurationTarget) {
case EditableConfigurationTarget.USER_LOCAL:
if (this.applicationConfiguration && this.configurationRegistry.getConfigurationProperties()[key].scope === ConfigurationScope.APPLICATION) {
@@ -21,7 +21,6 @@ import { IEditorService } from 'vs/workbench/services/editor/common/editorServic
import { INotificationService, Severity } from 'vs/platform/notification/common/notification';
import { IOpenSettingsOptions, IPreferencesService } from 'vs/workbench/services/preferences/common/preferences';
import { withUndefinedAsNull, withNullAsUndefined } from 'vs/base/common/types';
import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService';
import { IUriIdentityService } from 'vs/platform/uriIdentity/common/uriIdentity';
import { ITextModel } from 'vs/editor/common/model';
import { IReference } from 'vs/base/common/lifecycle';
@@ -144,14 +143,14 @@ interface ConfigurationEditingOptions extends IConfigurationEditingOptions {
handleDirtyFile?: 'save' | 'revert';
}
export class ConfigurationEditingService {
export class ConfigurationEditing {
public _serviceBrand: undefined;
private queue: Queue<void>;
private remoteSettingsResource: URI | null = null;
constructor(
private readonly remoteSettingsResource: URI | null,
@IConfigurationService private readonly configurationService: IConfigurationService,
@IWorkspaceContextService private readonly contextService: IWorkspaceContextService,
@IUserDataProfileService private readonly userDataProfileService: IUserDataProfileService,
@@ -162,15 +161,9 @@ export class ConfigurationEditingService {
@INotificationService private readonly notificationService: INotificationService,
@IPreferencesService private readonly preferencesService: IPreferencesService,
@IEditorService private readonly editorService: IEditorService,
@IRemoteAgentService remoteAgentService: IRemoteAgentService,
@IUriIdentityService private readonly uriIdentityService: IUriIdentityService,
) {
this.queue = new Queue<void>();
remoteAgentService.getEnvironment().then(environment => {
if (environment) {
this.remoteSettingsResource = environment.settingsPath;
}
});
}
async writeConfiguration(target: EditableConfigurationTarget, value: IConfigurationValue, options: IConfigurationEditingOptions = {}): Promise<void> {
@@ -14,7 +14,7 @@ import { TestEnvironmentService, TestTextFileService, workbenchInstantiationServ
import * as uuid from 'vs/base/common/uuid';
import { IConfigurationRegistry, Extensions as ConfigurationExtensions } from 'vs/platform/configuration/common/configurationRegistry';
import { WorkspaceService } from 'vs/workbench/services/configuration/browser/configurationService';
import { ConfigurationEditingService, ConfigurationEditingErrorCode, EditableConfigurationTarget } from 'vs/workbench/services/configuration/common/configurationEditingService';
import { ConfigurationEditing, ConfigurationEditingErrorCode, EditableConfigurationTarget } from 'vs/workbench/services/configuration/common/configurationEditing';
import { WORKSPACE_STANDALONE_CONFIGURATIONS, FOLDER_SETTINGS_PATH, USER_STANDALONE_CONFIGURATIONS, IConfigurationCache } from 'vs/workbench/services/configuration/common/configuration';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
@@ -56,14 +56,14 @@ export class ConfigurationCache implements IConfigurationCache {
async remove(): Promise<void> { }
}
suite('ConfigurationEditingService', () => {
suite('ConfigurationEditing', () => {
let instantiationService: TestInstantiationService;
let userDataProfileService: IUserDataProfileService;
let environmentService: IWorkbenchEnvironmentService;
let fileService: IFileService;
let workspaceService: WorkspaceService;
let testObject: ConfigurationEditingService;
let testObject: ConfigurationEditing;
const disposables = new DisposableStore();
@@ -130,7 +130,7 @@ suite('ConfigurationEditingService', () => {
instantiationService.stub(ITextFileService, disposables.add(instantiationService.createInstance(TestTextFileService)));
instantiationService.stub(ITextModelService, <ITextModelService>disposables.add(instantiationService.createInstance(TextModelResolverService)));
instantiationService.stub(ICommandService, CommandService);
testObject = instantiationService.createInstance(ConfigurationEditingService);
testObject = instantiationService.createInstance(ConfigurationEditing, null);
});
teardown(() => disposables.clear());
@@ -10,7 +10,7 @@ import { Registry } from 'vs/platform/registry/common/platform';
import { IEnvironmentService } from 'vs/platform/environment/common/environment';
import { IConfigurationRegistry, Extensions as ConfigurationExtensions, ConfigurationScope, keyFromOverrideIdentifiers } from 'vs/platform/configuration/common/configurationRegistry';
import { WorkspaceService } from 'vs/workbench/services/configuration/browser/configurationService';
import { ConfigurationEditingErrorCode } from 'vs/workbench/services/configuration/common/configurationEditingService';
import { ConfigurationEditingErrorCode } from 'vs/workbench/services/configuration/common/configurationEditing';
import { IFileService } from 'vs/platform/files/common/files';
import { IWorkspaceContextService, WorkbenchState, IWorkspaceFoldersChangeEvent, ISingleFolderWorkspaceIdentifier, IWorkspaceIdentifier } from 'vs/platform/workspace/common/workspace';
import { ConfigurationTarget, IConfigurationService, IConfigurationChangeEvent } from 'vs/platform/configuration/common/configuration';
@@ -271,6 +271,7 @@ suite('WorkspaceContextService - Workspace Editing', () => {
await testObject.initialize(getWorkspaceIdentifier(configResource));
instantiationService.stub(ITextFileService, disposables.add(instantiationService.createInstance(TestTextFileService)));
instantiationService.stub(ITextModelService, disposables.add(instantiationService.createInstance(TextModelResolverService)));
instantiationService.stub(IJSONEditingService, instantiationService.createInstance(JSONEditingService));
testObject.acquireInstantiationService(instantiationService);
});
@@ -1636,10 +1637,11 @@ suite('WorkspaceConfigurationService-Multiroot', () => {
instantiationService.stub(IKeybindingEditingService, instantiationService.createInstance(KeybindingsEditingService));
instantiationService.stub(ITextFileService, instantiationService.createInstance(TestTextFileService));
instantiationService.stub(ITextModelService, <ITextModelService>instantiationService.createInstance(TextModelResolverService));
jsonEditingServce = instantiationService.createInstance(JSONEditingService);
instantiationService.stub(IJSONEditingService, jsonEditingServce);
workspaceService.acquireInstantiationService(instantiationService);
workspaceContextService = workspaceService;
jsonEditingServce = instantiationService.createInstance(JSONEditingService);
testObject = workspaceService;
});
@@ -2299,6 +2301,7 @@ suite('WorkspaceConfigurationService - Remote Folder', () => {
await testObject.initialize(convertToWorkspacePayload(folder));
instantiationService.stub(ITextFileService, instantiationService.createInstance(TestTextFileService));
instantiationService.stub(ITextModelService, <ITextModelService>instantiationService.createInstance(TextModelResolverService));
instantiationService.stub(IJSONEditingService, instantiationService.createInstance(JSONEditingService));
testObject.acquireInstantiationService(instantiationService);
}