Parallelize loading include objects

spec/ParseQuery.spec.js

@@ -2439,6 +2439,27 @@ describe('Parse.Query testing', () => {
     });
   });
 
+  it('handles nested includes with no results', async () => {
+    const obj = new TestObject({ foo: 'bar' });
+    await obj.save();
+
+    const query = new Parse.Query(TestObject)
+      .equalTo('objectId', obj.id)
+      .include('fake.path');
+
+    // This query with should not reject
+    const results = await query.find();
+
+    // ... and we should get back the one object we saved
+    expect(results.length).toBe(1);
+
+    // ... and it should not add the `fake` attribute from the query that
+    // doesn't exist on the object
+    expect(
+      Object.prototype.hasOwnProperty.call(results[0].attributes, 'fake')
+    ).toBeFalse();
+  });
+
   it("include doesn't make dirty wrong", function(done) {
     const Parent = Parse.Object.extend('ParentObject');
     const Child = Parse.Object.extend('ChildObject');

src/RestQuery.js

@@ -747,30 +747,61 @@ RestQuery.prototype.handleExcludeKeys = function() {
 };
 
 // Augments this.response with data at the paths provided in this.include.
+//
+// Preconditions:
+// - `this.include` is an array of arrays of strings; (in flow parlance, Array<Array<string>>)
+//
+// - `this.include` is de-duplicated. This ensures that we don't try to fetch
+//   the same objects twice.
+//
+// - For each value in `this.include` with length > 1, there is also
+//   an earlier value for the prefix of that value.
+//
+//   Example: ['a', 'b', 'c'] in the array implies that ['a', 'b'] is also in
+//   the array, at an earlier position).
+//
+//   This prevents trying to follow pointers on unfetched objects.
 RestQuery.prototype.handleInclude = function() {
   if (this.include.length == 0) {
     return;
   }
 
-  var pathResponse = includePath(
-    this.config,
-    this.auth,
-    this.response,
-    this.include[0],
-    this.restOptions
-  );
-  if (pathResponse.then) {
-    return pathResponse.then(newResponse => {
-      this.response = newResponse;
-      this.include = this.include.slice(1);
-      return this.handleInclude();
-    });
-  } else if (this.include.length > 0) {
-    this.include = this.include.slice(1);
-    return this.handleInclude();
-  }
+  // The list of includes form a sort of a tree - Each path should wait to
+  // start trying to load until its parent path has finished loading (so that
+  // the pointers it is trying to read and fetch are in the object tree).
+  //
+  // So, for instance, if we have an include of ['a', 'b', 'c'], that must
+  // wait on the include of ['a', 'b'] to finish, which must wait on the include
+  // of ['a'] to finish.
+  //
+  // This `promises` object is a map of dotted paths to promises that resolve
+  // when that path has finished loading into the tree. One special case is the
+  // empty path (represented by the empty string). This represents the root of
+  // the tree, which is `this.response` and is already fetched. We set a
+  // pre-resolved promise at that level, meaning that include paths with only
+  // one component (like `['a']`) will chain onto that resolved promise and
+  // are immediately unblocked.
+  const promises = { '': Promise.resolve() };
+
+  this.include.forEach(path => {
+    const dottedPath = path.join('.');
+
+    // Get the promise for the parent path
+    const parentDottedPath = path.slice(0, -1).join('.');
+    const parentPromise = promises[parentDottedPath];
+
+    // Once the parent promise has resolved, do this path's load step
+    const loadPromise = parentPromise.then(() =>
+      includePath(this.config, this.auth, this.response, path, this.restOptions)
+    );
+
+    // Put our promise into the promises map, so child paths can find and chain
+    // off of it
+    promises[dottedPath] = loadPromise;
+  });
 
-  return pathResponse;
+  // Wait for all includes to be fetched and merged in to the response tree
+  return Promise.all(Object.values(promises));
 };
 
 //Returns a promise of a processed set of results
@@ -821,7 +852,10 @@ RestQuery.prototype.runAfterFindTrigger = function() {
 
 // Adds included values to the response.
 // Path is a list of field names.
-// Returns a promise for an augmented response.
+//
+// Modifies the response in place by replacing pointers with fetched objects
+// Returns a promise that resolves when all includes have been fetched and
+// substituted into the response.
 function includePath(config, auth, response, path, restOptions = {}) {
   var pointers = findPointers(response.results, path);
   if (pointers.length == 0) {
@@ -905,19 +939,12 @@ function includePath(config, auth, response, path, restOptions = {}) {
       return replace;
     }, {});
 
-    var resp = {
-      results: replacePointers(response.results, path, replace),
-    };
-    if (response.count) {
-      resp.count = response.count;
-    }
-    return resp;
+    replacePointers(response.results, path, replace);
   });
 }
 
 // Object may be a list of REST-format object to find pointers in, or
 // it may be a single object.
-// If the path yields things that aren't pointers, this throws an error.
 // Path is a list of fields to search into.
 // Returns a list of pointers in REST format.
 function findPointers(object, path) {
@@ -951,8 +978,8 @@ function findPointers(object, path) {
 // in, or it may be a single object.
 // Path is a list of fields to search into.
 // replace is a map from object id -> object.
-// Returns something analogous to object, but with the appropriate
-// pointers inflated.
+//
+// Performs in-place replacements on object
 function replacePointers(object, path, replace) {
   if (object instanceof Array) {
     return object
@@ -964,27 +991,27 @@ function replacePointers(object, path, replace) {
     return object;
   }
 
+  // base case: we're returning a replacement
   if (path.length === 0) {
     if (object && object.__type === 'Pointer') {
       return replace[object.objectId];
     }
     return object;
   }
 
+  // penultimate case: we're looking at the object holding a reference
+  // to be mutated
+  else if (path.length === 1) {
+    object[path[0]] = replacePointers(object[path[0]], [], replace);
+    return;
+  }
+
   var subobject = object[path[0]];
   if (!subobject) {
     return object;
   }
-  var newsub = replacePointers(subobject, path.slice(1), replace);
-  var answer = {};
-  for (var key in object) {
-    if (key == path[0]) {
-      answer[key] = newsub;
-    } else {
-      answer[key] = object[key];
-    }
-  }
-  return answer;
+
+  return replacePointers(subobject, path.slice(1), replace);
 }
 
 // Finds a subobject that has the given key, if there is one.