[Bug]: Fix async validations

src/index.ts

@@ -740,18 +740,23 @@ export class BufferedChangeset implements IChangeset {
 
     // TODO: Address case when Promise is rejected.
     if (isPromise(validation)) {
-      this._setIsValidating(key, true);
-
-      return (validation as Promise<ValidationResult>)
-        .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<ValidationResult>);
+
+      let running: RunningValidations = this[RUNNING_VALIDATIONS];
+      let promises = Object.entries(running);
+
+      return Promise.all(promises).then(() => {
+        return (validation as Promise<ValidationResult>)
+          .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 });
@@ -802,15 +807,20 @@ export class BufferedChangeset implements IChangeset {
     let content: Content = this[CONTENT];
 
     if (typeof validator === 'function') {
-      let isValid = validator({
+      let validationResult = validator({
         key,
         newValue,
         oldValue,
         changes: this.change,
         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;
@@ -841,16 +851,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<ValidationResult>): 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);
   }
 
   /**
@@ -963,8 +966,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 +977,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;

src/types/index.ts

@@ -162,7 +162,7 @@ export interface ChangesetDef {
     oldValue: any
   ) => ValidationResult | Promise<ValidationResult>;
   _setProperty: <T>(obj: NewProperty<T>) => void;
-  _setIsValidating: (key: string, value: boolean) => void;
+  _setIsValidating: (key: string, value: Promise<ValidationResult>) => void;
   _notifyVirtualProperties: (keys?: string[]) => string[] | undefined;
   _rollbackKeys: () => Array<string>;
   _deleteKey: (objName: InternalMapKey, key: string) => InternalMap;

test/index.test.ts

@@ -23,7 +23,7 @@ const dummyValidations: Record<string, any> = {
     }
 
     if (errors.length < 1) {
-      return;
+      return true;
     }
     return errors.length > 1 ? errors : errors[0];
   },
@@ -43,7 +43,7 @@ const dummyValidations: Record<string, any> = {
     );
   },
   async(_k: string, value: unknown) {
-    return Promise.resolve(value);
+    return Promise.resolve(value || '');
   },
   options(_k: string, value: unknown) {
     return !!value;
@@ -1702,16 +1702,21 @@ 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' } };
 
     await dummyChangeset.set('async', true);
     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', () => {
@@ -2664,6 +2669,7 @@ describe('Unit | Utility | changeset', () => {
       ...dummyModel,
       ...{
         name: 'Jim Bob',
+        email: 'jimmy@bob.com',
         password: true,
         passwordConfirmation: true,
         async: true,
@@ -3234,4 +3240,45 @@ 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 () => {
+    let latestDelayedAsyncResolver: Function = () => {};
+
+    dummyValidations.delayedAsync = () => {
+      return new Promise(resolve => {
+        latestDelayedAsyncResolver = resolve;
+      });
+    };
+
+    const dummyChangeset = Changeset(dummyModel, lookupValidator(dummyValidations));
+
+    dummyChangeset.set('delayedAsync', 'first');
+    let firstResolver = latestDelayedAsyncResolver;
+    dummyChangeset.set('delayedAsync', 'second');
+    let 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);
+  });
 });