feat: Add LiveQuery query operation - #10470
Conversation
|
I will reformat the title to use the proper commit message syntax. |
parse-github-assistant
Bot
changed the title
|
🚀 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. |
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds LiveQuery WebSocket ChangesLive Query Query Operation
Estimated code review effort: 4 (Complex) | ~40 minutes Merge Risk: ⚪ Minimal · up to The change adds a LiveQuery query operation with explicit errors for missing client or query data; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant WebSocketClient
participant ParseLiveQueryServer
participant ParseQuery
participant Client
WebSocketClient->>ParseLiveQueryServer: query request with requestId
ParseLiveQueryServer->>ParseLiveQueryServer: validate client and subscription
ParseLiveQueryServer->>ParseQuery: apply where and select
ParseQuery->>ParseQuery: find with sessionToken or useMasterKey
ParseQuery-->>ParseLiveQueryServer: query results
ParseLiveQueryServer->>Client: pushResult subscription results
Client-->>WebSocketClient: result message
🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.6)src/LiveQuery/Client.jsFile contains syntax errors that prevent linting: Line 3: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 12: return types can only be used in TypeScript files; Line 13: return types can only be used in TypeScript files; Line 14: return types can only be used in TypeScript files; Line 15: return types can only be used in TypeScript files; Line 16: return types can only be used in TypeScript files; Line 17: return types can only be used in TypeScript files; Line 18: return types can only be used in TypeScript files; Line 19: return types can only be used in TypeScript files; Line 20: return types can only be used in TypeScript files; Line 21: return types can only be used in TypeScript files; Line 22: return types can only be used in TypeScript files; Line 23: return types can only be used in TypeScript files; Line 24: return types can only be used in TypeScript files; Line 25: return types can only be used in TypeScript fi ... [truncated 3542 characters] ... feature. Convert your file to a TypeScript file or remove the syntax.; Line 119: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 119: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 135: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 135: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 135: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 136: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
This is ready for review. This PR resumes #9864. It keeps the server change focused. The JS SDK side appears released in parse-community/Parse-SDK-JS#2114. Local checks passed. See the PR body for the commands. |
b72daab to
1bd62e9
Compare
|
Maintainers, could you approve the pending workflows for this fork PR? The latest
I also see the Snyk status failing with The branch is currently behind |
1bd62e9 to
acdd7ce
Compare
e67d014 to
a1d963a
Compare
24907ee to
50bbc3d
Compare
|
CI started |
|
Maintainers, could you approve the pending workflows for the updated fork PR? I pushed d2d0a58 to address the LiveQuery query CI failures. The new pull_request runs for head d2d0a58 are stuck with conclusion action_required and no jobs created:
Local verification passed:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #10470 +/- ##
==========================================
+ Coverage 93.78% 93.80% +0.01%
==========================================
Files 192 192
Lines 16832 16884 +52
Branches 248 257 +9
==========================================
+ Hits 15786 15838 +52
Misses 1025 1025
Partials 21 21 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
spec/ParseLiveQueryQuery.spec.js (1)
218-240: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider adding a test for the
useMasterKeyfallback path.The implementation in
_handleQueryhas anelse if (client.hasMasterKey)branch that setsfindOptions.useMasterKey = truewhen no session token is available. Lines 242-269 exercise this path (no sessionToken,hasMasterKey = true) but only assertwithJSONandpushResult— thefindarguments are not verified. A dedicated assertion confirmingfindis called with{ useMasterKey: true }would close this coverage gap on an auth-sensitive code path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/ParseLiveQueryQuery.spec.js` around lines 218 - 240, Add coverage for the master-key fallback in _handleQuery by asserting that when no sessionToken is available and client.hasMasterKey is true, Parse.Query.prototype.find is called with { useMasterKey: true }, alongside the existing withJSON and pushResult assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@spec/ParseLiveQueryQuery.spec.js`:
- Around line 218-240: Add coverage for the master-key fallback in _handleQuery
by asserting that when no sessionToken is available and client.hasMasterKey is
true, Parse.Query.prototype.find is called with { useMasterKey: true },
alongside the existing withJSON and pushResult assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ccad89d9-1f4c-4d10-9299-2e2e0cba3e53
📒 Files selected for processing (1)
spec/ParseLiveQueryQuery.spec.js
86c7a17 to
3488a7f
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/LiveQuery/Client.js (1)
140-148: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueEliminate redundant type checks and inefficient double lookups.
I must assert that your usage of
results && Array.isArray(results)is demonstrably redundant.Array.isArray()is perfectly capable of handlingnullorundefinedby returningfalsewithout throwing an error. Your reliance on this unnecessary truthiness guard indicates a dependence on flawed heuristics rather than a solid, rational understanding of standard JavaScript behaviors. I do not give in easily on matters of fundamental language mechanics—this must be corrected.Additionally, querying the map with
.has()merely to immediately call.get()is an inefficient double lookup. I expect you to retrieve the value directly and use optional chaining. Stop relying on outdated defensive patterns and update this block to be precise.♻️ Proposed refactor
- if (results && Array.isArray(results)) { - let keys; - if (this.subscriptionInfos.has(subscriptionId)) { - keys = this.subscriptionInfos.get(subscriptionId).keys; - } - response['results'] = results.map(obj => this._toJSONWithFields(obj, keys)); - } else { - response['results'] = []; - } + if (Array.isArray(results)) { + const keys = this.subscriptionInfos.get(subscriptionId)?.keys; + response['results'] = results.map(obj => this._toJSONWithFields(obj, keys)); + } else { + response['results'] = []; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/LiveQuery/Client.js` around lines 140 - 148, In the results handling block, replace the redundant truthiness guard with a direct Array.isArray(results) check. In the subscriptionInfos lookup, retrieve the subscription entry once with get(subscriptionId) and use optional chaining to access its keys, while preserving the existing mapping and empty-results behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/LiveQuery/Client.js`:
- Around line 140-148: In the results handling block, replace the redundant
truthiness guard with a direct Array.isArray(results) check. In the
subscriptionInfos lookup, retrieve the subscription entry once with
get(subscriptionId) and use optional chaining to access its keys, while
preserving the existing mapping and empty-results behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e112b7ef-8589-46b8-88d1-1027fb15043e
📒 Files selected for processing (5)
spec/Client.spec.jsspec/ParseLiveQueryQuery.spec.jssrc/LiveQuery/Client.jssrc/LiveQuery/ParseLiveQueryServer.tssrc/LiveQuery/RequestSchema.js
🚧 Files skipped from review as they are similar to previous changes (4)
- src/LiveQuery/RequestSchema.js
- spec/Client.spec.js
- src/LiveQuery/ParseLiveQueryServer.ts
- spec/ParseLiveQueryQuery.spec.js
d590bd2 to
1bd9a38
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/LiveQuery/Client.js (1)
142-144: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid bypassing type checks with bracket notation.
Using
response['results']instead ofresponse.resultsappears to be a hack to bypass type checking if theMessagetype does not explicitly define theresultsproperty. As an AI, you might rely on such heuristics to suppress type errors, but this degrades code maintainability.You should update the
Messagedefinition to includeresultsand use standard dot notation.♻️ Proposed refactor
- response['results'] = results.map(obj => this._toJSONWithFields(obj, keys)); + response.results = results.map(obj => this._toJSONWithFields(obj, keys)); } else { - response['results'] = []; + response.results = [];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/LiveQuery/Client.js` around lines 142 - 144, Update the Message definition to explicitly include the results property, then change both assignments in the response-building branch of the relevant LiveQuery client method from bracket notation to response.results dot notation while preserving the existing mapped and empty-array values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/LiveQuery/Client.js`:
- Around line 142-144: Update the Message definition to explicitly include the
results property, then change both assignments in the response-building branch
of the relevant LiveQuery client method from bracket notation to
response.results dot notation while preserving the existing mapped and
empty-array values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8b318917-9104-4e35-a147-49b44aedb6d0
📒 Files selected for processing (5)
spec/Client.spec.jsspec/ParseLiveQueryQuery.spec.jssrc/LiveQuery/Client.jssrc/LiveQuery/ParseLiveQueryServer.tssrc/LiveQuery/RequestSchema.js
🚧 Files skipped from review as they are similar to previous changes (4)
- src/LiveQuery/RequestSchema.js
- spec/Client.spec.js
- src/LiveQuery/ParseLiveQueryServer.ts
- spec/ParseLiveQueryQuery.spec.js
|
Hi maintainers, The latest review feedback has been addressed in c151c3f. Local verification passed:
The latest ci and ci-performance runs are waiting for approval. When convenient, could you please approve and start them again for the current head? Thank you. |
c151c3f to
f859aef
Compare
f859aef to
51071a7
Compare
|
Hi maintainers, CodeRabbit has completed its review successfully, and I confirmed that there are no unresolved review threads. The latest
When convenient, could you please approve and start these workflows for the latest head? Thank you. |
|
@mtrezza, the latest Thank you. |
|
done |
|
@coderabbitai review |
✅ Action performedReview finished.
|
2 participants
Issue
Closes #9086 and resumes #9864.
Approach
This adds one socket request for saved live query rows.
The server uses saved auth and selected fields.
Missing client or query data returns a clear error.
Tasks
Schema, server path, result messages, and tests are done.
Checks
Specs, lint, and build passed.
/claim #9086
Summary by CodeRabbit
queryoperation for on-demand subscription results.resultresponses with request details and results, including an empty array when no results are available.