From 19900fcdf8c9f29a674fb62cf6e4b3341d796891 Mon Sep 17 00:00:00 2001 From: dblythy Date: Sat, 26 Mar 2022 13:39:16 +1100 Subject: [PATCH] fix: return correct response when revert is used in beforeSave (#7839) --- spec/CloudCode.spec.js | 134 +++++++++++++++++++++++++++++++++++++++++ src/RestWrite.js | 59 +++++++++--------- 2 files changed, 165 insertions(+), 28 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 4b8df9f9c9..faaa6b826a 100644 --- a/spec/CloudCode.spec.js +++ b/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 --git a/src/RestWrite.js b/src/RestWrite.js index 8b728731da..3e20328a9a 100644 --- a/src/RestWrite.js +++ b/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; }