From 3d705ff3d6a7221c767007ba098cb9380e860706 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Wed, 24 Feb 2021 08:11:21 -0600 Subject: [PATCH 1/5] [Bug]: Fix async validations --- src/index.ts | 13 ++++++++++--- test/index.test.ts | 5 +++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/index.ts b/src/index.ts index a897dd0..ec56fab 100644 --- a/src/index.ts +++ b/src/index.ts @@ -802,7 +802,7 @@ export class BufferedChangeset implements IChangeset { let content: Content = this[CONTENT]; if (typeof validator === 'function') { - let isValid = validator({ + let validationResult = validator({ key, newValue, oldValue, @@ -810,7 +810,12 @@ export class BufferedChangeset implements IChangeset { content }); - return typeof isValid === 'boolean' || Boolean(isValid) || isValid === '' ? isValid : true; + if (validationResult === undefined) { + // no validator function found for key + return true; + } + + return validationResult; } return true; @@ -963,8 +968,9 @@ export class BufferedChangeset implements IChangeset { } else if (typeof result !== 'undefined') { return result; } - const baseContent = this.safeGet(content, baseKey); + // TODO: make more obvious we are dealing with arrays with branches above + const baseContent = this.safeGet(content, baseKey); if (Array.isArray(baseContent)) { const subChanges = getSubObject(changes, key); @@ -973,6 +979,7 @@ export class BufferedChangeset implements IChangeset { } // give back an object that can further retrieve changes and/or content + // TODO: consider different construct to handle arrays. Arrays don't fit right with ObjectTreeNode const tree = new ObjectTreeNode(subChanges, baseContent, this.getDeep, this.isObject); return tree; diff --git a/test/index.test.ts b/test/index.test.ts index ed3d3c5..39e8744 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -23,7 +23,7 @@ const dummyValidations: Record = { } if (errors.length < 1) { - return; + return true; } return errors.length > 1 ? errors : errors[0]; }, @@ -43,7 +43,7 @@ const dummyValidations: Record = { ); }, async(_k: string, value: unknown) { - return Promise.resolve(value); + return Promise.resolve(value || ''); }, options(_k: string, value: unknown) { return !!value; @@ -2664,6 +2664,7 @@ describe('Unit | Utility | changeset', () => { ...dummyModel, ...{ name: 'Jim Bob', + email: 'jimmy@bob.com', password: true, passwordConfirmation: true, async: true, From 66a799574d9f660bafe348ffeed35d8b1ae07cd3 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Wed, 24 Feb 2021 11:37:23 -0600 Subject: [PATCH 2/5] add todo --- src/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/index.ts b/src/index.ts index ec56fab..235bb79 100644 --- a/src/index.ts +++ b/src/index.ts @@ -739,6 +739,7 @@ export class BufferedChangeset implements IChangeset { this.trigger(BEFORE_VALIDATION_EVENT, key); // TODO: Address case when Promise is rejected. + // TODO: await RunningValidations before checking the next in line if (isPromise(validation)) { this._setIsValidating(key, true); From 17e88e95a9d9bb4eabee549533f671c6c5c16fc9 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Wed, 24 Feb 2021 21:44:12 -0600 Subject: [PATCH 3/5] ensure sequential validations --- src/index.ts | 40 +++++++++++++++++------------------- src/types/index.ts | 2 +- test/index.test.ts | 51 ++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 69 insertions(+), 24 deletions(-) diff --git a/src/index.ts b/src/index.ts index 235bb79..da9a7d3 100644 --- a/src/index.ts +++ b/src/index.ts @@ -741,18 +741,23 @@ export class BufferedChangeset implements IChangeset { // TODO: Address case when Promise is rejected. // TODO: await RunningValidations before checking the next in line if (isPromise(validation)) { - this._setIsValidating(key, true); - - return (validation as Promise) - .then((resolvedValidation: ValidationResult) => { - this._setIsValidating(key, false); - - return this._handleValidation(resolvedValidation, { key, value }); - }) - .then(result => { - this.trigger(AFTER_VALIDATION_EVENT, key); - return result; - }); + this._setIsValidating(key, validation as Promise); + + let running: RunningValidations = this[RUNNING_VALIDATIONS]; + let promises = Object.entries(running); + + return Promise.all(promises).then(() => { + return (validation as Promise) + .then((resolvedValidation: ValidationResult) => { + delete running[key]; + + return this._handleValidation(resolvedValidation, { key, value }); + }) + .then(result => { + this.trigger(AFTER_VALIDATION_EVENT, key); + return result; + }); + }); } let result = this._handleValidation(validation as ValidationResult, { key, value }); @@ -847,16 +852,9 @@ export class BufferedChangeset implements IChangeset { * Increment or decrement the number of running validations for a * given key. */ - _setIsValidating(key: string, value: boolean): void { + _setIsValidating(key: string, promise: Promise): void { let running: RunningValidations = this[RUNNING_VALIDATIONS]; - let count: number = running[key] || 0; - - if (!value && count === 1) { - delete running[key]; - return; - } - - this.setDeep(running, key, value ? count + 1 : count - 1); + this.setDeep(running, key, promise); } /** diff --git a/src/types/index.ts b/src/types/index.ts index 6cb17e6..fe761fe 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -162,7 +162,7 @@ export interface ChangesetDef { oldValue: any ) => ValidationResult | Promise; _setProperty: (obj: NewProperty) => void; - _setIsValidating: (key: string, value: boolean) => void; + _setIsValidating: (key: string, value: Promise) => void; _notifyVirtualProperties: (keys?: string[]) => string[] | undefined; _rollbackKeys: () => Array; _deleteKey: (objName: InternalMapKey, key: string) => InternalMap; diff --git a/test/index.test.ts b/test/index.test.ts index 39e8744..9a7d693 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -1702,7 +1702,6 @@ describe('Unit | Utility | changeset', () => { it('it accepts async validations', async () => { const dummyChangeset = Changeset(dummyModel, lookupValidator(dummyValidations)); - /* const dummyChangeset = Changeset(dummyModel, dummyValidator); */ const expectedChanges = [{ key: 'async', value: true }]; const expectedError = { async: { validation: 'is invalid', value: 'is invalid' } }; @@ -1710,8 +1709,14 @@ describe('Unit | Utility | changeset', () => { expect(dummyChangeset.changes).toEqual(expectedChanges); dummyChangeset.set('async', 'is invalid'); - await dummyChangeset.save(); + expect(dummyChangeset.error).toEqual({}); + + await dummyChangeset.validate(); expect(dummyChangeset.error).toEqual(expectedError); + + await dummyChangeset.save(); + // save clears errors + expect(dummyChangeset.error).toEqual({}); }); it('it clears errors when setting to original value', () => { @@ -3235,4 +3240,46 @@ describe('Unit | Utility | changeset', () => { .then(() => c.set('org.usa.ny', 'should not fail')) .finally(done()); }); + + + async function delay(duration: number) { + return new Promise(function(resolve: Function){ + setTimeout(resolve, duration); + }); + }; + + it('it works with out of order async validations', async () => { + var latestDelayedAsyncResolver: Function = () => {}; + + dummyValidations.delayedAsync = () => { + return new Promise((resolve) => { + latestDelayedAsyncResolver = resolve; + }); + }; + + const dummyChangeset = Changeset(dummyModel, lookupValidator(dummyValidations)); + + dummyChangeset.set('delayedAsync', 'first'); + var firstResolver = latestDelayedAsyncResolver; + dummyChangeset.set('delayedAsync', 'second'); + var secondResolver = latestDelayedAsyncResolver; + + // second one resolves first with false + secondResolver(false); + // then the first resolves first with true + firstResolver(true); + + // allow promises to run + await delay(1); + + // clean up before running expectations + delete dummyValidations.delayedAsync; + + // current value state should be "second" + // current error state should be invalid + const expectedChanges = [{ key: 'delayedAsync', value: 'second' }]; + const expectedError = { delayedAsync: { validation: false, value: 'second' } }; + expect(dummyChangeset.changes).toEqual(expectedChanges); + expect(dummyChangeset.error).toEqual(expectedError); + }); }); From 850e70014001e6ab910ab34db678e763fbaf2dbb Mon Sep 17 00:00:00 2001 From: snewcomer Date: Wed, 24 Feb 2021 21:51:29 -0600 Subject: [PATCH 4/5] fix lint --- test/index.test.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/test/index.test.ts b/test/index.test.ts index 9a7d693..423f314 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -3241,18 +3241,17 @@ describe('Unit | Utility | changeset', () => { .finally(done()); }); - async function delay(duration: number) { - return new Promise(function(resolve: Function){ + return new Promise(function(resolve: Function) { setTimeout(resolve, duration); }); - }; + } it('it works with out of order async validations', async () => { - var latestDelayedAsyncResolver: Function = () => {}; + let latestDelayedAsyncResolver: Function = () => {}; dummyValidations.delayedAsync = () => { - return new Promise((resolve) => { + return new Promise(resolve => { latestDelayedAsyncResolver = resolve; }); }; @@ -3260,9 +3259,9 @@ describe('Unit | Utility | changeset', () => { const dummyChangeset = Changeset(dummyModel, lookupValidator(dummyValidations)); dummyChangeset.set('delayedAsync', 'first'); - var firstResolver = latestDelayedAsyncResolver; + let firstResolver = latestDelayedAsyncResolver; dummyChangeset.set('delayedAsync', 'second'); - var secondResolver = latestDelayedAsyncResolver; + let secondResolver = latestDelayedAsyncResolver; // second one resolves first with false secondResolver(false); From 9010f59be13a89ed7eccb1f05f071ed566fee3a5 Mon Sep 17 00:00:00 2001 From: snewcomer Date: Wed, 24 Feb 2021 22:51:30 -0600 Subject: [PATCH 5/5] rm comment --- src/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index da9a7d3..d995879 100644 --- a/src/index.ts +++ b/src/index.ts @@ -739,7 +739,6 @@ export class BufferedChangeset implements IChangeset { this.trigger(BEFORE_VALIDATION_EVENT, key); // TODO: Address case when Promise is rejected. - // TODO: await RunningValidations before checking the next in line if (isPromise(validation)) { this._setIsValidating(key, validation as Promise);