Skip to content

Commit

Permalink
preferences: fix duplicated resolution
Browse files Browse the repository at this point in the history
The `WorkspacePreferenceProvider` can do one of two things: It may
return preferences from the current workspace folder if the workspace is
a single root workspace, or it may return preferences as defined in the
workspace file if the workspace is a multi-root workspace.

Whenever we want to resolve a preference, the following scopes are
queried: `Default`, `User`, `Workspace`, and `Folder`.

When a single root workspace is opened the following happens:

- Query `Default` scope [...]
- Query `User` scope [...]
- Query `Workspace` scope: use `WorkspacePreferenceProvider` which
delegates to `FolderPreferenceProvider`.
- Query `Folder` scope: use `FoldersPreferenceProvider`.

Conclusion: The `FoldersPreferenceProvider` is queried twice.

In order to fix that issue this commit introduces a new component:
`TogglePreferenceProvider` which extends `PreferenceProvider` and wraps
another provider so that it can be enabled/disabled.

Whenever we are running a single root workspace, we'll enable the
`FoldersPreferenceProvider` used by `WorkspacePreferenceProvider` and
disable the one bound to the `Folder` scope.

Whenever we are running a multi root workspace, we'll disable the
`FoldersPreferenceProvider` used by `WorkspacePreferenceProvider` and
enable the one bound to the `Folder` scope.
  • Loading branch information
paul-marechal committed Feb 9, 2023
1 parent 6959b6a commit 120d1a3
Show file tree
Hide file tree
Showing 5 changed files with 185 additions and 53 deletions.
1 change: 1 addition & 0 deletions packages/core/src/browser/preferences/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ export * from './preference-provider';
export * from './preference-scope';
export * from './preference-language-override-service';
export * from './preference-validation-service';
export { TogglePreferenceProvider } from './toggle-preference-provider';
105 changes: 105 additions & 0 deletions packages/core/src/browser/preferences/toggle-preference-provider.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
// *****************************************************************************
// Copyright (C) 2023 Ericsson and others.
//
// This program and the accompanying materials are made available under the
// terms of the Eclipse Public License v. 2.0 which is available at
// http://www.eclipse.org/legal/epl-2.0.
//
// This Source Code may also be made available under the following Secondary
// Licenses when the conditions for such availability set forth in the Eclipse
// Public License v. 2.0 are satisfied: GNU General Public License, version 2
// with the GNU Classpath Exception which is available at
// https://www.gnu.org/software/classpath/license.html.
//
// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
// *****************************************************************************

/* eslint-disable @typescript-eslint/no-explicit-any */

import { URI } from '../../common';
import { PreferenceProvider, PreferenceProviderDataChanges, PreferenceResolveResult } from './preference-provider';

/**
* Allows enabling/disabling a {@link PreferenceProvider} by wrapping it.
*/
export class TogglePreferenceProvider extends PreferenceProvider {

#enabled: boolean;

constructor(
enabled: boolean,
protected provider: PreferenceProvider
) {
super();
this.#enabled = enabled;
this.provider.ready.then(() => this._ready.resolve());
this.provider.onDidPreferencesChanged(this.handleDidPreferencesChanged, this, this.toDispose);
}

get enabled(): boolean {
return this.#enabled;
}

set enabled(value: boolean) {
if (this.#enabled !== value) {
this.#enabled = value;
this.onDidPreferencesChangedEmitter.fire({});
}
}

getPreferences(resourceUri?: string): { [p: string]: any; } {
if (this.enabled) {
return this.provider.getPreferences(resourceUri);
}
return {};
}

async setPreference(key: string, value: any, resourceUri?: string): Promise<boolean> {
if (this.enabled) {
return this.provider.setPreference(key, value, resourceUri);
}
return false;
}

override get<T>(preferenceName: string, resourceUri?: string): T | undefined {
if (this.enabled) {
return this.provider.get<T>(preferenceName, resourceUri);
}
}

override resolve<T>(preferenceName: string, resourceUri?: string): PreferenceResolveResult<T> {
if (this.enabled) {
return this.provider.resolve<T>(preferenceName, resourceUri);
}
return {};
}

override getDomain(): string[] | undefined {
if (this.enabled) {
return this.provider.getDomain();
}
}

override getConfigUri(resourceUri?: string, sectionName?: string): URI | undefined {
if (this.enabled) {
return this.provider.getConfigUri(resourceUri, sectionName);
}
}

override getContainingConfigUri?(resourceUri?: string, sectionName?: string): URI | undefined {
if (this.enabled) {
return this.provider.getContainingConfigUri?.(resourceUri, sectionName);
}
}

override dispose(): void {
super.dispose();
this.provider.dispose();
}

protected handleDidPreferencesChanged(event: PreferenceProviderDataChanges): void {
if (this.enabled) {
this.onDidPreferencesChangedEmitter.fire(event);
}
}
}
23 changes: 8 additions & 15 deletions packages/preferences/src/browser/folders-preferences-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,21 +39,16 @@ export class FoldersPreferencesProvider extends PreferenceProvider {
protected readonly providers = new Map<string, FolderPreferenceProvider>();

@postConstruct()
protected async init(): Promise<void> {
await this.workspaceService.roots;

this.updateProviders();
this.workspaceService.onWorkspaceChanged(() => this.updateProviders());

const readyPromises: Promise<void>[] = [];
for (const provider of this.providers.values()) {
readyPromises.push(provider.ready.catch(e => console.error(e)));
}
Promise.all(readyPromises).then(() => this._ready.resolve());
protected init(): void {
this.workspaceService.roots.then(roots => {
this.updateProviders(roots);
this.workspaceService.onWorkspaceChanged(newRoots => this.updateProviders(newRoots));
const allReady = Array.from(this.providers.values(), provider => provider.ready);
Promise.allSettled(allReady).then(() => this._ready.resolve());
});
}

protected updateProviders(): void {
const roots = this.workspaceService.tryGetRoots();
protected updateProviders(roots: FileStat[]): void {
const toDelete = new Set(this.providers.keys());
for (const folder of roots) {
for (const configPath of this.configurations.getPaths()) {
Expand Down Expand Up @@ -94,7 +89,6 @@ export class FoldersPreferencesProvider extends PreferenceProvider {
return configUri;
}
}
return undefined;
}

override getDomain(): string[] {
Expand Down Expand Up @@ -232,5 +226,4 @@ export class FoldersPreferencesProvider extends PreferenceProvider {
this.toDispose.push(provider.onDidPreferencesChanged(change => this.onDidPreferencesChangedEmitter.fire(change)));
return provider;
}

}
89 changes: 65 additions & 24 deletions packages/preferences/src/browser/preference-bindings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,51 +14,92 @@
// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
// *****************************************************************************

import { Container, interfaces } from '@theia/core/shared/inversify';
import { PreferenceProvider, PreferenceScope } from '@theia/core/lib/browser/preferences';
import { interfaces } from '@theia/core/shared/inversify';
import { PreferenceProvider, PreferenceScope, TogglePreferenceProvider } from '@theia/core/lib/browser/preferences';
import { UserPreferenceProvider, UserPreferenceProviderFactory } from './user-preference-provider';
import { WorkspacePreferenceProvider } from './workspace-preference-provider';
import { WorkspaceFilePreferenceProvider, WorkspaceFilePreferenceProviderFactory, WorkspaceFilePreferenceProviderOptions } from './workspace-file-preference-provider';
import { FoldersPreferencesProvider } from './folders-preferences-provider';
import { FolderPreferenceProvider, FolderPreferenceProviderFactory, FolderPreferenceProviderFolder } from './folder-preference-provider';
import { UserConfigsPreferenceProvider } from './user-configs-preference-provider';
import { SectionPreferenceProviderUri, SectionPreferenceProviderSection } from './section-preference-provider';
import { WorkspaceService } from '@theia/workspace/lib/browser';

export function bindWorkspaceFilePreferenceProvider(bind: interfaces.Bind): void {
bind(WorkspaceFilePreferenceProviderFactory).toFactory(ctx => (options: WorkspaceFilePreferenceProviderOptions) => {
const child = new Container({ defaultScope: 'Singleton' });
child.parent = ctx.container;
child.bind(WorkspaceFilePreferenceProvider).toSelf();
const child = ctx.container.createChild();
child.bind(WorkspaceFilePreferenceProvider).toSelf().inSingletonScope();
child.bind(WorkspaceFilePreferenceProviderOptions).toConstantValue(options);
return child.get(WorkspaceFilePreferenceProvider);
});
}

export function bindFactory<F, C>(bind: interfaces.Bind,
export function bindFactory<F, C>(
bind: interfaces.Bind,
factoryId: interfaces.ServiceIdentifier<F>,
constructor: interfaces.Newable<C>,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
...parameterBindings: interfaces.ServiceIdentifier<any>[]): void {
bind(factoryId).toFactory(ctx =>
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(...args: any[]) => {
const child = new Container({ defaultScope: 'Singleton' });
child.parent = ctx.container;
for (let i = 0; i < parameterBindings.length; i++) {
child.bind(parameterBindings[i]).toConstantValue(args[i]);
}
child.bind(constructor).to(constructor);
return child.get(constructor);
}
);
...parameterBindings: interfaces.ServiceIdentifier<unknown>[]
): void {
bind(factoryId).toFactory(ctx => (...args: unknown[]) => {
const child = ctx.container.createChild();
parameterBindings.forEach((parameterBinding, i) => {
child.bind(parameterBinding).toConstantValue(args[i]);
});
child.bind(constructor).to(constructor).inSingletonScope();
return child.get(constructor);
});
}

export function bindPreferenceProviders(bind: interfaces.Bind, unbind: interfaces.Unbind): void {
unbind(PreferenceProvider);

bind(PreferenceProvider).to(UserConfigsPreferenceProvider).inSingletonScope().whenTargetNamed(PreferenceScope.User);
bind(PreferenceProvider).to(WorkspacePreferenceProvider).inSingletonScope().whenTargetNamed(PreferenceScope.Workspace);
bind(PreferenceProvider).to(FoldersPreferencesProvider).inSingletonScope().whenTargetNamed(PreferenceScope.Folder);
// #region bind FoldersPreferencesProvider based on the status of the workspace:
bind(FoldersPreferencesProvider)
.toSelf()
.inSingletonScope()
.whenTargetIsDefault();
// Bind a FoldersPreferencesProvider that's only enabled if the workspace
// is a single root workspace:
bind<PreferenceProvider>(FoldersPreferencesProvider)
.toDynamicValue(ctx => {
const workspaceService = ctx.container.get(WorkspaceService);
const foldersPreferencesProvider = ctx.container.get(FoldersPreferencesProvider);
const preferenceProvider = new TogglePreferenceProvider(!workspaceService.isMultiRootWorkspaceOpened, foldersPreferencesProvider);
workspaceService.onWorkspaceChanged(() => {
preferenceProvider.enabled = !workspaceService.isMultiRootWorkspaceOpened;
});
return preferenceProvider;
})
.inSingletonScope()
.whenTargetNamed(PreferenceScope.Workspace);
// Bind a FoldersPreferencesProvider that's only enabled if the workspace
// is a multi root workspace:
bind<PreferenceProvider>(FoldersPreferencesProvider)
.toDynamicValue(ctx => {
const workspaceService = ctx.container.get(WorkspaceService);
const foldersPreferencesProvider = ctx.container.get(FoldersPreferencesProvider);
const preferenceProvider = new TogglePreferenceProvider(workspaceService.isMultiRootWorkspaceOpened, foldersPreferencesProvider);
workspaceService.onWorkspaceChanged(() => {
preferenceProvider.enabled = workspaceService.isMultiRootWorkspaceOpened;
});
return preferenceProvider;
})
.inSingletonScope()
.whenTargetNamed(PreferenceScope.Folder);
// #endregion
// #region bind PreferenceProvider by PreferenceScope:
bind(PreferenceProvider)
.to(UserConfigsPreferenceProvider)
.inSingletonScope()
.whenTargetNamed(PreferenceScope.User);
bind(PreferenceProvider)
.to(WorkspacePreferenceProvider)
.inSingletonScope()
.whenTargetNamed(PreferenceScope.Workspace);
bind(PreferenceProvider)
.toDynamicValue(ctx => ctx.container.getNamed(FoldersPreferencesProvider, PreferenceScope.Folder))
.inSingletonScope()
.whenTargetNamed(PreferenceScope.Folder);
// #endregion
bindWorkspaceFilePreferenceProvider(bind);
bindFactory(bind, UserPreferenceProviderFactory, UserPreferenceProvider, SectionPreferenceProviderUri, SectionPreferenceProviderSection);
bindFactory(bind, FolderPreferenceProviderFactory, FolderPreferenceProvider, SectionPreferenceProviderUri, SectionPreferenceProviderSection, FolderPreferenceProviderFolder);
Expand Down
20 changes: 6 additions & 14 deletions packages/preferences/src/browser/workspace-preference-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { DisposableCollection } from '@theia/core/lib/common/disposable';
import { PreferenceScope, PreferenceProvider } from '@theia/core/lib/browser/preferences';
import { WorkspaceService } from '@theia/workspace/lib/browser/workspace-service';
import { WorkspaceFilePreferenceProviderFactory, WorkspaceFilePreferenceProvider } from './workspace-file-preference-provider';
import { FoldersPreferencesProvider } from './folders-preferences-provider';

@injectable()
export class WorkspacePreferenceProvider extends PreferenceProvider {
Expand All @@ -32,7 +33,7 @@ export class WorkspacePreferenceProvider extends PreferenceProvider {
@inject(WorkspaceFilePreferenceProviderFactory)
protected readonly workspaceFileProviderFactory: WorkspaceFilePreferenceProviderFactory;

@inject(PreferenceProvider) @named(PreferenceScope.Folder)
@inject(FoldersPreferencesProvider) @named(PreferenceScope.Workspace)
protected readonly folderPreferenceProvider: PreferenceProvider;

protected readonly toDisposeOnEnsureDelegateUpToDate = new DisposableCollection();
Expand Down Expand Up @@ -102,33 +103,24 @@ export class WorkspacePreferenceProvider extends PreferenceProvider {
}

override get<T>(preferenceName: string, resourceUri: string | undefined = this.ensureResourceUri()): T | undefined {
const delegate = this.delegate;
return delegate ? delegate.get<T>(preferenceName, resourceUri) : undefined;
return this.delegate?.get<T>(preferenceName, resourceUri);
}

override resolve<T>(preferenceName: string, resourceUri: string | undefined = this.ensureResourceUri()): { value?: T, configUri?: URI } {
const delegate = this.delegate;
return delegate ? delegate.resolve<T>(preferenceName, resourceUri) : {};
return this.delegate?.resolve<T>(preferenceName, resourceUri) ?? {};
}

getPreferences(resourceUri: string | undefined = this.ensureResourceUri()): { [p: string]: any } {
const delegate = this.delegate;
return delegate ? delegate.getPreferences(resourceUri) : {};
return this.delegate?.getPreferences(resourceUri) ?? {};
}

async setPreference(preferenceName: string, value: any, resourceUri: string | undefined = this.ensureResourceUri()): Promise<boolean> {
const delegate = this.delegate;
if (delegate) {
return delegate.setPreference(preferenceName, value, resourceUri);
}
return false;
return this.delegate?.setPreference(preferenceName, value, resourceUri) ?? false;
}

protected ensureResourceUri(): string | undefined {
if (this.workspaceService.workspace && !this.workspaceService.isMultiRootWorkspaceOpened) {
return this.workspaceService.workspace.resource.toString();
}
return undefined;
}

}

0 comments on commit 120d1a3

Please sign in to comment.