Skip to content

Commit

Permalink
feat(PropertyAccessorParser): Remove PropertyAccessorParser in favor of
Browse files Browse the repository at this point in the history
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 aurelia#485.
  • Loading branch information
TimVevida committed Apr 20, 2018
1 parent b7636b5 commit 87ae8bc
Show file tree
Hide file tree
Showing 14 changed files with 53 additions and 173 deletions.
5 changes: 1 addition & 4 deletions src/aurelia-validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';

/**
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/implementation/validation-messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
47 changes: 19 additions & 28 deletions src/implementation/validation-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand All @@ -17,7 +16,7 @@ export class FluentRuleCustomizer<TObject, TValue> {
config: object = {},
private fluentEnsure: FluentEnsure<TObject>,
private fluentRules: FluentRules<TObject, TValue>,
private parsers: Parsers
private messageParser: ValidationMessageParser
) {
this.rule = {
property,
Expand Down Expand Up @@ -55,7 +54,7 @@ export class FluentRuleCustomizer<TObject, TValue> {
*/
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;
}

Expand Down Expand Up @@ -84,8 +83,8 @@ export class FluentRuleCustomizer<TObject, TValue> {
* Target a property with validation rules.
* @param property The property to target. Can be the property name or a property accessor function.
*/
public ensure<TValue2>(subject: string | ((model: TObject) => TValue2)) {
return this.fluentEnsure.ensure<TValue2>(subject);
public ensure<TValue>(subject: keyof TObject) {
return this.fluentEnsure.ensure<TValue>(subject);
}

/**
Expand Down Expand Up @@ -217,7 +216,7 @@ export class FluentRules<TObject, TValue> {

constructor(
private fluentEnsure: FluentEnsure<TObject>,
private parsers: Parsers,
private messageParser: ValidationMessageParser,
private property: RuleProperty
) { }

Expand All @@ -237,7 +236,7 @@ export class FluentRules<TObject, TValue> {
*/
public satisfies(condition: (value: TValue, object?: TObject) => boolean | Promise<boolean>, config?: object) {
return new FluentRuleCustomizer<TObject, TValue>(
this.property, condition, config, this.fluentEnsure, this, this.parsers);
this.property, condition, config, this.fluentEnsure, this, this.messageParser);
}

/**
Expand Down Expand Up @@ -357,21 +356,21 @@ export class FluentEnsure<TObject> {
*/
public rules: Rule<TObject, any>[][] = [];

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<TValue>(property: string | PropertyAccessor<TObject, TValue>) {
public ensure<TValue>(property: keyof TObject) {
this.assertInitialized();
const name = this.parsers.property.parse(property);
const name = property as string;
const fluentRules = new FluentRules<TObject, TValue>(
this,
this.parsers,
this.messageParser,
{ name, displayName: null });
return this.mergeRules(fluentRules, name);
return this.mergeRules(fluentRules, property);
}

/**
Expand All @@ -380,7 +379,7 @@ export class FluentEnsure<TObject> {
public ensureObject() {
this.assertInitialized();
const fluentRules = new FluentRules<TObject, TObject>(
this, this.parsers, { name: null, displayName: null });
this, this.messageParser, { name: null, displayName: null });
return this.mergeRules(fluentRules, null);
}

Expand All @@ -405,7 +404,7 @@ export class FluentEnsure<TObject> {
}

private assertInitialized() {
if (this.parsers) {
if (this.messageParser) {
return;
}
throw new Error(`Did you forget to add ".plugin('aurelia-validation')" to your main.js?`);
Expand All @@ -428,28 +427,25 @@ export class FluentEnsure<TObject> {
* 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<TObject, TValue>(property: string | PropertyAccessor<TObject, TValue>) {
return new FluentEnsure<TObject>(ValidationRules.parsers).ensure(property);
public static ensure<TObject>(property: keyof TObject) {
return new FluentEnsure<TObject>(ValidationRules.messageParser).ensure(property);
}

/**
* Targets an object with validation rules.
*/
public static ensureObject<TObject>() {
return new FluentEnsure<TObject>(ValidationRules.parsers).ensureObject();
return new FluentEnsure<TObject>(ValidationRules.messageParser).ensureObject();
}

/**
Expand Down Expand Up @@ -495,8 +491,3 @@ export class ValidationRules {
Rules.unset(target);
}
}

export interface Parsers {
message: ValidationMessageParser;
property: PropertyAccessorParser;
}
39 changes: 0 additions & 39 deletions src/property-accessor-parser.ts

This file was deleted.

4 changes: 1 addition & 3 deletions src/validation-controller-factory.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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);
}

/**
Expand Down
15 changes: 4 additions & 11 deletions src/validation-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand All @@ -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<Binding, BindingInfo>();
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -101,15 +100,9 @@ export class ValidationController {
public addError<TObject>(
message: string,
object: TObject,
propertyName: string | PropertyAccessor<TObject, string> | 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;
}
Expand Down
11 changes: 2 additions & 9 deletions test/basic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
52 changes: 0 additions & 52 deletions test/property-accessor-parser.ts

This file was deleted.

2 changes: 1 addition & 1 deletion test/resources/nullable-object-form.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export class NullableObjectForm {
console.log(value);
}

public rules = ValidationRules.ensure('prop').required().rules as any;
public rules = ValidationRules.ensure<any>('prop').required().rules as any;

constructor(public controller: ValidationController) { }
}
15 changes: 8 additions & 7 deletions test/resources/registration-form.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<RegistrationForm>('firstName').required()
.ensure<RegistrationForm>('lastName').required()
.ensure<RegistrationForm>('email').required().email()
.ensure<RegistrationForm>('number1').satisfies(value => value > 0)
.ensure<RegistrationForm>('number2').satisfies(value => value > 0)
.withMessage('${displayName} gots to be greater than zero.')
.ensure<RegistrationForm>('password').required()
.ensure<RegistrationForm>('confirmPassword').required().satisfiesRule('matchesProperty', 'password')
.on(RegistrationForm);
Loading

0 comments on commit 87ae8bc

Please sign in to comment.