From 397f360422dae3dc590e58665fd526d04d7810c7 Mon Sep 17 00:00:00 2001 From: Oleksii Kurinnyi Date: Fri, 29 Mar 2019 17:32:11 +0200 Subject: [PATCH] fixes Signed-off-by: Oleksii Kurinnyi --- CHANGELOG.md | 1 + .../preferences/preference-provider.ts | 4 +- .../abstract-launch-preference-provider.ts | 2 +- .../src/browser/folders-launch-provider.ts | 2 +- .../abstract-resource-preference-provider.ts | 64 +++++---------- .../folder-launch-preference-provider.ts | 2 +- .../src/browser/folder-preference-provider.ts | 3 +- .../browser/folders-preferences-provider.ts | 80 +++++++++++-------- .../src/browser/preference-frontend-module.ts | 4 +- .../src/browser/preference-service.spec.ts | 4 +- 10 files changed, 77 insertions(+), 89 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6309d2d6439c1..e0d907467ac9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## v0.6.0 - [filesystem] added the menu item `Upload Files...` to easily upload files into a workspace +- [preferences] changed signature for methods `getProvider`, `setProvider` and `createFolderPreferenceProvider` of `FoldersPreferenceProvider`. ## v0.5.0 diff --git a/packages/core/src/browser/preferences/preference-provider.ts b/packages/core/src/browser/preferences/preference-provider.ts index 427e0383441a2..cec9ead420226 100644 --- a/packages/core/src/browser/preferences/preference-provider.ts +++ b/packages/core/src/browser/preferences/preference-provider.ts @@ -47,8 +47,8 @@ export abstract class PreferenceProvider implements Disposable { protected readonly onDidPreferencesChangedEmitter = new Emitter(); readonly onDidPreferencesChanged: Event = this.onDidPreferencesChangedEmitter.event; - protected readonly onDidNotValidPreferencesReadEmitter = new Emitter(); - readonly onDidNotValidPreferencesRead: Event = this.onDidNotValidPreferencesReadEmitter.event; + protected readonly onDidInvalidPreferencesReadEmitter = new Emitter<{ [key: string]: any }>(); + readonly onDidInvalidPreferencesRead: Event<{ [key: string]: any }> = this.onDidInvalidPreferencesReadEmitter.event; protected readonly toDispose = new DisposableCollection(); diff --git a/packages/debug/src/browser/abstract-launch-preference-provider.ts b/packages/debug/src/browser/abstract-launch-preference-provider.ts index d3c5ec0a79ef4..ea2e3e28f22fb 100644 --- a/packages/debug/src/browser/abstract-launch-preference-provider.ts +++ b/packages/debug/src/browser/abstract-launch-preference-provider.ts @@ -94,7 +94,7 @@ export abstract class AbstractLaunchPreferenceProvider implements LaunchPreferen this.toDispose.push(this.onDidLaunchChangedEmitter); this.toDispose.push( - this.preferenceProvider.onDidNotValidPreferencesRead(prefs => { + this.preferenceProvider.onDidInvalidPreferencesRead(prefs => { if (!prefs || !GlobalLaunchConfig.is(prefs.launch)) { return; } diff --git a/packages/debug/src/browser/folders-launch-provider.ts b/packages/debug/src/browser/folders-launch-provider.ts index c602a2fad6c15..f161ba98f742b 100644 --- a/packages/debug/src/browser/folders-launch-provider.ts +++ b/packages/debug/src/browser/folders-launch-provider.ts @@ -53,7 +53,7 @@ export class FoldersLaunchProvider implements LaunchPreferenceProvider, Disposab this.toDispose.push(this.onDidLaunchChangedEmitter); this.toDispose.push( - this.preferenceProvider.onDidNotValidPreferencesRead(prefs => { + this.preferenceProvider.onDidInvalidPreferencesRead(prefs => { if (!prefs || !GlobalLaunchConfig.is(prefs.launch)) { return; } diff --git a/packages/preferences/src/browser/abstract-resource-preference-provider.ts b/packages/preferences/src/browser/abstract-resource-preference-provider.ts index 79268ffa3f061..92306dae842d4 100644 --- a/packages/preferences/src/browser/abstract-resource-preference-provider.ts +++ b/packages/preferences/src/browser/abstract-resource-preference-provider.ts @@ -26,8 +26,6 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi // tslint:disable-next-line:no-any protected preferences: { [key: string]: any } = {}; - // tslint:disable-next-line:no-any - protected notValidPreferences: { [key: string]: any } = {}; protected resource: Promise; protected toDisposeOnWorkspaceLocationChanged: DisposableCollection = new DisposableCollection(); @@ -62,9 +60,11 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi this.toDispose.push(onDidResourceChanged); } - this.schemaProvider.onDidPreferenceSchemaChanged(() => { - this.handleNotValidPreferences(); - }); + this.toDispose.push( + this.schemaProvider.onDidPreferenceSchemaChanged(() => { + this.readPreferences(); + }) + ); } abstract getUri(root?: URI): MaybePromise; @@ -109,13 +109,8 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi protected async readPreferences(): Promise { const newContent = await this.readContents(); - const parsedData = this.parse(newContent); - const newPrefs = await this.getValidatedPreferences(parsedData); - await this.handlePreferenceChanges(newPrefs.valid); - this.notValidPreferences = newPrefs.notValid; - if (this.notValidPreferences && Object.keys(this.notValidPreferences).length > 0) { - this.onDidNotValidPreferencesReadEmitter.fire(this.notValidPreferences); - } + const newPrefs = await this.getParsedContent(newContent); + await this.handlePreferenceChanges(newPrefs); } protected async readContents(): Promise { @@ -128,35 +123,37 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi } // tslint:disable-next-line:no-any - protected async getValidatedPreferences(jsonData: { [key: string]: any }): Promise<{ valid: any, notValid: any }> { + protected async getParsedContent(content: string): Promise<{ [key: string]: any }> { + const jsonData = this.parse(content); // tslint:disable-next-line:no-any - const preferences: { valid: any, notValid: any } = { - valid: {}, - notValid: {} - }; + const preferences: { [key: string]: any } = {}; + // tslint:disable-next-line:no-any + const notValidPreferences: { [key: string]: any } = {}; if (typeof jsonData !== 'object') { return preferences; } const uri = (await this.resource).uri.toString(); - this.notValidPreferences = {}; // tslint:disable-next-line:forin for (const preferenceName in jsonData) { const preferenceValue = jsonData[preferenceName]; - if (this.isNotValid(preferenceName, preferenceValue)) { + if (preferenceValue !== undefined && !this.schemaProvider.validate(preferenceName, preferenceValue)) { console.warn(`Preference ${preferenceName} in ${uri} is invalid.`); - preferences.notValid[preferenceName] = preferenceValue; + notValidPreferences[preferenceName] = preferenceValue; continue; } if (this.schemaProvider.testOverrideValue(preferenceName, preferenceValue)) { // tslint:disable-next-line:forin for (const overriddenPreferenceName in preferenceValue) { const overriddeValue = preferenceValue[overriddenPreferenceName]; - preferences.valid[`${preferenceName}.${overriddenPreferenceName}`] = overriddeValue; + preferences[`${preferenceName}.${overriddenPreferenceName}`] = overriddeValue; } } else { - preferences.valid[preferenceName] = preferenceValue; + preferences[preferenceName] = preferenceValue; } } + if (Object.keys(notValidPreferences).length > 0) { + this.onDidInvalidPreferencesReadEmitter.fire(notValidPreferences); + } return preferences; } @@ -166,11 +163,6 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi return jsoncparser.parse(strippedContent); } - // tslint:disable-next-line:no-any - protected isNotValid(preferenceName: string, preferenceValue: any): boolean { - return (preferenceValue !== undefined && !this.schemaProvider.validate(preferenceName, preferenceValue)); - } - // tslint:disable-next-line:no-any protected async handlePreferenceChanges(newPrefs: { [key: string]: any }): Promise { const oldPrefs = Object.assign({}, this.preferences); @@ -204,24 +196,6 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi } } - protected async handleNotValidPreferences(): Promise { - const notValidPreferencesKeys = Object.keys(this.notValidPreferences); - if (notValidPreferencesKeys.length === 0) { - return; - } - - const validPrefs = Object.assign({}, this.preferences); - const newPrefs = notValidPreferencesKeys - .filter(prefName => !this.isNotValid(prefName, this.notValidPreferences[prefName])) - .reduce((prefs, prefName) => { - prefs[prefName] = this.notValidPreferences[prefName]; - delete this.notValidPreferences[prefName]; - return prefs; - }, validPrefs); - - return this.handlePreferenceChanges(newPrefs); - } - dispose(): void { const prefChanges: PreferenceProviderDataChange[] = []; for (const prefName of Object.keys(this.preferences)) { diff --git a/packages/preferences/src/browser/folder-launch-preference-provider.ts b/packages/preferences/src/browser/folder-launch-preference-provider.ts index 34f2ca4161c73..04d0054ed675c 100644 --- a/packages/preferences/src/browser/folder-launch-preference-provider.ts +++ b/packages/preferences/src/browser/folder-launch-preference-provider.ts @@ -32,7 +32,7 @@ export class FolderLaunchPreferenceProvider extends FolderPreferenceProvider { // tslint:disable-next-line:no-any protected parse(content: string): any { const parsedData = super.parse(content); - if (Object.keys(parsedData).length > 0) { + if (!!parsedData && Object.keys(parsedData).length > 0) { return { launch: parsedData }; } return parsedData; diff --git a/packages/preferences/src/browser/folder-preference-provider.ts b/packages/preferences/src/browser/folder-preference-provider.ts index 60b9db202a946..30699b9192cc9 100644 --- a/packages/preferences/src/browser/folder-preference-provider.ts +++ b/packages/preferences/src/browser/folder-preference-provider.ts @@ -19,6 +19,7 @@ import URI from '@theia/core/lib/common/uri'; import { PreferenceScope, PreferenceProvider, PreferenceProviderPriority } from '@theia/core/lib/browser'; import { AbstractResourcePreferenceProvider } from './abstract-resource-preference-provider'; import { FileSystem, FileStat } from '@theia/filesystem/lib/common'; +import { ResourceKind } from './folders-preferences-provider'; export const FolderPreferenceProviderFactory = Symbol('FolderPreferenceProviderFactory'); export interface FolderPreferenceProviderFactory { @@ -28,7 +29,7 @@ export interface FolderPreferenceProviderFactory { export const FolderPreferenceProviderOptions = Symbol('FolderPreferenceProviderOptions'); export interface FolderPreferenceProviderOptions { folder: FileStat; - fileName: string; + kind: ResourceKind; } @injectable() diff --git a/packages/preferences/src/browser/folders-preferences-provider.ts b/packages/preferences/src/browser/folders-preferences-provider.ts index d40ea3a51da1d..71ab0c8d2778d 100644 --- a/packages/preferences/src/browser/folders-preferences-provider.ts +++ b/packages/preferences/src/browser/folders-preferences-provider.ts @@ -21,9 +21,8 @@ import { FolderPreferenceProvider, FolderPreferenceProviderFactory } from './fol import { FileSystem, FileStat } from '@theia/filesystem/lib/common'; import URI from '@theia/core/lib/common/uri'; -export const SETTINGS_FILE_NAME = 'settings.json'; -export const LAUNCH_FILE_NAME = 'launch.json'; export const LAUNCH_PROPERTY_NAME = 'launch'; +export type ResourceKind = 'settings' | 'launch'; @injectable() export class FoldersPreferencesProvider extends PreferenceProvider { @@ -32,19 +31,18 @@ export class FoldersPreferencesProvider extends PreferenceProvider { @inject(FileSystem) protected readonly fileSystem: FileSystem; @inject(FolderPreferenceProviderFactory) protected readonly folderPreferenceProviderFactory: FolderPreferenceProviderFactory; - private providers: { [fileName: string]: FolderPreferenceProvider[] } = {}; + private providersByKind: Map = new Map(); + private resourceKinds: ResourceKind[] = ['launch', 'settings']; @postConstruct() protected async init(): Promise { - [SETTINGS_FILE_NAME, LAUNCH_FILE_NAME].forEach(fileName => this.providers[fileName] = []); - await this.workspaceService.roots; const readyPromises: Promise[] = []; for (const root of this.workspaceService.tryGetRoots()) { if (await this.fileSystem.exists(root.uri)) { - [SETTINGS_FILE_NAME, LAUNCH_FILE_NAME].forEach(fileName => { - const provider = this.createFolderPreferenceProvider(root, fileName); - this.providers[fileName].push(provider); + this.resourceKinds.forEach(kind => { + const provider = this.createFolderPreferenceProvider(root, kind); + this.setProvider(provider, kind); readyPromises.push(provider.ready); }); } @@ -59,23 +57,24 @@ export class FoldersPreferencesProvider extends PreferenceProvider { this.workspaceService.onWorkspaceChanged(roots => { for (const root of roots) { - [SETTINGS_FILE_NAME, LAUNCH_FILE_NAME].forEach(fileName => { - if (!this.existsProvider(root.uri, fileName)) { - const provider = this.createFolderPreferenceProvider(root, fileName); - this.providers[fileName].push(provider); + this.resourceKinds.forEach(kind => { + if (!this.existsProvider(root.uri, kind)) { + const provider = this.createFolderPreferenceProvider(root, kind); + this.setProvider(provider, kind); } }); } - [SETTINGS_FILE_NAME, LAUNCH_FILE_NAME].forEach(fileName => { - if (!this.providers[fileName]) { + this.resourceKinds.forEach(kind => { + const providers = this.providersByKind.get(kind); + if (!providers || providers.length === 0) { return; } - const numProviders = this.providers[fileName].length; + const numProviders = providers.length; for (let i = numProviders - 1; i >= 0; i--) { - const provider = this.providers[fileName][i]; + const provider = providers[i]; if (!this.existsRoot(roots, provider)) { - this.providers[fileName].splice(i, 1); + providers.splice(i, 1); provider.dispose(); } } @@ -83,8 +82,9 @@ export class FoldersPreferencesProvider extends PreferenceProvider { }); } - private existsProvider(folderUri: string, fileName: string): boolean { - return this.providers[fileName] && this.providers[fileName].some(p => !!p.uri && p.uri.toString() === folderUri); + private existsProvider(folderUri: string, kind: ResourceKind): boolean { + const providers = this.providersByKind.get(kind); + return !!providers && providers.some(p => !!p.uri && p.uri.toString() === folderUri); } private existsRoot(roots: FileStat[], provider: FolderPreferenceProvider): boolean { @@ -97,10 +97,10 @@ export class FoldersPreferencesProvider extends PreferenceProvider { return {}; } - const prefProvider = this.getProvider(resourceUri, SETTINGS_FILE_NAME); + const prefProvider = this.getProvider(resourceUri, 'settings'); const prefs = prefProvider ? prefProvider.getPreferences() : {}; - const launchProvider = this.getProvider(resourceUri, LAUNCH_FILE_NAME); + const launchProvider = this.getProvider(resourceUri, 'launch'); const launch = launchProvider ? launchProvider.getPreferences() : {}; const result = Object.assign({}, prefs, launch); @@ -110,8 +110,8 @@ export class FoldersPreferencesProvider extends PreferenceProvider { canProvide(preferenceName: string, resourceUri?: string): { priority: number, provider: PreferenceProvider } { if (resourceUri) { - const resourceName = preferenceName === LAUNCH_PROPERTY_NAME ? LAUNCH_FILE_NAME : SETTINGS_FILE_NAME; - const provider = this.getProvider(resourceUri, resourceName); + const resourceKind = preferenceName === LAUNCH_PROPERTY_NAME ? 'launch' : 'settings'; + const provider = this.getProvider(resourceUri, resourceKind); if (provider) { return { priority: provider.canProvide(preferenceName, resourceUri).priority, provider }; } @@ -119,9 +119,9 @@ export class FoldersPreferencesProvider extends PreferenceProvider { return super.canProvide(preferenceName, resourceUri); } - protected getProvider(resourceUri: string, fileName: string): PreferenceProvider | undefined { - const providers = this.providers[fileName]; - if (providers.length === 0) { + protected getProvider(resourceUri: string, kind: ResourceKind): PreferenceProvider | undefined { + const providers = this.providersByKind.get(kind); + if (!providers || providers.length === 0) { return; } @@ -139,22 +139,34 @@ export class FoldersPreferencesProvider extends PreferenceProvider { return provider; } - protected createFolderPreferenceProvider(folder: FileStat, fileName: string): FolderPreferenceProvider { - const provider = this.folderPreferenceProviderFactory({ folder, fileName }); + protected setProvider(provider: FolderPreferenceProvider, kind: ResourceKind): void { + const providers = this.providersByKind.get(kind); + if (providers && Array.isArray(providers)) { + providers.push(provider); + } else { + this.providersByKind.set(kind, [provider]); + } + } + + protected createFolderPreferenceProvider(folder: FileStat, kind: ResourceKind): FolderPreferenceProvider { + const provider = this.folderPreferenceProviderFactory({ folder, kind }); this.toDispose.push(provider); this.toDispose.push(provider.onDidPreferencesChanged(change => this.onDidPreferencesChangedEmitter.fire(change))); - this.toDispose.push(provider.onDidNotValidPreferencesRead(prefs => this.onDidNotValidPreferencesReadEmitter.fire(prefs))); + this.toDispose.push(provider.onDidInvalidPreferencesRead(prefs => this.onDidInvalidPreferencesReadEmitter.fire(prefs))); return provider; } // tslint:disable-next-line:no-any async setPreference(key: string, value: any, resourceUri?: string): Promise { if (resourceUri) { - const resourceName = key === LAUNCH_PROPERTY_NAME ? LAUNCH_FILE_NAME : SETTINGS_FILE_NAME; - for (const provider of this.providers[resourceName]) { - const providerResourceUri = await provider.getUri(); - if (providerResourceUri && providerResourceUri.toString() === resourceUri) { - return provider.setPreference(key, value); + const resourceKind = key === LAUNCH_PROPERTY_NAME ? 'launch' : 'settings'; + const providers = this.providersByKind.get(resourceKind); + if (providers && providers.length) { + for (const provider of providers) { + const providerResourceUri = await provider.getUri(); + if (providerResourceUri && providerResourceUri.toString() === resourceUri) { + return provider.setPreference(key, value); + } } } console.error(`FoldersPreferencesProvider did not find the provider for ${resourceUri} to update the preference ${key}`); diff --git a/packages/preferences/src/browser/preference-frontend-module.ts b/packages/preferences/src/browser/preference-frontend-module.ts index e4ba2c74e150d..f8839b9217a5b 100644 --- a/packages/preferences/src/browser/preference-frontend-module.ts +++ b/packages/preferences/src/browser/preference-frontend-module.ts @@ -24,7 +24,7 @@ import { createPreferencesTreeWidget } from './preference-tree-container'; import { PreferencesMenuFactory } from './preferences-menu-factory'; import { PreferencesFrontendApplicationContribution } from './preferences-frontend-application-contribution'; import { PreferencesContainer, PreferencesTreeWidget, PreferencesEditorsContainer } from './preferences-tree-widget'; -import { FoldersPreferencesProvider, SETTINGS_FILE_NAME } from './folders-preferences-provider'; +import { FoldersPreferencesProvider } from './folders-preferences-provider'; import { FolderPreferenceProvider, FolderPreferenceProviderFactory, FolderPreferenceProviderOptions } from './folder-preference-provider'; import { FolderLaunchPreferenceProvider } from './folder-launch-preference-provider'; @@ -43,7 +43,7 @@ export function bindPreferences(bind: interfaces.Bind, unbind: interfaces.Unbind const child = new Container({ defaultScope: 'Transient' }); child.parent = ctx.container; child.bind(FolderPreferenceProviderOptions).toConstantValue(options); - if (options.fileName === SETTINGS_FILE_NAME) { + if (options.kind === 'settings') { return child.get(FolderPreferenceProvider); } else { return child.get(FolderLaunchPreferenceProvider); diff --git a/packages/preferences/src/browser/preference-service.spec.ts b/packages/preferences/src/browser/preference-service.spec.ts index 49a58ff15c603..75f3b01748b3b 100644 --- a/packages/preferences/src/browser/preference-service.spec.ts +++ b/packages/preferences/src/browser/preference-service.spec.ts @@ -743,7 +743,7 @@ describe('Preference Service', () => { it('should fire "onDidLaunchChanged" event with correct argument', async () => { const spy = sinon.spy(); - userProvider.onDidNotValidPreferencesRead(spy); + userProvider.onDidInvalidPreferencesRead(spy); fs.writeFileSync(tempPath, userConfigStr); await (userProvider).readPreferences(); @@ -778,7 +778,7 @@ describe('Preference Service', () => { it('should not fire "onDidLaunchChanged"', async () => { const spy = sinon.spy(); - userProvider.onDidNotValidPreferencesRead(spy); + userProvider.onDidInvalidPreferencesRead(spy); fs.writeFileSync(tempPath, userConfigStr); await (userProvider).readPreferences();