From fb0ae8ed9e640419a06f50ec20f07dac673f83c1 Mon Sep 17 00:00:00 2001 From: dblythy <daniel-blyth@live.com.au> Date: Fri, 11 Mar 2022 16:50:08 +1100 Subject: [PATCH 1/9] fix: correct response when revert is used in beforeSave --- spec/CloudCode.spec.js | 76 ++++++++++++++++++++++++++++++++++++++++++ src/RestWrite.js | 52 +++++++++++++---------------- 2 files changed, 100 insertions(+), 28 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 4b8df9f9c9..20760bf834 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -1494,6 +1494,82 @@ describe('Cloud Code', () => { }); }); + it('before save can revert fields', async () => { + await reconfigureServer({ silent: false }); + + 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 () => { + await reconfigureServer({ silent: false }); + + 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('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(); diff --git a/src/RestWrite.js b/src/RestWrite.js index a651cf9c6c..ba3ae2e063 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -81,6 +81,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 @@ -211,18 +212,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(() => { @@ -1517,20 +1511,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 => { @@ -1556,7 +1537,7 @@ RestWrite.prototype.runAfterSaveTrigger = function () { ) .then(result => { if (result && typeof result === 'object') { - this.response.response = result; + this.response.response = this._updateResponseWithData(result._toFullJSON(), this.data); } }) .catch(function (err) { @@ -1590,7 +1571,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() @@ -1628,7 +1615,7 @@ RestWrite.prototype.buildUpdatedObject = function (extraData) { delete sanitized[attribute]; } updatedObject.set(sanitized); - return updatedObject; + return { updatedObject, originalObject }; }; RestWrite.prototype.cleanUserAuthData = function () { @@ -1648,6 +1635,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; } From 519a838abbb8e0e85119e9d57de3fc3b8123d830 Mon Sep 17 00:00:00 2001 From: dblythy <daniel-blyth@live.com.au> Date: Fri, 11 Mar 2022 17:03:17 +1100 Subject: [PATCH 2/9] add unset in afterSave --- spec/CloudCode.spec.js | 41 +++++++++++++++++++++++++++++++++++++---- src/RestWrite.js | 5 ++--- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 20760bf834..88a3a01a56 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -1495,8 +1495,6 @@ describe('Cloud Code', () => { }); it('before save can revert fields', async () => { - await reconfigureServer({ silent: false }); - Parse.Cloud.beforeSave('TestObject', ({ object }) => { object.revert('foo'); return object; @@ -1518,8 +1516,6 @@ describe('Cloud Code', () => { }); it('before save can revert fields with existing object', async () => { - await reconfigureServer({ silent: false }); - Parse.Cloud.beforeSave( 'TestObject', ({ object }) => { @@ -1552,6 +1548,43 @@ describe('Cloud Code', () => { 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; + }); + + Parse.Cloud.afterSave( + 'TestObject', + ({ object }) => { + object.unset('secret'); + }, + { + skipWithMasterKey: true, + } + ); + + Parse.Cloud.afterFind( + 'TestObject', + ({ objects }) => { + return objects.map(object => object.unset('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()) { diff --git a/src/RestWrite.js b/src/RestWrite.js index ba3ae2e063..39dc4fe0d0 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -1536,9 +1536,8 @@ RestWrite.prototype.runAfterSaveTrigger = function () { this.context ) .then(result => { - if (result && typeof result === 'object') { - this.response.response = this._updateResponseWithData(result._toFullJSON(), this.data); - } + const object = result && typeof result === 'object' ? result : updatedObject; + this.response.response = this._updateResponseWithData(object._toFullJSON(), this.data); }) .catch(function (err) { logger.warn('afterSave caught an error', err); From 348434cb3272d9566e834c7ac216d5947bd675b3 Mon Sep 17 00:00:00 2001 From: dblythy <daniel-blyth@live.com.au> Date: Fri, 11 Mar 2022 17:08:50 +1100 Subject: [PATCH 3/9] Update CloudCode.spec.js --- spec/CloudCode.spec.js | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 88a3a01a56..0d20442228 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -1553,23 +1553,16 @@ describe('Cloud Code', () => { if (!object.existed()) { object.set('secret', true); } - return object; }); - Parse.Cloud.afterSave( - 'TestObject', - ({ object }) => { - object.unset('secret'); - }, - { - skipWithMasterKey: true, - } - ); + Parse.Cloud.afterSave('TestObject', ({ object }) => { + object.unset('secret'); + }); Parse.Cloud.afterFind( 'TestObject', ({ objects }) => { - return objects.map(object => object.unset('secret')); + objects.map(object => object.unset('secret')); }, { skipWithMasterKey: true, From e175e7b12fccc5081879ca1341710d78152a5666 Mon Sep 17 00:00:00 2001 From: dblythy <daniel-blyth@live.com.au> Date: Fri, 11 Mar 2022 18:39:08 +1100 Subject: [PATCH 4/9] add json check --- spec/RestQuery.spec.js | 11 +++++------ src/RestWrite.js | 2 +- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/spec/RestQuery.spec.js b/spec/RestQuery.spec.js index c8e3adb49d..e2eb00315a 100644 --- a/spec/RestQuery.spec.js +++ b/spec/RestQuery.spec.js @@ -412,12 +412,11 @@ describe('RestQuery.each', () => { }); Parse.Cloud.afterSave('TestObject2', function (req) { - const jsonObject = req.object.toJSON(); - delete jsonObject.todelete; - delete jsonObject.tobeaddbeforeandremoveafter; - jsonObject.toadd = true; - - return jsonObject; + expect(req.object.get('tobeaddbefore')).toBeTruthy(); + expect(req.object.get('tobeaddbeforeandremoveafter')).toBeTruthy(); + req.object.set('todelete', undefined); + req.object.set('tobeaddbeforeandremoveafter', undefined); + req.object.set('toadd', true); }); rest.create(config, nobody, 'TestObject2', { todelete: true, tokeep: true }).then(response => { diff --git a/src/RestWrite.js b/src/RestWrite.js index 39dc4fe0d0..fd9233e9b6 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -1536,7 +1536,7 @@ RestWrite.prototype.runAfterSaveTrigger = function () { this.context ) .then(result => { - const object = result && typeof result === 'object' ? result : updatedObject; + const object = result?._toFullJSON ? result : updatedObject; this.response.response = this._updateResponseWithData(object._toFullJSON(), this.data); }) .catch(function (err) { From 62c87152c87ade15ee9255c35527369902792e48 Mon Sep 17 00:00:00 2001 From: Manuel <5673677+mtrezza@users.noreply.github.com> Date: Fri, 18 Mar 2022 16:41:27 +0100 Subject: [PATCH 5/9] docs: improve reverting in CONTRIBUTION guide (#7866) --- CONTRIBUTING.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index df5df279e5..3e0752541c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -363,6 +363,22 @@ If the commit reverts a previous commit, use the prefix `revert:`, followed by t This reverts commit 1234567890abcdef. ``` +⚠️ A `revert` prefix will *always* trigger a release. Generally, a commit that did not trigger a release when it was initially merged should also not trigger a release when it is reverted. For example, do not use the `revert` prefix when reverting a commit that has a `ci` prefix: + + ``` + ci: add something + ``` + is reverted with: + ``` + ci: remove something + ``` + instead of: + ``` + revert: ci: add something + + This reverts commit 1234567890abcdef. + ``` + ### Major Release / Long-Term-Support Long-Term-Support (LTS) is provided for the previous Parse Server major version. For example, Parse Server 4.x will receive security updates until Parse Server 5.x is superseded by Parse Server 6.x and becomes the new LTS version. While the current major version is published on branch `release`, a LTS version is published on branch `release-#.x.x`, for example `release-4.x.x` for the Parse Server 4.x LTS branch. From 0e00a376ba89a1fe5030dbb9d42cba103353aa0e Mon Sep 17 00:00:00 2001 From: dblythy <daniel-blyth@live.com.au> Date: Sat, 19 Mar 2022 16:43:38 +1100 Subject: [PATCH 6/9] revert rest return --- spec/RestQuery.spec.js | 11 ++++++----- src/RestWrite.js | 8 ++++++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/spec/RestQuery.spec.js b/spec/RestQuery.spec.js index e2eb00315a..c8e3adb49d 100644 --- a/spec/RestQuery.spec.js +++ b/spec/RestQuery.spec.js @@ -412,11 +412,12 @@ describe('RestQuery.each', () => { }); Parse.Cloud.afterSave('TestObject2', function (req) { - expect(req.object.get('tobeaddbefore')).toBeTruthy(); - expect(req.object.get('tobeaddbeforeandremoveafter')).toBeTruthy(); - req.object.set('todelete', undefined); - req.object.set('tobeaddbeforeandremoveafter', undefined); - req.object.set('toadd', true); + const jsonObject = req.object.toJSON(); + delete jsonObject.todelete; + delete jsonObject.tobeaddbeforeandremoveafter; + jsonObject.toadd = true; + + return jsonObject; }); rest.create(config, nobody, 'TestObject2', { todelete: true, tokeep: true }).then(response => { diff --git a/src/RestWrite.js b/src/RestWrite.js index 409a6f4e86..f8d1cda5fc 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -1550,8 +1550,12 @@ RestWrite.prototype.runAfterSaveTrigger = function () { this.context ) .then(result => { - const object = result?._toFullJSON ? result : updatedObject; - this.response.response = this._updateResponseWithData(object._toFullJSON(), this.data); + if (result && typeof result === 'object') { + if (!result._toFullJSON) { + this.pendingOps = {}; + } + this.response.response = result; + } }) .catch(function (err) { logger.warn('afterSave caught an error', err); From b36036d6b58319cd023c793138d70a6777af789f Mon Sep 17 00:00:00 2001 From: dblythy <daniel-blyth@live.com.au> Date: Sat, 19 Mar 2022 17:07:50 +1100 Subject: [PATCH 7/9] check if json is returned --- spec/CloudCode.spec.js | 30 ++++++++++++++++++++++++++++++ src/RestWrite.js | 12 ++++++++---- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 0d20442228..f1f6b290d6 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -1962,6 +1962,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 f8d1cda5fc..e17e5804e5 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -1550,11 +1550,15 @@ RestWrite.prototype.runAfterSaveTrigger = function () { this.context ) .then(result => { - if (result && typeof result === 'object') { - if (!result._toFullJSON) { - this.pendingOps = {}; - } + const jsonReturned = result && !result._toFullJSON; + if (jsonReturned) { + this.pendingOps = true; this.response.response = result; + } else { + this.response.response = this._updateResponseWithData( + (result || updatedObject)._toFullJSON(), + this.data + ); } }) .catch(function (err) { From 34b32c3b4567a3ce4b91014004491755e1fdd6e5 Mon Sep 17 00:00:00 2001 From: dblythy <daniel-blyth@live.com.au> Date: Mon, 21 Mar 2022 15:46:23 +1100 Subject: [PATCH 8/9] Update CloudCode.spec.js --- spec/CloudCode.spec.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index f1f6b290d6..faaa6b826a 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -1552,17 +1552,19 @@ describe('Cloud Code', () => { 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.afterFind( + Parse.Cloud.beforeFind( 'TestObject', - ({ objects }) => { - objects.map(object => object.unset('secret')); + ({ query }) => { + query.exclude('secret'); }, { skipWithMasterKey: true, From d93abb9f9f859f266edcc08e7a83ca9df745367b Mon Sep 17 00:00:00 2001 From: dblythy <daniel-blyth@live.com.au> Date: Wed, 23 Mar 2022 14:20:47 +1100 Subject: [PATCH 9/9] Update RestWrite.js --- src/RestWrite.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/RestWrite.js b/src/RestWrite.js index e17e5804e5..3e20328a9a 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -1552,7 +1552,7 @@ RestWrite.prototype.runAfterSaveTrigger = function () { .then(result => { const jsonReturned = result && !result._toFullJSON; if (jsonReturned) { - this.pendingOps = true; + this.pendingOps = {}; this.response.response = result; } else { this.response.response = this._updateResponseWithData(