feat: Add a new beforeLiveQueryEvent trigger#9445
Conversation
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. Our CI and AI review are safeguards, not development tools. If many issues are flagged, rethink your development approach. Invest more effort in planning and design rather than using review cycles to fix low-quality code. |
|
@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.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new ChangesLiveQuery trigger feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ParseCloud
participant triggers
participant RestWrite
participant liveQueryController
ParseCloud->>triggers: addTrigger(beforeEvent, handler)
RestWrite->>triggers: run beforeEvent trigger
alt trigger returns false
RestWrite-->>liveQueryController: skip onAfterSave
else trigger allows event
RestWrite->>liveQueryController: onAfterSave
end
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
IfbeforeEventis not aliased tobeforeLiveQueryEventinsidetriggers.js, user-registered callbacks will never be found and the feature will appear broken.Please verify that:
triggers.Types.beforeEventis 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
beforeLiveQueryEventhook 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
beforeEventtrigger type has been correctly added to theTypesobject.
282-284: Correct integration with context handling.The
beforeEventtrigger type has been properly integrated with the context copying mechanism in thegetRequestObjectfunction.
890-892: Properly integrated with context merging.The
beforeEventtrigger type has been correctly added to the condition that merges the request context back into the external context, which is essential for thepreventLiveQueryfunctionality.
|
@codeKonami Apologies for the long wait time; when you have some time, please take a look at the review comments. |
codeKonami
force-pushed
the
add-before-live-query-event
branch
from
82ea402 to
898bbfe
Compare
There was a problem hiding this comment.
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 thedoneparameter. 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)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
codeKonami
force-pushed
the
add-before-live-query-event
branch
from
898bbfe to
a80d876
Compare
|
I've reworked this PR based on the review feedback and rebased it onto the latest
A companion documentation update for the |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Cloud Code trigger, Parse.Cloud.beforeLiveQueryEvent, intended to run on the Parse Server instance before a LiveQuery publish occurs, allowing Cloud Code to optionally prevent publishing (by returning false).
Changes:
- Added a new trigger type (
beforeEvent) to supportbeforeLiveQueryEventregistration and execution. - Updated
RestWrite.runAfterSaveTriggerto run the new trigger before publishing LiveQuery events and skip publishing when it returnsfalse. - Added LiveQuery spec coverage for the new trigger behavior (runs on create/update; can prevent publish).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/triggers.js | Adds a new trigger type and ensures the trigger return value can be observed by callers. |
| src/RestWrite.js | Runs the new trigger before LiveQuery publish and conditionally suppresses publish. |
| src/cloud-code/Parse.Cloud.js | Exposes Parse.Cloud.beforeLiveQueryEvent API and documents expected behavior. |
| spec/ParseLiveQuery.spec.js | Adds tests ensuring the trigger runs and can prevent LiveQuery events. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #9445 +/- ##
=======================================
Coverage 92.66% 92.67%
=======================================
Files 193 193
Lines 16981 17000 +19
Branches 248 248
=======================================
+ Hits 15736 15755 +19
Misses 1224 1224
Partials 21 21 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Extract the trigger invocation into a shared triggers.maybeRunBeforeLiveQueryEventTrigger helper that returns whether the event should be published and never rejects. - Run the trigger detached from the save response: RestWrite.runAfterSaveTrigger is no longer async, so a slow trigger cannot delay the client's write request (restores the pre-existing fire-and-forget behavior of the LiveQuery notification). - Gate the delete path too: rest.js del() now consults the trigger before calling liveQueryController.onAfterDelete, so returning false also suppresses delete/leave events. - Fail closed on validator errors: a failing validator (e.g. requireMaster) prevents the publish instead of being swallowed; handler errors remain fail-open as documented. - Drop the redundant triggerExists pre-check (maybeRunTrigger already short-circuits). - Add tests for delete/leave suppression, the delete trigger request, and validator gating. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pull Request
Issue
Closes: #9431
Approach
Adds a new
beforeLiveQueryEventCloud Code trigger. It runs on the Parse Server instance, once per object write (create, update or delete), right before the change is published to the LiveQuery server. This is different fromafterLiveQueryEvent, which runs on the LiveQuery server once per matching subscription.Return
falsefrom the trigger to prevent the event from being published to the LiveQuery server. Returning any other value (includingundefined) publishes the event as usual. This lets developers skip publishing events that no client needs to receive (e.g. objects withstatus === 'draft'), avoiding the network and CPU cost of the LiveQuery server evaluating every subscription — the motivation described in #9431.The trigger and the publish run fire-and-forget, off the critical path of the save/delete response, so a slow trigger never delays the client's write request (the LiveQuery notification was already fire-and-forget before this PR).
Error semantics:
{ requireMaster: true }), the event is not published — a failing validator is a deliberate gate, so it fails closed.The trigger does not modify the published object (the object is published with its
objectId/timestamps intact).Registration mirrors the existing LiveQuery triggers (
Types.beforeEvent, analogous toafterEvent↔afterLiveQueryEvent):Note
For reviewers: adding
beforeEventtotriggers.Typesalso makes the REST Hooks API accept webhooks of this type (POST /hooks/triggerswithtriggerName: 'beforeEvent'), sinceHooksController.createOrUpdateHookvalidates the trigger name by membership intriggers.Types. This is consistent with how the other LiveQuery trigger types (beforeConnect,beforeSubscribe,afterEvent) are already exposed, but unlike those, abeforeEventwebhook is invoked from the main server's write path. The call is fire-and-forget so it does not delay the write response; flagging it here in case the webhook exposure should be restricted separately.Tasks
parse-community/docsrepository is prepared separately)Summary by CodeRabbit
beforeLiveQueryEventhook to run custom logic before LiveQuery publishes object changes.falsefrom the hook suppresses the corresponding LiveQuery create or update event.idin the published payload.