From 87ae8bc725db6a2c9ffab8c73eb6fa95407703cc Mon Sep 17 00:00:00 2001 From: TimVevida Date: Fri, 20 Apr 2018 12:14:17 +0200 Subject: [PATCH 1/2] feat(PropertyAccessorParser): Remove PropertyAccessorParser in favor of the keyof operator. Calls to .ensure() allowed both a string and a function as argument. The function was never actually executed, but parsed to get the string property name. The same effect can be achieved by using the keyof operator. In this case, only strings can be provided that are a property of the given class. Initial version, requiring the programmer to explicitly state the object class each time .ensure() is executed. This is because .ensure() is a static function, so the type cannot be determined yet. Fixes #485. --- src/aurelia-validation.ts | 5 +- src/implementation/validation-messages.ts | 2 +- src/implementation/validation-rules.ts | 47 +++++++----------- src/property-accessor-parser.ts | 39 --------------- src/validation-controller-factory.ts | 4 +- src/validation-controller.ts | 15 ++---- test/basic.ts | 11 +---- test/property-accessor-parser.ts | 52 -------------------- test/resources/nullable-object-form.ts | 2 +- test/resources/registration-form.ts | 15 +++--- test/resources/trigger-form.ts | 10 ++-- test/resources/validation-errors-form-one.ts | 2 +- test/server.ts | 4 +- test/validator.ts | 18 +++---- 14 files changed, 53 insertions(+), 173 deletions(-) delete mode 100644 src/property-accessor-parser.ts delete mode 100644 test/property-accessor-parser.ts diff --git a/src/aurelia-validation.ts b/src/aurelia-validation.ts index f5d8d47e..8c137770 100644 --- a/src/aurelia-validation.ts +++ b/src/aurelia-validation.ts @@ -3,7 +3,6 @@ export * from './controller-validate-result'; export * from './get-target-dom-element'; export * from './property-info'; -export * from './property-accessor-parser'; export * from './validate-binding-behavior'; export * from './validate-event'; export * from './validate-instruction'; @@ -30,7 +29,6 @@ import { Container } from 'aurelia-dependency-injection'; import { Validator } from './validator'; import { StandardValidator } from './implementation/standard-validator'; import { ValidationMessageParser } from './implementation/validation-message-parser'; -import { PropertyAccessorParser } from './property-accessor-parser'; import { ValidationRules } from './implementation/validation-rules'; /** @@ -65,8 +63,7 @@ export function configure( // the fluent rule definition API needs the parser to translate messages // to interpolation expressions. const messageParser = frameworkConfig.container.get(ValidationMessageParser); - const propertyParser = frameworkConfig.container.get(PropertyAccessorParser); - ValidationRules.initialize(messageParser, propertyParser); + ValidationRules.initialize(messageParser); // configure... const config = new AureliaValidationConfiguration(); diff --git a/src/implementation/validation-messages.ts b/src/implementation/validation-messages.ts index 5ab8f831..19e36c59 100644 --- a/src/implementation/validation-messages.ts +++ b/src/implementation/validation-messages.ts @@ -29,7 +29,7 @@ export const validationMessages: ValidationMessages = { export class ValidationMessageProvider { public static inject = [ValidationMessageParser]; - constructor(parser: ValidationMessageParser) { } + constructor(private parser: ValidationMessageParser) { } /** * Returns a message binding expression that corresponds to the key. diff --git a/src/implementation/validation-rules.ts b/src/implementation/validation-rules.ts index 2457f6e3..e6856028 100644 --- a/src/implementation/validation-rules.ts +++ b/src/implementation/validation-rules.ts @@ -2,7 +2,6 @@ import { Rule, RuleProperty, ValidationDisplayNameAccessor } from './rule'; import { ValidationMessageParser } from './validation-message-parser'; import { Rules } from './rules'; import { validationMessages } from './validation-messages'; -import { PropertyAccessorParser, PropertyAccessor } from '../property-accessor-parser'; import { isString } from '../util'; /** @@ -17,7 +16,7 @@ export class FluentRuleCustomizer { config: object = {}, private fluentEnsure: FluentEnsure, private fluentRules: FluentRules, - private parsers: Parsers + private messageParser: ValidationMessageParser ) { this.rule = { property, @@ -55,7 +54,7 @@ export class FluentRuleCustomizer { */ public withMessage(message: string) { this.rule.messageKey = 'custom'; - this.rule.message = this.parsers.message.parse(message); + this.rule.message = this.messageParser.parse(message); return this; } @@ -84,8 +83,8 @@ export class FluentRuleCustomizer { * Target a property with validation rules. * @param property The property to target. Can be the property name or a property accessor function. */ - public ensure(subject: string | ((model: TObject) => TValue2)) { - return this.fluentEnsure.ensure(subject); + public ensure(subject: keyof TObject) { + return this.fluentEnsure.ensure(subject); } /** @@ -217,7 +216,7 @@ export class FluentRules { constructor( private fluentEnsure: FluentEnsure, - private parsers: Parsers, + private messageParser: ValidationMessageParser, private property: RuleProperty ) { } @@ -237,7 +236,7 @@ export class FluentRules { */ public satisfies(condition: (value: TValue, object?: TObject) => boolean | Promise, config?: object) { return new FluentRuleCustomizer( - this.property, condition, config, this.fluentEnsure, this, this.parsers); + this.property, condition, config, this.fluentEnsure, this, this.messageParser); } /** @@ -357,21 +356,21 @@ export class FluentEnsure { */ public rules: Rule[][] = []; - constructor(private parsers: Parsers) { } + constructor(private messageParser: ValidationMessageParser) { } /** * Target a property with validation rules. * @param property The property to target. Can be the property name or a property accessor * function. */ - public ensure(property: string | PropertyAccessor) { + public ensure(property: keyof TObject) { this.assertInitialized(); - const name = this.parsers.property.parse(property); + const name = property as string; const fluentRules = new FluentRules( this, - this.parsers, + this.messageParser, { name, displayName: null }); - return this.mergeRules(fluentRules, name); + return this.mergeRules(fluentRules, property); } /** @@ -380,7 +379,7 @@ export class FluentEnsure { public ensureObject() { this.assertInitialized(); const fluentRules = new FluentRules( - this, this.parsers, { name: null, displayName: null }); + this, this.messageParser, { name: null, displayName: null }); return this.mergeRules(fluentRules, null); } @@ -405,7 +404,7 @@ export class FluentEnsure { } private assertInitialized() { - if (this.parsers) { + if (this.messageParser) { return; } throw new Error(`Did you forget to add ".plugin('aurelia-validation')" to your main.js?`); @@ -428,28 +427,25 @@ export class FluentEnsure { * Fluent rule definition API. */ export class ValidationRules { - private static parsers: Parsers; + private static messageParser: ValidationMessageParser; - public static initialize(messageParser: ValidationMessageParser, propertyParser: PropertyAccessorParser) { - this.parsers = { - message: messageParser, - property: propertyParser - }; + public static initialize(messageParser: ValidationMessageParser) { + this.messageParser = messageParser; } /** * Target a property with validation rules. * @param property The property to target. Can be the property name or a property accessor function. */ - public static ensure(property: string | PropertyAccessor) { - return new FluentEnsure(ValidationRules.parsers).ensure(property); + public static ensure(property: keyof TObject) { + return new FluentEnsure(ValidationRules.messageParser).ensure(property); } /** * Targets an object with validation rules. */ public static ensureObject() { - return new FluentEnsure(ValidationRules.parsers).ensureObject(); + return new FluentEnsure(ValidationRules.messageParser).ensureObject(); } /** @@ -495,8 +491,3 @@ export class ValidationRules { Rules.unset(target); } } - -export interface Parsers { - message: ValidationMessageParser; - property: PropertyAccessorParser; -} diff --git a/src/property-accessor-parser.ts b/src/property-accessor-parser.ts deleted file mode 100644 index 9bfc93ac..00000000 --- a/src/property-accessor-parser.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { - Parser, - AccessMember, - AccessScope -} from 'aurelia-binding'; -import { isString } from './util'; - -export type PropertyAccessor = (object: TObject) => TValue; - -export class PropertyAccessorParser { - public static inject = [Parser]; - - constructor(private parser: Parser) { } - - public parse(property: string | PropertyAccessor): string { - if (isString(property)) { - return property as string; - } - const accessorText = getAccessorExpression(property.toString()); - const accessor = this.parser.parse(accessorText); - if (accessor instanceof AccessScope - || accessor instanceof AccessMember && accessor.object instanceof AccessScope) { - return accessor.name; - } - throw new Error(`Invalid property expression: "${accessor}"`); - } -} - -export function getAccessorExpression(fn: string): string { - /* tslint:disable:max-line-length */ - const classic = /^function\s*\([$_\w\d]+\)\s*\{(?:\s*"use strict";)?\s*(?:[$_\w\d.['"\]+;]+)?\s*return\s+[$_\w\d]+\.([$_\w\d]+)\s*;?\s*\}$/; - /* tslint:enable:max-line-length */ - const arrow = /^\(?[$_\w\d]+\)?\s*=>\s*[$_\w\d]+\.([$_\w\d]+)$/; - const match = classic.exec(fn) || arrow.exec(fn); - if (match === null) { - throw new Error(`Unable to parse accessor function:\n${fn}`); - } - return match[1]; -} diff --git a/src/validation-controller-factory.ts b/src/validation-controller-factory.ts index 15a25ba9..de763b80 100644 --- a/src/validation-controller-factory.ts +++ b/src/validation-controller-factory.ts @@ -1,7 +1,6 @@ import { Container } from 'aurelia-dependency-injection'; import { ValidationController } from './validation-controller'; import { Validator } from './validator'; -import { PropertyAccessorParser } from './property-accessor-parser'; /** * Creates ValidationController instances. @@ -20,8 +19,7 @@ export class ValidationControllerFactory { if (!validator) { validator = this.container.get(Validator) as Validator; } - const propertyParser = this.container.get(PropertyAccessorParser) as PropertyAccessorParser; - return new ValidationController(validator, propertyParser); + return new ValidationController(validator); } /** diff --git a/src/validation-controller.ts b/src/validation-controller.ts index 9ef46acb..adab3942 100644 --- a/src/validation-controller.ts +++ b/src/validation-controller.ts @@ -6,7 +6,6 @@ import { ValidationRenderer, RenderInstruction } from './validation-renderer'; import { ValidateResult } from './validate-result'; import { ValidateInstruction } from './validate-instruction'; import { ControllerValidateResult } from './controller-validate-result'; -import { PropertyAccessorParser, PropertyAccessor } from './property-accessor-parser'; import { ValidateEvent } from './validate-event'; /** @@ -15,7 +14,7 @@ import { ValidateEvent } from './validate-event'; * Exposes the current list of validation results for binding purposes. */ export class ValidationController { - public static inject = [Validator, PropertyAccessorParser]; + public static inject = [Validator]; // Registered bindings (via the validate binding behavior) private bindings = new Map(); @@ -54,7 +53,7 @@ export class ValidationController { private eventCallbacks: ((event: ValidateEvent) => void)[] = []; - constructor(private validator: Validator, private propertyParser: PropertyAccessorParser) { } + constructor(private validator: Validator) { } /** * Subscribe to controller validate and reset events. These events occur when the @@ -101,15 +100,9 @@ export class ValidationController { public addError( message: string, object: TObject, - propertyName: string | PropertyAccessor | null = null + propertyName: string | null = null ): ValidateResult { - let resolvedPropertyName: string | null; - if (propertyName === null) { - resolvedPropertyName = propertyName; - } else { - resolvedPropertyName = this.propertyParser.parse(propertyName); - } - const result = new ValidateResult({ __manuallyAdded__: true }, object, resolvedPropertyName, false, message); + const result = new ValidateResult({ __manuallyAdded__: true }, object, propertyName, false, message); this.processResultDelta('validate', [], [result]); return result; } diff --git a/test/basic.ts b/test/basic.ts index b4e7a2a4..d62882f5 100644 --- a/test/basic.ts +++ b/test/basic.ts @@ -190,20 +190,13 @@ describe('end to end', () => { expect(error2.message).toBe('string property error'); expect(error2.object).toBe(viewModel); expect(error2.propertyName).toBe('lastName'); - const error3 = viewModel.controller.addError('expression property error', viewModel, vm => vm.firstName); - expect(error3.message).toBe('expression property error'); - expect(error3.object).toBe(viewModel); - expect(error3.propertyName).toBe('firstName'); - expect(viewModel.controller.errors.length).toBe(3); - - viewModel.controller.removeError(error1); expect(viewModel.controller.errors.length).toBe(2); - viewModel.controller.removeError(error2); + viewModel.controller.removeError(error1); expect(viewModel.controller.errors.length).toBe(1); - viewModel.controller.removeError(error3); + viewModel.controller.removeError(error2); expect(viewModel.controller.errors.length).toBe(0); }) // subscribe to error events diff --git a/test/property-accessor-parser.ts b/test/property-accessor-parser.ts deleted file mode 100644 index ed0d2723..00000000 --- a/test/property-accessor-parser.ts +++ /dev/null @@ -1,52 +0,0 @@ -import { Container } from 'aurelia-dependency-injection'; -import { BindingLanguage } from 'aurelia-templating'; -import { TemplatingBindingLanguage } from 'aurelia-templating-binding'; -import { - PropertyAccessorParser, - getAccessorExpression as parse -} from '../src/aurelia-validation'; - -describe('PropertyAccessorParser', () => { - let parser: PropertyAccessorParser; - - beforeAll(() => { - const container = new Container(); - container.registerInstance(BindingLanguage, container.get(TemplatingBindingLanguage)); - parser = container.get(PropertyAccessorParser); - }); - - it('parses properties', () => { - expect(parser.parse('firstName')).toEqual('firstName'); - expect(parser.parse('3_letter_id')).toEqual('3_letter_id'); - expect(parser.parse('. @$# ???')).toEqual('. @$# ???'); - expect(parser.parse((x: any) => x.firstName)).toEqual('firstName'); - }); - - it('parses function bodies', () => { - expect(parse('function(a){return a.b}')).toEqual('b'); - expect(parse('function(a){return a.b;}')).toEqual('b'); - expect(parse('function (a){return a.b;}')).toEqual('b'); - expect(parse('function(a) {return a.b;}')).toEqual('b'); - expect(parse('function (a) { return a.b; }')).toEqual('b'); - expect(parse('function (a1) { return a1.bcde; }')).toEqual('bcde'); - expect(parse('function ($a1) { return $a1.bcde; }')).toEqual('bcde'); - expect(parse('function (_) { return _.bc_de; }')).toEqual('bc_de'); - expect(parse('function (a) {"use strict"; return a.b; }')).toEqual('b'); - expect(parse('function (a) { "use strict"; return a.b; }')).toEqual('b'); - expect(parse('a=>a.b')).toEqual('b'); - expect(parse('a =>a.b')).toEqual('b'); - expect(parse('a=> a.b')).toEqual('b'); - expect(parse('a => a.b')).toEqual('b'); - expect(parse('a => a.bcde')).toEqual('bcde'); - expect(parse('_ => _.b')).toEqual('b'); - expect(parse('$ => $.b')).toEqual('b'); - expect(parse('(x) => x.name')).toEqual('name'); - - // tslint:disable-next-line:max-line-length - expect(parse('function(a){__gen$field.f[\'10\']++;__aGen$field.g[\'10\']++;return a.b;}')) - .toEqual('b'); - // tslint:disable-next-line:max-line-length - expect(parse('function(a){"use strict";_gen$field.f[\'10\']++;_aGen$field.g[\'10\']++;return a.b;}')) - .toEqual('b'); - }); -}); diff --git a/test/resources/nullable-object-form.ts b/test/resources/nullable-object-form.ts index 45fcd0b6..09ec6f98 100644 --- a/test/resources/nullable-object-form.ts +++ b/test/resources/nullable-object-form.ts @@ -27,7 +27,7 @@ export class NullableObjectForm { console.log(value); } - public rules = ValidationRules.ensure('prop').required().rules as any; + public rules = ValidationRules.ensure('prop').required().rules as any; constructor(public controller: ValidationController) { } } diff --git a/test/resources/registration-form.ts b/test/resources/registration-form.ts index 8724724a..121f940f 100644 --- a/test/resources/registration-form.ts +++ b/test/resources/registration-form.ts @@ -51,11 +51,12 @@ ValidationRules.customRule( ); ValidationRules - .ensure((f: RegistrationForm) => f.firstName).required() - .ensure(f => f.lastName).required() - .ensure('email').required().email() - .ensure(f => f.number1).satisfies(value => value > 0) - .ensure(f => f.number2).satisfies(value => value > 0).withMessage('${displayName} gots to be greater than zero.') - .ensure(f => f.password).required() - .ensure(f => f.confirmPassword).required().satisfiesRule('matchesProperty', 'password') + .ensure('firstName').required() + .ensure('lastName').required() + .ensure('email').required().email() + .ensure('number1').satisfies(value => value > 0) + .ensure('number2').satisfies(value => value > 0) + .withMessage('${displayName} gots to be greater than zero.') + .ensure('password').required() + .ensure('confirmPassword').required().satisfiesRule('matchesProperty', 'password') .on(RegistrationForm); diff --git a/test/resources/trigger-form.ts b/test/resources/trigger-form.ts index 4df83bfd..8c34e01b 100644 --- a/test/resources/trigger-form.ts +++ b/test/resources/trigger-form.ts @@ -34,9 +34,9 @@ export class TriggerForm { } ValidationRules - .ensure((f: TriggerForm) => f.standardProp).required() - .ensure(f => f.blurProp).required() - .ensure(f => f.changeProp).required() - .ensure(f => f.changeOrBlurProp).required() - .ensure(f => f.manualProp).required() + .ensure('standardProp').required() + .ensure('blurProp').required() + .ensure('changeProp').required() + .ensure('changeOrBlurProp').required() + .ensure('manualProp').required() .on(TriggerForm); diff --git a/test/resources/validation-errors-form-one.ts b/test/resources/validation-errors-form-one.ts index fd8b950f..be2739e9 100644 --- a/test/resources/validation-errors-form-one.ts +++ b/test/resources/validation-errors-form-one.ts @@ -36,5 +36,5 @@ export class ValidationErrorsFormOne { } ValidationRules - .ensure((f: ValidationErrorsFormOne) => f.standardProp).required() + .ensure('standardProp').required() .on(ValidationErrorsFormOne); diff --git a/test/server.ts b/test/server.ts index 2589be4d..c71c0977 100644 --- a/test/server.ts +++ b/test/server.ts @@ -15,8 +15,8 @@ configureBindingLanguage({ container }); configureValidation({ container }); const rules = ValidationRules - .ensure('firstName').required() - .ensure('lastName').required() + .ensure('firstName').required() + .ensure('lastName').required() .rules; const validator: Validator = container.get(Validator); diff --git a/test/validator.ts b/test/validator.ts index 5a0357a6..191269f2 100644 --- a/test/validator.ts +++ b/test/validator.ts @@ -6,7 +6,6 @@ import { ValidationRules, ValidationMessageParser, ValidateResult, - PropertyAccessorParser } from '../src/aurelia-validation'; describe('Validator', () => { @@ -16,14 +15,13 @@ describe('Validator', () => { const container = new Container(); container.registerInstance(BindingLanguage, container.get(TemplatingBindingLanguage)); const messageParser = container.get(ValidationMessageParser); - const propertyParser = container.get(PropertyAccessorParser); - ValidationRules.initialize(messageParser, propertyParser); + ValidationRules.initialize(messageParser); validator = container.get(StandardValidator); }); it('validates email', (done: () => void) => { let obj = { prop: 'foo@bar.com' as any }; - let rules = ValidationRules.ensure('prop').email().rules; + let rules = ValidationRules.ensure('prop').email().rules; validator.validateProperty(obj, 'prop', rules) .then(results => { const expected = [new ValidateResult(rules[0][0], obj, 'prop', true, null)]; @@ -32,7 +30,7 @@ describe('Validator', () => { }) .then(() => { obj = { prop: 'foo' }; - rules = ValidationRules.ensure('prop').email().rules; + rules = ValidationRules.ensure('prop').email().rules; return validator.validateProperty(obj, 'prop', rules); }) .then(results => { @@ -42,7 +40,7 @@ describe('Validator', () => { }) .then(() => { obj = { prop: null }; - rules = ValidationRules.ensure('prop').email().rules; + rules = ValidationRules.ensure('prop').email().rules; return validator.validateProperty(obj, 'prop', rules); }) .then(results => { @@ -55,7 +53,7 @@ describe('Validator', () => { it('validates equals', (done: () => void) => { let obj = { prop: 'test' as any }; - let rules = ValidationRules.ensure('prop').equals('test').rules; + let rules = ValidationRules.ensure('prop').equals('test').rules; validator.validateProperty(obj, 'prop', rules) .then(results => { const expected = [new ValidateResult(rules[0][0], obj, 'prop', true, null)]; @@ -64,7 +62,7 @@ describe('Validator', () => { }) .then(() => { obj = { prop: 'foo' }; - rules = ValidationRules.ensure('prop').equals('test').rules; + rules = ValidationRules.ensure('prop').equals('test').rules; return validator.validateProperty(obj, 'prop', rules); }) .then(results => { @@ -74,7 +72,7 @@ describe('Validator', () => { }) .then(() => { obj = { prop: null }; - rules = ValidationRules.ensure('prop').equals('test').rules; + rules = ValidationRules.ensure('prop').equals('test').rules; return validator.validateProperty(obj, 'prop', rules); }) .then(results => { @@ -90,7 +88,7 @@ describe('Validator', () => { const spy1 = jasmine.createSpy().and.returnValue(true); const spy2 = jasmine.createSpy().and.returnValue(true); const rules = ValidationRules - .ensure('prop').email().then().satisfies(spy1) + .ensure('prop').email().then().satisfies(spy1) .ensure('prop2').satisfies(spy2) .rules; validator.validateProperty(obj, 'prop', rules) From 3f589425ec3adbed95c5e210d493eba6ce1f8766 Mon Sep 17 00:00:00 2001 From: TimVevida Date: Fri, 20 Apr 2018 12:14:17 +0200 Subject: [PATCH 2/2] feat(PropertyAccessorParser): Remove PropertyAccessorParser in favor of the keyof operator. Calls to .ensure() allowed both a string and a function as argument. The function was never actually executed, but parsed to get the string property name. The same effect can be achieved by using the keyof operator. In this case, only strings can be provided that are a property of the given class. Initial version, requiring the programmer to explicitly state the object class each time .ensure() is executed. This is because .ensure() is a static function, so the type cannot be determined yet. Fixes #485. --- src/aurelia-validation.ts | 5 +- src/implementation/validation-messages.ts | 2 +- src/implementation/validation-rules.ts | 47 +++++++----------- src/property-accessor-parser.ts | 39 --------------- src/validation-controller-factory.ts | 4 +- src/validation-controller.ts | 15 ++---- test/basic.ts | 11 +---- test/property-accessor-parser.ts | 52 -------------------- test/resources/nullable-object-form.ts | 2 +- test/resources/registration-form.ts | 12 ++--- test/resources/trigger-form.ts | 10 ++-- test/resources/validation-errors-form-one.ts | 2 +- test/server.ts | 2 +- test/validator.ts | 18 +++---- 14 files changed, 50 insertions(+), 171 deletions(-) delete mode 100644 src/property-accessor-parser.ts delete mode 100644 test/property-accessor-parser.ts diff --git a/src/aurelia-validation.ts b/src/aurelia-validation.ts index f5d8d47e..8c137770 100644 --- a/src/aurelia-validation.ts +++ b/src/aurelia-validation.ts @@ -3,7 +3,6 @@ export * from './controller-validate-result'; export * from './get-target-dom-element'; export * from './property-info'; -export * from './property-accessor-parser'; export * from './validate-binding-behavior'; export * from './validate-event'; export * from './validate-instruction'; @@ -30,7 +29,6 @@ import { Container } from 'aurelia-dependency-injection'; import { Validator } from './validator'; import { StandardValidator } from './implementation/standard-validator'; import { ValidationMessageParser } from './implementation/validation-message-parser'; -import { PropertyAccessorParser } from './property-accessor-parser'; import { ValidationRules } from './implementation/validation-rules'; /** @@ -65,8 +63,7 @@ export function configure( // the fluent rule definition API needs the parser to translate messages // to interpolation expressions. const messageParser = frameworkConfig.container.get(ValidationMessageParser); - const propertyParser = frameworkConfig.container.get(PropertyAccessorParser); - ValidationRules.initialize(messageParser, propertyParser); + ValidationRules.initialize(messageParser); // configure... const config = new AureliaValidationConfiguration(); diff --git a/src/implementation/validation-messages.ts b/src/implementation/validation-messages.ts index 5ab8f831..19e36c59 100644 --- a/src/implementation/validation-messages.ts +++ b/src/implementation/validation-messages.ts @@ -29,7 +29,7 @@ export const validationMessages: ValidationMessages = { export class ValidationMessageProvider { public static inject = [ValidationMessageParser]; - constructor(parser: ValidationMessageParser) { } + constructor(private parser: ValidationMessageParser) { } /** * Returns a message binding expression that corresponds to the key. diff --git a/src/implementation/validation-rules.ts b/src/implementation/validation-rules.ts index 2457f6e3..e6856028 100644 --- a/src/implementation/validation-rules.ts +++ b/src/implementation/validation-rules.ts @@ -2,7 +2,6 @@ import { Rule, RuleProperty, ValidationDisplayNameAccessor } from './rule'; import { ValidationMessageParser } from './validation-message-parser'; import { Rules } from './rules'; import { validationMessages } from './validation-messages'; -import { PropertyAccessorParser, PropertyAccessor } from '../property-accessor-parser'; import { isString } from '../util'; /** @@ -17,7 +16,7 @@ export class FluentRuleCustomizer { config: object = {}, private fluentEnsure: FluentEnsure, private fluentRules: FluentRules, - private parsers: Parsers + private messageParser: ValidationMessageParser ) { this.rule = { property, @@ -55,7 +54,7 @@ export class FluentRuleCustomizer { */ public withMessage(message: string) { this.rule.messageKey = 'custom'; - this.rule.message = this.parsers.message.parse(message); + this.rule.message = this.messageParser.parse(message); return this; } @@ -84,8 +83,8 @@ export class FluentRuleCustomizer { * Target a property with validation rules. * @param property The property to target. Can be the property name or a property accessor function. */ - public ensure(subject: string | ((model: TObject) => TValue2)) { - return this.fluentEnsure.ensure(subject); + public ensure(subject: keyof TObject) { + return this.fluentEnsure.ensure(subject); } /** @@ -217,7 +216,7 @@ export class FluentRules { constructor( private fluentEnsure: FluentEnsure, - private parsers: Parsers, + private messageParser: ValidationMessageParser, private property: RuleProperty ) { } @@ -237,7 +236,7 @@ export class FluentRules { */ public satisfies(condition: (value: TValue, object?: TObject) => boolean | Promise, config?: object) { return new FluentRuleCustomizer( - this.property, condition, config, this.fluentEnsure, this, this.parsers); + this.property, condition, config, this.fluentEnsure, this, this.messageParser); } /** @@ -357,21 +356,21 @@ export class FluentEnsure { */ public rules: Rule[][] = []; - constructor(private parsers: Parsers) { } + constructor(private messageParser: ValidationMessageParser) { } /** * Target a property with validation rules. * @param property The property to target. Can be the property name or a property accessor * function. */ - public ensure(property: string | PropertyAccessor) { + public ensure(property: keyof TObject) { this.assertInitialized(); - const name = this.parsers.property.parse(property); + const name = property as string; const fluentRules = new FluentRules( this, - this.parsers, + this.messageParser, { name, displayName: null }); - return this.mergeRules(fluentRules, name); + return this.mergeRules(fluentRules, property); } /** @@ -380,7 +379,7 @@ export class FluentEnsure { public ensureObject() { this.assertInitialized(); const fluentRules = new FluentRules( - this, this.parsers, { name: null, displayName: null }); + this, this.messageParser, { name: null, displayName: null }); return this.mergeRules(fluentRules, null); } @@ -405,7 +404,7 @@ export class FluentEnsure { } private assertInitialized() { - if (this.parsers) { + if (this.messageParser) { return; } throw new Error(`Did you forget to add ".plugin('aurelia-validation')" to your main.js?`); @@ -428,28 +427,25 @@ export class FluentEnsure { * Fluent rule definition API. */ export class ValidationRules { - private static parsers: Parsers; + private static messageParser: ValidationMessageParser; - public static initialize(messageParser: ValidationMessageParser, propertyParser: PropertyAccessorParser) { - this.parsers = { - message: messageParser, - property: propertyParser - }; + public static initialize(messageParser: ValidationMessageParser) { + this.messageParser = messageParser; } /** * Target a property with validation rules. * @param property The property to target. Can be the property name or a property accessor function. */ - public static ensure(property: string | PropertyAccessor) { - return new FluentEnsure(ValidationRules.parsers).ensure(property); + public static ensure(property: keyof TObject) { + return new FluentEnsure(ValidationRules.messageParser).ensure(property); } /** * Targets an object with validation rules. */ public static ensureObject() { - return new FluentEnsure(ValidationRules.parsers).ensureObject(); + return new FluentEnsure(ValidationRules.messageParser).ensureObject(); } /** @@ -495,8 +491,3 @@ export class ValidationRules { Rules.unset(target); } } - -export interface Parsers { - message: ValidationMessageParser; - property: PropertyAccessorParser; -} diff --git a/src/property-accessor-parser.ts b/src/property-accessor-parser.ts deleted file mode 100644 index 9bfc93ac..00000000 --- a/src/property-accessor-parser.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { - Parser, - AccessMember, - AccessScope -} from 'aurelia-binding'; -import { isString } from './util'; - -export type PropertyAccessor = (object: TObject) => TValue; - -export class PropertyAccessorParser { - public static inject = [Parser]; - - constructor(private parser: Parser) { } - - public parse(property: string | PropertyAccessor): string { - if (isString(property)) { - return property as string; - } - const accessorText = getAccessorExpression(property.toString()); - const accessor = this.parser.parse(accessorText); - if (accessor instanceof AccessScope - || accessor instanceof AccessMember && accessor.object instanceof AccessScope) { - return accessor.name; - } - throw new Error(`Invalid property expression: "${accessor}"`); - } -} - -export function getAccessorExpression(fn: string): string { - /* tslint:disable:max-line-length */ - const classic = /^function\s*\([$_\w\d]+\)\s*\{(?:\s*"use strict";)?\s*(?:[$_\w\d.['"\]+;]+)?\s*return\s+[$_\w\d]+\.([$_\w\d]+)\s*;?\s*\}$/; - /* tslint:enable:max-line-length */ - const arrow = /^\(?[$_\w\d]+\)?\s*=>\s*[$_\w\d]+\.([$_\w\d]+)$/; - const match = classic.exec(fn) || arrow.exec(fn); - if (match === null) { - throw new Error(`Unable to parse accessor function:\n${fn}`); - } - return match[1]; -} diff --git a/src/validation-controller-factory.ts b/src/validation-controller-factory.ts index 15a25ba9..de763b80 100644 --- a/src/validation-controller-factory.ts +++ b/src/validation-controller-factory.ts @@ -1,7 +1,6 @@ import { Container } from 'aurelia-dependency-injection'; import { ValidationController } from './validation-controller'; import { Validator } from './validator'; -import { PropertyAccessorParser } from './property-accessor-parser'; /** * Creates ValidationController instances. @@ -20,8 +19,7 @@ export class ValidationControllerFactory { if (!validator) { validator = this.container.get(Validator) as Validator; } - const propertyParser = this.container.get(PropertyAccessorParser) as PropertyAccessorParser; - return new ValidationController(validator, propertyParser); + return new ValidationController(validator); } /** diff --git a/src/validation-controller.ts b/src/validation-controller.ts index 9ef46acb..adab3942 100644 --- a/src/validation-controller.ts +++ b/src/validation-controller.ts @@ -6,7 +6,6 @@ import { ValidationRenderer, RenderInstruction } from './validation-renderer'; import { ValidateResult } from './validate-result'; import { ValidateInstruction } from './validate-instruction'; import { ControllerValidateResult } from './controller-validate-result'; -import { PropertyAccessorParser, PropertyAccessor } from './property-accessor-parser'; import { ValidateEvent } from './validate-event'; /** @@ -15,7 +14,7 @@ import { ValidateEvent } from './validate-event'; * Exposes the current list of validation results for binding purposes. */ export class ValidationController { - public static inject = [Validator, PropertyAccessorParser]; + public static inject = [Validator]; // Registered bindings (via the validate binding behavior) private bindings = new Map(); @@ -54,7 +53,7 @@ export class ValidationController { private eventCallbacks: ((event: ValidateEvent) => void)[] = []; - constructor(private validator: Validator, private propertyParser: PropertyAccessorParser) { } + constructor(private validator: Validator) { } /** * Subscribe to controller validate and reset events. These events occur when the @@ -101,15 +100,9 @@ export class ValidationController { public addError( message: string, object: TObject, - propertyName: string | PropertyAccessor | null = null + propertyName: string | null = null ): ValidateResult { - let resolvedPropertyName: string | null; - if (propertyName === null) { - resolvedPropertyName = propertyName; - } else { - resolvedPropertyName = this.propertyParser.parse(propertyName); - } - const result = new ValidateResult({ __manuallyAdded__: true }, object, resolvedPropertyName, false, message); + const result = new ValidateResult({ __manuallyAdded__: true }, object, propertyName, false, message); this.processResultDelta('validate', [], [result]); return result; } diff --git a/test/basic.ts b/test/basic.ts index b4e7a2a4..d62882f5 100644 --- a/test/basic.ts +++ b/test/basic.ts @@ -190,20 +190,13 @@ describe('end to end', () => { expect(error2.message).toBe('string property error'); expect(error2.object).toBe(viewModel); expect(error2.propertyName).toBe('lastName'); - const error3 = viewModel.controller.addError('expression property error', viewModel, vm => vm.firstName); - expect(error3.message).toBe('expression property error'); - expect(error3.object).toBe(viewModel); - expect(error3.propertyName).toBe('firstName'); - expect(viewModel.controller.errors.length).toBe(3); - - viewModel.controller.removeError(error1); expect(viewModel.controller.errors.length).toBe(2); - viewModel.controller.removeError(error2); + viewModel.controller.removeError(error1); expect(viewModel.controller.errors.length).toBe(1); - viewModel.controller.removeError(error3); + viewModel.controller.removeError(error2); expect(viewModel.controller.errors.length).toBe(0); }) // subscribe to error events diff --git a/test/property-accessor-parser.ts b/test/property-accessor-parser.ts deleted file mode 100644 index ed0d2723..00000000 --- a/test/property-accessor-parser.ts +++ /dev/null @@ -1,52 +0,0 @@ -import { Container } from 'aurelia-dependency-injection'; -import { BindingLanguage } from 'aurelia-templating'; -import { TemplatingBindingLanguage } from 'aurelia-templating-binding'; -import { - PropertyAccessorParser, - getAccessorExpression as parse -} from '../src/aurelia-validation'; - -describe('PropertyAccessorParser', () => { - let parser: PropertyAccessorParser; - - beforeAll(() => { - const container = new Container(); - container.registerInstance(BindingLanguage, container.get(TemplatingBindingLanguage)); - parser = container.get(PropertyAccessorParser); - }); - - it('parses properties', () => { - expect(parser.parse('firstName')).toEqual('firstName'); - expect(parser.parse('3_letter_id')).toEqual('3_letter_id'); - expect(parser.parse('. @$# ???')).toEqual('. @$# ???'); - expect(parser.parse((x: any) => x.firstName)).toEqual('firstName'); - }); - - it('parses function bodies', () => { - expect(parse('function(a){return a.b}')).toEqual('b'); - expect(parse('function(a){return a.b;}')).toEqual('b'); - expect(parse('function (a){return a.b;}')).toEqual('b'); - expect(parse('function(a) {return a.b;}')).toEqual('b'); - expect(parse('function (a) { return a.b; }')).toEqual('b'); - expect(parse('function (a1) { return a1.bcde; }')).toEqual('bcde'); - expect(parse('function ($a1) { return $a1.bcde; }')).toEqual('bcde'); - expect(parse('function (_) { return _.bc_de; }')).toEqual('bc_de'); - expect(parse('function (a) {"use strict"; return a.b; }')).toEqual('b'); - expect(parse('function (a) { "use strict"; return a.b; }')).toEqual('b'); - expect(parse('a=>a.b')).toEqual('b'); - expect(parse('a =>a.b')).toEqual('b'); - expect(parse('a=> a.b')).toEqual('b'); - expect(parse('a => a.b')).toEqual('b'); - expect(parse('a => a.bcde')).toEqual('bcde'); - expect(parse('_ => _.b')).toEqual('b'); - expect(parse('$ => $.b')).toEqual('b'); - expect(parse('(x) => x.name')).toEqual('name'); - - // tslint:disable-next-line:max-line-length - expect(parse('function(a){__gen$field.f[\'10\']++;__aGen$field.g[\'10\']++;return a.b;}')) - .toEqual('b'); - // tslint:disable-next-line:max-line-length - expect(parse('function(a){"use strict";_gen$field.f[\'10\']++;_aGen$field.g[\'10\']++;return a.b;}')) - .toEqual('b'); - }); -}); diff --git a/test/resources/nullable-object-form.ts b/test/resources/nullable-object-form.ts index 45fcd0b6..09ec6f98 100644 --- a/test/resources/nullable-object-form.ts +++ b/test/resources/nullable-object-form.ts @@ -27,7 +27,7 @@ export class NullableObjectForm { console.log(value); } - public rules = ValidationRules.ensure('prop').required().rules as any; + public rules = ValidationRules.ensure('prop').required().rules as any; constructor(public controller: ValidationController) { } } diff --git a/test/resources/registration-form.ts b/test/resources/registration-form.ts index 8724724a..a686e2b7 100644 --- a/test/resources/registration-form.ts +++ b/test/resources/registration-form.ts @@ -51,11 +51,11 @@ ValidationRules.customRule( ); ValidationRules - .ensure((f: RegistrationForm) => f.firstName).required() - .ensure(f => f.lastName).required() + .ensure('firstName').required() + .ensure('lastName').required() .ensure('email').required().email() - .ensure(f => f.number1).satisfies(value => value > 0) - .ensure(f => f.number2).satisfies(value => value > 0).withMessage('${displayName} gots to be greater than zero.') - .ensure(f => f.password).required() - .ensure(f => f.confirmPassword).required().satisfiesRule('matchesProperty', 'password') + .ensure('number1').satisfies(value => value > 0) + .ensure('number2').satisfies(value => value > 0).withMessage('${displayName} gots to be greater than zero.') + .ensure('password').required() + .ensure('confirmPassword').required().satisfiesRule('matchesProperty', 'password') .on(RegistrationForm); diff --git a/test/resources/trigger-form.ts b/test/resources/trigger-form.ts index 4df83bfd..5bf286dc 100644 --- a/test/resources/trigger-form.ts +++ b/test/resources/trigger-form.ts @@ -34,9 +34,9 @@ export class TriggerForm { } ValidationRules - .ensure((f: TriggerForm) => f.standardProp).required() - .ensure(f => f.blurProp).required() - .ensure(f => f.changeProp).required() - .ensure(f => f.changeOrBlurProp).required() - .ensure(f => f.manualProp).required() + .ensure('standardProp').required() + .ensure('blurProp').required() + .ensure('changeProp').required() + .ensure('changeOrBlurProp').required() + .ensure('manualProp').required() .on(TriggerForm); diff --git a/test/resources/validation-errors-form-one.ts b/test/resources/validation-errors-form-one.ts index fd8b950f..be2739e9 100644 --- a/test/resources/validation-errors-form-one.ts +++ b/test/resources/validation-errors-form-one.ts @@ -36,5 +36,5 @@ export class ValidationErrorsFormOne { } ValidationRules - .ensure((f: ValidationErrorsFormOne) => f.standardProp).required() + .ensure('standardProp').required() .on(ValidationErrorsFormOne); diff --git a/test/server.ts b/test/server.ts index 2589be4d..a8898889 100644 --- a/test/server.ts +++ b/test/server.ts @@ -15,7 +15,7 @@ configureBindingLanguage({ container }); configureValidation({ container }); const rules = ValidationRules - .ensure('firstName').required() + .ensure('firstName').required() .ensure('lastName').required() .rules; diff --git a/test/validator.ts b/test/validator.ts index 5a0357a6..191269f2 100644 --- a/test/validator.ts +++ b/test/validator.ts @@ -6,7 +6,6 @@ import { ValidationRules, ValidationMessageParser, ValidateResult, - PropertyAccessorParser } from '../src/aurelia-validation'; describe('Validator', () => { @@ -16,14 +15,13 @@ describe('Validator', () => { const container = new Container(); container.registerInstance(BindingLanguage, container.get(TemplatingBindingLanguage)); const messageParser = container.get(ValidationMessageParser); - const propertyParser = container.get(PropertyAccessorParser); - ValidationRules.initialize(messageParser, propertyParser); + ValidationRules.initialize(messageParser); validator = container.get(StandardValidator); }); it('validates email', (done: () => void) => { let obj = { prop: 'foo@bar.com' as any }; - let rules = ValidationRules.ensure('prop').email().rules; + let rules = ValidationRules.ensure('prop').email().rules; validator.validateProperty(obj, 'prop', rules) .then(results => { const expected = [new ValidateResult(rules[0][0], obj, 'prop', true, null)]; @@ -32,7 +30,7 @@ describe('Validator', () => { }) .then(() => { obj = { prop: 'foo' }; - rules = ValidationRules.ensure('prop').email().rules; + rules = ValidationRules.ensure('prop').email().rules; return validator.validateProperty(obj, 'prop', rules); }) .then(results => { @@ -42,7 +40,7 @@ describe('Validator', () => { }) .then(() => { obj = { prop: null }; - rules = ValidationRules.ensure('prop').email().rules; + rules = ValidationRules.ensure('prop').email().rules; return validator.validateProperty(obj, 'prop', rules); }) .then(results => { @@ -55,7 +53,7 @@ describe('Validator', () => { it('validates equals', (done: () => void) => { let obj = { prop: 'test' as any }; - let rules = ValidationRules.ensure('prop').equals('test').rules; + let rules = ValidationRules.ensure('prop').equals('test').rules; validator.validateProperty(obj, 'prop', rules) .then(results => { const expected = [new ValidateResult(rules[0][0], obj, 'prop', true, null)]; @@ -64,7 +62,7 @@ describe('Validator', () => { }) .then(() => { obj = { prop: 'foo' }; - rules = ValidationRules.ensure('prop').equals('test').rules; + rules = ValidationRules.ensure('prop').equals('test').rules; return validator.validateProperty(obj, 'prop', rules); }) .then(results => { @@ -74,7 +72,7 @@ describe('Validator', () => { }) .then(() => { obj = { prop: null }; - rules = ValidationRules.ensure('prop').equals('test').rules; + rules = ValidationRules.ensure('prop').equals('test').rules; return validator.validateProperty(obj, 'prop', rules); }) .then(results => { @@ -90,7 +88,7 @@ describe('Validator', () => { const spy1 = jasmine.createSpy().and.returnValue(true); const spy2 = jasmine.createSpy().and.returnValue(true); const rules = ValidationRules - .ensure('prop').email().then().satisfies(spy1) + .ensure('prop').email().then().satisfies(spy1) .ensure('prop2').satisfies(spy2) .rules; validator.validateProperty(obj, 'prop', rules)