Skip to content

Commit

Permalink
Fixed double validation from triggers
Browse files Browse the repository at this point in the history
  • Loading branch information
Andrei Fangli committed Aug 30, 2024
1 parent d8bb6a6 commit 4732e38
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 67 deletions.
4 changes: 2 additions & 2 deletions src/validation/Validatable.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ export class Validatable<TValidationError = string> extends ViewModel implements
}

public set error(value: TValidationError | false | null | undefined) {
const normalizedError = value === false || value === null || value === undefined ? null : value;
const normalizedError = (value === false || value === null || value === undefined) ? null : value;

if (this._error !== normalizedError) {
this._error = normalizedError;
this.notifyPropertiesChanged('error');
this.notifyPropertiesChanged('error', 'isValid', 'isInvalid');
}
}
}
4 changes: 2 additions & 2 deletions src/validation/objectValidator/IObjectValidator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ import type { INotifyPropertiesChanged } from '../../viewModels';
import type { IValidatable } from '../IValidatable';
import type { IValidator, ValidatorCallback } from '../IValidator';
import type { IReadOnlyObjectValidator } from './IReadOnlyObjectValidator';
import type { ValidationTrigger } from '../triggers';
import type { WellKnownValidationTrigger, ValidationTrigger } from '../triggers';


export interface IObjectValidator<TValidatable extends IValidatable<TValidationError> & INotifyPropertiesChanged, TValidationError = string> extends IReadOnlyObjectValidator<TValidatable, TValidationError> {
readonly validators: IObservableCollection<IValidator<TValidatable, TValidationError>>;
readonly triggers: IObservableSet<ValidationTrigger>;
readonly triggers: IObservableSet<WellKnownValidationTrigger | ValidationTrigger>;

add(validator: IValidator<TValidatable, TValidationError> | ValidatorCallback<TValidatable, TValidationError>): this;
add(validator: IValidator<TValidatable, TValidationError> | ValidatorCallback<TValidatable, TValidationError>, triggers: readonly ValidationTrigger[]): this;
Expand Down
4 changes: 2 additions & 2 deletions src/validation/objectValidator/IReadOnlyObjectValidator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ import type { IReadOnlyObservableCollection, IReadOnlyObservableSet } from '../.
import type { INotifyPropertiesChanged } from '../../viewModels';
import type { IReadOnlyValidatable } from '../IReadOnlyValidatable';
import type { IValidator } from '../IValidator';
import type { ValidationTrigger } from '../triggers';
import type { WellKnownValidationTrigger, ValidationTrigger } from '../triggers';


export interface IReadOnlyObjectValidator<TValidatable extends IReadOnlyValidatable<TValidationError> & INotifyPropertiesChanged, TValidationError = string> {
readonly target: TValidatable;

readonly validators: IReadOnlyObservableCollection<IValidator<TValidatable, TValidationError>>;
readonly triggers: IReadOnlyObservableSet<ValidationTrigger>;
readonly triggers: IReadOnlyObservableSet<WellKnownValidationTrigger | ValidationTrigger>;

validate(): TValidationError | null;
}
101 changes: 51 additions & 50 deletions src/validation/objectValidator/ObjectValidator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,24 @@ import type { IObjectValidator } from './IObjectValidator';
import type { WellKnownValidationTrigger, ValidationTrigger } from '../triggers';
import type { INotifyPropertiesChanged } from '../../viewModels';
import { type IObservableCollection, type IObservableSet, ObservableCollection, ObservableSet } from '../../collections';
import { resolveValidationTriggers } from '../triggers/resolveValidationTriggers';
import { ViewModelChangedValidationTrigger, resolveValidationTriggers } from '../triggers';

export interface IObjectValidatorConfig<TValidatable extends IValidatable<TValidationError> & INotifyPropertiesChanged, TValidationError = string> {
readonly target: TValidatable;

shouldTargetTriggerValidation?(target: TValidatable, changedProperties: readonly (keyof TValidatable)[]): boolean;
}

export class ObjectValidator<TValidatable extends IValidatable<TValidationError> & INotifyPropertiesChanged, TValidationError = string> implements IObjectValidator<TValidatable, TValidationError> {
private _isValidating: boolean;
private readonly _validationTriggeredEventHandler: IEventHandler<unknown, unknown> & {
_validate(): void;
};
private static defaultShouldTargetTriggerValidation<TValidationError = string>(target: IValidatable<TValidationError>, changedProperties: readonly (keyof IValidatable<TValidationError>)[]): boolean {
return changedProperties.some(changedProperty => {
return changedProperty !== 'error'
&& changedProperty !== 'isValid'
&& changedProperty !== 'isInvalid'
});
}

public constructor(target: TValidatable) {
public constructor({ target, shouldTargetTriggerValidation = ObjectValidator.defaultShouldTargetTriggerValidation<TValidationError> }: IObjectValidatorConfig<TValidatable, TValidationError>) {
this.target = target;

this.validators = new ObservableCollection<IValidator<TValidatable, TValidationError>>();
Expand All @@ -29,54 +38,55 @@ export class ObjectValidator<TValidatable extends IValidatable<TValidationError>
}
});

this._validationTriggeredEventHandler = {
_validate: this.validate.bind(this),

handle() {
this._validate();
const validationTriggeredEventHandler: IEventHandler<unknown, unknown> = {
handle: () => {
this.validate();
}
};

this.target.propertiesChanged.subscribe(this._validationTriggeredEventHandler);
const targetValidationTrigger = new ViewModelChangedValidationTrigger({
viewModel: target,
shouldTriggerValidation: shouldTargetTriggerValidation
});
targetValidationTrigger.validationTriggered.subscribe(validationTriggeredEventHandler);

const resolvedValidationTriggersBySource = new Map<WellKnownValidationTrigger | ValidationTrigger, readonly ValidationTrigger[]>();
this.triggers = new ObservableSet<ValidationTrigger>();
this.triggers.setChanged.subscribe({
_validationTriggeredEventHandler: this._validationTriggeredEventHandler,

handle(_, { addedItems: addedTriggers, removedItems: removedTriggers }) {
removedTriggers.forEach(removedTrigger => {
removedTrigger.validationTriggered.unsubscribe(this._validationTriggeredEventHandler);
resolvedValidationTriggersBySource
.get(removedTrigger)
.forEach(resolvedValidationTrigger => {
resolvedValidationTrigger.validationTriggered.unsubscribe(validationTriggeredEventHandler);
});
resolvedValidationTriggersBySource.delete(removedTrigger);
});

addedTriggers.forEach(addedTrigger => {
addedTrigger.validationTriggered.subscribe(this._validationTriggeredEventHandler);
const resolvedValidationTriggers = resolveValidationTriggers(addedTrigger);
resolvedValidationTriggers.forEach(resolvedValidationTrigger => {
resolvedValidationTrigger.validationTriggered.subscribe(validationTriggeredEventHandler);
});
resolvedValidationTriggersBySource.set(addedTrigger, resolvedValidationTriggers);
});
}
});

try {
this._isValidating = true;

this.target.error = null;
}
finally {
this._isValidating = false;
}
this.target.error = null;
}

public readonly target: TValidatable;

public readonly validators: IObservableCollection<IValidator<TValidatable, TValidationError>>;
public readonly triggers: IObservableSet<ValidationTrigger>;
public readonly triggers: IObservableSet<WellKnownValidationTrigger | ValidationTrigger>;

public add(validator: IValidator<TValidatable, TValidationError> | ValidatorCallback<TValidatable, TValidationError>): this;
public add(validator: IValidator<TValidatable, TValidationError> | ValidatorCallback<TValidatable, TValidationError>, triggers: readonly (WellKnownValidationTrigger | ValidationTrigger)[]): this;

public add(validator: IValidator<TValidatable, TValidationError> | ValidatorCallback<TValidatable, TValidationError>, triggers?: readonly (WellKnownValidationTrigger | ValidationTrigger)[]): this {
if (triggers !== null && triggers !== undefined)
triggers.forEach(trigger => {
const resolvedValidationTriggers = resolveValidationTriggers(trigger);
resolvedValidationTriggers.forEach(this.triggers.add, this.triggers);
});
triggers.forEach(this.triggers.add, this.triggers);

if (validator !== null && validator !== undefined)
switch (typeof validator) {
Expand All @@ -93,28 +103,19 @@ export class ObjectValidator<TValidatable extends IValidatable<TValidationError>
}

public validate(): TValidationError | null {
if (this._isValidating)
return this.target.error;
else try {
this._isValidating = true;

let error: TValidationError | null = null;
let index: number = 0;

while (index < this.validators.length && error === null) {
const validationResult = this.validators[index].validate(this.target);
if (validationResult !== null && validationResult !== undefined)
error = validationResult;
else
index++;
}

this.target.error = error;
return error;
}
finally {
this._isValidating = false;
let error: TValidationError | null = null;
let index: number = 0;

while (index < this.validators.length && error === null) {
const validationResult = this.validators[index].validate(this.target);
if (validationResult !== null && validationResult !== undefined)
error = validationResult;
else
index++;
}

this.target.error = error;
return error;
}

public reset(): this {
Expand Down
65 changes: 55 additions & 10 deletions src/validation/tests/ObjectValidator.tests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ describe('ObjectValidator', (): void => {
let invocationCount = 0;
const validatable = new FakeValidatable();

const objectValidator = new ObjectValidator(validatable);
const objectValidator = new ObjectValidator({ target: validatable });
objectValidator.add(() => {
invocationCount++;
return 'test error';
Expand All @@ -22,7 +22,7 @@ describe('ObjectValidator', (): void => {
let invocationCount = 0;
const validatable = new FakeValidatable();

const objectValidator = new ObjectValidator(validatable);
const objectValidator = new ObjectValidator({ target: validatable });
objectValidator.add(() => {
invocationCount++;
return 'test error';
Expand All @@ -33,11 +33,31 @@ describe('ObjectValidator', (): void => {
expect(validatable.error).toBe('test error');
});

it('changing the target triggers the validation one more time when changed target properties are ignored', (): void => {
let invocationCount = 0;
const validatable = new FakeValidatable();

const objectValidator = new ObjectValidator({
target: validatable,
shouldTargetTriggerValidation() {
return true;
}
});
objectValidator.add(() => {
invocationCount++;
return 'test error';
});
validatable.notifyPropertiesChanged();

expect(invocationCount).toBe(3);
expect(validatable.error).toBe('test error');
});

it('using multiple validators executes them until first invalid one', (): void => {
const validatorCalls: string[] = [];
const validatable = new FakeValidatable();

const objectValidator = new ObjectValidator(validatable);
const objectValidator = new ObjectValidator({ target: validatable });
objectValidator.validators.push(
{
validate() {
Expand Down Expand Up @@ -73,7 +93,7 @@ describe('ObjectValidator', (): void => {
const validatable = new FakeValidatable();
const viewModelValidationTrigger = new FakeValidatable();

const objectValidator = new ObjectValidator(validatable);
const objectValidator = new ObjectValidator({ target: validatable });
objectValidator.add(
() => {
invocationCount++;
Expand All @@ -92,7 +112,7 @@ describe('ObjectValidator', (): void => {
const validatable = new FakeValidatable();
const observableCollectionValidationTrigger = new ObservableCollection<number>();

const objectValidator = new ObjectValidator(validatable);
const objectValidator = new ObjectValidator({ target: validatable });
objectValidator.add(
() => {
invocationCount++;
Expand All @@ -111,7 +131,7 @@ describe('ObjectValidator', (): void => {
const validatable = new FakeValidatable();
const observableCollectionValidationTrigger = new ObservableCollection<number>([1, 2]);

const objectValidator = new ObjectValidator(validatable);
const objectValidator = new ObjectValidator({ target: validatable });
objectValidator.add(
() => {
invocationCount++;
Expand All @@ -130,7 +150,7 @@ describe('ObjectValidator', (): void => {
const validatable = new FakeValidatable();
const observableSetValidationTrigger = new ObservableSet<number>();

const objectValidator = new ObjectValidator(validatable);
const objectValidator = new ObjectValidator({ target: validatable });
objectValidator.add(
() => {
invocationCount++;
Expand All @@ -149,7 +169,7 @@ describe('ObjectValidator', (): void => {
const validatable = new FakeValidatable();
const observableMapValidationTrigger = new ObservableMap<number, string>();

const objectValidator = new ObjectValidator(validatable);
const objectValidator = new ObjectValidator({ target: validatable });
objectValidator.add(
() => {
invocationCount++;
Expand All @@ -163,11 +183,36 @@ describe('ObjectValidator', (): void => {
expect(validatable.error).toBe('test error');
});

it('adding validation triggers multiple times only registers them once', () => {
const validatable = new FakeValidatable();
const viewModelValidationTrigger = new FakeValidatable();
const observableCollectionValidationTrigger = new ObservableCollection<number>();
const observableSetValidationTrigger = new ObservableSet<number>();
const observableMapValidationTrigger = new ObservableMap<number, string>();

const objectValidator = new ObjectValidator({ target: validatable });
objectValidator
.add(
() => 'test error 1',
[viewModelValidationTrigger, observableCollectionValidationTrigger, observableSetValidationTrigger, observableMapValidationTrigger]
)
.add(
() => 'test error 2',
[viewModelValidationTrigger, observableCollectionValidationTrigger, observableSetValidationTrigger, observableMapValidationTrigger]
);

expect(objectValidator.triggers.size).toBe(4);
expect(objectValidator.triggers).toContain(viewModelValidationTrigger);
expect(objectValidator.triggers).toContain(observableCollectionValidationTrigger);
expect(objectValidator.triggers).toContain(observableSetValidationTrigger);
expect(objectValidator.triggers).toContain(observableMapValidationTrigger);
});

it('adding a validator calls its onAdd hook', (): void => {
let hookInvocationCount = 0;
const validatable = new FakeValidatable();

const objectValidator = new ObjectValidator(validatable);
const objectValidator = new ObjectValidator({ target: validatable });
objectValidator.add({
onAdd(target) {
expect(target).toBe(validatable);
Expand All @@ -185,7 +230,7 @@ describe('ObjectValidator', (): void => {
let hookInvocationCount = 0;
const validatable = new FakeValidatable();

const objectValidator = new ObjectValidator(validatable);
const objectValidator = new ObjectValidator({ target: validatable });
objectValidator.add({
onRemove(target) {
expect(target).toBe(validatable);
Expand Down
4 changes: 3 additions & 1 deletion src/validation/triggers/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,6 @@ export { type IViewModelChangedValidationTriggerConfig, ViewModelChangedValidati
export { type ICollectionChangedValidationTriggerConfig, CollectionChangedValidationTrigger } from './CollectionChangedValidationTrigger';
export { type ICollectionReorderedValidationTriggerConfig, CollectionReorderedValidationTrigger } from './CollectionReorderedValidationTrigger';
export { type ISetChangedValidationTriggerConfig, SetChangedValidationTrigger } from './SetChangedValidationTrigger';
export { type IMapChangedValidationTriggerConfig, MapChangedValidationTrigger } from './MapChangedValidationTrigger';
export { type IMapChangedValidationTriggerConfig, MapChangedValidationTrigger } from './MapChangedValidationTrigger';

export { resolveValidationTriggers } from './resolveValidationTriggers';

0 comments on commit 4732e38

Please sign in to comment.