Skip to content
82 changes: 82 additions & 0 deletions spec/RestQuery.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,88 @@ describe('rest query', () => {
}
);
});

it('battle test parallel include with 100 nested includes', async () => {
const RootObject = Parse.Object.extend('RootObject');
const Level1Object = Parse.Object.extend('Level1Object');
const Level2Object = Parse.Object.extend('Level2Object');

// Create 100 level2 objects (10 per level1 object)
const level2Objects = [];
for (let i = 0; i < 100; i++) {
const level2 = new Level2Object({
index: i,
value: `level2_${i}`,
});
level2Objects.push(level2);
}
await Parse.Object.saveAll(level2Objects);

// Create 10 level1 objects, each with 10 pointers to level2 objects
const level1Objects = [];
for (let i = 0; i < 10; i++) {
const level1 = new Level1Object({
index: i,
value: `level1_${i}`,
});
// Set 10 pointer fields (level2_0 through level2_9)
for (let j = 0; j < 10; j++) {
level1.set(`level2_${j}`, level2Objects[i * 10 + j]);
}
level1Objects.push(level1);
}
await Parse.Object.saveAll(level1Objects);

// Create 1 root object with 10 pointers to level1 objects
const rootObject = new RootObject({
value: 'root',
});
for (let i = 0; i < 10; i++) {
rootObject.set(`level1_${i}`, level1Objects[i]);
}
await rootObject.save();

// Build include paths: level1_0 through level1_9, and level1_0.level2_0 through level1_9.level2_9
const includePaths = [];
for (let i = 0; i < 10; i++) {
includePaths.push(`level1_${i}`);
for (let j = 0; j < 10; j++) {
includePaths.push(`level1_${i}.level2_${j}`);
}
}

// Query with all includes
const query = new Parse.Query(RootObject);
query.equalTo('objectId', rootObject.id);
for (const path of includePaths) {
query.include(path);
}
console.time('query.find');
const results = await query.find();
console.timeEnd('query.find');
expect(results.length).toBe(1);

const result = results[0];
expect(result.id).toBe(rootObject.id);

// Verify all 10 level1 objects are included
for (let i = 0; i < 10; i++) {
const level1Field = result.get(`level1_${i}`);
expect(level1Field).toBeDefined();
expect(level1Field instanceof Parse.Object).toBe(true);
expect(level1Field.get('index')).toBe(i);
expect(level1Field.get('value')).toBe(`level1_${i}`);

// Verify all 10 level2 objects are included for each level1 object
for (let j = 0; j < 10; j++) {
const level2Field = level1Field.get(`level2_${j}`);
expect(level2Field).toBeDefined();
expect(level2Field instanceof Parse.Object).toBe(true);
expect(level2Field.get('index')).toBe(i * 10 + j);
expect(level2Field.get('value')).toBe(`level2_${i * 10 + j}`);
}
}
});
});

describe('RestQuery.each', () => {
Expand Down
61 changes: 41 additions & 20 deletions src/RestQuery.js
Original file line number Diff line number Diff line change
Expand Up @@ -856,31 +856,54 @@ _UnsafeRestQuery.prototype.handleExcludeKeys = function () {
};

// Augments this.response with data at the paths provided in this.include.
_UnsafeRestQuery.prototype.handleInclude = function () {
_UnsafeRestQuery.prototype.handleInclude = async function () {
if (this.include.length == 0) {
return;
}

var pathResponse = includePath(
this.config,
this.auth,
this.response,
this.include[0],
this.context,
this.restOptions
);
if (pathResponse.then) {
return pathResponse.then(newResponse => {
this.response = newResponse;
this.include = this.include.slice(1);
return this.handleInclude();
const indexedResults = this.response.results.reduce((indexed, result, i) => {
indexed[result.objectId] = i;
return indexed;
}, {});

// Build the execution tree
const executionTree = {}
this.include.forEach(path => {
let current = executionTree;
path.forEach((node) => {
if (!current[node]) {
current[node] = {
path,
children: {}
};
}
current = current[node].children
});
} else if (this.include.length > 0) {
this.include = this.include.slice(1);
return this.handleInclude();
});

const recursiveExecutionTree = async (treeNode) => {
const { path, children } = treeNode;
const pathResponse = includePath(
this.config,
this.auth,
this.response,
path,
this.context,
this.restOptions,
this,
);
if (pathResponse.then) {
const newResponse = await pathResponse
newResponse.results.forEach(newObject => {
// We hydrate the root of each result with sub results
this.response.results[indexedResults[newObject.objectId]][path[0]] = newObject[path[0]];
})
}
return Promise.all(Object.values(children).map(recursiveExecutionTree));
}

return pathResponse;
await Promise.all(Object.values(executionTree).map(recursiveExecutionTree));
Comment on lines +864 to +905
Copy link

@coderabbitai coderabbitai bot Nov 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Hydration must not depend solely on objectId.

indexedResults collapses every row whose objectId is missing (e.g. aggregate pipelines that $project out _id while keeping pointer fields) into the single undefined bucket. In that case only the last element in this.response.results is updated and the remaining rows keep their raw pointers, whereas the previous sequential implementation handled these projections correctly. That’s a correctness regression.

Please rework the assignment so we still hydrate by index whenever we cannot resolve an objectId, e.g. by falling back to the loop index.

-      newResponse.results.forEach(newObject => {
-        this.response.results[indexedResults[newObject.objectId]][path[0]] = newObject[path[0]];
-      })
+      newResponse.results.forEach((newObject, idx) => {
+        const targetIndex = indexedResults[newObject.objectId];
+        const target = targetIndex !== undefined ? this.response.results[targetIndex] : this.response.results[idx];
+        if (target) {
+          target[path[0]] = newObject[path[0]];
+        }
+      })
🤖 Prompt for AI Agents
In src/RestQuery.js around lines 864 to 905, the hydration currently collapses
rows with missing objectId because indexedResults maps undefined to a single
bucket; update the logic so indexedResults only maps defined objectId values to
their original index, and when applying newResponse.results use the iteration
index as a fallback: in the forEach use the callback index (e.g. (newObject, i))
and compute targetIndex = (newObject.objectId !== undefined &&
indexedResults[newObject.objectId] !== undefined) ?
indexedResults[newObject.objectId] : i, then assign
this.response.results[targetIndex][path[0]] = newObject[path[0]]; ensure you
handle falsy but valid objectId values correctly by explicitly checking
undefined.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

objectId is always present in parse returned include, does the issue still exists ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there are any safeguards preventing includes with aggregate/pipeline queries
rg -n -C3 "pipeline.*include|include.*pipeline" --type=js

# Search for how aggregate queries handle includes
rg -n -C3 "findOptions.pipeline" --type=js -g "src/**"

# Check if there's validation that prevents includes on aggregate queries
rg -n -C3 "aggregate.*include|pipeline.*include" --type=js -g "spec/**"

# Look for any code that explicitly checks for objectId before processing includes
rg -n -C3 "objectId.*include|include.*objectId" --type=js -g "src/RestQuery.js"

Length of output: 717


🏁 Script executed:

#!/bin/bash
# Check the full execute() method to see the order of operations
ast-grep --pattern $'_UnsafeRestQuery.prototype.execute = function ($$$) {
  $$$
}'

# Look for any validation that prevents includes with pipeline/aggregate
rg -n -B5 -A10 "handleInclude" src/RestQuery.js | head -60

# Search for tests that combine includes with aggregate queries
rg -n "aggregate.*include|pipeline.*include" spec/ --type=js -C2

# Check if aggregate queries can have the include option at all
rg -n "pipeline.*restOptions|aggregate.*restOptions" --type=js -g "spec/**" | head -20

Length of output: 3354


🏁 Script executed:

#!/bin/bash
# Check how AlwaysSelectedKeys (which includes objectId) is used
rg -n "AlwaysSelectedKeys" src/RestQuery.js -A3 -B3

# Look for aggregate query tests and whether they can have includes
rg -n "aggregate|pipeline" spec/RestQuery.spec.js -C3 | head -100

# Check if there's any code that strips objectId from results
rg -n "delete.*objectId|objectId.*delete" --type=js -g "src/**"

# Look for $project in aggregate pipelines that might exclude _id
rg -n '\$project' spec/ --type=js -C2 | head -50

Length of output: 4131


🏁 Script executed:

#!/bin/bash
# Check if aggregate queries can have include in their options
rg -n "include" spec/ParseQuery.Aggregate.spec.js -C2 | head -50

# Look for the aggregate route handler to see what options it accepts
rg -n "handleFind.*pipeline|pipeline.*handleFind" --type=js -g "src/**"

# Check RestQuery constructor to see if it validates pipeline + include
ast-grep --pattern $'function _UnsafeRestQuery(
  $$$
) {
  $$$
}'

# Search for any code that prevents includes on pipeline queries
rg -n "pipeline.*include|include.*pipeline" src/ --type=js -C3

Length of output: 9784


@Moumouls, You're right that in normal query results, objectId is always present. However, I found an edge case where it can be missing:

The edge case: When a RestQuery has both:

  1. A pipeline option with $project: { _id: 0 } (explicitly excluding objectId)
  2. An include option

Looking at the code:

  • The constructor accepts both pipeline (line 215) and include (line 236) without validation preventing their combination
  • handleInclude() does NOT skip aggregate queries (unlike runAfterFindTrigger() which checks for pipeline/distinct at line 927)
  • Tests show aggregate queries can use $project: { _id: 0 } to exclude objectId (e.g., spec/ParseQuery.Aggregate.spec.js:362-363)

While this is admittedly a rare/unusual scenario (combining includes with aggregates that exclude _id), the original concern is technically valid.

Recommendation: Either add a guard to skip handleInclude() when pipeline or distinct is present (similar to runAfterFindTrigger), or implement the fallback indexing logic suggested in the original comment to handle this edge case safely.

What do you think? Should we add protection against this edge case or is it acceptable to document that includes aren't supported with aggregate queries?


🧠 Learnings used
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: spec/CloudCode.spec.js:446-469
Timestamp: 2025-08-26T14:06:31.853Z
Learning: In the Parse Server codebase, when handling query objects in maybeRunAfterFindTrigger, objects without a where property that contain options like limit/skip should be treated as query JSON with an empty where clause using the spread pattern { where: {}, ...query }, not nested as { where: query }.

Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:467-477
Timestamp: 2025-08-27T12:33:06.237Z
Learning: In the Parse Server codebase, maybeRunAfterFindTrigger is called in production with Parse.Query objects constructed via withJSON(), so the plain object query handling bug only affects tests, not production code paths.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.917Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1312-1338
Timestamp: 2025-05-04T20:41:05.147Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mtrezza from you knowledge is it normal to allow _id: 0 on projections ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

this.include = []
};

//Returns a promise of a processed set of results
Expand Down Expand Up @@ -1018,7 +1041,6 @@ function includePath(config, auth, response, path, context, restOptions = {}) {
} else if (restOptions.readPreference) {
includeRestOptions.readPreference = restOptions.readPreference;
}

const queryPromises = Object.keys(pointersHash).map(async className => {
const objectIds = Array.from(pointersHash[className]);
let where;
Expand Down Expand Up @@ -1057,7 +1079,6 @@ function includePath(config, auth, response, path, context, restOptions = {}) {
}
return replace;
}, {});

var resp = {
results: replacePointers(response.results, path, replace),
};
Expand Down
Loading