-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
feature: run query via livequery #9864
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?
Conversation
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughAdds a LiveQuery "query" operation: request schema update, server-side handler to execute finds and return results, client API to push query results, and tests validating result delivery, field selection, where constraints, and error paths. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant SDK as LiveQuery Client SDK
participant WS as WebSocket Server
participant Srv as ParseLiveQueryServer
participant Sub as Subscription Registry
participant DB as Data Store
rect rgba(230,240,255,0.4)
note left of SDK: client sends a query tied to a subscriptionId
end
SDK->>WS: { op: "query", requestId, id: subscriptionId, where, keys }
WS->>Srv: route "query" request
Srv->>Sub: lookup subscription(subscriptionId)
alt subscription found
Srv->>Srv: resolve auth (subscription.sessionToken || client.sessionToken || master)
Srv->>DB: find(className, where, keys)
DB-->>Srv: results[]
Srv->>WS: push { op: "result", requestId, clientId, installationId, results }
WS-->>SDK: deliver "result"
else subscription missing
Srv->>WS: push error { op: "error", requestId, ... }
WS-->>SDK: deliver error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## alpha #9864 +/- ##
==========================================
- Coverage 93.02% 92.44% -0.59%
==========================================
Files 187 187
Lines 15096 15149 +53
Branches 174 183 +9
==========================================
- Hits 14043 14004 -39
- Misses 1041 1128 +87
- Partials 12 17 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
🧹 Nitpick comments (1)
spec/ParseLiveQueryQuery.spec.js (1)
6-19
: Remove unnecessarydone
callback from synchronous beforeEach.The
beforeEach
function is synchronous but declares adone
parameter and calls it. Jasmine only requires thedone
callback for asynchronous setup. This adds unnecessary overhead.Apply this diff:
- beforeEach(function (done) { + beforeEach(function () { Parse.CoreManager.getLiveQueryController().setDefaultLiveQueryClient(null); // Mock ParseWebSocketServer const mockParseWebSocketServer = jasmine.createSpy('ParseWebSocketServer'); jasmine.mockLibrary( '../lib/LiveQuery/ParseWebSocketServer', 'ParseWebSocketServer', mockParseWebSocketServer ); // Mock Client pushError const Client = require('../lib/LiveQuery/Client').Client; Client.pushError = jasmine.createSpy('pushError'); - done(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
spec/ParseLiveQueryQuery.spec.js
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-30T19:31:35.344Z
Learnt from: RahulLanjewar93
PR: parse-community/parse-server#9744
File: spec/ParseLiveQuery.spec.js:0-0
Timestamp: 2025-04-30T19:31:35.344Z
Learning: In the Parse Server codebase, the functions in QueryTools.js are typically tested through end-to-end behavior tests rather than direct unit tests, even though the functions are exported from the module.
Applied to files:
spec/ParseLiveQueryQuery.spec.js
🧬 Code graph analysis (1)
spec/ParseLiveQueryQuery.spec.js (2)
src/LiveQuery/RequestSchema.js (1)
query
(152-163)spec/helper.js (1)
reconfigureServer
(171-205)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: Redis Cache
- GitHub Check: Node 20
- GitHub Check: Node 18
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: Docker Build
🔇 Additional comments (7)
spec/ParseLiveQueryQuery.spec.js (7)
21-27
: LGTM! Proper async cleanup.The cleanup properly closes LiveQuery clients and restores mocked libraries, ensuring test isolation.
29-35
: LGTM! Well-structured test helper.The helper cleanly creates and registers a mock client with a spy for result verification.
37-64
: LGTM! Comprehensive subscription setup helper.The helper correctly establishes subscription state on both server and client sides, mirroring the production subscription structure.
124-150
: LGTM! Comprehensive error handling coverage.These tests properly validate error paths when clientId is missing or subscription doesn't exist, ensuring robust error handling.
163-167
: Verify serverURL configuration in field filtering and where constraint tests.Similar to the first test, these tests use
serverURL: 'http://localhost:1337/parse'
whilereconfigureServer
likely uses a different port. Ensure this doesn't cause query execution failures.The verification script from the earlier comment applies here as well.
Also applies to: 224-228
152-211
: LGTM! Field filtering validation is thorough.The test correctly validates that the
keys
parameter restricts returned fields, checking both included and excluded fields.
213-270
: LGTM! Where clause filtering is properly tested.The test validates that query constraints are correctly applied, returning only matching objects.
const { ParseLiveQueryServer } = require('../lib/LiveQuery/ParseLiveQueryServer'); | ||
const parseLiveQueryServer = new ParseLiveQueryServer({ | ||
appId: 'test', | ||
masterKey: 'test', | ||
serverURL: 'http://localhost:1337/parse' | ||
}); |
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
Verify serverURL configuration matches test setup.
The ParseLiveQueryServer
is configured with serverURL: 'http://localhost:1337/parse'
, but the test uses reconfigureServer
which typically runs on a different port. If the query operation makes REST calls to Parse Server, this mismatch could cause connection failures.
Check if the serverURL should use the same port as the reconfigured server:
🏁 Script executed:
#!/bin/bash
# Find how reconfigureServer sets the port for Parse Server
rg -nP -A5 'reconfigureServer.*=.*async' spec/helper.js
# Check if _handleQuery makes REST calls using serverURL
ast-grep --pattern '_handleQuery($$$) {
$$$
}'
Length of output: 279
🏁 Script executed:
#!/bin/bash
# Locate the ParseLiveQueryServer class definition
rg -nP -C3 'class\s+ParseLiveQueryServer' --glob '*.js'
# Inspect the _handleQuery method implementation
rg -nP -C3 '_handleQuery' --glob '*.js'
# Look for usage of serverURL within the file
rg -nP 'serverURL' -C3 --glob '*.js'
Length of output: 112115
🏁 Script executed:
#!/bin/bash
# Locate the ParseLiveQueryServer source
grep -R "class ParseLiveQueryServer" -n lib/LiveQuery/ParseLiveQueryServer.js
# Inspect constructor for serverURL usage
grep -R "constructor" -n lib/LiveQuery/ParseLiveQueryServer.js -A5
# Inspect query handler implementation
grep -R "_handleQuery" -n lib/LiveQuery/ParseLiveQueryServer.js -A5
# Check for directAccess default and serverURL fallback
grep -R "directAccess" -n lib/LiveQuery/ParseLiveQueryServer.js -A3
Length of output: 283
🏁 Script executed:
#!/bin/bash
# List top-level directories to locate code structure
ls -1
# Search for ParseLiveQueryServer definition anywhere in the repo
rg -nP 'class\s+ParseLiveQueryServer' -g '*.js'
# If found, inspect the constructor and _handleQuery methods
rg -nP --context 5 'constructor\s*\(' -g '*ParseLiveQueryServer.js'
rg -nP --context 5 '_handleQuery\s*\(' -g '*ParseLiveQueryServer.js'
Length of output: 650
#!/bin/bash
# Dynamically locate the ParseLiveQueryServer implementation
file=$(grep -R "class ParseLiveQueryServer" -l src | head -1)
echo "Inspecting $file"
# Show constructor to see how serverURL is stored
grep -nE 'constructor' -C3 "$file"
# Show how _handleQuery uses serverURL or directAccess
grep -nE '_handleQuery' -C3 "$file"
# Show any references to serverURL in this class
grep -nE 'serverURL' -C3 "$file"
🏁 Script executed:
#!/bin/bash
# Search for ParseLiveQueryServer implementation in TypeScript sources
rg -nP 'class\s+ParseLiveQueryServer' -g 'src/**/*.ts'
# Print the file path(s)
rg -lP 'class\s+ParseLiveQueryServer' -g 'src/**/*.ts'
# Inspect the found file
FILE=$(rg -lP 'class\s+ParseLiveQueryServer' -g 'src/**/*.ts' | head -1)
echo "Inspecting $FILE"
grep -nE 'constructor' -C3 "$FILE"
grep -nE '_handleQuery' -C3 "$FILE"
grep -nE 'serverURL' -C3 "$FILE"
Length of output: 1789
🏁 Script executed:
#!/bin/bash
sed -n '1000,1100p' src/LiveQuery/ParseLiveQueryServer.ts
Length of output: 3677
🏁 Script executed:
#!/bin/bash
sed -n '1,200p' src/LiveQuery/ParseLiveQueryServer.ts
Length of output: 7315
Replace hard-coded serverURL in query tests
In spec/ParseLiveQueryQuery.spec.js (around line 76), you’re instantiating ParseLiveQueryServer with serverURL: 'http://localhost:1337/parse'
, but the test helper reconfigures Parse Server on port 8378 with mount /1
. This mismatch causes _handleQuery
to point at the wrong endpoint. Use the helper’s serverURL
(or Parse.serverURL
) instead so LiveQueryServer and the test Parse Server align.
🤖 Prompt for AI Agents
In spec/ParseLiveQueryQuery.spec.js around lines 76 to 81, the
ParseLiveQueryServer is instantiated with a hard-coded serverURL
'http://localhost:1337/parse' which mismatches the test helper’s Parse Server
(port 8378, mount '/1'), causing _handleQuery to target the wrong endpoint;
replace the hard-coded URL with the helper-provided server URL (e.g., use the
helper’s serverURL variable or Parse.serverURL) so the ParseLiveQueryServer and
the test Parse Server use the same base URL and mount.
Pull Request
Issue
Closes: #9086
Added LiveQuery
query
operation to execute queries through existing WebSocket subscriptions, eliminating separate REST API calls for data syncing.Client SDK Changes Required:
Add
result
event type to LiveQuery constantsHandle
result
messages in WebSocket handlerAdd
query()
method to subscription classAdd tests
Summary by CodeRabbit
New Features
Bug Fixes
Tests