From caf4c65753d098c7822582303cbb575c3ed66233 Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Thu, 9 Jun 2016 22:01:14 -0700 Subject: [PATCH 1/2] Move ACL transforming into Parse Server For the database adapters, it will be more performant and easier to work with _rperm and _wperm than with the ACL object. This way we can type it as an array and so on, and once we have stronger validations in Parse Server, we can type it as an array containing strings of length < x, which will be much much better in sql databases. --- spec/MongoStorageAdapter.spec.js | 2 +- spec/MongoTransform.spec.js | 35 ++---- src/Adapters/Storage/Mongo/MongoCollection.js | 12 -- .../Storage/Mongo/MongoStorageAdapter.js | 6 +- src/Adapters/Storage/Mongo/MongoTransform.js | 112 +++++++----------- src/Controllers/DatabaseController.js | 53 ++++++++- 6 files changed, 107 insertions(+), 113 deletions(-) diff --git a/spec/MongoStorageAdapter.spec.js b/spec/MongoStorageAdapter.spec.js index fad763ca26..069e0166bf 100644 --- a/spec/MongoStorageAdapter.spec.js +++ b/spec/MongoStorageAdapter.spec.js @@ -54,7 +54,7 @@ describe('MongoStorageAdapter', () => { .then(results => { expect(results.length).toEqual(1); var obj = results[0]; - expect(typeof obj._id).toEqual('string'); + expect(obj._id).toEqual('abcde'); expect(obj.objectId).toBeUndefined(); done(); }); diff --git a/spec/MongoTransform.spec.js b/spec/MongoTransform.spec.js index cb5baf7758..46851e7355 100644 --- a/spec/MongoTransform.spec.js +++ b/spec/MongoTransform.spec.js @@ -49,10 +49,9 @@ describe('parseObjectToMongoObjectForCreate', () => { done(); }); - it('basic ACL', (done) => { + it('Doesnt allow ACL, as Parse Server should tranform ACL to _wperm + _rperm', done => { var input = {ACL: {'0123': {'read': true, 'write': true}}}; - var output = transform.parseObjectToMongoObjectForCreate(null, input, { fields: {} }); - // This just checks that it doesn't crash, but it should check format. + expect(() => transform.parseObjectToMongoObjectForCreate(null, input, { fields: {} })).toThrow(); done(); }); @@ -211,28 +210,10 @@ describe('transform schema key changes', () => { done(); }); - it('changes ACL storage to _rperm and _wperm', (done) => { - var input = { - ACL: { - "*": { "read": true }, - "Kevin": { "write": true } - } - }; - var output = transform.parseObjectToMongoObjectForCreate(null, input, { fields: {} }); - expect(typeof output._rperm).toEqual('object'); - expect(typeof output._wperm).toEqual('object'); - expect(output.ACL).toBeUndefined(); - expect(output._rperm[0]).toEqual('*'); - expect(output._wperm[0]).toEqual('Kevin'); - done(); - }); - it('writes the old ACL format in addition to rperm and wperm', (done) => { var input = { - ACL: { - "*": { "read": true }, - "Kevin": { "write": true } - } + _rperm: ['*'], + _wperm: ['Kevin'], }; var output = transform.parseObjectToMongoObjectForCreate(null, input, { fields: {} }); @@ -248,11 +229,9 @@ describe('transform schema key changes', () => { _wperm: ["Kevin"] }; var output = transform.mongoObjectToParseObject(null, input, { fields: {} }); - expect(typeof output.ACL).toEqual('object'); - expect(output._rperm).toBeUndefined(); - expect(output._wperm).toBeUndefined(); - expect(output.ACL['*']['read']).toEqual(true); - expect(output.ACL['Kevin']['write']).toEqual(true); + expect(output._rperm).toEqual(['*']); + expect(output._wperm).toEqual(['Kevin']); + expect(output.ACL).toBeUndefined() done(); }); diff --git a/src/Adapters/Storage/Mongo/MongoCollection.js b/src/Adapters/Storage/Mongo/MongoCollection.js index bf41582b19..2c080353e1 100644 --- a/src/Adapters/Storage/Mongo/MongoCollection.js +++ b/src/Adapters/Storage/Mongo/MongoCollection.js @@ -44,18 +44,6 @@ export default class MongoCollection { return this._mongoCollection.count(query, { skip, limit, sort }); } - // Atomically finds and updates an object based on query. - // The result is the promise with an object that was in the database !AFTER! changes. - // Postgres Note: Translates directly to `UPDATE * SET * ... RETURNING *`, which will return data after the change is done. - findOneAndUpdate(query, update) { - // arguments: query, sort, update, options(optional) - // Setting `new` option to true makes it return the after document, not the before one. - return this._mongoCollection.findAndModify(query, [], update, { new: true }).then(document => { - // Value is the object where mongo returns multiple fields. - return document.value; - }); - } - insertOne(object) { return this._mongoCollection.insertOne(object); } diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index 8c95c33879..56cb10f4c2 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -211,12 +211,14 @@ export class MongoStorageAdapter { .then(collection => collection.updateMany(mongoWhere, mongoUpdate)); } - // Hopefully we can get rid of this in favor of updateObjectsByQuery. + // Atomically finds and updates an object based on query. + // Resolve with the updated object. findOneAndUpdate(className, query, schema, update) { const mongoUpdate = transformUpdate(className, update, schema); const mongoWhere = transformWhere(className, query, schema); return this.adaptiveCollection(className) - .then(collection => collection.findOneAndUpdate(mongoWhere, mongoUpdate)); + .then(collection => collection._mongoCollection.findAndModify(mongoWhere, [], mongoUpdate, { new: true })) + .then(result => result.value); } // Hopefully we can get rid of this. It's only used for config and hooks. diff --git a/src/Adapters/Storage/Mongo/MongoTransform.js b/src/Adapters/Storage/Mongo/MongoTransform.js index 898c62e00f..a12b3422aa 100644 --- a/src/Adapters/Storage/Mongo/MongoTransform.js +++ b/src/Adapters/Storage/Mongo/MongoTransform.js @@ -273,11 +273,12 @@ const parseObjectKeyValueToMongoObjectKeyValue = (className, restKey, restValue, // Main exposed method to create new objects. // restCreate is the "create" clause in REST API form. -function parseObjectToMongoObjectForCreate(className, restCreate, schema) { +const parseObjectToMongoObjectForCreate = (className, restCreate, schema) => { if (className == '_User') { restCreate = transformAuthData(restCreate); } - var mongoCreate = transformACL(restCreate); + restCreate = addLegacyACL(restCreate); + let mongoCreate = {} for (let restKey in restCreate) { let { key, value } = parseObjectKeyValueToMongoObjectKeyValue( className, @@ -298,18 +299,18 @@ const transformUpdate = (className, restUpdate, parseFormatSchema) => { restUpdate = transformAuthData(restUpdate); } - var mongoUpdate = {}; - var acl = transformACL(restUpdate); - if (acl._rperm || acl._wperm || acl._acl) { - mongoUpdate['$set'] = {}; + let mongoUpdate = {}; + let acl = addLegacyACL(restUpdate)._acl; + if (acl) { + mongoUpdate.$set = {}; if (acl._rperm) { - mongoUpdate['$set']['_rperm'] = acl._rperm; + mongoUpdate.$set._rperm = acl._rperm; } if (acl._wperm) { - mongoUpdate['$set']['_wperm'] = acl._wperm; + mongoUpdate.$set._wperm = acl._wperm; } if (acl._acl) { - mongoUpdate['$set']['_acl'] = acl._acl; + mongoUpdate.$set._acl = acl._acl; } } for (var restKey in restUpdate) { @@ -347,67 +348,35 @@ function transformAuthData(restObject) { return restObject; } -// Transforms a REST API formatted ACL object to our two-field mongo format. -// This mutates the restObject passed in to remove the ACL key. -function transformACL(restObject) { - var output = {}; - if (!restObject['ACL']) { - return output; - } - var acl = restObject['ACL']; - var rperm = []; - var wperm = []; - var _acl = {}; // old format - - for (var entry in acl) { - if (acl[entry].read) { - rperm.push(entry); - _acl[entry] = _acl[entry] || {}; - _acl[entry]['r'] = true; - } - if (acl[entry].write) { - wperm.push(entry); - _acl[entry] = _acl[entry] || {}; - _acl[entry]['w'] = true; - } - } - output._rperm = rperm; - output._wperm = wperm; - output._acl = _acl; - delete restObject.ACL; - return output; -} +// Add the legacy _acl format. +const addLegacyACL = restObject => { + let restObjectCopy = {...restObject}; + let _acl = {}; -// Transforms a mongo format ACL to a REST API format ACL key -// This mutates the mongoObject passed in to remove the _rperm/_wperm keys -function untransformACL(mongoObject) { - var output = {}; - if (!mongoObject['_rperm'] && !mongoObject['_wperm']) { - return output; - } - var acl = {}; - var rperm = mongoObject['_rperm'] || []; - var wperm = mongoObject['_wperm'] || []; - rperm.map((entry) => { - if (!acl[entry]) { - acl[entry] = {read: true}; - } else { - acl[entry]['read'] = true; - } - }); - wperm.map((entry) => { - if (!acl[entry]) { - acl[entry] = {write: true}; - } else { - acl[entry]['write'] = true; - } - }); - output['ACL'] = acl; - delete mongoObject._rperm; - delete mongoObject._wperm; - return output; + if (restObject._wperm) { + restObject._wperm.forEach(entry => { + _acl[entry] = { w: true }; + }); + } + + if (restObject._rperm) { + restObject._rperm.forEach(entry => { + if (!(entry in _acl)) { + _acl[entry] = { r: true }; + } else { + _acl[entry].r = true; + } + }); + } + + if (Object.keys(_acl).length > 0) { + restObjectCopy._acl = _acl; + } + + return restObjectCopy; } + // A sentinel value that helper transformations return when they // cannot perform a transformation function CannotTransform() {} @@ -752,7 +721,14 @@ const mongoObjectToParseObject = (className, mongoObject, schema) => { return BytesCoder.databaseToJSON(mongoObject); } - var restObject = untransformACL(mongoObject); + let restObject = {}; + if (mongoObject._rperm || mongoObject._wperm) { + restObject._rperm = mongoObject._rperm || []; + restObject._wperm = mongoObject._wperm || []; + delete mongoObject._rperm; + delete mongoObject._wperm; + } + for (var key in mongoObject) { switch(key) { case '_id': diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index de9aa5955c..9f481267ac 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -24,6 +24,27 @@ function addReadACL(query, acl) { return newQuery; } +// Transforms a REST API formatted ACL object to our two-field mongo format. +const transformObjectACL = restObject => { + let { ACL, ...result } = restObject; + if (!ACL) { + return result; + } + + result._wperm = []; + result._rperm = []; + + for (let entry in ACL) { + if (ACL[entry].read) { + result._rperm.push(entry); + } + if (ACL[entry].write) { + result._wperm.push(entry); + } + } + return result; +} + const specialQuerykeys = ['$and', '$or', '_rperm', '_wperm', '_perishable_token', '_email_verify_token']; const validateQuery = query => { if (query.ACL) { @@ -218,6 +239,7 @@ DatabaseController.prototype.update = function(className, query, update, { throw new Parse.Error(Parse.Error.INVALID_NESTED_KEY, "Nested keys should not contain the '$' or '.' characters"); } } + update = transformObjectACL(update); if (many) { return this.adapter.updateObjectsByQuery(className, query, schema, update); } else if (upsert) { @@ -384,7 +406,7 @@ DatabaseController.prototype.destroy = function(className, query, { acl } = {}) DatabaseController.prototype.create = function(className, object, { acl } = {}) { // Make a copy of the object, so we don't mutate the incoming data. let originalObject = object; - object = deepcopy(object); + object = transformObjectACL(object); var isMaster = acl === undefined; var aclGroup = acl || []; @@ -680,13 +702,40 @@ DatabaseController.prototype.find = function(className, query, { return this.adapter.count(className, query, schema); } else { return this.adapter.find(className, query, schema, { skip, limit, sort }) - .then(objects => objects.map(object => filterSensitiveData(isMaster, aclGroup, className, object))); + .then(objects => objects.map(object => { + object = untransformObjectACL(object); + return filterSensitiveData(isMaster, aclGroup, className, object) + })); } }); }); }); }; +// Transforms a Database format ACL to a REST API format ACL +const untransformObjectACL = ({_rperm, _wperm, ...output}) => { + if (_rperm || _wperm) { + output.ACL = {}; + + (_rperm || []).forEach(entry => { + if (!output.ACL[entry]) { + output.ACL[entry] = { read: true }; + } else { + output.ACL[entry]['read'] = true; + } + }); + + (_wperm || []).forEach(entry => { + if (!output.ACL[entry]) { + output.ACL[entry] = { write: true }; + } else { + output.ACL[entry]['write'] = true; + } + }); + } + return output; +} + DatabaseController.prototype.deleteSchema = function(className) { return this.collectionExists(className) .then(exist => { From 3acc1e74237b080511beceef2c535722a99d213a Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Thu, 9 Jun 2016 22:16:26 -0700 Subject: [PATCH 2/2] Use destructuring --- src/Controllers/DatabaseController.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index 9f481267ac..c1b663b1cc 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -25,8 +25,7 @@ function addReadACL(query, acl) { } // Transforms a REST API formatted ACL object to our two-field mongo format. -const transformObjectACL = restObject => { - let { ACL, ...result } = restObject; +const transformObjectACL = ({ ACL, ...result }) => { if (!ACL) { return result; }