[core] Don't reject schemas with issues, but add valid properties.

Improves #4902 Signed-off-by: Sven Efftinge <[email protected]>
eclipse-theia · Oct 8, 2019 · 6311ef1 · 6311ef1
1 parent 0f7fc56
commit 6311ef1
Show file tree

Hide file tree

Showing 5 changed files with 4 additions and 62 deletions.
diff --git a/packages/core/src/browser/preferences/preference-contribution.ts b/packages/core/src/browser/preferences/preference-contribution.ts
@@ -77,7 +77,6 @@ export class PreferenceSchemaProvider extends PreferenceProvider {
 
     @inject(ContributionProvider) @named(PreferenceContribution)
     protected readonly preferenceContributions: ContributionProvider<PreferenceContribution>;
-    protected validateFunction: Ajv.ValidateFunction;
 
     @inject(PreferenceConfigurations)
     protected readonly configurations: PreferenceConfigurations;
@@ -94,8 +93,6 @@ export class PreferenceSchemaProvider extends PreferenceProvider {
             this.doSetSchema(contrib.schema);
         });
         this.combinedSchema.additionalProperties = false;
-        this.updateValidate();
-        this.onDidPreferencesChanged(() => this.updateValidate());
         this._ready.resolve();
     }
 
@@ -173,8 +170,7 @@ export class PreferenceSchemaProvider extends PreferenceProvider {
         const valid = ajv.validateSchema(schema);
         if (!valid) {
             const errors = !!ajv.errors ? ajv.errorsText(ajv.errors) : 'unknown validation error';
-            console.error('An invalid preference schema was rejected: ' + errors);
-            return [];
+            console.warn('A contributed preference schema has validation issues : ' + errors);
         }
         const scope = PreferenceScope.Default;
         const domain = this.getDomain();
@@ -193,7 +189,6 @@ export class PreferenceSchemaProvider extends PreferenceProvider {
                     this.overridePatternProperties.properties[preferenceName] = schemaProps;
                 }
                 this.combinedSchema.properties[preferenceName] = schemaProps;
-                this.unsupportedPreferences.delete(preferenceName);
 
                 const value = schemaProps.defaultValue = this.getDefaultValue(schemaProps, preferenceName);
                 if (this.testOverrideValue(preferenceName, value)) {
@@ -209,6 +204,7 @@ export class PreferenceSchemaProvider extends PreferenceProvider {
         }
         return changes;
     }
+
     protected doSetPreferenceValue(preferenceName: string, newValue: any, { scope, domain }: {
         scope: PreferenceScope,
         domain?: string[]
@@ -250,37 +246,6 @@ export class PreferenceSchemaProvider extends PreferenceProvider {
         return null;
     }
 
-    protected updateValidate(): void {
-        const schema = {
-            ...this.combinedSchema,
-            properties: {
-                ...this.combinedSchema.properties
-            }
-        };
-        for (const sectionName of this.configurations.getSectionNames()) {
-            delete schema.properties[sectionName];
-        }
-        this.validateFunction = new Ajv().compile(schema);
-    }
-
-    protected readonly unsupportedPreferences = new Set<string>();
-    validate(name: string, value: any): boolean {
-        if (this.configurations.isSectionName(name)) {
-            return true;
-        }
-        const overridden = this.overriddenPreferenceName(name);
-        const preferenceName = overridden && overridden.preferenceName || name;
-        const result = this.validateFunction({ [preferenceName]: value }) as boolean;
-        if (!result && !(name in this.combinedSchema.properties)) {
-            // in order to avoid reporting it on each change
-            if (!this.unsupportedPreferences.has(name)) {
-                this.unsupportedPreferences.add(name);
-                console.warn(`"${name}" preference is not supported`);
-            }
-        }
-        return result;
-    }
-
     getCombinedSchema(): PreferenceDataSchema {
         return this.combinedSchema;
     }

diff --git a/packages/core/src/browser/preferences/preference-proxy.ts b/packages/core/src/browser/preferences/preference-proxy.ts
@@ -86,15 +86,7 @@ export function createPreferenceProxy<T>(preferences: PreferenceService, schema:
         const preferenceName = OverridePreferenceName.is(arg) ?
             preferences.overridePreferenceName(arg) :
             <string>arg;
-        const value = preferences.get(preferenceName, defaultValue, resourceUri || opts.resourceUri);
-        if (preferences.validate(preferenceName, value)) {
-            return value;
-        }
-        if (defaultValue !== undefined) {
-            return defaultValue;
-        }
-        const values = preferences.inspect(preferenceName, resourceUri);
-        return values && values.defaultValue;
+        return preferences.get(preferenceName, defaultValue, resourceUri || opts.resourceUri);
     };
 
     const ownKeys: () => string[] = () => {
@@ -163,11 +155,7 @@ export function createPreferenceProxy<T>(preferences: PreferenceService, schema:
                 if (value === undefined) {
                     value = preferences.get(fullProperty, undefined, opts.resourceUri);
                 }
-                if (preferences.validate(fullProperty, value)) {
-                    return value;
-                }
-                const values = preferences.inspect(fullProperty, opts.resourceUri);
-                return values && values.defaultValue;
+                return value;
             }
         }
         if (property === 'onPreferenceChanged') {

diff --git a/packages/core/src/browser/preferences/preference-service.spec.ts b/packages/core/src/browser/preferences/preference-service.spec.ts
@@ -220,8 +220,6 @@ describe('Preference Service', () => {
         })), 'events before');
         assert.strictEqual(prefService.get('editor.insertSpaces'), true, 'get before');
         assert.strictEqual(prefService.get('[go].editor.insertSpaces'), false, 'get before overridden');
-        assert.strictEqual(prefSchema.validate('editor.insertSpaces', false), true, 'validate before');
-        assert.strictEqual(prefSchema.validate('[go].editor.insertSpaces', true), true, 'validate before overridden');
 
         events.length = 0;
         toUnset.dispose();
@@ -241,8 +239,6 @@ describe('Preference Service', () => {
         })), 'events after');
         assert.strictEqual(prefService.get('editor.insertSpaces'), undefined, 'get after');
         assert.strictEqual(prefService.get('[go].editor.insertSpaces'), undefined, 'get after overridden');
-        assert.strictEqual(prefSchema.validate('editor.insertSpaces', true), false, 'validate after');
-        assert.strictEqual(prefSchema.validate('[go].editor.insertSpaces', true), false, 'validate after overridden');
     });
 
     describe('overridden preferences', () => {

diff --git a/packages/core/src/browser/preferences/preference-service.ts b/packages/core/src/browser/preferences/preference-service.ts
@@ -88,7 +88,6 @@ export interface PreferenceService extends Disposable {
     overriddenPreferenceName(preferenceName: string): OverridePreferenceName | undefined;
 
     resolve<T>(preferenceName: string, defaultValue?: T, resourceUri?: string): PreferenceResolveResult<T>;
-    validate(name: string, value: any): boolean;
 }
 
 /**
@@ -358,8 +357,4 @@ export class PreferenceServiceImpl implements PreferenceService {
             value: result.value !== undefined ? deepFreeze(result.value) : defaultValue
         };
     }
-
-    validate(name: string, value: any): boolean {
-        return this.schema.validate(name, value);
-    }
 }
diff --git a/packages/editor-preview/src/browser/editor-preview-manager.spec.ts b/packages/editor-preview/src/browser/editor-preview-manager.spec.ts
@@ -87,12 +87,10 @@ 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 () => {