Syncing afterSave/afterDelete trigger calls (Issue #2489) (#2499)

* Implemented syncing afterSave/afterDelete trigger calls with REST request execution flow (Issue 2489). After this change, afterSave and afterDelete triggers CAN return a promise, which needs to be resolved inside a trigger for REST request flow to continue. If trigger doesn't return a promise, request flow continues.

* Added {} to multiline if.

* Fixed bad commit.

* Fixed problem with beforeSave triggers becoming async.

spec/CloudCode.spec.js

@@ -162,6 +162,162 @@ describe('Cloud Code', () => {
     }, 500);
   });
 
+  it('test afterSave ran on created object and returned a promise', function(done) {
+    Parse.Cloud.afterSave('AfterSaveTest2', function(req) {
+        let obj = req.object;
+        if(!obj.existed())
+        {
+            let promise = new Parse.Promise();
+            setTimeout(function(){
+                obj.set('proof', obj.id);
+                obj.save().then(function(){
+                    promise.resolve();
+                });
+            }, 1000);
+
+            return promise;
+        }
+    });
+
+    let obj = new Parse.Object('AfterSaveTest2');
+    obj.save().then(function(){
+        let query = new Parse.Query('AfterSaveTest2');
+        query.equalTo('proof', obj.id);
+        query.find().then(function(results) {
+            expect(results.length).toEqual(1);
+            let savedObject = results[0];
+            expect(savedObject.get('proof')).toEqual(obj.id);
+            done();
+        },
+        function(error) {
+            fail(error);
+            done();
+        });
+    });
+  });
+
+  it('test afterSave ignoring promise, object not found', function(done) {
+    Parse.Cloud.afterSave('AfterSaveTest2', function(req) {
+        let obj = req.object;
+        if(!obj.existed())
+        {
+            let promise = new Parse.Promise();
+            setTimeout(function(){
+                obj.set('proof', obj.id);
+                obj.save().then(function(){
+                    promise.resolve();
+                });
+            }, 1000);
+
+            return promise;
+        }
+    });
+
+    let obj = new Parse.Object('AfterSaveTest2');
+    obj.save().then(function(){
+        done();
+    })
+
+    let query = new Parse.Query('AfterSaveTest2');
+    query.equalTo('proof', obj.id);
+    query.find().then(function(results) {
+        expect(results.length).toEqual(0);
+    },
+    function(error) {
+        fail(error);
+    });
+  });
+
+  it('test afterSave rejecting promise', function(done) {
+      Parse.Cloud.afterSave('AfterSaveTest2', function(req) {
+          let promise = new Parse.Promise();
+          setTimeout(function(){
+              promise.reject("THIS SHOULD BE IGNORED");
+          }, 1000);
+
+          return promise;
+      });
+
+      let obj = new Parse.Object('AfterSaveTest2');
+      obj.save().then(function(){
+          done();
+      }, function(error){
+          fail(error);
+          done();
+      })
+  });
+
+  it('test afterDelete returning promise, object is deleted when destroy resolves', function(done) {
+      Parse.Cloud.afterDelete('AfterDeleteTest2', function(req) {
+        let promise = new Parse.Promise();
+
+        setTimeout(function(){
+            let obj = new Parse.Object('AfterDeleteTestProof');
+            obj.set('proof', req.object.id);
+            obj.save().then(function(){
+                promise.resolve();
+            });
+
+        }, 1000);
+
+        return promise;
+      });
+
+      let errorHandler = function(error) {
+          fail(error);
+          done();
+      }
+
+      let obj = new Parse.Object('AfterDeleteTest2');
+      obj.save().then(function(){
+          obj.destroy().then(function(){
+              let query = new Parse.Query('AfterDeleteTestProof');
+              query.equalTo('proof', obj.id);
+              query.find().then(function(results) {
+                  expect(results.length).toEqual(1);
+                  let deletedObject = results[0];
+                  expect(deletedObject.get('proof')).toEqual(obj.id);
+                  done();
+              }, errorHandler);
+          }, errorHandler)
+      }, errorHandler);
+  });
+
+  it('test afterDelete ignoring promise, object is not yet deleted', function(done) {
+      Parse.Cloud.afterDelete('AfterDeleteTest2', function(req) {
+        let promise = new Parse.Promise();
+
+        setTimeout(function(){
+            let obj = new Parse.Object('AfterDeleteTestProof');
+            obj.set('proof', req.object.id);
+            obj.save().then(function(){
+                promise.resolve();
+            });
+
+        }, 1000);
+
+        return promise;
+      });
+
+      let errorHandler = function(error) {
+          fail(error);
+          done();
+      }
+
+      let obj = new Parse.Object('AfterDeleteTest2');
+      obj.save().then(function(){
+          obj.destroy().then(function(){
+              done();
+          })
+
+          let query = new Parse.Query('AfterDeleteTestProof');
+          query.equalTo('proof', obj.id);
+          query.find().then(function(results) {
+              expect(results.length).toEqual(0);
+          }, errorHandler);
+      }, errorHandler);
+  });
+
   it('test beforeSave happens on update', function(done) {
     Parse.Cloud.beforeSave('BeforeSaveChanged', function(req, res) {
       req.object.set('foo', 'baz');

src/RestWrite.js

@@ -296,10 +296,10 @@ RestWrite.prototype.handleAuthData = function(authData) {
         // Login with auth data
         delete results[0].password;
         let userResult = results[0];
-        
+
         // need to set the objectId first otherwise location has trailing undefined
         this.data.objectId = userResult.objectId;
-        
+
         // Determine if authData was updated
         let mutatedAuthData = {};
         Object.keys(authData).forEach((provider) => {
@@ -309,7 +309,7 @@ RestWrite.prototype.handleAuthData = function(authData) {
             mutatedAuthData[provider] = providerData;
           }
         });
-        
+
         this.response = {
           response: userResult,
           location: this.location()
@@ -328,7 +328,7 @@ RestWrite.prototype.handleAuthData = function(authData) {
           return this.config.database.update(this.className, {objectId: this.data.objectId}, {authData: mutatedAuthData}, {});
         }
         return;
-        
+
       } else if (this.query && this.query.objectId) {
         // Trying to update auth data but users
         // are different
@@ -476,7 +476,7 @@ RestWrite.prototype.handleFollowup = function() {
     return this.config.database.destroy('_Session', sessionQuery)
     .then(this.handleFollowup.bind(this));
   }
-  
+
   if (this.storage && this.storage['generateNewSession']) {
     delete this.storage['generateNewSession'];
     return this.createSessionToken()
@@ -847,7 +847,7 @@ RestWrite.prototype.runDatabaseOperation = function() {
     .then(response => {
       response.objectId = this.data.objectId;
       response.createdAt = this.data.createdAt;
-      
+
       if (this.responseShouldHaveUsername) {
         response.username = this.data.username;
       }
@@ -895,7 +895,7 @@ RestWrite.prototype.runAfterTrigger = function() {
   this.config.liveQueryController.onAfterSave(updatedObject.className, updatedObject, originalObject);
 
   // Run afterSave trigger
-  triggers.maybeRunTrigger(triggers.Types.afterSave, this.auth, updatedObject, originalObject, this.config);
+  return triggers.maybeRunTrigger(triggers.Types.afterSave, this.auth, updatedObject, originalObject, this.config);
 };
 
 // A helper to figure out what location this operation happens at.
@@ -949,7 +949,7 @@ RestWrite.prototype._updateResponseWithData = function(response, data) {
     let responseValue = response[fieldName];
 
     response[fieldName] = responseValue || dataValue;
-    
+
     // Strips operations from responses
     if (response[fieldName] && response[fieldName].__op) {
       delete response[fieldName];

src/rest.js

@@ -86,8 +86,7 @@ function del(config, auth, className, objectId, clientSDK) {
       objectId: objectId
     }, options);
   }).then(() => {
-    triggers.maybeRunTrigger(triggers.Types.afterDelete, auth, inflatedObject, null, config);
+    return triggers.maybeRunTrigger(triggers.Types.afterDelete, auth, inflatedObject, null, config);    
-    return;
   });
 }
 

src/triggers.js

@@ -158,7 +158,7 @@ function logTrigger(triggerType, className, input) {
     return;
   }
   logger.info(`${triggerType} triggered for ${className}\nInput: ${JSON.stringify(input)}`, {
-    className, 
+    className,
     triggerType,
     input
   });
@@ -166,7 +166,7 @@ function logTrigger(triggerType, className, input) {
 
 function logTriggerSuccess(triggerType, className, input, result) {
   logger.info(`${triggerType} triggered for ${className}\nInput: ${JSON.stringify(input)}\nResult: ${JSON.stringify(result)}`, {
-    className, 
+    className,
     triggerType,
     input,
     result
@@ -175,7 +175,7 @@ function logTriggerSuccess(triggerType, className, input, result) {
 
 function logTriggerError(triggerType, className, input, error) {
   logger.error(`${triggerType} failed for ${className}\nInput: ${JSON.stringify(input)}\Error: ${JSON.stringify(error)}`, {
-    className, 
+    className,
     triggerType,
     input,
     error
@@ -209,7 +209,20 @@ export function maybeRunTrigger(triggerType, auth, parseObject, originalParseObj
     Parse.masterKey = config.masterKey;
     // For the afterSuccess / afterDelete
     logTrigger(triggerType, parseObject.className, parseObject.toJSON());
-    trigger(request, response);
+
+    //AfterSave and afterDelete triggers can return a promise, which if they do, needs to be resolved before this promise is resolved,
+    //so trigger execution is synced with RestWrite.execute() call.
+    //If triggers do not return a promise, they can run async code parallel to the RestWrite.execute() call.
+    var triggerPromise = trigger(request, response);
+    if(triggerType === Types.afterSave || triggerType === Types.afterDelete)
+    {
+        if(triggerPromise && typeof triggerPromise.then === "function") {
+            return triggerPromise.then(resolve, resolve);
+        }
+        else {
+            return resolve();
+        }
+    }
   });
 };