Support for nested .select() calls (#2737)

* Reproduction for #1567 * Recursive handling of nested pointer keys in select * Better support for multi-level nested keys * Adds support for selecting columns natively (mongo) * Support for postgres column selections * Filter-out empty keys for pg
parse-community · Sep 24, 2016 · 9c522be · 9c522be
1 parent 4974dbe
commit 9c522be
Show file tree

Hide file tree

Showing 7 changed files with 133 additions and 29 deletions.
diff --git a/spec/ParseQuery.spec.js b/spec/ParseQuery.spec.js
@@ -2567,6 +2567,76 @@ 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');
+    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('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/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
@@ -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
@@ -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/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,19 @@ export class PostgresStorageAdapter {
       sortPattern = `ORDER BY ${where.sorts.join(',')}`;
     }
 
-    const qs = `SELECT * FROM $1:name ${wherePattern} ${sortPattern} ${limitPattern} ${skipPattern}`;
+    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(',');
+      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/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
@@ -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,7 @@ 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');
@@ -390,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') {
@@ -411,19 +418,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;
@@ -455,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;
@@ -473,7 +467,7 @@ RestQuery.prototype.handleInclude = function() {
 // 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;
@@ -492,9 +486,26 @@ function includePath(config, auth, response, path) {
     }
   }
 
+  let includeRestOptions = {};
+  if (restOptions.keys) {
+    let keys = new Set(restOptions.keys.split(','));
+    let keySet = Array.from(keys).reduce((set, key) => {
+      let keyPath = key.split('.');
+      let i=0;
+      for (i; i<path.length; i++) {
+        if (path[i] != keyPath[i]) {
+          return set;
+        }
+      }
+      set.add(keyPath[i]);
+      return set;
+    }, new Set());
+    includeRestOptions.keys = Array.from(keySet).join(',');
+  }
+
   let queryPromises = Object.keys(pointersHash).map((className) => {
     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);