-
Notifications
You must be signed in to change notification settings - Fork 192
added realtime queries #1308
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: master
Are you sure you want to change the base?
added realtime queries #1308
Conversation
WalkthroughThe PR adds per-subscription query support across SDK templates (Android, iOS/Swift, Flutter, React Native, Web). Subscribe APIs gain an optional queries parameter; subscription types (RealtimeCallback/RealtimeSubscription) now include a queries field. Android also changes RealtimeCallback.channels from Collection to Set. Internal state tracks active queries, socket URL construction includes queries[] parameters, and cleanup/teardown logic is extended to prune unused queries. Tests and examples across languages were updated to exercise query-enabled subscription paths. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
templates/react-native/src/client.ts.twig (1)
278-285: Remove or document queries parameter—Appwrite Realtime does not support server-side query filtering.Appwrite's Realtime API uses channel-based subscriptions only and does not accept the
queries[]parameter. Database query filtering is available exclusively on REST/list endpoints, not realtime subscriptions. The code aggregates queries from multiple subscriptions and sends them to the realtime WebSocket endpoint (lines 283–285, 377–379, and similar in other SDKs), but these parameters are silently ignored by the server. Remove the query aggregation and cleanup logic, or document it as legacy/planned-future behavior if query filtering is intended to be supported later.templates/apple/Sources/Services/Realtime.swift.twig (1)
74-81: Add URL encoding for query parameters to prevent URL corruption.Query strings are JSON-encoded and contain special characters (quotes, brackets) that must be percent-encoded when appended to the URL. The Web SDK implementation uses
encodeURIComponent()for this; the Swift version currently does not, creating an inconsistency. Channels have the same issue (pre-existing pattern), but queries from theQueryclass are more likely to contain characters requiring encoding due to their JSON format.🔧 Suggested fix using percent encoding
for channel in activeChannels { - queryParams += "&channels[]=\(channel)" + queryParams += "&channels[]=\(channel.addingPercentEncoding(withAllowedCharacters: .urlQueryAllowed) ?? channel)" } for query in activeQueries { - queryParams += "&queries[]=\(query)" + queryParams += "&queries[]=\(query.addingPercentEncoding(withAllowedCharacters: .urlQueryAllowed) ?? query)" }
🤖 Fix all issues with AI agents
In
`@templates/android/library/src/main/java/io/package/services/Realtime.kt.twig`:
- Around line 65-68: The loop in Realtime.kt.twig that appends queries using
queryParamBuilder and activeQueries must URL-encode each query instead of raw
string concatenation; replace the manual append of "&queries[]=$it" with adding
each query via OkHttp's HttpUrl.Builder.addQueryParameter("queries[]", query)
(same approach used in Client.kt.twig) so project names, channels and query
strings are properly encoded and won't corrupt the URL; locate the activeQueries
iteration and switch to building the request URL with
HttpUrl.Builder.addQueryParameter for each query value.
🧹 Nitpick comments (1)
templates/flutter/test/src/realtime_subscription_test.dart.twig (1)
10-14: LGTM with a minor suggestion.The test correctly passes the new
queriesparameter to match the updated constructor signature. Consider adding an assertion to verify the queries field (e.g.,expect(subscription.queries, equals(const []))), though this is optional since the primary purpose of this test is validating stream and close function wiring.
templates/android/library/src/main/java/io/package/services/Realtime.kt.twig
Outdated
Show resolved
Hide resolved
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
🤖 Fix all issues with AI agents
In `@tests/languages/android/Tests.kt`:
- Around line 94-104: The test currently calls realtime.subscribe("tests",
payloadType = TestPayload::class.java, queries = ...) with an empty callback and
then immediately sets realtimeResponseWithQueries = "WS:/v1/realtime:passed",
which doesn't verify the subscription or query filtering; change the
subscription call to provide a non-empty callback that inspects the incoming
payload (from TestPayload) and sets realtimeResponseWithQueries based on the
payload contents (similar to the first subscription's callback pattern), and
remove the hardcoded assignment so the test only passes when the callback
receives the expected message; optionally add onOpen/onError handlers or
expectation logic if needed to mirror the Apple test behavior.
🧹 Nitpick comments (1)
tests/languages/flutter/tests.dart (1)
170-182: Inconsistent verification between subscriptions.The first subscription (line 172) verifies and prints the message payload, while the second subscription only checks that no exception is thrown when awaiting a message. The
message2variable on line 177 is unused.Consider verifying the message content consistently:
Suggested improvement
// Assert realtime outputs in a deterministic order (no-query then with-query) final message1 = await rtsub.stream.first.timeout(Duration(seconds: 10)); print(message1.payload["response"]); await rtsub.close(); - String realtimeResponseWithQueries = "WS:/v1/realtime:passed"; try { final message2 = await rtsubWithQueries.stream.first.timeout(Duration(seconds: 10)); + print("WS:/v1/realtime:passed"); } catch (e) { - realtimeResponseWithQueries = "Realtime failed!"; + print("Realtime failed!"); } - print(realtimeResponseWithQueries); await rtsubWithQueries.close();
ca81782 to
8665c91
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
templates/android/library/src/main/java/io/package/services/Realtime.kt.twig (1)
145-188: Queries are stored per-subscription but ignored during event dispatch.The
handleResponseEvent()method dispatches callbacks based only on channel matching (event.channels.any { subscription.channels.contains(it) }), without filtering againstsubscription.queries. SinceRealtimeResponseEventlacks query information, clients cannot perform per-subscription query filtering. If the backend emits events for the union of all subscribed queries, callbacks will receive all events regardless of their requested filters. Either confirm the server enforces per-subscription filtering server-side, or add query metadata toRealtimeResponseEventfor client-side filtering.
🧹 Nitpick comments (2)
tests/languages/flutter/tests.dart (1)
167-174: Consider adding error context for timeout failures.The assertions correctly validate both subscription paths sequentially. However, if a timeout occurs, the
TimeoutExceptionwon't indicate which subscription failed.♻️ Optional: Add context to timeout errors
// Assert realtime outputs in a deterministic order (no-query then with-query) - final message1 = await rtsub.stream.first.timeout(Duration(seconds: 10)); + final message1 = await rtsub.stream.first.timeout( + Duration(seconds: 10), + onTimeout: () => throw TimeoutException('No message received for no-query subscription'), + ); print(message1.payload["response"]); await rtsub.close(); - final message2 = await rtsubWithQueries.stream.first.timeout(Duration(seconds: 10)); + final message2 = await rtsubWithQueries.stream.first.timeout( + Duration(seconds: 10), + onTimeout: () => throw TimeoutException('No message received for with-query subscription'), + ); print(message2.payload["response"]); await rtsubWithQueries.close();This requires importing
dart:asyncforTimeoutException.tests/languages/apple/Tests.swift (1)
48-63: Inconsistent nil handling between subscriptions.Line 50 uses force unwrapping (
message.payload!["response"] as! String), which will crash if the payload is nil or missing the "response" key. Line 61 correctly uses safe optional chaining with a fallback. Consider aligning both to use the safer pattern for test robustness.♻️ Suggested improvement for consistency
// Subscribe without queries try await realtime.subscribe(channels: ["tests"]) { message in - realtimeResponse = message.payload!["response"] as! String + realtimeResponse = message.payload?["response"] as? String ?? realtimeResponse expectation.fulfill() }
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
🤖 Fix all issues with AI agents
In `@tests/languages/flutter/tests.dart`:
- Around line 43-48: The queries parameter to realtime.subscribe is being passed
as a nested list causing a type mismatch: change the queries argument from a
List<List<String>> to a List<String> by removing the extra array wrapping around
the Query.equal(...) call (update the rtsubWithQueries realtime.subscribe
invocation so queries contains Query.equal(...) directly rather than wrapped in
an additional list).
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: 3
🤖 Fix all issues with AI agents
In `@templates/apple/Sources/Services/Realtime.swift.twig`:
- Around line 74-83: The current query parameter encoding uses .urlQueryAllowed
which leaves reserved chars unescaped; update the encoding calls for project,
channel and query values to use a stricter character set (e.g.
CharacterSet.alphanumerics or a custom allowed set) when calling
addingPercentEncoding(withAllowedCharacters:), replacing the uses of
.urlQueryAllowed for encodedChannel and encodedQuery and also percent-encode
client.config["project"] when building queryParams; ensure you fallback to the
original value only if encoding returns nil.
In `@tests/languages/apple/Tests.swift`:
- Around line 54-63: The closure passed to realtime.subscribe is force-casting
message.payload?["response"] as! String which can crash; change it to safely
unwrap using an optional cast (as?) and a guard let binding inside the closure
(referencing realtime.subscribe, the closure body, realtimeResponseWithQueries
and expectationWithQueries) — if the cast fails, call XCTFail or otherwise
handle the error and return early; only assign to realtimeResponseWithQueries
and call expectationWithQueries.fulfill() after the guard succeeds.
- Around line 45-63: The subscriptions created with Realtime.subscribe are never
closed, which can lead to over-fulfillment and flaky tests; update the test to
capture the returned RealtimeSubscription handles (e.g., let sub = try await
realtime.subscribe(...)) for both the no-query and with-queries calls, and after
calling expectation.fulfill() / expectationWithQueries.fulfill() call await
sub.close() (handling throws) to stop further messages; ensure you reference the
captured handles when closing so only the intended message triggers each
expectation.
🧹 Nitpick comments (1)
templates/android/library/src/main/java/io/package/services/Realtime.kt.twig (1)
57-72: Consider using%20encoding for spaces instead of+.
URLEncoder.encode()encodes spaces as+, which is correct forapplication/x-www-form-urlencodedbut not for URL query parameters per RFC 3986 (which expects%20). While most servers handle both, for strict compliance consider replacing+with%20:♻️ Proposed fix
activeChannels.forEach { channel -> - val encodedChannel = java.net.URLEncoder.encode(channel, "UTF-8") + val encodedChannel = java.net.URLEncoder.encode(channel, "UTF-8").replace("+", "%20") queryParams += "&channels[]=$encodedChannel" } activeQueries.forEach { query -> - val encodedQuery = java.net.URLEncoder.encode(query, "UTF-8") + val encodedQuery = java.net.URLEncoder.encode(query, "UTF-8").replace("+", "%20") queryParams += "&queries[]=$encodedQuery" }Also, the
projectparameter at line 57 should be encoded for consistency with channels and queries.
| fun <T> subscribe( | ||
| vararg channels: Channel<*>, | ||
| payloadType: Class<T>, | ||
| queries: List<String> = emptyList(), |
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.
Let's use a Set here for free deduplication
| val channels: Collection<String>, | ||
| val queries: Collection<String>, |
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.
Let's change both of these to Set
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
templates/android/library/src/main/java/io/package/services/Realtime.kt.twig (1)
132-192: Snapshotqueriesto avoid external mutation side effects.
queriesis aSetinterface and can be backed by a mutable set. If callers mutate it after subscribing, active query tracking and cleanup can drift. Taking a snapshot prevents hard‑to‑debug state bugs.🛠️ Suggested fix (snapshot the queries set)
val counter = subscriptionsCounter++ activeChannels.addAll(channels) - activeQueries.addAll(queries) + val querySnapshot = queries.toSet() + activeQueries.addAll(querySnapshot) activeSubscriptions[counter] = RealtimeCallback( channels.toSet(), - queries, + querySnapshot, payloadType, callback as (RealtimeResponseEvent<*>) -> Unit ) @@ return RealtimeSubscription { activeSubscriptions.remove(counter) - cleanUp(channels = channels.toList(), queries = queries) + cleanUp(channels = channels.toList(), queries = querySnapshot) createSocket() }
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
🤖 Fix all issues with AI agents
In `@tests/languages/apple/Tests.swift`:
- Around line 67-75: The failure-case subscription uses the same query as the
success case so its callback will incorrectly match; in the realtime.subscribe
call that sets realtimeResponseWithQueriesFailure and fulfills
expectationWithQueriesFailure, change the Query.equal(...) predicate to use a
non-matching key/value (e.g., compare "type" or a different expected value than
"WS:/v1/realtime:passed") so the failure subscription does not match the
successful message and realtimeResponseWithQueriesFailure remains "Realtime
failed!".
♻️ Duplicate comments (1)
tests/languages/apple/Tests.swift (1)
62-65: Guard payload extraction to avoid potential crash.The optional chaining followed by force cast
as!can crash ifpayloadis nil, doesn't contain"response", or the value isn't aString. Use optional binding with a guard statement instead.🛠️ Suggested fix
) { message in - realtimeResponseWithQueries = message.payload?["response"] as! String + guard let response = message.payload?["response"] as? String else { + XCTFail("Missing or invalid 'response' in realtime payload") + return + } + realtimeResponseWithQueries = response expectationWithQueries.fulfill() }
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: 2
🤖 Fix all issues with AI agents
In `@tests/languages/apple/Tests.swift`:
- Around line 47-76: The test erroneously fulfills expectationWithQueriesFailure
immediately, so change the expectation to be inverted by setting
expectationWithQueriesFailure.isInverted = true and remove the immediate
expectationWithQueriesFailure.fulfill() call; instead call
expectationWithQueriesFailure.fulfill() only inside the non-matching
subscription callback (the closure passed to realtime.subscribe for
Query.equal("response", value: ["failed"])) so the inverted expectation will
fail if that callback is invoked and pass when no message arrives.
In `@tests/languages/flutter/tests.dart`:
- Around line 185-193: The test prints the same "Realtime failed!" in both the
success (timeout) and failure (message received) branches, making results
ambiguous; change the TimeoutException branch to print a success message such as
"Realtime passed: no matching message received" while keeping the
message-received branch as "Realtime failed!" so
rtsubWithQueriesFailure.stream.first.timeout and the TimeoutException handling
clearly reflect expected vs. unexpected outcomes.
♻️ Duplicate comments (2)
tests/languages/apple/Tests.swift (2)
51-74: Close realtime subscriptions after first message to avoid flakiness.Without closing, additional messages can over-fulfill expectations and make tests nondeterministic. Capture the returned
RealtimeSubscriptionhandles and close them after fulfillment.♻️ Proposed fix
- try await realtime.subscribe(channels: ["tests"]) { message in + var subscriptionNoQueries: RealtimeSubscription? + subscriptionNoQueries = try await realtime.subscribe(channels: ["tests"]) { message in realtimeResponse = message.payload!["response"] as! String expectation.fulfill() + Task { try? await subscriptionNoQueries?.close() } } - try await realtime.subscribe( + var subscriptionWithQueries: RealtimeSubscription? + subscriptionWithQueries = try await realtime.subscribe( channels: ["tests"], queries: [ Query.equal("response", value: ["WS:/v1/realtime:passed"]) ] ) { message in realtimeResponseWithQueries = message.payload?["response"] as! String expectationWithQueries.fulfill() + Task { try? await subscriptionWithQueries?.close() } }
51-74: Avoid force-unwrap/force-cast on realtime payloads.Line 52, Line 63, and Line 73 can crash if the payload is missing or not a
String. Prefer guarded optional casts withXCTFail.🛠️ Proposed fix
- try await realtime.subscribe(channels: ["tests"]) { message in - realtimeResponse = message.payload!["response"] as! String + try await realtime.subscribe(channels: ["tests"]) { message in + guard let response = message.payload?["response"] as? String else { + XCTFail("Missing or invalid `response` in realtime payload") + return + } + realtimeResponse = response expectation.fulfill() }(Apply the same guard pattern to the with-queries and failure subscriptions.)
| try { | ||
| final message3 = await rtsubWithQueriesFailure.stream.first.timeout(Duration(seconds: 10)); | ||
| // If we receive a message, it means the query filtering failed, so realtime failed | ||
| print("Realtime failed!"); | ||
| } on TimeoutException { | ||
| // Timeout means no matching message was received, which is expected for a failure query | ||
| print("Realtime failed!"); | ||
| } | ||
| await rtsubWithQueriesFailure.close(); |
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.
Misleading test output: both branches print the same "Realtime failed!" message.
The failure test logic prints "Realtime failed!" in both scenarios:
- When a message is received (line 188) - indicating query filtering failed
- When a timeout occurs (line 191) - indicating query filtering worked correctly
This makes it impossible to distinguish between actual test failures and expected behavior. The timeout case (query successfully filtered the message) should print a success message.
🐛 Proposed fix
try {
final message3 = await rtsubWithQueriesFailure.stream.first.timeout(Duration(seconds: 10));
// If we receive a message, it means the query filtering failed, so realtime failed
- print("Realtime failed!");
+ print("Realtime query filtering failed - unexpected message received!");
} on TimeoutException {
// Timeout means no matching message was received, which is expected for a failure query
- print("Realtime failed!");
+ print("Realtime query filtering passed!");
}📝 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.
| try { | |
| final message3 = await rtsubWithQueriesFailure.stream.first.timeout(Duration(seconds: 10)); | |
| // If we receive a message, it means the query filtering failed, so realtime failed | |
| print("Realtime failed!"); | |
| } on TimeoutException { | |
| // Timeout means no matching message was received, which is expected for a failure query | |
| print("Realtime failed!"); | |
| } | |
| await rtsubWithQueriesFailure.close(); | |
| try { | |
| final message3 = await rtsubWithQueriesFailure.stream.first.timeout(Duration(seconds: 10)); | |
| // If we receive a message, it means the query filtering failed, so realtime failed | |
| print("Realtime query filtering failed - unexpected message received!"); | |
| } on TimeoutException { | |
| // Timeout means no matching message was received, which is expected for a failure query | |
| print("Realtime query filtering passed!"); | |
| } | |
| await rtsubWithQueriesFailure.close(); |
🤖 Prompt for AI Agents
In `@tests/languages/flutter/tests.dart` around lines 185 - 193, The test prints
the same "Realtime failed!" in both the success (timeout) and failure (message
received) branches, making results ambiguous; change the TimeoutException branch
to print a success message such as "Realtime passed: no matching message
received" while keeping the message-received branch as "Realtime failed!" so
rtsubWithQueriesFailure.stream.first.timeout and the TimeoutException handling
clearly reflect expected vs. unexpected outcomes.
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
🤖 Fix all issues with AI agents
In `@tests/languages/web/index.html`:
- Around line 50-59: The subscribe calls pass a double-nested queries array
([[Query.equal(...)]]), but the Web SDK expects a flat array of queries
(queries?: (string | Query)[]); update the calls to use [Query.equal('response',
["WS:/v1/realtime:passed"])] instead of [[Query.equal(...)]], and do the same
for the second subscribe so each Query.equal(...) is a direct element of the
queries array in the subscribe(...) invocation (refer to the subscribe function
and Query.equal references and the responseRealtimeWithQueries variable to
locate the calls).
🧹 Nitpick comments (1)
tests/languages/android/Tests.kt (1)
107-115: Clarify the intent of this failure path test.The callback hardcodes
"WS:/v1/realtime:passed"instead of reading from the payload. Given the query filters forresponse: "failed"and the server sends"WS:/v1/realtime:passed", the callback should never fire, leavingrealtimeResponseWithQueriesFailureas"Realtime failed!".If the callback does fire (indicating query filtering failed), it would incorrectly log
"WS:/v1/realtime:passed"- making the test output indistinguishable from success. Consider either:
- Keeping the payload assignment like the other subscription for consistency
- Adding a comment explaining this is intentionally testing that the callback does NOT fire
Suggestion for clearer test intent
realtimeWithFailure.subscribe( "tests", payloadType = TestPayload::class.java, queries = setOf( Query.equal("response", listOf("failed")) ) ) { - realtimeResponseWithQueriesFailure = "WS:/v1/realtime:passed" + // This callback should NOT fire since query filters for "failed" + // but server sends "WS:/v1/realtime:passed" + realtimeResponseWithQueriesFailure = it.payload.response }
| // Subscribe with queries to ensure query array support works | ||
| await realtime.subscribe( | ||
| ['tests'], | ||
| message => { | ||
| responseRealtimeWithQueries = message.payload.response; | ||
| }, | ||
| [ | ||
| [Query.equal('response',["WS:/v1/realtime:passed"])] | ||
| ], | ||
| ); |
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
🏁 Script executed:
# Find subscribe method definitions and their signatures
fd -e ts -e js | xargs grep -l "subscribe" | head -20Repository: appwrite/sdk-generator
Length of output: 137
🏁 Script executed:
# Search for the subscribe method specifically in the realtime context
rg -n "subscribe\(" --type ts -A 15 -B 3 | head -150Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Look for query handling in the codebase
rg -n "queries" --type ts -C 3 | head -100Repository: appwrite/sdk-generator
Length of output: 6151
🏁 Script executed:
# Find the test file
fd "index.html" -type f | grep -i "web"Repository: appwrite/sdk-generator
Length of output: 236
🏁 Script executed:
# Read the specific lines from the test file
find . -path "*/tests/languages/web/index.html" -exec cat -n {} \; | sed -n '45,75p'Repository: appwrite/sdk-generator
Length of output: 1379
🏁 Script executed:
# Find the web SDK realtime subscribe implementation
find . -path "*/templates/*web*" -name "*.ts" -o -name "*.js" | grep -v node_modules | head -20Repository: appwrite/sdk-generator
Length of output: 187
🏁 Script executed:
# Search for subscribe method definition in the templates
rg -n "subscribe.*queries" --type ts -B 5 -A 10Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Look at how realtime is generated in web templates
fd -type f "*.ts" | xargs grep -l "realtime" | grep templates | head -10Repository: appwrite/sdk-generator
Length of output: 236
🏁 Script executed:
# Find web SDK templates
find ./templates -type d -name "*web*"Repository: appwrite/sdk-generator
Length of output: 81
🏁 Script executed:
# Look for realtime implementation in templates
find ./templates -type f -name "*.liquid" | head -20Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Search for subscribe pattern in all files
rg -n "def subscribe" --type-list | head -5Repository: appwrite/sdk-generator
Length of output: 455
🏁 Script executed:
# Look for the realtime class/file in templates
rg -l "subscribe" ./templates --type ts --type js | head -10Repository: appwrite/sdk-generator
Length of output: 141
🏁 Script executed:
# List contents of web templates
ls -la ./templates/web/Repository: appwrite/sdk-generator
Length of output: 806
🏁 Script executed:
# Find realtime-related files in web templates
find ./templates/web -type f -name "*realtime*"Repository: appwrite/sdk-generator
Length of output: 111
🏁 Script executed:
# Search for subscribe in web templates
find ./templates/web -type f \( -name "*.ts" -o -name "*.js" -o -name "*.liquid" \) | xargs grep -l subscribe | head -10Repository: appwrite/sdk-generator
Length of output: 48
🏁 Script executed:
# Read the realtime template file
cat -n ./templates/web/src/services/realtime.ts.twigRepository: appwrite/sdk-generator
Length of output: 20109
Fix the double-nested query array structure.
The queries parameter in the subscribe call uses a double-nested array [[Query.equal(...)]], but the Web SDK's subscribe method signature expects a flat array: queries?: (string | Query)[]. Each query should be a direct element of the array, not wrapped in another array. Change the queries parameter from [[Query.equal(...)]] to [Query.equal(...)].
This applies to both subscribe calls at lines 50-59 and lines 61-69.
🤖 Prompt for AI Agents
In `@tests/languages/web/index.html` around lines 50 - 59, The subscribe calls
pass a double-nested queries array ([[Query.equal(...)]]), but the Web SDK
expects a flat array of queries (queries?: (string | Query)[]); update the calls
to use [Query.equal('response', ["WS:/v1/realtime:passed"])] instead of
[[Query.equal(...)]], and do the same for the second subscribe so each
Query.equal(...) is a direct element of the queries array in the subscribe(...)
invocation (refer to the subscribe function and Query.equal references and the
responseRealtimeWithQueries variable to locate the calls).
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
🤖 Fix all issues with AI agents
In `@tests/languages/apple/Tests.swift`:
- Around line 52-56: The test currently force-unwraps message.payload with
message.payload! inside the realtime.subscribe callback which can crash if
payload is nil; change the callback in the realtime.subscribe invocation to
safely unwrap message.payload and the "response" value (e.g., use guard let
payload = message.payload, let resp = payload["response"] as? String) before
assigning to realtimeResponse and calling expectation.fulfill(), leaving
realtimeResponse and expectation untouched if the payload or response is
missing.
♻️ Duplicate comments (3)
tests/languages/apple/Tests.swift (3)
53-77: Capture subscription handles and close after fulfillment to prevent test flakiness.The
subscribecalls returnRealtimeSubscriptionhandles that are being discarded. If the server sends multiple messages, the expectations could be over-fulfilled, causing flaky tests. Capture the handles and close subscriptions after fulfillment:♻️ Suggested pattern
- try await realtime.subscribe(channels: ["tests"]) { message in + let subscriptionNoQueries = try await realtime.subscribe(channels: ["tests"]) { message in // ... handle message ... expectation.fulfill() + Task { try? await subscriptionNoQueries.close() } } - try await realtime.subscribe( + let subscriptionWithQueries = try await realtime.subscribe( channels: ["tests"], queries: [...] ) { message in // ... handle message ... expectationWithQueries.fulfill() + Task { try? await subscriptionWithQueries.close() } }
58-67: Guard realtime payload extraction to avoid crash.The optional chaining on
payload?["response"]followed by force castas!will crash if the response value is missing or not a String. Use an optional cast with a guard statement instead.🛠️ Suggested fix
try await realtime.subscribe( channels: ["tests"], queries: [ Query.equal("response", value: ["WS:/v1/realtime:passed"]) ] ) { message in - realtimeResponseWithQueries = message.payload?["response"] as! String + guard let response = message.payload?["response"] as? String else { + XCTFail("Missing or invalid `response` in realtime payload") + return + } + realtimeResponseWithQueries = response expectationWithQueries.fulfill() }
69-77: Same force cast issue in failure path callback.Apply the same safe unwrapping pattern here for consistency:
🛠️ Suggested fix
try await realtimeWithFailure.subscribe( channels: ["tests"], queries: [ Query.equal("response", value: ["failed"]) ] ) { message in - realtimeResponseWithQueriesFailure = message.payload?["response"] as! String + guard let response = message.payload?["response"] as? String else { + XCTFail("Missing or invalid `response` in realtime payload") + return + } + realtimeResponseWithQueriesFailure = response expectationWithQueriesFailure.fulfill() }
🧹 Nitpick comments (1)
tests/languages/apple/Tests.swift (1)
196-200: Remove redundant conditional—isInvertedis alwaystrue.The
isInvertedproperty was set totrueon line 50 and never changes, making theelsebranch dead code. The intent appears to be confirming the callback was never invoked by printing the unchanged initial value. Simplify by removing the conditional:♻️ Suggested simplification
wait(for: [expectationWithQueriesFailure], timeout: 10.0) - if expectationWithQueriesFailure.isInverted { - print(realtimeResponseWithQueriesFailure) - } else { - print("Realtime failed") - } + // Prints initial value "Realtime failed!" confirming callback was never invoked + print(realtimeResponseWithQueriesFailure)
| // Subscribe without queries | ||
| try await realtime.subscribe(channels: ["tests"]) { message in | ||
| realtimeResponse = message.payload!["response"] as! String | ||
| expectation.fulfill() | ||
| } |
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.
Guard against nil payload to avoid crash.
Line 54 uses message.payload! which will crash if payload is nil. This is inconsistent with lines 65 and 75 which use optional chaining (payload?). Consider using safe unwrapping:
🛠️ Suggested fix
try await realtime.subscribe(channels: ["tests"]) { message in
- realtimeResponse = message.payload!["response"] as! String
+ guard let response = message.payload?["response"] as? String else {
+ XCTFail("Missing or invalid `response` in realtime payload")
+ return
+ }
+ realtimeResponse = response
expectation.fulfill()
}🤖 Prompt for AI Agents
In `@tests/languages/apple/Tests.swift` around lines 52 - 56, The test currently
force-unwraps message.payload with message.payload! inside the
realtime.subscribe callback which can crash if payload is nil; change the
callback in the realtime.subscribe invocation to safely unwrap message.payload
and the "response" value (e.g., use guard let payload = message.payload, let
resp = payload["response"] as? String) before assigning to realtimeResponse and
calling expectation.fulfill(), leaving realtimeResponse and expectation
untouched if the payload or response is missing.
What does this PR do?
(Provide a description of what this PR does.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit
New Features
Public API
Behavior
Tests
✏️ Tip: You can customize this high-level summary in your review settings.