From 2579bac656fe3be1f557cf7654d258d36eaf2a83 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Sun, 18 Sep 2016 19:42:35 -0400 Subject: [PATCH 1/6] Reproduction for #1567 --- spec/ParseQuery.spec.js | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/spec/ParseQuery.spec.js b/spec/ParseQuery.spec.js index ff728dee4d..dc902fc717 100644 --- a/spec/ParseQuery.spec.js +++ b/spec/ParseQuery.spec.js @@ -2567,6 +2567,35 @@ describe('Parse.Query testing', () => { }) }); + it('select nested keys (issue #1567)', function(done) { + var BarBaz = new Parse.Object('Barbaz'); + BarBaz.set('key', 'value'); + BarBaz.set('otherKey', 'value'); + var Foobar = new Parse.Object('Foobar'); + Foobar.set('foo', 'bar'); + Foobar.set('fizz', 'buzz'); + Foobar.set('barBaz', BarBaz); + Foobar.save({ + success: function(savedFoobar){ + var foobarQuery = new Parse.Query('Foobar'); + foobarQuery.select(['fizz', 'barBaz.key']); + foobarQuery.get(savedFoobar.id,{ + success: function(foobarObj){ + equal(foobarObj.get('fizz'), 'buzz'); + equal(foobarObj.get('foo'), undefined); + if (foobarObj.has('barBaz')) { + equal(foobarObj.get('barBaz').get('key'), 'value'); + equal(foobarObj.get('barBaz').get('otherKey'), undefined); + } else { + fail('barBaz should be set'); + } + done(); + } + }); + } + }) + }); + it('properly handles nested ors', function(done) { var objects = []; while(objects.length != 4) { From d1822862ea90fb82beca281fe4078b662399a050 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Sun, 18 Sep 2016 20:29:23 -0400 Subject: [PATCH 2/6] Recursive handling of nested pointer keys in select --- spec/ParseQuery.spec.js | 45 +++++++++++++++++----------------- src/RestQuery.js | 54 +++++++++++++++++++++++++++++------------ 2 files changed, 61 insertions(+), 38 deletions(-) diff --git a/spec/ParseQuery.spec.js b/spec/ParseQuery.spec.js index dc902fc717..5a90fb4390 100644 --- a/spec/ParseQuery.spec.js +++ b/spec/ParseQuery.spec.js @@ -2568,32 +2568,33 @@ describe('Parse.Query testing', () => { }); it('select nested keys (issue #1567)', function(done) { + var Foobar = new Parse.Object('Foobar'); var BarBaz = new Parse.Object('Barbaz'); BarBaz.set('key', 'value'); BarBaz.set('otherKey', 'value'); - var Foobar = new Parse.Object('Foobar'); - Foobar.set('foo', 'bar'); - Foobar.set('fizz', 'buzz'); - Foobar.set('barBaz', BarBaz); - Foobar.save({ - success: function(savedFoobar){ - var foobarQuery = new Parse.Query('Foobar'); - foobarQuery.select(['fizz', 'barBaz.key']); - foobarQuery.get(savedFoobar.id,{ - success: function(foobarObj){ - equal(foobarObj.get('fizz'), 'buzz'); - equal(foobarObj.get('foo'), undefined); - if (foobarObj.has('barBaz')) { - equal(foobarObj.get('barBaz').get('key'), 'value'); - equal(foobarObj.get('barBaz').get('otherKey'), undefined); - } else { - fail('barBaz should be set'); - } - done(); + BarBaz.save().then(() => { + Foobar.set('foo', 'bar'); + Foobar.set('fizz', 'buzz'); + Foobar.set('barBaz', BarBaz); + return Foobar.save(); + }).then(function(savedFoobar){ + var foobarQuery = new Parse.Query('Foobar'); + foobarQuery.include('barBaz'); + foobarQuery.select(['fizz', 'barBaz.key']); + foobarQuery.get(savedFoobar.id,{ + success: function(foobarObj){ + equal(foobarObj.get('fizz'), 'buzz'); + equal(foobarObj.get('foo'), undefined); + if (foobarObj.has('barBaz')) { + equal(foobarObj.get('barBaz').get('key'), 'value'); + equal(foobarObj.get('barBaz').get('otherKey'), undefined); + } else { + fail('barBaz should be set'); } - }); - } - }) + done(); + } + }); + }); }); it('properly handles nested ors', function(done) { diff --git a/src/RestQuery.js b/src/RestQuery.js index 203a823470..25b6ab9e60 100644 --- a/src/RestQuery.js +++ b/src/RestQuery.js @@ -56,9 +56,6 @@ function RestQuery(config, auth, className, restWhere = {}, restOptions = {}, cl switch(option) { case 'keys': this.keys = new Set(restOptions.keys.split(',')); - this.keys.add('objectId'); - this.keys.add('createdAt'); - this.keys.add('updatedAt'); break; case 'count': this.doCount = true; @@ -120,6 +117,8 @@ RestQuery.prototype.execute = function() { return this.runCount(); }).then(() => { return this.handleInclude(); + }).then(() => { + return this.handleKeys(); }).then(() => { return this.response; }); @@ -411,19 +410,6 @@ RestQuery.prototype.runFind = function() { this.config.filesController.expandFilesInObject(this.config, results); - if (this.keys) { - var keySet = this.keys; - results = results.map((object) => { - var newObject = {}; - for (var key in object) { - if (keySet.has(key)) { - newObject[key] = object[key]; - } - } - return newObject; - }); - } - if (this.redirectClassName) { for (var r of results) { r.className = this.redirectClassName; @@ -470,6 +456,42 @@ RestQuery.prototype.handleInclude = function() { return pathResponse; }; +RestQuery.prototype.handleKeys = function() { + if (this.keys && Array.isArray(this.response.results)) { + this.response.results = this.response.results.map((object) => { + return includeOnlyKeySet(object, this.keys); + }); + } +} + +// Recursive call to include keys when resolving objects +function includeOnlyKeySet(object, keySet) { + // No additional filtering, return the full object + if (!keySet || keySet.size === 0 || !object) { + return object; + } + // Make a copy... + keySet = new Set(keySet); + // Add the default + keySet.add('objectId'); + keySet.add('createdAt'); + keySet.add('updatedAt'); + // Preserve the __type if needed + if (object.__type) { + keySet.add('__type'); + if (object.__type === 'Object') { + keySet.add('className'); + } + } + let keyArray = Array.from(keySet); + return keyArray.reduce((newObject, originalKey) => { + let subkeys = originalKey.split('.'); + let key = subkeys.shift(); + newObject[key] = includeOnlyKeySet(object[key], new Set(subkeys)); + return newObject; + }, {}); +} + // Adds included values to the response. // Path is a list of field names. // Returns a promise for an augmented response. From 60cd3f1d77e3e08f8deb07dadeae824917a57e98 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Sun, 18 Sep 2016 20:48:09 -0400 Subject: [PATCH 3/6] Better support for multi-level nested keys --- spec/ParseQuery.spec.js | 40 ++++++++++++++++++++++++++++++++++++++++ src/RestQuery.js | 27 ++++++++++++++------------- 2 files changed, 54 insertions(+), 13 deletions(-) diff --git a/spec/ParseQuery.spec.js b/spec/ParseQuery.spec.js index 5a90fb4390..d54efdc3f6 100644 --- a/spec/ParseQuery.spec.js +++ b/spec/ParseQuery.spec.js @@ -2597,6 +2597,46 @@ describe('Parse.Query testing', () => { }); }); + it('select nested keys 2 level (issue #1567)', function(done) { + var Foobar = new Parse.Object('Foobar'); + var BarBaz = new Parse.Object('Barbaz'); + var Bazoo = new Parse.Object('Bazoo'); + + Bazoo.set('some', 'thing'); + Bazoo.set('otherSome', 'value'); + Bazoo.save().then(() => { + BarBaz.set('key', 'value'); + BarBaz.set('otherKey', 'value'); + BarBaz.set('bazoo', Bazoo); + return BarBaz.save(); + }).then(() => { + Foobar.set('foo', 'bar'); + Foobar.set('fizz', 'buzz'); + Foobar.set('barBaz', BarBaz); + return Foobar.save(); + }).then(function(savedFoobar){ + var foobarQuery = new Parse.Query('Foobar'); + foobarQuery.include('barBaz'); + foobarQuery.include('barBaz.bazoo'); + foobarQuery.select(['fizz', 'barBaz.key', 'barBaz.bazoo.some']); + foobarQuery.get(savedFoobar.id,{ + success: function(foobarObj){ + equal(foobarObj.get('fizz'), 'buzz'); + equal(foobarObj.get('foo'), undefined); + if (foobarObj.has('barBaz')) { + equal(foobarObj.get('barBaz').get('key'), 'value'); + equal(foobarObj.get('barBaz').get('otherKey'), undefined); + equal(foobarObj.get('barBaz').get('bazoo').get('some'), 'thing'); + equal(foobarObj.get('barBaz').get('bazoo').get('otherSome'), undefined); + } else { + fail('barBaz should be set'); + } + done(); + } + }); + }); + }); + it('properly handles nested ors', function(done) { var objects = []; while(objects.length != 4) { diff --git a/src/RestQuery.js b/src/RestQuery.js index 25b6ab9e60..a0f41dbacb 100644 --- a/src/RestQuery.js +++ b/src/RestQuery.js @@ -459,37 +459,38 @@ RestQuery.prototype.handleInclude = function() { RestQuery.prototype.handleKeys = function() { if (this.keys && Array.isArray(this.response.results)) { this.response.results = this.response.results.map((object) => { - return includeOnlyKeySet(object, this.keys); + return includeOnlyKeys(object, Array.from(this.keys)); }); } } // Recursive call to include keys when resolving objects -function includeOnlyKeySet(object, keySet) { +function includeOnlyKeys(object, keys, currentObject = {}) { // No additional filtering, return the full object - if (!keySet || keySet.size === 0 || !object) { + if (!keys || keys.length === 0 || !object) { return object; } // Make a copy... - keySet = new Set(keySet); + keys = Array.from(keys); // Add the default - keySet.add('objectId'); - keySet.add('createdAt'); - keySet.add('updatedAt'); + keys.push('objectId'); + keys.push('createdAt'); + keys.push('updatedAt'); // Preserve the __type if needed if (object.__type) { - keySet.add('__type'); + keys.push('__type'); if (object.__type === 'Object') { - keySet.add('className'); + keys.push('className'); } } - let keyArray = Array.from(keySet); - return keyArray.reduce((newObject, originalKey) => { + + return keys.reduce((newObject, originalKey) => { let subkeys = originalKey.split('.'); let key = subkeys.shift(); - newObject[key] = includeOnlyKeySet(object[key], new Set(subkeys)); + let nextSubkeys = subkeys.length == 0 ? [] : [subkeys.join('.')]; + newObject[key] = includeOnlyKeys(object[key], nextSubkeys, newObject[key]); return newObject; - }, {}); + }, currentObject); } // Adds included values to the response. From 66d03e86fc63185b0f40f2a1f0348f36255086b4 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Sun, 18 Sep 2016 22:56:27 -0400 Subject: [PATCH 4/6] Adds support for selecting columns natively (mongo) --- src/Adapters/Storage/Mongo/MongoCollection.js | 16 +++-- .../Storage/Mongo/MongoStorageAdapter.js | 8 ++- src/Adapters/Storage/Mongo/MongoTransform.js | 4 +- src/Controllers/DatabaseController.js | 3 +- src/RestQuery.js | 71 ++++++++----------- 5 files changed, 50 insertions(+), 52 deletions(-) diff --git a/src/Adapters/Storage/Mongo/MongoCollection.js b/src/Adapters/Storage/Mongo/MongoCollection.js index 757be2091a..24d1397f83 100644 --- a/src/Adapters/Storage/Mongo/MongoCollection.js +++ b/src/Adapters/Storage/Mongo/MongoCollection.js @@ -13,8 +13,8 @@ export default class MongoCollection { // none, then build the geoindex. // This could be improved a lot but it's not clear if that's a good // idea. Or even if this behavior is a good idea. - find(query, { skip, limit, sort } = {}) { - return this._rawFind(query, { skip, limit, sort }) + find(query, { skip, limit, sort, keys } = {}) { + return this._rawFind(query, { skip, limit, sort, keys }) .catch(error => { // Check for "no geoindex" error if (error.code != 17007 && !error.message.match(/unable to find index for .geoNear/)) { @@ -30,14 +30,18 @@ export default class MongoCollection { index[key] = '2d'; return this._mongoCollection.createIndex(index) // Retry, but just once. - .then(() => this._rawFind(query, { skip, limit, sort })); + .then(() => this._rawFind(query, { skip, limit, sort, keys })); }); } - _rawFind(query, { skip, limit, sort } = {}) { - return this._mongoCollection + _rawFind(query, { skip, limit, sort, keys } = {}) { + let findOperation = this._mongoCollection .find(query, { skip, limit, sort }) - .toArray(); + + if (keys) { + findOperation = findOperation.project(keys); + } + return findOperation.toArray(); } count(query, { skip, limit, sort } = {}) { diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index 7fb426c55c..05231fc5bb 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -320,12 +320,16 @@ export class MongoStorageAdapter { } // Executes a find. Accepts: className, query in Parse format, and { skip, limit, sort }. - find(className, schema, query, { skip, limit, sort }) { + find(className, schema, query, { skip, limit, sort, keys }) { schema = convertParseSchemaToMongoSchema(schema); let mongoWhere = transformWhere(className, query, schema); let mongoSort = _.mapKeys(sort, (value, fieldName) => transformKey(className, fieldName, schema)); + let mongoKeys = _.reduce(keys, (memo, key) => { + memo[transformKey(className, key, schema)] = 1; + return memo; + }, {}); return this._adaptiveCollection(className) - .then(collection => collection.find(mongoWhere, { skip, limit, sort: mongoSort })) + .then(collection => collection.find(mongoWhere, { skip, limit, sort: mongoSort, keys: mongoKeys })) .then(objects => objects.map(object => mongoObjectToParseObject(className, object, schema))) } diff --git a/src/Adapters/Storage/Mongo/MongoTransform.js b/src/Adapters/Storage/Mongo/MongoTransform.js index 5f043252ed..6aadb67406 100644 --- a/src/Adapters/Storage/Mongo/MongoTransform.js +++ b/src/Adapters/Storage/Mongo/MongoTransform.js @@ -11,9 +11,11 @@ const transformKey = (className, fieldName, schema) => { case 'updatedAt': return '_updated_at'; case 'sessionToken': return '_session_token'; } - + if (schema.fields[fieldName] && schema.fields[fieldName].__type == 'Pointer') { fieldName = '_p_' + fieldName; + } else if (schema.fields[fieldName] && schema.fields[fieldName].type == 'Pointer') { + fieldName = '_p_' + fieldName; } return fieldName; diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index 618d304795..8d50d14c5c 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -711,6 +711,7 @@ DatabaseController.prototype.find = function(className, query, { acl, sort = {}, count, + keys } = {}) { let isMaster = acl === undefined; let aclGroup = acl || []; @@ -779,7 +780,7 @@ DatabaseController.prototype.find = function(className, query, { if (!classExists) { return []; } else { - return this.adapter.find(className, schema, query, { skip, limit, sort }) + return this.adapter.find(className, schema, query, { skip, limit, sort, keys }) .then(objects => objects.map(object => { object = untransformObjectACL(object); return filterSensitiveData(isMaster, aclGroup, className, object) diff --git a/src/RestQuery.js b/src/RestQuery.js index a0f41dbacb..933949c5f4 100644 --- a/src/RestQuery.js +++ b/src/RestQuery.js @@ -20,6 +20,7 @@ function RestQuery(config, auth, className, restWhere = {}, restOptions = {}, cl this.auth = auth; this.className = className; this.restWhere = restWhere; + this.restOptions = restOptions; this.clientSDK = clientSDK; this.response = null; this.findOptions = {}; @@ -56,6 +57,10 @@ function RestQuery(config, auth, className, restWhere = {}, restOptions = {}, cl switch(option) { case 'keys': this.keys = new Set(restOptions.keys.split(',')); + // Add the default + this.keys.add('objectId'); + this.keys.add('createdAt'); + this.keys.add('updatedAt'); break; case 'count': this.doCount = true; @@ -117,8 +122,6 @@ RestQuery.prototype.execute = function() { return this.runCount(); }).then(() => { return this.handleInclude(); - }).then(() => { - return this.handleKeys(); }).then(() => { return this.response; }); @@ -389,6 +392,11 @@ RestQuery.prototype.runFind = function() { this.response = {results: []}; return Promise.resolve(); } + if (this.keys) { + this.findOptions.keys = Array.from(this.keys).map((key) => { + return key.split('.')[0]; + }); + } return this.config.database.find( this.className, this.restWhere, this.findOptions).then((results) => { if (this.className === '_User') { @@ -441,7 +449,7 @@ RestQuery.prototype.handleInclude = function() { } var pathResponse = includePath(this.config, this.auth, - this.response, this.include[0]); + this.response, this.include[0], this.restOptions); if (pathResponse.then) { return pathResponse.then((newResponse) => { this.response = newResponse; @@ -456,47 +464,10 @@ RestQuery.prototype.handleInclude = function() { return pathResponse; }; -RestQuery.prototype.handleKeys = function() { - if (this.keys && Array.isArray(this.response.results)) { - this.response.results = this.response.results.map((object) => { - return includeOnlyKeys(object, Array.from(this.keys)); - }); - } -} - -// Recursive call to include keys when resolving objects -function includeOnlyKeys(object, keys, currentObject = {}) { - // No additional filtering, return the full object - if (!keys || keys.length === 0 || !object) { - return object; - } - // Make a copy... - keys = Array.from(keys); - // Add the default - keys.push('objectId'); - keys.push('createdAt'); - keys.push('updatedAt'); - // Preserve the __type if needed - if (object.__type) { - keys.push('__type'); - if (object.__type === 'Object') { - keys.push('className'); - } - } - - return keys.reduce((newObject, originalKey) => { - let subkeys = originalKey.split('.'); - let key = subkeys.shift(); - let nextSubkeys = subkeys.length == 0 ? [] : [subkeys.join('.')]; - newObject[key] = includeOnlyKeys(object[key], nextSubkeys, newObject[key]); - return newObject; - }, currentObject); -} - // Adds included values to the response. // Path is a list of field names. // Returns a promise for an augmented response. -function includePath(config, auth, response, path) { +function includePath(config, auth, response, path, restOptions = {}) { var pointers = findPointers(response.results, path); if (pointers.length == 0) { return response; @@ -515,9 +486,25 @@ function includePath(config, auth, response, path) { } } + let includeRestOptions = {}; + if (restOptions.keys) { + let keys = new Set(restOptions.keys.split(',')); + includeRestOptions.keys = Array.from(keys).reduce((memo, key) => { + let keyPath = key.split('.'); + let i=0; + for (i; i { var where = {'objectId': {'$in': pointersHash[className]}}; - var query = new RestQuery(config, auth, className, where); + var query = new RestQuery(config, auth, className, where, includeRestOptions); return query.execute().then((results) => { results.className = className; return Promise.resolve(results); From 0f3e3af8a8a21d1d5ba632f3fb38f86f94ef8f6e Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Sun, 18 Sep 2016 23:09:27 -0400 Subject: [PATCH 5/6] Support for postgres column selections --- .../Storage/Postgres/PostgresStorageAdapter.js | 14 +++++++++++--- src/RestQuery.js | 11 ++++++----- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js index c364e3c6f1..1b18f786e6 100644 --- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js +++ b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js @@ -921,8 +921,8 @@ export class PostgresStorageAdapter { }); } - find(className, schema, query, { skip, limit, sort }) { - debug('find', className, query, {skip, limit, sort}); + find(className, schema, query, { skip, limit, sort, keys }) { + debug('find', className, query, {skip, limit, sort, keys }); const hasLimit = limit !== undefined; const hasSkip = skip !== undefined; let values = [className]; @@ -954,7 +954,15 @@ export class PostgresStorageAdapter { sortPattern = `ORDER BY ${where.sorts.join(',')}`; } - const qs = `SELECT * FROM $1:name ${wherePattern} ${sortPattern} ${limitPattern} ${skipPattern}`; + let columns = '*'; + if (keys) { + columns = keys.map((key, index) => { + return `$${index+values.length+1}:name`; + }).join(','); + values = values.concat(keys); + } + + const qs = `SELECT ${columns} FROM $1:name ${wherePattern} ${sortPattern} ${limitPattern} ${skipPattern}`; debug(qs, values); return this._client.any(qs, values) .catch((err) => { diff --git a/src/RestQuery.js b/src/RestQuery.js index 933949c5f4..14f07c056c 100644 --- a/src/RestQuery.js +++ b/src/RestQuery.js @@ -489,17 +489,18 @@ function includePath(config, auth, response, path, restOptions = {}) { let includeRestOptions = {}; if (restOptions.keys) { let keys = new Set(restOptions.keys.split(',')); - includeRestOptions.keys = Array.from(keys).reduce((memo, key) => { + let keySet = Array.from(keys).reduce((set, key) => { let keyPath = key.split('.'); let i=0; for (i; i { From ec0e5b44feab233c4d4647554f686800f3de9118 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Sun, 18 Sep 2016 23:25:17 -0400 Subject: [PATCH 6/6] Filter-out empty keys for pg --- src/Adapters/Storage/Postgres/PostgresStorageAdapter.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js index 1b18f786e6..0c3380d8e6 100644 --- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js +++ b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js @@ -956,6 +956,10 @@ export class PostgresStorageAdapter { let columns = '*'; if (keys) { + // Exclude empty keys + keys = keys.filter((key) => { + return key.length > 0; + }); columns = keys.map((key, index) => { return `$${index+values.length+1}:name`; }).join(',');