From 9b173729497aa1b209b654d9c6417addce0a2aa8 Mon Sep 17 00:00:00 2001 From: Sudaakeran Balasubramanian Date: Wed, 27 Jul 2016 12:07:51 +0530 Subject: [PATCH 1/4] test case to check beforeSave changes clobbers fetched pointer fields Basically if beforeSave makes any changes to the object it is trying to save, the fetched pointer fields on the client gets clobbered to only pointer. --- spec/CloudCode.spec.js | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 3e5fdaa029..4a2c93d324 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -684,6 +684,44 @@ describe('Cloud Code', () => { }); }); +it('beforeSave should not affect fetched pointers', done => { + Parse.Cloud.beforeSave('BeforeSaveUnchanged', (req, res) => { + res.success(); + }); + + Parse.Cloud.beforeSave('BeforeSaveChanged', function(req, res) { + req.object.set('foo', 'baz'); + res.success(); + }); + + var TestObject = Parse.Object.extend("TestObject"); + var BeforeSaveUnchangedObject = Parse.Object.extend("BeforeSaveUnchanged"); + var BeforeSaveChangedObject = Parse.Object.extend("BeforeSaveChanged"); + + var aTestObject = new TestObject(); + aTestObject.set("foo", "bar"); + aTestObject.save() + .then(aTestObject => { + var aBeforeSaveUnchangedObject = new BeforeSaveUnchangedObject(); + aBeforeSaveUnchangedObject.set("aTestObject", aTestObject); + expect(aBeforeSaveUnchangedObject.get("aTestObject").get("foo")).toEqual("bar"); + return aBeforeSaveUnchangedObject.save(); + }) + .then(aBeforeSaveUnchangedObject => { + expect(aBeforeSaveUnchangedObject.get("aTestObject").get("foo")).toEqual("bar"); + + var aBeforeSaveChangedObject = new BeforeSaveChangedObject(); + aBeforeSaveChangedObject.set("aTestObject", aTestObject); + expect(aBeforeSaveChangedObject.get("aTestObject").get("foo")).toEqual("bar"); + return aBeforeSaveChangedObject.save(); + }) + .then(aBeforeSaveChangedObject => { + expect(aBeforeSaveChangedObject.get("aTestObject").get("foo")).toEqual("bar"); + expect(aBeforeSaveChangedObject.get("foo")).toEqual("baz"); + done(); + }); + }); + it_exclude_dbs(['postgres'])('should fully delete objects when using `unset` with beforeSave (regression test for #1840)', done => { var TestObject = Parse.Object.extend('TestObject'); var NoBeforeSaveObject = Parse.Object.extend('NoBeforeSave'); From e14ec7fc208aeddcc4b9549d6c7b9e347993c6c4 Mon Sep 17 00:00:00 2001 From: Sudaakeran Balasubramanian Date: Wed, 27 Jul 2016 12:29:55 +0530 Subject: [PATCH 2/4] propogate only changed fields to response. Earlier we were returning all fields even if any changes happened in beforeSave. This causes the fetched pointer fields on the client to get clobbered to only pointers. This fix returns only the changed fields thus avoiding pointer clobber. --- src/RestWrite.js | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/RestWrite.js b/src/RestWrite.js index c522a99188..adb5b708e8 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -165,9 +165,12 @@ RestWrite.prototype.runBeforeTrigger = function() { return triggers.maybeRunTrigger(triggers.Types.beforeSave, this.auth, updatedObject, originalObject, this.config); }).then((response) => { if (response && response.object) { - if (!_.isEqual(this.data, response.object)) { - this.storage.changedByTrigger = true; - } + this.storage.fieldsChangedByTrigger = _.reduce(response.object, (result, value, key) => { + if (!_.isEqual(this.data[key], value)) { + result.push(key); + } + return result; + }, []); this.data = response.object; // We should delete the objectId for an update write if (this.query && this.query.objectId) { @@ -792,9 +795,7 @@ RestWrite.prototype.runDatabaseOperation = function() { return this.config.database.update(this.className, this.query, this.data, this.runOptions) .then(response => { response.updatedAt = this.updatedAt; - if (this.storage.changedByTrigger) { - this.updateResponseWithData(response, this.data); - } + this._updateResponseWithData(response, this.data); this.response = { response }; }); } else { @@ -850,9 +851,7 @@ RestWrite.prototype.runDatabaseOperation = function() { if (this.responseShouldHaveUsername) { response.username = this.data.username; } - if (this.storage.changedByTrigger) { - this.updateResponseWithData(response, this.data); - } + this._updateResponseWithData(response, this.data); this.response = { status: 201, response, @@ -940,9 +939,12 @@ RestWrite.prototype.cleanUserAuthData = function() { } }; -RestWrite.prototype.updateResponseWithData = function(response, data) { +RestWrite.prototype._updateResponseWithData = function(response, data) { + if (_.isEmpty(this.storage.fieldsChangedByTrigger)) { + return response; + } let clientSupportsDelete = ClientSDK.supportsForwardDelete(this.clientSDK); - Object.keys(data).forEach(fieldName => { + this.storage.fieldsChangedByTrigger.forEach(fieldName => { let dataValue = data[fieldName]; let responseValue = response[fieldName]; From 6b299c2c8f71694a2d0179fc115537a9efb400a3 Mon Sep 17 00:00:00 2001 From: Sudaakeran Balasubramanian Date: Wed, 27 Jul 2016 12:40:35 +0530 Subject: [PATCH 3/4] The goal of this comparision seems to be checking that the all returns the user correctly. Also it is consistent with the hosted parse that user.username not returned from PUT request. --- spec/ParseUser.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index 8a4d918649..0be4ed171e 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -2374,7 +2374,7 @@ describe('Parse.User testing', () => { }, json: true }, (err, res, body) => { - expect(body.username).toEqual(user.username); + expect(body.username).toEqual('user'); expect(body.objectId).toEqual(originalUserId); if (err) { reject(err); From 19b2d3eaa4cd84d18d7258e9a26ef4e62c7ae088 Mon Sep 17 00:00:00 2001 From: Sudaakeran Balasubramanian Date: Wed, 27 Jul 2016 16:28:22 +0530 Subject: [PATCH 4/4] test file --- .testfile | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 .testfile diff --git a/.testfile b/.testfile new file mode 100644 index 0000000000..e69de29bb2