diff --git a/spec/ParseQuery.spec.js b/spec/ParseQuery.spec.js index ee4727404a..5075a08926 100644 --- a/spec/ParseQuery.spec.js +++ b/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'); diff --git a/src/RestQuery.js b/src/RestQuery.js index 468446561c..fbc3d1982b 100644 --- a/src/RestQuery.js +++ b/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>) +// +// - `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,6 +991,7 @@ 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]; @@ -971,20 +999,19 @@ function replacePointers(object, path, replace) { 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.