From a2a510529f89f13f237555108b0c881f5217e7a7 Mon Sep 17 00:00:00 2001 From: Tommy Goode Date: Sat, 28 May 2016 20:11:17 -0500 Subject: [PATCH 1/8] Adding a test demonstrating issue #1840. --- spec/CloudCode.spec.js | 56 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 06aef43d36..26e864b1bb 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -666,4 +666,60 @@ describe('Cloud Code', () => { done(); }); }); + + it('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'); + var BeforeSaveObject = Parse.Object.extend('BeforeSaveChanged'); + + Parse.Cloud.beforeSave('BeforeSaveChanged', (req, res) => { + var object = req.object; + object.set('before', 'save'); + res.success(); + }); + + Parse.Cloud.define('removeme', (req, res) => { + var testObject = new TestObject(); + testObject.save() + .then(testObject => { + var object = new NoBeforeSaveObject({remove: testObject}); + return object.save(); + }) + .then(object => { + object.unset('remove'); + return object.save(); + }) + .then(object => { + res.success(object); + }); + }); + + Parse.Cloud.define('removeme2', (req, res) => { + var testObject = new TestObject(); + testObject.save() + .then(testObject => { + var object = new BeforeSaveObject({remove: testObject}); + return object.save(); + }) + .then(object => { + object.unset('remove'); + return object.save(); + }) + .then(object => { + res.success(object); + }); + }); + + Parse.Cloud.run('removeme') + .then(aNoBeforeSaveObj => { + expect(aNoBeforeSaveObj.get('remove')).toEqual(undefined); + + return Parse.Cloud.run('removeme2'); + }) + .then(aBeforeSaveObj => { + expect(aBeforeSaveObj.get('before')).toEqual('save'); + expect(aBeforeSaveObj.get('remove')).toEqual(undefined); + done(); + }); + }); }); From eb9ee1a6cca2d662d434d0ef979e30cb6bb6edb4 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Sun, 29 May 2016 09:23:33 -0400 Subject: [PATCH 2/8] Fixes #1840 --- src/RestWrite.js | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/RestWrite.js b/src/RestWrite.js index cb438545f8..3ba404bc44 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -763,9 +763,7 @@ RestWrite.prototype.runDatabaseOperation = function() { .then(response => { response.updatedAt = this.updatedAt; if (this.storage.changedByTrigger) { - Object.keys(this.data).forEach(fieldName => { - response[fieldName] = response[fieldName] || this.data[fieldName]; - }); + updateResponseWithData(response, this.data); } this.response = { response }; }); @@ -823,9 +821,7 @@ RestWrite.prototype.runDatabaseOperation = function() { response.username = this.data.username; } if (this.storage.changedByTrigger) { - Object.keys(this.data).forEach(fieldName => { - response[fieldName] = response[fieldName] || this.data[fieldName]; - }); + updateResponseWithData(response, this.data); } this.response = { status: 201, @@ -914,5 +910,18 @@ RestWrite.prototype.cleanUserAuthData = function() { } }; +function updateResponseWithData(response, data) { + Object.keys(data).forEach(fieldName => { + let dataValue = data[fieldName]; + let responseValue = response[fieldName]; + if (dataValue && dataValue.__op === 'Delete') { + delete response[fieldName]; + } else { + response[fieldName] = responseValue || dataValue; + } + }); + return response; +} + export default RestWrite; module.exports = RestWrite; From ab0b719c99d396dfdbf26f256f2a5aa90963f2be Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Sun, 29 May 2016 10:50:36 -0400 Subject: [PATCH 3/8] Adds failing test with other use case - That test fails on parse.com as well --- spec/CloudCode.spec.js | 29 +++++++++++++++++++++++++++++ src/RestWrite.js | 2 +- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 26e864b1bb..4a65b53e10 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -722,4 +722,33 @@ describe('Cloud Code', () => { done(); }); }); + + it('should fully delete objects when using `unset` with beforeSave (regression test for #1840)', done => { + var TestObject = Parse.Object.extend('TestObject'); + var BeforeSaveObject = Parse.Object.extend('BeforeSaveChanged'); + + Parse.Cloud.beforeSave('BeforeSaveChanged', (req, res) => { + var object = req.object; + object.set('before', 'save'); + object.unset('remove'); + res.success(); + }); + + let object; + let testObject = new TestObject({key: 'value'}); + testObject.save().then(() => { + object = new BeforeSaveObject(); + return object.save().then(() => { + object.set({remove:testObject}) + return object.save(); + }); + }).then((objectAgain) => { + expect(objectAgain.get('remove')).toBeUndefined(); + expect(object.get('remove')).toBeUndefined(); + done(); + }).fail((err) => { + console.error(err); + done(); + }) + }); }); diff --git a/src/RestWrite.js b/src/RestWrite.js index 3ba404bc44..09326d2a62 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -915,7 +915,7 @@ function updateResponseWithData(response, data) { let dataValue = data[fieldName]; let responseValue = response[fieldName]; if (dataValue && dataValue.__op === 'Delete') { - delete response[fieldName]; + response[fieldName] = undefined; } else { response[fieldName] = responseValue || dataValue; } From 2e0fbfb39496729ebcbb0e6f7d09b1bd8ed09a05 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Tue, 31 May 2016 11:48:41 -0400 Subject: [PATCH 4/8] Bumps parse to 1.9.0 --- src/RestWrite.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/RestWrite.js b/src/RestWrite.js index 09326d2a62..cb14409b0a 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -914,11 +914,7 @@ function updateResponseWithData(response, data) { Object.keys(data).forEach(fieldName => { let dataValue = data[fieldName]; let responseValue = response[fieldName]; - if (dataValue && dataValue.__op === 'Delete') { - response[fieldName] = undefined; - } else { - response[fieldName] = responseValue || dataValue; - } + response[fieldName] = responseValue || dataValue; }); return response; } From 192e9c3a9b79b8a6786765d10afd91ef7470bf20 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Mon, 11 Jul 2016 19:44:06 -0400 Subject: [PATCH 5/8] exclude pg db --- spec/CloudCode.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 4a65b53e10..d0c2458d17 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -723,7 +723,7 @@ describe('Cloud Code', () => { }); }); - it('should fully delete objects when using `unset` with beforeSave (regression test for #1840)', 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 BeforeSaveObject = Parse.Object.extend('BeforeSaveChanged'); From b7600e1517503d92c995f4254e4905c014e008d4 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Tue, 12 Jul 2016 08:37:34 -0400 Subject: [PATCH 6/8] Exclude pg on other test --- spec/CloudCode.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index d0c2458d17..941dfd53ea 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -667,7 +667,7 @@ describe('Cloud Code', () => { }); }); - it('should fully delete objects when using `unset` with beforeSave (regression test for #1840)', 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'); var BeforeSaveObject = Parse.Object.extend('BeforeSaveChanged'); From 5b4638b57a2e2a53ba2c9282a4f6edcee3e27660 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Tue, 12 Jul 2016 14:54:11 -0400 Subject: [PATCH 7/8] Adds clientSDK compatibility check for forward deletion - Mark js1.9.0 as compatible --- package.json | 1 + spec/ClientSDK.spec.js | 41 ++++++++++++++++++++++++++++++++++++++++ spec/Middlewares.spec.js | 20 -------------------- src/ClientSDK.js | 40 +++++++++++++++++++++++++++++++++++++++ src/RestWrite.js | 14 ++++++++++---- src/middlewares.js | 15 ++------------- 6 files changed, 94 insertions(+), 37 deletions(-) create mode 100644 spec/ClientSDK.spec.js create mode 100644 src/ClientSDK.js diff --git a/package.json b/package.json index 9772f71f31..fa5b241b22 100644 --- a/package.json +++ b/package.json @@ -42,6 +42,7 @@ "redis": "2.6.2", "request": "2.73.0", "request-promise": "3.0.0", + "semver": "^5.2.0", "tv4": "1.2.7", "winston": "2.2.0", "winston-daily-rotate-file": "1.0.1", diff --git a/spec/ClientSDK.spec.js b/spec/ClientSDK.spec.js new file mode 100644 index 0000000000..e714818159 --- /dev/null +++ b/spec/ClientSDK.spec.js @@ -0,0 +1,41 @@ +var ClientSDK = require('../src/ClientSDK'); + +describe('ClientSDK', () => { + it('should properly parse the SDK versions', () => { + let clientSDKFromVersion = ClientSDK.fromString; + expect(clientSDKFromVersion('i1.1.1')).toEqual({ + sdk: 'i', + version: '1.1.1' + }); + expect(clientSDKFromVersion('i1')).toEqual({ + sdk: 'i', + version: '1' + }); + expect(clientSDKFromVersion('apple-tv1.13.0')).toEqual({ + sdk: 'apple-tv', + version: '1.13.0' + }); + expect(clientSDKFromVersion('js1.9.0')).toEqual({ + sdk: 'js', + version: '1.9.0' + }); + }); + + it('should properly sastisfy', () => { + expect(ClientSDK.compatible({ + js: '>=1.9.0' + })("js1.9.0")).toBe(true); + + expect(ClientSDK.compatible({ + js: '>=1.9.0' + })("js2.0.0")).toBe(true); + + expect(ClientSDK.compatible({ + js: '>=1.9.0' + })("js1.8.0")).toBe(false); + + expect(ClientSDK.compatible({ + js: '>=1.9.0' + })(undefined)).toBe(true); + }) +}) \ No newline at end of file diff --git a/spec/Middlewares.spec.js b/spec/Middlewares.spec.js index 643506d8fc..45efc2fd2d 100644 --- a/spec/Middlewares.spec.js +++ b/spec/Middlewares.spec.js @@ -66,24 +66,4 @@ describe('middlewares', () => { }); }); }); - - it('should properly parse the SDK versions', () => { - let clientSDKFromVersion = middlewares.clientSDKFromVersion; - expect(clientSDKFromVersion('i1.1.1')).toEqual({ - sdk: 'i', - version: '1.1.1' - }); - expect(clientSDKFromVersion('i1')).toEqual({ - sdk: 'i', - version: '1' - }); - expect(clientSDKFromVersion('apple-tv1.13.0')).toEqual({ - sdk: 'apple-tv', - version: '1.13.0' - }); - expect(clientSDKFromVersion('js1.9.0')).toEqual({ - sdk: 'js', - version: '1.9.0' - }); - }) }); \ No newline at end of file diff --git a/src/ClientSDK.js b/src/ClientSDK.js new file mode 100644 index 0000000000..4eebf203e5 --- /dev/null +++ b/src/ClientSDK.js @@ -0,0 +1,40 @@ +var semver = require('semver'); + +function compatible(compatibleSDK) { + return function(clientSDK) { + if (typeof clientSDK === 'string') { + clientSDK = fromString(clientSDK); + } + // REST API, or custom SDK + if (!clientSDK) { + return true; + } + let clientVersion = clientSDK.version; + let compatiblityVersion = compatibleSDK[clientSDK.sdk]; + return semver.satisfies(clientVersion, compatiblityVersion); + } +} + +function supportsForwardDelete(clientSDK) { + return compatible({ + js: '>=1.9.0' + })(clientSDK); +} + +function fromString(version) { + let versionRE = /([-a-zA-Z]+)([0-9\.]+)/; + let match = version.toLowerCase().match(versionRE); + if (match && match.length === 3) { + return { + sdk: match[1], + version: match[2] + } + } + return undefined; +} + +module.exports = { + compatible, + supportsForwardDelete, + fromString +} diff --git a/src/RestWrite.js b/src/RestWrite.js index cb14409b0a..6762873d13 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -11,6 +11,7 @@ var cryptoUtils = require('./cryptoUtils'); var passwordCrypto = require('./password'); var Parse = require('parse/node'); var triggers = require('./triggers'); +var ClientSDK = require('./ClientSDK'); import RestQuery from './RestQuery'; import _ from 'lodash'; @@ -763,7 +764,7 @@ RestWrite.prototype.runDatabaseOperation = function() { .then(response => { response.updatedAt = this.updatedAt; if (this.storage.changedByTrigger) { - updateResponseWithData(response, this.data); + this.updateResponseWithData(response, this.data); } this.response = { response }; }); @@ -821,7 +822,7 @@ RestWrite.prototype.runDatabaseOperation = function() { response.username = this.data.username; } if (this.storage.changedByTrigger) { - updateResponseWithData(response, this.data); + this.updateResponseWithData(response, this.data); } this.response = { status: 201, @@ -910,11 +911,16 @@ RestWrite.prototype.cleanUserAuthData = function() { } }; -function updateResponseWithData(response, data) { +RestWrite.prototype.updateResponseWithData = function(response, data) { + let clientSupportsDelete = ClientSDK.supportsForwardDelete(this.clientSDK); Object.keys(data).forEach(fieldName => { let dataValue = data[fieldName]; let responseValue = response[fieldName]; - response[fieldName] = responseValue || dataValue; + if (!clientSupportsDelete && dataValue && dataValue.__op === 'Delete') { + delete response[fieldName]; + } else { + response[fieldName] = responseValue || dataValue; + } }); return response; } diff --git a/src/middlewares.js b/src/middlewares.js index d409906b51..4e64c9ee31 100644 --- a/src/middlewares.js +++ b/src/middlewares.js @@ -5,17 +5,7 @@ var Parse = require('parse/node').Parse; var auth = require('./Auth'); var Config = require('./Config'); - -function clientSDKFromVersion(version) { - let versionRE = /([-a-zA-Z]+)([0-9\.]+)/; - let match = version.toLowerCase().match(versionRE); - if (match && match.length === 3) { - return { - sdk: match[1], - version: match[2] - } - } -} +var ClientSDK = require('./ClientSDK'); // Checks that the request is authorized for this app and checks user // auth too. @@ -106,7 +96,7 @@ function handleParseHeaders(req, res, next) { } if (info.clientVersion) { - info.clientSDK = clientSDKFromVersion(info.clientVersion); + info.clientSDK = ClientSDK.fromString(info.clientVersion); } if (fileViaJSON) { @@ -300,5 +290,4 @@ module.exports = { handleParseHeaders: handleParseHeaders, enforceMasterKeyAccess: enforceMasterKeyAccess, promiseEnforceMasterKeyAccess, - clientSDKFromVersion }; From dfbe60601625c266fa46df63a999af9b8c2ba1dc Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Tue, 12 Jul 2016 18:56:13 -0400 Subject: [PATCH 8/8] Strips all operations from result - fix for #1606 --- spec/CloudCode.spec.js | 25 +++++++++++++++++++++++++ src/RestWrite.js | 11 ++++++++--- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 941dfd53ea..3177462456 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -751,4 +751,29 @@ describe('Cloud Code', () => { done(); }) }); + + it_exclude_dbs(['postgres'])('should not include relation op (regression test for #1606)', done => { + var TestObject = Parse.Object.extend('TestObject'); + var BeforeSaveObject = Parse.Object.extend('BeforeSaveChanged'); + let testObj; + Parse.Cloud.beforeSave('BeforeSaveChanged', (req, res) => { + var object = req.object; + object.set('before', 'save'); + testObj = new TestObject(); + testObj.save().then(() => { + object.relation('testsRelation').add(testObj); + res.success(); + }) + }); + + let object = new BeforeSaveObject(); + object.save().then((objectAgain) => { + // Originally it would throw as it would be a non-relation + expect(() => { objectAgain.relation('testsRelation') }).not.toThrow(); + done(); + }).fail((err) => { + console.error(err); + done(); + }) + }); }); diff --git a/src/RestWrite.js b/src/RestWrite.js index 6762873d13..d512fb6b9c 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -916,10 +916,15 @@ RestWrite.prototype.updateResponseWithData = function(response, data) { Object.keys(data).forEach(fieldName => { let dataValue = data[fieldName]; let responseValue = response[fieldName]; - if (!clientSupportsDelete && dataValue && dataValue.__op === 'Delete') { + + response[fieldName] = responseValue || dataValue; + + // Strips operations from responses + if (response[fieldName] && response[fieldName].__op) { delete response[fieldName]; - } else { - response[fieldName] = responseValue || dataValue; + if (clientSupportsDelete && dataValue.__op == 'Delete') { + response[fieldName] = dataValue; + } } }); return response;