Skip to content

Commit

Permalink
[core] Don't reject schemas with issues, but add valid properties.
Browse files Browse the repository at this point in the history 
Improves #4902

Signed-off-by: Sven Efftinge <[email protected]>
  • Loading branch information
@svenefftinge
svenefftinge committed Oct 8, 2019
1 parent 0f7fc56 commit f65d9cb
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 79 deletions.
105 changes: 46 additions & 59 deletions packages/core/src/browser/preferences/preference-contribution.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { ContributionProvider, bindContributionProvider, escapeRegExpCharacters,
import { PreferenceScope } from './preference-scope';
import { PreferenceProvider, PreferenceProviderDataChange } from './preference-provider';
import {
PreferenceSchema, PreferenceSchemaProperties, PreferenceDataSchema, PreferenceItem, PreferenceSchemaProperty, PreferenceDataProperty, JsonType
PreferenceSchema, PreferenceSchemaProperties, PreferenceDataSchema, PreferenceItem, PreferenceSchemaProperty, PreferenceDataProperty, JsonType, PreferenceDataItem
} from '../../common/preferences/preference-schema';
import { FrontendApplicationConfigProvider } from '../frontend-application-config-provider';
import { FrontendApplicationConfig } from '@theia/application-package/lib/application-props';
Expand Down Expand Up @@ -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;
Expand All @@ -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();
}

Expand Down Expand Up @@ -173,42 +170,63 @@ 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 preference schema with validation issues was registered: ' + errors);
}
const changes: PreferenceProviderDataChange[] = [];
const scope = PreferenceScope.Default;
const domain = this.getDomain();
const changes: PreferenceProviderDataChange[] = [];
const defaultScope = PreferenceSchema.getDefaultScope(schema);
const overridable = schema.overridable || false;
for (const preferenceName of Object.keys(schema.properties)) {
if (this.combinedSchema.properties[preferenceName]) {
console.error('Preference name collision detected in the schema for property: ' + preferenceName);
} else {
const schemaProps = PreferenceDataProperty.fromPreferenceSchemaProperty(schema.properties[preferenceName], defaultScope);
if (typeof schemaProps.overridable !== 'boolean' && overridable) {
schemaProps.overridable = true;
}
if (schemaProps.overridable) {
this.overridePatternProperties.properties[preferenceName] = schemaProps;
const cleanedProps = this.cleanProperties(schema.properties, defaultScope, overridable);
for (const propertyName of Object.keys(cleanedProps)) {
const schemaProp = cleanedProps[propertyName];
this.combinedSchema.properties[propertyName] = <PreferenceDataProperty>schemaProp;

const value = schemaProp.defaultValue = this.getDefaultValue(schemaProp, propertyName);
if (this.testOverrideValue(propertyName, value)) {
for (const overriddenPreferenceName in value) {
const overrideValue = value[overriddenPreferenceName];
const overridePreferenceName = `${propertyName}.${overriddenPreferenceName}`;
changes.push(this.doSetPreferenceValue(overridePreferenceName, overrideValue, { scope, domain }));
}
this.combinedSchema.properties[preferenceName] = schemaProps;
this.unsupportedPreferences.delete(preferenceName);
} else {
changes.push(this.doSetPreferenceValue(propertyName, value, { scope, domain }));
}
}
return changes;
}

const value = schemaProps.defaultValue = this.getDefaultValue(schemaProps, preferenceName);
if (this.testOverrideValue(preferenceName, value)) {
for (const overriddenPreferenceName in value) {
const overrideValue = value[overriddenPreferenceName];
const overridePreferenceName = `${preferenceName}.${overriddenPreferenceName}`;
changes.push(this.doSetPreferenceValue(overridePreferenceName, overrideValue, { scope, domain }));
}
protected cleanProperties(properties: { [key: string]: PreferenceSchemaProperty | PreferenceDataProperty },
defaultScope: PreferenceScope, overrideable: boolean, prefix?: string): { [key: string]: PreferenceDataProperty } {
for (const preferenceName of Object.keys(properties)) {
const longName = (prefix ? prefix + '.' : '') + preferenceName;
if (this.combinedSchema.properties[longName]) {
console.error('Preference name collision detected in the schema for property: ' + longName);
delete properties[preferenceName];
} else {
const property = properties[preferenceName];
if (!PreferenceDataItem.isValid(property)) {
console.warn(`Removing invalid property ${preferenceName}`);
delete properties[preferenceName];
} else {
changes.push(this.doSetPreferenceValue(preferenceName, value, { scope, domain }));
const schemaProps = PreferenceDataProperty.fromPreferenceSchemaProperty(property, defaultScope);
if (typeof schemaProps.overridable !== 'boolean' && overrideable) {
schemaProps.overridable = true;
}
if (schemaProps.overridable) {
this.overridePatternProperties.properties[preferenceName] = schemaProps;
}
const nested = schemaProps.properties || schemaProps.items && schemaProps.items.properties;
if (nested) {
this.cleanProperties(nested, defaultScope, overrideable, longName);
}
properties[preferenceName] = schemaProps;
}
}
}
return changes;
return properties as { [k: string]: PreferenceDataProperty };
}

protected doSetPreferenceValue(preferenceName: string, newValue: any, { scope, domain }: {
scope: PreferenceScope,
domain?: string[]
Expand Down Expand Up @@ -250,37 +268,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;
}
Expand Down
16 changes: 2 additions & 14 deletions packages/core/src/browser/preferences/preference-proxy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = () => {
Expand Down Expand Up @@ -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') {
Expand Down
5 changes: 0 additions & 5 deletions packages/core/src/browser/preferences/preference-service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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);
}
}
11 changes: 10 additions & 1 deletion packages/core/src/common/preferences/preference-schema.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export interface PreferenceSchemaProperties {
}
export namespace PreferenceSchemaProperties {
export function is(obj: Object | undefined): obj is PreferenceSchemaProperties {
return !!obj && typeof obj === 'object';
return !!obj && typeof obj !== 'object';
}
}

Expand Down Expand Up @@ -77,6 +77,15 @@ export interface PreferenceItem {
[name: string]: any;
overridable?: boolean;
}
(window as any).invocations = 1;
export namespace PreferenceDataItem {
export function isValid(property: PreferenceItem): boolean {
if (!property || !(typeof property === 'object')) {
return false;
}
return true;
}
}

export interface PreferenceSchemaProperty extends PreferenceItem {
description?: string;
Expand Down

0 comments on commit f65d9cb

Please sign in to comment.