Skip to content

Commit

Permalink
validation in preference proxy
Browse files Browse the repository at this point in the history
  • Loading branch information
@colin-grant-work
colin-grant-work committed Jan 17, 2022
1 parent d144ca5 commit 95f6c5d
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 58 deletions.
76 changes: 48 additions & 28 deletions packages/core/src/browser/preferences/preference-proxy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@

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

import { Disposable, Event, MaybePromise } from '../../common';
import { Disposable, Emitter, Event, MaybePromise } from '../../common';
import { PreferenceService } from './preference-service';
import { PreferenceSchema } from './preference-contribution';
import { PreferenceScope } from './preference-scope';
import { OverridePreferenceName } from './preference-language-override-service';
import { PreferenceValidationService } from './preference-validation-service';

/**
* It is worth explaining the type for `PreferenceChangeEvent`:
Expand Down Expand Up @@ -167,7 +168,12 @@ export interface PreferenceProxyOptions {
*
* Note that if `schema` is a Promise, most actions will be no-ops until the promise is resolved.
*/
export function createPreferenceProxy<T>(preferences: PreferenceService, promisedSchema: MaybePromise<PreferenceSchema>, options?: PreferenceProxyOptions): PreferenceProxy<T> {
export function createPreferenceProxy<T>(
preferences: PreferenceService,
promisedSchema: MaybePromise<PreferenceSchema>,
options?: PreferenceProxyOptions,
validator?: PreferenceValidationService
): PreferenceProxy<T> {
const opts = options || {};
const prefix = opts.prefix || '';
const style = opts.style || 'flat';
Expand All @@ -179,31 +185,43 @@ export function createPreferenceProxy<T>(preferences: PreferenceService, promise
} else {
promisedSchema.then(s => schema = s);
}
const onPreferenceChanged = (listener: (e: PreferenceChangeEvent<T>) => any, thisArgs?: any, disposables?: Disposable[]) => preferences.onPreferencesChanged(changes => {
if (schema) {
for (const key of Object.keys(changes)) {
const e = changes[key];
const overridden = preferences.overriddenPreferenceName(e.preferenceName);
const preferenceName: any = overridden ? overridden.preferenceName : e.preferenceName;
if (preferenceName.startsWith(prefix) && (!overridden || !opts.overrideIdentifier || overridden.overrideIdentifier === opts.overrideIdentifier)) {
if (schema.properties[preferenceName]) {
const { newValue, oldValue } = e;
listener({
newValue, oldValue, preferenceName,
affects: (resourceUri, overrideIdentifier) => {
if (overrideIdentifier !== undefined) {
if (overridden && overridden.overrideIdentifier !== overrideIdentifier) {
return false;
let emitter: Emitter | undefined;
const onPreferenceChanged = (listener: (e: PreferenceChangeEvent<T>) => any, thisArgs?: any, disposables?: Disposable[]) => ensureEmitter()
.event(listener, thisArgs, disposables);

const ensureEmitter = (): Emitter<PreferenceChangeEvent<T>> => {
if (emitter !== undefined) {
return emitter;
}
emitter = new Emitter<PreferenceChangeEvent<T>>();
preferences.onPreferencesChanged(changes => {
if (schema) {
for (const key of Object.keys(changes)) {
const e = changes[key];
const overridden = preferences.overriddenPreferenceName(e.preferenceName);
const preferenceName: any = overridden ? overridden.preferenceName : e.preferenceName;
if (preferenceName.startsWith(prefix) && (!overridden || !opts.overrideIdentifier || overridden.overrideIdentifier === opts.overrideIdentifier)) {
if (schema.properties[preferenceName]) {
const { newValue, oldValue } = e;
const validatedNewValue = validator ? validator.validateByName(preferenceName, newValue) : newValue;
emitter!.fire(<PreferenceChangeEvent<T>>{
newValue: validatedNewValue, oldValue, preferenceName,
affects: (resourceUri, overrideIdentifier) => {
if (overrideIdentifier !== undefined) {
if (overridden && overridden.overrideIdentifier !== overrideIdentifier) {
return false;
}
}
return e.affects(resourceUri);
}
return e.affects(resourceUri);
}
});
});
}
}
}
}
}
}, thisArgs, disposables);
});
return emitter;
};

const unsupportedOperation = (_: any, __: string) => {
throw new Error('Unsupported operation');
Expand All @@ -213,7 +231,8 @@ export function createPreferenceProxy<T>(preferences: PreferenceService, promise
const preferenceName = OverridePreferenceName.is(arg) ?
preferences.overridePreferenceName(arg) :
<string>arg;
return preferences.get(preferenceName, defaultValue, resourceUri || opts.resourceUri);
const value = preferences.get(preferenceName, defaultValue, resourceUri || opts.resourceUri);
return validator ? validator.validateByName(preferenceName, value) : value;
};

const ownKeys: () => string[] = () => {
Expand Down Expand Up @@ -259,7 +278,7 @@ export function createPreferenceProxy<T>(preferences: PreferenceService, promise
resourceUri: opts.resourceUri,
overrideIdentifier: opts.overrideIdentifier,
style
});
}, validator);
for (const k of Object.keys(value)) {
subProxy[k] = value[k];
}
Expand All @@ -275,7 +294,7 @@ export function createPreferenceProxy<T>(preferences: PreferenceService, promise
}
const fullProperty = prefix ? prefix + property : property;
if (schema) {
if (isFlat || property.indexOf('.') === -1) {
if (isFlat || !property.includes('.')) {
if (schema.properties[fullProperty]) {
let value;
if (opts.overrideIdentifier) {
Expand All @@ -287,7 +306,7 @@ export function createPreferenceProxy<T>(preferences: PreferenceService, promise
if (value === undefined) {
value = preferences.get(fullProperty, undefined, opts.resourceUri);
}
return value;
return (validator && value !== undefined) ? validator.validateByName(fullProperty, value) : value;
}
}
}
Expand All @@ -310,7 +329,8 @@ export function createPreferenceProxy<T>(preferences: PreferenceService, promise
const newPrefix = fullProperty + '.';
for (const p of Object.keys(schema.properties)) {
if (p.startsWith(newPrefix)) {
return createPreferenceProxy(preferences, schema, { prefix: newPrefix, resourceUri: opts.resourceUri, overrideIdentifier: opts.overrideIdentifier, style });
return createPreferenceProxy(preferences, schema,
{ prefix: newPrefix, resourceUri: opts.resourceUri, overrideIdentifier: opts.overrideIdentifier, style }, validator);
}
}

Expand All @@ -330,7 +350,7 @@ export function createPreferenceProxy<T>(preferences: PreferenceService, promise
while (typeof value === 'object' && (segment = segments.pop())) {
value = value[segment];
}
return segments.length ? undefined : value;
return segments.length ? undefined : validator?.validateByName(fullProperty, value) ?? value;
}
return undefined;
};
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/browser/preferences/preference-validation-service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('Preference Validation Service', () => {
container.bind(PreferenceSchemaProvider).toConstantValue({ getDefaultValue: PreferenceSchemaProvider.prototype.getDefaultValue } as PreferenceSchemaProvider);
container.bind(PreferenceLanguageOverrideService).toSelf().inSingletonScope();
const validator = container.resolve(PreferenceValidationService);
const validateBySchema: (value: JSONValue, schema: PreferenceItem) => JSONValue = validator.validateBySchema.bind(validator, 'dummy');
const validateBySchema: (value: JSONValue, schema: PreferenceItem) => JSONValue = validator['doValidateBySchema'].bind(validator, 'dummy');

describe('should validate strings', () => {
const expected = 'expected';
Expand Down
19 changes: 13 additions & 6 deletions packages/core/src/browser/preferences/preference-validation-service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ export class PreferenceValidationService {
const validValue = this.validateByName(preferenceName, value);
if (validValue !== value) {
problemsDetected = true;
console.warn(`While validating options, found impermissible value for ${preferenceName}. Using valid value`, validValue, 'instead of configured value', value);
}
valid[preferenceName] = validValue;
}
Expand All @@ -62,6 +61,14 @@ export class PreferenceValidationService {
}

validateBySchema(key: string, value: JSONValue, schema: ValidatablePreference | undefined): JSONValue {
const validated = this.doValidateBySchema(key, value, schema);
if (validated !== value) {
console.warn(`While validating prefrences, found impermissible value for ${key}. Using valid value`, validated, 'instead of configured value', value, new Error());
}
return validated;
}

protected doValidateBySchema(key: string, value: JSONValue, schema: ValidatablePreference | undefined): JSONValue {
try {
if (!schema) {
console.warn('Request to validate preference with no schema registered:', key);
Expand Down Expand Up @@ -118,7 +125,7 @@ export class PreferenceValidationService {
const candidate = this.mapValidators(key, value, (function* (this: PreferenceValidationService): Iterable<ValueValidator> {
for (const type of schema.type) {
validation.type = type as JsonType;
yield toValidate => this.validateBySchema(key, toValidate, validation);
yield toValidate => this.doValidateBySchema(key, toValidate, validation);
}
}).bind(this)());
if (candidate !== value && (schema.default !== undefined || schema.defaultValue !== undefined)) {
Expand All @@ -131,7 +138,7 @@ export class PreferenceValidationService {
protected validateAnyOf(key: string, value: JSONValue, schema: ValidatablePreference & { anyOf: ValidatablePreference[] }): JSONValue {
const candidate = this.mapValidators(key, value, (function* (this: PreferenceValidationService): Iterable<ValueValidator> {
for (const option of schema.anyOf) {
yield toValidate => this.validateBySchema(key, toValidate, option);
yield toValidate => this.doValidateBySchema(key, toValidate, option);
}
}).bind(this)());
if (candidate !== value && (schema.default !== undefined || schema.defaultValue !== undefined)) {
Expand Down Expand Up @@ -164,7 +171,7 @@ export class PreferenceValidationService {
}
const valid = [];
for (const item of candidate) {
const validated = this.validateBySchema(key, item, schema.items);
const validated = this.doValidateBySchema(key, item, schema.items);
if (validated === item) {
valid.push(item);
}
Expand Down Expand Up @@ -242,12 +249,12 @@ export class PreferenceValidationService {
for (const [fieldKey, fieldValue] of Object.entries(value)) {
const fieldLabel = `${key}#${fieldKey}`;
if (schema.properties && fieldKey in schema.properties) {
const valid = this.validateBySchema(fieldLabel, fieldValue, schema.properties[fieldKey]);
const valid = this.doValidateBySchema(fieldLabel, fieldValue, schema.properties[fieldKey]);
if (valid !== fieldValue) {
return false;
}
} else if (additionalPropertyValidator) {
const valid = this.validateBySchema(fieldLabel, fieldValue, additionalPropertyValidator);
const valid = this.doValidateBySchema(fieldLabel, fieldValue, additionalPropertyValidator);
if (valid !== fieldValue) {
return false;
}
Expand Down
12 changes: 8 additions & 4 deletions packages/editor/src/browser/editor-preferences.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ import {
PreferenceContribution,
PreferenceSchema,
PreferenceChangeEvent,
PreferenceSchemaProperties
PreferenceSchemaProperties,
PreferenceValidationService
} from '@theia/core/lib/browser/preferences';
import { isWindows, isOSX, OS } from '@theia/core/lib/common/os';
import { nls } from '@theia/core/lib/common/nls';
Expand Down Expand Up @@ -1582,15 +1583,18 @@ export const EditorPreferenceContribution = Symbol('EditorPreferenceContribution
export const EditorPreferences = Symbol('EditorPreferences');
export type EditorPreferences = PreferenceProxy<EditorConfiguration>;

export function createEditorPreferences(preferences: PreferenceService, schema: PreferenceSchema = editorPreferenceSchema): EditorPreferences {
return createPreferenceProxy(preferences, schema);
export function createEditorPreferences(
preferences: PreferenceService, schema: PreferenceSchema = editorPreferenceSchema, validator?: PreferenceValidationService
): EditorPreferences {
return createPreferenceProxy(preferences, schema, undefined, validator);
}

export function bindEditorPreferences(bind: interfaces.Bind): void {
bind(EditorPreferences).toDynamicValue(ctx => {
const preferences = ctx.container.get<PreferenceService>(PreferenceService);
const contribution = ctx.container.get<PreferenceContribution>(EditorPreferenceContribution);
return createEditorPreferences(preferences, contribution.schema);
const validator = ctx.container.get(PreferenceValidationService);
return createEditorPreferences(preferences, contribution.schema, validator);
}).inSingletonScope();
bind(EditorPreferenceContribution).toConstantValue({ schema: editorPreferenceSchema });
bind(PreferenceContribution).toService(EditorPreferenceContribution);
Expand Down
27 changes: 8 additions & 19 deletions packages/monaco/src/browser/monaco-editor-provider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import { MonacoBulkEditService } from './monaco-bulk-edit-service';
import IEditorOverrideServices = monaco.editor.IEditorOverrideServices;
import { ApplicationServer } from '@theia/core/lib/common/application-protocol';
import { ContributionProvider } from '@theia/core';
import { KeybindingRegistry, OpenerService, open, WidgetOpenerOptions, FormatType, PreferenceValidationService } from '@theia/core/lib/browser';
import { KeybindingRegistry, OpenerService, open, WidgetOpenerOptions, FormatType } from '@theia/core/lib/browser';
import { MonacoResolvedKeybinding } from './monaco-resolved-keybinding';
import { HttpOpenHandlerOptions } from '@theia/core/lib/browser/http-open-handler';
import { MonacoToProtocolConverter } from './monaco-to-protocol-converter';
Expand Down Expand Up @@ -74,9 +74,6 @@ export class MonacoEditorProvider {
@inject(MonacoQuickInputImplementation)
protected readonly quickInputService: MonacoQuickInputImplementation;

@inject(PreferenceValidationService)
protected readonly preferenceValidator: PreferenceValidationService;

protected _current: MonacoEditor | undefined;
/**
* Returns the last focused MonacoEditor.
Expand Down Expand Up @@ -274,9 +271,7 @@ export class MonacoEditorProvider {
if (event) {
const preferenceName = event.preferenceName;
const overrideIdentifier = editor.document.languageId;
const newValue = this.preferenceValidator.validateByName(preferenceName,
this.editorPreferences.get({ preferenceName, overrideIdentifier }, undefined, editor.uri.toString())
);
const newValue = this.editorPreferences.get({ preferenceName, overrideIdentifier }, undefined, editor.uri.toString());
editor.getControl().updateOptions(this.setOption(preferenceName, newValue, this.preferencePrefixes));
} else {
const options = this.createMonacoEditorOptions(editor.document);
Expand Down Expand Up @@ -305,9 +300,7 @@ export class MonacoEditorProvider {
}
const overrideIdentifier = editor.document.languageId;
const uri = editor.uri.toString();
const formatOnSave = this.preferenceValidator.validateByName('editor.formatOnSave',
this.editorPreferences.get({ preferenceName: 'editor.formatOnSave', overrideIdentifier }, undefined, uri)!
);
const formatOnSave = this.editorPreferences.get({ preferenceName: 'editor.formatOnSave', overrideIdentifier }, undefined, uri)!;
if (formatOnSave) {
const formatOnSaveTimeout = this.editorPreferences.get({ preferenceName: 'editor.formatOnSaveTimeout', overrideIdentifier }, undefined, uri)!;
await Promise.race([
Expand Down Expand Up @@ -358,9 +351,7 @@ export class MonacoEditorProvider {
if (event) {
const preferenceName = event.preferenceName;
const overrideIdentifier = editor.document.languageId;
const newValue = this.preferenceValidator.validateByName(preferenceName,
this.editorPreferences.get({ preferenceName, overrideIdentifier }, undefined, resourceUri)
);
const newValue = this.editorPreferences.get({ preferenceName, overrideIdentifier }, undefined, resourceUri);
editor.diffEditor.updateOptions(this.setOption(preferenceName, newValue, this.diffPreferencePrefixes));
} else {
const options = this.createMonacoDiffEditorOptions(editor.originalModel, editor.modifiedModel);
Expand All @@ -372,12 +363,10 @@ export class MonacoEditorProvider {
protected createOptions(prefixes: string[], uri: string): { [name: string]: any };
protected createOptions(prefixes: string[], uri: string, overrideIdentifier: string): { [name: string]: any };
protected createOptions(prefixes: string[], uri: string, overrideIdentifier?: string): { [name: string]: any } {
const flat: Record<string, any> = {};
for (const preferenceName of Object.keys(this.editorPreferences)) {
flat[preferenceName] = (<any>this.editorPreferences).get({ preferenceName, overrideIdentifier }, undefined, uri);
}
const valid = this.preferenceValidator.validateOptions(flat);
return Object.entries(valid).reduce((tree, [preferenceName, value]) => this.setOption(preferenceName, deepClone(value), prefixes, tree), {});
return Object.keys(this.editorPreferences).reduce((options, preferenceName) => {
const value = (<any>this.editorPreferences).get({ preferenceName, overrideIdentifier }, undefined, uri);
return this.setOption(preferenceName, deepClone(value), prefixes, options);
}, {});
}

protected setOption(preferenceName: string, value: any, prefixes: string[], options: { [name: string]: any } = {}): {
Expand Down

0 comments on commit 95f6c5d

Please sign in to comment.