Skip to content

Conversation

@ArnabChatterjee20k
Copy link
Contributor

@ArnabChatterjee20k ArnabChatterjee20k commented Jan 21, 2026

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

    • Realtime subscriptions now support optional per-subscription queries alongside channels; connections include active queries.
  • Public API

    • Subscribe interfaces across platforms accept queries; subscription objects expose and track associated queries.
  • Behavior

    • Connection URL construction, reconnection and cleanup now consider queries when managing active subscriptions.
  • Tests

    • Cross-platform tests added/updated to validate query-enabled realtime subscription flows and expected message delivery.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 21, 2026

Walkthrough

The 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)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'added realtime queries' accurately summarizes the main change: adding query parameter support to realtime subscriptions across all SDK language templates.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 the Query class 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 queries parameter 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 message2 variable 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();

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 against subscription.queries. Since RealtimeResponseEvent lacks 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 to RealtimeResponseEvent for 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 TimeoutException won'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:async for TimeoutException.

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()
         }

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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).

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 %20 encoding for spaces instead of +.

URLEncoder.encode() encodes spaces as +, which is correct for application/x-www-form-urlencoded but 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 project parameter 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(),
Copy link
Member

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

Comment on lines 13 to 14
val channels: Collection<String>,
val queries: Collection<String>,
Copy link
Member

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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: Snapshot queries to avoid external mutation side effects.

queries is a Set interface 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()
         }

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 if payload is nil, doesn't contain "response", or the value isn't a String. 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()
         }

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 RealtimeSubscription handles 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 with XCTFail.

🛠️ 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.)

Comment on lines +185 to +193
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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Misleading test output: both branches print the same "Realtime failed!" message.

The failure test logic prints "Realtime failed!" in both scenarios:

  1. When a message is received (line 188) - indicating query filtering failed
  2. 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.

Suggested change
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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 for response: "failed" and the server sends "WS:/v1/realtime:passed", the callback should never fire, leaving realtimeResponseWithQueriesFailure as "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:

  1. Keeping the payload assignment like the other subscription for consistency
  2. 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
     }

Comment on lines +50 to +59
// 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"])]
],
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Find subscribe method definitions and their signatures
fd -e ts -e js | xargs grep -l "subscribe" | head -20

Repository: 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 -150

Repository: appwrite/sdk-generator

Length of output: 48


🏁 Script executed:

# Look for query handling in the codebase
rg -n "queries" --type ts -C 3 | head -100

Repository: 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 -20

Repository: 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 10

Repository: 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 -10

Repository: 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 -20

Repository: appwrite/sdk-generator

Length of output: 48


🏁 Script executed:

# Search for subscribe pattern in all files
rg -n "def subscribe" --type-list | head -5

Repository: 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 -10

Repository: 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 -10

Repository: appwrite/sdk-generator

Length of output: 48


🏁 Script executed:

# Read the realtime template file
cat -n ./templates/web/src/services/realtime.ts.twig

Repository: 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).

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 subscribe calls return RealtimeSubscription handles 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 cast as! 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—isInverted is always true.

The isInverted property was set to true on line 50 and never changes, making the else branch 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)

Comment on lines +52 to 56
// Subscribe without queries
try await realtime.subscribe(channels: ["tests"]) { message in
realtimeResponse = message.payload!["response"] as! String
expectation.fulfill()
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants