fix: return correct response when revert is used in beforeSave (#7839)

Author: dblythy

Diff for: spec/CloudCode.spec.js

@@ -1494,6 +1494,110 @@ describe('Cloud Code', () => {
     });
   });
 
+  it('before save can revert fields', async () => {
+    Parse.Cloud.beforeSave('TestObject', ({ object }) => {
+      object.revert('foo');
+      return object;
+    });
+
+    Parse.Cloud.afterSave('TestObject', ({ object }) => {
+      expect(object.get('foo')).toBeUndefined();
+      return object;
+    });
+
+    const obj = new TestObject();
+    obj.set('foo', 'bar');
+    await obj.save();
+
+    expect(obj.get('foo')).toBeUndefined();
+    await obj.fetch();
+
+    expect(obj.get('foo')).toBeUndefined();
+  });
+
+  it('before save can revert fields with existing object', async () => {
+    Parse.Cloud.beforeSave(
+      'TestObject',
+      ({ object }) => {
+        object.revert('foo');
+        return object;
+      },
+      {
+        skipWithMasterKey: true,
+      }
+    );
+
+    Parse.Cloud.afterSave(
+      'TestObject',
+      ({ object }) => {
+        expect(object.get('foo')).toBe('bar');
+        return object;
+      },
+      {
+        skipWithMasterKey: true,
+      }
+    );
+
+    const obj = new TestObject();
+    obj.set('foo', 'bar');
+    await obj.save(null, { useMasterKey: true });
+
+    expect(obj.get('foo')).toBe('bar');
+    obj.set('foo', 'yolo');
+    await obj.save();
+    expect(obj.get('foo')).toBe('bar');
+  });
+
+  it('can unset in afterSave', async () => {
+    Parse.Cloud.beforeSave('TestObject', ({ object }) => {
+      if (!object.existed()) {
+        object.set('secret', true);
+        return object;
+      }
+      object.revert('secret');
+    });
+
+    Parse.Cloud.afterSave('TestObject', ({ object }) => {
+      object.unset('secret');
+    });
+
+    Parse.Cloud.beforeFind(
+      'TestObject',
+      ({ query }) => {
+        query.exclude('secret');
+      },
+      {
+        skipWithMasterKey: true,
+      }
+    );
+
+    const obj = new TestObject();
+    await obj.save();
+    expect(obj.get('secret')).toBeUndefined();
+    await obj.fetch();
+    expect(obj.get('secret')).toBeUndefined();
+    await obj.fetch({ useMasterKey: true });
+    expect(obj.get('secret')).toBe(true);
+  });
+
+  it('should revert in beforeSave', async () => {
+    Parse.Cloud.beforeSave('MyObject', ({ object }) => {
+      if (!object.existed()) {
+        object.set('count', 0);
+        return object;
+      }
+      object.revert('count');
+      return object;
+    });
+    const obj = await new Parse.Object('MyObject').save();
+    expect(obj.get('count')).toBe(0);
+    obj.set('count', 10);
+    await obj.save();
+    expect(obj.get('count')).toBe(0);
+    await obj.fetch();
+    expect(obj.get('count')).toBe(0);
+  });
+
   it('beforeSave should not sanitize database', async done => {
     const { adapter } = Config.get(Parse.applicationId).database;
     const spy = spyOn(adapter, 'findOneAndUpdate').and.callThrough();
@@ -1860,6 +1964,36 @@ describe('afterSave hooks', () => {
     const myObject = new MyObject();
     myObject.save().then(() => done());
   });
+
+  it('should unset in afterSave', async () => {
+    Parse.Cloud.afterSave(
+      'MyObject',
+      ({ object }) => {
+        object.unset('secret');
+      },
+      {
+        skipWithMasterKey: true,
+      }
+    );
+    const obj = new Parse.Object('MyObject');
+    obj.set('secret', 'bar');
+    await obj.save();
+    expect(obj.get('secret')).toBeUndefined();
+    await obj.fetch();
+    expect(obj.get('secret')).toBe('bar');
+  });
+
+  it('should unset', async () => {
+    Parse.Cloud.beforeSave('MyObject', ({ object }) => {
+      object.set('secret', 'hidden');
+    });
+
+    Parse.Cloud.afterSave('MyObject', ({ object }) => {
+      object.unset('secret');
+    });
+    const obj = await new Parse.Object('MyObject').save();
+    expect(obj.get('secret')).toBeUndefined();
+  });
 });
 
 describe('beforeDelete hooks', () => {

Diff for: src/RestWrite.js

@@ -95,6 +95,7 @@ function RestWrite(config, auth, className, query, data, originalData, clientSDK
   // Shared SchemaController to be reused to reduce the number of loadSchema() calls per request
   // Once set the schemaData should be immutable
   this.validSchemaController = null;
+  this.pendingOps = {};
 }
 
 // A convenient method to perform all the steps of processing the
@@ -225,18 +226,11 @@ RestWrite.prototype.runBeforeSaveTrigger = function () {
     return Promise.resolve();
   }
 
-  // Cloud code gets a bit of extra data for its objects
-  var extraData = { className: this.className };
-  if (this.query && this.query.objectId) {
-    extraData.objectId = this.query.objectId;
-  }
+  const { originalObject, updatedObject } = this.buildParseObjects();
 
-  let originalObject = null;
-  const updatedObject = this.buildUpdatedObject(extraData);
-  if (this.query && this.query.objectId) {
-    // This is an update for existing object.
-    originalObject = triggers.inflate(extraData, this.originalData);
-  }
+  const stateController = Parse.CoreManager.getObjectStateController();
+  const [pending] = stateController.getPendingOps(updatedObject._getStateIdentifier());
+  this.pendingOps = { ...pending };
 
   return Promise.resolve()
     .then(() => {
@@ -1531,20 +1525,7 @@ RestWrite.prototype.runAfterSaveTrigger = function () {
     return Promise.resolve();
   }
 
-  var extraData = { className: this.className };
-  if (this.query && this.query.objectId) {
-    extraData.objectId = this.query.objectId;
-  }
-
-  // Build the original object, we only do this for a update write.
-  let originalObject;
-  if (this.query && this.query.objectId) {
-    originalObject = triggers.inflate(extraData, this.originalData);
-  }
-
-  // Build the inflated object, different from beforeSave, originalData is not empty
-  // since developers can change data in the beforeSave.
-  const updatedObject = this.buildUpdatedObject(extraData);
+  const { originalObject, updatedObject } = this.buildParseObjects();
   updatedObject._handleSaveResponse(this.response.response, this.response.status || 200);
 
   this.config.database.loadSchema().then(schemaController => {
@@ -1569,8 +1550,15 @@ RestWrite.prototype.runAfterSaveTrigger = function () {
       this.context
     )
     .then(result => {
-      if (result && typeof result === 'object') {
+      const jsonReturned = result && !result._toFullJSON;
+      if (jsonReturned) {
+        this.pendingOps = {};
         this.response.response = result;
+      } else {
+        this.response.response = this._updateResponseWithData(
+          (result || updatedObject)._toFullJSON(),
+          this.data
+        );
       }
     })
     .catch(function (err) {
@@ -1604,7 +1592,13 @@ RestWrite.prototype.sanitizedData = function () {
 };
 
 // Returns an updated copy of the object
-RestWrite.prototype.buildUpdatedObject = function (extraData) {
+RestWrite.prototype.buildParseObjects = function () {
+  const extraData = { className: this.className, objectId: this.query?.objectId };
+  let originalObject;
+  if (this.query && this.query.objectId) {
+    originalObject = triggers.inflate(extraData, this.originalData);
+  }
+
   const className = Parse.Object.fromJSON(extraData);
   const readOnlyAttributes = className.constructor.readOnlyAttributes
     ? className.constructor.readOnlyAttributes()
@@ -1642,7 +1636,7 @@ RestWrite.prototype.buildUpdatedObject = function (extraData) {
     delete sanitized[attribute];
   }
   updatedObject.set(sanitized);
-  return updatedObject;
+  return { updatedObject, originalObject };
 };
 
 RestWrite.prototype.cleanUserAuthData = function () {
@@ -1662,6 +1656,15 @@ RestWrite.prototype.cleanUserAuthData = function () {
 };
 
 RestWrite.prototype._updateResponseWithData = function (response, data) {
+  const { updatedObject } = this.buildParseObjects();
+  const stateController = Parse.CoreManager.getObjectStateController();
+  const [pending] = stateController.getPendingOps(updatedObject._getStateIdentifier());
+  for (const key in this.pendingOps) {
+    if (!pending[key]) {
+      data[key] = this.originalData ? this.originalData[key] : { __op: 'Delete' };
+      this.storage.fieldsChangedByTrigger.push(key);
+    }
+  }
   if (_.isEmpty(this.storage.fieldsChangedByTrigger)) {
     return response;
   }