-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
feat: Add a new beforeLiveQueryEvent
trigger
#9445
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: alpha
Are you sure you want to change the base?
feat: Add a new beforeLiveQueryEvent
trigger
#9445
Conversation
Thanks for opening this pull request! |
@codeKonami please let us know when this is ready for review and change the PR state. |
The code is ready, I'd be interested to have your opinion on the approach before going further on documentation and probably more unit tests. |
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 Walkthrough""" WalkthroughThe changes introduce a new Parse Server trigger called Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ParseServer
participant CloudCode
participant LiveQueryServer
Client->>ParseServer: Save or update object
ParseServer->>CloudCode: Run beforeLiveQueryEvent trigger (if defined)
alt beforeLiveQueryEvent sets preventLiveQuery
CloudCode-->>ParseServer: preventLiveQuery = true
ParseServer-->>LiveQueryServer: (No notification sent)
else beforeLiveQueryEvent allows event
CloudCode-->>ParseServer: allow event
ParseServer->>LiveQueryServer: Notify LiveQuery event
end
ParseServer->>CloudCode: Run afterSave trigger (if defined)
ParseServer-->>Client: Respond to save/update
Assessment against linked issues
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/cloud-code/Parse.Cloud.js (1)
631-631
: Fix indentation issue.There's an extra space at the beginning of this line causing an ESLint error.
- ParseCloud.beforeLiveQueryEvent = function (parseClass, handler, validationHandler) { + ParseCloud.beforeLiveQueryEvent = function (parseClass, handler, validationHandler) {🧰 Tools
🪛 ESLint
[error] 631-631: Expected indentation of 0 spaces but found 1.
(indent)
src/RestWrite.js (1)
1667-1675
: Trigger type name may be inconsistent with public APIThe new hook is exposed to users as
beforeLiveQueryEvent
, yet the internal lookup usestriggers.Types.beforeEvent
.
IfbeforeEvent
is not aliased tobeforeLiveQueryEvent
insidetriggers.js
, user-registered callbacks will never be found and the feature will appear broken.Please verify that:
triggers.Types.beforeEvent
is exported under the documented name, or- This lookup is switched to
beforeLiveQueryEvent
.Otherwise the hook will never fire.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
spec/ParseLiveQuery.spec.js
(1 hunks)src/RestWrite.js
(2 hunks)src/cloud-code/Parse.Cloud.js
(1 hunks)src/triggers.js
(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
spec/ParseLiveQuery.spec.js (2)
spec/helper.js (3)
reconfigureServer
(171-205)Parse
(4-4)TestObject
(257-259)spec/ParseLiveQueryServer.spec.js (6)
Parse
(1-1)query
(293-299)query
(330-336)query
(377-383)query
(1102-1108)query
(1152-1158)
🪛 ESLint
src/cloud-code/Parse.Cloud.js
[error] 631-631: Expected indentation of 0 spaces but found 1.
(indent)
spec/ParseLiveQuery.spec.js
[error] 1312-1312: 'it' is not defined.
(no-undef)
[error] 1313-1313: 'reconfigureServer' is not defined.
(no-undef)
[error] 1324-1324: 'expect' is not defined.
(no-undef)
[error] 1325-1325: 'expect' is not defined.
(no-undef)
[error] 1328-1328: 'TestObject' is not defined.
(no-undef)
[error] 1331-1331: 'expect' is not defined.
(no-undef)
[error] 1332-1332: 'done' is not defined.
(no-undef)
[error] 1335-1335: 'TestObject' is not defined.
(no-undef)
[error] 1340-1340: 'it' is not defined.
(no-undef)
[error] 1341-1341: 'reconfigureServer' is not defined.
(no-undef)
[error] 1349-1349: 'TestObject' is not defined.
(no-undef)
[error] 1354-1354: 'expect' is not defined.
(no-undef)
[error] 1357-1357: 'expect' is not defined.
(no-undef)
[error] 1361-1361: 'expect' is not defined.
(no-undef)
[error] 1362-1362: 'expect' is not defined.
(no-undef)
[error] 1363-1363: 'expect' is not defined.
(no-undef)
[error] 1366-1366: 'TestObject' is not defined.
(no-undef)
[error] 1369-1369: 'expect' is not defined.
(no-undef)
[error] 1377-1377: 'it' is not defined.
(no-undef)
[error] 1378-1378: 'reconfigureServer' is not defined.
(no-undef)
[error] 1389-1389: 'expect' is not defined.
(no-undef)
[error] 1395-1395: 'TestObject' is not defined.
(no-undef)
[error] 1398-1398: 'fail' is not defined.
(no-undef)
[error] 1401-1401: 'TestObject' is not defined.
(no-undef)
[error] 1406-1406: 'it' is not defined.
(no-undef)
[error] 1407-1407: 'reconfigureServer' is not defined.
(no-undef)
[error] 1415-1415: 'TestObject' is not defined.
(no-undef)
[error] 1420-1420: 'expect' is not defined.
(no-undef)
[error] 1426-1426: 'TestObject' is not defined.
(no-undef)
[error] 1429-1429: 'fail' is not defined.
(no-undef)
🔇 Additional comments (4)
src/cloud-code/Parse.Cloud.js (1)
607-641
: Well-implemented new trigger function!The new
beforeLiveQueryEvent
hook follows the established pattern of other trigger functions in the codebase. It's properly integrated with the trigger system and includes comprehensive documentation.🧰 Tools
🪛 ESLint
[error] 631-631: Expected indentation of 0 spaces but found 1.
(indent)
src/triggers.js (3)
17-17
: Appropriately added new trigger type.The
beforeEvent
trigger type has been correctly added to theTypes
object.
282-284
: Correct integration with context handling.The
beforeEvent
trigger type has been properly integrated with the context copying mechanism in thegetRequestObject
function.
890-892
: Properly integrated with context merging.The
beforeEvent
trigger type has been correctly added to the condition that merges the request context back into the external context, which is essential for thepreventLiveQuery
functionality.
Parse.Cloud.beforeLiveQueryEvent('TestObject', async req => { | ||
expect(req.object.get('foo')).toBe('baz'); | ||
if (req.object.get('foo') === 'baz') { | ||
req.context.preventLiveQuery === true; | ||
} | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix critical bug in preventLiveQuery logic.
Same issue as in the previous test - you're using a comparison operator (===
) instead of an assignment operator (=
).
- if (req.object.get('foo') === 'baz') {
- req.context.preventLiveQuery === true;
+ if (req.object.get('foo') === 'baz') {
+ req.context.preventLiveQuery = true;
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Parse.Cloud.beforeLiveQueryEvent('TestObject', async req => { | |
expect(req.object.get('foo')).toBe('baz'); | |
if (req.object.get('foo') === 'baz') { | |
req.context.preventLiveQuery === true; | |
} | |
}) | |
Parse.Cloud.beforeLiveQueryEvent('TestObject', async req => { | |
expect(req.object.get('foo')).toBe('baz'); | |
if (req.object.get('foo') === 'baz') { | |
req.context.preventLiveQuery = true; | |
} | |
}) |
🧰 Tools
🪛 ESLint
[error] 1420-1420: 'expect' is not defined.
(no-undef)
🤖 Prompt for AI Agents (early access)
In spec/ParseLiveQuery.spec.js around lines 1419 to 1424, the code mistakenly uses the comparison operator (===) instead of the assignment operator (=) when setting req.context.preventLiveQuery. Change req.context.preventLiveQuery === true to req.context.preventLiveQuery = true to correctly assign the value and fix the preventLiveQuery logic.
Parse.Cloud.beforeLiveQueryEvent('TestObject', req => { | ||
expect(req.object.get('foo')).toBe('bar'); | ||
if (req.object.get('foo') === 'bar') { | ||
req.context.preventLiveQuery === true; | ||
} | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix critical bug in preventLiveQuery logic.
You're using a comparison operator (===
) instead of an assignment operator (=
), which means you're comparing the value rather than setting it. This would cause the filtering functionality to fail.
- if (req.object.get('foo') === 'bar') {
- req.context.preventLiveQuery === true;
+ if (req.object.get('foo') === 'bar') {
+ req.context.preventLiveQuery = true;
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Parse.Cloud.beforeLiveQueryEvent('TestObject', req => { | |
expect(req.object.get('foo')).toBe('bar'); | |
if (req.object.get('foo') === 'bar') { | |
req.context.preventLiveQuery === true; | |
} | |
}) | |
Parse.Cloud.beforeLiveQueryEvent('TestObject', req => { | |
expect(req.object.get('foo')).toBe('bar'); | |
if (req.object.get('foo') === 'bar') { | |
req.context.preventLiveQuery = true; | |
} | |
}) |
🧰 Tools
🪛 ESLint
[error] 1389-1389: 'expect' is not defined.
(no-undef)
🤖 Prompt for AI Agents (early access)
In spec/ParseLiveQuery.spec.js around lines 1388 to 1393, replace the comparison operator (===) with the assignment operator (=) when setting req.context.preventLiveQuery to true. This change will correctly assign the value and enable the intended filtering functionality.
it('test beforeLiveQueryEvent ran while creating an object', async function () { | ||
await reconfigureServer({ | ||
liveQuery: { | ||
classNames: ['TestObject'], | ||
}, | ||
startLiveQueryServer: true, | ||
verbose: false, | ||
silent: true, | ||
}); | ||
|
||
|
||
Parse.Cloud.beforeLiveQueryEvent('TestObject', req => { | ||
expect(req.user).toBeUndefined(); | ||
expect(req.object.get('foo')).toBe('bar'); | ||
}) | ||
|
||
const query = new Parse.Query(TestObject); | ||
const subscription = await query.subscribe(); | ||
subscription.on('create', object => { | ||
expect(object.get('foo')).toBe('bar'); | ||
done(); | ||
}); | ||
|
||
const object = new TestObject(); | ||
object.set('foo', 'bar'); | ||
await object.save(); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix test callback handling.
This test is using done()
on line 1332, but the function doesn't receive the done
parameter. The test should either:
- Receive
done
as a parameter and call it at the end, or - Return a promise or use
async/await
properly
- it('test beforeLiveQueryEvent ran while creating an object', async function () {
+ it('test beforeLiveQueryEvent ran while creating an object', async function (done) {
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
it('test beforeLiveQueryEvent ran while creating an object', async function () { | |
await reconfigureServer({ | |
liveQuery: { | |
classNames: ['TestObject'], | |
}, | |
startLiveQueryServer: true, | |
verbose: false, | |
silent: true, | |
}); | |
Parse.Cloud.beforeLiveQueryEvent('TestObject', req => { | |
expect(req.user).toBeUndefined(); | |
expect(req.object.get('foo')).toBe('bar'); | |
}) | |
const query = new Parse.Query(TestObject); | |
const subscription = await query.subscribe(); | |
subscription.on('create', object => { | |
expect(object.get('foo')).toBe('bar'); | |
done(); | |
}); | |
const object = new TestObject(); | |
object.set('foo', 'bar'); | |
await object.save(); | |
}); | |
it('test beforeLiveQueryEvent ran while creating an object', async function (done) { | |
await reconfigureServer({ | |
liveQuery: { | |
classNames: ['TestObject'], | |
}, | |
startLiveQueryServer: true, | |
verbose: false, | |
silent: true, | |
}); | |
Parse.Cloud.beforeLiveQueryEvent('TestObject', req => { | |
expect(req.user).toBeUndefined(); | |
expect(req.object.get('foo')).toBe('bar'); | |
}); | |
const query = new Parse.Query(TestObject); | |
const subscription = await query.subscribe(); | |
subscription.on('create', object => { | |
expect(object.get('foo')).toBe('bar'); | |
done(); | |
}); | |
const object = new TestObject(); | |
object.set('foo', 'bar'); | |
await object.save(); | |
}); |
🧰 Tools
🪛 ESLint
[error] 1312-1312: 'it' is not defined.
(no-undef)
[error] 1313-1313: 'reconfigureServer' is not defined.
(no-undef)
[error] 1324-1324: 'expect' is not defined.
(no-undef)
[error] 1325-1325: 'expect' is not defined.
(no-undef)
[error] 1328-1328: 'TestObject' is not defined.
(no-undef)
[error] 1331-1331: 'expect' is not defined.
(no-undef)
[error] 1332-1332: 'done' is not defined.
(no-undef)
[error] 1335-1335: 'TestObject' is not defined.
(no-undef)
🤖 Prompt for AI Agents (early access)
In spec/ParseLiveQuery.spec.js around lines 1312 to 1338, the test uses done() without receiving done as a parameter, causing callback handling issues. Fix this by adding done as a parameter to the test function and calling done() inside the subscription event handler after assertions, or refactor the test to properly use async/await and return a promise without using done().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, we won't use done
anymore in new tests. Please restructure the test so that it isn't needed anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mtrezza I've restructured the test to eliminate the use of the done
callback pattern. Instead, it now uses a promise that resolves when the 'create' event fires:
it('test beforeLiveQueryEvent ran while creating an object', async function () {
await reconfigureServer({
liveQuery: {
classNames: ['TestObject'],
},
startLiveQueryServer: true,
verbose: false,
silent: true,
});
Parse.Cloud.beforeLiveQueryEvent('TestObject', req => {
expect(req.user).toBeUndefined();
expect(req.object.get('foo')).toBe('bar');
});
const query = new Parse.Query(TestObject);
const subscription = await query.subscribe();
// Create a promise that resolves when the create event happens
const createEventPromise = new Promise(resolve => {
subscription.on('create', object => {
expect(object.get('foo')).toBe('bar');
resolve();
});
});
// Create and save the object that should trigger the event
const object = new TestObject();
object.set('foo', 'bar');
await object.save();
// Wait for the create event to be processed
await createEventPromise;
});
✏️ Learnings added
Learnt from: mtrezza
PR: parse-community/parse-server#9445
File: spec/ParseLiveQuery.spec.js:1312-1338
Timestamp: 2025-05-04T20:41:05.138Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
this.config.database.loadSchema().then(schemaController => { | ||
// Notify LiveQueryServer if possible | ||
const perms = schemaController.getClassLevelPermissions(publishedObject.className); | ||
this.config.liveQueryController.onAfterSave( | ||
publishedObject.className, | ||
publishedObject, | ||
originalObject, | ||
perms | ||
); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Await the schema call to avoid silent LiveQuery failures
this.config.database.loadSchema()
returns a promise, but the surrounding async
function does not await
or return
it.
If the DB lookup rejects (e.g. transient connectivity, schema inconsistency) the rejection becomes an unhandled promise, the catch
in the caller never fires and LiveQuery delivery silently fails.
- this.config.database.loadSchema().then(schemaController => {
- // Notify LiveQueryServer if possible
- const perms = schemaController.getClassLevelPermissions(publishedObject.className);
- this.config.liveQueryController.onAfterSave(
- publishedObject.className,
- publishedObject,
- originalObject,
- perms
- );
- });
+ const schemaController = await this.config.database.loadSchema();
+ // Notify LiveQueryServer if possible
+ const perms = schemaController.getClassLevelPermissions(publishedObject.className);
+ this.config.liveQueryController.onAfterSave(
+ publishedObject.className,
+ publishedObject,
+ originalObject,
+ perms
+ );
This keeps the method “fire-and-forget” semantics for LiveQuery success but still propagates unexpected errors up the promise chain for proper logging / retries.
Please consider the change or add an explicit .catch()
to prevent unhandled rejections.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
this.config.database.loadSchema().then(schemaController => { | |
// Notify LiveQueryServer if possible | |
const perms = schemaController.getClassLevelPermissions(publishedObject.className); | |
this.config.liveQueryController.onAfterSave( | |
publishedObject.className, | |
publishedObject, | |
originalObject, | |
perms | |
); | |
}); | |
const schemaController = await this.config.database.loadSchema(); | |
// Notify LiveQueryServer if possible | |
const perms = schemaController.getClassLevelPermissions(publishedObject.className); | |
this.config.liveQueryController.onAfterSave( | |
publishedObject.className, | |
publishedObject, | |
originalObject, | |
perms | |
); |
🤖 Prompt for AI Agents (early access)
In src/RestWrite.js around lines 1677 to 1686, the call to this.config.database.loadSchema() returns a promise that is neither awaited nor returned, causing unhandled promise rejections if it fails. To fix this, modify the code to either await the loadSchema() call or return the promise chain, and add a .catch() handler to properly handle and propagate errors, ensuring that any failures in loading the schema do not silently fail and are logged or retried as needed.
@codeKonami Apologies for the long wait time; when you have some time, please take a look at the review comments. |
82ea402
to
898bbfe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
spec/ParseLiveQuery.spec.js (3)
1312-1338
:⚠️ Potential issueReplace callback pattern with promise-based approach.
The test is using
done()
at line 1332, but the function doesn't receive thedone
parameter. Based on the project's guidelines, new tests should use async/await with promise-based patterns rather than callback patterns withdone()
.Refactor the test to use a promise approach:
- it('test beforeLiveQueryEvent ran while creating an object', async function () { - await reconfigureServer({ - liveQuery: { - classNames: ['TestObject'], - }, - startLiveQueryServer: true, - verbose: false, - silent: true, - }); - - - Parse.Cloud.beforeLiveQueryEvent('TestObject', req => { - expect(req.user).toBeUndefined(); - expect(req.object.get('foo')).toBe('bar'); - }) - - const query = new Parse.Query(TestObject); - const subscription = await query.subscribe(); - subscription.on('create', object => { - expect(object.get('foo')).toBe('bar'); - done(); - }); - - const object = new TestObject(); - object.set('foo', 'bar'); - await object.save(); + it('test beforeLiveQueryEvent ran while creating an object', async function () { + await reconfigureServer({ + liveQuery: { + classNames: ['TestObject'], + }, + startLiveQueryServer: true, + verbose: false, + silent: true, + }); + + Parse.Cloud.beforeLiveQueryEvent('TestObject', req => { + expect(req.user).toBeUndefined(); + expect(req.object.get('foo')).toBe('bar'); + }); + + const query = new Parse.Query(TestObject); + const subscription = await query.subscribe(); + + // Create a promise that resolves when the create event happens + const createEventPromise = new Promise(resolve => { + subscription.on('create', object => { + expect(object.get('foo')).toBe('bar'); + resolve(); + }); + }); + + // Create and save the object that should trigger the event + const object = new TestObject(); + object.set('foo', 'bar'); + await object.save(); + + // Wait for the create event to be processed + await createEventPromise;
1388-1393
:⚠️ Potential issueFix critical bug in preventLiveQuery logic.
You're using a comparison operator (
===
) instead of an assignment operator (=
), which means you're comparing the value rather than setting it. This would cause the filtering functionality to fail.- if (req.object.get('foo') === 'bar') { - req.context.preventLiveQuery === true; + if (req.object.get('foo') === 'bar') { + req.context.preventLiveQuery = true; }
1419-1424
:⚠️ Potential issueFix critical bug in preventLiveQuery logic.
Same issue as in the previous test - you're using a comparison operator (
===
) instead of an assignment operator (=
).- if (req.object.get('foo') === 'baz') { - req.context.preventLiveQuery === true; + if (req.object.get('foo') === 'baz') { + req.context.preventLiveQuery = true; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
spec/ParseLiveQuery.spec.js
(1 hunks)src/RestWrite.js
(2 hunks)src/cloud-code/Parse.Cloud.js
(1 hunks)src/triggers.js
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/cloud-code/Parse.Cloud.js
- src/RestWrite.js
- src/triggers.js
🧰 Additional context used
🧠 Learnings (1)
spec/ParseLiveQuery.spec.js (1)
Learnt from: mtrezza
PR: parse-community/parse-server#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()`.
🧬 Code Graph Analysis (1)
spec/ParseLiveQuery.spec.js (6)
spec/helper.js (3)
reconfigureServer
(171-205)Parse
(4-4)TestObject
(257-259)spec/ParseQuery.spec.js (2)
Parse
(7-7)TestObject
(2937-2937)spec/CloudCode.spec.js (5)
Parse
(3-3)query
(196-196)query
(227-227)query
(262-262)query
(625-625)spec/ParseLiveQueryServer.spec.js (12)
Parse
(1-1)query
(293-299)query
(330-336)query
(377-383)query
(1102-1108)query
(1152-1158)subscription
(354-354)subscription
(413-413)subscription
(468-473)subscription
(1192-1194)subscription
(1205-1207)subscription
(1894-1896)spec/ParseUser.spec.js (1)
query
(4273-4273)spec/ParseGeoPoint.spec.js (1)
TestObject
(5-5)
it('test beforeLiveQueryEvent ran while updating an object', async function (done) { | ||
await reconfigureServer({ | ||
liveQuery: { | ||
classNames: ['TestObject'], | ||
}, | ||
startLiveQueryServer: true, | ||
verbose: false, | ||
silent: true, | ||
}); | ||
const object = new TestObject(); | ||
object.set('foo', 'bar'); | ||
await object.save(); | ||
|
||
Parse.Cloud.afterSave('TestObject', async req => { | ||
expect(req.object.get('foo')).toBe('baz'); | ||
}) | ||
Parse.Cloud.beforeLiveQueryEvent('TestObject', async req => { | ||
expect(req.object.get('foo')).toBe('baz'); | ||
req.object.set('foo', 'rebaz'); | ||
}) | ||
Parse.Cloud.afterLiveQueryEvent('TestObject', req => { | ||
expect(req.event).toBe('update'); | ||
expect(req.user).toBeUndefined(); | ||
expect(req.object.get('foo')).toBe('rebaz'); | ||
}); | ||
|
||
const query = new Parse.Query(TestObject); | ||
const subscription = await query.subscribe(); | ||
subscription.on('update', object => { | ||
expect(object.get('foo')).toBe('rebaz'); | ||
done(); | ||
}); | ||
|
||
object.set('foo', 'baz') | ||
await object.save(); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace callback pattern with promise-based approach.
The test is using done
as a parameter but should be restructured to use promises per maintainer feedback.
Refactor the test to use a promise approach:
- it('test beforeLiveQueryEvent ran while updating an object', async function (done) {
+ it('test beforeLiveQueryEvent ran while updating an object', async function () {
await reconfigureServer({
liveQuery: {
classNames: ['TestObject'],
},
startLiveQueryServer: true,
verbose: false,
silent: true,
});
const object = new TestObject();
object.set('foo', 'bar');
await object.save();
Parse.Cloud.afterSave('TestObject', async req => {
expect(req.object.get('foo')).toBe('baz');
})
Parse.Cloud.beforeLiveQueryEvent('TestObject', async req => {
expect(req.object.get('foo')).toBe('baz');
req.object.set('foo', 'rebaz');
})
Parse.Cloud.afterLiveQueryEvent('TestObject', req => {
expect(req.event).toBe('update');
expect(req.user).toBeUndefined();
expect(req.object.get('foo')).toBe('rebaz');
});
const query = new Parse.Query(TestObject);
const subscription = await query.subscribe();
- subscription.on('update', object => {
- expect(object.get('foo')).toBe('rebaz');
- done();
- });
+
+ // Create a promise that resolves when the update event happens
+ const updateEventPromise = new Promise(resolve => {
+ subscription.on('update', object => {
+ expect(object.get('foo')).toBe('rebaz');
+ resolve();
+ });
+ });
object.set('foo', 'baz')
await object.save();
+
+ // Wait for the update event to be processed
+ await updateEventPromise;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
it('test beforeLiveQueryEvent ran while updating an object', async function (done) { | |
await reconfigureServer({ | |
liveQuery: { | |
classNames: ['TestObject'], | |
}, | |
startLiveQueryServer: true, | |
verbose: false, | |
silent: true, | |
}); | |
const object = new TestObject(); | |
object.set('foo', 'bar'); | |
await object.save(); | |
Parse.Cloud.afterSave('TestObject', async req => { | |
expect(req.object.get('foo')).toBe('baz'); | |
}) | |
Parse.Cloud.beforeLiveQueryEvent('TestObject', async req => { | |
expect(req.object.get('foo')).toBe('baz'); | |
req.object.set('foo', 'rebaz'); | |
}) | |
Parse.Cloud.afterLiveQueryEvent('TestObject', req => { | |
expect(req.event).toBe('update'); | |
expect(req.user).toBeUndefined(); | |
expect(req.object.get('foo')).toBe('rebaz'); | |
}); | |
const query = new Parse.Query(TestObject); | |
const subscription = await query.subscribe(); | |
subscription.on('update', object => { | |
expect(object.get('foo')).toBe('rebaz'); | |
done(); | |
}); | |
object.set('foo', 'baz') | |
await object.save(); | |
}); | |
it('test beforeLiveQueryEvent ran while updating an object', async function () { | |
await reconfigureServer({ | |
liveQuery: { | |
classNames: ['TestObject'], | |
}, | |
startLiveQueryServer: true, | |
verbose: false, | |
silent: true, | |
}); | |
const object = new TestObject(); | |
object.set('foo', 'bar'); | |
await object.save(); | |
Parse.Cloud.afterSave('TestObject', async req => { | |
expect(req.object.get('foo')).toBe('baz'); | |
}) | |
Parse.Cloud.beforeLiveQueryEvent('TestObject', async req => { | |
expect(req.object.get('foo')).toBe('baz'); | |
req.object.set('foo', 'rebaz'); | |
}) | |
Parse.Cloud.afterLiveQueryEvent('TestObject', req => { | |
expect(req.event).toBe('update'); | |
expect(req.user).toBeUndefined(); | |
expect(req.object.get('foo')).toBe('rebaz'); | |
}); | |
const query = new Parse.Query(TestObject); | |
const subscription = await query.subscribe(); | |
// Create a promise that resolves when the update event happens | |
const updateEventPromise = new Promise(resolve => { | |
subscription.on('update', object => { | |
expect(object.get('foo')).toBe('rebaz'); | |
resolve(); | |
}); | |
}); | |
object.set('foo', 'baz') | |
await object.save(); | |
// Wait for the update event to be processed | |
await updateEventPromise; | |
}); |
🤖 Prompt for AI Agents (early access)
In spec/ParseLiveQuery.spec.js around lines 1340 to 1375, the test uses the
callback parameter 'done' which should be replaced with a promise-based
approach. Refactor the test by removing the 'done' parameter and instead return
a promise that resolves when the subscription's 'update' event is triggered and
assertions pass. This involves wrapping the subscription event handling in a
promise and resolving it after the expectations are met, ensuring the test
completes correctly without using the callback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's right, we don't use done
anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
@mtrezza I'll update the test to use a promise-based approach, removing the done
callback. Also, I noticed the first test also has a similar issue - it uses done()
but doesn't define the parameter. I'll refactor both tests to follow the async/await pattern.
🏁 Script executed:
# Just checking if there are more tests that may need similar refactoring
grep -A 5 "it('test beforeLiveQueryEvent" spec/ParseLiveQuery.spec.js
Length of output: 949
@mtrezza Looking at both tests, I'll refactor them to use the promise-based pattern instead of the done
callback:
- In the first test "ran while creating an object", the test calls
done()
but doesn't have the parameter defined in the function signature - In the second test "ran while updating an object", it explicitly uses the
done
callback pattern
Here's how both should be refactored:
it('test beforeLiveQueryEvent ran while creating an object', async function () {
await reconfigureServer({
liveQuery: {
classNames: ['TestObject'],
},
startLiveQueryServer: true,
verbose: false,
silent: true,
});
Parse.Cloud.beforeLiveQueryEvent('TestObject', req => {
expect(req.user).toBeUndefined();
expect(req.object.get('foo')).toBe('bar');
});
const query = new Parse.Query(TestObject);
const subscription = await query.subscribe();
// Create a promise that resolves when the create event happens
const createEventPromise = new Promise(resolve => {
subscription.on('create', object => {
expect(object.get('foo')).toBe('bar');
resolve();
});
});
const object = new TestObject();
object.set('foo', 'bar');
await object.save();
// Wait for the create event to be processed
await createEventPromise;
});
it('test beforeLiveQueryEvent ran while updating an object', async function () {
await reconfigureServer({
liveQuery: {
classNames: ['TestObject'],
},
startLiveQueryServer: true,
verbose: false,
silent: true,
});
const object = new TestObject();
object.set('foo', 'bar');
await object.save();
Parse.Cloud.afterSave('TestObject', async req => {
expect(req.object.get('foo')).toBe('baz');
});
Parse.Cloud.beforeLiveQueryEvent('TestObject', async req => {
expect(req.object.get('foo')).toBe('baz');
req.object.set('foo', 'rebaz');
});
Parse.Cloud.afterLiveQueryEvent('TestObject', req => {
expect(req.event).toBe('update');
expect(req.user).toBeUndefined();
expect(req.object.get('foo')).toBe('rebaz');
});
const query = new Parse.Query(TestObject);
const subscription = await query.subscribe();
// Create a promise that resolves when the update event happens
const updateEventPromise = new Promise(resolve => {
subscription.on('update', object => {
expect(object.get('foo')).toBe('rebaz');
resolve();
});
});
object.set('foo', 'baz');
await object.save();
// Wait for the update event to be processed
await updateEventPromise;
});
🧬 Code Graph Analysis Results
Relevant snippet from spec/ParseLiveQuery.spec.js
(lines 1-50 approx.)
The file spec/ParseLiveQuery.spec.js
itself contains multiple test cases using the Jasmine testing framework. Many tests use the done
callback parameter in the it
blocks, e.g.:
it('access user on onLiveQueryEvent disconnect', async done => {
// ...
done();
});
and
it('can subscribe to query', async done => {
// ...
done();
});
The user comment "That's right, we don't use done
anymore." likely refers to the fact that these tests are using the done
callback style for asynchronous tests, which is an older pattern. Modern Jasmine supports returning a Promise or using async
functions without needing done
.
Relevant snippet from spec/helper.js
(lines 171-205)
const reconfigureServer = async (changedConfiguration = {}) => {
if (parseServer) {
await shutdownServer(parseServer);
return reconfigureServer(changedConfiguration);
}
didChangeConfiguration = Object.keys(changedConfiguration).length !== 0;
databaseAdapter = new databaseAdapter.constructor({
uri: databaseURI,
collectionPrefix: 'test_',
});
defaultConfiguration.databaseAdapter = databaseAdapter;
global.databaseAdapter = databaseAdapter;
if (filesAdapter instanceof GridFSBucketAdapter) {
defaultConfiguration.filesAdapter = new GridFSBucketAdapter(mongoURI);
}
if (process.env.PARSE_SERVER_TEST_CACHE === 'redis') {
defaultConfiguration.cacheAdapter = new RedisCacheAdapter();
}
const newConfiguration = Object.assign({}, defaultConfiguration, changedConfiguration, {
mountPath: '/1',
port,
});
cache.clear();
parseServer = await ParseServer.startApp(newConfiguration);
Parse.CoreManager.setRESTController(RESTController);
parseServer.expressApp.use('/1', err => {
console.error(err);
fail('should not call next');
});
openConnections.track(parseServer.server);
if (parseServer.liveQueryServer?.server && parseServer.liveQueryServer.server !== parseServer.server) {
openConnections.track(parseServer.liveQueryServer.server);
}
return parseServer;
};
This helper function is used in many tests in ParseLiveQuery.spec.js
to reconfigure and start the Parse Server with LiveQuery enabled. It returns a Promise and is awaited in the tests.
Relevant snippet from src/TestUtils.js
(lines 30-40)
export function resolvingPromise() {
let res;
let rej;
const promise = new Promise((resolve, reject) => {
res = resolve;
rej = reject;
});
promise.resolve = res;
promise.reject = rej;
return promise;
}
This utility function creates a Promise with externally accessible resolve and reject functions. It is used in the tests for asynchronous coordination without using done
.
Summary of relevance to the user comment:
- The test file
spec/ParseLiveQuery.spec.js
contains many tests that use thedone
callback style for asynchronous tests. - Modern async/await style tests in Jasmine do not require
done
and can return a Promise or be declaredasync
. - The helper function
reconfigureServer
returns a Promise and is awaited, showing modern async usage. - The utility
resolvingPromise
is used for async coordination withoutdone
. - The user comment "That's right, we don't use
done
anymore." is relevant to the usage ofdone
in the test file and the possibility to refactor tests to use async/await withoutdone
.
No other code snippets from the provided list are directly relevant to the usage or removal of done
in the test file spec/ParseLiveQuery.spec.js
.
✏️ Learnings added
Learnt from: mtrezza
PR: parse-community/parse-server#9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.266Z
Learning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.
Learnt from: mtrezza
PR: parse-community/parse-server#9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.266Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: mtrezza
PR: parse-community/parse-server#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()`.
Pull Request
Issue
Closes: #9431
Approach
I added a beforeLiveQueryEvent trigger which allows the user to modify the updated object before it is passed to the LiveQuery server (via PubSub). It is a cloned version of the object so it won't modify the one passed to the afterSave trigger.
I've added an option which I'm not really convinced. You can set the context of the request to
req.context.preventLiveQuery === true
to prevent the publish to be made by the Parse Server and so limiting some network and CPU consumption depending on some business logic.Tasks
Summary by CodeRabbit
New Features
Bug Fixes
Tests