From fea73da7b817eef813cb3039dbd6a3cfa4546c68 Mon Sep 17 00:00:00 2001 From: Cai Xuye Date: Tue, 3 Sep 2019 14:59:51 +0800 Subject: [PATCH 1/2] fix #6046: workspace configuration contributed by VSCode extension and provided by .theia/settins.json is sometimes wrongly filtered out Signed-off-by: Cai Xuye --- .../abstract-resource-preference-provider.ts | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/packages/preferences/src/browser/abstract-resource-preference-provider.ts b/packages/preferences/src/browser/abstract-resource-preference-provider.ts index c45cf6adb1eb4..d73c5ea13dc79 100644 --- a/packages/preferences/src/browser/abstract-resource-preference-provider.ts +++ b/packages/preferences/src/browser/abstract-resource-preference-provider.ts @@ -147,14 +147,9 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi if (typeof jsonData !== 'object') { return preferences; } - const uri = this.getUri(); // tslint:disable-next-line:forin for (const preferenceName in jsonData) { const preferenceValue = jsonData[preferenceName]; - if (!this.validate(preferenceName, preferenceValue)) { - console.warn(`Preference ${preferenceName} in ${uri} is invalid.`); - continue; - } if (this.schemaProvider.testOverrideValue(preferenceName, preferenceValue)) { // tslint:disable-next-line:forin for (const overriddenPreferenceName in preferenceValue) { @@ -168,14 +163,6 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi return preferences; } - protected validate(preferenceName: string, preferenceValue: any): boolean { - // in case if preferences are loaded from .vscode folder we should load even invalid - if (this.configurations.getPath(this.getUri()) !== this.configurations.getPaths()[0]) { - return true; - } - return preferenceValue === undefined || this.schemaProvider.validate(preferenceName, preferenceValue); - } - protected parse(content: string): any { content = content.trim(); if (!content) { From 013dbf1e2a1756100989b7dc515e96e2b6a614c2 Mon Sep 17 00:00:00 2001 From: Cai Xuye Date: Fri, 6 Sep 2019 15:19:14 +0800 Subject: [PATCH 2/2] Add schema check to statically typed APIs. Signed-off-by: Cai Xuye --- .../browser/preferences/preference-proxy.ts | 20 ++++++++++++++++--- .../browser/preferences/preference-service.ts | 4 ++++ .../test/mock-preference-service.ts | 3 +++ .../browser/editor-preview-manager.spec.ts | 2 ++ 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/packages/core/src/browser/preferences/preference-proxy.ts b/packages/core/src/browser/preferences/preference-proxy.ts index cf1719e17ac3f..61d104e2dab16 100644 --- a/packages/core/src/browser/preferences/preference-proxy.ts +++ b/packages/core/src/browser/preferences/preference-proxy.ts @@ -68,15 +68,29 @@ export function createPreferenceProxy(preferences: PreferenceService, schema: throw new Error('Unsupported operation'); }; const getValue: PreferenceRetrieval['get'] = (arg, defaultValue, resourceUri) => { - const preferenceName = typeof arg === 'object' && arg.overrideIdentifier ? + const isArgOverridePreferenceName = typeof arg === 'object' && arg.overrideIdentifier; + const preferenceName = isArgOverridePreferenceName ? preferences.overridePreferenceName(arg) : arg; - return preferences.get(preferenceName, defaultValue, resourceUri); + const value = preferences.get(preferenceName, defaultValue, resourceUri); + if (preferences.validate(isArgOverridePreferenceName ? (arg).preferenceName : preferenceName, value)) { + return value; + } + if (defaultValue !== undefined) { + return defaultValue; + } + const values = preferences.inspect(preferenceName, resourceUri); + return values && values.defaultValue; }; return new Proxy({}, { get: (_, property: string) => { if (schema.properties[property]) { - return preferences.get(property); + const value = preferences.get(property); + if (preferences.validate(property, value)) { + return value; + } + const values = preferences.inspect(property); + return values && values.defaultValue; } if (property === 'onPreferenceChanged') { return onPreferenceChangedEmitter.event; diff --git a/packages/core/src/browser/preferences/preference-service.ts b/packages/core/src/browser/preferences/preference-service.ts index 28510288a7d4d..eb305912b1302 100644 --- a/packages/core/src/browser/preferences/preference-service.ts +++ b/packages/core/src/browser/preferences/preference-service.ts @@ -89,6 +89,7 @@ export interface PreferenceService extends Disposable { overriddenPreferenceName(preferenceName: string): OverridePreferenceName | undefined; resolve(preferenceName: string, defaultValue?: T, resourceUri?: string): PreferenceResolveResult; + validate(name: string, value: any): boolean; } /** @@ -412,4 +413,7 @@ export class PreferenceServiceImpl implements PreferenceService, FrontendApplica }; } + validate(name: string, value: any): boolean { + return this.schema.validate(name, value); + } } diff --git a/packages/core/src/browser/preferences/test/mock-preference-service.ts b/packages/core/src/browser/preferences/test/mock-preference-service.ts index 9841e2ef41f8d..b88c5eb1c75c1 100644 --- a/packages/core/src/browser/preferences/test/mock-preference-service.ts +++ b/packages/core/src/browser/preferences/test/mock-preference-service.ts @@ -55,4 +55,7 @@ export class MockPreferenceService implements PreferenceService { overriddenPreferenceName(preferenceName: string): OverridePreferenceName | undefined { return undefined; } + + // tslint:disable-next-line:no-any + validate(name: string, value: any): boolean { return true; } } diff --git a/packages/editor-preview/src/browser/editor-preview-manager.spec.ts b/packages/editor-preview/src/browser/editor-preview-manager.spec.ts index 31267e249bdf5..ce84c4316f24e 100644 --- a/packages/editor-preview/src/browser/editor-preview-manager.spec.ts +++ b/packages/editor-preview/src/browser/editor-preview-manager.spec.ts @@ -85,10 +85,12 @@ describe('editor-preview-manager', () => { it('should handle preview requests if editor.enablePreview enabled', async () => { (mockPreference.get as sinon.SinonStub).returns(true); + (mockPreference.validate as sinon.SinonStub).returns(true); expect(await previewManager.canHandle(new URI(), {preview: true})).to.be.greaterThan(0); }); it('should not handle preview requests if editor.enablePreview disabled', async () => { (mockPreference.get as sinon.SinonStub).returns(false); + (mockPreference.validate as sinon.SinonStub).returns(true); expect(await previewManager.canHandle(new URI(), {preview: true})).to.equal(0); }); it('should not handle requests that are not preview or currently being previewed', async () => {